Attachment 150651 Details for Bug 31856 – Bug 31856: Improve performance of serials subscriptions search

[patch] Bug 31856: Improve performance of serials subscriptions search

Bug-31856-Improve-performance-of-serials-subscript.patch (text/plain), 8.05 KB, created by Jonathan Druart on 2023-05-04 12:09:37 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2023-05-04 12:09:37 UTC

Size: 8.05 KB

patch

obsolete

>From 8ff5bf7c1ae73365e948bc6fe314bf5eaa3094ac 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                            | 16 ++++--
> Koha/AuthorisedValues.pm                 | 73 +++++++++++++++++-------
> Koha/Template/Plugin/AuthorisedValues.pm | 23 +++++---
> t/db_dependent/AuthorisedValues.t        |  8 +--
> 4 files changed, 81 insertions(+), 39 deletions(-)
>
>diff --git a/C4/Serials.pm b/C4/Serials.pm
>index e67881035d6..ed2986865d1 100644
>--- a/C4/Serials.pm
>+++ b/C4/Serials.pm
>@@ -36,6 +36,7 @@ use C4::Biblio qw( GetMarcFromKohaField ModBiblio );
> 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::Serial;
>@@ -617,14 +618,21 @@ sub SearchSubscriptions {
>     $sth->execute(@where_args);
>     my $results =  $sth->fetchall_arrayref( {} );
> 
>+    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_fields} = { map { $_->field->name => $_->value }
>-            $subscription_object->additional_field_values->as_list };
>-
>+        $subscription->{additional_fields} = {
>+            map { $additional_fields_by_id{$_->field_id} => $_->value } @additional_field_values
>+        };
>     }
> 
>     return @$results;
>diff --git a/Koha/AuthorisedValues.pm b/Koha/AuthorisedValues.pm
>index b52c6c79e62..69e8c5bd758 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,14 +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 $av = $self->find_by_koha_field($params);
>-    return {} unless defined $av;
>-    my $descriptions = { lib => $av->lib, opac_description => $av->opac_description };
>-    $memory_cache->set_in_cache( $cache_key, $descriptions );
>-    return $descriptions;
>+    my $description = $memory_cache->get_from_cache($cache_key);
>+
>+    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 );
>+    }
>+    return $description;
> }
> 
> sub get_descriptions_by_koha_field {
>@@ -123,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;
> }
> 
> sub categories {
>diff --git a/Koha/Template/Plugin/AuthorisedValues.pm b/Koha/Template/Plugin/AuthorisedValues.pm
>index 5cb91b83a3c..b121b2629b0 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 a1bf7b48125..91d3367e18f 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 => 16;
>+use Test::More tests => 15;
> use Try::Tiny;
> 
> use t::lib::TestBuilder;
>@@ -136,12 +136,8 @@ my $limits = $av1->library_limits->as_list;
> 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 => 6;
>-- 
>2.25.1

Actions: View | Diff | Splinter Review

Attachments on bug 31856: 142058 | 142156 | 142157 | 150651 | 150652 | 150653 | 151077 | 151078