Bug 191863 - Clean up Bugzilla.pm

r=gerv, justdave
a=justdave

12 files changed, 67 insertions, 106 deletions

diff --git a/Bugzilla.pm b/Bugzilla.pm
index 66831046d..a45c5ca0f 100644
--- a/Bugzilla.pm
+++ b/Bugzilla.pm
@@ -29,103 +29,71 @@ use Bugzilla::Config;
 use Bugzilla::DB;
 use Bugzilla::Template;
 
-sub create {
+my $_template;
+sub template {
     my $class = shift;
-    my $B = $class->instance;
-
-    # And set up the vars for this request
-    $B->_init_transient;
+    $_template ||= Bugzilla::Template->create();
+    return $_template;
+}
 
-    return $B;
+my $_cgi;
+sub cgi {
+    my $class = shift;
+    $_cgi ||= new Bugzilla::CGI();
+    return $_cgi;
 }
 
-# We don't use Class::Singleton, because theres no need. However, I'm keeping
-# the same interface in case we do change in the future
+my $_dbh;
+my $_dbh_main;
+my $_dbh_shadow;
 
-my $_instance;
-sub instance {
+sub dbh {
     my $class = shift;
 
-    $_instance = $class->_new_instance unless ($_instance);
+    # If we're not connected, then we must want the main db
+    if (!$_dbh) {
+        $_dbh = $_dbh_main = Bugzilla::DB::connect_main();
+    }
 
-    return $_instance;
+    return $_dbh;
 }
 
-sub template { return $_[0]->{_template}; }
-sub cgi { return $_[0]->{_cgi}; }
-sub dbh { return $_[0]->{_dbh}; }
-
 sub switch_to_shadow_db {
-    my $self = shift;
+    my $class = shift;
 
-    if (!$self->{_dbh_shadow}) {
+    if (!$_dbh_shadow) {
         if (Param('shadowdb')) {
-            $self->{_dbh_shadow} = Bugzilla::DB::connect_shadow();
+            $_dbh_shadow = Bugzilla::DB::connect_shadow();
         } else {
-            $self->{_dbh_shadow} = $self->{_dbh_main};
+            $_dbh_shadow = $_dbh_main;
         }
     }
 
-    $self->{_dbh} = $self->{_dbh_shadow};
+    $_dbh = $_dbh_shadow;
 }
 
 sub switch_to_main_db {
-    my $self = shift;
-    $self->{_dbh} = $self->{_dbh_main};
-}
-
-# PRIVATE methods below here
-
-# Called from instance
-sub _new_instance {
     my $class = shift;
 
-    my $self = { };
-    bless($self, $class);
-
-    $self->_init_persistent;
-
-    return $self;
-}
-
-# Initialise persistent items
-sub _init_persistent {
-    my $self = shift;
-
-    # We're always going to use the main db, so connect now
-    $self->{_dbh} = $self->{_dbh_main} = Bugzilla::DB::connect_main();
-
-    # Set up the template
-    $self->{_template} = Bugzilla::Template->create();
+    $_dbh = $_dbh_main;
 }
 
-# Initialise transient (per-request) items
-sub _init_transient {
-    my $self = shift;
+# Private methods
 
-    $self->{_cgi} = new Bugzilla::CGI if exists $::ENV{'GATEWAY_INTERFACE'};
-}
-
-# Clean up transient items such as database handles
+# Per process cleanup
 sub _cleanup {
-    my $self = shift;
-
-    delete $self->{_cgi};
+    undef $_cgi;
+
+    # When we support transactions, need to ->rollback here
+    $_dbh_main->disconnect if $_dbh_main;
+    $_dbh_shadow->disconnect if $_dbh_shadow and Param("shadowdb");
+    undef $_dbh_main;
+    undef $_dbh_shadow;
+    undef $_dbh;
 }
 
-sub DESTROY {
-    my $self = shift;
-
-    # Clean up transient items. We can't just let perl handle removing
-    # stuff from the $self hash because some stuff (eg database handles)
-    # may need special casing
-    # under a persistent environment (ie mod_perl)
-    $self->_cleanup;
-
-    # Now clean up the persistent items
-    $self->{_dbh_main}->disconnect if $self->{_dbh_main};
-    $self->{_dbh_shadow}->disconnect if
-      $self->{_dbh_shadow} and Param("shadowdb")
+sub END {
+    _cleanup();
 }
 
 1;
@@ -141,11 +109,9 @@ and modules
 
   use Bugzilla;
 
-  Bugzilla->create;
-
   sub someModulesSub {
-    my $B = Bugzilla->instance;
-    $B->template->process(...);
+    Bugzilla->dbh->prepare(...);
+    Bugzilla->template->process(...);
   }
 
 =head1 DESCRIPTION
@@ -180,32 +146,18 @@ templates), whilst destroying those which are only valid for a single request
 
 =back
 
-Note that items accessible via this object may be loaded when the Bugzilla
-object is created, or may be demand-loaded when requested.
+Note that items accessible via this object are demand-loaded when requested.
 
 For something to be added to this object, it should either be able to benefit
 from persistence when run under mod_perl (such as the a C<template> object),
 or should be something which is globally required by a large ammount of code
 (such as the current C<user> object).
 
-=head1 CREATION
-
-=over 4
-
-=item C<create>
-
-Creates the C<Bugzilla> object, and initialises any per-request data
-
-=item C<instance>
-
-Returns the current C<Bugzilla> instance. If one doesn't exist, then it will
-be created, but no per-request data will be set. The only use this method has
-for creating the object is from a mod_perl init script. (Its also what
-L<Class::Singleton> does, and I'm trying to keep that interface for this)
-
-=back
+=head1 METHODS
 
-=head1 FUNCTIONS
+Note that all C<Bugzilla> functionailty is method based; use C<Bugzilla->dbh>
+rather than C<Bugzilla::dbh>. Nothing cares about this now, but don't rely on
+that.
 
 =over 4
 
diff --git a/Bugzilla/DB.pm b/Bugzilla/DB.pm
index 681b0597a..23078369a 100644
--- a/Bugzilla/DB.pm
+++ b/Bugzilla/DB.pm
@@ -60,7 +60,7 @@ sub SendSQL {
 
     require Bugzilla;
 
-    $_current_sth = Bugzilla->instance->dbh->prepare($str);
+    $_current_sth = Bugzilla->dbh->prepare($str);
     return $_current_sth->execute;
 }
 
@@ -73,7 +73,7 @@ sub SqlQuote {
 
     require Bugzilla;
 
-    my $res = Bugzilla->instance->dbh->quote($str);
+    my $res = Bugzilla->dbh->quote($str);
 
     trick_taint($res);
 
diff --git a/Bugzilla/Template/Plugin/Bugzilla.pm b/Bugzilla/Template/Plugin/Bugzilla.pm
index 285553b7e..101bd06ee 100644
--- a/Bugzilla/Template/Plugin/Bugzilla.pm
+++ b/Bugzilla/Template/Plugin/Bugzilla.pm
@@ -31,7 +31,18 @@ use Bugzilla;
 sub new {
     my ($class, $context) = @_;
 
-    return Bugzilla->instance;
+    return bless {}, $class;
+}
+
+sub AUTOLOAD {
+    my $class = shift;
+    our $AUTOLOAD;
+
+    $AUTOLOAD =~ s/^.*:://;
+
+    return if $AUTOLOAD eq 'DESTROY';
+
+    return Bugzilla->$AUTOLOAD(@_);
 }
 
 1;
diff --git a/CGI.pl b/CGI.pl
index 6b9751df6..61797ac44 100644
--- a/CGI.pl
+++ b/CGI.pl
@@ -896,7 +896,7 @@ sub GetBugActivity {
 use Bugzilla;
 
 # XXX - mod_perl - reset this between runs
-$::cgi = Bugzilla->instance->cgi;
+$::cgi = Bugzilla->cgi;
 
 # Set up stuff for compatibility with the old CGI.pl code
 # This code will be removed as soon as possible, in favour of
diff --git a/buglist.cgi b/buglist.cgi
index 680b836cf..e8a21d76e 100755
--- a/buglist.cgi
+++ b/buglist.cgi
@@ -629,7 +629,7 @@ if ($serverpush) {
 
 # Connect to the shadow database if this installation is using one to improve
 # query performance.
-Bugzilla->instance->switch_to_shadow_db();
+Bugzilla->switch_to_shadow_db();
 
 # Normally, we ignore SIGTERM and SIGPIPE (see globals.pl) but we need to
 # respond to them here to prevent someone DOSing us by reloading a query
diff --git a/collectstats.pl b/collectstats.pl
index 27a6e1840..bdc027a68 100755
--- a/collectstats.pl
+++ b/collectstats.pl
@@ -43,7 +43,7 @@ if (chdir("graphs")) {
 ConnectToDatabase();
 GetVersionTable();
 
-Bugzilla->instance->switch_to_shadow_db();
+Bugzilla->switch_to_shadow_db();
 
 my @myproducts;
 push( @myproducts, "-All-", @::legal_product );
diff --git a/duplicates.cgi b/duplicates.cgi
index 45eb219ff..175fb649a 100755
--- a/duplicates.cgi
+++ b/duplicates.cgi
@@ -56,7 +56,7 @@ GetVersionTable();
 
 quietly_check_login();
 
-Bugzilla->instance->switch_to_shadow_db();
+Bugzilla->switch_to_shadow_db();
 
 use vars qw (%FORM $userid @legal_product);
 
diff --git a/globals.pl b/globals.pl
index 8162b9d21..1aee0bd02 100644
--- a/globals.pl
+++ b/globals.pl
@@ -1517,9 +1517,7 @@ sub GetFormat {
 
 use Bugzilla;
 
-$::BZ = Bugzilla->create();
-
-$::template = $::BZ->template();
+$::template = Bugzilla->template();
 
 $::vars = {};
 
diff --git a/report.cgi b/report.cgi
index d113e6d89..99566d67c 100755
--- a/report.cgi
+++ b/report.cgi
@@ -46,7 +46,7 @@ GetVersionTable();
 
 confirm_login();
 
-Bugzilla->instance->switch_to_shadow_db();
+Bugzilla->switch_to_shadow_db();
 
 my $action = $cgi->param('action') || 'menu';
 
diff --git a/reports.cgi b/reports.cgi
index 230fe32db..5c802ccfe 100755
--- a/reports.cgi
+++ b/reports.cgi
@@ -60,7 +60,7 @@ quietly_check_login();
 
 GetVersionTable();
 
-Bugzilla->instance->switch_to_shadow_db();
+Bugzilla->switch_to_shadow_db();
 
 # We only want those products that the user has permissions for.
 my @myproducts;
diff --git a/showdependencygraph.cgi b/showdependencygraph.cgi
index 24d0cdfa5..77a1d4dc0 100755
--- a/showdependencygraph.cgi
+++ b/showdependencygraph.cgi
@@ -33,7 +33,7 @@ quietly_check_login();
 
 # Connect to the shadow database if this installation is using one to improve
 # performance.
-Bugzilla->instance->switch_to_shadow_db();
+Bugzilla->switch_to_shadow_db();
 
 use vars qw($template $vars $userid);
 
diff --git a/showdependencytree.cgi b/showdependencytree.cgi
index e246d5c8d..4ee9e4cc8 100755
--- a/showdependencytree.cgi
+++ b/showdependencytree.cgi
@@ -39,7 +39,7 @@ quietly_check_login();
 
 # Connect to the shadow database if this installation is using one to improve
 # performance.
-Bugzilla->instance->switch_to_shadow_db();
+Bugzilla->switch_to_shadow_db();
 
 # More warning suppression silliness.
 $::userid = $::userid;