Bug 1490708 - Ensure we always call DBIx::Connector->dbh before any DBI method (#744)

The code didn't allow a way of doing this without a lot of work.

So I had to take the following approach:
The 'dbh' attribute is now a method that delegates to DBIx::Connector's dbh
method. Per the docs, ->dbh() "Returns the connection's database handle. It will
use a an existing handle if there is one, if the process has not been forked or
a new thread spawned, and if the database is pingable. Otherwise, it will
instantiate, cache, and return a new handle."

Then there is the matter of the 'handles' on dbh. I've used Package::Stash to
insert proxy methods into the class when it is loaded.

2 files changed, 29 insertions, 15 deletions

diff --git a/Bugzilla/DB.pm b/Bugzilla/DB.pm
index 80404131a..cd5954219 100644
--- a/Bugzilla/DB.pm
+++ b/Bugzilla/DB.pm
@@ -14,15 +14,9 @@ use DBI;
 use DBIx::Connector;
 our %Connector;
 
-has 'dbh' => (
+has 'connector' => (
     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
-        ]
-    ],
+    handles => [ qw( dbh ) ],
 );
 
 use Bugzilla::Constants;
@@ -44,6 +38,29 @@ has [qw(dsn user pass attrs)] => (
     required => 1,
 );
 
+
+# Install proxy methods to the DBI object.
+# We can't use handles() as DBIx::Connector->dbh has to be called each
+# time we need a DBI handle to ensure the connection is alive.
+{
+    my @DBI_METHODS = 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
+    );
+    my $stash = Package::Stash->new(__PACKAGE__);
+
+    foreach my $method (@DBI_METHODS) {
+        my $symbol = '&' . $method;
+        $stash->add_symbol(
+            $symbol => sub {
+                my $self = shift;
+                return $self->dbh->$method(@_);
+            }
+        );
+    }
+}
+
 #####################################################################
 # Constants
 #####################################################################
@@ -152,9 +169,7 @@ sub _connect {
 
     # instantiate the correct DB specific module
 
-    my $dbh = $pkg_module->new($params);
-
-    return $dbh;
+    return $pkg_module->new($params);
 }
 
 sub _handle_error {
@@ -1263,7 +1278,7 @@ sub bz_rollback_transaction {
 # Subclass Helpers
 #####################################################################
 
-sub _build_dbh {
+sub _build_connector {
     my ($self) = @_;
     my ($dsn, $user, $pass, $override_attrs) =
         map { $self->$_ } qw(dsn user pass attrs);
@@ -1295,9 +1310,7 @@ sub _build_dbh {
         }
     }
 
-    my $connector = $Connector{"$user.$dsn"} //= DBIx::Connector->new($dsn, $user, $pass, $attributes);
-
-    return $connector->dbh;
+    return $Connector{"$user.$dsn"} //= DBIx::Connector->new($dsn, $user, $pass, $attributes);
 }
 
 #####################################################################
diff --git a/Makefile.PL b/Makefile.PL
index 3c600bef6..6d4225e9a 100755
--- a/Makefile.PL
+++ b/Makefile.PL
@@ -72,6 +72,7 @@ my %requires = (
     'Moo'                      => '2.002004',
     'MooX::StrictConstructor'  => '0.008',
     'Mozilla::CA'              => '20160104',
+    'Package::Stash'           => '0.37',
     'Parse::CPAN::Meta'        => '1.44',
     'Role::Tiny'               => '2.000003',
     'Scope::Guard'             => '0.21',