Browse files

Review "Introduce ScriptEvaluator abstraction"

Content

 - Rename discoverEngine => determineScriptEngine

   'determine' naming is commonly used throughout the framework,
   particularly in protected methods such as the one in question;
   'discover' naming has little precedent by comparison.

 - Rename EvaluationType => EvaluationPolicy

   This enumeration indicates when a script should be evaluated as
   opposed to which strategy or approach should be used when
   evaluating it.

 - Add Javadoc where missing; correct and improve where appropriate

 - Eliminate printing to standard out and add meaningful assertions to
   Jsr223ScriptEvaluatorTests

Style

 - Eliminate trailing whitespace

 - Indent case blocks in switch statements

 - Wrap code and documentation at 90 characters where feasible

 - Use imperative statements in Javadoc, e.g.

   "Set the extension" instead of "Sets the extension"
   "Determine a name" instead of "Determines a name"

 - Add @since tags on all new members of the public API

 - Use @link tags where appropriate

 - Convert use of <code/> to {@code}

 - Use "may be null" vs "can be null"

 - JUnit test classes should end in "Tests" not "Test"

Issue: SPR-8999
  • Loading branch information...
1 parent 194f006 commit 319d8566fecba7d2e9f95ee5351581621dfaa003 @cbeams committed Jan 7, 2012
View
24 ...ngframework.context/src/main/java/org/springframework/scripting/ReadableScriptSource.java
@@ -20,32 +20,34 @@
import java.io.Reader;
/**
- * Extension to {@link ScriptSource} that provides a {@link Reader} implementation
- * for the target source (suitable for streaming).
- *
+ * Extension to {@link ScriptSource} that provides a {@link Reader} implementation for the
+ * target source (suitable for streaming).
+ *
* @author Costin Leau
+ * @since 3.1.1
*/
public interface ReadableScriptSource extends ScriptSource {
/**
- * Retrieves the script source text as {@link Reader}.
- *
+ * Retrieve the script source text as {@link Reader}.
+ *
* @return reader for the underlying script
* @throws IOException if script retrieval failed
*/
Reader getScriptAsReader() throws IOException;
/**
- * Determines a name for the underlying script.
- *
- * @return the suggested script name, or <code>null</code> if none available
+ * Determine a name for the underlying script.
+ *
+ * @return the suggested script name, or {@code null} if none available
*/
String suggestedScriptName();
/**
- * Indicate whether the underlying script data has been modified since
- * the last time {@link #getScriptAsString()} or {@link #getScriptAsReader() } was called.
- * Returns <code>true</code> if the script has not been read yet.
+ * Indicate whether the underlying script data has been modified since the last time
+ * {@link #getScriptAsString()} or {@link #getScriptAsReader()} was called. Return
+ * {@code true} if the script has not been read yet.
+ *
* @return whether the script data has been modified
*/
boolean isModified();
View
15 org.springframework.context/src/main/java/org/springframework/scripting/ScriptEvaluator.java
@@ -20,25 +20,26 @@
/**
* Script evaluator.
- *
+ *
* @author Costin Leau
+ * @since 3.1.1
*/
public interface ScriptEvaluator {
/**
- * Evaluates the given script (without any arguments) and returns the result (if any).
- *
+ * Evaluate the given script (without any arguments) and returns the result (if any).
+ *
* @param script script to evaluate
- * @return script result (can be null)
+ * @return script result (may be {@code null})
*/
Object evaluate(ScriptSource script);
/**
* Evaluates the given script and returns the result (if any).
- *
+ *
* @param script script to evaluate
- * @param arguments script arguments.
- * @return script result (can be null)
+ * @param arguments script arguments
+ * @return script result (may be {@code null})
*/
Object evaluate(ScriptSource script, Map<String, Object> arguments);
View
94 ...ork.context/src/main/java/org/springframework/scripting/jsr223/Jsr223ScriptEvaluator.java
@@ -1,18 +1,19 @@
/*
* Copyright 2002-2012 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.scripting.jsr223;
import java.io.IOException;
@@ -38,9 +39,14 @@
import org.springframework.util.StringUtils;
/**
- * Jsr233/javax.scripting implementation of {@link ScriptEvaluator}.
- *
+ * {@link ScriptEvaluator} implementation that delegates to an underlying
+ * JSR-233/javax.scripting {@link ScriptEngine} to evaluate scripts. The particular
+ * {@code ScriptEngine} to use is {@linkplain #determineScriptEngine determined} based
+ * based on the language/extension of the specified script.
+ *
* @author Costin Leau
+ * @author Chris Beams
+ * @since 3.1.1
*/
public class Jsr223ScriptEvaluator implements ScriptEvaluator {
@@ -52,16 +58,21 @@
/**
- * Constructs a new <code>Jsr223ScriptEvaluator</code> instance.
+ * Construct a new {@code Jsr223ScriptEvaluator} instance, allowing the underlying
+ * JSR-223 {@link ScriptEngineManager} to determine the class loader to use.
+ * @see ScriptEngineManager#ScriptEngineManager()
*/
public Jsr223ScriptEvaluator() {
this(null);
- };
+ }
/**
- * Constructs a new <code>Jsr223ScriptEvaluator</code> instance.
+ * Construct a new {@code Jsr223ScriptEvaluator} instance.
*
- * @param classLoader class loader to use
+ * @param classLoader class loader to use when constructing underlying JSR-223
+ * {@link ScriptEngineManager} (may be {@code null}).
+ * @see #determineScriptEngine(ScriptSource, Map)
+ * @see ScriptEngineManager#ScriptEngineManager(ClassLoader)
*/
public Jsr223ScriptEvaluator(ClassLoader classLoader) {
this.classLoader = classLoader;
@@ -72,18 +83,23 @@ public Object evaluate(ScriptSource script) {
}
public Object evaluate(ScriptSource script, Map<String, Object> arguments) {
- ScriptEngine engine = discoverEngine(script, arguments);
+ ScriptEngine engine = determineScriptEngine(script, arguments);
- Bindings bindings = (!CollectionUtils.isEmpty(arguments) ? new SimpleBindings(arguments) : null);
+ Bindings bindings = (!CollectionUtils.isEmpty(arguments) ?
+ new SimpleBindings(arguments) : null);
try {
- Reader scriptAsReader = (script instanceof ReadableScriptSource ? ((ReadableScriptSource) script).getScriptAsReader() : null);
+ Reader scriptAsReader = (script instanceof ReadableScriptSource ?
+ ((ReadableScriptSource) script).getScriptAsReader() : null);
if (bindings == null) {
- return (scriptAsReader == null ? engine.eval(script.getScriptAsString()) : engine.eval(scriptAsReader));
+ return (scriptAsReader == null ?
+ engine.eval(script.getScriptAsString()) : engine.eval(scriptAsReader));
}
else {
- return (scriptAsReader == null ? engine.eval(script.getScriptAsString(), bindings) : engine.eval(scriptAsReader, bindings));
+ return (scriptAsReader == null ?
+ engine.eval(script.getScriptAsString(), bindings) :
+ engine.eval(scriptAsReader, bindings));
}
} catch (IOException ex) {
throw new ScriptCompilationException(script, "Cannot access script", ex);
@@ -92,8 +108,21 @@ public Object evaluate(ScriptSource script, Map<String, Object> arguments) {
}
}
- protected ScriptEngine discoverEngine(ScriptSource script, Map<String, Object> arguments) {
- ScriptEngineManager engineManager = new ScriptEngineManager(classLoader);
+ /**
+ * Determine the JSR-223 {@link ScriptEngine} to be used for evaluating the given
+ * script based on the {@linkplain #setExtension extension} or
+ * {@linkplain #setLanguage language} of the script. If the script is a
+ * {@link ReadableScriptSource}, the extension will be discovered automatically.
+ * @param script the script to be evaluated
+ * @param arguments arguments provided to the script (ignored in this implementation)
+ * @throws IllegalArgumentException if language/extension have not been specified and
+ * no extension can be automatically discovered
+ * @throws IllegalArgumentException if no {@code ScriptEngine} associated with the
+ * given language/extension can be found
+ * @return the {@code ScriptEngine} to be used for the given language
+ */
+ protected ScriptEngine determineScriptEngine(ScriptSource script, Map<String, Object> arguments) {
+ ScriptEngineManager engineManager = new ScriptEngineManager(this.classLoader);
ScriptEngine engine = null;
if (StringUtils.hasText(language)) {
@@ -107,33 +136,44 @@ protected ScriptEngine discoverEngine(ScriptSource script, Map<String, Object> a
engine = engineManager.getEngineByExtension(extension);
}
- Assert.notNull(engine, "No suitable engine found for "
- + (StringUtils.hasText(language) ? "language " + language : "extension " + extension));
+ Assert.notNull(engine, String.format("No suitable engine found for %s",
+ (StringUtils.hasText(language) ? "language " + language : "extension " + extension)));
if (log.isDebugEnabled()) {
ScriptEngineFactory factory = engine.getFactory();
- log.debug(String.format("Using ScriptEngine %s (%s), language %s (%s)", factory.getEngineName(),
- factory.getEngineVersion(), factory.getLanguageName(), factory.getLanguageVersion()));
+ log.debug(String.format("Using ScriptEngine %s (%s), language %s (%s)",
+ factory.getEngineName(), factory.getEngineVersion(),
+ factory.getLanguageName(), factory.getLanguageVersion()));
}
return engine;
}
/**
- * Sets the extension of the language meant for evaluation the scripts..
- *
- * @param extension The extension to set.
+ * Set the script file extension, to be used when determining the JSR-223
+ * {@link ScriptEngine} to be used in evaluating the script. This property is not
+ * required if {@link #setLanguage} has been called or if the script provided is an
+ * implementation of {@link ReadableScriptSource}.
+ *
+ * @param extension the extension of the script, e.g. ".rb" for Ruby, ".js" for
+ * JavaScript, etc
+ * @see #determineScriptEngine
+ * @see ScriptEngineManager#getEngineByExtension
*/
public void setExtension(String extension) {
this.extension = extension;
}
/**
- * Sets the name of language meant for evaluation the scripts.
- *
- * @param language The language to set.
+ * Set the script file extension, to be used when determining the JSR-223
+ * {@link ScriptEngine} to be used in evaluating the script. This property is not
+ * required if {@link #setExtension} has been called or if the script provided is an
+ * implementation of {@link ReadableScriptSource}.
+ *
+ * @param language the language to set
+ * @see #determineScriptEngine
*/
public void setLanguage(String language) {
this.language = language;
}
-}
+}
View
110 .../src/main/java/org/springframework/scripting/jsr223/Jsr223ScriptEvaluatorFactoryBean.java
@@ -1,18 +1,19 @@
/*
* Copyright 2002-2012 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.scripting.jsr223;
import java.util.Map;
@@ -23,47 +24,68 @@
import org.springframework.scripting.ScriptSource;
/**
- * FactoryBean for easy declarative use of JSR-223 based {@link ScriptEvaluator}.
- *
+ * {@link FactoryBean} implementation allowing for use of JSR-223 based
+ * {@link ScriptEvaluator} within Spring XML configuration files.
+ *
* @author Costin Leau
+ * @author Chris Beams
+ * @since 3.1.1
*/
class Jsr223ScriptEvaluatorFactoryBean implements InitializingBean, BeanClassLoaderAware, FactoryBean<Object> {
- public enum EvaluationType {
- ONCE, IF_MODIFIED, ALWAYS;
+ /**
+ * Enumeration specifying the policy for how often the script should be evaluated
+ * during the lifetime of this factory bean.
+ */
+ public enum EvaluationPolicy {
+ /**
+ * Evaluate the script once and only once, regardless of whether the script has
+ * been modified.
+ */
+ ONCE,
+
+ /**
+ * Evaluate the script if it has been modified since the last time it was evaluated.
+ */
+ IF_MODIFIED,
+
+ /**
+ * Evaluate the script every time {@code getObject} is invoked.
+ */
+ ALWAYS;
}
private ClassLoader classLoader;
private ScriptSource script;
private Jsr223ScriptEvaluator evaluator;
private String language, extension;
private Map<String, Object> arguments;
- private EvaluationType evaluation = EvaluationType.ALWAYS;
+ private EvaluationPolicy evaluationPolicy = EvaluationPolicy.ALWAYS;
private final Object monitor = new Object();
private volatile boolean evaluated;
private Object result = null;
private boolean runAtStartup = false;
public Object getObject() {
- switch (evaluation) {
- case ONCE:
- if (!evaluated) {
- synchronized (monitor) {
- if (!evaluated) {
- evaluated = true;
- result = evaluator.evaluate(script, arguments);
+ switch (evaluationPolicy) {
+ case ONCE:
+ if (!evaluated) {
+ synchronized (monitor) {
+ if (!evaluated) {
+ evaluated = true;
+ result = evaluator.evaluate(script, arguments);
+ }
}
}
- }
- return result;
- case IF_MODIFIED:
- // isModified is synchronized so only one thread will see the update
- if (script.isModified()) {
- result = evaluator.evaluate(script, arguments);
- }
- return result;
- default:
- return evaluator.evaluate(script, arguments);
+ return result;
+ case IF_MODIFIED:
+ // isModified is synchronized so only one thread will see the update
+ if (script.isModified()) {
+ result = evaluator.evaluate(script, arguments);
+ }
+ return result;
+ default:
+ return evaluator.evaluate(script, arguments);
}
}
@@ -80,10 +102,10 @@ public void afterPropertiesSet() {
}
/**
- * Method for post-processing arguments. Useful for enhancing (adding) new arguments to scripts
- * being executed.
- *
- * @param arguments
+ * Method for post-processing script arguments. Useful for enhancing (adding) new
+ * arguments to scripts being executed. The default implementation is a no-op.
+ *
+ * @param arguments arguments provided to the script
*/
protected void postProcess(Map<String, Object> arguments) {
@@ -94,63 +116,63 @@ protected void postProcess(Map<String, Object> arguments) {
}
public boolean isSingleton() {
- return EvaluationType.ONCE.equals(evaluation);
+ return EvaluationPolicy.ONCE.equals(evaluationPolicy);
}
public void setBeanClassLoader(ClassLoader classLoader) {
this.classLoader = classLoader;
}
/**
- * @param resource The resource to set.
+ * @param script the script to be evaluated
*/
public void setScriptSource(ScriptSource script) {
this.script = script;
}
/**
- * @param language The language to set.
+ * @param language the language of the script to be evaluated.
+ * @see Jsr223ScriptEvaluator#setLanguage
*/
public void setLanguage(String language) {
this.language = language;
}
/**
- * @param extension The extension to set.
+ * @param extension the extension of the script to be evaluated.
+ * @see Jsr223ScriptEvaluator#setExtension
*/
public void setExtension(String extension) {
this.extension = extension;
}
/**
- * Sets the way the script is modified.
- *
- * @param singleton
+ * Set the {@link EvaluationPolicy} for the script.
*/
- public void setEvaluate(EvaluationType evaluation) {
- this.evaluation = evaluation;
+ public void setEvaluationPolicy(EvaluationPolicy evaluationPolicy) {
+ this.evaluationPolicy = evaluationPolicy;
}
/**
- * @param arguments The arguments to set.
+ * @param arguments the arguments to provide to the script
*/
public void setArguments(Map<String, Object> arguments) {
this.arguments = arguments;
}
/**
- * Indicates whether the script gets executed once the factory bean initializes or not (default).
- *
- * @return true if the script runs or not during startup
+ * Return whether the script should be evaluated immediately upon container startup.
*/
public boolean isRunAtStartup() {
return runAtStartup;
}
/**
- * @param runAtStartup The runStartUp to set.
+ * Indicate whether the {@linkplain #setScriptSource script} should be evaluated
+ * immediately upon container startup. If false, evaluation will occur only when
+ * {@link #getObject()} is called. The default is {@code false}.
*/
public void setRunAtStartup(boolean runAtStartup) {
this.runAtStartup = runAtStartup;
}
-}
+}
View
27 ...ork.context/src/main/java/org/springframework/scripting/support/ResourceScriptSource.java
@@ -31,12 +31,10 @@
import org.springframework.util.StringUtils;
/**
- * {@link org.springframework.scripting.ScriptSource} implementation
- * based on Spring's {@link org.springframework.core.io.Resource}
- * abstraction. Loads the script text from the underlying Resource's
- * {@link org.springframework.core.io.Resource#getFile() File} or
- * {@link org.springframework.core.io.Resource#getInputStream() InputStream},
- * and tracks the last-modified timestamp of the file (if possible).
+ * {@link ScriptSource} implementation based on Spring's {@link Resource} abstraction.
+ * Loads the script text from the underlying {@code Resource}'s
+ * {@link Resource#getFile() File} or {@link Resource#getInputStream() InputStream}, and
+ * tracks the last-modified timestamp of the file (if possible).
*
* @author Rob Harrop
* @author Juergen Hoeller
@@ -109,28 +107,29 @@ public String suggestedClassName() {
/**
* Sets the encoding used for reading the script resource. The default value is "UTF-8".
* A null value, implies the platform default.
- *
+ *
* @param encoding charset encoding used for reading the script.
*/
public void setEncoding(String encoding) {
this.encoding = encoding;
}
- @Override
- public String toString() {
- return this.resource.toString();
- }
-
public Reader getScriptAsReader() throws IOException {
synchronized (this.lastModifiedMonitor) {
this.lastModified = retrieveLastModifiedTime();
}
InputStream stream = this.resource.getInputStream();
- return (StringUtils.hasText(encoding) ? new InputStreamReader(stream, encoding) : new InputStreamReader(stream));
+ return (StringUtils.hasText(encoding) ?
+ new InputStreamReader(stream, encoding) : new InputStreamReader(stream));
}
public String suggestedScriptName() {
return getResource().getFilename();
}
-}
+
+ @Override
+ public String toString() {
+ return this.resource.toString();
+ }
+}
View
19 ...ework.context/src/main/java/org/springframework/scripting/support/StaticScriptSource.java
@@ -24,10 +24,9 @@
import org.springframework.util.Assert;
/**
- * Static implementation of the
- * {@link org.springframework.scripting.ScriptSource} interface,
- * encapsulating a given String that contains the script source text.
- * Supports programmatic updates of the script String.
+ * Static implementation of the {@link ScriptSource} interface, encapsulating a given
+ * String that contains the script source text. Supports programmatic updates of the
+ * script String.
*
* @author Rob Harrop
* @author Juergen Hoeller
@@ -55,8 +54,7 @@ public StaticScriptSource(String script) {
/**
* Create a new StaticScriptSource for the given script.
* @param script the script String
- * @param className the suggested class name for the script
- * (may be <code>null</code>)
+ * @param className the suggested class name for the script (may be {@code null})
*/
public StaticScriptSource(String script, String className) {
this(script, className, className);
@@ -65,11 +63,8 @@ public StaticScriptSource(String script, String className) {
/**
* Create a new StaticScriptSource for the given script.
* @param script the script String
- * @param className the suggested class name for the script
- * (may be <code>null</code>)
- * @param scriptName the suggested name for the script
- * (may be <code>null</code>)
- *
+ * @param className the suggested class name for the script (may be {@code null})
+ * @param scriptName the suggested name for the script (may be {@code null})
*/
public StaticScriptSource(String script, String className, String scriptName) {
setScript(script);
@@ -114,4 +109,4 @@ public Reader getScriptAsReader() {
public String suggestedScriptName() {
return scriptName;
}
-}
+}
View
32 ...scripting/jsr223/Jsr223EvaluatorTest.java → ...ng/jsr223/Jsr223ScriptEvaluatorTests.java
@@ -16,7 +16,7 @@
package org.springframework.scripting.jsr223;
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
import java.util.LinkedHashMap;
import java.util.Map;
@@ -27,47 +27,57 @@
import org.springframework.scripting.support.ResourceScriptSource;
import org.springframework.scripting.support.StaticScriptSource;
-public class Jsr223EvaluatorTest {
+/**
+ * Unit tests for {@link Jsr223ScriptEvaluator}.
+ *
+ * @author Costin Leau
+ * @author Chris Beams
+ * @since 3.1.1
+ */
+public class Jsr223ScriptEvaluatorTests {
@Test
public void testRhinoScript() throws Exception {
- ScriptSource script = new StaticScriptSource("print('Hello, world!')");
+ ScriptSource script = new StaticScriptSource("'Hello, js!' // return a greeting");
Jsr223ScriptEvaluator eval = new Jsr223ScriptEvaluator();
eval.setLanguage("javascript");
- eval.evaluate(script);
+ Object result = eval.evaluate(script);
+ assertEquals("Hello, js!", result);
}
@Test
public void testRhinoEvalScript() throws Exception {
ScriptSource script = new ResourceScriptSource(new UrlResource(getClass().getResource("basic-script.js")));
Jsr223ScriptEvaluator eval = new Jsr223ScriptEvaluator();
+ String arg1 = "testArg";
Map<String, Object> args = new LinkedHashMap<String, Object>();
- args.put("arg", eval);
- assertSame(eval, eval.evaluate(script, args));
+ args.put("arg", arg1);
+ assertEquals(arg1, eval.evaluate(script, args));
}
-
@Test
public void testRubyScript() throws Exception {
- ScriptSource script = new StaticScriptSource("puts 'Hello, world!'");
+ ScriptSource script = new StaticScriptSource("'Hello, ruby!' # return a greeting");
Jsr223ScriptEvaluator eval = new Jsr223ScriptEvaluator(getClass().getClassLoader());
eval.setLanguage("ruby");
- eval.evaluate(script);
+ Object result = eval.evaluate(script);
+ assertEquals("Hello, ruby!", result);
}
@Test
public void testRubyEvalScript() throws Exception {
ScriptSource script = new ResourceScriptSource(new UrlResource(getClass().getResource("basic-script.rb")));
Jsr223ScriptEvaluator eval = new Jsr223ScriptEvaluator(getClass().getClassLoader());
+ String arg1 = "testArg";
Map<String, Object> args = new LinkedHashMap<String, Object>();
- args.put("arg", eval);
+ args.put("arg", arg1);
- assertSame(eval, eval.evaluate(script, args));
+ assertEquals(arg1, eval.evaluate(script, args));
}
}
View
7 ...pringframework.context/src/test/java/org/springframework/scripting/jsr223/basic-script.js
@@ -1,6 +1,7 @@
importPackage(java.util);
+// perform some computation for good measure
+var uuid = UUID.randomUUID();
-println(UUID.randomUUID())
-// return the file length
-arg
+// return the argument passed in
+arg
View
8 ...pringframework.context/src/test/java/org/springframework/scripting/jsr223/basic-script.rb
@@ -1,5 +1,7 @@
require 'java'
-puts java.util.UUID.randomUUID().to_s
-# return the file length
-$arg
+# perform some computation for good measure
+uuid = java.util.UUID.randomUUID()
+
+# return the argument passed in
+$arg

0 comments on commit 319d856

Please sign in to comment.