Bugzilla – Attachment 169149 Details for
Bug 37389
REST API queries joining on extended_attributes may cause severe performance issues
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 37389: More WIP
Bug-37389-More-WIP.patch (text/plain), 12.30 KB, created by
Pedro Amorim
on 2024-07-18 16:23:53 UTC
(
hide
)
Description:
Bug 37389: More WIP
Filename:
MIME Type:
Creator:
Pedro Amorim
Created:
2024-07-18 16:23:53 UTC
Size:
12.30 KB
patch
obsolete
>From 9c736d2864f4f67331b56ee6434ed7a7af5732bd Mon Sep 17 00:00:00 2001 >From: Pedro Amorim <pedro.amorim@ptfs-europe.com> >Date: Thu, 18 Jul 2024 16:08:39 +0000 >Subject: [PATCH] Bug 37389: More WIP > >Hardcoded accessors (for now) >Rewrite the API query builder to dynamically get the extended_attributes from the query > >This patch already fixes the performance issue and keeps the functionality, but we're trying to generate the extended_attributes accessors dynamically instead of hardcoding them >--- > Koha/ILL/Requests.pm | 74 ++++++++++---------- > Koha/REST/Plugin/Query.pm | 62 +++++++---------- > Koha/Schema/Result/Illrequest.pm | 112 +++++++++++++++++++++++++++++++ > 3 files changed, 173 insertions(+), 75 deletions(-) > >diff --git a/Koha/ILL/Requests.pm b/Koha/ILL/Requests.pm >index 289ce73d97..cdf2a9de4a 100644 >--- a/Koha/ILL/Requests.pm >+++ b/Koha/ILL/Requests.pm >@@ -119,43 +119,43 @@ Returns a list of relation accessor names. > > =cut > >-sub _build_extended_attributes_relations { >- my ($self) = @_; >- >- my @distinct_types = Koha::ILL::Request::Attributes->_resultset->search( >- {}, # No specific conditions >- { >- select => ['type'], >- group_by => ['type'] >- } >- )->get_column('type')->all; >- >- for my $type (@distinct_types) { >- my $test = $self->_resultset->_result_class->has_many( >- "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 }, >- }; >- }, >- { cascade_copy => 0, cascade_delete => 0 }, >- ); >- >- my $schema = $self->_resultset->result_source->schema; >- >- $schema->unregister_source('Koha::Schema::Result::Illrequest'); >- $schema->register_class( 'Koha::Schema::Result::Illrequest', $test ); >- >- # Testing to check if the dynamically added method has been aded. It's not been!! >- $self->find(1)->extended_attributes_article_title; >- } >- >- return map { 'extended_attributes_'.$_ } @distinct_types; >-} >+# sub _build_extended_attributes_relations { >+# my ($self) = @_; >+ >+# my @distinct_types = Koha::ILL::Request::Attributes->_resultset->search( >+# {}, # No specific conditions >+# { >+# select => ['type'], >+# group_by => ['type'] >+# } >+# )->get_column('type')->all; >+ >+# for my $type (@distinct_types) { >+# my $test = $self->_resultset->_result_class->has_many( >+# "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 }, >+# }; >+# }, >+# { cascade_copy => 0, cascade_delete => 0 }, >+# ); >+ >+# my $schema = $self->_resultset->result_source->schema; >+ >+# $schema->unregister_source('Koha::Schema::Result::Illrequest'); >+# $schema->register_class( 'Koha::Schema::Result::Illrequest', $test ); >+ >+# # Testing to check if the dynamically added method has been aded. It's not been!! >+# $self->find(1)->extended_attributes_article_title; >+# } >+ >+# return map { 'extended_attributes_'.$_ } @distinct_types; >+# } > > =head3 _type > >diff --git a/Koha/REST/Plugin/Query.pm b/Koha/REST/Plugin/Query.pm >index b193d45812..d99a7cdb2d 100644 >--- a/Koha/REST/Plugin/Query.pm >+++ b/Koha/REST/Plugin/Query.pm >@@ -179,23 +179,11 @@ Generates the DBIC join attribute based on extended_attributes query entries, an > && reftype( $attributes->{prefetch} ) eq 'ARRAY' > && grep ( /extended_attributes/, @{ $attributes->{prefetch} } ) ) > { >- # my $ea_entries = $self->_get_extended_attributes_entries( $filtered_params, 0 ); >+ my @array = $self->_get_extended_attributes_entries( $filtered_params, 0 ); > >- # Calling our private method to build the extended attributes relations >- $result_set->_build_extended_attributes_relations(); >- >- # Testing out the extended attributes join (hardcoded for now) >- my @test = qw( extended_attributes_article_title extended_attributes_issue extended_attributes_volume ); >- foreach my $test (@test) { >- push( @{ $attributes->{join} }, $test ); >+ foreach my $test (@array) { >+ push( @{ $attributes->{join} }, 'extended_attributes_'.$test ); > } >- >- # while ( $ea_entries > 0 ) { >- # push( @{ $attributes->{join} }, 'extended_attributes_title' ); >- # push( @{ $attributes->{join} }, 'extended_attributes_author' ); >- # $ea_entries--; >- # } >- > } > } > ); >@@ -570,36 +558,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; > } > } > >@@ -638,20 +625,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_$key" . "." . $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_$key" . "." . $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; >diff --git a/Koha/Schema/Result/Illrequest.pm b/Koha/Schema/Result/Illrequest.pm >index 98d68f700b..39cb4564fc 100644 >--- a/Koha/Schema/Result/Illrequest.pm >+++ b/Koha/Schema/Result/Illrequest.pm >@@ -408,6 +408,118 @@ __PACKAGE__->has_many( > { cascade_copy => 0, cascade_delete => 0 }, > ); > >+__PACKAGE__->has_many( >+ "extended_attributes_author", >+ "Koha::Schema::Result::Illrequestattribute", >+ sub { >+ my $args = shift; >+ >+ return { >+ "$args->{foreign_alias}.illrequest_id" => { -ident => "$args->{self_alias}.illrequest_id" }, >+ "$args->{foreign_alias}.type" => { '=', 'author' }, >+ }; >+ }, >+ { cascade_copy => 0, cascade_delete => 0 }, >+); >+ >+__PACKAGE__->has_many( >+ "extended_attributes_title", >+ "Koha::Schema::Result::Illrequestattribute", >+ sub { >+ my $args = shift; >+ >+ return { >+ "$args->{foreign_alias}.illrequest_id" => { -ident => "$args->{self_alias}.illrequest_id" }, >+ "$args->{foreign_alias}.type" => { '=', 'title' }, >+ }; >+ }, >+ { cascade_copy => 0, cascade_delete => 0 }, >+); >+ >+__PACKAGE__->has_many( >+ "extended_attributes_article_title", >+ "Koha::Schema::Result::Illrequestattribute", >+ sub { >+ my $args = shift; >+ >+ return { >+ "$args->{foreign_alias}.illrequest_id" => { -ident => "$args->{self_alias}.illrequest_id" }, >+ "$args->{foreign_alias}.type" => { '=', 'article_title' }, >+ }; >+ }, >+ { cascade_copy => 0, cascade_delete => 0 }, >+ ); >+ >+__PACKAGE__->has_many( >+ "extended_attributes_issue", >+ "Koha::Schema::Result::Illrequestattribute", >+ sub { >+ my $args = shift; >+ >+ return { >+ "$args->{foreign_alias}.illrequest_id" => { -ident => "$args->{self_alias}.illrequest_id" }, >+ "$args->{foreign_alias}.type" => { '=', 'issue' }, >+ }; >+ }, >+ { cascade_copy => 0, cascade_delete => 0 }, >+); >+ >+__PACKAGE__->has_many( >+ "extended_attributes_volume", >+ "Koha::Schema::Result::Illrequestattribute", >+ sub { >+ my $args = shift; >+ >+ return { >+ "$args->{foreign_alias}.illrequest_id" => { -ident => "$args->{self_alias}.illrequest_id" }, >+ "$args->{foreign_alias}.type" => { '=', 'volume' }, >+ }; >+ }, >+ { cascade_copy => 0, cascade_delete => 0 }, >+); >+ >+__PACKAGE__->has_many( >+ "extended_attributes_year", >+ "Koha::Schema::Result::Illrequestattribute", >+ sub { >+ my $args = shift; >+ >+ return { >+ "$args->{foreign_alias}.illrequest_id" => { -ident => "$args->{self_alias}.illrequest_id" }, >+ "$args->{foreign_alias}.type" => { '=', 'year' }, >+ }; >+ }, >+ { cascade_copy => 0, cascade_delete => 0 }, >+); >+ >+__PACKAGE__->has_many( >+ "extended_attributes_pages", >+ "Koha::Schema::Result::Illrequestattribute", >+ sub { >+ my $args = shift; >+ >+ return { >+ "$args->{foreign_alias}.illrequest_id" => { -ident => "$args->{self_alias}.illrequest_id" }, >+ "$args->{foreign_alias}.type" => { '=', 'pages' }, >+ }; >+ }, >+ { cascade_copy => 0, cascade_delete => 0 }, >+); >+ >+__PACKAGE__->has_many( >+ "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' }, >+ }; >+ }, >+ { cascade_copy => 0, cascade_delete => 0 }, >+); >+ > __PACKAGE__->belongs_to( > "ill_batch", > "Koha::Schema::Result::Illbatch", >-- >2.39.2
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 37389
:
169130
|
169133
|
169140
|
169149
|
169159
|
169178
|
169185
|
169189
|
169191
|
169192
|
169234
|
169243
|
169244
|
169245
|
169246
|
169317
|
169318
|
169569
|
169570
|
169583
|
169584
|
169585
|
169586
|
169850
|
169851
|
169944
|
169945
|
169946
|
169954
|
169955
|
169956