From add6cf13925678f875d75c09007e489772ee29a4 Mon Sep 17 00:00:00 2001 From: Rebeca Gallardo Date: Wed, 25 Sep 2024 16:42:57 -0700 Subject: [PATCH] Throw ParseException when a configuration setting can't be parsed as the type requested. Homogenize the handling of parsing errors. Before this change, some code paths in the default Property implementation would silently swallow parsing errors and either return a defaultValue or null (even for non-nullable methods!) This leads to errors that are hard to debug. Throwing a ParseException is a behaviour change that will lead to simpler debugging, at the expense of surfacing latent bugs in user code. For example, if bad values were being set in config files, the code will now fail instead of silently returning a different value than expected. --- .../netflix/archaius/api/PropertyFactory.java | 4 +- .../archaius/bridge/DynamicPropertyTest.java | 6 +- .../archaius/DefaultPropertyFactory.java | 49 +- .../netflix/archaius/ConfigManagerTest.java | 6 +- .../DefaultPropertyContainerTest.java | 22 - .../netflix/archaius/ProxyFactoryTest.java | 598 ++++++++---------- .../archaius/config/MapConfigTest.java | 13 +- .../archaius/property/PropertyTest.java | 227 +++---- 8 files changed, 432 insertions(+), 493 deletions(-) delete mode 100644 archaius2-core/src/test/java/com/netflix/archaius/DefaultPropertyContainerTest.java diff --git a/archaius2-api/src/main/java/com/netflix/archaius/api/PropertyFactory.java b/archaius2-api/src/main/java/com/netflix/archaius/api/PropertyFactory.java index 924ba091..6792ca62 100644 --- a/archaius2-api/src/main/java/com/netflix/archaius/api/PropertyFactory.java +++ b/archaius2-api/src/main/java/com/netflix/archaius/api/PropertyFactory.java @@ -26,8 +26,8 @@ @Deprecated public interface PropertyFactory extends PropertyRepository { /** - * Create a property for the property name. - * @deprecated Use {@link PropertyRepository#get(String, Type)} instead. + * Create a {@link PropertyContainer} for the given name. + * @deprecated Use {@link PropertyRepository#get(String, Type)} or {@link PropertyRepository#get(String, Class)} instead. */ @Deprecated PropertyContainer getProperty(String propName); diff --git a/archaius2-archaius1-bridge/src/test/java/com/netflix/archaius/bridge/DynamicPropertyTest.java b/archaius2-archaius1-bridge/src/test/java/com/netflix/archaius/bridge/DynamicPropertyTest.java index 4feaa2e2..e53d12f6 100644 --- a/archaius2-archaius1-bridge/src/test/java/com/netflix/archaius/bridge/DynamicPropertyTest.java +++ b/archaius2-archaius1-bridge/src/test/java/com/netflix/archaius/bridge/DynamicPropertyTest.java @@ -1,5 +1,6 @@ package com.netflix.archaius.bridge; +import com.netflix.archaius.api.PropertyRepository; import org.apache.commons.configuration.AbstractConfiguration; import com.google.inject.Guice; @@ -7,7 +8,6 @@ import com.google.inject.Key; import com.netflix.archaius.api.Config; import com.netflix.archaius.api.Property; -import com.netflix.archaius.api.PropertyFactory; import com.netflix.archaius.api.config.SettableConfig; import com.netflix.archaius.api.inject.RuntimeLayer; import com.netflix.archaius.guice.ArchaiusModule; @@ -44,7 +44,7 @@ public void after() { @Test public void settingOnArchaius2UpdateArchaius1(TestInfo testInfo) { String methodName = testInfo.getTestMethod().map(Method::getName).orElse("unknown"); - Property a2prop = injector.getInstance(PropertyFactory.class).getProperty(methodName).asString("default"); + Property a2prop = injector.getInstance(PropertyRepository.class).get(methodName, String.class).orElse("default"); DynamicStringProperty a1prop = DynamicPropertyFactory.getInstance().getStringProperty(methodName, "default"); assertEquals("default", a1prop.get()); @@ -63,7 +63,7 @@ public void testNonStringDynamicProperty() { config.accept(new PrintStreamVisitor()); ConfigurationManager.getConfigInstance().setProperty("foo", 123); - Property prop2 = injector.getInstance(PropertyFactory.class).getProperty("foo").asInteger(1); + Property prop2 = injector.getInstance(PropertyRepository.class).get("foo", Integer.class).orElse(1); DynamicIntProperty prop = DynamicPropertyFactory.getInstance().getIntProperty("foo", 2); diff --git a/archaius2-core/src/main/java/com/netflix/archaius/DefaultPropertyFactory.java b/archaius2-core/src/main/java/com/netflix/archaius/DefaultPropertyFactory.java index cb86d91f..5cd3c771 100644 --- a/archaius2-core/src/main/java/com/netflix/archaius/DefaultPropertyFactory.java +++ b/archaius2-core/src/main/java/com/netflix/archaius/DefaultPropertyFactory.java @@ -6,6 +6,7 @@ import com.netflix.archaius.api.PropertyContainer; import com.netflix.archaius.api.PropertyFactory; import com.netflix.archaius.api.PropertyListener; +import com.netflix.archaius.exceptions.ParseException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -62,6 +63,7 @@ public DefaultPropertyFactory(Config config) { this.config.addListener(this); } + /** @deprecated Use {@link #get(String, Type)} or {@link #get(String, Class)} instead. */ @Override @Deprecated @SuppressWarnings("deprecation") @@ -124,20 +126,24 @@ public Property asType(Class type, T defaultValue) { @Override public Property asType(Function mapper, String defaultValue) { - T typedDefaultValue = mapper.apply(defaultValue); + T typedDefaultValue = applyOrThrow(mapper, defaultValue); return getFromSupplier(propName, null, () -> { String value = config.getString(propName, null); if (value != null) { - try { - return mapper.apply(value); - } catch (Exception e) { - LOG.error("Invalid value '{}' for property '{}'. Will return the default instead.", propName, value); - } + return applyOrThrow(mapper, value); } return typedDefaultValue; }); } + + private T applyOrThrow(Function mapper, String value) { + try { + return mapper.apply(value); + } catch (RuntimeException e) { + throw new ParseException("Invalid value '" + value + "' for property '" + propName + "'.", e); + } + } }; } @@ -208,23 +214,28 @@ public PropertyImpl(KeyAndType keyAndType, Supplier supplier) { @Override public T get() { - int cacheVersion = cache.getStamp(); + int[] cacheVersion = new int[1]; + T currentValue = cache.get(cacheVersion); int latestVersion = masterVersion.get(); - - if (cacheVersion != latestVersion) { - T currentValue = cache.getReference(); - T newValue = null; - try { - newValue = supplier.get(); - } catch (Exception e) { - LOG.error("Unable to get current version of property '{}'", keyAndType.key, e); - } - - if (cache.compareAndSet(currentValue, newValue, cacheVersion, latestVersion)) { - // Possible race condition here but not important enough to warrant locking + + if (cacheVersion[0] == latestVersion) { + return currentValue; + } + + try { + T newValue = supplier.get(); + + if (cache.compareAndSet(currentValue, newValue, cacheVersion[0], latestVersion)) { + // newValue could be stale here already, if the cache was updated *again* between the CAS and this line + // We don't care enough about this edge case to fix it. return newValue; } + + } catch (RuntimeException e) { + LOG.error("Unable to get current version of property '{}'", keyAndType.key, e); + throw e; // Rethrow the exception, our caller should know that the property is not available } + return cache.getReference(); } diff --git a/archaius2-core/src/test/java/com/netflix/archaius/ConfigManagerTest.java b/archaius2-core/src/test/java/com/netflix/archaius/ConfigManagerTest.java index ae529b11..c247f0cf 100644 --- a/archaius2-core/src/test/java/com/netflix/archaius/ConfigManagerTest.java +++ b/archaius2-core/src/test/java/com/netflix/archaius/ConfigManagerTest.java @@ -20,7 +20,7 @@ import java.util.concurrent.atomic.AtomicReference; import com.netflix.archaius.api.Property; -import com.netflix.archaius.api.PropertyFactory; +import com.netflix.archaius.api.PropertyRepository; import com.netflix.archaius.config.DefaultCompositeConfig; import com.netflix.archaius.config.MapConfig; @@ -45,9 +45,9 @@ public void testBasicReplacement() throws ConfigException { .build())); - PropertyFactory factory = DefaultPropertyFactory.from(config); + PropertyRepository factory = DefaultPropertyFactory.from(config); - Property prop = factory.getProperty("abc").asString("defaultValue"); + Property prop = factory.get("abc", String.class).orElse("defaultValue"); // Use an AtomicReference to capture the property value change AtomicReference capturedValue = new AtomicReference<>(""); diff --git a/archaius2-core/src/test/java/com/netflix/archaius/DefaultPropertyContainerTest.java b/archaius2-core/src/test/java/com/netflix/archaius/DefaultPropertyContainerTest.java deleted file mode 100644 index 03f522f7..00000000 --- a/archaius2-core/src/test/java/com/netflix/archaius/DefaultPropertyContainerTest.java +++ /dev/null @@ -1,22 +0,0 @@ -package com.netflix.archaius; - -import com.netflix.archaius.api.Property; -import com.netflix.archaius.api.config.SettableConfig; -import com.netflix.archaius.config.DefaultSettableConfig; -import org.junit.jupiter.api.Test; - -import static org.junit.jupiter.api.Assertions.assertEquals; - -public class DefaultPropertyContainerTest { - @Test - public void basicTest() { - SettableConfig config = new DefaultSettableConfig(); - DefaultPropertyFactory factory = DefaultPropertyFactory.from(config); - Property prop = factory.getProperty("foo").asString("default"); - - assertEquals("default", prop.get()); - config.setProperty("foo", "value1"); - assertEquals("value1", prop.get()); - assertEquals("value1", prop.get()); - } -} diff --git a/archaius2-core/src/test/java/com/netflix/archaius/ProxyFactoryTest.java b/archaius2-core/src/test/java/com/netflix/archaius/ProxyFactoryTest.java index 0eb09c2c..d650d64b 100644 --- a/archaius2-core/src/test/java/com/netflix/archaius/ProxyFactoryTest.java +++ b/archaius2-core/src/test/java/com/netflix/archaius/ProxyFactoryTest.java @@ -27,114 +27,36 @@ import com.netflix.archaius.api.Decoder; import com.netflix.archaius.api.TypeConverter; -import com.netflix.archaius.config.MapConfig; -import com.netflix.archaius.api.Config; import com.netflix.archaius.api.PropertyFactory; import com.netflix.archaius.api.annotations.Configuration; import com.netflix.archaius.api.annotations.DefaultValue; import com.netflix.archaius.api.annotations.PropertyName; import com.netflix.archaius.api.config.SettableConfig; import com.netflix.archaius.config.DefaultSettableConfig; -import com.netflix.archaius.config.EmptyConfig; +import com.netflix.archaius.exceptions.ParseException; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; @SuppressWarnings("deprecation") public class ProxyFactoryTest { - public enum TestEnum { - NONE, - A, - B, - C - } - - @Configuration(immutable=true) - public interface ImmutableConfig { - @DefaultValue("default") - String getValueWithDefault(); - - String getValueWithoutDefault1(); - - String getValueWithoutDefault2(); - } - - @SuppressWarnings("unused") - public interface BaseConfig { - @DefaultValue("basedefault") - String getStr(); - - Boolean getBaseBoolean(); - - @Nullable - Integer getNullable(); + private SettableConfig config; + private PropertyFactory propertyFactory; + private ConfigProxyFactory proxyFactory; - default long getLongValueWithDefault() { - return 42L; - } + @BeforeEach + void setUp() { + config = new DefaultSettableConfig(); + propertyFactory = DefaultPropertyFactory.from(config); + proxyFactory = new ConfigProxyFactory(config, config.getDecoder(), propertyFactory); } - - @SuppressWarnings("UnusedReturnValue") - public interface RootConfig extends BaseConfig { - @DefaultValue("default") - @Override - String getStr(); - - @DefaultValue("0") - int getInteger(); - - @DefaultValue("NONE") - TestEnum getEnum(); - - SubConfig getSubConfig(); - - @DefaultValue("default1:default2") - SubConfigFromString getSubConfigFromString(); - - @DefaultValue("") - Integer[] getIntArray(); - - int getRequiredValue(); - default long getOtherLongValueWithDefault() { - return 43L; - } - } - - public interface SubConfig { - @DefaultValue("default") - String str(); - } - - public static class SubConfigFromString { - private final String[] parts; - - public SubConfigFromString(String value) { - this.parts = value.split(":"); - } - - public String part1() { - return parts[0]; - } - - public String part2() { - return parts[1]; - } - - @Override - public String toString() { - return "SubConfigFromString[" + part1() + ", " + part2() + "]"; - } - } - @Test public void testImmutableDefaultValues() { - SettableConfig config = new DefaultSettableConfig(); config.setProperty("valueWithoutDefault2", "default2"); - PropertyFactory factory = DefaultPropertyFactory.from(config); - ConfigProxyFactory proxy = new ConfigProxyFactory(config, config.getDecoder(), factory); - ImmutableConfig c = proxy.newProxy(ImmutableConfig.class); + ImmutableConfig c = proxyFactory.newProxy(ImmutableConfig.class); assertThat(c.getValueWithDefault(), equalTo("default")); assertThat(c.getValueWithoutDefault2(), equalTo("default2")); @@ -146,13 +68,8 @@ public void testImmutableDefaultValues() { @Test public void testDefaultValues() { - Config config = EmptyConfig.INSTANCE; + RootConfig a = proxyFactory.newProxy(RootConfig.class); - PropertyFactory factory = DefaultPropertyFactory.from(config); - ConfigProxyFactory proxy = new ConfigProxyFactory(config, config.getDecoder(), factory); - - RootConfig a = proxy.newProxy(RootConfig.class); - assertThat(a.getStr(), equalTo("default")); assertThat(a.getInteger(), equalTo(0)); assertThat(a.getEnum(), equalTo(TestEnum.NONE)); @@ -165,15 +82,33 @@ public void testDefaultValues() { assertThat(a.getLongValueWithDefault(), equalTo(42L)); assertThat(a.getOtherLongValueWithDefault(), equalTo(43L)); } + + @Test + public void testBadValuesInConfig() { + config.setProperty("integer", "a"); + + RootConfig a = proxyFactory.newProxy(RootConfig.class); + + // We should be able to *build* the config object, but then get an error when trying to get the value + ParseException pe = assertThrows(ParseException.class, a::getInteger); + assertThat(pe.getMessage(), allOf( + containsString("'integer'"), + containsString("'a'"))); + + // Other values do work + assertEquals(TestEnum.NONE, a.getEnum()); + + // But a subsequent, invalid, change to the configs now results in an error + config.setProperty("enum", "NOTINENUM"); + ParseException pe2 = assertThrows(ParseException.class, a::getEnum); + assertThat(pe2.getMessage(), allOf( + containsString("'enum'"), + containsString("'NOTINENUM'"))); + } @Test public void testDefaultValuesImmutable() { - Config config = EmptyConfig.INSTANCE; - - PropertyFactory factory = DefaultPropertyFactory.from(config); - ConfigProxyFactory proxy = new ConfigProxyFactory(config, config.getDecoder(), factory); - - RootConfig a = proxy.newProxy(RootConfig.class, "", true); + RootConfig a = proxyFactory.newProxy(RootConfig.class, "", true); assertThat(a.getStr(), equalTo("default")); assertThat(a.getInteger(), equalTo(0)); @@ -188,7 +123,6 @@ public void testDefaultValuesImmutable() { @Test public void testAllPropertiesSet() { - SettableConfig config = new DefaultSettableConfig(); config.setProperty("prefix.str", "str1"); config.setProperty("prefix.integer", 1); config.setProperty("prefix.enum", TestEnum.A.name()); @@ -197,10 +131,7 @@ public void testAllPropertiesSet() { config.setProperty("prefix.baseBoolean", true); config.setProperty("prefix.intArray", "0,1,2,3"); - PropertyFactory factory = DefaultPropertyFactory.from(config); - ConfigProxyFactory proxy = new ConfigProxyFactory(config, config.getDecoder(), factory); - - RootConfig a = proxy.newProxy(RootConfig.class, "prefix"); + RootConfig a = proxyFactory.newProxy(RootConfig.class, "prefix"); assertThat(a.getStr(), equalTo("str1")); assertThat(a.getInteger(), equalTo(1)); @@ -222,35 +153,13 @@ public void testAllPropertiesSet() { } } - // Known bug: An interface with a default method MUST be public, otherwise proxy creation will fail. - public interface WithArguments { - @PropertyName(name="${0}.abc.${1}") - @DefaultValue("default") - String getProperty(String part0, int part1); - - @PropertyName(name="${0}.def.${1}") - List getListProperty(String part0, int part1); - - @PropertyName(name="${0}.def.${1}") - default List getListWithDefault(String part0, int part1) { - return Collections.singletonList(part0 + part1); - } - - @PropertyName(name="${0}.def.${1}") - @DefaultValue("default1,default2") - List getListWithAnnotation(String part0, int part1); - } - @Test public void testWithArguments() { - SettableConfig config = new DefaultSettableConfig(); config.setProperty("a.abc.1", "value1"); config.setProperty("b.abc.2", "value2"); config.setProperty("a.def.1", "v1,v2"); - PropertyFactory factory = DefaultPropertyFactory.from(config); - ConfigProxyFactory proxy = new ConfigProxyFactory(config, config.getDecoder(), factory); - WithArguments withArgs = proxy.newProxy(WithArguments.class); + WithArguments withArgs = proxyFactory.newProxy(WithArguments.class); assertEquals("value1", withArgs.getProperty("a", 1)); assertEquals("value2", withArgs.getProperty("b", 2)); @@ -266,27 +175,12 @@ public void testWithArguments() { assertEquals(Arrays.asList("default1", "default2"), withArgs.getListWithAnnotation("a", 2)); } - @Configuration(prefix = "foo.bar") - interface WithArgumentsAndPrefix { - @PropertyName(name="baz.${0}.abc.${1}") - @DefaultValue("default") - String getPropertyWithoutPrefix(String part0, int part1); - - // For backward compatibility, we need to accept PropertyNames that also include the prefix. - @PropertyName(name="foo.bar.baz.${0}.abc.${1}") - @DefaultValue("default") - String getPropertyWithPrefix(String part0, int part1); - } - @Test public void testWithArgumentsAndPrefix() { - SettableConfig config = new DefaultSettableConfig(); config.setProperty("foo.bar.baz.a.abc.1", "value1"); config.setProperty("foo.bar.baz.b.abc.2", "value2"); - PropertyFactory factory = DefaultPropertyFactory.from(config); - ConfigProxyFactory proxy = new ConfigProxyFactory(config, config.getDecoder(), factory); - WithArgumentsAndPrefix withArgs = proxy.newProxy(WithArgumentsAndPrefix.class); + WithArgumentsAndPrefix withArgs = proxyFactory.newProxy(WithArgumentsAndPrefix.class); assertEquals("value1", withArgs.getPropertyWithPrefix("a", 1)); assertEquals("value1", withArgs.getPropertyWithoutPrefix("a", 1)); @@ -296,30 +190,13 @@ public void testWithArgumentsAndPrefix() { assertEquals("default", withArgs.getPropertyWithoutPrefix("a", 2)); } - - @SuppressWarnings("unused") - public interface WithArgumentsAndDefaultMethod { - @PropertyName(name="${0}.abc.${1}") - default String getPropertyWith2Placeholders(String part0, int part1) { - return "defaultFor2"; - } - - @PropertyName(name="${0}.def") - default String getPropertyWith1Placeholder(String part0) { - return "defaultFor1"; - } - } - @Test public void testWithArgumentsAndDefaultMethod() { - SettableConfig config = new DefaultSettableConfig(); config.setProperty("a.abc.1", "value1"); config.setProperty("b.abc.2", "value2"); config.setProperty("c.def", "value_c"); - PropertyFactory factory = DefaultPropertyFactory.from(config); - ConfigProxyFactory proxy = new ConfigProxyFactory(config, config.getDecoder(), factory); - WithArgumentsAndDefaultMethod withArgsAndDefM = proxy.newProxy(WithArgumentsAndDefaultMethod.class); + WithArgumentsAndDefaultMethod withArgsAndDefM = proxyFactory.newProxy(WithArgumentsAndDefaultMethod.class); assertEquals("value1", withArgsAndDefM.getPropertyWith2Placeholders("a", 1)); assertEquals("value2", withArgsAndDefM.getPropertyWith2Placeholders("b", 2)); @@ -328,24 +205,13 @@ public void testWithArgumentsAndDefaultMethod() { assertEquals("value_c", withArgsAndDefM.getPropertyWith1Placeholder("c")); assertEquals("defaultFor1", withArgsAndDefM.getPropertyWith1Placeholder("q")); } - - public interface ConfigWithMaps { - @PropertyName(name="map") - default Map getStringToLongMap() { return Collections.singletonMap("default", 0L); } - - @PropertyName(name="map2") - default Map getLongToStringMap() { return Collections.singletonMap(0L, "default"); } - } @Test public void testWithLongMap() { - SettableConfig config = new DefaultSettableConfig(); config.setProperty("map", "a=123,b=456"); config.setProperty("map2", "1=a,2=b"); - PropertyFactory factory = DefaultPropertyFactory.from(config); - ConfigProxyFactory proxy = new ConfigProxyFactory(config, config.getDecoder(), factory); - ConfigWithMaps withMaps = proxy.newProxy(ConfigWithMaps.class); + ConfigWithMaps withMaps = proxyFactory.newProxy(ConfigWithMaps.class); long sub1 = withMaps.getStringToLongMap().get("a"); long sub2 = withMaps.getStringToLongMap().get("b"); @@ -363,11 +229,7 @@ public void testWithLongMap() { @Test public void testWithLongMapDefaultValue() { - SettableConfig config = new DefaultSettableConfig(); - - PropertyFactory factory = DefaultPropertyFactory.from(config); - ConfigProxyFactory proxy = new ConfigProxyFactory(config, config.getDecoder(), factory); - ConfigWithMaps withArgs = proxy.newProxy(ConfigWithMaps.class); + ConfigWithMaps withArgs = proxyFactory.newProxy(ConfigWithMaps.class); assertEquals(Collections.singletonMap("default", 0L), withArgs.getStringToLongMap()); @@ -376,28 +238,15 @@ public void testWithLongMapDefaultValue() { assertEquals(Collections.singletonMap("foo", 123L), withArgs.getStringToLongMap()); } - public interface ConfigWithCollections { - List getList(); - - Set getSet(); - - SortedSet getSortedSet(); - - LinkedList getLinkedList(); - } - @Test public void testCollections() { - SettableConfig config = new DefaultSettableConfig(); config.setProperty("list", "5,4,3,2,1"); config.setProperty("set", "1,2,3,5,4"); config.setProperty("sortedSet", "5,4,3,2,1"); config.setProperty("linkedList", "5,4,3,2,1"); - - PropertyFactory factory = DefaultPropertyFactory.from(config); - ConfigProxyFactory proxy = new ConfigProxyFactory(config, config.getDecoder(), factory); - ConfigWithCollections withCollections = proxy.newProxy(ConfigWithCollections.class); - + + ConfigWithCollections withCollections = proxyFactory.newProxy(ConfigWithCollections.class); + assertEquals(Arrays.asList(5,4,3,2,1), new ArrayList<>(withCollections.getLinkedList())); assertEquals(Arrays.asList(5,4,3,2,1), new ArrayList<>(withCollections.getList())); assertEquals(Arrays.asList(1,2,3,5,4), new ArrayList<>(withCollections.getSet())); @@ -409,15 +258,12 @@ public void testCollections() { @Test public void testCollectionsImmutable() { - SettableConfig config = new DefaultSettableConfig(); config.setProperty("list", "5,4,3,2,1"); config.setProperty("set", "1,2,3,5,4"); config.setProperty("sortedSet", "5,4,3,2,1"); config.setProperty("linkedList", "5,4,3,2,1"); - PropertyFactory factory = DefaultPropertyFactory.from(config); - ConfigProxyFactory proxy = new ConfigProxyFactory(config, config.getDecoder(), factory); - ConfigWithCollections withCollections = proxy.newProxy(ConfigWithCollections.class, "", true); + ConfigWithCollections withCollections = proxyFactory.newProxy(ConfigWithCollections.class, "", true); assertEquals(Arrays.asList(5,4,3,2,1), new ArrayList<>(withCollections.getLinkedList())); assertEquals(Arrays.asList(5,4,3,2,1), new ArrayList<>(withCollections.getList())); @@ -430,43 +276,27 @@ public void testCollectionsImmutable() { @Test public void emptyNonStringValuesIgnoredInCollections() { - SettableConfig config = new DefaultSettableConfig(); config.setProperty("list", ",4, ,2,1"); config.setProperty("set", ",2, ,5,4"); config.setProperty("sortedSet", ",4, ,2,1"); config.setProperty("linkedList", ",4, ,2,1"); - PropertyFactory factory = DefaultPropertyFactory.from(config); - ConfigProxyFactory proxy = new ConfigProxyFactory(config, config.getDecoder(), factory); - ConfigWithCollections withCollections = proxy.newProxy(ConfigWithCollections.class); + ConfigWithCollections withCollections = proxyFactory.newProxy(ConfigWithCollections.class); assertEquals(Arrays.asList(4,2,1), new ArrayList<>(withCollections.getLinkedList())); assertEquals(Arrays.asList(4,2,1), new ArrayList<>(withCollections.getList())); assertEquals(Arrays.asList(2,5,4), new ArrayList<>(withCollections.getSet())); assertEquals(Arrays.asList(1,2,4), new ArrayList<>(withCollections.getSortedSet())); } - - public interface ConfigWithStringCollections { - List getList(); - - Set getSet(); - - SortedSet getSortedSet(); - - LinkedList getLinkedList(); - } @Test public void emptyStringValuesAreAddedToCollection() { - SettableConfig config = new DefaultSettableConfig(); config.setProperty("list", ",4, ,2,1"); config.setProperty("set", ",2, ,5,4"); config.setProperty("sortedSet", ",4, ,2,1"); config.setProperty("linkedList", ",4, ,2,1"); - PropertyFactory factory = DefaultPropertyFactory.from(config); - ConfigProxyFactory proxy = new ConfigProxyFactory(config, config.getDecoder(), factory); - ConfigWithStringCollections withCollections = proxy.newProxy(ConfigWithStringCollections.class); + ConfigWithStringCollections withCollections = proxyFactory.newProxy(ConfigWithStringCollections.class); assertEquals(Arrays.asList("", "4","", "2","1"), new ArrayList<>(withCollections.getLinkedList())); assertEquals(Arrays.asList("", "4","", "2","1"), new ArrayList<>(withCollections.getList())); @@ -476,11 +306,7 @@ public void emptyStringValuesAreAddedToCollection() { @Test public void collectionsReturnEmptySetsByDefault() { - SettableConfig config = new DefaultSettableConfig(); - - PropertyFactory factory = DefaultPropertyFactory.from(config); - ConfigProxyFactory proxy = new ConfigProxyFactory(config, config.getDecoder(), factory); - ConfigWithStringCollections withCollections = proxy.newProxy(ConfigWithStringCollections.class); + ConfigWithStringCollections withCollections = proxyFactory.newProxy(ConfigWithStringCollections.class); assertTrue(withCollections.getLinkedList().isEmpty()); assertTrue(withCollections.getList().isEmpty()); @@ -490,11 +316,7 @@ public void collectionsReturnEmptySetsByDefault() { @Test public void testCollectionsWithoutValue() { - SettableConfig config = new DefaultSettableConfig(); - - PropertyFactory factory = DefaultPropertyFactory.from(config); - ConfigProxyFactory proxy = new ConfigProxyFactory(config, config.getDecoder(), factory); - ConfigWithCollections withCollections = proxy.newProxy(ConfigWithCollections.class); + ConfigWithCollections withCollections = proxyFactory.newProxy(ConfigWithCollections.class); System.out.println(withCollections.toString()); assertTrue(withCollections.getLinkedList().isEmpty()); @@ -502,75 +324,35 @@ public void testCollectionsWithoutValue() { assertTrue(withCollections.getSet().isEmpty()); assertTrue(withCollections.getSortedSet().isEmpty()); } - - @SuppressWarnings("unused") - public interface ConfigWithCollectionsWithDefaultValueAnnotation { - @DefaultValue("1,2") - LinkedList getLinkedList(); - - @DefaultValue("1,2") - Set getSet(); - @DefaultValue("a=b") - Map getMap(); - } - @Test public void testCollectionsWithDefaultValueAnnotation() { - SettableConfig config = new DefaultSettableConfig(); - - PropertyFactory factory = DefaultPropertyFactory.from(config); - ConfigProxyFactory proxy = new ConfigProxyFactory(config, config.getDecoder(), factory); - ConfigWithCollectionsWithDefaultValueAnnotation withAnnotations = proxy.newProxy(ConfigWithCollectionsWithDefaultValueAnnotation.class); + ConfigWithCollectionsWithDefaultValueAnnotation withAnnotations = proxyFactory.newProxy(ConfigWithCollectionsWithDefaultValueAnnotation.class); assertEquals(new LinkedList<>(Arrays.asList(1, 2)), withAnnotations.getLinkedList()); assertEquals(new HashSet<>(Arrays.asList(1L, 2L)), withAnnotations.getSet()); assertEquals(Collections.singletonMap("a", "b"), withAnnotations.getMap()); } - public interface ConfigWithDefaultStringCollections { - default List getList() { return Collections.singletonList("default"); } - - default Set getSet() { return Collections.singleton("default"); } - - default SortedSet getSortedSet() { return new TreeSet<>(Collections.singleton("default")); } - } - @Test public void interfaceDefaultCollections() { - SettableConfig config = new DefaultSettableConfig(); - - PropertyFactory factory = DefaultPropertyFactory.from(config); - ConfigProxyFactory proxy = new ConfigProxyFactory(config, config.getDecoder(), factory); - ConfigWithDefaultStringCollections withCollections = proxy.newProxy(ConfigWithDefaultStringCollections.class); + ConfigWithDefaultStringCollections withCollections = proxyFactory.newProxy(ConfigWithDefaultStringCollections.class); assertEquals(Collections.singletonList("default"), new ArrayList<>(withCollections.getList())); assertEquals(Collections.singletonList("default"), new ArrayList<>(withCollections.getSet())); assertEquals(Collections.singletonList("default"), new ArrayList<>(withCollections.getSortedSet())); } - @SuppressWarnings("UnusedReturnValue") - public interface FailingError { - default String getValue() { throw new IllegalStateException("error"); } - } - @Test public void interfaceWithBadDefault() { - SettableConfig config = new DefaultSettableConfig(); - - PropertyFactory factory = DefaultPropertyFactory.from(config); - ConfigProxyFactory proxy = new ConfigProxyFactory(config, config.getDecoder(), factory); - FailingError c = proxy.newProxy(FailingError.class); + FailingError c = proxyFactory.newProxy(FailingError.class); assertThrows(RuntimeException.class, c::getValue); } @Test public void testObjectMethods() { // These tests just ensure that toString, equals and hashCode have implementations that don't fail. - SettableConfig config = new DefaultSettableConfig(); - PropertyFactory factory = DefaultPropertyFactory.from(config); - ConfigProxyFactory proxy = new ConfigProxyFactory(config, config.getDecoder(), factory); - WithArguments withArgs = proxy.newProxy(WithArguments.class); + WithArguments withArgs = proxyFactory.newProxy(WithArguments.class); // An older version of this test used to check the entire string, but that's too fragile because the // order of the method descriptors is not stable under different JDKs. @@ -584,10 +366,7 @@ public void testObjectMethods() { @Test public void testObjectMethods_ClassWithArgumentsAndDefaultMethod() { // These tests just ensure that toString, equals and hashCode have implementations that don't fail. - SettableConfig config = new DefaultSettableConfig(); - PropertyFactory factory = DefaultPropertyFactory.from(config); - ConfigProxyFactory proxy = new ConfigProxyFactory(config, config.getDecoder(), factory); - WithArgumentsAndDefaultMethod withArgs = proxy.newProxy(WithArgumentsAndDefaultMethod.class); + WithArgumentsAndDefaultMethod withArgs = proxyFactory.newProxy(WithArgumentsAndDefaultMethod.class); // Printing 'null' here is a compromise. The default method in the interface is being called with a bad signature. // There's nothing the proxy could return here that isn't a lie, but at least this is a mostly harmless lie. @@ -602,9 +381,6 @@ public void testObjectMethods_ClassWithArgumentsAndDefaultMethod() { @Disabled("Manual test. Output is just log entries, can't be verified by CI") @Test public void testLogExcessiveUse() { - SettableConfig config = new DefaultSettableConfig(); - PropertyFactory propertyFactory = DefaultPropertyFactory.from(config); - for (int i = 0; i < 5; i++) { new ConfigProxyFactory(config, config.getDecoder(), propertyFactory); // Last one should emit a log! } @@ -614,21 +390,11 @@ public void testLogExcessiveUse() { new ConfigProxyFactory(otherConfig, config.getDecoder(), propertyFactory); // Should not log! It's only 4 and on a different config. } - ConfigProxyFactory factory = new ConfigProxyFactory(config, config.getDecoder(), propertyFactory); // Should not log, because we only log every 5. + ConfigProxyFactory proxyFactory = new ConfigProxyFactory(config, config.getDecoder(), propertyFactory); // Should not log, because we only log every 5. for (int i = 0; i < 5; i++) { - factory.newProxy(WithArguments.class, "aPrefix"); // Last one should emit a log - } - factory.newProxy(WithArguments.class, "somePrefix"); // This one should not log, because it's a new prefix. - } - - interface ConfigWithNestedInterface { - int intValue(); - - CustomObject customValue(); - - interface CustomObject { - String value(); + proxyFactory.newProxy(WithArguments.class, "aPrefix"); // Last one should emit a log } + proxyFactory.newProxy(WithArguments.class, "somePrefix"); // This one should not log, because it's a new prefix. } @Test @@ -641,11 +407,11 @@ public void testNestedInterfaceWithCustomDecoder() { return Optional.empty(); }; Decoder customDecoder = CustomDecoder.create(Collections.singletonList(customTypeConverterFactory)); - Config config = MapConfig.builder() - .put("intValue", "5") - .put("customValue", "blah") - .build(); + config.setProperty("intValue", "5"); + config.setProperty("customValue", "blah"); config.setDecoder(customDecoder); + + // Can't reuse the shared proxy factory because we want the custom decoder. ConfigProxyFactory proxyFactory = new ConfigProxyFactory(config, config.getDecoder(), DefaultPropertyFactory.from(config)); ConfigWithNestedInterface proxy = proxyFactory.newProxy(ConfigWithNestedInterface.class); @@ -653,6 +419,221 @@ public void testNestedInterfaceWithCustomDecoder() { assertEquals("BLAH", proxy.customValue().value()); } + @Test + public void testInterfaceWithStaticMethods() { + config.setProperty("config.foo", "foo-value-updated"); + ConfigWithStaticMethods configWithStaticMethods = proxyFactory.newProxy(ConfigWithStaticMethods.class); + assertEquals("foo-value-updated", configWithStaticMethods.foo()); + } + + @Test + public void testInvalidInterface() { + RuntimeException e = assertThrows(RuntimeException.class, () -> proxyFactory.newProxy(ConfigWithBadSettings.class)); + + assertThat(e.getMessage(), + allOf( + containsString("ConfigWithBadSettings"), + containsString("getInteger"), + containsString("getEnum"), + containsString("getAnIntWithParam"))); + } + + + ////////////////////////////////////////////////////////////////// + /// Test Interfaces + ////////////////////////////////////////////////////////////////// + public enum TestEnum { + NONE, + A, + B, + C + } + + @Configuration(immutable=true) + public interface ImmutableConfig { + @DefaultValue("default") + String getValueWithDefault(); + + String getValueWithoutDefault1(); + + String getValueWithoutDefault2(); + } + + @SuppressWarnings("unused") + public interface BaseConfig { + @DefaultValue("basedefault") + String getStr(); + + Boolean getBaseBoolean(); + + @Nullable + Integer getNullable(); + + default long getLongValueWithDefault() { + return 42L; + } + } + + @SuppressWarnings("UnusedReturnValue") + public interface RootConfig extends BaseConfig { + @DefaultValue("default") + @Override + String getStr(); + + @DefaultValue("0") + int getInteger(); + + @DefaultValue("NONE") + TestEnum getEnum(); + + SubConfig getSubConfig(); + + @DefaultValue("default1:default2") + SubConfigFromString getSubConfigFromString(); + + @DefaultValue("") + Integer[] getIntArray(); + + int getRequiredValue(); + + default long getOtherLongValueWithDefault() { + return 43L; + } + } + + public interface SubConfig { + @DefaultValue("default") + String str(); + } + + public static class SubConfigFromString { + private final String[] parts; + + public SubConfigFromString(String value) { + this.parts = value.split(":"); + } + + public String part1() { + return parts[0]; + } + + public String part2() { + return parts[1]; + } + + @Override + public String toString() { + return "SubConfigFromString[" + part1() + ", " + part2() + "]"; + } + } + + // Known bug: An interface with a default method MUST be public, otherwise proxy creation will fail. + public interface WithArguments { + @PropertyName(name="${0}.abc.${1}") + @DefaultValue("default") + String getProperty(String part0, int part1); + + @PropertyName(name="${0}.def.${1}") + List getListProperty(String part0, int part1); + + @PropertyName(name="${0}.def.${1}") + default List getListWithDefault(String part0, int part1) { + return Collections.singletonList(part0 + part1); + } + + @PropertyName(name="${0}.def.${1}") + @DefaultValue("default1,default2") + List getListWithAnnotation(String part0, int part1); + } + + @Configuration(prefix = "foo.bar") + interface WithArgumentsAndPrefix { + @PropertyName(name="baz.${0}.abc.${1}") + @DefaultValue("default") + String getPropertyWithoutPrefix(String part0, int part1); + + // For backward compatibility, we need to accept PropertyNames that also include the prefix. + @PropertyName(name="foo.bar.baz.${0}.abc.${1}") + @DefaultValue("default") + String getPropertyWithPrefix(String part0, int part1); + } + + @SuppressWarnings("unused") + public interface WithArgumentsAndDefaultMethod { + @PropertyName(name="${0}.abc.${1}") + default String getPropertyWith2Placeholders(String part0, int part1) { + return "defaultFor2"; + } + + @PropertyName(name="${0}.def") + default String getPropertyWith1Placeholder(String part0) { + return "defaultFor1"; + } + } + + public interface ConfigWithMaps { + @PropertyName(name="map") + default Map getStringToLongMap() { return Collections.singletonMap("default", 0L); } + + @PropertyName(name="map2") + default Map getLongToStringMap() { return Collections.singletonMap(0L, "default"); } + } + + public interface ConfigWithCollections { + List getList(); + + Set getSet(); + + SortedSet getSortedSet(); + + LinkedList getLinkedList(); + } + + public interface ConfigWithStringCollections { + List getList(); + + Set getSet(); + + SortedSet getSortedSet(); + + LinkedList getLinkedList(); + } + + @SuppressWarnings("unused") + public interface ConfigWithCollectionsWithDefaultValueAnnotation { + @DefaultValue("1,2") + LinkedList getLinkedList(); + + @DefaultValue("1,2") + Set getSet(); + + @DefaultValue("a=b") + Map getMap(); + } + + public interface ConfigWithDefaultStringCollections { + default List getList() { return Collections.singletonList("default"); } + + default Set getSet() { return Collections.singleton("default"); } + + default SortedSet getSortedSet() { return new TreeSet<>(Collections.singleton("default")); } + } + + @SuppressWarnings("UnusedReturnValue") + public interface FailingError { + default String getValue() { throw new IllegalStateException("error"); } + } + + interface ConfigWithNestedInterface { + int intValue(); + + CustomObject customValue(); + + interface CustomObject { + String value(); + } + } + @Configuration(prefix = "config") @SuppressWarnings("unused") public interface ConfigWithStaticMethods { @@ -669,15 +650,6 @@ static int baz(int x) { } } - @Test - public void testInterfaceWithStaticMethods() { - SettableConfig config = new DefaultSettableConfig(); - config.setProperty("config.foo", "foo-value-updated"); - ConfigProxyFactory proxyFactory = new ConfigProxyFactory(config, config.getDecoder(), DefaultPropertyFactory.from(config)); - ConfigWithStaticMethods configWithStaticMethods = proxyFactory.newProxy(ConfigWithStaticMethods.class); - assertEquals("foo-value-updated", configWithStaticMethods.foo()); - } - @Configuration(prefix = "config") @SuppressWarnings("unused") public interface ConfigWithBadSettings { @@ -690,20 +662,4 @@ public interface ConfigWithBadSettings { // A parametrized method requires a @PropertyName annotation int getAnIntWithParam(String param); } - - @Test - public void testInvalidInterface() { - SettableConfig config = new DefaultSettableConfig(); - PropertyFactory factory = DefaultPropertyFactory.from(config); - ConfigProxyFactory proxy = new ConfigProxyFactory(config, config.getDecoder(), factory); - - RuntimeException e = assertThrows(RuntimeException.class, () -> proxy.newProxy(ConfigWithBadSettings.class)); - - assertThat(e.getMessage(), - allOf( - containsString("ConfigWithBadSettings"), - containsString("getInteger"), - containsString("getEnum"), - containsString("getAnIntWithParam"))); - } } diff --git a/archaius2-core/src/test/java/com/netflix/archaius/config/MapConfigTest.java b/archaius2-core/src/test/java/com/netflix/archaius/config/MapConfigTest.java index cb0968f5..c86b7598 100644 --- a/archaius2-core/src/test/java/com/netflix/archaius/config/MapConfigTest.java +++ b/archaius2-core/src/test/java/com/netflix/archaius/config/MapConfigTest.java @@ -15,6 +15,7 @@ */ package com.netflix.archaius.config; +import java.time.Duration; import java.util.Collection; import java.util.HashMap; import java.util.HashSet; @@ -93,7 +94,12 @@ public void nonExistentLong() { public void nonExistentShort() { assertThrows(NoSuchElementException.class, () -> config.getShort("nonexistent")); } - + + @Test + public void nonExistentCustomType() { + assertThrows(NoSuchElementException.class, () -> config.get(Duration.class, "nonexistent")); + } + @Test public void invalidBigDecimal() { assertThrows(ParseException.class, () -> config.getBigDecimal("badnumber")); @@ -143,6 +149,11 @@ public void invalidLong() { public void invalidShort() { assertThrows(ParseException.class, () -> config.getShort("badnumber")); } + + @Test + public void invalidCustomType() { + assertThrows(ParseException.class, () -> config.get(Duration.class, "str")); + } @Test public void interpolationShouldWork() { diff --git a/archaius2-core/src/test/java/com/netflix/archaius/property/PropertyTest.java b/archaius2-core/src/test/java/com/netflix/archaius/property/PropertyTest.java index 65bd6ebe..c9fa8578 100644 --- a/archaius2-core/src/test/java/com/netflix/archaius/property/PropertyTest.java +++ b/archaius2-core/src/test/java/com/netflix/archaius/property/PropertyTest.java @@ -31,21 +31,24 @@ import java.util.function.Consumer; import java.util.function.Function; +import com.netflix.archaius.api.PropertyContainer; +import com.netflix.archaius.exceptions.ParseException; +import org.junit.jupiter.api.BeforeEach; import org.mockito.Mockito; import com.netflix.archaius.DefaultPropertyFactory; -import com.netflix.archaius.api.Config; import com.netflix.archaius.api.Property; import com.netflix.archaius.api.Property.Subscription; import com.netflix.archaius.api.PropertyFactory; import com.netflix.archaius.api.PropertyListener; import com.netflix.archaius.api.config.SettableConfig; -import com.netflix.archaius.api.exceptions.ConfigException; import com.netflix.archaius.config.DefaultSettableConfig; -import com.netflix.archaius.config.MapConfig; import org.junit.jupiter.api.Test; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.allOf; +import static org.hamcrest.Matchers.containsString; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotEquals; @@ -55,6 +58,7 @@ @SuppressWarnings("deprecation") public class PropertyTest { + static class MyService { private final Property value; private final Property value2; @@ -63,9 +67,9 @@ static class MyService { MyService(PropertyFactory config) { setValueCallsCounter = new AtomicInteger(0); - value = config.getProperty("foo").asInteger(1); + value = config.get("foo", Integer.class).orElse(1); value.addListener(new MethodInvoker<>(this, "setValue")); - value2 = config.getProperty("foo").asInteger(2); + value2 = config.get("foo", Integer.class).orElse(2); } // Called by the config listener. @@ -94,24 +98,30 @@ public String toString() { } } + private SettableConfig config; + private DefaultPropertyFactory factory; + + + @BeforeEach + void setUp() { + config = new DefaultSettableConfig(); + factory = DefaultPropertyFactory.from(config); + } + @Test public void testServiceInitializationWithDefaultProperties() { - SettableConfig config = new DefaultSettableConfig(); - DefaultPropertyFactory factory = DefaultPropertyFactory.from(config); MyService service = new MyService(factory); // Verify initial property values assertEquals(1, (int) service.value.get()); assertEquals(2, (int) service.value2.get()); - // Verify setValue() was called once for each initialization + assertEquals(0, service.setValueCallsCounter.get()); } @Test public void testPropertyValuesUpdateAndEffect() { - SettableConfig config = new DefaultSettableConfig(); - DefaultPropertyFactory factory = DefaultPropertyFactory.from(config); MyService service = new MyService(factory); // Setting up properties @@ -126,26 +136,22 @@ public void testPropertyValuesUpdateAndEffect() { @Test public void testBasicTypes() { - SettableConfig config = new DefaultSettableConfig(); - DefaultPropertyFactory factory = DefaultPropertyFactory.from(config); config.setProperty("foo", "10"); config.setProperty("shmoo", "true"); config.setProperty("loo", CustomType.ONE_TWO); - Property bigDecimalProp = factory.getProperty("foo").asType(BigDecimal.class, - BigDecimal.ONE); - Property bigIntegerProp = factory.getProperty("foo").asType(BigInteger.class, - BigInteger.ONE); - Property booleanProp = factory.getProperty("shmoo").asType(Boolean.class, false); - Property byteProp = factory.getProperty("foo").asType(Byte.class, (byte) 0x1); - Property doubleProp = factory.getProperty("foo").asType(Double.class, 1.0); - Property floatProp = factory.getProperty("foo").asType(Float.class, 1.0f); - Property intProp = factory.getProperty("foo").asType(Integer.class, 1); - Property longProp = factory.getProperty("foo").asType(Long.class, 1L); - Property shortProp = factory.getProperty("foo").asType(Short.class, (short) 1); - Property stringProp = factory.getProperty("foo").asType(String.class, "1"); - Property customTypeProp = factory.getProperty("loo").asType(CustomType.class, - CustomType.DEFAULT); + Property bigDecimalProp = factory.get("foo", BigDecimal.class); + Property bigIntegerProp = factory.get("foo", BigInteger.class); + Property booleanProp = factory.get("shmoo", Boolean.class); + Property byteProp = factory.get("foo", Byte.class); + Property doubleProp = factory.get("foo", Double.class); + Property floatProp = factory.get("foo", Float.class); + Property intProp = factory.get("foo", Integer.class); + Property longProp = factory.get("foo", Long.class); + Property shortProp = factory.get("foo", Short.class); + Property stringProp = factory.get("foo", String.class); + Property customTypeProp = factory.get("loo", CustomType.class); + assertEquals(BigDecimal.TEN, bigDecimalProp.get()); assertEquals(BigInteger.TEN, bigIntegerProp.get()); assertEquals(true, booleanProp.get()); @@ -161,8 +167,6 @@ public void testBasicTypes() { @Test public void testCollectionTypes() { - SettableConfig config = new DefaultSettableConfig(); - DefaultPropertyFactory factory = DefaultPropertyFactory.from(config); config.setProperty("foo", "10,13,13,20"); config.setProperty("shmoo", "1=PT15M,0=PT0S"); @@ -202,8 +206,6 @@ public void testCollectionTypes() { @Test public void testCollectionTypesImmutability() { - SettableConfig config = new DefaultSettableConfig(); - DefaultPropertyFactory factory = DefaultPropertyFactory.from(config); config.setProperty("foo", "10,13,13,20"); config.setProperty("bar", ""); config.setProperty("baz", "a=1,b=2"); @@ -229,12 +231,9 @@ public void testCollectionTypesImmutability() { @Test public void testUpdateDynamicChild() { - SettableConfig config = new DefaultSettableConfig(); - DefaultPropertyFactory factory = DefaultPropertyFactory.from(config); - - Property intProp1 = factory.getProperty("foo").asInteger(1); - Property intProp2 = factory.getProperty("foo").asInteger(2); - Property strProp = factory.getProperty("foo").asString("3"); + Property intProp1 = factory.get("foo", Integer.class).orElse(1); + Property intProp2 = factory.get("foo", Integer.class).orElse(2); + Property strProp = factory.get("foo", String.class).orElse("3"); assertEquals(1, (int)intProp1.get()); assertEquals(2, (int)intProp2.get()); @@ -248,30 +247,21 @@ public void testUpdateDynamicChild() { @Test public void testDefaultNull() { - SettableConfig config = new DefaultSettableConfig(); - DefaultPropertyFactory factory = DefaultPropertyFactory.from(config); - - Property prop = factory.getProperty("foo").asInteger(null); + Property prop = factory.get("foo", Integer.class); assertNull(prop.get()); } @Test - public void testDefault() { - SettableConfig config = new DefaultSettableConfig(); - DefaultPropertyFactory factory = DefaultPropertyFactory.from(config); - - Property prop = factory.getProperty("foo").asInteger(123); + public void testOrElse() { + Property prop = factory.get("foo", Integer.class).orElse(123); assertEquals(123, prop.get().intValue()); } @Test public void testUpdateValue() { - SettableConfig config = new DefaultSettableConfig(); config.setProperty("goo", "456"); - DefaultPropertyFactory factory = DefaultPropertyFactory.from(config); - - Property prop = factory.getProperty("foo").asInteger(123); + Property prop = factory.get("foo", Integer.class).orElse(123); config.setProperty("foo", 1); assertEquals(1, prop.get().intValue()); @@ -286,12 +276,9 @@ public void testUpdateValue() { @Test public void testUpdateCallback() { - SettableConfig config = new DefaultSettableConfig(); config.setProperty("goo", "456"); - DefaultPropertyFactory factory = DefaultPropertyFactory.from(config); - - Property prop = factory.getProperty("foo").asInteger(123); + Property prop = factory.get("foo", Integer.class).orElse(123); final AtomicReference current = new AtomicReference<>(); prop.addListener(new PropertyListener() { @Override @@ -318,14 +305,10 @@ public void onParseError(Throwable error) { @Test public void unregisterOldCallback() { - SettableConfig config = new DefaultSettableConfig(); - - DefaultPropertyFactory factory = DefaultPropertyFactory.from(config); - //noinspection unchecked PropertyListener listener = Mockito.mock(PropertyListener.class); - Property prop = factory.getProperty("foo").asInteger(1); + Property prop = factory.get("foo", Integer.class); prop.addListener(listener); Mockito.verify(listener, Mockito.never()).accept(Mockito.anyInt()); @@ -340,9 +323,6 @@ public void unregisterOldCallback() { @Test public void subscribePropertyChange() { - SettableConfig config = new DefaultSettableConfig(); - DefaultPropertyFactory factory = DefaultPropertyFactory.from(config); - Property prop = factory.get("foo", String.class) .map(Integer::parseInt) .orElse(2) @@ -360,14 +340,10 @@ public void subscribePropertyChange() { @Test public void unsubscribeOnChange() { - SettableConfig config = new DefaultSettableConfig(); - - DefaultPropertyFactory factory = DefaultPropertyFactory.from(config); - //noinspection unchecked Consumer consumer = Mockito.mock(Consumer.class); - Property prop = factory.getProperty("foo").asInteger(1); + Property prop = factory.get("foo", Integer.class); Subscription sub = prop.onChange(consumer); Mockito.verify(consumer, Mockito.never()).accept(Mockito.anyInt()); @@ -383,9 +359,6 @@ public void unsubscribeOnChange() { @Test public void chainedPropertyNoneSet() { - MapConfig config = MapConfig.builder().build(); - DefaultPropertyFactory factory = DefaultPropertyFactory.from(config); - Property prop = factory .get("first", Integer.class) .orElseGet("second"); @@ -395,9 +368,6 @@ public void chainedPropertyNoneSet() { @Test public void chainedPropertyDefault() { - SettableConfig config = new DefaultSettableConfig(); - DefaultPropertyFactory factory = DefaultPropertyFactory.from(config); - Property prop = factory .get("first", Integer.class) .orElseGet("second") @@ -408,8 +378,7 @@ public void chainedPropertyDefault() { @Test public void chainedPropertySecondSet() { - MapConfig config = MapConfig.builder().put("second", 2).build(); - DefaultPropertyFactory factory = DefaultPropertyFactory.from(config); + config.setProperty("second", 2); Property prop = factory .get("first", Integer.class) @@ -421,8 +390,7 @@ public void chainedPropertySecondSet() { @Test public void chainedPropertyFirstSet() { - MapConfig config = MapConfig.builder().put("first", 1).build(); - DefaultPropertyFactory factory = DefaultPropertyFactory.from(config); + config.setProperty("first", 1); Property prop = factory .get("first", Integer.class) @@ -434,9 +402,7 @@ public void chainedPropertyFirstSet() { @Test public void chainedPropertyNotification() { - SettableConfig config = new DefaultSettableConfig(); config.setProperty("first", 1); - DefaultPropertyFactory factory = DefaultPropertyFactory.from(config); //noinspection unchecked Consumer consumer = Mockito.mock(Consumer.class); @@ -471,12 +437,37 @@ public void chainedPropertyNotification() { config.clearProperty("second"); Mockito.verify(consumer, Mockito.times(1)).accept(3); } + + @Test + public void testBadValue() { + config.setProperty("first", "bad"); + + Property prop = factory + .get("first", Integer.class); + + ParseException pe = assertThrows(ParseException.class, prop::get); + assertThat(pe.getMessage(), allOf( + containsString("'first'"), + containsString("'bad'"))); + } + + @Test + public void chainedPropertyBadValue() { + config.setProperty("first", "bad"); + + Property prop = factory + .get("first", Integer.class) + .orElse(3); + + ParseException pe = assertThrows(ParseException.class, prop::get); + assertThat(pe.getMessage(), allOf( + containsString("'first'"), + containsString("'bad'"))); + } @Test public void testCache() { - SettableConfig config = new DefaultSettableConfig(); config.setProperty("foo", "1"); - DefaultPropertyFactory factory = DefaultPropertyFactory.from(config); // This can't be a lambda because then mockito can't subclass it to spy on it :-P //noinspection Convert2Lambda,Anonymous2MethodRef @@ -512,51 +503,15 @@ public Integer apply(String t) { @Test public void mapDiscardsType() { - MapConfig config = MapConfig.builder().build(); - DefaultPropertyFactory factory = DefaultPropertyFactory.from(config); - assertThrows(IllegalStateException.class, () -> factory .get("first", String.class) .orElseGet("second") .map(Integer::parseInt) .orElseGet("third")); } - - @Test - public void customMappingWithDefault() { - Config config = MapConfig.builder().build(); - PropertyFactory factory = DefaultPropertyFactory.from(config); - - Integer value = factory.getProperty("a").asType(Integer::parseInt, "1").get(); - assertEquals(1, value.intValue()); - } - - @Test - public void customMapping() { - Config config = MapConfig.builder() - .put("a", "2") - .build(); - PropertyFactory factory = DefaultPropertyFactory.from(config); - - Integer value = factory.getProperty("a").asType(Integer::parseInt, "1").get(); - assertEquals(2, value.intValue()); - } - - @Test - public void customMappingWithError() { - Config config = MapConfig.builder() - .put("a", "###bad_integer_value###") - .build(); - PropertyFactory factory = DefaultPropertyFactory.from(config); - - Integer value = factory.getProperty("a").asType(Integer::parseInt, "1").get(); - assertEquals(1, value.intValue()); - } @Test public void getListShouldReuseKey() { - SettableConfig config = new DefaultSettableConfig(); - DefaultPropertyFactory factory = DefaultPropertyFactory.from(config); config.setProperty("geralt", "of,rivia"); List expectedList = Arrays.asList("of", "rivia"); @@ -572,8 +527,6 @@ public void getListShouldReuseKey() { @Test public void getSetShouldReuseKey() { - SettableConfig config = new DefaultSettableConfig(); - DefaultPropertyFactory factory = DefaultPropertyFactory.from(config); config.setProperty("geralt", "of,rivia"); Set expectedSet = new HashSet<>(Arrays.asList("of", "rivia")); @@ -589,8 +542,6 @@ public void getSetShouldReuseKey() { @Test public void getMapShouldReuseKey() { - SettableConfig config = new DefaultSettableConfig(); - DefaultPropertyFactory factory = DefaultPropertyFactory.from(config); config.setProperty("geralt", "of=rivia"); Map expectedMap = new HashMap<>(); @@ -607,8 +558,6 @@ public void getMapShouldReuseKey() { @Test public void getCollectionShouldNotReuseKeyWithDifferentTypes() { - SettableConfig config = new DefaultSettableConfig(); - DefaultPropertyFactory factory = DefaultPropertyFactory.from(config); config.setProperty("geralt", "of,rivia"); Property> firstReference = factory.getList("geralt", String.class); @@ -649,4 +598,38 @@ private void ensureReferencesMatch(Property firstReference, Property secon firstReference, secondReference, shouldMatch ? "equal" : "not equal", e.getMessage())); } } + + // Tests for the implementation of the deprecated PropertyContainer interface + + @Test + public void testPropertyContainer() { + PropertyContainer container = factory.getProperty("foo"); + Property prop = container.asInteger(1); + + // Get the default + assertEquals(1, prop.get().intValue()); + // Read via the custom parser feature + assertEquals(1, container.asType(Integer::parseInt, "1").get().intValue()); + + // Set to a new value, as a string + config.setProperty("foo", "2"); + assertEquals(2, prop.get().intValue()); + + // Read via the custom parser feature + assertEquals(2, container.asType(Integer::parseInt, "1").get().intValue()); + + // Set to a new value, as a number + config.setProperty("foo", 3); + assertEquals(3, prop.get().intValue()); + + // Set to an invalid value + config.setProperty("foo", "notAnInt"); + ParseException pe = assertThrows(ParseException.class, prop::get); + assertThat(pe.getMessage(), allOf(containsString("foo"), containsString("notAnInt"))); + + // Using the custom parser feature should still return a ParseException. Older versions of the + // code would silently return the default value instead. + ParseException pe2 = assertThrows(ParseException.class, () -> container.asType(Integer::parseInt, "1").get()); + assertThat(pe2.getMessage(), allOf(containsString("foo"), containsString("notAnInt"))); + } }