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 45ba717 JEXL-363: getting captured variables and avoiding caching
mutable literals
45ba717 is described below
commit 45ba71775d7df9762101a36f6894ff4a14425b77
Author: henrib <[email protected]>
AuthorDate: Mon Mar 14 16:32:32 2022 +0100
JEXL-363: getting captured variables and avoiding caching mutable literals
---
.../org/apache/commons/jexl3/internal/Scope.java | 21 +++
.../apache/commons/jexl3/internal/SetBuilder.java | 2 +-
.../apache/commons/jexl3/parser/ASTJexlLambda.java | 6 +
.../apache/commons/jexl3/parser/ASTJexlScript.java | 8 ++
.../org/apache/commons/jexl3/parser/JexlNode.java | 7 +-
.../commons/jexl3/CollectionLiteralTest.java | 159 +++++++++++++++++++++
6 files changed, 200 insertions(+), 3 deletions(-)
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 1bd40a9..733c251 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/Scope.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/Scope.java
@@ -253,6 +253,27 @@ public final class Scope {
}
/**
+ * Gets this script captured symbols names, i.e. local variables defined
in outer scopes and used
+ * by this scope.
+ * @return the captured names
+ */
+ public String[] getCapturedVariables() {
+ if (capturedVariables != null) {
+ final List<String> captured = new ArrayList<String>(vars);
+ for (final Map.Entry<String, Integer> entry :
namedVariables.entrySet()) {
+ final int symnum = entry.getValue();
+ if (symnum >= parms && capturedVariables.containsKey(symnum)) {
+ captured.add(entry.getKey());
+ }
+ }
+ if (!captured.isEmpty()) {
+ return captured.toArray(new String[captured.size()]);
+ }
+ }
+ return EMPTY_STRS;
+ }
+
+ /**
* Gets the (maximum) number of arguments this script expects.
* @return the number of parameters
*/
diff --git a/src/main/java/org/apache/commons/jexl3/internal/SetBuilder.java
b/src/main/java/org/apache/commons/jexl3/internal/SetBuilder.java
index bb33ff2..65430ff 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/SetBuilder.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/SetBuilder.java
@@ -41,7 +41,7 @@ public class SetBuilder implements JexlArithmetic.SetBuilder {
}
@Override
- public Object create() {
+ public Set<?> create() {
return set;
}
}
diff --git a/src/main/java/org/apache/commons/jexl3/parser/ASTJexlLambda.java
b/src/main/java/org/apache/commons/jexl3/parser/ASTJexlLambda.java
index 95d8cb9..3d883c4 100644
--- a/src/main/java/org/apache/commons/jexl3/parser/ASTJexlLambda.java
+++ b/src/main/java/org/apache/commons/jexl3/parser/ASTJexlLambda.java
@@ -34,4 +34,10 @@ public final class ASTJexlLambda extends ASTJexlScript {
public boolean isTopLevel() {
return jjtGetParent() == null;
}
+
+ @Override
+ protected boolean isConstant(final boolean literal) {
+ // a closure is constant if it does not capture any variables
+ return getCapturedVariables().length == 0;
+ }
}
diff --git a/src/main/java/org/apache/commons/jexl3/parser/ASTJexlScript.java
b/src/main/java/org/apache/commons/jexl3/parser/ASTJexlScript.java
index 25eb84e..d7d3046 100644
--- a/src/main/java/org/apache/commons/jexl3/parser/ASTJexlScript.java
+++ b/src/main/java/org/apache/commons/jexl3/parser/ASTJexlScript.java
@@ -169,4 +169,12 @@ public class ASTJexlScript extends JexlLexicalNode {
public boolean isCapturedSymbol(final int symbol) {
return scope != null && scope.isCapturedSymbol(symbol);
}
+
+ /**
+ * Gets this script captured variable, i.e. symbols captured from outer
scopes.
+ * @return the captured variable names
+ */
+ public String[] getCapturedVariables() {
+ return scope != null? scope.getCapturedVariables() : null;
+ }
}
diff --git a/src/main/java/org/apache/commons/jexl3/parser/JexlNode.java
b/src/main/java/org/apache/commons/jexl3/parser/JexlNode.java
index 70ad3c9..6b2bde5 100644
--- a/src/main/java/org/apache/commons/jexl3/parser/JexlNode.java
+++ b/src/main/java/org/apache/commons/jexl3/parser/JexlNode.java
@@ -19,6 +19,8 @@ package org.apache.commons.jexl3.parser;
import org.apache.commons.jexl3.JexlArithmetic;
import org.apache.commons.jexl3.JexlException;
import org.apache.commons.jexl3.JexlInfo;
+import org.apache.commons.jexl3.internal.LexicalScope;
+import org.apache.commons.jexl3.internal.Scope;
import org.apache.commons.jexl3.internal.ScriptVisitor;
import org.apache.commons.jexl3.introspection.JexlMethod;
import org.apache.commons.jexl3.introspection.JexlPropertyGet;
@@ -128,8 +130,9 @@ public abstract class JexlNode extends SimpleNode {
}
/**
- * Whether this node is a constant node Its value can not change after the
first evaluation and can be cached
- * indefinitely.
+ * Whether this node is a constant node.
+ * <p>Its value can not change after the first evaluation and can be cached
+ * indefinitely.</p>
*
* @return true if constant, false otherwise
*/
diff --git a/src/test/java/org/apache/commons/jexl3/CollectionLiteralTest.java
b/src/test/java/org/apache/commons/jexl3/CollectionLiteralTest.java
new file mode 100644
index 0000000..4a242ec
--- /dev/null
+++ b/src/test/java/org/apache/commons/jexl3/CollectionLiteralTest.java
@@ -0,0 +1,159 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.apache.commons.jexl3;
+
+import org.apache.commons.jexl3.internal.ArrayBuilder;
+import org.apache.commons.jexl3.internal.MapBuilder;
+import org.apache.commons.jexl3.internal.SetBuilder;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicInteger;
+/**
+ * Counting the number of times map,sets,array literals are created.
+ * <p>
+ * Originally intended to prove one could cache results of constant map
literals when used to create
+ * libraries (a map of lamba).
+ * However, those literals must be newly instantiated since their result may
be modified; there is
+ * thus no point in trying to cache them. (Think harder and nod, dont try
again.)
+ * These pointless tests as a reminder of 'why' those behave the way they do.
+ * </p>
+ */
+public class CollectionLiteralTest extends JexlTestCase {
+ public CollectionLiteralTest() {
+ super("CollectionLiteralTest");
+ }
+
+
+ public static class Arithmetic363 extends JexlArithmetic {
+ final AtomicInteger maps = new AtomicInteger(0);
+ final AtomicInteger sets = new AtomicInteger(0);
+ final AtomicInteger arrays = new AtomicInteger(0);
+
+ public Arithmetic363(boolean strict) {
+ super(strict);
+ }
+
+ @Override public MapBuilder mapBuilder(int size) {
+ return new CountingMapBuilder(maps, size);
+ }
+ @Override public SetBuilder setBuilder(int size) {
+ return new CountingSetBuilder(sets, size);
+ }
+ @Override public ArrayBuilder arrayBuilder(int size) {
+ return new CountingArrayBuilder(arrays, size);
+ }
+ }
+
+ static class CountingSetBuilder extends SetBuilder {
+ final AtomicInteger count;
+ public CountingSetBuilder(AtomicInteger ai, int size) {
+ super(size);
+ count = ai;
+ }
+ @Override public Set<?> create() {
+ Set<?> set = super.create();
+ count.incrementAndGet();
+ return set;
+ }
+ }
+
+ static class CountingMapBuilder extends MapBuilder {
+ final AtomicInteger count;
+ public CountingMapBuilder(AtomicInteger ai, int size) {
+ super(size);
+ count = ai;
+ }
+ @Override public Map<Object, Object> create() {
+ Map<Object, Object> map = super.create();
+ count.incrementAndGet();
+ return map;
+ }
+ }
+
+ static class CountingArrayBuilder extends ArrayBuilder {
+ final AtomicInteger count;
+
+ public CountingArrayBuilder(AtomicInteger ai, int size) {
+ super(size);
+ count = ai;
+ }
+
+ @Override public Object create(boolean extended) {
+ Object array = super.create(extended);
+ count.incrementAndGet();
+ return array;
+ }
+ }
+
+ @Test
+ public void testMapLBuilder() {
+ Arithmetic363 jc = new Arithmetic363(true);
+ final JexlEngine jexl = new
JexlBuilder().cache(4).arithmetic(jc).create();
+ JexlScript script;
+ Object result;
+
+ script = jexl.createScript("{ 'x':(x)->{ 1 + x; }, 'y' : (y)->{ y - 1;
} }");
+ Object previous = null;
+ for(int i = 0; i < 4; ++i) {
+ result = script.execute(null);
+ Assert.assertNotNull(result);
+ Assert.assertNotSame(previous, result);
+ previous = result;
+ Assert.assertEquals(1 + i, jc.maps.get());
+ }
+ }
+
+ @Test
+ public void testSetBuilder() {
+ Arithmetic363 jc = new Arithmetic363(true);
+ final JexlEngine jexl = new
JexlBuilder().cache(4).arithmetic(jc).create();
+ JexlScript script;
+ Object result;
+
+ script = jexl.createScript("{ (x)->{ 1 + x; }, (y)->{ y - 1; } }");
+ Object previous = null;
+ for(int i = 0; i < 4; ++i) {
+ result = script.execute(null);
+ Assert.assertNotNull(result);
+ Assert.assertNotSame(previous, result);
+ previous = result;
+ Assert.assertEquals(1 + i, jc.sets.get());
+ }
+ }
+
+ @Test
+ public void testArrayBuilder() {
+ Arithmetic363 jc = new Arithmetic363(true);
+ final JexlEngine jexl = new
JexlBuilder().cache(4).arithmetic(jc).create();
+ JexlScript script;
+ Object result;
+
+ script = jexl.createScript("[ (x)->{ 1 + x; }, (y)->{ y - 1; } ]");
+ Object previous = null;
+ for(int i = 0; i < 4; ++i) {
+ result = script.execute(null);
+ Assert.assertNotNull(result);
+ Assert.assertNotSame(previous, result);
+ previous = result;
+ Assert.assertEquals( 1 + i, jc.arrays.get());
+ }
+ }
+
+}
