You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by he...@apache.org on 2022/12/27 18:03:28 UTC
[commons-jexl] branch master updated: JEXL-391: refined implementation of 'in' operator in JexlArithmetic; - Refined control of null operands; - Improved Javadoc; - Added tests;
This is an automated email from the ASF dual-hosted git repository.
henrib pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-jexl.git
The following commit(s) were added to refs/heads/master by this push:
new 2078d88e JEXL-391: refined implementation of 'in' operator in JexlArithmetic; - Refined control of null operands; - Improved Javadoc; - Added tests;
2078d88e is described below
commit 2078d88e75f693c055bd54d3bb38faae1a520315
Author: henrib <he...@apache.org>
AuthorDate: Tue Dec 27 19:03:21 2022 +0100
JEXL-391: refined implementation of 'in' operator in JexlArithmetic;
- Refined control of null operands;
- Improved Javadoc;
- Added tests;
---
RELEASE-NOTES.txt | 3 +
src/changes/changes.xml | 9 +++
.../org/apache/commons/jexl3/JexlArithmetic.java | 66 +++++++++++++++-------
.../apache/commons/jexl3/internal/Interpreter.java | 4 ++
.../internal/introspection/ArrayListWrapper.java | 3 +
.../org/apache/commons/jexl3/Arithmetic360.java | 9 +--
.../commons/jexl3/ArithmeticOperatorTest.java | 34 ++++++++++-
.../org/apache/commons/jexl3/ArithmeticTest.java | 22 +++++++-
.../org/apache/commons/jexl3/Issues300Test.java | 27 ++++-----
9 files changed, 135 insertions(+), 42 deletions(-)
diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt
index d7c35449..7a5ae01f 100644
--- a/RELEASE-NOTES.txt
+++ b/RELEASE-NOTES.txt
@@ -40,6 +40,9 @@ the skill set required to write scripts.
New Features in 3.3:
====================
+* JEXL-391: Improve in/=~ operator when arguments are arrays and collections
+* JEXL-390: Pragmas should not be statements
+* JEXL-389: Improve parsing timings
* JEXL-385: Support disabling fortran-style relational operators syntax
* JEXL-382: Simplify grammar and lexical state management
* JEXL-380: Multiple values per pragma key
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index c460ffde..4fbfa918 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -27,6 +27,15 @@
<body>
<release version="3.3" date="YYYY-MM-DD">
<!-- ADD -->
+ <action dev="henrib" type="add" issue="JEXL-391">
+ Improve in/=~ operator when arguments are arrays and collections
+ </action>
+ <action dev="henrib" type="add" issue="JEXL-390" due-to="Dmitri Blinov">
+ Pragmas should not be statements
+ </action>
+ <action dev="henrib" type="add" issue="JEXL-389" due-to="Dmitri Blinov">
+ Improve parsing timings
+ </action>
<action dev="Dmitri Blinov" type="add" issue="JEXL-385" due-to="Dmitri Blinov">
Support disabling fortran-style relational operators syntax
</action>
diff --git a/src/main/java/org/apache/commons/jexl3/JexlArithmetic.java b/src/main/java/org/apache/commons/jexl3/JexlArithmetic.java
index 937069a7..a90204cb 100644
--- a/src/main/java/org/apache/commons/jexl3/JexlArithmetic.java
+++ b/src/main/java/org/apache/commons/jexl3/JexlArithmetic.java
@@ -368,8 +368,8 @@ public class JexlArithmetic {
/**
* Checks if an operand is considered null.
- * @apram val the operand
- * @erturn true if operand is considered null
+ * @param value the operand
+ * @return true if operand is considered null
*/
protected boolean isNullOperand(Object value) {
return value == null;
@@ -384,7 +384,7 @@ public class JexlArithmetic {
*
* @param strictCast whether strict cast is required
* @param defaultValue the default value to return, if not strict
- * @eeturn the default value is strict is false
+ * @return the default value is strict is false
* @throws JexlArithmetic.NullOperand if strict-cast
* @since 3.3
*/
@@ -397,13 +397,13 @@ public class JexlArithmetic {
/**
* The result of +,/,-,*,% when both operands are null.
- * @param strictCast whether strict-cast is required
+ * @param operator the actual operator
* @return Integer(0) if lenient
* @throws JexlArithmetic.NullOperand if strict-cast
* @since 3.3
*/
- protected Object controlNullNullOperands(boolean strictCast) {
- if (strictCast ) {
+ protected Object controlNullNullOperands(JexlOperator operator) {
+ if (isStrict(operator)) {
throw new NullOperand();
}
return 0;
@@ -1201,7 +1201,7 @@ public class JexlArithmetic {
*/
public Object add(final Object left, final Object right) {
if (left == null && right == null) {
- return controlNullNullOperands();
+ return controlNullNullOperands(JexlOperator.ADD);
}
final boolean strconcat = strict
? left instanceof String || right instanceof String
@@ -1257,7 +1257,7 @@ public class JexlArithmetic {
*/
public Object divide(final Object left, final Object right) {
if (left == null && right == null) {
- return controlNullNullOperands();
+ return controlNullNullOperands(JexlOperator.DIVIDE);
}
// if both (non-null) args fit as long
final Number ln = asLongNumber(left);
@@ -1311,7 +1311,7 @@ public class JexlArithmetic {
*/
public Object mod(final Object left, final Object right) {
if (left == null && right == null) {
- return controlNullNullOperands();
+ return controlNullNullOperands(JexlOperator.MOD);
}
// if both (non-null) args fit as long
final Number ln = asLongNumber(left);
@@ -1381,7 +1381,7 @@ public class JexlArithmetic {
*/
public Object multiply(final Object left, final Object right) {
if (left == null && right == null) {
- return controlNullNullOperands();
+ return controlNullNullOperands(JexlOperator.MULTIPLY);
}
// if both (non-null) args fit as int
final Number ln = asLongNumber(left);
@@ -1426,7 +1426,7 @@ public class JexlArithmetic {
*/
public Object subtract(final Object left, final Object right) {
if (left == null && right == null) {
- return controlNullNullOperands();
+ return controlNullNullOperands(JexlOperator.SUBTRACT);
}
// if both (non-null) args fit as long
final Number ln = asLongNumber(left);
@@ -1574,9 +1574,11 @@ public class JexlArithmetic {
/**
* Test if left contains right (right matches/in left).
- * <p>Beware that this method arguments are the opposite of the operator arguments.
- * 'x in y' means 'y contains x'.</p>
- *
+ * <p>Beware that this "contains " method arguments order is the opposite of the
+ * "in/matches" operator arguments.
+ * <code>x =~ y</code> means <code>y contains x</code> thus <code>contains(x, y)</code>.</p>
+ * <p>When this method returns null during evaluation, the operator code continues trying to find
+ * one through the uberspect.</p>
* @param container the container
* @param value the value
* @return test result or null if there is no arithmetic solution
@@ -1605,16 +1607,42 @@ public class JexlArithmetic {
return ((Map<?, ?>) container).containsKey(value);
}
// try contains on collection
- if (container instanceof Collection<?>) {
- if (value instanceof Collection<?>) {
- return ((Collection<?>) container).containsAll((Collection<?>) value);
+ return collectionContains(container, value);
+ }
+
+ /**
+ * Checks whether a potential collection contains another.
+ * <p>Made protected to make it easier to override if needed.</p>
+ * @param collection the container which can be a collection or an array (even of primitive)
+ * @param value the value which can be a collection or an array (even of primitive) or a singleton
+ * @return test result or null if there is no arithmetic solution
+ */
+ protected Boolean collectionContains(final Object collection, final Object value) {
+ // convert arrays if needed
+ final Object left = arrayWrap(collection);
+ if (left instanceof Collection<?>) {
+ final Object right = arrayWrap(value);
+ if (right instanceof Collection<?>) {
+ return ((Collection<?>) left).containsAll((Collection<?>) right);
}
- // left in right ? <=> right.contains(left) ?
- return ((Collection<?>) container).contains(value);
+ return ((Collection<?>) left).contains(value);
}
return null;
}
+ /**
+ * Attempts transformation of potential array in an abstract list or leave as is.
+ * <p>An array (as in int[]) is not convenient to call methods so when encountered we turn them into lists</p>
+ * @param container an array or on object
+ * @return an abstract list wrapping the array instance or the initial argument
+ * @see org.apache.commons.jexl3.internal.introspection.ArrayListWrapper
+ */
+ private static Object arrayWrap(final Object container) {
+ return container.getClass().isArray()
+ ? new org.apache.commons.jexl3.internal.introspection.ArrayListWrapper(container)
+ : container;
+ }
+
/**
* Test if left ends with right.
*
diff --git a/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java b/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java
index 29cad932..ae9cb89d 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java
@@ -438,6 +438,8 @@ public class Interpreter extends InterpreterBase {
protected Object visit(final ASTERNode node, final Object data) {
final Object left = node.jjtGetChild(0).jjtAccept(this, data);
final Object right = node.jjtGetChild(1).jjtAccept(this, data);
+ // note the arguments inversion between 'in'/'matches' and 'contains'
+ // if x in y then y contains x
return operators.contains(node, "=~", right, left);
}
@@ -445,6 +447,8 @@ public class Interpreter extends InterpreterBase {
protected Object visit(final ASTNRNode node, final Object data) {
final Object left = node.jjtGetChild(0).jjtAccept(this, data);
final Object right = node.jjtGetChild(1).jjtAccept(this, data);
+ // note the arguments inversion between (not) 'in'/'matches' and (not) 'contains'
+ // if x not-in y then y not-contains x
return !operators.contains(node, "!~", right, left);
}
diff --git a/src/main/java/org/apache/commons/jexl3/internal/introspection/ArrayListWrapper.java b/src/main/java/org/apache/commons/jexl3/internal/introspection/ArrayListWrapper.java
index 65177ecb..067bfb0b 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/introspection/ArrayListWrapper.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/introspection/ArrayListWrapper.java
@@ -18,6 +18,7 @@ package org.apache.commons.jexl3.internal.introspection;
import java.lang.reflect.Array;
import java.util.AbstractList;
+import java.util.Iterator;
import java.util.RandomAccess;
/**
@@ -83,4 +84,6 @@ public class ArrayListWrapper extends AbstractList<Object> implements RandomAcce
public boolean contains(final Object o) {
return indexOf(o) != -1;
}
+ @Override
+ public Iterator<Object> iterator() { return new ArrayIterator(array); }
}
diff --git a/src/test/java/org/apache/commons/jexl3/Arithmetic360.java b/src/test/java/org/apache/commons/jexl3/Arithmetic360.java
index a44ec333..a0028d22 100644
--- a/src/test/java/org/apache/commons/jexl3/Arithmetic360.java
+++ b/src/test/java/org/apache/commons/jexl3/Arithmetic360.java
@@ -163,8 +163,7 @@ public class Arithmetic360 extends JexlArithmetic {
@Override
public Object shiftLeft(Object left, Object right) {
if (left == null && right == null) {
- boolean strictCast = isStrict(JexlOperator.SHIFTLEFT);
- return controlNullNullOperands(strictCast);
+ return controlNullNullOperands(JexlOperator.SHIFTLEFT);
}
final int r = toInteger(right);
Number l = asIntNumber(left);
@@ -188,8 +187,7 @@ public class Arithmetic360 extends JexlArithmetic {
@Override
public Object shiftRight(Object left, Object right) {
if (left == null && right == null) {
- boolean strictCast = isStrict(JexlOperator.SHIFTRIGHT);
- return controlNullNullOperands(strictCast);
+ return controlNullNullOperands(JexlOperator.SHIFTRIGHT);
}
final int r = toInteger(right);
Number l = asIntNumber(left);
@@ -213,8 +211,7 @@ public class Arithmetic360 extends JexlArithmetic {
@Override
public Object shiftRightUnsigned(Object left, Object right) {
if (left == null && right == null) {
- boolean strictCast = isStrict(JexlOperator.SHIFTRIGHTU);
- return controlNullNullOperands(strictCast);
+ return controlNullNullOperands(JexlOperator.SHIFTRIGHTU);
}
final int r = toInteger(right);
Number l = asIntNumber(left);
diff --git a/src/test/java/org/apache/commons/jexl3/ArithmeticOperatorTest.java b/src/test/java/org/apache/commons/jexl3/ArithmeticOperatorTest.java
index 930a3d81..c4e48c70 100644
--- a/src/test/java/org/apache/commons/jexl3/ArithmeticOperatorTest.java
+++ b/src/test/java/org/apache/commons/jexl3/ArithmeticOperatorTest.java
@@ -17,6 +17,7 @@
package org.apache.commons.jexl3;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
@@ -25,7 +26,6 @@ import java.util.Map;
import java.util.Set;
import java.util.SortedSet;
import java.util.TreeSet;
-import java.util.Collections;
import java.util.Calendar;
import java.util.Date;
import java.util.Locale;
@@ -93,12 +93,41 @@ public class ArithmeticOperatorTest extends JexlTestCase {
asserter.assertExpression("'z' !~ ['a','b','c','d','e','f']", Boolean.TRUE);
}
+ @Test
+ public void test391() throws Exception {
+ // with literals
+ for(String src : Arrays.asList(
+ "2 =~ [1, 2, 3, 4]",
+ "[2, 3] =~ [1, 2, 3, 4]",
+ "[2, 3,...] =~ [1, 2, 3, 4]",
+ "3 =~ [1, 2, 3, 4,...]",
+ "[2, 3] =~ [1, 2, 3, 4,...]",
+ "[2, 3,...] =~ [1, 2, 3, 4,...]")) {
+ asserter.assertExpression(src, Boolean.TRUE);
+ }
+ // with variables
+ int[] ic = new int[]{1, 2, 3, 4};
+ List<Integer> iic = new ArrayList<>();
+ for(int v : ic) { iic.add(v); }
+ int[] iv = new int[]{2, 3};
+ List<Integer> iiv = new ArrayList<>();
+ for(int v : iv) { iiv.add(v); }
+ String src = "(x,y) -> x =~ y ";
+ for(Object v : Arrays.asList(iv, iiv, 2)) {
+ for(Object c : Arrays.asList(ic, iic)) {
+ asserter.assertExpression(src, Boolean.TRUE, v, c);
+ }
+ }
+ }
+
@Test
public void testRegexp2() throws Exception {
asserter.setVariable("str", "abc456");
asserter.assertExpression("str =~ ~/.*456/", Boolean.TRUE);
asserter.assertExpression("str !~ ~/ABC.*/", Boolean.TRUE);
asserter.assertExpression("str =~ ~/abc\\d{3}/", Boolean.TRUE);
+ // legacy, deprecated
+ asserter.assertExpression("matches(str, ~/.*456/)", Boolean.TRUE);
asserter.setVariable("str", "4/6");
asserter.assertExpression("str =~ ~/\\d\\/\\d/", Boolean.TRUE);
}
@@ -198,7 +227,8 @@ public class ArithmeticOperatorTest extends JexlTestCase {
}
public boolean contains(final int[] i) {
- return values.containsAll(Collections.singletonList(i));
+ for(int ii : i) if (!values.contains(ii)) return false;
+ return true;
}
public boolean startsWith(final int i) {
diff --git a/src/test/java/org/apache/commons/jexl3/ArithmeticTest.java b/src/test/java/org/apache/commons/jexl3/ArithmeticTest.java
index 451e1db1..7f24ee0c 100644
--- a/src/test/java/org/apache/commons/jexl3/ArithmeticTest.java
+++ b/src/test/java/org/apache/commons/jexl3/ArithmeticTest.java
@@ -1314,6 +1314,20 @@ public class ArithmeticTest extends JexlTestCase {
Assert.assertEquals(36L, result);
result = script.execute(jc, new Var(35), new Var(7));
Assert.assertEquals(36L, ((Var) result).value);
+ // legacy
+ script = jexl.createScript("(x, y)->bitwiseXor(x,y)");
+ result = script.execute(jc, 35, 7);
+ Assert.assertEquals(36L, result);
+
+ script = jexl.createScript("(x, y)->{ x | y }");
+ result = script.execute(jc, 35, 7);
+ Assert.assertEquals(39L, result);
+ result = script.execute(jc, new Var(35), new Var(7));
+ Assert.assertEquals(39L, ((Var) result).value);
+ // legacy
+ script = jexl.createScript("(x, y)->bitwiseOr(x,y)");
+ result = script.execute(jc, 35, 7);
+ Assert.assertEquals(39L, result);
script = jexl.createScript("(x, y)->{ x << y }");
result = script.execute(jc, 35, 1);
@@ -1338,6 +1352,10 @@ public class ArithmeticTest extends JexlTestCase {
Assert.assertEquals(3L, result);
result = script.execute(jc, new Var(35), new Var(7));
Assert.assertEquals(3L, ((Var) result).value);
+ // legacy
+ script = jexl.createScript("(x, y)->bitwiseAnd(x,y)");
+ result = script.execute(jc, 35, 7);
+ Assert.assertEquals(3L, result);
script = jexl.createScript("(x, y)->{ x =^ y }");
result = script.execute(jc, 3115, 31);
@@ -1441,7 +1459,7 @@ public class ArithmeticTest extends JexlTestCase {
@Override
public Object divide(final Object left, final Object right) {
if (left == null && right == null) {
- return controlNullNullOperands();
+ return controlNullNullOperands(JexlOperator.DIVIDE);
}
// if either are bigdecimal use that type
if (left instanceof BigDecimal || right instanceof BigDecimal) {
@@ -1472,7 +1490,7 @@ public class ArithmeticTest extends JexlTestCase {
@Override
public Object mod(final Object left, final Object right) {
if (left == null && right == null) {
- return controlNullNullOperands();
+ return controlNullNullOperands(JexlOperator.MOD);
}
// if either are bigdecimal use that type
if (left instanceof BigDecimal || right instanceof BigDecimal) {
diff --git a/src/test/java/org/apache/commons/jexl3/Issues300Test.java b/src/test/java/org/apache/commons/jexl3/Issues300Test.java
index e742d4e4..56839de3 100644
--- a/src/test/java/org/apache/commons/jexl3/Issues300Test.java
+++ b/src/test/java/org/apache/commons/jexl3/Issues300Test.java
@@ -1036,26 +1036,26 @@ public class Issues300Test {
// local var
JexlScript s0 = jexl.createScript(src0, "a");
JexlScript s1 = jexl.createScript(src1, "a");
- Assert.assertEquals(2, s0.execute(null, null));
- Assert.assertEquals(1, s1.execute(null, null));
+ Assert.assertEquals(2, s0.execute(null, (Object) null));
+ Assert.assertEquals(1, s1.execute(null, (Object) null));
// global var undefined
s0 = jexl.createScript(src0);
s1 = jexl.createScript(src1);
try {
- Assert.assertEquals(2, s0.execute(null, null));
+ Assert.assertEquals(2, s0.execute(null, (Object) null));
} catch(JexlException.Variable xvar) {
Assert.assertEquals("a", xvar.getVariable());
}
try {
- Assert.assertEquals(1, s1.execute(null, null));
+ Assert.assertEquals(1, s1.execute(null, (Object) null));
} catch(JexlException.Variable xvar) {
Assert.assertEquals("a", xvar.getVariable());
}
// global var null
MapContext ctxt = new MapContext();
ctxt.set("a", null);
- Assert.assertEquals(2, s0.execute(ctxt, null));
- Assert.assertEquals(1, s1.execute(ctxt, null));
+ Assert.assertEquals(2, s0.execute(ctxt, (Object) null));
+ Assert.assertEquals(1, s1.execute(ctxt, (Object) null));
}
public static class Arithmetic384 extends JexlArithmetic {
@@ -1082,7 +1082,7 @@ public class Issues300Test {
JexlContext ctxt = new MapContext();
JexlScript s0 = jexl.createScript(src0);
try {
- s0.execute(ctxt, null);
+ s0.execute(ctxt, (Object) null);
Assert.fail("null argument should throw");
} catch (JexlException xvar) {
Assert.assertTrue(xvar.toString().contains("+"));
@@ -1093,7 +1093,7 @@ public class Issues300Test {
JexlContext ctxt = new MapContext();
JexlScript s1 = jexl.createScript(src1, "a");
try {
- s1.execute(ctxt, null);
+ s1.execute(ctxt, (Object) null);
Assert.fail("null argument should throw");
} catch (JexlException.Variable xvar) {
Assert.assertEquals("a", xvar.getVariable());
@@ -1101,7 +1101,7 @@ public class Issues300Test {
// undefined a
s1 = jexl.createScript(src1);
try {
- s1.execute(ctxt, null);
+ s1.execute(ctxt, (Object) null);
Assert.fail("null argument should throw");
} catch (JexlException.Variable xvar) {
Assert.assertEquals("a", xvar.getVariable());
@@ -1110,7 +1110,7 @@ public class Issues300Test {
// null a
ctxt.set("a", null);
try {
- s1.execute(ctxt, null);
+ s1.execute(ctxt, (Object) null);
Assert.fail("null argument should throw");
} catch (JexlException.Variable xvar) {
Assert.assertEquals("a", xvar.getVariable());
@@ -1136,11 +1136,11 @@ public class Issues300Test {
for(String src1 : Arrays.asList("'ABC' + a", "a + 'ABC'")) {
JexlContext ctxt = new MapContext();
JexlScript s1 = jexl.createScript(src1, "a");
- Assert.assertEquals("ABC", s1.execute(ctxt, null));
+ Assert.assertEquals("ABC", s1.execute(ctxt, (Object) null));
// undefined a
s1 = jexl.createScript(src1);
try {
- s1.execute(ctxt, null);
+ s1.execute(ctxt, (Object) null);
Assert.fail("null argument should throw");
} catch (JexlException.Variable xvar) {
Assert.assertEquals("a", xvar.getVariable());
@@ -1148,7 +1148,7 @@ public class Issues300Test {
}
// null a
ctxt.set("a", null);
- Assert.assertEquals("ABC", s1.execute(ctxt, null));
+ Assert.assertEquals("ABC", s1.execute(ctxt, (Object) null));
}
}
@Test
@@ -1172,4 +1172,5 @@ public class Issues300Test {
Object result = script.execute(null);
debuggerCheck(jexl);
}
+
}