From 13a8f90b03711137425511292943812f8bb3fe86 Mon Sep 17 00:00:00 2001
From: David Gustafsson <david.gustafsson@ub.gu.se>
Date: Mon, 17 Oct 2022 19:08:22 +0200
Subject: [PATCH] Bug 31856: Improve performance of serials subscriptions
search
To test:
1) Perform an advanced search that produces a reasonably large resultset
for serial subscriptions (preferably at least a couple of hundred)
and take not of the execution time.
2) Apply the patch.
3) Perform the search again, the execution should now be about one fifth
of the previous search.
4) Ensure tests in t/db_dependent/AuthorisedValues.t all pass.
Sponsored-by: Gothenburg University Library
---
C4/Serials.pm | 13 ++++-
Koha/AuthorisedValues.pm | 72 ++++++++++++++++--------
Koha/Template/Plugin/AuthorisedValues.pm | 23 +++++---
t/db_dependent/AuthorisedValues.t | 7 +--
4 files changed, 77 insertions(+), 38 deletions(-)
diff --git a/C4/Serials.pm b/C4/Serials.pm
index 4421bf35b0f..8878354a247 100644
--- a/C4/Serials.pm
+++ b/C4/Serials.pm
@@ -40,6 +40,7 @@ use C4::Context;
use C4::Log qw( logaction ); # logaction
use C4::Serials::Frequency qw( GetSubscriptionFrequency );
use C4::Serials::Numberpattern;
+use Koha::AdditionalFields;
use Koha::AdditionalFieldValues;
use Koha::Biblios;
use Koha::DateUtils qw( dt_from_string );
@@ -637,13 +638,21 @@ sub SearchSubscriptions {
if ( $params->{results_limit} && $total_results > $params->{results_limit} ) {
$results = [ splice( @{$results}, 0, $params->{results_limit} ) ];
}
+ my %additional_fields_by_id = map { $_->id => $_->name } Koha::AdditionalFields->search( { tablename => 'subscription' } )->as_list;
for my $subscription ( @$results ) {
$subscription->{cannotedit} = not can_edit_subscription( $subscription );
$subscription->{cannotdisplay} = not can_show_subscription( $subscription );
+ my @additional_field_values = Koha::AdditionalFieldValues->search(
+ {
+ field_id => { -in => [ keys %additional_fields_by_id ] },
+ record_id => $subscription->{subscriptionid}
+ }
+ )->as_list;
- my $subscription_object = Koha::Subscriptions->find($subscription->{subscriptionid});
- $subscription->{additional_field_values} = $subscription_object->get_additional_field_values_for_template;
+ $subscription->{additional_fields} = {
+ map { $additional_fields_by_id{$_->field_id} => $_->value } @additional_field_values
+ };
}
return wantarray ? @{$results} : { results => $results, total => $total_results };
diff --git a/Koha/AuthorisedValues.pm b/Koha/AuthorisedValues.pm
index 377d1de5526..039afc5cc36 100644
--- a/Koha/AuthorisedValues.pm
+++ b/Koha/AuthorisedValues.pm
@@ -93,7 +93,7 @@ sub find_by_koha_field {
distinct => 1,
}
);
- return $av->count ? $av->next : undef;
+ return $av->next;
}
sub get_description_by_koha_field {
@@ -106,17 +106,14 @@ sub get_description_by_koha_field {
my $memory_cache = Koha::Cache::Memory::Lite->get_instance;
my $cache_key = "AV_descriptions:$frameworkcode:$kohafield:$authorised_value";
- my $cached = $memory_cache->get_from_cache($cache_key);
- return $cached if $cached;
+ my $description = $memory_cache->get_from_cache($cache_key);
- my $av = $self->find_by_koha_field($params);
- if ( ! defined $av ){
- $memory_cache->set_in_cache( $cache_key, {} );
- return {};
+ unless (defined $description) {
+ my $av = $self->find_by_koha_field($params);
+ $description = defined $av ? { lib => $av->lib, opac_description => $av->opac_description } : {};
+ $memory_cache->set_in_cache( $cache_key, $description );
}
- my $descriptions = { lib => $av->lib, opac_description => $av->opac_description };
- $memory_cache->set_in_cache( $cache_key, $descriptions );
- return $descriptions;
+ return $description;
}
sub get_descriptions_by_koha_field {
@@ -126,19 +123,50 @@ sub get_descriptions_by_koha_field {
my $memory_cache = Koha::Cache::Memory::Lite->get_instance;
my $cache_key = "AV_descriptions:$frameworkcode:$kohafield";
- my $cached = $memory_cache->get_from_cache($cache_key);
- return @$cached if $cached;
+ my $descriptions = $memory_cache->get_from_cache($cache_key);
+
+ unless (defined $descriptions) {
+ my @avs = $self->search_by_koha_field($params)->as_list;
+ $descriptions = [
+ map {
+ {
+ authorised_value => $_->authorised_value,
+ lib => $_->lib,
+ opac_description => $_->opac_description
+ }
+ } @avs
+ ];
+ $memory_cache->set_in_cache( $cache_key, $descriptions );
+ }
+ return @{$descriptions};
+}
- my @avs = $self->search_by_koha_field($params)->as_list;
- my @descriptions = map {
- {
- authorised_value => $_->authorised_value,
- lib => $_->lib,
- opac_description => $_->opac_description
- }
- } @avs;
- $memory_cache->set_in_cache( $cache_key, \@descriptions );
- return @descriptions;
+sub get_description_by_category_and_authorised_value {
+ my ( $self, $params ) = @_;
+ return unless defined $params->{category} && defined $params->{authorised_value};
+
+ my $category = $params->{category};
+ my $value = $params->{authorised_value};
+
+ my $memory_cache = Koha::Cache::Memory::Lite->get_instance;
+ my $cache_key = "AV_description_by_category_and_authorised_value:$category:$value";
+ my $description = $memory_cache->get_from_cache($cache_key);
+
+ unless (defined $description) {
+ my $av = $self->search(
+ {
+ category => $category,
+ authorised_value => $value
+ }
+ )->next;
+ $description = defined $av ? {
+ lib => $av->lib,
+ opac_description => $av->opac_description
+ } : {};
+ $memory_cache->set_in_cache( $cache_key, $description );
+ }
+
+ return $description;
}
=head3 get_descriptions_by_marc_field
diff --git a/Koha/Template/Plugin/AuthorisedValues.pm b/Koha/Template/Plugin/AuthorisedValues.pm
index 4e74c1bbad5..f41a8b9e7af 100644
--- a/Koha/Template/Plugin/AuthorisedValues.pm
+++ b/Koha/Template/Plugin/AuthorisedValues.pm
@@ -26,17 +26,24 @@ use Koha::AuthorisedValues;
sub GetByCode {
my ( $self, $category, $code, $opac ) = @_;
- my $av = Koha::AuthorisedValues->search({ category => $category, authorised_value => $code });
- return $av->count
- ? $opac
- ? $av->next->opac_description
- : $av->next->lib
- : $code;
+ my $av = Koha::AuthorisedValues->get_description_by_category_and_authorised_value(
+ {
+ category => $category,
+ authorised_value => $code
+ }
+ );
+
+ my $description;
+ if ($av) {
+ $description = $opac ? $av->{opac_description} : $av->{lib};
+ }
+
+ return $description || $code;
}
sub Get {
- my ( $self, $category, $selected, $opac ) = @_;
- return GetAuthorisedValues( $category, $selected, $opac );
+ my ( $self, $category, $opac ) = @_;
+ return GetAuthorisedValues( $category, $opac );
}
sub GetAuthValueDropbox {
diff --git a/t/db_dependent/AuthorisedValues.t b/t/db_dependent/AuthorisedValues.t
index 02fcbc5ac8d..218c8d29aa4 100755
--- a/t/db_dependent/AuthorisedValues.t
+++ b/t/db_dependent/AuthorisedValues.t
@@ -1,7 +1,7 @@
#!/usr/bin/perl
use Modern::Perl;
-use Test::More tests => 17;
+use Test::More tests => 16;
use Test::Exception;
use Try::Tiny;
@@ -139,11 +139,6 @@ is( @$limits, 2, 'library_limits functions correctly both as setter and getter'
my @categories = Koha::AuthorisedValues->new->categories;
is( @categories, @existing_categories+3, 'There should have 3 categories inserted' );
-is_deeply(
- \@categories,
- [ sort { uc $a cmp uc $b } @categories ],
- 'categories must be ordered by category names'
-);
subtest 'search_by_*_field + find_by_koha_field + get_description + authorised_values' => sub {
plan tests => 7;
--
2.48.1