Merge pull request #61 from awslabs/spark-upgrade

Spark upgrade
aws · Aug 16, 2017 · b540553 · b540553
2 parents 628ed1b + 4fbe380
commit b540553
Show file tree

Hide file tree

Showing 13 changed files with 168 additions and 68 deletions.
diff --git a/README.md b/README.md
@@ -104,6 +104,16 @@ public class LambdaHandler implements RequestHandler<AwsProxyRequest, AwsProxyRe
 }
 ```
 
+If you configure an [`initExceptionHandler` method](http://sparkjava.com/documentation#stopping-the-server), make sure that you call `System.exit` at the end of the method. This framework keeps a `CountDownLatch` on the request
+and unless you forcefully exit from the thread, the Lambda function will hang waiting for a latch that is never released.
+
+```java
+initExceptionHandler((e) -> {
+    LOG.error("ignite failed", e);
+    System.exit(100);
+});
+```
+
 # Security context
 The `aws-serverless-java-container-core` contains a default implementation of the `SecurityContextWriter` that supports API Gateway's proxy integration. The generated security context uses the API Gateway `$context` object to establish the request security context. The context looks for the following values in order and returns the first matched type:
 

diff --git a/aws-serverless-java-container-core/pom.xml b/aws-serverless-java-container-core/pom.xml
@@ -50,22 +50,6 @@
             <version>1.3.2</version>
         </dependency>
 
-        <!-- https://mvnrepository.com/artifact/junit/junit -->
-        <dependency>
-            <groupId>junit</groupId>
-            <artifactId>junit</artifactId>
-            <version>4.12</version>
-            <scope>test</scope>
-        </dependency>
-
-        <!-- https://mvnrepository.com/artifact/org.mockito/mockito-all -->
-        <dependency>
-            <groupId>org.mockito</groupId>
-            <artifactId>mockito-all</artifactId>
-            <version>1.10.19</version>
-            <scope>test</scope>
-        </dependency>
-
         <!-- https://mvnrepository.com/artifact/org.apache.httpcomponents/httpmime -->
         <dependency>
             <groupId>org.apache.httpcomponents</groupId>

diff --git a/aws-serverless-java-container-jersey/pom.xml b/aws-serverless-java-container-jersey/pom.xml
@@ -33,14 +33,6 @@
             <version>${jersey.version}</version>
         </dependency>
 
-        <!-- https://mvnrepository.com/artifact/junit/junit -->
-        <dependency>
-            <groupId>junit</groupId>
-            <artifactId>junit</artifactId>
-            <version>4.12</version>
-            <scope>test</scope>
-        </dependency>
-
         <!-- https://mvnrepository.com/artifact/commons-codec/commons-codec -->
         <dependency>
             <groupId>commons-codec</groupId>

diff --git a/aws-serverless-java-container-spark/pom.xml b/aws-serverless-java-container-spark/pom.xml
@@ -15,7 +15,7 @@
     </parent>
 
     <properties>
-        <spark.version>2.5.3</spark.version>
+        <spark.version>2.6.0</spark.version>
     </properties>
 
     <dependencies>
@@ -31,21 +31,14 @@
             <groupId>com.sparkjava</groupId>
             <artifactId>spark-core</artifactId>
             <version>${spark.version}</version>
+            <!-- excluding slf4j from spark because it includes 1.7.13, the framework already pulls in 1.7.25 -->
             <exclusions>
                 <exclusion>
                     <groupId>org.slf4j</groupId>
                     <artifactId>slf4j-api</artifactId>
                 </exclusion>
             </exclusions>
         </dependency>
-
-        <!-- https://mvnrepository.com/artifact/junit/junit -->
-        <dependency>
-            <groupId>junit</groupId>
-            <artifactId>junit</artifactId>
-            <version>4.12</version>
-            <scope>test</scope>
-        </dependency>
     </dependencies>
 
 </project>
diff --git a/...spark/src/main/java/com/amazonaws/serverless/proxy/spark/SparkLambdaContainerHandler.java b/...spark/src/main/java/com/amazonaws/serverless/proxy/spark/SparkLambdaContainerHandler.java
@@ -26,6 +26,7 @@
 import org.slf4j.LoggerFactory;
 import spark.Service;
 import spark.Spark;
+import spark.embeddedserver.EmbeddedServerFactory;
 import spark.embeddedserver.EmbeddedServers;
 
 import java.lang.reflect.Field;
@@ -91,7 +92,8 @@ public static SparkLambdaContainerHandler<AwsProxyRequest, AwsProxyResponse> get
         return new SparkLambdaContainerHandler<>(new AwsProxyHttpServletRequestReader(),
                                                  new AwsProxyHttpServletResponseWriter(),
                                                  new AwsProxySecurityContextWriter(),
-                                                 new AwsProxyExceptionHandler());
+                                                 new AwsProxyExceptionHandler(),
+                                                 new LambdaEmbeddedServerFactory());
     }
 
 
@@ -102,31 +104,34 @@ public static SparkLambdaContainerHandler<AwsProxyRequest, AwsProxyResponse> get
     public SparkLambdaContainerHandler(RequestReader<RequestType, AwsProxyHttpServletRequest> requestReader,
                                        ResponseWriter<AwsHttpServletResponse, ResponseType> responseWriter,
                                        SecurityContextWriter<RequestType> securityContextWriter,
-                                       ExceptionHandler<ResponseType> exceptionHandler)
+                                       ExceptionHandler<ResponseType> exceptionHandler,
+                                       EmbeddedServerFactory embeddedServerFactory)
             throws ContainerInitializationException {
         super(requestReader, responseWriter, securityContextWriter, exceptionHandler);
 
-        EmbeddedServers.add(LAMBDA_EMBEDDED_SERVER_CODE, new LambdaEmbeddedServerFactory());
+        EmbeddedServers.add(LAMBDA_EMBEDDED_SERVER_CODE, embeddedServerFactory);
 
         // TODO: This is pretty bad but we are not given access to the embeddedServerIdentifier property of the
         // Service object
         try {
+            log.debug("Changing visibility of getInstance method and embeddedServerIdentifier properties");
             Method serviceInstanceMethod = Spark.class.getDeclaredMethod("getInstance");
             serviceInstanceMethod.setAccessible(true);
             Service sparkService = (Service) serviceInstanceMethod.invoke(null);
             Field serverIdentifierField = Service.class.getDeclaredField("embeddedServerIdentifier");
             serverIdentifierField.setAccessible(true);
             serverIdentifierField.set(sparkService, LAMBDA_EMBEDDED_SERVER_CODE);
-
-            // remove Jetty from embedded servers
-            EmbeddedServers.class.getDeclaredMethod("initialize");
         } catch (NoSuchFieldException e) {
+            log.error("Could not fine embeddedServerIdentifier field in Service class", e);
             throw new ContainerInitializationException("Cannot find embeddedServerIdentifier field in Service class", e);
         } catch (NoSuchMethodException e) {
+            log.error("Could not find getInstance method in Spark class", e);
             throw new ContainerInitializationException("Cannot find getInstance method in Spark class", e);
         } catch (IllegalAccessException e) {
+            log.error("Could not access getInstance method in Spark class", e);
             throw new ContainerInitializationException("Cannot access getInstance method in Spark class", e);
         } catch (InvocationTargetException e) {
+            log.error("Could not invoke getInstance method in Spark class", e);
             throw new ContainerInitializationException("Cannot invoke getInstance method in Spark class", e);
         }
     }
@@ -145,10 +150,12 @@ protected AwsHttpServletResponse getContainerResponse(AwsProxyHttpServletRequest
     @Override
     protected void handleRequest(AwsProxyHttpServletRequest httpServletRequest, AwsHttpServletResponse httpServletResponse, Context lambdaContext)
             throws Exception {
+
         // this method of the AwsLambdaServletContainerHandler sets the request context
         super.handleRequest(httpServletRequest, httpServletResponse, lambdaContext);
 
         if (embeddedServer == null) {
+            log.debug("First request, getting new server instance");
             embeddedServer = LambdaEmbeddedServerFactory.getServerInstance();
 
             // call the onStartup event if set to give developers a chance to set filters in the context

diff --git a/...c/main/java/com/amazonaws/serverless/proxy/spark/embeddedserver/LambdaEmbeddedServer.java b/...c/main/java/com/amazonaws/serverless/proxy/spark/embeddedserver/LambdaEmbeddedServer.java
@@ -15,7 +15,6 @@
 import java.io.IOException;
 import java.util.Map;
 import java.util.Optional;
-import java.util.concurrent.CountDownLatch;
 
 public class LambdaEmbeddedServer
         implements EmbeddedServer {
@@ -45,19 +44,14 @@ public class LambdaEmbeddedServer
     //-------------------------------------------------------------
     // Implementation - EmbeddedServer
     //-------------------------------------------------------------
-
     @Override
-    public int ignite(String host,
-                      int port,
-                      SslStores sslStores,
-                      CountDownLatch countDownLatch,
-                      int maxThreads,
-                      int minThreads,
-                      int threadIdleTimeoutMillis) {
+    public int ignite(String s, int i, SslStores sslStores, int i1, int i2, int i3)
+            throws Exception {
+        log.info("Starting Spark server, ignoring port and host");
         sparkFilter = new MatcherFilter(applicationRoutes, staticFilesConfiguration, false, hasMultipleHandler);
         sparkFilter.init(null);
 
-        countDownLatch.countDown();
+        //countDownLatch.countDown();
 
         return 0;
     }
@@ -71,18 +65,31 @@ public void configureWebSockets(Map<String, WebSocketHandlerWrapper> webSocketHa
     }
 
 
+    @Override
+    public void join()
+            throws InterruptedException {
+        log.info("Called join method, nothing to do here since Lambda only runs a single event per container");
+    }
+
+
     @Override
     public void extinguish() {
+        log.info("Called extinguish method, nothing to do here.");
     }
 
 
+    @Override
+    public int activeThreadCount() {
+        log.debug("Called activeThreadCount, since Lambda only runs one event per container we always return 1");
+        return 1;
+    }
+
     //-------------------------------------------------------------
     // Methods - Public
     //-------------------------------------------------------------
 
     public void handle(HttpServletRequest request, HttpServletResponse response)
             throws IOException, ServletException {
-        //RouteMatch route = applicationRoutes.find(HttpMethod.get(request.requestMethod()), request.contextPath(), "*/*");
         sparkFilter.doFilter(request, response, null);
     }
 }
diff --git a/...java/com/amazonaws/serverless/proxy/spark/embeddedserver/LambdaEmbeddedServerFactory.java b/...java/com/amazonaws/serverless/proxy/spark/embeddedserver/LambdaEmbeddedServerFactory.java
@@ -14,6 +14,21 @@ public class LambdaEmbeddedServerFactory implements EmbeddedServerFactory {
     private static LambdaEmbeddedServer embeddedServer;
 
 
+    /**
+     * Empty constructor, applications should always use this constructor.
+     */
+    public LambdaEmbeddedServerFactory() {}
+
+
+    /**
+     * Constructor used in unit tests to inject a mocked embedded server
+     * @param server The mocked server
+     */
+    public LambdaEmbeddedServerFactory(LambdaEmbeddedServer server) {
+        embeddedServer = server;
+    }
+
+
     //-------------------------------------------------------------
     // Implementation - EmbeddedServerFactory
     //-------------------------------------------------------------

diff --git a/...ntainer-spark/src/test/java/com/amazonaws/serverless/proxy/spark/HelloWorldSparkTest.java b/...ntainer-spark/src/test/java/com/amazonaws/serverless/proxy/spark/HelloWorldSparkTest.java
@@ -7,8 +7,10 @@
 import com.amazonaws.serverless.proxy.internal.testutils.AwsProxyRequestBuilder;
 import com.amazonaws.serverless.proxy.internal.testutils.MockLambdaContext;
 
+import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
+import spark.Spark;
 
 import javax.servlet.http.Cookie;
 import javax.ws.rs.core.HttpHeaders;
@@ -22,10 +24,10 @@ public class HelloWorldSparkTest {
     private static final String CUSTOM_HEADER_VALUE = "My Header Value";
     private static final String BODY_TEXT_RESPONSE = "Hello World";
 
-    public static final String COOKIE_NAME = "MyCookie";
-    public static final String COOKIE_VALUE = "CookieValue";
-    public static final String COOKIE_DOMAIN = "mydomain.com";
-    public static final String COOKIE_PATH = "/";
+    private static final String COOKIE_NAME = "MyCookie";
+    private static final String COOKIE_VALUE = "CookieValue";
+    private static final String COOKIE_DOMAIN = "mydomain.com";
+    private static final String COOKIE_PATH = "/";
 
     private static SparkLambdaContainerHandler<AwsProxyRequest, AwsProxyResponse> handler;
 
@@ -42,6 +44,11 @@ public static void initializeServer() {
         }
     }
 
+    @AfterClass
+    public static void stopSpark() {
+        Spark.stop();
+    }
+
     @Test
     public void basicServer_handleRequest_emptyFilters() {
         AwsProxyRequest req = new AwsProxyRequestBuilder().method("GET").path("/hello").build();

diff --git a/...er-spark/src/test/java/com/amazonaws/serverless/proxy/spark/InitExceptionHandlerTest.java b/...er-spark/src/test/java/com/amazonaws/serverless/proxy/spark/InitExceptionHandlerTest.java
@@ -0,0 +1,75 @@
+package com.amazonaws.serverless.proxy.spark;
+
+
+import com.amazonaws.serverless.exceptions.ContainerInitializationException;
+import com.amazonaws.serverless.proxy.internal.AwsProxyExceptionHandler;
+import com.amazonaws.serverless.proxy.internal.AwsProxySecurityContextWriter;
+import com.amazonaws.serverless.proxy.internal.servlet.AwsProxyHttpServletRequestReader;
+import com.amazonaws.serverless.proxy.internal.servlet.AwsProxyHttpServletResponseWriter;
+import com.amazonaws.serverless.proxy.spark.embeddedserver.LambdaEmbeddedServer;
+import com.amazonaws.serverless.proxy.spark.embeddedserver.LambdaEmbeddedServerFactory;
+
+import org.junit.AfterClass;
+import org.junit.Test;
+import spark.Spark;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+import static org.mockito.Matchers.anyInt;
+import static org.mockito.Matchers.anyObject;
+import static org.mockito.Matchers.anyString;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.reset;
+import static org.mockito.Mockito.when;
+import static spark.Spark.get;
+import static spark.Spark.initExceptionHandler;
+
+
+public class InitExceptionHandlerTest {
+
+    private static final String TEST_EXCEPTION_MESSAGE = "test exception";
+    private static LambdaEmbeddedServer embeddedServer = mock(LambdaEmbeddedServer.class);
+
+    @Test
+    public void initException_mockException_expectHandlerToRun() {
+        try {
+
+            when(embeddedServer.ignite(anyString(), anyInt(), anyObject(), anyInt(), anyInt(), anyInt()))
+                    .thenThrow(new ContainerInitializationException(TEST_EXCEPTION_MESSAGE, null));
+            LambdaEmbeddedServerFactory serverFactory = new LambdaEmbeddedServerFactory(embeddedServer);
+            new SparkLambdaContainerHandler<>(new AwsProxyHttpServletRequestReader(),
+                                                      new AwsProxyHttpServletResponseWriter(),
+                                                      new AwsProxySecurityContextWriter(),
+                                                      new AwsProxyExceptionHandler(),
+                                                      serverFactory);
+
+            configureRoutes();
+
+        } catch (Exception e) {
+            e.printStackTrace();
+            fail("Error while mocking server");
+        }
+
+    }
+
+    @AfterClass
+    public static void stopSpark() {
+        // un-mock the embedded server to avoid blocking other tests
+        reset(embeddedServer);
+        // reset the static variable in the factory so that it will be instantiated again next time
+        new LambdaEmbeddedServerFactory(null);
+        Spark.stop();
+    }
+
+    private static void configureRoutes() {
+        initExceptionHandler((e) -> {
+            System.out.println("Exception Handler called: " + e.getLocalizedMessage());
+            assertEquals(TEST_EXCEPTION_MESSAGE, e.getLocalizedMessage());
+        });
+
+        get("/test-route", (req, res) -> {
+            res.status(200);
+            return "test";
+        });
+    }
+}
diff --git a/...k/src/test/java/com/amazonaws/serverless/proxy/spark/SparkLambdaContainerHandlerTest.java b/...k/src/test/java/com/amazonaws/serverless/proxy/spark/SparkLambdaContainerHandlerTest.java
@@ -8,7 +8,9 @@
 import com.amazonaws.serverless.proxy.internal.testutils.MockLambdaContext;
 import com.amazonaws.serverless.proxy.spark.filter.CustomHeaderFilter;
 
+import org.junit.AfterClass;
 import org.junit.Test;
+import spark.Spark;
 
 import javax.servlet.DispatcherType;
 import javax.servlet.FilterRegistration;
@@ -57,6 +59,11 @@ public void filters_onStartupMethod_executeFilters() {
 
     }
 
+    @AfterClass
+    public static void stopSpark() {
+        Spark.stop();
+    }
+
     private void configureRoutes() {
         get("/header-filter", (req, res) -> {
             res.status(200);

diff --git a/aws-serverless-java-container-spring/pom.xml b/aws-serverless-java-container-spring/pom.xml
@@ -49,14 +49,6 @@
             <scope>test</scope>
         </dependency>
 
-        <!-- https://mvnrepository.com/artifact/junit/junit -->
-        <dependency>
-            <groupId>junit</groupId>
-            <artifactId>junit</artifactId>
-            <version>4.12</version>
-            <scope>test</scope>
-        </dependency>
-
         <dependency>
             <groupId>com.fasterxml.jackson.core</groupId>
             <artifactId>jackson-databind</artifactId>