Attachment 26621 Details for Bug 11998 – Bug 11998 - move most syspref db access into Context.pm

[patch] Bug 11998 - move most syspref db access into Context.pm

Bug-11998---move-most-syspref-db-access-into-Conte.patch (text/plain), 9.40 KB, created by Robin Sheat on 2014-03-27 04:47:23 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Robin Sheat

Created: 2014-03-27 04:47:23 UTC

Size: 9.40 KB

patch

obsolete

>From 7055d4a27088a6a90f83c951974d9b8ba43ef7e9 Mon Sep 17 00:00:00 2001
>From: Robin Sheat <robin@catalyst.net.nz>
>Date: Thu, 27 Mar 2014 17:44:23 +1300
>Subject: [PATCH] Bug 11998 - move most syspref db access into Context.pm
>
>There was a lot of direct database access in systempreferences.pl. This
>moves a lot of it into C4::Context so that it'll interact with caching
>better, and to centralise common functions and logging in one place.
>
>Test plan:
>* Make sure that creating, editing, deleting local use preferences works
>* Make sure that other system preferences still function normally
>---
> C4/Context.pm              | 76 ++++++++++++++++++++++++++++++++++++----------
> admin/systempreferences.pl | 61 ++++++++-----------------------------
> 2 files changed, 73 insertions(+), 64 deletions(-)
>
>diff --git a/C4/Context.pm b/C4/Context.pm
>index 0c48296..5249cab 100644
>--- a/C4/Context.pm
>+++ b/C4/Context.pm
>@@ -102,6 +102,7 @@ use ZOOM;
> use XML::Simple;
> use C4::Boolean;
> use C4::Debug;
>+use C4::Log qw/ logaction /;
> use POSIX ();
> use DateTime::TimeZone;
> use Module::Load::Conditional qw(can_load);
>@@ -623,37 +624,80 @@ sub clear_syspref_cache {
> 
> =head2 set_preference
> 
>-  C4::Context->set_preference( $variable, $value );
>+  C4::Context->set_preference( $variable, $value, [ $explanation, $type, $options ] );
> 
> This updates a preference's value both in the systempreferences table and in
>-the sysprefs cache.
>+the sysprefs cache. If the optional parameters are provided, then the query
>+becomes a create. It won't update the parameters (except value) for an existing
>+preference.
> 
> =cut
> 
> sub set_preference {
>-    my $self = shift;
>-    my $var = lc(shift);
>-    my $value = shift;
>+    my ( $self, $var, $value, $expl, $type, $options ) = @_;
>+    $var = lc($var);
> 
>     my $dbh = C4::Context->dbh or return 0;
> 
>-    my $type = $dbh->selectrow_array( "SELECT type FROM systempreferences WHERE variable = ?", {}, $var );
>-
>-    $value = 0 if ( $type && $type eq 'YesNo' && $value eq '' );
>+    my $db_type = $dbh->selectrow_array(
>+        "SELECT type FROM systempreferences WHERE variable = ?",
>+        {}, $var );
> 
>-    my $sth = $dbh->prepare( "
>-      INSERT INTO systempreferences
>-        ( variable, value )
>-        VALUES( ?, ? )
>-        ON DUPLICATE KEY UPDATE value = VALUES(value)
>-    " );
>+    $value = 0 if ( $db_type && $db_type eq 'YesNo' && $value eq '' );
> 
>-    if($sth->execute( $var, $value )) {
>-        $syspref_cache{$var} = [time, $value];
>+    my ( $query, @params, $logtype );
>+    if ( !defined($db_type) ) {
>+        $query = '
>+INSERT INTO systempreferences
>+    ( variable, value, explanation, type, options )
>+VALUES( ?, ?, ?, ?, ? )';
>+        @params = ( $var, $value, $expl, $type, $options );
>+        $logtype = 'ADD';
>+    }
>+    else {
>+        $query = '
>+INSERT INTO systempreferences
>+    ( variable, value )
>+VALUES( ?, ? )
>+ON DUPLICATE KEY UPDATE value = VALUES(value)';
>+        @params = ( $var, $value );
>+        $logtype = 'MODIFY';
>+    }
>+    my $sth = $dbh->prepare($query);
>+    if ( $sth->execute(@params) ) {
>+        $syspref_cache{$var} = [ time, $value ];
>+        logaction( 'SYSTEMPREFERENCE', $logtype, undef, $var . " | " . $value );
>     }
>     $sth->finish;
> }
> 
>+=head2 delete_preference
>+
>+    C4::Context->delete_preference( $variable );
>+
>+This deletes a system preference from the database. Returns a true value on
>+success. Failure means there was an issue with the database, not that there
>+was no syspref of the name.
>+
>+=cut
>+
>+sub delete_preference {
>+    my ( $self, $var ) = @_;
>+
>+    # We want to log the previous value
>+    my $val = C4::Context->preference($var);
>+    my $dbh = C4::Context->dbh or die "Unable to talk to the database";
>+    my $sth = $dbh->prepare("DELETE FROM systempreferences WHERE variable=?");
>+    my $res = $sth->execute($var);
>+    if ($res) {
>+        delete $syspref_cache{$var};
>+        logaction( 'SYSTEMPREFERENCE', 'DELETE', undef,
>+            $var . " | " . ( $var // 'undefined' ) );
>+        return 1;
>+    }
>+    warn "Unable to delete syspref: " . $sth->errstr;
>+    return 0;
>+}
> # AUTOLOAD
> # This implements C4::Config->foo, and simply returns
> # C4::Context->config("foo"), as described in the documentation for
>diff --git a/admin/systempreferences.pl b/admin/systempreferences.pl
>index d41f2a7..d043ea3 100755
>--- a/admin/systempreferences.pl
>+++ b/admin/systempreferences.pl
>@@ -50,7 +50,6 @@ use C4::Context;
> use C4::Koha;
> use C4::Languages qw(getTranslatedLanguages);
> use C4::ClassSource;
>-use C4::Log;
> use C4::Output;
> use YAML::Syck qw( Dump LoadFile );
> 
>@@ -269,24 +268,8 @@ if ( $op eq 'update_and_reedit' ) {
>             );    # we show only the TMPL_VAR names $op
>         }
>     }
>-    my $dbh   = C4::Context->dbh;
>-    my $query = "select * from systempreferences where variable=?";
>-    my $sth   = $dbh->prepare($query);
>-    $sth->execute( $input->param('variable') );
>-    if ( $sth->rows ) {
>-        unless ( C4::Context->config('demo') ) {
>-            my $sth = $dbh->prepare("update systempreferences set value=?,explanation=?,type=?,options=? where variable=?");
>-            $sth->execute( $value, $input->param('explanation'), $input->param('variable'), $input->param('preftype'), $input->param('prefoptions') );
>-            logaction( 'SYSTEMPREFERENCE', 'MODIFY', undef, $input->param('variable') . " | " . $value );
>-        }
>-    } else {
>-        unless ( C4::Context->config('demo') ) {
>-            my $sth = $dbh->prepare("insert into systempreferences (variable,value,explanation) values (?,?,?,?,?)");
>-            $sth->execute( $input->param('variable'), $input->param('value'), $input->param('explanation'), $input->param('preftype'), $input->param('prefoptions') );
>-            logaction( 'SYSTEMPREFERENCE', 'ADD', undef, $input->param('variable') . " | " . $input->param('value') );
>-        }
>-    }
>-
>+    my $variable = $input->param('variable');
>+    C4::Context->set_preference($variable, $value) unless C4::Context->config('demo');
> }
> 
> ################## ADD_FORM ##################################
>@@ -315,13 +298,14 @@ if ( $op eq 'add_form' ) {
> ################## ADD_VALIDATE ##################################
>     # called by add_form, used to insert/modify data in DB
> } elsif ( $op eq 'add_validate' ) {
>-    my $dbh = C4::Context->dbh;
>-    my $sth = $dbh->prepare("select * from systempreferences where variable=?");
>-    $sth->execute( $input->param('variable') );
>-
>     # to handle multiple values
>     my $value;
> 
>+    my $variable = $input->param('variable');
>+    my $expl     = $input->param('explanation');
>+    my $type     = $input->param('preftype');
>+    my $options  = $input->param('prefoptions');
>+
>     # handle multiple value strings (separated by ',')
>     my $params = $input->Vars;
>     if ( defined $params->{'value'} ) {
>@@ -338,49 +322,30 @@ if ( $op eq 'add_form' ) {
>         }
>     }
> 
>-    if ( $input->param('preftype') eq 'Upload' ) {
>+    if ( $type eq 'Upload' ) {
>         my $lgtfh = $input->upload('value');
>         $value = join '', <$lgtfh>;
>         $value = encode_base64($value);
>     }
> 
>-    if ( $sth->rows ) {
>-        unless ( C4::Context->config('demo') ) {
>-            my $sth = $dbh->prepare("update systempreferences set value=?,explanation=?,type=?,options=? where variable=?");
>-            $sth->execute( $value, $input->param('explanation'), $input->param('preftype'), $input->param('prefoptions'), $input->param('variable') );
>-            logaction( 'SYSTEMPREFERENCE', 'MODIFY', undef, $input->param('variable') . " | " . $value );
>-        }
>-    } else {
>-        unless ( C4::Context->config('demo') ) {
>-            my $sth = $dbh->prepare("insert into systempreferences (variable,value,explanation,type,options) values (?,?,?,?,?)");
>-            $sth->execute( $input->param('variable'), $value, $input->param('explanation'), $input->param('preftype'), $input->param('prefoptions') );
>-            logaction( 'SYSTEMPREFERENCE', 'ADD', undef, $input->param('variable') . " | " . $value );
>-        }
>-    }
>+    C4::Context->set_preference( $variable, $value, $expl, $type, $options )
>+      unless C4::Context->config('demo');
>     print "Content-Type: text/html\n\n<META HTTP-EQUIV=Refresh CONTENT=\"0; URL=systempreferences.pl?tab=\"></html>";
>     exit;
> ################## DELETE_CONFIRM ##################################
>     # called by default form, used to confirm deletion of data in DB
> } elsif ( $op eq 'delete_confirm' ) {
>-    my $dbh = C4::Context->dbh;
>-    my $sth = $dbh->prepare("select variable,value,explanation,type,options from systempreferences where variable=?");
>-    $sth->execute($searchfield);
>-    my $data = $sth->fetchrow_hashref;
>+    my $value = C4::Context->preference($searchfield);
>     $template->param(
>         searchfield => $searchfield,
>-        Tvalue      => $data->{'value'},
>+        Tvalue      => $value,
>     );
> 
>     # END $OP eq DELETE_CONFIRM
> ################## DELETE_CONFIRMED ##################################
>     # called by delete_confirm, used to effectively confirm deletion of data in DB
> } elsif ( $op eq 'delete_confirmed' ) {
>-    my $dbh = C4::Context->dbh;
>-    my $sth = $dbh->prepare("delete from systempreferences where variable=?");
>-    $sth->execute($searchfield);
>-    my $logstring = $searchfield . " | " . $Tvalue;
>-    logaction( 'SYSTEMPREFERENCE', 'DELETE', undef, $logstring );
>-
>+    C4::Context->delete_preference($searchfield);
>     # END $OP eq DELETE_CONFIRMED
> ################## DEFAULT ##################################
> } else {    # DEFAULT
>-- 
>1.8.3.2

Actions: View | Diff | Splinter Review

Attachments on bug 11998: 26575 | 26621 | 26714 | 26716 | 26717 | 27382 | 31801 | 31802 | 32870 | 32871 | 32872 | 32873 | 32874 | 33183 | 33447 | 33670 | 33671 | 33685 | 33724 | 33725 | 33746 | 33829 | 33940 | 36523 | 36524 | 36525 | 36526 | 36527 | 36528 | 36529 | 36530 | 36531 | 36532 | 36533 | 36534 | 36571 | 36583 | 36584 | 36585 | 36586 | 36587 | 36588 | 36589 | 36590 | 36591 | 36592 | 36593 | 36594 | 36595 | 48640 | 48652 | 48655 | 48687 | 48688 | 48727 | 48758 | 48928 | 48929 | 48930 | 48931 | 48932 | 48933 | 48934 | 48935 | 48936 | 48961 | 49022 | 49023 | 49024 | 49025 | 49026 | 49027 | 49028 | 49029 | 49030 | 49100 | 49101 | 49102 | 49103 | 49104 | 49105 | 49106 | 49107 | 49108