From 5527af1e9d1f55e5fd15f805e9092b1e7c664794 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Mon, 11 Jul 2022 18:54:45 -0300 Subject: [PATCH] Bug 31133: Add a way to handle multiple relationships on same column This patch introduces a syntax for TestBuilder so we can specify which is the preferred relationship in situations like Koha::Checkouts::Renewals that have more than one virtual FK [1] for the same column. In this case, there are the following relationships: * checkouts * old_checkouts both of which link the *checkout_id* column to the *issue_id* column of the **issues** and **old_issues** tables respectively. It does so by defining a mapping between the (concatenated, maybe multiple) related source column and foreign counterpart. The mapping structure is explained in the POD. To test: 1. Apply the regression tests 2. Run the tests a couple times: $ kshell k$ prove t/db_dependent/TestBuilder_multi_vfk.t => FAIL: Tests fail often, randomly! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! No random failures! 5. Bonus, run: $ kshell k$ prove t/db_dependent/TestBuilder.t => FAIL: It fails consistently about Koha::Biblio::ItemGroup [1] i.e. defined at DBIC level but not present on the DB structure. DISCLAIMER: This is not a complete solution as it still requires a way to override the hardcoded default. This would require a special syntax for build() and build_object() which I prefer to delay until we really need it. Signed-off-by: Tomas Cohen Arazi --- t/lib/TestBuilder.pm | 63 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 56 insertions(+), 7 deletions(-) diff --git a/t/lib/TestBuilder.pm b/t/lib/TestBuilder.pm index 94ffa97510..e51fe9d860 100644 --- a/t/lib/TestBuilder.pm +++ b/t/lib/TestBuilder.pm @@ -123,6 +123,7 @@ sub build { # loop thru all fk and create linked records if needed # fills remaining entries in $col_values my $foreign_keys = $self->_getForeignKeys( { source => $source } ); + for my $fk ( @$foreign_keys ) { # skip when FK points to itself: e.g. borrowers:guarantorid next if $fk->{source} eq $source; @@ -337,22 +338,36 @@ sub _getForeignKeys { for my $rel_name( @relationships ) { my $rel_info = $source->relationship_info($rel_name); if( $rel_info->{attrs}->{is_foreign_key_constraint} ) { + + my $column_to_fk = {}; + $rel_info->{source} =~ s/^.*:://g; my $rel = { source => $rel_info->{source} }; - my @keys; - while( my ($col_fk_name, $col_name) = each(%{$rel_info->{cond}}) ) { + # FIXME: This is not safe if we start adding + # more complex conditions at DBIC level + while ( my ( $col_fk_name, $col_name ) = each( %{ $rel_info->{cond} } ) ) { $col_name =~ s|self.(\w+)|$1|; $col_fk_name =~ s|foreign.(\w+)|$1|; - push @keys, { - col_name => $col_name, + push @keys, + { col_name => $col_name, col_fk_name => $col_fk_name, - }; + }; + $column_to_fk->{$col_name} = $col_fk_name; } + + my $col_names = join( '-', sort keys %{ $column_to_fk } ); + my $fk_names = join( '-', map { $column_to_fk->{$_} } sort keys %{ $column_to_fk } ); + + # In case more than one relationship has the same self.id and foreign.id, + # but pointing to different sources (e.g. biblio vs deletedbiblio) + my $default_rel = $self->_default_virtual_fk_relation( $params->{source}, $col_names, $fk_names ); + # skip if there's a defined default relationship and it is not this one + next if $default_rel and $default_rel ne $rel_name; + # check if the combination table and keys is unique # so skip double belongs_to relations (as in Biblioitem) - my $tag = $rel->{source}. ':'. - join ',', sort map { $_->{col_name} } @keys; + my $tag = $rel->{source} . ':' . $col_names; next if $check_dupl->{$tag}; $check_dupl->{$tag} = 1; $rel->{keys} = \@keys; @@ -695,6 +710,40 @@ returns the corresponding Koha::Object. my $patron = $builder->build_object({ class => 'Koha::Patrons' [, value => { ... }] }); +=head1 Internal methods + +=head2 _default_virtual_fk_relation + + my $default_rel = $self->_default_virtual_fk_relation( $source, $col_names, $fk_names ); + +Internal method that maps the parameters to a preferred relationship name. + +Parameters + +=over 4 + +=item B<$source>: A DBIC schema class name (as returned by _type). + +=item B<$col_names>: The result of doing I. + +=item B<$fk_names>: The result of doing I for the foreign keys that are mapped from the I<@column_names> in the exact same order. + +=back + +=cut + +sub _default_virtual_fk_relation { + my ($self, $source, $col_names, $fk_names) = @_; + + my $default = { + CheckoutRenewal => { + checkout_id => { issue_id => 'checkout' }, + }, + }; + + return $default->{$source}->{$col_names}->{$fk_names}; +} + =head1 AUTHOR Yohann Dufour -- 2.37.0