Issue #5032 - Updating based on review with @gregw and @janb

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
jetty · Sep 16, 2020 · 9d4b92b · 9d4b92b
1 parent c569c87
commit 9d4b92b
Show file tree

Hide file tree

Showing 5 changed files with 12 additions and 19 deletions.
diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/BaseHolder.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/BaseHolder.java
@@ -190,6 +190,12 @@ protected <W> T wrap(final T component, final Class<W> wrapperFunctionType, fina
     {
         T ret = component;
         ServletContextHandler contextHandler = getServletHandler().getServletContextHandler();
+        if (contextHandler == null)
+        {
+            ContextHandler.Context context = ContextHandler.getCurrentContext();
+            contextHandler = (ServletContextHandler)(context == null ? null : context.getContextHandler());
+        }
+
         if (contextHandler != null)
         {
             for (W wrapperFunction : contextHandler.getBeans(wrapperFunctionType))

diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/FilterHolder.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/FilterHolder.java
@@ -132,7 +132,7 @@ public void initialize() throws Exception
                     throw ex;
                 }
             }
-            _filter = wrap(_filter, WrapFunction.class, WrapFunction::wrapFilter);
+            _filter = wrap(_filter, FilterHolder.WrapFunction.class, FilterHolder.WrapFunction::wrapFilter);
             _config = new Config();
             if (LOG.isDebugEnabled())
                 LOG.debug("Filter.init {}", _filter);

diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHolder.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHolder.java
@@ -445,7 +445,7 @@ public void destroyInstance(Object o)
         if (o == null)
             return;
 
-        Servlet servlet = ((Servlet)o);
+        Servlet servlet = (Servlet)o;
 
         // need to use the unwrapped servlet because lifecycle callbacks such as
         // postconstruct and predestroy are based off the classname and the wrapper

diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ComponentWrapTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ComponentWrapTest.java
@@ -103,8 +103,6 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IO
             @Override
             public void init(FilterConfig filterConfig)
             {
-                // TODO: this shouldn't be required (see FIXME below)
-                filterConfig.getServletContext().addListener(LoggingRequestListener.class);
             }
 
             @Override
@@ -120,19 +118,8 @@ public void destroy()
         });
         contextHandler.addFilter(filterHolder, "/*", EnumSet.of(DispatcherType.REQUEST));
 
-        /* FIXME
-        These 2 options for event listeners don't work in embedded mode.
-
-        1) contextHandler.addEventListener(new LoggingRequestListener());
-           - this does not result in a ListenerHolder.
-
-        2) ListenerHolder listenerHolder = new ListenerHolder(LoggingRequestListener.class);
-           contextHandler.getServletHandler().addListener(listenerHolder);
-           - this results in a ServletHandler without a reference to
-             the ServletContextHandler
-
-        Only adding Listener during context initialization works (see Filter.init above)
-         */
+        ListenerHolder listenerHolder = new ListenerHolder(LoggingRequestListener.class);
+        contextHandler.getServletHandler().addListener(listenerHolder);
 
         contextHandler.addBean(wrapHandler);
         server.setHandler(contextHandler);

diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletLifeCycleTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletLifeCycleTest.java
@@ -110,10 +110,10 @@ public void testLifeCycle() throws Exception
         server.stop();
 
         assertThat(events, Matchers.contains(
-            "destroy class org.eclipse.jetty.servlet.ServletLifeCycleTest$TestFilter2",
             "Destroy class org.eclipse.jetty.servlet.ServletLifeCycleTest$TestFilter2",
-            "destroy class org.eclipse.jetty.servlet.ServletLifeCycleTest$TestFilter",
+            "destroy class org.eclipse.jetty.servlet.ServletLifeCycleTest$TestFilter2",
             "Destroy class org.eclipse.jetty.servlet.ServletLifeCycleTest$TestFilter",
+            "destroy class org.eclipse.jetty.servlet.ServletLifeCycleTest$TestFilter",
             "Destroy class org.eclipse.jetty.servlet.ServletLifeCycleTest$TestServlet3",
             "destroy class org.eclipse.jetty.servlet.ServletLifeCycleTest$TestServlet3",
             "Destroy class org.eclipse.jetty.servlet.ServletLifeCycleTest$TestServlet2",