Bug 37389: ExtendedAttributes mixin

This is what we're doing here:

- Creating a new mixin called ExtendedAttributes.pm
- Moving the extended_attributes 'join' logic out of REST/Plugin/Query and instead applying it to the aforementioned Mixin. Moving this to this level allows for this consistent behavior to happen on all search queries including, but not limited to, search queries happening on the REST API.
- 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' accessor is generated dynamically if a search with 'prefetch'=>'extended_attributes' AND the extended_attribute.code = 'height' is performed.
-- Rewrites the 'join' entries in the query to have the aliases as above.
-- Rewrites the WHERE conditions to match the above ruleset.

Example:

A DBIX search query as follows:

[
        {
            '-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 <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 3b613fb73f)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit is contained in:
Pedro Amorim 2024-07-25 15:28:37 +00:00 committed by Lucas Gass
parent 44e1714a2e
commit 4593ad7751
5 changed files with 263 additions and 163 deletions

View File

@ -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

View File

@ -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 <http://www.gnu.org/licenses>.
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;

View File

@ -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

View File

@ -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
}
);
$c->dbic_validate_operators( { filtered_params => $filtered_params } );
# Generate the resultset

View File

@ -160,34 +160,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 dbic_validate_operators
$attributes = $c->dbic_validate_operators( { filtered_params => $filtered_params } );
@ -532,131 +504,6 @@ 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;
}
=head3 _validate_operator
=cut