Skip to content

Commit

Permalink
Cleanup compiler warnings.
Browse files Browse the repository at this point in the history
  • Loading branch information
rgallardo-netflix committed Oct 11, 2023
1 parent 05d51af commit 1ae4674
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 53 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
* <p>
* For example,
* <pre>
* {@code
* {@literal @}Configuration(prefix="foo")
* @Configuration(prefix="foo")
* interface FooConfiguration {
* int getTimeout(); // maps to "foo.timeout"
*
Expand All @@ -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.
*
* <p>
* For example,
* <pre>
* {@code
* {@literal @}Configuration(prefix="foo")
* @Configuration(prefix="foo")
* interface FooConfiguration {
* @DefaultValue("1000")
* int getReadTimeout(); // maps to "foo.timeout"
Expand All @@ -73,22 +73,23 @@
* </pre>
*
* 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.
*
* <pre>
* {@code
* FooConfiguration fooConfiguration = configProxyFactory.newProxy(FooConfiguration.class, "otherprefix.foo");
* }
* </pre>
*
* 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);

Expand All @@ -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;
Expand All @@ -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> T newProxy(final Class<T> type) {
return newProxy(type, null);
Expand All @@ -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> T newProxy(final Class<T> 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 <T>
* Encapsulate the invocation of a single method of the interface
*/
interface MethodInvoker<T> {
/**
* Invoke the method with the provided arguments
* @param args
* @return
*/
T invoke(Object[] args);
}
Expand Down Expand Up @@ -194,30 +192,16 @@ <T> T newProxy(final Class<T> 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());
}
};

Expand Down Expand Up @@ -297,7 +281,8 @@ private static <T> Function<Object[], T> 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)));
}
Expand Down Expand Up @@ -370,4 +355,28 @@ <R> R getPropertyWithDefault(Class<R> type, String propName) {
}
};
}

/** Compute a reasonable toString() for a proxy object */
private static <T> String proxyToString(Class<T> type, String prefix, Map<Method, MethodInvoker<?>> invokers, Map<Method, String> 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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -46,6 +47,7 @@ public interface ImmutableConfig {
String getValueWithoutDefault2();
}

@SuppressWarnings("unused")
public interface BaseConfig {
@DefaultValue("basedefault")
String getStr();
Expand All @@ -60,6 +62,7 @@ default long getLongValueWithDefault() {
}
}

@SuppressWarnings("UnusedReturnValue")
public interface RootConfig extends BaseConfig {
@DefaultValue("default")
@Override
Expand Down Expand Up @@ -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(":");
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -259,6 +262,7 @@ public void testWithArgumentsAndPrefix() {
}


@SuppressWarnings("unused")
public interface WithArgumentsAndDefaultMethod {
@PropertyName(name="${0}.abc.${1}")
default String getPropertyWith2Placeholders(String part0, int part1) {
Expand Down Expand Up @@ -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<String> getList();

Set<String> getSet();
Expand Down Expand Up @@ -464,6 +468,7 @@ public void testCollectionsWithoutValue() {
Assert.assertTrue(withCollections.getSortedSet().isEmpty());
}

@SuppressWarnings("unused")
public interface ConfigWithCollectionsWithDefaultValueAnnotation {
@DefaultValue("")
LinkedList<Integer> getLinkedList();
Expand Down Expand Up @@ -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"); }
}

Expand All @@ -515,25 +521,30 @@ 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);
WithArgumentsAndDefaultMethod withArgs = proxy.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.
// 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());
Expand Down

0 comments on commit 1ae4674

Please sign in to comment.