Merge pull request #789 from apache/issue/WW-5363-velocity-order

WW-5363 Velocity: read chained contexts before ValueStack

plugins/velocity/src/main/java/org/apache/struts2/views/velocity/StrutsVelocityContext.java

@@ -24,6 +24,7 @@ import org.apache.velocity.VelocityContext;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
+import java.util.function.Function;
 
 public class StrutsVelocityContext extends VelocityContext {
 
@@ -54,36 +55,57 @@ public class StrutsVelocityContext extends VelocityContext {
     /**
      * @deprecated please use {@link #StrutsVelocityContext(List, ValueStack)}
      */
-    @Deprecated()
+    @Deprecated
     public StrutsVelocityContext(VelocityContext[] chainedContexts, ValueStack stack) {
         this(new ArrayList<>(Arrays.asList(chainedContexts)), stack);
     }
 
+    @Override
     public boolean internalContainsKey(String key) {
         return internalGet(key) != null;
     }
 
+    @Override
     public Object internalGet(String key) {
-        Object val = super.internalGet(key);
-        if (val != null) {
-            return val;
-        }
-        if (stack != null) {
-            val = stack.findValue(key);
-            if (val != null) {
-                return val;
-            }
-            val = stack.getContext().get(key);
+        for (Function<String, Object> contextGet : contextGetterList()) {
+            Object val = contextGet.apply(key);
             if (val != null) {
                 return val;
             }
         }
-        if (chainedContexts != null) {
-            for (VelocityContext chainedContext : chainedContexts) {
-                val = chainedContext.internalGet(key);
-                if (val != null) {
-                    return val;
-                }
+        return null;
+    }
+
+    protected List<Function<String, Object>> contextGetterList() {
+        return Arrays.asList(this::superGet, this::chainedContextGet, this::stackGet, this::stackContextGet);
+    }
+
+    protected Object superGet(String key) {
+        return super.internalGet(key);
+    }
+
+    protected Object stackGet(String key) {
+        if (stack == null) {
+            return null;
+        }
+        return stack.findValue(key);
+    }
+
+    protected Object stackContextGet(String key) {
+        if (stack == null) {
+            return null;
+        }
+        return stack.getContext().get(key);
+    }
+
+    protected Object chainedContextGet(String key) {
+        if (chainedContexts == null) {
+            return null;
+        }
+        for (VelocityContext chainedContext : chainedContexts) {
+            Object val = chainedContext.internalGet(key);
+            if (val != null) {
+                return val;
             }
         }
         return null;

plugins/velocity/src/test/java/org/apache/struts2/views/velocity/StrutsVelocityContextTest.java

@@ -0,0 +1,105 @@
+/*
+ * 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.struts2.views.velocity;
+
+import com.opensymphony.xwork2.util.ValueStack;
+import org.apache.velocity.VelocityContext;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnit;
+import org.mockito.junit.MockitoRule;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static java.util.Collections.singletonList;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+import static org.mockito.Mockito.when;
+
+public class StrutsVelocityContextTest {
+
+    @Rule
+    public MockitoRule mockitoRule = MockitoJUnit.rule();
+
+    private StrutsVelocityContext strutsVelocityContext;
+
+    @Mock
+    private VelocityContext chainedContext;
+
+    @Mock
+    private ValueStack stack;
+
+    private Map<String, Object> stackContext;
+
+    @Before
+    public void setUp() throws Exception {
+        stackContext = new HashMap<>();
+        when(stack.getContext()).thenReturn(stackContext);
+        strutsVelocityContext = new StrutsVelocityContext(singletonList(chainedContext), stack);
+    }
+
+    @Test
+    public void getChainedValue() {
+        when(chainedContext.internalGet("foo")).thenReturn("bar");
+        assertEquals("bar", strutsVelocityContext.internalGet("foo"));
+    }
+
+    @Test
+    public void getStackValue() {
+        when(stack.findValue("foo")).thenReturn("bar");
+        assertEquals("bar", strutsVelocityContext.internalGet("foo"));
+    }
+
+    @Test
+    public void getStackContextValue() {
+        stackContext.put("foo", "bar");
+        assertEquals("bar", strutsVelocityContext.internalGet("foo"));
+    }
+
+    @Test
+    public void getSuperValue() {
+        strutsVelocityContext.put("foo", "bar");
+        assertEquals("bar", strutsVelocityContext.internalGet("foo"));
+    }
+
+    @Test
+    public void getValuePrecedence() {
+        stackContext.put("foo", "quux");
+        assertEquals("quux", strutsVelocityContext.internalGet("foo"));
+
+        when(stack.findValue("foo")).thenReturn("qux");
+        assertEquals("qux", strutsVelocityContext.internalGet("foo"));
+
+        when(chainedContext.internalGet("foo")).thenReturn("baz");
+        assertEquals("baz", strutsVelocityContext.internalGet("foo"));
+
+        strutsVelocityContext.put("foo", "bar");
+        assertEquals("bar", strutsVelocityContext.internalGet("foo"));
+    }
+
+    @Test
+    public void nullArgs() {
+        strutsVelocityContext = new StrutsVelocityContext((List<VelocityContext>) null, null);
+        assertNull(strutsVelocityContext.internalGet("foo"));
+    }
+}