From 61ffdc29f8ded6053e1d87340a9c26c9236d5db8 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 16 Sep 2020 11:36:31 -0500 Subject: [PATCH] Issue #5137 - Adding test case to demonstrate usage. + Partially reverting change in commit 31175e98e59d0c3fcf2d375994584f8eeef907fd Signed-off-by: Joakim Erdfelt --- .../org/eclipse/jetty/servlet/BaseHolder.java | 31 +- .../eclipse/jetty/servlet/FilterHolder.java | 14 +- .../eclipse/jetty/servlet/ListenerHolder.java | 15 +- .../eclipse/jetty/servlet/ServletHolder.java | 14 +- .../jetty/servlet/ComponentWrapTest.java | 305 ++++++++++++++++++ 5 files changed, 355 insertions(+), 24 deletions(-) create mode 100644 jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ComponentWrapTest.java 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 index cb3e03ddc239..6693093d6eb2 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/BaseHolder.java +++ b/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,25 +186,30 @@ public synchronized boolean isInstance() return _instance != null; } - protected T wrap(T component, Class> wrapperType) + protected C wrap(final C component, final Class wrapperFunctionType, final BiFunction function) { + C ret = component; ServletContextHandler contextHandler = getServletHandler().getServletContextHandler(); if (contextHandler != null) { - for (BaseWrapFunction wrapperFunction : contextHandler.getBeans(wrapperType)) - component = wrapperFunction.wrap(component); + for (W wrapperFunction : contextHandler.getBeans(wrapperFunctionType)) + { + ret = function.apply(wrapperFunction, ret); + } } - return component; + return ret; } - protected T unwrap(T component) + protected C unwrap(final C component) { - while (component instanceof Wrapped) + C ret = component; + + while (ret instanceof Wrapped) { // noinspection unchecked,rawtypes - component = (T)((Wrapped)component).getWrapped(); + ret = (C)((Wrapped)ret).getWrapped(); } - return component; + return ret; } @Override @@ -218,13 +224,8 @@ public String dump() return Dumpable.dump(this); } - public interface BaseWrapFunction - { - T wrap(T component); - } - - interface Wrapped + interface Wrapped { - T getWrapped(); + C getWrapped(); } } 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 index c31d19ab036f..a5390b130e29 100644 --- 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); + _filter = wrap(_filter, WrapFunction.class, WrapFunction::wrapFilter); _config = new Config(); if (LOG.isDebugEnabled()) LOG.debug("Filter.init {}", _filter); @@ -297,8 +297,16 @@ public String getFilterName() * (before their {@link Filter#init(FilterConfig)} method is called) *

*/ - public interface WrapFunction extends BaseWrapFunction - {} + public interface WrapFunction + { + /** + * 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 Wrapper implements Filter, Wrapped { diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ListenerHolder.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ListenerHolder.java index 10504abe9bff..ef10588e5d3c 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ListenerHolder.java +++ b/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, WrapperFunction.class); + _listener = wrap(_listener, WrapFunction.class, WrapFunction::wrapEventListener); contextHandler.addEventListener(_listener); } } @@ -141,8 +141,17 @@ public String toString() * they are used for the first time. *

*/ - public interface WrapperFunction extends BaseWrapFunction - {} + public interface WrapFunction + { + /** + * 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 Wrapper implements EventListener, Wrapped { 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 146680f32d94..e41941727d3e 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 @@ -595,7 +595,7 @@ else if (_forcedPath != null) detectJspContainer(); initMultiPart(); - _servlet = wrap(_servlet, WrapperFunction.class); + _servlet = wrap(_servlet, WrapFunction.class, WrapFunction::wrapServlet); if (LOG.isDebugEnabled()) LOG.debug("Servlet.init {} for {}", _servlet, getName()); @@ -1281,8 +1281,16 @@ public UnavailableException getUnavailableException() * (before their {@link Servlet#init(ServletConfig)} method is called) *

*/ - public interface WrapperFunction extends BaseWrapFunction - {} + public interface WrapFunction + { + /** + * Optionally wrap the Servlet. + * + * @param servlet the servlet being passed in. + * @return the servlet (extend from {@link ServletHolder.Wrapper} if you do wrap the Servlet) + */ + Servlet wrapServlet(Servlet servlet); + } public static class Wrapper implements Servlet, Wrapped { 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 new file mode 100644 index 000000000000..2f7ebcf1210d --- /dev/null +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ComponentWrapTest.java @@ -0,0 +1,305 @@ +// +// ======================================================================== +// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.servlet; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.EnumSet; +import java.util.EventListener; +import java.util.List; +import java.util.concurrent.LinkedBlockingQueue; +import javax.servlet.DispatcherType; +import javax.servlet.Filter; +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; +import javax.servlet.Servlet; +import javax.servlet.ServletConfig; +import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletRequestEvent; +import javax.servlet.ServletRequestListener; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.eclipse.jetty.client.HttpClient; +import org.eclipse.jetty.client.api.ContentResponse; +import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.util.component.LifeCycle; +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.Logger; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; + +public class ComponentWrapTest +{ + private static final Logger LOG = Log.getLogger(ComponentWrapTest.class); + private Server server; + private HttpClient client; + + @BeforeEach + public void setUp() throws Exception + { + server = new Server(); + ServerConnector connector = new ServerConnector(server); + connector.setPort(0); + server.addConnector(connector); + + client = new HttpClient(); + client.start(); + } + + @AfterEach + public void tearDown() + { + LifeCycle.stop(client); + LifeCycle.stop(server); + } + + @Test + public void testSimpleFilterServletAndListener() throws Exception + { + EventQueue events = new EventQueue(); + WrapHandler wrapHandler = new WrapHandler(events); + + ServletContextHandler contextHandler = new ServletContextHandler(); + contextHandler.setContextPath("/"); + ServletHolder servletHolder = new ServletHolder(new HttpServlet() + { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException + { + resp.setContentType("text/plain"); + resp.setCharacterEncoding("utf-8"); + resp.getWriter().println("hello"); + } + }); + contextHandler.addServlet(servletHolder, "/hello"); + FilterHolder filterHolder = new FilterHolder(new Filter() + { + @Override + public void init(FilterConfig filterConfig) + { + // TODO: this shouldn't be required (see FIXME below) + filterConfig.getServletContext().addListener(LoggingRequestListener.class); + } + + @Override + public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException + { + chain.doFilter(request, response); + } + + @Override + 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) + */ + + contextHandler.addBean(wrapHandler); + server.setHandler(contextHandler); + server.start(); + + ContentResponse response = client.GET(server.getURI().resolve("/hello")); + assertThat("Response.status", response.getStatus(), is(HttpStatus.OK_200)); + + List expectedEvents = new ArrayList<>(); + expectedEvents.add("TestWrapFilter.init()"); + expectedEvents.add("TestWrapServlet.init()"); + expectedEvents.add("TestWrapListener.requestInitialized()"); + expectedEvents.add("TestWrapFilter.doFilter()"); + expectedEvents.add("TestWrapServlet.service()"); + expectedEvents.add("TestWrapListener.requestDestroyed()"); + + List actualEvents = new ArrayList<>(); + actualEvents.addAll(events); + + assertThat("Metrics Events Count", actualEvents.size(), is(expectedEvents.size())); + } + + public static class LoggingRequestListener implements ServletRequestListener + { + @Override + public void requestDestroyed(ServletRequestEvent sre) + { + LOG.info("requestDestroyed()"); + } + + @Override + public void requestInitialized(ServletRequestEvent sre) + { + LOG.info("requestInitialized()"); + } + } + + public static class EventQueue extends LinkedBlockingQueue + { + private static final Logger LOG = Log.getLogger(EventQueue.class); + + public void addEvent(String format, Object... args) + { + String eventText = String.format(format, args); + offer(eventText); + Throwable cause = null; + if (args.length > 0) + { + Object lastArg = args[args.length - 1]; + if (lastArg instanceof Throwable) + { + cause = (Throwable)lastArg; + } + } + LOG.info("[EVENT] {}", eventText, cause); + } + } + + public static class WrapHandler implements + FilterHolder.WrapFunction, + ServletHolder.WrapFunction, + ListenerHolder.WrapFunction + { + private EventQueue events; + + public WrapHandler(EventQueue events) + { + this.events = events; + } + + @Override + public Filter wrapFilter(Filter filter) + { + return new TestWrapFilter(filter, events); + } + + @Override + public EventListener wrapEventListener(EventListener listener) + { + if (listener instanceof ServletRequestListener) + { + return new TestWrapListener((ServletRequestListener)listener, events); + } + return listener; + } + + @Override + public Servlet wrapServlet(Servlet servlet) + { + return new TestWrapServlet(servlet, events); + } + } + + public static class TestWrapFilter extends FilterHolder.Wrapper + { + private EventQueue events; + + public TestWrapFilter(Filter filter, EventQueue events) + { + super(filter); + this.events = events; + } + + @Override + public void init(FilterConfig filterConfig) throws ServletException + { + events.addEvent("TestWrapFilter.init()"); + super.init(filterConfig); + } + + @Override + public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException + { + events.addEvent("TestWrapFilter.doFilter()"); + super.doFilter(request, response, chain); + } + } + + public static class TestWrapServlet extends ServletHolder.Wrapper + { + private EventQueue events; + + public TestWrapServlet(Servlet servlet, EventQueue events) + { + super(servlet); + this.events = events; + } + + @Override + public void init(ServletConfig config) throws ServletException + { + events.addEvent("TestWrapServlet.init()"); + super.init(config); + } + + @Override + public void service(ServletRequest req, ServletResponse res) throws ServletException, IOException + { + events.addEvent("TestWrapServlet.service()"); + super.service(req, res); + } + } + + public static class TestWrapListener extends ListenerHolder.Wrapper implements ServletRequestListener + { + private ServletRequestListener requestListener; + private EventQueue events; + + public TestWrapListener(ServletRequestListener listener, EventQueue events) + { + super(listener); + this.requestListener = listener; + this.events = events; + } + + @Override + public void requestDestroyed(ServletRequestEvent sre) + { + this.events.addEvent("TestWrapListener.requestDestroyed()"); + requestListener.requestDestroyed(sre); + } + + @Override + public void requestInitialized(ServletRequestEvent sre) + { + this.events.addEvent("TestWrapListener.requestInitialized()"); + requestListener.requestInitialized(sre); + } + } +}