Fix many_to_many bridges going back to the same table

The loop only looked at the moniker name, not the key columns, so
would pick a random one. The tests didn't cover the actual self-m2m
bridge at all, so this only got noticed by the Oracle loader being
unstable in which one it picked and triggering the no-rewrite test.

Also clean up the other m2m rel tests and group them correctly.

diff --git a/Changes b/Changes
index 13ad39b..8e14492 100644 (file)
--- a/Changes
+++ b/Changes
@@ -1,5 +1,7 @@
 Revision history for Perl extension DBIx::Class::Schema::Loader
 
+        - Fix many_to_many bridges going back to the same table
+
 0.07040  2014-05-27
         - Add options to omit the version and timestamp from the
           generated code (RT#92300)

diff --git a/Makefile.PL b/Makefile.PL
index f6e85f7..db297ef 100644 (file)
--- a/Makefile.PL
+++ b/Makefile.PL
@@ -35,6 +35,7 @@ test_requires 'DBD::SQLite'     => '1.29';
 test_requires 'Test::Exception' => '0.31';
 test_requires 'Test::More'      => '0.94';
 test_requires 'Test::Warn'      => '0.21';
+test_requires 'Test::Deep'      => '0.107';
 test_requires 'Test::Differences' => '0.60';
 
 requires 'Carp::Clan'                  => 0;

diff --git a/lib/DBIx/Class/Schema/Loader/RelBuilder.pm b/lib/DBIx/Class/Schema/Loader/RelBuilder.pm
index e8d1128..b08ab09 100644 (file)
--- a/lib/DBIx/Class/Schema/Loader/RelBuilder.pm
+++ b/lib/DBIx/Class/Schema/Loader/RelBuilder.pm
@@ -9,7 +9,7 @@ use Scalar::Util 'weaken';
 use DBIx::Class::Schema::Loader::Utils qw/split_name slurp_file array_eq/;
 use Try::Tiny;
 use List::Util 'first';
-use List::MoreUtils qw/apply uniq any/;
+use List::MoreUtils qw/apply uniq any all/;
 use namespace::clean;
 use Lingua::EN::Inflect::Phrase ();
 use Lingua::EN::Tagger ();
@@ -526,8 +526,14 @@ sub _generate_m2ms {
 
             $class{class} = $rels[$this]{args}[1];
 
+            my %link_cols = map { $_ => 1 } apply { s/^self\.//i } values %{ $rels[$this]{args}[2] };
+
             $class{link_table_rel} = first {
-                $_->{method} eq 'has_many' && $_->{args}[1] eq $link_class
+                $_->{method} eq 'has_many'
+                    and
+                $_->{args}[1] eq $link_class
+                    and
+                all { $link_cols{$_} } apply { s/^foreign\.//i } keys %{$_->{args}[2]}
             } @{ $all_code->{$class{class}} };
 
             next LINK_CLASS unless $class{link_table_rel};

diff --git a/t/lib/dbixcsl_common_tests.pm b/t/lib/dbixcsl_common_tests.pm
index d9f3485..d43f227 100644 (file)
--- a/t/lib/dbixcsl_common_tests.pm
+++ b/t/lib/dbixcsl_common_tests.pm
@@ -4,6 +4,7 @@ use strict;
 use warnings;
 
 use Test::More;
+use Test::Deep;
 use Test::Exception;
 use Test::Differences;
 use DBIx::Class::Schema::Loader;
@@ -122,7 +123,7 @@ sub run_tests {
     $num_rescans++ if $self->{vendor} eq 'Firebird';
 
     plan tests => @connect_info *
-        (228 + $num_rescans * $col_accessor_map_tests + $extra_count + ($self->{data_type_tests}{test_count} || 0));
+        (225 + $num_rescans * $col_accessor_map_tests + $extra_count + ($self->{data_type_tests}{test_count} || 0));
 
     foreach my $info_idx (0..$#connect_info) {
         my $info = $connect_info[$info_idx];
@@ -923,28 +924,61 @@ qr/\n__PACKAGE__->(?:belongs_to|has_many|might_have|has_one|many_to_many)\(
         isa_ok(try { $rs_rel17->single }, $class17);
         is(try { $rs_rel17->single->id }, 3, "search_related with multiple FKs from same table");
 
-        # XXX test m:m 18 <- 20 -> 19
+        # test many_to_many detection 18 -> 20 -> 19 and 19 -> 20 -> 18
         ok($class20->column_info('parent')->{is_foreign_key}, 'Foreign key detected');
         ok($class20->column_info('child')->{is_foreign_key}, 'Foreign key detected');
 
-        # XXX test double-fk m:m 21 <- 22 -> 21
-        ok($class22->column_info('parent')->{is_foreign_key}, 'Foreign key detected');
-        ok($class22->column_info('child')->{is_foreign_key}, 'Foreign key detected');
-
-        # test many_to_many detection 18 -> 20 -> 19 and 19 -> 20 -> 18
-        my $m2m;
+        cmp_deeply(
+            $class18->_m2m_metadata->{children},
+            superhashof({
+                relation => 'loader_test20s',
+                foreign_relation => 'child',
+                attrs => superhashof({ order_by => 'me.id' })
+            }),
+            'children m2m correct with ordering'
+        );
 
-        ok($m2m = (try { $class18->_m2m_metadata->{children} }), 'many_to_many created');
+        cmp_deeply(
+            $class19->_m2m_metadata->{parents},
+            superhashof({
+                relation => 'loader_test20s',
+                foreign_relation => 'parent',
+                attrs => superhashof({ order_by => 'me.id' })
+            }),
+            'parents m2m correct with ordering'
+        );
 
-        is $m2m->{relation}, 'loader_test20s', 'm2m near rel';
-        is $m2m->{foreign_relation}, 'child', 'm2m far rel';
-        is $m2m->{attrs}->{order_by}, 'me.id', 'm2m bridge attrs';
 
-        ok($m2m = (try { $class19->_m2m_metadata->{parents} }), 'many_to_many created');
+        # test double-fk m:m 21 <- 22 -> 21
+        ok($class22->column_info('parent')->{is_foreign_key}, 'Foreign key detected');
+        ok($class22->column_info('child')->{is_foreign_key}, 'Foreign key detected');
+        is_deeply(
+            $class21->relationship_info("loader_test22_parents")->{cond},
+            { 'foreign.parent' => 'self.id' },
+            'rel to foreign.parent correct'
+        );
+        is_deeply(
+            $class21->relationship_info("loader_test22_children")->{cond},
+            { 'foreign.child' => 'self.id' },
+            'rel to foreign.child correct'
+        );
 
-        is $m2m->{relation}, 'loader_test20s', 'm2m near rel';
-        is $m2m->{foreign_relation}, 'parent', 'm2m far rel';
-        is $m2m->{attrs}->{order_by}, 'me.id', 'm2m bridge attrs';
+        cmp_deeply(
+            $class21->_m2m_metadata,
+            {
+                parents => superhashof({
+                    accessor => 'parents',
+                    relation => 'loader_test22_children',
+                    foreign_relation => 'parent',
+                }),
+                children => superhashof({
+                    accessor => 'children',
+                    relation => 'loader_test22_parents',
+                    foreign_relation => 'child',
+                }),
+            },
+            'self-m2m correct'
+        );
 
         ok( $class37->relationship_info('parent'), 'parents rel created' );
         ok( $class37->relationship_info('child'), 'child rel created' );