Issue #5032 - Moving wrap/unwrap to BaseHolder

+ Fixing Filter destroy call order

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>

jetty-servlet/src/main/java/org/eclipse/jetty/servlet/BaseHolder.java

@@ -19,6 +19,7 @@
 package org.eclipse.jetty.servlet;
 
 import java.io.IOException;
+import java.util.function.BiFunction;
 import javax.servlet.ServletContext;
 import javax.servlet.UnavailableException;
 
@@ -185,6 +186,32 @@ public synchronized boolean isInstance()
         return _instance != null;
     }
 
+    protected <C, W> C wrap(final C component, final Class<W> wrapperFunctionType, final BiFunction<W, C, C> function)
+    {
+        C ret = component;
+        ServletContextHandler contextHandler = getServletHandler().getServletContextHandler();
+        if (contextHandler != null)
+        {
+            for (W wrapperFunction : contextHandler.getBeans(wrapperFunctionType))
+            {
+                ret = function.apply(wrapperFunction, ret);
+            }
+        }
+        return ret;
+    }
+
+    protected <C> C unwrap(final C component)
+    {
+        C ret = component;
+
+        while (ret instanceof Wrapped)
+        {
+            // noinspection unchecked,rawtypes
+            ret = (C)((Wrapped)ret).getWrapped();
+        }
+        return ret;
+    }
+
     @Override
     public void dump(Appendable out, String indent) throws IOException
     {
@@ -196,4 +223,9 @@ public String dump()
     {
         return Dumpable.dump(this);
     }
+
+    interface Wrapped<C>
+    {
+        C getWrapped();
+    }
 }

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);
+            _filter = wrap(_filter, WrapperFunction.class, WrapperFunction::wrapFilter);
             _config = new Config();
             if (LOG.isDebugEnabled())
                 LOG.debug("Filter.init {}", _filter);
@@ -166,36 +166,14 @@ public void destroyInstance(Object o)
             return;
 
         Filter filter = (Filter)o;
-        // destroy the wrapped filter, in case there is special behaviour
-        filter.destroy();
+
         // need to use the unwrapped filter because lifecycle callbacks such as
         // postconstruct and predestroy are based off the classname and the wrapper
         // classes are unknown outside the ServletHolder
         getServletHandler().destroyFilter(unwrap(filter));
-    }
 
-    private Filter wrap(final Filter filter)
-    {
-        Filter ret = filter;
-        ServletContextHandler contextHandler = getServletHandler().getServletContextHandler();
-        if (contextHandler != null)
-        {
-            for (FilterHolder.WrapperFunction wrapperFunction : contextHandler.getBeans(FilterHolder.WrapperFunction.class))
-            {
-                ret = wrapperFunction.wrapFilter(ret);
-            }
-        }
-        return ret;
-    }
-
-    private Filter unwrap(Filter filter)
-    {
-        Filter unwrapped = filter;
-        while (FilterHolder.WrapperFilter.class.isAssignableFrom(unwrapped.getClass()))
-        {
-            unwrapped = ((FilterHolder.WrapperFilter)unwrapped).getWrappedFilter();
-        }
-        return unwrapped;
+        // destroy the wrapped filter, in case there is special behaviour
+        filter.destroy();
     }
 
     public synchronized void setFilter(Filter filter)
@@ -314,25 +292,37 @@ public String getFilterName()
     /**
      * Experimental Wrapper mechanism for Filter objects.
      * <p>
-     * Beans in ServletContextHandler or WebAppContext that implement this interface
+     * Beans in {@code ServletContextHandler} or {@code WebAppContext} that implement this interface
      * will be called to optionally wrap any newly created Filters
      * (before their {@link Filter#init(FilterConfig)} method is called)
      * </p>
      */
     public interface WrapperFunction
     {
+        /**
+         * Optionally wrap the Filter.
+         *
+         * @param filter the Filter being passed in.
+         * @return the Filter (extend from {@link FilterHolder.Wrapper} if you do wrap the Filter)
+         */
         Filter wrapFilter(Filter filter);
     }
 
-    public static class WrapperFilter implements Filter
+    public static class Wrapper implements Filter, Wrapped<Filter>
     {
         private final Filter _filter;
 
-        public WrapperFilter(Filter filter)
+        public Wrapper(Filter filter)
         {
             _filter = Objects.requireNonNull(filter, "Filter cannot be null");
         }
 
+        @Override
+        public Filter getWrapped()
+        {
+            return _filter;
+        }
+
         @Override
         public void init(FilterConfig filterConfig) throws ServletException
         {
@@ -351,11 +341,6 @@ public void destroy()
             _filter.destroy();
         }
 
-        public Filter getWrappedFilter()
-        {
-            return _filter;
-        }
-
         @Override
         public String toString()
         {

jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ListenerHolder.java

@@ -102,7 +102,7 @@ public void doStart() throws Exception
                     throw ex;
                 }
             }
-            _listener = wrap(_listener);
+            _listener = wrap(_listener, WrapperFunction.class, WrapperFunction::wrapEventListener);
             contextHandler.addEventListener(_listener);
         }
     }
@@ -127,59 +127,43 @@ public void doStop() throws Exception
         }
     }
 
-    private EventListener wrap(final EventListener listener)
-    {
-        EventListener ret = listener;
-        ServletContextHandler contextHandler = getServletHandler().getServletContextHandler();
-        if (contextHandler != null)
-        {
-            for (ListenerHolder.WrapperFunction wrapperFunction : contextHandler.getBeans(ListenerHolder.WrapperFunction.class))
-            {
-                ret = wrapperFunction.wrapEventListener(ret);
-            }
-        }
-        return ret;
-    }
-
-    private static EventListener unwrap(EventListener listener)
-    {
-        EventListener unwrapped = listener;
-        while (ListenerHolder.WrapperEventListener.class.isAssignableFrom(unwrapped.getClass()))
-        {
-            unwrapped = ((ListenerHolder.WrapperEventListener)unwrapped).getWrappedListener();
-        }
-        return unwrapped;
-    }
-
     @Override
     public String toString()
     {
         return super.toString() + ": " + getClassName();
     }
 
     /**
-     * Experimental Wrapper mechanism for Servlet Listeners.
+     * Experimental Wrapper mechanism for Servlet EventListeners.
      * <p>
-     * Beans in ServletContextHandler or WebAppContext that implement this interface
-     * will be called to optionally wrap any newly created ServletListeners before
+     * Beans in {@code ServletContextHandler} or {@code WebAppContext} that implement this interface
+     * will be called to optionally wrap any newly created Servlet EventListeners before
      * they are used for the first time.
      * </p>
      */
     public interface WrapperFunction
     {
+        /**
+         * Optionally wrap the Servlet EventListener.
+         *
+         * @param listener the Servlet EventListener being passed in.
+         * @return the Servlet EventListener (extend from {@link ListenerHolder.Wrapper}
+         * if you do wrap the Servlet EventListener)
+         */
         EventListener wrapEventListener(EventListener listener);
     }
 
-    public static class WrapperEventListener implements EventListener
+    public static class Wrapper implements EventListener, Wrapped<EventListener>
     {
         final EventListener _listener;
 
-        public WrapperEventListener(EventListener listener)
+        public Wrapper(EventListener listener)
         {
             _listener = listener;
         }
 
-        public EventListener getWrappedListener()
+        @Override
+        public EventListener getWrapped()
         {
             return _listener;
         }