Cache getPropertyNames (#997)

implementation/src/main/java/io/smallrye/config/ConfigMappingProvider.java

@@ -972,7 +972,6 @@ private ConfigMappingContext mapConfigurationInternal(SmallRyeConfig config) thr
         }
 
         // eagerly populate roots
-        config.cachePropertyNames(true);
         for (Map.Entry<String, List<Class<?>>> entry : roots.entrySet()) {
             String path = entry.getKey();
             List<Class<?>> roots = entry.getValue();
@@ -1003,7 +1002,6 @@ private ConfigMappingContext mapConfigurationInternal(SmallRyeConfig config) thr
             throw new ConfigValidationException(problems.toArray(ConfigValidationException.Problem.NO_PROBLEMS));
         }
         context.fillInOptionals();
-        config.cachePropertyNames(false);
 
         return context;
     }
@@ -1068,11 +1066,6 @@ private static Set<String> additionalMappedProperties(
             envProperties.addAll(source.getPropertyNames());
         }
 
-        // Remove properties that we know that already exist
-        for (String propertyName : config.getPropertyNames()) {
-            mappedProperties.remove(propertyName);
-        }
-
         Set<String> envRoots = new HashSet<>(roots.size());
         for (String root : roots) {
             envRoots.add(replaceNonAlphanumericByUnderscores(root));

implementation/src/main/java/io/smallrye/config/SmallRyeConfig.java

@@ -410,11 +410,31 @@ public <T> T getConfigMapping(Class<T> type, String prefix) {
         return mappings.getConfigMapping(type, prefix);
     }
 
+    /**
+     * {@inheritDoc}
+     *
+     * This implementation caches the list of property names collected when {@link SmallRyeConfig} is built via
+     * {@link SmallRyeConfigBuilder#build()}.
+     *
+     * @return the cached names of all configured keys of the underlying configuration
+     * @see {@link SmallRyeConfig#getLatestPropertyNames()}
+     */
     @Override
     public Iterable<String> getPropertyNames() {
         return configSources.getPropertyNames().get();
     }
 
+    /**
+     * Provides a way to retrieve an updated list of all property names. The updated list replaces the cached list
+     * returned by {@link SmallRyeConfig#getPropertyNames()}.
+     *
+     * @return the names of all configured keys of the underlying configuration
+     */
+    @Experimental("Retrieve an updated list of all configuration property names")
+    public Iterable<String> getLatestPropertyNames() {
+        return configSources.getPropertyNames().get(true);
+    }
+
     /**
      * Checks if a property is present in the {@link Config} instance.
      * <br>
@@ -518,14 +538,6 @@ void addPropertyNames(Set<String> properties) {
         configSources.getPropertyNames().add(properties);
     }
 
-    void cachePropertyNames(boolean cache) {
-        if (cache) {
-            configSources.getPropertyNames().enableCache();
-        } else {
-            configSources.getPropertyNames().disableCache();
-        }
-    }
-
     private static class ConfigSources implements Serializable {
         private static final long serialVersionUID = 3483018375584151712L;
 
@@ -738,37 +750,28 @@ class PropertyNames implements Serializable {
             private static final long serialVersionUID = 4193517748286869745L;
 
             private final PropertyNamesConfigSourceInterceptor interceptor;
-
-            // TODO - Temporary cache to improve allocation. Mappings require multiple calls to getPropertyNames.
-            // TODO - Replace with a proper implementation to avoid recomputation of property names.
             private final Set<String> propertyNames = new HashSet<>();
-            private boolean cached = false;
 
             private PropertyNames(final PropertyNamesConfigSourceInterceptor propertyNamesInterceptor) {
                 this.interceptor = propertyNamesInterceptor;
             }
 
             Iterable<String> get() {
-                if (cached) {
-                    return propertyNames;
-                } else {
-                    final HashSet<String> names = new HashSet<>();
-                    final Iterator<String> namesIterator = interceptorChain.iterateNames();
-                    while (namesIterator.hasNext()) {
-                        names.add(namesIterator.next());
-                    }
-                    return names;
+                if (propertyNames.isEmpty()) {
+                    return get(true);
                 }
+                return propertyNames;
             }
 
-            void enableCache() {
-                propertyNames.addAll((Set<String>) get());
-                cached = true;
-            }
-
-            void disableCache() {
-                propertyNames.clear();
-                cached = false;
+            Iterable<String> get(boolean latest) {
+                if (latest) {
+                    propertyNames.clear();
+                    Iterator<String> namesIterator = interceptorChain.iterateNames();
+                    while (namesIterator.hasNext()) {
+                        propertyNames.add(namesIterator.next());
+                    }
+                }
+                return propertyNames;
             }
 
             void add(final Set<String> properties) {

sources/zookeeper/src/test/java/io/smallrye/config/source/zookeeper/tests/ZooKeeperConfigSourceTest.java

@@ -23,7 +23,6 @@
 import org.apache.curator.framework.CuratorFrameworkFactory;
 import org.apache.curator.retry.ExponentialBackoffRetry;
 import org.apache.curator.test.TestingServer;
-import org.eclipse.microprofile.config.Config;
 import org.eclipse.microprofile.config.ConfigProvider;
 import org.eclipse.microprofile.config.inject.ConfigProperty;
 import org.jboss.weld.junit5.WeldInitiator;
@@ -34,6 +33,7 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 
+import io.smallrye.config.SmallRyeConfig;
 import io.smallrye.config.inject.ConfigExtension;
 
 /**
@@ -92,20 +92,20 @@ static void tearDownClass() throws Exception {
     void testGettingProperty() {
         logger.info("ZooKeeperConfigSourceTest.testGettingProperty");
 
-        Config cfg = ConfigProvider.getConfig();
+        SmallRyeConfig config = ConfigProvider.getConfig().unwrap(SmallRyeConfig.class);
 
         //Check that the ZK ConfigSource will work
-        assertNotNull(cfg.getValue("io.smallrye.configsource.zookeeper.url", String.class));
+        assertNotNull(config.getValue("io.smallrye.configsource.zookeeper.url", String.class));
 
         //Check that a property doesn't exist yet
         try {
-            cfg.getValue(PROPERTY_NAME, String.class);
+            config.getValue(PROPERTY_NAME, String.class);
             fail("Property " + PROPERTY_NAME + " should not exist");
         } catch (NoSuchElementException ignored) {
         }
 
         //Check that the optional version of the property is not present
-        assertFalse(cfg.getOptionalValue(PROPERTY_NAME, String.class).isPresent());
+        assertFalse(config.getOptionalValue(PROPERTY_NAME, String.class).isPresent());
         //setup the property in ZK
         try {
             curatorClient.createContainers(ZK_KEY);
@@ -115,10 +115,10 @@ void testGettingProperty() {
         }
 
         //check the property can be optained by a property
-        assertEquals(PROPERTY_VALUE, cfg.getValue(PROPERTY_NAME, String.class));
+        assertEquals(PROPERTY_VALUE, config.getValue(PROPERTY_NAME, String.class));
 
         Set<String> propertyNames = new HashSet<>();
-        cfg.getPropertyNames().forEach(propertyNames::add);
+        config.getLatestPropertyNames().forEach(propertyNames::add);
         assertTrue(propertyNames.contains(PROPERTY_NAME));
     }