improvements to the method invokers implementation

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

@@ -137,6 +137,9 @@ private boolean requiresCleanup() {
 
         MethodHandle mh = MethodHandleUtils.createMethodHandle(method);
 
+        // single, array-typed parameter at the end for variable arity methods
+        mh = mh.asFixedArity();
+
         // instance transformer
         if (instanceTransformer != null && !isStaticMethod) {
             MethodHandle instanceTransformerMethod = MethodHandleUtils.createMethodHandleFromTransformer(method,

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

@@ -4,6 +4,7 @@
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
 import java.lang.reflect.Constructor;
+import java.lang.reflect.Executable;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.lang.reflect.Type;
@@ -12,9 +13,10 @@
 import java.util.function.Consumer;
 
 import jakarta.enterprise.inject.spi.BeanManager;
-import jakarta.enterprise.inject.spi.DeploymentException;
 import jakarta.enterprise.invoke.Invoker;
 
+import org.jboss.weld.exceptions.DeploymentException;
+
 class MethodHandleUtils {
     private MethodHandleUtils() {
     }
@@ -46,28 +48,36 @@ private MethodHandleUtils() {
         }
     }
 
-    static MethodHandle createMethodHandle(Method method) {
-        MethodHandles.Lookup lookup = Modifier.isPublic(method.getModifiers())
-                && Modifier.isPublic(method.getDeclaringClass().getModifiers())
-                        ? MethodHandles.publicLookup()
-                        : MethodHandles.lookup();
+    private static MethodHandles.Lookup lookupFor(Executable method) throws IllegalAccessException {
+        if (Modifier.isPublic(method.getModifiers()) && Modifier.isPublic(method.getDeclaringClass().getModifiers())) {
+            return MethodHandles.publicLookup();
+        }
+
+        // to create a method handle for a `protected`, package-private or `private` method,
+        // we need a private lookup in the declaring class
+        Module thisModule = MethodHandleUtils.class.getModule();
+        Class<?> targetClass = method.getDeclaringClass();
+        Module targetModule = targetClass.getModule();
+        if (!thisModule.canRead(targetModule)) {
+            // we need to read the other module in order to have privateLookup access
+            // see javadoc for MethodHandles.privateLookupIn()
+            thisModule.addReads(targetModule);
+        }
+        return MethodHandles.privateLookupIn(targetClass, MethodHandles.lookup());
+    }
 
+    static MethodHandle createMethodHandle(Method method) {
         try {
-            return lookup.unreflect(method);
+            return lookupFor(method).unreflect(method);
         } catch (ReflectiveOperationException e) {
             // TODO proper exception handling
             throw new RuntimeException(e);
         }
     }
 
     static MethodHandle createMethodHandle(Constructor<?> constructor) {
-        MethodHandles.Lookup lookup = Modifier.isPublic(constructor.getModifiers())
-                && Modifier.isPublic(constructor.getDeclaringClass().getModifiers())
-                        ? MethodHandles.publicLookup()
-                        : MethodHandles.lookup();
-
         try {
-            return lookup.unreflectConstructor(constructor);
+            return lookupFor(constructor).unreflectConstructor(constructor);
         } catch (ReflectiveOperationException e) {
             // TODO proper exception handling
             throw new RuntimeException(e);

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

@@ -1,5 +1,6 @@
 package org.jboss.weld.lite.extension.translator;
 
+import java.lang.reflect.Modifier;
 import java.util.Collection;
 import java.util.stream.Collectors;
 
@@ -12,6 +13,7 @@
 import jakarta.enterprise.inject.build.compatible.spi.StereotypeInfo;
 import jakarta.enterprise.inject.spi.AnnotatedMethod;
 import jakarta.enterprise.inject.spi.BeanManager;
+import jakarta.enterprise.inject.spi.Decorator;
 import jakarta.enterprise.inject.spi.InjectionPoint;
 import jakarta.enterprise.invoke.InvokerBuilder;
 import jakarta.enterprise.lang.model.AnnotationInfo;
@@ -20,6 +22,7 @@
 import jakarta.enterprise.lang.model.declarations.MethodInfo;
 import jakarta.enterprise.lang.model.types.Type;
 
+import org.jboss.weld.exceptions.DeploymentException;
 import org.jboss.weld.invokable.InvokerInfoBuilder;
 import org.jboss.weld.lite.extension.translator.util.reflection.AnnotatedTypes;
 
@@ -151,18 +154,40 @@ public Collection<InjectionPointInfo> injectionPoints() {
 
     @Override
     public InvokerBuilder<InvokerInfo> createInvoker(MethodInfo methodInfo) {
+        if (!isClassBean()) {
+            throw new DeploymentException("Cannot build invoker for a bean which is not a managed bean: " + this);
+        }
+        if (isInterceptor()) {
+            throw new DeploymentException("Cannot build invoker for an interceptor: " + this);
+        }
+        if (cdiBean instanceof Decorator) { // not representable in BCE, but can theoretically happen
+            throw new DeploymentException("Cannot build invoker for a decorator: " + this);
+        }
+
         if (methodInfo.isConstructor()) {
-            // TODO better exception
-            throw new IllegalArgumentException(
-                    "Constructor methods are not valid candidates for Invokers. MethodInfo: " + methodInfo);
+            throw new DeploymentException("Cannot build invoker for a constructor: " + methodInfo);
+        }
+        if (Modifier.isPrivate(methodInfo.modifiers())) {
+            throw new DeploymentException("Cannot build invoker for a private method: " + methodInfo);
         }
+        if ("java.lang.Object".equals(methodInfo.declaringClass().name())
+                && !"toString".equals(methodInfo.name())) {
+            throw new DeploymentException("Cannot build invoker for a method declared on java.lang.Object: " + methodInfo);
+        }
+
         if (methodInfo instanceof MethodInfoImpl) {
             // at this point we can be sure it is a Method, not a Constructor, so we cast it
             AnnotatedMethod<?> cdiMethod = (AnnotatedMethod<?>) ((MethodInfoImpl) methodInfo).cdiDeclaration;
+
+            // verify that the `methodInfo` belongs to this bean
+            if (!ReflectionMembers.allMethods(cdiBean.getBeanClass()).contains(cdiMethod.getJavaMember())) {
+                throw new DeploymentException("Method does not belong to bean " + cdiBean.getBeanClass().getName()
+                        + ": " + methodInfo);
+            }
+
             return new InvokerInfoBuilder<>(cdiBean.getBeanClass(), cdiMethod.getJavaMember(), bm);
         } else {
-            // TODO better exception
-            throw new IllegalArgumentException("Custom implementations of MethodInfo are not supported!");
+            throw new DeploymentException("Custom implementations of MethodInfo are not supported!");
         }
     }
 

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

@@ -12,6 +12,8 @@
 import jakarta.annotation.Priority;
 import jakarta.enterprise.inject.build.compatible.spi.BuildCompatibleExtension;
 import jakarta.enterprise.inject.build.compatible.spi.SkipIfPortableExtensionPresent;
+import jakarta.enterprise.inject.spi.DefinitionException;
+import jakarta.enterprise.inject.spi.DeploymentException;
 import jakarta.interceptor.Interceptor;
 
 import org.jboss.weld.lite.extension.translator.logging.LiteExtensionTranslatorLogger;
@@ -116,7 +118,17 @@ void callExtensionMethod(java.lang.reflect.Method method, List<Object> arguments
         Class<?> extensionClass = extensionClasses.get(method.getDeclaringClass().getName());
         Object extensionClassInstance = extensionClassInstances.get(extensionClass);
 
-        method.invoke(extensionClassInstance, arguments.toArray());
+        try {
+            method.invoke(extensionClassInstance, arguments.toArray());
+        } catch (InvocationTargetException e) {
+            if (e.getCause() instanceof DefinitionException) {
+                throw (DefinitionException) e.getCause();
+            } else if (e.getCause() instanceof DeploymentException) {
+                throw (DeploymentException) e.getCause();
+            } else {
+                throw e;
+            }
+        }
     }
 
     void clear() {

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

@@ -61,8 +61,13 @@ void runExtensionMethod(java.lang.reflect.Method method) {
                             disposer = ((jakarta.enterprise.inject.spi.ProcessProducerMethod<?, ?>) pb)
                                     .getAnnotatedDisposedParameter();
                         }
-
-                        argument = new BeanInfoImpl(pb.getBean(), pb.getAnnotated(), disposer, beanManager);
+                        if (pb.getBean() instanceof jakarta.enterprise.inject.spi.Interceptor) {
+                            jakarta.enterprise.inject.spi.Interceptor<?> cdiInterceptor = (jakarta.enterprise.inject.spi.Interceptor<?>) pb
+                                    .getBean();
+                            argument = new InterceptorInfoImpl(cdiInterceptor, pb.getAnnotated(), beanManager);
+                        } else {
+                            argument = new BeanInfoImpl(pb.getBean(), pb.getAnnotated(), disposer, beanManager);
+                        }
                     } else {
                         argument = argumentForExtensionMethod(parameter, method);
                     }

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

@@ -239,6 +239,8 @@ public void validation(@Priority(Integer.MAX_VALUE) @Observes jakarta.enterprise
             enhancementActions.clear();
             registrationActions.clear();
 
+            ReflectionMembers.clearCaches();
+
             this.bm = null;
         }
     }

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

@@ -1,42 +1,57 @@
 package org.jboss.weld.lite.extension.translator;
 
-import java.lang.reflect.Method;
 import java.util.ArrayDeque;
 import java.util.Arrays;
 import java.util.HashSet;
 import java.util.Queue;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
 import java.util.function.Consumer;
 
 class ReflectionMembers {
 
     private ReflectionMembers() {
     }
 
-    static Set<Method> allMethods(Class<?> clazz) {
-        Set<Method> result = new HashSet<>();
-        forEachSuperclass(clazz, it -> result.addAll(Arrays.asList(it.getDeclaredMethods())));
-        return result;
+    static ConcurrentMap<Class<?>, Set<java.lang.reflect.Method>> cachedMethods = new ConcurrentHashMap<>();
+    static ConcurrentMap<Class<?>, Set<java.lang.reflect.Field>> cachedFields = new ConcurrentHashMap<>();
+
+    static Set<java.lang.reflect.Method> allMethods(Class<?> clazz) {
+        return cachedMethods.computeIfAbsent(clazz, ignored -> {
+            Set<java.lang.reflect.Method> result = new HashSet<>();
+            forEachSuperclass(clazz, it -> result.addAll(Arrays.asList(it.getDeclaredMethods())));
+            return result;
+        });
     }
 
     static Set<java.lang.reflect.Field> allFields(Class<?> clazz) {
-        Set<java.lang.reflect.Field> result = new HashSet<>();
-        forEachSuperclass(clazz, it -> result.addAll(Arrays.asList(it.getDeclaredFields())));
-        return result;
+        return cachedFields.computeIfAbsent(clazz, ignored -> {
+            Set<java.lang.reflect.Field> result = new HashSet<>();
+            forEachSuperclass(clazz, it -> result.addAll(Arrays.asList(it.getDeclaredFields())));
+            return result;
+        });
+    }
+
+    static void clearCaches() {
+        cachedMethods.clear();
+        cachedFields.clear();
     }
 
     private static void forEachSuperclass(Class<?> clazz, Consumer<Class<?>> action) {
+        // an interface may be inherited multiple times, but we only want to process it once
+        Set<Class<?>> alreadySeen = new HashSet<>();
         Queue<Class<?>> workQueue = new ArrayDeque<>();
         workQueue.add(clazz);
         while (!workQueue.isEmpty()) {
             Class<?> item = workQueue.remove();
-
-            if (item.equals(Object.class)) {
+            if (alreadySeen.contains(item)) {
                 continue;
             }
+            alreadySeen.add(item);
 
             Class<?> superclass = item.getSuperclass();
-            if (superclass != null) {
+            if (superclass != null && !Object.class.equals(superclass)) {
                 workQueue.add(superclass);
             }
             workQueue.addAll(Arrays.asList(item.getInterfaces()));

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

@@ -5,9 +5,12 @@
 import java.util.Map;
 
 import jakarta.enterprise.inject.build.compatible.spi.InvokerInfo;
+import jakarta.enterprise.invoke.Invoker;
 import jakarta.enterprise.lang.model.AnnotationInfo;
 import jakarta.enterprise.lang.model.declarations.ClassInfo;
 
+import org.jboss.weld.invokable.InvokerImpl;
+
 public class SyntheticComponentBuilderBase<THIS extends SyntheticComponentBuilderBase<THIS>> {
     final Map<String, Object> params = new HashMap<>();
 
@@ -130,7 +133,11 @@ public THIS withParam(String key, InvokerInfo value) {
     }
 
     public THIS withParam(String key, InvokerInfo[] value) {
-        this.params.put(key, value);
+        Invoker<?, ?>[] array = new Invoker<?, ?>[value.length];
+        for (int i = 0; i < value.length; i++) {
+            array[i] = (InvokerImpl<?, ?>) value[i];
+        }
+        this.params.put(key, array);
         return self();
     }
 }