Support non thread-safe ScriptEngine in ScriptTemplateView

This commit adds a new sharedEngine property to ScriptTemplateConfigurer
and ScriptTemplateView in order to support non thread-safe ScriptEngine
implementations like Nashorn.

When this flag is set to false, the engine is retrieved from a
ThreadLocal<ScriptEngine> field instead of a ScriptEngine one.

Also as part of this commit, all the initialization logic has been moved from
ScriptTemplateConfigurer to ScriptTemplateView since the script engine can
now be lazily initialized multiple time in the view when sharedEngine is
set to false.

Issue: SPR-13034

spring-webmvc/src/main/java/org/springframework/web/servlet/config/ScriptTemplateConfigurerBeanDefinitionParser.java

@@ -73,6 +73,9 @@ public class ScriptTemplateConfigurerBeanDefinitionParser extends AbstractSimple
 		if (element.hasAttribute("resource-loader-path")) {
 			builder.addPropertyValue("resourceLoaderPath", element.getAttribute("resource-loader-path"));
 		}
+		if (element.hasAttribute("shared-engine")) {
+			builder.addPropertyValue("sharedEngine", element.getAttribute("shared-engine"));
+		}
 	}
 
 	@Override

spring-webmvc/src/main/java/org/springframework/web/servlet/view/script/ScriptTemplateConfig.java

@@ -19,8 +19,6 @@ package org.springframework.web.servlet.view.script;
 import java.nio.charset.Charset;
 import javax.script.ScriptEngine;
 
-import org.springframework.core.io.ResourceLoader;
-
 /**
  * Interface to be implemented by objects that configure and manage a
  * {@link ScriptEngine} for automatic lookup in a web environment.
@@ -33,12 +31,18 @@ public interface ScriptTemplateConfig {
 
 	ScriptEngine getEngine();
 
+	String getEngineName();
+
+	String[] getScripts();
+
 	String getRenderObject();
 
 	String getRenderFunction();
 
 	Charset getCharset();
 
-	ResourceLoader getResourceLoader();
+	String getResourceLoaderPath();
+
+	Boolean isShareEngine();
 
 }

spring-webmvc/src/main/java/org/springframework/web/servlet/view/script/ScriptTemplateConfigurer.java

@@ -16,28 +16,9 @@
 
 package org.springframework.web.servlet.view.script;
 
-import java.io.IOException;
-import java.io.InputStreamReader;
-import java.io.Reader;
-import java.net.URL;
-import java.net.URLClassLoader;
 import java.nio.charset.Charset;
-import java.util.ArrayList;
-import java.util.List;
 
-import javax.script.Invocable;
 import javax.script.ScriptEngine;
-import javax.script.ScriptEngineManager;
-import javax.script.ScriptException;
-
-import org.springframework.beans.factory.InitializingBean;
-import org.springframework.context.ApplicationContext;
-import org.springframework.context.ApplicationContextAware;
-import org.springframework.core.io.DefaultResourceLoader;
-import org.springframework.core.io.Resource;
-import org.springframework.core.io.ResourceLoader;
-import org.springframework.util.Assert;
-import org.springframework.util.StringUtils;
 
 /**
  * An implementation of Spring MVC's {@link ScriptTemplateConfig} for creating
@@ -59,37 +40,44 @@ import org.springframework.util.StringUtils;
  * }
  * </pre>
  *
+ * <p>It is possible to use non thread-safe script engines and templating libraries, like
+ * Handlebars or React running on Nashorn, by setting the
+ * {@link #setSharedEngine(Boolean) sharedEngine} property to {@code false}.
+ *
  * @author Sebastien Deleuze
  * @since 4.2
  * @see ScriptTemplateView
  */
-public class ScriptTemplateConfigurer implements ScriptTemplateConfig, ApplicationContextAware, InitializingBean {
+public class ScriptTemplateConfigurer implements ScriptTemplateConfig {
 
 	private ScriptEngine engine;
 
 	private String engineName;
 
-	private ApplicationContext applicationContext;
-
 	private String[] scripts;
 
 	private String renderObject;
 
 	private String renderFunction;
 
-	private Charset charset = Charset.forName("UTF-8");
+	private Charset charset;
 
-	private ResourceLoader resourceLoader;
+	private String resourceLoaderPath;
 
-	private String resourceLoaderPath = "classpath:";
+	private Boolean sharedEngine;
 
 	/**
 	 * Set the {@link ScriptEngine} to use by the view.
 	 * The script engine must implement {@code Invocable}.
 	 * You must define {@code engine} or {@code engineName}, not both.
+	 *
+	 * <p>When the {@code sharedEngine} flag is set to {@code false}, you should not specify
+	 * the script engine with this setter, but with the {@link #setEngineName(String)}
+	 * one (since it implies multiple lazy instanciations of the script engine).
+	 *
+	 * @see #setEngineName(String)
 	 */
 	public void setEngine(ScriptEngine engine) {
-		Assert.isInstanceOf(Invocable.class, engine);
 		this.engine = engine;
 	}
 
@@ -102,18 +90,15 @@ public class ScriptTemplateConfigurer implements ScriptTemplateConfig, Applicati
 	 * Set the engine name that will be used to instantiate the {@link ScriptEngine}.
 	 * The script engine must implement {@code Invocable}.
 	 * You must define {@code engine} or {@code engineName}, not both.
+	 * @see #setEngine(ScriptEngine)
 	 */
 	public void setEngineName(String engineName) {
 		this.engineName = engineName;
 	}
 
 	@Override
-	public void setApplicationContext(ApplicationContext applicationContext) {
-		this.applicationContext = applicationContext;
-	}
-
-	protected ApplicationContext getApplicationContext() {
-		return this.applicationContext;
+	public String getEngineName() {
+		return this.engineName;
 	}
 
 	/**
@@ -134,8 +119,8 @@ public class ScriptTemplateConfigurer implements ScriptTemplateConfig, Applicati
 	}
 
 	@Override
-	public String getRenderObject() {
-		return renderObject;
+	public String[] getScripts() {
+		return this.scripts;
 	}
 
 	/**
@@ -148,8 +133,8 @@ public class ScriptTemplateConfigurer implements ScriptTemplateConfig, Applicati
 	}
 
 	@Override
-	public String getRenderFunction() {
-		return renderFunction;
+	public String getRenderObject() {
+		return this.renderObject;
 	}
 
 	/**
@@ -164,6 +149,11 @@ public class ScriptTemplateConfigurer implements ScriptTemplateConfig, Applicati
 		this.renderFunction = renderFunction;
 	}
 
+	@Override
+	public String getRenderFunction() {
+		return this.renderFunction;
+	}
+
 	/**
 	 * Set the charset used to read script and template files.
 	 * ({@code UTF-8} by default).
@@ -189,69 +179,30 @@ public class ScriptTemplateConfigurer implements ScriptTemplateConfig, Applicati
 		this.resourceLoaderPath = resourceLoaderPath;
 	}
 
+	@Override
 	public String getResourceLoaderPath() {
-		return resourceLoaderPath;
+		return this.resourceLoaderPath;
+	}
+
+	/**
+	 * When set to {@code false}, use thread-local {@link ScriptEngine} instances instead
+	 * of one single shared instance. This flag should be set to {@code false} for those
+	 * using non thread-safe script engines and templating libraries, like Handlebars or
+	 * React running on Nashorn for example.
+	 *
+	 * <p>When this flag is set to {@code false}, the script engine must be specified using
+	 * {@link #setEngineName(String)}. Using {@link #setEngine(ScriptEngine)} is not
+	 * possible because multiple instances of the script engine need to be created lazily
+	 * (one per thread).
+	 * @see <a href="http://docs.oracle.com/javase/8/docs/api/javax/script/ScriptEngineFactory.html#getParameter-java.lang.String-">THREADING ScriptEngine parameter<a/>
+	 */
+	public void setSharedEngine(Boolean sharedEngine) {
+		this.sharedEngine = sharedEngine;
 	}
 
 	@Override
-	public ResourceLoader getResourceLoader() {
-		return resourceLoader;
-	}
-
-	@Override
-	public void afterPropertiesSet() throws Exception {
-		if (this.engine == null) {
-			this.engine = createScriptEngine();
-		}
-		Assert.state(this.renderFunction != null, "renderFunction property must be defined.");
-		this.resourceLoader  = new DefaultResourceLoader(createClassLoader());
-		if (this.scripts != null) {
-			try {
-				for (String script : this.scripts) {
-					this.engine.eval(read(script));
-				}
-			}
-			catch (ScriptException e) {
-				throw new IllegalStateException("could not load script", e);
-			}
-		}
-	}
-
-	protected ClassLoader createClassLoader() throws IOException {
-		String[] paths = StringUtils.commaDelimitedListToStringArray(this.resourceLoaderPath);
-		List<URL> urls = new ArrayList<URL>();
-		for (String path : paths) {
-			Resource[] resources = getApplicationContext().getResources(path);
-			if (resources.length > 0) {
-				for (Resource resource : resources) {
-					if (resource.exists()) {
-						urls.add(resource.getURL());
-					}
-				}
-			}
-		}
-		ClassLoader classLoader = getApplicationContext().getClassLoader();
-		return (urls.size() > 0 ? new URLClassLoader(urls.toArray(new URL[urls.size()]), classLoader) : classLoader);
-	}
-
-	private Reader read(String path) throws IOException {
-		Resource resource = this.resourceLoader.getResource(path);
-		Assert.state(resource.exists(), "Resource " + path + " not found.");
-		return new InputStreamReader(resource.getInputStream());
-	}
-
-	protected ScriptEngine createScriptEngine() throws IOException {
-		if (this.engine != null && this.engineName != null) {
-			throw new IllegalStateException("You should define engine or engineName properties, not both.");
-		}
-		if (this.engineName != null) {
-			ScriptEngine scriptEngine = new ScriptEngineManager().getEngineByName(this.engineName);
-			Assert.state(scriptEngine != null, "No engine \"" + this.engineName + "\" found.");
-			Assert.state(scriptEngine instanceof Invocable, "Script engine should be instance of Invocable");
-			this.engine = scriptEngine;
-		}
-		Assert.state(this.engine != null, "No script engine found, please specify valid engine or engineName properties.");
-		return this.engine;
+	public Boolean isShareEngine() {
+		return this.sharedEngine;
 	}
 
 }

spring-webmvc/src/main/java/org/springframework/web/servlet/view/script/ScriptTemplateView.java

@@ -17,10 +17,17 @@
 package org.springframework.web.servlet.view.script;
 
 import java.io.IOException;
+import java.io.InputStreamReader;
+import java.net.URL;
+import java.net.URLClassLoader;
 import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Map;
 import javax.script.Invocable;
 import javax.script.ScriptEngine;
+import javax.script.ScriptEngineManager;
+import javax.script.ScriptException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
@@ -29,17 +36,26 @@ import org.springframework.beans.factory.BeanFactoryUtils;
 import org.springframework.beans.factory.NoSuchBeanDefinitionException;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.ApplicationContextException;
+import org.springframework.core.NamedThreadLocal;
+import org.springframework.core.io.DefaultResourceLoader;
 import org.springframework.core.io.Resource;
 import org.springframework.core.io.ResourceLoader;
 import org.springframework.util.Assert;
 import org.springframework.util.StreamUtils;
+import org.springframework.util.StringUtils;
 import org.springframework.web.servlet.view.AbstractUrlBasedView;
 
 /**
  * An {@link org.springframework.web.servlet.view.AbstractUrlBasedView AbstractUrlBasedView}
  * designed to run any template library based on a JSR-223 script engine.
  *
- * <p>Nashorn Javascript engine requires Java 8+.
+ * <p>If not set, each property is auto-detected by looking up up a single
+ * {@link ScriptTemplateConfig} bean in the web application context and using
+ * it to obtain the configured properties.
+ *
+ * <p>Nashorn Javascript engine requires Java 8+, and may require setting the
+ * {@code sharedEngine} property to {@code false} in order to run properly. See
+ * {@link ScriptTemplateConfigurer#setSharedEngine(Boolean)} for more details.
  *
  * @author Sebastien Deleuze
  * @since 4.2
@@ -48,8 +64,20 @@ import org.springframework.web.servlet.view.AbstractUrlBasedView;
  */
 public class ScriptTemplateView extends AbstractUrlBasedView {
 
+	private static final Charset DEFAULT_CHARSET = Charset.forName("UTF-8");
+
+	private static final String DEFAULT_RESOURCE_LOADER_PATH = "classpath:";
+
+
 	private ScriptEngine engine;
 
+	private final ThreadLocal<ScriptEngine> engineHolder =
+			new NamedThreadLocal<ScriptEngine>("ScriptTemplateView engine");
+
+	private String engineName;
+
+	private String[] scripts;
+
 	private String renderObject;
 
 	private String renderFunction;
@@ -58,58 +86,182 @@ public class ScriptTemplateView extends AbstractUrlBasedView {
 
 	private ResourceLoader resourceLoader;
 
+	private String resourceLoaderPath;
+
+	private Boolean sharedEngine;
+
+
 	/**
-	 * Set the {@link ScriptEngine} to use in this view.
-	 * <p>If not set, the engine is auto-detected by looking up up a single
-	 * {@link ScriptTemplateConfig} bean in the web application context and using
-	 * it to obtain the configured {@code ScriptEngine} instance.
-	 * @see ScriptTemplateConfig
+	 * See {@link ScriptTemplateConfigurer#setEngine(ScriptEngine)} documentation.
 	 */
 	public void setEngine(ScriptEngine engine) {
+		Assert.isInstanceOf(Invocable.class, engine);
 		this.engine = engine;
 	}
 
+	protected ScriptEngine getEngine() {
+		if (Boolean.FALSE.equals(this.sharedEngine)) {
+			ScriptEngine engine = this.engineHolder.get();
+			if (engine == null) {
+				engine = createEngineFromName();
+				this.engineHolder.set(engine);
+			}
+			return engine;
+		}
+		else if (this.engine == null) {
+			setEngine(createEngineFromName());
+		}
+		return this.engine;
+	}
+
+	protected ScriptEngine createEngineFromName() {
+		Assert.notNull(this.engineName);
+		ScriptEngine engine = new ScriptEngineManager().getEngineByName(this.engineName);
+		Assert.state(engine != null, "No engine \"" + this.engineName + "\" found.");
+		loadScripts(engine);
+		return engine;
+	}
+
+	protected void loadScripts(ScriptEngine engine) {
+		Assert.notNull(engine);
+		if (this.scripts != null) {
+			try {
+				for (String script : this.scripts) {
+					Resource resource = this.resourceLoader.getResource(script);
+					Assert.state(resource.exists(), "Resource " + script + " not found.");
+					engine.eval(new InputStreamReader(resource.getInputStream()));
+				}
+			}
+			catch (ScriptException e) {
+				throw new IllegalStateException("could not load script", e);
+			}
+			catch (IOException e) {
+				throw new IllegalStateException("could not load script", e);
+			}
+		}
+	}
+
 	/**
-	 * Set the render function name. This function will be called with the
-	 * following parameters:
-	 * <ol>
-	 *     <li>{@code template}: the view template content (String)</li>
-	 *     <li>{@code model}: the view model (Map)</li>
-	 * </ol>
-	 * <p>If not set, the function name is auto-detected by looking up up a single
-	 * {@link ScriptTemplateConfig} bean in the web application context and using
-	 * it to obtain the configured {@code functionName} property.
-	 * @see ScriptTemplateConfig
+	 * See {@link ScriptTemplateConfigurer#setEngineName(String)} documentation.
+	 */
+	public void setEngineName(String engineName) {
+		this.engineName = engineName;
+	}
+
+	/**
+	 * See {@link ScriptTemplateConfigurer#setScripts(String...)} documentation.
+	 */
+	public void setScripts(String... scripts) {
+		this.scripts = scripts;
+	}
+
+	/**
+	 * See {@link ScriptTemplateConfigurer#setRenderObject(String)} documentation.
+	 */
+	public void setRenderObject(String renderObject) {
+		this.renderObject = renderObject;
+	}
+
+	/**
+	 * See {@link ScriptTemplateConfigurer#setRenderFunction(String)} documentation.
 	 */
 	public void setRenderFunction(String functionName) {
 		this.renderFunction = functionName;
 	}
 
-	public void setResourceLoader(ResourceLoader resourceLoader) {
-		this.resourceLoader = resourceLoader;
+	/**
+	 * See {@link ScriptTemplateConfigurer#setCharset(Charset)} documentation.
+	 */
+	public void setCharset(Charset charset) {
+		this.charset = charset;
+	}
+
+	/**
+	 * See {@link ScriptTemplateConfigurer#setResourceLoaderPath(String)} documentation.
+	 */
+	public void setResourceLoaderPath(String resourceLoaderPath) {
+		this.resourceLoaderPath = resourceLoaderPath;
+	}
+
+	/**
+	 * See {@link ScriptTemplateConfigurer#setSharedEngine(Boolean)} documentation.
+	 */
+	public void setSharedEngine(Boolean sharedEngine) {
+		this.sharedEngine = sharedEngine;
 	}
 
 	@Override
 	protected void initApplicationContext(ApplicationContext context) {
 		super.initApplicationContext(context);
+
 		ScriptTemplateConfig viewConfig = autodetectViewConfig();
-		if (this.engine == null) {
-			this.engine = viewConfig.getEngine();
-			Assert.state(this.engine != null, "Script engine should not be null.");
-			Assert.state(this.engine instanceof Invocable, "Script engine should be instance of Invocable");
+		if (this.engine == null && viewConfig.getEngine() != null) {
+			setEngine(viewConfig.getEngine());
 		}
-		if (this.resourceLoader == null) {
-			this.resourceLoader = viewConfig.getResourceLoader();
+		if (this.engineName == null && viewConfig.getEngineName() != null) {
+			this.engineName = viewConfig.getEngineName();
 		}
-		if (this.renderObject == null) {
+		if (this.scripts == null && viewConfig.getScripts() != null) {
+			this.scripts = viewConfig.getScripts();
+		}
+		if (this.renderObject == null && viewConfig.getRenderObject() != null) {
 			this.renderObject = viewConfig.getRenderObject();
 		}
-		if (this.renderFunction == null) {
+		if (this.renderFunction == null && viewConfig.getRenderFunction() != null) {
 			this.renderFunction = viewConfig.getRenderFunction();
 		}
 		if (this.charset == null) {
-			this.charset = viewConfig.getCharset();
+			this.charset = viewConfig.getCharset() == null ? DEFAULT_CHARSET : viewConfig.getCharset();
 		}
+		if (this.resourceLoaderPath == null) {
+			this.resourceLoaderPath = viewConfig.getResourceLoaderPath()  == null ?
+					DEFAULT_RESOURCE_LOADER_PATH : viewConfig.getResourceLoaderPath();
+		}
+		if (this.resourceLoader == null) {
+			this.resourceLoader = new DefaultResourceLoader(createClassLoader());
+		}
+		if (this.sharedEngine == null && viewConfig.isShareEngine() != null) {
+			this.sharedEngine = viewConfig.isShareEngine();
+		}
+
+		Assert.state(!(this.engine != null && this.engineName != null),
+				"You should define engine or engineName properties, not both.");
+		Assert.state(!(this.engine == null && this.engineName == null),
+				"No script engine found, please specify valid engine or engineName properties.");
+		if (Boolean.FALSE.equals(this.sharedEngine)) {
+			Assert.state(this.engineName != null,
+					"When sharedEngine property is set to false, you should specify the " +
+					"script engine using the engineName property, not the engine one.");
+		}
+		Assert.state(this.renderFunction != null, "renderFunction property must be defined.");
+
+		if (this.engine != null) {
+			loadScripts(this.engine);
+		}
+		else {
+			setEngine(createEngineFromName());
+		}
+	}
+
+	protected ClassLoader createClassLoader() {
+		String[] paths = StringUtils.commaDelimitedListToStringArray(this.resourceLoaderPath);
+		List<URL> urls = new ArrayList<URL>();
+		try {
+			for (String path : paths) {
+				Resource[] resources = getApplicationContext().getResources(path);
+				if (resources.length > 0) {
+					for (Resource resource : resources) {
+						if (resource.exists()) {
+							urls.add(resource.getURL());
+						}
+					}
+				}
+			}
+		} catch (IOException e) {
+			throw new IllegalStateException("Cannot create class loader: " + e.getMessage());
+		}
+		ClassLoader classLoader = getApplicationContext().getClassLoader();
+		return (urls.size() > 0 ? new URLClassLoader(urls.toArray(new URL[urls.size()]), classLoader) : classLoader);
 	}
 
 	protected ScriptTemplateConfig autodetectViewConfig() throws BeansException {
@@ -128,13 +280,13 @@ public class ScriptTemplateView extends AbstractUrlBasedView {
 		Assert.notNull("Render function must not be null", this.renderFunction);
 		try {
 			String template = getTemplate(getUrl());
-			Object html = null;
+			Object html;
 			if (this.renderObject != null) {
 				Object thiz = engine.eval(this.renderObject);
-				html = ((Invocable)this.engine).invokeMethod(thiz, this.renderFunction, template, model);
+				html = ((Invocable)getEngine()).invokeMethod(thiz, this.renderFunction, template, model);
 			}
 			else {
-				html = ((Invocable)this.engine).invokeFunction(this.renderFunction, template, model);
+				html = ((Invocable)getEngine()).invokeFunction(this.renderFunction, template, model);
 			}
 			response.getWriter().write(String.valueOf(html));
 		}