Merge pull request #204 from orphan-oss/OGNL-102-performance

OGNL-102 Improves performance when null was returned

src/main/java/ognl/ASTChain.java

@@ -28,6 +28,8 @@ public class ASTChain extends SimpleNode implements NodeType, OrderedReturn {
 
     private static final long serialVersionUID = 6689037266594707682L;
 
+    private final boolean shortCircuit = Boolean.parseBoolean(System.getProperty("ognl.chain.short-circuit", "true"));
+
     private Class<?> getterClass;
     private Class<?> setterClass;
     private String lastExpression;
@@ -53,11 +55,20 @@ public void jjtClose() {
         flattenTree();
     }
 
-    protected Object getValueBody(OgnlContext context, Object source)
-            throws OgnlException {
+    protected Object getValueBody(OgnlContext context, Object source) throws OgnlException {
         Object result = source;
 
+        // short-circuit the chain only in case if the root is null and this isn't IN operator
+        if (shortCircuit && result == null && !(parent instanceof ASTIn)) {
+            return null;
+        }
+
         for (int i = 0, ilast = children.length - 1; i <= ilast; ++i) {
+            // short-circuit the chain only in case if the root is null and accessing property
+            if (shortCircuit && result == null && (children[i] instanceof ASTProperty)) {
+                return null;
+            }
+
             boolean handled = false;
 
             if (i < ilast) {

src/test/java/ognl/test/ChainTest.java

@@ -15,19 +15,28 @@
  */
 package ognl.test;
 
-import junit.framework.TestCase;
 import ognl.DefaultMemberAccess;
 import ognl.Ognl;
 import ognl.OgnlContext;
+import ognl.OgnlException;
 import ognl.SimpleNode;
+import ognl.test.objects.Root;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 /**
  * Tests for {@link SimpleNode#isChain(OgnlContext)}.
  */
-public class ChainTest extends TestCase {
+public class ChainTest {
 
+    @Test
     public void test_isChain() throws Exception {
-        OgnlContext context = (OgnlContext) Ognl.createDefaultContext(null, new DefaultMemberAccess(false));
+        OgnlContext context = Ognl.createDefaultContext(null, new DefaultMemberAccess(false));
 
         SimpleNode node = (SimpleNode) Ognl.parseExpression("#name");
         assertFalse(node.isChain(context));
@@ -48,4 +57,51 @@ public void test_isChain() throws Exception {
         assertTrue(node.isChain(context));
     }
 
+    @Test
+    public void shouldShortCircuitAccessingNullChild() throws OgnlException {
+        OgnlContext context = Ognl.createDefaultContext(null);
+        Parent parent = new Parent(new Parent(null));
+        context.put("parent", parent);
+
+        assertNull(Ognl.getValue("#parent.child.child.name", context, parent));
+    }
+
+    @Test
+    public void shouldEvaluateThisProperty() throws OgnlException {
+        Root root = new Root();
+        OgnlContext context = Ognl.createDefaultContext(root);
+
+        assertEquals("empty", Ognl.getValue("map[$].(#this == null ? 'empty' : #this)", context, root));
+    }
+
+    public static class Child {
+        private final String name;
+
+        public Child(String name) {
+            this.name = name;
+        }
+
+        public String getName() {
+            return name;
+        }
+
+        @Override
+        public String toString() {
+            return name;
+        }
+    }
+
+    public static class Parent extends Child {
+        private final Child child;
+
+        public Parent(Child child) {
+            super("parent of " + child);
+            this.child = child;
+        }
+
+        public Child getChild() {
+            return child;
+        }
+    }
+
 }

src/test/java/ognl/test/LambdaExpressionTest.java

@@ -18,64 +18,117 @@
  */
 package ognl.test;
 
-import junit.framework.TestSuite;
+import ognl.DefaultMemberAccess;
+import ognl.Ognl;
+import ognl.OgnlContext;
+import ognl.SimpleNode;
+import org.junit.Before;
+import org.junit.Test;
 
 import java.math.BigInteger;
-import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.List;
 
-public class LambdaExpressionTest extends OgnlTestCase {
-
-    private static Object[][] TESTS = {
-            // Lambda expressions
-            {null, "#a=:[33](20).longValue().{0}.toArray().length", new Integer(33)},
-            {null, "#fact=:[#this<=1? 1 : #fact(#this-1) * #this], #fact(30)", new Integer(1409286144)},
-            {null, "#fact=:[#this<=1? 1 : #fact(#this-1) * #this], #fact(30L)", new Long(-8764578968847253504L)},
-            {null, "#fact=:[#this<=1? 1 : #fact(#this-1) * #this], #fact(30h)",
-                    new BigInteger("265252859812191058636308480000000")},
-            {null, "#bump = :[ #this.{ #this + 1 } ], (#bump)({ 1, 2, 3 })",
-                    new ArrayList(Arrays.asList(new Integer[]{new Integer(2), new Integer(3), new Integer(4)}))},
-            {null, "#call = :[ \"calling \" + [0] + \" on \" + [1] ], (#call)({ \"x\", \"y\" })", "calling x on y"},
-
-    };
-
-    /*
-     * =================================================================== Public static methods
-     * ===================================================================
-     */
-    public static TestSuite suite() {
-        TestSuite result = new TestSuite();
-
-        for (int i = 0; i < TESTS.length; i++) {
-            result.addTest(new LambdaExpressionTest((String) TESTS[i][1], TESTS[i][0], (String) TESTS[i][1],
-                    TESTS[i][2]));
-        }
-        return result;
+import static org.junit.Assert.assertEquals;
+
+public class LambdaExpressionTest {
+
+    private OgnlContext context;
+
+    @Before
+    public void setUp() throws Exception {
+        this.context = Ognl.createDefaultContext(null, new DefaultMemberAccess(false));
+    }
+
+    private SimpleNode getExpression(Object root, String expressionStr) throws Exception {
+        // validate expression
+        Ognl.parseExpression(expressionStr);
+        // compile expression
+        return (SimpleNode) Ognl.compileExpression(context, root, expressionStr);
     }
 
-    /*
-     * =================================================================== Constructors
-     * ===================================================================
-     */
-    public LambdaExpressionTest() {
-        super();
+    @Test
+    public void shouldReadArrayLength() throws Exception {
+        // given
+        Object root = new Object[]{};
+        String expressionStr = "#a=:[33](20).longValue().{0}.toArray().length";
+        int expectedResult = 33;
+
+        // when
+        SimpleNode expression = getExpression(root, expressionStr);
+
+        // then
+        assertEquals(expectedResult, Ognl.getValue(expression, context, root));
+    }
+
+    @Test
+    public void shouldEvaluateLambda1() throws Exception {
+        // given
+        Object root = null;
+        String expressionStr = "#fact=:[#this <=1 ? 1 : #fact(#this-1) * #this], #fact(30)";
+        int expectedResult = 1409286144;
+
+        // when
+        SimpleNode expression = getExpression(root, expressionStr);
+
+        // then
+        assertEquals(expectedResult, Ognl.getValue(expression, context, root));
     }
 
-    public LambdaExpressionTest(String name) {
-        super(name);
+    @Test
+    public void shouldEvaluateLambda2() throws Exception {
+        // given
+        Object root = null;
+        String expressionStr = "#fact=:[#this <= 1 ? 1 : #fact(#this-1) * #this], #fact(30L)";
+        long expectedResult = -8764578968847253504L;
+
+        // when
+        SimpleNode expression = getExpression(root, expressionStr);
+
+        // then
+        assertEquals(expectedResult, Ognl.getValue(expression, context, root));
     }
 
-    public LambdaExpressionTest(String name, Object root, String expressionString, Object expectedResult,
-                                Object setValue, Object expectedAfterSetResult) {
-        super(name, root, expressionString, expectedResult, setValue, expectedAfterSetResult);
+    @Test
+    public void shouldEvaluateLambda3() throws Exception {
+        // given
+        Object root = null;
+        String expressionStr = "#fact=:[#this <= 1 ? 1 : #fact(#this-1) * #this], #fact(30h)";
+        BigInteger expectedResult = new BigInteger("265252859812191058636308480000000");
+
+        // when
+        SimpleNode expression = getExpression(root, expressionStr);
+
+        // then
+        assertEquals(expectedResult, Ognl.getValue(expression, context, root));
     }
 
-    public LambdaExpressionTest(String name, Object root, String expressionString, Object expectedResult,
-                                Object setValue) {
-        super(name, root, expressionString, expectedResult, setValue);
+    @Test
+    public void shouldEvaluateLambda4() throws Exception {
+        // given
+        Object root = null;
+        String expressionStr = "#bump = :[ #this.{ #this + 1 } ], (#bump)({ 1, 2, 3 })";
+        List<Integer> expectedResult = Arrays.asList(2, 3, 4);
+
+        // when
+        SimpleNode expression = getExpression(root, expressionStr);
+
+        // then
+        assertEquals(expectedResult, Ognl.getValue(expression, context, root));
     }
 
-    public LambdaExpressionTest(String name, Object root, String expressionString, Object expectedResult) {
-        super(name, root, expressionString, expectedResult);
+    @Test
+    public void shouldEvaluateLambda5() throws Exception {
+        // given
+        Object root = null;
+        String expressionStr = "#call = :[ \"calling \" + [0] + \" on \" + [1] ], (#call)({ \"x\", \"y\" })";
+        String expectedResult = "calling x on y";
+
+        // when
+        SimpleNode expression = getExpression(root, expressionStr);
+
+        // then
+        assertEquals(expectedResult, Ognl.getValue(expression, context, root));
     }
+
 }

src/test/java/ognl/test/NullRootTest.java

@@ -0,0 +1,38 @@
+package ognl.test;
+
+import ognl.Ognl;
+import ognl.OgnlContext;
+import org.junit.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.junit.Assert.assertNull;
+
+public class NullRootTest {
+
+    @Test
+    public void testNullValue() throws Exception {
+        OgnlContext context = Ognl.createDefaultContext(null);
+        Map<String, Object> root = new HashMap<>();
+        root.put("key1", null);
+        String expr = "key1.key2.key3";
+        assertNull(Ognl.getValue(expr, context, root));
+    }
+
+    @Test
+    public void testEmptyRoot() throws Exception {
+        OgnlContext context = Ognl.createDefaultContext(null);
+        Map<String, Object> root = new HashMap<>();
+        String expr = "key1.key2.key3";
+        assertNull(Ognl.getValue(expr, context, root));
+    }
+
+    @Test
+    public void testNullRoot() throws Exception {
+        OgnlContext context = Ognl.createDefaultContext(null);
+        Map<String, Object> root = null;
+        String expr = "key1.key2.key3";
+        assertNull(Ognl.getValue(expr, context, root));
+    }
+}

src/test/java/ognl/test/ProjectionSelectionTest.java

@@ -35,7 +35,7 @@ public class ProjectionSelectionTest extends OgnlTestCase {
             {ROOT, "map.array.{^ #this > 2 }", Arrays.asList(new Integer[]{new Integer(3)})},
             {ROOT, "map.array.{$ #this > 2 }", Arrays.asList(new Integer[]{new Integer(4)})},
             {ROOT, "map.array[*].{?true} instanceof java.util.Collection", Boolean.TRUE},
-            {null, "#fact=1, 30H.{? #fact = #fact * (#this+1), false }, #fact",
+            {ROOT, "#fact=1, 30H.{? #fact = #fact * (#this+1), false }, #fact",
                     new BigInteger("265252859812191058636308480000000")},
     };