Merge pull request #1238 from jclouds/fix-reflection-bug-take-2

Fixing a bug in Reflection2.methodForParams which can lead to bridge methods being returned
This commit is contained in:
Adrian Cole 2013-01-24 07:42:11 -08:00
commit 6bae2f7b47
2 changed files with 114 additions and 4 deletions

View File

@ -18,6 +18,7 @@
*/
package org.jclouds.reflect;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Throwables.propagate;
import static com.google.common.collect.Iterables.tryFind;
@ -114,9 +115,13 @@ public class Reflection2 {
/**
* returns an {@link Invokable} object that reflects a method present in the {@link TypeToken} type.
* If there are multiple methods of the same name and parameter list, returns the method in the nearest
* ancestor with the most specific return type (see {@link Class#getDeclaredMethod}).
*
* @param ownerType
* corresponds to {@link Invokable#getOwnerType()}
* @param name
* name of the method to be returned
* @param parameterTypes
* corresponds to {@link Method#getParameterTypes()}
*
@ -224,15 +229,22 @@ public class Reflection2 {
.newBuilder().build(new CacheLoader<TypeTokenNameAndParameterTypes, Invokable<?, ?>>() {
public Invokable<?, ?> load(final TypeTokenNameAndParameterTypes key) {
Set<Invokable<?, ?>> methods = get(methodsForTypeToken, key.type);
/*
* There may be multiple methods, even on the most immediate ancestor,
* of a method with the required name and parameter set. This will occur
* if the method overrides one declared in a parent class with a less specific
* return type. These bridge methods inserted by the compiler will be marked
* as "synthetic".
*/
Optional<Invokable<?, ?>> method = tryFind(methods, new Predicate<Invokable<?, ?>>() {
public boolean apply(Invokable<?, ?> input) {
return Objects.equal(input.getName(), key.name)
// Invokable doesn't expose Method#isBridge
return !input.isSynthetic() && Objects.equal(input.getName(), key.name)
&& Objects.equal(toClasses(input.getParameters()), key.parameterTypes);
}
});
if (method.isPresent())
return method.get();
throw new IllegalArgumentException("no such method " + key.toString() + "in: " + methods);
checkArgument(method.isPresent(), "no such method %s in: %s", key.toString(), methods);
return method.get();
}
});
@ -265,6 +277,8 @@ public class Reflection2 {
/**
* this gets all declared methods, not just public ones. makes them accessible. Does not include Object methods.
* Invokables for a type are ordered so all invokables on a subtype are always listed before invokables on a
* supertype (see {@link TypeToken#getTypes()}).
*/
private static LoadingCache<TypeToken<?>, Set<Invokable<?, ?>>> methodsForTypeToken = CacheBuilder
.newBuilder().build(new CacheLoader<TypeToken<?>, Set<Invokable<?, ?>>>() {

View File

@ -19,12 +19,17 @@
package org.jclouds.reflect;
import static com.google.common.base.Functions.toStringFunction;
import static com.google.common.base.Throwables.propagate;
import static java.lang.String.format;
import static org.jclouds.reflect.Reflection2.constructors;
import static org.jclouds.reflect.Reflection2.method;
import static org.jclouds.reflect.Reflection2.methods;
import static org.jclouds.reflect.Reflection2.typeToken;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.fail;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.util.HashSet;
import java.util.Set;
import java.util.SortedSet;
@ -32,6 +37,9 @@ import java.util.SortedSet;
import org.testng.annotations.Test;
import com.google.common.base.Function;
import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableSet;
import com.google.common.reflect.Invokable;
@ -113,9 +121,97 @@ public class Reflection2Test {
.addAll(setMethods).build());
}
public void testOverriddenMethodWithNarrowedReturnType() throws NoSuchMethodException {
LoadingCache<TypeToken<?>, Set<Invokable<?, ?>>> methodsForTypeTokenBackup =
getStaticField(Reflection2.class, "methodsForTypeToken");
// expecting two methods: the declared "method" and the bridge version with return type Set
final Method[] methods = ChildOverridesAndNarrowsMethod.class.getDeclaredMethods();
try {
/*
* Force Reflection2.methodsForTypeToken to reflect the fact that the declared methods
* of a class are not returned in any particular order.
*/
setStaticField(Reflection2.class, "methodsForTypeToken", CacheBuilder.newBuilder().build(
new CacheLoader<TypeToken<?>, Set<Invokable<?, ?>>>() {
@Override
public Set<Invokable<?, ?>> load(TypeToken<?> key) throws Exception {
if (!key.equals(TypeToken.of(ChildOverridesAndNarrowsMethod.class))) {
fail(format("expected only key %s to be requested, but was %s",
TypeToken.of(ChildOverridesAndNarrowsMethod.class), key));
}
return ImmutableSet.<Invokable<?, ?>>of(Invokable.from(methods[0]), Invokable.from(methods[1]));
}
}));
// getMethod returns the method with the *narrowest* return type if one exists
assertEquals(Reflection2.method(ChildOverridesAndNarrowsMethod.class, "method"),
Invokable.from(ChildOverridesAndNarrowsMethod.class.getMethod("method")));
// now the opposite order
Reflection2Test.<LoadingCache<?, ?>>
getStaticField(Reflection2.class, "methodForParams").invalidateAll();
setStaticField(Reflection2.class, "methodsForTypeToken", CacheBuilder.newBuilder().build(
new CacheLoader<TypeToken<?>, Set<Invokable<?, ?>>>() {
@Override
public Set<Invokable<?, ?>> load(TypeToken<?> key) throws Exception {
if (!key.equals(TypeToken.of(ChildOverridesAndNarrowsMethod.class))) {
fail(format("expected only key %s to be requested, but was %s",
TypeToken.of(ChildOverridesAndNarrowsMethod.class), key));
}
return ImmutableSet.<Invokable<?, ?>>of(Invokable.from(methods[1]), Invokable.from(methods[0]));
}
}));
// getMethod returns the method with the *narrowest* return type if one exists
assertEquals(Reflection2.method(ChildOverridesAndNarrowsMethod.class, "method"),
Invokable.from(ChildOverridesAndNarrowsMethod.class.getMethod("method")));
} finally {
setStaticField(Reflection2.class, "methodsForTypeToken", methodsForTypeTokenBackup);
}
}
@SuppressWarnings("unchecked")
private static <T> T getStaticField(Class<?> declaringClass, String fieldName) {
try {
Field field = declaringClass.getDeclaredField(fieldName);
field.setAccessible(true);
// static field
return (T) field.get(null);
} catch (NoSuchFieldException exception) {
throw propagate(exception);
} catch (IllegalAccessException exception) {
throw propagate(exception);
}
}
private static void setStaticField(Class<?> declaringClass, String fieldName, Object value) {
try {
Field field = declaringClass.getDeclaredField(fieldName);
field.setAccessible(true);
// static field
field.set(null, value);
} catch (NoSuchFieldException exception) {
throw propagate(exception);
} catch (IllegalAccessException exception) {
throw propagate(exception);
}
}
static final Function<Invokable<?, ?>, String> invokableToName = new Function<Invokable<?, ?>, String>() {
public String apply(Invokable<?, ?> input) {
return input.getName();
}
};
private static class ParentWithMethod {
@SuppressWarnings("unused")
public Set<Object> method() {
return null;
}
}
private static class ChildOverridesAndNarrowsMethod extends ParentWithMethod {
@Override
public SortedSet<Object> method() {
return null;
}
}
}