From 25b1ece2e3c35f44e4b69242c6cd3e9fb52dc1fa 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 | 24 +++++++++++++++++++++++- 3 files changed, 43 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..954ffd165e3 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 ); @@ -427,6 +427,7 @@ sub run_tests { is( GetMarcPrice( $record_for_isbn, $marcflavour ), 100, "GetMarcPrice returns the correct value"); my $frameworkcode = GetFrameworkCode($biblionumber); + my $updatedrecord = $biblio->metadata->record; my ( $biblioitem_tag, $biblioitem_subfield ) = GetMarcFromKohaField( "biblioitems.biblioitemnumber" ); die qq{No biblioitemnumber tag for framework "$frameworkcode"} unless $biblioitem_tag; @@ -892,6 +893,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