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 2023/02/16 19:03:40 UTC
[commons-jexl] branch master updated: JEXL-384: restore potentially broken upwards compatibility;
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 ebc54cea JEXL-384: restore potentially broken upwards compatibility;
ebc54cea is described below
commit ebc54ceafab85733881f8313c9df9af6a7461c41
Author: henrib <he...@apache.org>
AuthorDate: Thu Feb 16 20:03:32 2023 +0100
JEXL-384: restore potentially broken upwards compatibility;
---
.../org/apache/commons/jexl3/JexlArithmetic.java | 37 +++++++++++--
.../org/apache/commons/jexl3/internal/Closure.java | 32 -----------
.../org/apache/commons/jexl3/internal/Frame.java | 17 ------
.../org/apache/commons/jexl3/internal/Scope.java | 23 --------
.../org/apache/commons/jexl3/Issues300Test.java | 63 ++++++++++++++++++++++
5 files changed, 97 insertions(+), 75 deletions(-)
diff --git a/src/main/java/org/apache/commons/jexl3/JexlArithmetic.java b/src/main/java/org/apache/commons/jexl3/JexlArithmetic.java
index 8693671b..75f50ec3 100644
--- a/src/main/java/org/apache/commons/jexl3/JexlArithmetic.java
+++ b/src/main/java/org/apache/commons/jexl3/JexlArithmetic.java
@@ -22,6 +22,7 @@ import org.apache.commons.jexl3.introspection.JexlMethod;
import java.lang.reflect.Array;
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
import java.math.BigDecimal;
import java.math.BigInteger;
import java.math.MathContext;
@@ -61,8 +62,8 @@ public class JexlArithmetic {
/** Marker class for null operand exceptions. */
public static class NullOperand extends ArithmeticException {
-
- private static final long serialVersionUID = 1L;}
+ private static final long serialVersionUID = 1L;
+ }
/** Double.MAX_VALUE as BigDecimal. */
protected static final BigDecimal BIGD_DOUBLE_MAX_VALUE = BigDecimal.valueOf(Double.MAX_VALUE);
@@ -1873,6 +1874,8 @@ public class JexlArithmetic {
}
/**
+ * Any override of this method (pre 3.3) should be modified to match the new signature.
+ * @link JexlArithmetic.compare(String, String, JexlOperator);
* @deprecated 3.3
*/
@Deprecated
@@ -1884,7 +1887,27 @@ public class JexlArithmetic {
// ignore
operator = JexlOperator.EQ;
}
- return compare(left, right, operator);
+ return doCompare(left, right, operator);
+ }
+
+ /**
+ * Determines if the compare method(Object, Object, String) is overriden in this class or one of its
+ * superclasses.
+ */
+ private final boolean compare321 = computeCompare321(this);
+ private static boolean computeCompare321(final JexlArithmetic arithmetic) {
+ Class<?> arithmeticClass = arithmetic.getClass();
+ while(arithmeticClass != JexlArithmetic.class) {
+ try {
+ Method cmp = arithmeticClass.getDeclaredMethod("compare", Object.class, Object.class, String.class);
+ if (cmp != null && cmp.getDeclaringClass() != JexlArithmetic.class) {
+ return true;
+ }
+ } catch (NoSuchMethodException xany) {
+ arithmeticClass = arithmeticClass.getSuperclass();
+ }
+ }
+ return false;
}
/**
@@ -1897,6 +1920,14 @@ public class JexlArithmetic {
* @throws ArithmeticException if either left or right is null
*/
protected int compare(final Object left, final Object right, final JexlOperator operator) {
+ // this is a temporary way of allowing pre-3.3 code that overrode compare() to still call
+ // the user method. This method will merge with doCompare in 3.4 and the compare321 flag will disappear.
+ return compare321
+ ? compare(left, right, operator.toString())
+ : doCompare(left, right, operator);
+ }
+
+ private int doCompare(final Object left, final Object right, final JexlOperator operator) {
final boolean strictCast = isStrict(operator);
if (left != null && right != null) {
if (left instanceof BigDecimal || right instanceof BigDecimal) {
diff --git a/src/main/java/org/apache/commons/jexl3/internal/Closure.java b/src/main/java/org/apache/commons/jexl3/internal/Closure.java
index 0c6a2376..599ff1c0 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/Closure.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/Closure.java
@@ -61,38 +61,6 @@ public class Closure extends Script {
options = closureOptions != null ? closureOptions.copy() : null;
}
- @Override
- public int hashCode() {
- // CSOFF: Magic number
- int hash = 17;
- hash = 31 * hash + (this.jexl != null ? this.jexl.hashCode() : 0);
- hash = 31 * hash + (this.source != null ? this.source.hashCode() : 0);
- hash = 31 * hash + (this.frame != null ? this.frame.hashCode() : 0);
- // CSON: Magic number
- return hash;
- }
-
- @Override
- public boolean equals(final Object obj) {
- if (obj == null) {
- return false;
- }
- if (getClass() != obj.getClass()) {
- return false;
- }
- final Closure other = (Closure) obj;
- if (this.jexl != other.jexl) {
- return false;
- }
- if (!Objects.equals(this.source, other.source)) {
- return false;
- }
- if (!Objects.equals(this.frame, other.frame)) {
- return false;
- }
- return true;
- }
-
@Override
public String[] getUnboundParameters() {
return frame.getUnboundParameters();
diff --git a/src/main/java/org/apache/commons/jexl3/internal/Frame.java b/src/main/java/org/apache/commons/jexl3/internal/Frame.java
index 6fc58d97..9101a95f 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/Frame.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/Frame.java
@@ -58,23 +58,6 @@ public final class Frame {
return scope;
}
- @Override
- public int hashCode() {
- return Arrays.deepHashCode(this.stack);
- }
-
- @Override
- public boolean equals(final Object obj) {
- if (obj == null) {
- return false;
- }
- if (getClass() != obj.getClass()) {
- return false;
- }
- final Frame other = (Frame) obj;
- return Arrays.deepEquals(this.stack, other.stack);
- }
-
/**
* Gets a value.
* @param s the offset in this frame
diff --git a/src/main/java/org/apache/commons/jexl3/internal/Scope.java b/src/main/java/org/apache/commons/jexl3/internal/Scope.java
index a82ce321..1572468f 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/Scope.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/Scope.java
@@ -95,29 +95,6 @@ public final class Scope {
parent = scope;
}
- @Override
- public int hashCode() {
- return namedVariables == null ? 0 : parms ^ namedVariables.hashCode();
- }
-
- @Override
- public boolean equals(final Object o) {
- if (this == o) {
- return true;
- }
- if (!(o instanceof Scope)) {
- return false;
- }
- final Scope scope = (Scope) o;
- if (parms != scope.parms) {
- return false;
- }
- if (namedVariables == null) {
- return scope.namedVariables == null;
- }
- return namedVariables.equals(scope.namedVariables);
- }
-
/**
* Checks whether an identifier is a local variable or argument, ie a symbol.
* If this fails, look in parents for symbol that can be captured.
diff --git a/src/test/java/org/apache/commons/jexl3/Issues300Test.java b/src/test/java/org/apache/commons/jexl3/Issues300Test.java
index 56839de3..6a418f04 100644
--- a/src/test/java/org/apache/commons/jexl3/Issues300Test.java
+++ b/src/test/java/org/apache/commons/jexl3/Issues300Test.java
@@ -24,6 +24,7 @@ import org.junit.Test;
import java.io.StringReader;
import java.io.StringWriter;
+import java.math.MathContext;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
@@ -31,6 +32,7 @@ import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
+import java.util.concurrent.atomic.AtomicInteger;
import java.util.stream.Collectors;
import java.util.stream.Stream;
@@ -1173,4 +1175,65 @@ public class Issues300Test {
debuggerCheck(jexl);
}
+ public static class Arithmetic384c extends JexlArithmetic {
+ AtomicInteger cmp = new AtomicInteger(0);
+ int getCmpCalls() {
+ return cmp.get();
+ }
+ public Arithmetic384c(boolean astrict) {
+ super(astrict);
+ }
+ public Arithmetic384c(boolean astrict, MathContext bigdContext, int bigdScale) {
+ super(astrict, bigdContext, bigdScale);
+ }
+ @Override
+ protected int compare(Object l, Object r, String op) {
+ cmp.incrementAndGet();
+ return super.compare(l, r, op);
+ }
+ }
+
+ public static class Arithmetic384d extends Arithmetic384c {
+ public Arithmetic384d(boolean astrict) {
+ super(astrict);
+ }
+ public Arithmetic384d(boolean astrict, MathContext bigdContext, int bigdScale) {
+ super(astrict, bigdContext, bigdScale);
+ }
+ }
+
+ @Test
+ public void test384c() {
+ Arithmetic384c ja = new Arithmetic384c(true);
+ JexlEngine jexl = new JexlBuilder()
+ .safe(false)
+ .strict(true)
+ .arithmetic(ja)
+ .create();
+ Assert.assertTrue(ja.toBoolean(jexl.createExpression("3 < 4").evaluate(null)));
+ Assert.assertTrue(ja.toBoolean(jexl.createExpression("6 <= 8").evaluate(null)));
+ Assert.assertFalse(ja.toBoolean(jexl.createExpression("6 == 7").evaluate(null)));
+ Assert.assertTrue(ja.toBoolean(jexl.createExpression("4 > 2").evaluate(null)));
+ Assert.assertTrue(ja.toBoolean(jexl.createExpression("8 > 6").evaluate(null)));
+ Assert.assertTrue(ja.toBoolean(jexl.createExpression("7 != 6").evaluate(null)));
+ Assert.assertEquals(6, ja.getCmpCalls());
+ }
+
+ @Test
+ public void test384d() {
+ Arithmetic384c ja = new Arithmetic384d(true);
+ JexlEngine jexl = new JexlBuilder()
+ .safe(false)
+ .strict(true)
+ .arithmetic(ja)
+ .create();
+ Assert.assertTrue(ja.toBoolean(jexl.createExpression("3 < 4").evaluate(null)));
+ Assert.assertTrue(ja.toBoolean(jexl.createExpression("6 <= 8").evaluate(null)));
+ Assert.assertFalse(ja.toBoolean(jexl.createExpression("6 == 7").evaluate(null)));
+ Assert.assertTrue(ja.toBoolean(jexl.createExpression("4 > 2").evaluate(null)));
+ Assert.assertTrue(ja.toBoolean(jexl.createExpression("8 > 6").evaluate(null)));
+ Assert.assertTrue(ja.toBoolean(jexl.createExpression("7 != 6").evaluate(null)));
+ Assert.assertEquals(6, ja.getCmpCalls());
+ }
+
}