Remove client address (#280)

* Remove clientAddress and duplicate HttpInterceptorContext classes

components/api/src/main/java/com/hotels/styx/api/FullHttpRequest.java

@@ -19,7 +19,6 @@
 import io.netty.buffer.Unpooled;
 import rx.Observable;
 
-import java.net.InetSocketAddress;
 import java.nio.charset.Charset;
 import java.util.Collection;
 import java.util.Collections;
@@ -36,8 +35,6 @@
 import static com.hotels.styx.api.HttpHeaderNames.COOKIE;
 import static com.hotels.styx.api.HttpHeaderNames.HOST;
 import static com.hotels.styx.api.HttpHeaderValues.KEEP_ALIVE;
-import static com.hotels.styx.api.RequestCookie.decode;
-import static com.hotels.styx.api.RequestCookie.encode;
 import static com.hotels.styx.api.HttpMethod.DELETE;
 import static com.hotels.styx.api.HttpMethod.GET;
 import static com.hotels.styx.api.HttpMethod.HEAD;
@@ -46,8 +43,9 @@
 import static com.hotels.styx.api.HttpMethod.POST;
 import static com.hotels.styx.api.HttpMethod.PUT;
 import static com.hotels.styx.api.HttpVersion.HTTP_1_1;
+import static com.hotels.styx.api.RequestCookie.decode;
+import static com.hotels.styx.api.RequestCookie.encode;
 import static java.lang.Integer.parseInt;
-import static java.net.InetSocketAddress.createUnresolved;
 import static java.util.Arrays.asList;
 import static java.util.Objects.requireNonNull;
 import static java.util.UUID.randomUUID;
@@ -82,8 +80,6 @@
  */
 public class FullHttpRequest implements FullHttpMessage {
     private final Object id;
-    // Relic of old API, kept for conversions
-    private final InetSocketAddress clientAddress;
     private final HttpVersion version;
     private final HttpMethod method;
     private final Url url;
@@ -92,7 +88,6 @@ public class FullHttpRequest implements FullHttpMessage {
 
     FullHttpRequest(Builder builder) {
         this.id = builder.id == null ? randomUUID() : builder.id;
-        this.clientAddress = builder.clientAddress;
         this.version = builder.version;
         this.method = builder.method;
         this.url = builder.url;
@@ -258,11 +253,6 @@ public boolean keepAlive() {
     }
 
 
-    // Relic of old API, kept only for conversions
-    InetSocketAddress clientAddress() {
-        return this.clientAddress;
-    }
-
     /**
      * Get a query parameter by name if present.
      *
@@ -322,8 +312,7 @@ public Builder newBuilder() {
      */
     public HttpRequest toStreamingRequest() {
         HttpRequest.Builder streamingBuilder = new HttpRequest.Builder(this)
-                .disableValidation()
-                .clientAddress(clientAddress);
+                .disableValidation();
 
         if (this.body.length == 0) {
             return streamingBuilder.body(new StyxCoreObservable<>(Observable.empty())).build();
@@ -370,11 +359,8 @@ public String toString() {
      * Builder.
      */
     public static final class Builder {
-        private static final InetSocketAddress LOCAL_HOST = createUnresolved("127.0.0.1", 0);
-
         private Object id;
         private HttpMethod method = HttpMethod.GET;
-        private InetSocketAddress clientAddress = LOCAL_HOST;
         private boolean validate = true;
         private Url url;
         private HttpHeaders.Builder headers;
@@ -411,7 +397,6 @@ public Builder(HttpMethod method, String uri) {
         public Builder(HttpRequest request, byte[] body) {
             this.id = request.id();
             this.method = request.method();
-            this.clientAddress = request.clientAddress();
             this.url = request.url();
             this.version = request.version();
             this.headers = request.headers().newBuilder();
@@ -421,7 +406,6 @@ public Builder(HttpRequest request, byte[] body) {
         Builder(FullHttpRequest request) {
             this.id = request.id();
             this.method = request.method();
-            this.clientAddress = request.clientAddress;
             this.url = request.url();
             this.version = request.version();
             this.headers = request.headers().newBuilder();

components/api/src/main/java/com/hotels/styx/api/HttpInterceptor.java

@@ -15,6 +15,7 @@
  */
 package com.hotels.styx.api;
 
+import java.net.InetSocketAddress;
 import java.util.Optional;
 
 /**
@@ -58,6 +59,13 @@ default <T> Optional<T> getIfAvailable(String key, Class<T> clazz) {
          * @return returns true if this request was received over a secure connection
          */
         boolean isSecure();
+
+        /**
+         * Provides address of the client that sent the request to Styx.
+         *
+         * @return address of the client that sent the request to Styx
+         */
+        Optional<InetSocketAddress> clientAddress();
     }
 
     /**

components/api/src/main/java/com/hotels/styx/api/HttpRequest.java

@@ -20,7 +20,6 @@
 import io.netty.buffer.CompositeByteBuf;
 import rx.Observable;
 
-import java.net.InetSocketAddress;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
@@ -55,7 +54,6 @@
 import static io.netty.util.ReferenceCountUtil.release;
 import static java.lang.Integer.parseInt;
 import static java.lang.String.format;
-import static java.net.InetSocketAddress.createUnresolved;
 import static java.util.Arrays.asList;
 import static java.util.Objects.requireNonNull;
 import static java.util.UUID.randomUUID;
@@ -101,8 +99,6 @@
  */
 public class HttpRequest implements StreamingHttpMessage {
     private final Object id;
-    // Relic of old API, kept for conversions
-    private final InetSocketAddress clientAddress;
     private final HttpVersion version;
     private final HttpMethod method;
     private final Url url;
@@ -111,7 +107,6 @@ public class HttpRequest implements StreamingHttpMessage {
 
     HttpRequest(Builder builder) {
         this.id = builder.id == null ? randomUUID() : builder.id;
-        this.clientAddress = builder.clientAddress;
         this.version = builder.version;
         this.method = builder.method;
         this.url = builder.url;
@@ -288,14 +283,6 @@ public boolean keepAlive() {
         return HttpMessageSupport.keepAlive(headers, version);
     }
 
-    /**
-     * @deprecated will be removed from Styx 1.0 api release
-     */
-    @Deprecated
-    public InetSocketAddress clientAddress() {
-        return this.clientAddress;
-    }
-
     /**
      * Get a query parameter by name if present.
      *
@@ -429,19 +416,15 @@ public String toString() {
                 .add("uri", url)
                 .add("headers", headers)
                 .add("id", id)
-                .add("clientAddress", clientAddress)
                 .toString();
     }
 
     /**
      * An HTTP request builder.
      */
     public static final class Builder {
-        private static final InetSocketAddress LOCAL_HOST = createUnresolved("127.0.0.1", 0);
-
         private Object id;
         private HttpMethod method = HttpMethod.GET;
-        private InetSocketAddress clientAddress = LOCAL_HOST;
         private boolean validate = true;
         private Url url;
         private HttpHeaders.Builder headers;
@@ -478,7 +461,6 @@ public Builder(HttpMethod method, String uri) {
         public Builder(HttpRequest request, StyxObservable<ByteBuf> contentStream) {
             this.id = request.id();
             this.method = httpMethod(request.method().name());
-            this.clientAddress = request.clientAddress();
             this.url = request.url();
             this.version = httpVersion(request.version().toString());
             this.headers = request.headers().newBuilder();
@@ -488,7 +470,6 @@ public Builder(HttpRequest request, StyxObservable<ByteBuf> contentStream) {
         Builder(HttpRequest request) {
             this.id = request.id();
             this.method = request.method();
-            this.clientAddress = request.clientAddress();
             this.url = request.url();
             this.version = request.version();
             this.headers = request.headers().newBuilder();
@@ -498,7 +479,6 @@ public Builder(HttpRequest request, StyxObservable<ByteBuf> contentStream) {
         Builder(FullHttpRequest request) {
             this.id = request.id();
             this.method = request.method();
-            this.clientAddress = request.clientAddress();
             this.url = request.url();
             this.version = request.version();
             this.headers = request.headers().newBuilder();
@@ -620,20 +600,6 @@ public Builder method(HttpMethod method) {
             return this;
         }
 
-        /**
-         * Do not use in any new code.
-         *
-         * @deprecated Will not appear in 1.0 API.
-         *
-         * @param clientAddress
-         * @return {@code this}
-         */
-        @Deprecated
-        public Builder clientAddress(InetSocketAddress clientAddress) {
-            this.clientAddress = clientAddress;
-            return this;
-        }
-
         /**
          * Enable validation of uri and some headers.
          *

components/api/src/test/java/com/hotels/styx/api/HttpRequestTest.java

@@ -21,24 +21,23 @@
 import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
 
-import java.net.InetSocketAddress;
 import java.util.concurrent.ExecutionException;
 import java.util.stream.Stream;
 
 import static com.hotels.styx.api.HttpHeader.header;
 import static com.hotels.styx.api.HttpHeaderNames.CONTENT_LENGTH;
 import static com.hotels.styx.api.HttpHeaderNames.HOST;
+import static com.hotels.styx.api.HttpMethod.DELETE;
+import static com.hotels.styx.api.HttpMethod.GET;
+import static com.hotels.styx.api.HttpMethod.POST;
 import static com.hotels.styx.api.HttpRequest.get;
 import static com.hotels.styx.api.HttpRequest.patch;
 import static com.hotels.styx.api.HttpRequest.post;
 import static com.hotels.styx.api.HttpRequest.put;
-import static com.hotels.styx.api.Url.Builder.url;
-import static com.hotels.styx.api.RequestCookie.requestCookie;
-import static com.hotels.styx.api.HttpMethod.DELETE;
-import static com.hotels.styx.api.HttpMethod.GET;
-import static com.hotels.styx.api.HttpMethod.POST;
 import static com.hotels.styx.api.HttpVersion.HTTP_1_0;
 import static com.hotels.styx.api.HttpVersion.HTTP_1_1;
+import static com.hotels.styx.api.RequestCookie.requestCookie;
+import static com.hotels.styx.api.Url.Builder.url;
 import static com.hotels.styx.support.matchers.IsOptional.isAbsent;
 import static com.hotels.styx.support.matchers.IsOptional.isValue;
 import static com.hotels.styx.support.matchers.MapMatcher.isMap;
@@ -119,7 +118,6 @@ public void createsARequestWithDefaultValues() throws Exception {
         assertThat(request.version(), is(HTTP_1_1));
         assertThat(request.url().toString(), is("/index"));
         assertThat(request.path(), is("/index"));
-        assertThat(request.clientAddress().getHostName(), is("127.0.0.1"));
         assertThat(request.id(), is(notNullValue()));
         assertThat(request.cookies(), is(emptyIterable()));
         assertThat(request.headers(), is(emptyIterable()));
@@ -143,8 +141,7 @@ public void canUseBuilderToSetRequestProperties() {
                 .cookies(requestCookie("cfoo", "bar"))
                 .build();
 
-        assertThat(request.toString(), is("HttpRequest{version=HTTP/1.0, method=PATCH, uri=https://hotels.com, headers=[headerName=a, Cookie=cfoo=bar, Host=hotels.com]," +
-                " id=id, clientAddress=127.0.0.1:0}"));
+        assertThat(request.toString(), is("HttpRequest{version=HTTP/1.0, method=PATCH, uri=https://hotels.com, headers=[headerName=a, Cookie=cfoo=bar, Host=hotels.com], id=id}"));
 
         assertThat(request.headers("headerName"), is(singletonList("a")));
     }
@@ -375,22 +372,6 @@ public void createsANewRequestWithSameVersionAsBefore() {
         assertThat(newRequest.version(), is(HTTP_1_0));
     }
 
-    // Make tests to ensure conversion from HttpRequest and back again preserves clientAddress - do it for Fullhttprequest too
-    @Test
-    public void conversionPreservesClientAddress() throws Exception {
-        InetSocketAddress address = InetSocketAddress.createUnresolved("styx.io", 8080);
-        HttpRequest original = HttpRequest.post("/foo").clientAddress(address).build();
-
-        HttpRequest streaming = new HttpRequest.Builder(original).build();
-
-        HttpRequest shouldMatchOriginal = streaming.toFullRequest(0x100)
-                .asCompletableFuture()
-                .get()
-                .toStreamingRequest();
-
-        assertThat(shouldMatchOriginal.clientAddress(), is(address));
-    }
-
     @Test
     public void addsCookies() {
         HttpRequest request = HttpRequest.get("/")

components/api/src/test/java/com/hotels/styx/api/extension/service/spi/AbstractStyxServiceTest.java

@@ -17,13 +17,12 @@
 
 import com.hotels.styx.api.FullHttpResponse;
 import com.hotels.styx.api.HttpRequest;
-import com.hotels.styx.api.extension.service.spi.AbstractStyxService;
 import org.testng.annotations.Test;
 
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ExecutionException;
 
-import static com.hotels.styx.api.MockContext.MOCK_CONTEXT;
+import static com.hotels.styx.api.extension.service.spi.MockContext.MOCK_CONTEXT;
 import static com.hotels.styx.api.extension.service.spi.StyxServiceStatus.FAILED;
 import static com.hotels.styx.api.extension.service.spi.StyxServiceStatus.RUNNING;
 import static com.hotels.styx.api.extension.service.spi.StyxServiceStatus.STARTING;

components/api/src/test/java/com/hotels/styx/api/MockContext.java → components/api/src/test/java/com/hotels/styx/api/extension/service/spi/MockContext.java

@@ -13,11 +13,15 @@
   See the License for the specific language governing permissions and
   limitations under the License.
  */
-package com.hotels.styx.api;
+package com.hotels.styx.api.extension.service.spi;
 
-public class MockContext implements HttpInterceptor.Context {
+import com.hotels.styx.api.HttpInterceptor;
 
-    public static final HttpInterceptor.Context MOCK_CONTEXT = new MockContext();
+import java.net.InetSocketAddress;
+import java.util.Optional;
+
+class MockContext implements HttpInterceptor.Context {
+    static final HttpInterceptor.Context MOCK_CONTEXT = new MockContext();
 
     @Override
     public void add(String key, Object value) {
@@ -33,4 +37,9 @@ public <T> T get(String key, Class<T> clazz) {
     public boolean isSecure() {
         return false;
     }
+
+    @Override
+    public Optional<InetSocketAddress> clientAddress() {
+        return Optional.empty();
+    }
 }