Bugzilla – Attachment 137600 Details for
Bug 31133
TestBuilder fragile on virtual fks
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 31133: Add a way to handle multiple relationships on same column
Bug-31133-Add-a-way-to-handle-multiple-relationshi.patch (text/plain), 5.29 KB, created by
Tomás Cohen Arazi (tcohen)
on 2022-07-12 03:06:01 UTC
(
hide
)
Description:
Bug 31133: Add a way to handle multiple relationships on same column
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2022-07-12 03:06:01 UTC
Size:
5.29 KB
patch
obsolete
>From 5527af1e9d1f55e5fd15f805e9092b1e7c664794 Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >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 <tomascohen@theke.io> >--- > 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<join( '-', @column_names )>. >+ >+=item B<$fk_names>: The result of doing I<join( '-', @fk_names )> 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 <yohann.dufour@biblibre.com> >-- >2.37.0
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 31133
:
137599
|
137600
|
137603
|
137604
|
137634