From 45ff70a818fd7114102fd41015e1a064db78cb80 Mon Sep 17 00:00:00 2001
From: James Bloom <733179+jamesdbloom@users.noreply.github.com>
Date: Mon, 15 Jul 2019 09:37:31 +0100
Subject: [PATCH] #649 fixed multi-threaded handling of javascript templates

---
 .../javascript/JavaScriptTemplateEngine.java  |  13 +-
 .../JavaScriptTemplateEngineTest.java         | 346 +++++++++++-------
 2 files changed, 228 insertions(+), 131 deletions(-)

diff --git a/mockserver-core/src/main/java/org/mockserver/templates/engine/javascript/JavaScriptTemplateEngine.java b/mockserver-core/src/main/java/org/mockserver/templates/engine/javascript/JavaScriptTemplateEngine.java
index ef6337cdf..10d1cc1d9 100644
--- a/mockserver-core/src/main/java/org/mockserver/templates/engine/javascript/JavaScriptTemplateEngine.java
+++ b/mockserver-core/src/main/java/org/mockserver/templates/engine/javascript/JavaScriptTemplateEngine.java
@@ -1,15 +1,13 @@
 package org.mockserver.templates.engine.javascript;
 
-import org.mockserver.serialization.model.DTO;
 import org.mockserver.logging.MockServerLogger;
 import org.mockserver.model.HttpRequest;
+import org.mockserver.serialization.model.DTO;
 import org.mockserver.templates.engine.TemplateEngine;
 import org.mockserver.templates.engine.model.HttpRequestTemplateObject;
 import org.mockserver.templates.engine.serializer.HttpTemplateOutputDeserializer;
 
-import javax.script.Invocable;
-import javax.script.ScriptEngine;
-import javax.script.ScriptEngineManager;
+import javax.script.*;
 
 import static org.mockserver.formatting.StringFormatter.formatLogMessage;
 import static org.mockserver.formatting.StringFormatter.indentAndToString;
@@ -20,7 +18,7 @@
  */
 public class JavaScriptTemplateEngine implements TemplateEngine {
 
-    private static ScriptEngine engine = new ScriptEngineManager().getEngineByName("nashorn");
+    private static ScriptEngine engine;
     private final MockServerLogger logFormatter;
     private HttpTemplateOutputDeserializer httpTemplateOutputDeserializer;
 
@@ -38,7 +36,10 @@ public <T> T executeTemplate(String template, HttpRequest request, Class<? exten
         String script = "function handle(request) {" + indentAndToString(template)[0] + "}";
         try {
             if (engine != null) {
-                engine.eval(script + " function serialise(request) { return JSON.stringify(handle(JSON.parse(request)), null, 2); }");
+                SimpleScriptContext context = new SimpleScriptContext();
+                context.setBindings(engine.getBindings(ScriptContext.GLOBAL_SCOPE), ScriptContext.GLOBAL_SCOPE);
+                context.setBindings(engine.getBindings(ScriptContext.ENGINE_SCOPE), ScriptContext.ENGINE_SCOPE);
+                engine.eval(script + " function serialise(request) { return JSON.stringify(handle(JSON.parse(request)), null, 2); }", context);
                 // HttpResponse handle(HttpRequest httpRequest) - ES5
                 Object stringifiedResponse = ((Invocable) engine).invokeFunction("serialise", new HttpRequestTemplateObject(request));
                 logFormatter.info(TEMPLATE_GENERATED, request, "generated output:{}from template:{}for request:{}", stringifiedResponse, script, request);
diff --git a/mockserver-core/src/test/java/org/mockserver/templates/engine/javascript/JavaScriptTemplateEngineTest.java b/mockserver-core/src/test/java/org/mockserver/templates/engine/javascript/JavaScriptTemplateEngineTest.java
index 8f87f11f0..696987f2a 100644
--- a/mockserver-core/src/test/java/org/mockserver/templates/engine/javascript/JavaScriptTemplateEngineTest.java
+++ b/mockserver-core/src/test/java/org/mockserver/templates/engine/javascript/JavaScriptTemplateEngineTest.java
@@ -5,11 +5,11 @@
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 import org.mockito.Mock;
-import org.mockserver.serialization.model.HttpRequestDTO;
-import org.mockserver.serialization.model.HttpResponseDTO;
 import org.mockserver.logging.MockServerLogger;
 import org.mockserver.model.HttpRequest;
 import org.mockserver.model.HttpResponse;
+import org.mockserver.serialization.model.HttpRequestDTO;
+import org.mockserver.serialization.model.HttpResponseDTO;
 
 import javax.script.ScriptEngineManager;
 import javax.script.ScriptException;
@@ -19,7 +19,6 @@
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.core.Is.is;
 import static org.hamcrest.core.Is.isA;
-import static org.mockito.Mockito.verify;
 import static org.mockito.MockitoAnnotations.initMocks;
 import static org.mockserver.character.Character.NEW_LINE;
 import static org.mockserver.model.HttpRequest.request;
@@ -44,32 +43,33 @@ public void setupTestFixture() {
     @Test
     public void shouldHandleHttpRequestsWithJavaScriptTemplateFirstExample() {
         // given
-        String template = "if (request.method === 'POST' && request.path === '/somePath') {" + NEW_LINE +
-                "    return {" + NEW_LINE +
-                "        'statusCode': 200," + NEW_LINE +
-                "        'body': JSON.stringify({name: 'value'})" + NEW_LINE +
-                "    };" + NEW_LINE +
-                "} else {" + NEW_LINE +
-                "    return {" + NEW_LINE +
-                "        'statusCode': 406," + NEW_LINE +
-                "        'body': request.body" + NEW_LINE +
-                "    };" + NEW_LINE +
-                "}";
+        String template = "" +
+            "if (request.method === 'POST' && request.path === '/somePath') {" + NEW_LINE +
+            "    return {" + NEW_LINE +
+            "        'statusCode': 200," + NEW_LINE +
+            "        'body': JSON.stringify({name: 'value'})" + NEW_LINE +
+            "    };" + NEW_LINE +
+            "} else {" + NEW_LINE +
+            "    return {" + NEW_LINE +
+            "        'statusCode': 406," + NEW_LINE +
+            "        'body': request.body" + NEW_LINE +
+            "    };" + NEW_LINE +
+            "}";
 
         // when
         HttpResponse actualHttpResponse = new JavaScriptTemplateEngine(logFormatter).executeTemplate(template, request()
-                        .withPath("/somePath")
-                        .withMethod("POST")
-                        .withBody("some_body"),
-                HttpResponseDTO.class
+                .withPath("/somePath")
+                .withMethod("POST")
+                .withBody("some_body"),
+            HttpResponseDTO.class
         );
 
         // then
         if (new ScriptEngineManager().getEngineByName("nashorn") != null) {
             assertThat(actualHttpResponse, is(
-                    response()
-                            .withStatusCode(200)
-                            .withBody("{\"name\":\"value\"}")
+                response()
+                    .withStatusCode(200)
+                    .withBody("{\"name\":\"value\"}")
             ));
         } else {
             assertThat(actualHttpResponse, nullValue());
@@ -77,33 +77,127 @@ public void shouldHandleHttpRequestsWithJavaScriptTemplateFirstExample() {
     }
 
     @Test
-    public void shouldHandleHttpRequestsWithJavaScriptTemplateSecondExample() {
+    public void shouldHandleHttpRequestsWithSlowJavaScriptTemplate() {
         // given
-        String template = "if (request.method === 'POST' && request.path === '/somePath') {" + NEW_LINE +
-                "    return {" + NEW_LINE +
-                "        'statusCode': 200," + NEW_LINE +
-                "        'body': JSON.stringify({name: 'value'})" + NEW_LINE +
-                "    };" + NEW_LINE +
-                "} else {" + NEW_LINE +
-                "    return {" + NEW_LINE +
-                "        'statusCode': 406," + NEW_LINE +
-                "        'body': request.body" + NEW_LINE +
-                "    };" + NEW_LINE +
-                "}";
+        String template = "" +
+            "for (var i = 0; i < 1000000000; i++) {" + NEW_LINE +
+            "  i * i;" + NEW_LINE +
+            "}" + NEW_LINE +
+            "if (request.method === 'POST' && request.path === '/somePath') {" + NEW_LINE +
+            "    return {" + NEW_LINE +
+            "        'statusCode': 200," + NEW_LINE +
+            "        'body': JSON.stringify({name: 'value'})" + NEW_LINE +
+            "    };" + NEW_LINE +
+            "} else {" + NEW_LINE +
+            "    return {" + NEW_LINE +
+            "        'statusCode': 406," + NEW_LINE +
+            "        'body': request.body" + NEW_LINE +
+            "    };" + NEW_LINE +
+            "}";
 
         // when
         HttpResponse actualHttpResponse = new JavaScriptTemplateEngine(logFormatter).executeTemplate(template, request()
-                        .withPath("/someOtherPath")
-                        .withBody("some_body"),
+                .withPath("/somePath")
+                .withMethod("POST")
+                .withBody("some_body"),
+            HttpResponseDTO.class
+        );
+
+        // then
+        if (new ScriptEngineManager().getEngineByName("nashorn") != null) {
+            assertThat(actualHttpResponse, is(
+                response()
+                    .withStatusCode(200)
+                    .withBody("{\"name\":\"value\"}")
+            ));
+        } else {
+            assertThat(actualHttpResponse, nullValue());
+        }
+    }
+
+    @Test
+    public void shouldHandleMultipleHttpRequestsInParallel() throws InterruptedException {
+        // given
+        final String template = "" +
+            "for (var i = 0; i < 1000000000; i++) {" + NEW_LINE +
+            "  i * i;" + NEW_LINE +
+            "}" + NEW_LINE +
+            "if (request.method === 'POST' && request.path === '/somePath') {" + NEW_LINE +
+            "    return {" + NEW_LINE +
+            "        'statusCode': 200," + NEW_LINE +
+            "        'body': JSON.stringify({name: 'value'})" + NEW_LINE +
+            "    };" + NEW_LINE +
+            "} else {" + NEW_LINE +
+            "    return {" + NEW_LINE +
+            "        'statusCode': 406," + NEW_LINE +
+            "        'body': request.body" + NEW_LINE +
+            "    };" + NEW_LINE +
+            "}";
+
+        // when
+        final JavaScriptTemplateEngine javaScriptTemplateEngine = new JavaScriptTemplateEngine(logFormatter);
+
+        // then
+        final HttpRequest request = request()
+            .withPath("/somePath")
+            .withMethod("POST")
+            .withBody("some_body");
+        if (new ScriptEngineManager().getEngineByName("nashorn") != null) {
+            Thread[] threads = new Thread[3];
+            for (int i = 0; i < threads.length; i++) {
+                threads[i] = new Thread(new Runnable() {
+                    @Override
+                    public void run() {
+                        assertThat(javaScriptTemplateEngine.executeTemplate(template, request,
+                            HttpResponseDTO.class
+                        ), is(
+                            response()
+                                .withStatusCode(200)
+                                .withBody("{\"name\":\"value\"}")
+                        ));
+                    }
+                });
+                threads[i].start();
+            }
+            for (Thread thread : threads) {
+                thread.join();
+            }
+        } else {
+            assertThat(javaScriptTemplateEngine.executeTemplate(template, request,
                 HttpResponseDTO.class
+            ), nullValue());
+        }
+    }
+
+    @Test
+    public void shouldHandleHttpRequestsWithJavaScriptTemplateSecondExample() {
+        // given
+        String template = "" +
+            "if (request.method === 'POST' && request.path === '/somePath') {" + NEW_LINE +
+            "    return {" + NEW_LINE +
+            "        'statusCode': 200," + NEW_LINE +
+            "        'body': JSON.stringify({name: 'value'})" + NEW_LINE +
+            "    };" + NEW_LINE +
+            "} else {" + NEW_LINE +
+            "    return {" + NEW_LINE +
+            "        'statusCode': 406," + NEW_LINE +
+            "        'body': request.body" + NEW_LINE +
+            "    };" + NEW_LINE +
+            "}";
+
+        // when
+        HttpResponse actualHttpResponse = new JavaScriptTemplateEngine(logFormatter).executeTemplate(template, request()
+                .withPath("/someOtherPath")
+                .withBody("some_body"),
+            HttpResponseDTO.class
         );
 
         // then
         if (new ScriptEngineManager().getEngineByName("nashorn") != null) {
             assertThat(actualHttpResponse, is(
-                    response()
-                            .withStatusCode(406)
-                            .withBody("some_body")
+                response()
+                    .withStatusCode(406)
+                    .withBody("some_body")
             ));
         } else {
             assertThat(actualHttpResponse, nullValue());
@@ -113,39 +207,40 @@ public void shouldHandleHttpRequestsWithJavaScriptTemplateSecondExample() {
     @Test
     public void shouldHandleHttpRequestsWithJavaScriptForwardTemplateFirstExample() {
         // given
-        String template = "return {" + NEW_LINE +
-                "    'path' : \"/somePath\"," + NEW_LINE +
-                "    'cookies' : [ {" + NEW_LINE +
-                "        'name' : request.cookies['someCookie']," + NEW_LINE +
-                "        'value' : \"someCookie\"" + NEW_LINE +
-                "    }, {" + NEW_LINE +
-                "        'name' : \"someCookie\"," + NEW_LINE +
-                "        'value' : request.cookies['someCookie']" + NEW_LINE +
-                "    } ]," + NEW_LINE +
-                "    'keepAlive' : true," + NEW_LINE +
-                "    'secure' : true," + NEW_LINE +
-                "    'body' : \"some_body\"" + NEW_LINE +
-                "};";
+        String template = "" +
+            "return {" + NEW_LINE +
+            "    'path' : \"/somePath\"," + NEW_LINE +
+            "    'cookies' : [ {" + NEW_LINE +
+            "        'name' : request.cookies['someCookie']," + NEW_LINE +
+            "        'value' : \"someCookie\"" + NEW_LINE +
+            "    }, {" + NEW_LINE +
+            "        'name' : \"someCookie\"," + NEW_LINE +
+            "        'value' : request.cookies['someCookie']" + NEW_LINE +
+            "    } ]," + NEW_LINE +
+            "    'keepAlive' : true," + NEW_LINE +
+            "    'secure' : true," + NEW_LINE +
+            "    'body' : \"some_body\"" + NEW_LINE +
+            "};";
 
         // when
         HttpRequest actualHttpRequest = new JavaScriptTemplateEngine(logFormatter).executeTemplate(template, request()
-                        .withPath("/somePath")
-                        .withCookie("someCookie", "someValue")
-                        .withMethod("POST")
-                        .withBody("some_body"),
-                HttpRequestDTO.class
+                .withPath("/somePath")
+                .withCookie("someCookie", "someValue")
+                .withMethod("POST")
+                .withBody("some_body"),
+            HttpRequestDTO.class
         );
 
         // then
         if (new ScriptEngineManager().getEngineByName("nashorn") != null) {
             assertThat(actualHttpRequest, is(
-                    request()
-                            .withPath("/somePath")
-                            .withCookie("someCookie", "someValue")
-                            .withCookie("someValue", "someCookie")
-                            .withKeepAlive(true)
-                            .withSecure(true)
-                            .withBody("some_body")
+                request()
+                    .withPath("/somePath")
+                    .withCookie("someCookie", "someValue")
+                    .withCookie("someValue", "someCookie")
+                    .withKeepAlive(true)
+                    .withSecure(true)
+                    .withBody("some_body")
             ));
         } else {
             assertThat(actualHttpRequest, nullValue());
@@ -155,36 +250,37 @@ public void shouldHandleHttpRequestsWithJavaScriptForwardTemplateFirstExample()
     @Test
     public void shouldHandleHttpRequestsWithJavaScriptForwardTemplateSecondExample() {
         // given
-        String template = "return {" + NEW_LINE +
-                "    'path' : \"/somePath\"," + NEW_LINE +
-                "    'queryStringParameters' : [ {" + NEW_LINE +
-                "        'name' : \"queryParameter\"," + NEW_LINE +
-                "        'values' : request.queryStringParameters['queryParameter']" + NEW_LINE +
-                "    } ]," + NEW_LINE +
-                "    'headers' : [ {" + NEW_LINE +
-                "        'name' : \"Host\"," + NEW_LINE +
-                "        'values' : [ \"localhost:1080\" ]" + NEW_LINE +
-                "    } ]," + NEW_LINE +
-                "    'body': \"{'name': 'value'}\"" + NEW_LINE +
-                "};";
+        String template = "" +
+            "return {" + NEW_LINE +
+            "    'path' : \"/somePath\"," + NEW_LINE +
+            "    'queryStringParameters' : [ {" + NEW_LINE +
+            "        'name' : \"queryParameter\"," + NEW_LINE +
+            "        'values' : request.queryStringParameters['queryParameter']" + NEW_LINE +
+            "    } ]," + NEW_LINE +
+            "    'headers' : [ {" + NEW_LINE +
+            "        'name' : \"Host\"," + NEW_LINE +
+            "        'values' : [ \"localhost:1080\" ]" + NEW_LINE +
+            "    } ]," + NEW_LINE +
+            "    'body': \"{'name': 'value'}\"" + NEW_LINE +
+            "};";
 
 
         // when
         HttpRequest actualHttpRequest = new JavaScriptTemplateEngine(logFormatter).executeTemplate(template, request()
-                        .withPath("/someOtherPath")
-                        .withQueryStringParameter("queryParameter", "someValue")
-                        .withBody("some_body"),
-                HttpRequestDTO.class
+                .withPath("/someOtherPath")
+                .withQueryStringParameter("queryParameter", "someValue")
+                .withBody("some_body"),
+            HttpRequestDTO.class
         );
 
         // then
         if (new ScriptEngineManager().getEngineByName("nashorn") != null) {
             assertThat(actualHttpRequest, is(
-                    request()
-                            .withHeader("Host", "localhost:1080")
-                            .withPath("/somePath")
-                            .withQueryStringParameter("queryParameter", "someValue")
-                            .withBody("{'name': 'value'}")
+                request()
+                    .withHeader("Host", "localhost:1080")
+                    .withPath("/somePath")
+                    .withQueryStringParameter("queryParameter", "someValue")
+                    .withBody("{'name': 'value'}")
             ));
         } else {
             assertThat(actualHttpRequest, nullValue());
@@ -195,55 +291,55 @@ public void shouldHandleHttpRequestsWithJavaScriptForwardTemplateSecondExample()
     public void shouldHandleInvalidJavaScript() {
         // given
         String template = "{" + NEW_LINE +
-                "    'path' : \"/somePath\"," + NEW_LINE +
-                "    'queryStringParameters' : [ {" + NEW_LINE +
-                "        'name' : \"queryParameter\"," + NEW_LINE +
-                "        'values' : request.queryStringParameters['queryParameter']" + NEW_LINE +
-                "    } ]," + NEW_LINE +
-                "    'headers' : [ {" + NEW_LINE +
-                "        'name' : \"Host\"," + NEW_LINE +
-                "        'values' : [ \"localhost:1080\" ]" + NEW_LINE +
-                "    } ]," + NEW_LINE +
-                "    'body': \"{'name': 'value'}\"" + NEW_LINE +
-                "};";
+            "    'path' : \"/somePath\"," + NEW_LINE +
+            "    'queryStringParameters' : [ {" + NEW_LINE +
+            "        'name' : \"queryParameter\"," + NEW_LINE +
+            "        'values' : request.queryStringParameters['queryParameter']" + NEW_LINE +
+            "    } ]," + NEW_LINE +
+            "    'headers' : [ {" + NEW_LINE +
+            "        'name' : \"Host\"," + NEW_LINE +
+            "        'values' : [ \"localhost:1080\" ]" + NEW_LINE +
+            "    } ]," + NEW_LINE +
+            "    'body': \"{'name': 'value'}\"" + NEW_LINE +
+            "};";
         if (new ScriptEngineManager().getEngineByName("nashorn") != null) {
             exception.expect(RuntimeException.class);
             exception.expectCause(isA(ScriptException.class));
             exception.expectMessage(containsString("Exception transforming template:" + NEW_LINE +
-                    NEW_LINE +
-                    "\tfunction handle(request) {" + NEW_LINE +
-                    "\t" + NEW_LINE +
-                    "\t\t{" + NEW_LINE +
-                    "\t\t    'path' : \"/somePath\"," + NEW_LINE +
-                    "\t\t    'queryStringParameters' : [ {" + NEW_LINE +
-                    "\t\t        'name' : \"queryParameter\"," + NEW_LINE +
-                    "\t\t        'values' : request.queryStringParameters['queryParameter']" + NEW_LINE +
-                    "\t\t    } ]," + NEW_LINE +
-                    "\t\t    'headers' : [ {" + NEW_LINE +
-                    "\t\t        'name' : \"Host\"," + NEW_LINE +
-                    "\t\t        'values' : [ \"localhost:1080\" ]" + NEW_LINE +
-                    "\t\t    } ]," + NEW_LINE +
-                    "\t\t    'body': \"{'name': 'value'}\"" + NEW_LINE +
-                    "\t\t};" + NEW_LINE +
-                    "\t}" + NEW_LINE +
-                    NEW_LINE +
-                    " for request:" + NEW_LINE +
-                    NEW_LINE +
-                    "\t{" + NEW_LINE +
-                    "\t  \"path\" : \"/someOtherPath\"," + NEW_LINE +
-                    "\t  \"queryStringParameters\" : {" + NEW_LINE +
-                    "\t    \"queryParameter\" : [ \"someValue\" ]" + NEW_LINE +
-                    "\t  }," + NEW_LINE +
-                    "\t  \"body\" : \"some_body\"" + NEW_LINE +
-                    "\t}"));
+                NEW_LINE +
+                "\tfunction handle(request) {" + NEW_LINE +
+                "\t" + NEW_LINE +
+                "\t\t{" + NEW_LINE +
+                "\t\t    'path' : \"/somePath\"," + NEW_LINE +
+                "\t\t    'queryStringParameters' : [ {" + NEW_LINE +
+                "\t\t        'name' : \"queryParameter\"," + NEW_LINE +
+                "\t\t        'values' : request.queryStringParameters['queryParameter']" + NEW_LINE +
+                "\t\t    } ]," + NEW_LINE +
+                "\t\t    'headers' : [ {" + NEW_LINE +
+                "\t\t        'name' : \"Host\"," + NEW_LINE +
+                "\t\t        'values' : [ \"localhost:1080\" ]" + NEW_LINE +
+                "\t\t    } ]," + NEW_LINE +
+                "\t\t    'body': \"{'name': 'value'}\"" + NEW_LINE +
+                "\t\t};" + NEW_LINE +
+                "\t}" + NEW_LINE +
+                NEW_LINE +
+                " for request:" + NEW_LINE +
+                NEW_LINE +
+                "\t{" + NEW_LINE +
+                "\t  \"path\" : \"/someOtherPath\"," + NEW_LINE +
+                "\t  \"queryStringParameters\" : {" + NEW_LINE +
+                "\t    \"queryParameter\" : [ \"someValue\" ]" + NEW_LINE +
+                "\t  }," + NEW_LINE +
+                "\t  \"body\" : \"some_body\"" + NEW_LINE +
+                "\t}"));
         }
 
         // when
         new JavaScriptTemplateEngine(logFormatter).executeTemplate(template, request()
-                        .withPath("/someOtherPath")
-                        .withQueryStringParameter("queryParameter", "someValue")
-                        .withBody("some_body"),
-                HttpRequestDTO.class
+                .withPath("/someOtherPath")
+                .withQueryStringParameter("queryParameter", "someValue")
+                .withBody("some_body"),
+            HttpRequestDTO.class
         );
     }