From 169c4fdce5473d87f00d36868be22abea1dc636d Mon Sep 17 00:00:00 2001 From: Fridolin Somers Date: Wed, 10 Jun 2015 15:06:13 +0200 Subject: [PATCH] Bug 14375 - DBIx::Connector should be stored in C4::Context instead of dbh DBIx::Connector doc about $conn->dbh() indicates he will manage database handler cache and reconnection : http://search.cpan.org/perldoc?DBIx%3A%3AConnector#dbh So C4::Context should not store dbh object but DBIx::Connector object and get handler from it. We experimented issues with SIP server, since its database connections will run all day long. Looks like DBIx::Connector manages very well the reconnection. Note that mysql_enable_utf8 option is passed in DBIx::Connector creation so it can be set again when the connector recreates a dbh. This patch has to drop methods set_dbh() and restore_dbh() because they seem to be impossible to implement with this changes. Test plan : - Check database connection does not fail for pages - Check database connection does not fail for SIP Server - One may check its ok with Plack --- C4/Context.pm | 109 ++++++++++++---------------------------------------------- 1 file changed, 22 insertions(+), 87 deletions(-) diff --git a/C4/Context.pm b/C4/Context.pm index 4386792..ef1e20e 100644 --- a/C4/Context.pm +++ b/C4/Context.pm @@ -170,11 +170,8 @@ environment variable to the pathname of a configuration file to use. # A reference-to-hash whose keys and values are the # configuration variables and values specified in the config # file (/etc/koha/koha-conf.xml). -# dbh -# A handle to the appropriate database for this context. -# dbh_stack -# Used by &set_dbh and &restore_dbh to hold other database -# handles for this context. +# DBconn +# A connection to the appropriate database for this context. # Zconn # A connection object for the Zebra server @@ -365,7 +362,7 @@ sub new { warn "read_config_file($conf_fname) returned undef" if !defined($self->{"config"}); return if !defined($self->{"config"}); - $self->{"dbh"} = undef; # Database handle + $self->{"DBconn"} = undef; # Database connexion $self->{"Zconn"} = undef; # Zebra Connections $self->{"stopwords"} = undef; # stopwords list $self->{"marcfromkohafield"} = undef; # the hash with relations between koha table fields and MARC field/subfield @@ -521,8 +518,6 @@ sub preference { return $sysprefs{lc $var}; } - my $dbh = C4::Context->dbh or return 0; - my $value; if ( defined $ENV{"OVERRIDE_SYSPREF_$var"} ) { $value = $ENV{"OVERRIDE_SYSPREF_$var"}; @@ -737,11 +732,11 @@ sub _new_Zconn { return $Zconn; } -# _new_dbh +# _new_DBconn # Internal helper function (not a method!). This creates a new # database connection from the data given in the current context, and # returns it. -sub _new_dbh +sub _new_DBconn { ## $context @@ -753,14 +748,17 @@ sub _new_dbh my $db_port = $context->config("port") || ''; my $db_user = $context->config("user"); my $db_passwd = $context->config("pass"); - # MJR added or die here, as we can't work without dbh - my $dbh = DBIx::Connector->connect( + # MJR added or die here, as we can't work without database connection + my $DBconn = DBIx::Connector->new( "dbi:$db_driver:dbname=$db_name;host=$db_host;port=$db_port", $db_user, $db_passwd, { - 'RaiseError' => $ENV{DEBUG} ? 1 : 0 + 'RaiseError' => $ENV{DEBUG} ? 1 : 0, + 'mysql_enable_utf8' => 1, } ); + # Get handler from connector + my $dbh = $DBconn->dbh; # Check for the existence of a systempreference table; if we don't have this, we don't # have a valid database and should not set RaiseError in order to allow the installer @@ -778,9 +776,8 @@ sub _new_dbh my $tz = $ENV{TZ}; if ( $db_driver eq 'mysql' ) { - # Koha 3.0 is utf-8, so force utf8 communication between mySQL and koha, whatever the mysql default config. + # Koha 3.0 is utf-8, so force utf8 communication between MySQL and koha, whatever the MySQL default config. # this is better than modifying my.cnf (and forcing all communications to be in utf8) - $dbh->{'mysql_enable_utf8'}=1; #enable $dbh->do("set NAMES 'utf8'"); ($tz) and $dbh->do(qq(SET time_zone = "$tz")); } @@ -788,7 +785,7 @@ sub _new_dbh $dbh->do( "set client_encoding = 'UTF8';" ); ($tz) and $dbh->do(qq(SET TIME ZONE = "$tz")); } - return $dbh; + return $DBconn; } =head2 dbh @@ -811,20 +808,17 @@ sub dbh { my $self = shift; my $params = shift; - my $sth; - unless ( $params->{new} ) { - if ( defined($context->{db_driver}) && $context->{db_driver} eq 'mysql' && $context->{"dbh"} ) { - return $context->{"dbh"}; - } elsif ( defined($context->{"dbh"}) && $context->{"dbh"}->ping() ) { - return $context->{"dbh"}; - } + if ( $context->{'DBconn'} && !$params->{'new'} ) { + # DBIx::Connector will manage database handler cache and reconnection + # See http://search.cpan.org/perldoc?DBIx%3A%3AConnector#dbh + return $context->{'DBconn'}->dbh; } - # No database handle or it died . Create one. - $context->{"dbh"} = &_new_dbh(); + # No database connection, create one. + $context->{'DBconn'} = &_new_DBconn; - return $context->{"dbh"}; + return $context->{'DBconn'}->dbh; } =head2 new_dbh @@ -834,7 +828,7 @@ sub dbh Creates a new connection to the Koha database for the current context, and returns the database handle (a C object). -The handle is not saved anywhere: this method is strictly a +The connection is not saved anywhere: this method is strictly a convenience function; the point is that it knows which database to connect to so that the caller doesn't have to know. @@ -844,66 +838,7 @@ connect to so that the caller doesn't have to know. sub new_dbh { my $self = shift; - - return &_new_dbh(); -} - -=head2 set_dbh - - $my_dbh = C4::Connect->new_dbh; - C4::Connect->set_dbh($my_dbh); - ... - C4::Connect->restore_dbh; - -C<&set_dbh> and C<&restore_dbh> work in a manner analogous to -C<&set_context> and C<&restore_context>. - -C<&set_dbh> saves the current database handle on a stack, then sets -the current database handle to C<$my_dbh>. - -C<$my_dbh> is assumed to be a good database handle. - -=cut - -#' -sub set_dbh -{ - my $self = shift; - my $new_dbh = shift; - - # Save the current database handle on the handle stack. - # We assume that $new_dbh is all good: if the caller wants to - # screw himself by passing an invalid handle, that's fine by - # us. - push @{$context->{"dbh_stack"}}, $context->{"dbh"}; - $context->{"dbh"} = $new_dbh; -} - -=head2 restore_dbh - - C4::Context->restore_dbh; - -Restores the database handle saved by an earlier call to -Cset_dbh>. - -=cut - -#' -sub restore_dbh -{ - my $self = shift; - - if ($#{$context->{"dbh_stack"}} < 0) - { - # Stack underflow - die "DBH stack underflow"; - } - - # Pop the old database handle and set it. - $context->{"dbh"} = pop @{$context->{"dbh_stack"}}; - - # FIXME - If it is determined that restore_context should - # return something, then this function should, too. + return &_new_DBconn->dbh; } =head2 queryparser -- 2.1.0