From 32fc3f9ed14df5cec9e13064c93959fce8dec69c Mon Sep 17 00:00:00 2001
From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Date: Thu, 12 Apr 2018 08:54:34 +0200
Subject: [PATCH] Bug 14769: (QA follow-up) Remove global var
 $cached_indicators

As requested by RM, this patch replaces using the global $cached_indicators
by saving state temporarily during the (limited) lifetime of the object.

Essentially this affects two places in code:
[1] blinddetail-biblio-search.pl (loading auth record in editor)
[2] AuthoritiesMarc::merge (merging authority into biblios)

Concurrent runs of [1] and/or [2] together with a simultaneous pref change
just in between could cause slight (hypothetical) side-effects.
The current approach of keeping state in the object makes that a series of
controlled_indicators calls during an immediate merge of one specific
authority is not affected by a simultaneous pref change. So the same rules
are applied to the set of attached biblio record for that authority.
Note also that the cron job ignores a simultaneous pref change, since
it reads from the unchanged L1 cache (yes, also hypothetical).

Test plan:
[1] Run t/Koha/Authority/ControlledIndicators.t
[2] Run t/db_dependent/Authority/Merge.t
[3] Run t/db_dependent/Koha/Authorities.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
---
 Koha/Authority/ControlledIndicators.pm  | 22 ++++------------------
 t/Koha/Authority/ControlledIndicators.t | 20 +-------------------
 2 files changed, 5 insertions(+), 37 deletions(-)

diff --git a/Koha/Authority/ControlledIndicators.pm b/Koha/Authority/ControlledIndicators.pm
index 64a87348d8..0cd688d99c 100644
--- a/Koha/Authority/ControlledIndicators.pm
+++ b/Koha/Authority/ControlledIndicators.pm
@@ -20,8 +20,6 @@ package Koha::Authority::ControlledIndicators;
 use Modern::Perl;
 use C4::Context;
 
-our $cached_indicators;
-
 =head1 NAME
 
 Koha::Authority::ControlledIndicators - Obtain biblio indicators, controlled by authority record
@@ -39,7 +37,6 @@ Koha::Authority::ControlledIndicators - Obtain biblio indicators, controlled by
 sub new {
     my ( $class, $params ) = @_;
     $params = {} if ref($params) ne 'HASH';
-    $cached_indicators = undef;
     return bless $params, $class;
 }
 
@@ -66,11 +63,11 @@ sub get {
     $flavour = uc($flavour);
     $flavour = 'UNIMARC' if $flavour eq 'UNIMARCAUTH';
 
-    $cached_indicators //= _load_pref();
+    $self->{_parsed} //= _load_pref();
     my $result = {};
-    return $result if !exists $cached_indicators->{$flavour};
-    my $rule = $cached_indicators->{$flavour}->{$tag} //
-        $cached_indicators->{$flavour}->{'*'} //
+    return $result if !exists $self->{_parsed}->{$flavour};
+    my $rule = $self->{_parsed}->{$flavour}->{$tag} //
+        $self->{_parsed}->{$flavour}->{'*'} //
         {};
     my $report_fld = $record ? $record->field( $report_tag ) : undef;
 
@@ -146,17 +143,6 @@ sub _thesaurus_info {
     return ( $ind, $sub2 );
 }
 
-=head3 clear
-
-    Clear internal cache.
-
-=cut
-
-sub clear {
-    my ( $self, $params ) = @_;
-    $cached_indicators = undef;
-}
-
 =head1 AUTHOR
 
     Marcel de Rooy, Rijksmuseum Amsterdam, The Netherlands
diff --git a/t/Koha/Authority/ControlledIndicators.t b/t/Koha/Authority/ControlledIndicators.t
index 8552507f7c..1d51e3027d 100644
--- a/t/Koha/Authority/ControlledIndicators.t
+++ b/t/Koha/Authority/ControlledIndicators.t
@@ -10,7 +10,7 @@ use t::lib::Mocks;
 use Koha::Authority::ControlledIndicators;
 
 subtest "Simple tests" => sub {
-    plan tests => 10;
+    plan tests => 8;
 
     t::lib::Mocks::mock_preference('AuthorityControlledIndicators', q|
 marc21,600,ind1:auth1,ind2:x
@@ -52,24 +52,6 @@ marc21,800,ind1:,
     });
     is( $res->{ind1}, '', 'ind1: clears 1st indicator' );
     is( exists $res->{ind2}, '', 'Check if 2nd indicator key does not exist' );
-
-    # Test caching
-    t::lib::Mocks::mock_preference('AuthorityControlledIndicators', q{} );
-    $res = $oInd->get({
-        flavour => "MARC21",
-        report_tag  => '100',
-        auth_record => $record,
-        biblio_tag  => '700',
-    });
-    is( $res->{ind1}, '4', 'Cache not cleared yet' );
-    $oInd->clear;
-    $res = $oInd->get({
-        flavour => "MARC21",
-        report_tag  => '100',
-        auth_record => $record,
-        biblio_tag  => '700',
-    });
-    is_deeply( $res, {}, 'Cache cleared' );
 };
 
 subtest "Tests for sub _thesaurus_info" => sub {
-- 
2.39.2