Merge pull request #838 from joakime/issue-698

Issue #698 - Upgrade to Jetty 9.4.4

pom.xml

@@ -30,7 +30,7 @@
 
     <properties>
         <java.version>1.8</java.version>
-        <jetty.version>9.3.6.v20151106</jetty.version>
+        <jetty.version>9.4.4.v20170414</jetty.version>
         <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
         <powermock.version>1.6.4</powermock.version>
         <mockito.version>1.10.19</mockito.version>

src/main/java/spark/embeddedserver/jetty/websocket/WebSocketServletContextHandlerFactory.java

@@ -19,9 +19,10 @@
 import java.util.Map;
 import java.util.Optional;
 
+import org.eclipse.jetty.http.pathmap.ServletPathSpec;
 import org.eclipse.jetty.servlet.ServletContextHandler;
+import org.eclipse.jetty.websocket.server.NativeWebSocketConfiguration;
 import org.eclipse.jetty.websocket.server.WebSocketUpgradeFilter;
-import org.eclipse.jetty.websocket.server.pathmap.ServletPathSpec;
 import org.eclipse.jetty.websocket.servlet.WebSocketCreator;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -50,9 +51,14 @@ public static ServletContextHandler create(Map<String, WebSocketHandlerWrapper>
                 if (webSocketIdleTimeoutMillis.isPresent()) {
                     webSocketUpgradeFilter.getFactory().getPolicy().setIdleTimeout(webSocketIdleTimeoutMillis.get());
                 }
+                // Since we are configuring WebSockets before the ServletContextHandler and WebSocketUpgradeFilter is
+                // even initialized / started, then we have to pre-populate the configuration that will eventually
+                // be used by Jetty's WebSocketUpgradeFilter.
+                NativeWebSocketConfiguration webSocketConfiguration = (NativeWebSocketConfiguration) webSocketServletContextHandler
+                    .getServletContext().getAttribute(NativeWebSocketConfiguration.class.getName());
                 for (String path : webSocketHandlers.keySet()) {
                     WebSocketCreator webSocketCreator = WebSocketCreatorFactory.create(webSocketHandlers.get(path));
-                    webSocketUpgradeFilter.addMapping(new ServletPathSpec(path), webSocketCreator);
+                    webSocketConfiguration.addMapping(new ServletPathSpec(path), webSocketCreator);
                 }
             } catch (Exception ex) {
                 logger.error("creation of websocket context handler failed.", ex);

src/test/java/spark/embeddedserver/jetty/websocket/WebSocketServletContextHandlerFactoryTest.java

@@ -1,23 +1,28 @@
 package spark.embeddedserver.jetty.websocket;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+
+import javax.servlet.ServletContext;
+
+import org.eclipse.jetty.http.pathmap.MappedResource;
+import org.eclipse.jetty.http.pathmap.PathSpec;
 import org.eclipse.jetty.servlet.ServletContextHandler;
+import org.eclipse.jetty.websocket.server.NativeWebSocketConfiguration;
 import org.eclipse.jetty.websocket.server.WebSocketServerFactory;
 import org.eclipse.jetty.websocket.server.WebSocketUpgradeFilter;
-import org.eclipse.jetty.websocket.server.pathmap.PathMappings;
-import org.eclipse.jetty.websocket.server.pathmap.PathSpec;
 import org.eclipse.jetty.websocket.servlet.WebSocketCreator;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.powermock.api.mockito.PowerMockito;
 import org.powermock.core.classloader.annotations.PrepareForTest;
 import org.powermock.modules.junit4.PowerMockRunner;
 
-import java.util.HashMap;
-import java.util.Map;
-import java.util.Optional;
-
-import static org.junit.Assert.*;
-
 @RunWith(PowerMockRunner.class)
 public class WebSocketServletContextHandlerFactoryTest {
 
@@ -41,22 +46,30 @@ public void testCreate_whenNoIdleTimeoutIsPresent() throws Exception {
         webSocketHandlers.put(webSocketPath, new WebSocketHandlerClassWrapper(WebSocketTestHandler.class));
 
         servletContextHandler = WebSocketServletContextHandlerFactory.create(webSocketHandlers, Optional.empty());
-
+
+        ServletContext servletContext = servletContextHandler.getServletContext();
+
         WebSocketUpgradeFilter webSocketUpgradeFilter =
-                (WebSocketUpgradeFilter) servletContextHandler.getAttribute("org.eclipse.jetty.websocket.server.WebSocketUpgradeFilter");
+            (WebSocketUpgradeFilter) servletContext.getAttribute("org.eclipse.jetty.websocket.server.WebSocketUpgradeFilter");
 
         assertNotNull("Should return a WebSocketUpgradeFilter because we configured it to have one", webSocketUpgradeFilter);
+
+        NativeWebSocketConfiguration webSocketConfiguration =
+            (NativeWebSocketConfiguration) servletContext.getAttribute(NativeWebSocketConfiguration.class.getName());
+
+        MappedResource<WebSocketCreator> mappedResource = webSocketConfiguration.getMatch("/websocket");
+        PathSpec pathSpec = mappedResource.getPathSpec();
 
-        PathMappings.MappedResource<WebSocketCreator> mappedResource = webSocketUpgradeFilter.getMappings().getMatch("/websocket");
-        WebSocketCreatorFactory.SparkWebSocketCreator sc = (WebSocketCreatorFactory.SparkWebSocketCreator) mappedResource.getResource();
-        PathSpec pathSpec = (PathSpec) mappedResource.getPathSpec();
-
-        assertEquals("Should return the WebSocket path specified when contexst handler was created",
-                webSocketPath, pathSpec.getPathSpec());
-
-        assertTrue("Should return true because handler should be an instance of the one we passed when it was created",
-                sc.getHandler() instanceof WebSocketTestHandler);
-
+        assertEquals("Should return the WebSocket path specified when context handler was created",
+                webSocketPath, pathSpec.getDeclaration());
+
+        // Because spark works on a non-initialized / non-started ServletContextHandler and WebSocketUpgradeFilter
+        // the stored WebSocketCreator is wrapped for persistence through the start/stop of those contexts.
+        // You cannot unwrap or cast to that WebSocketTestHandler this way.
+        // Only websockets that are added during a live context can be cast this way.
+        // WebSocketCreator sc = mappedResource.getResource();
+        // assertTrue("Should return true because handler should be an instance of the one we passed when it was created",
+        //        sc.getHandler() instanceof WebSocketTestHandler);
     }
 
     @Test
@@ -69,25 +82,34 @@ public void testCreate_whenTimeoutIsPresent() throws Exception {
         webSocketHandlers.put(webSocketPath, new WebSocketHandlerClassWrapper(WebSocketTestHandler.class));
 
         servletContextHandler = WebSocketServletContextHandlerFactory.create(webSocketHandlers, Optional.of(timeout));
+
+        ServletContext servletContext = servletContextHandler.getServletContext();
 
         WebSocketUpgradeFilter webSocketUpgradeFilter =
-                (WebSocketUpgradeFilter) servletContextHandler.getAttribute("org.eclipse.jetty.websocket.server.WebSocketUpgradeFilter");
+                (WebSocketUpgradeFilter) servletContext.getAttribute("org.eclipse.jetty.websocket.server.WebSocketUpgradeFilter");
 
         assertNotNull("Should return a WebSocketUpgradeFilter because we configured it to have one", webSocketUpgradeFilter);
+
+        NativeWebSocketConfiguration webSocketConfiguration =
+            (NativeWebSocketConfiguration) servletContext.getAttribute(NativeWebSocketConfiguration.class.getName());
 
-        WebSocketServerFactory webSocketServerFactory = webSocketUpgradeFilter.getFactory();
+        WebSocketServerFactory webSocketServerFactory = webSocketConfiguration.getFactory();
         assertEquals("Timeout value should be the same as the timeout specified when context handler was created",
                 timeout.longValue(), webSocketServerFactory.getPolicy().getIdleTimeout());
 
-        PathMappings.MappedResource<WebSocketCreator> mappedResource = webSocketUpgradeFilter.getMappings().getMatch("/websocket");
-        WebSocketCreatorFactory.SparkWebSocketCreator sc = (WebSocketCreatorFactory.SparkWebSocketCreator) mappedResource.getResource();
-        PathSpec pathSpec = (PathSpec) mappedResource.getPathSpec();
+        MappedResource<WebSocketCreator> mappedResource = webSocketConfiguration.getMatch("/websocket");
+        PathSpec pathSpec = mappedResource.getPathSpec();
 
         assertEquals("Should return the WebSocket path specified when context handler was created",
-                webSocketPath, pathSpec.getPathSpec());
-
-        assertTrue("Should return true because handler should be an instance of the one we passed when it was created",
-                sc.getHandler() instanceof WebSocketTestHandler);
+                webSocketPath, pathSpec.getDeclaration());
+
+        // Because spark works on a non-initialized / non-started ServletContextHandler and WebSocketUpgradeFilter
+        // the stored WebSocketCreator is wrapped for persistence through the start/stop of those contexts.
+        // You cannot unwrap or cast to that WebSocketTestHandler this way.
+        // Only websockets that are added during a live context can be cast this way.
+        // WebSocketCreator sc = mappedResource.getResource();
+        // assertTrue("Should return true because handler should be an instance of the one we passed when it was created",
+        //        sc.getHandler() instanceof WebSocketTestHandler);
     }
 
     @Test
@@ -105,4 +127,4 @@ public void testCreate_whenWebSocketContextHandlerCreationFails_thenThrowExcepti
         assertNull("Should return null because Websocket context handler was not created", servletContextHandler);
 
     }
-}
+}