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>
This commit is contained in:
Marcel de Rooy 2018-04-12 08:54:34 +02:00 committed by Jonathan Druart
parent 0ea53c7456
commit 32fc3f9ed1
2 changed files with 5 additions and 37 deletions

View File

@ -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

View File

@ -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 {