Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chunking is now limited to the body itself #449

Merged
merged 1 commit into from
Mar 4, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 2 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -401,14 +401,11 @@ Logbook logbook = Logbook.builder()

##### Chunking

The `ChunkingHttpLogWriter` will split long messages into smaller chunks and will write them individually while delegating to another writer:
The `ChunkingSink` will split long messages into smaller chunks and will write them individually while delegating to another sink:

```java
Logbook logbook = Logbook.builder()
.sink(new DefaultSink(
new DefaultHttpFormatter(),
new ChunkingHttpLogWriter(1000, new DefaultHttpLogWriter())
))
.sink(new ChunkingSink(sink, 1000))
.build();

```
Expand Down
6 changes: 5 additions & 1 deletion logbook-api/src/main/java/org/zalando/logbook/Sink.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ default boolean isActive() {
void write(Precorrelation precorrelation, HttpRequest request) throws IOException;
void write(Correlation correlation, HttpRequest request, HttpResponse response) throws IOException;

void writeBoth(Correlation correlation, HttpRequest request, HttpResponse response) throws IOException;
default void writeBoth(final Correlation correlation, final HttpRequest request, final HttpResponse response)
throws IOException {
write(correlation, request);
write(correlation, request, response);
}

}
24 changes: 23 additions & 1 deletion logbook-api/src/test/java/org/zalando/logbook/SinkTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,38 @@
import org.junit.jupiter.api.Test;
import org.mockito.invocation.InvocationOnMock;

import java.io.IOException;

import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doCallRealMethod;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

class SinkTest {

private final Correlation correlation = mock(Correlation.class);
private final HttpRequest request = mock(HttpRequest.class);
private final HttpResponse response = mock(HttpResponse.class);

private final Sink unit = mock(Sink.class);

@Test
void shouldBeActiveByDefault() {
final Sink unit = mock(Sink.class, InvocationOnMock::callRealMethod);
when(unit.isActive()).thenCallRealMethod();

assertTrue(unit.isActive());
}

@Test
void shouldDelegateToWriteRequestAndWriteResponseByDefault() throws IOException {
doCallRealMethod().when(unit).writeBoth(any(), any(), any());

unit.writeBoth(correlation, request, response);

verify(unit).write(correlation, request);
verify(unit).write(correlation, request, response);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,6 @@ public HttpRequest delegate() {
return request;
}

@Override
public HttpRequest withBody() {
request.withoutBody();
return this;
}

@Override
public HttpRequest withoutBody() {
// TODO set replacement to empty string?!
return this;
}

@Override
public byte[] getBody() {
return replacement.getBytes(UTF_8);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,4 @@ public String getBodyAsString() {
return replacement;
}

@Override
public HttpResponse withBody() {
response.withoutBody();
return this;
}

@Override
public HttpResponse withoutBody() {
// TODO set body to empty string?
return this;
}

}

This file was deleted.

54 changes: 54 additions & 0 deletions logbook-core/src/main/java/org/zalando/logbook/ChunkingSink.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package org.zalando.logbook;

import org.apiguardian.api.API;

import java.io.IOException;
import java.util.stream.Stream;

import static java.util.stream.StreamSupport.stream;
import static org.apiguardian.api.API.Status.EXPERIMENTAL;
import static org.zalando.fauxpas.FauxPas.throwingConsumer;

@API(status = EXPERIMENTAL)
public final class ChunkingSink implements Sink {

private static final int MIN_MAX_DELTA = 16;

private final Sink delegate;
private final int minChunkSize;
private final int maxChunkSize;

public ChunkingSink(final Sink delegate, final int size) {
this.delegate = delegate;

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.maxChunkSize = size;
}

@Override
public boolean isActive() {
return delegate.isActive();
}

@Override
public void write(final Precorrelation precorrelation, final HttpRequest original) throws IOException {
chunk(original)
.map(chunk -> new BodyReplacementHttpRequest(original, chunk))
.forEach(throwingConsumer(replaced -> delegate.write(precorrelation, replaced)));
}

@Override
public void write(final Correlation correlation, final HttpRequest request, final HttpResponse original) throws IOException {
chunk(original)
.map(chunk -> new BodyReplacementHttpResponse(original, chunk))
.forEach(throwingConsumer(replaced -> delegate.write(correlation, request, replaced)));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about going on step further and hide the headers, like

final Iterator<String> chunks = chunk(original).iterator();
final String firstChunk = chunks.next();
delegate.write(correlation, request, new BodyReplacementHttpResponse(original, firstChunk));
chunks.forEachRemaining(chunk -> delegate.write(correlation, request, new BodyReplacementHttpResponse(new HeaderHidingHttpResponse(original), chunk)));

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would move it a bit closer to be a properly usable feature. Until now it was basically breaking our format, but even with my change it's still a gimmick and not 100% "production-ready".

As an example a parser wouldn't know that this is a chunk. I believe it would be nice to get attribute-support in (see #381) and use that to tag individual chunks with their number. Parsers like Scalyr could then exclude anything with $chunk > 1 when counting requests/responses (as an example).

}

private Stream<String> chunk(final HttpMessage message) throws IOException {
return stream(new ChunkingSpliterator(message.getBodyAsString(), minChunkSize, maxChunkSize), false);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,4 @@ public void write(final Correlation correlation, final HttpRequest request, fina
writer.write(correlation, formatter.format(correlation, response));
}

@Override
public void writeBoth(final Correlation correlation, final HttpRequest request, final HttpResponse response) throws IOException {
write(correlation, request);
write(correlation, request, response);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,10 @@ public static RequestFilter defaultValue() {
public static RequestFilter replaceBody(final BodyReplacer<HttpRequest> replacer) {
return request -> {
@Nullable final String replacement = replacer.replace(request);
return replacement == null ?
request :
new BodyReplacementHttpRequest(request, replacement);
if (replacement == null) {
return request;
}
return new BodyReplacementHttpRequest(request.withoutBody(), replacement);
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,10 @@ public static ResponseFilter defaultValue() {
public static ResponseFilter replaceBody(final BodyReplacer<HttpResponse> replacer) {
return response -> {
@Nullable final String replacement = replacer.replace(response);
return replacement == null ?
response :
new BodyReplacementHttpResponse(response, replacement);
if (replacement == null) {
return response;
}
return new BodyReplacementHttpResponse(response.withoutBody(), replacement);
};
}

Expand Down

This file was deleted.