Added possibility to delay logging of requests until response can be …

…inspected.

As a side effect a writer now has access to the request/response object in case any decisions need to be taken based on them.

Fixes #165

logbook-api/src/main/java/org/zalando/logbook/HttpLogWriter.java

@@ -8,8 +8,23 @@ default boolean isActive(final RawHttpRequest request) throws IOException {
         return true;
     }
 
-    void writeRequest(final Precorrelation<String> precorrelation) throws IOException;
+    default void write(final HttpRequest request, final Precorrelation<String> precorrelation) throws IOException {
+        writeRequest(precorrelation);
+    }
+
+    // TODO deprecate
+    default void writeRequest(final Precorrelation<String> precorrelation) throws IOException {
+
+    }
 
-    void writeResponse(final Correlation<String, String> correlation) throws IOException;
+    default void write(final HttpRequest request, final HttpResponse response,
+            final Correlation<String, String> correlation) throws IOException {
+        writeResponse(correlation);
+    }
+
+    // TODO deprecate
+    default void writeResponse(final Correlation<String, String> correlation) throws IOException {
+
+    }
 
 }

logbook-api/src/test/java/org/zalando/logbook/HttpLogWriterTest.java

@@ -3,11 +3,13 @@
 import org.junit.Test;
 
 import java.io.IOException;
+import java.time.Duration;
 
 import static org.hamcrest.Matchers.is;
 import static org.junit.Assert.assertThat;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
 
 public final class HttpLogWriterTest {
 
@@ -18,4 +20,49 @@ public void shouldBeActiveByDefault() throws IOException {
         assertThat(unit.isActive(mock(RawHttpRequest.class)), is(true));
     }
 
+    @Test
+    public void shouldDelegateWriteRequest() throws IOException {
+        final HttpLogWriter unit = spy(HttpLogWriter.class);
+
+        unit.write(mock(HttpRequest.class), MockCorrelation.INSTANCE);
+
+        verify(unit).writeRequest(MockCorrelation.INSTANCE);
+    }
+
+    @Test
+    public void shouldDelegateWriteResponse() throws IOException {
+        final HttpLogWriter unit = spy(HttpLogWriter.class);
+
+        unit.write(mock(HttpRequest.class), mock(HttpResponse.class), MockCorrelation.INSTANCE);
+
+        verify(unit).writeResponse(MockCorrelation.INSTANCE);
+    }
+
+    private enum MockCorrelation implements Precorrelation<String>, Correlation<String, String> {
+
+        INSTANCE;
+
+        @Override
+        public String getId() {
+            return "";
+        }
+
+        @Override
+        public Duration getDuration() {
+            return Duration.ZERO;
+        }
+
+
+        @Override
+        public String getRequest() {
+            return "";
+        }
+
+        @Override
+        public String getResponse() {
+            return "";
+        }
+
+    }
+
 }

logbook-core/src/main/java/org/zalando/logbook/ChunkingHttpLogWriter.java

@@ -23,7 +23,7 @@ public ChunkingHttpLogWriter(final int size, final HttpLogWriter writer) {
         if (size <= 0) {
             throw new IllegalArgumentException("size is expected to be greater than zero");
         }
-        this.minChunkSize = (size > MIN_MAX_DELTA ? size - MIN_MAX_DELTA : size);
+        this.minChunkSize = size > MIN_MAX_DELTA ? size - MIN_MAX_DELTA : size;
         this.maxChunkSize = size;
         this.writer = writer;
     }

logbook-core/src/main/java/org/zalando/logbook/DefaultLogbook.java

@@ -46,7 +46,7 @@ public Optional<Correlator> write(final RawHttpRequest rawHttpRequest) throws IO
 
             final Precorrelation<HttpRequest> precorrelation = new SimplePrecorrelation<>(correlationId, request);
             final String format = formatter.format(precorrelation);
-            writer.writeRequest(new SimplePrecorrelation<>(correlationId, format));
+            writer.write(request, new SimplePrecorrelation<>(correlationId, format));
 
             return Optional.of(rawHttpResponse -> {
                 final Instant end = Instant.now(clock);
@@ -56,7 +56,7 @@ public Optional<Correlator> write(final RawHttpRequest rawHttpRequest) throws IO
                 final Correlation<HttpRequest, HttpResponse> correlation =
                         new SimpleCorrelation<>(correlationId, duration, request, response);
                 final String message = formatter.format(correlation);
-                writer.writeResponse(new SimpleCorrelation<>(correlationId, duration, format, message));
+                writer.write(request, response, new SimpleCorrelation<>(correlationId, duration, format, message));
             });
         } else {
             return Optional.empty();

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

@@ -0,0 +1,27 @@
+package org.zalando.logbook;
+
+import org.zalando.logbook.DefaultLogbook.SimplePrecorrelation;
+
+import java.io.IOException;
+
+// proof of concept
+final class DelayedResponseLogWriter implements HttpLogWriter {
+
+    private final HttpLogWriter delegate;
+
+    DelayedResponseLogWriter(final HttpLogWriter delegate) {
+        this.delegate = delegate;
+    }
+
+    @Override
+    public void write(final HttpRequest request, final HttpResponse response,
+            final Correlation<String, String> correlation) throws IOException {
+
+        if (response.getStatus() >= 400) {
+            // delayed request logging until we have the response at hand
+            delegate.write(request, new SimplePrecorrelation<>(correlation.getId(), correlation.getRequest()));
+            delegate.write(request, response, correlation);
+        }
+    }
+
+}

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

@@ -0,0 +1,31 @@
+package org.zalando.logbook;
+
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import java.io.IOException;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+
+public final class DelayedResponseLogWriterTest {
+
+    private final HttpLogWriter delegate = Mockito.mock(HttpLogWriter.class);
+    private final HttpLogWriter unit = new DelayedResponseLogWriter(delegate);
+
+    @Test
+    public void shouldDelayRequestLogging() throws IOException {
+        final Logbook logbook = Logbook.builder().writer(unit).build();
+
+        final Correlator correlator = logbook.write(MockRawHttpRequest.create()).orElseThrow(AssertionError::new);
+
+        verify(delegate, never()).write(any(), any());
+
+        correlator.write(MockRawHttpResponse.create().withStatus(404));
+
+        verify(delegate).write(any(), any());
+        verify(delegate).write(any(), any(), any());
+    }
+
+}

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

@@ -18,8 +18,8 @@
 import static org.hamcrest.Matchers.startsWith;
 import static org.junit.Assert.assertThat;
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
@@ -28,12 +28,7 @@ public abstract class AbstractHttpTest {
     @Rule
     public final ClientDriverRule driver = new ClientDriverRule();
 
-    protected final HttpLogWriter writer = mock(HttpLogWriter.class);
-
-    @Before
-    public void defaultBehaviour() throws IOException {
-        when(writer.isActive(any())).thenReturn(true);
-    }
+    protected final HttpLogWriter writer = spy(HttpLogWriter.class);
 
     @Before
     public void start() {

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

@@ -24,7 +24,6 @@
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertThat;
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.spy;
@@ -41,7 +40,7 @@
 public final class MultiFilterSecurityTest {
 
     private final HttpLogFormatter formatter = spy(new ForwardingHttpLogFormatter(new JsonHttpLogFormatter()));
-    private final HttpLogWriter writer = mock(HttpLogWriter.class);
+    private final HttpLogWriter writer = spy(HttpLogWriter.class);
     private final SecurityFilter securityFilter = spy(new SecurityFilter());
 
     private final Logbook logbook = Logbook.builder()
@@ -62,8 +61,6 @@ public final class MultiFilterSecurityTest {
     @Before
     public void setUp() throws IOException {
         reset(formatter, writer);
-
-        when(writer.isActive(any())).thenReturn(true);
     }
 
     @Test

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

@@ -21,12 +21,10 @@
 import static org.hamcrest.Matchers.is;
 import static org.junit.Assert.assertThat;
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
 
 /**
@@ -35,7 +33,7 @@
 public final class MultiFilterTest {
 
     private final HttpLogFormatter formatter = spy(new ForwardingHttpLogFormatter(new DefaultHttpLogFormatter()));
-    private final HttpLogWriter writer = mock(HttpLogWriter.class);
+    private final HttpLogWriter writer = spy(HttpLogWriter.class);
 
     private final Logbook logbook = Logbook.builder()
             .formatter(formatter)
@@ -54,8 +52,6 @@ public final class MultiFilterTest {
     @Before
     public void setUp() throws IOException {
         reset(formatter, writer);
-
-        when(writer.isActive(any())).thenReturn(true);
     }
 
     @Test

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

@@ -19,12 +19,9 @@
 import static org.hamcrest.Matchers.endsWith;
 import static org.hamcrest.Matchers.startsWith;
 import static org.junit.Assert.assertThat;
-import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
 
 /**
@@ -33,7 +30,7 @@
 public final class WritingTest {
 
     private final HttpLogFormatter formatter = spy(new ForwardingHttpLogFormatter(new DefaultHttpLogFormatter()));
-    private final HttpLogWriter writer = mock(HttpLogWriter.class);
+    private final HttpLogWriter writer = spy(HttpLogWriter.class);
 
     private final MockMvc mvc = MockMvcBuilders
             .standaloneSetup(new ExampleController())
@@ -46,8 +43,6 @@ public final class WritingTest {
     @Before
     public void setUp() throws IOException {
         reset(formatter, writer);
-
-        when(writer.isActive(any())).thenReturn(true);
     }
 
     @Test

logbook-spring-boot-starter/src/test/java/org/zalando/logbook/spring/FormatStyleCurlTest.java

@@ -14,13 +14,10 @@
 import java.io.IOException;
 import java.util.function.Function;
 
-import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.is;
 import static org.hobsoft.hamcrest.compose.ComposeMatchers.hasFeature;
-import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
 import static org.mockito.hamcrest.MockitoHamcrest.argThat;
 
 @SpringBootTest(
@@ -32,10 +29,8 @@ public final class FormatStyleCurlTest extends AbstractTest {
     public static class TestConfiguration {
 
         @Bean
-        public HttpLogWriter writer() throws IOException {
-            final HttpLogWriter writer = mock(HttpLogWriter.class);
-            when(writer.isActive(any())).thenReturn(true);
-            return writer;
+        public HttpLogWriter writer() {
+            return spy(HttpLogWriter.class);
         }
 
     }

logbook-spring-boot-starter/src/test/java/org/zalando/logbook/spring/FormatStyleDefaultTest.java

@@ -18,10 +18,8 @@
 import static org.hamcrest.Matchers.endsWith;
 import static org.hamcrest.Matchers.startsWith;
 import static org.hobsoft.hamcrest.compose.ComposeMatchers.hasFeature;
-import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
 import static org.mockito.hamcrest.MockitoHamcrest.argThat;
 
 @SpringBootTest(classes = {Application.class, FormatStyleDefaultTest.TestConfiguration.class})
@@ -31,10 +29,8 @@ public final class FormatStyleDefaultTest extends AbstractTest {
     public static class TestConfiguration {
 
         @Bean
-        public HttpLogWriter writer() throws IOException {
-            final HttpLogWriter writer = mock(HttpLogWriter.class);
-            when(writer.isActive(any())).thenReturn(true);
-            return writer;
+        public HttpLogWriter writer() {
+            return spy(HttpLogWriter.class);
         }
 
     }

logbook-spring-boot-starter/src/test/java/org/zalando/logbook/spring/FormatStyleHttpTest.java

@@ -16,10 +16,8 @@
 
 import static org.hamcrest.Matchers.containsString;
 import static org.hobsoft.hamcrest.compose.ComposeMatchers.hasFeature;
-import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
 import static org.mockito.hamcrest.MockitoHamcrest.argThat;
 
 @SpringBootTest(
@@ -31,10 +29,8 @@ public final class FormatStyleHttpTest extends AbstractTest {
     public static class TestConfiguration {
 
         @Bean
-        public HttpLogWriter writer() throws IOException {
-            final HttpLogWriter writer = mock(HttpLogWriter.class);
-            when(writer.isActive(any())).thenReturn(true);
-            return writer;
+        public HttpLogWriter writer() {
+            return spy(HttpLogWriter.class);
         }
 
     }

logbook-spring-boot-starter/src/test/java/org/zalando/logbook/spring/ObfuscateBodyCustomTest.java

@@ -18,10 +18,8 @@
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.not;
 import static org.junit.Assert.assertThat;
-import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
 
 @SpringBootTest(
         classes = {Application.class, ObfuscateBodyCustomTest.TestConfiguration.class},
@@ -32,10 +30,8 @@ public final class ObfuscateBodyCustomTest extends AbstractTest {
     public static class TestConfiguration {
 
         @Bean
-        public HttpLogWriter writer() throws IOException {
-            final HttpLogWriter writer = mock(HttpLogWriter.class);
-            when(writer.isActive(any())).thenReturn(true);
-            return writer;
+        public HttpLogWriter writer() {
+            return spy(HttpLogWriter.class);
         }
 
         @Bean