From: Peter Rabbitson <ribasushi@cpan.org>
Date: Sun, 7 Jun 2009 23:24:06 +0000 (+0000)
Subject: Fix find_or_new/create to stop returning random rows when default value insert is... 
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=def17c59d7cf96fccfb7b89ee150f14f38153f24;p=dbsrgits%2FDBIx-Class-Historic.git

Fix find_or_new/create to stop returning random rows when default value insert is requested
---

diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm
index e105e79..52001d6 100644
--- a/lib/DBIx/Class/ResultSet.pm
+++ b/lib/DBIx/Class/ResultSet.pm
@@ -1977,8 +1977,10 @@ sub find_or_new {
   my $self     = shift;
   my $attrs    = (@_ > 1 && ref $_[$#_] eq 'HASH' ? pop(@_) : {});
   my $hash     = ref $_[0] eq 'HASH' ? shift : {@_};
-  my $exists   = $self->find($hash, $attrs);
-  return defined $exists ? $exists : $self->new_result($hash);
+  if (keys %$hash and my $row = $self->find($hash, $attrs) ) {
+    return $row;
+  }
+  return $self->new_result($hash);
 }
 
 =head2 create
@@ -2108,8 +2110,10 @@ sub find_or_create {
   my $self     = shift;
   my $attrs    = (@_ > 1 && ref $_[$#_] eq 'HASH' ? pop(@_) : {});
   my $hash     = ref $_[0] eq 'HASH' ? shift : {@_};
-  my $exists   = $self->find($hash, $attrs);
-  return defined $exists ? $exists : $self->create($hash);
+  if (keys %$hash and my $row = $self->find($hash, $attrs) ) {
+    return $row;
+  }
+  return $self->create($hash);
 }
 
 =head2 update_or_create
diff --git a/t/86sqlt.t b/t/86sqlt.t
index 24d573e..4b89019 100644
--- a/t/86sqlt.t
+++ b/t/86sqlt.t
@@ -210,7 +210,7 @@ my %fk_constraints = (
       'name' => 'bookmark_fk_link', 'index_name' => 'bookmark_idx_link',
       'selftable' => 'bookmark', 'foreigntable' => 'link', 
       'selfcols'  => ['link'], 'foreigncols' => ['id'],
-      on_delete => '', on_update => '', deferrable => 1,
+      on_delete => 'SET NULL', on_update => 'CASCADE', deferrable => 1,
     },
   ],
   # ForceForeign
diff --git a/t/lib/DBICTest/Schema/Bookmark.pm b/t/lib/DBICTest/Schema/Bookmark.pm
index bb32a14..c468936 100644
--- a/t/lib/DBICTest/Schema/Bookmark.pm
+++ b/t/lib/DBICTest/Schema/Bookmark.pm
@@ -19,6 +19,6 @@ __PACKAGE__->add_columns(
 );
 
 __PACKAGE__->set_primary_key('id');
-__PACKAGE__->belongs_to(link => 'DBICTest::Schema::Link' );
+__PACKAGE__->belongs_to(link => 'DBICTest::Schema::Link', 'link', { on_delete => 'SET NULL' } );
 
 1;
diff --git a/t/lib/DBICTest/Schema/Link.pm b/t/lib/DBICTest/Schema/Link.pm
index bf6d623..19b7aa0 100644
--- a/t/lib/DBICTest/Schema/Link.pm
+++ b/t/lib/DBICTest/Schema/Link.pm
@@ -25,6 +25,8 @@ __PACKAGE__->add_columns(
 );
 __PACKAGE__->set_primary_key('id');
 
+__PACKAGE__->has_many ( bookmarks => 'DBICTest::Schema::Bookmark', 'link', { cascade_delete => 0 } );
+
 use overload '""' => sub { shift->url }, fallback=> 1;
 
 1;
diff --git a/t/multi_create/insert_defaults.t b/t/multi_create/insert_defaults.t
new file mode 100644
index 0000000..e8054a7
--- /dev/null
+++ b/t/multi_create/insert_defaults.t
@@ -0,0 +1,45 @@
+use strict;
+use warnings;
+
+use Test::More;
+use Test::Exception;
+use lib qw(t/lib);
+use DBICTest;
+
+plan tests => 8;
+
+my $schema = DBICTest->init_schema();
+
+# Attempt sequential nested find_or_create with autoinc
+# As a side effect re-test nested default create (both the main object and the relation are {})
+my $bookmark_rs = $schema->resultset('Bookmark');
+my $last_bookmark = $bookmark_rs->search ({}, { order_by => { -desc => 'id' }, rows => 1})->single;
+my $last_link = $bookmark_rs->search_related ('link', {}, { order_by => { -desc => 'link.id' }, rows => 1})->single;
+
+# find_or_create a bookmark-link combo with data for a non-existing link
+my $o1 = $bookmark_rs->find_or_create ({ link => { url => 'something-weird' } });
+is ($o1->id, $last_bookmark->id + 1, '1st bookmark ID');
+is ($o1->link->id, $last_link->id + 1, '1st related link ID');
+
+# find_or_create a bookmark-link combo without any data at all (default insert)
+# should extend this test to all available Storage's, and fix them accordingly
+my $o2 = $bookmark_rs->find_or_create ({ link => {} });
+is ($o2->id, $last_bookmark->id + 2, '2nd bookmark ID');
+is ($o2->link->id, $last_link->id + 2, '2nd related link ID');
+
+# make sure the pre-existing link has only one related bookmark
+is ($last_link->bookmarks->count, 1, 'Expecting only 1 bookmark and 1 link, someone mucked with the table!');
+
+# find_or_create a bookmark withouyt any data, but supplying an existing link object
+# should return $last_bookmark
+my $o0 = $bookmark_rs->find_or_create ({ link => $last_link });
+is_deeply ({ $o0->columns}, {$last_bookmark->columns}, 'Correctly identify a row given a relationship');
+
+# inject an additional bookmark and repeat the test
+# should warn and return the first row
+my $o3 = $last_link->create_related ('bookmarks', {});
+is ($o3->id, $last_bookmark->id + 3, '3rd bookmark ID');
+
+local $SIG{__WARN__} = sub { warn @_ unless $_[0] =~ /Query returned more than one row/ };
+my $oX = $bookmark_rs->find_or_create ({ link => $last_link });
+is_deeply ({ $oX->columns}, {$last_bookmark->columns}, 'Correctly identify a row given a relationship');