From 9b87bf087fffbfa2cba65b16c64b0f82278885d4 Mon Sep 17 00:00:00 2001 From: Pedro Amorim Date: Thu, 25 Jul 2024 15:28:37 +0000 Subject: [PATCH] Bug 37389: ExtendedAttributes mixin This is what we're doing here: - Creating a new mixin called ExtendedAttributes.pm - Applying this Mixin to Patrons and ILL::Requests (we don't apply it to AdditionalFields.pm here yet because no AdditionalFields supporting classes have the extended_attributes accessor yet, I'll tackle this when rebasing 35287) - The aforementioned mixin does the following: -- Generates dynamic accessors for extended_attributes e.g. if there is a borrower attribute with code 'height', the 'extended_attributes_height' is generated dynamically when a search with 'prefetch'=>'extended_attributes' is performed. -- Rewrites the 'join' entries in the query to have the alias as above. -- Rewrites the WHERE conditions to match the above ruleset. Example: A DBIX search query as follow: [ { '-and' => [ [ { 'extended_attributes.attribute' => { 'like' => 'abc%' }, 'extended_attributes.code' => 'CODE_1' } ], [ { 'extended_attributes.code' => 'CODE_2', 'extended_attributes.attribute' => { 'like' => '123%' } } ] ] } ] Results in the following SQL: SELECT `me`.`borrowernumber` FROM `borrowers` `me` LEFT JOIN `borrower_attributes` `extended_attributes_CODE_1` ON ( `extended_attributes_CODE_1`.`borrowernumber` = `me`.`borrowernumber` AND `extended_attributes_CODE_1`.`code` = ? ) LEFT JOIN `borrower_attributes` `extended_attributes_CODE_2` ON ( `extended_attributes_CODE_2`.`borrowernumber` = `me`.`borrowernumber` AND `extended_attributes_CODE_2`.`code` = ? ) WHERE ( ( ( `extended_attributes_CODE_1`.`attribute` LIKE ? AND `extended_attributes_CODE_1`.`code` = ? ) AND ( `extended_attributes_CODE_2`.`attribute` LIKE ? AND `extended_attributes_CODE_2`.`code` = ? ) ) ) What fixes the performance issue that originated this work is the 'AND `extended_attributes_CODE_1`.`code` = ?' that was missing on the LEFT JOIN. All of the above is explained using Borrowers and Borrower attributes, but it all also applies to ILL::Requests and ILL::Request::Attributes. Co-authored-by: Martin Renvoize --- Koha/ILL/Requests.pm | 16 +- Koha/Objects/Mixin/ExtendedAttributes.pm | 234 +++++++++++++++++++++++ Koha/Patrons.pm | 15 +- Koha/REST/Plugin/Objects.pm | 8 - Koha/REST/Plugin/Query.pm | 153 --------------- 5 files changed, 263 insertions(+), 163 deletions(-) create mode 100644 Koha/Objects/Mixin/ExtendedAttributes.pm diff --git a/Koha/ILL/Requests.pm b/Koha/ILL/Requests.pm index 468bd34d82..39ca48d3e9 100644 --- a/Koha/ILL/Requests.pm +++ b/Koha/ILL/Requests.pm @@ -22,8 +22,9 @@ use Modern::Perl; use Koha::Database; use Koha::ILL::Request; use Koha::ILL::Request::Config; +use Koha::Objects::Mixin::ExtendedAttributes; -use base qw(Koha::Objects); +use base qw(Koha::Objects::Mixin::ExtendedAttributes Koha::Objects); =head1 NAME @@ -106,6 +107,19 @@ sub search_incomplete { } ); } +=head3 extended_attributes_config + +=cut + +sub extended_attributes_config { + my ( $self ) = @_; + return { + 'id_field' => { 'foreign' => 'illrequest_id', 'self' => 'illrequest_id' }, + 'key_field' => 'type', + 'schema_class' => 'Koha::Schema::Result::Illrequestattribute', + }; +} + =head2 Internal methods =head3 _type diff --git a/Koha/Objects/Mixin/ExtendedAttributes.pm b/Koha/Objects/Mixin/ExtendedAttributes.pm new file mode 100644 index 0000000000..386d657535 --- /dev/null +++ b/Koha/Objects/Mixin/ExtendedAttributes.pm @@ -0,0 +1,234 @@ +package Koha::Objects::Mixin::ExtendedAttributes; + +# Copyright 2024 PTFS Europe Ltd +# +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Koha is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Koha; if not, see . + +use Modern::Perl; +use Scalar::Util qw( reftype ); + +=head1 NAME + +Koha::Objects::Mixin::ExtendedAttributes + +=head2 Class methods + + +=head3 search + + Overwrites the search method to include extended attributes rewrite and dynamic relation accessors + +=cut + +sub search { + my ( $self, $params, $attributes ) = @_; + + my $class = ref($self) ? ref($self) : $self; + + $self->handle_query_extended_attributes( + { + attributes => $attributes, + filtered_params => $params, + } + ); + + my $rs = $self->_resultset()->search( $params, $attributes ); + return $class->_new_from_dbic($rs); +} + +=head3 handle_query_extended_attributes + + Checks for the presence of extended_attributes in a query + If present, it builds the dynamic extended attributes relations and rewrites the query to include the extended_attributes relation + +=cut + +sub handle_query_extended_attributes { + my ( $self, $args ) = @_; + + my $attributes = $args->{attributes}; + my $filtered_params = $args->{filtered_params}; + + if ( reftype( $attributes->{prefetch} ) + && reftype( $attributes->{prefetch} ) eq 'ARRAY' + && grep ( /extended_attributes/, @{ $attributes->{prefetch} } ) ) + { + + my @array = $self->_get_extended_attributes_entries($filtered_params); + + # Calling our private method to build the extended attributes relations + my @joins = $self->_build_extended_attributes_relations( \@array ); + push @{ $attributes->{join} }, @joins; + + } +} + +=head3 _get_extended_attributes_entries + + $self->_get_extended_attributes_entries( $filtered_params, 0 ) + +Recursive function that returns the rewritten extended_attributes query entries. + +Given: +[ + '-and', + [ + { + 'extended_attributes.code' => 'CODE_1', + 'extended_attributes.attribute' => { 'like' => '%Bar%' } + }, + { + 'extended_attributes.attribute' => { 'like' => '%Bar%' }, + 'extended_attributes.code' => 'CODE_2' + } + ] +]; + +Returns : + +[ + 'CODE_1', + 'CODE_2' +] + +=cut + +sub _get_extended_attributes_entries { + my ( $self, $params, @array ) = @_; + + if ( reftype($params) && reftype($params) eq 'HASH' ) { + + # rewrite additional_field_values table query params + @array = _rewrite_related_metadata_query( $params, 'field_id', 'value', @array ) + if $params->{'extended_attributes.field_id'}; + + # rewrite borrower_attributes table query params + @array = _rewrite_related_metadata_query( $params, 'code', 'attribute', @array ) + if $params->{'extended_attributes.code'}; + + # rewrite illrequestattributes table query params + @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}, @array ); + } + } elsif ( reftype($params) && reftype($params) eq 'ARRAY' ) { + foreach my $ea_instance (@$params) { + @array = $self->_get_extended_attributes_entries( $ea_instance, @array ); + } + return @array; + } else { + return @array; + } +} + +=head3 _rewrite_related_metadata_query + + $extended_attributes_entries = + _rewrite_related_metadata_query( $params, 'field_id', 'value', @array ) + +Helper function that rewrites all subsequent extended_attributes queries to match the alias generated by the dbic self left join +Take the below example (patrons): + [ + { + "extended_attributes.attribute":{"like":"%123%"}, + "extended_attributes.code":"CODE_1" + } + ], + [ + { + "extended_attributes.attribute":{"like":"%abc%" }, + "extended_attributes.code":"CODE_2" + } + ] + +It'll be rewritten as: + [ + { + 'extended_attributes_CODE_1.attribute' => { 'like' => '%123%' }, + 'extended_attributes_CODE_1.code' => 'CODE_1' + } + ], + [ + { + 'extended_attributes_CODE_2.attribute' => { 'like' => '%abc%' }, + 'extended_attributes_CODE_2.code' => 'CODE_2' + } + ] + +=cut + +sub _rewrite_related_metadata_query { + my ( $params, $key, $value, @array ) = @_; + + if ( ref \$params->{ 'extended_attributes.' . $key } eq 'SCALAR' ) { + 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_$old_key_value" . "." . $value; + $params->{$new_value_value} = $old_value_value; + push @array, $old_key_value; + } + + return @array; +} + +=head3 _build_extended_attributes_relations + + Method to dynamically add has_many relations for Koha classes that support extended_attributes. + + Returns a list of relation accessor names. + +=cut + +sub _build_extended_attributes_relations { + my ( $self, $types ) = @_; + + return if( !grep ( /extended_attributes/, keys %{$self->_resultset->result_source->_relationships} ) ); + + my $ea_config = $self->extended_attributes_config; + + my $result_source = $self->_resultset->result_source; + for my $type ( @{$types} ) { + $result_source->add_relationship( + "extended_attributes_$type", + "$ea_config->{schema_class}", + sub { + my $args = shift; + + return { + "$args->{foreign_alias}.$ea_config->{id_field}->{foreign}" => + { -ident => "$args->{self_alias}.$ea_config->{id_field}->{self}" }, + "$args->{foreign_alias}.$ea_config->{key_field}" => { '=', $type }, + }; + }, + { + accessor => 'multi', + join_type => 'LEFT', + cascade_copy => 0, + cascade_delete => 0, + is_depends_on => 0 + }, + ); + + } + return map { 'extended_attributes_' . $_ } @{$types}; +} + +1; \ No newline at end of file diff --git a/Koha/Patrons.pm b/Koha/Patrons.pm index eb89573312..813d741c2a 100644 --- a/Koha/Patrons.pm +++ b/Koha/Patrons.pm @@ -29,7 +29,7 @@ use Koha::Patron; use Koha::Exceptions::Patron; use Koha::Patron::Categories; -use base qw(Koha::Objects); +use base qw(Koha::Objects::Mixin::ExtendedAttributes Koha::Objects); =head1 NAME @@ -566,6 +566,19 @@ sub filter_by_have_permission { ); } +=head3 extended_attributes_config + +=cut + +sub extended_attributes_config { + my ( $self ) = @_; + return { + 'id_field' => { 'foreign' => 'borrowernumber', 'self' => 'borrowernumber' }, + 'key_field' => 'code', + 'schema_class' => 'Koha::Schema::Result::BorrowerAttribute', + }; +} + =head3 _type =cut diff --git a/Koha/REST/Plugin/Objects.pm b/Koha/REST/Plugin/Objects.pm index 7f06582b4d..32dab06d4b 100644 --- a/Koha/REST/Plugin/Objects.pm +++ b/Koha/REST/Plugin/Objects.pm @@ -287,14 +287,6 @@ controller, and thus shouldn't be called twice in it. $c->stash('koha.pagination.base_total' => $result_set->count); $c->stash('koha.pagination.query_params' => $args); - # Check and handle related metadata table joins - $c->dbic_extended_attributes_join( - { - attributes => $attributes, - filtered_params => $filtered_params - } - ); - # Generate the resultset my $objects_rs = $result_set->search( $filtered_params, $attributes ); # Stash the page total if requires, total otherwise diff --git a/Koha/REST/Plugin/Query.pm b/Koha/REST/Plugin/Query.pm index 53decc94bc..4fa3709f7c 100644 --- a/Koha/REST/Plugin/Query.pm +++ b/Koha/REST/Plugin/Query.pm @@ -159,34 +159,6 @@ Generates the DBIC prefetch attribute based on embedded relations, and merges in } ); -=head3 dbic_extended_attributes_join - - $attributes = $c->dbic_extended_attributes_join({ attributes => $attributes, result_set => $result_set }); - -Generates the DBIC join attribute based on extended_attributes query entries, and merges into I<$attributes>. - -=cut - - $app->helper( - 'dbic_extended_attributes_join' => sub { - my ( $c, $args ) = @_; - - my $attributes = $args->{attributes}; - my $filtered_params = $args->{filtered_params}; - - 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--; - } - } - } - ); - =head3 _build_query_params_from_api my $params = _build_query_params_from_api( $filtered_params, $reserved_params ); @@ -516,129 +488,4 @@ sub _parse_dbic_query { } } -=head3 _get_extended_attributes_entries - - $self->_get_extended_attributes_entries( $filtered_params, 0 ) - -Recursive function that returns the number of extended_attributes entries present in a query. -Example: Returns 2 if given a $filtered_params containing the below: - -{ - "-and":[ - [ - { - "extended_attributes.value":{ - "like":"abc%" - }, - "extended_attributes.code":[ - [ - "arbitrary_attr_code", - "another_attr_code" - ] - ] - } - ], - [ - { - "extended_attributes.value":{ - "like":"123%" - }, - "extended_attributes.code":[ - [ - "arbitrary_attr_code", - "another_attr_code" - ] - ] - } - ] - ] -} - -=cut - -sub _get_extended_attributes_entries { - my ( $self, $params, $extended_attributes_entries ) = @_; - - 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' ) - 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' ) - if $params->{'extended_attributes.code'}; - - # rewrite illrequestattributes table query params - $extended_attributes_entries = - _rewrite_related_metadata_query( $params, $extended_attributes_entries, 'type', 'value' ) - if $params->{'extended_attributes.type'}; - - foreach my $key ( keys %{$params} ) { - return $self->_get_extended_attributes_entries( $params->{$key}, $extended_attributes_entries ); - } - } 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 ); - } - return $extended_attributes_entries; - } else { - return $extended_attributes_entries; - } -} - -=head3 _rewrite_related_metadata_query - - $extended_attributes_entries = - _rewrite_related_metadata_query( $params, $extended_attributes_entries, 'field_id', 'value' ) - -Helper function that rewrites all subsequent extended_attributes queries to match the alias generated by the dbic self left join -Take the below example (patrons), assuming it is the third instance of an extended_attributes query: -{ - "extended_attributes.value":{ - "like":"123%" - }, - "extended_attributes.code":[ - [ - "arbitrary_attr_code", - "another_attr_code" - ] - ] -} - -It'll be rewritten as: -{ - "extended_attributes_3.value":{ - "like":"123%" - }, - "extended_attributes_3.code":[ - [ - "arbitrary_attr_code", - "another_attr_code" - ] - ] -} - -=cut - -sub _rewrite_related_metadata_query { - my ( $params, $extended_attributes_entries, $key, $value ) = @_; - - $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_value_value = delete $params->{ 'extended_attributes.' . $value }; - my $new_value_value = "extended_attributes_$extended_attributes_entries" . "." . $value; - $params->{$new_value_value} = $old_value_value; - } - - return $extended_attributes_entries; -} - 1; -- 2.39.2