From 4b2842265ac80641f88447de9f23666925cf2b7b Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Mon, 6 Apr 2020 11:30:33 +0200 Subject: [PATCH] Issue #4737 Ensure lifecycle callbacks happen for run-as and non async servlets (#4744) * Issue #4737 Ensure lifecycle callbacks happen for run-as and non async servlets Signed-off-by: Jan Bartel --- .../LifeCycleCallbackCollectionTest.java | 100 ++++++++++++++++++ .../eclipse/jetty/servlet/ServletHolder.java | 17 ++- 2 files changed, 116 insertions(+), 1 deletion(-) diff --git a/jetty-plus/src/test/java/org/eclipse/jetty/plus/annotation/LifeCycleCallbackCollectionTest.java b/jetty-plus/src/test/java/org/eclipse/jetty/plus/annotation/LifeCycleCallbackCollectionTest.java index a6d3466f4459..9916b57ab09e 100644 --- a/jetty-plus/src/test/java/org/eclipse/jetty/plus/annotation/LifeCycleCallbackCollectionTest.java +++ b/jetty-plus/src/test/java/org/eclipse/jetty/plus/annotation/LifeCycleCallbackCollectionTest.java @@ -20,15 +20,38 @@ import java.lang.reflect.Method; +import javax.servlet.http.HttpServlet; + +import org.eclipse.jetty.plus.webapp.PlusDecorator; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.servlet.ServletHolder; +import org.eclipse.jetty.toolchain.test.MavenTestingUtils; +import org.eclipse.jetty.webapp.WebAppContext; import org.hamcrest.Matchers; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.hasSize; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.fail; public class LifeCycleCallbackCollectionTest { + public static class TestServlet extends HttpServlet + { + public static int postConstructCount = 0; + public static int preDestroyCount = 0; + + public void postconstruct() + { + ++postConstructCount; + } + + public void predestroy() + { + ++preDestroyCount; + } + } /** * An unsupported lifecycle callback type @@ -154,6 +177,83 @@ public void testUnsupportedType() throws Exception //expected } } + + @Test + public void testServletPostConstructPreDestroy() throws Exception + { + Server server = new Server(); + WebAppContext context = new WebAppContext(); + context.setResourceBase(MavenTestingUtils.getTargetTestingDir("predestroy-test").toURI().toURL().toString()); + context.setContextPath("/"); + server.setHandler(context); + + //add a non-async servlet + ServletHolder notAsync = new ServletHolder(); + notAsync.setHeldClass(TestServlet.class); + notAsync.setName("notAsync"); + notAsync.setAsyncSupported(false); + notAsync.setInitOrder(1); + context.getServletHandler().addServletWithMapping(notAsync, "/notasync/*"); + + //add an async servlet + ServletHolder async = new ServletHolder(); + async.setHeldClass(TestServlet.class); + async.setName("async"); + async.setAsyncSupported(true); + async.setInitOrder(1); + context.getServletHandler().addServletWithMapping(async, "/async/*"); + + //add a run-as servlet + ServletHolder runas = new ServletHolder(); + runas.setHeldClass(TestServlet.class); + runas.setName("runas"); + runas.setRunAsRole("admin"); + runas.setInitOrder(1); + context.getServletHandler().addServletWithMapping(runas, "/runas/*"); + + //add both run-as and non async servlet + ServletHolder both = new ServletHolder(); + both.setHeldClass(TestServlet.class); + both.setName("both"); + both.setRunAsRole("admin"); + both.setAsyncSupported(false); + both.setInitOrder(1); + context.getServletHandler().addServletWithMapping(both, "/both/*"); + + //Make fake lifecycle callbacks for all servlets + LifeCycleCallbackCollection collection = new LifeCycleCallbackCollection(); + context.setAttribute(LifeCycleCallbackCollection.LIFECYCLE_CALLBACK_COLLECTION, collection); + PostConstructCallback pcNotAsync = new PostConstructCallback(TestServlet.class, "postconstruct"); + collection.add(pcNotAsync); + PreDestroyCallback pdNotAsync = new PreDestroyCallback(TestServlet.class, "predestroy"); + collection.add(pdNotAsync); + + PostConstructCallback pcAsync = new PostConstructCallback(TestServlet.class, "postconstruct"); + collection.add(pcAsync); + PreDestroyCallback pdAsync = new PreDestroyCallback(TestServlet.class, "predestroy"); + collection.add(pdAsync); + + PostConstructCallback pcRunAs = new PostConstructCallback(TestServlet.class, "postconstruct"); + collection.add(pcRunAs); + PreDestroyCallback pdRunAs = new PreDestroyCallback(TestServlet.class, "predestroy"); + collection.add(pdRunAs); + + PostConstructCallback pcBoth = new PostConstructCallback(TestServlet.class, "postconstruct"); + collection.add(pcBoth); + PreDestroyCallback pdBoth = new PreDestroyCallback(TestServlet.class, "predestroy"); + collection.add(pdBoth); + + //ensure we invoke the lifecyclecallbacks + context.getObjectFactory().addDecorator(new PlusDecorator(context)); + + server.start(); + + assertEquals(4, TestServlet.postConstructCount); + + server.stop(); + + assertEquals(4, TestServlet.preDestroyCount); + } @Test public void testAddForPreDestroy() throws Exception 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 index 20d31710ea6f..28c4974ba6f0 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHolder.java +++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHolder.java @@ -457,7 +457,14 @@ public void destroyInstance(Object o) if (o == null) return; Servlet servlet = ((Servlet)o); - getServletHandler().destroyServlet(servlet); + //need to use the unwrapped servlet because lifecycle callbacks such as + //postconstruct and predestroy are based off the classname and the wrapper + //classes are unknown outside the ServletHolder + Servlet unwrapped = servlet; + while (WrapperServlet.class.isAssignableFrom(unwrapped.getClass())) + unwrapped = ((WrapperServlet)unwrapped).getWrappedServlet(); + getServletHandler().destroyServlet(unwrapped); + //destroy the wrapped servlet, in case there is special behaviour servlet.destroy(); } @@ -1304,6 +1311,14 @@ public void destroy() { _servlet.destroy(); } + + /** + * @return the original servlet + */ + public Servlet getWrappedServlet() + { + return _servlet; + } @Override public String toString()