diff --git a/core/src/main/java/org/apache/struts2/components/Set.java b/core/src/main/java/org/apache/struts2/components/Set.java index 8cb1ca4617..cca990ea84 100644 --- a/core/src/main/java/org/apache/struts2/components/Set.java +++ b/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 @@ * * * * * @@ -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); } diff --git a/core/src/main/java/org/apache/struts2/util/AttributeMap.java b/core/src/main/java/org/apache/struts2/dispatcher/AttributeMap.java similarity index 63% rename from core/src/main/java/org/apache/struts2/util/AttributeMap.java rename to core/src/main/java/org/apache/struts2/dispatcher/AttributeMap.java index ea088b4af8..dbdc686bbd 100644 --- a/core/src/main/java/org/apache/struts2/util/AttributeMap.java +++ b/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 @@ *
  • Session scope
    • *
  • Application scope
    • * - * + *

    * 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 { protected static final String UNSUPPORTED = "method makes no sense for a simplified map"; - Map context; + private final Map context; - public AttributeMap(Map context) { + public AttributeMap(Map context) { this.context = context; } @@ -73,18 +74,22 @@ public boolean containsValue(Object value) { } @Override - public Set entrySet() { - return Collections.EMPTY_SET; + public Set> 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 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 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); + } + } diff --git a/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java b/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java index 51ae95d27b..e378633e27 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java +++ b/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 createContextMap(Map 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; } diff --git a/core/src/main/java/org/apache/struts2/dispatcher/DispatcherConstants.java b/core/src/main/java/org/apache/struts2/dispatcher/DispatcherConstants.java new file mode 100644 index 0000000000..7ccd220cb2 --- /dev/null +++ b/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() { + } +} diff --git a/core/src/main/java/org/apache/struts2/dispatcher/RequestMap.java b/core/src/main/java/org/apache/struts2/dispatcher/RequestMap.java index a75dffb75c..b8e5b8e671 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/RequestMap.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/RequestMap.java @@ -32,8 +32,9 @@ public class RequestMap extends AbstractMap implements Serializa private static final long serialVersionUID = -7675640869293787926L; + private final HttpServletRequest request; + private Set> 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. */ diff --git a/core/src/main/java/org/apache/struts2/interceptor/debugging/DebuggingInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/debugging/DebuggingInterceptor.java index 62f8716903..7f116829e3 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/debugging/DebuggingInterceptor.java +++ b/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 ignoreKeys = new HashSet<>(Arrays.asList(_ignoreKeys)); + private final String[] ignorePrefixes = new String[]{"org.apache.struts.", "com.opensymphony.xwork2.", "xwork."}; + private final HashSet 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 requestMap = (Map) ctx.get("request"); + RequestMap requestMap = (RequestMap) ctx.get(DispatcherConstants.REQUEST); serializeIt(requestMap, "request", writer, filterValueStack(requestMap)); serializeIt(ctx.getSession(), "session", writer, new ArrayList<>()); diff --git a/core/src/main/java/org/apache/struts2/views/jsp/TagUtils.java b/core/src/main/java/org/apache/struts2/views/jsp/TagUtils.java index f813e00d1b..044f5d1af0 100644 --- a/core/src/main/java/org/apache/struts2/views/jsp/TagUtils.java +++ b/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; diff --git a/core/src/main/java/org/apache/struts2/views/util/ContextUtil.java b/core/src/main/java/org/apache/struts2/views/util/ContextUtil.java index 4ab3c27865..0f1c85aae7 100644 --- a/core/src/main/java/org/apache/struts2/views/util/ContextUtil.java +++ b/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"; diff --git a/core/src/test/java/org/apache/struts2/dispatcher/AttributeMapTest.java b/core/src/test/java/org/apache/struts2/dispatcher/AttributeMapTest.java new file mode 100644 index 0000000000..792cd0909b --- /dev/null +++ b/core/src/test/java/org/apache/struts2/dispatcher/AttributeMapTest.java @@ -0,0 +1,323 @@ +/* + * 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; + +import org.apache.struts2.StrutsStatics; +import org.junit.Test; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpSession; +import org.springframework.mock.web.MockPageContext; +import org.springframework.mock.web.MockServletContext; + +import javax.servlet.ServletContext; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpSession; +import javax.servlet.jsp.PageContext; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + +import static org.hamcrest.CoreMatchers.allOf; +import static org.hamcrest.CoreMatchers.hasItems; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.CoreMatchers.hasItem; +import static org.junit.Assert.assertThrows; + +public class AttributeMapTest { + + @Test + public void shouldRetrievePageContextAttribute() { + // given + PageContext pc = new MockPageContext(); + pc.setAttribute("attr", "value"); + + Map context = new HashMap() {{ + put(StrutsStatics.PAGE_CONTEXT, pc); + }}; + + // when + AttributeMap am = new AttributeMap(context); + Object value = am.get("attr"); + + // then + assertEquals("value", value); + } + + @Test + public void shouldPutAttribute() { + // given + PageContext pc = new MockPageContext(); + + Map context = new HashMap() {{ + put(StrutsStatics.PAGE_CONTEXT, pc); + }}; + + // when + AttributeMap am = new AttributeMap(context); + Object value = am.put("attr", "value"); + + // then + assertEquals("value", value); + assertEquals("value", pc.getAttribute("attr")); + } + + @Test + public void shouldRetrieveRequestAttribute() { + // given + HttpServletRequest request = new MockHttpServletRequest(); + request.setAttribute("attr", "value"); + + Map context = new HashMap() {{ + put(DispatcherConstants.REQUEST, new RequestMap(request)); + }}; + + // when + AttributeMap am = new AttributeMap(context); + Object value = am.get("attr"); + + // then + assertEquals("value", value); + } + + @Test + public void shouldRetrieveSessionAttribute() { + // given + HttpSession session = new MockHttpSession(); + session.setAttribute("attr", "value"); + + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setSession(session); + + Map context = new HashMap() {{ + put(DispatcherConstants.SESSION, new SessionMap(request)); + }}; + + // when + AttributeMap am = new AttributeMap(context); + Object value = am.get("attr"); + + // then + assertEquals("value", value); + } + + @Test + public void shouldRetrieveApplicationAttribute() { + // given + ServletContext sc = new MockServletContext(); + sc.setAttribute("attr", "value"); + + Map context = new HashMap() {{ + put(DispatcherConstants.APPLICATION, new ApplicationMap(sc)); + }}; + + // when + AttributeMap am = new AttributeMap(context); + Object value = am.get("attr"); + + // then + assertEquals("value", value); + } + + @Test + public void shouldReturnNullIfKeyIsNull() { + // given + // when + AttributeMap am = new AttributeMap(new HashMap<>()); + Object value = am.get(null); + + // then + assertNull(value); + } + + @Test + public void shouldThrowExceptionOnRemove() { + // given + PageContext pc = new MockPageContext(); + pc.setAttribute("attr", "value"); + + Map context = new HashMap() {{ + put(StrutsStatics.PAGE_CONTEXT, pc); + }}; + + // when + AttributeMap am = new AttributeMap(context); + + // then + Object value = am.get("attr"); + assertEquals("value", value); + + assertThrows(UnsupportedOperationException.class, () -> am.remove("attr")); + } + + @Test + public void shouldThrowExceptionOnClear() { + // given + PageContext pc = new MockPageContext(); + pc.setAttribute("attr", "value"); + + Map context = new HashMap() {{ + put(StrutsStatics.PAGE_CONTEXT, pc); + }}; + + // when + AttributeMap am = new AttributeMap(context); + Object value = am.get("attr"); + + // then + assertEquals("value", value); + + // when + assertThrows(UnsupportedOperationException.class, am::clear); + } + + @Test + public void shouldThrowExceptionOnIsEmpty() { + // given + PageContext pc = new MockPageContext(); + pc.setAttribute("attr", "value"); + + Map context = new HashMap() {{ + put(StrutsStatics.PAGE_CONTEXT, pc); + }}; + + // when + AttributeMap am = new AttributeMap(context); + Object value = am.get("attr"); + + // then + assertEquals("value", value); + + // when + assertThrows(UnsupportedOperationException.class, am::isEmpty); + } + + @Test + public void shouldThrowExceptionOnContainsValue() { + // given + PageContext pc = new MockPageContext(); + pc.setAttribute("attr", "value"); + + Map context = new HashMap() {{ + put(StrutsStatics.PAGE_CONTEXT, pc); + }}; + + // when + AttributeMap am = new AttributeMap(context); + Object value = am.get("attr"); + + // then + assertEquals("value", value); + + // when + assertThrows(UnsupportedOperationException.class, () -> am.containsValue("attr")); + } + + @Test + public void shouldThrowExceptionOnPutAll() { + // given + PageContext pc = new MockPageContext(); + pc.setAttribute("attr", "value"); + + Map context = new HashMap() {{ + put(StrutsStatics.PAGE_CONTEXT, pc); + }}; + + Map values = Collections.emptyMap(); + + // when + AttributeMap am = new AttributeMap(context); + Object value = am.get("attr"); + + // then + assertEquals("value", value); + + // when + assertThrows(UnsupportedOperationException.class, () -> am.putAll(values)); + } + + @Test + public void shouldThrowExceptionOnKeySet() { + // given + PageContext pc = new MockPageContext(); + pc.setAttribute("attr", "value"); + + Map context = new HashMap() {{ + put(StrutsStatics.PAGE_CONTEXT, pc); + }}; + + // when + AttributeMap am = new AttributeMap(context); + Object value = am.get("attr"); + + // then + assertEquals("value", value); + + // when + assertEquals(am.keySet(), context.keySet()); + } + + @Test + public void shouldThrowExceptionOnSize() { + // given + PageContext pc = new MockPageContext(); + pc.setAttribute("attr", "value"); + + Map context = new HashMap() {{ + put(StrutsStatics.PAGE_CONTEXT, pc); + }}; + + // when + AttributeMap am = new AttributeMap(context); + Object value = am.get("attr"); + + // then + assertEquals("value", value); + + // when + assertThrows(UnsupportedOperationException.class, am::size); + } + + @Test + public void shouldGetAllValues() { + // given + PageContext pc = new MockPageContext(); + pc.setAttribute("attr", "value"); + + Map context = new HashMap() {{ + put(StrutsStatics.PAGE_CONTEXT, pc); + }}; + + // when + AttributeMap am = new AttributeMap(context); + Object value = am.get("attr"); + + // then + assertEquals("value", value); + + // when + Collection values = am.values(); + + // then + assertThat(values, hasItem(pc)); + } + +} \ No newline at end of file diff --git a/plugins/portlet/src/main/java/org/apache/struts2/portlet/dispatcher/Jsr168Dispatcher.java b/plugins/portlet/src/main/java/org/apache/struts2/portlet/dispatcher/Jsr168Dispatcher.java index cc7b00c5bc..394ef60420 100644 --- a/plugins/portlet/src/main/java/org/apache/struts2/portlet/dispatcher/Jsr168Dispatcher.java +++ b/plugins/portlet/src/main/java/org/apache/struts2/portlet/dispatcher/Jsr168Dispatcher.java @@ -33,6 +33,7 @@ import org.apache.struts2.StrutsStatics; import org.apache.struts2.dispatcher.ApplicationMap; import org.apache.struts2.dispatcher.Dispatcher; +import org.apache.struts2.dispatcher.DispatcherConstants; import org.apache.struts2.dispatcher.HttpParameters; import org.apache.struts2.dispatcher.RequestMap; import org.apache.struts2.dispatcher.SessionMap; @@ -48,7 +49,7 @@ import org.apache.struts2.portlet.servlet.PortletServletContext; import org.apache.struts2.portlet.servlet.PortletServletRequest; import org.apache.struts2.portlet.servlet.PortletServletResponse; -import org.apache.struts2.util.AttributeMap; +import org.apache.struts2.dispatcher.AttributeMap; import javax.portlet.ActionRequest; import javax.portlet.ActionResponse; @@ -377,7 +378,7 @@ public Map createContextMap(Map requestMap, Map< container.inject(servletRequest); // ServletActionContext - Map extraContext = ActionContext.of(new HashMap()) + Map extraContext = ActionContext.of(new HashMap<>()) .withServletRequest(servletRequest) .withServletResponse(servletResponse) .withServletContext(servletContext) @@ -392,10 +393,10 @@ public Map createContextMap(Map requestMap, Map< .with(PORTLET_NAMESPACE, portletNamespace) .with(DEFAULT_ACTION_FOR_MODE, actionMap.get(request.getPortletMode())) // helpers to get access to request/session/application scope - .with("request", requestMap) - .with("session", sessionMap) - .with("application", applicationMap) - .with("parameters", parameterMap) + .with(DispatcherConstants.REQUEST, requestMap) + .with(DispatcherConstants.SESSION, sessionMap) + .with(DispatcherConstants.APPLICATION, applicationMap) + .with(DispatcherConstants.PARAMETERS, parameterMap) .with(MODE_NAMESPACE_MAP, modeMap) .with(PortletConstants.DEFAULT_ACTION_MAP, actionMap) .with(PortletConstants.PHASE, phase)