Skip to content

Commit 05d8953

Browse files
jknackjknack
committed
value api: make reflective bean converter a converter and use method handle for invocation
- ReflectiveBeanConverter should take MethodHandles.Lookup fix #3498
1 parent fc7516c commit 05d8953

File tree

5 files changed

+85
-63
lines changed

5 files changed

+85
-63
lines changed

‎jooby/src/main/java/io/jooby/internal/converter/ReflectiveBeanConverter.java‎

Lines changed: 50 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,8 @@
77

88
import static io.jooby.SneakyThrows.propagate;
99

10-
import java.lang.annotation.Annotation;
11-
import java.lang.reflect.AnnotatedElement;
12-
import java.lang.reflect.Constructor;
13-
import java.lang.reflect.Executable;
14-
import java.lang.reflect.InvocationTargetException;
15-
import java.lang.reflect.Method;
16-
import java.lang.reflect.Modifier;
17-
import java.lang.reflect.Parameter;
10+
import java.lang.invoke.MethodHandles;
11+
import java.lang.reflect.*;
1812
import java.util.ArrayList;
1913
import java.util.Collection;
2014
import java.util.HashSet;
@@ -33,56 +27,64 @@
3327
import io.jooby.exception.BadRequestException;
3428
import io.jooby.exception.ProvisioningException;
3529
import io.jooby.internal.reflect.$Types;
30+
import io.jooby.value.ConversionHint;
31+
import io.jooby.value.Converter;
3632
import io.jooby.value.Value;
3733
import jakarta.inject.Inject;
3834
import jakarta.inject.Named;
3935

40-
public class ReflectiveBeanConverter {
36+
public class ReflectiveBeanConverter implements Converter {
4137

4238
private record Setter(Method method, Object arg) {
43-
public void invoke(Object instance) throws InvocationTargetException, IllegalAccessException {
44-
method.invoke(instance, arg);
39+
public void invoke(MethodHandles.Lookup lookup, Object instance) throws Throwable {
40+
var handle = lookup.unreflect(method);
41+
handle.invoke(instance, arg);
4542
}
4643
}
4744

4845
private static final String AMBIGUOUS_CONSTRUCTOR =
4946
"Ambiguous constructor found. Expecting a single constructor or only one annotated with "
5047
+ Inject.class.getName();
5148

52-
private static final Object[] NO_ARGS = new Object[0];
49+
private MethodHandles.Lookup lookup;
5350

54-
public Object convert(@NonNull Value node, @NonNull Class type, boolean allowEmptyBean) {
51+
public ReflectiveBeanConverter(MethodHandles.Lookup lookup) {
52+
this.lookup = lookup;
53+
}
54+
55+
@Override
56+
public Object convert(@NonNull Type type, @NonNull Value value, @NonNull ConversionHint hint) {
57+
return convert(value, $Types.parameterizedType0(type), hint == ConversionHint.Empty);
58+
}
59+
60+
private Object convert(@NonNull Value node, @NonNull Class type, boolean allowEmptyBean) {
5561
try {
5662
return newInstance(type, node, allowEmptyBean);
57-
} catch (InstantiationException | IllegalAccessException | NoSuchMethodException x) {
58-
throw propagate(x);
5963
} catch (InvocationTargetException x) {
6064
throw propagate(x.getCause());
65+
} catch (Throwable x) {
66+
throw propagate(x);
6167
}
6268
}
6369

64-
private static Object newInstance(Class type, Value node, boolean allowEmptyBean)
65-
throws IllegalAccessException,
66-
InstantiationException,
67-
InvocationTargetException,
68-
NoSuchMethodException {
69-
Constructor[] constructors = type.getConstructors();
70+
private Object newInstance(Class type, Value node, boolean allowEmptyBean) throws Throwable {
71+
var constructors = type.getConstructors();
7072
Set<Value> state = new HashSet<>();
71-
Constructor constructor;
73+
Constructor<?> constructor;
7274
if (constructors.length == 0) {
7375
constructor = type.getDeclaredConstructor();
7476
} else {
7577
constructor = selectConstructor(constructors);
7678
}
77-
Object[] args =
78-
constructor.getParameterCount() == 0 ? NO_ARGS : inject(node, constructor, state::add);
79-
List<Setter> setters = setters(type, node, state);
79+
var args = inject(node, constructor, state::add);
80+
var setters = setters(type, node, state);
8081
if (!allowEmptyBean(type, allowEmptyBean) && state.stream().allMatch(Value::isMissing)) {
8182
return null;
8283
}
83-
var instance = constructor.newInstance(args);
84-
for (Setter setter : setters) {
85-
setter.invoke(instance);
84+
var handle = lookup.unreflectConstructor(constructor);
85+
var instance = handle.invokeWithArguments(args);
86+
for (var setter : setters) {
87+
setter.invoke(lookup, instance);
8688
}
8789
return instance;
8890
}
@@ -97,9 +99,9 @@ private static Constructor selectConstructor(Constructor[] constructors) {
9799
} else {
98100
Constructor injectConstructor = null;
99101
Constructor defaultConstructor = null;
100-
for (Constructor constructor : constructors) {
102+
for (var constructor : constructors) {
101103
if (Modifier.isPublic(constructor.getModifiers())) {
102-
Annotation inject = constructor.getAnnotation(Inject.class);
104+
var inject = constructor.getAnnotation(Inject.class);
103105
if (inject == null) {
104106
if (constructor.getParameterCount() == 0) {
105107
defaultConstructor = constructor;
@@ -113,31 +115,31 @@ private static Constructor selectConstructor(Constructor[] constructors) {
113115
}
114116
}
115117
}
116-
Constructor result = injectConstructor == null ? defaultConstructor : injectConstructor;
118+
var result = injectConstructor == null ? defaultConstructor : injectConstructor;
117119
if (result == null) {
118120
throw new IllegalStateException(AMBIGUOUS_CONSTRUCTOR);
119121
}
120122
return result;
121123
}
122124
}
123125

124-
public static Object[] inject(Value scope, Executable method, Consumer<Value> state) {
125-
Parameter[] parameters = method.getParameters();
126+
public static List<Object> inject(Value scope, Executable method, Consumer<Value> state) {
127+
var parameters = method.getParameters();
126128
if (parameters.length == 0) {
127-
return NO_ARGS;
129+
return List.of();
128130
}
129-
Object[] args = new Object[parameters.length];
131+
var args = new ArrayList<>(parameters.length);
130132
for (int i = 0; i < parameters.length; i++) {
131-
Parameter parameter = parameters[i];
132-
String name = paramName(parameter);
133-
Value param = scope.get(name);
133+
var parameter = parameters[i];
134+
var name = paramName(parameter);
135+
var param = scope.get(name);
134136
var arg = value(parameter, scope, param);
135137
if (arg == null) {
136138
state.accept(Value.missing(name));
137139
} else {
138140
state.accept(param);
139141
}
140-
args[i] = arg;
142+
args.add(arg);
141143
}
142144
return args;
143145
}
@@ -176,13 +178,13 @@ private static List<Setter> setters(Class type, Value node, Set<Value> nodes) {
176178
var methods = type.getMethods();
177179
var result = new ArrayList<Setter>();
178180
for (String name : names(node)) {
179-
Value value = node.get(name);
181+
var value = node.get(name);
180182
if (nodes.add(value)) {
181-
Method method = findSetter(methods, name);
183+
var method = findSetter(methods, name);
182184
if (method != null) {
183-
Parameter parameter = method.getParameters()[0];
185+
var parameter = method.getParameters()[0];
184186
try {
185-
Object arg = value(parameter, node, value);
187+
var arg = value(parameter, node, value);
186188
result.add(new Setter(method, arg));
187189
} catch (ProvisioningException x) {
188190
throw x;
@@ -200,9 +202,9 @@ private static List<Setter> setters(Class type, Value node, Set<Value> nodes) {
200202
private static Object value(Parameter parameter, Value node, Value value) {
201203
try {
202204
if (isFileUpload(node, parameter)) {
203-
Formdata formdata = (Formdata) node;
205+
var formdata = (Formdata) node;
204206
if (Set.class.isAssignableFrom(parameter.getType())) {
205-
return new HashSet<>(formdata.files(value.name()));
207+
return new LinkedHashSet<>(formdata.files(value.name()));
206208
} else if (Collection.class.isAssignableFrom(parameter.getType())) {
207209
return formdata.files(value.name());
208210
} else if (Optional.class.isAssignableFrom(parameter.getType())) {
@@ -222,7 +224,7 @@ private static Object value(Parameter parameter, Value node, Value value) {
222224
if (isNullable(parameter)) {
223225
if (value.isSingle()) {
224226
var str = value.valueOrNull();
225-
if (str == null || str.length() == 0) {
227+
if (str == null || str.isEmpty()) {
226228
// treat empty values as null
227229
return null;
228230
}
@@ -252,7 +254,7 @@ private static boolean isNullable(Parameter parameter) {
252254

253255
private static boolean hasAnnotation(AnnotatedElement element, String... names) {
254256
var nameList = List.of(names);
255-
for (Annotation annotation : element.getAnnotations()) {
257+
for (var annotation : element.getAnnotations()) {
256258
if (nameList.stream()
257259
.anyMatch(name -> annotation.annotationType().getSimpleName().endsWith(name))) {
258260
return true;
@@ -273,7 +275,7 @@ private static boolean isFileUpload(Class type) {
273275
private static Method findSetter(Method[] methods, String name) {
274276
var setter = "set" + Character.toUpperCase(name.charAt(0)) + name.substring(1);
275277
var candidates = new LinkedList<Method>();
276-
for (Method method : methods) {
278+
for (var method : methods) {
277279
if ((method.getName().equals(name) || method.getName().equals(setter))
278280
&& method.getParameterCount() == 1) {
279281
if (method.getName().startsWith("set")) {

‎jooby/src/main/java/io/jooby/value/ValueFactory.java‎

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import java.util.*;
1313

1414
import edu.umd.cs.findbugs.annotations.NonNull;
15+
import edu.umd.cs.findbugs.annotations.Nullable;
1516
import io.jooby.SneakyThrows;
1617
import io.jooby.exception.TypeMismatchException;
1718
import io.jooby.internal.converter.ReflectiveBeanConverter;
@@ -28,28 +29,44 @@ public enum ConversionType {
2829

2930
private final Map<Type, Converter> converterMap = new HashMap<>();
3031

31-
private MethodHandles.Lookup lookup = MethodHandles.publicLookup();
32+
private ConversionHint defaultHint = ConversionHint.Strict;
3233

33-
public ValueFactory() {
34+
private MethodHandles.Lookup lookup;
35+
36+
private Converter fallback;
37+
38+
public ValueFactory(@NonNull MethodHandles.Lookup lookup) {
39+
this.lookup = lookup;
40+
this.fallback = new ReflectiveBeanConverter(lookup);
3441
StandardConverter.register(this);
3542
}
3643

37-
public Converter get(Type type) {
44+
public ValueFactory() {
45+
this(MethodHandles.publicLookup());
46+
}
47+
48+
public @NonNull ValueFactory lookup(@NonNull MethodHandles.Lookup lookup) {
49+
this.lookup = lookup;
50+
this.fallback = new ReflectiveBeanConverter(lookup);
51+
return this;
52+
}
53+
54+
public @NonNull ValueFactory hint(@NonNull ConversionHint defaultHint) {
55+
this.defaultHint = defaultHint;
56+
return this;
57+
}
58+
59+
public @Nullable Converter get(Type type) {
3860
return converterMap.get(type);
3961
}
4062

41-
public ValueFactory put(Type type, Converter converter) {
63+
public @NonNull ValueFactory put(@NonNull Type type, @NonNull Converter converter) {
4264
converterMap.put(type, converter);
4365
return this;
4466
}
4567

4668
public <T> T convert(@NonNull Type type, @NonNull Value value) {
47-
T result = convert(type, value, ConversionHint.Strict);
48-
if (result == null) {
49-
throw new TypeMismatchException(value.name(), type);
50-
}
51-
52-
return result;
69+
return convert(type, value, defaultHint);
5370
}
5471

5572
@SuppressWarnings("unchecked")
@@ -84,8 +101,11 @@ public <T> T convert(@NonNull Type type, @NonNull Value value, @NonNull Conversi
84101
}
85102
}
86103
// anything else fallback to reflective
87-
var reflective = new ReflectiveBeanConverter();
88-
return (T) reflective.convert(value, rawType, hint == ConversionHint.Empty);
104+
var result = (T) fallback.convert(type, value, hint);
105+
if (result == null && hint == ConversionHint.Strict) {
106+
throw new TypeMismatchException(value.name(), type);
107+
}
108+
return result;
89109
}
90110
}
91111

‎jooby/src/test/java/io/jooby/Issue2525.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public class Issue2525 {
2626

2727
public class VC2525 implements Converter {
2828
@Override
29-
public Object convert(@NotNull Type type, @NotNull Value value, ConversionHint hint) {
29+
public Object convert(@NotNull Type type, @NotNull Value value, @NotNull ConversionHint hint) {
3030
return new MyID2525(value.value());
3131
}
3232
}

‎modules/jooby-apt/src/test/java/tests/i2325/VC2325.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515

1616
public class VC2325 implements Converter {
1717
@Override
18-
public Object convert(@NotNull Type type, @NotNull Value value, ConversionHint hint) {
18+
public Object convert(@NotNull Type type, @NotNull Value value, @NotNull ConversionHint hint) {
1919
return new MyID2325(value.value());
2020
}
2121
}

‎tests/src/test/java/io/jooby/i2325/VC2325.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616

1717
public class VC2325 implements Converter {
1818
@Override
19-
public Object convert(@NotNull Type type, @NotNull Value value, ConversionHint hint) {
19+
public Object convert(@NotNull Type type, @NotNull Value value, @NotNull ConversionHint hint) {
2020
var v = value instanceof QueryString query ? query.get("value").value() : value.value();
2121
return new MyID2325(v);
2222
}

0 commit comments

Comments
 (0)