Skip to content

Commit

Permalink
Merge pull request #724 from Netflix/Collection-types-in-proxies-RG
Browse files Browse the repository at this point in the history 
Small fixes for collection types in configuration proxies:
  • Loading branch information
@rgallardo-netflix
rgallardo-netflix authored May 22, 2024
2 parents 9d20251 + 0cf06f4 commit 19dfd7e
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 20 deletions.
20 changes: 7 additions & 13 deletions archaius2-core/src/main/java/com/netflix/archaius/ConfigProxyFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,6 @@ public class ConfigProxyFactory {
* @param decoder Used to parse strings from {@link DefaultValue} annotations into the proper types.
* @param factory Used to access the config values that are returned by proxies created by this factory.
*/
@SuppressWarnings("DIAnnotationInspectionTool")
@Inject
public ConfigProxyFactory(Config config, Decoder decoder, PropertyFactory factory) {
this.decoder = decoder;
Expand Down Expand Up @@ -191,7 +190,7 @@ public <T> T newProxy(final Class<T> type, final String initialPrefix) {
/**
* Encapsulate the invocation of a single method of the interface
*/
interface PropertyValueGetter<T> {
protected interface PropertyValueGetter<T> {
/**
* Invoke the method with the provided arguments
*/
Expand Down Expand Up @@ -286,7 +285,7 @@ private <T> MethodInvokerHolder buildInvokerForMethod(Class<T> type, String pref
final Function defaultValueSupplier;

if (m.getAnnotation(DefaultValue.class) != null) {
defaultValueSupplier = createAnnotatedMethodSupplier(m, returnType, config, decoder);
defaultValueSupplier = createAnnotatedMethodSupplier(m, m.getGenericReturnType(), config, decoder);
} else if (m.isDefault()) {
defaultValueSupplier = createDefaultMethodSupplier(m, type, proxyObject);
} else {
Expand Down Expand Up @@ -319,7 +318,7 @@ private <T> MethodInvokerHolder buildInvokerForMethod(Class<T> type, String pref
propertyNameTemplate = nameAnnot.name();
}

propertyValueGetter = createParameterizedProperty(returnType, propertyNameTemplate, defaultValueSupplier);
propertyValueGetter = createParameterizedProperty(m.getGenericReturnType(), propertyNameTemplate, defaultValueSupplier);

} else {
// Anything else.
Expand Down Expand Up @@ -351,16 +350,10 @@ private static String getPropertyName(String prefix, Method m, PropertyName name


/** Build a supplier that returns the (interpolated and decoded) value from the method's @DefaultValue annotation */
private static <T> Function<Object[], T> createAnnotatedMethodSupplier(Method m, Class<T> returnType, Config config, Decoder decoder) {
private static <T> Function<Object[], T> createAnnotatedMethodSupplier(Method m, Type returnType, Config config, Decoder decoder) {
if (m.isDefault()) {
throw new IllegalArgumentException("@DefaultValue cannot be defined on a method with a default implementation for method "
+ m.getDeclaringClass().getName() + "#" + m.getName());
} else if (
Map.class.isAssignableFrom(returnType) ||
List.class.isAssignableFrom(returnType) ||
Set.class.isAssignableFrom(returnType) ) {
throw new IllegalArgumentException("@DefaultValue cannot be used with collections. Use default method implemenation instead "
+ m.getDeclaringClass().getName() + "#" + m.getName());
}

String value = m.getAnnotation(DefaultValue.class).value();
Expand Down Expand Up @@ -448,7 +441,7 @@ protected <T> PropertyValueGetter<T> createScalarProperty(final Type type, final
* into the property name from the method's @PropertyName annotation, then returns the value set in config for the
* computed property name. If not set, it forwards the call with the same parameters to the defaultValueSupplier.
*/
protected <T> PropertyValueGetter<T> createParameterizedProperty(final Class<T> returnType, final String propertyNameTemplate, Function<Object[], T> defaultValueSupplier) {
protected <T> PropertyValueGetter<T> createParameterizedProperty(final Type returnType, final String propertyNameTemplate, Function<Object[], T> defaultValueSupplier) {
LOG.debug("Creating parameterized property `{}` for type `{}`", propertyNameTemplate, returnType);

return args -> {
Expand All @@ -470,7 +463,8 @@ protected <T> PropertyValueGetter<T> createParameterizedProperty(final Class<T>
String interpolatedPropertyName = new StrSubstitutor(new ArrayLookup<>(args), "${", "}", '$')
.replace(propertyNameTemplate);

T result = propertyRepository.get(interpolatedPropertyName, returnType).get();
//noinspection unchecked
T result = (T) propertyRepository.get(interpolatedPropertyName, returnType).get();
if (result == null) {
result = defaultValueSupplier.apply(args);
}
Expand Down
50 changes: 43 additions & 7 deletions archaius2-core/src/test/java/com/netflix/archaius/ProxyFactoryTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -218,26 +219,49 @@ public void testAllPropertiesSet() {
catch (Exception expected) {
}
}

interface WithArguments {

// 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<String> getListProperty(String part0, int part1);

@PropertyName(name="${0}.def.${1}")
default List<String> getListWithDefault(String part0, int part1) {
return Collections.singletonList(part0 + part1);
}

@PropertyName(name="${0}.def.${1}")
@DefaultValue("default1,default2")
List<String> 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);

assertEquals("value1", withArgs.getProperty("a", 1));
assertEquals("value2", withArgs.getProperty("b", 2));
assertEquals("default", withArgs.getProperty("a", 2));

assertEquals(Arrays.asList("v1", "v2"), withArgs.getListProperty("a", 1));
assertEquals(Collections.emptyList(), withArgs.getListProperty("b", 2));

assertEquals(Arrays.asList("v1", "v2"), withArgs.getListWithDefault("a", 1));
assertEquals(Collections.singletonList("a2"), withArgs.getListWithDefault("a", 2));

assertEquals(Arrays.asList("v1", "v2"), withArgs.getListWithAnnotation("a", 1));
assertEquals(Arrays.asList("default1", "default2"), withArgs.getListWithAnnotation("a", 2));
}

@Configuration(prefix = "foo.bar")
Expand Down Expand Up @@ -479,8 +503,14 @@ public void testCollectionsWithoutValue() {

@SuppressWarnings("unused")
public interface ConfigWithCollectionsWithDefaultValueAnnotation {
@DefaultValue("")
@DefaultValue("1,2")
LinkedList<Integer> getLinkedList();

@DefaultValue("1,2")
Set<Long> getSet();

@DefaultValue("a=b")
Map<String, String> getMap();
}

@Test
Expand All @@ -489,7 +519,11 @@ public void testCollectionsWithDefaultValueAnnotation() {

PropertyFactory factory = DefaultPropertyFactory.from(config);
ConfigProxyFactory proxy = new ConfigProxyFactory(config, config.getDecoder(), factory);
assertThrows(RuntimeException.class, () -> proxy.newProxy(ConfigWithCollectionsWithDefaultValueAnnotation.class));
ConfigWithCollectionsWithDefaultValueAnnotation withAnnotations = proxy.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 {
Expand Down Expand Up @@ -535,8 +569,10 @@ public void testObjectMethods() {
PropertyFactory factory = DefaultPropertyFactory.from(config);
ConfigProxyFactory proxy = new ConfigProxyFactory(config, config.getDecoder(), factory);
WithArguments withArgs = proxy.newProxy(WithArguments.class);

assertEquals("WithArguments[${0}.abc.${1}='default']", withArgs.toString());

// 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.
assertTrue(withArgs.toString().startsWith("WithArguments["), "Expected toString() to start with the simple name of the proxy class" );
//noinspection ObviousNullCheck
assertNotNull(withArgs.hashCode());
//noinspection EqualsWithItself
Expand Down

0 comments on commit 19dfd7e

Please sign in to comment.