Attachment 32874 Details for Bug 11998 – Bug 11998 - make syspref caching use Koha::Cache

[patch] Bug 11998 - make syspref caching use Koha::Cache

Bug-11998---make-syspref-caching-use-KohaCache.patch (text/plain), 6.35 KB, created by Brendan Gallagher on 2014-10-28 21:45:51 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Brendan Gallagher

Created: 2014-10-28 21:45:51 UTC

Size: 6.35 KB

patch

obsolete

>From 3bb1bf7541681f849057dc2323977d4762854d46 Mon Sep 17 00:00:00 2001
>From: Robin Sheat <robin@catalyst.net.nz>
>Date: Mon, 22 Sep 2014 16:39:49 +1200
>Subject: [PATCH] Bug 11998 - make syspref caching use Koha::Cache
>
>This modifies the caching system in C4::Context to use Koha::Cache
>instead of a custom implementation.
>
>It should be applied on top of the other syspref caching patches.
>
>Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
>---
> C4/Context.pm                   |   80 +++++----------------------------------
> t/db_dependent/Circulation.t    |    1 +
> t/db_dependent/Context.t        |    1 +
> t/db_dependent/check_sysprefs.t |    1 +
> 4 files changed, 12 insertions(+), 71 deletions(-)
>
>diff --git a/C4/Context.pm b/C4/Context.pm
>index a37c90e..d9b8376 100644
>--- a/C4/Context.pm
>+++ b/C4/Context.pm
>@@ -103,6 +103,7 @@ use XML::Simple;
> use C4::Boolean;
> use C4::Debug;
> use C4::Log qw/ logaction /;
>+use Koha::Cache;
> use POSIX ();
> use DateTime::TimeZone;
> use Module::Load::Conditional qw(can_load);
>@@ -375,7 +376,7 @@ sub new {
>             return;
>         }
>     }
>-    
>+
>     if ($ismemcached) {
>         # retreive from memcached
>         $self = $memcached->get('kohaconf');
>@@ -518,67 +519,6 @@ sub ModZebrations {
> 	return _common_config($_[1],'serverinfo');
> }
> 
>-=head2 _cache_preference($variable, [$value])
>-
>-  _cache_preference('opacusercss', $css);
>-  my $val = _cache_preference('opacusercss');
>-
>-An interface into the syspref caching. One argument to get a value, two to set
>-it. The single argument version will return C<undef> if the cache doesn't
>-contain the value. If the second argument is C<undef>, then the cache will be
>-cleared of that value.
>-
>-The cache timeout length is 30 seconds when memcache is not in use. When
>-memcache is in use, it uses the default cache time for that.
>-
>-=cut
>-
>-my %syspref_cache;
>-my $use_syspref_cache = 1;
>-
>-sub _cache_preference {
>-    return undef unless $use_syspref_cache;
>-    my $memcached        = memcached();
>-    my $memcached_prefix = "_syspref_";
>-    if ( scalar(@_) == 1 ) {
>-        # This means we want to fetch from the cache
>-        my ($var) = @_;
>-        if ($memcached) {
>-            my $val = $memcached->get( $memcached_prefix . lc($var) );
>-            return $val;
>-        }
>-        else {
>-            my $now          = time;
>-            my $cache_oldest = $now - 30;
>-            return undef unless exists $syspref_cache{ lc $var };
>-            my $val = $syspref_cache{ lc $var };
>-            return undef unless $val->[0] > $cache_oldest;
>-            return $val->[1];
>-        }
>-    }
>-    elsif ( scalar(@_) == 2 ) {
>-        # Set something in the cache
>-        my ( $var, $val ) = @_;
>-        if ($memcached) {
>-            if ( !defined($val) ) {
>-                $memcached->delete( $memcached_prefix . lc($var) );
>-            }
>-            else {
>-                $memcached->set( $memcached_prefix . lc($var), $val );
>-            }
>-        }
>-        else {
>-            if ( !defined($val) ) {
>-                delete $syspref_cache{ lc $var };
>-                return;
>-            }
>-            $syspref_cache{ lc $var } = [ time, $val ];
>-        }
>-    } else {
>-        confess "Invalid arguments to _cache_preference: " . join(', ', @_);
>-    }
>-}
>-
> =head2 preference
> 
>   $sys_preference = C4::Context->preference('some_variable');
>@@ -595,11 +535,13 @@ with this method.
> 
> =cut
> 
>+my $syspref_cache = Koha::Cache->get_instance();
>+my $use_syspref_cache = 1;
> sub preference {
>     my $self = shift;
>     my $var  = shift;    # The system preference to return
> 
>-    my $cached_var = _cache_preference($var);
>+    my $cached_var = $syspref_cache->get_from_cache("syspref_$var") if $use_syspref_cache;
>     return $cached_var if defined $cached_var;
> 
>     my $dbh  = C4::Context->dbh or return 0;
>@@ -618,7 +560,7 @@ sub preference {
>         $value = $dbh->selectrow_array( $sql, {}, lc $var );
>     }
> 
>-    _cache_preference($var, $value);
>+    $syspref_cache->set_in_cache("syspref_$var", $value) if $use_syspref_cache;
>     return $value;
> }
> 
>@@ -669,11 +611,7 @@ will not be seen by this process.
> =cut
> 
> sub clear_syspref_cache {
>-    %syspref_cache= ();
>-    # It doesn't seem to be possible to list all the keys via Cache::Memcached
>-    # so we'll clear the whole cache.
>-    my $memcached = memcached();
>-    $memcached->flush_all if $memcached;
>+    $syspref_cache->flush_all if $use_syspref_cache;
> }
> 
> =head2 set_preference
>@@ -719,7 +657,7 @@ ON DUPLICATE KEY UPDATE value = VALUES(value)';
>     }
>     my $sth = $dbh->prepare($query);
>     if ( $sth->execute(@params) ) {
>-        _cache_preference($var, $value);
>+        $syspref_cache->set_in_cache("syspref_$var", $value) if $use_syspref_cache;
>         logaction( 'SYSTEMPREFERENCE', $logtype, undef, $var . " | " . $value );
>     }
>     $sth->finish;
>@@ -744,7 +682,7 @@ sub delete_preference {
>     my $sth = $dbh->prepare("DELETE FROM systempreferences WHERE variable=?");
>     my $res = $sth->execute($var);
>     if ($res) {
>-        _cache_preference($var, undef);
>+        $syspref_cache->clear_from_cache("syspref_$var") if $use_syspref_cache;
>         logaction( 'SYSTEMPREFERENCE', 'DELETE', undef,
>             $var . " | " . ( $val // 'undefined' ) );
>         return 1;
>diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t
>index 1740795..e7d7cfe 100755
>--- a/t/db_dependent/Circulation.t
>+++ b/t/db_dependent/Circulation.t
>@@ -55,6 +55,7 @@ my $borrower = {
> };
> 
> # No userenv, PickupLibrary
>+C4::Context->disable_syspref_cache();
> C4::Context->set_preference('CircControl', 'PickupLibrary');
> is(
>     C4::Context->preference('CircControl'),
>diff --git a/t/db_dependent/Context.t b/t/db_dependent/Context.t
>index feb2a43..f90fdab 100755
>--- a/t/db_dependent/Context.t
>+++ b/t/db_dependent/Context.t
>@@ -53,6 +53,7 @@ $module->mock(
> );
> 
> my $history;
>+C4::Context->disable_syspref_cache();
> $dbh = C4::Context->dbh({ new => 1 });
> 
> $dbh->{mock_add_resultset} = [ ['value'], ['thing1'] ];
>diff --git a/t/db_dependent/check_sysprefs.t b/t/db_dependent/check_sysprefs.t
>index f7fc914..a862a11 100755
>--- a/t/db_dependent/check_sysprefs.t
>+++ b/t/db_dependent/check_sysprefs.t
>@@ -23,6 +23,7 @@ use warnings;
> use Getopt::Long;
> use C4::Context;
> 
>+C4::Context->disable_syspref_cache();
> # When this option is set, no tests are performed.
> # The missing sysprefs are displayed as sql inserts instead.
> our $showsql = 0;
>-- 
>1.7.10.4

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