Bugzilla->instance and friends

[Bradley Baetz]

Currently, to get a template object, you do:

my $template = Bugzilla->instance->template

to get a dbh:

my $dbh = Bugzilla->instance->dbh

And so on. IOW, Bugzilla is a singleton class which has references to
various objects we want to keep arround for some span of time.

There were several anticipated advantage behind all this. One was that
all the global 'stuff' would be in one place, which would make it easier
to handle, both for normal use and mod_perl. There was also this vague
hope that we could do user logins in one step, by passing a separate arg
(or rather, hashkey, in the spirit of other perl-based stuff) to
represent quietly_check vs require vs that-param-joel-added. This would
allow one command to do everything.

I've been playing with this when trying to fix bug 187861, and I'm not
sure that this is the way to go anymore.

My original idea for the fix was to do what I'd been planning for a
while - make the Bugzilla object creation routine take an argument
indicating what type of program it was, and then only initalise whats
required. (With defaults so that the majority of cgi files wouldn't
have to specify anything, and would get Bugzilla::TYPE_CGI
automaticaly)

Then it was mentioned that the ->instance was a bit ugly. Removing that
points to the fact that we don't really need an object, whch leaves us
with a whle set of unrelated subroutines.

So, the alternative is to use Class::Singleton either directly (or just
use the standard ||= construct), possibly with Apache::Singleton too
(despite the name, this sort of works without mod_perl, although the
install tests fail). To get a template, you then:

Bugzilla::Template->instance

and to get a cgi object:

Bugzilla::CGI->instance

DBH's will be Bugzilla::DB->dbh() (rather than instance) because of the
shadowdb stuff and the fact that we don't subclass DBI, nor do we want
to. The shadowdb switch routines can move to Bugzilla::DB, and the
Bugzilla.pm file then becomes the location of generic functions like
get_product_id and friends, as well as $VERSION. Stuff like the
versioncache may go there too.

Code which wants a cgi object then explictly asks for it, and its
created as needed. Code which doesn't never asks, and so never inits
stuff it doesn't want. (This doesn't actually solve the current problem
with processmail, because globals.pl gets the cgi object, so we probably
have to $^0 test that instead. Just forget about that for now, and
imagine a world where globals.pl is gone. :)

mod_perl can explictly call
Bugzilla->load_everything_persistent_right_now_please, or something. (TT
devel versions have a Template->preload to do much the same thing)

Pros of the new way:
- Stuff isn't mangled together
  - No circular links like we will have with Bugzilla->user =>
    Bugzilla::User -> Bugzilla->dbh
- More explicit
- Stuff can be lazily instantiated
  - Not much benefits from this - initing the Template for an
    attachment.cgi?action=view is the only case I can think of where we
    won't ever use a template.
- Simpler/cleaner/etc

Cons vs the current way:
- May require an extra module (Class::Singleton)
  - I don't object, but I know others may
- We can never have two Template/CGI objects
  - Well, we can, but it would be a bit of hackery
  - Do we care?
- Errors may be delayed. For example, if theres an error creating the
  Template object, we won't know until we're about to use the result
  - Again, do we care? I don't think so.
- Things are separated, and so persistency isn't as obvious.
  - We may need separate cleanup routines, although doing something like
    what Apache::Singleton does will make this simple.
- More code in .cgis
  - although not more than the current code.
- More characters to type
  - We get a template/CGI once per script. DBH is the only one we do
    more of
- Need to find somewhere to stick the currently logged in user where it
  is globally accessible
  - This is the main one, but not having modularised the login code yet,
    I don't have details. Bugzilla::Login->current_user perhaps. In any
    event, explicit initliasation will be needed for this one so that
    we can differentiate between the types of logins.

Thoughts? I'm gong to start hacking on this tonight, and will see if its
workable. Yes, the cons list takes up more lines, but thats not the
point :)

Bradley