From 3f6c9fe752ffcf7a1271b7eb2b135874825d1db2 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 --- C4/Biblio.pm | 17 +++++++++++++---- Koha/Item.pm | 18 +++++++----------- t/db_dependent/Biblio.t | 23 ++++++++++++++++++++++- 3 files changed, 42 insertions(+), 16 deletions(-) diff --git a/C4/Biblio.pm b/C4/Biblio.pm index dbb4026816c..cc2a043c6ea 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -1830,10 +1830,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; } @@ -2555,6 +2561,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 ea2677a01ff..fddc92bab9b 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -1051,16 +1051,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'; @@ -2099,10 +2097,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"); @@ -2117,7 +2113,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 3d6f96e7591..61925b49289 100755 --- a/t/db_dependent/Biblio.t +++ b/t/db_dependent/Biblio.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 17; +use Test::More tests => 18; use Test::MockModule; use Test::Warn; use List::MoreUtils qw( uniq ); @@ -909,6 +909,27 @@ subtest 'record_schema test' => sub { }; +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.25.1