The conversation threads are getting kind of crazy, so I'm going to skip inline quotes for parts of this.
Craig, now I do see your point about using the ConfigRuleSet to digest a portion of an arbitrary XML file. That is very cool, and now I understand why the names of the XML elements are configurable. I always understood the value of letting attributes to the <command> element (and other elements). This is a much slicker approach than creating nested <set-property> elements. Still, I think it could be useful to have a DTD for the *default* behavior of the ConfigRuleSet. I think in general users will start off using the default behavior, and then may at a later date decide to fold their commands into some other file, so a DTD will be nice to get people started with the Chain package. I agree with most your points about the constraints I placed in the DTD being inappropriate, and will explicitly address each if we ever decide to make a DTD. As I've thought more about the package, I don't understand why some design decisions were made. First let me say that I think of Chains as being an OO way to simulate procedural logic. Many of my comments will basically be concerning why some of the standard control flow abilities (if/then statements, loops, exception handling, etc) in Java aren't more easily done/simulated using the Chain package. So, here I go with questions: 1) How come Chains have a static structure? Related to this, how come Command.execute returns boolean instead of returning Command? If it returned Command this would basically eliminate the need for a Chain interface altogether. Chain would become a concrete implementation of Command that repeatedly executed Commands until the last command executed returned null (which would be the new value to indicate the end of a chain). Static chains (such as those configured using an XML file) would easily be supported by another concrete implementation of Command which executed a series of commands in order, completely ignoring their return values. 2) How come Filters have a postprocess method but no catchexception method? The postprocess block can deal with exceptions, but it seems to me like it would be more natural for exceptions to be dealt with in a catchexception block and for postprocess to be strictly for freeing resources that the Command acquired when its execute method was called. Ted Husted wrote: > Of course, another way to go would be to make the Catalog a singleton, > or available through some registry, but I'm thinking that going through > the Context may be the cleanest approach, since the Context is > essentially a Registry too. How about making a Register class which is an implementation of Context which stores only references to Catalogs? We could make the registry itself a singleton, and write in design notes that since the registry is shared between apps, each app should store its Catalog(s) in an application-specific attribute like below: Registry | |---org.apache.struts |---|---actions.RequestProcessor |---|---somethingElse |---com.bah.krm |---org.apache.commons.something That's not explained incredibly well, but if each application component reserves its own spot in the registry, we should be able to make the registry a singleton everyone can share. This keeps us from tying ourselves to the Servlet API, as was mentioned in the ChainServlet discussion thread. Matt ----- Original Message ----- From: "Craig R. McClanahan" <[EMAIL PROTECTED]> To: "Jakarta Commons Developers List" <[EMAIL PROTECTED]> Sent: Monday, September 22, 2003 9:33 PM Subject: Re: [Chain] examples XML file available? > Sgarlata Matt wrote: > > >Comments below... > >----- Original Message ----- > >From: "Craig R. McClanahan" <[EMAIL PROTECTED]> > >To: "Jakarta Commons Developers List" <[EMAIL PROTECTED]> > >Sent: Sunday, September 21, 2003 5:53 PM > >Subject: Re: [Chain] examples XML file available? > > > > > > > > > >>On Sun, 21 Sep 2003, Sgarlata Matt wrote: > >> > >> > >> > >>>Date: Sun, 21 Sep 2003 13:45:37 -0400 > >>>From: Sgarlata Matt <[EMAIL PROTECTED]> > >>>Reply-To: Jakarta Commons Developers List > >>> > >>> > ><[EMAIL PROTECTED]> > > > > > >>>To: [EMAIL PROTECTED] > >>>Subject: [Chain] examples XML file available? > >>> > >>>I'm interested in possibly using the Chain package in some of my > >>> > >>> > >projects > > > > > >>>and I was hoping I could get an example XML file to feed to the > >>> > >>> > >Digester? I > > > > > >>>don't need the Command classes associated with the file or anything like > >>>that, I'd just like to see an example so I don't have to > >>> > >>> > >reverse-engineer > > > > > >>>its structure from the ConfigRuleSet. > >>> > >>> > >>> > >>There's an example in the unit tests of the source module > >> > >> src/test/org/apache/commons/chain/config/test-config.xml > >> > >>although, in truth, this is only a test of the default rules -- you > >>actually get to configure what element and attribute names are recognized > >>by setting properties on your own copy of ConfigRuleSet instead of using > >>the default one (unit tests for this would be definitely appreciated :-). > >> > >> > > > >Wow, that is a very cool how you set that up :) Is there a use-case you had > >in mind where this type of functionality would be useful? To be honest, I > >think that it might be better to make the ConfigRuleSet a little less > >powerful and expect a set DTD. More comments on how this would work > >below... > > > > > The use case goes like this: > > * Each command should be a JavaBean (i.e. has a zero-args constructor and > is configured with property setters). > > * The set of properties each command class is interested in will be unique > to that command class. > > * Users that are utilizing the Digester-based mechanism for configuring > command chains > will want the most concise mechanism for configuring commands as possible. > > Experience with processing both Tomcat and Struts configuration files > (both of which use Digester) make it pretty clear this use case is > common -- and it is exactly the use case that Digester was designed for. > > > > > > >>>If I play with the package some more and like it, I could provide a DTD > >>> > >>> > >for > > > > > >>>the XML file and documentation for it. That should save Craig and Ted > >>> > >>> > >from > > > > > >>>some boring work :) > >>> > >>> > >>While we definitely appreciate the help :-), I don't think a DTD (or a > >>schema) that captures all of the possible functionality of the XML based > >>configuration. Consider one of the examples in the unit test: > >> > >> <!-- Configurable command with settable properties --> > >> <command name="Configurable" > >> className="org.apache.commons.chain.config.TestCommand" > >> foo="Foo Value" > >> bar="Bar Value"/> > >> > >>Besides actually instantiating TestCommand and inserting it into the > >>parent Catalog, this leverages Digester's "Set Properties Rule" ability to > >>match attributes to writeable propertes and configure them. The entry > >>above, then, calls setFoo() and setBar() on the instantiated bean -- but > >>this is the same <command> element that is used to configure a different > >>type of command, with different properties, later on: > >> > >> <command id="1" > >> is2="a" > >> className="org.apache.commons.chain.impl.DelegatingFilter"/> > >> > >>I'm not a total guru on XML, but I don't see how one can define a DTD that > >>covers this case, since the set of possible attributes is not limited. > >> > >> > > > >Yeah, you can't have a DTD verify the attributes for commands since they are > >not set. However, we can specify that the className attribute for <command> > >is required and that the <command> element must be empty. > > > Why should it be empty? You're perfectly free to add your own rules to > the ones returned by the ConfigRuleSet, and process the innards yourself. > > In addition, if you're not going to support configuring JavaBean > properties on the command instances with arbitrary attributes, you will > certainly want something like a <set-property> element inside to > accomplish this goal. So an empty command is probably not the right answer. > > > I think these are > >two very useful things to have the digester automatically check. It would > >certainly beat a random NullPointerException :) In fact, the attached DTD > >can take care of the items listed below. This would certainly avoid lots of > >painful debugging sessions when <command> was actually typed <Command> and > >other stuff like that. > > > >- Ensure there are 1 or more <chain> elements inside <chains> > > > If you want to register single-command "chains" in the catalog, you > probably won't want this. > > >- Ensure there are 1 or more <command> elements inside <chain> > > > No "placeholder" chains for future fleshing out? > > >- Ensure each <chain> has a name > >- Ensure each <command> has a className specified > > > >- Ensure each <command> element is empty > > > As above, I don't buy into this one. > > >- Limit the XML file to only <chains>, <chain>, and <command> elements with > >the proper nesting. > > > If a person wants to define their command chains in the same document > that they are using for other aspects of the operation (for example, if > Struts wanted to let you put this stuff into a struts-config.xml file), > you'd want the ability to just pull out the elements you care about and > ignore the rest. That's the beauty of the rules-based matching patterns > that Digester supports -- you specify the patterns you are interested > in, and that's all your rules have to deal with. > > > > >The attached DTD is just a first pass at a DTD. I know it isn't pretty, and > >would follow the format used in struts-config_1_0.dtd before recommending it > >for checkin. Before I go any further though, I would actually like to see > >the XML file get a little more sophisticated. Once we settle on the format > >of the XML file I can update ConfigRuleSet and finish up the DTD (and > >provide test cases for ConfigRuleSet... ugh I hate writing test cases!) > >Here's what I was thinking: > > > ><chains> > > <global-commands> > > <global-command > > name="globalCommand1" > > className="something" > > otherAttribute1="hi"/> > > </global-commands> > > <chain name="myChain"> > > <!-- here otherAttribute1="hi" and className="something" are > > implied since the command was defined earlier --> > > <command name="globalCommand1"/> > > <!-- this is a local command --> > > <command className="something"/> > > </chain> > ></chains> > > > >One cool thing we can do is ensure that each <command> that is actually a > >reference to a <global-command> has a name attribute which corresponds to > >one of the names of a global command. This is actually something supported > >directly by DTDs, so we don't have to do anything fancy to get this > >automatic validation :) > > > > > How do you propose to set individual configurable properties on the > command classes? My experience so far on real ones (as opposed to toy > ones) is that this is a mission critical requirement. > > >Matt > > > > > Craig > > >------------------------------------------------------------------------ > > > ><!-- > > DTD for the optional Chain of Responsibility Configuration File, > > Version 1.0 > > > > To support validation of your configuration file, include the following > > DOCTYPE element at the beginning (after the "xml" declaration): > > > > <!DOCTYPE chains PUBLIC > > "-//Apache Software Foundation//DTD Chain Configuration 1.0//EN" > > "http://jakarta.apache.org/commons/chain/dtds/chains_1_0.dtd";> > >--> > > > ><!-- The "chains" element is the root of the configuration file hierarchy, and > > contains one or more nested chain elements. > >--> > ><!ELEMENT chains (chain+)> > > > ><!-- The "chain" element represents a class which implements the > > org.apache.commons.chain.Chain interface. Thus it represents a configured > > list of Commands that will be executed in order to perform processing on a > > specified Context. If no className is specified for the chain, the chain > > will be an instance of the class org.apache.commons.chain.impl.ChainBase. > >--> > ><!ELEMENT chain (command+)> > ><!ATTLIST chain > > name CDATA #REQUIRED > > > > > ><!ATTLIST chain > > className CDATA #IMPLIED > > > > > ><!ELEMENT command EMPTY> > ><!ATTLIST command > > className CDATA #REQUIRED > >> > > > > > > > >------------------------------------------------------------------------ > > > >--------------------------------------------------------------------- > >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] > --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
