Merge pull request #34 from whiskeysierra/feature/absolute-request-url

Requests will now contain an absolute request uri

README.md

@@ -7,7 +7,10 @@
 [![Release](https://img.shields.io/github/release/zalando/logbook.svg)](https://github.com/zalando/logbook/releases)
 [![Maven Central](https://img.shields.io/maven-central/v/org.zalando/logbook.svg)](https://maven-badges.herokuapp.com/maven-central/org.zalando/logbook)
 
-*Logbook* is an extensible library to enable request and response logging for different client- and server-side technologies. It comes with a core module `logbook-core` and specific modules per framework, e.g. [`logbook-servlet`](#servlet) for Servlet 3.0 environments and [`logbook-httpclient`](#http-client) for applications using Apache's `HttpClient`.
+*Logbook* is an extensible library to enable request and response logging for different client- and server-side 
+technologies. It comes with a core module `logbook-core` and specific modules per framework, e.g. 
+[`logbook-servlet`](#servlet) for Servlet 3.0 environments and [`logbook-httpclient`](#http-client) for applications 
+using Apache's `HttpClient`.
 
 ## Dependency
 
@@ -20,7 +23,8 @@
 ```
 ## Usage
 
-All integrations require an instance of `Logbook` which holds all configuration and wires all necessary parts together. You can either create one using all the defaults:
+All integrations require an instance of `Logbook` which holds all configuration and wires all necessary parts together. 
+You can either create one using all the defaults:
 
 ```java
 Logbook logbook = Logbook.create()
@@ -37,9 +41,9 @@ Logbook logbook = Logbook.builder()
 
 Logbook works in three phases:
 
-1. [Obfuscation](#obfuscate),
+1. [Obfuscation](#obfuscation),
 2. [Formatting](#formatting) and
-3. [Writing](#write)
+3. [Writing](#writing)
 
 Each phase is represented by one or more interfaces that can be used for customization and every phase has a sensible
 default:
@@ -97,7 +101,7 @@ in production.
 
 ```http
 Request: 2d66e4bc-9a0d-11e5-a84c-1f39510f0d6b
-GET /test HTTP/1.1
+GET http://example.org/test HTTP/1.1
 Accept: application/json
 Host: localhost
 Content-Type: text/plain
@@ -128,7 +132,7 @@ used for production since it's easily consumed by parsers and log consumers.
   "correlation": "2d66e4bc-9a0d-11e5-a84c-1f39510f0d6b",
   "sender": "127.0.0.1",
   "method": "GET",
-  "path": "/test",
+  "path": "http://example.org/test",
   "headers": {
     "Accept": ["application/json"],
     "Content-Type": ["text/plain"]
@@ -175,7 +179,8 @@ Writing defines where formatted requests and responses are written to. Logback c
 
 ### Logger
 
-By default requests and responses are logged using a *slf4j* logger that uses the `org.zalando.logbook.Logbook` category and the log level `trace`. This can be customized though:
+By default requests and responses are logged using a *slf4j* logger that uses the `org.zalando.logbook.Logbook` 
+category and the log level `trace`. This can be customized though:
 
 ```java
 Logbook logbook = Logbook.builder()
@@ -187,7 +192,9 @@ Logbook logbook = Logbook.builder()
 
 ### Stream
 
-An alternative implementation is logging requests and responses to a `PrintStream`, e.g. `System.out` or `System.err`. This is usually a bad choice for running in production, but might be used for short-term local development and/or investigations.
+An alternative implementation is logging requests and responses to a `PrintStream`, e.g. `System.out` or `System.err`. 
+This is usually a bad choice for running in production, but might be used for short-term local development and/or 
+investigations.
 
 ```java
 Logbook logbook = Logbook.builder()
@@ -357,7 +364,7 @@ You may obtain a copy of the License at
 
     http://www.apache.org/licenses/LICENSE-2.0
 
-Unless required by applicable law or agreed to in write, software
+Unless required by applicable law or agreed to in writing, software
 distributed under the License is distributed on an "AS IS" BASIS,
 WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 See the License for the specific language governing permissions and

logbook-core/src/test/java/org/zalando/logbook/ForwardingHttpRequestTest.java

@@ -44,7 +44,7 @@ protected HttpRequest delegate() {
     public void shouldDelegate() throws IOException {
         assertThat(unit.getRemote(), is("127.0.0.1"));
         assertThat(unit.getMethod(), is("GET"));
-        assertThat(unit.getRequestUri(), hasToString("/"));
+        assertThat(unit.getRequestUri(), hasToString("http://localhost/"));
         assertThat(unit.getHeaders().values(), is(empty()));
         assertThat(unit.getContentType(), is(""));
         assertThat(unit.getCharset(), is(UTF_8));

logbook-core/src/test/java/org/zalando/logbook/ForwardingRawHttpRequestTest.java

@@ -46,7 +46,7 @@ protected RawHttpRequest delegate() {
     public void shouldDelegate() throws IOException {
         assertThat(unit.getRemote(), is("127.0.0.1"));
         assertThat(unit.getMethod(), is("GET"));
-        assertThat(unit.getRequestUri(), hasToString("/"));
+        assertThat(unit.getRequestUri(), hasToString("http://localhost/"));
     }
 
     @Test
@@ -55,7 +55,7 @@ public void shouldDelegateWithBody() throws IOException {
 
         assertThat(request.getRemote(), is("127.0.0.1"));
         assertThat(request.getMethod(), is("GET"));
-        assertThat(request.getRequestUri(), hasToString("/"));
+        assertThat(request.getRequestUri(), hasToString("http://localhost/"));
         assertThat(request.getHeaders().values(), is(empty()));
         assertThat(request.getContentType(), is(""));
         assertThat(request.getCharset(), is(UTF_8));

logbook-core/src/test/java/org/zalando/logbook/MockHttpRequest.java

@@ -55,7 +55,7 @@ public MockHttpRequest(@Nullable final String remote,
             @Nullable final String body) {
         this.remote = firstNonNull(remote, "127.0.0.1");
         this.method = firstNonNull(method, "GET");
-        this.requestUri = URI.create(firstNonNull(requestUri, "/"));
+        this.requestUri = URI.create(firstNonNull(requestUri, "http://localhost/"));
         this.headers = firstNonNullNorEmpty(headers, ImmutableMap.of());
         this.contentType = firstNonNull(contentType, "");
         this.charset = firstNonNull(charset, StandardCharsets.UTF_8);

logbook-core/src/test/java/org/zalando/logbook/MockRawHttpRequest.java

@@ -36,7 +36,7 @@ public class MockRawHttpRequest implements RawHttpRequest {
 
     private String remote = "127.0.0.1";
     private String method = "GET";
-    private URI requestUri = URI.create("/");
+    private URI requestUri = URI.create("http://localhost/");
 
     @Override
     public HttpRequest withBody() throws IOException {

logbook-core/src/test/java/org/zalando/logbook/ObfuscatedHttpRequestTest.java

@@ -61,7 +61,7 @@ public void shouldNotObfuscateEmptyQueryString() {
                 Obfuscator.obfuscate(x -> true, "*"),
                 BodyObfuscator.none());
 
-        assertThat(request.getRequestUri(), hasToString("/"));
+        assertThat(request.getRequestUri(), hasToString("http://localhost/"));
     }
 
     @Test

logbook-httpclient/src/main/java/org/zalando/logbook/httpclient/Request.java

@@ -27,6 +27,8 @@
 import org.apache.http.Header;
 import org.apache.http.HttpEntityEnclosingRequest;
 import org.apache.http.HttpRequest;
+import org.apache.http.client.methods.HttpRequestWrapper;
+import org.apache.http.client.methods.HttpUriRequest;
 import org.apache.http.entity.ByteArrayEntity;
 import org.apache.http.entity.ContentType;
 import org.zalando.logbook.RawHttpRequest;
@@ -70,7 +72,13 @@ public String getMethod() {
 
     @Override
     public URI getRequestUri() {
-        return URI.create(request.getRequestLine().getUri());
+        final HttpRequest original = request instanceof HttpRequestWrapper ?
+                HttpRequestWrapper.class.cast(request).getOriginal() :
+                request;
+
+        return original instanceof HttpUriRequest ?
+                HttpUriRequest.class.cast(original).getURI():
+                URI.create(request.getRequestLine().getUri());
     }
 
     @Override

logbook-httpclient/src/test/java/org/zalando/logbook/httpclient/LogbookHttpInterceptorsTest.java

@@ -40,6 +40,7 @@
 import static com.github.restdriver.clientdriver.RestClientDriver.giveResponse;
 import static com.github.restdriver.clientdriver.RestClientDriver.onRequestTo;
 import static com.google.common.io.ByteStreams.toByteArray;
+import static java.lang.String.format;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.is;
@@ -86,7 +87,7 @@ public void shouldLogRequest() throws IOException {
         final String request = captor.getValue().getRequest();
 
         assertThat(request, startsWith("Request:"));
-        assertThat(request, containsString("GET / HTTP/1.1"));
+        assertThat(request, containsString(format("GET http://localhost:%d HTTP/1.1", driver.getPort())));
     }
 
     @Test

logbook-httpclient/src/test/java/org/zalando/logbook/httpclient/RequestTest.java

@@ -22,21 +22,33 @@
 
 
 import org.apache.http.HttpEntityEnclosingRequest;
+import org.apache.http.HttpHost;
+import org.apache.http.HttpRequest;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.methods.HttpPost;
+import org.apache.http.client.methods.HttpRequestWrapper;
+import org.apache.http.client.utils.URIUtils;
+import org.apache.http.conn.routing.HttpRoute;
 import org.apache.http.entity.StringEntity;
-import org.apache.http.message.BasicHttpEntityEnclosingRequest;
+import org.apache.http.message.BasicHttpRequest;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
+import org.zalando.logbook.BaseHttpRequest;
 
 import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
 import java.net.UnknownHostException;
 import java.nio.charset.StandardCharsets;
 
 import static com.google.common.io.ByteStreams.toByteArray;
 import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.hamcrest.Matchers.hasToString;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.matchesPattern;
+import static org.hobsoft.hamcrest.compose.ComposeMatchers.hasFeature;
 import static org.junit.Assert.assertThat;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -46,13 +58,23 @@ public final class RequestTest {
     @Rule
     public final ExpectedException exception = ExpectedException.none();
 
-    private final HttpEntityEnclosingRequest delegate = new BasicHttpEntityEnclosingRequest("GET", "/");
     private final Localhost localhost = mock(Localhost.class);
-    private final Request unit = new Request(delegate, localhost);
+
+    private HttpRequest get(String uri) {
+        return new HttpGet(uri);
+    }
+
+    private HttpEntityEnclosingRequest post(final String uri) {
+        return new HttpPost(uri);
+    }
+
+    private Request unit(final HttpRequest request) {
+        return new Request(request, localhost);
+    }
 
     @Test
     public void shouldResolveLocalhost() {
-        final Request unit = new Request(delegate, Localhost.resolve());
+        final Request unit = new Request(get("/"), Localhost.resolve());
 
         assertThat(unit.getRemote(), matchesPattern("(\\d{1,3}\\.){3}\\d{1,3}"));
     }
@@ -64,24 +86,57 @@ public void shouldHandleUnknownHostException() throws UnknownHostException {
         exception.expect(IllegalStateException.class);
         exception.expectCause(instanceOf(UnknownHostException.class));
 
-        unit.getRemote();
+        unit(get("/")).getRemote();
+    }
+
+    @Test
+    public void shouldRetrieveAbsoluteRequestUri() {
+        final Request unit = unit(get("http://localhost/"));
+        assertThat(unit, hasFeature("request uri", BaseHttpRequest::getRequestUri, hasToString("http://localhost/")));
+    }
+
+    @Test
+    public void shouldRetrieveAbsoluteRequestUriForWrappedRequests() throws URISyntaxException {
+        final Request 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/")));
+
+        assertThat(unit, hasFeature("request uri", BaseHttpRequest::getRequestUri, hasToString("/")));
+    }
+
+    private HttpRequestWrapper wrap(HttpRequest delegate) throws URISyntaxException {
+        final HttpHost target = HttpHost.create("localhost");
+        final HttpRequestWrapper wrap = HttpRequestWrapper.wrap(delegate, target);
+        wrap.setURI(URIUtils.rewriteURIForRoute(URI.create("http://localhost/"), new HttpRoute(target)));
+        return wrap;
     }
 
     @Test
     public void shouldReturnContentTypesCharsetIfGiven() {
+        final HttpRequest delegate = get("/");
         delegate.addHeader("Content-Type", "text/plain;charset=ISO-8859-1");
+        final Request unit = unit(delegate);
         assertThat(unit.getCharset(), is(StandardCharsets.ISO_8859_1));
     }
 
     @Test
     public void shouldReturnDefaultCharsetIfNoneGiven() {
+        final Request unit = unit(get("/"));
         assertThat(unit.getCharset(), is(UTF_8));
     }
 
     @Test
     public void shouldReadBodyIfPresent() throws IOException {
+        final HttpEntityEnclosingRequest delegate = post("/");
         delegate.setEntity(new StringEntity("Hello, world!", UTF_8));
-
+
+        final Request 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!"));
     }

logbook-servlet/src/main/java/org/zalando/logbook/servlet/TeeRequest.java

@@ -67,7 +67,7 @@ public String getRemote() {
 
     @Override
     public URI getRequestUri() {
-        final String uri = getRequestURI();
+        final String uri = getRequestURL().toString();
         @Nullable final String queryString = getQueryString();
         return URI.create(queryString == null ? uri : uri + "?" + queryString);
     }

logbook-servlet/src/test/java/org/zalando/logbook/servlet/AsyncDispatchTest.java

@@ -92,7 +92,8 @@ public void shouldFormatAsyncRequest() throws Exception {
 
         assertThat(request, hasFeature("remote address", HttpRequest::getRemote, is("127.0.0.1")));
         assertThat(request, hasFeature("method", HttpRequest::getMethod, is("GET")));
-        assertThat(request, hasFeature("url", HttpRequest::getRequestUri, hasToString("/api/async")));
+        assertThat(request, hasFeature("url", HttpRequest::getRequestUri,
+                hasToString("http://localhost/api/async")));
         assertThat(request, hasFeature("headers", HttpRequest::getHeaders, is(ImmutableMultimap.of())));
         assertThat(request, hasFeature("body", this::getBodyAsString, is(emptyOrNullString())));
     }

logbook-servlet/src/test/java/org/zalando/logbook/servlet/FormattingTest.java

@@ -90,7 +90,8 @@ public void shouldFormatRequest() throws Exception {
 
         assertThat(request, hasFeature("remote address", HttpRequest::getRemote, is("127.0.0.1")));
         assertThat(request, hasFeature("method", HttpRequest::getMethod, is("GET")));
-        assertThat(request, hasFeature("url", HttpRequest::getRequestUri, hasToString("/api/sync?limit=1")));
+        assertThat(request, hasFeature("url", HttpRequest::getRequestUri,
+                hasToString("http://localhost/api/sync?limit=1")));
         assertThat(request, hasFeature("headers", HttpRequest::getHeaders, is(of("Accept", "text/plain"))));
         assertThat(request, hasFeature("body", this::getBody, is(notNullValue())));
         assertThat(request, hasFeature("body", this::getBodyAsString, is(emptyString())));

logbook-servlet/src/test/java/org/zalando/logbook/servlet/WritingTest.java

@@ -85,7 +85,7 @@ public void shouldLogRequest() throws Exception {
 
         assertThat(precorrelation.getRequest(), startsWith("Request:"));
         assertThat(precorrelation.getRequest(), endsWith(
-                "GET /api/sync HTTP/1.1\n" +
+                "GET http://localhost/api/sync HTTP/1.1\n" +
                 "Accept: application/json\n" +
                 "Host: localhost\n" +
                 "Content-Type: text/plain\n" +