ensure system properties set during extension startup lifecycle are usable at runtime, even with our copy strategy
diff --git a/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigBean.java b/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigBean.java
deleted file mode 100644
index 0549a88..0000000
--- a/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigBean.java
+++ /dev/null
@@ -1,104 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You 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 org.apache.geronimo.config.cdi;
-
-import static java.util.Arrays.asList;
-import static java.util.Collections.singleton;
-
-import java.lang.annotation.Annotation;
-import java.lang.reflect.Type;
-import java.util.Collections;
-import java.util.HashSet;
-import java.util.Set;
-
-import javax.enterprise.context.ApplicationScoped;
-import javax.enterprise.context.spi.CreationalContext;
-import javax.enterprise.inject.spi.Bean;
-import javax.enterprise.inject.spi.InjectionPoint;
-import javax.enterprise.inject.spi.PassivationCapable;
-
-import org.eclipse.microprofile.config.Config;
-import org.eclipse.microprofile.config.ConfigProvider;
-
-public class ConfigBean<T> implements Bean<T>, PassivationCapable {
-
- private final String id = ConfigBean.class.getName();
-
- private final Set<Annotation> qualifiers = singleton(new DefaultLiteral());
-
- private final Set<Type> types = new HashSet<>(asList(Config.class, Object.class));
-
- @Override
- public Set<InjectionPoint> getInjectionPoints() {
- return Collections.emptySet();
- }
-
- @Override
- public Class<?> getBeanClass() {
- return Config.class;
- }
-
- @Override
- public boolean isNullable() {
- return false;
- }
-
- @Override
- public T create(final CreationalContext<T> context) {
- return (T) ConfigProvider.getConfig();
- }
-
- @Override
- public void destroy(final T instance, final CreationalContext<T> context) {
- // no-op
- }
-
- @Override
- public Set<Type> getTypes() {
- return types;
- }
-
- @Override
- public Set<Annotation> getQualifiers() {
- return qualifiers;
- }
-
- @Override
- public Class<? extends Annotation> getScope() {
- return ApplicationScoped.class;
- }
-
- @Override
- public String getName() {
- return null;
- }
-
- @Override
- public Set<Class<? extends Annotation>> getStereotypes() {
- return Collections.emptySet();
- }
-
- @Override
- public boolean isAlternative() {
- return false;
- }
-
- @Override
- public String getId() {
- return id;
- }
-}
diff --git a/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigExtension.java b/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigExtension.java
index dd6c114..8870ff0 100644
--- a/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigExtension.java
+++ b/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigExtension.java
@@ -18,10 +18,11 @@
import static java.util.stream.Collectors.toList;
-import org.apache.geronimo.config.DefaultConfigProvider;
+import org.apache.geronimo.config.cdi.configsource.Reloadable;
import org.eclipse.microprofile.config.Config;
import org.eclipse.microprofile.config.ConfigProvider;
import org.eclipse.microprofile.config.inject.ConfigProperty;
+import org.eclipse.microprofile.config.spi.ConfigProviderResolver;
import javax.enterprise.event.Observes;
import javax.enterprise.inject.spi.AfterBeanDiscovery;
@@ -32,15 +33,12 @@
import javax.enterprise.inject.spi.Extension;
import javax.enterprise.inject.spi.InjectionPoint;
import javax.enterprise.inject.spi.ProcessAnnotatedType;
-import javax.enterprise.inject.spi.ProcessBean;
import javax.enterprise.inject.spi.ProcessInjectionPoint;
import javax.inject.Provider;
-import java.lang.reflect.Method;
import java.lang.reflect.ParameterizedType;
import java.lang.reflect.Type;
import java.util.ArrayList;
-import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
@@ -50,6 +48,7 @@
import java.util.function.Predicate;
import java.util.stream.Collectors;
import java.util.stream.Stream;
+import java.util.stream.StreamSupport;
/**
* @author <a href="mailto:struberg@yahoo.de">Mark Struberg</a>
@@ -72,7 +71,6 @@
private Set<Class<?>> proxies = new HashSet<>();
private List<Class<?>> validProxies;
private List<ProxyBean<?>> proxyBeans;
- private boolean foundConfig;
public void findProxies(@Observes ProcessAnnotatedType<?> pat) {
@@ -91,10 +89,6 @@
}
}
- void onConfig(@Observes final ProcessAnnotatedType<Config> configProcessBean) {
- foundConfig = true;
- }
-
public void registerConfigProducer(@Observes AfterBeanDiscovery abd, BeanManager bm) {
Set<Type> types = injectionPoints.stream()
.filter(NOT_PROVIDERS)
@@ -121,16 +115,18 @@
.collect(toList());
proxyBeans.forEach(abd::addBean);
} // else there are errors
-
- if (!foundConfig) {
- abd.addBean(new ConfigInjectionBean<>(bm, Config.class));
- }
}
public void validate(@Observes AfterDeploymentValidation add) {
List<String> deploymentProblems = new ArrayList<>();
config = ConfigProvider.getConfig();
+
+ StreamSupport.stream(config.getConfigSources().spliterator(), false)
+ .filter(Reloadable.class::isInstance)
+ .map(Reloadable.class::cast)
+ .forEach(Reloadable::reload);
+
proxyBeans.forEach(b -> b.init(config));
proxyBeans.clear();
@@ -167,7 +163,7 @@
}
public void shutdown(@Observes BeforeShutdown bsd) {
- DefaultConfigProvider.instance().releaseConfig(config);
+ ConfigProviderResolver.instance().releaseConfig(config);
}
private boolean isValidProxy(final Class<?> api) {
diff --git a/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigurationHandler.java b/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigurationHandler.java
index 838d452..7880771 100644
--- a/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigurationHandler.java
+++ b/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigurationHandler.java
@@ -127,6 +127,8 @@
final String finalDefaultValue = defaultValue;
if (lookupType == long.class || lookupType == Long.class) {
this.defaultValue = Long.parseLong(finalDefaultValue);
+ } else if (lookupType == boolean.class || lookupType == Boolean.class) {
+ this.defaultValue = Boolean.parseBoolean(finalDefaultValue);
} else if (lookupType == int.class || lookupType == Integer.class) {
this.defaultValue = Integer.parseInt(finalDefaultValue);
} else if (lookupType == double.class || lookupType == Double.class) {
diff --git a/impl/src/main/java/org/apache/geronimo/config/cdi/configsource/Reloadable.java b/impl/src/main/java/org/apache/geronimo/config/cdi/configsource/Reloadable.java
new file mode 100644
index 0000000..62e916d
--- /dev/null
+++ b/impl/src/main/java/org/apache/geronimo/config/cdi/configsource/Reloadable.java
@@ -0,0 +1,29 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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 org.apache.geronimo.config.cdi.configsource;
+
+/**
+ * Enable config source to be reloaded in AfterDeploymentValidation
+ * phase. Particularly useful for sources relying on a state which can
+ * be mutated in previous phases like system properties but which
+ * desire to stay immutable at runtime for perf/lock reasons.
+ */
+public interface Reloadable {
+ void reload();
+}
diff --git a/impl/src/main/java/org/apache/geronimo/config/configsource/SystemPropertyConfigSource.java b/impl/src/main/java/org/apache/geronimo/config/configsource/SystemPropertyConfigSource.java
index 553c197..066bc0d 100644
--- a/impl/src/main/java/org/apache/geronimo/config/configsource/SystemPropertyConfigSource.java
+++ b/impl/src/main/java/org/apache/geronimo/config/configsource/SystemPropertyConfigSource.java
@@ -18,6 +18,7 @@
*/
package org.apache.geronimo.config.configsource;
+import org.apache.geronimo.config.cdi.configsource.Reloadable;
import org.eclipse.microprofile.config.spi.ConfigSource;
import javax.enterprise.inject.Typed;
@@ -35,18 +36,18 @@
*/
@Typed
@Vetoed
-public class SystemPropertyConfigSource extends BaseConfigSource {
+public class SystemPropertyConfigSource extends BaseConfigSource implements Reloadable {
private static final String COPY_PROPERTY = "org.apache.geronimo.config.configsource.SystemPropertyConfigSource.copy";
private final Map<String, String> instance;
+ private final boolean shouldReload;
public SystemPropertyConfigSource() {
this(valueOf(System.getProperty(COPY_PROPERTY, "true")));
}
public SystemPropertyConfigSource(boolean copy) {
- instance = copy ?
- System.getProperties().stringPropertyNames().stream().collect(toMap(identity(), System::getProperty)) :
- Map.class.cast(System.getProperties());
+ instance = load(copy);
+ shouldReload = copy;
initOrdinal(400);
}
@@ -64,4 +65,19 @@
public String getName() {
return "system-properties";
}
+
+ @Override
+ public void reload() {
+ if (!shouldReload) {
+ return;
+ }
+ instance.clear();
+ instance.putAll(load(true));
+ }
+
+ private Map<String, String> load(final boolean copy) {
+ return copy ?
+ System.getProperties().stringPropertyNames().stream().collect(toMap(identity(), System::getProperty)) :
+ Map.class.cast(System.getProperties());
+ }
}
diff --git a/impl/src/test/java/org/apache/geronimo/config/test/internal/SystemPropertyConfigSourceTest.java b/impl/src/test/java/org/apache/geronimo/config/test/internal/SystemPropertyConfigSourceTest.java
new file mode 100644
index 0000000..a0addba
--- /dev/null
+++ b/impl/src/test/java/org/apache/geronimo/config/test/internal/SystemPropertyConfigSourceTest.java
@@ -0,0 +1,102 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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 org.apache.geronimo.config.test.internal;
+
+import static org.testng.Assert.assertTrue;
+
+import javax.enterprise.context.ApplicationScoped;
+import javax.enterprise.event.Observes;
+import javax.enterprise.inject.spi.AfterBeanDiscovery;
+import javax.enterprise.inject.spi.BeforeBeanDiscovery;
+import javax.enterprise.inject.spi.BeforeShutdown;
+import javax.enterprise.inject.spi.Extension;
+import javax.inject.Inject;
+
+import org.eclipse.microprofile.config.Config;
+import org.eclipse.microprofile.config.ConfigProvider;
+import org.eclipse.microprofile.config.inject.ConfigProperty;
+import org.jboss.arquillian.container.test.api.Deployment;
+import org.jboss.arquillian.testng.Arquillian;
+import org.jboss.shrinkwrap.api.Archive;
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.jboss.shrinkwrap.api.asset.EmptyAsset;
+import org.jboss.shrinkwrap.api.spec.WebArchive;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+/**
+ * It is important to ensure the config is reloaded before going live in case some
+ * system properties are set during the starting extension lifecycle.
+ */
+public class SystemPropertyConfigSourceTest extends Arquillian {
+ @Deployment
+ public static Archive<?> archive() {
+ return ShrinkWrap.create(WebArchive.class, SystemPropertyConfigSourceTest.class.getSimpleName() + ".war")
+ .addAsWebInfResource(EmptyAsset.INSTANCE, "classes/META-INF/beans.xml")
+ .addAsServiceProvider(Extension.class, InitInExtension.class)
+ .addClasses(SystemPropertyConfigSourceTest.class, Injected.class);
+ }
+
+ @Inject
+ private Injected injected;
+
+ @Test
+ public void testSystemPropsLoadedExtensionValue() {
+ assertTrue(injected.getSet());
+ }
+
+ @ApplicationScoped
+ public static class Injected {
+ @Inject
+ @ConfigProperty(name = "org.apache.geronimo.config.test.internal.SystemPropertyConfigSourceTest$InitInExtension")
+ private Boolean set;
+
+ public Boolean getSet() {
+ return set;
+ }
+ }
+
+ public static class InitInExtension implements Extension {
+ private String originalCopy;
+
+ void eagerInit(@Observes final BeforeBeanDiscovery beforeBeanDiscovery) {
+ originalCopy = System.getProperty("org.apache.geronimo.config.configsource.SystemPropertyConfigSource.copy");
+ // enfore the default, it is overriden for surefire
+ System.setProperty("org.apache.geronimo.config.configsource.SystemPropertyConfigSource.copy", "true");
+
+ // eager load -> loads system props and copy
+ ConfigProvider.getConfig();
+ }
+
+ // before validation to ensure config validation passes
+ void afterBeanDiscovery(@Observes final AfterBeanDiscovery afterBeanDiscovery) {
+ // with copy this should get ignored but we will reload it before the validation
+ System.setProperty(InitInExtension.class.getName(), "true");
+ }
+
+ void beforeShutdown(@Observes final BeforeShutdown beforeShutdown) {
+ System.clearProperty(InitInExtension.class.getName());
+ if (originalCopy != null) {
+ System.setProperty("org.apache.geronimo.config.configsource.SystemPropertyConfigSource.copy", originalCopy);
+ } else {
+ System.clearProperty("org.apache.geronimo.config.configsource.SystemPropertyConfigSource.copy");
+ }
+ }
+ }
+}
