From 961a9be727c75c99696503db79f3d3ccb73fedde Mon Sep 17 00:00:00 2001 From: Andreas Kluth Date: Sun, 11 Mar 2018 00:24:51 +0100 Subject: [PATCH 1/6] Started to add a request ttl per content and per route. --- .../composing/AttoParserBasedComposer.java | 2 + .../composer/composing/ComposerFactory.java | 1 + .../composer/composing/ContentFetcher.java | 14 ----- .../composer/composing/IncludeProcessor.java | 2 + .../composer/composing/IncludedService.java | 27 ++++++++- .../composing/fetch/ContentFetcher.java | 15 +++++ .../composing/fetch/FetchContext.java | 45 +++++++++++++++ .../RecursionAwareContentFetcher.java | 17 +++--- .../{ => fetch}/ValidatingContentFetcher.java | 21 ++++--- .../rewedigital/composer/routing/Match.java | 27 ++++++--- .../composer/routing/RouteMatch.java | 11 +++- .../routing/RoutingConfiguration.java | 11 +++- .../session/CookieBasedSessionHandler.java | 2 +- .../composing/TemplateComposerTest.java | 1 + .../fetch/ValidatingContentFetcherTest.java | 48 ++++++++++++++++ .../fetch/ValidatingContentFetcherTest2.java} | 55 +++++++++++++++---- ....java => ComposingRequestHandlerTest.java} | 14 ++--- .../composer/routing/BackendRoutingTest.java | 8 ++- .../routing/RoutingConfigurationTest.java | 5 +- 19 files changed, 257 insertions(+), 69 deletions(-) delete mode 100755 src/main/java/com/rewedigital/composer/composing/ContentFetcher.java create mode 100755 src/main/java/com/rewedigital/composer/composing/fetch/ContentFetcher.java create mode 100644 src/main/java/com/rewedigital/composer/composing/fetch/FetchContext.java rename src/main/java/com/rewedigital/composer/composing/{ => fetch}/RecursionAwareContentFetcher.java (58%) rename src/main/java/com/rewedigital/composer/composing/{ => fetch}/ValidatingContentFetcher.java (70%) create mode 100644 src/test/java/com/rewedigital/composer/composing/fetch/ValidatingContentFetcherTest.java rename src/test/java/com/rewedigital/composer/{proxy/ValidatingContentFetcherTest.java => composing/fetch/ValidatingContentFetcherTest2.java} (69%) rename src/test/java/com/rewedigital/composer/proxy/{ComposingHandlerTest.java => ComposingRequestHandlerTest.java} (94%) diff --git a/src/main/java/com/rewedigital/composer/composing/AttoParserBasedComposer.java b/src/main/java/com/rewedigital/composer/composing/AttoParserBasedComposer.java index c2069c5..b1539af 100755 --- a/src/main/java/com/rewedigital/composer/composing/AttoParserBasedComposer.java +++ b/src/main/java/com/rewedigital/composer/composing/AttoParserBasedComposer.java @@ -4,6 +4,8 @@ import java.util.concurrent.CompletableFuture; +import com.rewedigital.composer.composing.fetch.ContentFetcher; +import com.rewedigital.composer.composing.fetch.RecursionAwareContentFetcher; import com.rewedigital.composer.parser.Parser; import com.rewedigital.composer.session.ResponseWithSession; import com.rewedigital.composer.session.SessionFragment; diff --git a/src/main/java/com/rewedigital/composer/composing/ComposerFactory.java b/src/main/java/com/rewedigital/composer/composing/ComposerFactory.java index 9fa8259..b347aea 100644 --- a/src/main/java/com/rewedigital/composer/composing/ComposerFactory.java +++ b/src/main/java/com/rewedigital/composer/composing/ComposerFactory.java @@ -2,6 +2,7 @@ import java.util.Map; +import com.rewedigital.composer.composing.fetch.ValidatingContentFetcher; import com.rewedigital.composer.session.SessionRoot; import com.spotify.apollo.Client; import com.typesafe.config.Config; diff --git a/src/main/java/com/rewedigital/composer/composing/ContentFetcher.java b/src/main/java/com/rewedigital/composer/composing/ContentFetcher.java deleted file mode 100755 index 0f7d565..0000000 --- a/src/main/java/com/rewedigital/composer/composing/ContentFetcher.java +++ /dev/null @@ -1,14 +0,0 @@ -package com.rewedigital.composer.composing; - -import java.util.concurrent.CompletableFuture; - -import com.spotify.apollo.Response; - -/** - * Fetches content for some include using a provided fallback in case of an error. - */ -public interface ContentFetcher { - - CompletableFuture> fetch(String path, String fallback, CompositionStep step); - -} diff --git a/src/main/java/com/rewedigital/composer/composing/IncludeProcessor.java b/src/main/java/com/rewedigital/composer/composing/IncludeProcessor.java index 45ba720..56aa8a9 100755 --- a/src/main/java/com/rewedigital/composer/composing/IncludeProcessor.java +++ b/src/main/java/com/rewedigital/composer/composing/IncludeProcessor.java @@ -7,6 +7,8 @@ import java.util.stream.Collectors; import java.util.stream.Stream; +import com.rewedigital.composer.composing.fetch.ContentFetcher; + /** * Processes a list of includes by fetching the content for each include and building a composition object that * describes the composition of the template containing the includes and the content fetched. diff --git a/src/main/java/com/rewedigital/composer/composing/IncludedService.java b/src/main/java/com/rewedigital/composer/composing/IncludedService.java index dd33ee9..7cbf81c 100755 --- a/src/main/java/com/rewedigital/composer/composing/IncludedService.java +++ b/src/main/java/com/rewedigital/composer/composing/IncludedService.java @@ -1,5 +1,6 @@ package com.rewedigital.composer.composing; +import java.time.Duration; import java.util.HashMap; import java.util.Map; import java.util.concurrent.CompletableFuture; @@ -7,6 +8,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.rewedigital.composer.composing.fetch.ContentFetcher; +import com.rewedigital.composer.composing.fetch.FetchContext; import com.spotify.apollo.Response; /** @@ -90,7 +93,7 @@ private IncludedService(final Builder builder) { public CompletableFuture fetch(final ContentFetcher fetcher, final CompositionStep parentStep) { final CompositionStep step = parentStep.childWith(path()); - return fetcher.fetch(path(), fallback(), step) + return fetcher.fetch(FetchContext.of(path(), fallback(), ttl()), step) .thenApply(r -> new WithResponse(step, startOffset, endOffset, r)); } @@ -102,6 +105,28 @@ private String path() { return attributes.getOrDefault("path", ""); } + private Duration ttl() { + // FIXME: Initialise with global default... + final Duration defaultDuration = Duration.ofMillis(Long.MAX_VALUE); + + if (!attributes.containsKey("ttl")) { + return defaultDuration; + } + + // FIMXE: + final String unparsedTtl = attributes.get("ttl"); + long ttl = Long.MAX_VALUE; + try { + ttl = Long.parseLong(unparsedTtl); + } catch (final NumberFormatException nfEx) { + LOGGER.info( + "Not able to evaluate ttl for path {} with value {} falling back to the default of {}ms", + path(), unparsedTtl, defaultDuration); + } + + return Duration.ofMillis(ttl); + } + public boolean isInRage(final ContentRange contentRange) { return contentRange.isInRange(startOffset); } diff --git a/src/main/java/com/rewedigital/composer/composing/fetch/ContentFetcher.java b/src/main/java/com/rewedigital/composer/composing/fetch/ContentFetcher.java new file mode 100755 index 0000000..ed7ec99 --- /dev/null +++ b/src/main/java/com/rewedigital/composer/composing/fetch/ContentFetcher.java @@ -0,0 +1,15 @@ +package com.rewedigital.composer.composing.fetch; + +import java.util.concurrent.CompletableFuture; + +import com.rewedigital.composer.composing.CompositionStep; +import com.spotify.apollo.Response; + +/** + * Fetches content for some include using a provided fallback in case of an error. + */ +public interface ContentFetcher { + + CompletableFuture> fetch(FetchContext fetchContext, CompositionStep step); + +} diff --git a/src/main/java/com/rewedigital/composer/composing/fetch/FetchContext.java b/src/main/java/com/rewedigital/composer/composing/fetch/FetchContext.java new file mode 100644 index 0000000..0d4d87b --- /dev/null +++ b/src/main/java/com/rewedigital/composer/composing/fetch/FetchContext.java @@ -0,0 +1,45 @@ +package com.rewedigital.composer.composing.fetch; + +import static java.util.Objects.requireNonNull; + +import java.time.Duration; + +/** + * A simple parameter object for {@link ContentFetcher}s. + */ +public class FetchContext { + + private final String path; + private final String fallback; + private final Duration ttl; + + private FetchContext(final String path, final String fallback, final Duration ttl) { + this.path = path; + this.fallback = fallback; + this.ttl = requireNonNull(ttl); + } + + /** + * Builds a simple parameter object for {@link ContentFetcher}s. + * + * @param path to fetch from. + * @param fallback the fallback returned in case of an error. + * @param ttl how long the fetch should take. + * @return the parameter object. + */ + public static FetchContext of(final String path, final String fallback, final Duration ttl) { + return new FetchContext(path, fallback, ttl); + } + + public String path() { + return path; + } + + public String fallback() { + return fallback; + } + + public Duration ttl() { + return ttl; + } +} diff --git a/src/main/java/com/rewedigital/composer/composing/RecursionAwareContentFetcher.java b/src/main/java/com/rewedigital/composer/composing/fetch/RecursionAwareContentFetcher.java similarity index 58% rename from src/main/java/com/rewedigital/composer/composing/RecursionAwareContentFetcher.java rename to src/main/java/com/rewedigital/composer/composing/fetch/RecursionAwareContentFetcher.java index 5b07ed4..86c37ab 100644 --- a/src/main/java/com/rewedigital/composer/composing/RecursionAwareContentFetcher.java +++ b/src/main/java/com/rewedigital/composer/composing/fetch/RecursionAwareContentFetcher.java @@ -1,11 +1,13 @@ -package com.rewedigital.composer.composing; +package com.rewedigital.composer.composing.fetch; + +import static java.util.Objects.requireNonNull; -import java.util.Objects; import java.util.concurrent.CompletableFuture; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.rewedigital.composer.composing.CompositionStep; import com.spotify.apollo.Response; public class RecursionAwareContentFetcher implements ContentFetcher { @@ -16,18 +18,17 @@ public class RecursionAwareContentFetcher implements ContentFetcher { private final int maxRecursion; public RecursionAwareContentFetcher(final ContentFetcher contentFetcher, final int maxRecursion) { - this.contentFetcher = Objects.requireNonNull(contentFetcher); + this.contentFetcher = requireNonNull(contentFetcher); this.maxRecursion = maxRecursion; } @Override - public CompletableFuture> fetch(final String path, final String fallback, - final CompositionStep step) { + public CompletableFuture> fetch(final FetchContext context, final CompositionStep step) { if (maxRecursion < step.depth()) { - LOGGER.warn("Max recursion depth execeeded for " + step.callStack()); - return CompletableFuture.completedFuture(Response.forPayload(fallback)); + LOGGER.warn("Max recursion depth exceeded for " + step.callStack()); + return CompletableFuture.completedFuture(Response.forPayload(context.fallback())); } - return contentFetcher.fetch(path, fallback, step); + return contentFetcher.fetch(context, step); } } diff --git a/src/main/java/com/rewedigital/composer/composing/ValidatingContentFetcher.java b/src/main/java/com/rewedigital/composer/composing/fetch/ValidatingContentFetcher.java similarity index 70% rename from src/main/java/com/rewedigital/composer/composing/ValidatingContentFetcher.java rename to src/main/java/com/rewedigital/composer/composing/fetch/ValidatingContentFetcher.java index 1aefd6b..99870b5 100755 --- a/src/main/java/com/rewedigital/composer/composing/ValidatingContentFetcher.java +++ b/src/main/java/com/rewedigital/composer/composing/fetch/ValidatingContentFetcher.java @@ -1,14 +1,16 @@ -package com.rewedigital.composer.composing; +package com.rewedigital.composer.composing.fetch; import static java.util.Objects.requireNonNull; import java.util.Map; +import java.util.Objects; import java.util.concurrent.CompletableFuture; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import com.damnhandy.uri.template.UriTemplate; +import com.rewedigital.composer.composing.CompositionStep; import com.rewedigital.composer.session.SessionRoot; import com.spotify.apollo.Client; import com.spotify.apollo.Request; @@ -26,23 +28,26 @@ public class ValidatingContentFetcher implements ContentFetcher { public ValidatingContentFetcher(final Client client, final Map parsedPathArguments, final SessionRoot session) { - this.session = session; + this.session = requireNonNull(session); this.client = requireNonNull(client); this.parsedPathArguments = requireNonNull(parsedPathArguments); } @Override - public CompletableFuture> fetch(final String path, final String fallback, - final CompositionStep step) { - if (path == null || path.trim().isEmpty()) { + public CompletableFuture> fetch(final FetchContext context, final CompositionStep step) { + Objects.requireNonNull(context); + Objects.requireNonNull(step); + + if (context.path() == null || context.path().trim().isEmpty()) { LOGGER.warn("Empty path attribute in include found - callstack: " + step.callStack()); return CompletableFuture.completedFuture(Response.forPayload("")); } - final String expandedPath = UriTemplate.fromTemplate(path).expand(parsedPathArguments); - return client.send(session.enrich(Request.forUri(expandedPath, "GET"))) + final String expandedPath = UriTemplate.fromTemplate(context.path()).expand(parsedPathArguments); + final Request request = session.enrich(Request.forUri(expandedPath, "GET").withTtl(context.ttl())); + return client.send(request) .thenApply(response -> acceptHtmlOnly(response, expandedPath)) - .thenApply(r -> toStringPayload(r, fallback)) + .thenApply(r -> toStringPayload(r, context.fallback())) .toCompletableFuture(); } diff --git a/src/main/java/com/rewedigital/composer/routing/Match.java b/src/main/java/com/rewedigital/composer/routing/Match.java index 4048d7d..eba45b7 100644 --- a/src/main/java/com/rewedigital/composer/routing/Match.java +++ b/src/main/java/com/rewedigital/composer/routing/Match.java @@ -1,45 +1,54 @@ package com.rewedigital.composer.routing; +import java.time.Duration; import java.util.Objects; public class Match { private final String backend; + private final Duration ttl; private final RouteTypeName routeType; - private Match(final String backend, final RouteTypeName routeType) { + private Match(final String backend, final Duration ttl, final RouteTypeName routeType) { this.backend = backend; + this.ttl = ttl; this.routeType = routeType; } - public static Match of(final String backend, final RouteTypeName routeType) { - return new Match(backend, routeType); + public static Match of(final String backend, final Duration ttl, final RouteTypeName routeType) { + return new Match(backend, ttl, routeType); } public String backend() { return backend; } + public Duration ttl() { + return ttl; + } + public RouteType routeType(final RouteTypes routeTypes) { return routeType.from(routeTypes); } @Override public int hashCode() { - return Objects.hash(backend, routeType); + return Objects.hash(backend, ttl, routeType); } @Override public boolean equals(final Object obj) { - if (this == obj) + if (this == obj) { return true; - if (obj == null) + } + if (obj == null) { return false; - if (getClass() != obj.getClass()) + } + if (getClass() != obj.getClass()) { return false; + } final Match other = (Match) obj; - return Objects.equals(backend, other.backend) && routeType == other.routeType; + return Objects.equals(backend, other.backend) && routeType == other.routeType && ttl == other.ttl; } - } diff --git a/src/main/java/com/rewedigital/composer/routing/RouteMatch.java b/src/main/java/com/rewedigital/composer/routing/RouteMatch.java index ac19657..b778e5c 100644 --- a/src/main/java/com/rewedigital/composer/routing/RouteMatch.java +++ b/src/main/java/com/rewedigital/composer/routing/RouteMatch.java @@ -1,5 +1,8 @@ package com.rewedigital.composer.routing; +import static java.util.Objects.requireNonNull; + +import java.time.Duration; import java.util.Collections; import java.util.Map; @@ -11,7 +14,7 @@ public class RouteMatch { private final Map parsedPathArguments; public RouteMatch(final Match backend, final Map parsedPathArguments) { - this.backend = backend; + this.backend = requireNonNull(backend); this.parsedPathArguments = Collections.unmodifiableMap(parsedPathArguments); } @@ -19,6 +22,10 @@ public String backend() { return backend.backend(); } + public Duration ttl() { + return backend.ttl(); + } + public RouteType routeType(final RouteTypes routeTypes) { return backend.routeType(routeTypes); } @@ -35,4 +42,4 @@ public String toString() { public String expandedPath() { return UriTemplate.fromTemplate(backend.backend()).expand(parsedPathArguments); } -} \ No newline at end of file +} diff --git a/src/main/java/com/rewedigital/composer/routing/RoutingConfiguration.java b/src/main/java/com/rewedigital/composer/routing/RoutingConfiguration.java index be514ab..49da072 100644 --- a/src/main/java/com/rewedigital/composer/routing/RoutingConfiguration.java +++ b/src/main/java/com/rewedigital/composer/routing/RoutingConfiguration.java @@ -1,5 +1,6 @@ package com.rewedigital.composer.routing; +import java.time.Duration; import java.util.List; import java.util.stream.Collectors; @@ -35,9 +36,13 @@ private static Rule buildLocalRoute(final ConfigValue configValue) { final String type = config.getString("type"); final String target = config.getString("target"); - final Rule result = Rule.fromUri(path, method, Match.of(target, RouteTypeName.valueOf(type))); - LOGGER.info("Registered local route for path={}, method={}, target={}, type={}", path, method, - target, type); + // FIXME: Don't error out if none is configured use a sane default. + final Duration ttl = config.getDuration("ttl"); + + final Rule result = Rule.fromUri(path, method, Match.of(target, ttl, RouteTypeName.valueOf(type))); + LOGGER.info("Registered local route for path={}, method={}, target={}, type={} with a request ttl={}", path, + method, + target, type, ttl); return result; } diff --git a/src/main/java/com/rewedigital/composer/session/CookieBasedSessionHandler.java b/src/main/java/com/rewedigital/composer/session/CookieBasedSessionHandler.java index 7ede5a3..358a373 100644 --- a/src/main/java/com/rewedigital/composer/session/CookieBasedSessionHandler.java +++ b/src/main/java/com/rewedigital/composer/session/CookieBasedSessionHandler.java @@ -108,7 +108,7 @@ private List parseCookieHeader(final String cookieHeader) { try { return HttpCookie.parse(cookieHeader); } catch (final Exception e) { - LOGGER.warn("erro parsing cookie headers, defaulting to empty session", e); + LOGGER.warn("error parsing cookie headers, defaulting to empty session", e); return emptyList(); } } diff --git a/src/test/java/com/rewedigital/composer/composing/TemplateComposerTest.java b/src/test/java/com/rewedigital/composer/composing/TemplateComposerTest.java index 5fd9b4c..d14cca7 100644 --- a/src/test/java/com/rewedigital/composer/composing/TemplateComposerTest.java +++ b/src/test/java/com/rewedigital/composer/composing/TemplateComposerTest.java @@ -15,6 +15,7 @@ import org.junit.Test; +import com.rewedigital.composer.composing.fetch.ValidatingContentFetcher; import com.rewedigital.composer.session.SessionRoot; import com.spotify.apollo.Client; import com.spotify.apollo.Response; diff --git a/src/test/java/com/rewedigital/composer/composing/fetch/ValidatingContentFetcherTest.java b/src/test/java/com/rewedigital/composer/composing/fetch/ValidatingContentFetcherTest.java new file mode 100644 index 0000000..a5330b8 --- /dev/null +++ b/src/test/java/com/rewedigital/composer/composing/fetch/ValidatingContentFetcherTest.java @@ -0,0 +1,48 @@ +package com.rewedigital.composer.composing.fetch; + +import java.time.Duration; +import java.util.Collections; +import java.util.concurrent.CompletableFuture; + +import static org.assertj.core.api.Assertions.assertThat; +import org.junit.Test; + +import com.rewedigital.composer.composing.CompositionStep; +import com.rewedigital.composer.session.SessionRoot; +import com.spotify.apollo.Request; +import com.spotify.apollo.Response; +import com.spotify.apollo.test.StubClient; + +import okio.ByteString; + +public class ValidatingContentFetcherTest { + + private static final Duration TIME_OUT = Duration.ofMillis(222); + + @Test + public void validatingContentFetcherAppliesTtl() { + final StubClient client = aClient(); + + final ValidatingContentFetcher fetcher = + new ValidatingContentFetcher(client, Collections.emptyMap(), SessionRoot.empty()); + + final CompletableFuture> response = + fetcher.fetch(FetchContext.of("path", "fallback", TIME_OUT), CompositionStep.root("")); + + assertThat(response).isDone(); + assertThat(client.sentRequests()).isNotEmpty().allMatch(r -> ttlIsSet(r)); + } + + private boolean ttlIsSet(final Request r) { + return r.ttl().isPresent() && r.ttl().get() == TIME_OUT; + } + + private StubClient aClient() { + final StubClient client = new StubClient(); + final Response emptyResponse = + Response.forPayload(ByteString.EMPTY).withHeader("Content-Type", "text/html"); + client.respond(emptyResponse).to("path"); + return client; + } + +} diff --git a/src/test/java/com/rewedigital/composer/proxy/ValidatingContentFetcherTest.java b/src/test/java/com/rewedigital/composer/composing/fetch/ValidatingContentFetcherTest2.java similarity index 69% rename from src/test/java/com/rewedigital/composer/proxy/ValidatingContentFetcherTest.java rename to src/test/java/com/rewedigital/composer/composing/fetch/ValidatingContentFetcherTest2.java index e60b242..a4266cf 100644 --- a/src/test/java/com/rewedigital/composer/proxy/ValidatingContentFetcherTest.java +++ b/src/test/java/com/rewedigital/composer/composing/fetch/ValidatingContentFetcherTest2.java @@ -1,4 +1,4 @@ -package com.rewedigital.composer.proxy; +package com.rewedigital.composer.composing.fetch; import static java.util.Collections.emptyMap; import static java.util.concurrent.CompletableFuture.completedFuture; @@ -10,31 +10,36 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.time.Duration; import java.util.HashMap; import java.util.Map; import java.util.Optional; import java.util.concurrent.CompletionStage; - import org.junit.Test; import org.mockito.ArgumentMatcher; import com.rewedigital.composer.composing.CompositionStep; -import com.rewedigital.composer.composing.ValidatingContentFetcher; +import com.rewedigital.composer.composing.fetch.FetchContext; +import com.rewedigital.composer.composing.fetch.ValidatingContentFetcher; import com.rewedigital.composer.session.SessionRoot; import com.spotify.apollo.Client; import com.spotify.apollo.Request; import com.spotify.apollo.Response; +import com.spotify.apollo.test.StubClient; import okio.ByteString; -public class ValidatingContentFetcherTest { +public class ValidatingContentFetcherTest2 { private final SessionRoot emptySession = SessionRoot.empty(); + private final Duration timeOut = Duration.ofMillis(222); @Test public void fetchesEmptyContentForMissingPath() throws Exception { final Response result = - new ValidatingContentFetcher(mock(Client.class), emptyMap(), emptySession).fetch(null, null, aStep()).get(); + new ValidatingContentFetcher(mock(Client.class), emptyMap(), emptySession) + .fetch(FetchContext.of(null, null, timeOut), aStep()) + .get(); assertThat(result.payload()).contains(""); } @@ -43,7 +48,8 @@ public void fetchesContentFromPath() throws Exception { final Client client = mock(Client.class); when(client.send(aRequestWithPath("/some/path"))).thenReturn(aResponse("ok")); final Response result = - new ValidatingContentFetcher(client, emptyMap(), emptySession).fetch("/some/path", "fallback", aStep()) + new ValidatingContentFetcher(client, emptyMap(), emptySession) + .fetch(FetchContext.of("/some/path", "fallback", timeOut), aStep()) .get(); assertThat(result.payload()).contains("ok"); } @@ -54,7 +60,8 @@ public void expandsPathWithParameters() throws Exception { when(client.send(aRequestWithPath("/some/path/val"))).thenReturn(aResponse("ok")); final Response result = new ValidatingContentFetcher(client, params("var", "val"), emptySession) - .fetch("/some/path/{var}", "fallback", aStep()).get(); + .fetch(FetchContext.of("/some/path/{var}", "fallback", timeOut), aStep()) + .get(); assertThat(result.payload()).contains("ok"); } @@ -63,7 +70,9 @@ public void defaultsNonHTMLResponsesToEmpty() throws Exception { final Client client = mock(Client.class); when(client.send(aRequestWithPath("/some/path"))).thenReturn(aResponse("ok", "text/json")); final Response result = - new ValidatingContentFetcher(client, emptyMap(), emptySession).fetch("/some/path", "", aStep()).get(); + new ValidatingContentFetcher(client, emptyMap(), emptySession) + .fetch(FetchContext.of("/some/path", "", timeOut), aStep()) + .get(); assertThat(result.payload()).contains(""); } @@ -72,16 +81,42 @@ public void forwardsSession() throws Exception { final Client client = mock(Client.class); when(client.send(any())).thenReturn(aResponse("")); final SessionRoot session = session("x-rd-key", "value"); - new ValidatingContentFetcher(client, emptyMap(), session).fetch("/some/path", "", aStep()).get(); + new ValidatingContentFetcher(client, emptyMap(), session) + .fetch(FetchContext.of("/some/path", "", timeOut), aStep()) + .get(); verify(client).send(aRequestWithSession("x-rd-key", "value")); } + @Test + public void validatingContentFetcherAppliesTtl() throws Exception { + final StubClient client = aStubClient(); + + new ValidatingContentFetcher(client, emptyMap(), emptySession) + .fetch(FetchContext.of("path", "fallback", timeOut), aStep()) + .get(); + + assertThat(client.sentRequests()).isNotEmpty().allSatisfy(r -> ttlIsSet(r)); + } + + private void ttlIsSet(final Request request) { + assertThat(request.ttl()).isNotEmpty(); + assertThat(request.ttl().get()).isEqualTo(timeOut); + } + + private StubClient aStubClient() { + final StubClient client = new StubClient(); + final Response emptyResponse = + Response.forPayload(ByteString.EMPTY).withHeader("Content-Type", "text/html"); + client.respond(emptyResponse).to("path"); + return client; + } + private static Request aRequestWithPath(final String path) { return argThat(new ArgumentMatcher() { @Override public boolean matches(final Object argument) { - return (Request.class.isAssignableFrom(argument.getClass())) && + return Request.class.isAssignableFrom(argument.getClass()) && path.equals(((Request) argument).uri()); } }); diff --git a/src/test/java/com/rewedigital/composer/proxy/ComposingHandlerTest.java b/src/test/java/com/rewedigital/composer/proxy/ComposingRequestHandlerTest.java similarity index 94% rename from src/test/java/com/rewedigital/composer/proxy/ComposingHandlerTest.java rename to src/test/java/com/rewedigital/composer/proxy/ComposingRequestHandlerTest.java index f72d1af..651d05c 100644 --- a/src/test/java/com/rewedigital/composer/proxy/ComposingHandlerTest.java +++ b/src/test/java/com/rewedigital/composer/proxy/ComposingRequestHandlerTest.java @@ -8,6 +8,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import java.time.Duration; import java.util.Optional; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionStage; @@ -35,9 +36,10 @@ import okio.ByteString; -public class ComposingHandlerTest { +public class ComposingRequestHandlerTest { private static final String SERVICE_RESPONSE = "test"; + private final Duration ttl = Duration.ZERO; @Test public void returnsResponseFromTemplateRoute() throws Exception { @@ -75,7 +77,7 @@ public void returnsErrorResponseFromProxyRoute() throws Exception { } private BackendRouting aRouter(final String pattern, final RouteTypeName routeType) { - final Rule sampleRule = Rule.fromUri(pattern, "GET", Match.of("http://target", routeType)); + final Rule sampleRule = Rule.fromUri(pattern, "GET", Match.of("http://target", ttl, routeType)); return new BackendRouting(singletonList(sampleRule)); } @@ -93,13 +95,7 @@ private RequestContext aContext() { } private SessionHandlerFactory sessionSerializer() { - return new SessionHandlerFactory() { - - @Override - public SessionHandler build() { - return SessionHandler.noSession(); - } - }; + return () -> SessionHandler.noSession(); } private static class RoutingResult extends SessionAwareProxyClient { diff --git a/src/test/java/com/rewedigital/composer/routing/BackendRoutingTest.java b/src/test/java/com/rewedigital/composer/routing/BackendRoutingTest.java index 77981dc..796e842 100644 --- a/src/test/java/com/rewedigital/composer/routing/BackendRoutingTest.java +++ b/src/test/java/com/rewedigital/composer/routing/BackendRoutingTest.java @@ -5,6 +5,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import java.time.Duration; import java.util.Optional; import org.junit.Test; @@ -20,10 +21,11 @@ public class BackendRoutingTest { private final SessionRoot emptySession = SessionRoot.empty(); + private final Duration ttl = Duration.ZERO; @Test public void findsRouteForSimpleRule() { - final Rule simpleRule = Rule.fromUri("/", "GET", Match.of("http://test.com/", PROXY)); + final Rule simpleRule = Rule.fromUri("/", "GET", Match.of("http://test.com/", ttl, PROXY)); final BackendRouting backendRouting = new BackendRouting(ImmutableList.of(simpleRule)); final Optional matchResult = backendRouting.matches(requestFor("GET", "/"), emptySession); @@ -34,7 +36,7 @@ public void findsRouteForSimpleRule() { @Test public void findsRouteWithPathArguments() { final Rule ruleWithPath = - Rule.fromUri("/", "GET", Match.of("http://test.com/{someValue}", PROXY)); + Rule.fromUri("/", "GET", Match.of("http://test.com/{someValue}", ttl, PROXY)); final BackendRouting backendRouting = new BackendRouting(ImmutableList.of(ruleWithPath)); final Optional matchResult = backendRouting.matches(requestFor("GET", "/123"), emptySession); @@ -45,7 +47,7 @@ public void findsRouteWithPathArguments() { @Test public void findsNoRouteThatIsNotConfigured() { - final Rule simpleRule = Rule.fromUri("/", "GET", Match.of("http://test.com/", PROXY)); + final Rule simpleRule = Rule.fromUri("/", "GET", Match.of("http://test.com/", ttl, PROXY)); final BackendRouting backendRouting = new BackendRouting(ImmutableList.of(simpleRule)); final Optional matchResult = backendRouting.matches(requestFor("PUT", "/"), emptySession); diff --git a/src/test/java/com/rewedigital/composer/routing/RoutingConfigurationTest.java b/src/test/java/com/rewedigital/composer/routing/RoutingConfigurationTest.java index 642b911..786c553 100644 --- a/src/test/java/com/rewedigital/composer/routing/RoutingConfigurationTest.java +++ b/src/test/java/com/rewedigital/composer/routing/RoutingConfigurationTest.java @@ -2,6 +2,7 @@ import static org.assertj.core.api.Assertions.assertThat; +import java.time.Duration; import java.util.Arrays; import java.util.HashMap; import java.util.List; @@ -19,6 +20,8 @@ public class RoutingConfigurationTest { + private final Duration ttl = Duration.ZERO; + @Test public void allConfigParametersAreCoveredByDefaultConfig() { final RoutingConfiguration configuration = @@ -35,7 +38,7 @@ public void createsLocalRouteFromConfiguration() { assertThat(localRules).anySatisfy(rule -> { assertThat(rule.getPath()).isEqualTo("/test/path/"); assertThat(rule.getMethods()).contains("GET"); - assertThat(rule.getTarget()).isEqualTo(Match.of("https://target.service/{arg}", RouteTypeName.PROXY)); + assertThat(rule.getTarget()).isEqualTo(Match.of("https://target.service/{arg}", ttl, RouteTypeName.PROXY)); }); } From cf4557b1fa77f2e4cab493a07b0afeee65d0a1c7 Mon Sep 17 00:00:00 2001 From: Andreas Kluth Date: Sun, 11 Mar 2018 00:32:53 +0100 Subject: [PATCH 2/6] Removed duplicate test. --- .../fetch/ValidatingContentFetcherTest.java | 142 ++++++++++++++-- .../fetch/ValidatingContentFetcherTest2.java | 158 ------------------ 2 files changed, 126 insertions(+), 174 deletions(-) delete mode 100644 src/test/java/com/rewedigital/composer/composing/fetch/ValidatingContentFetcherTest2.java diff --git a/src/test/java/com/rewedigital/composer/composing/fetch/ValidatingContentFetcherTest.java b/src/test/java/com/rewedigital/composer/composing/fetch/ValidatingContentFetcherTest.java index a5330b8..ab5bc55 100644 --- a/src/test/java/com/rewedigital/composer/composing/fetch/ValidatingContentFetcherTest.java +++ b/src/test/java/com/rewedigital/composer/composing/fetch/ValidatingContentFetcherTest.java @@ -1,14 +1,28 @@ package com.rewedigital.composer.composing.fetch; -import java.time.Duration; -import java.util.Collections; -import java.util.concurrent.CompletableFuture; - +import static java.util.Collections.emptyMap; +import static java.util.concurrent.CompletableFuture.completedFuture; +import static okio.ByteString.encodeUtf8; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.argThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.time.Duration; +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.CompletionStage; import org.junit.Test; +import org.mockito.ArgumentMatcher; import com.rewedigital.composer.composing.CompositionStep; +import com.rewedigital.composer.composing.fetch.FetchContext; +import com.rewedigital.composer.composing.fetch.ValidatingContentFetcher; import com.rewedigital.composer.session.SessionRoot; +import com.spotify.apollo.Client; import com.spotify.apollo.Request; import com.spotify.apollo.Response; import com.spotify.apollo.test.StubClient; @@ -17,27 +31,79 @@ public class ValidatingContentFetcherTest { - private static final Duration TIME_OUT = Duration.ofMillis(222); + private final SessionRoot emptySession = SessionRoot.empty(); + private final Duration timeOut = Duration.ofMillis(222); @Test - public void validatingContentFetcherAppliesTtl() { - final StubClient client = aClient(); + public void fetchesEmptyContentForMissingPath() throws Exception { + final Response result = + new ValidatingContentFetcher(mock(Client.class), emptyMap(), emptySession) + .fetch(FetchContext.of(null, null, timeOut), aStep()) + .get(); + assertThat(result.payload()).contains(""); + } - final ValidatingContentFetcher fetcher = - new ValidatingContentFetcher(client, Collections.emptyMap(), SessionRoot.empty()); + @Test + public void fetchesContentFromPath() throws Exception { + final Client client = mock(Client.class); + when(client.send(aRequestWithPath("/some/path"))).thenReturn(aResponse("ok")); + final Response result = + new ValidatingContentFetcher(client, emptyMap(), emptySession) + .fetch(FetchContext.of("/some/path", "fallback", timeOut), aStep()) + .get(); + assertThat(result.payload()).contains("ok"); + } - final CompletableFuture> response = - fetcher.fetch(FetchContext.of("path", "fallback", TIME_OUT), CompositionStep.root("")); + @Test + public void expandsPathWithParameters() throws Exception { + final Client client = mock(Client.class); + when(client.send(aRequestWithPath("/some/path/val"))).thenReturn(aResponse("ok")); + final Response result = + new ValidatingContentFetcher(client, params("var", "val"), emptySession) + .fetch(FetchContext.of("/some/path/{var}", "fallback", timeOut), aStep()) + .get(); + assertThat(result.payload()).contains("ok"); + } - assertThat(response).isDone(); - assertThat(client.sentRequests()).isNotEmpty().allMatch(r -> ttlIsSet(r)); + @Test + public void defaultsNonHTMLResponsesToEmpty() throws Exception { + final Client client = mock(Client.class); + when(client.send(aRequestWithPath("/some/path"))).thenReturn(aResponse("ok", "text/json")); + final Response result = + new ValidatingContentFetcher(client, emptyMap(), emptySession) + .fetch(FetchContext.of("/some/path", "", timeOut), aStep()) + .get(); + assertThat(result.payload()).contains(""); + } + + @Test + public void forwardsSession() throws Exception { + final Client client = mock(Client.class); + when(client.send(any())).thenReturn(aResponse("")); + final SessionRoot session = session("x-rd-key", "value"); + new ValidatingContentFetcher(client, emptyMap(), session) + .fetch(FetchContext.of("/some/path", "", timeOut), aStep()) + .get(); + verify(client).send(aRequestWithSession("x-rd-key", "value")); } - private boolean ttlIsSet(final Request r) { - return r.ttl().isPresent() && r.ttl().get() == TIME_OUT; + @Test + public void validatingContentFetcherAppliesTtl() throws Exception { + final StubClient client = aStubClient(); + + new ValidatingContentFetcher(client, emptyMap(), emptySession) + .fetch(FetchContext.of("path", "fallback", timeOut), aStep()) + .get(); + + assertThat(client.sentRequests()).isNotEmpty().allSatisfy(r -> ttlIsSet(r)); } - private StubClient aClient() { + private void ttlIsSet(final Request request) { + assertThat(request.ttl()).isNotEmpty(); + assertThat(request.ttl().get()).isEqualTo(timeOut); + } + + private StubClient aStubClient() { final StubClient client = new StubClient(); final Response emptyResponse = Response.forPayload(ByteString.EMPTY).withHeader("Content-Type", "text/html"); @@ -45,4 +111,48 @@ private StubClient aClient() { return client; } + private static Request aRequestWithPath(final String path) { + return argThat(new ArgumentMatcher() { + + @Override + public boolean matches(final Object argument) { + return Request.class.isAssignableFrom(argument.getClass()) && + path.equals(((Request) argument).uri()); + } + }); + } + + private static Request aRequestWithSession(final String key, final String value) { + return argThat(new ArgumentMatcher() { + + @Override + public boolean matches(final Object argument) { + return Request.class.isAssignableFrom(argument.getClass()) + && ((Request) argument).header(key).equals(Optional.of(value)); + } + }); + } + + private static CompletionStage> aResponse(final String payload) { + return aResponse(payload, "text/html"); + } + + private static CompletionStage> aResponse(final String payload, final String contentType) { + return completedFuture( + Response.forPayload(encodeUtf8(payload)).withHeader("Content-Type", contentType)); + } + + private static CompositionStep aStep() { + return CompositionStep.root("some-path"); + } + + private static SessionRoot session(final String key, final String value) { + return SessionRoot.of(params(key, value)); + } + + private static Map params(final String name, final T value) { + final Map params = new HashMap<>(); + params.put(name, value); + return params; + } } diff --git a/src/test/java/com/rewedigital/composer/composing/fetch/ValidatingContentFetcherTest2.java b/src/test/java/com/rewedigital/composer/composing/fetch/ValidatingContentFetcherTest2.java deleted file mode 100644 index a4266cf..0000000 --- a/src/test/java/com/rewedigital/composer/composing/fetch/ValidatingContentFetcherTest2.java +++ /dev/null @@ -1,158 +0,0 @@ -package com.rewedigital.composer.composing.fetch; - -import static java.util.Collections.emptyMap; -import static java.util.concurrent.CompletableFuture.completedFuture; -import static okio.ByteString.encodeUtf8; -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Matchers.any; -import static org.mockito.Matchers.argThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -import java.time.Duration; -import java.util.HashMap; -import java.util.Map; -import java.util.Optional; -import java.util.concurrent.CompletionStage; -import org.junit.Test; -import org.mockito.ArgumentMatcher; - -import com.rewedigital.composer.composing.CompositionStep; -import com.rewedigital.composer.composing.fetch.FetchContext; -import com.rewedigital.composer.composing.fetch.ValidatingContentFetcher; -import com.rewedigital.composer.session.SessionRoot; -import com.spotify.apollo.Client; -import com.spotify.apollo.Request; -import com.spotify.apollo.Response; -import com.spotify.apollo.test.StubClient; - -import okio.ByteString; - -public class ValidatingContentFetcherTest2 { - - private final SessionRoot emptySession = SessionRoot.empty(); - private final Duration timeOut = Duration.ofMillis(222); - - @Test - public void fetchesEmptyContentForMissingPath() throws Exception { - final Response result = - new ValidatingContentFetcher(mock(Client.class), emptyMap(), emptySession) - .fetch(FetchContext.of(null, null, timeOut), aStep()) - .get(); - assertThat(result.payload()).contains(""); - } - - @Test - public void fetchesContentFromPath() throws Exception { - final Client client = mock(Client.class); - when(client.send(aRequestWithPath("/some/path"))).thenReturn(aResponse("ok")); - final Response result = - new ValidatingContentFetcher(client, emptyMap(), emptySession) - .fetch(FetchContext.of("/some/path", "fallback", timeOut), aStep()) - .get(); - assertThat(result.payload()).contains("ok"); - } - - @Test - public void expandsPathWithParameters() throws Exception { - final Client client = mock(Client.class); - when(client.send(aRequestWithPath("/some/path/val"))).thenReturn(aResponse("ok")); - final Response result = - new ValidatingContentFetcher(client, params("var", "val"), emptySession) - .fetch(FetchContext.of("/some/path/{var}", "fallback", timeOut), aStep()) - .get(); - assertThat(result.payload()).contains("ok"); - } - - @Test - public void defaultsNonHTMLResponsesToEmpty() throws Exception { - final Client client = mock(Client.class); - when(client.send(aRequestWithPath("/some/path"))).thenReturn(aResponse("ok", "text/json")); - final Response result = - new ValidatingContentFetcher(client, emptyMap(), emptySession) - .fetch(FetchContext.of("/some/path", "", timeOut), aStep()) - .get(); - assertThat(result.payload()).contains(""); - } - - @Test - public void forwardsSession() throws Exception { - final Client client = mock(Client.class); - when(client.send(any())).thenReturn(aResponse("")); - final SessionRoot session = session("x-rd-key", "value"); - new ValidatingContentFetcher(client, emptyMap(), session) - .fetch(FetchContext.of("/some/path", "", timeOut), aStep()) - .get(); - verify(client).send(aRequestWithSession("x-rd-key", "value")); - } - - @Test - public void validatingContentFetcherAppliesTtl() throws Exception { - final StubClient client = aStubClient(); - - new ValidatingContentFetcher(client, emptyMap(), emptySession) - .fetch(FetchContext.of("path", "fallback", timeOut), aStep()) - .get(); - - assertThat(client.sentRequests()).isNotEmpty().allSatisfy(r -> ttlIsSet(r)); - } - - private void ttlIsSet(final Request request) { - assertThat(request.ttl()).isNotEmpty(); - assertThat(request.ttl().get()).isEqualTo(timeOut); - } - - private StubClient aStubClient() { - final StubClient client = new StubClient(); - final Response emptyResponse = - Response.forPayload(ByteString.EMPTY).withHeader("Content-Type", "text/html"); - client.respond(emptyResponse).to("path"); - return client; - } - - private static Request aRequestWithPath(final String path) { - return argThat(new ArgumentMatcher() { - - @Override - public boolean matches(final Object argument) { - return Request.class.isAssignableFrom(argument.getClass()) && - path.equals(((Request) argument).uri()); - } - }); - } - - private static Request aRequestWithSession(final String key, final String value) { - return argThat(new ArgumentMatcher() { - - @Override - public boolean matches(final Object argument) { - return Request.class.isAssignableFrom(argument.getClass()) - && ((Request) argument).header(key).equals(Optional.of(value)); - } - }); - } - - private static CompletionStage> aResponse(final String payload) { - return aResponse(payload, "text/html"); - } - - private static CompletionStage> aResponse(final String payload, final String contentType) { - return completedFuture( - Response.forPayload(encodeUtf8(payload)).withHeader("Content-Type", contentType)); - } - - private static CompositionStep aStep() { - return CompositionStep.root("some-path"); - } - - private static SessionRoot session(final String key, final String value) { - return SessionRoot.of(params(key, value)); - } - - private static Map params(final String name, final T value) { - final Map params = new HashMap<>(); - params.put(name, value); - return params; - } -} From dd15e6ef4bf0b0c4390e84e706727940d368fee6 Mon Sep 17 00:00:00 2001 From: Andreas Kluth Date: Mon, 12 Mar 2018 00:00:49 +0100 Subject: [PATCH 3/6] Support ttl on a request basis. --- .../composing/AttoParserBasedComposer.java | 2 - .../composer/composing/ComposerFactory.java | 1 - .../composing/{fetch => }/ContentFetcher.java | 3 +- .../composing/{fetch => }/FetchContext.java | 11 +++--- .../composer/composing/IncludeProcessor.java | 2 - .../composer/composing/IncludedService.java | 38 ++++++++----------- .../RecursionAwareContentFetcher.java | 3 +- .../{fetch => }/ValidatingContentFetcher.java | 14 +++++-- .../rewedigital/composer/routing/Match.java | 9 +++-- .../composer/routing/RouteMatch.java | 3 +- .../routing/RoutingConfiguration.java | 19 +++++----- .../session/LocalSessionIdInterceptor.java | 2 +- src/main/resources/composer.conf | 4 ++ .../composing/TemplateComposerTest.java | 1 - .../fetch/ValidatingContentFetcherTest.java | 25 ++++++------ .../proxy/ComposingRequestHandlerTest.java | 4 +- .../composer/routing/BackendRoutingTest.java | 8 ++-- .../routing/RoutingConfigurationTest.java | 35 ++++++++++++++--- 18 files changed, 105 insertions(+), 79 deletions(-) rename src/main/java/com/rewedigital/composer/composing/{fetch => }/ContentFetcher.java (71%) rename src/main/java/com/rewedigital/composer/composing/{fetch => }/FetchContext.java (80%) rename src/main/java/com/rewedigital/composer/composing/{fetch => }/RecursionAwareContentFetcher.java (90%) rename src/main/java/com/rewedigital/composer/composing/{fetch => }/ValidatingContentFetcher.java (85%) diff --git a/src/main/java/com/rewedigital/composer/composing/AttoParserBasedComposer.java b/src/main/java/com/rewedigital/composer/composing/AttoParserBasedComposer.java index b1539af..c2069c5 100755 --- a/src/main/java/com/rewedigital/composer/composing/AttoParserBasedComposer.java +++ b/src/main/java/com/rewedigital/composer/composing/AttoParserBasedComposer.java @@ -4,8 +4,6 @@ import java.util.concurrent.CompletableFuture; -import com.rewedigital.composer.composing.fetch.ContentFetcher; -import com.rewedigital.composer.composing.fetch.RecursionAwareContentFetcher; import com.rewedigital.composer.parser.Parser; import com.rewedigital.composer.session.ResponseWithSession; import com.rewedigital.composer.session.SessionFragment; diff --git a/src/main/java/com/rewedigital/composer/composing/ComposerFactory.java b/src/main/java/com/rewedigital/composer/composing/ComposerFactory.java index b347aea..9fa8259 100644 --- a/src/main/java/com/rewedigital/composer/composing/ComposerFactory.java +++ b/src/main/java/com/rewedigital/composer/composing/ComposerFactory.java @@ -2,7 +2,6 @@ import java.util.Map; -import com.rewedigital.composer.composing.fetch.ValidatingContentFetcher; import com.rewedigital.composer.session.SessionRoot; import com.spotify.apollo.Client; import com.typesafe.config.Config; diff --git a/src/main/java/com/rewedigital/composer/composing/fetch/ContentFetcher.java b/src/main/java/com/rewedigital/composer/composing/ContentFetcher.java similarity index 71% rename from src/main/java/com/rewedigital/composer/composing/fetch/ContentFetcher.java rename to src/main/java/com/rewedigital/composer/composing/ContentFetcher.java index ed7ec99..ff0b1b7 100755 --- a/src/main/java/com/rewedigital/composer/composing/fetch/ContentFetcher.java +++ b/src/main/java/com/rewedigital/composer/composing/ContentFetcher.java @@ -1,8 +1,7 @@ -package com.rewedigital.composer.composing.fetch; +package com.rewedigital.composer.composing; import java.util.concurrent.CompletableFuture; -import com.rewedigital.composer.composing.CompositionStep; import com.spotify.apollo.Response; /** diff --git a/src/main/java/com/rewedigital/composer/composing/fetch/FetchContext.java b/src/main/java/com/rewedigital/composer/composing/FetchContext.java similarity index 80% rename from src/main/java/com/rewedigital/composer/composing/fetch/FetchContext.java rename to src/main/java/com/rewedigital/composer/composing/FetchContext.java index 0d4d87b..4729ad8 100644 --- a/src/main/java/com/rewedigital/composer/composing/fetch/FetchContext.java +++ b/src/main/java/com/rewedigital/composer/composing/FetchContext.java @@ -1,8 +1,9 @@ -package com.rewedigital.composer.composing.fetch; +package com.rewedigital.composer.composing; import static java.util.Objects.requireNonNull; import java.time.Duration; +import java.util.Optional; /** * A simple parameter object for {@link ContentFetcher}s. @@ -11,9 +12,9 @@ public class FetchContext { private final String path; private final String fallback; - private final Duration ttl; + private final Optional ttl; - private FetchContext(final String path, final String fallback, final Duration ttl) { + private FetchContext(final String path, final String fallback, final Optional ttl) { this.path = path; this.fallback = fallback; this.ttl = requireNonNull(ttl); @@ -27,7 +28,7 @@ private FetchContext(final String path, final String fallback, final Duration tt * @param ttl how long the fetch should take. * @return the parameter object. */ - public static FetchContext of(final String path, final String fallback, final Duration ttl) { + public static FetchContext of(final String path, final String fallback, final Optional ttl) { return new FetchContext(path, fallback, ttl); } @@ -39,7 +40,7 @@ public String fallback() { return fallback; } - public Duration ttl() { + public Optional ttl() { return ttl; } } diff --git a/src/main/java/com/rewedigital/composer/composing/IncludeProcessor.java b/src/main/java/com/rewedigital/composer/composing/IncludeProcessor.java index 56aa8a9..45ba720 100755 --- a/src/main/java/com/rewedigital/composer/composing/IncludeProcessor.java +++ b/src/main/java/com/rewedigital/composer/composing/IncludeProcessor.java @@ -7,8 +7,6 @@ import java.util.stream.Collectors; import java.util.stream.Stream; -import com.rewedigital.composer.composing.fetch.ContentFetcher; - /** * Processes a list of includes by fetching the content for each include and building a composition object that * describes the composition of the template containing the includes and the content fetched. diff --git a/src/main/java/com/rewedigital/composer/composing/IncludedService.java b/src/main/java/com/rewedigital/composer/composing/IncludedService.java index 7cbf81c..45b0bcf 100755 --- a/src/main/java/com/rewedigital/composer/composing/IncludedService.java +++ b/src/main/java/com/rewedigital/composer/composing/IncludedService.java @@ -3,13 +3,12 @@ import java.time.Duration; import java.util.HashMap; import java.util.Map; +import java.util.Optional; import java.util.concurrent.CompletableFuture; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.rewedigital.composer.composing.fetch.ContentFetcher; -import com.rewedigital.composer.composing.fetch.FetchContext; import com.spotify.apollo.Response; /** @@ -89,7 +88,6 @@ private IncludedService(final Builder builder) { this.fallback = builder.fallback; } - public CompletableFuture fetch(final ContentFetcher fetcher, final CompositionStep parentStep) { final CompositionStep step = parentStep.childWith(path()); @@ -97,6 +95,10 @@ public CompletableFuture fetch(final ContentFetche .thenApply(r -> new WithResponse(step, startOffset, endOffset, r)); } + public boolean isInRage(final ContentRange contentRange) { + return contentRange.isInRange(startOffset); + } + private String fallback() { return fallback; } @@ -105,30 +107,22 @@ private String path() { return attributes.getOrDefault("path", ""); } - private Duration ttl() { - // FIXME: Initialise with global default... - final Duration defaultDuration = Duration.ofMillis(Long.MAX_VALUE); + private Optional ttl() { + return longFromMap("ttl").map(Duration::ofMillis); + } - if (!attributes.containsKey("ttl")) { - return defaultDuration; - } - // FIMXE: - final String unparsedTtl = attributes.get("ttl"); - long ttl = Long.MAX_VALUE; + private Optional longFromMap(final String name) { + if (!attributes.containsKey(name)) { + return Optional.empty(); + } + final String unparsedValue = attributes.get(name); try { - ttl = Long.parseLong(unparsedTtl); + return Optional.of(Long.parseLong(unparsedValue)); } catch (final NumberFormatException nfEx) { - LOGGER.info( - "Not able to evaluate ttl for path {} with value {} falling back to the default of {}ms", - path(), unparsedTtl, defaultDuration); + LOGGER.info("Not able to evaluate {} for path {} with value {}.", name, path(), unparsedValue); } - - return Duration.ofMillis(ttl); - } - - public boolean isInRage(final ContentRange contentRange) { - return contentRange.isInRange(startOffset); + return Optional.empty(); } } diff --git a/src/main/java/com/rewedigital/composer/composing/fetch/RecursionAwareContentFetcher.java b/src/main/java/com/rewedigital/composer/composing/RecursionAwareContentFetcher.java similarity index 90% rename from src/main/java/com/rewedigital/composer/composing/fetch/RecursionAwareContentFetcher.java rename to src/main/java/com/rewedigital/composer/composing/RecursionAwareContentFetcher.java index 86c37ab..b26bbf7 100644 --- a/src/main/java/com/rewedigital/composer/composing/fetch/RecursionAwareContentFetcher.java +++ b/src/main/java/com/rewedigital/composer/composing/RecursionAwareContentFetcher.java @@ -1,4 +1,4 @@ -package com.rewedigital.composer.composing.fetch; +package com.rewedigital.composer.composing; import static java.util.Objects.requireNonNull; @@ -7,7 +7,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.rewedigital.composer.composing.CompositionStep; import com.spotify.apollo.Response; public class RecursionAwareContentFetcher implements ContentFetcher { diff --git a/src/main/java/com/rewedigital/composer/composing/fetch/ValidatingContentFetcher.java b/src/main/java/com/rewedigital/composer/composing/ValidatingContentFetcher.java similarity index 85% rename from src/main/java/com/rewedigital/composer/composing/fetch/ValidatingContentFetcher.java rename to src/main/java/com/rewedigital/composer/composing/ValidatingContentFetcher.java index 99870b5..f903225 100755 --- a/src/main/java/com/rewedigital/composer/composing/fetch/ValidatingContentFetcher.java +++ b/src/main/java/com/rewedigital/composer/composing/ValidatingContentFetcher.java @@ -1,4 +1,4 @@ -package com.rewedigital.composer.composing.fetch; +package com.rewedigital.composer.composing; import static java.util.Objects.requireNonNull; @@ -10,7 +10,6 @@ import org.slf4j.LoggerFactory; import com.damnhandy.uri.template.UriTemplate; -import com.rewedigital.composer.composing.CompositionStep; import com.rewedigital.composer.session.SessionRoot; import com.spotify.apollo.Client; import com.spotify.apollo.Request; @@ -44,13 +43,22 @@ public CompletableFuture> fetch(final FetchContext context, fin } final String expandedPath = UriTemplate.fromTemplate(context.path()).expand(parsedPathArguments); - final Request request = session.enrich(Request.forUri(expandedPath, "GET").withTtl(context.ttl())); + final Request request = session.enrich(withTtl(Request.forUri(expandedPath, "GET"), context)); + + return client.send(request) .thenApply(response -> acceptHtmlOnly(response, expandedPath)) .thenApply(r -> toStringPayload(r, context.fallback())) .toCompletableFuture(); } + private Request withTtl(final Request request, final FetchContext context) { + if (context.ttl().isPresent()) { + return request.withTtl(context.ttl().get()); + } + return request; + } + private Response toStringPayload(final Response response, final String fallback) { final String value = response.payload().map(p -> p.utf8()).orElse(fallback); return response.withPayload(value); diff --git a/src/main/java/com/rewedigital/composer/routing/Match.java b/src/main/java/com/rewedigital/composer/routing/Match.java index eba45b7..3a66d44 100644 --- a/src/main/java/com/rewedigital/composer/routing/Match.java +++ b/src/main/java/com/rewedigital/composer/routing/Match.java @@ -2,20 +2,21 @@ import java.time.Duration; import java.util.Objects; +import java.util.Optional; public class Match { private final String backend; - private final Duration ttl; + private final Optional ttl; private final RouteTypeName routeType; - private Match(final String backend, final Duration ttl, final RouteTypeName routeType) { + private Match(final String backend, final Optional ttl, final RouteTypeName routeType) { this.backend = backend; this.ttl = ttl; this.routeType = routeType; } - public static Match of(final String backend, final Duration ttl, final RouteTypeName routeType) { + public static Match of(final String backend, final Optional ttl, final RouteTypeName routeType) { return new Match(backend, ttl, routeType); } @@ -23,7 +24,7 @@ public String backend() { return backend; } - public Duration ttl() { + public Optional ttl() { return ttl; } diff --git a/src/main/java/com/rewedigital/composer/routing/RouteMatch.java b/src/main/java/com/rewedigital/composer/routing/RouteMatch.java index b778e5c..fb1e645 100644 --- a/src/main/java/com/rewedigital/composer/routing/RouteMatch.java +++ b/src/main/java/com/rewedigital/composer/routing/RouteMatch.java @@ -5,6 +5,7 @@ import java.time.Duration; import java.util.Collections; import java.util.Map; +import java.util.Optional; import com.damnhandy.uri.template.UriTemplate; @@ -22,7 +23,7 @@ public String backend() { return backend.backend(); } - public Duration ttl() { + public Optional ttl() { return backend.ttl(); } diff --git a/src/main/java/com/rewedigital/composer/routing/RoutingConfiguration.java b/src/main/java/com/rewedigital/composer/routing/RoutingConfiguration.java index 49da072..02e783e 100644 --- a/src/main/java/com/rewedigital/composer/routing/RoutingConfiguration.java +++ b/src/main/java/com/rewedigital/composer/routing/RoutingConfiguration.java @@ -1,9 +1,10 @@ package com.rewedigital.composer.routing; +import static java.util.stream.Collectors.toList; + import java.time.Duration; import java.util.List; -import java.util.stream.Collectors; - +import java.util.Optional; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -26,7 +27,7 @@ public static RoutingConfiguration fromConfig(final Config config) { } private static List> buildLocalRoutes(final ConfigList routesConfig) { - return routesConfig.stream().map(RoutingConfiguration::buildLocalRoute).collect(Collectors.toList()); + return routesConfig.stream().map(RoutingConfiguration::buildLocalRoute).collect(toList()); } private static Rule buildLocalRoute(final ConfigValue configValue) { @@ -35,20 +36,20 @@ private static Rule buildLocalRoute(final ConfigValue configValue) { final String method = config.getString("method"); final String type = config.getString("type"); final String target = config.getString("target"); - - // FIXME: Don't error out if none is configured use a sane default. - final Duration ttl = config.getDuration("ttl"); + final Optional ttl = optionalDuration(config, "ttl"); final Rule result = Rule.fromUri(path, method, Match.of(target, ttl, RouteTypeName.valueOf(type))); LOGGER.info("Registered local route for path={}, method={}, target={}, type={} with a request ttl={}", path, - method, - target, type, ttl); + method,target, type, ttl); return result; - } public List> localRules() { return localRoutes; } + private static Optional optionalDuration(final Config config, final String path) { + return config.hasPath(path) ? Optional.of(config.getDuration(path)) : Optional.empty(); + } + } diff --git a/src/main/java/com/rewedigital/composer/session/LocalSessionIdInterceptor.java b/src/main/java/com/rewedigital/composer/session/LocalSessionIdInterceptor.java index d00b754..a259622 100644 --- a/src/main/java/com/rewedigital/composer/session/LocalSessionIdInterceptor.java +++ b/src/main/java/com/rewedigital/composer/session/LocalSessionIdInterceptor.java @@ -74,7 +74,7 @@ private long parseExpiresAt(final String s) { try { return Long.parseLong(s); } catch (final NumberFormatException ex) { - LOGGER.error("maleformatted expires-at: {} - expireing session", s, ex); + LOGGER.error("Malformatted expires-at: {} - expiring session", s, ex); return -1; } } diff --git a/src/main/resources/composer.conf b/src/main/resources/composer.conf index 590ebad..6971bb2 100644 --- a/src/main/resources/composer.conf +++ b/src/main/resources/composer.conf @@ -4,6 +4,10 @@ http.server.port = 8000 http.server.port = ${?HTTP_PORT} +# http client configuration, timeouts are in ms +http.client.connectTimeout = 2000 +http.client.readTimeout = 5000 +http.client.writeTimeout = 5000 # parser configuration composer.html.include-tag = rewe-digital-include diff --git a/src/test/java/com/rewedigital/composer/composing/TemplateComposerTest.java b/src/test/java/com/rewedigital/composer/composing/TemplateComposerTest.java index d14cca7..5fd9b4c 100644 --- a/src/test/java/com/rewedigital/composer/composing/TemplateComposerTest.java +++ b/src/test/java/com/rewedigital/composer/composing/TemplateComposerTest.java @@ -15,7 +15,6 @@ import org.junit.Test; -import com.rewedigital.composer.composing.fetch.ValidatingContentFetcher; import com.rewedigital.composer.session.SessionRoot; import com.spotify.apollo.Client; import com.spotify.apollo.Response; diff --git a/src/test/java/com/rewedigital/composer/composing/fetch/ValidatingContentFetcherTest.java b/src/test/java/com/rewedigital/composer/composing/fetch/ValidatingContentFetcherTest.java index ab5bc55..e132488 100644 --- a/src/test/java/com/rewedigital/composer/composing/fetch/ValidatingContentFetcherTest.java +++ b/src/test/java/com/rewedigital/composer/composing/fetch/ValidatingContentFetcherTest.java @@ -19,8 +19,8 @@ import org.mockito.ArgumentMatcher; import com.rewedigital.composer.composing.CompositionStep; -import com.rewedigital.composer.composing.fetch.FetchContext; -import com.rewedigital.composer.composing.fetch.ValidatingContentFetcher; +import com.rewedigital.composer.composing.FetchContext; +import com.rewedigital.composer.composing.ValidatingContentFetcher; import com.rewedigital.composer.session.SessionRoot; import com.spotify.apollo.Client; import com.spotify.apollo.Request; @@ -32,13 +32,13 @@ public class ValidatingContentFetcherTest { private final SessionRoot emptySession = SessionRoot.empty(); - private final Duration timeOut = Duration.ofMillis(222); + private final Optional defaultTimeout = Optional.empty(); @Test public void fetchesEmptyContentForMissingPath() throws Exception { final Response result = new ValidatingContentFetcher(mock(Client.class), emptyMap(), emptySession) - .fetch(FetchContext.of(null, null, timeOut), aStep()) + .fetch(FetchContext.of(null, null, defaultTimeout), aStep()) .get(); assertThat(result.payload()).contains(""); } @@ -49,7 +49,7 @@ public void fetchesContentFromPath() throws Exception { when(client.send(aRequestWithPath("/some/path"))).thenReturn(aResponse("ok")); final Response result = new ValidatingContentFetcher(client, emptyMap(), emptySession) - .fetch(FetchContext.of("/some/path", "fallback", timeOut), aStep()) + .fetch(FetchContext.of("/some/path", "fallback", defaultTimeout), aStep()) .get(); assertThat(result.payload()).contains("ok"); } @@ -60,7 +60,7 @@ public void expandsPathWithParameters() throws Exception { when(client.send(aRequestWithPath("/some/path/val"))).thenReturn(aResponse("ok")); final Response result = new ValidatingContentFetcher(client, params("var", "val"), emptySession) - .fetch(FetchContext.of("/some/path/{var}", "fallback", timeOut), aStep()) + .fetch(FetchContext.of("/some/path/{var}", "fallback", defaultTimeout), aStep()) .get(); assertThat(result.payload()).contains("ok"); } @@ -71,7 +71,7 @@ public void defaultsNonHTMLResponsesToEmpty() throws Exception { when(client.send(aRequestWithPath("/some/path"))).thenReturn(aResponse("ok", "text/json")); final Response result = new ValidatingContentFetcher(client, emptyMap(), emptySession) - .fetch(FetchContext.of("/some/path", "", timeOut), aStep()) + .fetch(FetchContext.of("/some/path", "", defaultTimeout), aStep()) .get(); assertThat(result.payload()).contains(""); } @@ -82,7 +82,7 @@ public void forwardsSession() throws Exception { when(client.send(any())).thenReturn(aResponse("")); final SessionRoot session = session("x-rd-key", "value"); new ValidatingContentFetcher(client, emptyMap(), session) - .fetch(FetchContext.of("/some/path", "", timeOut), aStep()) + .fetch(FetchContext.of("/some/path", "", defaultTimeout), aStep()) .get(); verify(client).send(aRequestWithSession("x-rd-key", "value")); } @@ -90,17 +90,18 @@ public void forwardsSession() throws Exception { @Test public void validatingContentFetcherAppliesTtl() throws Exception { final StubClient client = aStubClient(); + final Optional timeout = Optional.of(Duration.ofMillis(200)); new ValidatingContentFetcher(client, emptyMap(), emptySession) - .fetch(FetchContext.of("path", "fallback", timeOut), aStep()) + .fetch(FetchContext.of("path", "fallback", timeout), aStep()) .get(); - assertThat(client.sentRequests()).isNotEmpty().allSatisfy(r -> ttlIsSet(r)); + assertThat(client.sentRequests()).isNotEmpty().allSatisfy(r -> ttlIsSet(r, timeout)); } - private void ttlIsSet(final Request request) { + private void ttlIsSet(final Request request, final Optional timeout) { assertThat(request.ttl()).isNotEmpty(); - assertThat(request.ttl().get()).isEqualTo(timeOut); + assertThat(request.ttl()).isEqualTo(timeout); } private StubClient aStubClient() { diff --git a/src/test/java/com/rewedigital/composer/proxy/ComposingRequestHandlerTest.java b/src/test/java/com/rewedigital/composer/proxy/ComposingRequestHandlerTest.java index 651d05c..0d034f6 100644 --- a/src/test/java/com/rewedigital/composer/proxy/ComposingRequestHandlerTest.java +++ b/src/test/java/com/rewedigital/composer/proxy/ComposingRequestHandlerTest.java @@ -39,7 +39,7 @@ public class ComposingRequestHandlerTest { private static final String SERVICE_RESPONSE = "test"; - private final Duration ttl = Duration.ZERO; + private final Optional notTtl = Optional.empty(); @Test public void returnsResponseFromTemplateRoute() throws Exception { @@ -77,7 +77,7 @@ public void returnsErrorResponseFromProxyRoute() throws Exception { } private BackendRouting aRouter(final String pattern, final RouteTypeName routeType) { - final Rule sampleRule = Rule.fromUri(pattern, "GET", Match.of("http://target", ttl, routeType)); + final Rule sampleRule = Rule.fromUri(pattern, "GET", Match.of("http://target", notTtl, routeType)); return new BackendRouting(singletonList(sampleRule)); } diff --git a/src/test/java/com/rewedigital/composer/routing/BackendRoutingTest.java b/src/test/java/com/rewedigital/composer/routing/BackendRoutingTest.java index 796e842..4c22bde 100644 --- a/src/test/java/com/rewedigital/composer/routing/BackendRoutingTest.java +++ b/src/test/java/com/rewedigital/composer/routing/BackendRoutingTest.java @@ -21,11 +21,11 @@ public class BackendRoutingTest { private final SessionRoot emptySession = SessionRoot.empty(); - private final Duration ttl = Duration.ZERO; + private final Optional noTtl = Optional.empty(); @Test public void findsRouteForSimpleRule() { - final Rule simpleRule = Rule.fromUri("/", "GET", Match.of("http://test.com/", ttl, PROXY)); + final Rule simpleRule = Rule.fromUri("/", "GET", Match.of("http://test.com/", noTtl, PROXY)); final BackendRouting backendRouting = new BackendRouting(ImmutableList.of(simpleRule)); final Optional matchResult = backendRouting.matches(requestFor("GET", "/"), emptySession); @@ -36,7 +36,7 @@ public void findsRouteForSimpleRule() { @Test public void findsRouteWithPathArguments() { final Rule ruleWithPath = - Rule.fromUri("/", "GET", Match.of("http://test.com/{someValue}", ttl, PROXY)); + Rule.fromUri("/", "GET", Match.of("http://test.com/{someValue}", noTtl, PROXY)); final BackendRouting backendRouting = new BackendRouting(ImmutableList.of(ruleWithPath)); final Optional matchResult = backendRouting.matches(requestFor("GET", "/123"), emptySession); @@ -47,7 +47,7 @@ public void findsRouteWithPathArguments() { @Test public void findsNoRouteThatIsNotConfigured() { - final Rule simpleRule = Rule.fromUri("/", "GET", Match.of("http://test.com/", ttl, PROXY)); + final Rule simpleRule = Rule.fromUri("/", "GET", Match.of("http://test.com/", noTtl, PROXY)); final BackendRouting backendRouting = new BackendRouting(ImmutableList.of(simpleRule)); final Optional matchResult = backendRouting.matches(requestFor("PUT", "/"), emptySession); diff --git a/src/test/java/com/rewedigital/composer/routing/RoutingConfigurationTest.java b/src/test/java/com/rewedigital/composer/routing/RoutingConfigurationTest.java index 786c553..a1e272f 100644 --- a/src/test/java/com/rewedigital/composer/routing/RoutingConfigurationTest.java +++ b/src/test/java/com/rewedigital/composer/routing/RoutingConfigurationTest.java @@ -1,12 +1,14 @@ package com.rewedigital.composer.routing; +import static com.rewedigital.composer.routing.RouteTypeName.PROXY; +import static java.time.Duration.ofMillis; import static org.assertj.core.api.Assertions.assertThat; -import java.time.Duration; import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; import org.junit.Test; @@ -20,7 +22,8 @@ public class RoutingConfigurationTest { - private final Duration ttl = Duration.ZERO; + private final Optional withoutTtl = Optional.empty(); + private final Optional withTtl = Optional.of(200); @Test public void allConfigParametersAreCoveredByDefaultConfig() { @@ -32,22 +35,42 @@ public void allConfigParametersAreCoveredByDefaultConfig() { @Test public void createsLocalRouteFromConfiguration() { final RoutingConfiguration configuration = - RoutingConfiguration.fromConfig(configWithLocalRoutes( - localRoute("/test/path/", "GET", "https://target.service/{arg}", RouteTypeName.PROXY))); + RoutingConfiguration.fromConfig(configWithSingleLocalRoute(withoutTtl)); final List> localRules = configuration.localRules(); assertThat(localRules).anySatisfy(rule -> { assertThat(rule.getPath()).isEqualTo("/test/path/"); assertThat(rule.getMethods()).contains("GET"); - assertThat(rule.getTarget()).isEqualTo(Match.of("https://target.service/{arg}", ttl, RouteTypeName.PROXY)); + final Match routeMatchWithoutTtl = + Match.of("https://target.service/{arg}", Optional.empty(), PROXY); + assertThat(rule.getTarget()).isEqualTo(routeMatchWithoutTtl); }); } + @Test + public void setsTtlIfConfigures() { + final RoutingConfiguration configuration = + RoutingConfiguration.fromConfig(configWithSingleLocalRoute(withTtl)); + final List> localRules = configuration.localRules(); + assertThat(localRules).anySatisfy(rule -> { + final Match routeMatchWithTtl = + Match.of("https://target.service/{arg}", Optional.of(ofMillis(200)), PROXY); + assertThat(routeMatchWithTtl); + }); + } + + private Config configWithSingleLocalRoute(final Optional ttl) { + return configWithLocalRoutes( + localRoute("/test/path/", "GET", "https://target.service/{arg}", ttl, PROXY)); + } + + private static Map localRoute(final String path, final String method, final String target, - final RouteTypeName type) { + final Optional ttl, final RouteTypeName type) { final Map result = new HashMap<>(); result.put("path", path); result.put("method", method); result.put("target", target); + ttl.ifPresent(t -> result.put("ttl", t)); result.put("type", type.toString()); return result; } From 7d923d7222afdc4f186e07a3ce995dd4205a3165 Mon Sep 17 00:00:00 2001 From: Andreas Kluth Date: Mon, 12 Mar 2018 00:30:00 +0100 Subject: [PATCH 4/6] Add test for IncludeMarkupHandler. --- .../composing/IncludeMarkupHandler.java | 4 +++ .../composer/composing/IncludedService.java | 7 ++--- .../composing/ValidatingContentFetcher.java | 4 --- .../composing/IncludeMarkupHandlerTest.java | 31 +++++++++++++++++++ .../ValidatingContentFetcherTest.java | 2 +- 5 files changed, 39 insertions(+), 9 deletions(-) create mode 100644 src/test/java/com/rewedigital/composer/composing/IncludeMarkupHandlerTest.java rename src/test/java/com/rewedigital/composer/composing/{fetch => }/ValidatingContentFetcherTest.java (99%) diff --git a/src/main/java/com/rewedigital/composer/composing/IncludeMarkupHandler.java b/src/main/java/com/rewedigital/composer/composing/IncludeMarkupHandler.java index ee0b427..1c842a6 100755 --- a/src/main/java/com/rewedigital/composer/composing/IncludeMarkupHandler.java +++ b/src/main/java/com/rewedigital/composer/composing/IncludeMarkupHandler.java @@ -41,6 +41,10 @@ private List assets() { return contentMarkupHandler.assets(); } + List includedServices() { + return includedServices; + } + @Override public void handleOpenElementStart(final char[] buffer, final int nameOffset, final int nameLen, final int line, final int col) diff --git a/src/main/java/com/rewedigital/composer/composing/IncludedService.java b/src/main/java/com/rewedigital/composer/composing/IncludedService.java index 45b0bcf..4fa5a0b 100755 --- a/src/main/java/com/rewedigital/composer/composing/IncludedService.java +++ b/src/main/java/com/rewedigital/composer/composing/IncludedService.java @@ -99,19 +99,18 @@ public boolean isInRage(final ContentRange contentRange) { return contentRange.isInRange(startOffset); } - private String fallback() { + String fallback() { return fallback; } - private String path() { + String path() { return attributes.getOrDefault("path", ""); } - private Optional ttl() { + Optional ttl() { return longFromMap("ttl").map(Duration::ofMillis); } - private Optional longFromMap(final String name) { if (!attributes.containsKey(name)) { return Optional.empty(); diff --git a/src/main/java/com/rewedigital/composer/composing/ValidatingContentFetcher.java b/src/main/java/com/rewedigital/composer/composing/ValidatingContentFetcher.java index f903225..e8892b9 100755 --- a/src/main/java/com/rewedigital/composer/composing/ValidatingContentFetcher.java +++ b/src/main/java/com/rewedigital/composer/composing/ValidatingContentFetcher.java @@ -3,7 +3,6 @@ import static java.util.Objects.requireNonNull; import java.util.Map; -import java.util.Objects; import java.util.concurrent.CompletableFuture; import org.slf4j.Logger; @@ -34,9 +33,6 @@ public ValidatingContentFetcher(final Client client, final Map p @Override public CompletableFuture> fetch(final FetchContext context, final CompositionStep step) { - Objects.requireNonNull(context); - Objects.requireNonNull(step); - if (context.path() == null || context.path().trim().isEmpty()) { LOGGER.warn("Empty path attribute in include found - callstack: " + step.callStack()); return CompletableFuture.completedFuture(Response.forPayload("")); diff --git a/src/test/java/com/rewedigital/composer/composing/IncludeMarkupHandlerTest.java b/src/test/java/com/rewedigital/composer/composing/IncludeMarkupHandlerTest.java new file mode 100644 index 0000000..a7ad1e0 --- /dev/null +++ b/src/test/java/com/rewedigital/composer/composing/IncludeMarkupHandlerTest.java @@ -0,0 +1,31 @@ +package com.rewedigital.composer.composing; + +import static com.rewedigital.composer.parser.Parser.PARSER; +import static org.assertj.core.api.Assertions.assertThat; + +import java.time.Duration; + +import org.junit.Test; + +public class IncludeMarkupHandlerTest { + + @Test + public void extractsTtlAndPathAttributes() { + final IncludeMarkupHandler handler = parse("Fallback"); + + assertThat(handler.includedServices()).isNotEmpty(); + assertThat(handler.includedServices()).allSatisfy(included -> { + assertThat(included.ttl()).isPresent().contains(Duration.ofMillis(123)); + assertThat(included.path()).contains("value"); + }); + + } + + private IncludeMarkupHandler parse(final String data) { + final ComposerHtmlConfiguration configuration = new ComposerHtmlConfiguration("include", "content", "asset", 1); + final IncludeMarkupHandler markupHandler = + new IncludeMarkupHandler(ContentRange.allUpToo(data.length()), configuration); + PARSER.parse(data, markupHandler); + return markupHandler; + } +} diff --git a/src/test/java/com/rewedigital/composer/composing/fetch/ValidatingContentFetcherTest.java b/src/test/java/com/rewedigital/composer/composing/ValidatingContentFetcherTest.java similarity index 99% rename from src/test/java/com/rewedigital/composer/composing/fetch/ValidatingContentFetcherTest.java rename to src/test/java/com/rewedigital/composer/composing/ValidatingContentFetcherTest.java index e132488..58a58d1 100644 --- a/src/test/java/com/rewedigital/composer/composing/fetch/ValidatingContentFetcherTest.java +++ b/src/test/java/com/rewedigital/composer/composing/ValidatingContentFetcherTest.java @@ -1,4 +1,4 @@ -package com.rewedigital.composer.composing.fetch; +package com.rewedigital.composer.composing; import static java.util.Collections.emptyMap; import static java.util.concurrent.CompletableFuture.completedFuture; From edc5874525d18b16115c5d1183076b121053beb9 Mon Sep 17 00:00:00 2001 From: Andreas Kluth Date: Mon, 12 Mar 2018 22:04:58 +0100 Subject: [PATCH 5/6] First batch of changes based on code review. --- .../composer/composing/FetchContext.java | 4 ++++ .../composing/IncludeMarkupHandler.java | 3 +++ .../composer/composing/IncludedService.java | 10 ++++++--- .../composing/ValidatingContentFetcher.java | 8 ++----- .../composer/configuration/ConfigUtil.java | 21 +++++++++++++++++++ .../rewedigital/composer/routing/Match.java | 10 ++++++++- .../composer/routing/ProxyRoute.java | 2 +- .../composer/routing/RouteMatch.java | 7 ++++--- .../routing/RoutingConfiguration.java | 6 ++---- .../routing/SessionAwareProxyClient.java | 11 ++++++++-- .../composer/routing/TemplateRoute.java | 2 +- .../composing/IncludeMarkupHandlerTest.java | 4 ++-- .../ValidatingContentFetcherTest.java | 7 +++---- .../proxy/ComposingRequestHandlerTest.java | 7 +++---- .../composer/routing/BackendRoutingTest.java | 8 +++---- .../routing/RoutingConfigurationTest.java | 15 ++++++------- .../routing/SessionAwareProxyClientTest.java | 13 +++++++++--- 17 files changed, 92 insertions(+), 46 deletions(-) create mode 100644 src/main/java/com/rewedigital/composer/configuration/ConfigUtil.java diff --git a/src/main/java/com/rewedigital/composer/composing/FetchContext.java b/src/main/java/com/rewedigital/composer/composing/FetchContext.java index 4729ad8..fd48c1a 100644 --- a/src/main/java/com/rewedigital/composer/composing/FetchContext.java +++ b/src/main/java/com/rewedigital/composer/composing/FetchContext.java @@ -32,6 +32,10 @@ public static FetchContext of(final String path, final String fallback, final Op return new FetchContext(path, fallback, ttl); } + public boolean hasEmptyPath() { + return path == null || path().trim().isEmpty(); + } + public String path() { return path; } diff --git a/src/main/java/com/rewedigital/composer/composing/IncludeMarkupHandler.java b/src/main/java/com/rewedigital/composer/composing/IncludeMarkupHandler.java index 1c842a6..de57c8d 100755 --- a/src/main/java/com/rewedigital/composer/composing/IncludeMarkupHandler.java +++ b/src/main/java/com/rewedigital/composer/composing/IncludeMarkupHandler.java @@ -11,6 +11,8 @@ import org.attoparser.output.OutputMarkupHandler; import org.attoparser.util.TextUtil; +import com.google.common.annotations.VisibleForTesting; + class IncludeMarkupHandler extends AbstractMarkupHandler { private final char[] includeTag; @@ -41,6 +43,7 @@ private List assets() { return contentMarkupHandler.assets(); } + @VisibleForTesting List includedServices() { return includedServices; } diff --git a/src/main/java/com/rewedigital/composer/composing/IncludedService.java b/src/main/java/com/rewedigital/composer/composing/IncludedService.java index 4fa5a0b..3ae9195 100755 --- a/src/main/java/com/rewedigital/composer/composing/IncludedService.java +++ b/src/main/java/com/rewedigital/composer/composing/IncludedService.java @@ -9,6 +9,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.google.common.annotations.VisibleForTesting; import com.spotify.apollo.Response; /** @@ -99,15 +100,18 @@ public boolean isInRage(final ContentRange contentRange) { return contentRange.isInRange(startOffset); } - String fallback() { + @VisibleForTesting + public String fallback() { return fallback; } - String path() { + @VisibleForTesting + public String path() { return attributes.getOrDefault("path", ""); } - Optional ttl() { + @VisibleForTesting + public Optional ttl() { return longFromMap("ttl").map(Duration::ofMillis); } diff --git a/src/main/java/com/rewedigital/composer/composing/ValidatingContentFetcher.java b/src/main/java/com/rewedigital/composer/composing/ValidatingContentFetcher.java index e8892b9..5a97bb9 100755 --- a/src/main/java/com/rewedigital/composer/composing/ValidatingContentFetcher.java +++ b/src/main/java/com/rewedigital/composer/composing/ValidatingContentFetcher.java @@ -33,7 +33,7 @@ public ValidatingContentFetcher(final Client client, final Map p @Override public CompletableFuture> fetch(final FetchContext context, final CompositionStep step) { - if (context.path() == null || context.path().trim().isEmpty()) { + if (context.hasEmptyPath()) { LOGGER.warn("Empty path attribute in include found - callstack: " + step.callStack()); return CompletableFuture.completedFuture(Response.forPayload("")); } @@ -41,7 +41,6 @@ public CompletableFuture> fetch(final FetchContext context, fin final String expandedPath = UriTemplate.fromTemplate(context.path()).expand(parsedPathArguments); final Request request = session.enrich(withTtl(Request.forUri(expandedPath, "GET"), context)); - return client.send(request) .thenApply(response -> acceptHtmlOnly(response, expandedPath)) .thenApply(r -> toStringPayload(r, context.fallback())) @@ -49,10 +48,7 @@ public CompletableFuture> fetch(final FetchContext context, fin } private Request withTtl(final Request request, final FetchContext context) { - if (context.ttl().isPresent()) { - return request.withTtl(context.ttl().get()); - } - return request; + return context.ttl().map(t -> request.withTtl(t)).orElse(request); } private Response toStringPayload(final Response response, final String fallback) { diff --git a/src/main/java/com/rewedigital/composer/configuration/ConfigUtil.java b/src/main/java/com/rewedigital/composer/configuration/ConfigUtil.java new file mode 100644 index 0000000..806dd94 --- /dev/null +++ b/src/main/java/com/rewedigital/composer/configuration/ConfigUtil.java @@ -0,0 +1,21 @@ +package com.rewedigital.composer.configuration; + +import java.time.Duration; +import java.util.Optional; + +import com.typesafe.config.Config; + +/** + * Utility class inspired by {@link com.spotify.apollo.environment.ConfigUtil} eases working with {@link Config}. + */ +public class ConfigUtil { + + private ConfigUtil() { + // Construction is not permitted. + } + + public static Optional optionalDuration(final Config config, final String path) { + return config.hasPath(path) ? Optional.of(config.getDuration(path)) : Optional.empty(); + } + +} diff --git a/src/main/java/com/rewedigital/composer/routing/Match.java b/src/main/java/com/rewedigital/composer/routing/Match.java index 3a66d44..3904b7f 100644 --- a/src/main/java/com/rewedigital/composer/routing/Match.java +++ b/src/main/java/com/rewedigital/composer/routing/Match.java @@ -16,6 +16,14 @@ private Match(final String backend, final Optional ttl, final RouteTyp this.routeType = routeType; } + public static Match of(final String backend, final RouteTypeName routeType) { + return new Match(backend, Optional.empty(), routeType); + } + + public static Match of(final String backend, final Duration ttl, final RouteTypeName routeType) { + return new Match(backend, Optional.of(ttl), routeType); + } + public static Match of(final String backend, final Optional ttl, final RouteTypeName routeType) { return new Match(backend, ttl, routeType); } @@ -49,7 +57,7 @@ public boolean equals(final Object obj) { return false; } final Match other = (Match) obj; - return Objects.equals(backend, other.backend) && routeType == other.routeType && ttl == other.ttl; + return Objects.equals(backend, other.backend) && routeType == other.routeType && Objects.equals(ttl, other.ttl); } } diff --git a/src/main/java/com/rewedigital/composer/routing/ProxyRoute.java b/src/main/java/com/rewedigital/composer/routing/ProxyRoute.java index 02d68b9..4688a1e 100644 --- a/src/main/java/com/rewedigital/composer/routing/ProxyRoute.java +++ b/src/main/java/com/rewedigital/composer/routing/ProxyRoute.java @@ -23,6 +23,6 @@ public ProxyRoute(final SessionAwareProxyClient templateClient) { @Override public CompletionStage> execute(final RouteMatch rm, final RequestContext context, final SessionRoot session) { - return templateClient.fetch(rm.expandedPath(), context, session); + return templateClient.fetch(rm, context, session); } } diff --git a/src/main/java/com/rewedigital/composer/routing/RouteMatch.java b/src/main/java/com/rewedigital/composer/routing/RouteMatch.java index fb1e645..03fe857 100644 --- a/src/main/java/com/rewedigital/composer/routing/RouteMatch.java +++ b/src/main/java/com/rewedigital/composer/routing/RouteMatch.java @@ -35,12 +35,13 @@ public Map parsedPathArguments() { return parsedPathArguments; } + public String expandedPath() { + return UriTemplate.fromTemplate(backend.backend()).expand(parsedPathArguments); + } + @Override public String toString() { return "RouteMatch [backend=" + backend + ", parsedPathArguments=" + parsedPathArguments + "]"; } - public String expandedPath() { - return UriTemplate.fromTemplate(backend.backend()).expand(parsedPathArguments); - } } diff --git a/src/main/java/com/rewedigital/composer/routing/RoutingConfiguration.java b/src/main/java/com/rewedigital/composer/routing/RoutingConfiguration.java index 02e783e..bd6a00b 100644 --- a/src/main/java/com/rewedigital/composer/routing/RoutingConfiguration.java +++ b/src/main/java/com/rewedigital/composer/routing/RoutingConfiguration.java @@ -1,5 +1,6 @@ package com.rewedigital.composer.routing; +import static com.rewedigital.composer.configuration.ConfigUtil.optionalDuration; import static java.util.stream.Collectors.toList; import java.time.Duration; @@ -8,6 +9,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.rewedigital.composer.configuration.ConfigUtil; import com.spotify.apollo.route.Rule; import com.typesafe.config.Config; import com.typesafe.config.ConfigList; @@ -48,8 +50,4 @@ public List> localRules() { return localRoutes; } - private static Optional optionalDuration(final Config config, final String path) { - return config.hasPath(path) ? Optional.of(config.getDuration(path)) : Optional.empty(); - } - } diff --git a/src/main/java/com/rewedigital/composer/routing/SessionAwareProxyClient.java b/src/main/java/com/rewedigital/composer/routing/SessionAwareProxyClient.java index c3b0579..5bf8179 100644 --- a/src/main/java/com/rewedigital/composer/routing/SessionAwareProxyClient.java +++ b/src/main/java/com/rewedigital/composer/routing/SessionAwareProxyClient.java @@ -1,5 +1,7 @@ package com.rewedigital.composer.routing; +import java.time.Duration; +import java.util.Optional; import java.util.concurrent.CompletionStage; import com.rewedigital.composer.session.ResponseWithSession; @@ -12,10 +14,15 @@ public class SessionAwareProxyClient { - public CompletionStage> fetch(final String path, final RequestContext context, + public CompletionStage> fetch(final RouteMatch rm, final RequestContext context, final SessionRoot session) { return context.requestScopedClient() - .send(session.enrich(Request.forUri(path, context.request().method()))) + .send(session.enrich(withTtl(Request.forUri(rm.expandedPath(), context.request().method()), rm.ttl()))) .thenApply(r -> new ResponseWithSession<>(r, session.mergedWith(SessionFragment.of(r)))); } + + private Request withTtl(final Request request, final Optional ttl) { + return ttl.map(t -> request.withTtl(t)).orElse(request); + } + } diff --git a/src/main/java/com/rewedigital/composer/routing/TemplateRoute.java b/src/main/java/com/rewedigital/composer/routing/TemplateRoute.java index d636c08..8d3685e 100644 --- a/src/main/java/com/rewedigital/composer/routing/TemplateRoute.java +++ b/src/main/java/com/rewedigital/composer/routing/TemplateRoute.java @@ -34,7 +34,7 @@ public TemplateRoute(final SessionAwareProxyClient templateClient, final Compose @Override public CompletionStage> execute(final RouteMatch rm, final RequestContext context, final SessionRoot session) { - return templateClient.fetch(rm.expandedPath(), context, session) + return templateClient.fetch(rm, context, session) .thenCompose( templateResponse -> process(context.requestScopedClient(), rm.parsedPathArguments(), templateResponse, rm.expandedPath())); diff --git a/src/test/java/com/rewedigital/composer/composing/IncludeMarkupHandlerTest.java b/src/test/java/com/rewedigital/composer/composing/IncludeMarkupHandlerTest.java index a7ad1e0..6a34ea4 100644 --- a/src/test/java/com/rewedigital/composer/composing/IncludeMarkupHandlerTest.java +++ b/src/test/java/com/rewedigital/composer/composing/IncludeMarkupHandlerTest.java @@ -15,10 +15,10 @@ public void extractsTtlAndPathAttributes() { assertThat(handler.includedServices()).isNotEmpty(); assertThat(handler.includedServices()).allSatisfy(included -> { - assertThat(included.ttl()).isPresent().contains(Duration.ofMillis(123)); + assertThat(included.ttl()).contains(Duration.ofMillis(123)); assertThat(included.path()).contains("value"); + assertThat(included.fallback()).contains("Fallback"); }); - } private IncludeMarkupHandler parse(final String data) { diff --git a/src/test/java/com/rewedigital/composer/composing/ValidatingContentFetcherTest.java b/src/test/java/com/rewedigital/composer/composing/ValidatingContentFetcherTest.java index 58a58d1..c8caa9f 100644 --- a/src/test/java/com/rewedigital/composer/composing/ValidatingContentFetcherTest.java +++ b/src/test/java/com/rewedigital/composer/composing/ValidatingContentFetcherTest.java @@ -96,12 +96,11 @@ public void validatingContentFetcherAppliesTtl() throws Exception { .fetch(FetchContext.of("path", "fallback", timeout), aStep()) .get(); - assertThat(client.sentRequests()).isNotEmpty().allSatisfy(r -> ttlIsSet(r, timeout)); + assertThat(client.sentRequests()).isNotEmpty().allSatisfy(r -> ttlIsSet(r, timeout.get())); } - private void ttlIsSet(final Request request, final Optional timeout) { - assertThat(request.ttl()).isNotEmpty(); - assertThat(request.ttl()).isEqualTo(timeout); + private void ttlIsSet(final Request request, final Duration timeout) { + assertThat(request.ttl()).contains(timeout); } private StubClient aStubClient() { diff --git a/src/test/java/com/rewedigital/composer/proxy/ComposingRequestHandlerTest.java b/src/test/java/com/rewedigital/composer/proxy/ComposingRequestHandlerTest.java index 0d034f6..b373084 100644 --- a/src/test/java/com/rewedigital/composer/proxy/ComposingRequestHandlerTest.java +++ b/src/test/java/com/rewedigital/composer/proxy/ComposingRequestHandlerTest.java @@ -8,7 +8,6 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -import java.time.Duration; import java.util.Optional; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionStage; @@ -20,6 +19,7 @@ import com.rewedigital.composer.proxy.ComposingRequestHandler; import com.rewedigital.composer.routing.BackendRouting; import com.rewedigital.composer.routing.Match; +import com.rewedigital.composer.routing.RouteMatch; import com.rewedigital.composer.routing.RouteTypeName; import com.rewedigital.composer.routing.RouteTypes; import com.rewedigital.composer.routing.SessionAwareProxyClient; @@ -39,7 +39,6 @@ public class ComposingRequestHandlerTest { private static final String SERVICE_RESPONSE = "test"; - private final Optional notTtl = Optional.empty(); @Test public void returnsResponseFromTemplateRoute() throws Exception { @@ -77,7 +76,7 @@ public void returnsErrorResponseFromProxyRoute() throws Exception { } private BackendRouting aRouter(final String pattern, final RouteTypeName routeType) { - final Rule sampleRule = Rule.fromUri(pattern, "GET", Match.of("http://target", notTtl, routeType)); + final Rule sampleRule = Rule.fromUri(pattern, "GET", Match.of("http://target", routeType)); return new BackendRouting(singletonList(sampleRule)); } @@ -103,7 +102,7 @@ private static class RoutingResult extends SessionAwareProxyClient { public static RouteTypes returning(final Status status, final String responseBody) { return new RouteTypes(composerFactory(), new SessionAwareProxyClient() { @Override - public CompletionStage> fetch(final String path, + public CompletionStage> fetch(final RouteMatch rm, final RequestContext context, final SessionRoot session) { return CompletableFuture.completedFuture( new ResponseWithSession<>(Response.of(status, ByteString.encodeUtf8(responseBody)), session)); diff --git a/src/test/java/com/rewedigital/composer/routing/BackendRoutingTest.java b/src/test/java/com/rewedigital/composer/routing/BackendRoutingTest.java index 4c22bde..77981dc 100644 --- a/src/test/java/com/rewedigital/composer/routing/BackendRoutingTest.java +++ b/src/test/java/com/rewedigital/composer/routing/BackendRoutingTest.java @@ -5,7 +5,6 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -import java.time.Duration; import java.util.Optional; import org.junit.Test; @@ -21,11 +20,10 @@ public class BackendRoutingTest { private final SessionRoot emptySession = SessionRoot.empty(); - private final Optional noTtl = Optional.empty(); @Test public void findsRouteForSimpleRule() { - final Rule simpleRule = Rule.fromUri("/", "GET", Match.of("http://test.com/", noTtl, PROXY)); + final Rule simpleRule = Rule.fromUri("/", "GET", Match.of("http://test.com/", PROXY)); final BackendRouting backendRouting = new BackendRouting(ImmutableList.of(simpleRule)); final Optional matchResult = backendRouting.matches(requestFor("GET", "/"), emptySession); @@ -36,7 +34,7 @@ public void findsRouteForSimpleRule() { @Test public void findsRouteWithPathArguments() { final Rule ruleWithPath = - Rule.fromUri("/", "GET", Match.of("http://test.com/{someValue}", noTtl, PROXY)); + Rule.fromUri("/", "GET", Match.of("http://test.com/{someValue}", PROXY)); final BackendRouting backendRouting = new BackendRouting(ImmutableList.of(ruleWithPath)); final Optional matchResult = backendRouting.matches(requestFor("GET", "/123"), emptySession); @@ -47,7 +45,7 @@ public void findsRouteWithPathArguments() { @Test public void findsNoRouteThatIsNotConfigured() { - final Rule simpleRule = Rule.fromUri("/", "GET", Match.of("http://test.com/", noTtl, PROXY)); + final Rule simpleRule = Rule.fromUri("/", "GET", Match.of("http://test.com/", PROXY)); final BackendRouting backendRouting = new BackendRouting(ImmutableList.of(simpleRule)); final Optional matchResult = backendRouting.matches(requestFor("PUT", "/"), emptySession); diff --git a/src/test/java/com/rewedigital/composer/routing/RoutingConfigurationTest.java b/src/test/java/com/rewedigital/composer/routing/RoutingConfigurationTest.java index a1e272f..effd37c 100644 --- a/src/test/java/com/rewedigital/composer/routing/RoutingConfigurationTest.java +++ b/src/test/java/com/rewedigital/composer/routing/RoutingConfigurationTest.java @@ -1,9 +1,9 @@ package com.rewedigital.composer.routing; import static com.rewedigital.composer.routing.RouteTypeName.PROXY; -import static java.time.Duration.ofMillis; import static org.assertj.core.api.Assertions.assertThat; +import java.time.Duration; import java.util.Arrays; import java.util.HashMap; import java.util.List; @@ -22,8 +22,10 @@ public class RoutingConfigurationTest { + private final int timeoutInMs = 200; private final Optional withoutTtl = Optional.empty(); - private final Optional withTtl = Optional.of(200); + private final Optional withTtl = Optional.of(timeoutInMs); + private final Duration ttlDuration = Duration.ofMillis(timeoutInMs); @Test public void allConfigParametersAreCoveredByDefaultConfig() { @@ -41,20 +43,19 @@ public void createsLocalRouteFromConfiguration() { assertThat(rule.getPath()).isEqualTo("/test/path/"); assertThat(rule.getMethods()).contains("GET"); final Match routeMatchWithoutTtl = - Match.of("https://target.service/{arg}", Optional.empty(), PROXY); + Match.of("https://target.service/{arg}", PROXY); assertThat(rule.getTarget()).isEqualTo(routeMatchWithoutTtl); }); } @Test - public void setsTtlIfConfigures() { + public void setsTtlIfConfigured() { final RoutingConfiguration configuration = RoutingConfiguration.fromConfig(configWithSingleLocalRoute(withTtl)); final List> localRules = configuration.localRules(); assertThat(localRules).anySatisfy(rule -> { - final Match routeMatchWithTtl = - Match.of("https://target.service/{arg}", Optional.of(ofMillis(200)), PROXY); - assertThat(routeMatchWithTtl); + final Match routeMatchWithTtl = Match.of("https://target.service/{arg}", ttlDuration, PROXY); + assertThat(rule.getTarget()).isEqualTo(routeMatchWithTtl); }); } diff --git a/src/test/java/com/rewedigital/composer/routing/SessionAwareProxyClientTest.java b/src/test/java/com/rewedigital/composer/routing/SessionAwareProxyClientTest.java index 61d991a..3edf612 100644 --- a/src/test/java/com/rewedigital/composer/routing/SessionAwareProxyClientTest.java +++ b/src/test/java/com/rewedigital/composer/routing/SessionAwareProxyClientTest.java @@ -4,6 +4,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import java.util.Collections; import java.util.concurrent.CompletableFuture; import org.junit.Test; @@ -21,22 +22,28 @@ public class SessionAwareProxyClientTest { @Test public void fetchesTemplateHandlingSession() throws Exception { - final String path = "https://some.path/test"; final String method = "GET"; - final Request expectedRequest = Request.forUri(path, method).withHeader("x-rd-key", "value"); + final Request expectedRequest = + Request.forUri(aRouteMatch().expandedPath(), method).withHeader("x-rd-key", "value"); final Response response = Response.ok().withPayload(ByteString.EMPTY).withHeader("x-rd-response-key", "other-value"); final RequestContext context = contextWith(aClient(expectedRequest, response), method); final ResponseWithSession templateResponse = - new SessionAwareProxyClient().fetch(path, context, Sessions.sessionRoot("x-rd-key", "value")).toCompletableFuture() + new SessionAwareProxyClient().fetch(aRouteMatch(), context, Sessions.sessionRoot("x-rd-key", "value")) + .toCompletableFuture() .get(); assertThat(templateResponse.session().get("key")).contains("value"); assertThat(templateResponse.session().get("response-key")).contains("other-value"); } + private RouteMatch aRouteMatch() { + return new RouteMatch(Match.of("https://some.path/test", RouteTypeName.TEMPLATE), + Collections.emptyMap()); + } + private RequestContext contextWith(final Client client, final String method) { final RequestContext context = mock(RequestContext.class); when(context.requestScopedClient()).thenReturn(client); From 38449361ac975943a53c8b4ffb2ad81cd07c99a2 Mon Sep 17 00:00:00 2001 From: Andreas Kluth Date: Mon, 12 Mar 2018 22:09:38 +0100 Subject: [PATCH 6/6] Fixed dependecy cycles. --- .../composer/configuration/ConfigUtil.java | 21 ------------------- .../routing/RoutingConfiguration.java | 6 ++++-- 2 files changed, 4 insertions(+), 23 deletions(-) delete mode 100644 src/main/java/com/rewedigital/composer/configuration/ConfigUtil.java diff --git a/src/main/java/com/rewedigital/composer/configuration/ConfigUtil.java b/src/main/java/com/rewedigital/composer/configuration/ConfigUtil.java deleted file mode 100644 index 806dd94..0000000 --- a/src/main/java/com/rewedigital/composer/configuration/ConfigUtil.java +++ /dev/null @@ -1,21 +0,0 @@ -package com.rewedigital.composer.configuration; - -import java.time.Duration; -import java.util.Optional; - -import com.typesafe.config.Config; - -/** - * Utility class inspired by {@link com.spotify.apollo.environment.ConfigUtil} eases working with {@link Config}. - */ -public class ConfigUtil { - - private ConfigUtil() { - // Construction is not permitted. - } - - public static Optional optionalDuration(final Config config, final String path) { - return config.hasPath(path) ? Optional.of(config.getDuration(path)) : Optional.empty(); - } - -} diff --git a/src/main/java/com/rewedigital/composer/routing/RoutingConfiguration.java b/src/main/java/com/rewedigital/composer/routing/RoutingConfiguration.java index bd6a00b..9833bba 100644 --- a/src/main/java/com/rewedigital/composer/routing/RoutingConfiguration.java +++ b/src/main/java/com/rewedigital/composer/routing/RoutingConfiguration.java @@ -1,6 +1,5 @@ package com.rewedigital.composer.routing; -import static com.rewedigital.composer.configuration.ConfigUtil.optionalDuration; import static java.util.stream.Collectors.toList; import java.time.Duration; @@ -9,7 +8,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.rewedigital.composer.configuration.ConfigUtil; import com.spotify.apollo.route.Rule; import com.typesafe.config.Config; import com.typesafe.config.ConfigList; @@ -49,5 +47,9 @@ private static Rule buildLocalRoute(final ConfigValue configValue) { public List> localRules() { return localRoutes; } + + private static Optional optionalDuration(final Config config, final String path) { + return config.hasPath(path) ? Optional.of(config.getDuration(path)) : Optional.empty(); + } }