From a10428d6749cf2d5a69074cf7ac625e004d6f54c Mon Sep 17 00:00:00 2001 From: Laird Nelson Date: Wed, 17 Jul 2019 17:07:03 -0700 Subject: [PATCH 1/2] Interim checkin Signed-off-by: Laird Nelson --- integrations/cdi/datasource-hikaricp/pom.xml | 18 +++- .../hikaricp/cdi/TestConfiguration.java | 90 +++++++++++++++++++ .../src/test/logging.properties | 6 ++ .../src/test/resources/application.yaml | 10 +++ 4 files changed, 122 insertions(+), 2 deletions(-) create mode 100644 integrations/cdi/datasource-hikaricp/src/test/java/io/helidon/integrations/datasource/hikaricp/cdi/TestConfiguration.java create mode 100644 integrations/cdi/datasource-hikaricp/src/test/logging.properties create mode 100644 integrations/cdi/datasource-hikaricp/src/test/resources/application.yaml diff --git a/integrations/cdi/datasource-hikaricp/pom.xml b/integrations/cdi/datasource-hikaricp/pom.xml index effa9981407..d1285d94786 100644 --- a/integrations/cdi/datasource-hikaricp/pom.xml +++ b/integrations/cdi/datasource-hikaricp/pom.xml @@ -122,9 +122,23 @@ test - org.slf4j - slf4j-simple + io.helidon.microprofile.server + helidon-microprofile-server + ${project.version} test + + + + + maven-surefire-plugin + + + ${project.basedir}/src/test/logging.properties + + + + + diff --git a/integrations/cdi/datasource-hikaricp/src/test/java/io/helidon/integrations/datasource/hikaricp/cdi/TestConfiguration.java b/integrations/cdi/datasource-hikaricp/src/test/java/io/helidon/integrations/datasource/hikaricp/cdi/TestConfiguration.java new file mode 100644 index 00000000000..5e0d5e97c0a --- /dev/null +++ b/integrations/cdi/datasource-hikaricp/src/test/java/io/helidon/integrations/datasource/hikaricp/cdi/TestConfiguration.java @@ -0,0 +1,90 @@ +/* + * Copyright (c) 2018, 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.integrations.datasource.hikaricp.cdi; + +import javax.enterprise.context.ApplicationScoped; +import javax.enterprise.context.Initialized; +import javax.enterprise.event.Observes; +import javax.enterprise.inject.se.SeContainer; +import javax.enterprise.inject.se.SeContainerInitializer; +import javax.inject.Inject; +import javax.inject.Named; +import javax.sql.DataSource; + +import io.helidon.microprofile.config.MpConfig; +import io.helidon.microprofile.server.Server; + +import org.eclipse.microprofile.config.spi.ConfigProviderResolver; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; + +@ApplicationScoped +class TestConfiguration { + + @Inject + @Named("test") + private DataSource test; + + private Server server; + + TestConfiguration() { + super(); + } + + @BeforeEach + void startServer() { + this.stopServer(); + // The sequence of calls here is the only way to supply a CDI + // container to Helidon's MicroProfile Server such that all of + // the normal configuration is loaded properly. + final Server.Builder builder = Server.builder(); + assertNotNull(builder); + builder.config((MpConfig) ConfigProviderResolver.instance().getConfig(Thread.currentThread().getContextClassLoader())); + final SeContainerInitializer initializer = SeContainerInitializer.newInstance() + .addBeanClasses(TestConfiguration.class); + assertNotNull(initializer); + builder.cdiContainer(initializer.initialize()); + this.server = builder.build(); + assertNotNull(this.server); + this.server.start(); + } + + @AfterEach + void stopServer() { + if (this.server != null) { + this.server.stop(); + this.server = null; + } + } + + private void onStartup(@Observes @Initialized(ApplicationScoped.class) final Object event) { + assertNotNull(this.test); + assertNotNull(this.test.toString()); + } + + @Test + void testConfiguration() { + + } + +} diff --git a/integrations/cdi/datasource-hikaricp/src/test/logging.properties b/integrations/cdi/datasource-hikaricp/src/test/logging.properties new file mode 100644 index 00000000000..7647d00eb81 --- /dev/null +++ b/integrations/cdi/datasource-hikaricp/src/test/logging.properties @@ -0,0 +1,6 @@ +.level=INFO +handlers=java.util.logging.ConsoleHandler +java.util.logging.ConsoleHandler.formatter=java.util.logging.SimpleFormatter +java.util.logging.ConsoleHandler.level=FINEST + +io.helidon.level=FINER diff --git a/integrations/cdi/datasource-hikaricp/src/test/resources/application.yaml b/integrations/cdi/datasource-hikaricp/src/test/resources/application.yaml new file mode 100644 index 00000000000..b93334fa055 --- /dev/null +++ b/integrations/cdi/datasource-hikaricp/src/test/resources/application.yaml @@ -0,0 +1,10 @@ +javax: + sql: + DataSource: + test: + dataSourceClassName: org.postgresql.ds.PGPoolingDataSource + dataSource: + url: jdbc:postgresql://127.0.0.1:5432/hello_world + user: benchmarkdbuser + password: benchmarkdbpass + From 04562e76fd2e770f450bd563fce2ac772910782d Mon Sep 17 00:00:00 2001 From: Laird Nelson Date: Wed, 17 Jul 2019 19:28:38 -0700 Subject: [PATCH 2/2] Restores the ability to use application.yaml as a final source of configuration after all MicroProfile ConfigSources (including META-INF/microprofile-config.properties) in data source injection integration projects. Signed-off-by: Laird Nelson --- integrations/cdi/datasource-hikaricp/pom.xml | 8 +- .../hikaricp/cdi/TestConfiguration.java | 25 +++--- .../src/test/logging.properties | 15 +++- .../src/test/resources/application.yaml | 21 ++++- .../ucp/cdi/UCPBackedDataSourceExtension.java | 2 - .../cdi/AbstractDataSourceExtension.java | 82 +++++++++++++++++-- 6 files changed, 126 insertions(+), 27 deletions(-) diff --git a/integrations/cdi/datasource-hikaricp/pom.xml b/integrations/cdi/datasource-hikaricp/pom.xml index d1285d94786..6eb10543a2d 100644 --- a/integrations/cdi/datasource-hikaricp/pom.xml +++ b/integrations/cdi/datasource-hikaricp/pom.xml @@ -34,6 +34,7 @@ + io.helidon.integrations.cdi @@ -127,6 +128,7 @@ ${project.version} test + @@ -134,8 +136,12 @@ maven-surefire-plugin + + io.helidon.serviceconfiguration:helidon-serviceconfiguration-hikaricp-accs + io.helidon.serviceconfiguration:helidon-serviceconfiguration-hikaricp-localhost + - ${project.basedir}/src/test/logging.properties + ${project.basedir}/src/test/logging.properties diff --git a/integrations/cdi/datasource-hikaricp/src/test/java/io/helidon/integrations/datasource/hikaricp/cdi/TestConfiguration.java b/integrations/cdi/datasource-hikaricp/src/test/java/io/helidon/integrations/datasource/hikaricp/cdi/TestConfiguration.java index 5e0d5e97c0a..dc73ce808d3 100644 --- a/integrations/cdi/datasource-hikaricp/src/test/java/io/helidon/integrations/datasource/hikaricp/cdi/TestConfiguration.java +++ b/integrations/cdi/datasource-hikaricp/src/test/java/io/helidon/integrations/datasource/hikaricp/cdi/TestConfiguration.java @@ -15,6 +15,9 @@ */ package io.helidon.integrations.datasource.hikaricp.cdi; +import java.sql.Connection; +import java.sql.SQLException; + import javax.enterprise.context.ApplicationScoped; import javax.enterprise.context.Initialized; import javax.enterprise.event.Observes; @@ -24,7 +27,6 @@ import javax.inject.Named; import javax.sql.DataSource; -import io.helidon.microprofile.config.MpConfig; import io.helidon.microprofile.server.Server; import org.eclipse.microprofile.config.spi.ConfigProviderResolver; @@ -32,11 +34,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assertions.fail; @ApplicationScoped class TestConfiguration { @@ -54,12 +52,13 @@ class TestConfiguration { @BeforeEach void startServer() { this.stopServer(); - // The sequence of calls here is the only way to supply a CDI - // container to Helidon's MicroProfile Server such that all of - // the normal configuration is loaded properly. final Server.Builder builder = Server.builder(); assertNotNull(builder); - builder.config((MpConfig) ConfigProviderResolver.instance().getConfig(Thread.currentThread().getContextClassLoader())); + // The Helidon MicroProfile server implementation uses + // ConfigProviderResolver#getConfig(ClassLoader) directly + // instead of ConfigProvider#getConfig() so we follow suit + // here for fidelity. + builder.config(ConfigProviderResolver.instance().getConfig(Thread.currentThread().getContextClassLoader())); final SeContainerInitializer initializer = SeContainerInitializer.newInstance() .addBeanClasses(TestConfiguration.class); assertNotNull(initializer); @@ -77,9 +76,15 @@ void stopServer() { } } - private void onStartup(@Observes @Initialized(ApplicationScoped.class) final Object event) { + private void onStartup(@Observes @Initialized(ApplicationScoped.class) final Object event) throws SQLException { assertNotNull(this.test); assertNotNull(this.test.toString()); + Connection connection = null; + try { + connection = this.test.getConnection(); + } finally { + connection.close(); + } } @Test diff --git a/integrations/cdi/datasource-hikaricp/src/test/logging.properties b/integrations/cdi/datasource-hikaricp/src/test/logging.properties index 7647d00eb81..2269f171a94 100644 --- a/integrations/cdi/datasource-hikaricp/src/test/logging.properties +++ b/integrations/cdi/datasource-hikaricp/src/test/logging.properties @@ -1,6 +1,17 @@ +# 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. .level=INFO handlers=java.util.logging.ConsoleHandler java.util.logging.ConsoleHandler.formatter=java.util.logging.SimpleFormatter java.util.logging.ConsoleHandler.level=FINEST - -io.helidon.level=FINER diff --git a/integrations/cdi/datasource-hikaricp/src/test/resources/application.yaml b/integrations/cdi/datasource-hikaricp/src/test/resources/application.yaml index b93334fa055..2a0226d0daa 100644 --- a/integrations/cdi/datasource-hikaricp/src/test/resources/application.yaml +++ b/integrations/cdi/datasource-hikaricp/src/test/resources/application.yaml @@ -1,10 +1,23 @@ +# 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. javax: sql: DataSource: test: - dataSourceClassName: org.postgresql.ds.PGPoolingDataSource + dataSourceClassName: org.h2.jdbcx.JdbcDataSource dataSource: - url: jdbc:postgresql://127.0.0.1:5432/hello_world - user: benchmarkdbuser - password: benchmarkdbpass + url: jdbc:h2:mem:test + user: sa + password: "" diff --git a/integrations/cdi/datasource-ucp/src/main/java/io/helidon/integrations/datasource/ucp/cdi/UCPBackedDataSourceExtension.java b/integrations/cdi/datasource-ucp/src/main/java/io/helidon/integrations/datasource/ucp/cdi/UCPBackedDataSourceExtension.java index 31098e78002..d0bea9fbf61 100644 --- a/integrations/cdi/datasource-ucp/src/main/java/io/helidon/integrations/datasource/ucp/cdi/UCPBackedDataSourceExtension.java +++ b/integrations/cdi/datasource-ucp/src/main/java/io/helidon/integrations/datasource/ucp/cdi/UCPBackedDataSourceExtension.java @@ -37,7 +37,6 @@ import oracle.ucp.jdbc.PoolDataSource; import oracle.ucp.jdbc.PoolDataSourceFactory; -import oracle.ucp.jdbc.PoolDataSourceImpl; /** * An {@link Extension} that arranges for named {@link DataSource} @@ -97,7 +96,6 @@ protected final void addBean(final BeanConfigurator beanConfigurator beanConfigurator .addQualifier(dataSourceName) .addTransitiveTypeClosure(PoolDataSource.class) - .beanClass(PoolDataSourceImpl.class) .scope(ApplicationScoped.class) .createWith(cc -> { try { diff --git a/integrations/cdi/datasource/src/main/java/io/helidon/integrations/datasource/cdi/AbstractDataSourceExtension.java b/integrations/cdi/datasource/src/main/java/io/helidon/integrations/datasource/cdi/AbstractDataSourceExtension.java index 78ed50d1129..fd3082b0480 100644 --- a/integrations/cdi/datasource/src/main/java/io/helidon/integrations/datasource/cdi/AbstractDataSourceExtension.java +++ b/integrations/cdi/datasource/src/main/java/io/helidon/integrations/datasource/cdi/AbstractDataSourceExtension.java @@ -28,7 +28,6 @@ import javax.enterprise.event.Observes; import javax.enterprise.inject.literal.NamedLiteral; import javax.enterprise.inject.spi.AfterBeanDiscovery; -import javax.enterprise.inject.spi.BeforeBeanDiscovery; import javax.enterprise.inject.spi.Extension; import javax.enterprise.inject.spi.configurator.BeanConfigurator; import javax.inject.Named; @@ -41,11 +40,18 @@ /** * An abstract {@link Extension} whose subclasses arrange for {@link * DataSource} instances to be added as CDI beans. + * + *

Thread Safety

+ * + *

As with all CDI portable extensions, this class' instances are + * not safe for concurrent use by multiple threads.

*/ public abstract class AbstractDataSourceExtension implements Extension { private final Map masterProperties; + private final Map explicitlySetProperties; + private final Config config; /** @@ -54,6 +60,7 @@ public abstract class AbstractDataSourceExtension implements Extension { protected AbstractDataSourceExtension() { super(); this.masterProperties = new HashMap<>(); + this.explicitlySetProperties = new HashMap<>(); this.config = ConfigProvider.getConfig(); } @@ -167,7 +174,17 @@ protected final Config getConfig() { * of known data source names */ protected final Set getDataSourceNames() { - return Collections.unmodifiableSet(this.masterProperties.keySet()); + final Set returnValue; + if (this.masterProperties.isEmpty()) { + if (this.explicitlySetProperties.isEmpty()) { + returnValue = Collections.emptySet(); + } else { + returnValue = Collections.unmodifiableSet(this.explicitlySetProperties.keySet()); + } + } else { + returnValue = Collections.unmodifiableSet(this.masterProperties.keySet()); + } + return returnValue; } /** @@ -188,10 +205,28 @@ protected final Set getDataSourceNames() { * {@code dataSourceName}, or {@code null} */ protected final Properties putDataSourceProperties(final String dataSourceName, final Properties properties) { - return this.masterProperties.put(dataSourceName, properties); + return this.explicitlySetProperties.put(dataSourceName, properties); } - private void initializeMasterProperties(@Observes final BeforeBeanDiscovery event) { + /** + * {@linkplain Map#clear() Clears} and then + * builds or rebuilds an internal map of data source properties + * whose contents will be processed eventually by the {@link + * #addBean(BeanConfigurator, Named, Properties)} method. + * + *

If no subclass explicitly calls this method, it will be + * called by the {@link #addBean(BeanConfigurator, Named, + * Properties)} method just prior to its other activities.

+ * + *

Once the {@link #addBean(BeanConfigurator, Named, + * Properties)} method has run to completion, while this method + * may be called freely its use is discouraged and its effects + * will no longer be used.

+ * + * @see #addBean(BeanConfigurator, Named, Properties) + */ + protected final void initializeMasterProperties() { + this.masterProperties.clear(); final Set allPropertyNames = this.getPropertyNames(); if (allPropertyNames != null && !allPropertyNames.isEmpty()) { for (final String propertyName : allPropertyNames) { @@ -211,10 +246,12 @@ private void initializeMasterProperties(@Observes final BeforeBeanDiscovery even } } } + this.masterProperties.putAll(this.explicitlySetProperties); } /** - * Returns a {@link Set} of all known configuration property names. + * Returns a {@link Set} of all known configuration property + * names. * *

This method never returns {@code null}.

* @@ -249,6 +286,16 @@ protected final Set getPropertyNames() { // make sure to get all property names from all ConfigSources // "by hand". // + // Additionally, the Helidon MicroProfile Config + // implementation may add on some Helidon SE Config sources + // that are not represented as MicroProfile Config sources. + // Consequently we have to source property names from both the + // MicroProfile Config ConfigSources and from Helidon itself. + // We do this by first iterating over the MicroProfile Config + // ConfigSources and then augmenting where necessary with any + // other (cached) property names reported by the Helidon + // MicroProfile Config implementation. + // // (The MicroProfile Config specification also does not say // whether a ConfigSource is thread-safe // (https://github.com/eclipse/microprofile-config/issues/369), @@ -259,8 +306,23 @@ protected final Set getPropertyNames() { // implementation caches all property names up front, which // may not be correct, but is also not forbidden. final Set returnValue; - final Set propertyNames = getPropertyNames(this.config.getConfigSources()); - if (propertyNames == null || propertyNames.isEmpty()) { + + // Start by getting all the property names directly from our + // MicroProfile Config ConfigSources. They take precedence. + Set propertyNames = getPropertyNames(this.config.getConfigSources()); + assert propertyNames != null; + + // Add any property names that the Config itself might report + // that aren't reflected, for whatever reason, in the + // ConfigSources' property names. + final Iterable configPropertyNames = this.config.getPropertyNames(); + if (configPropertyNames != null) { + for (final String configPropertyName : configPropertyNames) { + propertyNames.add(configPropertyName); + } + } + + if (propertyNames.isEmpty()) { returnValue = Collections.emptySet(); } else { returnValue = Collections.unmodifiableSet(propertyNames); @@ -280,11 +342,14 @@ private static Set getPropertyNames(final Iterable> masterPropertiesEntries = this.masterProperties.entrySet(); if (masterPropertiesEntries != null && !masterPropertiesEntries.isEmpty()) { @@ -296,6 +361,7 @@ private void afterBeanDiscovery(@Observes final AfterBeanDiscovery event) { } } this.masterProperties.clear(); + this.explicitlySetProperties.clear(); } }