From a99f6c4694356ed4207395f5f8a0965762c7d1bf Mon Sep 17 00:00:00 2001 From: Roberto Cortez Date: Tue, 12 Sep 2023 18:06:47 +0100 Subject: [PATCH] Cache getPropertyNames --- .../config/ConfigMappingProvider.java | 7 --- .../io/smallrye/config/SmallRyeConfig.java | 61 ++++++++++--------- .../tests/ZooKeeperConfigSourceTest.java | 14 ++--- 3 files changed, 39 insertions(+), 43 deletions(-) diff --git a/implementation/src/main/java/io/smallrye/config/ConfigMappingProvider.java b/implementation/src/main/java/io/smallrye/config/ConfigMappingProvider.java index 75bcbf9ae..799b07789 100644 --- a/implementation/src/main/java/io/smallrye/config/ConfigMappingProvider.java +++ b/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>> entry : roots.entrySet()) { String path = entry.getKey(); List> 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 additionalMappedProperties( envProperties.addAll(source.getPropertyNames()); } - // Remove properties that we know that already exist - for (String propertyName : config.getPropertyNames()) { - mappedProperties.remove(propertyName); - } - Set envRoots = new HashSet<>(roots.size()); for (String root : roots) { envRoots.add(replaceNonAlphanumericByUnderscores(root)); diff --git a/implementation/src/main/java/io/smallrye/config/SmallRyeConfig.java b/implementation/src/main/java/io/smallrye/config/SmallRyeConfig.java index a04234b25..2a32fda5e 100644 --- a/implementation/src/main/java/io/smallrye/config/SmallRyeConfig.java +++ b/implementation/src/main/java/io/smallrye/config/SmallRyeConfig.java @@ -410,11 +410,31 @@ public T getConfigMapping(Class 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 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 getLatestPropertyNames() { + return configSources.getPropertyNames().get(true); + } + /** * Checks if a property is present in the {@link Config} instance. *
@@ -518,14 +538,6 @@ void addPropertyNames(Set 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 propertyNames = new HashSet<>(); - private boolean cached = false; private PropertyNames(final PropertyNamesConfigSourceInterceptor propertyNamesInterceptor) { this.interceptor = propertyNamesInterceptor; } Iterable get() { - if (cached) { - return propertyNames; - } else { - final HashSet names = new HashSet<>(); - final Iterator 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) get()); - cached = true; - } - - void disableCache() { - propertyNames.clear(); - cached = false; + Iterable get(boolean latest) { + if (latest) { + propertyNames.clear(); + Iterator namesIterator = interceptorChain.iterateNames(); + while (namesIterator.hasNext()) { + propertyNames.add(namesIterator.next()); + } + } + return propertyNames; } void add(final Set properties) { diff --git a/sources/zookeeper/src/test/java/io/smallrye/config/source/zookeeper/tests/ZooKeeperConfigSourceTest.java b/sources/zookeeper/src/test/java/io/smallrye/config/source/zookeeper/tests/ZooKeeperConfigSourceTest.java index e12c21e50..877258b61 100644 --- a/sources/zookeeper/src/test/java/io/smallrye/config/source/zookeeper/tests/ZooKeeperConfigSourceTest.java +++ b/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 propertyNames = new HashSet<>(); - cfg.getPropertyNames().forEach(propertyNames::add); + config.getLatestPropertyNames().forEach(propertyNames::add); assertTrue(propertyNames.contains(PROPERTY_NAME)); }