Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Small fixes for collection types in configuration proxies: #724

Merged
merged 3 commits into from
May 22, 2024
Merged

Small fixes for collection types in configuration proxies: #724

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0cb7ba8
Small fixes for collection types in configuration proxies:
rgallardo-netflix May 21, 2024
d77a3e3
Merge branch 'refs/heads/2.x' into Collection-types-in-proxies-RG
rgallardo-netflix May 21, 2024
0cf06f4
A config proxy's toString() method's output can change subtly under d…
rgallardo-netflix May 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 7 additions & 13 deletions archaius2-core/src/main/java/com/netflix/archaius/ConfigProxyFactory.java
View file
Open in desktop
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
Open in desktop
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
Loading