This is an automated email from the ASF dual-hosted git repository.
kusal pushed a commit to branch WW-5385-strutsvelocitycontext
in repository https://gitbox.apache.org/repos/asf/struts.git
commit 215a5fc31569c7eeb37186dbdce020c8ac8a275d
Author: Kusal Kithul-Godage <g...@kusal.io>
AuthorDate: Mon Jan 8 18:17:11 2024 +1100
WW-5385 Decompose StrutsVelocityContext for better re-usability
---
.../views/velocity/ChainedVelocityContext.java | 28 ++++++++
.../struts2/views/velocity/CompositeContext.java | 70 ++++++++++++++++++
.../views/velocity/DirectiveVelocityContext.java | 26 +++++++
.../views/velocity/StrutsVelocityContext.java | 83 ++++++----------------
.../struts2/views/velocity/VelocityManager.java | 2 +-
.../views/velocity/CompositeContextTest.java | 52 ++++++++++++++
.../views/velocity/StrutsVelocityContextTest.java | 17 +++--
7 files changed, 208 insertions(+), 70 deletions(-)
diff --git
a/plugins/velocity/src/main/java/org/apache/struts2/views/velocity/ChainedVelocityContext.java
b/plugins/velocity/src/main/java/org/apache/struts2/views/velocity/ChainedVelocityContext.java
new file mode 100644
index 000000000..d9efaa0c4
--- /dev/null
+++
b/plugins/velocity/src/main/java/org/apache/struts2/views/velocity/ChainedVelocityContext.java
@@ -0,0 +1,28 @@
+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.
+ */
+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]);
+ }
+}
diff --git
a/plugins/velocity/src/main/java/org/apache/struts2/views/velocity/CompositeContext.java
b/plugins/velocity/src/main/java/org/apache/struts2/views/velocity/CompositeContext.java
new file mode 100644
index 000000000..27797cb48
--- /dev/null
+++
b/plugins/velocity/src/main/java/org/apache/struts2/views/velocity/CompositeContext.java
@@ -0,0 +1,70 @@
+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.
+ */
+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();
+ }
+}
diff --git
a/plugins/velocity/src/main/java/org/apache/struts2/views/velocity/DirectiveVelocityContext.java
b/plugins/velocity/src/main/java/org/apache/struts2/views/velocity/DirectiveVelocityContext.java
new file mode 100644
index 000000000..cda0532c2
--- /dev/null
+++
b/plugins/velocity/src/main/java/org/apache/struts2/views/velocity/DirectiveVelocityContext.java
@@ -0,0 +1,26 @@
+package org.apache.struts2.views.velocity;
+
+import com.opensymphony.xwork2.util.ValueStack;
+import org.apache.struts2.util.ValueStackProvider;
+import org.apache.velocity.context.Context;
+
+/**
+ * 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 ValueStack stack;
+
+ public DirectiveVelocityContext(ValueStack stack, Context...contexts) {
+ super(new CompositeContext(contexts));
+ this.stack = stack;
+ }
+
+ @Override
+ public ValueStack getValueStack() {
+ return stack;
+ }
+}
diff --git
a/plugins/velocity/src/main/java/org/apache/struts2/views/velocity/StrutsVelocityContext.java
b/plugins/velocity/src/main/java/org/apache/struts2/views/velocity/StrutsVelocityContext.java
index d18ca6bcf..53acff960 100644
---
a/plugins/velocity/src/main/java/org/apache/struts2/views/velocity/StrutsVelocityContext.java
+++
b/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;
+ 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;
}
}
diff --git
a/plugins/velocity/src/main/java/org/apache/struts2/views/velocity/VelocityManager.java
b/plugins/velocity/src/main/java/org/apache/struts2/views/velocity/VelocityManager.java
index 2b9e00191..a802bbe0a 100644
---
a/plugins/velocity/src/main/java/org/apache/struts2/views/velocity/VelocityManager.java
+++
b/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);
diff --git
a/plugins/velocity/src/test/java/org/apache/struts2/views/velocity/CompositeContextTest.java
b/plugins/velocity/src/test/java/org/apache/struts2/views/velocity/CompositeContextTest.java
new file mode 100644
index 000000000..ff06343d9
--- /dev/null
+++
b/plugins/velocity/src/test/java/org/apache/struts2/views/velocity/CompositeContextTest.java
@@ -0,0 +1,52 @@
+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");
+ }
+}
diff --git
a/plugins/velocity/src/test/java/org/apache/struts2/views/velocity/StrutsVelocityContextTest.java
b/plugins/velocity/src/test/java/org/apache/struts2/views/velocity/StrutsVelocityContextTest.java
index 60490c6af..421206202 100644
---
a/plugins/velocity/src/test/java/org/apache/struts2/views/velocity/StrutsVelocityContextTest.java
+++
b/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"));
}
}
