From 59ffc5978f2eb67c697d6a369165e4814d75351b Mon Sep 17 00:00:00 2001 From: Mikko Karjalainen Date: Thu, 25 Oct 2018 11:12:43 +0100 Subject: [PATCH 1/4] Restrict visibility of `ByteStream.aggregate` to work around Netty reference counting problem. --- .../api/src/main/java/com/hotels/styx/api/ByteStream.java | 2 +- .../client/src/main/java/com/hotels/styx/client/Transport.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/components/api/src/main/java/com/hotels/styx/api/ByteStream.java b/components/api/src/main/java/com/hotels/styx/api/ByteStream.java index 0cfe5e3394..492e3a2267 100644 --- a/components/api/src/main/java/com/hotels/styx/api/ByteStream.java +++ b/components/api/src/main/java/com/hotels/styx/api/ByteStream.java @@ -132,7 +132,7 @@ public ByteStream doOnCancel(Runnable action) { * @param maxContentBytes maximum size for the aggregated buffer * @return a future of aggregated buffer */ - public CompletableFuture aggregate(int maxContentBytes) { + CompletableFuture aggregate(int maxContentBytes) { return new ByteStreamAggregator(this.stream, maxContentBytes) .apply(); } diff --git a/components/client/src/main/java/com/hotels/styx/client/Transport.java b/components/client/src/main/java/com/hotels/styx/client/Transport.java index 9de8d9ba4f..301e762ada 100644 --- a/components/client/src/main/java/com/hotels/styx/client/Transport.java +++ b/components/client/src/main/java/com/hotels/styx/client/Transport.java @@ -84,7 +84,7 @@ private Observable connection(LiveHttpRequest request, Optional { // Aggregates an empty body: - request.body().drop().aggregate(1); + request.consume(); return Observable.error(new NoAvailableHostsException(appId)); }); } From 42bea9fcf5863c191a1c74043a522e6f887ab16e Mon Sep 17 00:00:00 2001 From: Mikko Karjalainen Date: Thu, 25 Oct 2018 15:26:50 +0100 Subject: [PATCH 2/4] * Add request Transformer class for LiveHttpRequest class. * Remove AsyncRequestContentSpec and AsyncResponseContentSpec e2e tests because asynchronous content transformation is not yet supported in 1.0 API. --- .../com/hotels/styx/api/LiveHttpMessage.java | 2 - .../com/hotels/styx/api/LiveHttpRequest.java | 295 +++++++++++++++++- .../hotels/styx/api/LiveHttpRequestTest.java | 148 ++++++++- .../com/hotels/styx/client/TransportTest.java | 7 +- .../HttpRequestOperationTest.java | 2 +- .../HopByHopHeadersRemovingInterceptor.java | 2 +- .../RequestEnrichingInterceptor.java | 2 +- .../plugins/AsyncRequestContentSpec.scala | 104 ------ .../plugins/AsyncResponseContentSpec.scala | 99 ------ 9 files changed, 430 insertions(+), 231 deletions(-) delete mode 100644 system-tests/e2e-suite/src/test/scala/com/hotels/styx/plugins/AsyncRequestContentSpec.scala delete mode 100644 system-tests/e2e-suite/src/test/scala/com/hotels/styx/plugins/AsyncResponseContentSpec.scala diff --git a/components/api/src/main/java/com/hotels/styx/api/LiveHttpMessage.java b/components/api/src/main/java/com/hotels/styx/api/LiveHttpMessage.java index af911a7d14..6cba013d73 100644 --- a/components/api/src/main/java/com/hotels/styx/api/LiveHttpMessage.java +++ b/components/api/src/main/java/com/hotels/styx/api/LiveHttpMessage.java @@ -15,8 +15,6 @@ */ package com.hotels.styx.api; -import reactor.core.publisher.Flux; - import java.util.List; import java.util.Optional; diff --git a/components/api/src/main/java/com/hotels/styx/api/LiveHttpRequest.java b/components/api/src/main/java/com/hotels/styx/api/LiveHttpRequest.java index c01be39fa6..331fb50cf1 100644 --- a/components/api/src/main/java/com/hotels/styx/api/LiveHttpRequest.java +++ b/components/api/src/main/java/com/hotels/styx/api/LiveHttpRequest.java @@ -321,8 +321,8 @@ public Iterable queryParamNames() { * * @return new builder based on this request */ - public Builder newBuilder() { - return new Builder(this); + public Transformer newBuilder() { + return new Transformer(this); } /** @@ -394,10 +394,271 @@ public String toString() { .toString(); } + private interface BuilderTransformer { + BuilderTransformer uri(String uri); + + BuilderTransformer id(Object id); + + /** + * Sets the (only) value for the header with the specified name. + *

+ * All existing values for the same header will be removed. + * + * @param name The name of the header + * @param value The value of the header + * @return {@code this} + */ + BuilderTransformer header(CharSequence name, Object value); + + /** + * Sets the headers. + * + * @param headers headers + * @return {@code this} + */ + BuilderTransformer headers(HttpHeaders headers); + + /** + * Adds a new header with the specified {@code name} and {@code value}. + *

+ * Will not replace any existing values for the header. + * + * @param name The name of the header + * @param value The value of the header + * @return {@code this} + */ + BuilderTransformer addHeader(CharSequence name, Object value); + + /** + * Removes the header with the specified name. + * + * @param name The name of the header to remove + * @return {@code this} + */ + BuilderTransformer removeHeader(CharSequence name); + + /** + * Sets the request fully qualified url. + * + * @param url fully qualified url + * @return {@code this} + */ + BuilderTransformer url(Url url); + + /** + * Sets the HTTP version. + * + * @param version HTTP version + * @return {@code this} + */ + BuilderTransformer version(HttpVersion version); + + /** + * Enable validation of uri and some headers. + * + * @return {@code this} + */ + BuilderTransformer disableValidation(); + + /** + * Enables Keep-Alive. + * + * @return {@code this} + */ + BuilderTransformer enableKeepAlive(); + + /** + * Sets the cookies on this request by overwriting the value of the "Cookie" header. + * + * @param cookies cookies + * @return this builder + */ + BuilderTransformer cookies(RequestCookie... cookies); + + /** + * Sets the cookies on this request by overwriting the value of the "Cookie" header. + * + * @param cookies cookies + * @return this builder + */ + BuilderTransformer cookies(Collection cookies); + + /** + * Adds cookies into the "Cookie" header. If the name matches an already existing cookie, the value will be overwritten. + *

+ * Note that this requires decoding the current header value before re-encoding, so it is most efficient to + * add all new cookies in one call to the method rather than spreading them out. + * + * @param cookies new cookies + * @return this builder + */ + BuilderTransformer addCookies(RequestCookie... cookies); + + /** + * Adds cookies into the "Cookie" header. If the name matches an already existing cookie, the value will be overwritten. + *

+ * Note that this requires decoding the current header value before re-encoding, so it is most efficient to + * add all new cookies in one call to the method rather than spreading them out. + * + * @param cookies new cookies + * @return this builder + */ + BuilderTransformer addCookies(Collection cookies); + + /** + * Removes all cookies matching one of the supplied names by overwriting the value of the "Cookie" header. + * + * @param names cookie names + * @return this builder + */ + BuilderTransformer removeCookies(String... names); + + /** + * Removes all cookies matching one of the supplied names by overwriting the value of the "Cookie" header. + * + * @param names cookie names + * @return this builder + */ + BuilderTransformer removeCookies(Collection names); + + /** + * Builds a new full request based on the settings configured in this builder. + * If {@code validate} is set to true: + *

    + *
  • the host header will be set if absent
    • + *
  • an exception will be thrown if the content length is not an integer, or more than one content length exists
    • + *
  • an exception will be thrown if the request method is not a valid HTTP method
    • + *
+ * + * @return a new full request + */ + LiveHttpRequest build(); + } + + public static final class Transformer implements BuilderTransformer { + private final Builder builder; + + public Transformer(LiveHttpRequest liveHttpRequest) { + this.builder = new Builder(liveHttpRequest); + } + + @Override + public Transformer uri(String uri) { + builder.uri(uri); + return this; + } + + /** + * Transforms request body. + * + * @param transformation a Function from ByteStream to ByteStream. + * @return a LiveHttpRequest builder with a transformed message body. + */ + public Transformer body(Function transformation) { + builder.body(transformation.apply(builder.body)); + return this; + } + + @Override + public Transformer id(Object id) { + builder.id(id); + return this; + } + + @Override + public Transformer header(CharSequence name, Object value) { + builder.header(name, value); + return this; + } + + @Override + public Transformer headers(HttpHeaders headers) { + builder.headers(headers); + return this; + } + + @Override + public Transformer addHeader(CharSequence name, Object value) { + builder.addHeader(name, value); + return this; + } + + @Override + public Transformer removeHeader(CharSequence name) { + builder.removeHeader(name); + return this; + } + + @Override + public Transformer url(Url url) { + builder.url(url); + return this; + } + + @Override + public Transformer version(HttpVersion version) { + builder.version(version); + return this; + } + + @Override + public Transformer disableValidation() { + builder.disableValidation(); + return this; + } + + @Override + public Transformer enableKeepAlive() { + builder.enableKeepAlive(); + return this; + } + + @Override + public Transformer cookies(RequestCookie... cookies) { + builder.cookies(cookies); + return this; + } + + @Override + public Transformer cookies(Collection cookies) { + builder.cookies(cookies); + return this; + } + + @Override + public Transformer addCookies(RequestCookie... cookies) { + builder.addCookies(cookies); + return this; + } + + @Override + public Transformer addCookies(Collection cookies) { + builder.addCookies(cookies); + return this; + } + + @Override + public Transformer removeCookies(String... names) { + builder.removeCookies(names); + return this; + } + + @Override + public Transformer removeCookies(Collection names) { + builder.removeCookies(names); + return this; + } + + @Override + public LiveHttpRequest build() { + return builder.build(); + } + } + /** * An HTTP request builder. */ - public static final class Builder { + public static final class Builder implements BuilderTransformer { private Object id; private HttpMethod method = HttpMethod.GET; private boolean validate = true; @@ -466,6 +727,7 @@ public Builder(LiveHttpRequest request, ByteStream contentStream) { * @param uri URI * @return {@code this} */ + @Override public Builder uri(String uri) { return this.url(Url.Builder.url(uri).build()); } @@ -481,23 +743,13 @@ public Builder body(ByteStream content) { return this; } - /** - * Transforms request body. - * - * @param transformation a Function from ByteStream to ByteStream. - * @return a LiveHttpRequest builder with a transformed message body. - */ - public Builder body(Function transformation) { - this.body = transformation.apply(this.body); - return this; - } - /** * Sets the unique ID for this request. * * @param id request ID * @return {@code this} */ + @Override public Builder id(Object id) { this.id = id; return this; @@ -512,6 +764,7 @@ public Builder id(Object id) { * @param value The value of the header * @return {@code this} */ + @Override public Builder header(CharSequence name, Object value) { this.headers.set(name, value); return this; @@ -523,6 +776,7 @@ public Builder header(CharSequence name, Object value) { * @param headers headers * @return {@code this} */ + @Override public Builder headers(HttpHeaders headers) { this.headers = headers.newBuilder(); return this; @@ -537,6 +791,7 @@ public Builder headers(HttpHeaders headers) { * @param value The value of the header * @return {@code this} */ + @Override public Builder addHeader(CharSequence name, Object value) { this.headers.add(name, value); return this; @@ -548,6 +803,7 @@ public Builder addHeader(CharSequence name, Object value) { * @param name The name of the header to remove * @return {@code this} */ + @Override public Builder removeHeader(CharSequence name) { headers.remove(name); return this; @@ -559,6 +815,7 @@ public Builder removeHeader(CharSequence name) { * @param url fully qualified url * @return {@code this} */ + @Override public Builder url(Url url) { this.url = url; return this; @@ -570,6 +827,7 @@ public Builder url(Url url) { * @param version HTTP version * @return {@code this} */ + @Override public Builder version(HttpVersion version) { this.version = requireNonNull(version); return this; @@ -591,6 +849,7 @@ public Builder method(HttpMethod method) { * * @return {@code this} */ + @Override public Builder disableValidation() { this.validate = false; return this; @@ -601,6 +860,7 @@ public Builder disableValidation() { * * @return {@code this} */ + @Override public Builder enableKeepAlive() { return header(CONNECTION, KEEP_ALIVE); } @@ -611,6 +871,7 @@ public Builder enableKeepAlive() { * @param cookies cookies * @return this builder */ + @Override public Builder cookies(RequestCookie... cookies) { return cookies(asList(cookies)); } @@ -621,6 +882,7 @@ public Builder cookies(RequestCookie... cookies) { * @param cookies cookies * @return this builder */ + @Override public Builder cookies(Collection cookies) { requireNonNull(cookies); @@ -641,6 +903,7 @@ public Builder cookies(Collection cookies) { * @param cookies new cookies * @return this builder */ + @Override public Builder addCookies(RequestCookie... cookies) { return addCookies(asList(cookies)); } @@ -654,6 +917,7 @@ public Builder addCookies(RequestCookie... cookies) { * @param cookies new cookies * @return this builder */ + @Override public Builder addCookies(Collection cookies) { requireNonNull(cookies); @@ -669,6 +933,7 @@ public Builder addCookies(Collection cookies) { * @param names cookie names * @return this builder */ + @Override public Builder removeCookies(String... names) { return removeCookies(asList(names)); } @@ -679,6 +944,7 @@ public Builder removeCookies(String... names) { * @param names cookie names * @return this builder */ + @Override public Builder removeCookies(Collection names) { requireNonNull(names); @@ -710,6 +976,7 @@ private static Set toSet(Collection collection) { * * @return a new full request */ + @Override public LiveHttpRequest build() { if (validate) { ensureContentLengthIsValid(); diff --git a/components/api/src/test/java/com/hotels/styx/api/LiveHttpRequestTest.java b/components/api/src/test/java/com/hotels/styx/api/LiveHttpRequestTest.java index 7426e6daaa..31c0e1ebe9 100644 --- a/components/api/src/test/java/com/hotels/styx/api/LiveHttpRequestTest.java +++ b/components/api/src/test/java/com/hotels/styx/api/LiveHttpRequestTest.java @@ -15,26 +15,27 @@ */ package com.hotels.styx.api; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import reactor.core.publisher.Flux; +import java.util.Optional; import java.util.concurrent.ExecutionException; import java.util.stream.Stream; import static com.hotels.styx.api.HttpHeader.header; import static com.hotels.styx.api.HttpHeaderNames.CONTENT_LENGTH; import static com.hotels.styx.api.HttpHeaderNames.HOST; -import static com.hotels.styx.api.HttpMethod.DELETE; import static com.hotels.styx.api.HttpMethod.GET; import static com.hotels.styx.api.HttpMethod.POST; +import static com.hotels.styx.api.HttpVersion.HTTP_1_0; +import static com.hotels.styx.api.HttpVersion.HTTP_1_1; import static com.hotels.styx.api.LiveHttpRequest.get; import static com.hotels.styx.api.LiveHttpRequest.patch; import static com.hotels.styx.api.LiveHttpRequest.post; import static com.hotels.styx.api.LiveHttpRequest.put; -import static com.hotels.styx.api.HttpVersion.HTTP_1_0; -import static com.hotels.styx.api.HttpVersion.HTTP_1_1; import static com.hotels.styx.api.RequestCookie.requestCookie; import static com.hotels.styx.api.Url.Builder.url; import static com.hotels.styx.support.matchers.IsOptional.isAbsent; @@ -51,6 +52,7 @@ import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; +import static org.testng.Assert.assertEquals; public class LiveHttpRequestTest { @Test @@ -151,12 +153,10 @@ public void canModifyPreviouslyCreatedRequest() { .build(); LiveHttpRequest newRequest = request.newBuilder() - .method(DELETE) .uri("/home") .header("remove", "notanymore") .build(); - assertThat(newRequest.method(), is(DELETE)); assertThat(newRequest.url().path(), is("/home")); assertThat(newRequest.headers(), hasItem(header("remove", "notanymore"))); } @@ -426,6 +426,144 @@ public void removesCookiesInSameBuilder() { assertThat(r1.cookie("x"), isAbsent()); } + @Test + public void transformsUri() { + LiveHttpRequest request = LiveHttpRequest.get("/x").build() + .newBuilder() + .uri("/y") + .build(); + + assertEquals(request.url().path(), "/y"); + } + + @Test + public void transformsId() { + LiveHttpRequest request = LiveHttpRequest.get("/").id("abc").build() + .newBuilder() + .id("xyz") + .build(); + + assertEquals(request.id(), "xyz"); + } + + @Test + public void transformsHeader() { + LiveHttpRequest request = LiveHttpRequest.get("/").header("X-Styx-ID", "test").build() + .newBuilder() + .header("X-Styx-ID", "bar") + .build(); + + assertEquals(request.header("X-Styx-ID"), Optional.of("bar")); + } + + @Test + public void transformsHeaders() { + LiveHttpRequest request = LiveHttpRequest.get("/").headers( + new HttpHeaders.Builder() + .add("x", "y") + .build()) + .build() + .newBuilder() + .headers( + new HttpHeaders.Builder() + .add("a", "b") + .build()) + .build(); + + assertThat(request.header("x"), is(Optional.empty())); + assertThat(request.header("a"), is(Optional.of("b"))); + } + + @Test + public void transformerAddsHeader() { + LiveHttpRequest request = LiveHttpRequest.get("/").build() + .newBuilder() + .addHeader("x", "y") + .build(); + + assertEquals(request.header("x"), Optional.of("y")); + } + + @Test + public void transformerRemovesHeader() { + LiveHttpRequest request = LiveHttpRequest.get("/").addHeader("x", "y").build() + .newBuilder() + .removeHeader("x") + .build(); + + assertEquals(request.header("x"), Optional.empty()); + } + + @Test + public void transformsUrl() { + LiveHttpRequest request = LiveHttpRequest.get("/").build() + .newBuilder() + .url(url("/z").build()) + .build(); + + assertEquals(request.url().path(), "/z"); + } + + @Test + public void transformsCookies() { + LiveHttpRequest request = LiveHttpRequest.get("/").addCookies(requestCookie("cookie", "xyz010")).build() + .newBuilder() + .cookies(requestCookie("cookie", "xyz202")) + .build(); + + assertEquals(request.cookie("cookie"), Optional.of(requestCookie("cookie", "xyz202"))); + } + + @Test + public void transformsCookiesViaList() { + LiveHttpRequest request = LiveHttpRequest.get("/").addCookies(requestCookie("cookie", "xyz010")).build() + .newBuilder() + .cookies(ImmutableList.of(requestCookie("cookie", "xyz202"))) + .build(); + + assertEquals(request.cookie("cookie"), Optional.of(requestCookie("cookie", "xyz202"))); + } + + @Test + public void transformsByAddingCookies() { + LiveHttpRequest request = LiveHttpRequest.get("/").build() + .newBuilder() + .addCookies(requestCookie("cookie", "xyz202")) + .build(); + + assertEquals(request.cookie("cookie"), Optional.of(requestCookie("cookie", "xyz202"))); + } + + @Test + public void transformsByAddingCookiesList() { + LiveHttpRequest request = LiveHttpRequest.get("/").build() + .newBuilder() + .addCookies(ImmutableList.of(requestCookie("cookie", "xyz202"))) + .build(); + + assertEquals(request.cookie("cookie"), Optional.of(requestCookie("cookie", "xyz202"))); + } + + @Test + public void transformsByRemovingCookies() { + LiveHttpRequest request = LiveHttpRequest.get("/").addCookies(requestCookie("cookie", "xyz202")).build() + .newBuilder() + .removeCookies("cookie") + .build(); + + assertEquals(request.cookie("cookie"), Optional.empty()); + } + + @Test + public void transformsByRemovingCookieList() { + LiveHttpRequest request = LiveHttpRequest.get("/").addCookies(requestCookie("cookie", "xyz202")).build() + .newBuilder() + .removeCookies(ImmutableList.of("cookie")) + .build(); + + assertEquals(request.cookie("cookie"), Optional.empty()); + } + private static ByteStream body(String... contents) { return new ByteStream( diff --git a/components/client/src/test/unit/java/com/hotels/styx/client/TransportTest.java b/components/client/src/test/unit/java/com/hotels/styx/client/TransportTest.java index 7edf31a8c4..a12cbda134 100644 --- a/components/client/src/test/unit/java/com/hotels/styx/client/TransportTest.java +++ b/components/client/src/test/unit/java/com/hotels/styx/client/TransportTest.java @@ -18,9 +18,9 @@ import com.google.common.collect.ImmutableList; import com.hotels.styx.api.Buffer; import com.hotels.styx.api.ByteStream; +import com.hotels.styx.api.Id; import com.hotels.styx.api.LiveHttpRequest; import com.hotels.styx.api.LiveHttpResponse; -import com.hotels.styx.api.Id; import com.hotels.styx.api.exceptions.NoAvailableHostsException; import com.hotels.styx.client.connectionpool.ConnectionPool; import org.testng.annotations.BeforeMethod; @@ -34,9 +34,9 @@ import java.util.Optional; import static com.hotels.styx.api.Buffers.toByteBuf; -import static com.hotels.styx.api.LiveHttpRequest.get; import static com.hotels.styx.api.HttpResponseStatus.OK; import static com.hotels.styx.api.Id.id; +import static com.hotels.styx.api.LiveHttpRequest.get; import static java.nio.charset.StandardCharsets.UTF_8; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; @@ -221,8 +221,7 @@ public void releasesContentStreamBuffersWhenPoolIsNotProvided() { Buffer chunk2 = new Buffer("y", UTF_8); Buffer chunk3 = new Buffer("z", UTF_8); - LiveHttpRequest aRequest = request - .newBuilder() + LiveHttpRequest aRequest = get("/") .body(new ByteStream(toPublisher(Observable.from(ImmutableList.of(chunk1, chunk2, chunk3))))) .build(); diff --git a/components/client/src/test/unit/java/com/hotels/styx/client/netty/connectionpool/HttpRequestOperationTest.java b/components/client/src/test/unit/java/com/hotels/styx/client/netty/connectionpool/HttpRequestOperationTest.java index 05f1dda598..31b03164f3 100644 --- a/components/client/src/test/unit/java/com/hotels/styx/client/netty/connectionpool/HttpRequestOperationTest.java +++ b/components/client/src/test/unit/java/com/hotels/styx/client/netty/connectionpool/HttpRequestOperationTest.java @@ -60,7 +60,7 @@ public void shouldTransformUrlQueryParametersToNettyRequest() { .uri("https://www.example.com/foo?some=value&blah=blah") .build(); - LiveHttpRequest.Builder builder = request.newBuilder(); + LiveHttpRequest.Transformer builder = request.newBuilder(); LiveHttpRequest newRequest = builder.url( request.url().newBuilder() diff --git a/components/proxy/src/main/java/com/hotels/styx/proxy/interceptors/HopByHopHeadersRemovingInterceptor.java b/components/proxy/src/main/java/com/hotels/styx/proxy/interceptors/HopByHopHeadersRemovingInterceptor.java index 51d7afc7d8..b9439b32f2 100644 --- a/components/proxy/src/main/java/com/hotels/styx/proxy/interceptors/HopByHopHeadersRemovingInterceptor.java +++ b/components/proxy/src/main/java/com/hotels/styx/proxy/interceptors/HopByHopHeadersRemovingInterceptor.java @@ -63,7 +63,7 @@ private static LiveHttpResponse removeHopByHopHeaders(LiveHttpResponse response) } private static LiveHttpRequest removeHopByHopHeaders(LiveHttpRequest request) { - LiveHttpRequest.Builder newRequest = request.newBuilder(); + LiveHttpRequest.Transformer newRequest = request.newBuilder(); request.header(CONNECTION).ifPresent(connection -> { for (String connectToken : connection.split(",")) { diff --git a/components/proxy/src/main/java/com/hotels/styx/proxy/interceptors/RequestEnrichingInterceptor.java b/components/proxy/src/main/java/com/hotels/styx/proxy/interceptors/RequestEnrichingInterceptor.java index f327acbb68..f4f5dafda0 100644 --- a/components/proxy/src/main/java/com/hotels/styx/proxy/interceptors/RequestEnrichingInterceptor.java +++ b/components/proxy/src/main/java/com/hotels/styx/proxy/interceptors/RequestEnrichingInterceptor.java @@ -48,7 +48,7 @@ public Eventual intercept(LiveHttpRequest request, Chain chain } private LiveHttpRequest enrich(LiveHttpRequest request, Context context) { - LiveHttpRequest.Builder builder = request.newBuilder(); + LiveHttpRequest.Transformer builder = request.newBuilder(); xForwardedFor(request, context) .ifPresent(headerValue -> builder.header(X_FORWARDED_FOR, headerValue)); diff --git a/system-tests/e2e-suite/src/test/scala/com/hotels/styx/plugins/AsyncRequestContentSpec.scala b/system-tests/e2e-suite/src/test/scala/com/hotels/styx/plugins/AsyncRequestContentSpec.scala deleted file mode 100644 index 9b80572d88..0000000000 --- a/system-tests/e2e-suite/src/test/scala/com/hotels/styx/plugins/AsyncRequestContentSpec.scala +++ /dev/null @@ -1,104 +0,0 @@ -/* - Copyright (C) 2013-2018 Expedia Inc. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - 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 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 - limitations under the License. - */ -package com.hotels.styx.plugins - -import java.nio.charset.StandardCharsets.UTF_8 - -import com.github.tomakehurst.wiremock.client.WireMock._ -import com.hotels.styx._ -import com.hotels.styx.api.HttpInterceptor.Chain -import com.hotels.styx.api.HttpRequest.get -import com.hotels.styx.api.{ByteStream, _} -import com.hotels.styx.support.backends.FakeHttpServer -import com.hotels.styx.support.configuration.{HttpBackend, Origins, StyxConfig} -import com.hotels.styx.support.server.UrlMatchingStrategies._ -import io.netty.handler.codec.http.HttpHeaders.Names._ -import io.netty.handler.codec.http.HttpHeaders.Values._ -import org.scalatest.{BeforeAndAfterAll, FunSpec} -import rx.RxReactiveStreams.{toObservable, toPublisher} - -import scala.concurrent.duration._ -import scala.compat.java8.FutureConverters.CompletionStageOps -import scala.concurrent.Await - - -class AsyncRequestContentSpec extends FunSpec - with StyxProxySpec - with BeforeAndAfterAll - with StyxClientSupplier { - - val mockServer = FakeHttpServer.HttpStartupConfig() - .start() - .reset() - .stub(urlStartingWith("/foobar"), aResponse - .withStatus(200) - .withHeader(TRANSFER_ENCODING, CHUNKED) - .withBody("I should be here!") - ) - - override val styxConfig = StyxConfig(plugins = List("asyncDelayPlugin" -> new AsyncRequestContentDelayPlugin())) - - override protected def beforeAll(): Unit = { - super.beforeAll() - styxServer.setBackends( - "/foobar" -> HttpBackend("appOne", Origins(mockServer), responseTimeout = 5.seconds) - ) - } - - override protected def afterAll(): Unit = { - mockServer.stop() - super.afterAll() - } - - describe("Styx as a plugin container") { - it("Proxies requests when plugin processes request content asynchronously on a separate thread pool") { - val request = get(styxServer.routerURL("/foobar")) - .addHeader("Content-Length", "0") - .build() - - val response = Await.result(client.sendRequest(request).toScala, 10.seconds) - - mockServer.verify(1, getRequestedFor(urlStartingWith("/foobar"))) - response.bodyAs(UTF_8) should be("I should be here!") - } - } -} - -import com.hotels.styx.support.ImplicitScalaRxConversions.toJavaObservable -import rx.lang.scala.JavaConversions.toScalaObservable -import rx.lang.scala.Observable -import rx.lang.scala.schedulers._ - -import scala.compat.java8.FunctionConverters.asJavaFunction - -class AsyncRequestContentDelayPlugin extends PluginAdapter { - override def intercept(request: LiveHttpRequest, chain: Chain): Eventual[LiveHttpResponse] = { - val contentTransformation: rx.Observable[Buffer] = - toObservable(request.body()) - .observeOn(ComputationScheduler()) - .flatMap(byteBuf => { - Thread.sleep(1000) - Observable.just(byteBuf) - }) - - // This was split apart as it no longer compiles without the type annotation Eventual[LiveHttpRequest] - val mapped: Eventual[LiveHttpRequest] = Eventual.of(request) - .map(asJavaFunction((request: LiveHttpRequest) => request.newBuilder().body(new ByteStream(toPublisher(contentTransformation))).build())) - - mapped - .flatMap(asJavaFunction((request: LiveHttpRequest) => chain.proceed(request))) - } -} diff --git a/system-tests/e2e-suite/src/test/scala/com/hotels/styx/plugins/AsyncResponseContentSpec.scala b/system-tests/e2e-suite/src/test/scala/com/hotels/styx/plugins/AsyncResponseContentSpec.scala deleted file mode 100644 index 0a96270fef..0000000000 --- a/system-tests/e2e-suite/src/test/scala/com/hotels/styx/plugins/AsyncResponseContentSpec.scala +++ /dev/null @@ -1,99 +0,0 @@ -/* - Copyright (C) 2013-2018 Expedia Inc. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - 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 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 - limitations under the License. - */ -package com.hotels.styx.plugins - -import java.nio.charset.StandardCharsets.UTF_8 - -import _root_.io.netty.handler.codec.http.HttpHeaders.Names._ -import _root_.io.netty.handler.codec.http.HttpHeaders.Values._ -import com.github.tomakehurst.wiremock.client.WireMock._ -import com.hotels.styx.api.HttpRequest.get -import com.hotels.styx.api.HttpInterceptor.Chain -import com.hotels.styx.api._ -import com.hotels.styx.support._ -import com.hotels.styx.support.backends.FakeHttpServer -import com.hotels.styx.support.configuration.{HttpBackend, Origins, StyxConfig} -import com.hotels.styx.support.server.UrlMatchingStrategies._ -import com.hotels.styx.{PluginAdapter, StyxClientSupplier, StyxProxySpec} -import org.scalatest.FunSpec -import rx.Observable -import rx.RxReactiveStreams.{toObservable, toPublisher} -import rx.schedulers.Schedulers - -import scala.concurrent.duration._ - - -class AsyncResponseContentSpec extends FunSpec - with StyxProxySpec - with StyxClientSupplier - with NettyOrigins { - val mockServer = FakeHttpServer.HttpStartupConfig().start() - - override val styxConfig = StyxConfig(plugins = List("asyncDelayPlugin" -> new AsyncDelayPlugin())) - - override protected def beforeAll(): Unit = { - super.beforeAll() - mockServer.start() - - styxServer.setBackends( - "/foobar" -> HttpBackend("appOne", Origins(mockServer), responseTimeout = 5.seconds) - ) - } - - override protected def afterAll(): Unit = { - mockServer.stop() - super.afterAll() - } - - describe("Styx as a plugin container") { - it("Proxies requests when plugin processes response content asynchronously on a separate thread pool") { - mockServer.stub(urlStartingWith("/foobar"), aResponse - .withStatus(200) - .withHeader(TRANSFER_ENCODING, CHUNKED) - .withBody("I should be here!") - ) - - val request = get(styxServer.routerURL("/foobar")) - .addHeader("Content-Length", "0") - .build() - - val response = decodedRequest(request) - - mockServer.verify(1, getRequestedFor(urlStartingWith("/foobar"))) - response.bodyAs(UTF_8) should be("I should be here!") - } - } -} - -import rx.lang.scala.ImplicitFunctionConversions._ -import scala.compat.java8.FunctionConverters.asJavaFunction - -class AsyncDelayPlugin extends PluginAdapter { - override def intercept(request: LiveHttpRequest, chain: Chain): Eventual[LiveHttpResponse] = { - chain.proceed(request) - .flatMap(asJavaFunction((response: LiveHttpResponse) => { - - val transformedContent: Observable[Buffer] = toObservable(response.body()) - .observeOn(Schedulers.computation()) - .flatMap((buffer: Buffer) => { - Thread.sleep(1000) - Observable.just(buffer) - }) - - Eventual.of(response.newBuilder().body(new ByteStream(toPublisher(transformedContent))).build()) - })) - } -} From 4e32ca4d45738f8466a8d282ea7da60b17800873 Mon Sep 17 00:00:00 2001 From: Mikko Karjalainen Date: Thu, 25 Oct 2018 17:33:13 +0100 Subject: [PATCH 3/4] Add response Transformer class for LiveHttpResponse class. --- .../com/hotels/styx/api/LiveHttpResponse.java | 308 +++++++++++++++++- .../hotels/styx/api/LiveHttpResponseTest.java | 144 ++++++-- .../hotels/styx/proxy/ProxyServerBuilder.java | 2 +- .../HopByHopHeadersRemovingInterceptor.java | 2 +- .../netty/connectors/HttpPipelineHandler.java | 8 +- .../netty/connectors/ResponseEnhancer.java | 4 +- .../connectors/HttpPipelineHandlerTest.java | 18 +- .../styx/support/origins/AppHandler.java | 52 ++- 8 files changed, 461 insertions(+), 77 deletions(-) diff --git a/components/api/src/main/java/com/hotels/styx/api/LiveHttpResponse.java b/components/api/src/main/java/com/hotels/styx/api/LiveHttpResponse.java index 548a666335..e26980b8db 100644 --- a/components/api/src/main/java/com/hotels/styx/api/LiveHttpResponse.java +++ b/components/api/src/main/java/com/hotels/styx/api/LiveHttpResponse.java @@ -153,8 +153,8 @@ public HttpVersion version() { * * @return new builder based on this response */ - public Builder newBuilder() { - return new Builder(this); + public Transformer newBuilder() { + return new Transformer(this); } /** @@ -258,10 +258,299 @@ public boolean equals(Object obj) { && Objects.equal(this.headers, other.headers); } + private interface BuilderTransformer { + /** + * Sets the response status. + * + * @param status response status + * @return {@code this} + */ + BuilderTransformer status(HttpResponseStatus status); + + + /** + * Sets the HTTP version. + * + * @param version HTTP version + * @return {@code this} + */ + BuilderTransformer version(HttpVersion version); + + /** + * Disables client-side caching of this response. + * + * @return {@code this} + */ + BuilderTransformer disableCaching(); + + /** + * Makes this response chunked. + * + * @return {@code this} + */ + BuilderTransformer setChunked(); + + /** + * Sets the cookies on this response by removing existing "Set-Cookie" headers and adding new ones. + * + * @param cookies cookies + * @return {@code this} + */ + BuilderTransformer cookies(ResponseCookie... cookies); + + /** + * Sets the cookies on this response by removing existing "Set-Cookie" headers and adding new ones. + * + * @param cookies cookies + * @return {@code this} + */ + BuilderTransformer cookies(Collection cookies); + + /** + * Adds cookies into this response by adding "Set-Cookie" headers. + * + * @param cookies cookies + * @return {@code this} + */ + BuilderTransformer addCookies(ResponseCookie... cookies); + /** + * Adds cookies into this response by adding "Set-Cookie" headers. + * + * @param cookies cookies + * @return {@code this} + */ + BuilderTransformer addCookies(Collection cookies); + + /** + * Removes all cookies matching one of the supplied names by removing their "Set-Cookie" headers. + * + * @param names cookie names + * @return {@code this} + */ + BuilderTransformer removeCookies(String... names); + + /** + * Removes all cookies matching one of the supplied names by removing their "Set-Cookie" headers. + * + * @param names cookie names + * @return {@code this} + */ + BuilderTransformer removeCookies(Collection names); + + /** + * Sets the (only) value for the header with the specified name. + *

+ * All existing values for the same header will be removed. + * + * @param name The name of the header + * @param value The value of the header + * @return {@code this} + */ + BuilderTransformer header(CharSequence name, Object value); + + /** + * Adds a new header with the specified {@code name} and {@code value}. + *

+ * Will not replace any existing values for the header. + * + * @param name The name of the header + * @param value The value of the header + * @return {@code this} + */ + BuilderTransformer addHeader(CharSequence name, Object value); + + /** + * Removes the header with the specified name. + * + * @param name The name of the header to remove + * @return {@code this} + */ + BuilderTransformer removeHeader(CharSequence name); + + /** + * (UNSTABLE) Removes body stream from this request. + *

+ * This method is unstable until Styx 1.0 API is frozen. + *

+ * Inappropriate use can lead to stability issues. These issues + * will be addressed before Styx 1.0 API is frozen. Therefore + * it is best to avoid until then. Consult + * https://github.com/HotelsDotCom/styx/issues/201 for more + * details. + * + * @return {@code this} + */ + // TODO: See https://github.com/HotelsDotCom/styx/issues/201 + BuilderTransformer removeBody(); + + + /** + * Sets the headers. + * + * @param headers headers + * @return {@code this} + */ + BuilderTransformer headers(HttpHeaders headers); + + /** + * Disables automatic validation of some HTTP headers. + *

+ * Normally the {@link Builder} validates the message when {@code build} + * method is invoked. Specifically that: + * + *

  • + *
      There is maximum of only one {@code Content-Length} header
    + *
      The {@code Content-Length} header is zero or positive integer
    + *
    • + * + * @return {@code this} + */ + BuilderTransformer disableValidation(); + + /** + * Builds a new full response based on the settings configured in this builder. + *

    + * Validates and builds a {link LiveHttpResponse} object. Object validation can be + * disabled with {@link this.disableValidation} method. + *

    + * When validation is enabled (by default), ensures that: + * + *

  • + *
      There is maximum of only one {@code Content-Length} header
    + *
      The {@code Content-Length} header is zero or positive integer
    + *
    • + * + * @return a new full response. + * @throws IllegalArgumentException when validation fails + */ + LiveHttpResponse build(); + + } + + public static final class Transformer implements BuilderTransformer { + private final Builder builder; + + public Transformer(LiveHttpResponse response) { + this.builder = new Builder(response); + } + + /** + * Transforms request body. + * + * @param transformation a Function from ByteStream to ByteStream. + * @return a HttpResponhse builder with a transformed message body. + */ + public Transformer body(Function transformation) { + this.builder.body(requireNonNull(transformation.apply(this.builder.body))); + return this; + } + + @Override + public Transformer status(HttpResponseStatus status) { + builder.status(status); + return this; + } + + @Override + public Transformer version(HttpVersion version) { + builder.version(version); + return this; + } + + @Override + public Transformer disableCaching() { + builder.disableCaching(); + return this; + } + + @Override + public Transformer setChunked() { + builder.setChunked(); + return this; + } + + @Override + public Transformer cookies(ResponseCookie... cookies) { + builder.cookies(cookies); + return this; + } + + @Override + public Transformer cookies(Collection cookies) { + builder.cookies(cookies); + return this; + } + + @Override + public Transformer addCookies(ResponseCookie... cookies) { + builder.addCookies(cookies); + return this; + } + + @Override + public Transformer addCookies(Collection cookies) { + builder.addCookies(cookies); + return this; + } + + @Override + public Transformer removeCookies(String... names) { + builder.removeCookies(names); + return this; + } + + @Override + public Transformer removeCookies(Collection names) { + builder.removeCookies(names); + return this; + } + + @Override + public Transformer header(CharSequence name, Object value) { + builder.header(name, value); + return this; + } + + @Override + public Transformer addHeader(CharSequence name, Object value) { + builder.addHeader(name, value); + return this; + } + + @Override + public Transformer removeHeader(CharSequence name) { + builder.removeHeader(name); + return this; + } + + @Override + public Transformer removeBody() { + builder.removeBody(); + return this; + } + + @Override + public Transformer headers(HttpHeaders headers) { + builder.headers(headers); + return this; + } + + @Override + public Transformer disableValidation() { + builder.disableValidation(); + return this; + } + + @Override + public LiveHttpResponse build() { + return this.builder.build(); + } + } + /** * An HTTP response builder. */ - public static final class Builder { + public static final class Builder implements BuilderTransformer { private HttpResponseStatus status = OK; private HttpHeaders.Builder headers; private HttpVersion version = HTTP_1_1; @@ -337,17 +626,6 @@ public Builder body(ByteStream content) { return this; } - /** - * Transforms request body. - * - * @param transformation a Function from ByteStream to ByteStream. - * @return a HttpResponhse builder with a transformed message body. - */ - public Builder body(Function transformation) { - this.body = requireNonNull(transformation.apply(this.body)); - return this; - } - /** * Sets the HTTP version. * @@ -450,7 +728,7 @@ public Builder removeCookies(String... names) { * @param names cookie names * @return {@code this} */ - public Builder removeCookies(Collection names) { + public Builder removeCookies(Collection names) { requireNonNull(names); if (names.isEmpty()) { diff --git a/components/api/src/test/java/com/hotels/styx/api/LiveHttpResponseTest.java b/components/api/src/test/java/com/hotels/styx/api/LiveHttpResponseTest.java index aa8f7ab9bf..7f4c2f2ac0 100644 --- a/components/api/src/test/java/com/hotels/styx/api/LiveHttpResponseTest.java +++ b/components/api/src/test/java/com/hotels/styx/api/LiveHttpResponseTest.java @@ -15,10 +15,12 @@ */ package com.hotels.styx.api; +import com.google.common.collect.ImmutableList; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import reactor.core.publisher.Flux; +import java.util.Optional; import java.util.Set; import java.util.concurrent.ExecutionException; import java.util.stream.Stream; @@ -166,25 +168,6 @@ public void canRemoveAHeader() { assertThat(shouldRemoveHeader.headers(), contains(header("a", "b"))); } - @Test - public void canRemoveResponseBody() throws ExecutionException, InterruptedException { - Buffer originalContent = new Buffer("I'm going to get removed.", UTF_8); - - LiveHttpResponse response = response(NO_CONTENT) - .body(new ByteStream(Flux.just(originalContent))) - .build(); - - HttpResponse fullResponse = response.newBuilder() - .body(ByteStream::drop) - .build() - .aggregate(1000) - .asCompletableFuture() - .get(); - - assertThat(fullResponse.body().length, is(0)); - assertThat(originalContent.delegate().refCnt(), is(0)); - } - @Test public void supportsCaseInsensitiveHeaderNames() { LiveHttpResponse response = response(OK).header("Content-Type", "text/plain").build(); @@ -369,6 +352,129 @@ public void consumesBody() { assertEquals(buf2.delegate().refCnt(), 0); } + @Test + public void transformsStatus() { + LiveHttpResponse response = response(OK).build() + .newBuilder() + .status(MOVED_PERMANENTLY) + .build(); + + assertEquals(response.status(), MOVED_PERMANENTLY); + } + + @Test + public void transformsCookies() { + LiveHttpResponse response = response().build() + .newBuilder() + .cookies(responseCookie("x", "y").build()) + .build(); + + assertEquals(response.cookie("x"), Optional.of(responseCookie("x", "y").build())); + } + + @Test + public void transformsWithCookieList() { + LiveHttpResponse response = response().build() + .newBuilder() + .cookies(ImmutableList.of(responseCookie("x", "y").build())) + .build(); + + assertEquals(response.cookie("x"), Optional.of(responseCookie("x", "y").build())); + } + + @Test + public void transformerAddsCookies() { + LiveHttpResponse response = response().build() + .newBuilder() + .addCookies(responseCookie("x", "y").build()) + .build(); + + assertEquals(response.cookie("x"), Optional.of(responseCookie("x", "y").build())); + } + + @Test + public void transformerAddsCookiesList() { + LiveHttpResponse response = response().build() + .newBuilder() + .addCookies(ImmutableList.of(responseCookie("x", "y").build())) + .build(); + + assertEquals(response.cookie("x"), Optional.of(responseCookie("x", "y").build())); + } + + @Test + public void transformerRemovesCookies() { + LiveHttpResponse response = response() + .addCookies(ImmutableList.of(responseCookie("x", "y").build())) + .build() + .newBuilder() + .removeCookies("x") + .build(); + + assertEquals(response.cookie("x"), Optional.empty()); + } + + @Test + public void transformerRemovesCookiesWithList() { + LiveHttpResponse response = response() + .addCookies(ImmutableList.of(responseCookie("x", "y").build())) + .build() + .newBuilder() + .removeCookies(ImmutableList.of("x")) + .build(); + + assertEquals(response.cookie("x"), Optional.empty()); + } + + @Test + public void transformerAddsHeaders() { + LiveHttpResponse response = response().build() + .newBuilder() + .addHeader("X-Styx-ID", "y") + .build(); + + assertEquals(response.header("X-Styx-ID"), Optional.of("y")); + } + + @Test + public void transformerRemovesHeaders() { + LiveHttpResponse response = response().addHeader("X-Styx-ID", "y").build() + .newBuilder() + .removeHeader("X-Styx-ID") + .build(); + + assertEquals(response.header("X-Styx-ID"), Optional.empty()); + } + + @Test + public void transformerSetsHeaders() { + LiveHttpResponse response = response().build() + .newBuilder() + .headers(new HttpHeaders.Builder().add("X-Styx-ID", "z").build()) + .build(); + + assertEquals(response.header("X-Styx-ID"), Optional.of("z")); + } + + @Test + public void transformsBody() throws ExecutionException, InterruptedException { + Buffer buffer = new Buffer("I'm going to get removed.", UTF_8); + + LiveHttpResponse response = response(NO_CONTENT) + .body(new ByteStream(Flux.just(buffer))) + .build(); + + HttpResponse fullResponse = response.newBuilder() + .body(ByteStream::drop) + .build() + .aggregate(1000) + .asCompletableFuture() + .get(); + + assertThat(fullResponse.body().length, is(0)); + assertThat(buffer.delegate().refCnt(), is(0)); + } + private static LiveHttpResponse.Builder response() { return LiveHttpResponse.response(); } diff --git a/components/proxy/src/main/java/com/hotels/styx/proxy/ProxyServerBuilder.java b/components/proxy/src/main/java/com/hotels/styx/proxy/ProxyServerBuilder.java index 8aae65e23e..934f45c275 100644 --- a/components/proxy/src/main/java/com/hotels/styx/proxy/ProxyServerBuilder.java +++ b/components/proxy/src/main/java/com/hotels/styx/proxy/ProxyServerBuilder.java @@ -56,7 +56,7 @@ public HttpServer build() { .build(); } - private LiveHttpResponse.Builder addInfoHeader(LiveHttpResponse.Builder responseBuilder, LiveHttpRequest request) { + private LiveHttpResponse.Transformer addInfoHeader(LiveHttpResponse.Transformer responseBuilder, LiveHttpRequest request) { return responseBuilder.header(styxInfoHeaderName, responseInfoFormat.format(request)); } diff --git a/components/proxy/src/main/java/com/hotels/styx/proxy/interceptors/HopByHopHeadersRemovingInterceptor.java b/components/proxy/src/main/java/com/hotels/styx/proxy/interceptors/HopByHopHeadersRemovingInterceptor.java index b9439b32f2..b70a9cad4f 100644 --- a/components/proxy/src/main/java/com/hotels/styx/proxy/interceptors/HopByHopHeadersRemovingInterceptor.java +++ b/components/proxy/src/main/java/com/hotels/styx/proxy/interceptors/HopByHopHeadersRemovingInterceptor.java @@ -40,7 +40,7 @@ public Eventual intercept(LiveHttpRequest request, Chain chain } private static LiveHttpResponse removeHopByHopHeaders(LiveHttpResponse response) { - LiveHttpResponse.Builder newResponse = response.newBuilder(); + LiveHttpResponse.Transformer newResponse = response.newBuilder(); response.header(CONNECTION).ifPresent(connection -> { for (String connectToken : connection.split(",")) { diff --git a/components/server/src/main/java/com/hotels/styx/server/netty/connectors/HttpPipelineHandler.java b/components/server/src/main/java/com/hotels/styx/server/netty/connectors/HttpPipelineHandler.java index 54197ed87a..b399cffd23 100644 --- a/components/server/src/main/java/com/hotels/styx/server/netty/connectors/HttpPipelineHandler.java +++ b/components/server/src/main/java/com/hotels/styx/server/netty/connectors/HttpPipelineHandler.java @@ -495,9 +495,13 @@ private LiveHttpResponse exceptionToResponse(Throwable exception, LiveHttpReques String message = status.code() >= 500 ? "Site temporarily unavailable." : status.description(); - return responseEnhancer.enhance(LiveHttpResponse.response(status), request) + return responseEnhancer.enhance( + LiveHttpResponse + .response(status) + .body(new ByteStream(Flux.just(new Buffer(message, UTF_8)))) + .build() + .newBuilder(), request) .header(CONTENT_LENGTH, message.getBytes(UTF_8).length) - .body(new ByteStream(Flux.just(new Buffer(message, UTF_8)))) .build(); } diff --git a/components/server/src/main/java/com/hotels/styx/server/netty/connectors/ResponseEnhancer.java b/components/server/src/main/java/com/hotels/styx/server/netty/connectors/ResponseEnhancer.java index 082db6a868..0ad2ce4cdc 100644 --- a/components/server/src/main/java/com/hotels/styx/server/netty/connectors/ResponseEnhancer.java +++ b/components/server/src/main/java/com/hotels/styx/server/netty/connectors/ResponseEnhancer.java @@ -31,11 +31,11 @@ public interface ResponseEnhancer { * @param request request to which Styx is responding * @return enhanced response builder */ - LiveHttpResponse.Builder enhance(LiveHttpResponse.Builder responseBuilder, LiveHttpRequest request); + LiveHttpResponse.Transformer enhance(LiveHttpResponse.Transformer responseBuilder, LiveHttpRequest request); /** * Create a new enhanced response, based on an existing one. This is less efficient than - * {@link #enhance(LiveHttpResponse.Builder, LiveHttpRequest)} as it has to create a new builder + * {@link #enhance(LiveHttpResponse.Transformer, LiveHttpRequest)} as it has to create a new builder * and build, but it suitable for cases where that would have to happen anyway. * * @param response response diff --git a/components/server/src/test/java/com/hotels/styx/server/netty/connectors/HttpPipelineHandlerTest.java b/components/server/src/test/java/com/hotels/styx/server/netty/connectors/HttpPipelineHandlerTest.java index 1fa4cba4d5..446175bb49 100644 --- a/components/server/src/test/java/com/hotels/styx/server/netty/connectors/HttpPipelineHandlerTest.java +++ b/components/server/src/test/java/com/hotels/styx/server/netty/connectors/HttpPipelineHandlerTest.java @@ -156,7 +156,7 @@ public void setUp() throws Exception { response = response().build(); responseEnhancer = mock(ResponseEnhancer.class); - when(responseEnhancer.enhance(any(LiveHttpResponse.Builder.class), any(LiveHttpRequest.class))).thenAnswer(invocationOnMock -> invocationOnMock.getArguments()[0]); + when(responseEnhancer.enhance(any(LiveHttpResponse.Transformer.class), any(LiveHttpRequest.class))).thenAnswer(invocationOnMock -> invocationOnMock.getArguments()[0]); setupHandlerTo(ACCEPTING_REQUESTS); } @@ -221,7 +221,7 @@ public void mapsWrappedBadRequestExceptionToBadRequest400ResponseCode() { DefaultHttpResponse response = (DefaultHttpResponse) channel.readOutbound(); assertThat(response.getStatus(), is(io.netty.handler.codec.http.HttpResponseStatus.BAD_REQUEST)); - verify(responseEnhancer).enhance(any(LiveHttpResponse.Builder.class), eq(null)); + verify(responseEnhancer).enhance(any(LiveHttpResponse.Transformer.class), eq(null)); verify(errorListener, only()).proxyErrorOccurred(eq(BAD_REQUEST), any(BadRequestException.class)); } @@ -236,7 +236,7 @@ public void mapsUnrecoverableInternalErrorsToInternalServerError500ResponseCode( DefaultHttpResponse response = (DefaultHttpResponse) channel.readOutbound(); assertThat(response.status(), is(io.netty.handler.codec.http.HttpResponseStatus.INTERNAL_SERVER_ERROR)); - verify(responseEnhancer).enhance(any(LiveHttpResponse.Builder.class), any(LiveHttpRequest.class)); + verify(responseEnhancer).enhance(any(LiveHttpResponse.Transformer.class), any(LiveHttpRequest.class)); verify(errorListener, only()).proxyErrorOccurred(any(LiveHttpRequest.class), any(InetSocketAddress.class), eq(INTERNAL_SERVER_ERROR), any(RuntimeException.class)); } @@ -383,7 +383,7 @@ public void decrementsRequestsOngoingOnExceptionCaught() throws Exception { adapter.channelInactive(ctx); assertThat(metrics.counter("outstanding").getCount(), is(0L)); - verify(responseEnhancer).enhance(any(LiveHttpResponse.Builder.class), eq(request)); + verify(responseEnhancer).enhance(any(LiveHttpResponse.Transformer.class), eq(request)); } @Test @@ -596,7 +596,7 @@ public void pluginPipelineThrowsAnExceptionInAcceptingRequestsState() throws Exc .build() .stream()); - verify(responseEnhancer).enhance(any(LiveHttpResponse.Builder.class), eq(request)); + verify(responseEnhancer).enhance(any(LiveHttpResponse.Transformer.class), eq(request)); verify(errorListener).proxyErrorOccurred(request, InetSocketAddress.createUnresolved("localhost", 2), INTERNAL_SERVER_ERROR, cause); verify(statsCollector).onTerminate(request.id()); assertThat(handler.state(), is(TERMINATED)); @@ -608,7 +608,7 @@ public void requestTimeoutExceptionOccursInAcceptingRequestsStateAndTcpConnectio handler.exceptionCaught(ctx, cause); verify(errorListener).proxyErrorOccurred(REQUEST_TIMEOUT, cause); - verify(responseEnhancer).enhance(any(LiveHttpResponse.Builder.class), eq(null)); + verify(responseEnhancer).enhance(any(LiveHttpResponse.Transformer.class), eq(null)); verify(responseWriter).write(response(REQUEST_TIMEOUT) .header(CONTENT_LENGTH, 15) .build()); @@ -622,7 +622,7 @@ public void tooLongFrameExceptionOccursInIdleStateAndTcpConnectionRemainsActive( handler.exceptionCaught(ctx, cause); verify(errorListener).proxyErrorOccurred(REQUEST_ENTITY_TOO_LARGE, cause); - verify(responseEnhancer).enhance(any(LiveHttpResponse.Builder.class), eq(request)); + verify(responseEnhancer).enhance(any(LiveHttpResponse.Transformer.class), eq(request)); verify(responseWriter).write(response(REQUEST_ENTITY_TOO_LARGE) .header(CONTENT_LENGTH, 24) .build()); @@ -636,7 +636,7 @@ public void badRequestExceptionExceptionOccursInIdleStateAndTcpConnectionRemains handler.exceptionCaught(ctx, cause); verify(errorListener).proxyErrorOccurred(BAD_REQUEST, cause); - verify(responseEnhancer).enhance(any(LiveHttpResponse.Builder.class), eq(request)); + verify(responseEnhancer).enhance(any(LiveHttpResponse.Transformer.class), eq(request)); verify(responseWriter).write(response(BAD_REQUEST) .header(CONTENT_LENGTH, 11) .build()); @@ -673,7 +673,7 @@ public void responseObservableEmitsContentOverflowExceptionInWaitingForResponseS .body("Site temporarily unavailable.", UTF_8) .build() .stream()); - verify(responseEnhancer).enhance(any(LiveHttpResponse.Builder.class), eq(request)); + verify(responseEnhancer).enhance(any(LiveHttpResponse.Transformer.class), eq(request)); writerFuture.complete(null); verify(statsCollector).onComplete(request.id(), 502); diff --git a/support/origins-starter-app/src/main/java/com/hotels/styx/support/origins/AppHandler.java b/support/origins-starter-app/src/main/java/com/hotels/styx/support/origins/AppHandler.java index e3aaf8261d..06a20f0c2c 100644 --- a/support/origins-starter-app/src/main/java/com/hotels/styx/support/origins/AppHandler.java +++ b/support/origins-starter-app/src/main/java/com/hotels/styx/support/origins/AppHandler.java @@ -15,21 +15,20 @@ */ package com.hotels.styx.support.origins; -import com.hotels.styx.api.Buffer; -import com.hotels.styx.api.ByteStream; import com.hotels.styx.api.Eventual; import com.hotels.styx.api.HttpHandler; import com.hotels.styx.api.HttpInterceptor; +import com.hotels.styx.api.HttpResponse; +import com.hotels.styx.api.HttpResponseStatus; import com.hotels.styx.api.LiveHttpRequest; import com.hotels.styx.api.LiveHttpResponse; -import com.hotels.styx.api.HttpResponseStatus; import com.hotels.styx.api.extension.Origin; -import com.hotels.styx.common.http.handler.StaticBodyHttpHandler; -import reactor.core.publisher.Flux; + +import java.util.Optional; import static com.google.common.net.MediaType.HTML_UTF_8; -import static com.hotels.styx.api.HttpHeaderNames.CONTENT_LENGTH; import static com.hotels.styx.api.HttpHeaderNames.CONTENT_TYPE; +import static com.hotels.styx.api.HttpResponseStatus.OK; import static com.hotels.styx.utils.StubOriginHeader.STUB_ORIGIN_INFO; import static java.lang.Integer.parseInt; import static java.lang.String.format; @@ -38,37 +37,34 @@ import static java.util.UUID.randomUUID; public class AppHandler implements HttpHandler { - private final HttpHandler handler; private final Origin origin; + private final HttpResponse standardResponse; public AppHandler(Origin origin) { this.origin = origin; - this.handler = new StaticBodyHttpHandler(HTML_UTF_8, makeAResponse(origin)); + this.standardResponse = HttpResponse.response(OK) + .header(CONTENT_TYPE, HTML_UTF_8.toString()) + .body(makeAResponse(origin), UTF_8) + .build(); } @Override public Eventual handle(LiveHttpRequest request, HttpInterceptor.Context context) { - return handler.handle(request, context) - .map(response -> { - LiveHttpResponse.Builder responseBuilder = response.newBuilder() - .headers(request.headers()) - .header(STUB_ORIGIN_INFO, origin.applicationInfo()); - - response.contentLength().ifPresent(contentLength -> responseBuilder.header(CONTENT_LENGTH, contentLength)); - response.contentType().ifPresent(contentType -> responseBuilder.header(CONTENT_TYPE, contentType)); - - request.queryParam("status").ifPresent(status -> - responseBuilder - .status(httpResponseStatus(status)) - .body(new ByteStream(Flux.just(new Buffer("Returning requested status (" + status + ")", UTF_8)))) - ); - - request.queryParam("length").ifPresent(length -> - responseBuilder.body(new ByteStream(Flux.just(new Buffer(generateContent(parseInt(length)), UTF_8)))) - ); + HttpResponse.Builder responseBuilder = standardResponse.newBuilder() + .headers(request.headers()) + .header(STUB_ORIGIN_INFO, origin.applicationInfo()); - return responseBuilder.build(); - }); + return Eventual.of(Optional.ofNullable(responseBuilder) + .map(it -> request.queryParam("status") + .map(status -> it.status(httpResponseStatus(status)) + .body("Returning requested status (" + status + ")", UTF_8)) + .orElse(it)) + .map(it -> request.queryParam("length") + .map(length -> it.body(generateContent(parseInt(length)), UTF_8)) + .orElse(it)) + .orElse(responseBuilder) + .build() + .stream()); } private static String makeAResponse(Origin origin) { From 6a9f27ef5ac436c939fdcecc2b2bb11a4484841e Mon Sep 17 00:00:00 2001 From: Mikko Karjalainen Date: Tue, 30 Oct 2018 09:34:58 +0000 Subject: [PATCH 4/4] Remove misleading commit message. --- .../java/com/hotels/styx/api/LiveHttpResponse.java | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/components/api/src/main/java/com/hotels/styx/api/LiveHttpResponse.java b/components/api/src/main/java/com/hotels/styx/api/LiveHttpResponse.java index e26980b8db..6f3abaf609 100644 --- a/components/api/src/main/java/com/hotels/styx/api/LiveHttpResponse.java +++ b/components/api/src/main/java/com/hotels/styx/api/LiveHttpResponse.java @@ -792,19 +792,13 @@ public Builder removeHeader(CharSequence name) { } /** - * (UNSTABLE) Removes body stream from this request. - *

    - * This method is unstable until Styx 1.0 API is frozen. + * Removes message body content from this request. *

    - * Inappropriate use can lead to stability issues. These issues - * will be addressed before Styx 1.0 API is frozen. Therefore - * it is best to avoid until then. Consult - * https://github.com/HotelsDotCom/styx/issues/201 for more - * details. + * Transforms the content {@link ByteStream} to an empty stream such + * that all received content events are discarded. * * @return {@code this} */ - // TODO: See https://github.com/HotelsDotCom/styx/issues/201 public Builder removeBody() { this.body = body.drop(); return this;