From ba0d582cb0125ad9a29ca17f87c78a4217f70a3b 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 --- 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 7251a8a094f..036c56622ba 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'; @@ -2098,10 +2096,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"); @@ -2116,7 +2112,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 99310eb6a2e..d9fd57de470 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 ); @@ -892,6 +892,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 acutally 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.40.0