From: Peter Rabbitson Date: Sun, 7 Mar 2010 10:38:32 +0000 (+0000) Subject: Fix MC bug reported by felix X-Git-Tag: v0.08121~74 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=68888c09820ea25810c03cdc7748ee374a7772b2;p=dbsrgits%2FDBIx-Class.git Fix MC bug reported by felix --- diff --git a/Changes b/Changes index 4495626..1fa7499 100644 --- 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 diff --git a/lib/DBIx/Class/Row.pm b/lib/DBIx/Class/Row.pm index 0819375..7a2b90c 100644 --- a/lib/DBIx/Class/Row.pm +++ b/lib/DBIx/Class/Row.pm @@ -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 { diff --git a/t/60core.t b/t/60core.t index 03fe3b6..69d99ed 100644 --- a/t/60core.t +++ b/t/60core.t @@ -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; diff --git a/t/admin/03data.t b/t/admin/03data.t index 1fe59b7..884b120 100644 --- a/t/admin/03data.t +++ b/t/admin/03data.t @@ -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'; diff --git a/t/lib/DBICTest/Schema/Employee.pm b/t/lib/DBICTest/Schema/Employee.pm index 30c2bca..35f6075 100644 --- a/t/lib/DBICTest/Schema/Employee.pm +++ b/t/lib/DBICTest/Schema/Employee.pm @@ -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; diff --git a/t/lib/DBICTest/Schema/Encoded.pm b/t/lib/DBICTest/Schema/Encoded.pm index 7fd77dc..234846d 100644 --- a/t/lib/DBICTest/Schema/Encoded.pm +++ b/t/lib/DBICTest/Schema/Encoded.pm @@ -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' ){ diff --git a/t/lib/sqlite.sql b/t/lib/sqlite.sql index cce0275..7478cb7 100644 --- a/t/lib/sqlite.sql +++ b/t/lib/sqlite.sql @@ -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 (