From 811138c36bb04c8d81c514dfcf0f36e91306937b Mon Sep 17 00:00:00 2001 From: Rouzbeh Delavari Date: Wed, 18 Nov 2015 01:07:51 +0100 Subject: [PATCH 1/2] cleaner EndpointInvocationHandler --- .../dispatch/EndpointInvocationHandler.java | 14 +++++++++++-- .../com/spotify/apollo/request/Handlers.java | 8 ++----- .../EndpointInvocationHandlerTest.java | 21 +------------------ 3 files changed, 15 insertions(+), 28 deletions(-) diff --git a/apollo-api-impl/src/main/java/com/spotify/apollo/dispatch/EndpointInvocationHandler.java b/apollo-api-impl/src/main/java/com/spotify/apollo/dispatch/EndpointInvocationHandler.java index 019789fac..2ce34d142 100644 --- a/apollo-api-impl/src/main/java/com/spotify/apollo/dispatch/EndpointInvocationHandler.java +++ b/apollo-api-impl/src/main/java/com/spotify/apollo/dispatch/EndpointInvocationHandler.java @@ -23,6 +23,7 @@ import com.google.common.base.Strings; import com.spotify.apollo.RequestContext; +import com.spotify.apollo.request.EndpointRunnableFactory; import com.spotify.apollo.request.OngoingRequest; import org.slf4j.Logger; @@ -33,15 +34,24 @@ import static com.spotify.apollo.Response.forStatus; import static com.spotify.apollo.Status.INTERNAL_SERVER_ERROR; -public class EndpointInvocationHandler { +public class EndpointInvocationHandler implements EndpointRunnableFactory { private static final Logger LOG = LoggerFactory.getLogger(EndpointInvocationHandler.class); + @Override + public Runnable create( + OngoingRequest ongoingRequest, + RequestContext requestContext, + Endpoint endpoint) { + + return () -> handle(ongoingRequest, requestContext, endpoint); + } + /** * Fires off the request processing asynchronously - that is, this method is likely to return * before the request processing finishes. */ - public void handle(OngoingRequest ongoingRequest, RequestContext requestContext, Endpoint endpoint) { + void handle(OngoingRequest ongoingRequest, RequestContext requestContext, Endpoint endpoint) { try { endpoint.invoke(requestContext) .whenComplete((message, throwable) -> { diff --git a/apollo-api-impl/src/main/java/com/spotify/apollo/request/Handlers.java b/apollo-api-impl/src/main/java/com/spotify/apollo/request/Handlers.java index 9b5a0674d..df15b55a0 100644 --- a/apollo-api-impl/src/main/java/com/spotify/apollo/request/Handlers.java +++ b/apollo-api-impl/src/main/java/com/spotify/apollo/request/Handlers.java @@ -19,9 +19,9 @@ */ package com.spotify.apollo.request; -import com.spotify.apollo.environment.IncomingRequestAwareClient; import com.spotify.apollo.dispatch.Endpoint; import com.spotify.apollo.dispatch.EndpointInvocationHandler; +import com.spotify.apollo.environment.IncomingRequestAwareClient; import com.spotify.apollo.meta.IncomingCallsGatherer; import com.spotify.apollo.route.ApplicationRouter; @@ -30,9 +30,6 @@ */ public final class Handlers { - private static final EndpointInvocationHandler ENDPOINT_INVOCATION_HANDLER = - new EndpointInvocationHandler(); - private Handlers() { } @@ -54,8 +51,7 @@ public static RequestRunnableFactory requestRunnableFactory( } public static EndpointRunnableFactory endpointRunnableFactory() { - return (request, requestContext, endpoint) -> - () -> ENDPOINT_INVOCATION_HANDLER.handle(request, requestContext, endpoint); + return new EndpointInvocationHandler(); } public static EndpointRunnableFactory withTracking( diff --git a/apollo-api-impl/src/test/java/com/spotify/apollo/dispatch/EndpointInvocationHandlerTest.java b/apollo-api-impl/src/test/java/com/spotify/apollo/dispatch/EndpointInvocationHandlerTest.java index 4e1e8b57d..526293d6d 100644 --- a/apollo-api-impl/src/test/java/com/spotify/apollo/dispatch/EndpointInvocationHandlerTest.java +++ b/apollo-api-impl/src/test/java/com/spotify/apollo/dispatch/EndpointInvocationHandlerTest.java @@ -24,9 +24,6 @@ import com.spotify.apollo.Response; import com.spotify.apollo.request.OngoingRequest; -import org.hamcrest.BaseMatcher; -import org.hamcrest.Description; -import org.hamcrest.Matcher; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -55,7 +52,6 @@ public class EndpointInvocationHandlerTest { EndpointInvocationHandler handler; @Mock private OngoingRequest ongoingRequest; - private Request requestMessage; @Mock private RequestContext requestContext; @Mock private Endpoint endpoint; @@ -69,10 +65,9 @@ public class EndpointInvocationHandlerTest { public void setUp() throws Exception { handler = new EndpointInvocationHandler(); - requestMessage = Request.forUri("http://foo/bar").withService("nameless-registry"); + Request requestMessage = Request.forUri("http://foo/bar").withService("nameless-registry"); when(ongoingRequest.request()).thenReturn(requestMessage); - when(requestContext.request()).thenReturn(requestMessage); future = new CompletableFuture<>(); } @@ -171,18 +166,4 @@ public void shouldRespondWithDetailMessageForSyncExceptionsToNonClientCallers() assertThat(messageArgumentCaptor.getValue().status().reasonPhrase(), containsString(exception.getMessage())); } - - private Matcher nullOrEmpty() { - return new BaseMatcher() { - @Override - public boolean matches(Object item) { - return item == null || (item instanceof String) && ((String) item).isEmpty(); - } - - @Override - public void describeTo(Description description) { - description.appendText("a null or empty string"); - } - }; - } } From 4762998648777580c36dea24d94913c007261d40 Mon Sep 17 00:00:00 2001 From: Rouzbeh Delavari Date: Wed, 18 Nov 2015 01:08:08 +0100 Subject: [PATCH 2/2] document how to decorate incoming/outgoing request handling --- apollo-environment/README.md | 60 ++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/apollo-environment/README.md b/apollo-environment/README.md index 49dab688d..49014cde4 100644 --- a/apollo-environment/README.md +++ b/apollo-environment/README.md @@ -51,3 +51,63 @@ public static void main(String[] args) { For a runnable example, see [`MinimalRunner`] (../apollo-http-service/src/test/java/com/spotify/apollo/httpservice/MinimalRunner.java) + + +## Extending incoming/outgoing request handling + +`ApolloEnvironmentModule` has a few extension points that allow 3rd party modules to +decorate internal components involved in incoming/outgoing request handling. + + +### IncomingRequestAwareClient +One important aspect of the Apollo `Client` is that it does not come with any protocol +support out of the box. Instead, support for different protocols should be added by +modules. These modules do so by injecting themselves into the `IncomingRequestAwareClient` +decoration chain. + +The decoration chain looks like: + +1. `OutgoingCallsGatheringClient` +1. `ServiceSettingClient` +1. `[IncomingRequestAwareClient]* <- Set` +1. `NoopClient` + + +### RequestRunnableFactory + +1. `[RequestRunnableFactory]* <- Set` +1. `RequestRunnableImpl` + + +### EndpointRunnableFactory + +1. `TrackingEndpointRunnableFactory` +1. `[EndpointRunnableFactory]* <- Set` +1. `EndpointInvocationHandler` + + +### RequestHandler +This is what is ultimately created from the `ApolloEnvironmentModule`. It will use +the `RequestRunnableFactory`, `EndpointRunnableFactory` and `IncomingRequestAwareClient` +decoration chains that were constructed above. See `RequestHandlerImpl` for how they are +used, but in short terms it's something like: + +```java +RequestHandler requestHandler = ongoingRequest -> + rrf.create(ongoingRequest).run((ongoingRequest, match) -> + erf.create(ongoingRequest, requestContext, endpoint).run()); +``` + + +### Injecting decorators +To contribute to any of the sets of decorators mentioned above, use Guice Multibinder. + +Here's an example of how a `ClientDecorator` is injected: + +```java +@Override +protected void configure() { + Multibinder.newSetBinder(binder(), ClientDecorator.class) + .addBinding().toProvider(HttpClientDecoratorProvider.class); +} +```