Skip to content

Commit 14377b8

Browse files
LadicekLadicek
committed
improvements to the method invokers implementation
Includes one tiny improvement to the Language Model implementation that should be clarified in the next version of the specification.
1 parent 8c1438f commit 14377b8

File tree

6 files changed

+83
-23
lines changed

6 files changed

+83
-23
lines changed

‎impl/src/main/java/org/jboss/weld/invokable/AbstractInvokerBuilder.java

+3
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,9 @@ private boolean requiresCleanup() {
137137

138138
MethodHandle mh = MethodHandleUtils.createMethodHandle(method);
139139

140+
// single, array-typed parameter at the end for variable arity methods
141+
mh = mh.asFixedArity();
142+
140143
// instance transformer
141144
if (instanceTransformer != null && !isStaticMethod) {
142145
MethodHandle instanceTransformerMethod = MethodHandleUtils.createMethodHandleFromTransformer(method,

‎impl/src/main/java/org/jboss/weld/invokable/MethodHandleUtils.java

+23-13
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import java.lang.invoke.MethodHandle;
55
import java.lang.invoke.MethodHandles;
66
import java.lang.reflect.Constructor;
7+
import java.lang.reflect.Executable;
78
import java.lang.reflect.Method;
89
import java.lang.reflect.Modifier;
910
import java.lang.reflect.Type;
@@ -12,9 +13,10 @@
1213
import java.util.function.Consumer;
1314

1415
import jakarta.enterprise.inject.spi.BeanManager;
15-
import jakarta.enterprise.inject.spi.DeploymentException;
1616
import jakarta.enterprise.invoke.Invoker;
1717

18+
import org.jboss.weld.exceptions.DeploymentException;
19+
1820
class MethodHandleUtils {
1921
private MethodHandleUtils() {
2022
}
@@ -46,28 +48,36 @@ private MethodHandleUtils() {
4648
}
4749
}
4850

49-
static MethodHandle createMethodHandle(Method method) {
50-
MethodHandles.Lookup lookup = Modifier.isPublic(method.getModifiers())
51-
&& Modifier.isPublic(method.getDeclaringClass().getModifiers())
52-
? MethodHandles.publicLookup()
53-
: MethodHandles.lookup();
51+
private static MethodHandles.Lookup lookupFor(Executable method) throws IllegalAccessException {
52+
if (Modifier.isPublic(method.getModifiers()) && Modifier.isPublic(method.getDeclaringClass().getModifiers())) {
53+
return MethodHandles.publicLookup();
54+
}
55+
56+
// to create a method handle for a `protected`, package-private or `private` method,
57+
// we need a private lookup in the declaring class
58+
Module thisModule = MethodHandleUtils.class.getModule();
59+
Class<?> targetClass = method.getDeclaringClass();
60+
Module targetModule = targetClass.getModule();
61+
if (!thisModule.canRead(targetModule)) {
62+
// we need to read the other module in order to have privateLookup access
63+
// see javadoc for MethodHandles.privateLookupIn()
64+
thisModule.addReads(targetModule);
65+
}
66+
return MethodHandles.privateLookupIn(targetClass, MethodHandles.lookup());
67+
}
5468

69+
static MethodHandle createMethodHandle(Method method) {
5570
try {
56-
return lookup.unreflect(method);
71+
return lookupFor(method).unreflect(method);
5772
} catch (ReflectiveOperationException e) {
5873
// TODO proper exception handling
5974
throw new RuntimeException(e);
6075
}
6176
}
6277

6378
static MethodHandle createMethodHandle(Constructor<?> constructor) {
64-
MethodHandles.Lookup lookup = Modifier.isPublic(constructor.getModifiers())
65-
&& Modifier.isPublic(constructor.getDeclaringClass().getModifiers())
66-
? MethodHandles.publicLookup()
67-
: MethodHandles.lookup();
68-
6979
try {
70-
return lookup.unreflectConstructor(constructor);
80+
return lookupFor(constructor).unreflectConstructor(constructor);
7181
} catch (ReflectiveOperationException e) {
7282
// TODO proper exception handling
7383
throw new RuntimeException(e);

‎weld-lite-extension-translator/src/main/java/org/jboss/weld/lite/extension/translator/BeanInfoImpl.java

+31-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package org.jboss.weld.lite.extension.translator;
22

3+
import java.lang.reflect.Modifier;
34
import java.util.Collection;
45
import java.util.stream.Collectors;
56

@@ -12,6 +13,7 @@
1213
import jakarta.enterprise.inject.build.compatible.spi.StereotypeInfo;
1314
import jakarta.enterprise.inject.spi.AnnotatedMethod;
1415
import jakarta.enterprise.inject.spi.BeanManager;
16+
import jakarta.enterprise.inject.spi.Decorator;
1517
import jakarta.enterprise.inject.spi.InjectionPoint;
1618
import jakarta.enterprise.invoke.InvokerBuilder;
1719
import jakarta.enterprise.lang.model.AnnotationInfo;
@@ -20,6 +22,7 @@
2022
import jakarta.enterprise.lang.model.declarations.MethodInfo;
2123
import jakarta.enterprise.lang.model.types.Type;
2224

25+
import org.jboss.weld.exceptions.DeploymentException;
2326
import org.jboss.weld.invokable.InvokerInfoBuilder;
2427
import org.jboss.weld.lite.extension.translator.util.reflection.AnnotatedTypes;
2528

@@ -151,18 +154,41 @@ public Collection<InjectionPointInfo> injectionPoints() {
151154

152155
@Override
153156
public InvokerBuilder<InvokerInfo> createInvoker(MethodInfo methodInfo) {
157+
if (!isClassBean()) {
158+
throw new DeploymentException("Cannot build invoker for a bean which is not a managed bean: " + this);
159+
}
160+
if (isInterceptor()) {
161+
throw new DeploymentException("Cannot build invoker for an interceptor: " + this);
162+
}
163+
if (cdiBean instanceof Decorator) { // not representable in BCE, but can theoretically happen
164+
throw new DeploymentException("Cannot build invoker for a decorator: " + this);
165+
}
166+
154167
if (methodInfo.isConstructor()) {
155-
// TODO better exception
156-
throw new IllegalArgumentException(
157-
"Constructor methods are not valid candidates for Invokers. MethodInfo: " + methodInfo);
168+
throw new DeploymentException("Cannot build invoker for a constructor: " + methodInfo);
169+
}
170+
if (Modifier.isPrivate(methodInfo.modifiers())) {
171+
throw new DeploymentException("Cannot build invoker for a private method: " + methodInfo);
158172
}
173+
if ("java.lang.Object".equals(methodInfo.declaringClass().name())
174+
&& !"toString".equals(methodInfo.name())) {
175+
throw new DeploymentException("Cannot build invoker for a method declared on java.lang.Object: " + methodInfo);
176+
}
177+
159178
if (methodInfo instanceof MethodInfoImpl) {
160179
// at this point we can be sure it is a Method, not a Constructor, so we cast it
161180
AnnotatedMethod<?> cdiMethod = (AnnotatedMethod<?>) ((MethodInfoImpl) methodInfo).cdiDeclaration;
181+
182+
// verify that the `methodInfo` belongs to this bean
183+
// TODO inefficient when many invokers are created for methods of a single bean
184+
if (!ReflectionMembers.allMethods(cdiBean.getBeanClass()).contains(cdiMethod.getJavaMember())) {
185+
throw new DeploymentException("Method does not belong to bean " + cdiBean.getBeanClass().getName()
186+
+ ": " + methodInfo);
187+
}
188+
162189
return new InvokerInfoBuilder<>(cdiBean.getBeanClass(), cdiMethod.getJavaMember(), bm);
163190
} else {
164-
// TODO better exception
165-
throw new IllegalArgumentException("Custom implementations of MethodInfo are not supported!");
191+
throw new DeploymentException("Custom implementations of MethodInfo are not supported!");
166192
}
167193
}
168194

‎weld-lite-extension-translator/src/main/java/org/jboss/weld/lite/extension/translator/ExtensionInvoker.java

+13-1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
import jakarta.annotation.Priority;
1313
import jakarta.enterprise.inject.build.compatible.spi.BuildCompatibleExtension;
1414
import jakarta.enterprise.inject.build.compatible.spi.SkipIfPortableExtensionPresent;
15+
import jakarta.enterprise.inject.spi.DefinitionException;
16+
import jakarta.enterprise.inject.spi.DeploymentException;
1517
import jakarta.interceptor.Interceptor;
1618

1719
import org.jboss.weld.lite.extension.translator.logging.LiteExtensionTranslatorLogger;
@@ -116,7 +118,17 @@ void callExtensionMethod(java.lang.reflect.Method method, List<Object> arguments
116118
Class<?> extensionClass = extensionClasses.get(method.getDeclaringClass().getName());
117119
Object extensionClassInstance = extensionClassInstances.get(extensionClass);
118120

119-
method.invoke(extensionClassInstance, arguments.toArray());
121+
try {
122+
method.invoke(extensionClassInstance, arguments.toArray());
123+
} catch (InvocationTargetException e) {
124+
if (e.getCause() instanceof DefinitionException) {
125+
throw (DefinitionException) e.getCause();
126+
} else if (e.getCause() instanceof DeploymentException) {
127+
throw (DeploymentException) e.getCause();
128+
} else {
129+
throw e;
130+
}
131+
}
120132
}
121133

122134
void clear() {

‎weld-lite-extension-translator/src/main/java/org/jboss/weld/lite/extension/translator/ReflectionMembers.java

+5-3
Original file line numberDiff line numberDiff line change
@@ -26,17 +26,19 @@ static Set<java.lang.reflect.Field> allFields(Class<?> clazz) {
2626
}
2727

2828
private static void forEachSuperclass(Class<?> clazz, Consumer<Class<?>> action) {
29+
// an interface may be inherited multiple times, but we only want to process it once
30+
Set<Class<?>> alreadySeen = new HashSet<>();
2931
Queue<Class<?>> workQueue = new ArrayDeque<>();
3032
workQueue.add(clazz);
3133
while (!workQueue.isEmpty()) {
3234
Class<?> item = workQueue.remove();
33-
34-
if (item.equals(Object.class)) {
35+
if (alreadySeen.contains(item)) {
3536
continue;
3637
}
38+
alreadySeen.add(item);
3739

3840
Class<?> superclass = item.getSuperclass();
39-
if (superclass != null) {
41+
if (superclass != null && !Object.class.equals(superclass)) {
4042
workQueue.add(superclass);
4143
}
4244
workQueue.addAll(Arrays.asList(item.getInterfaces()));

‎weld-lite-extension-translator/src/main/java/org/jboss/weld/lite/extension/translator/SyntheticComponentBuilderBase.java

+8-1
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,12 @@
55
import java.util.Map;
66

77
import jakarta.enterprise.inject.build.compatible.spi.InvokerInfo;
8+
import jakarta.enterprise.invoke.Invoker;
89
import jakarta.enterprise.lang.model.AnnotationInfo;
910
import jakarta.enterprise.lang.model.declarations.ClassInfo;
1011

12+
import org.jboss.weld.invokable.InvokerImpl;
13+
1114
public class SyntheticComponentBuilderBase<THIS extends SyntheticComponentBuilderBase<THIS>> {
1215
final Map<String, Object> params = new HashMap<>();
1316

@@ -130,7 +133,11 @@ public THIS withParam(String key, InvokerInfo value) {
130133
}
131134

132135
public THIS withParam(String key, InvokerInfo[] value) {
133-
this.params.put(key, value);
136+
Invoker<?, ?>[] array = new Invoker<?, ?>[value.length];
137+
for (int i = 0; i < value.length; i++) {
138+
array[i] = (InvokerImpl<?, ?>) value[i];
139+
}
140+
this.params.put(key, array);
134141
return self();
135142
}
136143
}

0 commit comments

Comments
 (0)