Changed logic for determining foreign key constraints
Stephan Szabo [Tue, 28 Mar 2006 18:28:37 +0000 (18:28 +0000)]
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.

lib/DBIx/Class.pm
lib/SQL/Translator/Parser/DBIx/Class.pm
t/helperrels/26sqlt.t [new file with mode: 0644]
t/lib/DBICTest/Schema.pm
t/lib/DBICTest/Schema/TwoKeyTreeLike.pm [new file with mode: 0644]

index 7df5165..64bec50 100644 (file)
@@ -239,6 +239,8 @@ konobi: Scott McWhirter
 
 scotty: Scotty Allen <scotty@scottyallen.com>
 
+sszabo: Stephan Szabo <sszabo@bigpanda.com>
+
 Todd Lipcon
 
 wdh: Will Hawes
index b638fd0..73c0e80 100644 (file)
@@ -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 (file)
index 0000000..4ea58ec
--- /dev/null
@@ -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;
+}
index f2ee2d7..fbf5383 100644 (file)
@@ -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 (file)
index 0000000..9547baf
--- /dev/null
@@ -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;