445374 - Reevaluate org.eclipse.jetty.websocket.jsr356 enablement concepts

+ Since SCI adds filters, but init() isn't run till later, that means
  the context attribute for the WebSocketUpgradeFilter isn't present
  during jsr356 runs.  Added ability for manual filter creation to call
  setToAttribute() as a pre-init step, allowing the init() itself to
  bypass the set to attribute for that specific filter instance.
+ This also means ServletException is now thrown out from the
  various configureContext() static methods.

3 files changed, 68 insertions, 21 deletions

diff --git a/jetty-websocket/javax-websocket-server-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/server/deploy/WebSocketServerContainerInitializer.java b/jetty-websocket/javax-websocket-server-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/server/deploy/WebSocketServerContainerInitializer.java
index 65ae460aed..f3bdee9072 100644
--- a/jetty-websocket/javax-websocket-server-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/server/deploy/WebSocketServerContainerInitializer.java
+++ b/jetty-websocket/javax-websocket-server-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/server/deploy/WebSocketServerContainerInitializer.java
@@ -50,7 +50,7 @@ public class WebSocketServerContainerInitializer implements ServletContainerInit
      * <p>
      * Note: this will add the Upgrade filter to the existing list, with no regard for order.  It will just be tacked onto the end of the list.
      */
-    public static ServerContainer configureContext(ServletContextHandler context)
+    public static ServerContainer configureContext(ServletContextHandler context) throws ServletException
     {
         // Create Filter
         WebSocketUpgradeFilter filter = WebSocketUpgradeFilter.configureContext(context);
@@ -70,7 +70,7 @@ public class WebSocketServerContainerInitializer implements ServletContainerInit
      * <p>
      * This will use Servlet 3.1 techniques on the {@link ServletContext} to add a filter at the start of the filter chain.
      */
-    public static ServerContainer configureContext(ServletContext context, ServletContextHandler jettyContext)
+    public static ServerContainer configureContext(ServletContext context, ServletContextHandler jettyContext) throws ServletException
     {
         // Create Filter
         WebSocketUpgradeFilter filter = WebSocketUpgradeFilter.configureContext(context);
diff --git a/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/browser/JsrBrowserDebugTool.java b/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/browser/JsrBrowserDebugTool.java
index 78c1574ed4..70d2242ea2 100644
--- a/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/browser/JsrBrowserDebugTool.java
+++ b/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/browser/JsrBrowserDebugTool.java
@@ -18,6 +18,7 @@
 
 package org.eclipse.jetty.websocket.jsr356.server.browser;
 
+import javax.servlet.ServletException;
 import javax.websocket.DeploymentException;
 
 import org.eclipse.jetty.server.Server;
@@ -75,7 +76,7 @@ public class JsrBrowserDebugTool
         server.join();
     }
 
-    private void setupServer(int port) throws DeploymentException
+    private void setupServer(int port) throws DeploymentException, ServletException
     {
         server = new Server();
         ServerConnector connector = new ServerConnector(server);
diff --git a/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilter.java b/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilter.java
index 08551886ad..1be0734fa6 100644
--- a/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilter.java
+++ b/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilter.java
@@ -55,13 +55,21 @@ import org.eclipse.jetty.websocket.servlet.WebSocketCreator;
 @ManagedObject("WebSocket Upgrade Filter")
 public class WebSocketUpgradeFilter extends ContainerLifeCycle implements Filter, MappedWebSocketCreator, Dumpable
 {
-    private static final String CONTEXT_ATTRIBUTE_KEY = "contextAttributeKey";
+    public static final String CONTEXT_ATTRIBUTE_KEY = "contextAttributeKey";
     private static final Logger LOG = Log.getLogger(WebSocketUpgradeFilter.class);
 
-    public static WebSocketUpgradeFilter configureContext(ServletContextHandler context)
+    public static WebSocketUpgradeFilter configureContext(ServletContextHandler context) throws ServletException
     {
+        // Prevent double configure
+        WebSocketUpgradeFilter filter = (WebSocketUpgradeFilter)context.getAttribute(WebSocketUpgradeFilter.class.getName());
+        if (filter != null)
+        {
+            return filter;
+        }
+        
         // Dynamically add filter
-        WebSocketUpgradeFilter filter = new WebSocketUpgradeFilter();
+        filter = new WebSocketUpgradeFilter();
+        filter.setToAttribute(context, WebSocketUpgradeFilter.class.getName());
 
         String name = "Jetty_WebSocketUpgradeFilter";
         String pathSpec = "/*";
@@ -77,15 +85,21 @@ public class WebSocketUpgradeFilter extends ContainerLifeCycle implements Filter
             LOG.debug("Adding [{}] {} mapped to {} to {}",name,filter,pathSpec,context);
         }
 
-        // Store reference to the WebSocketUpgradeFilter
-        context.setAttribute(WebSocketUpgradeFilter.class.getName(),filter);
-
         return filter;
     }
 
-    public static WebSocketUpgradeFilter configureContext(ServletContext context)
+    public static WebSocketUpgradeFilter configureContext(ServletContext context) throws ServletException
     {
-        WebSocketUpgradeFilter filter = new WebSocketUpgradeFilter();
+        // Prevent double configure
+        WebSocketUpgradeFilter filter = (WebSocketUpgradeFilter)context.getAttribute(WebSocketUpgradeFilter.class.getName());
+        if (filter != null)
+        {
+            return filter;
+        }
+        
+        // Dynamically add filter
+        filter = new WebSocketUpgradeFilter();
+        filter.setToAttribute(context, WebSocketUpgradeFilter.class.getName());
 
         String name = "Jetty_Dynamic_WebSocketUpgradeFilter";
         String pathSpec = "/*";
@@ -106,8 +120,9 @@ public class WebSocketUpgradeFilter extends ContainerLifeCycle implements Filter
     }
 
     private final WebSocketServerFactory factory;
-    private String fname;
     private final PathMappings<WebSocketCreator> pathmap = new PathMappings<>();
+    private String fname;
+    private boolean alreadySetToAttribute = false;
 
     public WebSocketUpgradeFilter()
     {
@@ -272,15 +287,8 @@ public class WebSocketUpgradeFilter extends ContainerLifeCycle implements Filter
                 // assume default
                 key = WebSocketUpgradeFilter.class.getName();
             }
-
-            if (config.getServletContext().getAttribute(key) != null)
-            {
-                throw new ServletException(WebSocketUpgradeFilter.class.getName() + 
-                        " is defined twice for the same context attribute key '" + key
-                        + "'.  Make sure you have different init-param '" + 
-                        CONTEXT_ATTRIBUTE_KEY + "' values set");
-            }
-            config.getServletContext().setAttribute(key,this);
+            
+            setToAttribute(config.getServletContext(), key);
 
             factory.start();
         }
@@ -289,6 +297,44 @@ public class WebSocketUpgradeFilter extends ContainerLifeCycle implements Filter
             throw new ServletException(x);
         }
     }
+    
+    private void setToAttribute(ServletContextHandler context, String key) throws ServletException
+    {
+        if(alreadySetToAttribute)
+        {
+            return;
+        }
+        
+        if (context.getAttribute(key) != null)
+        {
+            throw new ServletException(WebSocketUpgradeFilter.class.getName() + 
+                    " is defined twice for the same context attribute key '" + key
+                    + "'.  Make sure you have different init-param '" + 
+                    CONTEXT_ATTRIBUTE_KEY + "' values set");
+        }
+        context.setAttribute(key,this);
+
+        alreadySetToAttribute = true;
+    }
+
+    public void setToAttribute(ServletContext context, String key) throws ServletException
+    {
+        if(alreadySetToAttribute)
+        {
+            return;
+        }
+        
+        if (context.getAttribute(key) != null)
+        {
+            throw new ServletException(WebSocketUpgradeFilter.class.getName() + 
+                    " is defined twice for the same context attribute key '" + key
+                    + "'.  Make sure you have different init-param '" + 
+                    CONTEXT_ATTRIBUTE_KEY + "' values set");
+        }
+        context.setAttribute(key,this);
+
+        alreadySetToAttribute = true;
+    }
 
     @Override
     public String toString()