From: Peter Rabbitson 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');