Added ability to pass ThreadPool to Jetty embedded server via factory (…

…#853) * Added ability to pass thread pool through to jetty sever via embedded server factory useful for overriding default thread pool like io.dropwizard.metrics.InstrumentedQueuedThreadPool. * Forgot to remove temp @ignore for jetty tests during other test development.
perwendel · Oct 24, 2017 · 2eecb4d · 2eecb4d
1 parent bd014ba
commit 2eecb4d
Show file tree

Hide file tree

Showing 8 changed files with 139 additions and 14 deletions.
diff --git a/pom.xml b/pom.xml
@@ -9,7 +9,7 @@
     <groupId>com.sparkjava</groupId>
     <artifactId>spark-core</artifactId>
     <packaging>bundle</packaging>
-    <version>2.6.1-SNAPSHOT</version>
+    <version>2.6.2-SNAPSHOT</version>
     <name>Spark</name>
     <description>A Sinatra inspired java web framework</description>
     <url>http://www.sparkjava.com</url>

diff --git a/src/main/java/spark/embeddedserver/jetty/EmbeddedJettyFactory.java b/src/main/java/spark/embeddedserver/jetty/EmbeddedJettyFactory.java
@@ -16,6 +16,7 @@
  */
 package spark.embeddedserver.jetty;
 
+import org.eclipse.jetty.util.thread.ThreadPool;
 import spark.embeddedserver.EmbeddedServer;
 import spark.embeddedserver.EmbeddedServerFactory;
 import spark.http.matching.MatcherFilter;
@@ -27,9 +28,10 @@
  */
 public class EmbeddedJettyFactory implements EmbeddedServerFactory {
     private final JettyServerFactory serverFactory;
+    private ThreadPool threadPool;
 
     public EmbeddedJettyFactory() {
-        this.serverFactory = JettyServer::create;
+        this.serverFactory = new JettyServer();
     }
 
     public EmbeddedJettyFactory(JettyServerFactory serverFactory) {
@@ -41,7 +43,17 @@ public EmbeddedServer create(Routes routeMatcher, StaticFilesConfiguration stati
         matcherFilter.init(null);
 
         JettyHandler handler = new JettyHandler(matcherFilter);
-        return new EmbeddedJettyServer(serverFactory, handler);
+        return new EmbeddedJettyServer(serverFactory, handler).withThreadPool(threadPool);
     }
 
+    /**
+     * Sets optional thread pool for jetty server.  This is useful for overriding the default thread pool
+     * behaviour for example io.dropwizard.metrics.jetty9.InstrumentedQueuedThreadPool.
+     * @param threadPool thread pool
+     * @return Builder pattern - returns this instance
+     */
+    public EmbeddedJettyFactory withThreadPool(ThreadPool threadPool) {
+        this.threadPool = threadPool;
+        return this;
+    }
 }
diff --git a/src/main/java/spark/embeddedserver/jetty/EmbeddedJettyServer.java b/src/main/java/spark/embeddedserver/jetty/EmbeddedJettyServer.java
@@ -29,6 +29,7 @@
 import org.eclipse.jetty.server.ServerConnector;
 import org.eclipse.jetty.server.handler.HandlerList;
 import org.eclipse.jetty.servlet.ServletContextHandler;
+import org.eclipse.jetty.util.thread.ThreadPool;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -56,6 +57,8 @@ public class EmbeddedJettyServer implements EmbeddedServer {
     private Map<String, WebSocketHandlerWrapper> webSocketHandlers;
     private Optional<Integer> webSocketIdleTimeoutMillis;
 
+    private ThreadPool threadPool = null;
+
     public EmbeddedJettyServer(JettyServerFactory serverFactory, Handler handler) {
         this.serverFactory = serverFactory;
         this.handler = handler;
@@ -92,7 +95,12 @@ public int ignite(String host,
             }
         }
 
-        server = serverFactory.create(maxThreads, minThreads, threadIdleTimeoutMillis);
+        // Create instance of jetty server with either default or supplied queued thread pool
+        if(threadPool == null) {
+            server = serverFactory.create(maxThreads, minThreads, threadIdleTimeoutMillis);
+        } else {
+            server = serverFactory.create(threadPool);
+        }
 
         ServerConnector connector;
 
@@ -174,4 +182,15 @@ public int activeThreadCount() {
         }
         return server.getThreadPool().getThreads() - server.getThreadPool().getIdleThreads();
     }
+
+    /**
+     * Sets optional thread pool for jetty server.  This is useful for overriding the default thread pool
+     * behaviour for example io.dropwizard.metrics.jetty9.InstrumentedQueuedThreadPool.
+     * @param threadPool thread pool
+     * @return Builder pattern - returns this instance
+     */
+    public EmbeddedJettyServer withThreadPool(ThreadPool threadPool) {
+        this.threadPool = threadPool;
+        return this;
+    }
 }
diff --git a/src/main/java/spark/embeddedserver/jetty/JettyServer.java b/src/main/java/spark/embeddedserver/jetty/JettyServer.java
@@ -18,11 +18,12 @@
 
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.util.thread.QueuedThreadPool;
+import org.eclipse.jetty.util.thread.ThreadPool;
 
 /**
  * Creates Jetty Server instances.
  */
-class JettyServer {
+class JettyServer implements JettyServerFactory {
 
     /**
      * Creates a Jetty server.
@@ -32,7 +33,7 @@ class JettyServer {
      * @param threadTimeoutMillis threadTimeoutMillis
      * @return a new jetty server instance
      */
-    public static Server create(int maxThreads, int minThreads, int threadTimeoutMillis) {
+    public Server create(int maxThreads, int minThreads, int threadTimeoutMillis) {
         Server server;
 
         if (maxThreads > 0) {
@@ -48,4 +49,13 @@ public static Server create(int maxThreads, int minThreads, int threadTimeoutMil
         return server;
     }
 
+    /**
+     * Creates a Jetty server with supplied thread pool
+     * @param threadPool thread pool
+     * @return a new jetty server instance
+     */
+    @Override
+    public Server create(ThreadPool threadPool) {
+        return threadPool != null ? new Server(threadPool) : new Server();
+    }
 }
diff --git a/src/main/java/spark/embeddedserver/jetty/JettyServerFactory.java b/src/main/java/spark/embeddedserver/jetty/JettyServerFactory.java
@@ -1,12 +1,12 @@
 package spark.embeddedserver.jetty;
 
 import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.util.thread.ThreadPool;
 
 /**
  * This interface can be implemented to provide custom Jetty server instances
  * with specific settings or features.
  */
-@FunctionalInterface
 public interface JettyServerFactory {
     /**
      * Creates a Jetty server.
@@ -17,4 +17,6 @@ public interface JettyServerFactory {
      * @return a new jetty server instance
      */
     Server create(int maxThreads, int minThreads, int threadTimeoutMillis);
+
+    Server create(ThreadPool threadPool);
 }
diff --git a/src/test/java/spark/embeddedserver/EmbeddedServersTest.java b/src/test/java/spark/embeddedserver/EmbeddedServersTest.java
@@ -4,7 +4,9 @@
 
 import org.eclipse.jetty.server.NCSARequestLog;
 import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.util.thread.ThreadPool;
 import org.junit.AfterClass;
+import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
@@ -52,10 +54,18 @@ public void testAdd_whenConfigureRoutes_createsCustomServer() throws Exception {
         File requestLogDir = temporaryFolder.newFolder();
         File requestLogFile = new File(requestLogDir, "request.log");
         // Register custom server
-        EmbeddedServers.add(EmbeddedServers.Identifiers.JETTY, new EmbeddedJettyFactory((i, j, k) -> {
-            Server server = new Server();
-            server.setRequestLog(new NCSARequestLog(requestLogFile.getAbsolutePath()));
-            return server;
+        EmbeddedServers.add(EmbeddedServers.Identifiers.JETTY, new EmbeddedJettyFactory(new JettyServerFactory() {
+            @Override
+            public Server create(int maxThreads, int minThreads, int threadTimeoutMillis) {
+                Server server = new Server();
+                server.setRequestLog(new NCSARequestLog(requestLogFile.getAbsolutePath()));
+                return server;
+            }
+
+            @Override
+            public Server create(ThreadPool threadPool) {
+                return null;
+            }
         }));
         Spark.get("/", (request, response) -> "OK");
         Spark.awaitInitialization();

diff --git a/src/test/java/spark/embeddedserver/jetty/EmbeddedJettyFactoryTest.java b/src/test/java/spark/embeddedserver/jetty/EmbeddedJettyFactoryTest.java
@@ -0,0 +1,72 @@
+package spark.embeddedserver.jetty;
+
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.util.thread.QueuedThreadPool;
+import org.junit.After;
+import org.junit.Test;
+import spark.embeddedserver.EmbeddedServer;
+import spark.route.Routes;
+import spark.staticfiles.StaticFilesConfiguration;
+
+import static org.mockito.Mockito.*;
+
+public class EmbeddedJettyFactoryTest {
+    private EmbeddedServer embeddedServer;
+
+    @Test
+    public void create() throws Exception {
+        final JettyServerFactory jettyServerFactory = mock(JettyServerFactory.class);
+        final StaticFilesConfiguration staticFilesConfiguration = mock(StaticFilesConfiguration.class);
+        final Routes routes = mock(Routes.class);
+
+        when(jettyServerFactory.create(100,10,10000)).thenReturn(new Server());
+
+        final EmbeddedJettyFactory embeddedJettyFactory = new EmbeddedJettyFactory(jettyServerFactory);
+        embeddedServer = embeddedJettyFactory.create(routes, staticFilesConfiguration, false);
+
+        embeddedServer.ignite("localhost", 8080, null, 100,10,10000);
+
+        verify(jettyServerFactory, times(1)).create(100,10,10000);
+        verifyNoMoreInteractions(jettyServerFactory);
+    }
+
+    @Test
+    public void create_withThreadPool() throws Exception {
+        final QueuedThreadPool threadPool = new QueuedThreadPool(100);
+        final JettyServerFactory jettyServerFactory = mock(JettyServerFactory.class);
+        final StaticFilesConfiguration staticFilesConfiguration = mock(StaticFilesConfiguration.class);
+        final Routes routes = mock(Routes.class);
+
+        when(jettyServerFactory.create(threadPool)).thenReturn(new Server(threadPool));
+
+        final EmbeddedJettyFactory embeddedJettyFactory = new EmbeddedJettyFactory(jettyServerFactory).withThreadPool(threadPool);
+        embeddedServer = embeddedJettyFactory.create(routes, staticFilesConfiguration, false);
+
+        embeddedServer.ignite("localhost", 8080, null, 0,0,0);
+
+        verify(jettyServerFactory, times(1)).create(threadPool);
+        verifyNoMoreInteractions(jettyServerFactory);
+    }
+
+    @Test
+    public void create_withNullThreadPool() throws Exception {
+        final JettyServerFactory jettyServerFactory = mock(JettyServerFactory.class);
+        final StaticFilesConfiguration staticFilesConfiguration = mock(StaticFilesConfiguration.class);
+        final Routes routes = mock(Routes.class);
+
+        when(jettyServerFactory.create(100,10,10000)).thenReturn(new Server());
+
+        final EmbeddedJettyFactory embeddedJettyFactory = new EmbeddedJettyFactory(jettyServerFactory).withThreadPool(null);
+        embeddedServer = embeddedJettyFactory.create(routes, staticFilesConfiguration, false);
+
+        embeddedServer.ignite("localhost", 8080, null, 100,10,10000);
+
+        verify(jettyServerFactory, times(1)).create(100,10,10000);
+        verifyNoMoreInteractions(jettyServerFactory);
+    }
+
+    @After
+    public void tearDown() throws Exception {
+        if(embeddedServer != null) embeddedServer.extinguish();
+    }
+}
diff --git a/src/test/java/spark/embeddedserver/jetty/JettyServerTest.java b/src/test/java/spark/embeddedserver/jetty/JettyServerTest.java
@@ -12,7 +12,7 @@ public class JettyServerTest {
     @Test
     public void testCreateServer_useDefaults() throws Exception {
 
-        Server server = JettyServer.create(0, 0, 0);
+        Server server = new JettyServer().create(0, 0, 0);
 
         QueuedThreadPool threadPool = (QueuedThreadPool) server.getThreadPool();
 
@@ -29,7 +29,7 @@ public void testCreateServer_useDefaults() throws Exception {
     @Test
     public void testCreateServer_whenNonDefaultMaxThreadOnly_thenUseDefaultMinThreadAndTimeout() throws Exception {
 
-        Server server = JettyServer.create(1, 0, 0);
+        Server server = new JettyServer().create(1, 0, 0);
 
         QueuedThreadPool threadPool = (QueuedThreadPool) server.getThreadPool();
 
@@ -42,4 +42,4 @@ public void testCreateServer_whenNonDefaultMaxThreadOnly_thenUseDefaultMinThread
         assertEquals("Server thread pool default idleTimeout should be 60000", 60000, idleTimeout);
 
     }
-}
+}