On Fri, 17 Dec 2004 09:09:16 -0600, Joe Germuska <[EMAIL PROTECTED]> wrote: > At 10:29 PM -0800 12/16/04, Don Brown wrote: > >Martin Cooper wrote: > >>On Thu, 16 Dec 2004 21:47:53 -0800, Don Brown <[EMAIL PROTECTED]> wrote: > >> > >>>Who is working on bringing Struts chain into Struts core? If no one, I > >>>wouldn't mind doing the integration. > > I haven't made the time to do anything. I was hoping to have some > this weekend, but if you're ready to plunge ahead, I don't want to > hold things up. If you think there's a good way to split tasks up, > let me know. Below I've interspersed what I was thinking about doing. > > >>>I'm thinking about using the ComposableRequestProcessor to keep as much > >>>backwards compatibility as possible. The changes I'm proposing by layer: > >>> > >>>web.xml > >>>- A "chainConfig" parameter to override default catalog. If none > >>>specified, a default chain config will be loaded from the Struts jar. > >>>- A "chainCommand" parameter to specify the command to execute if none > >>>specified in modules (optional) > >> > >> > >>I'm probably misunderstanding something, but > >>ComposableRequestProcessor uses two context initialisation params for > >>these already. Those names and default values seem fine to me. > > > >The idea of these in web.xml is the ability to specify a custom > >chainConfig that would be used as the default for all modules; same > >with the command name. My goal would be to make it possible to use > >an existing Struts 1.1/1.2 app without any changes. To meet this > >goal, all these configuration options are optional and defaults will > >be used if necessary. > > Earlier, we had discussed using one init parameter in web.xml as a > list of chain-config files, each of which would be sought first in > the servlet context and then on the classpath, each of which would be > processed using the default chain digester that adds catalogs to the > pool.
I'm not keen on adding init params in general, and even less keen on adding such params that take lists of files. I do think we need to have a mechanism for specifying global stuff, but I think we can come up with a better way. For example, I don't like that I have to replicate the <controller> config in each module's struts-config.xml in a multi-module app. That's error prone. We've talked about allowing for <extend> for things like form bean definitions. What about having this concept right at the top of the config scheme? So I could define a global struts-config.xml file (which wouldn't be used directly by any module), and then <extend> that file to provide module-specific config files. If we added an optional section to struts-config, we could then specify a list of Chain config files in there. > I would suggest skipping the chainConfig and chainCommand parameters > in the web.xml in favor of explicitly setting them in each > controller. Too many config options can be more trouble than they > are worth. +1 > The discussion of how to configure the Controller or the > RequestProcessor touched off a lot of talk earlier on struts-dev, but > I'd be happy to settle for two properties on ControllerConfig just to > move forward. > > >>>struts-config.xml > >>>- The default request processor class would now be > >>>ComposableRequestProcessor, but the legacy ones would still be available > >>>(deprecated) > >> > >> > >>+1. One thing that I'm a little confused about, though, is that the > >>new request processor is (currently) in a package that includes > >>'legacy'. Anyone know why that is? > > I don't know, but I think we should move it out to > o.a.s.chain.ComposableRequestProcessor. Fine with me. > >>>- The controller would have additional optional attributes: > > I think the controller should simply have optional command and > catalog attributes which would be used like this: > > CatalogFactory.getCatalog(catalog).getCommand(command) > > if no catalog is specified, the "default" catalog is retrieved with > getCatalog(). If no command is specified, then these default values > would be used: > catalog = struts > command = servlet-complete I believe that's what's in place now. > If catalog were specified without a command, an exception would be thrown. > > I am still inclined to package most of what is now struts-chain > separately from the core. I'm not so sure I agree here. Most, if not all, of the commands you list below are used by the chain that you suggest as a default above. I think if we want the ComposableRequestProcessor to be the default for 1.3, we really need to keep the commands in the core as well. However, I do think they should be pushed into a separate package. Just as we have o.a.s.actions, we could have o.a.s.chain.commands for these. -- Martin Cooper > This may be part of why I haven't moved as > quickly. Here are all the classes in struts-chain: > > org.apache.struts.chain.legacy.CatalogConfiguratorPlugIn > becomes obsolete > > org.apache.struts.chain.legacy.TilesPlugin > can be made obsolete if we remove the check in the base TilesPlugin > which insists on a specific request processor. I don't see any > reason not to do that. > > org.apache.struts.chain.legacy.ComposableRequestProcessor > org.apache.struts.chain.legacy.DispatchAction > org.apache.struts.chain.legacy.ChainAction > Move these three to core and up to o.a.s.chain > > org.apache.struts.chain.AbstractAuthorizeAction > org.apache.struts.chain.AbstractExceptionHandler > org.apache.struts.chain.AbstractExecuteAction > org.apache.struts.chain.AbstractPerformForward > org.apache.struts.chain.AbstractPerformInclude > org.apache.struts.chain.AbstractPopulateActionForm > org.apache.struts.chain.AbstractRequestNoCache > org.apache.struts.chain.AbstractSelectAction > org.apache.struts.chain.AbstractSelectForward > org.apache.struts.chain.AbstractSelectInput > org.apache.struts.chain.AbstractSelectLocale > org.apache.struts.chain.AbstractSelectModule > org.apache.struts.chain.AbstractSetContentType > org.apache.struts.chain.AbstractValidateActionForm > org.apache.struts.chain.Constants > org.apache.struts.chain.CreateAction > org.apache.struts.chain.CreateActionForm > org.apache.struts.chain.ExceptionCatcher > org.apache.struts.chain.SelectInclude > org.apache.struts.chain.UnauthorizedActionException > org.apache.struts.chain.util.ClassUtils > org.apache.struts.chain.servlet.AuthorizeAction > org.apache.struts.chain.servlet.ExceptionHandler > org.apache.struts.chain.servlet.ExecuteAction > org.apache.struts.chain.servlet.PerformForward > org.apache.struts.chain.servlet.PerformInclude > org.apache.struts.chain.servlet.PopulateActionForm > org.apache.struts.chain.servlet.RequestNoCache > org.apache.struts.chain.servlet.SelectAction > org.apache.struts.chain.servlet.SelectForward > org.apache.struts.chain.servlet.SelectInput > org.apache.struts.chain.servlet.SelectLocale > org.apache.struts.chain.servlet.SelectModule > org.apache.struts.chain.servlet.SetContentType > org.apache.struts.chain.servlet.TilesPreProcessor > org.apache.struts.chain.servlet.ValidateActionForm > > Move all of these to a Struts TLP like "legacy-chain" or perhaps just > "struts-chain". I wasn't involved in the initial implementation, but > I don't think these are something that belongs in the core. They > work fine, but I think the Abstract classes are over-engineered. > Also, I think we should move towards actions which operate on a > StrutsContext or ActionContext rather than having such a highly > configurable mechanism for looking up context-relevant objects (like > ActionMapping and ActionForward). I think the config for setting > context keys for each important object is an example of over > abstraction which make these harder to understand. This TLP would > also include a default chain-config which works with this set of > classes in the JAR. > > Is there a compelling reason to put specific command implementations > in the core? I kind of feel like it would be nice to feel less bound > to them. > > If we do this repackaging, perhaps it would make sense to change the > Java package for these classes. I don't have a strong opinion. > > Joe > > -- > Joe Germuska > [EMAIL PROTECTED] > http://blog.germuska.com > "Narrow minds are weapons made for mass destruction" -The Ex > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] > > --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]