From: Dagfinn Ilmari Mannsåker Date: Tue, 29 Jul 2014 13:04:08 +0000 (+0100) Subject: Fix many_to_many bridges going back to the same table X-Git-Tag: 0.07041~11 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=239fa6bcab5973d85739273213c1a36e3e5dc7f9;p=dbsrgits%2FDBIx-Class-Schema-Loader.git 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 --- 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 --- 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 --- 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 --- 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' );