Author: henrib
Date: Thu Aug 4 14:56:15 2016
New Revision: 1755195
URL: http://svn.apache.org/viewvc?rev=1755195&view=rev
Log:
JEXL:
Bulk fix for JEXL-210, JEXL-207 and JEXL-197; reverted to a sane version of
error handling, exceptions stop script execution, annotation may create a new
arithmetic if needed
Modified:
commons/proper/jexl/trunk/RELEASE-NOTES.txt
commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java
commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/InterpreterBase.java
commons/proper/jexl/trunk/src/site/xdoc/changes.xml
commons/proper/jexl/trunk/src/test/java/org/apache/commons/jexl3/AnnotationTest.java
commons/proper/jexl/trunk/src/test/java/org/apache/commons/jexl3/IssuesTest.java
Modified: commons/proper/jexl/trunk/RELEASE-NOTES.txt
URL:
http://svn.apache.org/viewvc/commons/proper/jexl/trunk/RELEASE-NOTES.txt?rev=1755195&r1=1755194&r2=1755195&view=diff
==============================================================================
--- commons/proper/jexl/trunk/RELEASE-NOTES.txt (original)
+++ commons/proper/jexl/trunk/RELEASE-NOTES.txt Thu Aug 4 14:56:15 2016
@@ -27,12 +27,15 @@ Version 3.0.1 is a micro release to fix
Bugs Fixed in 3.0.1:
====================
+* JEXL-210: The way to cancel script execution with an error
* JEXL-209: Unsolvable function/method '<?>.<null>(...)'
* JEXL-207: Inconsistent error handling
* JEXL-206: testCallableCancel() test hangs sporadically
* JEXL-205: testCancelForever() is not terminated properly as 'Fixed'
* JEXL-204: Script is not interrupted by a method call throwing Exception
+* JEXL-203: JexlArithmetic.options() diverts Interpreter to use default
implementation of JexlArithmetic instead of custom one
* JEXL-202: Detect invalid assignment operator usage with non-assignable
l-value during script parsing
+* JEXL-201: Allow Interpreter to use live values from JexlEngine.Option
interface implemented by JexlContext
* JEXL-198: JxltEngine Template does not expose pragmas
* JEXL-196: Script execution hangs while calling method with one argument
without parameter
* JEXL-194 allow synchronization on iterableValue in foreach statement
Modified:
commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java
URL:
http://svn.apache.org/viewvc/commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java?rev=1755195&r1=1755194&r2=1755195&view=diff
==============================================================================
---
commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java
(original)
+++
commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java
Thu Aug 4 14:56:15 2016
@@ -151,6 +151,21 @@ public class Interpreter extends Interpr
}
/**
+ * Copy constructor.
+ * @param ii the interpreter to copy
+ * @param jexla the arithmetic instance to use (or null)
+ */
+ protected Interpreter(Interpreter ii, JexlArithmetic jexla) {
+ super(ii, jexla);
+ operators = ii.operators;
+ cache = ii.cache;
+ frame = ii.frame;
+ ns = ii.ns;
+ functions = ii.functions;
+ functors = ii.functors;
+ }
+
+ /**
* Interpret the given script/expression.
* <p>
* If the underlying JEXL engine is silent, errors will be logged through
@@ -180,7 +195,9 @@ public class Interpreter extends Interpr
if (!isSilent()) {
throw xjexl.clean();
}
- logger.warn(xjexl.getMessage(), xjexl.getCause());
+ if (logger.isWarnEnabled()) {
+ logger.warn(xjexl.getMessage(), xjexl.getCause());
+ }
} finally {
synchronized(this) {
if (functors != null) {
@@ -1473,7 +1490,6 @@ public class Interpreter extends Interpr
return unsolvableMethod(node, "?");
}
// at this point, either the functor is a non null (hopefully)
'invocable' object or we do have the methodName
- JexlException xjexl;
Object caller = target;
try {
boolean cacheable = cache;
@@ -1602,9 +1618,8 @@ public class Interpreter extends Interpr
} catch (JexlException xthru) {
throw xthru;
} catch (Exception xany) {
- xjexl = exceptionOnInvocation(node, methodName, xany);
+ throw invocationException(node, methodName, xany);
}
- return invocationFailed(xjexl);
}
@Override
@@ -1655,9 +1670,8 @@ public class Interpreter extends Interpr
throw xthru;
} catch (Exception xany) {
String dbgStr = cobject != null ? cobject.toString() : null;
- xjexl = exceptionOnInvocation(node, dbgStr, xany);
+ throw invocationException(node, dbgStr, xany);
}
- return invocationFailed(xjexl);
}
/**
@@ -1845,7 +1859,23 @@ public class Interpreter extends Interpr
final int last = stmt.jjtGetNumChildren() - 1;
if (index == last) {
JexlNode block = stmt.jjtGetChild(last);
- return block.jjtAccept(Interpreter.this, data);
+ // if the context has changed, might need a new interpreter
+ final JexlArithmetic jexla = arithmetic.options(context);
+ if (jexla != arithmetic) {
+ if (!arithmetic.getClass().equals(jexla.getClass())) {
+ logger.warn("expected arithmetic to be " +
arithmetic.getClass().getSimpleName()
+ + ", got " + jexla.getClass().getSimpleName()
+ );
+ }
+ Interpreter ii = new Interpreter(Interpreter.this, jexla);
+ Object r = block.jjtAccept(ii, data);
+ if (ii.isCancelled()) {
+ Interpreter.this.cancel();
+ }
+ return r;
+ } else {
+ return block.jjtAccept(Interpreter.this, data);
+ }
}
// tracking whether we processed the annotation
final boolean[] processed = new boolean[]{false};
Modified:
commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/InterpreterBase.java
URL:
http://svn.apache.org/viewvc/commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/InterpreterBase.java?rev=1755195&r1=1755194&r2=1755195&view=diff
==============================================================================
---
commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/InterpreterBase.java
(original)
+++
commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/InterpreterBase.java
Thu Aug 4 14:56:15 2016
@@ -60,19 +60,29 @@ public abstract class InterpreterBase ex
this.logger = jexl.logger;
this.uberspect = jexl.uberspect;
this.context = aContext != null ? aContext : Engine.EMPTY_CONTEXT;
- if (this.context instanceof JexlEngine.Options) {
- JexlEngine.Options opts = (JexlEngine.Options) context;
- this.arithmetic = jexl.arithmetic.options(opts);
- if (!arithmetic.getClass().equals(jexl.arithmetic.getClass())) {
- logger.error("expected arithmetic to be " +
jexl.arithmetic.getClass().getSimpleName()
- + ", got " +
arithmetic.getClass().getSimpleName()
- );
- }
- } else {
- this.arithmetic = jexl.arithmetic;
+ JexlArithmetic jexla = jexl.arithmetic;
+ this.arithmetic = jexla.options(context);
+ if (arithmetic != jexla &&
!arithmetic.getClass().equals(jexla.getClass())) {
+ logger.warn("expected arithmetic to be " +
jexla.getClass().getSimpleName()
+ + ", got " + arithmetic.getClass().getSimpleName()
+ );
}
}
+ /**
+ * Copy constructor.
+ * @param ii the base to copy
+ * @param jexla the arithmetic instance to use (or null)
+ */
+ protected InterpreterBase(InterpreterBase ii, JexlArithmetic jexla) {
+ jexl = ii.jexl;
+ logger = ii.logger;
+ uberspect = ii.uberspect;
+ context = ii.context;
+ arithmetic = ii.arithmetic;
+ }
+
+
/** Java7 AutoCloseable interface defined?. */
protected static final Class<?> AUTOCLOSEABLE;
static {
@@ -175,12 +185,8 @@ public abstract class InterpreterBase ex
* @return throws JexlException if strict and not silent, null otherwise
*/
protected Object unsolvableVariable(JexlNode node, String var, boolean
undef) {
- if (isStrictEngine()) {
- if (isSilent()) {
- logger.warn(JexlException.variableError(node, var, undef));
- } else if (undef || arithmetic.isStrict()){
- throw new JexlException.Variable(node, var, undef);
- }
+ if (isStrictEngine() && (undef || arithmetic.isStrict())) {
+ throw new JexlException.Variable(node, var, undef);
} else if (logger.isDebugEnabled()) {
logger.debug(JexlException.variableError(node, var, undef));
}
@@ -195,11 +201,7 @@ public abstract class InterpreterBase ex
*/
protected Object unsolvableMethod(JexlNode node, String method) {
if (isStrictEngine()) {
- if (isSilent()) {
- logger.warn(JexlException.methodError(node, method));
- } else {
- throw new JexlException.Method(node, method);
- }
+ throw new JexlException.Method(node, method);
} else if (logger.isDebugEnabled()) {
logger.debug(JexlException.methodError(node, method));
}
@@ -215,11 +217,7 @@ public abstract class InterpreterBase ex
*/
protected Object unsolvableProperty(JexlNode node, String var, Throwable
cause) {
if (isStrictEngine()) {
- if (isSilent()) {
- logger.warn(JexlException.propertyError(node, var), cause);
- } else {
- throw new JexlException.Property(node, var, cause);
- }
+ throw new JexlException.Property(node, var, cause);
} else if (logger.isDebugEnabled()) {
logger.debug(JexlException.propertyError(node, var), cause);
}
@@ -235,11 +233,7 @@ public abstract class InterpreterBase ex
*/
protected Object operatorError(JexlNode node, JexlOperator operator,
Throwable cause) {
if (isStrictEngine()) {
- if (isSilent()) {
- logger.warn(JexlException.operatorError(node,
operator.getOperatorSymbol()), cause);
- } else {
- throw new JexlException.Operator(node,
operator.getOperatorSymbol(), cause);
- }
+ throw new JexlException.Operator(node,
operator.getOperatorSymbol(), cause);
} else if (logger.isDebugEnabled()) {
logger.debug(JexlException.operatorError(node,
operator.getOperatorSymbol()), cause);
}
@@ -247,35 +241,29 @@ public abstract class InterpreterBase ex
}
/**
- * Triggered when method, function or constructor invocation fails.
- * @param xjexl the JexlException wrapping the original error
+ * Triggered when an annotation processing fails.
+ * @param node the node where the error originated from
+ * @param annotation the annotation name
+ * @param cause the cause of error (if any)
* @return throws a JexlException if strict and not silent, null otherwise
*/
- protected Object invocationFailed(JexlException xjexl) {
- if (xjexl instanceof JexlException.Return
- || xjexl instanceof JexlException.Cancel) {
- throw xjexl;
- }
- if (isStrictEngine()) {
- if (isSilent()) {
- logger.warn(xjexl.getMessage(), xjexl.getCause());
- } else {
- throw xjexl;
- }
+ protected Object annotationError(JexlNode node, String annotation,
Throwable cause) {
+ if (isStrictEngine()) {
+ throw new JexlException.Annotation(node, annotation, cause);
} else if (logger.isDebugEnabled()) {
- logger.debug(xjexl);
+ logger.debug(JexlException.annotationError(node, annotation),
cause);
}
return null;
}
/**
- * Wraps an exception thrown by an invocation.
+ * Triggered when method, function or constructor invocation fails with an
exception.
* @param node the node triggering the exception
* @param methodName the method/function name
* @param xany the cause
- * @return a JexlException
+ * @return a JexlException that will be thrown
*/
- protected JexlException exceptionOnInvocation(JexlNode node, String
methodName, Exception xany) {
+ protected JexlException invocationException(JexlNode node, String
methodName, Exception xany) {
Throwable cause = xany.getCause();
if (cause instanceof JexlException) {
return (JexlException) cause;
@@ -288,30 +276,10 @@ public abstract class InterpreterBase ex
}
/**
- * Triggered when an annotation processing fails.
- * @param node the node where the error originated from
- * @param annotation the annotation name
- * @param cause the cause of error (if any)
- * @return throws a JexlException if strict and not silent, null otherwise
- */
- protected Object annotationError(JexlNode node, String annotation,
Throwable cause) {
- if (isStrictEngine()) {
- if (isSilent()) {
- logger.warn(JexlException.annotationError(node, annotation),
cause);
- } else {
- throw new JexlException.Annotation(node, annotation, cause);
- }
- } else if (logger.isDebugEnabled()) {
- logger.debug(JexlException.annotationError(node, annotation),
cause);
- }
- return null;
- }
-
- /**
* Cancels this evaluation, setting the cancel flag that will result in a
JexlException.Cancel to be thrown.
* @return false if already cancelled, true otherwise
*/
- protected synchronized boolean cancel() {
+ protected synchronized boolean cancel() {
if (cancelled) {
return false;
} else {
Modified: commons/proper/jexl/trunk/src/site/xdoc/changes.xml
URL:
http://svn.apache.org/viewvc/commons/proper/jexl/trunk/src/site/xdoc/changes.xml?rev=1755195&r1=1755194&r2=1755195&view=diff
==============================================================================
--- commons/proper/jexl/trunk/src/site/xdoc/changes.xml (original)
+++ commons/proper/jexl/trunk/src/site/xdoc/changes.xml Thu Aug 4 14:56:15 2016
@@ -25,7 +25,10 @@
<author email="[email protected]">Commons Developers</author>
</properties>
<body>
- <release version="3.0.1" date="unreleased">
+ <release version="3.0.1" date="unreleased">
+ <action dev="henrib" type="fix" issue="JEXL-2010" due-to="Dmitri
Blinov">
+ The way to cancel script execution with an error
+ </action>
<action dev="henrib" type="fix" issue="JEXL-209" due-to="Dmitri
Blinov">
Unsolvable function/method '<?>.<null>(...)'
</action>
@@ -41,9 +44,15 @@
<action dev="henrib" type="fix" issue="JEXL-204" due-to="Dmitri
Blinov">
Script is not interrupted by a method call throwing Exception
</action>
+ <action dev="henrib" type="fix" issue="JEXL-203" due-to="Dmitri
Blinov">
+ JexlArithmetic.options() diverts Interpreter to use default
implementation of JexlArithmetic instead of custom one
+ </action>
<action dev="henrib" type="fix" issue="JEXL-202" due-to="Dmitri
Blinov">
Detect invalid assignment operator usage with non-assignable
l-value during script parsing
</action>
+ <action dev="henrib" type="add" issue="JEXL-201" due-to="Dmitri
Blinov">
+ Allow Interpreter to use live values from JexlEngine.Option
interface implemented by JexlContext
+ </action>
<action dev="henrib" type="fix" issue="JEXL-198" due-to="Terefang
Verigorn">
JxltEngine Template does not expose pragmas
</action>
Modified:
commons/proper/jexl/trunk/src/test/java/org/apache/commons/jexl3/AnnotationTest.java
URL:
http://svn.apache.org/viewvc/commons/proper/jexl/trunk/src/test/java/org/apache/commons/jexl3/AnnotationTest.java?rev=1755195&r1=1755194&r2=1755195&view=diff
==============================================================================
---
commons/proper/jexl/trunk/src/test/java/org/apache/commons/jexl3/AnnotationTest.java
(original)
+++
commons/proper/jexl/trunk/src/test/java/org/apache/commons/jexl3/AnnotationTest.java
Thu Aug 4 14:56:15 2016
@@ -16,6 +16,8 @@
*/
package org.apache.commons.jexl3;
+import java.math.MathContext;
+import java.nio.charset.Charset;
import java.util.Set;
import java.util.TreeSet;
import java.util.concurrent.Callable;
@@ -74,6 +76,88 @@ public class AnnotationTest extends Jexl
}
}
+ public class OptAnnotationContext extends JexlEvalContext implements
JexlContext.AnnotationProcessor {
+ @Override
+ public Object processAnnotation(String name, Object[] args,
Callable<Object> statement) throws Exception {
+ // transient side effect for strict
+ if ("strict".equals(name)) {
+ boolean s = (Boolean) args[0];
+ boolean b = this.isStrict();
+ setStrict(s);
+ Object r = statement.call();
+ setStrict(b);
+ return r;
+ }
+ // transient side effect for silent
+ if ("silent".equals(name)) {
+ boolean s = (Boolean) args[0];
+ boolean b = this.isSilent();
+ setSilent(s);
+ Object r = statement.call();
+ setSilent(b);
+ return r;
+ }
+ // durable side effect for scale
+ if ("scale".equals(name)) {
+ this.setMathScale((Integer) args[0]);
+ return statement.call();
+ }
+ return statement.call();
+ }
+ }
+
+ @Test
+ public void testVarStmt() throws Exception {
+ OptAnnotationContext jc = new OptAnnotationContext();
+ JexlEngine jexl = new
JexlBuilder().strict(true).silent(false).create();
+ jc.setOptions(jexl);
+ JexlScript e;
+ Object r;
+ e = jexl.createScript("(s, v)->{ @strict(s) @silent(v) var x = y ; 42;
}");
+
+ // wont make an error
+ try {
+ r = e.execute(jc, false, true);
+ Assert.assertEquals(42, r);
+ } catch (JexlException.Variable xjexl) {
+ Assert.fail("should not have thrown");
+ }
+
+ r = null;
+ // will make an error and throw
+ try {
+ r = e.execute(jc, true, false);
+ Assert.fail("should have thrown");
+ } catch (JexlException.Variable xjexl) {
+ Assert.assertNull(r);
+ }
+
+ r = null;
+ // will make an error and will not throw but result is null
+ try {
+ r = e.execute(jc, true, true);
+ Assert.assertEquals(null, r);
+ } catch (JexlException.Variable xjexl) {
+ Assert.fail("should not have thrown");
+ }
+
+ r = null;
+ // will not make an error and will not throw
+ try {
+ r = e.execute(jc, false, false);
+ Assert.assertEquals(42, r);
+ } catch (JexlException.Variable xjexl) {
+ Assert.fail("should not have thrown");
+ }
+ //Assert.assertEquals(42, r);
+ Assert.assertTrue(jc.isStrict());
+ e = jexl.createScript("@scale(5) 42;");
+ r = e.execute(jc);
+ Assert.assertEquals(42, r);
+ Assert.assertTrue(jc.isStrict());
+ Assert.assertEquals(5, jc.getArithmeticMathScale());
+ }
+
@Test
public void testNoArg() throws Exception {
AnnotationContext jc = new AnnotationContext();
Modified:
commons/proper/jexl/trunk/src/test/java/org/apache/commons/jexl3/IssuesTest.java
URL:
http://svn.apache.org/viewvc/commons/proper/jexl/trunk/src/test/java/org/apache/commons/jexl3/IssuesTest.java?rev=1755195&r1=1755194&r2=1755195&view=diff
==============================================================================
---
commons/proper/jexl/trunk/src/test/java/org/apache/commons/jexl3/IssuesTest.java
(original)
+++
commons/proper/jexl/trunk/src/test/java/org/apache/commons/jexl3/IssuesTest.java
Thu Aug 4 14:56:15 2016
@@ -1211,8 +1211,8 @@ public class IssuesTest extends JexlTest
JexlScript e = jexl.createScript("var x = 0; var f = (y)->{ x = y; };
f(42); x");
Object r = e.execute(jc);
Assert.assertEquals(0, r);
- }
-
+ }
+
@Test
public void test209a() throws Exception {
JexlContext jc = new MapContext();
@@ -1230,4 +1230,37 @@ public class IssuesTest extends JexlTest
Object r = e.execute(jc);
Assert.assertEquals(1, r);
}
+
+ public class T210 {
+ public void npe() {
+ throw new NullPointerException("NPE");
+ }
+ }
+
+ @Test
+ public void test210() throws Exception {
+ JexlContext jc = new MapContext();
+ jc.set("v210", new T210());
+ JexlEngine jexl;
+ JexlScript e;
+ Object r;
+ jexl = new JexlBuilder().strict(true).silent(false).create();
+ e = jexl.createScript("v210.npe()");
+ try {
+ r = e.execute(jc);
+ Assert.fail("should have thrown an exception");
+ } catch(JexlException xjexl) {
+ Throwable th = xjexl.getCause();
+
Assert.assertTrue(NullPointerException.class.equals(th.getClass()));
+ }
+ jexl = new JexlBuilder().strict(false).silent(false).create();
+ e = jexl.createScript("v210.npe()");
+ try {
+ r = e.execute(jc);
+ Assert.fail("should have thrown an exception");
+ } catch(JexlException xjexl) {
+ Throwable th = xjexl.getCause();
+
Assert.assertTrue(NullPointerException.class.equals(th.getClass()));
+ }
+ }
}
