Merge pull request #779 from apache/feature/WW-5333-attribute-map

[WW-5333] Refactors AttributeMap

core/src/main/java/org/apache/struts2/components/Set.java

@@ -20,6 +20,7 @@
 
 import java.io.Writer;
 
+import org.apache.struts2.dispatcher.DispatcherConstants;
 import org.apache.struts2.views.annotations.StrutsTag;
 import org.apache.struts2.views.annotations.StrutsTagAttribute;
 
@@ -53,16 +54,11 @@
  * <!-- START SNIPPET: params -->
  *
  * <ul>
- *
  * <li>var* (String): The name of the new variable that is assigned the value of <i>value</i></li>
- *
  * <li>value (Object): The value that is assigned to the variable named <i>name</i></li>
- *
  * <li>scope (String): The scope in which to assign the variable. Can be <b>application</b>, <b>session</b>,
  * <b>request</b>, <b>page</b>, or <b>action</b>. By default it is <b>action</b>.</li>
- * 
  * <li>Note: With the <b>action</b> scope, the variable is <em>also</em> assigned to the <b>page</b> scope.
- *
  * </ul>
  *
  * <!-- END SNIPPET: params -->
@@ -107,16 +103,16 @@ public boolean end(Writer writer, String body) {
 
         body="";
 
-        if ("application".equalsIgnoreCase(scope)) {
+        if (DispatcherConstants.APPLICATION.equalsIgnoreCase(scope)) {
             stack.setValue("#application['" + getVar() + "']", o);
-        } else if ("session".equalsIgnoreCase(scope)) {
+        } else if (DispatcherConstants.SESSION.equalsIgnoreCase(scope)) {
             stack.setValue("#session['" + getVar() + "']", o);
-        } else if ("request".equalsIgnoreCase(scope)) {
+        } else if (DispatcherConstants.REQUEST.equalsIgnoreCase(scope)) {
             stack.setValue("#request['" + getVar() + "']", o);
-        } else if ("page".equalsIgnoreCase(scope)) {
+        } else if (DispatcherConstants.PAGE.equalsIgnoreCase(scope)) {
             stack.setValue("#attr['" + getVar() + "']", o, false);
         } else {
-            // Default scope is action.  Note: The action acope handling also adds the var to the page scope.
+            // Default scope is action. Note: The action scope handling also adds the var to the page scope.
             stack.getContext().put(getVar(), o);
             stack.setValue("#attr['" + getVar() + "']", o, false);
         }

core/src/main/java/org/apache/struts2/util/AttributeMap.java → core/src/main/java/org/apache/struts2/dispatcher/AttributeMap.java

@@ -16,14 +16,16 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.struts2.util;
+package org.apache.struts2.dispatcher;
 
-import org.apache.struts2.ServletActionContext;
+import org.apache.struts2.StrutsStatics;
 
 import javax.servlet.jsp.PageContext;
+import java.util.AbstractMap;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 
 /**
@@ -38,17 +40,16 @@
  *   <li>Session scope</li>
  *   <li>Application scope</li>
  * </ul>
- *
+ * <p>
  * A object is searched in the order above, starting from page and ending at application scope.
- *
  */
-public class AttributeMap implements Map {
+public class AttributeMap extends AbstractMap<String, Object> {
 
     protected static final String UNSUPPORTED = "method makes no sense for a simplified map";
 
-    Map context;
+    private final Map<String, Object> context;
 
-    public AttributeMap(Map context) {
+    public AttributeMap(Map<String, Object> context) {
         this.context = context;
     }
 
@@ -73,18 +74,22 @@ public boolean containsValue(Object value) {
     }
 
     @Override
-    public Set entrySet() {
-        return Collections.EMPTY_SET;
+    public Set<Map.Entry<String, Object>> entrySet() {
+        return Collections.unmodifiableSet(this.context.entrySet());
     }
 
     @Override
     public Object get(Object key) {
+        if (key == null) {
+            return null;
+        }
+
         PageContext pc = getPageContext();
 
         if (pc == null) {
-            Map request = (Map) context.get("request");
-            Map session = (Map) context.get("session");
-            Map application = (Map) context.get("application");
+            RequestMap request = (RequestMap) context.get(DispatcherConstants.REQUEST);
+            SessionMap session = (SessionMap) context.get(DispatcherConstants.SESSION);
+            ApplicationMap application = (ApplicationMap) context.get(DispatcherConstants.APPLICATION);
 
             if ((request != null) && (request.get(key) != null)) {
                 return request.get(key);
@@ -94,26 +99,23 @@ public Object get(Object key) {
                 return application.get(key);
             }
         } else {
-            try {
-                return pc.findAttribute(key.toString());
-            } catch (NullPointerException npe) {
-                return null;
-            }
+            return pc.findAttribute(key.toString());
         }
 
         return null;
     }
 
     @Override
-    public Set keySet() {
-        return Collections.EMPTY_SET;
+    public Set<String> keySet() {
+        return Collections.unmodifiableSet(this.context.keySet());
     }
 
     @Override
-    public Object put(Object key, Object value) {
+    public Object put(String key, Object value) {
         PageContext pc = getPageContext();
         if (pc != null) {
-            pc.setAttribute(key.toString(), value);
+            pc.setAttribute(key, value);
+            return value;
         }
 
         return null;
@@ -135,21 +137,21 @@ public int size() {
     }
 
     @Override
-    public Collection values() {
-        return Collections.EMPTY_SET;
+    public Collection<Object> values() {
+        return Collections.unmodifiableCollection(this.context.values());
     }
 
     private PageContext getPageContext() {
-        return (PageContext) context.get(ServletActionContext.PAGE_CONTEXT);
+        return (PageContext) context.get(StrutsStatics.PAGE_CONTEXT);
     }
 
     @Override
     public String toString() {
         return "AttributeMap {" +
-                "request=" + toStringSafe(context.get("request")) +
-                ", session=" +  toStringSafe(context.get("session")) +
-                ", application=" + toStringSafe(context.get("application")) +
-                '}';
+            "request=" + toStringSafe(context.get(DispatcherConstants.REQUEST)) +
+            ", session=" + toStringSafe(context.get(DispatcherConstants.SESSION)) +
+            ", application=" + toStringSafe(context.get(DispatcherConstants.APPLICATION)) +
+            '}';
     }
 
     private String toStringSafe(Object obj) {
@@ -163,4 +165,18 @@ private String toStringSafe(Object obj) {
         }
     }
 
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (!(o instanceof AttributeMap)) return false;
+        if (!super.equals(o)) return false;
+        AttributeMap that = (AttributeMap) o;
+        return Objects.equals(context, that.context);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(super.hashCode(), context);
+    }
+
 }

core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java

@@ -67,7 +67,6 @@
 import org.apache.struts2.dispatcher.mapper.ActionMapping;
 import org.apache.struts2.dispatcher.multipart.MultiPartRequest;
 import org.apache.struts2.dispatcher.multipart.MultiPartRequestWrapper;
-import org.apache.struts2.util.AttributeMap;
 import org.apache.struts2.util.ObjectFactoryDestroyable;
 import org.apache.struts2.util.fs.JBossFileManager;
 
@@ -779,14 +778,14 @@ public Map<String, Object> createContextMap(Map<String, Object> requestMap,
             .withServletResponse(response)
             .withServletContext(servletContext)
             // helpers to get access to request/session/application scope
-            .with("request", requestMap)
-            .with("session", sessionMap)
-            .with("application", applicationMap)
-            .with("parameters", parameters)
+            .with(DispatcherConstants.REQUEST, requestMap)
+            .with(DispatcherConstants.SESSION, sessionMap)
+            .with(DispatcherConstants.APPLICATION, applicationMap)
+            .with(DispatcherConstants.PARAMETERS, parameters)
             .getContextMap();
 
         AttributeMap attrMap = new AttributeMap(extraContext);
-        extraContext.put("attr", attrMap);
+        extraContext.put(DispatcherConstants.ATTRIBUTES, attrMap);
 
         return extraContext;
     }

core/src/main/java/org/apache/struts2/dispatcher/DispatcherConstants.java

@@ -0,0 +1,33 @@
+/*
+ * 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.dispatcher;
+
+public final class DispatcherConstants {
+
+    public static final String REQUEST = "request";
+    public static final String RESPONSE = "response";
+    public static final String SESSION = "session";
+    public static final String APPLICATION = "application";
+    public static final String PARAMETERS = "parameters";
+    public static final String ATTRIBUTES = "attr";
+    public static final String PAGE = "page";
+
+    private DispatcherConstants() {
+    }
+}

core/src/main/java/org/apache/struts2/dispatcher/RequestMap.java

@@ -32,8 +32,9 @@ public class RequestMap extends AbstractMap<String, Object> implements Serializa
 
     private static final long serialVersionUID = -7675640869293787926L;
 
+    private final HttpServletRequest request;
+
     private Set<Entry<String, Object>> entries;
-    private HttpServletRequest request;
 
     /**
      * Saves the request to use as the backing for getting and setting values
@@ -44,7 +45,6 @@ public RequestMap(final HttpServletRequest request) {
         this.request = request;
     }
 
-
     /**
      * Removes all attributes from the request as well as clears entries in this map.
      */

core/src/main/java/org/apache/struts2/interceptor/debugging/DebuggingInterceptor.java

@@ -29,8 +29,10 @@
 import org.apache.logging.log4j.Logger;
 import org.apache.struts2.ServletActionContext;
 import org.apache.struts2.StrutsConstants;
+import org.apache.struts2.dispatcher.DispatcherConstants;
 import org.apache.struts2.dispatcher.Parameter;
 import org.apache.struts2.dispatcher.PrepareOperations;
+import org.apache.struts2.dispatcher.RequestMap;
 import org.apache.struts2.views.freemarker.FreemarkerManager;
 import org.apache.struts2.views.freemarker.FreemarkerResult;
 
@@ -97,11 +99,13 @@ public class DebuggingInterceptor extends AbstractInterceptor {
 
     private final static Logger LOG = LogManager.getLogger(DebuggingInterceptor.class);
 
-    private String[] ignorePrefixes = new String[]{"org.apache.struts.",
-        "com.opensymphony.xwork2.", "xwork."};
-    private String[] _ignoreKeys = new String[]{"application", "session",
-        "parameters", "request"};
-    private HashSet<String> ignoreKeys = new HashSet<>(Arrays.asList(_ignoreKeys));
+    private final String[] ignorePrefixes = new String[]{"org.apache.struts.", "com.opensymphony.xwork2.", "xwork."};
+    private final HashSet<String> ignoreKeys = new HashSet<>(Arrays.asList(
+        DispatcherConstants.APPLICATION,
+        DispatcherConstants.SESSION,
+        DispatcherConstants.PARAMETERS,
+        DispatcherConstants.REQUEST
+    ));
 
     private final static String XML_MODE = "xml";
     private final static String CONSOLE_MODE = "console";
@@ -319,7 +323,7 @@ protected void printContext(PrettyPrintWriter writer) {
             }
         }
         writer.endNode();
-        Map<String, Object> requestMap = (Map<String, Object>) ctx.get("request");
+        RequestMap requestMap = (RequestMap) ctx.get(DispatcherConstants.REQUEST);
         serializeIt(requestMap, "request", writer, filterValueStack(requestMap));
         serializeIt(ctx.getSession(), "session", writer, new ArrayList<>());
 

core/src/main/java/org/apache/struts2/views/jsp/TagUtils.java

@@ -24,7 +24,7 @@
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.apache.struts2.ServletActionContext;
-import org.apache.struts2.util.AttributeMap;
+import org.apache.struts2.dispatcher.AttributeMap;
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.jsp.PageContext;

core/src/main/java/org/apache/struts2/views/util/ContextUtil.java

@@ -20,6 +20,7 @@
 
 import com.opensymphony.xwork2.ActionInvocation;
 import com.opensymphony.xwork2.util.ValueStack;
+import org.apache.struts2.dispatcher.DispatcherConstants;
 import org.apache.struts2.util.StrutsUtil;
 
 import javax.servlet.http.HttpServletRequest;
@@ -31,9 +32,9 @@
  * Value Stack's Context related Utilities.
  */
 public class ContextUtil {
-    public static final String REQUEST = "request";
-    public static final String RESPONSE = "response";
-    public static final String SESSION = "session";
+    public static final String REQUEST = DispatcherConstants.REQUEST;
+    public static final String RESPONSE = DispatcherConstants.RESPONSE;
+    public static final String SESSION = DispatcherConstants.SESSION;
     public static final String BASE = "base";
     public static final String STACK = "stack";
     public static final String STRUTS = "struts";