Issue #3167 cleanups from review

Signed-off-by: Greg Wilkins <gregw@webtide.com>

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

@@ -97,6 +97,18 @@ public class ServletContextHandler extends ContextHandler
     public final static int NO_SESSIONS=0;
     public final static int NO_SECURITY=0;
 
+    public static ServletContextHandler getServletContextHandler(ServletContext servletContext, String purpose) throws ServletException
+    {
+        ContextHandler contextHandler = ContextHandler.getContextHandler(servletContext);
+        if (contextHandler == null)
+            throw new ServletException("Not running on Jetty, " + purpose+ " unavailable");
+
+        if (!(contextHandler instanceof ServletContextHandler))
+            throw new ServletException("Not running in Jetty ServletContextHandler, " + purpose + " unavailable");
+
+        return (ServletContextHandler)contextHandler;
+    }
+
     public interface ServletContainerInitializerCaller extends LifeCycle {};
 
     protected final DecoratedObjectFactory _objFactory;

jetty-websocket/javax-websocket-server/src/main/java/org/eclipse/jetty/websocket/javax/server/JavaxWebSocketServerContainer.java

@@ -24,8 +24,6 @@ import java.util.List;
 import java.util.concurrent.Executor;
 
 import javax.servlet.ServletContext;
-import javax.servlet.ServletContextEvent;
-import javax.servlet.ServletContextListener;
 import javax.servlet.ServletException;
 import javax.websocket.DeploymentException;
 import javax.websocket.EndpointConfig;
@@ -80,12 +78,13 @@ public class JavaxWebSocketServerContainer
         if (!(handler instanceof ServletContextHandler))
             return null;
 
-        return (javax.websocket.WebSocketContainer)handler.getServletContext().getAttribute(ServerContainer.class.getName());
+        return (javax.websocket.WebSocketContainer)handler.getServletContext().getAttribute("javax.websocket.server.ServerContainer");
     }
 
     public static JavaxWebSocketServerContainer ensureContainer(ServletContext servletContext) throws ServletException
     {
-        ContextHandler contextHandler = ContextHandler.getContextHandler(servletContext);
+        ContextHandler contextHandler = ServletContextHandler.getServletContextHandler(servletContext, "Javax Websocket");
+
         JavaxWebSocketServerContainer container = contextHandler.getBean(JavaxWebSocketServerContainer.class);
         if (container==null)
         {
@@ -116,7 +115,7 @@ public class JavaxWebSocketServerContainer
         return container;
     }
 
-    private final WebSocketMapping _webSocketMapping;
+    private final WebSocketMapping webSocketMapping;
     private final JavaxWebSocketServerFrameHandlerFactory frameHandlerFactory;
     private final Executor executor;
     private final FrameHandler.ConfigurationCustomizer customizer = new FrameHandler.ConfigurationCustomizer();
@@ -138,7 +137,7 @@ public class JavaxWebSocketServerContainer
                 client.getHttpClient().setExecutor(executor);
             return client;
         });
-        this._webSocketMapping = webSocketMapping;
+        this.webSocketMapping = webSocketMapping;
         this.executor = executor;
         this.frameHandlerFactory = new JavaxWebSocketServerFrameHandlerFactory(this);
     }
@@ -148,7 +147,7 @@ public class JavaxWebSocketServerContainer
     {
         ContextHandler contextHandler = (ContextHandler) context;
         JavaxWebSocketServerContainer container = contextHandler.getBean(JavaxWebSocketServerContainer.class);
-        if (container==null)
+        if (container==this)
         {
             contextHandler.removeBean(container);
             LifeCycle.stop(container);
@@ -159,7 +158,7 @@ public class JavaxWebSocketServerContainer
     @Override
     public ByteBufferPool getBufferPool()
     {
-        return this._webSocketMapping.getBufferPool();
+        return this.webSocketMapping.getBufferPool();
     }
 
     @Override
@@ -171,7 +170,7 @@ public class JavaxWebSocketServerContainer
     @Override
     public WebSocketExtensionRegistry getExtensionRegistry()
     {
-        return this._webSocketMapping.getExtensionRegistry();
+        return this.webSocketMapping.getExtensionRegistry();
     }
 
     @Override
@@ -183,7 +182,7 @@ public class JavaxWebSocketServerContainer
     @Override
     public DecoratedObjectFactory getObjectFactory()
     {
-        return this._webSocketMapping.getObjectFactory();
+        return this.webSocketMapping.getObjectFactory();
     }
 
     @Override
@@ -269,10 +268,11 @@ public class JavaxWebSocketServerContainer
     {
         frameHandlerFactory.getMetadata(config.getEndpointClass(), config);
 
-        JavaxWebSocketCreator creator = new JavaxWebSocketCreator(this, config, this._webSocketMapping
+        JavaxWebSocketCreator creator = new JavaxWebSocketCreator(this, config, this.webSocketMapping
             .getExtensionRegistry());
 
-        this._webSocketMapping.addMapping(new UriTemplatePathSpec(config.getPath()), creator, frameHandlerFactory, customizer);
+        this.webSocketMapping
+            .addMapping(new UriTemplatePathSpec(config.getPath()), creator, frameHandlerFactory, customizer);
     }
 
     @Override

jetty-websocket/jetty-websocket-common/src/main/java/org/eclipse/jetty/websocket/common/JettyWebSocketFrameHandlerFactory.java

@@ -18,6 +18,23 @@
 
 package org.eclipse.jetty.websocket.common;
 
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.Reader;
+import java.lang.annotation.Annotation;
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
+import java.nio.ByteBuffer;
+import java.time.Duration;
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.Executor;
+
 import org.eclipse.jetty.util.component.ContainerLifeCycle;
 import org.eclipse.jetty.websocket.api.InvalidWebSocketException;
 import org.eclipse.jetty.websocket.api.Session;
@@ -46,24 +63,21 @@ import org.eclipse.jetty.websocket.common.message.StringMessageSink;
 import org.eclipse.jetty.websocket.common.util.ReflectUtils;
 import org.eclipse.jetty.websocket.core.Frame;
 
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.Reader;
-import java.lang.annotation.Annotation;
-import java.lang.invoke.MethodHandle;
-import java.lang.invoke.MethodHandles;
-import java.lang.reflect.Constructor;
-import java.lang.reflect.InvocationTargetException;
-import java.lang.reflect.Method;
-import java.lang.reflect.Modifier;
-import java.nio.ByteBuffer;
-import java.time.Duration;
-import java.util.Map;
-import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.Executor;
-
 /**
+ * Factory to create {@link JettyWebSocketFrameHandler} instances suitable for
+ * use with jetty-native websocket API.
+ * <p>
+ * Will create a {@link FrameHandler} suitable for use with classes/objects that:
+ * </p>
+ * <ul>
+ * <li>Is &#64;{@link org.eclipse.jetty.websocket.api.annotations.WebSocket} annotated</li>
+ * <li>Extends {@link org.eclipse.jetty.websocket.api.WebSocketAdapter}</li>
+ * <li>Implements {@link org.eclipse.jetty.websocket.api.WebSocketListener}</li>
+ * <li>Implements {@link org.eclipse.jetty.websocket.api.WebSocketConnectionListener}</li>
+ * <li>Implements {@link org.eclipse.jetty.websocket.api.WebSocketPartialListener}</li>
+ * <li>Implements {@link org.eclipse.jetty.websocket.api.WebSocketPingPongListener}</li>
+ * <li>Implements {@link org.eclipse.jetty.websocket.api.WebSocketFrameListener}</li>
+ * </ul>
  */
 public class JettyWebSocketFrameHandlerFactory extends ContainerLifeCycle
 {

jetty-websocket/jetty-websocket-server/src/main/java/org/eclipse/jetty/websocket/server/JettyServerFrameHandlerFactory.java

@@ -18,7 +18,14 @@
 
 package org.eclipse.jetty.websocket.server;
 
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.Executor;
+
+import javax.servlet.ServletContext;
+import javax.servlet.ServletException;
+
 import org.eclipse.jetty.server.handler.ContextHandler;
+import org.eclipse.jetty.servlet.ServletContextHandler;
 import org.eclipse.jetty.util.component.LifeCycle;
 import org.eclipse.jetty.websocket.common.JettyWebSocketFrameHandlerFactory;
 import org.eclipse.jetty.websocket.core.FrameHandler;
@@ -28,18 +35,15 @@ import org.eclipse.jetty.websocket.servlet.FrameHandlerFactory;
 import org.eclipse.jetty.websocket.servlet.ServletUpgradeRequest;
 import org.eclipse.jetty.websocket.servlet.ServletUpgradeResponse;
 
-import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.Executor;
-
-import javax.servlet.ServletContext;
-
 public class JettyServerFrameHandlerFactory
     extends JettyWebSocketFrameHandlerFactory
     implements FrameHandlerFactory, LifeCycle.Listener
 {
     public static JettyServerFrameHandlerFactory ensureFactory(ServletContext servletContext)
+        throws ServletException
     {
-        ContextHandler contextHandler = ContextHandler.getContextHandler(servletContext);
+        ContextHandler contextHandler = ServletContextHandler.getServletContextHandler(servletContext, "Jetty Websocket");
+
         JettyServerFrameHandlerFactory factory = contextHandler.getBean(JettyServerFrameHandlerFactory.class);
         if (factory == null)
         {

jetty-websocket/websocket-servlet/src/main/java/org/eclipse/jetty/websocket/servlet/WebSocketMapping.java

@@ -18,14 +18,19 @@
 
 package org.eclipse.jetty.websocket.servlet;
 
+import static javax.servlet.http.HttpServletResponse.SC_SERVICE_UNAVAILABLE;
+
 import java.io.IOException;
 import java.net.URISyntaxException;
-import java.util.EnumSet;
 import java.util.HashSet;
-import java.util.Map;
 import java.util.Set;
 import java.util.function.Consumer;
 
+import javax.servlet.ServletContext;
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
 import org.eclipse.jetty.http.pathmap.MappedResource;
 import org.eclipse.jetty.http.pathmap.PathMappings;
 import org.eclipse.jetty.http.pathmap.PathSpec;
@@ -36,9 +41,6 @@ import org.eclipse.jetty.io.ByteBufferPool;
 import org.eclipse.jetty.io.MappedByteBufferPool;
 import org.eclipse.jetty.io.RuntimeIOException;
 import org.eclipse.jetty.server.handler.ContextHandler;
-import org.eclipse.jetty.servlet.FilterHolder;
-import org.eclipse.jetty.servlet.ServletContextHandler;
-import org.eclipse.jetty.servlet.ServletHandler;
 import org.eclipse.jetty.util.DecoratedObjectFactory;
 import org.eclipse.jetty.util.component.Dumpable;
 import org.eclipse.jetty.util.component.LifeCycle;
@@ -50,18 +52,6 @@ import org.eclipse.jetty.websocket.core.server.Handshaker;
 import org.eclipse.jetty.websocket.core.server.Negotiation;
 import org.eclipse.jetty.websocket.core.server.WebSocketNegotiator;
 
-import static javax.servlet.http.HttpServletResponse.SC_SERVICE_UNAVAILABLE;
-
-import javax.servlet.DispatcherType;
-import javax.servlet.ServletContext;
-import javax.servlet.ServletContextEvent;
-import javax.servlet.ServletContextListener;
-import javax.servlet.ServletException;
-import javax.servlet.ServletRequest;
-import javax.servlet.ServletResponse;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
-
 /**
  * Mapping of pathSpec to a tupple of {@link WebSocketCreator}, {@link FrameHandlerFactory} and
  * {@link org.eclipse.jetty.websocket.core.FrameHandler.Customizer}.
@@ -119,8 +109,11 @@ public class WebSocketMapping implements Dumpable, LifeCycle.Listener
     {
         ContextHandler contextHandler = (ContextHandler) context;
         WebSocketMapping mapping = contextHandler.getBean(WebSocketMapping.class);
-        if (mapping == null)
+        if (mapping == this)
+        {
             contextHandler.removeBean(mapping);
+            mappings.reset();
+        }
     }
 
     /**