From: Stephan Szabo Date: Tue, 28 Mar 2006 18:28:37 +0000 (+0000) Subject: Changed logic for determining foreign key constraints X-Git-Tag: v0.06001~21 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=637ca936ff3aeee87af89aaaf75e2035c29576f8;p=dbsrgits%2FDBIx-Class.git Changed logic for determining foreign key constraints in SQL::Translator::Parser::DBIx::Class to compare self keys against the primary key. Made SQL::Translator::Parser::DBIx::Class handle multi-column foreign key constraints. Added tests on helperrels for these. --- diff --git a/lib/DBIx/Class.pm b/lib/DBIx/Class.pm index 7df5165..64bec50 100644 --- a/lib/DBIx/Class.pm +++ b/lib/DBIx/Class.pm @@ -239,6 +239,8 @@ konobi: Scott McWhirter scotty: Scotty Allen +sszabo: Stephan Szabo + Todd Lipcon wdh: Will Hawes diff --git a/lib/SQL/Translator/Parser/DBIx/Class.pm b/lib/SQL/Translator/Parser/DBIx/Class.pm index b638fd0..73c0e80 100644 --- a/lib/SQL/Translator/Parser/DBIx/Class.pm +++ b/lib/SQL/Translator/Parser/DBIx/Class.pm @@ -77,35 +77,67 @@ sub parse { foreach my $rel (@rels) { my $rel_info = $source->relationship_info($rel); - next if(!exists $rel_info->{attrs}{accessor} || - $rel_info->{attrs}{accessor} eq 'multi'); - # Going by the accessor type isn't such a good idea (yes, I know - # I suggested it). I think the best way to tell if something is a - # foreign key constraint is to assume if it doesn't include our - # primaries then it is (dumb but it'll do). Ignore any rel cond - # that isn't a straight hash, but get both sets of keys in full - # so you don't barf on multi-primaries. Oh, and a dog-simple - # deploy method to chuck the results of this exercise at a db - # for testing is - # $schema->storage->dbh->do($_) for split(";\n", $sql); - # -- mst (03:42 local time, please excuse any mistakes) + my $rel_table = $source->related_source($rel)->name; - my $cond = (keys (%{$rel_info->{cond}}))[0]; - my ($refkey) = $cond =~ /^\w+\.(\w+)$/; - my ($key) = $rel_info->{cond}->{$cond} =~ /^\w+\.(\w+)$/; - if($rel_table && $refkey) + + # Ignore any rel cond that isn't a straight hash + next unless ref $rel_info->{cond} eq 'HASH'; + + # Get the key information, mapping off the foreign/self markers + my @cond = keys(%{$rel_info->{cond}}); + my @refkeys = map {/^\w+\.(\w+)$/} @cond; + my @keys = map {$rel_info->{cond}->{$_} =~ /^\w+\.(\w+)$/} @cond; + + if($rel_table) { - $table->add_constraint( - type => 'foreign_key', - name => "fk_${key}", - fields => $key, - reference_fields => $refkey, - reference_table => $rel_table, - ); + + #Decide if this is a foreign key based on whether the self + #items are our primary columns. + + # Make sure every self key is in the primary key list + my $found; + foreach my $key (@keys) { + $found = 0; + foreach my $prim ($source->primary_columns) { + if ($prim eq $key) { + $found = 1; + last; + } + } + last unless $found; + } + + # Make sure every primary key column is in the self keys + if ($found) { + foreach my $prim ($source->primary_columns) { + $found = 0; + foreach my $key (@keys) { + if ($prim eq $key) { + $found = 1; + last; + } + } + last unless $found; + } + } + + # if $found then the two sets are equal. + + # If the sets are different, then we assume it's a foreign key from + # us to another table. + if (!$found) { + $table->add_constraint( + type => 'foreign_key', + name => "fk_$keys[0]", + fields => \@keys, + reference_fields => \@refkeys, + reference_table => $rel_table, + ); + } } } } - + return 1; } 1; diff --git a/t/helperrels/26sqlt.t b/t/helperrels/26sqlt.t new file mode 100644 index 0000000..4ea58ec --- /dev/null +++ b/t/helperrels/26sqlt.t @@ -0,0 +1,147 @@ +use Test::More; +use lib qw(t/lib); +use DBICTest; +use DBICTest::HelperRels; + +eval "use SQL::Translator"; +plan skip_all => 'SQL::Translator required' if $@; + +my $schema = DBICTest::Schema; + +plan tests => 27; + +my $translator = SQL::Translator->new( + parser_args => { + 'DBIx::Schema' => $schema, + }, + producer_args => { + }, +); + +$translator->parser('SQL::Translator::Parser::DBIx::Class'); +$translator->producer('SQLite'); + +my $output = $translator->translate(); + +my @constraints = + ( + {'display' => 'twokeys->cd', + 'selftable' => 'twokeys', 'foreigntable' => 'cd', + 'selfcols' => ['cd'], 'foreigncols' => ['cdid'], + 'needed' => 1, on_delete => '', on_update => ''}, + {'display' => 'twokeys->artist', + 'selftable' => 'twokeys', 'foreigntable' => 'artist', + 'selfcols' => ['artist'], 'foreigncols' => ['artistid'], + 'needed' => 1, on_delete => '', on_update => ''}, + {'display' => 'cd_to_producer->cd', + 'selftable' => 'cd_to_producer', 'foreigntable' => 'cd', + 'selfcols' => ['cd'], 'foreigncols' => ['cdid'], + 'needed' => 1, on_delete => '', on_update => ''}, + {'display' => 'cd_to_producer->producer', + 'selftable' => 'cd_to_producer', 'foreigntable' => 'producer', + 'selfcols' => ['producer'], 'foreigncols' => ['producerid'], + 'needed' => 1, on_delete => '', on_update => ''}, + {'display' => 'self_ref_alias -> self_ref for self_ref', + 'selftable' => 'self_ref_alias', 'foreigntable' => 'self_ref', + 'selfcols' => ['self_ref'], 'foreigncols' => ['id'], + 'needed' => 1, on_delete => '', on_update => ''}, + {'display' => 'self_ref_alias -> self_ref for alias', + 'selftable' => 'self_ref_alias', 'foreigntable' => 'self_ref', + 'selfcols' => ['alias'], 'foreigncols' => ['id'], + 'needed' => 1, on_delete => '', on_update => ''}, + {'display' => 'cd -> artist', + 'selftable' => 'cd', 'foreigntable' => 'artist', + 'selfcols' => ['artist'], 'foreigncols' => ['artistid'], + 'needed' => 1, on_delete => '', on_update => ''}, + {'display' => 'artist_undirected_map -> artist for id1', + 'selftable' => 'artist_undirected_map', 'foreigntable' => 'artist', + 'selfcols' => ['id1'], 'foreigncols' => ['artistid'], + 'needed' => 1, on_delete => '', on_update => ''}, + {'display' => 'artist_undirected_map -> artist for id2', + 'selftable' => 'artist_undirected_map', 'foreigntable' => 'artist', + 'selfcols' => ['id2'], 'foreigncols' => ['artistid'], + 'needed' => 1, on_delete => '', on_update => ''}, + {'display' => 'track->cd', + 'selftable' => 'track', 'foreigntable' => 'cd', + 'selfcols' => ['cd'], 'foreigncols' => ['cdid'], + 'needed' => 2, on_delete => '', on_update => ''}, + {'display' => 'treelike -> treelike for parent', + 'selftable' => 'treelike', 'foreigntable' => 'treelike', + 'selfcols' => ['parent'], 'foreigncols' => ['id'], + 'needed' => 1, on_delete => '', on_update => ''}, + {'display' => 'twokeytreelike -> twokeytreelike for parent1,parent2', + 'selftable' => 'twokeytreelike', 'foreigntable' => 'twokeytreelike', + 'selfcols' => ['parent1', 'parent2'], 'foreigncols' => ['id1','id2'], + 'needed' => 1, on_delete => '', on_update => ''}, + {'display' => 'tags -> cd', + 'selftable' => 'tags', 'foreigntable' => 'cd', + 'selfcols' => ['cd'], 'foreigncols' => ['cdid'], + 'needed' => 1, on_delete => '', on_update => ''}, + ); + +my $tschema = $translator->schema(); +for my $table ($tschema->get_tables) { + my $table_name = $table->name; + for my $c ( $table->get_constraints ) { + next unless $c->type eq 'FOREIGN KEY'; + + ok(check($table_name, scalar $c->fields, + $c->reference_table, scalar $c->reference_fields, + $c->on_delete, $c->on_update), "Constraint on $table_name matches an expected constraint"); + } +} + +my $i; +for ($i = 0; $i <= $#constraints; ++$i) { + ok(!$constraints[$i]->{'needed'}, "Constraint $constraints[$i]->{display}"); +} + +sub check { + my ($selftable, $selfcol, $foreigntable, $foreigncol, $ondel, $onupd) = @_; + + $ondel = '' if (!defined($ondel)); + $onupd = '' if (!defined($onupd)); + + my $i; + for ($i = 0; $i <= $#constraints; ++$i) { + if ($selftable eq $constraints[$i]->{'selftable'} && + $foreigntable eq $constraints[$i]->{'foreigntable'} && + ($ondel eq $constraints[$i]->{on_delete}) && + ($onupd eq $constraints[$i]->{on_update})) { + # check columns + + my $found = 0; + for (my $j = 0; $j <= $#$selfcol; ++$j) { + $found = 0; + for (my $k = 0; $k <= $#{$constraints[$i]->{'selfcols'}}; ++$k) { + if ($selfcol->[$j] eq $constraints[$i]->{'selfcols'}->[$k] && + $foreigncol->[$j] eq $constraints[$i]->{'foreigncols'}->[$k]) { + $found = 1; + last; + } + } + last unless $found; + } + + if ($found) { + for (my $j = 0; $j <= $#{$constraints[$i]->{'selfcols'}}; ++$j) { + $found = 0; + for (my $k = 0; $k <= $#$selfcol; ++$k) { + if ($selfcol->[$k] eq $constraints[$i]->{'selfcols'}->[$j] && + $foreigncol->[$k] eq $constraints[$i]->{'foreigncols'}->[$j]) { + $found = 1; + last; + } + } + last unless $found; + } + } + + if ($found) { + --$constraints[$i]->{needed}; + return 1; + } + } + } + return 0; +} diff --git a/t/lib/DBICTest/Schema.pm b/t/lib/DBICTest/Schema.pm index f2ee2d7..fbf5383 100644 --- a/t/lib/DBICTest/Schema.pm +++ b/t/lib/DBICTest/Schema.pm @@ -27,7 +27,7 @@ __PACKAGE__->load_classes(qw/ 'Producer', 'CD_to_Producer', ), - qw/SelfRefAlias TreeLike/ + qw/SelfRefAlias TreeLike TwoKeyTreeLike/ ); 1; diff --git a/t/lib/DBICTest/Schema/TwoKeyTreeLike.pm b/t/lib/DBICTest/Schema/TwoKeyTreeLike.pm new file mode 100644 index 0000000..9547baf --- /dev/null +++ b/t/lib/DBICTest/Schema/TwoKeyTreeLike.pm @@ -0,0 +1,22 @@ +package # hide from PAUSE + DBICTest::Schema::TwoKeyTreeLike; + +use base qw/DBIx::Class/; + +__PACKAGE__->load_components(qw/Core/); + +__PACKAGE__->table('twokeytreelike'); +__PACKAGE__->add_columns( + 'id1' => { data_type => 'integer' }, + 'id2' => { data_type => 'integer' }, + 'parent1' => { data_type => 'integer' }, + 'parent2' => { data_type => 'integer' }, + 'name' => { data_type => 'varchar', + size => 100, + }, +); +__PACKAGE__->set_primary_key(qw/id1 id2/); +__PACKAGE__->belongs_to('parent', 'TwoKeyTreeLike', + { 'foreign.id1' => 'self.parent1', 'foreign.id2' => 'self.parent2'}); + +1;