Fixed tests

oneandonlyflexo · Apr 18, 2016 · 117f4c8 · 117f4c8
1 parent 7716f54
commit 117f4c8
Show file tree

Hide file tree

Showing 25 changed files with 131 additions and 115 deletions.
diff --git a/logbook-api/src/main/java/org/zalando/logbook/BaseHttpMessage.java b/logbook-api/src/main/java/org/zalando/logbook/BaseHttpMessage.java
@@ -39,8 +39,17 @@ public interface BaseHttpMessage {
 
     Origin getOrigin();
 
-    static ListMultimap<String, String> createHeaders() {
-        return newListMultimap(new TreeMap<>(CASE_INSENSITIVE_ORDER), ArrayList::new);
+    class Headers {
+
+        public static ListMultimap<String, String> create() {
+            return newListMultimap(new TreeMap<>(CASE_INSENSITIVE_ORDER), ArrayList::new);
+        }
+
+        public static ListMultimap<String, String> copy(final ListMultimap<String, String> headers) {
+            final ListMultimap<String, String> copy = create();
+            copy.putAll(headers);
+            return copy;
+        }
     }
 
 }
diff --git a/logbook-core/src/test/java/org/zalando/logbook/ForwardingHttpRequestTest.java b/logbook-core/src/test/java/org/zalando/logbook/ForwardingHttpRequestTest.java
@@ -46,6 +46,7 @@ public void shouldDelegate() throws IOException {
         assertThat(unit.getRemote(), is("127.0.0.1"));
         assertThat(unit.getMethod(), is("GET"));
         assertThat(unit.getRequestUri(), is("http://localhost/"));
+        assertThat(unit.getQueryParameters().values(), is(empty()));
         assertThat(unit.getHeaders().values(), is(empty()));
         assertThat(unit.getContentType(), is(""));
         assertThat(unit.getCharset(), is(UTF_8));

diff --git a/logbook-core/src/test/java/org/zalando/logbook/ForwardingRawHttpRequestTest.java b/logbook-core/src/test/java/org/zalando/logbook/ForwardingRawHttpRequestTest.java
@@ -45,22 +45,25 @@ protected RawHttpRequest delegate() {
 
     @Test
     public void shouldDelegate() throws IOException {
-        assertThat(unit.getHeaders(), is(ImmutableMultimap.of()));
-        assertThat(unit.getContentType(), is(""));
-        assertThat(unit.getCharset(), is(UTF_8));
         assertThat(unit.getOrigin(), is(REMOTE));
         assertThat(unit.getRemote(), is("127.0.0.1"));
         assertThat(unit.getMethod(), is("GET"));
         assertThat(unit.getRequestUri(), is("http://localhost/"));
+        assertThat(unit.getQueryParameters().values(), is(empty()));
+        assertThat(unit.getHeaders(), is(ImmutableMultimap.of()));
+        assertThat(unit.getContentType(), is(""));
+        assertThat(unit.getCharset(), is(UTF_8));
     }
 
     @Test
     public void shouldDelegateWithBody() throws IOException {
         final HttpRequest request = unit.withBody();
 
+        assertThat(request.getOrigin(), is(REMOTE));
         assertThat(request.getRemote(), is("127.0.0.1"));
         assertThat(request.getMethod(), is("GET"));
         assertThat(request.getRequestUri(), is("http://localhost/"));
+        assertThat(request.getQueryParameters().values(), is(empty()));
         assertThat(request.getHeaders().values(), is(empty()));
         assertThat(request.getContentType(), is(""));
         assertThat(request.getCharset(), is(UTF_8));

diff --git a/...g/zalando/logbook/httpclient/Request.java → ...ando/logbook/httpclient/LocalRequest.java b/...g/zalando/logbook/httpclient/Request.java → ...ando/logbook/httpclient/LocalRequest.java
@@ -20,11 +20,13 @@
  * #L%
  */
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.ImmutableListMultimap;
 import com.google.common.collect.ListMultimap;
 import com.google.common.collect.Multimaps;
 import com.google.common.io.ByteStreams;
+import lombok.SneakyThrows;
 import org.apache.http.Header;
 import org.apache.http.HttpEntityEnclosingRequest;
 import org.apache.http.HttpRequest;
@@ -33,31 +35,29 @@
 import org.apache.http.client.methods.HttpUriRequest;
 import org.apache.http.entity.ByteArrayEntity;
 import org.apache.http.entity.ContentType;
-import org.zalando.logbook.BaseHttpMessage;
 import org.zalando.logbook.Origin;
 import org.zalando.logbook.RawHttpRequest;
 
 import javax.annotation.Nullable;
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.URI;
-import java.net.URISyntaxException;
 import java.net.UnknownHostException;
 import java.nio.charset.Charset;
 import java.util.Optional;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.apache.http.client.utils.URLEncodedUtils.parse;
 
-final class Request implements RawHttpRequest, org.zalando.logbook.HttpRequest {
+final class LocalRequest implements RawHttpRequest, org.zalando.logbook.HttpRequest {
 
     private final HttpRequest request;
     private final Localhost localhost;
     private final URI originalRequestUri;
 
     private byte[] body;
 
-    Request(final HttpRequest request, final Localhost localhost) {
+    LocalRequest(final HttpRequest request, final Localhost localhost) {
         this.request = request;
         this.localhost = localhost;
         this.originalRequestUri = getOriginalRequestUri(request);
@@ -94,19 +94,20 @@ public String getMethod() {
 
     @Override
     public String getRequestUri() {
-        try {
-            return new URI(
-                    originalRequestUri.getScheme(), 
-                    originalRequestUri.getUserInfo(), 
-                    originalRequestUri.getHost(), 
-                    originalRequestUri.getPort(),
-                    originalRequestUri.getPath(), 
-                    null, 
-                    originalRequestUri.getFragment()
-            ).toASCIIString();
-        } catch (URISyntaxException e) {
-            throw new IllegalStateException(e);
-        }
+        return stripQueryString(
+                originalRequestUri.getScheme(), 
+                originalRequestUri.getUserInfo(), 
+                originalRequestUri.getHost(), 
+                originalRequestUri.getPort(), 
+                originalRequestUri.getPath(), 
+                originalRequestUri.getFragment());
+    }
+
+    @SneakyThrows
+    @VisibleForTesting
+    static String stripQueryString(final String scheme, final String userInfo, final String host, final int port, 
+            final String path, final String fragment) {
+        return new URI(scheme, userInfo, host, port, path, null, fragment).toASCIIString();
     }
 
     @Override
@@ -128,7 +129,7 @@ public ListMultimap<String, String> getQueryParameters() {
 
     @Override
     public ListMultimap<String, String> getHeaders() {
-        final ListMultimap<String, String> headers = BaseHttpMessage.createHeaders();
+        final ListMultimap<String, String> headers = Headers.create();
 
         for (Header header : request.getAllHeaders()) {
             headers.put(header.getName(), header.getValue());

diff --git a/...ttpclient/src/main/java/org/zalando/logbook/httpclient/LogbookHttpRequestInterceptor.java b/...ttpclient/src/main/java/org/zalando/logbook/httpclient/LogbookHttpRequestInterceptor.java
@@ -49,7 +49,7 @@ public LogbookHttpRequestInterceptor(Logbook logbook) {
 
     @Override
     public void process(final HttpRequest httpRequest, final HttpContext context) throws HttpException, IOException {
-        final Request request = new Request(httpRequest, localhost);
+        final LocalRequest request = new LocalRequest(httpRequest, localhost);
         final Optional<Correlator> correlator = logbook.write(request);
         correlator.ifPresent(writeCorrelator(context));
     }

diff --git a/...tpclient/src/main/java/org/zalando/logbook/httpclient/LogbookHttpResponseInterceptor.java b/...tpclient/src/main/java/org/zalando/logbook/httpclient/LogbookHttpResponseInterceptor.java
@@ -36,7 +36,7 @@ public void process(final HttpResponse original, final HttpContext context) thro
         final Optional<Correlator> correlator = getCorrelator(context);
 
         if (correlator.isPresent()) {
-            correlator.get().write(new Response(original));
+            correlator.get().write(new RemoteResponse(original));
         }
     }
 

diff --git a/.../zalando/logbook/httpclient/Response.java → ...do/logbook/httpclient/RemoteResponse.java b/.../zalando/logbook/httpclient/Response.java → ...do/logbook/httpclient/RemoteResponse.java
@@ -28,7 +28,6 @@
 import org.apache.http.HttpResponse;
 import org.apache.http.entity.ByteArrayEntity;
 import org.apache.http.entity.ContentType;
-import org.zalando.logbook.BaseHttpMessage;
 import org.zalando.logbook.Origin;
 import org.zalando.logbook.RawHttpResponse;
 
@@ -38,12 +37,12 @@
 import java.nio.charset.StandardCharsets;
 import java.util.Optional;
 
-final class Response implements RawHttpResponse, org.zalando.logbook.HttpResponse {
+final class RemoteResponse implements RawHttpResponse, org.zalando.logbook.HttpResponse {
 
     private final HttpResponse response;
     private byte[] body;
 
-    Response(final HttpResponse response) {
+    RemoteResponse(final HttpResponse response) {
         this.response = response;
     }
 
@@ -59,7 +58,7 @@ public int getStatus() {
 
     @Override
     public ListMultimap<String, String> getHeaders() {
-        final ListMultimap<String, String> headers = BaseHttpMessage.createHeaders();
+        final ListMultimap<String, String> headers = Headers.create();
 
         for (Header header : response.getAllHeaders()) {
             headers.put(header.getName(), header.getValue());

diff --git a/...lando/logbook/httpclient/RequestTest.java → .../logbook/httpclient/LocalRequestTest.java b/...lando/logbook/httpclient/RequestTest.java → .../logbook/httpclient/LocalRequestTest.java
@@ -22,7 +22,6 @@
 
 
 import com.google.common.collect.ImmutableListMultimap;
-import com.google.common.collect.ImmutableMultimap;
 import org.apache.http.HttpEntityEnclosingRequest;
 import org.apache.http.HttpHost;
 import org.apache.http.HttpRequest;
@@ -57,7 +56,7 @@
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
-public final class RequestTest {
+public final class LocalRequestTest {
 
     @Rule
     public final ExpectedException exception = ExpectedException.none();
@@ -72,13 +71,13 @@ private HttpEntityEnclosingRequest post(final String uri) {
         return new HttpPost(uri);
     }
 
-    private Request unit(final HttpRequest request) {
-        return new Request(request, localhost);
+    private LocalRequest unit(final HttpRequest request) {
+        return new LocalRequest(request, localhost);
     }
 
     @Test
     public void shouldResolveLocalhost() {
-        final Request unit = new Request(get("/"), Localhost.resolve());
+        final LocalRequest unit = new LocalRequest(get("/"), Localhost.resolve());
 
         assertThat(unit.getRemote(), matchesPattern("(\\d{1,3}\\.){3}\\d{1,3}"));
     }
@@ -95,36 +94,36 @@ public void shouldHandleUnknownHostException() throws UnknownHostException {
 
     @Test
     public void shouldRetrieveAbsoluteRequestUri() {
-        final Request unit = unit(get("http://localhost/"));
+        final LocalRequest unit = unit(get("http://localhost/"));
         assertThat(unit, hasFeature("request uri", BaseHttpRequest::getRequestUri, hasToString("http://localhost/")));
     }
 
     @Test
     public void shouldTrimQueryStringFromRequestUri() {
-        final Request unit = unit(get("http://localhost/?limit=1"));
+        final LocalRequest unit = unit(get("http://localhost/?limit=1"));
 
         assertThat(unit, hasFeature("request uri", BaseHttpRequest::getRequestUri,
                 hasToString("http://localhost/")));
     }
 
     @Test
     public void shouldParseQueryStringIntoQueryParameters() {
-        final Request unit = unit(get("http://localhost/?limit=1"));
+        final LocalRequest unit = unit(get("http://localhost/?limit=1"));
 
         assertThat(unit, hasFeature("query parameters", BaseHttpRequest::getQueryParameters,
                 is(ImmutableListMultimap.of("limit", "1"))));
     }
 
     @Test
     public void shouldRetrieveAbsoluteRequestUriForWrappedRequests() throws URISyntaxException {
-        final Request unit = unit(wrap(get("http://localhost/")));
+        final LocalRequest unit = unit(wrap(get("http://localhost/")));
 
         assertThat(unit, hasFeature("request uri", BaseHttpRequest::getRequestUri, hasToString("http://localhost/")));
     }
 
     @Test
     public void shouldRetrieveRelativeUriForNonHttpUriRequests() throws URISyntaxException {
-        final Request unit = unit(wrap(new BasicHttpRequest("GET", "http://localhost/")));
+        final LocalRequest unit = unit(wrap(new BasicHttpRequest("GET", "http://localhost/")));
 
         assertThat(unit, hasFeature("request uri", BaseHttpRequest::getRequestUri, hasToString("/")));
     }
@@ -135,20 +134,25 @@ private HttpRequestWrapper wrap(HttpRequest delegate) throws URISyntaxException
         wrap.setURI(URIUtils.rewriteURIForRoute(URI.create("http://localhost/"), new HttpRoute(target)));
         return wrap;
     }
+
+    @Test(expected = URISyntaxException.class)
+    public void shouldFailOnMalformedUrl() {
+        LocalRequest.stripQueryString(":", null, "localhost", 80, "/", null);
+    }
 
     @Test
     public void shouldReturnContentTypesCharsetIfGiven() {
         final HttpRequest delegate = get("/");
         delegate.addHeader("Content-Type", "text/plain;charset=ISO-8859-1");
-        final Request unit = unit(delegate);
+        final LocalRequest unit = unit(delegate);
         assertThat(unit.getCharset(), is(StandardCharsets.ISO_8859_1));
     }
 
     @Test
     public void shouldReturnContentTypeHeader() {
         final HttpRequest delegate = get("/");
         delegate.addHeader("Content-Type", "text/plain;");
-        final Request unit = unit(delegate);
+        final LocalRequest unit = unit(delegate);
         assertThat(unit.getHeaders().asMap(), aMapWithSize(1));
     }
 
@@ -157,14 +161,14 @@ public void shouldHandleDuplicateHeaders() {
         final HttpRequest delegate = post("/");
         delegate.addHeader("Content-Type", "text/plain;");
         delegate.addHeader("Content-Type", "text/plain;");
-        final Request unit = unit(delegate);
+        final LocalRequest unit = unit(delegate);
         assertThat(unit.getHeaders().asMap(), aMapWithSize(1));
         assertThat(unit.getHeaders().get("Content-Type"), hasSize(2));
     }
 
     @Test
     public void shouldReturnDefaultCharsetIfNoneGiven() {
-        final Request unit = unit(get("/"));
+        final LocalRequest unit = unit(get("/"));
         assertThat(unit.getCharset(), is(UTF_8));
     }
 
@@ -173,7 +177,7 @@ public void shouldReadBodyIfPresent() throws IOException {
         final HttpEntityEnclosingRequest delegate = post("/");
         delegate.setEntity(new StringEntity("Hello, world!", UTF_8));
 
-        final Request unit = unit(delegate);
+        final LocalRequest unit = unit(delegate);
 
         assertThat(new String(unit.withBody().getBody(), UTF_8), is("Hello, world!"));
         assertThat(new String(toByteArray(delegate.getEntity().getContent()), UTF_8), is("Hello, world!"));

diff --git a/...ando/logbook/httpclient/ResponseTest.java → ...ogbook/httpclient/RemoteResponseTest.java b/...ando/logbook/httpclient/ResponseTest.java → ...ogbook/httpclient/RemoteResponseTest.java
@@ -36,13 +36,13 @@
 import static org.hamcrest.Matchers.nullValue;
 import static org.junit.Assert.assertThat;
 
-public final class ResponseTest {
+public final class RemoteResponseTest {
 
     @Rule
     public final ExpectedException exception = ExpectedException.none();
 
     private final HttpResponse delegate = new BasicHttpResponse(new ProtocolVersion("HTTP", 1, 1), 200, "OK");
-    private final Response unit = new Response(delegate);
+    private final RemoteResponse unit = new RemoteResponse(delegate);
 
     @Test
     public void shouldReturnContentTypesCharsetIfGiven() {

diff --git a/logbook-servlet/src/main/java/org/zalando/logbook/servlet/Attributes.java b/logbook-servlet/src/main/java/org/zalando/logbook/servlet/Attributes.java
@@ -24,7 +24,7 @@
 
 final class Attributes {
 
-    static final String CORRELATION = Logbook.class.getName() + ".CORRELATOR";
+    static final String CORRELATOR = Logbook.class.getName() + ".CORRELATOR";
 
     Attributes() {
         // package private so we can trick code coverage

diff --git a/.../zalando/logbook/servlet/TeeResponse.java → ...alando/logbook/servlet/LocalResponse.java b/.../zalando/logbook/servlet/TeeResponse.java → ...alando/logbook/servlet/LocalResponse.java
@@ -24,7 +24,6 @@
 import com.google.common.collect.ListMultimap;
 import com.google.common.io.ByteArrayDataOutput;
 import com.google.common.io.ByteStreams;
-import org.zalando.logbook.BaseHttpMessage;
 import org.zalando.logbook.HttpResponse;
 import org.zalando.logbook.Origin;
 import org.zalando.logbook.RawHttpResponse;
@@ -44,7 +43,7 @@
 import static com.google.common.collect.Multimaps.unmodifiableListMultimap;
 import static java.nio.charset.StandardCharsets.ISO_8859_1;
 
-final class TeeResponse extends HttpServletResponseWrapper implements RawHttpResponse, HttpResponse {
+final class LocalResponse extends HttpServletResponseWrapper implements RawHttpResponse, HttpResponse {
 
     private final ByteArrayDataOutput output = ByteStreams.newDataOutput();
 
@@ -57,7 +56,7 @@ final class TeeResponse extends HttpServletResponseWrapper implements RawHttpRes
     @Nullable
     private byte[] body;
 
-    TeeResponse(final HttpServletResponse response) throws IOException {
+    LocalResponse(final HttpServletResponse response) throws IOException {
         super(response);
         this.stream = new TeeServletOutputStream();
         this.writer = new TeePrintWriter();
@@ -70,7 +69,7 @@ public Origin getOrigin() {
 
     @Override
     public ListMultimap<String, String> getHeaders() {
-        final ListMultimap<String, String> headers = BaseHttpMessage.createHeaders();
+        final ListMultimap<String, String> headers = Headers.create();
 
         for (final String header : getHeaderNames()) {
             headers.putAll(header, getHeaders(header));
@@ -128,7 +127,7 @@ private final class TeeServletOutputStream extends ServletOutputStream {
         private final OutputStream original;
 
         private TeeServletOutputStream() throws IOException {
-            this.original = TeeResponse.super.getOutputStream();
+            this.original = LocalResponse.super.getOutputStream();
         }
 
         @Override