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"))); + } }