From 5aa6ad9da9f73b387dd3e89b9b0571c0d8acb907 Mon Sep 17 00:00:00 2001
From: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Date: Thu, 18 Jul 2024 13:04:22 +0100
Subject: [PATCH] Bug 37389: Fix performance problems with extended attributes
 on the API

This patch adds a number of methods that work together to result the
problem as a whole.

We add a builder method to dynamically create DBIx::Class relations when
required for the Koha::ILL::Requests resultset.

We update Koha::REST::Plugin::Query to call the aforementioned builder
method so we can subsequently use these new relations to create a
performance join condition. (This also required passing the ResultSet
into the dbic_extended_attributes_join helper from
Koha::REST::Plugin::Objects)

We that add an AUTOLOAD into Koha::ILL::Request to allow use of the
new dynamically added DBIx::Class relations in the above as
extended_attributes_$type from the corresponding Koha::Object.

Finally, we reworked the query builder code to ensure we're using the
new relations added when using the API.

Co-authored-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
---
 Koha/ILL/Request.pm         | 36 ++++++++++++++++++++++++
 Koha/ILL/Requests.pm        | 40 ++++++++++++++++++++++++++
 Koha/REST/Plugin/Objects.pm |  3 +-
 Koha/REST/Plugin/Query.pm   | 56 ++++++++++++++++++-------------------
 4 files changed, 106 insertions(+), 29 deletions(-)

diff --git a/Koha/ILL/Request.pm b/Koha/ILL/Request.pm
index e7347e5ee32..d8b669fe95a 100644
--- a/Koha/ILL/Request.pm
+++ b/Koha/ILL/Request.pm
@@ -2220,6 +2220,42 @@ sub get_staff_table_actions {
     return $ill_table_actions;
 }
 
+=head3 AUTOLOAD
+
+=cut
+
+our $AUTOLOAD;
+
+sub AUTOLOAD {
+    my ($self) = @_;
+
+    my $name = $AUTOLOAD;
+    $name =~ s/.*:://;    # Remove package name
+
+    if ( $name =~ /^extended_attributes_(\w+)$/ ) {
+        my $type = $1;
+
+        # Define the method dynamically
+        no strict 'refs';
+        *{$AUTOLOAD} = sub {
+            my ($self)   = @_;
+            my $relation = 'extended_attributes_' . $type;
+            my $rs       = $self->_result->$relation;
+            return Koha::ILL::Request::Attributes->_new_from_dbic($rs)->search;
+        };
+
+        # Call the newly defined method
+        return $self->$name();
+    }
+    my $wt = 'SUPER::'.$name;
+    return $self->$wt;
+}
+
+sub get_column(){
+    my ($self, $column_name) = @_;
+    return $self->_result->get_column($column_name);
+}
+
 =head3 _type
 
 =cut
diff --git a/Koha/ILL/Requests.pm b/Koha/ILL/Requests.pm
index 468bd34d825..46fbc9d9ce3 100644
--- a/Koha/ILL/Requests.pm
+++ b/Koha/ILL/Requests.pm
@@ -108,6 +108,46 @@ sub search_incomplete {
 
 =head2 Internal methods
 
+
+=head3 _build_extended_attributes_relations
+
+Method to dynamically add has_many relations for each ILLRequestAttribute type stored in the ILLRequestAttributes table.
+
+Used in the API to allow for advanced joins.
+
+Returns a list of relation accessor names.
+
+=cut
+
+sub _build_extended_attributes_relations {
+    my ( $self, $types ) = @_;
+
+    my $result_source = $self->_resultset->result_source;
+    for my $type ( @{$types} ) {
+        $result_source->add_relationship(
+            "extended_attributes_$type",
+            "Koha::Schema::Result::Illrequestattribute",
+            sub {
+                my $args = shift;
+
+                return {
+                    "$args->{foreign_alias}.illrequest_id" => { -ident => "$args->{self_alias}.illrequest_id" },
+                    "$args->{foreign_alias}.type"          => { '=', $type },
+                };
+            },
+            {
+                accessor       => 'multi',
+                join_type      => 'LEFT',
+                cascade_copy   => 0,
+                cascade_delete => 0,
+                is_depends_on  => 0
+            },
+        );
+
+    }
+    return map { 'extended_attributes_' . $_ } @{$types};
+}
+
 =head3 _type
 
 =cut
diff --git a/Koha/REST/Plugin/Objects.pm b/Koha/REST/Plugin/Objects.pm
index 7f06582b4dd..d14d8c99a4a 100644
--- a/Koha/REST/Plugin/Objects.pm
+++ b/Koha/REST/Plugin/Objects.pm
@@ -291,7 +291,8 @@ controller, and thus shouldn't be called twice in it.
             $c->dbic_extended_attributes_join(
                 {
                     attributes      => $attributes,
-                    filtered_params => $filtered_params
+                    filtered_params => $filtered_params,
+                    result_set      => $result_set
                 }
             );
 
diff --git a/Koha/REST/Plugin/Query.pm b/Koha/REST/Plugin/Query.pm
index 53decc94bcf..fd9dcf04822 100644
--- a/Koha/REST/Plugin/Query.pm
+++ b/Koha/REST/Plugin/Query.pm
@@ -173,16 +173,18 @@ Generates the DBIC join attribute based on extended_attributes query entries, an
 
             my $attributes      = $args->{attributes};
             my $filtered_params = $args->{filtered_params};
+            my $result_set      = $args->{result_set};
 
             if (   reftype( $attributes->{prefetch} )
                 && reftype( $attributes->{prefetch} ) eq 'ARRAY'
                 && grep ( /extended_attributes/, @{ $attributes->{prefetch} } ) )
             {
-                my $ea_entries = $self->_get_extended_attributes_entries( $filtered_params, 0 );
-                while ( $ea_entries > 0 ) {
-                    push( @{ $attributes->{join} }, 'extended_attributes' );
-                    $ea_entries--;
-                }
+                my @array = $self->_get_extended_attributes_entries( $filtered_params, 0 );
+
+                # Calling our private method to build the extended attributes relations
+                my @joins = $result_set->_build_extended_attributes_relations(\@array);
+                push @{ $attributes->{join} }, @joins;
+
             }
         }
     );
@@ -557,36 +559,35 @@ Example: Returns 2 if given a $filtered_params containing the below:
 =cut
 
 sub _get_extended_attributes_entries {
-    my ( $self, $params, $extended_attributes_entries ) = @_;
+    my ( $self, $params, $extended_attributes_entries, @array ) = @_;
 
     if ( reftype($params) && reftype($params) eq 'HASH' ) {
 
         # rewrite additional_field_values table query params
-        $extended_attributes_entries =
-            _rewrite_related_metadata_query( $params, $extended_attributes_entries, 'field_id', 'value' )
+        @array =
+            _rewrite_related_metadata_query( $params, 'field_id', 'value', @array )
             if $params->{'extended_attributes.field_id'};
 
         # rewrite borrower_attributes table query params
-        $extended_attributes_entries =
-            _rewrite_related_metadata_query( $params, $extended_attributes_entries, 'code', 'attribute' )
+        @array =
+            _rewrite_related_metadata_query( $params, 'code', 'attribute', @array )
             if $params->{'extended_attributes.code'};
 
         # rewrite illrequestattributes table query params
-        $extended_attributes_entries =
-            _rewrite_related_metadata_query( $params, $extended_attributes_entries, 'type', 'value' )
+        @array =
+            _rewrite_related_metadata_query( $params, 'type', 'value', @array )
             if $params->{'extended_attributes.type'};
 
         foreach my $key ( keys %{$params} ) {
-            return $self->_get_extended_attributes_entries( $params->{$key}, $extended_attributes_entries );
+            return $self->_get_extended_attributes_entries( $params->{$key}, $extended_attributes_entries, @array );
         }
     } elsif ( reftype($params) && reftype($params) eq 'ARRAY' ) {
         foreach my $ea_instance (@$params) {
-            $extended_attributes_entries =
-                +$self->_get_extended_attributes_entries( $ea_instance, $extended_attributes_entries );
+             @array = $self->_get_extended_attributes_entries( $ea_instance, $extended_attributes_entries, @array );
         }
-        return $extended_attributes_entries;
+        return @array;
     } else {
-        return $extended_attributes_entries;
+        return @array;
     }
 }
 
@@ -625,20 +626,19 @@ It'll be rewritten as:
 =cut
 
 sub _rewrite_related_metadata_query {
-    my ( $params, $extended_attributes_entries, $key, $value ) = @_;
+    my ( $params, $key, $value, @array ) = @_;
 
-    $extended_attributes_entries++;
-    if ( $extended_attributes_entries > 1 ) {
-        my $old_key_value = delete $params->{ 'extended_attributes.' . $key };
-        my $new_key_value = "extended_attributes_$extended_attributes_entries" . "." . $key;
-        $params->{$new_key_value} = $old_key_value;
+    my $old_key_value = delete $params->{ 'extended_attributes.' . $key };
+    my $new_key_value = "extended_attributes_$old_key_value" . "." . $key;
+    $params->{$new_key_value} = $old_key_value;
 
-        my $old_value_value = delete $params->{ 'extended_attributes.' . $value };
-        my $new_value_value = "extended_attributes_$extended_attributes_entries" . "." . $value;
-        $params->{$new_value_value} = $old_value_value;
-    }
+    my $old_value_value = delete $params->{ 'extended_attributes.' . $value };
+    my $new_value_value = "extended_attributes_$old_key_value" . "." . $value;
+    $params->{$new_value_value} = $old_value_value;
+
+    push @array, $old_key_value;
 
-    return $extended_attributes_entries;
+    return @array;
 }
 
 1;
-- 
2.45.2