Skip to content

Commit

Permalink
Tidy up.
Browse files Browse the repository at this point in the history
Address code review comments.
Remove all safe calls from Kotlin tests.
  • Loading branch information
mikkokar committed Jun 14, 2019
1 parent ae09dbe commit a3faffb
Show file tree
Hide file tree
Showing 31 changed files with 267 additions and 224 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
import com.hotels.styx.api.HttpRequest;
import com.hotels.styx.api.HttpResponse;
import com.hotels.styx.api.WebServiceHandler;
import com.hotels.styx.routing.RoutingObjectDecorator;
import com.hotels.styx.routing.RoutingMetadataDecorator;
import com.hotels.styx.routing.RoutingObjectRecord;
import com.hotels.styx.routing.config.Builtins;
import com.hotels.styx.routing.config.RoutingObjectFactory;
Expand Down Expand Up @@ -96,9 +96,9 @@ public RoutingObjectHandler(StyxObjectStore<RoutingObjectRecord> routeDatabase,

try {
RoutingObjectDefinition payload = YAML_MAPPER.readValue(body, RoutingObjectDefinition.class);
RoutingObjectDecorator adapter = new RoutingObjectDecorator(Builtins.build(emptyList(), routingObjectFactoryContext, payload));
RoutingMetadataDecorator decorator = new RoutingMetadataDecorator(Builtins.build(emptyList(), routingObjectFactoryContext, payload));

routeDatabase.insert(name, new RoutingObjectRecord(payload.type(), ImmutableSet.copyOf(payload.tags()), payload.config(), adapter))
routeDatabase.insert(name, new RoutingObjectRecord(payload.type(), ImmutableSet.copyOf(payload.tags()), payload.config(), decorator))
.ifPresent(previous -> previous.getRoutingObject().stop());

return Eventual.of(response(CREATED).build());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
import static java.util.Objects.requireNonNull;

/**
* RegistryServiceAdapter - add a description later.
* RegistryServiceAdapter adapts BackendService registry to a Styx Service.
*/
public class RegistryServiceAdapter extends AbstractStyxService implements Registry<BackendService> {
private Registry<BackendService> delegate;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,11 @@
import static java.util.Objects.requireNonNull;

/**
* Adapts routing object into Styx core.
* Provides a load balancing metric for the adapted routing object.
* Decorates a RoutingObject instance with additional metadata. This includes, but not
* limited to, a load balancing metric. Styx core needs this metadata to use the
* routing object in the forwarding path.
*/
public class RoutingObjectDecorator implements RoutingObject {
public class RoutingMetadataDecorator implements RoutingObject {

private final RoutingObject delegate;

Expand All @@ -42,7 +43,7 @@ public class RoutingObjectDecorator implements RoutingObject {
* Routing object adapater constructor.
* @param routingObject
*/
public RoutingObjectDecorator(RoutingObject routingObject) {
public RoutingMetadataDecorator(RoutingObject routingObject) {
this.delegate = requireNonNull(routingObject);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@
import static java.nio.charset.StandardCharsets.UTF_8;

/**
* Builds a routing object based on its actual type.
* Contains mappings of builtin routing object and interceptor names to their factory methods.
*/
public class Builtins {
public final class Builtins {
public static final ImmutableMap<String, Schema.FieldType> BUILTIN_HANDLER_SCHEMAS;
public static final ImmutableMap<String, RoutingObjectFactory> BUILTIN_HANDLER_FACTORIES;
public static final RouteRefLookup DEFAULT_REFERENCE_LOOKUP = reference -> (request, ctx) ->
Expand Down Expand Up @@ -84,6 +84,9 @@ public class Builtins {
.build();
}

private Builtins() {
}

public static RoutingObject build(List<String> parents, RoutingObjectFactory.Context context, RoutingObjectConfiguration configNode) {
if (configNode instanceof RoutingObjectDefinition) {
RoutingObjectDefinition configBlock = (RoutingObjectDefinition) configNode;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,25 +28,24 @@
import static java.util.Objects.requireNonNull;

/**
* A factory for constructing HTTP handler objects from a RoutingObjectDefinition yaml config block.
* A factory for constructing Styx routing objects from a RoutingObjectDefinition
* yaml config block.
*/
public interface RoutingObjectFactory {
/**
* Constructs a terminal action handler according to routing configuration block.
* Constructs a RoutingObject instance according to configuration block.
* <p>
* Constructs a terminal action handler for the HTTP request. The handler is constructed
* according to the definition codified in the RoutingObjectDefinition instance.
* The RoutingObjectFactory is a factory object for constructing any dependant routing
* objects. The objectVariables is a map of already instantiated routing objects
* that can be referred from the handler being built.
* The routing object is constructed according to the definition codified in
* the RoutingObjectDefinition instance. Context provides access to
* core Styx components necessary for constructing dependant objects.
* <p>
*
* @param parents
* @param context
* @param configBlock
* @return
* @param fullName a fully qualified attribute name, including parents
* @param context a routing object factory context
* @param configBlock routing object configuration
* @return a RoutingObject with all dependant objects
*/
RoutingObject build(List<String> parents, Context context, RoutingObjectDefinition configBlock);
RoutingObject build(List<String> fullName, Context context, RoutingObjectDefinition configBlock);

/**
* Contextual information for factory class.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,16 +92,16 @@ public Factory(Environment environment, Map<String, Registry<BackendService>> ba
}

@Override
public RoutingObject build(List<String> parents, Context context, RoutingObjectDefinition configBlock) {
public RoutingObject build(List<String> fullName, Context context, RoutingObjectDefinition configBlock) {
JsonNodeConfig config = new JsonNodeConfig(configBlock.config());
String provider = config.get("backendProvider")
.orElseThrow(() -> missingAttributeError(configBlock, join(".", parents), "backendProvider"));
.orElseThrow(() -> missingAttributeError(configBlock, join(".", fullName), "backendProvider"));

Registry<BackendService> registry = backendRegistries.get(provider);
if (registry == null) {
throw new IllegalArgumentException(
format("No such backend service provider exists, attribute='%s', name='%s'",
join(".", append(parents, "backendProvider")), provider));
join(".", append(fullName, "backendProvider")), provider));
}

PlatformAwareClientEventLoopGroupFactory factory = new PlatformAwareClientEventLoopGroupFactory("BackendServiceProxy", 0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,23 +122,23 @@ private static Route buildRoute(
}

@Override
public RoutingObject build(List<String> parents, Context context, RoutingObjectDefinition configBlock) {
public RoutingObject build(List<String> fullName, Context context, RoutingObjectDefinition configBlock) {
ConditionRouterConfig config = new JsonNodeConfig(configBlock.config()).as(ConditionRouterConfig.class);
if (config.routes == null) {
throw missingAttributeError(configBlock, join(".", parents), "routes");
throw missingAttributeError(configBlock, join(".", fullName), "routes");
}

AtomicInteger index = new AtomicInteger(0);
List<Route> routes = config.routes.stream()
.map(routeConfig -> buildRoute(
append(parents, "routes"),
append(fullName, "routes"),
context,
index.getAndIncrement(),
routeConfig.condition,
routeConfig.destination))
.collect(Collectors.toList());

RoutingObject fallbackHandler = buildFallbackHandler(parents, context, config);
RoutingObject fallbackHandler = buildFallbackHandler(fullName, context, config);

ConditionRouter router = new ConditionRouter(routes, fallbackHandler);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ public static class Factory implements RoutingObjectFactory {
private static final int DEFAULT_HTTP_PORT = 80;

@Override
public RoutingObject build(List<String> parents, Context context, RoutingObjectDefinition configBlock) {
public RoutingObject build(List<String> fullName, Context context, RoutingObjectDefinition configBlock) {
JsonNodeConfig config = new JsonNodeConfig(configBlock.config());

ConnectionPoolSettings poolSettings = config.get("connectionPool", ConnectionPoolSettings.class)
Expand All @@ -166,7 +166,7 @@ public RoutingObject build(List<String> parents, Context context, RoutingObjectD
HostAndPort hostAndPort = config.get("host")
.map(HostAndPort::fromString)
.map(it -> addDefaultPort(it, tlsSettings))
.orElseThrow(() -> missingAttributeError(configBlock, join(".", parents), "host"));
.orElseThrow(() -> missingAttributeError(configBlock, join(".", fullName), "host"));

return createHostProxyHandler(context, hostAndPort, poolSettings, tlsSettings, responseTimeoutMillis, metricPrefix);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,17 +87,17 @@ public CompletableFuture<Void> stop() {
public static class Factory implements RoutingObjectFactory {

@Override
public RoutingObject build(List<String> parents, Context context, RoutingObjectDefinition configBlock) {
public RoutingObject build(List<String> fullName, Context context, RoutingObjectDefinition configBlock) {
JsonNode pipeline = configBlock.config().get("pipeline");
List<HttpInterceptor> interceptors = getHttpInterceptors(append(parents, "pipeline"), toMap(context.plugins()), context.interceptorFactories(), pipeline);
List<HttpInterceptor> interceptors = getHttpInterceptors(append(fullName, "pipeline"), toMap(context.plugins()), context.interceptorFactories(), pipeline);

JsonNode handlerConfig = new JsonNodeConfig(configBlock.config())
.get("handler", JsonNode.class)
.orElseThrow(() -> missingAttributeError(configBlock, join(".", parents), "handler"));
.orElseThrow(() -> missingAttributeError(configBlock, join(".", fullName), "handler"));

return new HttpInterceptorPipeline(
interceptors,
Builtins.build(append(parents, "handler"), context, toRoutingConfigNode(handlerConfig)),
Builtins.build(append(fullName, "handler"), context, toRoutingConfigNode(handlerConfig)),
context.requestTracking());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,10 @@ public Optional<RoutingObject> route(LiveHttpRequest request) {
public static class Factory implements RoutingObjectFactory {

@Override
public RoutingObject build(List<String> parents, Context context, RoutingObjectDefinition configBlock) {
public RoutingObject build(List<String> fullName, Context context, RoutingObjectDefinition configBlock) {
PathPrefixRouterConfig config = new JsonNodeConfig(configBlock.config()).as(PathPrefixRouterConfig.class);
if (config.routes == null) {
throw missingAttributeError(configBlock, join(".", parents), "routes");
throw missingAttributeError(configBlock, join(".", fullName), "routes");
}

PathPrefixRouter pathPrefixRouter = new PathPrefixRouter(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,8 @@ static RoutingObject build(List<String> parents, Context context, RoutingObjectD
}

@Override
public RoutingObject build(List<String> parents, Context context, RoutingObjectDefinition configBlock) {
return build(parents, context, configBlock, new StyxBackendServiceClientFactory(context.environment()));
public RoutingObject build(List<String> fullName, Context context, RoutingObjectDefinition configBlock) {
return build(fullName, context, configBlock, new StyxBackendServiceClientFactory(context.environment()));
}

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public StaticResponseConfig(@JsonProperty("status") int status,
* Builds a static response handler from Yaml configuration.
*/
public static class Factory implements RoutingObjectFactory {
public RoutingObject build(List<String> parents, Context context, RoutingObjectDefinition configBlock) {
public RoutingObject build(List<String> fullName, Context context, RoutingObjectDefinition configBlock) {
requireNonNull(configBlock.config());

StaticResponseConfig config = new JsonNodeConfig(configBlock.config())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
import com.hotels.styx.client.netty.eventloop.PlatformAwareClientEventLoopGroupFactory;
import com.hotels.styx.infrastructure.configuration.yaml.JsonNodeConfig;
import com.hotels.styx.proxy.plugin.NamedPlugin;
import com.hotels.styx.routing.RoutingObjectDecorator;
import com.hotels.styx.routing.RoutingMetadataDecorator;
import com.hotels.styx.routing.RoutingObjectRecord;
import com.hotels.styx.routing.config.Builtins;
import com.hotels.styx.routing.config.RoutingObjectFactory;
Expand Down Expand Up @@ -109,7 +109,7 @@ private StyxServerComponents(Builder builder) {
.map(StyxServerComponents::readHttpHandlers)
.orElse(ImmutableMap.of())
.forEach((name, record) -> {
RoutingObjectDecorator adapter = new RoutingObjectDecorator(Builtins.build(ImmutableList.of(name), routingObjectContext, record));
RoutingMetadataDecorator adapter = new RoutingMetadataDecorator(Builtins.build(ImmutableList.of(name), routingObjectContext, record));
routeObjectStore.insert(name, new RoutingObjectRecord(record.type(), ImmutableSet.copyOf(record.tags()), record.config(), adapter))
.ifPresent(previous -> previous.getRoutingObject().stop());
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,18 @@ package com.hotels.styx.routing
import com.fasterxml.jackson.databind.JsonNode

/**
* Holds routing object and its associated metadata.
* A routing object and its associated configuration metadata.
*/
internal data class RoutingObjectRecord(
val type: String,
val tags: Set<String>,
val config: JsonNode,
val routingObject: RoutingObjectDecorator) {
val routingObject: RoutingMetadataDecorator) {
companion object {
fun create(type: String, tags: Set<String>, config: JsonNode, routingObject: RoutingObject) = RoutingObjectRecord(
type,
tags,
config,
RoutingObjectDecorator(routingObject))
RoutingMetadataDecorator(routingObject))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ import java.util.concurrent.atomic.AtomicReference
*/
internal class LoadBalancingGroup(val client: StyxBackendServiceClient, val changeWatcher: Disposable) : RoutingObject {

override fun handle(request: LiveHttpRequest?, context: HttpInterceptor.Context?) = Eventual(client.sendRequest(request))
override fun handle(request: LiveHttpRequest, context: HttpInterceptor.Context) = Eventual(client.sendRequest(request))

override fun stop(): CompletableFuture<Void> {
changeWatcher.dispose()
Expand All @@ -80,9 +80,9 @@ internal class LoadBalancingGroup(val client: StyxBackendServiceClient, val chan
}

class Factory : RoutingObjectFactory {
override fun build(parents: List<String>, context: RoutingObjectFactory.Context, configBlock: RoutingObjectDefinition): RoutingObject {
override fun build(fullName: List<String>, context: RoutingObjectFactory.Context, configBlock: RoutingObjectDefinition): RoutingObject {

val appId = parents.last()
val appId = fullName.last()
val config = JsonNodeConfig(configBlock.config()).`as`(Config::class.java)

val routeDb = context.routeDb()
Expand Down
Loading

0 comments on commit a3faffb

Please sign in to comment.