diff --git a/archaius2-core/src/main/java/com/netflix/archaius/ConfigProxyFactory.java b/archaius2-core/src/main/java/com/netflix/archaius/ConfigProxyFactory.java index bb3171a3..ceb391c0 100644 --- a/archaius2-core/src/main/java/com/netflix/archaius/ConfigProxyFactory.java +++ b/archaius2-core/src/main/java/com/netflix/archaius/ConfigProxyFactory.java @@ -33,11 +33,11 @@ * Factory for binding a configuration interface to properties in a {@link PropertyFactory} * instance. Getter methods on the interface are mapped by naming convention * by the property name may be overridden using the @PropertyName annotation. - * + *

* For example, *

  * {@code
- * {@literal @}Configuration(prefix="foo")
+ * @Configuration(prefix="foo")
  * interface FooConfiguration {
  *    int getTimeout();     // maps to "foo.timeout"
  *
@@ -50,11 +50,11 @@
  * that the default value type is a string to allow for interpolation.  Alternatively, methods can
  * provide a default method implementation.  Note that {@literal @}DefaultValue cannot be added to a default
  * method as it would introduce ambiguity as to which mechanism wins.
- *
+ * 

* For example, *

  * {@code
- * {@literal @}Configuration(prefix="foo")
+ * @Configuration(prefix="foo")
  * interface FooConfiguration {
  *    @DefaultValue("1000")
  *    int getReadTimeout();     // maps to "foo.timeout"
@@ -73,7 +73,7 @@
  * 
* * To override the prefix in {@literal @}Configuration or provide a prefix when there is no - * @Configuration annotation simply pass in a prefix in the call to newProxy. + * {@literal @}Configuration annotation simply pass in a prefix in the call to newProxy. * *
  * {@code 
@@ -81,14 +81,15 @@
  * }
  * 
* - * By default all properties are dynamic and can therefore change from call to call. To make the + * By default, all properties are dynamic and can therefore change from call to call. To make the * configuration static set the immutable attributes of @Configuration to true. * * Note that an application should normally have just one instance of ConfigProxyFactory * and PropertyFactory since PropertyFactory caches {@link com.netflix.archaius.api.Property} objects. * - * @see {@literal }@Configuration + * @see Configuration */ +@SuppressWarnings("deprecation") public class ConfigProxyFactory { private static final Logger LOG = LoggerFactory.getLogger(ConfigProxyFactory.class); @@ -99,6 +100,7 @@ public class ConfigProxyFactory { private final PropertyRepository propertyRepository; private final Config config; + @SuppressWarnings("DIAnnotationInspectionTool") @Inject public ConfigProxyFactory(Config config, Decoder decoder, PropertyFactory factory) { this.decoder = decoder; @@ -123,10 +125,6 @@ public ConfigProxyFactory(Config config) { /** * Create a proxy for the provided interface type for which all getter methods are bound * to a Property. - * - * @param type - * @param config - * @return */ public T newProxy(final Class type) { return newProxy(type, null); @@ -147,22 +145,22 @@ private String derivePrefix(Configuration annot, String prefix) { return prefix + "."; } - + + /** + * Create a proxy for the provided interface type for which all getter methods are bound + * to a Property. The proxy uses the provided prefix, even if there is a {@link Configuration} annotation in TYPE. + */ public T newProxy(final Class type, final String initialPrefix) { Configuration annot = type.getAnnotation(Configuration.class); - return newProxy(type, initialPrefix, annot == null ? false : annot.immutable()); + return newProxy(type, initialPrefix, annot != null && annot.immutable()); } /** - * Encapsulated the invocation of a single method of the interface - * - * @param + * Encapsulate the invocation of a single method of the interface */ interface MethodInvoker { /** * Invoke the method with the provided arguments - * @param args - * @return */ T invoke(Object[] args); } @@ -194,30 +192,16 @@ T newProxy(final Class type, final String initialPrefix, boolean immutabl if (invoker != null) { return invoker.invoke(args); } - if ("equals".equals(method.getName())) { - return proxy == args[0]; - } - else if ("hashCode".equals(method.getName())) { - return System.identityHashCode(proxy); - } - else if ("toString".equals(method.getName())) { - StringBuilder sb = new StringBuilder(); - sb.append(type.getSimpleName()).append("["); - sb.append(invokers.entrySet().stream().map(entry -> { - StringBuilder sbProperty = new StringBuilder(); - sbProperty.append(propertyNames.get(entry.getKey()).substring(prefix.length())).append("='"); - try { - sbProperty.append(entry.getValue().invoke(null)); - } catch (Exception e) { - sbProperty.append(e.getMessage()); - } - sbProperty.append("'"); - return sbProperty.toString(); - }).collect(Collectors.joining(","))); - sb.append("]"); - return sb.toString(); - } else { - throw new NoSuchMethodError(method.getName() + " not found on interface " + type.getName()); + + switch (method.getName()) { + case "equals": + return proxy == args[0]; + case "hashCode": + return System.identityHashCode(proxy); + case "toString": + return proxyToString(type, prefix, invokers, propertyNames); + default: + throw new NoSuchMethodError(method.getName() + " not found on interface " + type.getName()); } }; @@ -297,7 +281,8 @@ private static Function createAnnotatedMethodSupplier(Method m, String value = m.getAnnotation(DefaultValue.class).value(); if (returnType == String.class) { - return memoize((T) config.resolve(value)); + //noinspection unchecked + return memoize((T) config.resolve(value)); // The cast is actually a no-op, T == String here! } else { return memoize(decoder.decode(returnType, config.resolve(value))); } @@ -370,4 +355,28 @@ R getPropertyWithDefault(Class type, String propName) { } }; } + + /** Compute a reasonable toString() for a proxy object */ + private static String proxyToString(Class type, String prefix, Map> invokers, Map propertyNames) { + StringBuilder sb = new StringBuilder(); + sb.append(type.getSimpleName()).append("["); + + sb.append(invokers.entrySet().stream().map(entry -> { + StringBuilder sbProperty = new StringBuilder(); + sbProperty.append(propertyNames.get(entry.getKey()).substring(prefix.length())).append("='"); + + try { + sbProperty.append(entry.getValue().invoke(null)); + } catch (Exception e) { + sbProperty.append(e.getMessage()); + } + + sbProperty.append("'"); + return sbProperty.toString(); + + }).collect(Collectors.joining(","))); + + sb.append("]"); + return sb.toString(); + } } 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 310a19cc..f07674f2 100644 --- a/archaius2-core/src/test/java/com/netflix/archaius/ProxyFactoryTest.java +++ b/archaius2-core/src/test/java/com/netflix/archaius/ProxyFactoryTest.java @@ -28,6 +28,7 @@ import com.netflix.archaius.config.DefaultSettableConfig; import com.netflix.archaius.config.EmptyConfig; +@SuppressWarnings("deprecation") public class ProxyFactoryTest { public enum TestEnum { NONE, @@ -46,6 +47,7 @@ public interface ImmutableConfig { String getValueWithoutDefault2(); } + @SuppressWarnings("unused") public interface BaseConfig { @DefaultValue("basedefault") String getStr(); @@ -60,6 +62,7 @@ default long getLongValueWithDefault() { } } + @SuppressWarnings("UnusedReturnValue") public interface RootConfig extends BaseConfig { @DefaultValue("default") @Override @@ -92,7 +95,7 @@ public interface SubConfig { } public static class SubConfigFromString { - private String[] parts; + private final String[] parts; public SubConfigFromString(String value) { this.parts = value.split(":"); @@ -203,11 +206,11 @@ public void testAllPropertiesSet() { a.getRequiredValue(); Assert.fail("should have failed with no value for requiredValue"); } - catch (Exception e) { + catch (Exception expected) { } } - static interface WithArguments { + interface WithArguments { @PropertyName(name="${0}.abc.${1}") @DefaultValue("default") String getProperty(String part0, int part1); @@ -229,7 +232,7 @@ public void testWithArguments() { } @Configuration(prefix = "foo.bar") - static interface WithArgumentsAndPrefix { + interface WithArgumentsAndPrefix { @PropertyName(name="baz.${0}.abc.${1}") @DefaultValue("default") String getPropertyWithoutPrefix(String part0, int part1); @@ -259,6 +262,7 @@ public void testWithArgumentsAndPrefix() { } + @SuppressWarnings("unused") public interface WithArgumentsAndDefaultMethod { @PropertyName(name="${0}.abc.${1}") default String getPropertyWith2Placeholders(String part0, int part1) { @@ -407,7 +411,7 @@ public void emptyNonStringValuesIgnoredInCollections() { Assert.assertEquals(Arrays.asList(1,2,4), new ArrayList<>(withCollections.getSortedSet())); } - public static interface ConfigWithStringCollections { + public interface ConfigWithStringCollections { List getList(); Set getSet(); @@ -464,6 +468,7 @@ public void testCollectionsWithoutValue() { Assert.assertTrue(withCollections.getSortedSet().isEmpty()); } + @SuppressWarnings("unused") public interface ConfigWithCollectionsWithDefaultValueAnnotation { @DefaultValue("") LinkedList getLinkedList(); @@ -494,12 +499,13 @@ public void interfaceDefaultCollections() { ConfigProxyFactory proxy = new ConfigProxyFactory(config, config.getDecoder(), factory); ConfigWithDefaultStringCollections withCollections = proxy.newProxy(ConfigWithDefaultStringCollections.class); - Assert.assertEquals(Arrays.asList("default"), new ArrayList<>(withCollections.getList())); - Assert.assertEquals(Arrays.asList("default"), new ArrayList<>(withCollections.getSet())); - Assert.assertEquals(Arrays.asList("default"), new ArrayList<>(withCollections.getSortedSet())); + Assert.assertEquals(Collections.singletonList("default"), new ArrayList<>(withCollections.getList())); + Assert.assertEquals(Collections.singletonList("default"), new ArrayList<>(withCollections.getSet())); + Assert.assertEquals(Collections.singletonList("default"), new ArrayList<>(withCollections.getSortedSet())); } - public static interface FailingError { + @SuppressWarnings("UnusedReturnValue") + public interface FailingError { default String getValue() { throw new IllegalStateException("error"); } } @@ -515,18 +521,22 @@ public void interfaceWithBadDefault() { @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); Assert.assertEquals("WithArguments[${0}.abc.${1}='default']", withArgs.toString()); + //noinspection ObviousNullCheck Assert.assertNotNull(withArgs.hashCode()); - Assert.assertTrue(withArgs.equals(withArgs)); + //noinspection EqualsWithItself + Assert.assertEquals(withArgs, withArgs); } @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); @@ -534,6 +544,7 @@ public void testObjectMethods_ClassWithArgumentsAndDefaultMethod() { // 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. + // This test depends implicitly on the iteration order for HashMap, which could change on future Java releases. Assert.assertEquals("WithArgumentsAndDefaultMethod[${0}.def='null',${0}.abc.${1}='null']", withArgs.toString()); //noinspection ObviousNullCheck Assert.assertNotNull(withArgs.hashCode());