From b6f02212aed9e3dc305f8e516fa934574bfd2ee2 Mon Sep 17 00:00:00 2001 From: David Gustafsson Date: Tue, 1 Nov 2022 15:30:01 +0100 Subject: [PATCH] Bug 32060: Improve performance of columns_to_str To test: 1) Ensure the following tests pass t/db_dependent/Koha/Item.t t/db_dependent/Koha/Bibio.t 2) Go to a biblio (preferably as serial) with many items and click "New" -> "New item" and note down the response time. 3) Apply the patch 4) Ensure tests in 1) still pass 5) Repeat step 2), the response time should be substantially improved Sponsored-by: Gothenburg University Library Signed-off-by: Jonathan Druart Signed-off-by: Marcel de Rooy Signed-off-by: Tomas Cohen Arazi (cherry picked from commit 850f6f403ba07d6c963a11f52d774b7a94df4802) Signed-off-by: Matt Blenkinsop --- C4/Biblio.pm | 17 +++++++++++++---- Koha/Item.pm | 18 +++++++----------- t/db_dependent/Biblio.t | 22 +++++++++++++++++++++- 3 files changed, 41 insertions(+), 16 deletions(-) diff --git a/C4/Biblio.pm b/C4/Biblio.pm index 0bbd8645a3..5f2c7f518b 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -1832,10 +1832,16 @@ sub UpsertMarcControlField { sub GetFrameworkCode { my ($biblionumber) = @_; - my $dbh = C4::Context->dbh; - my $sth = $dbh->prepare("SELECT frameworkcode FROM biblio WHERE biblionumber=?"); - $sth->execute($biblionumber); - my ($frameworkcode) = $sth->fetchrow; + my $cache = Koha::Cache::Memory::Lite->get_instance(); + my $cache_key = "FrameworkCode-$biblionumber"; + my $frameworkcode = $cache->get_from_cache($cache_key); + unless (defined $frameworkcode) { + my $dbh = C4::Context->dbh; + my $sth = $dbh->prepare("SELECT frameworkcode FROM biblio WHERE biblionumber=?"); + $sth->execute($biblionumber); + ($frameworkcode) = $sth->fetchrow; + $cache->set_in_cache($cache_key, $frameworkcode); + } return $frameworkcode; } @@ -2557,6 +2563,9 @@ sub _koha_modify_biblio { $biblio->{'abstract'}, $biblio->{'biblionumber'} ) if $biblio->{'biblionumber'}; + my $cache = Koha::Cache::Memory::Lite->get_instance(); + $cache->clear_from_cache("FrameworkCode-" . $biblio->{biblionumber}); + if ( $dbh->errstr || !$biblio->{'biblionumber'} ) { $error .= "ERROR in _koha_modify_biblio $query" . $dbh->errstr; warn $error; diff --git a/Koha/Item.pm b/Koha/Item.pm index 681da5877e..6d5a75a235 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -1052,16 +1052,14 @@ This is meant to be used for display purpose only. sub columns_to_str { my ( $self ) = @_; + my $frameworkcode = C4::Biblio::GetFrameworkCode($self->biblionumber); + my $tagslib = C4::Biblio::GetMarcStructure( 1, $frameworkcode, { unsafe => 1 } ); + my $mss = C4::Biblio::GetMarcSubfieldStructure( $frameworkcode, { unsafe => 1 } ); - my $frameworkcode = $self->biblio->frameworkcode; - my $tagslib = C4::Biblio::GetMarcStructure(1, $frameworkcode); my ( $itemtagfield, $itemtagsubfield) = C4::Biblio::GetMarcFromKohaField( "items.itemnumber" ); - my $columns_info = $self->_result->result_source->columns_info; - - my $mss = C4::Biblio::GetMarcSubfieldStructure( $frameworkcode, { unsafe => 1 } ); my $values = {}; - for my $column ( keys %$columns_info ) { + for my $column ( @{$self->_columns}) { next if $column eq 'more_subfields_xml'; @@ -2093,10 +2091,8 @@ or staff client strings. sub strings_map { my ( $self, $params ) = @_; - - my $columns_info = $self->_result->result_source->columns_info; - my $frameworkcode = $self->biblio->frameworkcode; - my $tagslib = C4::Biblio::GetMarcStructure( 1, $frameworkcode ); + my $frameworkcode = C4::Biblio::GetFrameworkCode($self->biblionumber); + my $tagslib = C4::Biblio::GetMarcStructure( 1, $frameworkcode, { unsafe => 1 } ); my $mss = C4::Biblio::GetMarcSubfieldStructure( $frameworkcode, { unsafe => 1 } ); my ( $itemtagfield, $itemtagsubfield ) = C4::Biblio::GetMarcFromKohaField("items.itemnumber"); @@ -2111,7 +2107,7 @@ sub strings_map { # Handle not null and default values for integers and dates my $strings = {}; - foreach my $col ( keys %{$columns_info} ) { + foreach my $col ( @{$self->_columns} ) { # By now, we are done with known columns, now check the framework for mappings my $field = $self->_result->result_source->name . '.' . $col; diff --git a/t/db_dependent/Biblio.t b/t/db_dependent/Biblio.t index 44623e351b..28c90e1f2b 100755 --- a/t/db_dependent/Biblio.t +++ b/t/db_dependent/Biblio.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 16; +use Test::More tests => 17; use Test::MockModule; use Test::Warn; use List::MoreUtils qw( uniq ); @@ -905,7 +905,27 @@ subtest 'record test' => sub { $biblio->metadata->record->as_formatted ); }; +subtest 'GetFrameworkCode' => sub { + plan tests => 4; + + my $biblio = $builder->build_sample_biblio({ frameworkcode => 'OBP' }); + + is(GetFrameworkCode($biblio->biblionumber), 'OBP', 'GetFrameworkCode returns correct frameworkcode'); + + my $cache = Koha::Cache::Memory::Lite->get_instance(); + my $cache_key = "FrameworkCode-" . $biblio->biblionumber; + my $frameworkcode = $cache->get_from_cache($cache_key); + is($frameworkcode, 'OBP', 'Cache has been set in GetFrameworkCode'); + # Set new value directly in cache to make sure it's actually being used + $cache->set_in_cache($cache_key, 'OD'); + is(GetFrameworkCode($biblio->biblionumber), 'OD', 'GetFrameworkCode returns correct frameworkcode, using cache'); + + # Test cache invalidation + ModBiblio($biblio->metadata->record, $biblio->biblionumber, 'OGR'); + is(GetFrameworkCode($biblio->biblionumber), 'OGR', 'GetFrameworkCode returns correct frameworkcode after setting a new one though ModBiblio'); + +}; # Cleanup Koha::Caches->get_instance->clear_from_cache( "MarcSubfieldStructure-" ); -- 2.39.5