Issue #10356 Update Weld integration (#10359)

* Issue #10356 Update Weld integration Signed-off-by: Olivier Lamy <[email protected]> Co-authored-by: Olivier Lamy <[email protected]>
jetty · Aug 29, 2023 · 5808a62 · 5808a62
1 parent 1551376
commit 5808a62
Show file tree

Hide file tree

Showing 25 changed files with 206 additions and 123 deletions.
diff --git a/jetty-ee10/jetty-ee10-cdi/src/main/java/org/eclipse/jetty/ee10/cdi/CdiConfiguration.java b/jetty-ee10/jetty-ee10-cdi/src/main/java/org/eclipse/jetty/ee10/cdi/CdiConfiguration.java
@@ -13,14 +13,18 @@
 
 package org.eclipse.jetty.ee10.cdi;
 
+import java.util.function.Predicate;
+
 import org.eclipse.jetty.ee10.annotations.AnnotationConfiguration;
 import org.eclipse.jetty.ee10.plus.webapp.PlusConfiguration;
 import org.eclipse.jetty.ee10.webapp.AbstractConfiguration;
 
 /**
  * <p>CDI Configuration</p>
  * <p>This configuration configures the WebAppContext server/system classes to
- * be able to see the {@link CdiServletContainerInitializer}.
+ * be able to see the {@link CdiServletContainerInitializer}. Also hides the
+ * jakarta cdi classes that are on the environment/server classpath and allows
+ * the webapp to provide their own.
  * </p>
  */
 public class CdiConfiguration extends AbstractConfiguration
@@ -29,7 +33,16 @@ public CdiConfiguration()
     {
         super(new Builder()
             .protectAndExpose("org.eclipse.jetty.ee10.cdi.CdiServletContainerInitializer")
+            .hide(getHiddenClasses())
             .addDependents(AnnotationConfiguration.class, PlusConfiguration.class));
     }
-}
 
+    private static String[] getHiddenClasses()
+    {
+        //Only hide the cdi api classes if there is not also an impl on the
+        //environment classpath - vital for embedded uses.
+        if (CdiConfiguration.class.getClassLoader().getResource("META-INF/services/jakarta.enterprise.inject.spi.CDIProvider") == null)
+            return new String[]{"jakarta.enterprise.", "jakarta.decorator."};
+        return new String[0];
+    }
+}
diff --git a/...y-ee10/jetty-ee10-cdi/src/main/java/org/eclipse/jetty/ee10/cdi/CdiDecoratingListener.java b/...y-ee10/jetty-ee10-cdi/src/main/java/org/eclipse/jetty/ee10/cdi/CdiDecoratingListener.java
@@ -17,12 +17,15 @@
 import org.eclipse.jetty.ee10.servlet.ServletContextHandler;
 
 /**
- * A DecoratingListener that listens for "org.eclipse.jetty.ee10.cdi.decorator"
+ * A DecoratingListener that listens for "org.eclipse.jetty.cdi.decorator"
  */
 public class CdiDecoratingListener extends DecoratingListener
 {
     public static final String MODE = "CdiDecoratingListener";
-    public static final String ATTRIBUTE = "org.eclipse.jetty.ee10.cdi.decorator";
+    /**
+     * Attribute used by Weld to communicate to Jetty that it has created a WeldDecorator
+     */
+    public static final String ATTRIBUTE = "org.eclipse.jetty.cdi.decorator";
 
     public CdiDecoratingListener(ServletContextHandler contextHandler)
     {

diff --git a/...tty-ee10-cdi/src/main/java/org/eclipse/jetty/ee10/cdi/CdiServletContainerInitializer.java b/...tty-ee10-cdi/src/main/java/org/eclipse/jetty/ee10/cdi/CdiServletContainerInitializer.java
@@ -28,21 +28,21 @@
  * <p>A {@link ServletContainerInitializer} that introspects for a CDI API
  * implementation within a web application and applies an integration
  * mode if CDI is found.  CDI integration modes can be selected per webapp with
- * the "org.eclipse.jetty.ee10.cdi" init parameter or default to the mode set by the
- * "org.eclipse.jetty.ee10.cdi" server attribute.  Supported modes are:</p>
+ * the "org.eclipse.jetty.cdi" init parameter or default to the mode set by the
+ * "org.eclipse.jetty.cdi" server attribute.  Supported modes are:</p>
  * <dl>
  * <dt>CdiSpiDecorator</dt>
  *     <dd>Jetty will call the CDI SPI within the webapp to decorate objects (default).</dd>
  * <dt>CdiDecoratingLister</dt>
  *     <dd>The webapp may register a decorator on the context attribute
- *     "org.eclipse.jetty.ee10.cdi.decorator".</dd>
+ *     "org.eclipse.jetty.cdi.decorator".</dd>
  * </dl>
  *
  * @see AnnotationConfiguration.ServletContainerInitializerOrdering
  */
 public class CdiServletContainerInitializer implements ServletContainerInitializer
 {
-    public static final String CDI_INTEGRATION_ATTRIBUTE = "org.eclipse.jetty.ee10.cdi";
+    public static final String CDI_INTEGRATION_ATTRIBUTE = "org.eclipse.jetty.cdi";
     private static final Logger LOG = LoggerFactory.getLogger(CdiServletContainerInitializer.class);
 
     @Override

diff --git a/jetty-ee10/jetty-ee10-cdi/src/main/java/org/eclipse/jetty/ee10/cdi/CdiSpiDecorator.java b/jetty-ee10/jetty-ee10-cdi/src/main/java/org/eclipse/jetty/ee10/cdi/CdiSpiDecorator.java
@@ -14,12 +14,11 @@
 package org.eclipse.jetty.ee10.cdi;
 
 import java.lang.invoke.MethodHandle;
-import java.lang.invoke.MethodHandles;
-import java.lang.invoke.MethodType;
+import java.lang.reflect.Method;
 import java.util.Arrays;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
@@ -32,7 +31,7 @@
  * A Decorator that invokes the CDI provider within a webapp to decorate objects created by
  * the contexts {@link org.eclipse.jetty.util.DecoratedObjectFactory}
  * (typically Listeners, Filters and Servlets).
- * The CDI provider is invoked using {@link MethodHandle}s to avoid any CDI instance
+ * The CDI provider is invoked using reflection to avoid any CDI instance
  * or dependencies within the server scope. The code invoked is equivalent to:
  * <pre>
  * public &lt;T&gt; T decorate(T o)
@@ -47,20 +46,25 @@
 public class CdiSpiDecorator implements Decorator
 {
     private static final Logger LOG = LoggerFactory.getLogger(CdiServletContainerInitializer.class);
+
+    private static final Object NULL_SINGLETON_ARG = null;
+    private static final Object[] NULL_ARRAY_ARG = new Object[]{null};
+
     public static final String MODE = "CdiSpiDecorator";
 
     private final ServletContextHandler _context;
     private final Map<Object, Decorated> _decorated = new HashMap<>();
 
-    private final MethodHandle _current;
-    private final MethodHandle _getBeanManager;
-    private final MethodHandle _createAnnotatedType;
-    private final MethodHandle _createInjectionTarget;
-    private final MethodHandle _createCreationalContext;
-    private final MethodHandle _inject;
-    private final MethodHandle _dispose;
-    private final MethodHandle _release;
-    private final Set<String> _undecorated = new HashSet<>(Collections.singletonList("org.jboss.weld.environment.servlet.Listener"));
+    private final Method _current;
+    private final Method _getBeanManager;
+    private final Method _createAnnotatedType;
+    private final Method _createInjectionTarget;
+    private final Method _getInjectionTargetFactory;
+    private final Method _createCreationalContext;
+    private final Method _inject;
+    private final Method _dispose;
+    private final Method _release;
+    private final Set<String> _undecorated = new HashSet<>(List.of("org.jboss.weld.environment.servlet.Listener", "org.jboss.weld.environment.servlet.EnhancedListener"));
 
     public CdiSpiDecorator(ServletContextHandler context) throws UnsupportedOperationException
     {
@@ -73,21 +77,26 @@ public CdiSpiDecorator(ServletContextHandler context) throws UnsupportedOperatio
         try
         {
             Class<?> cdiClass = classLoader.loadClass("jakarta.enterprise.inject.spi.CDI");
+            Class<?> beanClass = classLoader.loadClass("jakarta.enterprise.inject.spi.Bean");
             Class<?> beanManagerClass = classLoader.loadClass("jakarta.enterprise.inject.spi.BeanManager");
             Class<?> annotatedTypeClass = classLoader.loadClass("jakarta.enterprise.inject.spi.AnnotatedType");
             Class<?> injectionTargetClass = classLoader.loadClass("jakarta.enterprise.inject.spi.InjectionTarget");
+            Class<?> injectionTargetFactoryClass = classLoader.loadClass("jakarta.enterprise.inject.spi.InjectionTargetFactory");
             Class<?> creationalContextClass = classLoader.loadClass("jakarta.enterprise.context.spi.CreationalContext");
             Class<?> contextualClass = classLoader.loadClass("jakarta.enterprise.context.spi.Contextual");
 
-            MethodHandles.Lookup lookup = MethodHandles.lookup();
-            _current = lookup.findStatic(cdiClass, "current", MethodType.methodType(cdiClass));
-            _getBeanManager = lookup.findVirtual(cdiClass, "getBeanManager", MethodType.methodType(beanManagerClass));
-            _createAnnotatedType = lookup.findVirtual(beanManagerClass, "createAnnotatedType", MethodType.methodType(annotatedTypeClass, Class.class));
-            _createInjectionTarget = lookup.findVirtual(beanManagerClass, "createInjectionTarget", MethodType.methodType(injectionTargetClass, annotatedTypeClass));
-            _createCreationalContext = lookup.findVirtual(beanManagerClass, "createCreationalContext", MethodType.methodType(creationalContextClass, contextualClass));
-            _inject = lookup.findVirtual(injectionTargetClass, "inject", MethodType.methodType(Void.TYPE, Object.class, creationalContextClass));
-            _dispose = lookup.findVirtual(injectionTargetClass, "dispose", MethodType.methodType(Void.TYPE, Object.class));
-            _release = lookup.findVirtual(creationalContextClass, "release", MethodType.methodType(Void.TYPE));
+            //Use reflection rather than MethodHandles. Reflection respects the classloader that loaded the class, which means
+            //that as it's a WebAppClassLoader it will do hiding of the cdi spi classes that are on the server classpath. MethodHandles
+            //see both the cdi api classes from the server classpath and the webapp classpath and throws an exception.
+            _current = cdiClass.getMethod("current", null);
+            _getBeanManager = cdiClass.getMethod("getBeanManager", null);
+            _createAnnotatedType = beanManagerClass.getMethod("createAnnotatedType", Class.class);
+            _getInjectionTargetFactory = beanManagerClass.getMethod("getInjectionTargetFactory", annotatedTypeClass);
+            _createInjectionTarget = injectionTargetFactoryClass.getMethod("createInjectionTarget", beanClass);
+            _createCreationalContext = beanManagerClass.getMethod("createCreationalContext", contextualClass);
+            _inject = injectionTargetClass.getMethod("inject", Object.class, creationalContextClass);
+            _dispose = injectionTargetClass.getMethod("dispose", Object.class);
+            _release = creationalContextClass.getMethod("release", null);
         }
         catch (Exception e)
         {
@@ -198,13 +207,15 @@ private class Decorated
         Decorated(Object o) throws Throwable
         {
             // BeanManager manager = CDI.current().getBeanManager();
-            Object manager = _getBeanManager.invoke(_current.invoke());
+            Object manager = _getBeanManager.invoke(_current.invoke(null));
             // AnnotatedType annotatedType = manager.createAnnotatedType((Class<T>)o.getClass());
             Object annotatedType = _createAnnotatedType.invoke(manager, o.getClass());
             // CreationalContext creationalContext = manager.createCreationalContext(null);
-            _creationalContext = _createCreationalContext.invoke(manager, null);
-            // InjectionTarget injectionTarget = manager.createInjectionTarget();
-            _injectionTarget = _createInjectionTarget.invoke(manager, annotatedType);
+            _creationalContext = _createCreationalContext.invoke(manager, NULL_SINGLETON_ARG);
+            //InjectionTargetFactory injectionTargetFactory = manager.getInjectionTargetFactory(AnnotatedType<T)
+            Object injectionTargetFactory = _getInjectionTargetFactory.invoke(manager, annotatedType);
+            // InjectionTarget injectionTarget = injectionTargetFactory.createInjectionTarget();
+            _injectionTarget = _createInjectionTarget.invoke(injectionTargetFactory, NULL_ARRAY_ARG);
             // injectionTarget.inject(o, creationalContext);
             _inject.invoke(_injectionTarget, o, _creationalContext);
         }

diff --git a/...etty-ee10-test-cdi-common-webapp/src/main/java/org/eclipse/jetty/test/ServerIDFilter.java b/...etty-ee10-test-cdi-common-webapp/src/main/java/org/eclipse/jetty/test/ServerIDFilter.java
@@ -41,7 +41,7 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
         if (response instanceof HttpServletResponse)
         {
             String serverID = (String)request.getServletContext().getAttribute("ServerID");
-            ((HttpServletResponse)response).setHeader("Server", serverID);
+            ((HttpServletResponse)response).setHeader("CDI-Server", serverID);
         }
         chain.doFilter(request, response);
     }

diff --git a/.../jetty-ee10-test-cdi/src/test/java/org/eclipse/jetty/ee10/cdi/tests/EmbeddedWeldTest.java b/.../jetty-ee10-test-cdi/src/test/java/org/eclipse/jetty/ee10/cdi/tests/EmbeddedWeldTest.java
@@ -28,6 +28,7 @@
 import org.eclipse.jetty.ee10.webapp.WebAppContext;
 import org.eclipse.jetty.server.LocalConnector;
 import org.eclipse.jetty.server.Server;
+import org.junit.jupiter.api.Assumptions;
 import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
@@ -36,7 +37,6 @@
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsString;
 
-@Disabled //TODO misatch weld version and cdi api?
 public class EmbeddedWeldTest
 {
     static
@@ -107,7 +107,7 @@ public static Server createServerWithServletContext(String mode)
             case "CdiServletContainerInitializer+Listener":
                 // Expect:INFO: WELD-ENV-001213: Jetty CDI SPI support detected, CDI injection will be available in Listeners, Servlets and Filters.
                 context.addServletContainerInitializer(new CdiServletContainerInitializer());
-                context.addEventListener(new org.jboss.weld.environment.servlet.Listener());
+                context.getServletHandler().addListener(new ListenerHolder(org.jboss.weld.environment.servlet.Listener.class));
                 break;
 
             case "CdiServletContainerInitializer(CdiDecoratingListener)+Listener":
@@ -149,6 +149,13 @@ public static Server createServerWithServletContext(String mode)
         })
     public void testServletContext(String mode) throws Exception
     {
+        //Mode "none" is incompatible with jetty-12. This mode delegates to Weld to work out how to
+        //instantiate injection capabilities into jetty, but as of Weld 5.1.1 this relied on pre jetty-12 classes.
+        //Mode "DecoratingListener+Listener is incompatible with jetty-12. The name of the attribute that communicates
+        //the injector decorator between Weld and jetty-12 relies on the classname of the DecoratingListener class, which
+        //in pre jetty-12 was org.eclipse.jetty.webapp.DecoratingListener, but is now org.eclipse.jetty.ee10.webapp.DecoratingListener.
+        Assumptions.assumeFalse(mode.equals("none") || mode.equals("DecoratingListener+Listener"));
+
         Server server = createServerWithServletContext(mode);
         server.start();
         LocalConnector connector = server.getBean(LocalConnector.class);
@@ -169,7 +176,7 @@ public void testServletContext(String mode) throws Exception
     @Test
     public void testServletContextSimone() throws Exception
     {
-        Server server = createServerWithServletContext("none");
+        Server server = createServerWithServletContext("CdiDecoratingListener+Listener");
         server.start();
         LocalConnector connector = server.getBean(LocalConnector.class);
         String response = connector.getResponse("GET / HTTP/1.0\r\n\r\n");
@@ -225,12 +232,12 @@ public void testWebappContextDiscovered() throws Exception
         webapp.setBaseResourceAsPath(Paths.get("src", "test", "weldtest"));
         server.setHandler(webapp);
 
-        // Need the AnnotationConfiguration to detect SCIs
-        webapp.addConfiguration(new AnnotationConfiguration());
-
-        // Need to expose our SCI.  This is ugly could be made better in jetty-10 with a CdiConfiguration
+        // Need to expose our SCI.
         webapp.addConfiguration(new CdiConfiguration());
 
+        //ensure our CDI SCI is run first so the decorator is set up before Weld runs
+        webapp.setAttribute(AnnotationConfiguration.SERVLET_CONTAINER_INITIALIZER_ORDER, "org.eclipse.jetty.ee10.cdi.CdiServletContainerInitializer, *");
+
         // This is ugly but needed for maven for testing in a overlaid war pom
         String pkg = EmbeddedWeldTest.class.getPackage().getName();
         webapp.getServerClassMatcher().add("-" + pkg + ".");
@@ -244,7 +251,6 @@ public void testWebappContextDiscovered() throws Exception
 
         LocalConnector connector = server.getBean(LocalConnector.class);
         String response = connector.getResponse("GET / HTTP/1.0\r\n\r\n");
-        System.err.println(response);
         assertThat(response, containsString("HTTP/1.1 200 OK"));
         assertThat(response, containsString("Hello GreetingsServlet filtered by Weld BeanManager "));
         assertThat(response, containsString("Beans from Weld BeanManager "));

diff --git a/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-cdi/src/test/resources/jetty-logging.properties b/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-cdi/src/test/resources/jetty-logging.properties
@@ -1,4 +1,5 @@
 # Jetty Logging using jetty-slf4j-impl
 org.eclipse.jetty.LEVEL=INFO
-org.eclipse.jetty.ee10.cdi.LEVEL=DEBUG
-org.jboss.LEVEL=DEBUG
+#org.eclipse.jetty.ee10.annotations.LEVEL=DEBUG
+#org.eclipse.jetty.ee10.cdi.LEVEL=DEBUG
+#org.jboss.LEVEL=DEBUG
diff --git a/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-owb-cdi-webapp/pom.xml b/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-owb-cdi-webapp/pom.xml
@@ -37,10 +37,10 @@
     </dependency>
 
     <!-- included in webapp -->
-    <dependency>
+    <!-- dependency>
       <groupId>jakarta.annotation</groupId>
       <artifactId>jakarta.annotation-api</artifactId>
-    </dependency>
+    </dependency -->
     <dependency>
       <groupId>jakarta.enterprise</groupId>
       <artifactId>jakarta.enterprise.cdi-api</artifactId>
@@ -58,10 +58,10 @@
       <artifactId>openwebbeans-web</artifactId>
       <version>${openwebbeans.version}</version>
     </dependency>
-    <dependency>
+    <!-- dependency>
       <groupId>org.apache.openwebbeans</groupId>
       <artifactId>openwebbeans-jetty9</artifactId>
       <version>${openwebbeans.version}</version>
-    </dependency>
+    </dependency -->
   </dependencies>
 </project>
diff --git a/...i-webapp/src/main/resources/META-INF/services/jakarta.servlet.ServletContainerInitializer b/...i-webapp/src/main/resources/META-INF/services/jakarta.servlet.ServletContainerInitializer
diff --git a/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-owb-cdi-webapp/src/main/webapp/WEB-INF/web.xml b/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-owb-cdi-webapp/src/main/webapp/WEB-INF/web.xml
@@ -5,11 +5,19 @@
   version="6.0">
   <display-name>OWB CDI Integration Test WebApp</display-name>
 
-  <!-- Required by org.apache.webbeans.servlet.WebBeansConfigurationListener$Auto -->
-  <context-param>
+  <!--  This will be required by the SCI org.apache.webbeans.servlet.WebBeansConfigurationListener$Auto 
+        however, the current release of OWB has not renamed their META-INF/services to jakarta.servlet
+        so this SCI will not be found.
+  -->
+  <!-- context-param>
     <param-name>openwebbeans.web.sci.active</param-name>
     <param-value>true</param-value>
-  </context-param>
+  </context-param -->
+
+  <!-- Remove in favour of SCI when it has been fixed -->
+  <listener>
+    <listener-class>org.apache.webbeans.servlet.WebBeansConfigurationListener</listener-class>
+  </listener>
 
   <resource-env-ref>
     <description>Object factory for the CDI Bean Manager</description>

diff --git a/...cdi/src/main/config/etc/cdi/jetty-cdi.xml → ...src/main/config/etc/cdi/jetty-ee9-cdi.xml b/...cdi/src/main/config/etc/cdi/jetty-cdi.xml → ...src/main/config/etc/cdi/jetty-ee9-cdi.xml
diff --git a/.../src/main/config/modules/cdi-decorate.mod → .../main/config/modules/ee9-cdi-decorate.mod b/.../src/main/config/modules/cdi-decorate.mod → .../main/config/modules/ee9-cdi-decorate.mod
@@ -4,14 +4,17 @@
 Configures Jetty to use the "CdiDecoratingListener" as the default CDI mode.
 This mode that allows a webapp to register it's own CDI decorator.
 
+[environment]
+ee9
+
 [tag]
 cdi
 
 [provides]
 cdi-mode
 
 [depend]
-cdi
+ee9-cdi
 
 [ini]
 jetty.cdi.mode=CdiDecoratingListener
diff --git a/...9-cdi/src/main/config/modules/cdi-spi.mod → ...i/src/main/config/modules/ee9-cdi-spi.mod b/...9-cdi/src/main/config/modules/cdi-spi.mod → ...i/src/main/config/modules/ee9-cdi-spi.mod
@@ -4,14 +4,17 @@
 Configures Jetty to use the "CdiSpiDecorator" as the default CDI mode.
 This mode uses the CDI SPI to integrate an arbitrary CDI implementation.
 
+[environment]
+ee9
+
 [tag]
 cdi
 
 [provides]
 cdi-mode
 
 [depend]
-cdi
+ee9-cdi
 
 [ini]
 jetty.cdi.mode=CdiSpiDecorator
diff --git a/...y-ee9-cdi/src/main/config/modules/cdi.mod → ...9-cdi/src/main/config/modules/ee9-cdi.mod b/...y-ee9-cdi/src/main/config/modules/cdi.mod → ...9-cdi/src/main/config/modules/ee9-cdi.mod
@@ -12,6 +12,8 @@ CdiSpiDecorator     - Jetty will call the CDI SPI within the webapp to decorate
                       objects (default).
 CdiDecoratingLister - The webapp may register a decorator on the context attribute
                       "org.eclipse.jetty.ee9.cdi.decorator".
+[environment]
+ee9
 
 [tag]
 cdi
@@ -23,7 +25,7 @@ cdi
 deploy
 
 [xml]
-etc/cdi/jetty-cdi.xml
+etc/cdi/jetty-ee9-cdi.xml
 
 [lib]
-lib/jetty-cdi-${jetty.version}.jar
+lib/jetty-ee9-cdi-${jetty.version}.jar