From 011a7267dea347bd76a1c6c3f79de6993c722f8c Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Fri, 25 Nov 2022 14:27:39 +1100 Subject: [PATCH] Cleanup ContextHandler (#8928) * Cleanup ContextHandler Extracted some of the goodness from #8793: + Clear enter/exit scope methods rather than opaque suppliers and Runnables + Removed overloading of "Context" class name to avoid accidental usage of wrong type. + Less holding onto request/response as fields * Cleanup ContextHandler fixed test with no server * Updates from review. --- .../eclipse/jetty/jndi/ContextFactory.java | 2 +- .../jetty/server/handler/ContextHandler.java | 146 ++++++++---------- .../jetty/server/handler/ContextRequest.java | 59 ++----- .../jetty/server/handler/ContextResponse.java | 4 +- .../jetty/server/handler/ResourceHandler.java | 7 +- .../jetty/session/AbstractSessionManager.java | 2 +- .../server/WebSocketServerComponents.java | 2 +- .../core/WebSocketServerComponentsTest.java | 2 +- .../jetty/ee10/servlet/AsyncContextEvent.java | 6 +- .../jetty/ee10/servlet/BaseHolder.java | 11 +- .../jetty/ee10/servlet/DebugListener.java | 4 +- .../jetty/ee10/servlet/DefaultServlet.java | 9 +- .../jetty/ee10/servlet/ErrorHandler.java | 2 +- .../jetty/ee10/servlet/ListenerHolder.java | 26 +++- .../jetty/ee10/servlet/ServletChannel.java | 11 +- .../ServletContainerInitializerHolder.java | 9 +- .../ee10/servlet/ServletContextHandler.java | 46 +++--- .../ee10/servlet/ServletContextRequest.java | 21 +-- .../ee10/servlet/ServletContextResponse.java | 2 +- .../jetty/ee10/servlet/ServletHandler.java | 9 +- .../jetty/ee10/servlet/SessionHandler.java | 9 +- .../jetty/ee10/servlet/StatisticsServlet.java | 2 +- .../jetty/ee10/servlet/ComponentWrapTest.java | 19 ++- .../servlet/ServletContextHandlerTest.java | 4 +- .../jetty/ee9/nested/ContextHandler.java | 14 +- 25 files changed, 205 insertions(+), 223 deletions(-) diff --git a/jetty-core/jetty-jndi/src/main/java/org/eclipse/jetty/jndi/ContextFactory.java b/jetty-core/jetty-jndi/src/main/java/org/eclipse/jetty/jndi/ContextFactory.java index 88f93d00ac0..0c07ce431db 100644 --- a/jetty-core/jetty-jndi/src/main/java/org/eclipse/jetty/jndi/ContextFactory.java +++ b/jetty-core/jetty-jndi/src/main/java/org/eclipse/jetty/jndi/ContextFactory.java @@ -162,7 +162,7 @@ public class ContextFactory implements ObjectFactory LOG.debug("Trying classloader of current org.eclipse.jetty.server.handler.ContextHandler"); try (AutoLock l = __lock.lock()) { - loader = ContextHandler.getCurrentContext().getContextHandler().getClassLoader(); + loader = ContextHandler.getCurrentContext().getClassLoader(); ctx = (Context)__contextMap.get(loader); if (ctx == null && loader != null) diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java index aa49712aedf..014ae479b73 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java @@ -29,7 +29,6 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; -import java.util.function.Supplier; import java.util.stream.Collectors; import org.eclipse.jetty.http.HttpField; @@ -38,6 +37,7 @@ import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.MimeTypes; import org.eclipse.jetty.server.AliasCheck; import org.eclipse.jetty.server.Connector; +import org.eclipse.jetty.server.Context; import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Response; @@ -74,15 +74,28 @@ public class ContextHandler extends Handler.Wrapper implements Attributes, Grace private static final ThreadLocal __context = new ThreadLocal<>(); /** - * Get the current ServletContext implementation. + * Get the current Context if any. * - * @return ServletContext implementation + * @return The {@link Context} from a {@link ContextHandler}; + * or {@link Server#getContext()} if the current {@link Thread} is not scoped to a {@link ContextHandler}. */ public static Context getCurrentContext() { return __context.get(); } + public static ContextHandler getCurrentContextHandler() + { + Context context = getCurrentContext(); + return (context instanceof ScopedContext scopedContext) ? scopedContext.getContextHandler() : null; + } + + public static ContextHandler getContextHandler(Request request) + { + ContextRequest contextRequest = Request.as(request, ContextRequest.class); + return (contextRequest == null) ? null : contextRequest.getContext().getContextHandler(); + } + public static String getServerInfo() { return "jetty/" + Server.getVersion(); @@ -94,7 +107,7 @@ public class ContextHandler extends Handler.Wrapper implements Attributes, Grace * wrap it. This is so that any cross context dispatch does not inherit attributes and types from * the dispatching context. */ - private final Context _context; + private final ScopedContext _context; private final Attributes _persistentAttributes = new Mapped(); private final MimeTypes.Wrapper _mimeTypes = new MimeTypes.Wrapper(); private final List _contextListeners = new CopyOnWriteArrayList<>(); @@ -129,12 +142,6 @@ public class ContextHandler extends Handler.Wrapper implements Attributes, Grace PREFIX } - public static ContextHandler getContextHandler(Request request) - { - ContextRequest contextRequest = Request.as(request, ContextRequest.class); - return (contextRequest == null) ? null : contextRequest.getContext().getContextHandler(); - } - private final AtomicReference _availability = new AtomicReference<>(Availability.STOPPED); public ContextHandler() @@ -166,10 +173,10 @@ public class ContextHandler extends Handler.Wrapper implements Attributes, Grace super.setServer(server); _mimeTypes.setWrapped(server.getMimeTypes()); } - - protected Context newContext() + + protected ScopedContext newContext() { - return new Context(); + return new ScopedContext(); } /** @@ -192,7 +199,7 @@ public class ContextHandler extends Handler.Wrapper implements Attributes, Grace } @ManagedAttribute(value = "Context") - public ContextHandler.Context getContext() + public ScopedContext getContext() { return _context; } @@ -432,10 +439,20 @@ public class ContextHandler extends Handler.Wrapper implements Attributes, Grace return false; } + protected ClassLoader enterScope(Request contextRequest) + { + ClassLoader lastLoader = Thread.currentThread().getContextClassLoader(); + __context.set(_context); + if (_classLoader != null) + Thread.currentThread().setContextClassLoader(_classLoader); + notifyEnterScope(contextRequest); + return lastLoader; + } + /** * @param request A request that is applicable to the scope, or null */ - protected void enterScope(Request request) + protected void notifyEnterScope(Request request) { for (ContextScopeListener listener : _contextListeners) { @@ -450,10 +467,17 @@ public class ContextHandler extends Handler.Wrapper implements Attributes, Grace } } + protected void exitScope(Request request, Context lastContext, ClassLoader lastLoader) + { + notifyExitScope(request); + __context.set(lastContext == null && getServer() != null ? getServer().getContext() : lastContext); + Thread.currentThread().setContextClassLoader(lastLoader); + } + /** * @param request A request that is applicable to the scope, or null */ - protected void exitScope(Request request) + protected void notifyExitScope(Request request) { for (int i = _contextListeners.size(); i-- > 0; ) { @@ -555,6 +579,9 @@ public class ContextHandler extends Handler.Wrapper implements Attributes, Grace if (getContextPath() == null) throw new IllegalStateException("Null contextPath"); + Server server = getServer(); + if (server != null) + __context.set(server.getContext()); _availability.set(Availability.STARTING); try { @@ -668,11 +695,17 @@ public class ContextHandler extends Handler.Wrapper implements Attributes, Grace if (processor != null) return processor; - // The contextRequest is-a Supplier that calls effectively calls getHandler().handle(request). - // Call this supplier in the scope of the context. - Request.Processor contextScopedProcessor = _context.get(contextRequest, contextRequest); - // Wrap the contextScopedProcessor with a wrapper that uses the wrapped request - return contextRequest.wrapProcessor(contextScopedProcessor); + ClassLoader lastLoader = enterScope(contextRequest); + try + { + processor = getHandler().handle(contextRequest); + } + finally + { + exitScope(contextRequest, request.getContext(), lastLoader); + } + + return contextRequest.wrapProcessor(processor); } protected void processMovedPermanently(Request request, Response response, Callback callback) @@ -989,9 +1022,9 @@ public class ContextHandler extends Handler.Wrapper implements Attributes, Grace return host; } - public class Context extends Attributes.Layer implements org.eclipse.jetty.server.Context + public class ScopedContext extends Attributes.Layer implements Context { - public Context() + public ScopedContext() { // TODO Should the ScopedContext attributes be a layer over the ServerContext attributes? super(_persistentAttributes); @@ -1056,32 +1089,6 @@ public class ContextHandler extends Handler.Wrapper implements Attributes, Grace return ContextHandler.this.getVirtualHosts(); } - public T get(Supplier supplier, Request request) - { - Context lastContext = __context.get(); - if (lastContext == this) - return supplier.get(); - - ClassLoader loader = getClassLoader(); - ClassLoader lastLoader = Thread.currentThread().getContextClassLoader(); - try - { - __context.set(this); - if (loader != null) - Thread.currentThread().setContextClassLoader(loader); - - enterScope(request); - return supplier.get(); - } - finally - { - exitScope(request); - __context.set(lastContext); - if (loader != null) - Thread.currentThread().setContextClassLoader(lastLoader); - } - } - public void call(Invocable.Callable callable, Request request) throws Exception { Context lastContext = __context.get(); @@ -1089,23 +1096,14 @@ public class ContextHandler extends Handler.Wrapper implements Attributes, Grace callable.call(); else { - ClassLoader loader = getClassLoader(); - ClassLoader lastLoader = Thread.currentThread().getContextClassLoader(); + ClassLoader lastLoader = enterScope(request); try { - __context.set(this); - if (loader != null) - Thread.currentThread().setContextClassLoader(loader); - - enterScope(request); callable.call(); } finally { - exitScope(request); - __context.set(lastContext); - if (loader != null) - Thread.currentThread().setContextClassLoader(lastLoader); + exitScope(request, lastContext, lastLoader); } } } @@ -1117,22 +1115,14 @@ public class ContextHandler extends Handler.Wrapper implements Attributes, Grace consumer.accept(t); else { - ClassLoader loader = getClassLoader(); - ClassLoader lastLoader = Thread.currentThread().getContextClassLoader(); + ClassLoader lastLoader = enterScope(request); try { - __context.set(this); - if (loader != null) - Thread.currentThread().setContextClassLoader(loader); - enterScope(request); consumer.accept(t); } finally { - exitScope(request); - __context.set(lastContext); - if (loader != null) - Thread.currentThread().setContextClassLoader(lastLoader); + exitScope(request, lastContext, lastLoader); } } } @@ -1150,22 +1140,14 @@ public class ContextHandler extends Handler.Wrapper implements Attributes, Grace runnable.run(); else { - ClassLoader loader = getClassLoader(); - ClassLoader lastLoader = Thread.currentThread().getContextClassLoader(); + ClassLoader lastLoader = enterScope(request); try { - __context.set(this); - if (loader != null) - Thread.currentThread().setContextClassLoader(loader); - enterScope(request); runnable.run(); } finally { - exitScope(request); - __context.set(lastContext); - if (loader != null) - Thread.currentThread().setContextClassLoader(lastLoader); + exitScope(request, lastContext, lastLoader); } } } @@ -1225,13 +1207,13 @@ public class ContextHandler extends Handler.Wrapper implements Attributes, Grace * @param context The context being entered * @param request A request that is applicable to the scope, or null */ - default void enterScope(org.eclipse.jetty.server.Context context, Request request) {} + default void enterScope(Context context, Request request) {} /** * @param context The context being exited * @param request A request that is applicable to the scope, or null */ - default void exitScope(org.eclipse.jetty.server.Context context, Request request) {} + default void exitScope(Context context, Request request) {} } private static class VHost diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextRequest.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextRequest.java index 7a4193c593f..0cc0108ea08 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextRequest.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextRequest.java @@ -14,10 +14,7 @@ package org.eclipse.jetty.server.handler; import java.util.function.Predicate; -import java.util.function.Supplier; -import org.eclipse.jetty.http.BadMessageException; -import org.eclipse.jetty.io.QuietException; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Response; import org.eclipse.jetty.util.Callback; @@ -25,15 +22,13 @@ import org.eclipse.jetty.util.thread.Invocable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class ContextRequest extends Request.WrapperProcessor implements Invocable, Supplier, Runnable +public class ContextRequest extends Request.WrapperProcessor implements Invocable { private static final Logger LOG = LoggerFactory.getLogger(ContextRequest.class); private final ContextHandler _contextHandler; - private final ContextHandler.Context _context; - private Response _response; - private Callback _callback; + private final ContextHandler.ScopedContext _context; - protected ContextRequest(ContextHandler contextHandler, ContextHandler.Context context, Request wrapped) + protected ContextRequest(ContextHandler contextHandler, ContextHandler.ScopedContext context, Request wrapped) { super(wrapped); _contextHandler = contextHandler; @@ -41,34 +36,25 @@ public class ContextRequest extends Request.WrapperProcessor implements Invocabl } @Override - public Processor get() + public void process(Request request, Response response, Callback callback) throws Exception { + assert this.getWrapped() == request; + ContextResponse contextResponse = newContextResponse(this, response); + ClassLoader lastLoader = _contextHandler.enterScope(this); try { - return _contextHandler.getHandler().handle(this); + super.process(this, contextResponse, callback); } catch (Throwable t) { - // Let's be less verbose with BadMessageExceptions & QuietExceptions - if (!LOG.isDebugEnabled() && (t instanceof BadMessageException || t instanceof QuietException)) - LOG.warn("context bad message {}", t.getMessage()); - else - LOG.warn("context handle failed {}", this, t); + Response.writeError(this, contextResponse, callback, t); + } + finally + { + // We exit scope here, even though process is asynchronous, as we have wrapped + // all our callbacks to re-enter the scope. + _contextHandler.exitScope(this, request.getContext(), lastLoader); } - return null; - } - - @Override - public void process(Request request, Response response, Callback callback) throws Exception - { - _response = response; - _callback = callback; - _context.run(this, this); - } - - public Callback getCallback() - { - return _callback; } protected ContextResponse newContextResponse(Request request, Response response) @@ -76,19 +62,6 @@ public class ContextRequest extends Request.WrapperProcessor implements Invocabl return new ContextResponse(_context, request, response); } - @Override - public void run() - { - try - { - super.process(this, newContextResponse(this, _response), _callback); - } - catch (Throwable t) - { - Response.writeError(this, _response, _callback, t); - } - } - @Override public void demand(Runnable demandCallback) { @@ -108,7 +81,7 @@ public class ContextRequest extends Request.WrapperProcessor implements Invocabl } @Override - public ContextHandler.Context getContext() + public ContextHandler.ScopedContext getContext() { return _context; } diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextResponse.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextResponse.java index 011f0853a80..3e25792c3ee 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextResponse.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextResponse.java @@ -22,9 +22,9 @@ import org.eclipse.jetty.util.thread.Invocable; public class ContextResponse extends Response.Wrapper { - private final ContextHandler.Context _context; + private final ContextHandler.ScopedContext _context; - public ContextResponse(ContextHandler.Context context, Request request, Response response) + public ContextResponse(ContextHandler.ScopedContext context, Request request, Response response) { super(request, response); _context = context; diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ResourceHandler.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ResourceHandler.java index fedefec7123..c0ca5f5d51b 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ResourceHandler.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ResourceHandler.java @@ -26,6 +26,7 @@ import org.eclipse.jetty.http.ResourceHttpContentFactory; import org.eclipse.jetty.http.ValidatingCachingHttpContentFactory; import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.io.NoopByteBufferPool; +import org.eclipse.jetty.server.Context; import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.ResourceService; @@ -67,7 +68,7 @@ public class ResourceHandler extends Handler.Wrapper @Override public void doStart() throws Exception { - ContextHandler.Context context = ContextHandler.getCurrentContext(); + Context context = ContextHandler.getCurrentContext(); if (_resourceBase == null) { if (context != null) @@ -85,11 +86,11 @@ public class ResourceHandler extends Handler.Wrapper super.doStart(); } - private static ByteBufferPool getByteBufferPool(ContextHandler.Context context) + private ByteBufferPool getByteBufferPool(Context context) { if (context == null) return new NoopByteBufferPool(); - Server server = context.getContextHandler().getServer(); + Server server = getServer(); if (server == null) return new NoopByteBufferPool(); ByteBufferPool byteBufferPool = server.getBean(ByteBufferPool.class); diff --git a/jetty-core/jetty-session/src/main/java/org/eclipse/jetty/session/AbstractSessionManager.java b/jetty-core/jetty-session/src/main/java/org/eclipse/jetty/session/AbstractSessionManager.java index 0690258363e..6586d8ee3aa 100644 --- a/jetty-core/jetty-session/src/main/java/org/eclipse/jetty/session/AbstractSessionManager.java +++ b/jetty-core/jetty-session/src/main/java/org/eclipse/jetty/session/AbstractSessionManager.java @@ -32,11 +32,11 @@ import org.eclipse.jetty.http.HttpCookie; import org.eclipse.jetty.http.HttpURI; import org.eclipse.jetty.http.MetaData; import org.eclipse.jetty.http.Syntax; +import org.eclipse.jetty.server.Context; import org.eclipse.jetty.server.HttpStream; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.handler.ContextHandler; -import org.eclipse.jetty.server.handler.ContextHandler.Context; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.annotation.ManagedAttribute; diff --git a/jetty-core/jetty-websocket/jetty-websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/WebSocketServerComponents.java b/jetty-core/jetty-websocket/jetty-websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/WebSocketServerComponents.java index 575948ce913..dc34b96d129 100644 --- a/jetty-core/jetty-websocket/jetty-websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/WebSocketServerComponents.java +++ b/jetty-core/jetty-websocket/jetty-websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/WebSocketServerComponents.java @@ -62,7 +62,7 @@ public class WebSocketServerComponents extends WebSocketComponents */ public static WebSocketComponents ensureWebSocketComponents(Server server, ContextHandler contextHandler) { - ContextHandler.Context context = contextHandler.getContext(); + ContextHandler.ScopedContext context = contextHandler.getContext(); WebSocketComponents components = (WebSocketComponents)context.getAttribute(WEBSOCKET_COMPONENTS_ATTRIBUTE); if (components != null) return components; diff --git a/jetty-core/jetty-websocket/jetty-websocket-core-tests/src/test/java/org/eclipse/jetty/websocket/core/WebSocketServerComponentsTest.java b/jetty-core/jetty-websocket/jetty-websocket-core-tests/src/test/java/org/eclipse/jetty/websocket/core/WebSocketServerComponentsTest.java index db09085058e..a312564668e 100644 --- a/jetty-core/jetty-websocket/jetty-websocket-core-tests/src/test/java/org/eclipse/jetty/websocket/core/WebSocketServerComponentsTest.java +++ b/jetty-core/jetty-websocket/jetty-websocket-core-tests/src/test/java/org/eclipse/jetty/websocket/core/WebSocketServerComponentsTest.java @@ -86,7 +86,7 @@ public class WebSocketServerComponentsTest @Test public void testCompressionPoolsManagedByContext() throws Exception { - ContextHandler.Context context = contextHandler.getContext(); + ContextHandler.ScopedContext context = contextHandler.getContext(); // Use a custom InflaterPool and DeflaterPool that are not started or managed. InflaterPool inflaterPool = new InflaterPool(333, false); diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/AsyncContextEvent.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/AsyncContextEvent.java index ae4515b7fff..ba7e26f85d3 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/AsyncContextEvent.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/AsyncContextEvent.java @@ -26,7 +26,7 @@ import org.eclipse.jetty.util.thread.Scheduler; public class AsyncContextEvent extends AsyncEvent implements Runnable { private final ServletContext _servletContext; - private final ContextHandler.Context _context; + private final ContextHandler.ScopedContext _context; private final AsyncContextState _asyncContext; private final HttpURI _baseURI; private final ServletRequestState _state; @@ -35,7 +35,7 @@ public class AsyncContextEvent extends AsyncEvent implements Runnable private volatile Scheduler.Task _timeoutTask; private Throwable _throwable; - public AsyncContextEvent(ContextHandler.Context context, AsyncContextState asyncContext, ServletRequestState state, ServletRequest request, ServletResponse response) + public AsyncContextEvent(ContextHandler.ScopedContext context, AsyncContextState asyncContext, ServletRequestState state, ServletRequest request, ServletResponse response) { super(null, request, response, null); _context = context; @@ -76,7 +76,7 @@ public class AsyncContextEvent extends AsyncEvent implements Runnable return _dispatchContext == null ? _servletContext : _dispatchContext; } - public ContextHandler.Context getContext() + public ContextHandler.ScopedContext getContext() { return _context; } diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/BaseHolder.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/BaseHolder.java index 4c37b6971fc..0be9b3dd4c3 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/BaseHolder.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/BaseHolder.java @@ -18,6 +18,7 @@ import java.util.function.BiFunction; import jakarta.servlet.ServletContext; import jakarta.servlet.UnavailableException; +import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.util.Loader; import org.eclipse.jetty.util.annotation.ManagedAttribute; import org.eclipse.jetty.util.component.AbstractLifeCycle; @@ -165,7 +166,7 @@ public abstract class BaseHolder extends AbstractLifeCycle implements Dumpabl if (_servletHandler != null) { ServletContext context = _servletHandler.getServletContext(); - if ((context instanceof org.eclipse.jetty.server.handler.ContextHandler.Context) && ((org.eclipse.jetty.server.handler.ContextHandler.Context)context).getContextHandler().isStarted()) + if ((context instanceof ContextHandler.ScopedContext) && ((ContextHandler.ScopedContext)context).getContextHandler().isStarted()) throw new IllegalStateException("Started"); } } @@ -222,14 +223,14 @@ public abstract class BaseHolder extends AbstractLifeCycle implements Dumpabl public ServletContextHandler getServletContextHandler() { - ServletContext scontext = null; + ServletContext context = null; //try the ServletHandler first if (getServletHandler() != null) - scontext = getServletHandler().getServletContext(); + context = getServletHandler().getServletContext(); - if (scontext instanceof ServletContextHandler.Context) - return ((ServletContextHandler.Context)scontext).getServletContextHandler(); + if (context instanceof ServletContextHandler.ServletContextApi api) + return api.getContext().getServletContextHandler(); //try the ServletContextHandler next return ServletContextHandler.getCurrentServletContextHandler(); diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DebugListener.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DebugListener.java index a0007adceea..7bbac4352c9 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DebugListener.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DebugListener.java @@ -289,7 +289,7 @@ public class DebugListener extends AbstractLifeCycle implements ServletContextLi final ServletContextScopeListener _contextScopeListener = new ServletContextScopeListener() { @Override - public void enterScope(ServletContextHandler.Context context, ServletContextRequest request) + public void enterScope(ServletContextHandler.ServletScopedContext context, ServletContextRequest request) { String cname = findContextName(context.getServletContext()); if (request == null) @@ -309,7 +309,7 @@ public class DebugListener extends AbstractLifeCycle implements ServletContextLi } @Override - public void exitScope(ServletContextHandler.Context context, ServletContextRequest request) + public void exitScope(ServletContextHandler.ServletScopedContext context, ServletContextRequest request) { String cname = findContextName(context.getServletContext()); if (request == null) diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java index 6e15ca6bee7..a899bd183ac 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java @@ -61,6 +61,7 @@ import org.eclipse.jetty.http.ValidatingCachingHttpContentFactory; import org.eclipse.jetty.io.ByteBufferInputStream; import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.io.NoopByteBufferPool; +import org.eclipse.jetty.server.Context; import org.eclipse.jetty.server.HttpStream; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.ResourceService; @@ -337,12 +338,12 @@ public class DefaultServlet extends HttpServlet if (servletContext instanceof ServletContextHandler.ServletContextApi api) return api.getContext().getServletContextHandler(); - ContextHandler.Context context = ContextHandler.getCurrentContext(); - if (context != null) - return context.getContextHandler(); + Context context = ContextHandler.getCurrentContext(); + if (context instanceof ContextHandler.ScopedContext scopedContext) + return scopedContext.getContextHandler(); throw new IllegalArgumentException("The servletContext " + servletContext + " " + - servletContext.getClass().getName() + " is not " + ContextHandler.Context.class.getName()); + servletContext.getClass().getName() + " is not " + ContextHandler.ScopedContext.class.getName()); } protected boolean isPathInfoOnly() diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ErrorHandler.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ErrorHandler.java index 9cd28b5fab0..ea071c55792 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ErrorHandler.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ErrorHandler.java @@ -99,7 +99,7 @@ public class ErrorHandler implements Request.Processor // This logic really should be in ErrorPageErrorHandler, but some implementations extend ErrorHandler // and implement ErrorPageMapper directly, so we do this here in the base class. String errorPage = (this instanceof ErrorPageMapper) ? ((ErrorPageMapper)this).getErrorPage(servletContextRequest.getHttpServletRequest()) : null; - ServletContextHandler.Context context = servletContextRequest.getErrorContext(); + ServletContextHandler.ServletScopedContext context = servletContextRequest.getErrorContext(); Dispatcher errorDispatcher = (errorPage != null && context != null) ? (Dispatcher)context.getServletContext().getRequestDispatcher(errorPage) : null; diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ListenerHolder.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ListenerHolder.java index 7cf5d2cb645..929b613400a 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ListenerHolder.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ListenerHolder.java @@ -16,6 +16,7 @@ package org.eclipse.jetty.ee10.servlet; import java.util.EventListener; import jakarta.servlet.ServletContext; +import org.eclipse.jetty.server.Context; import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.util.thread.AutoLock; @@ -74,13 +75,7 @@ public class ListenerHolder extends BaseHolder throw new IllegalStateException(msg); } - ContextHandler contextHandler = null; - if (getServletHandler() != null) - contextHandler = getServletHandler().getServletContextHandler(); - if (contextHandler == null && ContextHandler.getCurrentContext() != null) - contextHandler = ContextHandler.getCurrentContext().getContextHandler(); - if (contextHandler == null) - throw new IllegalStateException("No Context"); + ContextHandler contextHandler = getContextHandler(); _listener = getInstance(); if (_listener == null) @@ -92,6 +87,21 @@ public class ListenerHolder extends BaseHolder contextHandler.addEventListener(_listener); } + private ContextHandler getContextHandler() + { + Context context = ContextHandler.getCurrentContext(); + if (context instanceof ContextHandler.ScopedContext scopedContext) + return scopedContext.getContextHandler(); + + ContextHandler contextHandler = null; + if (getServletHandler() != null) + contextHandler = getServletHandler().getServletContextHandler(); + if (contextHandler != null) + return contextHandler; + + throw new IllegalStateException("No Context"); + } + @Override protected EventListener createInstance() throws Exception { @@ -116,7 +126,7 @@ public class ListenerHolder extends BaseHolder { try { - ContextHandler contextHandler = ContextHandler.getCurrentContext().getContextHandler(); + ContextHandler contextHandler = getContextHandler(); if (contextHandler != null) contextHandler.removeEventListener(_listener); getServletHandler().destroyListener(unwrap(_listener)); diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java index 2b18ddb7075..6640b6b582c 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java @@ -73,7 +73,7 @@ public class ServletChannel private static final Logger LOG = LoggerFactory.getLogger(ServletChannel.class); private final ServletRequestState _state; - private final ServletContextHandler.Context _context; + private final ServletContextHandler.ServletScopedContext _context; private final ServletContextHandler.ServletContextApi _servletContextApi; private final AtomicLong _requests = new AtomicLong(); private final Connector _connector; @@ -109,6 +109,11 @@ public class ServletChannel _callback = callback; } + public Callback getCallback() + { + return _callback; + } + /** * Associate this channel with a specific request. * @param servletContextRequest The request to associate @@ -128,7 +133,7 @@ public class ServletChannel _state); } - public ServletContextHandler.Context getContext() + public ServletContextHandler.ServletScopedContext getContext() { return _context; } @@ -488,7 +493,7 @@ public class ServletChannel // by then. Response.ensureConsumeAvailableOrNotPersistent(_servletContextRequest, _servletContextRequest.getResponse()); - ContextHandler.Context context = (ContextHandler.Context)_servletContextRequest.getAttribute(ErrorHandler.ERROR_CONTEXT); + ContextHandler.ScopedContext context = (ContextHandler.ScopedContext)_servletContextRequest.getAttribute(ErrorHandler.ERROR_CONTEXT); Request.Processor errorProcessor = ErrorHandler.getErrorProcessor(getServer(), context == null ? null : context.getContextHandler()); // If we can't have a body or have no processor, then create a minimal error response. diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContainerInitializerHolder.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContainerInitializerHolder.java index 5cb2631c806..e94dd2e9c83 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContainerInitializerHolder.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContainerInitializerHolder.java @@ -24,6 +24,7 @@ import java.util.regex.Pattern; import jakarta.servlet.ServletContainerInitializer; import jakarta.servlet.ServletContext; import jakarta.servlet.ServletException; +import org.eclipse.jetty.server.Context; import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.util.Loader; import org.eclipse.jetty.util.NanoTime; @@ -115,7 +116,7 @@ public class ServletContainerInitializerHolder extends BaseHolder _sessionIdListeners = new CopyOnWriteArrayList<>(); private final SessionCookieConfig _cookieConfig = new CookieConfig(); - private ServletContextHandler.Context _servletContextHandlerContext; + private ServletContextHandler.ServletScopedContext _servletContextHandlerContext; private Server _server; private Handler _handler; @@ -312,7 +311,7 @@ public class SessionHandler extends AbstractSessionManager implements Handler.Ne @Override public ServletContext getServletContext() { - return ServletContextHandler.getServletContext((ContextHandler.Context)_session.getSessionManager().getContext()); + return ServletContextHandler.getServletContext((ContextHandler.ScopedContext)_session.getSessionManager().getContext()); } @Override @@ -494,9 +493,9 @@ public class SessionHandler extends AbstractSessionManager implements Handler.Ne public void doStart() throws Exception { super.doStart(); - if (!(getContext() instanceof ServletContextHandler.Context)) + if (!(getContext() instanceof ServletContextHandler.ServletScopedContext)) throw new IllegalStateException("!ServlerContextHandler.Context"); - _servletContextHandlerContext = (ServletContextHandler.Context)getContext(); + _servletContextHandlerContext = (ServletContextHandler.ServletScopedContext)getContext(); configureCookies(); } diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/StatisticsServlet.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/StatisticsServlet.java index 384ad22659b..10ebc602752 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/StatisticsServlet.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/StatisticsServlet.java @@ -74,7 +74,7 @@ public class StatisticsServlet extends HttpServlet public void init() throws ServletException { ServletContext context = getServletContext(); - ContextHandler.Context scontext = ServletContextHandler.getServletContextHandler(context).getContext(); + ContextHandler.ScopedContext scontext = ServletContextHandler.getServletContextHandler(context).getContext(); Server server = scontext.getContextHandler().getServer(); _statsHandler = server.getDescendant(StatisticsHandler.class); diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ComponentWrapTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ComponentWrapTest.java index ac21880bccd..395a83f6ccb 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ComponentWrapTest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ComponentWrapTest.java @@ -17,6 +17,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.EnumSet; import java.util.EventListener; +import java.util.Iterator; import java.util.List; import java.util.concurrent.LinkedBlockingQueue; @@ -48,6 +49,8 @@ import org.slf4j.LoggerFactory; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; public class ComponentWrapTest { @@ -98,12 +101,15 @@ public class ComponentWrapTest @Override public void init(FilterConfig filterConfig) { + events.addEvent("TestFilter.init"); } @Override public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { + events.addEvent("TestFilter.doFilter"); chain.doFilter(request, response); + events.addEvent("TestFilter.filtered"); } @Override @@ -125,16 +131,21 @@ public class ComponentWrapTest List expectedEvents = new ArrayList<>(); expectedEvents.add("TestWrapFilter.init()"); + expectedEvents.add("TestFilter.init"); expectedEvents.add("TestWrapServlet.init()"); expectedEvents.add("TestWrapListener.requestInitialized()"); expectedEvents.add("TestWrapFilter.doFilter()"); + expectedEvents.add("TestFilter.doFilter"); expectedEvents.add("TestWrapServlet.service()"); + expectedEvents.add("TestFilter.filtered"); expectedEvents.add("TestWrapListener.requestDestroyed()"); - List actualEvents = new ArrayList<>(); - actualEvents.addAll(events); - - assertThat("Metrics Events Count", actualEvents.size(), is(expectedEvents.size())); + Iterator i = events.iterator(); + for (String s: expectedEvents) + { + assertEquals(s, i.next()); + } + assertFalse(i.hasNext()); } public static class LoggingRequestListener implements ServletRequestListener diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ServletContextHandlerTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ServletContextHandlerTest.java index d56e67ff43d..6793e7604fd 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ServletContextHandlerTest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ServletContextHandlerTest.java @@ -254,9 +254,9 @@ public class ServletContextHandlerTest public static class MySCIStarter extends AbstractLifeCycle implements ServletContextHandler.ServletContainerInitializerCaller { ServletContainerInitializer _sci; - ServletContextHandler.Context _ctx; + ServletContextHandler.ServletScopedContext _ctx; - MySCIStarter(ServletContextHandler.Context ctx, ServletContainerInitializer sci) + MySCIStarter(ServletContextHandler.ServletScopedContext ctx, ServletContainerInitializer sci) { _ctx = ctx; _sci = sci; diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ContextHandler.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ContextHandler.java index 46f5a7b906d..90700c234b3 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ContextHandler.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ContextHandler.java @@ -1679,7 +1679,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu */ public class APIContext implements ServletContext { - private final org.eclipse.jetty.server.handler.ContextHandler.Context _coreContext; + private final org.eclipse.jetty.server.handler.ContextHandler.ScopedContext _coreContext; protected boolean _enabled = true; // whether or not the dynamic API is enabled for callers protected boolean _extendedListenerTypes = false; private int _effectiveMajorVersion = SERVLET_MAJOR_VERSION; @@ -2347,7 +2347,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu private final HttpChannel _httpChannel; protected CoreContextRequest(org.eclipse.jetty.server.handler.ContextHandler contextHandler, - org.eclipse.jetty.server.handler.ContextHandler.Context context, + org.eclipse.jetty.server.handler.ContextHandler.ScopedContext context, org.eclipse.jetty.server.Request wrapped, HttpChannel httpChannel) { @@ -2456,10 +2456,10 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu } @Override - protected void enterScope(org.eclipse.jetty.server.Request coreRequest) + protected void notifyEnterScope(org.eclipse.jetty.server.Request coreRequest) { __context.set(_apiContext); - super.enterScope(coreRequest); + super.notifyEnterScope(coreRequest); Request request = (coreRequest instanceof CoreContextRequest coreContextRequest) ? coreContextRequest.getHttpChannel().getRequest() : null; @@ -2476,7 +2476,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu } @Override - protected void exitScope(org.eclipse.jetty.server.Request coreRequest) + protected void notifyExitScope(org.eclipse.jetty.server.Request coreRequest) { try { @@ -2484,7 +2484,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu ? coreContextRequest.getHttpChannel().getRequest() : null; ContextHandler.this.exitScope(request); - super.exitScope(coreRequest); + super.notifyExitScope(coreRequest); } finally { @@ -2492,7 +2492,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu } } - class CoreContext extends org.eclipse.jetty.server.handler.ContextHandler.Context + class CoreContext extends ScopedContext { public APIContext getAPIContext() {