From 943ae5ca9d198aac44c7e3cf29a4176e7e7dbd68 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Thu, 26 May 2022 16:31:30 +0100 Subject: [PATCH] Bug 30848: Caching for performance This patch adds caching to ExpandAuthorizedValues for performance. We cache each hash involved in C4::Biblio::GetAuthorisedValueDesc as well as caching the reduced marc subfield structure that filter down to just those marc fields with coded substitutions. --- C4/Biblio.pm | 67 +++++++++-- Koha/AuthorisedValue.pm | 33 +++++- Koha/ClassSource.pm | 29 ++++- Koha/Filter/MARC/ExpandAuthorizedValues.pm | 109 ++++++++++-------- Koha/ItemType.pm | 31 +++++ Koha/Library.pm | 30 +++++ Koha/Localization.pm | 29 +++++ admin/biblio_framework.pl | 2 + admin/marc_subfields_structure.pl | 2 + admin/marctagstructure.pl | 2 + .../Koha/Filter/ExpandAuthorizedValues.t | 1 + 11 files changed, 273 insertions(+), 62 deletions(-) diff --git a/C4/Biblio.pm b/C4/Biblio.pm index 459a94510a..93c675e622 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -98,6 +98,7 @@ use C4::Charset qw( SetUTF8Flag StripNonXmlChars ); +use C4::Languages; use C4::Linker; use C4::OAI::Sets; use C4::Items qw( GetHiddenItemnumbers GetMarcItem ); @@ -1464,25 +1465,56 @@ descriptions rather than normal ones when they exist. sub GetAuthorisedValueDesc { my ( $tag, $subfield, $value, $framework, $tagslib, $category, $opac ) = @_; + my $cache = Koha::Caches->get_instance(); + my $cache_key; if ( !$category ) { return $value unless defined $tagslib->{$tag}->{$subfield}->{'authorised_value'}; #---- branch if ( $tagslib->{$tag}->{$subfield}->{'authorised_value'} eq "branches" ) { - my $branch = Koha::Libraries->find($value); - return $branch? $branch->branchname: q{}; + $cache_key = "LibraryNames"; + my $libraries = $cache->get_from_cache( $cache_key, { unsafe => 1 } ); + if ( !$libraries ) { + $libraries = { + map { $_->branchcode => $_->branchname } + Koha::Libraries->search( {}, + { columns => [ 'branchcode', 'branchname' ] } ) + ->as_list + }; + $cache->set_in_cache($cache_key, $libraries); + } + return $libraries->{$value}; } #---- itemtypes if ( $tagslib->{$tag}->{$subfield}->{'authorised_value'} eq "itemtypes" ) { - my $itemtype = Koha::ItemTypes->find( $value ); - return $itemtype ? $itemtype->translated_description : q||; + my $lang = C4::Languages::getlanguage; + $lang //= 'en'; + $cache_key = $lang . 'ItemTypeDescriptions'; + my $itypes = $cache->get_from_cache( $cache_key, { unsafe => 1 } ); + if ( !$itypes ) { + $itypes = + { map { $_->itemtype => $_->translated_description } + Koha::ItemTypes->search()->as_list }; + $cache->set_in_cache( $cache_key, $itypes ); + } + return $itypes->{$value}; } if ( $tagslib->{$tag}->{$subfield}->{'authorised_value'} eq "cn_source" ) { - my $source = GetClassSource($value); - return $source ? $source->{description} : q||; + $cache_key = "ClassSources"; + my $cn_sources = $cache->get_from_cache( $cache_key, { unsafe => 1 } ); + if ( !$cn_sources ) { + $cn_sources = { + map { $_->cn_source => $_->description } + Koha::ClassSources->search( {}, + { columns => [ 'cn_source', 'description' ] } ) + ->as_list + }; + $cache->set_in_cache($cache_key, $cn_sources); + } + return $cn_sources->{$value}; } #---- "true" authorized value @@ -1491,10 +1523,25 @@ sub GetAuthorisedValueDesc { my $dbh = C4::Context->dbh; if ( $category ne "" ) { - my $sth = $dbh->prepare( "SELECT lib, lib_opac FROM authorised_values WHERE category = ? AND authorised_value = ?" ); - $sth->execute( $category, $value ); - my $data = $sth->fetchrow_hashref; - return ( $opac && $data->{'lib_opac'} ) ? $data->{'lib_opac'} : $data->{'lib'}; + $cache_key = "AVDescriptions-" . $category; + my $av_descriptions = $cache->get_from_cache( $cache_key, { unsafe => 1 } ); + if ( !$av_descriptions ) { + $av_descriptions = { + map { + $_->authorised_value => + { lib => $_->lib, lib_opac => $_->lib_opac } + } Koha::AuthorisedValues->search( + { category => $category }, + { + columns => [ 'authorised_value', 'lib_opac', 'lib' ] + } + )->as_list + }; + $cache->set_in_cache($cache_key, $av_descriptions); + } + return ( $opac && $av_descriptions->{$value}->{'lib_opac'} ) + ? $av_descriptions->{$value}->{'lib_opac'} + : $av_descriptions->{$value}->{'lib'}; } else { return $value; # if nothing is found return the original value } diff --git a/Koha/AuthorisedValue.pm b/Koha/AuthorisedValue.pm index 8f97bb1ede..5f292de3e6 100644 --- a/Koha/AuthorisedValue.pm +++ b/Koha/AuthorisedValue.pm @@ -19,7 +19,7 @@ package Koha::AuthorisedValue; use Modern::Perl; - +use Koha::Caches; use Koha::Database; use base qw(Koha::Object Koha::Object::Limit::Library); @@ -34,6 +34,37 @@ Koha::AuthorisedValue - Koha Authorised value Object class =cut +=head3 store + +AuthorisedValue specific store to ensure relevant caches are flushed on change + +=cut + +sub store { + my ($self) = @_; + + my $flush = 0; + + if ( !$self->in_storage ) { + $flush = 1; + } + else { + my $self_from_storage = $self->get_from_storage; + $flush = 1 if ( $self_from_storage->lib ne $self->lib ); + $flush = 1 if ( $self_from_storage->lib_opac ne $self->lib_opac ); + } + + $self = $self->SUPER::store; + + if ($flush) { + my $cache = Koha::Caches->get_instance(); + my $key = "AVDescriptions-".$self->category; + $cache->clear_from_cache($key); + } + + return $self; +} + =head3 opac_description my $description = $av->opac_description(); diff --git a/Koha/ClassSource.pm b/Koha/ClassSource.pm index 3b192ee124..5a2011a65f 100644 --- a/Koha/ClassSource.pm +++ b/Koha/ClassSource.pm @@ -17,7 +17,7 @@ package Koha::ClassSource; use Modern::Perl; - +use Koha::Caches; use Koha::Database; use base qw(Koha::Object); @@ -30,8 +30,35 @@ Koha::ClassSource - Koha Classfication Source Object class =head2 Class Methods +=head3 store + +ClassSource specific store to ensure relevant caches are flushed on change + =cut +sub store { + my ($self) = @_; + + my $flush = 0; + + if ( !$self->in_storage ) { + $flush = 1; + } + else { + my $self_from_storage = $self->get_from_storage; + $flush = 1 if ( $self_from_storage->description ne $self->description ); + } + + $self = $self->SUPER::store; + + if ($flush) { + my $cache = Koha::Caches->get_instance(); + $cache->clear_from_cache('ClassSources'); + } + + return $self; +} + =head3 _type Returns name of corresponding DBIC resultset diff --git a/Koha/Filter/MARC/ExpandAuthorizedValues.pm b/Koha/Filter/MARC/ExpandAuthorizedValues.pm index 3306839728..6c7b2a6837 100644 --- a/Koha/Filter/MARC/ExpandAuthorizedValues.pm +++ b/Koha/Filter/MARC/ExpandAuthorizedValues.pm @@ -49,13 +49,16 @@ Filter to replace Koha AV codes in MARC::Records with their Koha AV descriptions use Modern::Perl; -use C4::Biblio qw( GetAuthorisedValueDesc GetMarcStructure ); +use C4::Biblio qw(GetAuthorisedValueDesc); + +use Koha::Caches; +use Koha::ClassSources; +use Koha::Libraries; +use Koha::ItemTypes; use base qw(Koha::RecordProcessor::Base); our $NAME = 'ExpandAuthorizedValues'; -my %authval_per_framework; # Cache for tagfield-tagsubfield to decode per framework. - =head2 filter Embed items into the MARC::Record object. @@ -69,65 +72,71 @@ sub filter { return unless defined $record and ref($record) eq 'MARC::Record'; my $params = $self->params; - my $interface = $params->{options}->{interface} // 'opac'; - my $opac = $interface eq 'opac' ? 1 : 0; + my $interface = $params->{options}->{interface} // 'opac'; my $frameworkcode = $params->{options}->{frameworkcode} // q{}; - my $marcstructure = $params->{options}->{marcstructure} - // GetMarcStructure( 1, $frameworkcode, { unsafe => 1 } ); + my $opac = $interface eq 'opac' ? 1 : 0; my $marcflavour = C4::Context->preference('marcflavour'); - my $av = _getAuthorisedValues4MARCSubfields($frameworkcode); - foreach my $tag ( keys %$av ) { - foreach my $field ( $record->field($tag) ) { - if ( $av->{$tag} ) { - my @new_subfields = (); - for my $subfield ( $field->subfields() ) { - my ( $letter, $value ) = @$subfield; - - # Replace the field value with the authorised value - # *except* for MARC21 field 942$n (suppression in opac) - if ( !( $tag eq '942' && $subfield->[0] eq 'n' ) - || $marcflavour eq 'UNIMARC' ) - { - $value = - GetAuthorisedValueDesc( $tag, $letter, $value, '', - $marcstructure, undef, $opac ) - if $av->{$tag}->{$letter}; - } - push( @new_subfields, $letter, $value ); + my $coded_fields = _getCodedFields($frameworkcode); + + my $desc_field = $opac ? 'lib_opac' : 'lib'; + for my $tag ( keys %$coded_fields ) { + for my $field ( $record->field($tag) ) { + my @new_subfields = (); + for my $subfield ( $field->subfields() ) { + my ( $letter, $value ) = @$subfield; + + # Replace the field value with the authorised value + # *except* for MARC21 field 942$n (suppression in opac) + if ( !( $tag eq '942' && $subfield->[0] eq 'n' ) + || $marcflavour eq 'UNIMARC' ) + { + $value = GetAuthorisedValueDesc( $tag, $letter, $value, '', $coded_fields, undef, $opac ) if $coded_fields->{$tag}->{$letter}; } - $field->replace_with( - MARC::Field->new( - $tag, $field->indicator(1), - $field->indicator(2), @new_subfields - ) - ); + push( @new_subfields, $letter, $value ); } + $field->replace_with( + MARC::Field->new( + $tag, $field->indicator(1), + $field->indicator(2), @new_subfields + ) + ); } } return $record; } -sub _getAuthorisedValues4MARCSubfields { +sub _getCodedFields { my ($frameworkcode) = @_; - unless ( $authval_per_framework{$frameworkcode} ) { - my $dbh = C4::Context->dbh; - my $sth = $dbh->prepare( - "SELECT DISTINCT tagfield, tagsubfield - FROM marc_subfield_structure - WHERE authorised_value IS NOT NULL - AND authorised_value!='' - AND frameworkcode=?" - ); - $sth->execute($frameworkcode); - my $av = {}; - while ( my ( $tag, $letter ) = $sth->fetchrow() ) { - $av->{$tag}->{$letter} = 1; - } - $authval_per_framework{$frameworkcode} = $av; - } - return $authval_per_framework{$frameworkcode}; + $frameworkcode //= ""; + + my $cache = Koha::Caches->get_instance(); + my $cache_key = "MarcCodedFields-$frameworkcode"; + my $cached = $cache->get_from_cache( $cache_key, { unsafe => 1 } ); + return $cached if $cached; + + my $coded_fields = { + map { + $_->tagfield => { + $_->tagsubfield => { + 'authorised_value' => $_->authorised_value + } + } + } Koha::MarcSubfieldStructures->search( + { + frameworkcode => $frameworkcode, + authorised_value => { '>' => '' } + }, + { + columns => [ 'tagfield', 'tagsubfield', 'authorised_value' ], + order_by => [ 'tagfield', 'tagsubfield' ] + } + )->as_list + }; + + $cache->set_in_cache( $cache_key, $coded_fields ); + return $coded_fields; } 1; diff --git a/Koha/ItemType.pm b/Koha/ItemType.pm index 4b77433e8e..754acc97d1 100644 --- a/Koha/ItemType.pm +++ b/Koha/ItemType.pm @@ -20,6 +20,7 @@ use Modern::Perl; use C4::Koha qw( getitemtypeimagelocation ); use C4::Languages; +use Koha::Caches; use Koha::Database; use Koha::CirculationRules; use Koha::Localizations; @@ -36,6 +37,36 @@ Koha::ItemType - Koha Item type Object class =cut +=head3 store + +ItemType specific store to ensure relevant caches are flushed on change + +=cut + +sub store { + my ($self) = @_; + + my $flush = 0; + + if ( !$self->in_storage ) { + $flush = 1; + } + else { + my $self_from_storage = $self->get_from_storage; + $flush = 1 if ( $self_from_storage->description ne $self->description ); + } + + $self = $self->SUPER::store; + + if ($flush) { + my $cache = Koha::Caches->get_instance(); + my $key = "enItemTypeDescriptions"; + $cache->clear_from_cache($key); + } + + return $self; +} + =head3 image_location =cut diff --git a/Koha/Library.pm b/Koha/Library.pm index 48e125529b..7db111ab8d 100644 --- a/Koha/Library.pm +++ b/Koha/Library.pm @@ -22,6 +22,7 @@ use Modern::Perl; use C4::Context; +use Koha::Caches; use Koha::Database; use Koha::StockRotationStages; use Koha::SMTP::Servers; @@ -36,6 +37,35 @@ Koha::Library - Koha Library Object class =head2 Class methods +=head3 store + +Library specific store to ensure relevant caches are flushed on change + +=cut + +sub store { + my ($self) = @_; + + my $flush = 0; + + if ( !$self->in_storage ) { + $flush = 1; + } + else { + my $self_from_storage = $self->get_from_storage; + $flush = 1 if ( $self_from_storage->branchname ne $self->branchname ); + } + + $self = $self->SUPER::store; + + if ($flush) { + my $cache = Koha::Caches->get_instance(); + $cache->clear_from_cache('LibraryNames'); + } + + return $self; +} + =head3 stockrotationstages my $stages = Koha::Library->stockrotationstages; diff --git a/Koha/Localization.pm b/Koha/Localization.pm index 3f698303c4..c700c42d6e 100644 --- a/Koha/Localization.pm +++ b/Koha/Localization.pm @@ -21,6 +21,35 @@ use Koha::Database; use base qw(Koha::Object); +=head1 NAME + +Koha::Localization - Koha Localization type Object class + +=head1 API + +=head2 Class methods + +=cut + +=head3 store + +Localization specific store to ensure relevant caches are flushed on change + +=cut + +sub store { + my ($self) = @_; + $self = $self->SUPER::store; + + if ($self->entity eq 'itemtypes') { + my $cache = Koha::Caches->get_instance(); + my $key = $self->lang."ItemTypeDescriptions"; + $cache->clear_from_cache($key); + } + + return $self; +} + sub _type { return 'Localization'; } diff --git a/admin/biblio_framework.pl b/admin/biblio_framework.pl index 32f2ac11eb..4712f0ea72 100755 --- a/admin/biblio_framework.pl +++ b/admin/biblio_framework.pl @@ -80,6 +80,7 @@ if ( $op eq 'add_form' ) { $cache->clear_from_cache("MarcStructure-1-$frameworkcode"); $cache->clear_from_cache("default_value_for_mod_marc-"); $cache->clear_from_cache("MarcSubfieldStructure-$frameworkcode"); + $cache->clear_from_cache("MarcCodedFields-$frameworkcode"); $op = 'list'; } elsif ( $op eq 'delete_confirm' ) { my $framework = Koha::BiblioFrameworks->find($frameworkcode); @@ -111,6 +112,7 @@ if ( $op eq 'add_form' ) { $cache->clear_from_cache("MarcStructure-1-$frameworkcode"); $cache->clear_from_cache("default_value_for_mod_marc-"); $cache->clear_from_cache("MarcSubfieldStructure-$frameworkcode"); + $cache->clear_from_cache("MarcCodedFields-$frameworkcode"); $op = 'list'; } diff --git a/admin/marc_subfields_structure.pl b/admin/marc_subfields_structure.pl index a2117eb912..a492e5c568 100755 --- a/admin/marc_subfields_structure.pl +++ b/admin/marc_subfields_structure.pl @@ -309,6 +309,7 @@ elsif ( $op eq 'add_validate' ) { $cache->clear_from_cache("MarcStructure-1-$frameworkcode"); $cache->clear_from_cache("default_value_for_mod_marc-"); $cache->clear_from_cache("MarcSubfieldStructure-$frameworkcode"); + $cache->clear_from_cache("MarcCodedFields-$frameworkcode"); print $input->redirect("/cgi-bin/koha/admin/marc_subfields_structure.pl?tagfield=$tagfield&frameworkcode=$frameworkcode"); exit; @@ -347,6 +348,7 @@ elsif ( $op eq 'delete_confirmed' ) { $cache->clear_from_cache("MarcStructure-1-$frameworkcode"); $cache->clear_from_cache("default_value_for_mod_marc-"); $cache->clear_from_cache("MarcSubfieldStructure-$frameworkcode"); + $cache->clear_from_cache("MarcCodedFields-$frameworkcode"); print $input->redirect("/cgi-bin/koha/admin/marc_subfields_structure.pl?tagfield=$tagfield&frameworkcode=$frameworkcode"); exit; diff --git a/admin/marctagstructure.pl b/admin/marctagstructure.pl index 7ce19f99ac..3514baca93 100755 --- a/admin/marctagstructure.pl +++ b/admin/marctagstructure.pl @@ -155,6 +155,7 @@ if ($op eq 'add_form') { $cache->clear_from_cache("MarcStructure-1-$frameworkcode"); $cache->clear_from_cache("default_value_for_mod_marc-"); $cache->clear_from_cache("MarcSubfieldStructure-$frameworkcode"); + $cache->clear_from_cache("MarcCodedFields-$frameworkcode"); print $input->redirect("/cgi-bin/koha/admin/marctagstructure.pl?searchfield=$tagfield&frameworkcode=$frameworkcode"); exit; # END $OP eq ADD_VALIDATE @@ -180,6 +181,7 @@ if ($op eq 'add_form') { $cache->clear_from_cache("MarcStructure-1-$frameworkcode"); $cache->clear_from_cache("default_value_for_mod_marc-"); $cache->clear_from_cache("MarcSubfieldStructure-$frameworkcode"); + $cache->clear_from_cache("MarcCodedFields-$frameworkcode"); $template->param( searchfield => $searchfield ); # END $OP eq DELETE_CONFIRMED ################## ITEMTYPE_CREATE ################################## diff --git a/t/db_dependent/Koha/Filter/ExpandAuthorizedValues.t b/t/db_dependent/Koha/Filter/ExpandAuthorizedValues.t index e2e6281dcc..d8f18af0de 100644 --- a/t/db_dependent/Koha/Filter/ExpandAuthorizedValues.t +++ b/t/db_dependent/Koha/Filter/ExpandAuthorizedValues.t @@ -62,6 +62,7 @@ subtest 'ExpandAuthorizedValues tests' => sub { my $cache = Koha::Caches->get_instance; $cache->clear_from_cache("MarcStructure-0-"); $cache->clear_from_cache("MarcStructure-1-"); + $cache->clear_from_cache("MarcCodedFields-"); $cache->clear_from_cache("default_value_for_mod_marc-"); $cache->clear_from_cache("MarcSubfieldStructure-"); -- 2.20.1