From 5aa6ad9da9f73b387dd3e89b9b0571c0d8acb907 Mon Sep 17 00:00:00 2001 From: Pedro Amorim 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 --- 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