Skip to content

Commit

Permalink
Back off if encoding is already given
Browse files Browse the repository at this point in the history
  • Loading branch information
lukasniemeier-zalando committed Nov 16, 2019
1 parent af70c79 commit f2dde3c
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 9 deletions.
21 changes: 21 additions & 0 deletions riptide-compression/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,27 @@ In order to specify the compression algorithm you can pass in a custom `Compress
new RequestCompressionPlugin(Compression.of("br", BrotliOutputStream::new));
```

## Usage

```java
http.post("/events")
.contentType(MediaType.APPLICATION_JSON)
.body(asList(events))
.call(pass())
.join();
```

All request bodies will be compressed using the configured compression method using `chunked` transfer-encoding.

If there is already a `Content-Encoding` specified on the request, the plugin does nothing.

### Limitations

* You must only configure a single `RequestCompressionPlugin` as only a single encoding is applied currently.
* Starting with Spring 4.3 the `Netty4ClientHttpRequestFactory` unconditionally adds a `Content-Length` header,
which breaks if used together with `RequestCompressionPlugin`. Use `riptide-httpclient` instead.


## Getting Help

If you have questions, concerns, bug reports, etc., please file an issue in this repository's [Issue Tracker](../../../../issues).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public RequestExecution aroundNetwork(final RequestExecution execution) {
return arguments -> {
final Entity entity = arguments.getEntity();

if (entity.isEmpty()) {
if (entity.isEmpty() || arguments.getHeaders().containsKey(CONTENT_ENCODING)) {
return execution.execute(arguments);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@
@Hack
final class GzipClientDriver extends ClientDriver {

private int port;

static ClientDriver create() {
return new GzipClientDriver(new DefaultClientDriverJettyHandler(new DefaultRequestMatcher()));
}

private int port;

private GzipClientDriver(ClientDriverJettyHandler handler) {
super(handler);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import static java.util.concurrent.Executors.newSingleThreadExecutor;
import static org.hamcrest.Matchers.emptyString;
import static org.hamcrest.Matchers.equalTo;
import static org.springframework.http.HttpHeaders.CONTENT_ENCODING;
import static org.zalando.riptide.PassRoute.pass;

class RequestCompressionPluginTest {
Expand All @@ -46,7 +47,7 @@ void tearDown() throws Exception {

@ParameterizedTest
@ArgumentsSource(RequestFactorySource.class)
void shouldCompressRequestUsingFactory(final ClientHttpRequestFactory factory) {
void shouldCompressRequestBody(final ClientHttpRequestFactory factory) {
driver.addExpectation(onRequestTo("/")
.withMethod(POST)
.withHeader("X-Content-Encoding", "gzip") // written by Jetty's GzipHandler
Expand All @@ -63,16 +64,17 @@ void shouldCompressRequestUsingFactory(final ClientHttpRequestFactory factory) {

@ParameterizedTest
@ArgumentsSource(RequestFactorySource.class)
void shouldNotCompressEmptyBody(final ClientHttpRequestFactory factory) {
void shouldNotCompressEmptyRequestBody(final ClientHttpRequestFactory factory) {
driver.addExpectation(onRequestTo("/")
.withBody(emptyString(), "text/plain")
.withMethod(POST)
.withBody(emptyString(), "application/json")
.withoutHeader("Content-Encoding")
.withoutHeader("X-Content-Encoding"),
giveResponse("", "text/plain"));

final Http http = buildHttp(factory, new RequestCompressionPlugin());
http.get("/")
.contentType(MediaType.TEXT_PLAIN)
http.post("/")
.contentType(MediaType.APPLICATION_JSON)
.call(pass())
.join();
}
Expand All @@ -95,6 +97,25 @@ void shouldCompressWithGivenAlgorithm(final ClientHttpRequestFactory factory) {
.join();
}

@ParameterizedTest
@ArgumentsSource(RequestFactorySource.class)
void shouldBackOffIfAlreadyEncoded(final ClientHttpRequestFactory factory) {
driver.addExpectation(onRequestTo("/")
.withMethod(POST)
.withHeader("Content-Encoding", "custom") // not handled by Jetty
.withoutHeader("X-Content-Encoding")
.withBody(equalTo("{}"), "application/json"),
giveResponse("", "text/plain"));

final Http http = buildHttp(factory, new RequestCompressionPlugin());
http.post("/")
.header(CONTENT_ENCODING, "custom")
.contentType(MediaType.APPLICATION_JSON)
.body(new HashMap<>())
.call(pass())
.join();
}

private Http buildHttp(final ClientHttpRequestFactory factory, final Plugin... plugins) {
return Http.builder()
.executor(executor)
Expand All @@ -109,7 +130,7 @@ static class RequestFactorySource implements ArgumentsProvider {
public Stream<? extends Arguments> provideArguments(ExtensionContext extensionContext) {
return Stream.of(
new SimpleClientHttpRequestFactory(),
new Netty4ClientHttpRequestFactory(),
// new Netty4ClientHttpRequestFactory(), # broken, see #823
new BufferingClientHttpRequestFactory(new SimpleClientHttpRequestFactory()),
new ApacheClientHttpRequestFactory(HttpClients.createDefault(), Mode.BUFFERING),
new ApacheClientHttpRequestFactory(HttpClients.createDefault(), Mode.STREAMING)
Expand Down

0 comments on commit f2dde3c

Please sign in to comment.