Fix MC bug reported by felix
Peter Rabbitson [Sun, 7 Mar 2010 10:38:32 +0000 (10:38 +0000)]
Changes
lib/DBIx/Class/Row.pm
t/60core.t
t/admin/03data.t
t/lib/DBICTest/Schema/Employee.pm
t/lib/DBICTest/Schema/Encoded.pm
t/lib/sqlite.sql

diff --git a/Changes b/Changes
index 4495626..1fa7499 100644 (file)
--- a/Changes
+++ b/Changes
@@ -5,6 +5,8 @@ Revision history for DBIx::Class
         - Add req_group_list to Opt::Deps (RT#55211)
         - Cascading delete/update are now wrapped in a transaction
           for atomicity
+        - Fix multiple deficiencies when using MultiCreate with
+          data-encoder components (e.g. ::EncodedColumn)
         - Fix regression where SQL files with comments were not
           handled properly by ::Schema::Versioned.
         - Fix regression on not properly throwing when $obj->relationship
index 0819375..7a2b90c 100644 (file)
@@ -105,26 +105,40 @@ with NULL as the default, and save yourself a SELECT.
 
 sub __new_related_find_or_new_helper {
   my ($self, $relname, $data) = @_;
-  if ($self->__their_pk_needs_us($relname, $data)) {
+
+  # create a mock-object so all new/set_column component overrides will run:
+  my $rel_rs = $self->result_source
+                    ->related_source($relname)
+                    ->resultset;
+  my $new_rel_obj = $rel_rs->new_result($data);
+  my $proc_data = { $new_rel_obj->get_columns };
+
+  if ($self->__their_pk_needs_us($relname)) {
     MULTICREATE_DEBUG and warn "MC $self constructing $relname via new_result";
-    return $self->result_source
-                ->related_source($relname)
-                ->resultset
-                ->new_result($data);
+    return $new_rel_obj;
+  }
+  elsif ($self->result_source->_pk_depends_on($relname, $proc_data )) {
+    if (! keys %$proc_data) {
+      # there is nothing to search for - blind create
+      MULTICREATE_DEBUG and warn "MC $self constructing default-insert $relname";
+    }
+    else {
+      MULTICREATE_DEBUG and warn "MC $self constructing $relname via find_or_new";
+      # this is not *really* find or new, as we don't want to double-new the
+      # data (thus potentially double encoding or whatever)
+      my $exists = $rel_rs->find ($proc_data);
+      return $exists if $exists;
+    }
+    return $new_rel_obj;
   }
-  if ($self->result_source->_pk_depends_on($relname, $data)) {
-    MULTICREATE_DEBUG and warn "MC $self constructing $relname via find_or_new";
-    return $self->result_source
-                ->related_source($relname)
-                ->resultset
-                ->find_or_new($data);
+  else {
+    my $us = $self->source_name;
+    $self->throw_exception ("'$us' neither depends nor is depended on by '$relname', something is wrong...");
   }
-  MULTICREATE_DEBUG and warn "MC $self constructing $relname via find_or_new_related";
-  return $self->find_or_new_related($relname, $data);
 }
 
 sub __their_pk_needs_us { # this should maybe be in resultsource.
-  my ($self, $relname, $data) = @_;
+  my ($self, $relname) = @_;
   my $source = $self->result_source;
   my $reverse = $source->reverse_relationship_info($relname);
   my $rel_source = $source->related_source($relname);
@@ -301,12 +315,20 @@ sub insert {
       MULTICREATE_DEBUG and warn "MC $self pre-reconstructing $relname $rel_obj\n";
 
       my $them = { %{$rel_obj->{_relationship_data} || {} }, $rel_obj->get_inflated_columns };
-      my $re = $self->result_source
-                    ->related_source($relname)
-                    ->resultset
-                    ->find_or_create($them);
+      my $existing;
+
+      # if there are no keys - nothing to search for
+      if (keys %$them and $existing = $self->result_source
+                                           ->related_source($relname)
+                                           ->resultset
+                                           ->find($them)
+      ) {
+        %{$rel_obj} = %{$existing};
+      }
+      else {
+        $rel_obj->insert;
+      }
 
-      %{$rel_obj} = %{$re};
       $self->{_rel_in_storage}{$relname} = 1;
     }
 
@@ -367,7 +389,7 @@ sub insert {
       foreach my $obj (@cands) {
         $obj->set_from_related($_, $self) for keys %$reverse;
         my $them = { %{$obj->{_relationship_data} || {} }, $obj->get_inflated_columns };
-        if ($self->__their_pk_needs_us($relname, $them)) {
+        if ($self->__their_pk_needs_us($relname)) {
           if (exists $self->{_ignore_at_insert}{$relname}) {
             MULTICREATE_DEBUG and warn "MC $self skipping post-insert on $relname";
           } else {
index 03fe3b6..69d99ed 100644 (file)
@@ -419,6 +419,47 @@ SKIP: {
   is($en_row->encoded, 'amliw', 'insert does not encode again');
 }
 
+#make sure multicreate encoding still works
+{
+  my $empl_rs = $schema->resultset('Employee');
+
+  my $empl = $empl_rs->create ({
+    name => 'Secret holder',
+    secretkey => {
+      encoded => 'CAN HAZ',
+    },
+  });
+  is($empl->secretkey->encoded, 'ZAH NAC', 'correctly encoding on multicreate');
+
+  my $empl2 = $empl_rs->create ({
+    name => 'Same secret holder',
+    secretkey => {
+      encoded => 'CAN HAZ',
+    },
+  });
+  is($empl2->secretkey->encoded, 'ZAH NAC', 'correctly encoding on preexisting multicreate');
+
+  $empl_rs->create ({
+    name => 'cat1',
+    secretkey => {
+      encoded => 'CHEEZBURGER',
+      keyholders => [
+        {
+          name => 'cat2',
+        },
+        {
+          name => 'cat3',
+        },
+      ],
+    },
+  });
+
+  is($empl_rs->find({name => 'cat1'})->secretkey->encoded, 'REGRUBZEEHC', 'correct secret in database for empl1');
+  is($empl_rs->find({name => 'cat2'})->secretkey->encoded, 'REGRUBZEEHC', 'correct secret in database for empl2');
+  is($empl_rs->find({name => 'cat3'})->secretkey->encoded, 'REGRUBZEEHC', 'correct secret in database for empl3');
+
+}
+
 # make sure we got rid of the compat shims
 SKIP: {
     skip "Remove in 0.082", 3 if $DBIx::Class::VERSION < 0.082;
index 1fe59b7..884b120 100644 (file)
@@ -48,8 +48,8 @@ use_ok 'DBIx::Class::Admin';
 
   my $expected_data = [ 
     [$employee->result_source->columns() ],
-    [1,1,undef,undef,undef,'Trout'],
-    [2,2,undef,undef,undef,'Aran']
+    [1,1,undef,undef,undef,'Trout',undef],
+    [2,2,undef,undef,undef,'Aran',undef]
   ];
   my $data;
   lives_ok { $data = $admin->select('Employee')} 'can retrive data from database';
index 30c2bca..35f6075 100644 (file)
@@ -32,6 +32,10 @@ __PACKAGE__->add_columns(
         size      => 100,
         is_nullable => 1,
     },
+    encoded => {
+        data_type => 'integer',
+        is_nullable => 1,
+    },
 );
 
 __PACKAGE__->set_primary_key('employee_id');
@@ -40,4 +44,8 @@ __PACKAGE__->position_column('position');
 # Do not add unique constraints here - different groups are used throughout
 # the ordered tests
 
+__PACKAGE__->belongs_to (secretkey => 'DBICTest::Schema::Encoded', 'encoded', {
+  join_type => 'left'
+});
+
 1;
index 7fd77dc..234846d 100644 (file)
@@ -21,6 +21,8 @@ __PACKAGE__->add_columns(
 
 __PACKAGE__->set_primary_key('id');
 
+__PACKAGE__->has_many (keyholders => 'DBICTest::Schema::Employee', 'encoded');
+
 sub set_column {
   my ($self, $col, $value) = @_;
   if( $col eq 'encoded' ){
index cce0275..7478cb7 100644 (file)
@@ -1,6 +1,6 @@
 -- 
 -- Created by SQL::Translator::Producer::SQLite
--- Created on Sat Mar  6 13:01:48 2010
+-- Created on Sun Mar  7 11:14:14 2010
 -- 
 ;
 
@@ -35,18 +35,6 @@ CREATE TABLE collection (
 );
 
 --
--- Table: employee
---
-CREATE TABLE employee (
-  employee_id INTEGER PRIMARY KEY NOT NULL,
-  position integer NOT NULL,
-  group_id integer,
-  group_id_2 integer,
-  group_id_3 integer,
-  name varchar(100)
-);
-
---
 -- Table: encoded
 --
 CREATE TABLE encoded (
@@ -253,6 +241,21 @@ CREATE TABLE books (
 CREATE INDEX books_idx_owner ON books (owner);
 
 --
+-- Table: employee
+--
+CREATE TABLE employee (
+  employee_id INTEGER PRIMARY KEY NOT NULL,
+  position integer NOT NULL,
+  group_id integer,
+  group_id_2 integer,
+  group_id_3 integer,
+  name varchar(100),
+  encoded integer
+);
+
+CREATE INDEX employee_idx_encoded ON employee (encoded);
+
+--
 -- Table: forceforeign
 --
 CREATE TABLE forceforeign (