From 7183da60b6c3797398508fd01e3844ab22fd9a63 Mon Sep 17 00:00:00 2001 From: Jeff Mesnil Date: Fri, 18 Jan 2019 16:38:27 +0100 Subject: [PATCH] Refactor #384 Update the onAttributeChange method to return a ChangeSupport class that can be used to handle release of resources (in the close method). When a Config is released, it should call ChangeSupport.close to ensure that the ConfigSource has a chance to properly release any resources held by the callbacks. * Add TCK test ConfigAccessorTest#testOnAttributeChange Signed-off-by: Jeff Mesnil --- .../microprofile/config/spi/ConfigSource.java | 67 +++++++++---------- .../config/tck/ConfigAccessorTest.java | 15 +++++ .../ConfigurableConfigSource.java | 7 +- 3 files changed, 52 insertions(+), 37 deletions(-) diff --git a/api/src/main/java/org/eclipse/microprofile/config/spi/ConfigSource.java b/api/src/main/java/org/eclipse/microprofile/config/spi/ConfigSource.java index 582e2892..bc151a32 100644 --- a/api/src/main/java/org/eclipse/microprofile/config/spi/ConfigSource.java +++ b/api/src/main/java/org/eclipse/microprofile/config/spi/ConfigSource.java @@ -26,11 +26,13 @@ * Extracted the Config part out of DeltaSpike and proposed as Microprofile-Config cf41cf130bcaf5447ff8 * 2016-11-14 - Emily Jiang / IBM Corp * Methods renamed, JavaDoc and cleanup - * + * * *******************************************************************************/ package org.eclipse.microprofile.config.spi; +import java.io.Closeable; +import java.io.Serializable; import java.util.Map; import java.util.Set; import java.util.function.Consumer; @@ -164,18 +166,6 @@ default int getOrdinal() { */ String getName(); - /** - * Determines if this config source should be scanned for its list of properties. - * - * Generally, slow ConfigSources should return false here. - * - * @return {@code true} if this ConfigSource should be scanned for its list of properties, - * {@code false} if it should not be scanned. - */ - default boolean isScannable() { - return true; - } - /** * The callback should get invoked if an attribute change got detected inside the ConfigSource. * @@ -188,7 +178,7 @@ default boolean isScannable() { default ChangeSupport onAttributeChange(Consumer> callback) { // do nothing by default. Just for compat with older ConfigSources. // return unsupported to tell config that it must re-query this source every time - return ChangeSupport.UNSUPPORTED; + return () -> ChangeSupport.Type.UNSUPPORTED; } /** @@ -196,25 +186,34 @@ default ChangeSupport onAttributeChange(Consumer> callback) { *

* {@link org.eclipse.microprofile.config.Config} implementations may use this information for internal optimizations. */ - enum ChangeSupport { - /** - * Config change is supported, this config source will invoke the callback provided by - * {@link ConfigSource#onAttributeChange(Consumer)}. - *

- * Example: File based config source that watches the file for changes - */ - SUPPORTED, - /** - * Config change is not supported. Configuration values can change, though this change is not reported back. - *

- * Example: LDAP based config source - */ - UNSUPPORTED, - /** - * Configuration values cannot change for the lifetime of this {@link ConfigSource}. - *

- * Example: Environment variables config source, classpath resource config source - */ - IMMUTABLE + interface ChangeSupport extends Closeable, Serializable { + + enum Type { + /** + * Config change is supported, this config source will invoke the callback provided by + * {@link ConfigSource#onAttributeChange(Consumer)}. + *

+ * Example: File based config source that watches the file for changes + */ + SUPPORTED, + /** + * Config change is not supported. Configuration values can change, though this change is not reported back. + *

+ * Example: LDAP based config source + */ + UNSUPPORTED, + /** + * Configuration values cannot change for the lifetime of this {@link ConfigSource}. + *

+ * Example: Environment variables config source, classpath resource config source + */ + IMMUTABLE + } + + Type getType(); + + @Override + default void close() { + } } } diff --git a/tck/src/main/java/org/eclipse/microprofile/config/tck/ConfigAccessorTest.java b/tck/src/main/java/org/eclipse/microprofile/config/tck/ConfigAccessorTest.java index 898fafe1..7e410fba 100644 --- a/tck/src/main/java/org/eclipse/microprofile/config/tck/ConfigAccessorTest.java +++ b/tck/src/main/java/org/eclipse/microprofile/config/tck/ConfigAccessorTest.java @@ -196,6 +196,21 @@ public void testCacheFor() throws Exception { Assert.assertEquals(val.getValue(), "secondvalue"); } + @Test + public void testOnAttributeChange() { + String key = "tck.config.test.onattributechange.key"; + ConfigurableConfigSource.configure(config, key, "firstvalue"); + + ConfigAccessor val = config.access(key, String.class).cacheFor(Duration.ofMillis(30)).build(); + Assert.assertEquals(val.getValue(), "firstvalue"); + + // immediately change the value on the ConfigurableConfigSource that will notify the Config of the change + ConfigurableConfigSource.configure(config, key, "secondvalue"); + + // we should see the new value right now as the ConfigurableConfigSource has notified that its attribute has changed + Assert.assertEquals(val.getValue(), "secondvalue"); + } + @Test public void testDefaultValue() { String key = "tck.config.test.javaconfig.somerandom.default.key"; diff --git a/tck/src/main/java/org/eclipse/microprofile/config/tck/configsources/ConfigurableConfigSource.java b/tck/src/main/java/org/eclipse/microprofile/config/tck/configsources/ConfigurableConfigSource.java index 5ee23f2a..81b83df7 100644 --- a/tck/src/main/java/org/eclipse/microprofile/config/tck/configsources/ConfigurableConfigSource.java +++ b/tck/src/main/java/org/eclipse/microprofile/config/tck/configsources/ConfigurableConfigSource.java @@ -64,14 +64,15 @@ public String getName() { @Override public ConfigSource.ChangeSupport onAttributeChange(Consumer> reportAttributeChange) { this.reportAttributeChange = reportAttributeChange; - return ChangeSupport.SUPPORTED; + return () -> ChangeSupport.Type.SUPPORTED; } public static void configure(Config cfg, String propertyName, String value) { for (ConfigSource configSource : cfg.getConfigSources()) { if (configSource instanceof ConfigurableConfigSource) { - ((ConfigurableConfigSource) configSource).properties.put(propertyName, value); - ((ConfigurableConfigSource) configSource).reportAttributeChange.accept(Collections.singleton(propertyName)); + ConfigurableConfigSource configurableConfigSource = (ConfigurableConfigSource) configSource; + configurableConfigSource.properties.put(propertyName, value); + configurableConfigSource.reportAttributeChange.accept(Collections.singleton(propertyName)); return; } }