Bug 1461379 - refactor Bugzilla::DB to not subclass DBI

6 files changed, 75 insertions, 95 deletions

diff --git a/Bugzilla/DB.pm b/Bugzilla/DB.pm
index 0dfa47c23..893c9235e 100644
--- a/Bugzilla/DB.pm
+++ b/Bugzilla/DB.pm
@@ -8,13 +8,20 @@
 package Bugzilla::DB;
 
 use 5.10.1;
-use strict;
-use warnings;
+use Moo;
 
 use DBI;
 
-# Inherit the DB class from DBI::db.
-use base qw(DBI::db);
+has 'dbh' => (
+    is      => 'lazy',
+    handles => [
+        qw[
+            begin_work column_info commit disconnect do errstr get_info last_insert_id ping prepare
+            primary_key quote_identifier rollback selectall_arrayref selectall_hashref
+            selectcol_arrayref selectrow_array selectrow_arrayref selectrow_hashref table_info
+        ]
+    ],
+);
 
 use Bugzilla::Constants;
 use Bugzilla::Install::Requirements;
@@ -27,11 +34,14 @@ use Bugzilla::Error;
 use Bugzilla::DB::Schema;
 use Bugzilla::Version;
 
-use Bugzilla::Metrics::Mysql;
-
 use List::Util qw(max);
 use Storable qw(dclone);
 
+has [qw(dsn user pass attrs)] => (
+    is       => 'ro',
+    required => 1,
+);
+
 #####################################################################
 # Constants
 #####################################################################
@@ -91,7 +101,7 @@ use constant INDEX_DROPS_REQUIRE_FK_DROPS => 1;
 
 sub quote {
     my $self = shift;
-    my $retval = $self->SUPER::quote(@_);
+    my $retval = $self->dbh->quote(@_);
     trick_taint($retval) if defined $retval;
     return $retval;
 }
@@ -140,11 +150,6 @@ sub _connect {
 
     # instantiate the correct DB specific module
 
-    # BMO - enable instrumentation of db calls
-    if (Bugzilla->metrics_enabled) {
-        $pkg_module = 'Bugzilla::Metrics::Mysql';
-    }
-
     my $dbh = $pkg_module->new($params);
 
     return $dbh;
@@ -1249,10 +1254,10 @@ sub bz_rollback_transaction {
 # Subclass Helpers
 #####################################################################
 
-sub db_new {
-    my ($class, $params) = @_;
+sub _build_dbh {
+    my ($self) = @_;
     my ($dsn, $user, $pass, $override_attrs) =
-        @$params{qw(dsn user pass attrs)};
+        map { $self->$_ } qw(dsn user pass attrs);
 
     # set up default attributes used to connect to the database
     # (may be overridden by DB driver implementations)
@@ -1274,20 +1279,24 @@ sub db_new {
             $attributes->{$key} = $override_attrs->{$key};
         }
     }
+    my $class = ref $self;
+    if ($class->can('on_dbi_connected')) {
+        $attributes->{Callbacks} = {
+            connected => sub { $class->on_dbi_connected(@_); return },
+        }
+    }
 
     # connect using our known info to the specified db
-    my $self = DBI->connect($dsn, $user, $pass, $attributes)
+    my $dbh = DBI->connect($dsn, $user, $pass, $attributes)
         or die "\nCan't connect to the database.\nError: $DBI::errstr\n"
         . "  Is your database installed and up and running?\n  Do you have"
         . " the correct username and password selected in localconfig?\n\n";
 
     # RaiseError was only set to 0 so that we could catch the
     # above "die" condition.
-    $self->{RaiseError} = 1;
+    $dbh->{RaiseError} = 1;
 
-    bless ($self, $class);
-
-    return $self;
+    return $dbh;
 }
 
 #####################################################################
diff --git a/Bugzilla/DB/Mysql.pm b/Bugzilla/DB/Mysql.pm
index 727fe2316..d0b9724eb 100644
--- a/Bugzilla/DB/Mysql.pm
+++ b/Bugzilla/DB/Mysql.pm
@@ -22,10 +22,9 @@ For interface details see L<Bugzilla::DB> and L<DBI>.
 package Bugzilla::DB::Mysql;
 
 use 5.10.1;
-use strict;
-use warnings;
+use Moo;
 
-use base qw(Bugzilla::DB);
+extends qw(Bugzilla::DB);
 
 use Bugzilla::Constants;
 use Bugzilla::Install::Util qw(install_string);
@@ -43,7 +42,7 @@ use constant MAX_COMMENTS => 50;
 
 use constant FULLTEXT_OR => '|';
 
-sub new {
+sub BUILDARGS {
     my ($class, $params) = @_;
     my ($user, $pass, $host, $dbname, $port, $sock) =
         @$params{qw(db_user db_pass db_host db_name db_port db_sock)};
@@ -55,28 +54,20 @@ sub new {
 
     my %attrs = (
         mysql_enable_utf8 => Bugzilla->params->{'utf8'},
-        # Needs to be explicitly specified for command-line processes.
-        mysql_auto_reconnect => 1,
     );
 
-    my $self = $class->db_new({ dsn => $dsn, user => $user,
-                                pass => $pass, attrs => \%attrs });
+    return { dsn => $dsn, user => $user, pass => $pass, attrs => \%attrs };
+}
+
+sub on_dbi_connected {
+    my ($class, $dbh) = @_;
 
     # This makes sure that if the tables are encoded as UTF-8, we
     # return their data correctly.
-    $self->do("SET NAMES utf8") if Bugzilla->params->{'utf8'};
-
-    # all class local variables stored in DBI derived class needs to have
-    # a prefix 'private_'. See DBI documentation.
-    $self->{private_bz_tables_locked} = "";
-
-    # Needed by TheSchwartz
-    $self->{private_bz_dsn} = $dsn;
-
-    bless ($self, $class);
+    $dbh->do("SET NAMES utf8") if Bugzilla->params->{'utf8'};
 
     # Bug 321645 - disable MySQL strict mode, if set
-    my ($var, $sql_mode) = $self->selectrow_array(
+    my ($var, $sql_mode) = $dbh->selectrow_array(
         "SHOW VARIABLES LIKE 'sql\\_mode'");
 
     if ($sql_mode) {
@@ -88,15 +79,13 @@ sub new {
                             split(/,/, $sql_mode));
 
         if ($sql_mode ne $new_sql_mode) {
-            $self->do("SET SESSION sql_mode = ?", undef, $new_sql_mode);
+            $dbh->do("SET SESSION sql_mode = ?", undef, $new_sql_mode);
         }
     }
 
     # Allow large GROUP_CONCATs (largely for inserting comments
     # into bugs_fulltext).
-    $self->do('SET SESSION group_concat_max_len = 128000000');
-
-    return $self;
+    $dbh->do('SET SESSION group_concat_max_len = 128000000');
 }
 
 # when last_insert_id() is supported on MySQL by lowest DBI/DBD version
diff --git a/Bugzilla/DB/Oracle.pm b/Bugzilla/DB/Oracle.pm
index 5c9ea68a3..a519bb796 100644
--- a/Bugzilla/DB/Oracle.pm
+++ b/Bugzilla/DB/Oracle.pm
@@ -22,10 +22,9 @@ For interface details see L<Bugzilla::DB> and L<DBI>.
 package Bugzilla::DB::Oracle;
 
 use 5.10.1;
-use strict;
-use warnings;
+use Moo;
 
-use base qw(Bugzilla::DB);
+extends qw(Bugzilla::DB);
 
 use DBD::Oracle;
 use DBD::Oracle qw(:ora_types);
@@ -47,7 +46,7 @@ use constant FULLTEXT_OR => ' OR ';
 
 our $fulltext_label = 0;
 
-sub new {
+sub BUILDARGS {
     my ($class, $params) = @_;
     my ($user, $pass, $host, $dbname, $port) =
         @$params{qw(db_user db_pass db_host db_name db_port)};
@@ -66,22 +65,20 @@ sub new {
                   LongReadLen => max(Bugzilla->params->{'maxattachmentsize'} || 0,
                                      MIN_LONG_READ_LEN) * 1024,
                 };
-    my $self = $class->db_new({ dsn => $dsn, user => $user,
-                                pass => $pass, attrs => $attrs });
-    # Needed by TheSchwartz
-    $self->{private_bz_dsn} = $dsn;
+    return { dsn => $dsn, user => $user, pass => $pass, attrs => $attrs };
+}
 
-    bless ($self, $class);
+sub on_dbi_connected {
+    my ($class, $dbh) = @_;
 
     # Set the session's default date format to match MySQL
-    $self->do("ALTER SESSION SET NLS_DATE_FORMAT='YYYY-MM-DD HH24:MI:SS'");
-    $self->do("ALTER SESSION SET NLS_TIMESTAMP_FORMAT='YYYY-MM-DD HH24:MI:SS'");
-    $self->do("ALTER SESSION SET NLS_LENGTH_SEMANTICS='CHAR'")
+    $dbh->do("ALTER SESSION SET NLS_DATE_FORMAT='YYYY-MM-DD HH24:MI:SS'");
+    $dbh->do("ALTER SESSION SET NLS_TIMESTAMP_FORMAT='YYYY-MM-DD HH24:MI:SS'");
+    $dbh->do("ALTER SESSION SET NLS_LENGTH_SEMANTICS='CHAR'")
         if Bugzilla->params->{'utf8'};
     # To allow case insensitive query.
-    $self->do("ALTER SESSION SET NLS_COMP='ANSI'");
-    $self->do("ALTER SESSION SET NLS_SORT='BINARY_AI'");
-    return $self;
+    $dbh->do("ALTER SESSION SET NLS_COMP='ANSI'");
+    $dbh->do("ALTER SESSION SET NLS_SORT='BINARY_AI'");
 }
 
 sub bz_last_key {
diff --git a/Bugzilla/DB/Pg.pm b/Bugzilla/DB/Pg.pm
index 8bcbc3262..0db349412 100644
--- a/Bugzilla/DB/Pg.pm
+++ b/Bugzilla/DB/Pg.pm
@@ -22,19 +22,18 @@ For interface details see L<Bugzilla::DB> and L<DBI>.
 package Bugzilla::DB::Pg;
 
 use 5.10.1;
-use strict;
-use warnings;
+use Moo;
 
 use Bugzilla::Error;
 use Bugzilla::Version;
 use DBD::Pg;
 
 # This module extends the DB interface via inheritance
-use base qw(Bugzilla::DB);
+extends qw(Bugzilla::DB);
 
 use constant BLOB_TYPE => { pg_type => DBD::Pg::PG_BYTEA };
 
-sub new {
+sub BUILDARGS {
     my ($class, $params) = @_;
     my ($user, $pass, $host, $dbname, $port) =
         @$params{qw(db_user db_pass db_host db_name db_port)};
@@ -55,18 +54,7 @@ sub new {
 
     my $attrs = { pg_enable_utf8 => Bugzilla->params->{'utf8'} };
 
-    my $self = $class->db_new({ dsn => $dsn, user => $user,
-                                pass => $pass, attrs => $attrs });
-
-    # all class local variables stored in DBI derived class needs to have
-    # a prefix 'private_'. See DBI documentation.
-    $self->{private_bz_tables_locked} = "";
-    # Needed by TheSchwartz
-    $self->{private_bz_dsn} = $dsn;
-
-    bless ($self, $class);
-
-    return $self;
+    return { dsn => $dsn, user => $user, pass => $pass, attrs => $attrs }
 }
 
 # if last_insert_id is supported on PostgreSQL by lowest DBI/DBD version
diff --git a/Bugzilla/DB/Sqlite.pm b/Bugzilla/DB/Sqlite.pm
index 87f45415b..3890d0795 100644
--- a/Bugzilla/DB/Sqlite.pm
+++ b/Bugzilla/DB/Sqlite.pm
@@ -8,10 +8,9 @@
 package Bugzilla::DB::Sqlite;
 
 use 5.10.1;
-use strict;
-use warnings;
+use Moo;
 
-use base qw(Bugzilla::DB);
+extends qw(Bugzilla::DB);
 
 use Bugzilla::Constants;
 use Bugzilla::Error;
@@ -69,7 +68,7 @@ sub _sqlite_position_ci {
 # Constructor #
 ###############
 
-sub new {
+sub BUILDARGS {
     my ($class, $params) = @_;
     my $db_name = $params->{db_name};
 
@@ -97,10 +96,11 @@ sub new {
         sqlite_unicode => Bugzilla->params->{'utf8'},
     };
 
-    my $self = $class->db_new({ dsn => $dsn, user => '',
-                                pass => '', attrs => $attrs });
-    # Needed by TheSchwartz
-    $self->{private_bz_dsn} = $dsn;
+    return { dsn => $dsn, user => '', pass => '', attrs => $attrs };
+}
+
+sub on_dbi_connected {
+    my ($class, $dbh) = @_;
 
     my %pragmas = (
         # Make sure that the sqlite file doesn't grow without bound.
@@ -122,23 +122,20 @@ sub new {
     );
 
     while (my ($name, $value) = each %pragmas) {
-        $self->do("PRAGMA $name = $value");
+        $dbh->do("PRAGMA $name = $value");
     }
 
-    $self->sqlite_create_collation('bugzilla', \&_sqlite_collate_ci);
-    $self->sqlite_create_function('position', 2, \&_sqlite_position);
-    $self->sqlite_create_function('iposition', 2, \&_sqlite_position_ci);
+    $dbh->sqlite_create_collation('bugzilla', \&_sqlite_collate_ci);
+    $dbh->sqlite_create_function('position', 2, \&_sqlite_position);
+    $dbh->sqlite_create_function('iposition', 2, \&_sqlite_position_ci);
     # SQLite has a "substr" function, but other DBs call it "SUBSTRING"
     # so that's what we use, and I don't know of any way in SQLite to
     # alias the SQL "substr" function to be called "SUBSTRING".
-    $self->sqlite_create_function('substring', 3, \&CORE::substr);
-    $self->sqlite_create_function('mod', 2, \&_sqlite_mod);
-    $self->sqlite_create_function('now', 0, \&_sqlite_now);
-    $self->sqlite_create_function('localtimestamp', 1, \&_sqlite_now);
-    $self->sqlite_create_function('floor', 1, \&POSIX::floor);
-
-    bless ($self, $class);
-    return $self;
+    $dbh->sqlite_create_function('substring', 3, \&CORE::substr);
+    $dbh->sqlite_create_function('mod', 2, \&_sqlite_mod);
+    $dbh->sqlite_create_function('now', 0, \&_sqlite_now);
+    $dbh->sqlite_create_function('localtimestamp', 1, \&_sqlite_now);
+    $dbh->sqlite_create_function('floor', 1, \&POSIX::floor);
 }
 
 ###############
diff --git a/Bugzilla/JobQueue.pm b/Bugzilla/JobQueue.pm
index afb36673f..a78a4d0ae 100644
--- a/Bugzilla/JobQueue.pm
+++ b/Bugzilla/JobQueue.pm
@@ -59,7 +59,7 @@ sub new {
     # to write to it.
     my $self = $class->SUPER::new(
         databases => [{
-            dsn    => Bugzilla->dbh_main->{private_bz_dsn},
+            dsn    => Bugzilla->dbh_main->dsn,
             user   => $lc->{db_user},
             pass   => $lc->{db_pass},
             prefix => 'ts_',