From: Dylan William Hardison <dylan@hardison.net>
Date: Sun, 7 Jul 2019 20:57:04 +0000 (-0400)
Subject: Ensure we always call DBIx::Connector->dbh before any DBI method
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2bdd08653c3f52172735a0ada8adf060876d8d6a;p=thirdparty%2Fbugzilla.git

Ensure we always call DBIx::Connector->dbh before any DBI method

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.
---

diff --git a/Bugzilla/DB.pm b/Bugzilla/DB.pm
index 715c3b7fef..e3bbff6187 100644
--- a/Bugzilla/DB.pm
+++ b/Bugzilla/DB.pm
@@ -14,14 +14,7 @@ use DBI;
 use DBIx::Connector;
 our %Connector;
 
-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
-  ]],
-);
+has 'connector' => (is => 'lazy', handles => [qw( dbh )],);
 
 use Bugzilla::Constants;
 use Bugzilla::Mailer;
@@ -38,6 +31,29 @@ use Storable qw(dclone);
 
 has [qw(dsn user pass attrs)] => (is => 'ro', 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
 #####################################################################
@@ -139,9 +155,7 @@ sub _connect {
     "'$driver' is not a valid choice for \$db_driver in " . " localconfig: " . $@);
 
   # instantiate the correct DB specific module
-  my $dbh = $pkg_module->new($params);
-
-  return $dbh;
+  return $pkg_module->new($params);
 }
 
 sub _handle_error {
@@ -1307,7 +1321,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);
@@ -1338,10 +1352,8 @@ sub _build_dbh {
       = {connected => sub { $class->on_dbi_connected(@_); return },};
   }
 
-  my $connector = $Connector{"$user.$dsn"}
+  return $Connector{"$user.$dsn"}
     //= DBIx::Connector->new($dsn, $user, $pass, $attributes);
-
-  return $connector->dbh;
 }
 
 #####################################################################