Bug 17530: (QA follow-up) Replace our variable by cached value

Instead of saving the state locally in a variable during Plack lifetime,
we move the saved hash to the cache. We clear the key when we enter
smart-rules.pl. This makes a change in circ rules immediately effective.

Test plan:
Run the modified tests.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

diff --git a/Koha/IssuingRules.pm b/Koha/IssuingRules.pm
index 642c55d..c72dd46 100644 (file)
--- a/Koha/IssuingRules.pm
+++ b/Koha/IssuingRules.pm
@@ -21,11 +21,14 @@ package Koha::IssuingRules;
 use Modern::Perl;
 
 use Koha::Database;
+use Koha::Caches;
 
 use Koha::IssuingRule;
 
 use base qw(Koha::Objects);
 
+use constant GUESSED_ITEMTYPES_KEY => 'Koha_IssuingRules_last_guess';
+
 =head1 NAME
 
 Koha::IssuingRules - Koha IssuingRule Object set class
@@ -157,13 +160,13 @@ sub article_requestable_rules {
 
 =cut
 
-our $last_article_requestable_guesses; # used during Plack life time
-
 sub guess_article_requestable_itemtypes {
     my ( $class_or_self, $params ) = @_;
     my $category = $params->{categorycode};
     return {} if !C4::Context->preference('ArticleRequests');
 
+    my $cache = Koha::Caches->get_instance;
+    my $last_article_requestable_guesses = $cache->get_from_cache(GUESSED_ITEMTYPES_KEY);
     my $key = $category || '*';
     return $last_article_requestable_guesses->{$key}
         if $last_article_requestable_guesses && exists $last_article_requestable_guesses->{$key};
@@ -176,7 +179,7 @@ sub guess_article_requestable_itemtypes {
     foreach my $rule ( $rules->as_list ) {
         $res->{ $rule->itemtype } = 1;
     }
-    $last_article_requestable_guesses->{$key} = $res;
+    $cache->set_in_cache(GUESSED_ITEMTYPES_KEY, $res);
     return $res;
 }
 

diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl
index eb4ef7b..d87d8e5 100755 (executable)
--- a/admin/smart-rules.pl
+++ b/admin/smart-rules.pl
@@ -33,6 +33,7 @@ use Koha::RefundLostItemFeeRule;
 use Koha::RefundLostItemFeeRules;
 use Koha::Libraries;
 use Koha::Patron::Categories;
+use Koha::Caches;
 
 my $input = CGI->new;
 my $dbh = C4::Context->dbh;
@@ -64,6 +65,9 @@ $branch = '*' if $branch eq 'NO_LIBRARY_SET';
 my $op = $input->param('op') || q{};
 my $language = C4::Languages::getlanguage();
 
+my $cache = Koha::Caches->get_instance;
+$cache->clear_from_cache( Koha::IssuingRules::GUESSED_ITEMTYPES_KEY );
+
 if ($op eq 'delete') {
     my $itemtype     = $input->param('itemtype');
     my $categorycode = $input->param('categorycode');

diff --git a/t/db_dependent/ArticleRequests.t b/t/db_dependent/ArticleRequests.t
index d38bf02..c0b92b6 100755 (executable)
--- a/t/db_dependent/ArticleRequests.t
+++ b/t/db_dependent/ArticleRequests.t
@@ -30,6 +30,7 @@ use Koha::Notice::Messages;
 use Koha::Patron;
 use Koha::Library::Group;
 use Koha::IssuingRules;
+use Koha::Caches;
 
 BEGIN {
     use_ok('Koha::ArticleRequest');
@@ -40,6 +41,7 @@ BEGIN {
 my $schema = Koha::Database->new()->schema();
 $schema->storage->txn_begin();
 my $builder = t::lib::TestBuilder->new;
+our $cache = Koha::Caches->get_instance;
 
 my $dbh = C4::Context->dbh;
 $dbh->{RaiseError} = 1;
@@ -223,11 +225,11 @@ subtest 'may_article_request' => sub {
 
     # mocking
     t::lib::Mocks::mock_preference('ArticleRequests', 1);
-    $Koha::IssuingRules::last_article_requestable_guesses = {
+    $cache->set_in_cache( Koha::IssuingRules::GUESSED_ITEMTYPES_KEY, {
         '*'  => { 'CR' => 1 },
         'S'  => { '*'  => 1 },
         'PT' => { 'BK' => 1 },
-    };
+    });
 
     # tests for class method call
     is( Koha::Biblio->may_article_request({ itemtype => 'CR' }), 1, 'SER/* should be true' );
@@ -243,7 +245,7 @@ subtest 'may_article_request' => sub {
     is( $biblio->may_article_request({ categorycode => 'PT' }), 1, 'BK/PT true' );
 
     # Cleanup
-    $Koha::IssuingRules::last_article_requestable_guesses = undef;
+    $cache->clear_from_cache( Koha::IssuingRules::GUESSED_ITEMTYPES_KEY );
 };
 
 $schema->storage->txn_rollback();

diff --git a/t/db_dependent/Koha/IssuingRules/guess_article_requestable_itemtypes.t b/t/db_dependent/Koha/IssuingRules/guess_article_requestable_itemtypes.t
index 71c6c66..5552918 100644 (file)
--- a/t/db_dependent/Koha/IssuingRules/guess_article_requestable_itemtypes.t
+++ b/t/db_dependent/Koha/IssuingRules/guess_article_requestable_itemtypes.t
@@ -7,15 +7,18 @@ use t::lib::Mocks;
 use t::lib::TestBuilder;
 use Koha::Database;
 use Koha::IssuingRules;
+use Koha::Caches;
 
 my $schema = Koha::Database->new->schema;
 $schema->storage->txn_begin;
 our $builder = t::lib::TestBuilder->new;
+our $cache = Koha::Caches->get_instance;
 
 subtest 'guess_article_requestable_itemtypes' => sub {
     plan tests => 12;
 
     t::lib::Mocks::mock_preference('ArticleRequests', 1);
+    $cache->clear_from_cache( Koha::IssuingRules::GUESSED_ITEMTYPES_KEY );
     Koha::IssuingRules->delete;
     my $itype1 = $builder->build_object({ class => 'Koha::ItemTypes' });
     my $itype2 = $builder->build_object({ class => 'Koha::ItemTypes' });
@@ -51,7 +54,7 @@ subtest 'guess_article_requestable_itemtypes' => sub {
 
     # Change the rules
     $rule2->itemtype('*')->store;
-    $Koha::IssuingRules::last_article_requestable_guesses = {};
+    $cache->clear_from_cache( Koha::IssuingRules::GUESSED_ITEMTYPES_KEY );
     $res = Koha::IssuingRules->guess_article_requestable_itemtypes;
     is( $res->{'*'}, 1, 'Item type * seems permitted' );
     is( $res->{$itype1->itemtype}, 1, 'Item type 1 seems permitted' );
@@ -61,7 +64,7 @@ subtest 'guess_article_requestable_itemtypes' => sub {
     is( $res->{$itype1->itemtype}, 1, 'Item type 1 seems permitted' );
     is( $res->{$itype2->itemtype}, undef, 'Item type 2 seems not permitted' );
 
-    $Koha::IssuingRules::last_article_requestable_guesses = {};
+    $cache->clear_from_cache( Koha::IssuingRules::GUESSED_ITEMTYPES_KEY );
 };
 
 $schema->storage->txn_rollback;