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 2019/06/08 13:21:09 UTC
[commons-jexl] 02/02: JEXL-302: added jexl builder flag to control
variable collecting behavior (collectAll);
fixed wrong collecting behavior on array references Task #JEXL-302 -
JexlScript.getVariables returns strange values for array access
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
commit a3c210cfc5eabd1bbb1d6ae4b009fe831e2f1d9b
Author: Henri Biestro <hb...@gmail.com>
AuthorDate: Sat Jun 8 15:20:45 2019 +0200
JEXL-302: added jexl builder flag to control variable collecting behavior (collectAll); fixed wrong collecting behavior on array references
Task #JEXL-302 - JexlScript.getVariables returns strange values for array access
---
.../java/org/apache/commons/jexl3/JexlBuilder.java | 27 ++++++++++--
.../org/apache/commons/jexl3/internal/Engine.java | 50 +++++++++++++++++++---
.../commons/jexl3/internal/TemplateEngine.java | 4 +-
.../commons/jexl3/internal/TemplateScript.java | 2 +-
.../org/apache/commons/jexl3/JexlTestCase.java | 8 ++--
.../java/org/apache/commons/jexl3/VarTest.java | 48 +++++++++++++++++++++
6 files changed, 121 insertions(+), 18 deletions(-)
diff --git a/src/main/java/org/apache/commons/jexl3/JexlBuilder.java b/src/main/java/org/apache/commons/jexl3/JexlBuilder.java
index 38786a8..ad69d67 100644
--- a/src/main/java/org/apache/commons/jexl3/JexlBuilder.java
+++ b/src/main/java/org/apache/commons/jexl3/JexlBuilder.java
@@ -84,13 +84,16 @@ public class JexlBuilder {
/** Whether this engine is in tolerant mode. */
private Boolean safe = false;
-
+
/** Whether error messages will carry debugging information. */
private Boolean debug = null;
/** Whether interrupt throws JexlException.Cancel. */
private Boolean cancellable = null;
+ /** Whether getVariables considers all potential equivalent syntactic forms. */
+ private Boolean collectAll = null;
+
/** The map of 'prefix:function' to object implementing the namespaces. */
private Map<String, Object> namespaces = null;
@@ -99,7 +102,7 @@ public class JexlBuilder {
/** The cache size. */
private int cache = -1;
-
+
/** The stack overflow limit. */
private int stackOverflow = Integer.MAX_VALUE;
@@ -309,7 +312,7 @@ public class JexlBuilder {
public Boolean safe() {
return this.safe;
}
-
+
/**
* Sets whether the engine will report debugging information when error occurs.
*
@@ -348,6 +351,22 @@ public class JexlBuilder {
}
/**
+ * Sets whether the engine variable collectors considers all potential forms of variable syntaxes.
+ *
+ * @param flag true means var collections considers constant array accesses equivalent to dotted references
+ * @return this builder
+ */
+ public JexlBuilder collectAll(boolean flag) {
+ this.collectAll = flag;
+ return this;
+ }
+
+ /** @return true if collect all, false otherwise */
+ public Boolean collectAll() {
+ return this.collectAll;
+ }
+
+ /**
* Sets the default namespaces map the engine will use.
* <p>
* Each entry key is used as a prefix, each entry value used as a bean implementing
@@ -442,7 +461,7 @@ public class JexlBuilder {
public int stackOverflow() {
return stackOverflow;
}
-
+
/**
* @return a {@link JexlEngine} instance
*/
diff --git a/src/main/java/org/apache/commons/jexl3/internal/Engine.java b/src/main/java/org/apache/commons/jexl3/internal/Engine.java
index 475bfcb..a98af2e 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/Engine.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/Engine.java
@@ -149,6 +149,10 @@ public class Engine extends JexlEngine {
* The default jxlt engine.
*/
protected volatile TemplateEngine jxlt = null;
+ /**
+ * Collect all or only dot references.
+ */
+ protected final boolean collectAll;
/**
* Creates an engine with default arguments.
@@ -163,11 +167,12 @@ public class Engine extends JexlEngine {
*/
public Engine(JexlBuilder conf) {
// options:
- this.strict = conf.strict() == null ? true : conf.strict();
- this.safe = conf.safe() == null ? false : conf.safe();
- this.silent = conf.silent() == null ? false : conf.silent();
- this.cancellable = conf.cancellable() == null ? !silent && strict : conf.cancellable();
- this.debug = conf.debug() == null ? true : conf.debug();
+ this.strict = option(conf.strict(), true);
+ this.safe = option(conf.safe(), false);
+ this.silent = option(conf.silent(), false);
+ this.cancellable = option(conf.cancellable(), !silent && strict );
+ this.debug = option(conf.debug(), true);
+ this.collectAll = option(conf.collectAll(), true);
this.stackOverflow = conf.stackOverflow() > 0? conf.stackOverflow() : Integer.MAX_VALUE;
// core properties:
JexlUberspect uber = conf.uberspect() == null ? getUberspect(conf.logger(), conf.strategy()) : conf.uberspect();
@@ -198,6 +203,16 @@ public class Engine extends JexlEngine {
}
/**
+ * Solves an optional option.
+ * @param conf the option as configured, may be null
+ * @param def the default value if null
+ * @return true or false
+ */
+ private boolean option(Boolean conf, boolean def) {
+ return conf == null? def : conf;
+ }
+
+ /**
* Gets the default instance of Uberspect.
* <p>This is lazily initialized to avoid building a default instance if there
* is no use for it. The main reason for not using the default Uberspect instance is to
@@ -493,12 +508,20 @@ public class Engine extends JexlEngine {
* or the empty set if no variables are used
*/
protected Set<List<String>> getVariables(ASTJexlScript script) {
- VarCollector collector = new VarCollector();
+ VarCollector collector = varCollector();
getVariables(script, script, collector);
return collector.collected();
}
/**
+ * Creates a collector instance.
+ * @return a collector instance
+ */
+ protected VarCollector varCollector() {
+ return new VarCollector(this.collectAll);
+ }
+
+ /**
* Utility class to collect variables.
*/
protected static class VarCollector {
@@ -514,6 +537,18 @@ public class Engine extends JexlEngine {
* The node that started the collect.
*/
private JexlNode root = null;
+ /**
+ * Whether constant array-access is considered equivalent to dot-access.
+ */
+ private boolean semantic = true;
+
+ /**
+ * Constructor.
+ * @param constaa whether constant array-access is considered equivalent to dot-access
+ */
+ protected VarCollector(boolean constaa) {
+ semantic = constaa;
+ }
/**
* Starts/stops a variable collect.
@@ -585,7 +620,7 @@ public class Engine extends JexlEngine {
if (collector.isCollecting()) {
collector.add(((ASTIdentifierAccess) node).getName());
}
- } else if (node instanceof ASTArrayAccess) {
+ } else if (node instanceof ASTArrayAccess && collector.semantic) {
int num = node.jjtGetNumChildren();
// collect only if array access is const and follows an identifier
boolean collecting = collector.isCollecting();
@@ -598,6 +633,7 @@ public class Engine extends JexlEngine {
collecting = false;
collector.collect(null);
getVariables(script, child, collector);
+ collector.collect(null);
}
}
} else {
diff --git a/src/main/java/org/apache/commons/jexl3/internal/TemplateEngine.java b/src/main/java/org/apache/commons/jexl3/internal/TemplateEngine.java
index da05161..076ba1d 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/TemplateEngine.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/TemplateEngine.java
@@ -428,7 +428,7 @@ public final class TemplateEngine extends JxltEngine {
@Override
public Set<List<String>> getVariables() {
- Engine.VarCollector collector = new Engine.VarCollector();
+ Engine.VarCollector collector = jexl.varCollector();
getVariables(collector);
return collector.collected();
}
@@ -591,7 +591,7 @@ public final class TemplateEngine extends JxltEngine {
@Override
public Set<List<String>> getVariables() {
- Engine.VarCollector collector = new Engine.VarCollector();
+ Engine.VarCollector collector = jexl.varCollector();
for (TemplateExpression expr : exprs) {
expr.getVariables(collector);
}
diff --git a/src/main/java/org/apache/commons/jexl3/internal/TemplateScript.java b/src/main/java/org/apache/commons/jexl3/internal/TemplateScript.java
index 321d2b8..23ab623 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/TemplateScript.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/TemplateScript.java
@@ -195,7 +195,7 @@ public final class TemplateScript implements JxltEngine.Template {
@Override
public Set<List<String>> getVariables() {
- Engine.VarCollector collector = new Engine.VarCollector();
+ Engine.VarCollector collector = jxlt.getEngine().varCollector();
for (TemplateExpression expr : exprs) {
expr.getVariables(collector);
}
diff --git a/src/test/java/org/apache/commons/jexl3/JexlTestCase.java b/src/test/java/org/apache/commons/jexl3/JexlTestCase.java
index 666b706..2662fc7 100644
--- a/src/test/java/org/apache/commons/jexl3/JexlTestCase.java
+++ b/src/test/java/org/apache/commons/jexl3/JexlTestCase.java
@@ -32,9 +32,9 @@ import org.junit.Assert;
*/
public class JexlTestCase {
/** No parameters signature for test run. */
- private static final Class<?>[] noParms = {};
+ private static final Class<?>[] NO_PARMS = {};
/** String parameter signature for test run. */
- private static final Class<?>[] stringParm = {String.class};
+ private static final Class<?>[] STRING_PARM = {String.class};
/** A default JEXL engine instance. */
protected final JexlEngine JEXL;
@@ -86,7 +86,7 @@ public class JexlTestCase {
}
Method method = null;
try {
- method = this.getClass().getDeclaredMethod(name, noParms);
+ method = this.getClass().getDeclaredMethod(name, NO_PARMS);
}
catch(Exception xany) {
Assert.fail("no such test: " + name);
@@ -127,7 +127,7 @@ public class JexlTestCase {
// find ctor & instantiate
Constructor<JexlTestCase> ctor = null;
try {
- ctor = clazz.getConstructor(stringParm);
+ ctor = clazz.getConstructor(STRING_PARM);
test = ctor.newInstance("debug");
}
catch(NoSuchMethodException xctor) {
diff --git a/src/test/java/org/apache/commons/jexl3/VarTest.java b/src/test/java/org/apache/commons/jexl3/VarTest.java
index df2c440..f0f3126 100644
--- a/src/test/java/org/apache/commons/jexl3/VarTest.java
+++ b/src/test/java/org/apache/commons/jexl3/VarTest.java
@@ -298,6 +298,54 @@ public class VarTest extends JexlTestCase {
vars = e.getVariables();
expect = mkref(new String[][]{{"a", "b"}, {"c"}});
Assert.assertTrue(eq(expect, vars));
+
+ e = JEXL.createScript("a[b].c");
+ vars = e.getVariables();
+ expect = mkref(new String[][]{{"a"}, {"b"}});
+ Assert.assertTrue(eq(expect, vars));
+
+ e = JEXL.createScript("a[b].c[d]");
+ vars = e.getVariables();
+ expect = mkref(new String[][]{{"a"}, {"b"}, {"d"}});
+ Assert.assertTrue(eq(expect, vars));
+
+ e = JEXL.createScript("a[b][e].c[d][f]");
+ vars = e.getVariables();
+ expect = mkref(new String[][]{{"a"}, {"b"}, {"d"}, {"e"}, {"f"}});
+ Assert.assertTrue(eq(expect, vars));
+ }
+
+ @Test
+ public void testVarCollectNotAll() throws Exception {
+ JexlScript e;
+ Set<List<String>> vars;
+ Set<List<String>> expect;
+ JexlEngine jexl = new JexlBuilder().strict(true).silent(false).cache(32).collectAll(false).create();
+
+ e = jexl.createScript("a['b'][c]");
+ vars = e.getVariables();
+ expect = mkref(new String[][]{{"a"}, {"c"}});
+ Assert.assertTrue(eq(expect, vars));
+
+ e = jexl.createScript(" A + B[C] + D[E[F]] + x[y[z]] ");
+ vars = e.getVariables();
+ expect = mkref(new String[][]{{"A"}, {"B"}, {"C"}, {"D"}, {"E"}, {"F"}, {"x"} , {"y"}, {"z"}});
+ Assert.assertTrue(eq(expect, vars));
+
+ e = jexl.createScript("e['f']['g']");
+ vars = e.getVariables();
+ expect = mkref(new String[][]{{"e"}});
+ Assert.assertTrue(eq(expect, vars));
+
+ e = jexl.createScript("a[b][e].c[d][f]");
+ vars = e.getVariables();
+ expect = mkref(new String[][]{{"a"}, {"b"}, {"d"}, {"e"}, {"f"}});
+ Assert.assertTrue(eq(expect, vars));
+
+ e = jexl.createScript("a + b.c + b.c.d + e['f']");
+ vars = e.getVariables();
+ expect = mkref(new String[][]{{"a"}, {"b", "c"}, {"b", "c", "d"}, {"e"}});
+ Assert.assertTrue(eq(expect, vars));
}
@Test