Skip to content
This repository has been archived by the owner on Jun 12, 2020. It is now read-only.

Add the facility to override the request timeout per include and route. #28

Merged
merged 6 commits into from
Mar 12, 2018
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -9,6 +9,6 @@
*/
public interface ContentFetcher {

CompletableFuture<Response<String>> fetch(String path, String fallback, CompositionStep step);
CompletableFuture<Response<String>> fetch(FetchContext fetchContext, CompositionStep step);

}
50 changes: 50 additions & 0 deletions src/main/java/com/rewedigital/composer/composing/FetchContext.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
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.
*/
public class FetchContext {

private final String path;
private final String fallback;
private final Optional<Duration> ttl;

private FetchContext(final String path, final String fallback, final Optional<Duration> ttl) {
this.path = path;
this.fallback = fallback;
this.ttl = requireNonNull(ttl);
Copy link
Member

Choose a reason for hiding this comment

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

Why requireNonNull for ttl but not for path or fallback? I really start to feel we need to find some consistent rule for these checks...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Path and fallback is used as if it is valid to have null in there...

Copy link
Member

Choose a reason for hiding this comment

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

I agree. But than, maybe Optional is better for both. I would like to have a codebase where null is always a mistake ;-)

I'll open an issue to discuss this kind of global code style questions, ok?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perfect.

}

/**
* 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.
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if ttl is the right name: shouldn't it be timeout or something like that?

Copy link
Contributor Author

@AndreasKl AndreasKl Mar 12, 2018

Choose a reason for hiding this comment

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

Let's discuss that @ work.

* @return the parameter object.
*/
public static FetchContext of(final String path, final String fallback, final Optional<Duration> ttl) {
return new FetchContext(path, fallback, ttl);
}

public boolean hasEmptyPath() {
return path == null || path().trim().isEmpty();
}

public String path() {
return path;
}

public String fallback() {
return fallback;
}

public Optional<Duration> ttl() {
return ttl;
}
}
Original file line number Diff line number Diff line change
@@ -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,11 @@ private List<Asset> assets() {
return contentMarkupHandler.assets();
}

@VisibleForTesting
List<IncludedService> includedServices() {
return includedServices;
}

@Override
public void handleOpenElementStart(final char[] buffer, final int nameOffset, final int nameLen, final int line,
final int col)
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
package com.rewedigital.composer.composing;

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.google.common.annotations.VisibleForTesting;
import com.spotify.apollo.Response;

/**
@@ -86,24 +89,43 @@ private IncludedService(final Builder builder) {
this.fallback = builder.fallback;
}


public CompletableFuture<IncludedService.WithResponse> 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));
}

private String fallback() {
public boolean isInRage(final ContentRange contentRange) {
return contentRange.isInRange(startOffset);
}

@VisibleForTesting
public String fallback() {
return fallback;
}

private String path() {
@VisibleForTesting
public String path() {
return attributes.getOrDefault("path", "");
}

public boolean isInRage(final ContentRange contentRange) {
return contentRange.isInRange(startOffset);
@VisibleForTesting
public Optional<Duration> ttl() {
return longFromMap("ttl").map(Duration::ofMillis);
}

private Optional<Long> longFromMap(final String name) {
if (!attributes.containsKey(name)) {
return Optional.empty();
}
final String unparsedValue = attributes.get(name);
try {
return Optional.of(Long.parseLong(unparsedValue));
} catch (final NumberFormatException nfEx) {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add a test showing what happens when the data type is wrong?

LOGGER.info("Not able to evaluate {} for path {} with value {}.", name, path(), unparsedValue);
}
return Optional.empty();
}

}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.rewedigital.composer.composing;

import java.util.Objects;
import static java.util.Objects.requireNonNull;

import java.util.concurrent.CompletableFuture;

import org.slf4j.Logger;
@@ -16,18 +17,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<Response<String>> fetch(final String path, final String fallback,
final CompositionStep step) {
public CompletableFuture<Response<String>> 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);
}

}
Original file line number Diff line number Diff line change
@@ -26,26 +26,31 @@ public class ValidatingContentFetcher implements ContentFetcher {

public ValidatingContentFetcher(final Client client, final Map<String, Object> parsedPathArguments,
final SessionRoot session) {
this.session = session;
this.session = requireNonNull(session);
this.client = requireNonNull(client);
this.parsedPathArguments = requireNonNull(parsedPathArguments);
}

@Override
public CompletableFuture<Response<String>> fetch(final String path, final String fallback,
final CompositionStep step) {
if (path == null || path.trim().isEmpty()) {
public CompletableFuture<Response<String>> fetch(final FetchContext context, final CompositionStep step) {
if (context.hasEmptyPath()) {
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(withTtl(Request.forUri(expandedPath, "GET"), context));

return client.send(request)
.thenApply(response -> acceptHtmlOnly(response, expandedPath))
.thenApply(r -> toStringPayload(r, fallback))
.thenApply(r -> toStringPayload(r, context.fallback()))
.toCompletableFuture();
}

private Request withTtl(final Request request, final FetchContext context) {
return context.ttl().map(t -> request.withTtl(t)).orElse(request);
}

private Response<String> toStringPayload(final Response<ByteString> response, final String fallback) {
final String value = response.payload().map(p -> p.utf8()).orElse(fallback);
return response.withPayload(value);
34 changes: 26 additions & 8 deletions src/main/java/com/rewedigital/composer/routing/Match.java
Original file line number Diff line number Diff line change
@@ -1,45 +1,63 @@
package com.rewedigital.composer.routing;

import java.time.Duration;
import java.util.Objects;
import java.util.Optional;

public class Match {

private final String backend;
private final Optional<Duration> ttl;
private final RouteTypeName routeType;

private Match(final String backend, final RouteTypeName routeType) {
private Match(final String backend, final Optional<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);
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<Duration> ttl, final RouteTypeName routeType) {
return new Match(backend, ttl, routeType);
}

public String backend() {
return backend;
}

public Optional<Duration> ttl() {
Copy link
Member

Choose a reason for hiding this comment

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

Might be me not understanding the coverage report, but it looks like this new method is not covered by any test?

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 && Objects.equals(ttl, other.ttl);
}


}
Original file line number Diff line number Diff line change
@@ -23,6 +23,6 @@ public ProxyRoute(final SessionAwareProxyClient templateClient) {
@Override
public CompletionStage<ResponseWithSession<ByteString>> execute(final RouteMatch rm, final RequestContext context,
final SessionRoot session) {
return templateClient.fetch(rm.expandedPath(), context, session);
return templateClient.fetch(rm, context, session);
}
}
19 changes: 14 additions & 5 deletions src/main/java/com/rewedigital/composer/routing/RouteMatch.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
package com.rewedigital.composer.routing;

import static java.util.Objects.requireNonNull;

import java.time.Duration;
import java.util.Collections;
import java.util.Map;
import java.util.Optional;

import com.damnhandy.uri.template.UriTemplate;

@@ -11,14 +15,18 @@ public class RouteMatch {
private final Map<String, Object> parsedPathArguments;

public RouteMatch(final Match backend, final Map<String, String> parsedPathArguments) {
this.backend = backend;
this.backend = requireNonNull(backend);
this.parsedPathArguments = Collections.<String, Object>unmodifiableMap(parsedPathArguments);
}

public String backend() {
return backend.backend();
}

public Optional<Duration> ttl() {
Copy link
Member

Choose a reason for hiding this comment

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

Might be me not understanding the coverage report, but it looks like there is not test covering this new method?

Copy link
Member

Choose a reason for hiding this comment

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

And actually, I don't find the code using the route timeout?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

return backend.ttl();
}

public RouteType routeType(final RouteTypes routeTypes) {
return backend.routeType(routeTypes);
}
@@ -27,12 +35,13 @@ public Map<String, Object> 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);
}
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package com.rewedigital.composer.routing;

import java.util.List;
import java.util.stream.Collectors;
import static java.util.stream.Collectors.toList;

import java.time.Duration;
import java.util.List;
import java.util.Optional;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

@@ -25,7 +27,7 @@ public static RoutingConfiguration fromConfig(final Config config) {
}

private static List<Rule<Match>> buildLocalRoutes(final ConfigList routesConfig) {
return routesConfig.stream().map(RoutingConfiguration::buildLocalRoute).collect(Collectors.toList());
return routesConfig.stream().map(RoutingConfiguration::buildLocalRoute).collect(toList());
}

private static Rule<Match> buildLocalRoute(final ConfigValue configValue) {
@@ -34,16 +36,20 @@ private static Rule<Match> buildLocalRoute(final ConfigValue configValue) {
final String method = config.getString("method");
final String type = config.getString("type");
final String target = config.getString("target");
final Optional<Duration> ttl = optionalDuration(config, "ttl");

final Rule<Match> 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);
final Rule<Match> 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;

}

public List<Rule<Match>> localRules() {
return localRoutes;
}

private static Optional<Duration> optionalDuration(final Config config, final String path) {
return config.hasPath(path) ? Optional.of(config.getDuration(path)) : Optional.empty();
}

}
Original file line number Diff line number Diff line change
@@ -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<ResponseWithSession<ByteString>> fetch(final String path, final RequestContext context,
public CompletionStage<ResponseWithSession<ByteString>> 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<Duration> ttl) {
return ttl.map(t -> request.withTtl(t)).orElse(request);
}

}
Original file line number Diff line number Diff line change
@@ -34,7 +34,7 @@ public TemplateRoute(final SessionAwareProxyClient templateClient, final Compose
@Override
public CompletionStage<ResponseWithSession<ByteString>> 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()));
Original file line number Diff line number Diff line change
@@ -108,7 +108,7 @@ private List<HttpCookie> 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();
}
}
Original file line number Diff line number Diff line change
@@ -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;
}
}
4 changes: 4 additions & 0 deletions src/main/resources/composer.conf
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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("<include path=\"value\" ttl=123>Fallback</include>");

assertThat(handler.includedServices()).isNotEmpty();
assertThat(handler.includedServices()).allSatisfy(included -> {
assertThat(included.ttl()).contains(Duration.ofMillis(123));
assertThat(included.path()).contains("value");
assertThat(included.fallback()).contains("Fallback");
});
}

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;
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package com.rewedigital.composer.proxy;
package com.rewedigital.composer.composing;

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.FetchContext;
import com.rewedigital.composer.composing.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 {

private final SessionRoot emptySession = SessionRoot.empty();
private final Optional<Duration> defaultTimeout = Optional.empty();

@Test
public void fetchesEmptyContentForMissingPath() throws Exception {
final Response<String> 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, defaultTimeout), 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<String> result =
new ValidatingContentFetcher(client, emptyMap(), emptySession).fetch("/some/path", "fallback", aStep())
new ValidatingContentFetcher(client, emptyMap(), emptySession)
.fetch(FetchContext.of("/some/path", "fallback", defaultTimeout), 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<String> result =
new ValidatingContentFetcher(client, params("var", "val"), emptySession)
.fetch("/some/path/{var}", "fallback", aStep()).get();
.fetch(FetchContext.of("/some/path/{var}", "fallback", defaultTimeout), 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<String> result =
new ValidatingContentFetcher(client, emptyMap(), emptySession).fetch("/some/path", "", aStep()).get();
new ValidatingContentFetcher(client, emptyMap(), emptySession)
.fetch(FetchContext.of("/some/path", "", defaultTimeout), 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", "", defaultTimeout), aStep())
.get();
verify(client).send(aRequestWithSession("x-rd-key", "value"));
}

@Test
public void validatingContentFetcherAppliesTtl() throws Exception {
final StubClient client = aStubClient();
final Optional<Duration> timeout = Optional.of(Duration.ofMillis(200));

new ValidatingContentFetcher(client, emptyMap(), emptySession)
.fetch(FetchContext.of("path", "fallback", timeout), aStep())
.get();

assertThat(client.sentRequests()).isNotEmpty().allSatisfy(r -> ttlIsSet(r, timeout.get()));
}

private void ttlIsSet(final Request request, final Duration timeout) {
assertThat(request.ttl()).contains(timeout);
}

private StubClient aStubClient() {
final StubClient client = new StubClient();
final Response<ByteString> 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<Request>() {

@Override
public boolean matches(final Object argument) {
return (Request.class.isAssignableFrom(argument.getClass())) &&
return Request.class.isAssignableFrom(argument.getClass()) &&
path.equals(((Request) argument).uri());
}
});
Original file line number Diff line number Diff line change
@@ -19,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;
@@ -35,7 +36,7 @@

import okio.ByteString;

public class ComposingHandlerTest {
public class ComposingRequestHandlerTest {

private static final String SERVICE_RESPONSE = "<html>test</html>";

@@ -93,21 +94,15 @@ 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 {

public static RouteTypes returning(final Status status, final String responseBody) {
return new RouteTypes(composerFactory(), new SessionAwareProxyClient() {
@Override
public CompletionStage<ResponseWithSession<ByteString>> fetch(final String path,
public CompletionStage<ResponseWithSession<ByteString>> fetch(final RouteMatch rm,
final RequestContext context, final SessionRoot session) {
return CompletableFuture.completedFuture(
new ResponseWithSession<>(Response.of(status, ByteString.encodeUtf8(responseBody)), session));
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
package com.rewedigital.composer.routing;

import static com.rewedigital.composer.routing.RouteTypeName.PROXY;
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;

@@ -19,6 +22,11 @@

public class RoutingConfigurationTest {

private final int timeoutInMs = 200;
private final Optional<Integer> withoutTtl = Optional.empty();
private final Optional<Integer> withTtl = Optional.of(timeoutInMs);
private final Duration ttlDuration = Duration.ofMillis(timeoutInMs);

@Test
public void allConfigParametersAreCoveredByDefaultConfig() {
final RoutingConfiguration configuration =
@@ -29,22 +37,41 @@ public void allConfigParametersAreCoveredByDefaultConfig() {
@Test
public void createsLocalRouteFromConfiguration() {
final RoutingConfiguration configuration =
RoutingConfiguration.fromConfig(configWithLocalRoutes(
localRoute("/test/path/<arg>", "GET", "https://target.service/{arg}", RouteTypeName.PROXY)));
RoutingConfiguration.fromConfig(configWithSingleLocalRoute(withoutTtl));
final List<Rule<Match>> localRules = configuration.localRules();
assertThat(localRules).anySatisfy(rule -> {
assertThat(rule.getPath()).isEqualTo("/test/path/<arg>");
assertThat(rule.getMethods()).contains("GET");
assertThat(rule.getTarget()).isEqualTo(Match.of("https://target.service/{arg}", RouteTypeName.PROXY));
final Match routeMatchWithoutTtl =
Match.of("https://target.service/{arg}", PROXY);
assertThat(rule.getTarget()).isEqualTo(routeMatchWithoutTtl);
});
}

@Test
public void setsTtlIfConfigured() {
final RoutingConfiguration configuration =
RoutingConfiguration.fromConfig(configWithSingleLocalRoute(withTtl));
final List<Rule<Match>> localRules = configuration.localRules();
assertThat(localRules).anySatisfy(rule -> {
final Match routeMatchWithTtl = Match.of("https://target.service/{arg}", ttlDuration, PROXY);
assertThat(rule.getTarget()).isEqualTo(routeMatchWithTtl);
});
}

private Config configWithSingleLocalRoute(final Optional<Integer> ttl) {
return configWithLocalRoutes(
localRoute("/test/path/<arg>", "GET", "https://target.service/{arg}", ttl, PROXY));
}


private static Map<String, Object> localRoute(final String path, final String method, final String target,
final RouteTypeName type) {
final Optional<Integer> ttl, final RouteTypeName type) {
final Map<String, Object> 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;
}
Original file line number Diff line number Diff line change
@@ -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<ByteString> response =
Response.ok().withPayload(ByteString.EMPTY).withHeader("x-rd-response-key", "other-value");

final RequestContext context = contextWith(aClient(expectedRequest, response), method);
final ResponseWithSession<ByteString> 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);