On Mon, 14 Jan 2002, Donnie Hale wrote:
> Date: Mon, 14 Jan 2002 23:54:33 -0500
> From: Donnie Hale <[EMAIL PROTECTED]>
> Reply-To: Struts Developers List <[EMAIL PROTECTED]>
> To: Struts Developers List <[EMAIL PROTECTED]>
> Subject: RE: The BIG Check-In for Multi-App Support
>
> I've finally had a chance to download this check-in. Please note that while
> I've built it, I haven't yet had a chance to test it with anything. Are
> there any examples yet which include all these new features (exceptions,
> roles, multi-apps, etc.)? There's a lot get one's arms around.
>
> Most importantly, this is great stuff and really appreciated. If one of the
> committers would propose giving Craig a raise, I'd +1 it. ;)
>
> So... Some general comments and comments from looking at pieces of the code:
>
> 1. I don't know if this is carved in stone yet, but IMHO this is much more
> than a 1.1 release. Perhaps not a 2.0 release, but at least a 1.5 release.
> The implications of a single point release to me are minor functional
> enhancements and bug fixes. These are substantive, major functional
> enhancements which still provide backward compatibility. Again, just my
> opinion...
>
I don't consider any of it cast in stone. There are way to many smart
people contributing to the future of Struts for me to have the only good
ideas.
The reason I still want to call the next release 1.1 is this: despite the
huge number of internal changes, the fundamental APIs for applications
*are* pretty much backwards compatible. That's a promise that was made
early on in Struts development, and I want to honor that. Calling it 1.5
or 2.0 would raise concerns in people's minds about compatibility -- even
if those fears turned out to be unfounded.
> 2. I'm thrilled to see all the config-related stuff broken out from the
> servlet. I did this myself in some recent attempts at refactoring
> ActionServlet, and the effort was desperately needed to make ActionServlet
> more approachable. Naturally, Craig did a better job than I, as he gave each
> piece of config info its own class. Much more manageable and flexible that
> way.
>
Besides making things easier to understand, it's already had an additional
benefit -- new configuration parameters can be added with no code changes
to the controller servlet. In fact, if you're adding a property to an
existing config object, it is just a matter of a getter and setter, and
the automatic introspection logic in Digester takes care of things.
> 3. In the DTD, the BeanName entity is being overloaded somewhat in that it's
> used for naming keys to various context attributes as well as for the form
> beans. BeanName is documented as having to be a legal Java identifier since
> it's used in JSPs; not sure off the top of my head if that is a constraint
> on attribute names in the various servlet contexts or not. In any case,
> might I suggest a "ContextKey" entity be added to the DTD and that used
> where appropriate (data-source.key, action.attribute, etc.).
>
That definitely sounds like a good idea. At some point we might add an
XML Schema (or other) validation of the struts-config.xml file that adds
more semantic checks, so it would be good to get the names right now.
> 4. Breaking out the request processing into RequestProcessor is also huge.
> ActionServlet plus the config stuff seem like they can now stand alone with
> pluggable request processing easily able to reuse those elements.
Yep. A subsequent check-in enabled having different RequestProcessor
instances (and even implementation classes) per sub-application.
>
> 5. In the new ActionServlet, doGet and doPost are identical. Would it be
> better to delegate their implementations, simple though they are, to a
> protected overridable method?
>
When it was a single hard-coded request processor instance, the
implementation was one line each - I guess three lines is enough to make a
subordinate method again.
> 6. It's a little confusing seeing member variables of type and name
> "FastHashmap actions" in both ApplicationConfig and RequestProcessor.
> Perhaps the variable names could be clarified.
>
In ApplicationConfig, maybe the variable name should be actionConfigs?
That corresponds to what the Map actually contains.
> 7. Is the special handling for the default app's ControllerConfig and
> MessageResourcesConfig so that existing web.xml files will still work for
> setting those parameters?
>
Yes. It actually took just as long to figure out how to deal with this as
the rest of the design.
> 8. ActionForward and ActionMappings both still retain references to
> ActionServlet, and in both cases it seems primarily to allow the user of an
> ActionForward or ActionMappings to get a reference to the servlet. With the
> drastic changes to ActionServlet, what method on ActionServlet would the
> user of an ActionForward or ActionMappings need to call? I guess what I'm
> saying is that it would be nice to deprecate and/or remove those
> getServlet/setServlet methods from those classes if they're not really
> required, esp. if the "best practice" way to get to a resource is through
> one of the contexts. FWIW...
>
Deprecation is probably a reasonable idea, but I want to walk through the
code paths to see if the servlet references are used internally anywhere
first.
> Thanks for bearing with me. Please note that I'm not trying to avoid
> contributing by bringing up these issues. I want to make sure I don't jump
> to any false conclusions before I try to help out.
>
Not a problem -- thanks for the good ideas.
> Thanks again,
>
> Donnie
>
Craig
--
To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]>
For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
