Dave,
One thing I am a little unsure of from looking at the code is what the
expected behavior is meant to be for the 'manual' vs. 'auto' install
types. This is what I would expect, let me know if this matches what
you are thinking ...
if( install == 'auto' ) {
if( install/upgrade required ) {
// skip bootstrapping/initialization and wait for user involvement
} else {
// app is up to date, so startup as usual
}
} else {
// startup app as usual
}
So a couple things that I am expecting in particular are that if the
install type is set to 'manual' then we don't do any special
install/upgrade logic. Also, if the install type is 'auto' and we
recognize that there is work still needed, then we are requiring user
involvement, namely for the user to actually click a button indicating
that they want to do the install/upgrade.
Assuming that's correct then I have a few things to commit around this.
-- Allen
Dave wrote:
On 6/18/07, Allen Gilliland <[EMAIL PROTECTED]> wrote:
Some comments after looking at the latest code on this ...
1. I'm not really liking that the RollerContext object has become a
DatabaseScriptProvider. I think that functionality should be separated
out into its own class or something. I think we were guilty of putting
way too much logic in the RollerContext before and we should try and
avoid that.
2. I still think the BootstrapFilter should be simplified even further.
We want to do as little work there as possible.
3. Your struts2 actions are ApplicationAware and are pulling things out
of the servlet context, to me that's a very confusing way of managing
access to that database script provider object. Why does that class
need to be a singleton anyways? Why wouldn't the struts2 action just
instantiate that class when it's actually going to use it?
4. Unless you have a good reason, I'd really like to keep all the
struts2 actions extending the UIAction class rather than directly using
ActionSupport.
Overall I'm still feeling like this code is lacking an overall
cohesiveness and the code doesn't really match the workflow of doing an
installation and bootstrapping. So I think this still needs some work.
Thanks for the comments and I've got no objection to any of your
suggestions. I'm working removing pretty much all logic from the
BootstrapFilter and moving it to a controlling Struts action so we can
have some sort of cohesive page flow instead of the dead-enders we
have now.
- Dave
