This is an automated email from the ASF dual-hosted git repository.
henrib pushed a commit to branch JEXL-368
in repository https://gitbox.apache.org/repos/asf/commons-jexl.git
commit 0a5417c68a9722b40b36002f227ab799f7d4f94f
Author: henrib <hen...@apache.org>
AuthorDate: Tue May 3 18:35:03 2022 +0200
JEXL-368: refactored namespace resolution and cache functor properly
---
.../commons/jexl3/internal/InterpreterBase.java | 126 ++++++++++++---------
.../internal/introspection/ConstructorMethod.java | 37 +++---
.../java/org/apache/commons/jexl3/AssignTest.java | 72 ++++++++++--
.../apache/commons/jexl3/ContextNamespaceTest.java | 74 ++++++++++++
4 files changed, 227 insertions(+), 82 deletions(-)
diff --git
a/src/main/java/org/apache/commons/jexl3/internal/InterpreterBase.java
b/src/main/java/org/apache/commons/jexl3/internal/InterpreterBase.java
index ac90cc37..f09ca21f 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/InterpreterBase.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/InterpreterBase.java
@@ -189,74 +189,92 @@ public abstract class InterpreterBase extends
ParserVisitor {
throw new JexlException(node, "no such function namespace " +
prefix, null);
}
}
- // shortcut if ns is known to be not-a-functor
- final boolean cacheable = cache;
- final Object cached = cacheable ? node.jjtGetValue() : null;
- if (cached != JexlContext.NamespaceFunctor.class) {
- // allow namespace to instantiate a functor with context if
possible, not an error otherwise
- Object functor = null;
- if (namespace instanceof JexlContext.NamespaceFunctor) {
- functor = ((JexlContext.NamespaceFunctor)
namespace).createFunctor(context);
- } else if (namespace instanceof Class<?> || namespace instanceof
String) {
- // attempt to reuse last ctor cached in volatile JexlNode.value
- if (cached instanceof JexlMethod) {
- try {
- final Object eval = ((JexlMethod)
cached).tryInvoke(null, context);
- if (JexlEngine.TRY_FAILED != eval) {
- functor = eval;
- }
- } catch (final JexlException.TryFailed xtry) {
- throw new JexlException(node, "unable to instantiate
namespace " + prefix, xtry.getCause());
- }
+ Object functor = null;
+ // class or string (*1)
+ if (namespace instanceof Class<?> || namespace instanceof String) {
+ // the namespace(d) identifier
+ final ASTIdentifier nsNode = (ASTIdentifier) node.jjtGetChild(0);
+ final boolean cacheable = cache && prefix != null;
+ final Object cached = cacheable ? nsNode.jjtGetValue() : null;
+ // we know the class is used as namespace of static methods, no
functor
+ if (cached instanceof Class<?>) {
+ return (Class<?>) cached;
+ }
+ // attempt to reuse last cached constructor
+ if (cached instanceof JexlContext.NamespaceFunctor) {
+ Object eval = ((JexlContext.NamespaceFunctor)
cached).createFunctor(context);
+ if (JexlEngine.TRY_FAILED != eval) {
+ functor = eval;
+ namespace = cached;
}
- // find a ctor with that context class
- if (functor == null) {
- JexlMethod ctor = uberspect.getConstructor(namespace,
context);
+ }
+ if (functor == null) {
+ // find a constructor with that context as argument or without
+ for (int tried = 0; tried < 2; ++tried) {
+ final boolean withContext = tried == 0;
+ final JexlMethod ctor = withContext
+ ? uberspect.getConstructor(namespace, context)
+ : uberspect.getConstructor(namespace);
if (ctor != null) {
try {
- functor = ctor.invoke(namespace, context);
- if (cacheable && ctor.isCacheable()) {
- node.jjtSetValue(ctor);
+ functor = withContext
+ ? ctor.invoke(namespace, context)
+ : ctor.invoke(namespace);
+ // defensive
+ if (functor != null) {
+ // wrap the namespace in a NamespaceFunctor to
shield us from the actual
+ // number of arguments to call it with.
+ final Object ns = namespace;
+ // make it a class (not a lambda!) so
instanceof (see *2) will catch it
+ namespace = new JexlContext.NamespaceFunctor()
{
+ @Override
+ public Object createFunctor(JexlContext
context) {
+ return withContext
+ ? ctor.tryInvoke(null, ns,
context)
+ : ctor.tryInvoke(null, ns);
+ }
+ };
+ if (cacheable && ctor.isCacheable()) {
+ nsNode.jjtSetValue(namespace);
+ }
+ break; // we found a constructor that did
create a functor
}
} catch (final Exception xinst) {
throw new JexlException(node, "unable to
instantiate namespace " + prefix, xinst);
}
}
- // try again; find a ctor with no arg
- if (functor == null) {
- ctor = uberspect.getConstructor(namespace);
- if (ctor != null) {
- try {
- functor = ctor.invoke(namespace);
- } catch (final Exception xinst) {
- throw new JexlException(node, "unable to
instantiate namespace " + prefix, xinst);
- }
- }
- // try again; use a class, namespace of static methods
+ }
+ // did not, will not create a functor instance; use a class,
namespace of static methods
+ if (functor == null) {
+ try {
// try to find a class with that name
- if (functor == null && namespace instanceof String) {
- try {
- namespace =
uberspect.getClassLoader().loadClass((String) namespace);
- } catch (final ClassNotFoundException xignore) {
- // not a class
- namespace = null;
- }
- } // we know it's a class
+ if (namespace instanceof String) {
+ namespace =
uberspect.getClassLoader().loadClass((String) namespace);
+ }
+ // we know it's a class in all cases (see *1)
+ if (cacheable) {
+ nsNode.jjtSetValue(namespace);
+ }
+ } catch (final ClassNotFoundException xignore) {
+ // not a class
+ throw new JexlException(node, "no such class namespace
" + prefix, null);
}
}
}
- // got a functor, store it and return it
- if (functor != null) {
- synchronized (this) {
- if (functors == null) {
- functors = new HashMap<>();
- }
- functors.put(prefix, functor);
+ }
+ // if a namespace functor, instantiate the functor (if not done
already) and store it (*2)
+ if (functor == null && namespace instanceof
JexlContext.NamespaceFunctor) {
+ functor = ((JexlContext.NamespaceFunctor)
namespace).createFunctor(context);
+ }
+ // got a functor, store it and return it
+ if (functor != null) {
+ synchronized (this) {
+ if (functors == null) {
+ functors = new HashMap<>();
}
- return functor;
+ functors.put(prefix, functor);
}
- // use the NamespaceFunctor class to tag this node as not-a-functor
- node.jjtSetValue(JexlContext.NamespaceFunctor.class);
+ return functor;
}
return namespace;
}
diff --git
a/src/main/java/org/apache/commons/jexl3/internal/introspection/ConstructorMethod.java
b/src/main/java/org/apache/commons/jexl3/internal/introspection/ConstructorMethod.java
index bb578730..e38dddab 100644
---
a/src/main/java/org/apache/commons/jexl3/internal/introspection/ConstructorMethod.java
+++
b/src/main/java/org/apache/commons/jexl3/internal/introspection/ConstructorMethod.java
@@ -79,25 +79,28 @@ public final class ConstructorMethod implements JexlMethod {
}
@Override
- public Object tryInvoke(final String name, final Object obj, final
Object... params) {
- try {
- final Class<?> ctorClass = ctor.getDeclaringClass();
- boolean invoke = true;
- if (obj != null) {
- if (obj instanceof Class<?>) {
- invoke = ctorClass.equals(obj);
- } else {
- invoke = ctorClass.getName().equals(obj.toString());
+ public Object tryInvoke(final String name, final Object obj, final
Object... args) {
+ // dont try to invoke if no parameter but call has arguments
+ if (ctor.getParameterCount() > 0 || args.length == 0) {
+ try {
+ final Class<?> ctorClass = ctor.getDeclaringClass();
+ boolean invoke = true;
+ if (obj != null) {
+ if (obj instanceof Class<?>) {
+ invoke = ctorClass.equals(obj);
+ } else {
+ invoke = ctorClass.getName().equals(obj.toString());
+ }
}
+ invoke &= name == null || ctorClass.getName().equals(name);
+ if (invoke) {
+ return ctor.newInstance(args);
+ }
+ } catch (InstantiationException | IllegalArgumentException |
IllegalAccessException xinstance) {
+ return Uberspect.TRY_FAILED;
+ } catch (final InvocationTargetException xinvoke) {
+ throw JexlException.tryFailed(xinvoke); // throw
}
- invoke &= name == null || ctorClass.getName().equals(name);
- if (invoke) {
- return ctor.newInstance(params);
- }
- } catch (InstantiationException | IllegalArgumentException |
IllegalAccessException xinstance) {
- return Uberspect.TRY_FAILED;
- } catch (final InvocationTargetException xinvoke) {
- throw JexlException.tryFailed(xinvoke); // throw
}
return Uberspect.TRY_FAILED;
}
diff --git a/src/test/java/org/apache/commons/jexl3/AssignTest.java
b/src/test/java/org/apache/commons/jexl3/AssignTest.java
index f4d3fcdb..82c40ba1 100644
--- a/src/test/java/org/apache/commons/jexl3/AssignTest.java
+++ b/src/test/java/org/apache/commons/jexl3/AssignTest.java
@@ -19,6 +19,8 @@ package org.apache.commons.jexl3;
import org.junit.Assert;
import org.junit.Test;
+import java.util.Arrays;
+
/**
* Test cases for assignment.
*
@@ -72,10 +74,9 @@ public class AssignTest extends JexlTestCase {
/**
* Make sure bean assignment works
*
- * @throws Exception on any error
*/
@Test
- public void testAntish() throws Exception {
+ public void testAntish() {
final JexlExpression assign = JEXL.createExpression("froboz.value =
10");
final JexlExpression check = JEXL.createExpression("froboz.value");
final JexlContext jc = new MapContext();
@@ -86,7 +87,7 @@ public class AssignTest extends JexlTestCase {
}
@Test
- public void testAntishInteger() throws Exception {
+ public void testAntishInteger() {
final JexlExpression assign = JEXL.createExpression("froboz.0 = 10");
final JexlExpression check = JEXL.createExpression("froboz.0");
final JexlContext jc = new MapContext();
@@ -97,7 +98,7 @@ public class AssignTest extends JexlTestCase {
}
@Test
- public void testBeanish() throws Exception {
+ public void testBeanish() {
final JexlExpression assign = JEXL.createExpression("froboz.value =
10");
final JexlExpression check = JEXL.createExpression("froboz.value");
final JexlContext jc = new MapContext();
@@ -110,7 +111,7 @@ public class AssignTest extends JexlTestCase {
}
@Test
- public void testAmbiguous() throws Exception {
+ public void testAmbiguous() {
final JexlExpression assign = JEXL.createExpression("froboz.nosuchbean
= 10");
final JexlContext jc = new MapContext();
final Froboz froboz = new Froboz(-169);
@@ -129,7 +130,7 @@ public class AssignTest extends JexlTestCase {
}
@Test
- public void testArray() throws Exception {
+ public void testArray() {
final JexlExpression assign = JEXL.createExpression("froboz[\"value\"]
= 10");
final JexlExpression check =
JEXL.createExpression("froboz[\"value\"]");
final JexlContext jc = new MapContext();
@@ -142,7 +143,7 @@ public class AssignTest extends JexlTestCase {
}
@Test
- public void testMini() throws Exception {
+ public void testMini() {
final JexlContext jc = new MapContext();
final JexlExpression assign = JEXL.createExpression("quux = 10");
final Object o = assign.evaluate(jc);
@@ -151,7 +152,7 @@ public class AssignTest extends JexlTestCase {
}
@Test
- public void testMore() throws Exception {
+ public void testMore() {
final JexlContext jc = new MapContext();
jc.set("quuxClass", Quux.class);
final JexlExpression create = JEXL.createExpression("quux =
new(quuxClass, 'xuuq', 100)");
@@ -167,7 +168,7 @@ public class AssignTest extends JexlTestCase {
}
@Test
- public void testUtil() throws Exception {
+ public void testUtil() {
final Quux quux = JEXL.newInstance(Quux.class, "xuuq",
Integer.valueOf(100));
Assert.assertNotNull(quux);
JEXL.setProperty(quux, "froboz.value", Integer.valueOf(100));
@@ -179,7 +180,7 @@ public class AssignTest extends JexlTestCase {
}
@Test
- public void testRejectLocal() throws Exception {
+ public void testRejectLocal() {
final JexlContext jc = new MapContext();
JexlScript assign = JEXL.createScript("var quux = null;
quux.froboz.value = 10");
try {
@@ -194,4 +195,53 @@ public class AssignTest extends JexlTestCase {
final Object o = assign.execute(jc);
Assert.assertEquals(10, o);
}
-}
\ No newline at end of file
+
+ @Test
+ public void testPropertyInError0() {
+ JexlScript script;
+ for(String op : Arrays.asList(
+ " = ", "+= ", " -= "," *= "," /= "," %= ",
+ " &= ", " |= ", " ^= ",
+ " <<= ", " >>= ", " >>>= ")) {
+ script = JEXL.createScript("x -> x.y " +op+ "42");
+ try {
+ script.execute(null, "the_x_value");
+ } catch (final JexlException.Property xprop) {
+ Assert.assertEquals("y", xprop.getProperty());
+ }
+ }
+ script = JEXL.createScript("x -> x.y ");
+ try {
+ script.execute(null, "the_x_value");
+ } catch (final JexlException.Property xprop) {
+ Assert.assertEquals("y", xprop.getProperty());
+ }
+ }
+
+ @Test
+ public void testSetInError1() {
+ try {
+ JEXL.setProperty("the_x_value", "y", 42);
+ } catch (final JexlException.Property xprop) {
+ Assert.assertEquals("y", xprop.getProperty());
+ }
+ try {
+ JEXL.setProperty(null, "y", 42);
+ } catch (final JexlException xprop) {
+ //
+ }
+ }
+ @Test
+ public void testGetInError1() {
+ try {
+ JEXL.getProperty("the_x_value", "y");
+ } catch (final JexlException.Property xprop) {
+ Assert.assertEquals("y", xprop.getProperty());
+ }
+ try {
+ JEXL.getProperty(null, "y");
+ } catch (final JexlException xprop) {
+ //
+ }
+ }
+}
diff --git a/src/test/java/org/apache/commons/jexl3/ContextNamespaceTest.java
b/src/test/java/org/apache/commons/jexl3/ContextNamespaceTest.java
index 942fa95c..ad8d9c1d 100644
--- a/src/test/java/org/apache/commons/jexl3/ContextNamespaceTest.java
+++ b/src/test/java/org/apache/commons/jexl3/ContextNamespaceTest.java
@@ -21,9 +21,11 @@ import org.junit.Ignore;
import org.junit.Test;
import java.util.Arrays;
+import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.TreeMap;
+import java.util.concurrent.atomic.AtomicInteger;
/**
* Tests JexlContext (advanced) features.
@@ -225,6 +227,9 @@ public class ContextNamespaceTest extends JexlTestCase {
ctxt.set("x", "42");
result = script.execute(ctxt);
Assert.assertEquals(169, result);
+ //ctxt.set("x", "42");
+ result = script.execute(ctxt);
+ Assert.assertEquals(169, result);
}
private void run348c(JexlEngine jexl, JexlContext ctxt) {
@@ -318,4 +323,73 @@ public class ContextNamespaceTest extends JexlTestCase {
}
}
+ static AtomicInteger nsnsCtor = new AtomicInteger(0);
+
+ public static class NsNs {
+ private final int constVar;
+ public NsNs(JexlContext ctxt) {
+ nsnsCtor.incrementAndGet();
+ Object n = ctxt.get("NUMBER");
+ constVar = (n instanceof Number) ? ((Number) n).intValue() : -1;
+ }
+
+ public int callIt(int n) {
+ return n + constVar;
+ }
+ }
+
+ @Test
+ public void testNsNsContext0() throws Exception {
+ nsnsCtor.set(0);
+ String clsName = NsNs.class.getName();
+ runNsNsContext(Collections.singletonMap("nsns", clsName));
+ }
+
+ @Test
+ public void testNsNsContext1() throws Exception {
+ nsnsCtor.set(0);
+ runNsNsContext(Collections.singletonMap("nsns", NsNs.class));
+ }
+
+ private void runNsNsContext(Map<String,Object> nsMap) {
+ JexlContext ctxt = new MapContext();
+ ctxt.set("NUMBER", 19);
+ final JexlEngine jexl = new
JexlBuilder().strict(true).silent(false).cache(32)
+ .namespaces(nsMap).create();
+ final JexlScript script = jexl.createScript("x ->{ nsns:callIt(x);
nsns:callIt(x); }");
+ Number result = (Number) script.execute(ctxt, 23);
+ Assert.assertEquals(42, result);
+ Assert.assertEquals(1, nsnsCtor.get());
+ result = (Number) script.execute(ctxt, 623);
+ Assert.assertEquals(642, result);
+ Assert.assertEquals(2, nsnsCtor.get());
+ }
+
+ public static class StaticNs {
+ private StaticNs() { }
+ public static int callIt(int n) {
+ return n + 19;
+ }
+ }
+
+ @Test
+ public void testStaticNs0() {
+ runStaticNsContext(Collections.singletonMap("sns", StaticNs.class));
+ }
+
+ @Test
+ public void testStaticNs1() {
+ runStaticNsContext(Collections.singletonMap("sns",
StaticNs.class.getName()));
+ }
+
+ private void runStaticNsContext(Map<String,Object> nsMap) {
+ JexlContext ctxt = new MapContext();
+ final JexlEngine jexl = new
JexlBuilder().strict(true).silent(false).cache(32)
+ .namespaces(nsMap).create();
+ final JexlScript script = jexl.createScript("x ->{ sns:callIt(x);
sns:callIt(x); }");
+ Number result = (Number) script.execute(ctxt, 23);
+ Assert.assertEquals(42, result);
+ result = (Number) script.execute(ctxt, 623);
+ Assert.assertEquals(642, result);
+ }
}
