Bugzilla – Attachment 135716 Details for
Bug 30848
Introduce Koha::Filter::ExpandCodedFields
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 30848: Caching for performance
Bug-30848-Caching-for-performance.patch (text/plain), 19.54 KB, created by
Martin Renvoize (ashimema)
on 2022-06-06 10:44:15 UTC
(
hide
)
Description:
Bug 30848: Caching for performance
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2022-06-06 10:44:15 UTC
Size:
19.54 KB
patch
obsolete
>From 943ae5ca9d198aac44c7e3cf29a4176e7e7dbd68 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@ptfs-europe.com> >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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 30848
:
135338
|
135353
|
135363
|
135364
|
135386
|
135387
|
135388
|
135418
|
135421
|
135422
|
135423
|
135424
|
135425
|
135707
|
135708
|
135709
|
135710
|
135711
|
135712
|
135713
|
135714
|
135715
|
135716
|
135717
|
135770
|
136025
|
136026
|
136027
|
136259
|
136260
|
136261
|
136262
|
136774
|
136775
|
136776
|
136777