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.
-- Allen
Anil Gangolli wrote:
----- Original Message Excerpt ---
Dave wrote:
....
The logic is simple, but it is not as you describe. Currently, it is
this:
* If DB connection fails forward to DatabaseError action
* Else if Roller tables not found direct to CreateDatabase action
* Else if Roller tables need upgrade direct to UpgradeDatabase action
I like your idea of having one central install action instead of
routing logic in the filter.
Once all of these have passed at least once in a given instance of the
process, we should be able to retain state to avoid repeating at least
the latter two checks each time.
I don't know whether actually testing the connection each time is
worthwhile.
--a.
