commons-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From hen...@apache.org
Subject svn commit: r1755195 - in /commons/proper/jexl/trunk: ./ src/main/java/org/apache/commons/jexl3/internal/ src/site/xdoc/ src/test/java/org/apache/commons/jexl3/
Date Thu, 04 Aug 2016 14:56:16 GMT
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="dev@commons.apache.org">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 '&lt;?&gt;.&lt;null&gt;(...)'
             </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()));
+        }
+    }
 }



Mime
View raw message