diff --git a/ribbon-archaius/src/main/java/com/netflix/client/config/ArchaiusPropertyResolver.java b/ribbon-archaius/src/main/java/com/netflix/client/config/ArchaiusPropertyResolver.java index 9b9a9cc1..f7eed664 100644 --- a/ribbon-archaius/src/main/java/com/netflix/client/config/ArchaiusPropertyResolver.java +++ b/ribbon-archaius/src/main/java/com/netflix/client/config/ArchaiusPropertyResolver.java @@ -44,8 +44,6 @@ private static void invokeAction(Runnable action) { @Override public Optional get(String key, Class type) { - LOG.debug("Loading property {}", key); - if (Integer.class.equals(type)) { return Optional.ofNullable((T) config.getInteger(key, null)); } else if (Boolean.class.equals(type)) { @@ -70,7 +68,8 @@ public Optional get(String key, Class type) { .orElseThrow(() -> new IllegalArgumentException("Unable to convert value to desired type " + type)); } }); - } } + } + } @Override public void forEach(String prefix, BiConsumer consumer) { diff --git a/ribbon-core/src/main/java/com/netflix/client/config/ReloadableClientConfig.java b/ribbon-core/src/main/java/com/netflix/client/config/ReloadableClientConfig.java index 470b38fb..eb16a9fb 100644 --- a/ribbon-core/src/main/java/com/netflix/client/config/ReloadableClientConfig.java +++ b/ribbon-core/src/main/java/com/netflix/client/config/ReloadableClientConfig.java @@ -59,12 +59,24 @@ public abstract class ReloadableClientConfig implements IClientConfig { private boolean isLoaded = false; + /** + * @deprecated Use {@link #ReloadableClientConfig(PropertyResolver, String, String)} + */ + @Deprecated protected ReloadableClientConfig(PropertyResolver resolver) { - this.clientName = DEFAULT_CLIENT_NAME; - this.resolver = resolver; + this(resolver, DEFAULT_CLIENT_NAME); } + /** + * @deprecated Use {@link #ReloadableClientConfig(PropertyResolver, String, String)} + */ + @Deprecated protected ReloadableClientConfig(PropertyResolver resolver, String clientName) { + this(resolver, DEFAULT_NAMESPACE, DEFAULT_CLIENT_NAME); + } + + protected ReloadableClientConfig(PropertyResolver resolver, String namespace, String clientName) { + this.namespace = namespace; this.clientName = clientName; this.resolver = resolver; } @@ -106,14 +118,11 @@ public final void setNameSpace(String nameSpace) { @Override public void loadProperties(String clientName) { - Preconditions.checkState(isLoaded == false, "Config '{}' can only be loaded once", clientName); - if (!isLoaded) { - loadDefaultValues(); - this.isLoaded = true; - resolver.onChange(this::reload); - } - + Preconditions.checkState(!isLoaded, "Config '{}' can only be loaded once", clientName); this.clientName = clientName; + this.isLoaded = true; + loadDefaultValues(); + resolver.onChange(this::reload); } @Override @@ -202,8 +211,6 @@ interface ReloadableProperty extends Property { } private ReloadableProperty getClientDynamicProperty(IClientConfigKey key) { - LOG.debug("Get dynamic property key={} ns={} client={}", key.key(), getNameSpace(), clientName); - return createProperty( () -> resolveFinalProperty(key), key::defaultValue); @@ -295,11 +302,15 @@ public Optional getIfSet(IClientConfigKey key) { @Override public final Property getDynamicProperty(IClientConfigKey key) { + LOG.debug("Get dynamic property key={} ns={} client={}", key.key(), getNameSpace(), clientName); + return getClientDynamicProperty(key); } @Override public Property getScopedProperty(IClientConfigKey key) { + LOG.debug("Get dynamic property key={} ns={} client={}", key.key(), getNameSpace(), clientName); + return (Property) dynamicProperties.computeIfAbsent(key, ignore -> createProperty( () -> resolverScopedProperty(key), key::defaultValue)); @@ -307,6 +318,8 @@ public Property getScopedProperty(IClientConfigKey key) { @Override public Property getPrefixMappedProperty(IClientConfigKey key) { + LOG.debug("Get dynamic property key={} ns={} client={}", key.key(), getNameSpace(), clientName); + return (Property) dynamicProperties.computeIfAbsent(key, ignore -> createProperty( getPrefixedMapPropertySupplier(key), key::defaultValue)); diff --git a/ribbon-core/src/test/java/com/netflix/client/config/ReloadableClientConfigTest.java b/ribbon-core/src/test/java/com/netflix/client/config/ReloadableClientConfigTest.java new file mode 100644 index 00000000..54305020 --- /dev/null +++ b/ribbon-core/src/test/java/com/netflix/client/config/ReloadableClientConfigTest.java @@ -0,0 +1,53 @@ +package com.netflix.client.config; + +import com.netflix.config.ConfigurationManager; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TestName; + +public class ReloadableClientConfigTest { + @Rule + public TestName testName = new TestName(); + + private CommonClientConfigKey testKey; + + @Before + public void before() { + this.testKey = new CommonClientConfigKey(testName.getMethodName(), -1) {}; + } + + @Test + public void testOverrideLoadedConfig() { + final DefaultClientConfigImpl overrideconfig = new DefaultClientConfigImpl(); + overrideconfig.set(testKey, 123); + + final DefaultClientConfigImpl config = new DefaultClientConfigImpl(); + config.loadDefaultValues(); + config.applyOverride(overrideconfig); + + Assert.assertEquals(123, config.get(testKey).intValue()); + } + + @Test + public void setBeforeLoading() { + ConfigurationManager.getConfigInstance().setProperty("ribbon." + testKey.key(), "123"); + + final DefaultClientConfigImpl config = new DefaultClientConfigImpl(); + config.loadProperties("foo"); + + Assert.assertEquals(123, config.get(testKey).intValue()); + } + + @Test + public void setAfterLoading() { + final DefaultClientConfigImpl config = new DefaultClientConfigImpl(); + config.loadProperties("foo"); + config.set(testKey, 456); + + ConfigurationManager.getConfigInstance().setProperty("ribbon." + testKey.key(), "123"); + + Assert.assertEquals(123, config.get(testKey).intValue()); + } +}