From d6732394266001b7cc8d5d03d76f7d121c9e8804 Mon Sep 17 00:00:00 2001 From: Robert Alexandersson Date: Wed, 23 Mar 2016 09:54:57 +0100 Subject: [PATCH] - Added support for a new flag (failOnCacheResize) to make it easier to see when the cache is to small in dev / test. - Added support for Incompatible "void" returns on method invocation. - Improved the Value Tree to allow scenarios similar to the el 2.2 implementation - Added support for empty string defaults even for objects that are not strings. --- .../de/odysseus/el/ExpressionFactoryImpl.java | 17 ++- .../de/odysseus/el/TreeValueExpression.java | 13 ++- .../java/de/odysseus/el/tree/impl/Cache.java | 10 ++ .../java/de/odysseus/el/tree/impl/Parser.java | 2 +- .../odysseus/el/tree/impl/ast/AstObject.java | 101 ++++++++++++++++++ .../el/tree/impl/ast/AstProperty.java | 12 ++- .../de/odysseus/el/tree/impl/ast/AstText.java | 2 +- .../el/tree/impl/ast/AstTextForNull.java | 10 ++ .../odysseus/el/TreeMethodExpressionTest.java | 13 +++ .../el/tree/impl/ast/AstDateTest.java | 40 +++++++ .../el/tree/impl/ast/AstIdentifierTest.java | 9 +- 11 files changed, 222 insertions(+), 7 deletions(-) create mode 100644 modules/impl/src/main/java/de/odysseus/el/tree/impl/ast/AstObject.java create mode 100644 modules/impl/src/main/java/de/odysseus/el/tree/impl/ast/AstTextForNull.java create mode 100644 modules/impl/src/test/java/de/odysseus/el/tree/impl/ast/AstDateTest.java diff --git a/modules/impl/src/main/java/de/odysseus/el/ExpressionFactoryImpl.java b/modules/impl/src/main/java/de/odysseus/el/ExpressionFactoryImpl.java index 2321847..90b33ad 100644 --- a/modules/impl/src/main/java/de/odysseus/el/ExpressionFactoryImpl.java +++ b/modules/impl/src/main/java/de/odysseus/el/ExpressionFactoryImpl.java @@ -134,6 +134,11 @@ boolean contains(Feature feature) { */ public static final String PROP_CACHE_SIZE = "javax.el.cacheSize"; + /** + * javax.el.cacheSize + */ + public static final String PROP_CACHE_SIZE_FAIL_ON_RESIZE = "javax.el.cacheSize.failOnResize"; + private final TreeStore store; private final TypeConverter converter; @@ -343,6 +348,7 @@ protected TreeStore createTreeStore(int defaultCacheSize, Profile profile, Prope } // create cache + Cache cache = null; int cacheSize = defaultCacheSize; if (properties != null && properties.containsKey(PROP_CACHE_SIZE)) { try { @@ -351,7 +357,16 @@ protected TreeStore createTreeStore(int defaultCacheSize, Profile profile, Prope throw new ELException("Cannot parse EL property " + PROP_CACHE_SIZE, e); } } - Cache cache = cacheSize > 0 ? new Cache(cacheSize) : null; + if (properties != null && properties.containsKey(PROP_CACHE_SIZE_FAIL_ON_RESIZE)) { + try { + boolean failOnCacheResize = Boolean.parseBoolean(properties.getProperty(PROP_CACHE_SIZE_FAIL_ON_RESIZE)); + cache = cacheSize > 0 ? new Cache(cacheSize, failOnCacheResize) : null; + } catch (NumberFormatException e) { + throw new ELException("Cannot parse EL property " + PROP_CACHE_SIZE, e); + } + }else{ + cache = cacheSize > 0 ? new Cache(cacheSize) : null; + } return new TreeStore(builder, cache); } diff --git a/modules/impl/src/main/java/de/odysseus/el/TreeValueExpression.java b/modules/impl/src/main/java/de/odysseus/el/TreeValueExpression.java index 268ec54..5e769ea 100644 --- a/modules/impl/src/main/java/de/odysseus/el/TreeValueExpression.java +++ b/modules/impl/src/main/java/de/odysseus/el/TreeValueExpression.java @@ -18,6 +18,7 @@ import java.io.IOException; import java.io.ObjectInputStream; import java.io.PrintWriter; +import java.util.Date; import javax.el.ELContext; import javax.el.ELException; @@ -33,6 +34,8 @@ import de.odysseus.el.tree.Tree; import de.odysseus.el.tree.TreeBuilder; import de.odysseus.el.tree.TreeStore; +import de.odysseus.el.tree.impl.ast.AstObject; +import de.odysseus.el.tree.impl.ast.AstTextForNull; /** * A value expression is ready to be evaluated (by calling either @@ -73,7 +76,7 @@ public TreeValueExpression(TreeStore store, FunctionMapper functions, VariableMa this.bindings = tree.bind(functions, variables, converter); this.expr = expr; this.type = type; - this.node = tree.getRoot(); + this.node = getRootBasedOnType(tree, type); this.deferred = tree.isDeferred(); if (type == null) { @@ -81,6 +84,14 @@ public TreeValueExpression(TreeStore store, FunctionMapper functions, VariableMa } } + private ExpressionNode getRootBasedOnType(Tree tree, Class type) { + ExpressionNode expressionNode = tree.getRoot(); + if(expressionNode instanceof AstTextForNull && type != String.class){ + return new AstObject(""); + } + return tree.getRoot(); + } + private String getStructuralId() { if (structure == null) { structure = node.getStructuralId(bindings); diff --git a/modules/impl/src/main/java/de/odysseus/el/tree/impl/Cache.java b/modules/impl/src/main/java/de/odysseus/el/tree/impl/Cache.java index 14eb57e..b9f6f7a 100644 --- a/modules/impl/src/main/java/de/odysseus/el/tree/impl/Cache.java +++ b/modules/impl/src/main/java/de/odysseus/el/tree/impl/Cache.java @@ -35,6 +35,7 @@ public final class Cache implements TreeCache { private final ConcurrentLinkedQueue queue; private final AtomicInteger size; private final int capacity; + private boolean failOnResize; /** * Creates a new cache with the specified capacity @@ -45,6 +46,12 @@ public final class Cache implements TreeCache { */ public Cache(int capacity) { this(capacity, 16); + this.failOnResize = false; + } + + public Cache(int capacity, boolean failOnResize) { + this(capacity, 16); + this.failOnResize = failOnResize; } /** @@ -76,6 +83,9 @@ public void put(String expression, Tree tree) { if (map.putIfAbsent(expression, tree) == null) { queue.offer(expression); if (size.incrementAndGet() > capacity) { + if(failOnResize){ + throw new IllegalMonitorStateException("Resize occured for capacity:"+capacity); + } size.decrementAndGet(); map.remove(queue.poll()); } diff --git a/modules/impl/src/main/java/de/odysseus/el/tree/impl/Parser.java b/modules/impl/src/main/java/de/odysseus/el/tree/impl/Parser.java index 154d501..e4bb310 100644 --- a/modules/impl/src/main/java/de/odysseus/el/tree/impl/Parser.java +++ b/modules/impl/src/main/java/de/odysseus/el/tree/impl/Parser.java @@ -284,7 +284,7 @@ public Tree tree() throws ScanException, ParseException { AstNode t = text(); if (token.getSymbol() == EOF) { if (t == null) { - t = new AstText(""); + t = new AstTextForNull(""); } return new Tree(t, functions, identifiers, false); } diff --git a/modules/impl/src/main/java/de/odysseus/el/tree/impl/ast/AstObject.java b/modules/impl/src/main/java/de/odysseus/el/tree/impl/ast/AstObject.java new file mode 100644 index 0000000..54b1455 --- /dev/null +++ b/modules/impl/src/main/java/de/odysseus/el/tree/impl/ast/AstObject.java @@ -0,0 +1,101 @@ +/* + * Copyright 2006-2009 Odysseus Software GmbH + * + * Licensed 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 de.odysseus.el.tree.impl.ast; + +import de.odysseus.el.misc.LocalMessages; +import de.odysseus.el.tree.Bindings; + +import javax.el.ELContext; +import javax.el.ELException; +import javax.el.MethodInfo; +import javax.el.ValueReference; + +public class AstObject extends AstNode { + private final String value; + + public AstObject(String value) { + this.value = value; + } + + public boolean isLiteralText() { + return false; + } + + public boolean isLeftValue() { + return false; + } + + public boolean isMethodInvocation() { + return false; + } + + public Class getType(Bindings bindings, ELContext context) { + return null; + } + + public boolean isReadOnly(Bindings bindings, ELContext context) { + return true; + } + + public void setValue(Bindings bindings, ELContext context, Object value) { + throw new ELException(LocalMessages.get("error.value.set.rvalue", getStructuralId(bindings))); + } + + public ValueReference getValueReference(Bindings bindings, ELContext context) { + return null; + } + + @Override + public Object eval(Bindings bindings, ELContext context) { + return value; + } + + public MethodInfo getMethodInfo(Bindings bindings, ELContext context, Class returnType, Class[] paramTypes) { + return null; + } + + public Object invoke(Bindings bindings, ELContext context, Class returnType, Class[] paramTypes, Object[] paramValues) { + return returnType == null ? value : bindings.convert(value, returnType); + } + + @Override + public String toString() { + return "\"" + value + "\""; + } + + @Override + public void appendStructure(StringBuilder b, Bindings bindings) { + int end = value.length() - 1; + for (int i = 0; i < end; i++) { + char c = value.charAt(i); + if ((c == '#' || c == '$') && value.charAt(i + 1) == '{') { + b.append('\\'); + } + b.append(c); + } + if (end >= 0) { + b.append(value.charAt(end)); + } + } + + public int getCardinality() { + return 0; + } + + public AstNode getChild(int i) { + return null; + } +} diff --git a/modules/impl/src/main/java/de/odysseus/el/tree/impl/ast/AstProperty.java b/modules/impl/src/main/java/de/odysseus/el/tree/impl/ast/AstProperty.java index c3cc78b..ddc689c 100644 --- a/modules/impl/src/main/java/de/odysseus/el/tree/impl/ast/AstProperty.java +++ b/modules/impl/src/main/java/de/odysseus/el/tree/impl/ast/AstProperty.java @@ -170,12 +170,20 @@ protected Method findMethod(String name, Class clazz, Class returnType, Cl if (method == null) { throw new MethodNotFoundException(LocalMessages.get("error.property.method.notfound", name, clazz)); } - if (!ignoreReturnType && returnType != null && !returnType.isAssignableFrom(method.getReturnType())) { + if (!ignoreReturnType && returnType != null + && !isReturnTypeSame(returnType, method)) { throw new MethodNotFoundException(LocalMessages.get("error.property.method.returntype", method.getReturnType(), name, clazz, returnType)); } return method; } - + + private boolean isReturnTypeSame(Class returnType, Method method) { + if(java.lang.Void.class == returnType){ + return true; + } + return returnType.isAssignableFrom(method.getReturnType()); + } + public MethodInfo getMethodInfo(Bindings bindings, ELContext context, Class returnType, Class[] paramTypes) { Object base = prefix.eval(bindings, context); if (base == null) { diff --git a/modules/impl/src/main/java/de/odysseus/el/tree/impl/ast/AstText.java b/modules/impl/src/main/java/de/odysseus/el/tree/impl/ast/AstText.java index e12b494..8b16c1f 100644 --- a/modules/impl/src/main/java/de/odysseus/el/tree/impl/ast/AstText.java +++ b/modules/impl/src/main/java/de/odysseus/el/tree/impl/ast/AstText.java @@ -23,7 +23,7 @@ import de.odysseus.el.misc.LocalMessages; import de.odysseus.el.tree.Bindings; -public final class AstText extends AstNode { +public class AstText extends AstNode { private final String value; public AstText(String value) { diff --git a/modules/impl/src/main/java/de/odysseus/el/tree/impl/ast/AstTextForNull.java b/modules/impl/src/main/java/de/odysseus/el/tree/impl/ast/AstTextForNull.java new file mode 100644 index 0000000..acde5d0 --- /dev/null +++ b/modules/impl/src/main/java/de/odysseus/el/tree/impl/ast/AstTextForNull.java @@ -0,0 +1,10 @@ +package de.odysseus.el.tree.impl.ast; + +/** + * Created by alexbrob on 2016-03-14. + */ +public final class AstTextForNull extends AstText { + public AstTextForNull(String value) { + super(value); + } +} diff --git a/modules/impl/src/test/java/de/odysseus/el/TreeMethodExpressionTest.java b/modules/impl/src/test/java/de/odysseus/el/TreeMethodExpressionTest.java index 3b49478..88899ad 100644 --- a/modules/impl/src/test/java/de/odysseus/el/TreeMethodExpressionTest.java +++ b/modules/impl/src/test/java/de/odysseus/el/TreeMethodExpressionTest.java @@ -34,6 +34,10 @@ public int bar() { return 0; } + public void voidMethod(){ + + } + SimpleContext context; TreeStore store = new TreeStore(new Builder(Feature.METHOD_INVOCATIONS), null); @@ -83,6 +87,15 @@ public void testInvoke() { assertEquals(0, new TreeMethodExpression(store, null, null, null, "${base.foo()}", null, null).invoke(context, null)); } + public void testInvokeVoid() { + assertEquals(null, new TreeMethodExpression(store, null, null, null, "#{base.voidMethod}", Void.TYPE, new Class[0]).invoke(context, null)); + assertEquals(null, new TreeMethodExpression(store, null, null, null, "#{base.voidMethod}", Void.class, new Class[0]).invoke(context, null)); + assertEquals(null, new TreeMethodExpression(store, null, null, null, "#{base.voidMethod()}", null, null).invoke(context, null)); + + // TODO: Should this only be supported via the returnType flag? + // assertEquals(null, new TreeMethodExpression(store, null, null, null, "#{base.voidMethod}", Object.class, new Class[0]).invoke(context, null)); + } + public void testSerialize() throws Exception { TreeMethodExpression expression = new TreeMethodExpression(store, null, null, null, "${base.foo}", null, new Class[0]); diff --git a/modules/impl/src/test/java/de/odysseus/el/tree/impl/ast/AstDateTest.java b/modules/impl/src/test/java/de/odysseus/el/tree/impl/ast/AstDateTest.java new file mode 100644 index 0000000..7de67d4 --- /dev/null +++ b/modules/impl/src/test/java/de/odysseus/el/tree/impl/ast/AstDateTest.java @@ -0,0 +1,40 @@ +package de.odysseus.el.tree.impl.ast; + +import de.odysseus.el.ObjectValueExpression; +import de.odysseus.el.TestCase; +import de.odysseus.el.TreeValueExpression; +import de.odysseus.el.misc.TypeConverter; +import de.odysseus.el.tree.TreeStore; +import de.odysseus.el.util.SimpleContext; +import de.odysseus.el.util.SimpleResolver; + +import javax.el.MethodExpression; +import java.lang.reflect.Method; +import java.util.Date; + +/** + * Created by alexbrob on 2016-03-14. + */ +public class AstDateTest extends TestCase { + + SimpleContext context; + + @Override + protected void setUp() throws Exception { + context = new SimpleContext(new SimpleResolver()); + + TypeConverter converter = TypeConverter.DEFAULT; + + context.setVariable("var_date_1", new TreeValueExpression(new TreeStore(BUILDER, null), null, context.getVariableMapper(), converter, "", Date.class)); + context.setVariable("var_string_1", new TreeValueExpression(new TreeStore(BUILDER, null), null, context.getVariableMapper(), converter, "", String.class)); + } + + public void testIsLiteralTextFalseForDate() throws Exception { + assertFalse(context.getVariableMapper().resolveVariable("var_date_1").isLiteralText()); + } + + public void testIsLiteralTextTrueForString() throws Exception { + assertTrue(context.getVariableMapper().resolveVariable("var_string_1").isLiteralText()); + } + +} diff --git a/modules/impl/src/test/java/de/odysseus/el/tree/impl/ast/AstIdentifierTest.java b/modules/impl/src/test/java/de/odysseus/el/tree/impl/ast/AstIdentifierTest.java index ad6e58b..460cbc9 100644 --- a/modules/impl/src/test/java/de/odysseus/el/tree/impl/ast/AstIdentifierTest.java +++ b/modules/impl/src/test/java/de/odysseus/el/tree/impl/ast/AstIdentifierTest.java @@ -17,6 +17,7 @@ import java.lang.reflect.Method; import java.util.Arrays; +import java.util.Date; import javax.el.ELContext; import javax.el.ELException; @@ -91,6 +92,7 @@ protected void setUp() throws Exception { // variables var_long_1, indentifier_string context.setVariable("var_long_1", new ObjectValueExpression(converter, 1l, long.class)); + context.setVariable("indentifier_string", new ObjectValueExpression(converter, "foo", String.class)); context.setVariable("var_method_1", new ObjectValueExpression(converter, getClass().getMethod("method_1"), Method.class)); context.setVariable("var_method_1_expr", new ObjectValueExpression(converter, new TestMethodExpression(getClass().getMethod("method_1")), MethodExpression.class)); @@ -103,7 +105,8 @@ protected void setUp() throws Exception { // var_var_long_1 --> var_long_1, var_property_long_1 --> property_long_1 context.setVariable("var_var_long_1", new TreeValueExpression(new TreeStore(BUILDER, null), null, context.getVariableMapper(), null, "${var_long_1}", long.class)); - context.setVariable("var_property_long_1", new TreeValueExpression(new TreeStore(BUILDER, null), null, context.getVariableMapper(), null, "${property_long_1}", long.class)); + context.setVariable("var_property_long_1", new TreeValueExpression(new TreeStore(BUILDER, null), null, context.getVariableMapper(), null, "${property_long_1}", long.class)); + context.setVariable("var_date_1", new TreeValueExpression(new TreeStore(BUILDER, null), null, context.getVariableMapper(), converter, "", Date.class)); } public void testEval() { @@ -141,6 +144,10 @@ public void testAppendStructure() { assertEquals("foo", s.toString()); } + public void testIsLiteralTextFalseForDate() throws Exception { + assertFalse(context.getVariableMapper().resolveVariable("var_date_1").isLiteralText()); + } + public void testIsLiteralText() { assertFalse(parseNode("${foo}").isLiteralText()); }