WW-5729 fix(jasperreports): override HashMap.get/containsKey in ValueStackShadowMap

plugins/jasperreports/src/main/java/org/apache/struts2/views/jasperreports/ValueStackShadowMap.java

@@ -53,10 +53,11 @@
      * @return <tt>true</tt>, if contains key, <tt>false</tt> otherwise.
      * @see java.util.HashMap#containsKey
      */
-    public boolean containsKey(String key) {
+    @Override
+    public boolean containsKey(Object key) {
         boolean hasKey = super.containsKey(key);
 
-        if (!hasKey && valueStack.findValue(key) != null) {
+        if (!hasKey && key != null && valueStack.findValue(key.toString()) != null) {
             hasKey = true;
         }
 
@@ -67,16 +68,30 @@
      * Implementation of get(), overriding HashMap implementation.
      *
      * @param key - The key to get in HashMap and if not found there from the valueStack.
-     * @return value - The object from HashMap or if null, from the valueStack.
+     * @return value - The object from HashMap or, if the key is absent, from the valueStack.
      * @see java.util.HashMap#get
      */
-    public Object get(String key) {
-        Object value = super.get(key);
-
-        if ((value == null)) {
-            value = valueStack.findValue((String) key);
+    @Override
+    public Object get(Object key) {
+        if (key == null || super.containsKey(key)) {
+            return super.get(key);
         }
+        return valueStack.findValue(key.toString());
+    }
+
+    /**
+     * @deprecated since 7.4.0, use {@link #containsKey(Object)}
+     */
+    @Deprecated(since = "7.4.0", forRemoval = true)
+    public boolean containsKey(String key) {
+        return containsKey((Object) key);
+    }
 
-        return value;
+    /**
+     * @deprecated since 7.4.0, use {@link #get(Object)}
+     */
+    @Deprecated(since = "7.4.0", forRemoval = true)
+    public Object get(String key) {
+        return get((Object) key);
     }
 }

plugins/jasperreports/src/test/java/org/apache/struts2/views/jasperreports/JasperReportsResultTest.java

@@ -171,6 +171,39 @@ public void testReportParametersExpressionAccepted() throws Exception {
         assertTrue(response.getContentAsString().contains("Qux Report"));
     }
 
+    public void testDeclaredParameterResolvedFromValueStack() throws Exception {
+        result.setDataSource("{#{'firstName':'ignore', 'lastName':'ignore'}}");
+        stack.push(new Object() {
+            public String getTitle() {
+                return "Shadow";
+            }
+        });
+
+        result.execute(this.invocation);
+
+        assertTrue(response.getContentAsString().contains("Shadow Report"));
+    }
+
+    public void testExplicitNullParameterIsNotShadowedByValueStack() throws Exception {
+        result.setDataSource("{#{'firstName':'ignore', 'lastName':'ignore'}}");
+        stack.push(new Object() {
+            public String getTitle() {
+                return "Shadow";
+            }
+
+            public Map<String, Object> getReportParameters() {
+                Map<String, Object> params = new HashMap<>();
+                params.put("title", null);
+                return params;
+            }
+        });
+        result.setReportParameters("reportParameters");
+
+        result.execute(this.invocation);
+
+        assertTrue(response.getContentAsString().contains("null Report"));
+    }
+
     public void testFillFromReportParametersWithoutDataSourceOrConnection() throws Exception {
         stack.push(new Object() {
             public Map<String, Object> getReportParameters() {

plugins/jasperreports/src/test/java/org/apache/struts2/views/jasperreports/ValueStackShadowMapTest.java

@@ -0,0 +1,71 @@
+/*
+ * 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.jasperreports;
+
+import org.apache.struts2.ActionContext;
+import org.apache.struts2.junit.StrutsTestCase;
+import org.apache.struts2.util.ValueStack;
+
+import java.util.Map;
+
+public class ValueStackShadowMapTest extends StrutsTestCase {
+
+    private ValueStackShadowMap map;
+
+    public void testFallsBackToValueStackThroughMapInterface() {
+        Map<String, Object> asMap = map;
+
+        assertTrue(asMap.containsKey("title"));
+        assertEquals("Shadow", asMap.get("title"));
+        assertFalse(asMap.containsKey("missing"));
+        assertNull(asMap.get("missing"));
+    }
+
+    public void testExplicitEntryWinsOverValueStack() {
+        map.put("title", "Explicit");
+        map.put("other", null);
+
+        assertEquals("Explicit", map.get((Object) "title"));
+        assertTrue(map.containsKey((Object) "other"));
+        assertNull(map.get((Object) "other"));
+    }
+
+    public void testNullKeyDoesNotReachValueStack() {
+        assertFalse(map.containsKey((Object) null));
+        assertNull(map.get((Object) null));
+    }
+
+    @SuppressWarnings("removal")
+    public void testStringOverloadsDelegateToOverrides() {
+        assertTrue(map.containsKey("title"));
+        assertEquals("Shadow", map.get("title"));
+    }
+
+    @Override
+    protected void setUp() throws Exception {
+        super.setUp();
+        ValueStack stack = ActionContext.getContext().getValueStack();
+        stack.push(new Object() {
+            public String getTitle() {
+                return "Shadow";
+            }
+        });
+        map = new ValueStackShadowMap(stack);
+    }
+}

plugins/jasperreports7/src/main/java/org/apache/struts2/views/jasperreports7/ValueStackShadowMap.java

@@ -66,17 +66,14 @@ public boolean containsKey(Object key) {
      * Implementation of get(), overriding HashMap implementation.
      *
      * @param key - The key to get in HashMap and if not found there from the valueStack.
-     * @return value - The object from HashMap or if null, from the valueStack.
+     * @return value - The object from HashMap or, if the key is absent, from the valueStack.
      * @see java.util.HashMap#get
      */
     @Override
     public Object get(Object key) {
-        Object value = super.get(key);
-
-        if (key != null && value == null) {
-            value = valueStack.findValue(key.toString());
+        if (key == null || super.containsKey(key)) {
+            return super.get(key);
         }
-
-        return value;
+        return valueStack.findValue(key.toString());
     }
 }

plugins/jasperreports7/src/test/java/org/apache/struts2/views/jasperreports7/JasperReport7ResultTest.java

@@ -197,6 +197,29 @@ public Map<String, String> getReportParameters() {
         assertThat(response.getContentAsString()).contains("Baz Report");
     }
 
+    public void testExplicitNullParameterIsNotShadowedByValueStack() throws Exception {
+        // given
+        result.setDataSource("{#{'firstName':'ignore', 'lastName':'ignore'}}");
+        stack.push(new Object() {
+            public String getTitle() {
+                return "Shadow";
+            }
+
+            public Map<String, Object> getReportParameters() {
+                Map<String, Object> params = new HashMap<>();
+                params.put("title", null);
+                return params;
+            }
+        });
+        result.setReportParameters("reportParameters");
+
+        // when
+        result.execute(this.invocation);
+
+        // then
+        assertThat(response.getContentAsString()).contains("null Report");
+    }
+
     public void testFillFromReportParametersWithoutDataSourceOrConnection() throws Exception {
         // given
         stack.push(new Object() {