Author: lukaszlenart Date: Wed Oct 16 11:31:21 2013 New Revision: 1532732 URL: http://svn.apache.org/r1532732 Log: WW-4100 Solves problem with global "error" result when used with Convention plugin
Added: struts/struts2/trunk/plugins/convention/src/test/java/org/apache/struts2/convention/actions/result/GlobalResultAction.java struts/struts2/trunk/plugins/convention/src/test/java/org/apache/struts2/convention/actions/result/GlobalResultOverrideAction.java Modified: struts/struts2/trunk/plugins/convention/src/main/java/org/apache/struts2/convention/DefaultResultMapBuilder.java struts/struts2/trunk/plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java struts/struts2/trunk/plugins/convention/src/test/java/org/apache/struts2/convention/DefaultResultMapBuilderTest.java struts/struts2/trunk/plugins/convention/src/test/java/org/apache/struts2/convention/PackageBasedActionConfigBuilderTest.java Modified: struts/struts2/trunk/plugins/convention/src/main/java/org/apache/struts2/convention/DefaultResultMapBuilder.java URL: http://svn.apache.org/viewvc/struts/struts2/trunk/plugins/convention/src/main/java/org/apache/struts2/convention/DefaultResultMapBuilder.java?rev=1532732&r1=1532731&r2=1532732&view=diff ============================================================================== --- struts/struts2/trunk/plugins/convention/src/main/java/org/apache/struts2/convention/DefaultResultMapBuilder.java (original) +++ struts/struts2/trunk/plugins/convention/src/main/java/org/apache/struts2/convention/DefaultResultMapBuilder.java Wed Oct 16 11:31:21 2013 @@ -20,21 +20,6 @@ */ package org.apache.struts2.convention; -import java.io.IOException; -import java.net.URL; -import java.util.Arrays; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Map; -import java.util.Set; - -import javax.servlet.ServletContext; - -import org.apache.commons.lang3.ObjectUtils; -import org.apache.commons.lang3.StringUtils; -import org.apache.struts2.convention.annotation.Result; -import org.apache.struts2.convention.annotation.Results; - import com.opensymphony.xwork2.Action; import com.opensymphony.xwork2.ActionContext; import com.opensymphony.xwork2.config.ConfigurationException; @@ -49,6 +34,19 @@ import com.opensymphony.xwork2.util.find import com.opensymphony.xwork2.util.finder.Test; import com.opensymphony.xwork2.util.logging.Logger; import com.opensymphony.xwork2.util.logging.LoggerFactory; +import org.apache.commons.lang3.ObjectUtils; +import org.apache.commons.lang3.StringUtils; +import org.apache.struts2.convention.annotation.Result; +import org.apache.struts2.convention.annotation.Results; + +import javax.servlet.ServletContext; +import java.io.IOException; +import java.net.URL; +import java.util.Arrays; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; /** * <p> @@ -103,7 +101,7 @@ import com.opensymphony.xwork2.util.logg * </p> * * <p> - * All results that are conigured from resources are given a type corresponding + * All results that are configured from resources are given a type corresponding * to the resources extension. The extensions and types are given in the * table below: * </p> @@ -178,7 +176,7 @@ public class DefaultResultMapBuilder imp LOG.trace("Using final calculated namespace [#0]", namespace); } - // Add that ending slash for concatentation + // Add that ending slash for concatenation if (!defaultResultPath.endsWith("/")) { defaultResultPath += "/"; } @@ -266,7 +264,7 @@ public class DefaultResultMapBuilder imp } else if(fileName.lastIndexOf(".") > 0){ String suffix = fileName.substring(fileName.lastIndexOf(".")+1); - + if(conventionsService.getResultTypesByExtension(packageConfig).get(suffix) == null) { if (LOG.isDebugEnabled()) LOG.debug("No result type defined for file suffix : [#0]. Ignoring file #1", suffix, fileName); @@ -356,6 +354,7 @@ public class DefaultResultMapBuilder imp protected void makeResults(Class<?> actionClass, String path, String resultPrefix, Map<String, ResultConfig> results, PackageConfig packageConfig, Map<String, ResultTypeConfig> resultsByExtension) { + if (path.startsWith(resultPrefix)) { int indexOfDot = path.indexOf('.', resultPrefix.length()); @@ -367,24 +366,9 @@ public class DefaultResultMapBuilder imp " be overridden by another result file or an annotation.", path); } - if (!results.containsKey(Action.SUCCESS)) { - ResultConfig success = createResultConfig(actionClass, - new ResultInfo(Action.SUCCESS, path, packageConfig, resultsByExtension), - packageConfig, null); - results.put(Action.SUCCESS, success); - } - if (!results.containsKey(Action.INPUT)) { - ResultConfig input = createResultConfig(actionClass, - new ResultInfo(Action.INPUT, path, packageConfig, resultsByExtension), - packageConfig, null); - results.put(Action.INPUT, input); - } - if (!results.containsKey(Action.ERROR)) { - ResultConfig error = createResultConfig(actionClass, - new ResultInfo(Action.ERROR, path, packageConfig, resultsByExtension), - packageConfig, null); - results.put(Action.ERROR, error); - } + addResult(actionClass, path, results, packageConfig, resultsByExtension, Action.SUCCESS); + addResult(actionClass, path, results, packageConfig, resultsByExtension, Action.INPUT); + addResult(actionClass, path, results, packageConfig, resultsByExtension, Action.ERROR); // This case is when the path contains a result code } else if (indexOfDot > resultPrefix.length()) { @@ -402,6 +386,34 @@ public class DefaultResultMapBuilder imp } } + /** + * Checks if result was already assigned, if not checks global results first and if exists, adds reference to it. + * If not, creates package specific result. + * + * @param actionClass The action class the results are being built for. + * @param path The path to build the result for. + * @param results The Map to place the result(s) + * @param packageConfig The package config the results belong to. + * @param resultsByExtension The map of extensions to result type configuration instances. + * @param resultKey The result name to use + */ + protected void addResult(Class<?> actionClass, String path, Map<String, ResultConfig> results, + PackageConfig packageConfig, Map<String, ResultTypeConfig> resultsByExtension, + String resultKey) { + + if (!results.containsKey(resultKey)) { + Map<String, ResultConfig> globalResults = packageConfig.getAllGlobalResults(); + if (globalResults.containsKey(resultKey)) { + results.put(resultKey, globalResults.get(resultKey)); + } else { + ResultConfig resultConfig = createResultConfig(actionClass, + new ResultInfo(resultKey, path, packageConfig, resultsByExtension), + packageConfig, null); + results.put(resultKey, resultConfig); + } + } + } + protected void createFromAnnotations(Map<String, ResultConfig> resultConfigs, String resultPath, PackageConfig packageConfig, Result[] results, Class<?> actionClass, Map<String, ResultTypeConfig> resultsByExtension) { @@ -517,4 +529,4 @@ public class DefaultResultMapBuilder imp } } } -} \ No newline at end of file +} Modified: struts/struts2/trunk/plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java URL: http://svn.apache.org/viewvc/struts/struts2/trunk/plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java?rev=1532732&r1=1532731&r2=1532732&view=diff ============================================================================== --- struts/struts2/trunk/plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java (original) +++ struts/struts2/trunk/plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java Wed Oct 16 11:31:21 2013 @@ -160,8 +160,8 @@ public class PackageBasedActionConfigBui public void setReload(String reload) { this.reload = "true".equals(reload); } - - + + @Inject(StrutsConstants.STRUTS_ENABLE_SLASHES_IN_ACTION_NAMES) public void setSlashesInActionNames(String slashesInActionNames) { this.slashesInActionNames = "true".equals(slashesInActionNames); @@ -185,7 +185,7 @@ public class PackageBasedActionConfigBui } /** - * File URLs whose protocol are in these list will be processed as jars containing classes + * File URLs whose protocol are in these list will be processed as jars containing classes * @param fileProtocols Comma separated list of file protocols that will be considered as jar files and scanned */ @Inject("struts.convention.action.fileProtocols") @@ -204,7 +204,7 @@ public class PackageBasedActionConfigBui } /** - * @param includeJars Comma separated list of regular expressions of jars to be included. + * @param includeJars Comma separated list of regular expressions of jars to be included. */ @Inject(value = "struts.convention.action.includeJars", required = false) public void setIncludeJars(String includeJars) { @@ -326,7 +326,7 @@ public class PackageBasedActionConfigBui public void buildActionConfigs() { //setup reload class loader based on dev settings initReloadClassLoader(); - + if (!disableActionScanning) { if (actionPackages == null && packageLocators == null) { throw new ConfigurationException("At least a list of action packages or action package locators " + @@ -379,7 +379,7 @@ public class PackageBasedActionConfigBui Set<Class> classes = new HashSet<Class>(); try { if (actionPackages != null || (packageLocators != null && !disablePackageLocatorsScanning)) { - + // By default, ClassFinder scans EVERY class in the specified // url set, which can produce spurious warnings for non-action // classes that can't be loaded. We pass a package filter that @@ -504,7 +504,7 @@ public class PackageBasedActionConfigBui * || implements Action) test will have to remain until later. See * {@link #getActionClassTest()} for the test performed on the loaded * {@link ClassInfo} structure. - * + * * @param className the name of the class to test * @return true if the specified class should be included in the * package-based action scan @@ -581,7 +581,7 @@ public class PackageBasedActionConfigBui * Note that the goal is to avoid loading the class, so the test should only * rely on information in the class name itself. The default implementation * is to return the result of {@link #includeClassNameInActionScan(String)}. - * + * * @return a {@link Test} object that returns true if the specified class * name should be included in the package scan */ @@ -599,14 +599,14 @@ public class PackageBasedActionConfigBui * of the class. At this point, the class has been loaded, so it's ok to * perform tests such as checking annotations or looking at interfaces or * super-classes of the specified class. - * + * * @return a {@link Test} object that returns true if the specified class * should be included in the package scan */ protected Test<ClassFinder.ClassInfo> getActionClassTest() { return new Test<ClassFinder.ClassInfo>() { public boolean test(ClassFinder.ClassInfo classInfo) { - + // Why do we call includeClassNameInActionScan here, when it's // already been called to in the initial call to ClassFinder? // When some action class passes our package filter in that step, @@ -916,7 +916,7 @@ public class PackageBasedActionConfigBui className = annotation.className(); } } - + ActionConfig.Builder actionConfig = new ActionConfig.Builder(pkgCfg.getName(), actionName, className); actionConfig.methodName(actionMethod); @@ -991,7 +991,7 @@ public class PackageBasedActionConfigBui LOG.trace("Using non-default action namespace from the Action annotation of [#0]", action.value()); } String actionName = action.value(); - actionNamespace = StringUtils.contains(actionName, "/") ? StringUtils.substringBeforeLast(actionName, "/") : StringUtils.EMPTY; + actionNamespace = StringUtils.contains(actionName, "/") ? StringUtils.substringBeforeLast(actionName, "/") : StringUtils.EMPTY; } // Next grab the parent annotation from the class @@ -1017,7 +1017,7 @@ public class PackageBasedActionConfigBui PackageConfig parentPkg = configuration.getPackageConfig(parentName); if (parentPkg == null) { - throw new ConfigurationException("Unable to locate parent package [" + parentName + "]"); + throw new ConfigurationException("Unable to locate parent package [" + parentName + "] for [" + actionClass + "]"); } // Grab based on package-namespace and if it exists, we need to ensure the existing one has Modified: struts/struts2/trunk/plugins/convention/src/test/java/org/apache/struts2/convention/DefaultResultMapBuilderTest.java URL: http://svn.apache.org/viewvc/struts/struts2/trunk/plugins/convention/src/test/java/org/apache/struts2/convention/DefaultResultMapBuilderTest.java?rev=1532732&r1=1532731&r2=1532732&view=diff ============================================================================== --- struts/struts2/trunk/plugins/convention/src/test/java/org/apache/struts2/convention/DefaultResultMapBuilderTest.java (original) +++ struts/struts2/trunk/plugins/convention/src/test/java/org/apache/struts2/convention/DefaultResultMapBuilderTest.java Wed Oct 16 11:31:21 2013 @@ -20,32 +20,34 @@ */ package org.apache.struts2.convention; -import java.util.HashSet; -import java.util.Map; -import java.util.Set; -import javax.servlet.ServletContext; - +import com.opensymphony.xwork2.config.entities.PackageConfig; +import com.opensymphony.xwork2.config.entities.ResultConfig; +import com.opensymphony.xwork2.config.entities.ResultTypeConfig; +import com.opensymphony.xwork2.inject.Container; import junit.framework.TestCase; - -import static org.apache.struts2.convention.ReflectionTools.*; import org.apache.struts2.convention.actions.NoAnnotationAction; -import org.apache.struts2.convention.actions.result.OverrideResultAction; import org.apache.struts2.convention.actions.result.ActionLevelResultAction; import org.apache.struts2.convention.actions.result.ActionLevelResultsAction; import org.apache.struts2.convention.actions.result.ClassLevelResultAction; import org.apache.struts2.convention.actions.result.ClassLevelResultsAction; +import org.apache.struts2.convention.actions.result.GlobalResultAction; +import org.apache.struts2.convention.actions.result.GlobalResultOverrideAction; import org.apache.struts2.convention.actions.result.InheritedResultExtends; import org.apache.struts2.convention.actions.result.InheritedResultsExtends; import org.apache.struts2.convention.actions.result.OverrideInheritedResultExtends; +import org.apache.struts2.convention.actions.result.OverrideResultAction; import org.apache.struts2.convention.actions.resultpath.ClassLevelResultPathAction; import org.apache.struts2.convention.annotation.Action; +import org.apache.struts2.dispatcher.ServletDispatcherResult; import org.easymock.EasyMock; import org.easymock.IAnswer; -import com.opensymphony.xwork2.config.entities.PackageConfig; -import com.opensymphony.xwork2.config.entities.ResultConfig; -import com.opensymphony.xwork2.config.entities.ResultTypeConfig; -import com.opensymphony.xwork2.inject.Container; +import javax.servlet.ServletContext; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +import static org.apache.struts2.convention.ReflectionTools.getAnnotation; /** * <p> @@ -89,6 +91,65 @@ public class DefaultResultMapBuilderTest assertEquals("/WEB-INF/location/namespace/error-overriden.jsp", result.getParams().get("location")); } + public void testGlobalResult() throws Exception { + + ServletContext context = mockServletContext("/WEB-INF/location"); + this.conventionsService = new ConventionsServiceImpl("/WEB-INF/location"); + DefaultResultMapBuilder builder = new DefaultResultMapBuilder(context, container, "dispatcher,velocity,freemarker"); + + ResultTypeConfig resultType = new ResultTypeConfig.Builder("dispatcher", ServletDispatcherResult.class.getName()). + addParam("key", "value").addParam("key1", "value1").defaultResultParam("location").build(); + ResultConfig globalError = new ResultConfig.Builder("error", ServletDispatcherResult.class.getName()). + addParam("location", "/globalError.jsp"). + build(); + PackageConfig packageConfig = new PackageConfig.Builder("package"). + namespace("/namespace"). + defaultResultType("dispatcher"). + addResultTypeConfig(resultType). + addGlobalResultConfig(globalError). + build(); + + + Map<String, ResultConfig> results = builder.build(GlobalResultAction.class, null, "action", packageConfig); + ResultConfig result = results.get("error"); + assertNotNull(result); + assertEquals("/globalError.jsp", result.getParams().get("location")); + } + + public void testGlobalResultOverride() throws Exception { + + ServletContext context = EasyMock.createStrictMock(ServletContext.class); + String resultPath = "/WEB-INF/location"; + // Setup some mock jsps + Set<String> resources = new HashSet<String>(); + resources.add(resultPath + "/namespace/action.jsp"); + resources.add(resultPath + "/namespace/action-success.jsp"); + resources.add(resultPath + "/namespace/action-error.jsp"); + EasyMock.expect(context.getResourcePaths(resultPath + "/namespace/")).andReturn(resources); + EasyMock.replay(context); + + this.conventionsService = new ConventionsServiceImpl("/WEB-INF/location"); + DefaultResultMapBuilder builder = new DefaultResultMapBuilder(context, container, "dispatcher,velocity,freemarker"); + + ResultTypeConfig resultType = new ResultTypeConfig.Builder("dispatcher", ServletDispatcherResult.class.getName()). + addParam("key", "value").addParam("key1", "value1").defaultResultParam("location").build(); + ResultConfig globalError = new ResultConfig.Builder("error", ServletDispatcherResult.class.getName()). + addParam("location", "/globalError.jsp"). + build(); + PackageConfig packageConfig = new PackageConfig.Builder("package"). + namespace("/namespace"). + defaultResultType("dispatcher"). + addResultTypeConfig(resultType). + addGlobalResultConfig(globalError). + build(); + + + Map<String, ResultConfig> results = builder.build(GlobalResultOverrideAction.class, null, "action", packageConfig); + ResultConfig result = results.get("error"); + assertNotNull(result); + assertEquals(resultPath + "/namespace/action-error.jsp", result.getParams().get("location")); + } + public void testNull() throws Exception { ServletContext context = EasyMock.createStrictMock(ServletContext.class); EasyMock.expect(context.getResourcePaths("/WEB-INF/location/namespace/")).andReturn(null); @@ -501,4 +562,4 @@ public class DefaultResultMapBuilderTest }).anyTimes(); EasyMock.replay(this.container); } -} \ No newline at end of file +} Modified: struts/struts2/trunk/plugins/convention/src/test/java/org/apache/struts2/convention/PackageBasedActionConfigBuilderTest.java URL: http://svn.apache.org/viewvc/struts/struts2/trunk/plugins/convention/src/test/java/org/apache/struts2/convention/PackageBasedActionConfigBuilderTest.java?rev=1532732&r1=1532731&r2=1532732&view=diff ============================================================================== --- struts/struts2/trunk/plugins/convention/src/test/java/org/apache/struts2/convention/PackageBasedActionConfigBuilderTest.java (original) +++ struts/struts2/trunk/plugins/convention/src/test/java/org/apache/struts2/convention/PackageBasedActionConfigBuilderTest.java Wed Oct 16 11:31:21 2013 @@ -82,6 +82,8 @@ import org.apache.struts2.convention.act import org.apache.struts2.convention.actions.result.ActionLevelResultsAction; import org.apache.struts2.convention.actions.result.ClassLevelResultAction; import org.apache.struts2.convention.actions.result.ClassLevelResultsAction; +import org.apache.struts2.convention.actions.result.GlobalResultAction; +import org.apache.struts2.convention.actions.result.GlobalResultOverrideAction; import org.apache.struts2.convention.actions.result.InheritedResultExtends; import org.apache.struts2.convention.actions.result.OverrideResultAction; import org.apache.struts2.convention.actions.resultpath.ClassLevelResultPathAction; @@ -207,6 +209,8 @@ public class PackageBasedActionConfigBui "/namespaces4", strutsDefault, null); PackageConfig resultPkg = makePackageConfig("org.apache.struts2.convention.actions.result#struts-default#/result", "/result", strutsDefault, null); + PackageConfig globalResultPkg = makePackageConfig("org.apache.struts2.convention.actions.result#class-level#/result", + "/result", classLevelParentPkg, null); PackageConfig resultPathPkg = makePackageConfig("org.apache.struts2.convention.actions.resultpath#struts-default#/resultpath", "/resultpath", strutsDefault, null); PackageConfig skipPkg = makePackageConfig("org.apache.struts2.convention.actions.skip#struts-default#/skip", @@ -300,6 +304,8 @@ public class PackageBasedActionConfigBui expect(resultMapBuilder.build(ActionLevelResultsAction.class, getAnnotation(ActionLevelResultsAction.class, "execute", Action.class), "action-level-results", resultPkg)).andReturn(results); expect(resultMapBuilder.build(InheritedResultExtends.class, null, "inherited-result-extends", resultPkg)).andReturn(results); expect(resultMapBuilder.build(OverrideResultAction.class, getAnnotation(OverrideResultAction.class, "execute", Action.class), "override-result", resultPkg)).andReturn(results); + expect(resultMapBuilder.build(GlobalResultAction.class, null, "global-result", globalResultPkg)).andReturn(results); + expect(resultMapBuilder.build(GlobalResultOverrideAction.class, null, "global-result-override", globalResultPkg)).andReturn(results); /* org.apache.struts2.convention.actions.resultpath */ expect(resultMapBuilder.build(ClassLevelResultPathAction.class, null, "class-level-result-path", resultPathPkg)).andReturn(results); @@ -553,7 +559,7 @@ public class PackageBasedActionConfigBui verifyActionConfig(pkgConfig, "action-level-result", ActionLevelResultAction.class, "execute", pkgConfig.getName()); verifyActionConfig(pkgConfig, "action-level-results", ActionLevelResultsAction.class, "execute", pkgConfig.getName()); verifyActionConfig(pkgConfig, "inherited-result-extends", InheritedResultExtends.class, "execute", pkgConfig.getName()); - + /* org.apache.struts2.convention.actions.resultpath */ pkgConfig = configuration.getPackageConfig("org.apache.struts2.convention.actions.resultpath#struts-default#/resultpath"); assertNotNull(pkgConfig); @@ -829,4 +835,4 @@ public class PackageBasedActionConfigBui } } -} \ No newline at end of file +} Added: struts/struts2/trunk/plugins/convention/src/test/java/org/apache/struts2/convention/actions/result/GlobalResultAction.java URL: http://svn.apache.org/viewvc/struts/struts2/trunk/plugins/convention/src/test/java/org/apache/struts2/convention/actions/result/GlobalResultAction.java?rev=1532732&view=auto ============================================================================== --- struts/struts2/trunk/plugins/convention/src/test/java/org/apache/struts2/convention/actions/result/GlobalResultAction.java (added) +++ struts/struts2/trunk/plugins/convention/src/test/java/org/apache/struts2/convention/actions/result/GlobalResultAction.java Wed Oct 16 11:31:21 2013 @@ -0,0 +1,16 @@ +package org.apache.struts2.convention.actions.result; + +import org.apache.struts2.convention.annotation.ParentPackage; + +/** + * Used to test that <global-results> in struts.xml are respected. + * + * @author Mark Woon + */ +@ParentPackage("class-level") +public class GlobalResultAction { + + public String execute() { + return "error"; + } +} Added: struts/struts2/trunk/plugins/convention/src/test/java/org/apache/struts2/convention/actions/result/GlobalResultOverrideAction.java URL: http://svn.apache.org/viewvc/struts/struts2/trunk/plugins/convention/src/test/java/org/apache/struts2/convention/actions/result/GlobalResultOverrideAction.java?rev=1532732&view=auto ============================================================================== --- struts/struts2/trunk/plugins/convention/src/test/java/org/apache/struts2/convention/actions/result/GlobalResultOverrideAction.java (added) +++ struts/struts2/trunk/plugins/convention/src/test/java/org/apache/struts2/convention/actions/result/GlobalResultOverrideAction.java Wed Oct 16 11:31:21 2013 @@ -0,0 +1,17 @@ +package org.apache.struts2.convention.actions.result; + +import org.apache.struts2.convention.annotation.ParentPackage; + +/** + * Used to test that <global-results> in struts.xml are are overridden when a matching result location can be + * found. For example, action-error.jsp overrides a global "error" result. + * + * @author Mark Woon + */ +@ParentPackage("class-level") +public class GlobalResultOverrideAction { + + public String execute() { + return "error"; + } +}