From 32fc3f9ed14df5cec9e13064c93959fce8dec69c Mon Sep 17 00:00:00 2001 From: Marcel de Rooy 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 Signed-off-by: Jonathan Druart --- 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.5