WW-5385 Decompose StrutsVelocityContext for better re-usability

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

@@ -0,0 +1,48 @@
+/*
+ * 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 org.apache.velocity.VelocityContext;
+import org.apache.velocity.context.Context;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+
+/**
+ * Extends the default {@link VelocityContext} to ensure that the {@link #getKeys()} method returns all keys from the
+ * current context and the chained context.
+ *
+ * @since 6.4.0
+ */
+public class ChainedVelocityContext extends VelocityContext {
+
+    public ChainedVelocityContext(Context delegate) {
+        super(delegate);
+    }
+
+    @Override
+    public String[] getKeys() {
+        Set<String> allKeys = new HashSet<>(Arrays.asList(internalGetKeys()));
+        if (getChainedContext() != null) {
+            allKeys.addAll(Arrays.asList(getChainedContext().getKeys()));
+        }
+        return allKeys.toArray(new String[0]);
+    }
+}

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

@@ -0,0 +1,90 @@
+/*
+ * 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 org.apache.velocity.VelocityContext;
+import org.apache.velocity.context.Context;
+
+import java.util.Arrays;
+import java.util.stream.StreamSupport;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Simple immutable composite Velocity {@link Context} implementation that delegates to a collection of other contexts.
+ * The order of the contexts is significant as it checks them in iteration order. This context should be wrapped in a
+ * {@link org.apache.velocity.VelocityContext} or other chained context before use, using the provided static factory
+ * methods or otherwise.
+ *
+ * @since 6.4.0
+ */
+public class CompositeContext implements Context {
+    private final Iterable<? extends Context> contexts;
+
+    public static VelocityContext composite(Iterable<? extends Context> contexts) {
+        return new ChainedVelocityContext(new CompositeContext(contexts));
+    }
+
+    public static VelocityContext composite(Context ...contexts) {
+        return new ChainedVelocityContext(new CompositeContext(contexts));
+    }
+
+    public CompositeContext(Context ...contexts) {
+        this(Arrays.asList(contexts));
+    }
+
+    public CompositeContext(Iterable<? extends Context> contexts) {
+        this.contexts = requireNonNull(contexts);
+    }
+
+    @Override
+    public Object get(String key) {
+        for (Context context : contexts) {
+            Object value = context.get(key);
+            if (value != null) {
+                return value;
+            }
+        }
+        return null;
+    }
+
+    @Override
+    public boolean containsKey(String key) {
+        return StreamSupport.stream(contexts.spliterator(), false).anyMatch(context -> context.containsKey(key));
+    }
+
+    /**
+     * Union of all keys for all contexts.
+     */
+    @Override
+    public String[] getKeys() {
+        return StreamSupport.stream(contexts.spliterator(), false)
+                .map(Context::getKeys).flatMap(Arrays::stream).distinct().toArray(String[]::new);
+    }
+
+    @Override
+    public Object remove(String key) {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public Object put(String key, Object value) {
+        throw new UnsupportedOperationException();
+    }
+}

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

@@ -0,0 +1,51 @@
+/*
+ * 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.struts2.util.ValueStackProvider;
+import org.apache.velocity.VelocityContext;
+import org.apache.velocity.context.Context;
+
+import java.util.Map;
+
+/**
+ * A Velocity {@link Context} which implements {@link ValueStackProvider} as required to render
+ * {@link org.apache.struts2.views.velocity.components.AbstractDirective Struts directives}.
+ *
+ * @since 6.4.0
+ */
+public class DirectiveVelocityContext extends ChainedVelocityContext implements ValueStackProvider {
+
+    private final transient ValueStack stack;
+
+    public DirectiveVelocityContext(ValueStack stack, Context ...contexts) {
+        super(new CompositeContext(contexts));
+        this.stack = stack;
+    }
+
+    public DirectiveVelocityContext(ValueStack stack, Map<String, Object> map) {
+        this(stack, new VelocityContext(map));
+    }
+
+    @Override
+    public ValueStack getValueStack() {
+        return stack;
+    }
+}

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

@@ -19,94 +19,57 @@
 package org.apache.struts2.views.velocity;
 
 import com.opensymphony.xwork2.util.ValueStack;
-import org.apache.struts2.util.ValueStackProvider;
 import org.apache.velocity.VelocityContext;
+import org.apache.velocity.context.Context;
 
-import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.List;
-import java.util.function.Function;
 
-public class StrutsVelocityContext extends VelocityContext implements ValueStackProvider {
+/**
+ * A Velocity {@link Context} which falls back to a {@link ValueStack} lookup after looking in any other provided
+ * Velocity contexts.
+ */
+public class StrutsVelocityContext extends DirectiveVelocityContext {
 
-    private final ValueStack stack;
-    private final List<VelocityContext> chainedContexts;
+    /**
+     * @since 6.4.0.
+     */
+    public StrutsVelocityContext(ValueStack stack, Context ...contexts) {
+        super(stack, contexts);
+    }
 
     /**
      * Creates a content with link to the ValueStack and any other Velocity contexts
      *
      * @param chainedContexts Existing Velocity contexts to chain to
-     * @param stack Struts ValueStack
+     * @param stack           Struts ValueStack
      * @since 6.0.0
      */
-    public StrutsVelocityContext(List<VelocityContext> chainedContexts, ValueStack stack) {
-        this.chainedContexts = chainedContexts;
-        this.stack = stack;
+    public StrutsVelocityContext(List<? extends Context> chainedContexts, ValueStack stack) {
+        super(stack, chainedContexts == null ? new Context[0] : chainedContexts.toArray(new Context[0]));
     }
 
     /**
-     * @deprecated please use {@link #StrutsVelocityContext(List, ValueStack)}
-     * and pass {null} or empty list if no chained contexts were defined
+     * @deprecated since 6.0.0, use {@link #StrutsVelocityContext(ValueStack, Context...)} instead.
      */
     @Deprecated
     public StrutsVelocityContext(ValueStack stack) {
-        this((List<VelocityContext>) null, stack);
+        this(stack, new Context[0]);
     }
 
     /**
-     * @deprecated please use {@link #StrutsVelocityContext(List, ValueStack)}
+     * @deprecated since 6.0.0, use {@link #StrutsVelocityContext(ValueStack, Context...)} instead.
      */
     @Deprecated
     public StrutsVelocityContext(VelocityContext[] chainedContexts, ValueStack stack) {
-        this(new ArrayList<>(Arrays.asList(chainedContexts)), stack);
+        this(stack, chainedContexts);
     }
 
     @Override
-    public boolean internalContainsKey(String key) {
-        return internalGet(key) != null;
-    }
-
-    @Override
-    public Object internalGet(String key) {
-        for (Function<String, Object> contextGet : contextGetterList()) {
-            Object val = contextGet.apply(key);
-            if (val != null) {
-                return val;
-            }
+    public Object get(String key) {
+        Object value = super.get(key);
+        if (value == null && getValueStack() != null) {
+            value = getValueStack().findValue(key);
         }
-        return null;
-    }
-
-    protected List<Function<String, Object>> contextGetterList() {
-        return Arrays.asList(this::superInternalGet, this::chainedContextGet, this::stackGet);
-    }
-
-    protected Object superInternalGet(String key) {
-        return super.internalGet(key);
-    }
-
-    protected Object stackGet(String key) {
-        if (stack == null) {
-            return null;
-        }
-        return stack.findValue(key);
-    }
-
-    protected Object chainedContextGet(String key) {
-        if (chainedContexts == null) {
-            return null;
-        }
-        for (VelocityContext chainedContext : chainedContexts) {
-            Object val = chainedContext.get(key);
-            if (val != null) {
-                return val;
-            }
-        }
-        return null;
-    }
-
-    @Override
-    public ValueStack getValueStack() {
-        return stack;
+        return value;
     }
 }

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

@@ -128,7 +128,7 @@ public class VelocityManager {
 
     protected Context buildContext(ValueStack stack, HttpServletRequest req, HttpServletResponse res) {
         List<VelocityContext> chainedContexts = prepareChainedContexts(req, res, stack.getContext());
-        Context context = new StrutsVelocityContext(chainedContexts, stack);
+        Context context = new StrutsVelocityContext(stack, chainedContexts.toArray(new Context[0]));
         ContextUtil.getStandardContext(stack, req, res).forEach(context::put);
         VelocityStrutsUtil util = new VelocityStrutsUtil(velocityEngine, context, stack, req, res);
         context.put(STRUTS, util);

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

@@ -0,0 +1,70 @@
+/*
+ * 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 org.apache.velocity.context.Context;
+import org.junit.Test;
+
+import static java.util.Arrays.asList;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+public class CompositeContextTest {
+
+    @Test
+    public void compositeLooksUpInForwardOrder_factory() {
+        Context context1 = mock(Context.class);
+        Context context2 = mock(Context.class);
+
+        Context compositeContext = CompositeContext.composite(asList(context1, context2));
+
+        when(context1.get("x")).thenReturn("x1");
+        when(context2.get("x")).thenReturn("x2");
+
+        assertEquals("x1", compositeContext.get("x"));
+    }
+
+    @Test
+    public void compositeLooksUpInForwardOrder_altFactory() {
+        Context context1 = mock(Context.class);
+        Context context2 = mock(Context.class);
+
+        Context compositeContext = CompositeContext.composite(context1, context2);
+
+        when(context1.get("x")).thenReturn("x1");
+        when(context2.get("x")).thenReturn("x2");
+
+        assertEquals("x1", compositeContext.get("x"));
+    }
+
+    @Test
+    public void compositeGeneratesSupersetOfKeys() {
+        final Context context1 = mock(Context.class);
+        final Context context2 = mock(Context.class);
+
+        when(context1.getKeys()).thenReturn(new String[]{"a", "b"});
+        when(context2.getKeys()).thenReturn(new String[]{"b", "c"});
+
+        Context compositeContext = CompositeContext.composite(asList(context1, context2));
+
+        assertThat(compositeContext.getKeys()).containsExactlyInAnyOrder("a", "b", "c");
+    }
+}

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

@@ -29,7 +29,6 @@ import org.mockito.junit.MockitoRule;
 
 import java.util.List;
 
-import static java.util.Collections.singletonList;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
 import static org.mockito.Mockito.when;
@@ -49,42 +48,42 @@ public class StrutsVelocityContextTest {
 
     @Before
     public void setUp() throws Exception {
-        strutsVelocityContext = new StrutsVelocityContext(singletonList(chainedContext), stack);
+        strutsVelocityContext = new StrutsVelocityContext(stack, chainedContext);
     }
 
     @Test
     public void getChainedValue() {
         when(chainedContext.get("foo")).thenReturn("bar");
-        assertEquals("bar", strutsVelocityContext.internalGet("foo"));
+        assertEquals("bar", strutsVelocityContext.get("foo"));
     }
 
     @Test
     public void getStackValue() {
         when(stack.findValue("foo")).thenReturn("bar");
-        assertEquals("bar", strutsVelocityContext.internalGet("foo"));
+        assertEquals("bar", strutsVelocityContext.get("foo"));
     }
 
     @Test
     public void getSuperValue() {
         strutsVelocityContext.put("foo", "bar");
-        assertEquals("bar", strutsVelocityContext.internalGet("foo"));
+        assertEquals("bar", strutsVelocityContext.get("foo"));
     }
 
     @Test
     public void getValuePrecedence() {
         when(stack.findValue("foo")).thenReturn("qux");
-        assertEquals("qux", strutsVelocityContext.internalGet("foo"));
+        assertEquals("qux", strutsVelocityContext.get("foo"));
 
         when(chainedContext.get("foo")).thenReturn("baz");
-        assertEquals("baz", strutsVelocityContext.internalGet("foo"));
+        assertEquals("baz", strutsVelocityContext.get("foo"));
 
         strutsVelocityContext.put("foo", "bar");
-        assertEquals("bar", strutsVelocityContext.internalGet("foo"));
+        assertEquals("bar", strutsVelocityContext.get("foo"));
     }
 
     @Test
     public void nullArgs() {
         strutsVelocityContext = new StrutsVelocityContext((List<VelocityContext>) null, null);
-        assertNull(strutsVelocityContext.internalGet("foo"));
+        assertNull(strutsVelocityContext.get("foo"));
     }
 }