Skip to content

Commit

Permalink
Merge pull request #628 from rgallardo-netflix/default_methods_fix
Browse files Browse the repository at this point in the history 
Fixed interaction of default methods and parametrized properties.
  • Loading branch information
@rgallardo-netflix
rgallardo-netflix authored Aug 17, 2022
2 parents d7bea50 + 6491abc commit 1a8c26f
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 43 deletions.
79 changes: 38 additions & 41 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 @@ -30,7 +30,7 @@
import java.util.Optional;
import java.util.Set;
import java.util.SortedSet;
import java.util.function.Supplier;
import java.util.function.Function;
import java.util.stream.Collectors;

/**
Expand Down Expand Up @@ -171,36 +171,14 @@ interface MethodInvoker<T> {
T invoke(Object[] args);
}

/**
* Abstract method invoker that encapsulates a property
* @param <T>
*/
private static abstract class PropertyMethodInvoker<T> extends AbstractProperty<T> implements MethodInvoker<T> {
private final Supplier<T> next;

public PropertyMethodInvoker(String key, Supplier<T> next) {
super(key);
this.next = next;
}

@Override
public T invoke(Object[] args) {
T result = get();
if (result == null) {
return next.get();
}
return result;
}
}

private static Map<Type, Supplier<?>> knownCollections = new HashMap<>();
private static Map<Type, Function<Object[], ?>> knownCollections = new HashMap<>();

static {
knownCollections.put(Map.class, Collections::emptyMap);
knownCollections.put(Set.class, Collections::emptySet);
knownCollections.put(SortedSet.class, Collections::emptySortedSet);
knownCollections.put(List.class, Collections::emptyList);
knownCollections.put(LinkedList.class, LinkedList::new);
knownCollections.put(Map.class, (ignored) -> Collections.emptyMap());
knownCollections.put(Set.class, (ignored) -> Collections.emptySet());
knownCollections.put(SortedSet.class, (ignored) -> Collections.emptySortedSet());
knownCollections.put(List.class, (ignored) -> Collections.emptyList());
knownCollections.put(LinkedList.class, (ignored) -> new LinkedList());
}

@SuppressWarnings({ "rawtypes", "unchecked" })
Expand Down Expand Up @@ -266,7 +244,7 @@ else if ("toString".equals(method.getName())) {

final Class<?> returnType = m.getReturnType();

Supplier defaultSupplier = knownCollections.getOrDefault(returnType, () -> null);
Function defaultSupplier = knownCollections.getOrDefault(returnType, (ignored) -> null);

if (m.getAnnotation(DefaultValue.class) != null) {
if (m.isDefault()) {
Expand Down Expand Up @@ -324,11 +302,11 @@ else if ("toString".equals(method.getName())) {
return proxyObject;
}

private static <T> Supplier<T> memoize(T value) {
return () -> value;
private static <T> Function<Object[], T> memoize(T value) {
return (ignored) -> value;
}

private static <T> Supplier<T> createDefaultMethodSupplier(Method method, Class<T> type, T proxyObject) {
private static <T> Function<Object[], T> createDefaultMethodSupplier(Method method, Class<T> type, T proxyObject) {
final MethodHandle methodHandle;

try {
Expand All @@ -345,18 +323,28 @@ private static <T> Supplier<T> createDefaultMethodSupplier(Method method, Class<
methodHandle = MethodHandles.lookup()
.findSpecial(type,
method.getName(),
MethodType.methodType(method.getReturnType(), new Class[0]),
MethodType.methodType(method.getReturnType(), method.getParameterTypes()),
type)
.bindTo(proxyObject);
}
} catch (Throwable e) {
throw new RuntimeException("Failed to create temporary object for " + type.getName(), e);
}

return () -> {
return (args) -> {
try {
//noinspection unchecked
return (T) methodHandle.invokeWithArguments();
if (methodHandle.type().parameterCount() == 0) {
//noinspection unchecked
return (T) methodHandle.invokeWithArguments();
} else if (args != null) {
//noinspection unchecked
return (T) methodHandle.invokeWithArguments(args);
} else {
// This is a handle to a method WITH arguments, being called with none. This happens when toString()
// is trying to build a representation of a proxy that has a parametrized property AND the interface
// provides a default method for it. There's no good default to return here, so we'll just use null
return null;
}
} catch (Throwable e) {
throw new RuntimeException(e);
}
Expand All @@ -368,17 +356,26 @@ protected <T> MethodInvoker<T> createInterfaceProperty(String propName, final T
return (args) -> proxy;
}

protected <T> MethodInvoker<T> createScalarProperty(final Type type, final String propName, Supplier<T> next) {
protected <T> MethodInvoker<T> createScalarProperty(final Type type, final String propName, Function<Object[], T> next) {
LOG.debug("Creating scalar property `{}` for type `{}`", propName, type.getClass());
final Property<T> prop = propertyRepository.get(propName, type);
return args -> Optional.ofNullable(prop.get()).orElseGet(next);
return args -> Optional.ofNullable(prop.get()).orElseGet(() -> next.apply(null));
}

protected <T> MethodInvoker<T> createParameterizedProperty(final Class<T> returnType, final String propName, final String nameAnnot, Supplier<T> next) {
protected <T> MethodInvoker<T> createParameterizedProperty(final Class<T> returnType, final String propName, final String nameAnnot, Function<Object[], T> next) {
LOG.debug("Creating parameterized property `{}` for type `{}`", propName, returnType);
return new MethodInvoker<T>() {
@Override
public T invoke(Object[] args) {
if (args == null) {
// Why would args be null if this is a parametrized property? Because toString() abuses its
// access to this internal representation :-/
// We'll fall back to trying to call the provider for the default value. That works properly if
// it comes from an annotation or the known collections. Our wrapper for default interface methods
// catches this case and just returns a null, which is probably the least bad response.
return next.apply(null);
}

// Determine the actual property name by replacing with arguments using the argument index
// to the method. For example,
// @PropertyName(name="foo.${1}.${0}")
Expand All @@ -392,7 +389,7 @@ public T invoke(Object[] args) {
String propName = new StrSubstitutor(values, "${", "}", '$').replace(nameAnnot);
T result = getPropertyWithDefault(returnType, propName);
if (result == null) {
result = next.get();
result = next.apply(args);
}
return result;
}
Expand Down
40 changes: 38 additions & 2 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 @@ -217,7 +217,29 @@ public void testWithArguments() {
Assert.assertEquals("value2", withArgs.getProperty("b", 2));
Assert.assertEquals("default", withArgs.getProperty("a", 2));
}


public interface WithArgumentsAndDefaultMethod {
@PropertyName(name="${0}.abc.${1}")
default String getProperty(String part0, int part1) {
return "default";
}
}

@Test
public void testWithArgumentsAndDefaultMethod() {
SettableConfig config = new DefaultSettableConfig();
config.setProperty("a.abc.1", "value1");
config.setProperty("b.abc.2", "value2");

PropertyFactory factory = DefaultPropertyFactory.from(config);
ConfigProxyFactory proxy = new ConfigProxyFactory(config, config.getDecoder(), factory);
WithArgumentsAndDefaultMethod withArgsAndDefM = proxy.newProxy(WithArgumentsAndDefaultMethod.class);

Assert.assertEquals("value1", withArgsAndDefM.getProperty("a", 1));
Assert.assertEquals("value2", withArgsAndDefM.getProperty("b", 2));
Assert.assertEquals("default", withArgsAndDefM.getProperty("a", 2));
}

public interface ConfigWithMaps {
@PropertyName(name="map")
default Map<String, Long> getStringToLongMap() { return Collections.singletonMap("default", 0L); }
Expand Down Expand Up @@ -448,7 +470,21 @@ public void testObjectMethods() {
ConfigProxyFactory proxy = new ConfigProxyFactory(config, config.getDecoder(), factory);
WithArguments withArgs = proxy.newProxy(WithArguments.class);

Assert.assertEquals("WithArguments[${0}.abc.${1}='null']", withArgs.toString());
Assert.assertEquals("WithArguments[${0}.abc.${1}='default']", withArgs.toString());
Assert.assertNotNull(withArgs.hashCode());
Assert.assertTrue(withArgs.equals(withArgs));
}

@Test
public void testObjectMethods_ClassWithArgumentsAndDefaultMethod() {
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.
Assert.assertEquals("WithArgumentsAndDefaultMethod[${0}.abc.${1}='null']", withArgs.toString());
Assert.assertNotNull(withArgs.hashCode());
Assert.assertTrue(withArgs.equals(withArgs));
}
Expand Down

0 comments on commit 1a8c26f

Please sign in to comment.