diff --git a/apps/showcase/pom.xml b/apps/showcase/pom.xml index 900c85693..1030d66a8 100644 --- a/apps/showcase/pom.xml +++ b/apps/showcase/pom.xml @@ -121,6 +121,11 @@ log4j-jcl ${log4j2.version} + + org.apache.logging.log4j + log4j-slf4j-impl + ${log4j2.version} + opensymphony @@ -191,10 +196,6 @@ CTRL+C 8999 - - log4j.configuration - file:${basedir}/src/main/resources/log4j2.xml - slf4j false diff --git a/apps/showcase/src/main/java/org/apache/struts2/showcase/source/ViewSourceAction.java b/apps/showcase/src/main/java/org/apache/struts2/showcase/source/ViewSourceAction.java index 541adccff..ce5b5b949 100644 --- a/apps/showcase/src/main/java/org/apache/struts2/showcase/source/ViewSourceAction.java +++ b/apps/showcase/src/main/java/org/apache/struts2/showcase/source/ViewSourceAction.java @@ -22,16 +22,13 @@ package org.apache.struts2.showcase.source; import com.opensymphony.xwork2.ActionSupport; import com.opensymphony.xwork2.util.ClassLoaderUtil; -import org.apache.struts2.ServletActionContext; import org.apache.struts2.action.ServletContextAware; import javax.servlet.ServletContext; import java.io.BufferedReader; -import java.io.File; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; -import java.net.MalformedURLException; import java.net.URL; import java.util.ArrayList; import java.util.List; @@ -42,202 +39,192 @@ import java.util.List; */ public class ViewSourceAction extends ActionSupport implements ServletContextAware { - private String page; - private String className; - private String config; + private String page; + private String className; + private String config; - private List pageLines; - private List classLines; - private List configLines; + private List pageLines; + private List classLines; + private List configLines; - private int configLine; - private int padding = 10; + private int configLine; + private int padding = 10; - private ServletContext servletContext; + private ServletContext servletContext; - public String execute() throws MalformedURLException, IOException { + public String execute() throws IOException { - if (page != null) { + if (page != null) { - InputStream in = ClassLoaderUtil.getResourceAsStream(page.substring(page.indexOf("//") + 1), getClass()); - page = page.replace("//", "/"); + InputStream in = ClassLoaderUtil.getResourceAsStream(page.substring(page.indexOf("//") + 1), getClass()); + page = page.replace("//", "/"); - if (in == null) { - in = servletContext.getResourceAsStream(page); - while (in == null && page.indexOf('/', 1) > 0) { - page = page.substring(page.indexOf('/', 1)); - in = servletContext.getResourceAsStream(page); - } - } - pageLines = read(in, -1); + if (in == null) { + in = servletContext.getResourceAsStream(page); + while (in == null && page.indexOf('/', 1) > 0) { + page = page.substring(page.indexOf('/', 1)); + in = servletContext.getResourceAsStream(page); + } + } + pageLines = read(in, -1); - if (in != null) { - in.close(); - } - } + if (in != null) { + in.close(); + } + } - if (className != null) { - className = "/" + className.replace('.', '/') + ".java"; - InputStream in = getClass().getResourceAsStream(className); - if (in == null) { - in = servletContext.getResourceAsStream("/WEB-INF/src" + className); - } - classLines = read(in, -1); + if (className != null) { + className = "/" + className.replace('.', '/') + ".java"; + InputStream in = getClass().getResourceAsStream(className); + if (in == null) { + in = servletContext.getResourceAsStream("/WEB-INF/src/java" + className); + } + classLines = read(in, -1); - if (in != null) { - in.close(); - } - } + if (in != null) { + in.close(); + } + } - final String rootPath = ServletActionContext.getServletContext().getRealPath("/"); - final String rootPathUnix = (rootPath != null ? rootPath.replace(File.separator, "/") : null); // Make path Unix-like for comparison (e.g. on Windows) - final String rootPathFileURI = "file://" + rootPathUnix; - final String collapsedRootPathFileURI = rootPathFileURI.replace("//", "/"); // Config string may have been transformed - final String rootPathWarFileURI = "war:file://" + rootPathUnix; - final String collapsedRootPathWarFileURI = rootPathWarFileURI.replace("//", "/"); // Config string may have been transformed - - if (config != null && (rootPath == null || config.startsWith(rootPath) || - config.startsWith(rootPathFileURI) || config.startsWith(collapsedRootPathFileURI) || - config.startsWith(rootPathWarFileURI) || config.startsWith(collapsedRootPathWarFileURI))) { - int pos = config.lastIndexOf(':'); - configLine = Integer.parseInt(config.substring(pos + 1)); - config = config.substring(0, pos).replace("//", "/"); - configLines = read(new URL(config).openStream(), configLine); - } - return SUCCESS; - } + if (config != null && config.startsWith("file:/")) { + int pos = config.lastIndexOf(':'); + configLine = Integer.parseInt(config.substring(pos + 1)); + configLines = read(new URL(config.substring(0, pos)).openStream(), configLine); + } + return SUCCESS; + } - /** - * @param className the className to set - */ - public void setClassName(String className) { - if (className != null && className.trim().length() > 0) { - this.className = className; - } - } + /** + * @param className the className to set + */ + public void setClassName(String className) { + if (className != null && className.trim().length() > 0) { + this.className = className; + } + } - /** - * @param config the config to set - */ - public void setConfig(String config) { - if (config != null && config.trim().length() > 0) { - this.config = config; - } - } + /** + * @param config the config to set + */ + public void setConfig(String config) { + if (config != null && config.trim().length() > 0) { + this.config = config; + } + } - /** - * @param page the page to set - */ - public void setPage(String page) { - if (page != null && page.trim().length() > 0) { - this.page = page; - } - } + /** + * @param page the page to set + */ + public void setPage(String page) { + if (page != null && page.trim().length() > 0) { + this.page = page; + } + } - /** - * @param padding the padding to set - */ - public void setPadding(int padding) { - this.padding = padding; - } + /** + * @param padding the padding to set + */ + public void setPadding(int padding) { + this.padding = padding; + } - /** - * @return the classLines - */ - public List getClassLines() { - return classLines; - } + /** + * @return the classLines + */ + public List getClassLines() { + return classLines; + } - /** - * @return the configLines - */ - public List getConfigLines() { - return configLines; - } + /** + * @return the configLines + */ + public List getConfigLines() { + return configLines; + } - /** - * @return the pageLines - */ - public List getPageLines() { - return pageLines; - } + /** + * @return the pageLines + */ + public List getPageLines() { + return pageLines; + } - /** - * @return the className - */ - public String getClassName() { - return className; - } + /** + * @return the className + */ + public String getClassName() { + return className; + } - /** - * @return the config - */ - public String getConfig() { - return config; - } + /** + * @return the config + */ + public String getConfig() { + return config; + } - /** - * @return the page - */ - public String getPage() { - return page; - } + /** + * @return the page + */ + public String getPage() { + return page; + } - /** - * @return the configLine - */ - public int getConfigLine() { - return configLine; - } + /** + * @return the configLine + */ + public int getConfigLine() { + return configLine; + } - /** - * @return the padding - */ - public int getPadding() { - return padding; - } + /** + * @return the padding + */ + public int getPadding() { + return padding; + } - /** - * Reads in a stream, optionally only including the target line number - * and its padding - * - * @param in The input stream - * @param targetLineNumber The target line number, negative to read all - * @return A list of lines - */ - private List read(InputStream in, int targetLineNumber) { - List snippet = null; - if (in != null) { - snippet = new ArrayList(); - int startLine = 0; - int endLine = Integer.MAX_VALUE; - if (targetLineNumber > 0) { - startLine = targetLineNumber - padding; - endLine = targetLineNumber + padding; - } - try { - BufferedReader reader = new BufferedReader(new InputStreamReader(in)); + /** + * Reads in a stream, optionally only including the target line number + * and its padding + * + * @param in The input stream + * @param targetLineNumber The target line number, negative to read all + * @return A list of lines + */ + private List read(InputStream in, int targetLineNumber) { + List snippet = null; + if (in != null) { + snippet = new ArrayList<>(); + int startLine = 0; + int endLine = Integer.MAX_VALUE; + if (targetLineNumber > 0) { + startLine = targetLineNumber - padding; + endLine = targetLineNumber + padding; + } + try { + BufferedReader reader = new BufferedReader(new InputStreamReader(in)); - int lineno = 0; - String line; - while ((line = reader.readLine()) != null) { - lineno++; - if (lineno >= startLine && lineno <= endLine) { - snippet.add(line); - } - } - } catch (Exception ex) { - // ignoring as snippet not available isn't a big deal - } - } - return snippet; - } + int lineno = 0; + String line; + while ((line = reader.readLine()) != null) { + lineno++; + if (lineno >= startLine && lineno <= endLine) { + snippet.add(line); + } + } + } catch (Exception ex) { + // ignoring as snippet not available isn't a big deal + } + } + return snippet; + } - public void withServletContext(ServletContext arg0) { - this.servletContext = arg0; - } + public void withServletContext(ServletContext arg0) { + this.servletContext = arg0; + } } diff --git a/apps/showcase/src/main/resources/log4j2.xml b/apps/showcase/src/main/resources/log4j2.xml index 4b7a86515..33dafdd7e 100644 --- a/apps/showcase/src/main/resources/log4j2.xml +++ b/apps/showcase/src/main/resources/log4j2.xml @@ -22,15 +22,14 @@ - + - - - + + - \ No newline at end of file + diff --git a/apps/showcase/src/main/resources/struts-dispatcher.xml b/apps/showcase/src/main/resources/struts-dispatcher.xml new file mode 100644 index 000000000..0a89502fe --- /dev/null +++ b/apps/showcase/src/main/resources/struts-dispatcher.xml @@ -0,0 +1,40 @@ + + + + + + + + + + /WEB-INF/dispatcher/dispatch-result.jsp + + + + + /dispatcher/dispatch.action + + + + diff --git a/apps/showcase/src/main/resources/struts.xml b/apps/showcase/src/main/resources/struts.xml index 36d0370c3..cfdac4b39 100644 --- a/apps/showcase/src/main/resources/struts.xml +++ b/apps/showcase/src/main/resources/struts.xml @@ -78,6 +78,8 @@ + + diff --git a/apps/showcase/src/main/webapp/WEB-INF/decorators/main.jsp b/apps/showcase/src/main/webapp/WEB-INF/decorators/main.jsp index 0d09eb647..d63f61f9a 100644 --- a/apps/showcase/src/main/webapp/WEB-INF/decorators/main.jsp +++ b/apps/showcase/src/main/webapp/WEB-INF/decorators/main.jsp @@ -244,6 +244,8 @@
  • Model Driven
  • Async
    • +
  • Dispatcher result - dispatch
    • +
  • Dispatcher result - forward
  • diff --git a/apps/showcase/src/main/webapp/WEB-INF/dispatcher/dispatch-result.jsp b/apps/showcase/src/main/webapp/WEB-INF/dispatcher/dispatch-result.jsp new file mode 100644 index 000000000..d493b146b --- /dev/null +++ b/apps/showcase/src/main/webapp/WEB-INF/dispatcher/dispatch-result.jsp @@ -0,0 +1,42 @@ + +<%@ page contentType="text/html; charset=UTF-8" pageEncoding="UTF-8" %> +<%@ taglib prefix="s" uri="/struts-tags" %> + + + Struts2 Showcase - Dispatcher result Example + + + + +
    +

    Dispatcher Result Example

    +
    + +
    +
    +
    + This page is a result of "dispatching" to it from an action +
    +
    +
    + + diff --git a/apps/showcase/src/main/webapp/WEB-INF/viewSource.jsp b/apps/showcase/src/main/webapp/WEB-INF/viewSource.jsp index 1255d2a3b..61bb6fc87 100644 --- a/apps/showcase/src/main/webapp/WEB-INF/viewSource.jsp +++ b/apps/showcase/src/main/webapp/WEB-INF/viewSource.jsp @@ -1,19 +1,19 @@ sitemesh-freemarker @@ -113,7 +129,7 @@ default_encoding UTF-8 - 1 + 3 @@ -123,14 +139,7 @@ default_encoding UTF-8 - 1 - - - - strutsServlet - org.apache.struts2.dispatcher.servlet.StrutsServlet - 1 - true + 4 diff --git a/apps/showcase/src/test/java/it/org/apache/struts2/showcase/DispatcherResultTest.java b/apps/showcase/src/test/java/it/org/apache/struts2/showcase/DispatcherResultTest.java new file mode 100644 index 000000000..6564270c2 --- /dev/null +++ b/apps/showcase/src/test/java/it/org/apache/struts2/showcase/DispatcherResultTest.java @@ -0,0 +1,56 @@ +/* + * 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 it.org.apache.struts2.showcase; + +import com.gargoylesoftware.htmlunit.WebClient; +import com.gargoylesoftware.htmlunit.html.DomElement; +import com.gargoylesoftware.htmlunit.html.HtmlPage; +import org.junit.Assert; +import org.junit.Test; + +public class DispatcherResultTest { + + @Test + public void testDispatchingToJSP() throws Exception { + try (final WebClient webClient = new WebClient()) { + final HtmlPage page = webClient.getPage(ParameterUtils.getBaseUrl() + "/dispatcher/dispatch.action"); + + DomElement div = page.getElementById("dispatcher-result"); + + Assert.assertEquals("This page is a result of \"dispatching\" to it from an action", div.asNormalizedText()); + } + } + + @Test + public void testDispatchingToAction() throws Exception { + try (final WebClient webClient = new WebClient()) { + webClient.getOptions().setThrowExceptionOnFailingStatusCode(false); + final HtmlPage page = webClient.getPage(ParameterUtils.getBaseUrl() + "/dispatcher/forward.action"); + + //DomElement div = page.getElementById("dispatcher-result"); + //Assert.assertEquals("This page is a result of \"dispatching\" to it from an action", div.asNormalizedText()); + // support for forwarding to another action is broken on StrutsPrepareFilter/StrutsExecuteFilter + // it only works in StrutsPrepareAndExecuteFilter + // this will be fixed in Struts 6.1.x + + Assert.assertEquals(404, page.getWebResponse().getStatusCode()); + } + } + +} 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 7b9a8e1da..43794c1c5 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java @@ -83,6 +83,7 @@ import java.util.HashSet; import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.concurrent.CopyOnWriteArrayList; import java.util.regex.Pattern; @@ -612,20 +613,12 @@ public class Dispatcher { } try { - String namespace = mapping.getNamespace(); - String name = mapping.getName(); - String method = mapping.getMethod(); + String actionNamespace = mapping.getNamespace(); + String actionName = mapping.getName(); + String actionMethod = mapping.getMethod(); - ActionProxy proxy; - - //check if we are probably in an async resuming - ActionInvocation invocation = ActionContext.getContext().getActionInvocation(); - if (invocation == null || invocation.isExecuted()) { - proxy = getContainer().getInstance(ActionProxyFactory.class).createActionProxy(namespace, name, method, - extraContext, true, false); - } else { - proxy = invocation.getProxy(); - } + LOG.trace("Processing action, namespace: {}, name: {}, method: {}", actionNamespace, actionName, actionMethod); + ActionProxy proxy = prepareActionProxy(extraContext, actionNamespace, actionName, actionMethod); request.setAttribute(ServletActionContext.STRUTS_VALUESTACK_KEY, proxy.getInvocation().getStack()); @@ -656,6 +649,36 @@ public class Dispatcher { } } + private ActionProxy prepareActionProxy(Map extraContext, String actionNamespace, String actionName, String actionMethod) { + ActionProxy proxy; + //check if we are probably in an async resuming + ActionInvocation invocation = ActionContext.getContext().getActionInvocation(); + if (invocation == null || invocation.isExecuted()) { + LOG.trace("Creating a new action, namespace: {}, name: {}, method: {}", actionNamespace, actionName, actionMethod); + proxy = createActionProxy(actionNamespace, actionName, actionMethod, extraContext); + } else { + proxy = invocation.getProxy(); + if (isSameAction(proxy, actionNamespace, actionName, actionMethod)) { + LOG.trace("Proxy: {} matches requested action, namespace: {}, name: {}, method: {} - reusing proxy", proxy, actionNamespace, actionName, actionMethod); + } else { + LOG.trace("Proxy: {} doesn't match action namespace: {}, name: {}, method: {} - creating new proxy", proxy, actionNamespace, actionName, actionMethod); + proxy = createActionProxy(actionNamespace, actionName, actionMethod, extraContext); + } + } + return proxy; + } + + private ActionProxy createActionProxy(String namespace, String name, String method, Map extraContext) { + ActionProxyFactory actionProxyFactory = getContainer().getInstance(ActionProxyFactory.class); + return actionProxyFactory.createActionProxy(namespace, name, method, extraContext, true, false); + } + + private boolean isSameAction(ActionProxy actionProxy, String namespace, String actionName, String method) { + return Objects.equals(namespace, actionProxy.getNamespace()) + && Objects.equals(actionName, actionProxy.getActionName()) + && Objects.equals(method, actionProxy.getMethod()); + } + /** * Performs logging of missing action/result configuration exception * diff --git a/core/src/main/java/org/apache/struts2/dispatcher/filter/StrutsPrepareAndExecuteFilter.java b/core/src/main/java/org/apache/struts2/dispatcher/filter/StrutsPrepareAndExecuteFilter.java index 73f5860ec..54ee6883d 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/filter/StrutsPrepareAndExecuteFilter.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/filter/StrutsPrepareAndExecuteFilter.java @@ -126,18 +126,18 @@ public class StrutsPrepareAndExecuteFilter implements StrutsStatics, Filter { LOG.trace("Checking if {} is a static resource", uri); boolean handled = execute.executeStaticResourceRequest(request, response); if (!handled) { - LOG.trace("Assuming uri {} as a normal action", uri); + LOG.trace("Uri {} is not a static resource, assuming action", uri); prepare.setEncodingAndLocale(request, response); prepare.createActionContext(request, response); prepare.assignDispatcherToThread(); - request = prepare.wrapRequest(request); - ActionMapping mapping = prepare.findActionMapping(request, response, true); + HttpServletRequest wrappedRequest = prepare.wrapRequest(request); + ActionMapping mapping = prepare.findActionMapping(wrappedRequest, response, true); if (mapping == null) { LOG.trace("Cannot find mapping for {}, passing to other filters", uri); chain.doFilter(request, response); } else { LOG.trace("Found mapping {} for {}", mapping, uri); - execute.executeAction(request, response, mapping); + execute.executeAction(wrappedRequest, response, mapping); } } } diff --git a/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java b/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java index a9679a3b1..9c09e187e 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java @@ -21,6 +21,7 @@ package org.apache.struts2.dispatcher.mapper; import com.opensymphony.xwork2.ActionContext; import com.opensymphony.xwork2.config.Configuration; import com.opensymphony.xwork2.config.ConfigurationManager; +import com.opensymphony.xwork2.config.entities.ActionConfig; import com.opensymphony.xwork2.config.entities.PackageConfig; import com.opensymphony.xwork2.inject.Container; import com.opensymphony.xwork2.inject.Inject; @@ -29,7 +30,6 @@ import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.struts2.RequestUtils; -import org.apache.struts2.ServletActionContext; import org.apache.struts2.StrutsConstants; import org.apache.struts2.util.PrefixTrie; @@ -115,7 +115,7 @@ public class DefaultActionMapper implements ActionMapper { protected boolean allowDynamicMethodCalls = false; protected boolean allowSlashesInActionNames = false; protected boolean alwaysSelectFullNamespace = false; - protected PrefixTrie prefixTrie = null; + protected PrefixTrie prefixTrie; protected Pattern allowedNamespaceNames = Pattern.compile("[a-zA-Z0-9._/\\-]*"); protected String defaultNamespaceName = "/"; @@ -139,39 +139,35 @@ public class DefaultActionMapper implements ActionMapper { public DefaultActionMapper() { prefixTrie = new PrefixTrie() { { - put(METHOD_PREFIX, new ParameterAction() { - public void execute(String key, ActionMapping mapping) { - if (allowDynamicMethodCalls) { - mapping.setMethod(cleanupMethodName(key.substring(METHOD_PREFIX.length()))); - } + put(METHOD_PREFIX, (ParameterAction) (key, mapping) -> { + if (allowDynamicMethodCalls) { + mapping.setMethod(cleanupMethodName(key.substring(METHOD_PREFIX.length()))); } }); - put(ACTION_PREFIX, new ParameterAction() { - public void execute(final String key, ActionMapping mapping) { - if (allowActionPrefix) { - String name = key.substring(ACTION_PREFIX.length()); - if (allowDynamicMethodCalls) { - int bang = name.indexOf('!'); - if (bang != -1) { - String method = cleanupMethodName(name.substring(bang + 1)); - mapping.setMethod(method); - name = name.substring(0, bang); - } + put(ACTION_PREFIX, (ParameterAction) (key, mapping) -> { + if (allowActionPrefix) { + String name = key.substring(ACTION_PREFIX.length()); + if (allowDynamicMethodCalls) { + int bang = name.indexOf('!'); + if (bang != -1) { + String method = cleanupMethodName(name.substring(bang + 1)); + mapping.setMethod(method); + name = name.substring(0, bang); } - String actionName = cleanupActionName(name); - if (allowSlashesInActionNames && !allowActionCrossNamespaceAccess) { - if (actionName.startsWith("/")) { - actionName = actionName.substring(1); - } - } - if (!allowSlashesInActionNames && !allowActionCrossNamespaceAccess) { - if (actionName.lastIndexOf('/') != -1) { - actionName = actionName.substring(actionName.lastIndexOf('/') + 1); - } - } - mapping.setName(actionName); } + String actionName = cleanupActionName(name); + if (allowSlashesInActionNames && !allowActionCrossNamespaceAccess) { + if (actionName.startsWith("/")) { + actionName = actionName.substring(1); + } + } + if (!allowSlashesInActionNames && !allowActionCrossNamespaceAccess) { + if (actionName.lastIndexOf('/') != -1) { + actionName = actionName.substring(actionName.lastIndexOf('/') + 1); + } + } + mapping.setName(actionName); } }); @@ -293,6 +289,7 @@ public class DefaultActionMapper implements ActionMapper { } parseNameAndNamespace(uri, mapping, configManager); + extractMethodName(mapping, configManager); handleSpecialParameters(request, mapping); return parseActionName(mapping); } @@ -324,9 +321,8 @@ public class DefaultActionMapper implements ActionMapper { public void handleSpecialParameters(HttpServletRequest request, ActionMapping mapping) { // handle special parameter prefixes. Set uniqueParameters = new HashSet<>(); - Map parameterMap = request.getParameterMap(); - for (Object o : parameterMap.keySet()) { - String key = (String) o; + Map parameterMap = request.getParameterMap(); + for (String key : parameterMap.keySet()) { // Strip off the image button location info, if found if (key.endsWith(".x") || key.endsWith(".y")) { @@ -353,33 +349,33 @@ public class DefaultActionMapper implements ActionMapper { * @param configManager configuration manager */ protected void parseNameAndNamespace(String uri, ActionMapping mapping, ConfigurationManager configManager) { - String namespace, name; + String actionNamespace, actionName; int lastSlash = uri.lastIndexOf('/'); if (lastSlash == -1) { - namespace = ""; - name = uri; + actionNamespace = ""; + actionName = uri; } else if (lastSlash == 0) { // ww-1046, assume it is the root namespace, it will fallback to // default // namespace anyway if not found in root namespace. - namespace = "/"; - name = uri.substring(lastSlash + 1); + actionNamespace = "/"; + actionName = uri.substring(lastSlash + 1); } else if (alwaysSelectFullNamespace) { // Simply select the namespace as everything before the last slash - namespace = uri.substring(0, lastSlash); - name = uri.substring(lastSlash + 1); + actionNamespace = uri.substring(0, lastSlash); + actionName = uri.substring(lastSlash + 1); } else { // Try to find the namespace in those defined, defaulting to "" Configuration config = configManager.getConfiguration(); String prefix = uri.substring(0, lastSlash); - namespace = ""; + actionNamespace = ""; boolean rootAvailable = false; // Find the longest matching namespace, defaulting to the default for (PackageConfig cfg : config.getPackageConfigs().values()) { String ns = cfg.getNamespace(); if (ns != null && prefix.startsWith(ns) && (prefix.length() == ns.length() || prefix.charAt(ns.length()) == '/')) { - if (ns.length() > namespace.length()) { - namespace = ns; + if (ns.length() > actionNamespace.length()) { + actionNamespace = ns; } } if ("/".equals(ns)) { @@ -387,23 +383,23 @@ public class DefaultActionMapper implements ActionMapper { } } - name = uri.substring(namespace.length() + 1); + actionName = uri.substring(actionNamespace.length() + 1); // Still none found, use root namespace if found - if (rootAvailable && "".equals(namespace)) { - namespace = "/"; + if (rootAvailable && "".equals(actionNamespace)) { + actionNamespace = "/"; } } if (!allowSlashesInActionNames) { - int pos = name.lastIndexOf('/'); - if (pos > -1 && pos < name.length() - 1) { - name = name.substring(pos + 1); + int pos = actionName.lastIndexOf('/'); + if (pos > -1 && pos < actionName.length() - 1) { + actionName = actionName.substring(pos + 1); } } - mapping.setNamespace(cleanupNamespaceName(namespace)); - mapping.setName(cleanupActionName(name)); + mapping.setNamespace(cleanupNamespaceName(actionNamespace)); + mapping.setName(cleanupActionName(actionName)); } /** @@ -454,6 +450,30 @@ public class DefaultActionMapper implements ActionMapper { } } + /** + * Reads defined method name for a given action from configuration + * + * @param mapping current instance of {@link ActionMapping} + * @param configurationManager current instance of {@link ConfigurationManager} + */ + protected void extractMethodName(ActionMapping mapping, ConfigurationManager configurationManager) { + String methodName = null; + for (PackageConfig cfg : configurationManager.getConfiguration().getPackageConfigs().values()) { + if (cfg.getNamespace().equals(mapping.getNamespace())) { + ActionConfig actionCfg = cfg.getActionConfigs().get(mapping.getName()); + if (actionCfg != null) { + methodName = actionCfg.getMethodName(); + LOG.trace("Using method: {} for action mapping: {}", methodName, mapping); + } else { + LOG.debug("No action config for action mapping: {}", mapping); + } + break; + } + } + + mapping.setMethod(methodName); + } + /** * Drops the extension from the action name, storing it in the mapping for later use * @@ -551,7 +571,7 @@ public class DefaultActionMapper implements ActionMapper { String extension = lookupExtension(mapping.getExtension()); if (extension != null) { - if (extension.length() == 0 || (extension.length() > 0 && uri.indexOf('.' + extension) == -1)) { + if (extension.length() == 0 || uri.indexOf('.' + extension) == -1) { if (extension.length() > 0) { uri.append(".").append(extension); } diff --git a/core/src/test/java/org/apache/struts2/dispatcher/DispatcherTest.java b/core/src/test/java/org/apache/struts2/dispatcher/DispatcherTest.java index b71c94b37..6e996df0d 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/DispatcherTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/DispatcherTest.java @@ -21,6 +21,7 @@ package org.apache.struts2.dispatcher; import com.mockobjects.dynamic.C; import com.mockobjects.dynamic.Mock; import com.opensymphony.xwork2.ActionContext; +import com.opensymphony.xwork2.LocalizedTextProvider; import com.opensymphony.xwork2.ObjectFactory; import com.opensymphony.xwork2.StubValueStack; import com.opensymphony.xwork2.config.Configuration; @@ -30,7 +31,6 @@ import com.opensymphony.xwork2.config.entities.InterceptorStackConfig; import com.opensymphony.xwork2.config.entities.PackageConfig; import com.opensymphony.xwork2.inject.Container; import com.opensymphony.xwork2.interceptor.Interceptor; -import com.opensymphony.xwork2.LocalizedTextProvider; import com.opensymphony.xwork2.mock.MockActionInvocation; import com.opensymphony.xwork2.mock.MockActionProxy; import org.apache.struts2.ServletActionContext; @@ -54,24 +54,23 @@ import java.util.Map; /** * Test case for Dispatcher. - * */ public class DispatcherTest extends StrutsInternalTestCase { - public void testDefaultResurceBundlePropertyLoaded() throws Exception { + public void testDefaultResourceBundlePropertyLoaded() { LocalizedTextProvider localizedTextProvider = container.getInstance(LocalizedTextProvider.class); // some i18n messages from xwork-messages.properties assertEquals(localizedTextProvider.findDefaultText("xwork.error.action.execution", Locale.US), - "Error during Action invocation"); + "Error during Action invocation"); // some i18n messages from struts-messages.properties assertEquals(localizedTextProvider.findDefaultText("struts.messages.error.uploading", Locale.US, - new Object[] { "some error messages" }), - "Error uploading: some error messages"); + new Object[]{"some error messages"}), + "Error uploading: some error messages"); } - public void testPrepareSetEncodingProperly() throws Exception { + public void testPrepareSetEncodingProperly() { HttpServletRequest req = new MockHttpServletRequest(); HttpServletResponse res = new MockHttpServletResponse(); @@ -84,7 +83,7 @@ public class DispatcherTest extends StrutsInternalTestCase { assertEquals(res.getCharacterEncoding(), "utf-8"); } - public void testEncodingForXMLHttpRequest() throws Exception { + public void testEncodingForXMLHttpRequest() { // given MockHttpServletRequest req = new MockHttpServletRequest(); req.addHeader("X-Requested-With", "XMLHttpRequest"); @@ -103,7 +102,7 @@ public class DispatcherTest extends StrutsInternalTestCase { assertEquals(res.getCharacterEncoding(), "UTF-8"); } - public void testSetEncodingIfDiffer() throws Exception { + public void testSetEncodingIfDiffer() { // given Mock mock = new Mock(HttpServletRequest.class); mock.expectAndReturn("getCharacterEncoding", "utf-8"); @@ -127,7 +126,7 @@ public class DispatcherTest extends StrutsInternalTestCase { mock.verify(); } - public void testPrepareSetEncodingPropertyWithMultipartRequest() throws Exception { + public void testPrepareSetEncodingPropertyWithMultipartRequest() { MockHttpServletRequest req = new MockHttpServletRequest(); MockHttpServletResponse res = new MockHttpServletResponse(); @@ -147,7 +146,7 @@ public class DispatcherTest extends StrutsInternalTestCase { req.setMethod("post"); req.setContentType("multipart/form-data; boundary=asdcvb345asd"); - Dispatcher du = initDispatcher(Collections.emptyMap()); + Dispatcher du = initDispatcher(Collections.emptyMap()); du.prepare(req, res); HttpServletRequest wrapped = du.wrapRequest(req); @@ -160,7 +159,7 @@ public class DispatcherTest extends StrutsInternalTestCase { req.setMethod("post"); req.setContentType("multipart/form-data; boundary=01=23a.bC:D((e)d'z?p+o_r,e-"); - Dispatcher du = initDispatcher(Collections.emptyMap()); + Dispatcher du = initDispatcher(Collections.emptyMap()); du.prepare(req, res); HttpServletRequest wrapped = du.wrapRequest(req); @@ -173,46 +172,46 @@ public class DispatcherTest extends StrutsInternalTestCase { req.setMethod("post"); req.setContentType("multipart/form-data; boundary=01=2;3a.bC:D((e)d'z?p+o_r,e-"); - Dispatcher du = initDispatcher(Collections.emptyMap()); + Dispatcher du = initDispatcher(Collections.emptyMap()); du.prepare(req, res); HttpServletRequest wrapped = du.wrapRequest(req); assertFalse(wrapped instanceof MultiPartRequestWrapper); } - public void testDispatcherListener() throws Exception { + public void testDispatcherListener() { - final DispatcherListenerState state = new DispatcherListenerState(); + final DispatcherListenerState state = new DispatcherListenerState(); - Dispatcher.addDispatcherListener(new DispatcherListener() { - public void dispatcherDestroyed(Dispatcher du) { - state.isDestroyed = true; - } - public void dispatcherInitialized(Dispatcher du) { - state.isInitialized = true; - } - }); + Dispatcher.addDispatcherListener(new DispatcherListener() { + public void dispatcherDestroyed(Dispatcher du) { + state.isDestroyed = true; + } + + public void dispatcherInitialized(Dispatcher du) { + state.isInitialized = true; + } + }); - assertFalse(state.isDestroyed); - assertFalse(state.isInitialized); + assertFalse(state.isDestroyed); + assertFalse(state.isInitialized); - Dispatcher du = initDispatcher(new HashMap() ); + Dispatcher du = initDispatcher(new HashMap<>()); - assertTrue(state.isInitialized); + assertTrue(state.isInitialized); - du.cleanup(); + du.cleanup(); - assertTrue(state.isDestroyed); + assertTrue(state.isDestroyed); } - public void testConfigurationManager() { - Dispatcher du; - final InternalConfigurationManager configurationManager = new InternalConfigurationManager(Container.DEFAULT_NAME); - try { - du = new MockDispatcher(new MockServletContext(), new HashMap(), configurationManager); - du.init(); + Dispatcher du; + final InternalConfigurationManager configurationManager = new InternalConfigurationManager(Container.DEFAULT_NAME); + try { + du = new MockDispatcher(new MockServletContext(), new HashMap<>(), configurationManager); + du.init(); Dispatcher.setInstance(du); assertFalse(configurationManager.destroyConfiguration); @@ -221,18 +220,17 @@ public class DispatcherTest extends StrutsInternalTestCase { assertTrue(configurationManager.destroyConfiguration); - } - finally { - Dispatcher.setInstance(null); - } + } finally { + Dispatcher.setInstance(null); + } } public void testInitLoadsDefaultConfig() { - Dispatcher du = new Dispatcher(new MockServletContext(), new HashMap()); + Dispatcher du = new Dispatcher(new MockServletContext(), new HashMap<>()); du.init(); Configuration config = du.getConfigurationManager().getConfiguration(); assertNotNull(config); - HashSet expected = new HashSet(); + HashSet expected = new HashSet<>(); expected.add("struts-default.xml"); expected.add("struts-plugin.xml"); expected.add("struts.xml"); @@ -243,17 +241,17 @@ public class DispatcherTest extends StrutsInternalTestCase { assertTrue(packageConfig.getResultTypeConfigs().size() > 0); } - public void testObjectFactoryDestroy() throws Exception { + public void testObjectFactoryDestroy() { ConfigurationManager cm = new ConfigurationManager(Container.DEFAULT_NAME); - Dispatcher du = new MockDispatcher(new MockServletContext(), new HashMap(), cm); + Dispatcher du = new MockDispatcher(new MockServletContext(), new HashMap<>(), cm); Mock mockConfiguration = new Mock(Configuration.class); - cm.setConfiguration((Configuration)mockConfiguration.proxy()); + cm.setConfiguration((Configuration) mockConfiguration.proxy()); Mock mockContainer = new Mock(Container.class); String reloadConfigs = container.getInstance(String.class, StrutsConstants.STRUTS_CONFIGURATION_XML_RELOAD); mockContainer.expectAndReturn("getInstance", C.args(C.eq(String.class), C.eq(StrutsConstants.STRUTS_CONFIGURATION_XML_RELOAD)), - reloadConfigs); + reloadConfigs); final InnerDestroyableObjectFactory destroyedObjectFactory = new InnerDestroyableObjectFactory(); destroyedObjectFactory.setContainer((Container) mockContainer.proxy()); mockContainer.expectAndReturn("getInstance", C.args(C.eq(ObjectFactory.class)), destroyedObjectFactory); @@ -271,7 +269,7 @@ public class DispatcherTest extends StrutsInternalTestCase { mockContainer.verify(); } - public void testInterceptorDestroy() throws Exception { + public void testInterceptorDestroy() { Mock mockInterceptor = new Mock(Interceptor.class); mockInterceptor.matchAndReturn("hashCode", 0); mockInterceptor.expect("destroy"); @@ -282,14 +280,14 @@ public class DispatcherTest extends StrutsInternalTestCase { PackageConfig packageConfig = new PackageConfig.Builder("test").addInterceptorStackConfig(isc).build(); - Map packageConfigs = new HashMap(); + Map packageConfigs = new HashMap<>(); packageConfigs.put("test", packageConfig); Mock mockContainer = new Mock(Container.class); mockContainer.matchAndReturn("getInstance", C.args(C.eq(ObjectFactory.class)), new ObjectFactory()); String reloadConfigs = container.getInstance(String.class, StrutsConstants.STRUTS_CONFIGURATION_XML_RELOAD); mockContainer.expectAndReturn("getInstance", C.args(C.eq(String.class), C.eq(StrutsConstants.STRUTS_CONFIGURATION_XML_RELOAD)), - reloadConfigs); + reloadConfigs); Mock mockConfiguration = new Mock(Configuration.class); mockConfiguration.matchAndReturn("getPackageConfigs", packageConfigs); @@ -299,7 +297,7 @@ public class DispatcherTest extends StrutsInternalTestCase { ConfigurationManager configurationManager = new ConfigurationManager(Container.DEFAULT_NAME); configurationManager.setConfiguration((Configuration) mockConfiguration.proxy()); - Dispatcher dispatcher = new MockDispatcher(new MockServletContext(), new HashMap(), configurationManager); + Dispatcher dispatcher = new MockDispatcher(new MockServletContext(), new HashMap<>(), configurationManager); dispatcher.init(); dispatcher.cleanup(); @@ -308,22 +306,22 @@ public class DispatcherTest extends StrutsInternalTestCase { mockConfiguration.verify(); } - public void testMultipartSupportEnabledByDefault() throws Exception { + public void testMultipartSupportEnabledByDefault() { HttpServletRequest req = new MockHttpServletRequest(); HttpServletResponse res = new MockHttpServletResponse(); - Dispatcher du = initDispatcher(Collections.emptyMap()); + Dispatcher du = initDispatcher(Collections.emptyMap()); du.prepare(req, res); assertTrue(du.isMultipartSupportEnabled(req)); } - public void testIsMultipartRequest() throws Exception { + public void testIsMultipartRequest() { MockHttpServletRequest req = new MockHttpServletRequest(); HttpServletResponse res = new MockHttpServletResponse(); req.setMethod("POST"); - Dispatcher du = initDispatcher(Collections.emptyMap()); + Dispatcher du = initDispatcher(Collections.emptyMap()); du.prepare(req, res); req.setContentType("multipart/form-data"); @@ -370,7 +368,7 @@ public class DispatcherTest extends StrutsInternalTestCase { } public void testServiceActionResumePreviousProxy() throws Exception { - Dispatcher du = initDispatcher(Collections.emptyMap()); + Dispatcher du = initDispatcher(Collections.emptyMap()); MockActionInvocation mai = new MockActionInvocation(); ActionContext.getContext().withActionInvocation(mai); @@ -393,14 +391,41 @@ public class DispatcherTest extends StrutsInternalTestCase { assertTrue("should execute previous proxy", actionProxy.isExecutedCalled()); } + public void testServiceActionCreatesNewProxyIfDifferentMapping() throws Exception { + Dispatcher du = initDispatcher(Collections.emptyMap()); + container.inject(du); + + MockActionInvocation mai = new MockActionInvocation(); + ActionContext.getContext().withActionInvocation(mai); + + MockActionProxy previousActionProxy = new MockActionProxy(); + previousActionProxy.setActionName("first-action"); + previousActionProxy.setNamespace("namespace1"); + previousActionProxy.setInvocation(mai); + mai.setProxy(previousActionProxy); + + mai.setStack(new StubValueStack()); + + HttpServletRequest request = new MockHttpServletRequest(); + request.setAttribute(ServletActionContext.STRUTS_VALUESTACK_KEY, mai.getStack()); + + HttpServletResponse response = new MockHttpServletResponse(); + + assertFalse(previousActionProxy.isExecutedCalled()); + + ActionMapping newActionMapping = new ActionMapping(); + newActionMapping.setName("hello"); + du.serviceAction(request, response, newActionMapping); + + assertFalse(previousActionProxy.isExecutedCalled()); + } + /** * Verify proper default (true) handleExceptionState for Dispatcher and that * it properly reflects a manually configured change to false. - * - * @throws Exception */ - public void testHandleException() throws Exception { - Dispatcher du = initDispatcher(new HashMap()); + public void testHandleException() { + Dispatcher du = initDispatcher(new HashMap<>()); assertTrue("Default Dispatcher handleException state not true ?", du.isHandleException()); Dispatcher du2 = initDispatcher(new HashMap() {{ @@ -412,11 +437,9 @@ public class DispatcherTest extends StrutsInternalTestCase { /** * Verify proper default (false) devMode for Dispatcher and that * it properly reflects a manually configured change to true. - * - * @throws Exception */ - public void testDevMode() throws Exception { - Dispatcher du = initDispatcher(new HashMap()); + public void testDevMode() { + Dispatcher du = initDispatcher(new HashMap<>()); assertFalse("Default Dispatcher devMode state not false ?", du.isDevMode()); Dispatcher du2 = initDispatcher(new HashMap() {{ @@ -425,7 +448,7 @@ public class DispatcherTest extends StrutsInternalTestCase { assertTrue("Modified Dispatcher devMode state not true ?", du2.isDevMode()); } - public void testGetLocale_With_DefaultLocale_FromConfiguration() throws Exception { + public void testGetLocale_With_DefaultLocale_FromConfiguration() { // Given Mock mock = new Mock(HttpServletRequest.class); MockHttpSession mockHttpSession = new MockHttpSession(); @@ -451,7 +474,7 @@ public class DispatcherTest extends StrutsInternalTestCase { mock.verify(); } - public void testGetLocale_With_DefaultLocale_fr_CA() throws Exception { + public void testGetLocale_With_DefaultLocale_fr_CA() { // Given Mock mock = new Mock(HttpServletRequest.class); MockHttpSession mockHttpSession = new MockHttpSession(); @@ -477,7 +500,7 @@ public class DispatcherTest extends StrutsInternalTestCase { mock.verify(); } - public void testGetLocale_With_BadDefaultLocale_RequestLocale_en_UK() throws Exception { + public void testGetLocale_With_BadDefaultLocale_RequestLocale_en_UK() { // Given Mock mock = new Mock(HttpServletRequest.class); MockHttpSession mockHttpSession = new MockHttpSession(); @@ -505,7 +528,7 @@ public class DispatcherTest extends StrutsInternalTestCase { mock.verify(); } - public void testGetLocale_With_BadDefaultLocale_And_RuntimeException() throws Exception { + public void testGetLocale_With_BadDefaultLocale_And_RuntimeException() { // Given Mock mock = new Mock(HttpServletRequest.class); MockHttpSession mockHttpSession = new MockHttpSession(); @@ -533,7 +556,7 @@ public class DispatcherTest extends StrutsInternalTestCase { mock.verify(); } - public void testGetLocale_With_NullDefaultLocale() throws Exception { + public void testGetLocale_With_NullDefaultLocale() { // Given Mock mock = new Mock(HttpServletRequest.class); MockHttpSession mockHttpSession = new MockHttpSession(); @@ -563,7 +586,7 @@ public class DispatcherTest extends StrutsInternalTestCase { mock.verify(); } - public void testGetLocale_With_NullDefaultLocale_And_RuntimeException() throws Exception { + public void testGetLocale_With_NullDefaultLocale_And_RuntimeException() { // Given Mock mock = new Mock(HttpServletRequest.class); MockHttpSession mockHttpSession = new MockHttpSession(); @@ -595,19 +618,14 @@ public class DispatcherTest extends StrutsInternalTestCase { /** * Create a test context Map from a Dispatcher instance. - * + *

    * The method directly calls getParameterMap() and getSession(true) on the HttpServletRequest. - * + *

    * The method indirectly calls getLocale(request) on the Dispatcher instance, allowing a test of that code path. * The derived Struts Dispatcher Locale can be retrieved from the Map afterwards. - * - * @param dispatcher - * @param request - * @param response - * @return */ protected static Map createTestContextMap(Dispatcher dispatcher, - HttpServletRequest request, HttpServletResponse response) { + HttpServletRequest request, HttpServletResponse response) { if (dispatcher == null) { throw new IllegalArgumentException("Cannot create a test ContextMap from a null Dispatcher"); } @@ -619,31 +637,31 @@ public class DispatcherTest extends StrutsInternalTestCase { } return dispatcher.createContextMap(new RequestMap(request), - HttpParameters.create(request.getParameterMap()).build(), - new SessionMap(request), - new ApplicationMap(request.getSession(true).getServletContext()), - request, - response); + HttpParameters.create(request.getParameterMap()).build(), + new SessionMap(request), + new ApplicationMap(request.getSession(true).getServletContext()), + request, + response); } - class InternalConfigurationManager extends ConfigurationManager { - public boolean destroyConfiguration = false; + static class InternalConfigurationManager extends ConfigurationManager { + public boolean destroyConfiguration = false; public InternalConfigurationManager(String name) { super(name); } @Override - public synchronized void destroyConfiguration() { - super.destroyConfiguration(); - destroyConfiguration = true; - } + public synchronized void destroyConfiguration() { + super.destroyConfiguration(); + destroyConfiguration = true; + } } - class DispatcherListenerState { - public boolean isInitialized = false; - public boolean isDestroyed = false; + static class DispatcherListenerState { + public boolean isInitialized = false; + public boolean isDestroyed = false; } public static class InnerDestroyableObjectFactory extends ObjectFactory implements ObjectFactoryDestroyable {