iSharkFly-Open/jfaker
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix method resolution with overloaded methods (#147)

Browse Source 
Fixes DiUS/java-faker#143

There was an intermittent test failure where the error message would say :
Expected: is "Unable to coerce x to Long via Long(String) constructor."
but: was "Unable to coerce x to Integer via Integer(String) constructor."

The issue is getMethod() returning the methods in a non-deterministic order.
Fixing this could have been easy by simply changing the check to look for Long or Integer but
this would hide a real issue where by methodName(int) called via methodName(Long.MAX_VALUE) would
die even if methodName(long) existed because it would attempt to coerce the arguments
to int ONLY and ignore the long variation.

I altered the logic to attempt to coerce the arguments at the same time the method is located
by name.  This means that methodName(Long.MAX_VALUE) will skip over methodName(int) because it would
fail and will continue to find methodName(int).
This commit is contained in:
Trever Shick
2016-10-22 22:40:49 -04:00
committed by Ricky Yim
parent b2f756cb9d
commit 078d44e232
2 changed files with 55 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/main/java/com/github/javafaker/service/FakeValuesService.java
+52 -28
View File
@@ -9,6 +9,7 @@ import org.yaml.snakeyaml.Yaml;
import java.io.InputStream;
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.*;
import java.util.logging.Level;
@@ -430,20 +431,10 @@ public class FakeValuesService {
return null;
}
try {
Method accessor = accessor(obj, directive, args);
if (accessor == null) {
return null;
}
// coerce the string arguments into the correct argument types
List<Object> coerced = null;
try {
coerced = coerceArguments(accessor, args);
} catch (RuntimeException re) {
log.log(Level.FINE, "Unable to coerce arguments : " + re.getMessage());
}
return (accessor == null || coerced == null)
final MethodAndCoercedArgs accessor = accessor(obj, directive, args);
return (accessor == null)
? null
: string(accessor.invoke(obj, coerced.toArray()));
: string(accessor.invoke(obj));
} catch (Exception e) {
log.log(Level.FINE, "Can't call " + directive + " on " + obj, e);
return null;
@@ -460,23 +451,20 @@ public class FakeValuesService {
try {
String fakerMethodName = classAndMethod[0].replaceAll("_", "");
Method fakerAccessor = accessor(faker, fakerMethodName, Collections.<String>emptyList());
MethodAndCoercedArgs fakerAccessor = accessor(faker, fakerMethodName, Collections.<String>emptyList());
if (fakerAccessor == null) {
throw new RuntimeException("Can't find top level faker object named " + fakerMethodName + ".");
}
Object objectWithMethodToInvoke = fakerAccessor.invoke(faker);
String nestedMethodName = classAndMethod[1].replaceAll("_", "");
final Method accessor = accessor(objectWithMethodToInvoke, classAndMethod[1].replaceAll("_", ""), args);
final MethodAndCoercedArgs accessor = accessor(objectWithMethodToInvoke, classAndMethod[1].replaceAll("_", ""), args);
if (accessor == null) {
throw new RuntimeException("Can't find method on "
+ objectWithMethodToInvoke.getClass().getSimpleName()
+ " called " + nestedMethodName + ".");
}
final List<Object> coerced = coerceArguments(accessor, args);
Object ret = accessor.invoke(objectWithMethodToInvoke, coerced.toArray());
return ret == null ? null : ret.toString();
return string(accessor.invoke(objectWithMethodToInvoke));
} catch (Exception e) {
if (e instanceof RuntimeException) {
throw (RuntimeException) e;
@@ -489,19 +477,23 @@ public class FakeValuesService {
/**
* Find an accessor by name ignoring case.
*/
private Method accessor(Object onObject, String name, List<String> args) {
private MethodAndCoercedArgs accessor(Object onObject, String name, List<String> args) {
log.log(Level.FINE, "Find accessor named " + name + " on " + onObject.getClass().getSimpleName() + " with args " + args);
Method fakerAccessor = null;
for (Method m : onObject.getClass().getMethods()) {
if (m.getName().equalsIgnoreCase(name) && m.getParameterTypes().length == args.size()) {
fakerAccessor = m;
break;
if (m.getName().equalsIgnoreCase(name)
&& m.getParameterTypes().length == args.size()) {
final List<Object> coercedArguments = coerceArguments(m, args);
if (coercedArguments != null) {
return new MethodAndCoercedArgs(m, coercedArguments);
}
}
}
if (fakerAccessor == null && name.contains("_")) {
fakerAccessor = accessor(onObject, name.replaceAll("_", ""), args);
if (name.contains("_")) {
return accessor(onObject, name.replaceAll("_", ""), args);
}
return fakerAccessor;
return null;
}
/**
@@ -521,7 +513,8 @@ public class FakeValuesService {
coerced.add(coercedArgument);
} catch (Exception e) {
throw new RuntimeException("Unable to coerce " + args.get(i) + " to " + toType.getSimpleName() + " via " + toType.getSimpleName() + "(String) constructor.");
log.fine("Unable to coerce " + args.get(i) + " to " + toType.getSimpleName() + " via " + toType.getSimpleName() + "(String) constructor.");
return null;
}
}
return coerced;
@@ -530,4 +523,35 @@ public class FakeValuesService {
private String string(Object obj) {
return (obj == null) ? null : obj.toString();
}
/**
* simple wrapper class around an accessor and a list of coerced arguments.
* this is useful as we get to find the method and coerce the arguments in one
* shot, returning both when successful. This saves us from doing it more than once (coercing args).
*/
private class MethodAndCoercedArgs {
private final Method method;
private final List<Object> coerced;
private MethodAndCoercedArgs(Method m, List<Object> coerced) {
this.method = requireNonNull(m, "method cannot be null");
this.coerced = requireNonNull(coerced, "coerced arguments cannot be null");
}
private Object invoke(Object on) throws InvocationTargetException, IllegalAccessException {
return method.invoke(on, coerced.toArray());
}
/**
* source level precludes me from using Objects.requireNonNull
*/
private <T> T requireNonNull(T instance, String messageIfNull) {
if (instance == null) {
throw new NullPointerException(messageIfNull);
}
return instance;
}
}
}
src/test/java/com/github/javafaker/service/FakeValuesServiceTest.java
+3 -3
View File
@@ -226,12 +226,12 @@ public class FakeValuesServiceTest extends AbstractFakerTest {
* if the message changes, it's ok to update the test provided
* the two conditions above are still true.
*/
@Test/*(expected = RuntimeException.class)*/
@Test
public void expressionWithValidFakerObjectValidMethodInvalidArgs() {
expressionShouldFailWith("#{Number.number_between 'x','y'}",
"Unable to coerce x to Long via Long(String) constructor.");
"Can't find method on Number called numberbetween.");
}
/**
* Two things are important here:
* 1) the message in the exception should be USEFUL