Author: hrabago Date: Tue Jul 25 17:41:36 2006 New Revision: 425573 URL: http://svn.apache.org/viewvc?rev=425573&view=rev Log: STR-2917 Fix bug that prevents action-mapping-level forwards and exception handlers from inheriting onfig data from global forwards and exception handlers.
Modified: struts/struts1/trunk/core/src/main/java/org/apache/struts/action/ActionServlet.java struts/struts1/trunk/core/src/test/java/org/apache/struts/action/TestActionServlet.java Modified: struts/struts1/trunk/core/src/main/java/org/apache/struts/action/ActionServlet.java URL: http://svn.apache.org/viewvc/struts/struts1/trunk/core/src/main/java/org/apache/struts/action/ActionServlet.java?rev=425573&r1=425572&r2=425573&view=diff ============================================================================== --- struts/struts1/trunk/core/src/main/java/org/apache/struts/action/ActionServlet.java (original) +++ struts/struts1/trunk/core/src/main/java/org/apache/struts/action/ActionServlet.java Tue Jul 25 17:41:36 2006 @@ -1061,7 +1061,7 @@ for (int i = 0; i < forwards.length; i++) { ForwardConfig forward = forwards[i]; - processForwardExtension(forward, config); + processForwardExtension(forward, config, null); } for (int i = 0; i < forwards.length; i++) { @@ -1076,14 +1076,18 @@ } /** - * <p>Extend the forward's configuration as necessary.</p> + * <p>Extend the forward's configuration as necessary. If actionConfig is + * provided, then this method will process the forwardConfig as part + * of that actionConfig. If actionConfig is null, the forwardConfig + * will be processed as a global forward.</p> * * @param forwardConfig the configuration to process. * @param moduleConfig the module configuration for this module. + * @param actionConfig If applicable, the config for the current action. * @throws ServletException if initialization cannot be performed. */ protected void processForwardExtension(ForwardConfig forwardConfig, - ModuleConfig moduleConfig) + ModuleConfig moduleConfig, ActionConfig actionConfig) throws ServletException { try { if (!forwardConfig.isExtensionProcessed()) { @@ -1093,9 +1097,10 @@ } forwardConfig = - processForwardConfigClass(forwardConfig, moduleConfig); + processForwardConfigClass(forwardConfig, moduleConfig, + actionConfig); - forwardConfig.processExtends(moduleConfig, null); + forwardConfig.processExtends(moduleConfig, actionConfig); } } catch (ServletException e) { throw e; @@ -1107,10 +1112,14 @@ /** * <p>Checks if the current forwardConfig is using the correct class based - * on the class of its configuration ancestor.</p> + * on the class of its configuration ancestor. If actionConfig is + * provided, then this method will process the forwardConfig as part + * of that actionConfig. If actionConfig is null, the forwardConfig + * will be processed as a global forward.</p> * * @param forwardConfig The forward to check. * @param moduleConfig The config for the current module. + * @param actionConfig If applicable, the config for the current action. * @return The forward config using the correct class as determined by the * config's ancestor and its own overridden value. * @throws UnavailableException if an instance of the forward config class @@ -1118,7 +1127,8 @@ * @throws ServletException on class creation error */ protected ForwardConfig processForwardConfigClass( - ForwardConfig forwardConfig, ModuleConfig moduleConfig) + ForwardConfig forwardConfig, ModuleConfig moduleConfig, + ActionConfig actionConfig) throws ServletException { String ancestor = forwardConfig.getExtends(); @@ -1128,7 +1138,17 @@ } // Make sure that this config is of the right class - ForwardConfig baseConfig = moduleConfig.findForwardConfig(ancestor); + ForwardConfig baseConfig = null; + if (actionConfig != null) { + // Look for this in the actionConfig + baseConfig = actionConfig.findForwardConfig(ancestor); + } + + if (baseConfig == null) { + // Either this is a forwardConfig that inherits a global config, + // or actionConfig is null + baseConfig = moduleConfig.findForwardConfig(ancestor); + } if (baseConfig == null) { throw new UnavailableException("Unable to find " + "forward '" @@ -1145,7 +1165,8 @@ try { newForwardConfig = - (ForwardConfig) RequestUtils.applicationInstance(baseConfigClassName); + (ForwardConfig) RequestUtils.applicationInstance( + baseConfigClassName); // copy the values BeanUtils.copyProperties(newForwardConfig, forwardConfig); @@ -1154,8 +1175,14 @@ } // replace forwardConfig with newForwardConfig - moduleConfig.removeForwardConfig(forwardConfig); - moduleConfig.addForwardConfig(newForwardConfig); + if (actionConfig != null) { + actionConfig.removeForwardConfig(forwardConfig); + actionConfig.addForwardConfig(newForwardConfig); + } else { + // this is a global forward + moduleConfig.removeForwardConfig(forwardConfig); + moduleConfig.addForwardConfig(newForwardConfig); + } forwardConfig = newForwardConfig; } } @@ -1183,7 +1210,7 @@ for (int i = 0; i < exceptions.length; i++) { ExceptionConfig exception = exceptions[i]; - processExceptionExtension(exception, config); + processExceptionExtension(exception, config, null); } for (int i = 0; i < exceptions.length; i++) { @@ -1198,14 +1225,18 @@ } /** - * <p>Extend the exception's configuration as necessary.</p> + * <p>Extend the exception's configuration as necessary. If actionConfig is + * provided, then this method will process the exceptionConfig as part + * of that actionConfig. If actionConfig is null, the exceptionConfig + * will be processed as a global forward.</p> * * @param exceptionConfig the configuration to process. * @param moduleConfig the module configuration for this module. + * @param actionConfig If applicable, the config for the current action. * @throws ServletException if initialization cannot be performed. */ protected void processExceptionExtension(ExceptionConfig exceptionConfig, - ModuleConfig moduleConfig) + ModuleConfig moduleConfig, ActionConfig actionConfig) throws ServletException { try { if (!exceptionConfig.isExtensionProcessed()) { @@ -1215,9 +1246,10 @@ } exceptionConfig = - processExceptionConfigClass(exceptionConfig, moduleConfig); + processExceptionConfigClass(exceptionConfig, moduleConfig, + actionConfig); - exceptionConfig.processExtends(moduleConfig, null); + exceptionConfig.processExtends(moduleConfig, actionConfig); } } catch (ServletException e) { throw e; @@ -1229,17 +1261,22 @@ /** * <p>Checks if the current exceptionConfig is using the correct class - * based on the class of its configuration ancestor.</p> + * based on the class of its configuration ancestor. If actionConfig is + * provided, then this method will process the exceptionConfig as part + * of that actionConfig. If actionConfig is null, the exceptionConfig + * will be processed as a global forward.</p> * * @param exceptionConfig The config to check. * @param moduleConfig The config for the current module. + * @param actionConfig If applicable, the config for the current action. * @return The exception config using the correct class as determined by * the config's ancestor and its own overridden value. * @throws ServletException if an instance of the exception config class * cannot be created. */ protected ExceptionConfig processExceptionConfigClass( - ExceptionConfig exceptionConfig, ModuleConfig moduleConfig) + ExceptionConfig exceptionConfig, ModuleConfig moduleConfig, + ActionConfig actionConfig) throws ServletException { String ancestor = exceptionConfig.getExtends(); @@ -1249,7 +1286,16 @@ } // Make sure that this config is of the right class - ExceptionConfig baseConfig = moduleConfig.findExceptionConfig(ancestor); + ExceptionConfig baseConfig = null; + if (actionConfig != null) { + baseConfig = actionConfig.findExceptionConfig(ancestor); + } + + if (baseConfig == null) { + // This means either there's no actionConfig anyway, or the + // ancestor is not defined within the action. + baseConfig = moduleConfig.findExceptionConfig(ancestor); + } if (baseConfig == null) { throw new UnavailableException("Unable to find " @@ -1266,17 +1312,24 @@ try { newExceptionConfig = - (ExceptionConfig) RequestUtils.applicationInstance(baseConfigClassName); + (ExceptionConfig) RequestUtils.applicationInstance( + baseConfigClassName); // copy the values - BeanUtils.copyProperties(newExceptionConfig, exceptionConfig); + BeanUtils.copyProperties(newExceptionConfig, + exceptionConfig); } catch (Exception e) { handleCreationException(baseConfigClassName, e); } // replace exceptionConfig with newExceptionConfig - moduleConfig.removeExceptionConfig(exceptionConfig); - moduleConfig.addExceptionConfig(newExceptionConfig); + if (actionConfig != null) { + actionConfig.removeExceptionConfig(exceptionConfig); + actionConfig.addExceptionConfig(newExceptionConfig); + } else { + moduleConfig.removeExceptionConfig(exceptionConfig); + moduleConfig.addExceptionConfig(newExceptionConfig); + } exceptionConfig = newExceptionConfig; } } @@ -1358,6 +1411,21 @@ processActionConfigClass(actionConfig, moduleConfig); actionConfig.processExtends(moduleConfig); + } + + // Process forwards extensions. + ForwardConfig[] forwards = actionConfig.findForwardConfigs(); + for (int i = 0; i < forwards.length; i++) { + ForwardConfig forward = forwards[i]; + processForwardExtension(forward, moduleConfig, actionConfig); + } + + // Process exception extensions. + ExceptionConfig[] exceptions = actionConfig.findExceptionConfigs(); + for (int i = 0; i < exceptions.length; i++) { + ExceptionConfig exception = exceptions[i]; + processExceptionExtension(exception, moduleConfig, + actionConfig); } } catch (ServletException e) { throw e; Modified: struts/struts1/trunk/core/src/test/java/org/apache/struts/action/TestActionServlet.java URL: http://svn.apache.org/viewvc/struts/struts1/trunk/core/src/test/java/org/apache/struts/action/TestActionServlet.java?rev=425573&r1=425572&r2=425573&view=diff ============================================================================== --- struts/struts1/trunk/core/src/test/java/org/apache/struts/action/TestActionServlet.java (original) +++ struts/struts1/trunk/core/src/test/java/org/apache/struts/action/TestActionServlet.java Tue Jul 25 17:41:36 2006 @@ -519,7 +519,7 @@ handler.setType("java.lang.NullPointerException"); moduleConfig.addExceptionConfig(handler); - actionServlet.processExceptionExtension(handler, moduleConfig); + actionServlet.processExceptionExtension(handler, moduleConfig, null); assertTrue("processExtends() was not called", handler.processExtendsCalled); @@ -544,7 +544,8 @@ moduleConfig.addExceptionConfig(customSub); ExceptionConfig result = - actionServlet.processExceptionConfigClass(customSub, moduleConfig); + actionServlet.processExceptionConfigClass(customSub, moduleConfig, + null); assertTrue("Incorrect class of exception config", result instanceof CustomExceptionConfig); @@ -570,7 +571,7 @@ try { result = actionServlet.processExceptionConfigClass(baseException, - moduleConfig); + moduleConfig, null); } catch (UnavailableException e) { fail("An exception should not be thrown when there's nothing to do"); } @@ -594,7 +595,8 @@ moduleConfig.addExceptionConfig(customSub); ExceptionConfig result = - actionServlet.processExceptionConfigClass(customSub, moduleConfig); + actionServlet.processExceptionConfigClass(customSub, moduleConfig, + null); assertSame("The instance returned should be the param given it.", customSub, result); @@ -618,7 +620,8 @@ moduleConfig.addExceptionConfig(customSub); try { - actionServlet.processExceptionConfigClass(customSub, moduleConfig); + actionServlet.processExceptionConfigClass(customSub, moduleConfig, + null); fail("Exception should be thrown"); } catch (UnavailableException e) { // success @@ -643,7 +646,8 @@ moduleConfig.addExceptionConfig(customSub); try { - actionServlet.processExceptionConfigClass(customSub, moduleConfig); + actionServlet.processExceptionConfigClass(customSub, moduleConfig, + null); } catch (Exception e) { fail("Exception should not be thrown"); } @@ -694,7 +698,7 @@ new CustomForwardConfig("forward", "/forward.jsp"); moduleConfig.addForwardConfig(forward); - actionServlet.processForwardExtension(forward, moduleConfig); + actionServlet.processForwardExtension(forward, moduleConfig, null); assertTrue("processExtends() was not called", forward.processExtendsCalled); @@ -718,7 +722,8 @@ moduleConfig.addForwardConfig(customSub); ForwardConfig result = - actionServlet.processForwardConfigClass(customSub, moduleConfig); + actionServlet.processForwardConfigClass(customSub, moduleConfig, + null); assertTrue("Incorrect class of forward config", result instanceof CustomForwardConfig); @@ -744,7 +749,7 @@ try { result = actionServlet.processForwardConfigClass(baseForward, - moduleConfig); + moduleConfig, null); } catch (UnavailableException e) { fail("An exception should not be thrown when there's nothing to do"); } @@ -768,7 +773,8 @@ moduleConfig.addForwardConfig(customSub); ForwardConfig result = - actionServlet.processForwardConfigClass(customSub, moduleConfig); + actionServlet.processForwardConfigClass(customSub, moduleConfig, + null); assertSame("The instance returned should be the param given it.", customSub, result); @@ -792,7 +798,8 @@ moduleConfig.addForwardConfig(customSub); try { - actionServlet.processForwardConfigClass(customSub, moduleConfig); + actionServlet.processForwardConfigClass(customSub, moduleConfig, + null); fail("Exception should be thrown"); } catch (UnavailableException e) { // success @@ -817,7 +824,8 @@ moduleConfig.addForwardConfig(customSub); try { - actionServlet.processForwardConfigClass(customSub, moduleConfig); + actionServlet.processForwardConfigClass(customSub, moduleConfig, + null); } catch (Exception e) { fail("Exception should not be thrown"); }