From 3eb9b255cd8a438f6078a2dc68a0f134c4a2e733 Mon Sep 17 00:00:00 2001 From: "tim.quinn@oracle.com" Date: Fri, 6 Sep 2019 11:54:15 -0500 Subject: [PATCH 1/5] Refactor the routing decision-making that was repeated in health and metrics MP services into one place; add use of that to OpenAPI MP service --- .../microprofile/health/HealthMpService.java | 20 +--- .../metrics/MetricsMpService.java | 21 +--- .../openapi/OpenAPIMpService.java | 3 +- .../microprofile/server/RoutingBuilders.java | 107 ++++++++++++++++++ 4 files changed, 117 insertions(+), 34 deletions(-) create mode 100644 microprofile/server/src/main/java/io/helidon/microprofile/server/RoutingBuilders.java diff --git a/microprofile/health/src/main/java/io/helidon/microprofile/health/HealthMpService.java b/microprofile/health/src/main/java/io/helidon/microprofile/health/HealthMpService.java index 39dfe894418..40923da3e00 100644 --- a/microprofile/health/src/main/java/io/helidon/microprofile/health/HealthMpService.java +++ b/microprofile/health/src/main/java/io/helidon/microprofile/health/HealthMpService.java @@ -17,7 +17,6 @@ package io.helidon.microprofile.health; import java.lang.annotation.Annotation; -import java.util.Optional; import java.util.ServiceLoader; import javax.enterprise.inject.se.SeContainer; @@ -25,6 +24,7 @@ import io.helidon.common.serviceloader.HelidonServiceLoader; import io.helidon.config.Config; import io.helidon.health.HealthSupport; +import io.helidon.microprofile.server.RoutingBuilders; import io.helidon.microprofile.server.spi.MpService; import io.helidon.microprofile.server.spi.MpServiceContext; @@ -85,22 +85,8 @@ public void configure(MpServiceContext mpServiceContext) { healthCheckProvider.readinessChecks().forEach(builder::addReadiness); }); - healthConfig.get("routing") - .asString() - .flatMap(routeName -> { - // support for overriding the routing back to default port using config - if ("@default".equals(routeName)) { - return Optional.empty(); - } else { - return Optional.of(routeName); - } - }) - // use named routing - .map(mpServiceContext::serverNamedRoutingBuilder) - // use default server routing - .orElseGet(mpServiceContext::serverRoutingBuilder) - // register health support + RoutingBuilders.newRoutingBuilders(mpServiceContext, "health") + .builder() .register(builder.build()); - } } diff --git a/microprofile/metrics/src/main/java/io/helidon/microprofile/metrics/MetricsMpService.java b/microprofile/metrics/src/main/java/io/helidon/microprofile/metrics/MetricsMpService.java index 736a1c0f22e..b43be1561e9 100644 --- a/microprofile/metrics/src/main/java/io/helidon/microprofile/metrics/MetricsMpService.java +++ b/microprofile/metrics/src/main/java/io/helidon/microprofile/metrics/MetricsMpService.java @@ -21,12 +21,11 @@ import io.helidon.common.CollectionsHelper; import io.helidon.config.Config; -import io.helidon.config.ConfigValue; import io.helidon.metrics.MetricsSupport; import io.helidon.metrics.RegistryFactory; +import io.helidon.microprofile.server.RoutingBuilders; import io.helidon.microprofile.server.spi.MpService; import io.helidon.microprofile.server.spi.MpServiceContext; -import io.helidon.webserver.Routing; /** * Extension of microprofile {@link io.helidon.microprofile.server.Server} to enable support for metrics @@ -46,22 +45,12 @@ public void configure(MpServiceContext serviceContext) { MetricsSupport metricsSupport = MetricsSupport.create(metricsConfig); - ConfigValue routingNameConfig = metricsConfig.get("routing").asString(); - Routing.Builder defaultRouting = serviceContext.serverRoutingBuilder(); + RoutingBuilders routingBuilders = RoutingBuilders.newRoutingBuilders( + serviceContext, metricsConfig); - Routing.Builder endpointRouting = defaultRouting; - - if (routingNameConfig.isPresent()) { - String routingName = routingNameConfig.get(); - // support for overriding this back to default routing using config - if (!"@default".equals(routingName)) { - endpointRouting = serviceContext.serverNamedRoutingBuilder(routingName); - } - } - - metricsSupport.configureVendorMetrics(null, defaultRouting); + metricsSupport.configureVendorMetrics(null, routingBuilders.defaultBuilder()); vendorMetricsAdded.add("@default"); - metricsSupport.configureEndpoint(endpointRouting); + metricsSupport.configureEndpoint(routingBuilders.builder()); // now we may have additional sockets we want to add vendor metrics to metricsConfig.get("vendor-metrics-routings") diff --git a/microprofile/openapi/src/main/java/io/helidon/microprofile/openapi/OpenAPIMpService.java b/microprofile/openapi/src/main/java/io/helidon/microprofile/openapi/OpenAPIMpService.java index a70ed1aadd9..381c9ae72cb 100644 --- a/microprofile/openapi/src/main/java/io/helidon/microprofile/openapi/OpenAPIMpService.java +++ b/microprofile/openapi/src/main/java/io/helidon/microprofile/openapi/OpenAPIMpService.java @@ -18,6 +18,7 @@ import java.io.IOException; +import io.helidon.microprofile.server.RoutingBuilders; import io.helidon.microprofile.server.spi.MpService; import io.helidon.microprofile.server.spi.MpServiceContext; import io.helidon.openapi.OpenAPISupport; @@ -44,6 +45,6 @@ public void configure(MpServiceContext context) { throw new RuntimeException(ex); } - openAPISupport.configureEndpoint(context.serverRoutingBuilder()); + openAPISupport.configureEndpoint(RoutingBuilders.newRoutingBuilders(context, "openapi").builder()); } } diff --git a/microprofile/server/src/main/java/io/helidon/microprofile/server/RoutingBuilders.java b/microprofile/server/src/main/java/io/helidon/microprofile/server/RoutingBuilders.java new file mode 100644 index 00000000000..83018be8d96 --- /dev/null +++ b/microprofile/server/src/main/java/io/helidon/microprofile/server/RoutingBuilders.java @@ -0,0 +1,107 @@ +/* + * Copyright (c) 2019 Oracle and/or its affiliates. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ +package io.helidon.microprofile.server; + +import io.helidon.config.Config; +import io.helidon.microprofile.server.spi.MpServiceContext; +import io.helidon.webserver.Routing; +import java.util.Optional; + +/** + * Provides {@link Routing.Builder} instances (for the default and the actual) + * for a Helidon MP service, based on configuration for the component (if any) + * and defaults otherwise. + */ +public interface RoutingBuilders { + + /** + * + * @return the default {@code Routing.Builder} for the component + */ + Routing.Builder defaultBuilder(); + + /** + * + * @return the actual {@code Routing.Builder} for the component; might be the default + */ + Routing.Builder builder(); + + /** + * Prepares the default and actual {@link Routing.Builder} instances based + * on the "routing" configuration for the specific component. + * + * @param context the {@code MpServiceContext} for the calling service + * @param componentName config key under which "routing" config might exist for the component of interest + * @return {@code RoutingBuilders} based on the named config (or default) + */ + static RoutingBuilders newRoutingBuilders(MpServiceContext context, String componentName) { + return newRoutingBuilders(context, context.helidonConfig().get(componentName)); + } + + /** + * Prepares the default and actual {@link Routing.Builder} instances based + * on the "routing" configuration for the specific component configuration. + * + * @param context the {@code MpServiceContext} for the calling service + * @param componentConfig the configuration for the calling service + * @return {@code RoutingBuilders} based on the config (or default) + */ + static RoutingBuilders newRoutingBuilders(MpServiceContext context, Config componentConfig) { + final Routing.Builder defaultRoutingBuilder = context.serverRoutingBuilder(); + final Routing.Builder actualRoutingBuilder + = componentConfig.get("routing") + .asString() + .flatMap(routeName -> { + // support for overriding the routing back to default port using config + if ("@default".equals(routeName)) { + return Optional.empty(); + } else { + return Optional.of(routeName); + } + }) + // use named routing + .map(context::serverNamedRoutingBuilder) + // use default server routing + .orElse(defaultRoutingBuilder); + return new RoutingBuildersImpl(defaultRoutingBuilder, actualRoutingBuilder); + } + + /** + * Package-private implementation of the {@code RoutingBuilders} interface. + */ + class RoutingBuildersImpl implements RoutingBuilders { + + private final Routing.Builder defaultBuilder; + private final Routing.Builder effectiveBuilder; + + private RoutingBuildersImpl(Routing.Builder defaultBuilder, + Routing.Builder effectiveBuilder) { + this.defaultBuilder = defaultBuilder; + this.effectiveBuilder = effectiveBuilder; + } + + @Override + public Routing.Builder defaultBuilder() { + return defaultBuilder; + } + + @Override + public Routing.Builder builder() { + return effectiveBuilder; + } + } +} From 4c934ba647e5d773db39a3f708e811e236d1f848 Mon Sep 17 00:00:00 2001 From: "tim.quinn@oracle.com" Date: Fri, 6 Sep 2019 11:55:34 -0500 Subject: [PATCH 2/5] Include latest change to health MP service --- .../java/io/helidon/microprofile/health/HealthMpService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/microprofile/health/src/main/java/io/helidon/microprofile/health/HealthMpService.java b/microprofile/health/src/main/java/io/helidon/microprofile/health/HealthMpService.java index 40923da3e00..aa38298edbf 100644 --- a/microprofile/health/src/main/java/io/helidon/microprofile/health/HealthMpService.java +++ b/microprofile/health/src/main/java/io/helidon/microprofile/health/HealthMpService.java @@ -85,7 +85,7 @@ public void configure(MpServiceContext mpServiceContext) { healthCheckProvider.readinessChecks().forEach(builder::addReadiness); }); - RoutingBuilders.newRoutingBuilders(mpServiceContext, "health") + RoutingBuilders.newRoutingBuilders(mpServiceContext, healthConfig) .builder() .register(builder.build()); } From eafd39610591ad13c22e52a4225094507ef7533b Mon Sep 17 00:00:00 2001 From: "tim.quinn@oracle.com" Date: Fri, 6 Sep 2019 12:03:43 -0500 Subject: [PATCH 3/5] Fix import order --- .../java/io/helidon/microprofile/server/RoutingBuilders.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/microprofile/server/src/main/java/io/helidon/microprofile/server/RoutingBuilders.java b/microprofile/server/src/main/java/io/helidon/microprofile/server/RoutingBuilders.java index 83018be8d96..91d2983bfcf 100644 --- a/microprofile/server/src/main/java/io/helidon/microprofile/server/RoutingBuilders.java +++ b/microprofile/server/src/main/java/io/helidon/microprofile/server/RoutingBuilders.java @@ -16,10 +16,11 @@ */ package io.helidon.microprofile.server; +import java.util.Optional; + import io.helidon.config.Config; import io.helidon.microprofile.server.spi.MpServiceContext; import io.helidon.webserver.Routing; -import java.util.Optional; /** * Provides {@link Routing.Builder} instances (for the default and the actual) From 6b4d1ba23fd249dbc111120e1ea3c75306b1daf7 Mon Sep 17 00:00:00 2001 From: "tim.quinn@oracle.com" Date: Mon, 9 Sep 2019 11:23:59 -0500 Subject: [PATCH 4/5] Incorporate review comments: move impl class to its own pkg private class; rename the factory methods to create... instead of new...; change the accessor method names for clarity --- .../microprofile/health/HealthMpService.java | 4 +- .../metrics/MetricsMpService.java | 6 +-- .../openapi/OpenAPIMpService.java | 2 +- .../microprofile/server/RoutingBuilders.java | 35 +++------------ .../server/RoutingBuildersImpl.java | 43 +++++++++++++++++++ 5 files changed, 54 insertions(+), 36 deletions(-) create mode 100644 microprofile/server/src/main/java/io/helidon/microprofile/server/RoutingBuildersImpl.java diff --git a/microprofile/health/src/main/java/io/helidon/microprofile/health/HealthMpService.java b/microprofile/health/src/main/java/io/helidon/microprofile/health/HealthMpService.java index 177d3c21464..8884c0d55fe 100644 --- a/microprofile/health/src/main/java/io/helidon/microprofile/health/HealthMpService.java +++ b/microprofile/health/src/main/java/io/helidon/microprofile/health/HealthMpService.java @@ -94,8 +94,8 @@ public void configure(MpServiceContext mpServiceContext) { healthCheckProvider.readinessChecks().forEach(builder::addReadiness); }); - RoutingBuilders.newRoutingBuilders(mpServiceContext, healthConfig) - .builder() + RoutingBuilders.createRoutingBuilders(mpServiceContext, healthConfig) + .routingBuilder() .register(builder.build()); } } diff --git a/microprofile/metrics/src/main/java/io/helidon/microprofile/metrics/MetricsMpService.java b/microprofile/metrics/src/main/java/io/helidon/microprofile/metrics/MetricsMpService.java index b43be1561e9..cf34d0a7dd2 100644 --- a/microprofile/metrics/src/main/java/io/helidon/microprofile/metrics/MetricsMpService.java +++ b/microprofile/metrics/src/main/java/io/helidon/microprofile/metrics/MetricsMpService.java @@ -45,12 +45,12 @@ public void configure(MpServiceContext serviceContext) { MetricsSupport metricsSupport = MetricsSupport.create(metricsConfig); - RoutingBuilders routingBuilders = RoutingBuilders.newRoutingBuilders( + RoutingBuilders routingBuilders = RoutingBuilders.createRoutingBuilders( serviceContext, metricsConfig); - metricsSupport.configureVendorMetrics(null, routingBuilders.defaultBuilder()); + metricsSupport.configureVendorMetrics(null, routingBuilders.defaultRoutingBuilder()); vendorMetricsAdded.add("@default"); - metricsSupport.configureEndpoint(routingBuilders.builder()); + metricsSupport.configureEndpoint(routingBuilders.routingBuilder()); // now we may have additional sockets we want to add vendor metrics to metricsConfig.get("vendor-metrics-routings") diff --git a/microprofile/openapi/src/main/java/io/helidon/microprofile/openapi/OpenAPIMpService.java b/microprofile/openapi/src/main/java/io/helidon/microprofile/openapi/OpenAPIMpService.java index 381c9ae72cb..a92dff19f3f 100644 --- a/microprofile/openapi/src/main/java/io/helidon/microprofile/openapi/OpenAPIMpService.java +++ b/microprofile/openapi/src/main/java/io/helidon/microprofile/openapi/OpenAPIMpService.java @@ -45,6 +45,6 @@ public void configure(MpServiceContext context) { throw new RuntimeException(ex); } - openAPISupport.configureEndpoint(RoutingBuilders.newRoutingBuilders(context, "openapi").builder()); + openAPISupport.configureEndpoint(RoutingBuilders.createRoutingBuilders(context, "openapi").routingBuilder()); } } diff --git a/microprofile/server/src/main/java/io/helidon/microprofile/server/RoutingBuilders.java b/microprofile/server/src/main/java/io/helidon/microprofile/server/RoutingBuilders.java index 91d2983bfcf..b8876788a4e 100644 --- a/microprofile/server/src/main/java/io/helidon/microprofile/server/RoutingBuilders.java +++ b/microprofile/server/src/main/java/io/helidon/microprofile/server/RoutingBuilders.java @@ -33,13 +33,13 @@ public interface RoutingBuilders { * * @return the default {@code Routing.Builder} for the component */ - Routing.Builder defaultBuilder(); + Routing.Builder defaultRoutingBuilder(); /** * * @return the actual {@code Routing.Builder} for the component; might be the default */ - Routing.Builder builder(); + Routing.Builder routingBuilder(); /** * Prepares the default and actual {@link Routing.Builder} instances based @@ -49,8 +49,8 @@ public interface RoutingBuilders { * @param componentName config key under which "routing" config might exist for the component of interest * @return {@code RoutingBuilders} based on the named config (or default) */ - static RoutingBuilders newRoutingBuilders(MpServiceContext context, String componentName) { - return newRoutingBuilders(context, context.helidonConfig().get(componentName)); + static RoutingBuilders createRoutingBuilders(MpServiceContext context, String componentName) { + return createRoutingBuilders(context, context.helidonConfig().get(componentName)); } /** @@ -61,7 +61,7 @@ static RoutingBuilders newRoutingBuilders(MpServiceContext context, String compo * @param componentConfig the configuration for the calling service * @return {@code RoutingBuilders} based on the config (or default) */ - static RoutingBuilders newRoutingBuilders(MpServiceContext context, Config componentConfig) { + static RoutingBuilders createRoutingBuilders(MpServiceContext context, Config componentConfig) { final Routing.Builder defaultRoutingBuilder = context.serverRoutingBuilder(); final Routing.Builder actualRoutingBuilder = componentConfig.get("routing") @@ -80,29 +80,4 @@ static RoutingBuilders newRoutingBuilders(MpServiceContext context, Config compo .orElse(defaultRoutingBuilder); return new RoutingBuildersImpl(defaultRoutingBuilder, actualRoutingBuilder); } - - /** - * Package-private implementation of the {@code RoutingBuilders} interface. - */ - class RoutingBuildersImpl implements RoutingBuilders { - - private final Routing.Builder defaultBuilder; - private final Routing.Builder effectiveBuilder; - - private RoutingBuildersImpl(Routing.Builder defaultBuilder, - Routing.Builder effectiveBuilder) { - this.defaultBuilder = defaultBuilder; - this.effectiveBuilder = effectiveBuilder; - } - - @Override - public Routing.Builder defaultBuilder() { - return defaultBuilder; - } - - @Override - public Routing.Builder builder() { - return effectiveBuilder; - } - } } diff --git a/microprofile/server/src/main/java/io/helidon/microprofile/server/RoutingBuildersImpl.java b/microprofile/server/src/main/java/io/helidon/microprofile/server/RoutingBuildersImpl.java new file mode 100644 index 00000000000..0d48dc14bf9 --- /dev/null +++ b/microprofile/server/src/main/java/io/helidon/microprofile/server/RoutingBuildersImpl.java @@ -0,0 +1,43 @@ +/* + * Copyright (c) 2019 Oracle and/or its affiliates. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ +package io.helidon.microprofile.server; + +import io.helidon.webserver.Routing; + +/** + * Package-private implementation of the {@code RoutingBuilders} interface. + */ +class RoutingBuildersImpl implements RoutingBuilders { + + private final Routing.Builder defaultBuilder; + private final Routing.Builder effectiveBuilder; + + RoutingBuildersImpl(Routing.Builder defaultBuilder, Routing.Builder effectiveBuilder) { + this.defaultBuilder = defaultBuilder; + this.effectiveBuilder = effectiveBuilder; + } + + @Override + public Routing.Builder defaultRoutingBuilder() { + return defaultBuilder; + } + + @Override + public Routing.Builder routingBuilder() { + return effectiveBuilder; + } +} From a443cfb78adf2bce67c0ecc01d87dd85fbc14204 Mon Sep 17 00:00:00 2001 From: "tim.quinn@oracle.com" Date: Mon, 9 Sep 2019 16:04:55 -0500 Subject: [PATCH 5/5] Rename factory methods again (misread the comment on the PR earlier) --- .../io/helidon/microprofile/health/HealthMpService.java | 2 +- .../io/helidon/microprofile/metrics/MetricsMpService.java | 2 +- .../io/helidon/microprofile/openapi/OpenAPIMpService.java | 2 +- .../io/helidon/microprofile/server/RoutingBuilders.java | 6 +++--- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/microprofile/health/src/main/java/io/helidon/microprofile/health/HealthMpService.java b/microprofile/health/src/main/java/io/helidon/microprofile/health/HealthMpService.java index 8884c0d55fe..d3414605b0d 100644 --- a/microprofile/health/src/main/java/io/helidon/microprofile/health/HealthMpService.java +++ b/microprofile/health/src/main/java/io/helidon/microprofile/health/HealthMpService.java @@ -94,7 +94,7 @@ public void configure(MpServiceContext mpServiceContext) { healthCheckProvider.readinessChecks().forEach(builder::addReadiness); }); - RoutingBuilders.createRoutingBuilders(mpServiceContext, healthConfig) + RoutingBuilders.create(mpServiceContext, healthConfig) .routingBuilder() .register(builder.build()); } diff --git a/microprofile/metrics/src/main/java/io/helidon/microprofile/metrics/MetricsMpService.java b/microprofile/metrics/src/main/java/io/helidon/microprofile/metrics/MetricsMpService.java index cf34d0a7dd2..3ce17a33317 100644 --- a/microprofile/metrics/src/main/java/io/helidon/microprofile/metrics/MetricsMpService.java +++ b/microprofile/metrics/src/main/java/io/helidon/microprofile/metrics/MetricsMpService.java @@ -45,7 +45,7 @@ public void configure(MpServiceContext serviceContext) { MetricsSupport metricsSupport = MetricsSupport.create(metricsConfig); - RoutingBuilders routingBuilders = RoutingBuilders.createRoutingBuilders( + RoutingBuilders routingBuilders = RoutingBuilders.create( serviceContext, metricsConfig); metricsSupport.configureVendorMetrics(null, routingBuilders.defaultRoutingBuilder()); diff --git a/microprofile/openapi/src/main/java/io/helidon/microprofile/openapi/OpenAPIMpService.java b/microprofile/openapi/src/main/java/io/helidon/microprofile/openapi/OpenAPIMpService.java index a92dff19f3f..b02156fc46e 100644 --- a/microprofile/openapi/src/main/java/io/helidon/microprofile/openapi/OpenAPIMpService.java +++ b/microprofile/openapi/src/main/java/io/helidon/microprofile/openapi/OpenAPIMpService.java @@ -45,6 +45,6 @@ public void configure(MpServiceContext context) { throw new RuntimeException(ex); } - openAPISupport.configureEndpoint(RoutingBuilders.createRoutingBuilders(context, "openapi").routingBuilder()); + openAPISupport.configureEndpoint(RoutingBuilders.create(context, "openapi").routingBuilder()); } } diff --git a/microprofile/server/src/main/java/io/helidon/microprofile/server/RoutingBuilders.java b/microprofile/server/src/main/java/io/helidon/microprofile/server/RoutingBuilders.java index b8876788a4e..b65705c0c7f 100644 --- a/microprofile/server/src/main/java/io/helidon/microprofile/server/RoutingBuilders.java +++ b/microprofile/server/src/main/java/io/helidon/microprofile/server/RoutingBuilders.java @@ -49,8 +49,8 @@ public interface RoutingBuilders { * @param componentName config key under which "routing" config might exist for the component of interest * @return {@code RoutingBuilders} based on the named config (or default) */ - static RoutingBuilders createRoutingBuilders(MpServiceContext context, String componentName) { - return createRoutingBuilders(context, context.helidonConfig().get(componentName)); + static RoutingBuilders create(MpServiceContext context, String componentName) { + return create(context, context.helidonConfig().get(componentName)); } /** @@ -61,7 +61,7 @@ static RoutingBuilders createRoutingBuilders(MpServiceContext context, String co * @param componentConfig the configuration for the calling service * @return {@code RoutingBuilders} based on the config (or default) */ - static RoutingBuilders createRoutingBuilders(MpServiceContext context, Config componentConfig) { + static RoutingBuilders create(MpServiceContext context, Config componentConfig) { final Routing.Builder defaultRoutingBuilder = context.serverRoutingBuilder(); final Routing.Builder actualRoutingBuilder = componentConfig.get("routing")