Skip to content

[XBEAN-453] try to explicit the parameter types matched instead of us… #48

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

Open
rmannibucau wants to merge 5 commits into
base: trunk
Choose a base branch
from
Open

[XBEAN-453] try to explicit the parameter types matched instead of us… #48

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d83ef54
[XBEAN-453] try to explicit the parameter types matched instead of us…
rmannibucau Dec 31, 2025
a59f793
Fix typo, fix comparison in findInstanceFactory(...)
rzo1 Jan 3, 2026
862602d
WIP - Add unit test for findStaticFactory(...)
rzo1 Jan 3, 2026
1b2a85d
WIP - Update test with missing properties (like in TomEE)
rzo1 Jan 3, 2026
0e2d7aa
WIP - Update test with additional cases
rzo1 Jan 3, 2026
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
90 changes: 59 additions & 31 deletions xbean-reflect/src/main/java/org/apache/xbean/recipe/ReflectionUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.util.Collections;
import java.util.Comparator;
import java.util.EnumSet;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.LinkedList;
import java.util.List;
Expand Down Expand Up @@ -601,7 +602,9 @@ public static ConstructorFactory findConstructor(Class typeClass, List<String> p
boolean useNoArgConstructor = false;
// verify parameter names and types are the same length
if (parameterNames != null) {
if (!hasParameterTypes) parameterTypes = Collections.nCopies(parameterNames.size(), null);
if (!hasParameterTypes) {
parameterTypes = Collections.nCopies(parameterNames.size(), Object.class);
}
if (parameterNames.size() != parameterTypes.size()) {
throw new ConstructionException("Invalid ObjectRecipe: recipe has " + parameterNames.size() +
" parameter names and " + parameterTypes.size() + " parameter types");
Expand All @@ -616,11 +619,14 @@ public static ConstructorFactory findConstructor(Class typeClass, List<String> p


// get all methods sorted so that the methods with the most constructor args are first
List<Constructor> constructors = new ArrayList<Constructor>(Arrays.asList(typeClass.getConstructors()));
constructors.addAll(Arrays.asList(typeClass.getDeclaredConstructors()));
List<Constructor> constructors = new ArrayList<Constructor>(Arrays.asList(typeClass.getDeclaredConstructors()));
Collections.sort(constructors, new Comparator<Constructor>() {
public int compare(Constructor constructor1, Constructor constructor2) {
return constructor2.getParameterTypes().length - constructor1.getParameterTypes().length;
int diff = constructor2.getParameterTypes().length - constructor1.getParameterTypes().length;
if (diff == 0) { // prefer public over private
return visibilityLevel(constructor1.getModifiers()) - visibilityLevel(constructor2.getModifiers());
}
return diff;
}
});

Expand All @@ -635,7 +641,7 @@ public int compare(Constructor constructor1, Constructor constructor2) {
if (constructor.getParameterTypes().length != parameterTypes.size()) {
if (matchLevel < 1) {
matchLevel = 1;
missException = new MissingFactoryMethodException("Constructor has " + constructor.getParameterTypes().length + " arugments " +
missException = new MissingFactoryMethodException("Constructor has " + constructor.getParameterTypes().length + " arguments " +
"but expected " + parameterTypes.size() + " arguments: " + constructor);
}
continue;
Expand Down Expand Up @@ -686,11 +692,26 @@ public int compare(Constructor constructor1, Constructor constructor2) {

if (missException != null) {
throw missException;
} else {
StringBuffer buffer = new StringBuffer("Unable to find a valid constructor: ");
buffer.append("public void ").append(typeClass.getName()).append(toParameterList(parameterTypes));
throw new ConstructionException(buffer.toString());
}

StringBuilder buffer = new StringBuilder("Unable to find a valid constructor: ");
buffer.append("public void ").append(typeClass.getName()).append(toParameterList(parameterTypes));
throw new ConstructionException(buffer.toString());
}

private static int visibilityLevel(int modifiers) {
if (Modifier.isPrivate(modifiers)) {
return 3;
}
if (Modifier.isProtected(modifiers)) {
return 2;
}
// 1 = package
if (Modifier.isPublic(modifiers)) {
return 0;
}
// unlikely
return 4;
}

public static StaticFactory findStaticFactory(Class typeClass, String factoryMethod, List<? extends Class<?>> parameterTypes, Set<Option> options) {
Expand All @@ -714,8 +735,11 @@ public static StaticFactory findStaticFactory(Class typeClass, String factoryMet
}

// verify parameter names and types are the same length
boolean specifiedParameterTypes = parameterTypes != null;
if (parameterNames != null) {
if (parameterTypes == null) parameterTypes = Collections.nCopies(parameterNames.size(), null);
if (parameterTypes == null) {
parameterTypes = Collections.nCopies(parameterNames.size(), Object.class);
}
if (parameterNames.size() != parameterTypes.size()) {
throw new ConstructionException("Invalid ObjectRecipe: recipe has " + parameterNames.size() +
" parameter names and " + parameterTypes.size() + " parameter types");
Expand All @@ -725,36 +749,40 @@ public static StaticFactory findStaticFactory(Class typeClass, String factoryMet
// so we will only use the no-arg constructor
parameterNames = Collections.emptyList();
parameterTypes = Collections.emptyList();
specifiedParameterTypes = true;
}

// get all methods sorted so that the methods with the most constructor args are first
List<Method> methods = new ArrayList<Method>(Arrays.asList(typeClass.getMethods()));
methods.addAll(Arrays.asList(typeClass.getDeclaredMethods()));
Collections.sort(methods, new Comparator<Method>() {
public int compare(Method method2, Method method1) {
return method1.getParameterTypes().length - method2.getParameterTypes().length;
int diff = method1.getParameterTypes().length - method2.getParameterTypes().length;
if (diff == 0) { // prefer public over private
return visibilityLevel(method1.getModifiers()) - visibilityLevel(method2.getModifiers());
}
return diff;
}
});


// as we check each constructor, we remember the closest invalid match so we can throw a nice exception to the user
int matchLevel = 0;
MissingFactoryMethodException missException = null;

boolean allowPrivate = options.contains(Option.PRIVATE_FACTORY);
boolean caseInsesnitive = options.contains(Option.CASE_INSENSITIVE_FACTORY);
for (Method method : methods) {
boolean caseInsensitive = options.contains(Option.CASE_INSENSITIVE_FACTORY);
for (Method method : new LinkedHashSet<Method>(methods)) {
// Only consider methods where the name matches
if (!method.getName().equals(factoryMethod) && (!caseInsesnitive || !method.getName().equalsIgnoreCase(method.getName()))) {
if (!method.getName().equals(factoryMethod) && (!caseInsensitive || !method.getName().equalsIgnoreCase(factoryMethod))) {
continue;
}

// if an explicit constructor is specified (via parameter types), look a constructor that matches
if (parameterTypes != null) {
if (specifiedParameterTypes) {
if (method.getParameterTypes().length != parameterTypes.size()) {
if (matchLevel < 1) {
matchLevel = 1;
missException = new MissingFactoryMethodException("Static factory method has " + method.getParameterTypes().length + " arugments " +
missException = new MissingFactoryMethodException("Static factory method has " + method.getParameterTypes().length + " arguments " +
"but expected " + parameterTypes.size() + " arguments: " + method);
}
continue;
Expand Down Expand Up @@ -821,12 +849,12 @@ public int compare(Method method2, Method method1) {

if (missException != null) {
throw missException;
} else {
StringBuffer buffer = new StringBuffer("Unable to find a valid factory method: ");
buffer.append("public void ").append(typeClass.getName()).append(".");
buffer.append(factoryMethod).append(toParameterList(parameterTypes));
throw new MissingFactoryMethodException(buffer.toString());
}

StringBuilder buffer = new StringBuilder("Unable to find a valid factory method: ");
buffer.append("public void ").append(typeClass.getName()).append(".");
buffer.append(factoryMethod).append(toParameterList(parameterTypes));
throw new MissingFactoryMethodException(buffer.toString());
}

public static Method findInstanceFactory(Class typeClass, String factoryMethod, Set<Option> options) {
Expand All @@ -839,12 +867,12 @@ public static Method findInstanceFactory(Class typeClass, String factoryMethod,
MissingFactoryMethodException missException = null;

boolean allowPrivate = options.contains(Option.PRIVATE_FACTORY);
boolean caseInsesnitive = options.contains(Option.CASE_INSENSITIVE_FACTORY);
boolean caseInsensitive = options.contains(Option.CASE_INSENSITIVE_FACTORY);

List<Method> methods = new ArrayList<Method>(Arrays.asList(typeClass.getMethods()));
Set<Method> methods = new HashSet<Method>(Arrays.asList(typeClass.getMethods()));
methods.addAll(Arrays.asList(typeClass.getDeclaredMethods()));
for (Method method : methods) {
if (method.getName().equals(factoryMethod) || (caseInsesnitive && method.getName().equalsIgnoreCase(method.getName()))) {
if (method.getName().equals(factoryMethod) || (caseInsensitive && method.getName().equalsIgnoreCase(factoryMethod))) {
if (Modifier.isStatic(method.getModifiers())) {
if (matchLevel < 1) {
matchLevel = 1;
Expand Down Expand Up @@ -898,12 +926,11 @@ public static Method findInstanceFactory(Class typeClass, String factoryMethod,

if (missException != null) {
throw missException;
} else {
StringBuffer buffer = new StringBuffer("Unable to find a valid factory method: ");
buffer.append("public void ").append(typeClass.getName()).append(".");
buffer.append(factoryMethod).append("()");
throw new MissingFactoryMethodException(buffer.toString());
}
StringBuilder buffer = new StringBuilder("Unable to find a valid factory method: ");
buffer.append("public void ").append(typeClass.getName()).append(".");
buffer.append(factoryMethod).append("()");
throw new MissingFactoryMethodException(buffer.toString());
}

public static List<String> getParameterNames(Constructor<?> constructor) {
Expand Down Expand Up @@ -1028,8 +1055,9 @@ private static void setAccessible(final AccessibleObject accessibleObject) {
accessibleObject.setAccessible(true);
}

// should almost never be called with null as parameter to ensure we have explicit error messages
private static String toParameterList(Class<?>[] parameterTypes) {
return toParameterList(parameterTypes != null ? Arrays.asList(parameterTypes) : null);
return parameterTypes != null ? toParameterList(Arrays.asList(parameterTypes)) : "...";
}

private static String toParameterList(List<? extends Class<?>> parameterTypes) {
Expand Down
31 changes: 31 additions & 0 deletions xbean-reflect/src/test/java/org/apache/xbean/recipe/ObjectRecipeTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,38 @@ public void testWhitespaceInjection() throws Exception {
assertEquals(name, c.name);
assertEquals(ch, c.type);
}

public void testMissedConstructor_TOMEECase() {
final String id = "Default Singleton Container";

{ // failling case
final ObjectRecipe objectRecipe = new ObjectRecipe(SingletonContainer.class);
objectRecipe.setConstructorArgNames(new String[]{"id"});
objectRecipe.setProperty("id_typo", id);
try {
objectRecipe.create();
fail();
} catch (final ConstructionException ce) {
assertEquals("Unable to find a valid constructor: public void org.apache.xbean.recipe.ObjectRecipeTest$SingletonContainer(java.lang.Object)", ce.getMessage());
}
}

{ // passing
final ObjectRecipe objectRecipe = new ObjectRecipe(SingletonContainer.class);
objectRecipe.setConstructorArgNames(new String[]{"id"});
objectRecipe.setProperty("id", id);
assertEquals(id, ((SingletonContainer) objectRecipe.create()).id);
}
}

public static class SingletonContainer {
public final String id;

public SingletonContainer(final String id) {
this.id = id;
}
}

public static class Value {
public String name;
public char type;
Expand Down
Loading