From d3e00529333fb2eadac9c22a45f20c1050401952 Mon Sep 17 00:00:00 2001 From: Dylan William Hardison Date: Thu, 13 Sep 2018 22:37:55 +0200 Subject: 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. --- Bugzilla/DB.pm | 43 ++++++++++++++++++++++++++++--------------- Makefile.PL | 1 + 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', -- cgit v1.2.3-24-g4f1b