Keep belongs_to related object / fk values in sync

diff --git a/Changes b/Changes
index 33f24a5..09bfebb 100644 (file)
--- a/Changes
+++ b/Changes
@@ -29,6 +29,8 @@ Revision history for DBIx::Class
         - Properly quote table name on INSERT with no values
         - Work around possible Storage destruction warnings
         - Fix count of grouped resultsets using HAVING with aliases
+        - Setting belongs_to columns/relationships no longer leaves the
+          FK value and related object out of sync
 
     * Misc
         - Switch all serialization to use Storable::nfreeze for portable

diff --git a/lib/DBIx/Class/Relationship/BelongsTo.pm b/lib/DBIx/Class/Relationship/BelongsTo.pm
index 0f19bd2..76ffb50 100644 (file)
--- a/lib/DBIx/Class/Relationship/BelongsTo.pm
+++ b/lib/DBIx/Class/Relationship/BelongsTo.pm
@@ -43,11 +43,8 @@ sub belongs_to {
       "$fk is not a column of $class"
     ) unless $class->has_column($fk);
 
-    my $acc_type = $class->has_column($rel) ? 'filter' : 'single';
-    $class->add_relationship($rel, $f_class,
-      { "foreign.${pri}" => "self.${fk}" },
-      { accessor => $acc_type, %{$attrs || {}} }
-    );
+    $cond = { "foreign.${pri}" => "self.${fk}" };
+
   }
   # explicit join condition
   elsif (ref $cond) {
@@ -62,22 +59,37 @@ sub belongs_to {
       }
       $cond = $cond_rel;
     }
-    my $acc_type = ((ref $cond eq 'HASH')
-                       && keys %$cond == 1
-                       && $class->has_column($rel))
-                     ? 'filter'
-                     : 'single';
-    $class->add_relationship($rel, $f_class,
-      $cond,
-      { accessor => $acc_type, %{$attrs || {}} }
-    );
   }
+  # dunno
   else {
     $class->throw_exception(
       'third argument for belongs_to must be undef, a column name, '.
       'or a join condition'
     );
   }
+
+  my $acc_type = (
+    ref $cond eq 'HASH'
+      and
+    keys %$cond == 1
+      and
+    $class->has_column($rel)
+  ) ? 'filter' : 'single';
+
+  my $fk_columns = ($acc_type eq 'single' and ref $cond eq 'HASH')
+    ? { map { $_ =~ /^self\.(.+)/ ? ( $1 => 1 ) : () } (values %$cond ) }
+    : undef
+  ;
+
+  $class->add_relationship($rel, $f_class,
+    $cond,
+    {
+      accessor => $acc_type,
+      $fk_columns ? ( fk_columns => $fk_columns ) : (),
+      %{$attrs || {}}
+    }
+  );
+
   return 1;
 }
 

diff --git a/lib/DBIx/Class/Row.pm b/lib/DBIx/Class/Row.pm
index b8c4da7..19364ca 100644 (file)
--- a/lib/DBIx/Class/Row.pm
+++ b/lib/DBIx/Class/Row.pm
@@ -880,11 +880,31 @@ sub set_column {
       : 1
   ;
 
-  # FIXME sadly the update code just checks for keys, not for their value
-  $self->{_dirty_columns}{$column} = 1 if $dirty;
-
-  # XXX clear out the relation cache for this column
-  delete $self->{related_resultsets}{$column};
+  if ($dirty) {
+    # FIXME sadly the update code just checks for keys, not for their value
+    $self->{_dirty_columns}{$column} = 1;
+
+    # Clear out the relation/inflation cache related to this column
+    #
+    # FIXME - this is a quick *largely incorrect* hack, pending a more
+    # serious rework during the merge of single and filter rels
+    my $rels = $self->result_source->{_relationships};
+    for my $rel (keys %$rels) {
+
+      my $acc = $rels->{$rel}{attrs}{accessor} || '';
+
+      if ( $acc eq 'single' and $rels->{$rel}{attrs}{fk_columns}{$column} ) {
+        delete $self->{related_resultsets}{$rel};
+        delete $self->{_relationship_data}{$rel};
+        #delete $self->{_inflated_column}{$rel};
+      }
+      elsif ( $acc eq 'filter' and $rel eq $column) {
+        delete $self->{related_resultsets}{$rel};
+        #delete $self->{_relationship_data}{$rel};
+        delete $self->{_inflated_column}{$rel};
+      }
+    }
+  }
 
   return $new_value;
 }

diff --git a/t/relationship/core.t b/t/relationship/core.t
index 8996b1d..d6cb3a3 100644 (file)
--- a/t/relationship/core.t
+++ b/t/relationship/core.t
@@ -89,13 +89,10 @@ $track->set_from_related( cd => undef );
 
 ok( !defined($track->cd), 'set_from_related with undef ok');
 
-TODO: {
-    local $TODO = 'accessing $object->rel and set_from_related';
-    my $track = $schema->resultset("Track")->new( {} );
-    $track->cd;
-    $track->set_from_related( cd => $cd ); 
-    ok ($track->cd, 'set_from_related ok after using the accessor' );
-};
+$track = $schema->resultset("Track")->new( {} );
+$track->cd;
+$track->set_from_related( cd => $cd ); 
+ok ($track->cd, 'set_from_related ok after using the accessor' );
 
 # update_from_related, the same as set_from_related, but it calls update afterwards
 $track = $schema->resultset("Track")->create( {

diff --git a/t/relationship/set_column_on_fk.t b/t/relationship/set_column_on_fk.t
new file mode 100644 (file)
index 0000000..9f49427
--- /dev/null
+++ b/t/relationship/set_column_on_fk.t
@@ -0,0 +1,73 @@
+use strict;
+use warnings;
+
+use Test::More;
+use lib qw(t/lib);
+use DBICTest;
+
+my $schema = DBICTest->init_schema();
+
+
+# test with relname == colname
+my $bookmark = $schema->resultset("Bookmark")->find(1);
+ok( $bookmark->has_column ('link'), 'Right column name' );
+ok( $bookmark->has_relationship ('link'), 'Right rel name' );
+
+my $link = $bookmark->link;
+
+my $new_link = $schema->resultset("Link")->create({
+  url     => "http://bugsarereal.com",
+  title   => "bugsarereal.com",
+  id      => 9,
+});
+
+is( $bookmark->link->id, 1, 'Initial relation id' );
+
+$bookmark->set_column( 'link', 9 );
+is( $bookmark->link->id, 9, 'Correct object re-selected after belongs_to set' );
+
+$bookmark->discard_changes;
+is( $bookmark->link->id, 1, 'Pulled the correct old object after belongs_to reset' );
+
+
+$bookmark->link($new_link);
+is( $bookmark->get_column('link'), 9, 'Correct column set from related' );
+
+$bookmark->discard_changes;
+is( $bookmark->link->id, 1, 'Pulled the correct old object after belongs_to reset' );
+
+
+$bookmark->link(9);
+is( $bookmark->link->id, 9, 'Correct object selected on deflated accessor set');
+
+$bookmark->discard_changes;
+is( $bookmark->link->id, 1, 'Pulled the correct old object after belongs_to reset' );
+
+
+$bookmark->update({ link => 9 });
+is( $bookmark->link->id, 9, 'Correct relationship after update' );
+is( $bookmark->get_from_storage->link->id, 9, 'Correct relationship after re-select' );
+
+
+# test with relname != colname
+my $lyric = $schema->resultset('Lyrics')->create({ track_id => 5 });
+is( $lyric->track->id, 5, 'Initial relation id');
+
+$lyric->track_id(6);
+my $track6 = $lyric->track;
+is( $track6->trackid, 6, 'Correct object re-selected after belongs_to set');
+
+$lyric->discard_changes;
+is( $lyric->track->trackid, 5, 'Pulled the correct old rel object after belongs_to reset');
+
+$lyric->track($track6);
+is( $lyric->track_id, 6, 'Correct column set from related');
+
+$lyric->discard_changes;
+is( $lyric->track->trackid, 5, 'Pulled the correct old rel object after belongs_to reset');
+
+$lyric->update({ track => $track6 });
+is( $lyric->track->trackid, 6, 'Correct relationship obj after update' );
+is( $lyric->get_from_storage->track->trackid, 6, 'Correct relationship after re-select' );
+
+done_testing;