From 34de167e597dd7a56ab563163730782f59ed1e17 Mon Sep 17 00:00:00 2001 From: Sebastien Deleuze Date: Wed, 15 Jul 2015 13:34:16 +0200 Subject: [PATCH] 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 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 --- ...emplateConfigurerBeanDefinitionParser.java | 3 + .../view/script/ScriptTemplateConfig.java | 10 +- .../view/script/ScriptTemplateConfigurer.java | 141 ++++-------- .../view/script/ScriptTemplateView.java | 212 +++++++++++++++--- .../web/servlet/config/spring-mvc-4.2.xsd | 9 + .../web/servlet/config/MvcNamespaceTests.java | 1 + .../script/ScriptTemplateConfigurerTests.java | 115 ---------- .../view/script/ScriptTemplateViewTests.java | 197 ++++++++++++---- .../config/mvc-config-view-resolution.xml | 3 +- 9 files changed, 407 insertions(+), 284 deletions(-) delete mode 100644 spring-webmvc/src/test/java/org/springframework/web/servlet/view/script/ScriptTemplateConfigurerTests.java diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/ScriptTemplateConfigurerBeanDefinitionParser.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/ScriptTemplateConfigurerBeanDefinitionParser.java index eb640e2aa0..3a726af77f 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/ScriptTemplateConfigurerBeanDefinitionParser.java +++ b/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 diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/view/script/ScriptTemplateConfig.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/view/script/ScriptTemplateConfig.java index 4919492b15..da3d7c8e63 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/view/script/ScriptTemplateConfig.java +++ b/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(); } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/view/script/ScriptTemplateConfigurer.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/view/script/ScriptTemplateConfigurer.java index 5e3c1b755a..374d95f76d 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/view/script/ScriptTemplateConfigurer.java +++ b/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; * } * * + *

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. + * + *

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. + * + *

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 THREADING ScriptEngine parameter + */ + 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 urls = new ArrayList(); - 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; } } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/view/script/ScriptTemplateView.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/view/script/ScriptTemplateView.java index 1d306e9f2e..0f9e7b9b1f 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/view/script/ScriptTemplateView.java +++ b/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. * - *

Nashorn Javascript engine requires Java 8+. + *

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. + * + *

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 engineHolder = + new NamedThreadLocal("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. - *

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: - *

    - *
  1. {@code template}: the view template content (String)
    2. - *
  2. {@code model}: the view model (Map)
    3. - *
- *

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 urls = new ArrayList(); + 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)); } diff --git a/spring-webmvc/src/main/resources/org/springframework/web/servlet/config/spring-mvc-4.2.xsd b/spring-webmvc/src/main/resources/org/springframework/web/servlet/config/spring-mvc-4.2.xsd index ccb04b50b3..b2ecd06d04 100644 --- a/spring-webmvc/src/main/resources/org/springframework/web/servlet/config/spring-mvc-4.2.xsd +++ b/spring-webmvc/src/main/resources/org/springframework/web/servlet/config/spring-mvc-4.2.xsd @@ -1260,6 +1260,15 @@ ]]> + + + + + diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/config/MvcNamespaceTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/config/MvcNamespaceTests.java index fe7ba2f77f..fc40bd4b13 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/config/MvcNamespaceTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/config/MvcNamespaceTests.java @@ -807,6 +807,7 @@ public class MvcNamespaceTests { assertEquals("render", scriptTemplateConfigurer.getRenderFunction()); assertEquals(StandardCharsets.ISO_8859_1, scriptTemplateConfigurer.getCharset()); assertEquals("classpath:", scriptTemplateConfigurer.getResourceLoaderPath()); + assertFalse(scriptTemplateConfigurer.isShareEngine()); String[] scripts = { "org/springframework/web/servlet/view/script/nashorn/render.js" }; accessor = new DirectFieldAccessor(scriptTemplateConfigurer); assertArrayEquals(scripts, (String[]) accessor.getPropertyValue("scripts")); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/view/script/ScriptTemplateConfigurerTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/view/script/ScriptTemplateConfigurerTests.java deleted file mode 100644 index e5c46c45df..0000000000 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/view/script/ScriptTemplateConfigurerTests.java +++ /dev/null @@ -1,115 +0,0 @@ -/* - * Copyright 2002-2015 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.web.servlet.view.script; - -import java.net.URLClassLoader; -import java.nio.charset.StandardCharsets; -import java.util.Arrays; -import javax.script.Invocable; -import javax.script.ScriptEngine; - -import org.hamcrest.Matchers; -import static org.junit.Assert.*; - -import org.junit.Before; -import org.junit.Test; -import static org.mockito.BDDMockito.given; -import static org.mockito.Mockito.mock; - -import org.springframework.context.support.StaticApplicationContext; - -/** - * Unit tests for {@link ScriptTemplateConfigurer}. - * - * @author Sebastien Deleuze - */ -public class ScriptTemplateConfigurerTests { - - private static final String RESOURCE_LOADER_PATH = "classpath:org/springframework/web/servlet/view/script/"; - - private StaticApplicationContext applicationContext; - - private ScriptTemplateConfigurer configurer; - - - @Before - public void setup() throws Exception { - this.applicationContext = new StaticApplicationContext(); - this.configurer = new ScriptTemplateConfigurer(); - this.configurer.setResourceLoaderPath(RESOURCE_LOADER_PATH); - } - - @Test - public void customEngineAndRenderFunction() throws Exception { - this.configurer.setApplicationContext(this.applicationContext); - ScriptEngine engine = mock(InvocableScriptEngine.class); - given(engine.get("key")).willReturn("value"); - this.configurer.setEngine(engine); - this.configurer.setRenderFunction("render"); - this.configurer.afterPropertiesSet(); - - engine = this.configurer.getEngine(); - assertNotNull(engine); - assertEquals("value", engine.get("key")); - assertNull(this.configurer.getRenderObject()); - assertEquals("render", this.configurer.getRenderFunction()); - assertEquals(StandardCharsets.UTF_8, this.configurer.getCharset()); - } - - @Test(expected = IllegalArgumentException.class) - public void nonInvocableScriptEngine() throws Exception { - this.configurer.setApplicationContext(this.applicationContext); - ScriptEngine engine = mock(ScriptEngine.class); - this.configurer.setEngine(engine); - } - - @Test(expected = IllegalStateException.class) - public void noRenderFunctionDefined() throws Exception { - this.configurer.setApplicationContext(this.applicationContext); - ScriptEngine engine = mock(InvocableScriptEngine.class); - this.configurer.setEngine(engine); - this.configurer.afterPropertiesSet(); - } - - @Test - public void parentLoader() throws Exception { - - this.configurer.setApplicationContext(this.applicationContext); - - ClassLoader classLoader = this.configurer.createClassLoader(); - assertNotNull(classLoader); - URLClassLoader urlClassLoader = (URLClassLoader) classLoader; - assertThat(Arrays.asList(urlClassLoader.getURLs()), Matchers.hasSize(1)); - assertThat(Arrays.asList(urlClassLoader.getURLs()).get(0).toString(), - Matchers.endsWith("org/springframework/web/servlet/view/script/")); - - this.configurer.setResourceLoaderPath(RESOURCE_LOADER_PATH + ",classpath:org/springframework/web/servlet/view/"); - classLoader = this.configurer.createClassLoader(); - assertNotNull(classLoader); - urlClassLoader = (URLClassLoader) classLoader; - assertThat(Arrays.asList(urlClassLoader.getURLs()), Matchers.hasSize(2)); - assertThat(Arrays.asList(urlClassLoader.getURLs()).get(0).toString(), - Matchers.endsWith("org/springframework/web/servlet/view/script/")); - assertThat(Arrays.asList(urlClassLoader.getURLs()).get(1).toString(), - Matchers.endsWith("org/springframework/web/servlet/view/")); - } - - - private interface InvocableScriptEngine extends ScriptEngine, Invocable { - } - -} diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/view/script/ScriptTemplateViewTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/view/script/ScriptTemplateViewTests.java index 1ae173d5c9..708eace985 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/view/script/ScriptTemplateViewTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/view/script/ScriptTemplateViewTests.java @@ -16,16 +16,22 @@ package org.springframework.web.servlet.view.script; +import java.net.URLClassLoader; import java.nio.charset.StandardCharsets; -import java.util.HashMap; -import java.util.Map; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; import javax.script.Invocable; import javax.script.ScriptEngine; -import javax.servlet.ServletContext; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; +import org.hamcrest.Matchers; +import static org.hamcrest.Matchers.containsString; +import static org.junit.Assert.*; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThat; import org.junit.Before; import org.junit.Test; import static org.mockito.BDDMockito.given; @@ -33,9 +39,7 @@ import static org.mockito.Mockito.mock; import org.springframework.beans.DirectFieldAccessor; import org.springframework.context.ApplicationContextException; -import org.springframework.mock.web.test.MockServletContext; -import org.springframework.web.context.WebApplicationContext; - +import org.springframework.context.support.StaticApplicationContext; /** * Unit tests for {@link ScriptTemplateView}. @@ -44,63 +48,176 @@ import org.springframework.web.context.WebApplicationContext; */ public class ScriptTemplateViewTests { - private WebApplicationContext webAppContext; + private ScriptTemplateView view; + + private ScriptTemplateConfigurer configurer; + + private StaticApplicationContext applicationContext; + + private static final String RESOURCE_LOADER_PATH = "classpath:org/springframework/web/servlet/view/script/"; - private ServletContext servletContext; @Before public void setup() { - this.webAppContext = mock(WebApplicationContext.class); - this.servletContext = new MockServletContext(); - this.servletContext.setAttribute(WebApplicationContext.ROOT_WEB_APPLICATION_CONTEXT_ATTRIBUTE, this.webAppContext); + this.configurer = new ScriptTemplateConfigurer(); + this.applicationContext = new StaticApplicationContext(); + this.applicationContext.getBeanFactory().registerSingleton("scriptTemplateConfigurer", this.configurer); + this.view = new ScriptTemplateView(); + this.view.setUrl("sampleView"); } @Test public void missingScriptTemplateConfig() throws Exception { - ScriptTemplateView view = new ScriptTemplateView(); - given(this.webAppContext.getBeansOfType(ScriptTemplateConfig.class, true, false)) - .willReturn(new HashMap()); - - view.setUrl("sampleView"); try { - view.setApplicationContext(this.webAppContext); - fail(); + this.view.setApplicationContext(new StaticApplicationContext()); } catch (ApplicationContextException ex) { assertTrue(ex.getMessage().contains("ScriptTemplateConfig")); + return; } + fail(); } @Test - public void dectectScriptTemplateConfig() throws Exception { + public void detectScriptTemplateConfigWithEngine() { InvocableScriptEngine engine = mock(InvocableScriptEngine.class); - ScriptTemplateConfigurer configurer = new ScriptTemplateConfigurer(); - configurer.setEngine(engine); - configurer.setRenderObject("Template"); - configurer.setRenderFunction("render"); - configurer.setCharset(StandardCharsets.ISO_8859_1); - Map configMap = new HashMap(); - configMap.put("scriptTemplateConfigurer", configurer); - ScriptTemplateView view = new ScriptTemplateView(); - given(this.webAppContext.getBeansOfType(ScriptTemplateConfig.class, true, false)).willReturn(configMap); - - DirectFieldAccessor accessor = new DirectFieldAccessor(view); - view.setUrl("sampleView"); - view.setApplicationContext(this.webAppContext); + this.configurer.setEngine(engine); + this.configurer.setRenderObject("Template"); + this.configurer.setRenderFunction("render"); + this.configurer.setCharset(StandardCharsets.ISO_8859_1); + DirectFieldAccessor accessor = new DirectFieldAccessor(this.view); + this.view.setApplicationContext(this.applicationContext); assertEquals(engine, accessor.getPropertyValue("engine")); - assertEquals(StandardCharsets.ISO_8859_1, accessor.getPropertyValue("charset")); assertEquals("Template", accessor.getPropertyValue("renderObject")); assertEquals("render", accessor.getPropertyValue("renderFunction")); + assertEquals(StandardCharsets.ISO_8859_1, accessor.getPropertyValue("charset")); } - @Test(expected = IllegalArgumentException.class) + @Test + public void detectScriptTemplateConfigWithEngineName() { + this.configurer.setEngineName("nashorn"); + this.configurer.setRenderObject("Template"); + this.configurer.setRenderFunction("render"); + this.configurer.setCharset(StandardCharsets.ISO_8859_1); + this.configurer.setSharedEngine(true); + DirectFieldAccessor accessor = new DirectFieldAccessor(this.view); + this.view.setApplicationContext(this.applicationContext); + assertEquals("nashorn", accessor.getPropertyValue("engineName")); + assertNotNull(accessor.getPropertyValue("engine")); + assertEquals("Template", accessor.getPropertyValue("renderObject")); + assertEquals("render", accessor.getPropertyValue("renderFunction")); + assertEquals(StandardCharsets.ISO_8859_1, accessor.getPropertyValue("charset")); + assertEquals(true, accessor.getPropertyValue("sharedEngine")); + } + + @Test + public void customEngineAndRenderFunction() throws Exception { + ScriptEngine engine = mock(InvocableScriptEngine.class); + given(engine.get("key")).willReturn("value"); + this.view.setEngine(engine); + this.view.setRenderFunction("render"); + this.view.setApplicationContext(this.applicationContext); + engine = this.view.getEngine(); + assertNotNull(engine); + assertEquals("value", engine.get("key")); + DirectFieldAccessor accessor = new DirectFieldAccessor(this.view); + assertNull(accessor.getPropertyValue("renderObject")); + assertEquals("render", accessor.getPropertyValue("renderFunction")); + assertEquals(StandardCharsets.UTF_8, accessor.getPropertyValue("charset")); + } + + @Test + public void nonSharedEngine() throws Exception { + int iterations = 20; + this.view.setEngineName("nashorn"); + this.view.setRenderFunction("render"); + this.view.setSharedEngine(false); + this.view.setApplicationContext(this.applicationContext); + ExecutorService executor = Executors.newFixedThreadPool(4); + List> results = new ArrayList<>(); + for(int i = 0; i < iterations; i++) { + results.add(executor.submit(() -> view.getEngine() != null)); + } + assertEquals(iterations, results.size()); + for(int i = 0; i < iterations; i++) { + assertTrue(results.get(i).get()); + } + executor.shutdown(); + } + + @Test public void nonInvocableScriptEngine() throws Exception { - ScriptEngine engine = mock(ScriptEngine.class); - ScriptTemplateConfigurer configurer = new ScriptTemplateConfigurer(); - configurer.setEngine(engine); + try { + this.view.setEngine(mock(ScriptEngine.class)); + } catch(IllegalArgumentException ex) { + assertThat(ex.getMessage(), containsString("instance")); + return; + } fail(); } + @Test + public void noRenderFunctionDefined() { + this.view.setEngine(mock(InvocableScriptEngine.class)); + try { + this.view.setApplicationContext(this.applicationContext); + } catch(IllegalStateException ex) { + assertThat(ex.getMessage(), containsString("renderFunction")); + return; + } + fail(); + } + + @Test + public void engineAndEngineNameBothDefined() { + this.view.setEngine(mock(InvocableScriptEngine.class)); + this.view.setEngineName("test"); + this.view.setRenderFunction("render"); + try { + this.view.setApplicationContext(this.applicationContext); + } catch(IllegalStateException ex) { + assertThat(ex.getMessage(), containsString("engine or engineName")); + return; + } + fail(); + } + + @Test + public void engineSetterAndNonSharedEngine() { + this.view.setEngine(mock(InvocableScriptEngine.class)); + this.view.setRenderFunction("render"); + this.view.setSharedEngine(false); + try { + this.view.setApplicationContext(this.applicationContext); + } catch(IllegalStateException ex) { + assertThat(ex.getMessage(), containsString("sharedEngine")); + return; + } + fail(); + } + + @Test + public void parentLoader() { + this.view.setEngine(mock(InvocableScriptEngine.class)); + this.view.setRenderFunction("render"); + this.view.setResourceLoaderPath(RESOURCE_LOADER_PATH); + this.view.setApplicationContext(this.applicationContext); + ClassLoader classLoader = this.view.createClassLoader(); + assertNotNull(classLoader); + URLClassLoader urlClassLoader = (URLClassLoader) classLoader; + assertThat(Arrays.asList(urlClassLoader.getURLs()), Matchers.hasSize(1)); + assertThat(Arrays.asList(urlClassLoader.getURLs()).get(0).toString(), + Matchers.endsWith("org/springframework/web/servlet/view/script/")); + this.view.setResourceLoaderPath(RESOURCE_LOADER_PATH + ",classpath:org/springframework/web/servlet/view/"); + classLoader = this.view.createClassLoader(); + assertNotNull(classLoader); + urlClassLoader = (URLClassLoader) classLoader; + assertThat(Arrays.asList(urlClassLoader.getURLs()), Matchers.hasSize(2)); + assertThat(Arrays.asList(urlClassLoader.getURLs()).get(0).toString(), Matchers.endsWith("org/springframework/web/servlet/view/script/")); + assertThat(Arrays.asList(urlClassLoader.getURLs()).get(1).toString(), Matchers.endsWith("org/springframework/web/servlet/view/")); + } + + private interface InvocableScriptEngine extends ScriptEngine, Invocable { } diff --git a/spring-webmvc/src/test/resources/org/springframework/web/servlet/config/mvc-config-view-resolution.xml b/spring-webmvc/src/test/resources/org/springframework/web/servlet/config/mvc-config-view-resolution.xml index 222650f8ee..3488a15b48 100644 --- a/spring-webmvc/src/test/resources/org/springframework/web/servlet/config/mvc-config-view-resolution.xml +++ b/spring-webmvc/src/test/resources/org/springframework/web/servlet/config/mvc-config-view-resolution.xml @@ -34,7 +34,8 @@ - +