From 85deb16ae16a5d0c37c8a547851cf0eeca190805 Mon Sep 17 00:00:00 2001 From: Eugen Konkov Date: Tue, 12 May 2015 23:09:59 +0300 Subject: [PATCH 01/11] Postpone 'create_related' rows until main row is created We can add related (children) row *ONLY AFTER* main (parent) row is created!!! So we *MUST* postpone creation. --- lib/DBIx/Class/Row.pm | 59 ++++++++++++++++++++++++------------------- 1 file changed, 33 insertions(+), 26 deletions(-) diff --git a/lib/DBIx/Class/Row.pm b/lib/DBIx/Class/Row.pm index a4a18b97f..4f8ba9f31 100644 --- a/lib/DBIx/Class/Row.pm +++ b/lib/DBIx/Class/Row.pm @@ -176,6 +176,29 @@ sub __their_pk_needs_us { # this should maybe be in resultsource. return 0; } +use Data::Dump qw/ pp /; +my $create_related_row = sub { + my( $new, $key, $others ) = @_; + my $total = @$others; + my @objects; + foreach my $idx (0 .. $#$others) { + my $rel_obj = $others->[$idx]; + if(!blessed $rel_obj) { + $rel_obj = $new->__new_related_find_or_new_helper($key, $rel_obj); + } + + if ($rel_obj->in_storage) { + $rel_obj->throw_exception ('A multi relationship can not be pre-existing when doing multicreate. Something went wrong'); + } else { + MULTICREATE_DEBUG and + print STDERR "MC $new uninserted $key $rel_obj (${\($idx+1)} of $total)\n"; + } + push(@objects, $rel_obj); + } + + return @objects; +}; + sub new { my ($class, $attrs) = @_; $class = ref $class if ref $class; @@ -198,7 +221,6 @@ sub new { } my ($related,$inflated); - foreach my $key (keys %$attrs) { if (ref $attrs->{$key} and ! is_literal_value($attrs->{$key}) ) { ## Can we extract this lot to use with update(_or .. ) ? @@ -223,24 +245,9 @@ sub new { next; } elsif ($acc_type eq 'multi' && ref $attrs->{$key} eq 'ARRAY' ) { - my $others = delete $attrs->{$key}; - my $total = @$others; - my @objects; - foreach my $idx (0 .. $#$others) { - my $rel_obj = $others->[$idx]; - if(!blessed $rel_obj) { - $rel_obj = $new->__new_related_find_or_new_helper($key, $rel_obj); - } - - if ($rel_obj->in_storage) { - $rel_obj->throw_exception ('A multi relationship can not be pre-existing when doing multicreate. Something went wrong'); - } else { - MULTICREATE_DEBUG and - print STDERR "MC $new uninserted $key $rel_obj (${\($idx+1)} of $total)\n"; - } - push(@objects, $rel_obj); - } - $related->{$key} = \@objects; + # We can add related (children) row *ONLY AFTER* main (parent) row is created!!! + # So we postpone creation (see below) + $related->{$key} = delete $attrs->{$key}; next; } elsif ($acc_type eq 'filter') { @@ -269,6 +276,12 @@ sub new { } $new->store_column($key => $attrs->{$key}); } + # After main (master) row's columns are stored (new row is created) + # we can add related (children) rows + #die pp $new; + foreach my $key ( keys %$related ) { + $related->{$key} = [ $create_related_row->( $new, $key, $related->{$key} ) ]; + } $new->{_relationship_data} = $related if $related; $new->{_inflated_column} = $inflated if $inflated; @@ -1215,13 +1228,7 @@ sub store_column { unless exists $self->{_column_data}{$column} || $self->result_source->has_column($column); $self->throw_exception( "set_column called for ${column} without value" ) if @_ < 3; - - # stringify all refs explicitly, guards against overloaded objects - # with defined stringification AND fallback => 0 (ugh!) - $self->{_column_data}{$column} = ( length ref $value and is_plain_value( $value ) ) - ? "$value" - : $value - ; + return $self->{_column_data}{$column} = $value; } =head2 inflate_result From 4eee7124a2463fa0ada0b8825443c13920c2c885 Mon Sep 17 00:00:00 2001 From: Eugen Konkov Date: Tue, 12 May 2015 23:09:59 +0300 Subject: [PATCH 02/11] Postpone 'create_related' rows until main row is created We can add related (children) row *ONLY AFTER* main (parent) row is created!!! So we *MUST* postpone creation. --- lib/DBIx/Class/Row.pm | 51 +++++++++++++++++++++++++++---------------- 1 file changed, 32 insertions(+), 19 deletions(-) diff --git a/lib/DBIx/Class/Row.pm b/lib/DBIx/Class/Row.pm index a4a18b97f..d3777924a 100644 --- a/lib/DBIx/Class/Row.pm +++ b/lib/DBIx/Class/Row.pm @@ -176,6 +176,29 @@ sub __their_pk_needs_us { # this should maybe be in resultsource. return 0; } +use Data::Dump qw/ pp /; +my $create_related_row = sub { + my( $new, $key, $others ) = @_; + my $total = @$others; + my @objects; + foreach my $idx (0 .. $#$others) { + my $rel_obj = $others->[$idx]; + if(!blessed $rel_obj) { + $rel_obj = $new->__new_related_find_or_new_helper($key, $rel_obj); + } + + if ($rel_obj->in_storage) { + $rel_obj->throw_exception ('A multi relationship can not be pre-existing when doing multicreate. Something went wrong'); + } else { + MULTICREATE_DEBUG and + print STDERR "MC $new uninserted $key $rel_obj (${\($idx+1)} of $total)\n"; + } + push(@objects, $rel_obj); + } + + return @objects; +}; + sub new { my ($class, $attrs) = @_; $class = ref $class if ref $class; @@ -198,7 +221,6 @@ sub new { } my ($related,$inflated); - foreach my $key (keys %$attrs) { if (ref $attrs->{$key} and ! is_literal_value($attrs->{$key}) ) { ## Can we extract this lot to use with update(_or .. ) ? @@ -223,24 +245,9 @@ sub new { next; } elsif ($acc_type eq 'multi' && ref $attrs->{$key} eq 'ARRAY' ) { - my $others = delete $attrs->{$key}; - my $total = @$others; - my @objects; - foreach my $idx (0 .. $#$others) { - my $rel_obj = $others->[$idx]; - if(!blessed $rel_obj) { - $rel_obj = $new->__new_related_find_or_new_helper($key, $rel_obj); - } - - if ($rel_obj->in_storage) { - $rel_obj->throw_exception ('A multi relationship can not be pre-existing when doing multicreate. Something went wrong'); - } else { - MULTICREATE_DEBUG and - print STDERR "MC $new uninserted $key $rel_obj (${\($idx+1)} of $total)\n"; - } - push(@objects, $rel_obj); - } - $related->{$key} = \@objects; + # We can add related (children) row *ONLY AFTER* main (parent) row is created!!! + # So we postpone creation (see below) + $related->{$key} = delete $attrs->{$key}; next; } elsif ($acc_type eq 'filter') { @@ -269,6 +276,12 @@ sub new { } $new->store_column($key => $attrs->{$key}); } + # After main (master) row's columns are stored (new row is created) + # we can add related (children) rows + #die pp $new; + foreach my $key ( keys %$related ) { + $related->{$key} = [ $create_related_row->( $new, $key, $related->{$key} ) ]; + } $new->{_relationship_data} = $related if $related; $new->{_inflated_column} = $inflated if $inflated; From 97205363b7a019584b3a987eda90c66454dbac15 Mon Sep 17 00:00:00 2001 From: Eugen Konkov Date: Tue, 12 May 2015 23:17:12 +0300 Subject: [PATCH 03/11] return deleted code --- lib/DBIx/Class/Row.pm | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/DBIx/Class/Row.pm b/lib/DBIx/Class/Row.pm index 4f8ba9f31..d3777924a 100644 --- a/lib/DBIx/Class/Row.pm +++ b/lib/DBIx/Class/Row.pm @@ -1228,7 +1228,13 @@ sub store_column { unless exists $self->{_column_data}{$column} || $self->result_source->has_column($column); $self->throw_exception( "set_column called for ${column} without value" ) if @_ < 3; - return $self->{_column_data}{$column} = $value; + + # stringify all refs explicitly, guards against overloaded objects + # with defined stringification AND fallback => 0 (ugh!) + $self->{_column_data}{$column} = ( length ref $value and is_plain_value( $value ) ) + ? "$value" + : $value + ; } =head2 inflate_result From 932038497a476c461f8d9af2cebe447f6be2fc6c Mon Sep 17 00:00:00 2001 From: Eugen Konkov Date: Wed, 13 May 2015 17:25:59 +0300 Subject: [PATCH 04/11] Remove unused module --- lib/DBIx/Class/Row.pm | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/DBIx/Class/Row.pm b/lib/DBIx/Class/Row.pm index d3777924a..b27804ec7 100644 --- a/lib/DBIx/Class/Row.pm +++ b/lib/DBIx/Class/Row.pm @@ -176,7 +176,6 @@ sub __their_pk_needs_us { # this should maybe be in resultsource. return 0; } -use Data::Dump qw/ pp /; my $create_related_row = sub { my( $new, $key, $others ) = @_; my $total = @$others; @@ -278,7 +277,6 @@ sub new { } # After main (master) row's columns are stored (new row is created) # we can add related (children) rows - #die pp $new; foreach my $key ( keys %$related ) { $related->{$key} = [ $create_related_row->( $new, $key, $related->{$key} ) ]; } From ebf25f0e630caf1f7dc65f5bfaf30673387d8aa4 Mon Sep 17 00:00:00 2001 From: Eugen Konkov Date: Thu, 14 May 2015 22:06:22 +0300 Subject: [PATCH 05/11] IMPLEMENTED: 'filter' now disappeared and get merged in with 'single' above! Copy-past code is merged together into 'create_related_rows' and 'filter', 'single', 'multi' work through that sub --- lib/DBIx/Class/Row.pm | 85 ++++++++++++++++++++++--------------------- 1 file changed, 43 insertions(+), 42 deletions(-) diff --git a/lib/DBIx/Class/Row.pm b/lib/DBIx/Class/Row.pm index b27804ec7..6524ff7e2 100644 --- a/lib/DBIx/Class/Row.pm +++ b/lib/DBIx/Class/Row.pm @@ -176,28 +176,45 @@ sub __their_pk_needs_us { # this should maybe be in resultsource. return 0; } -my $create_related_row = sub { - my( $new, $key, $others ) = @_; - my $total = @$others; + +my $create_related_rows = sub { + my( $new, $key, $type, $others ) = @_; + + if( ref $others ne 'ARRAY' ) { + $others = [ $others ]; + #assert $type must be 'multi' + } + + my $total = @$others; my @objects; - foreach my $idx (0 .. $#$others) { - my $rel_obj = $others->[$idx]; - if(!blessed $rel_obj) { - $rel_obj = $new->__new_related_find_or_new_helper($key, $rel_obj); + foreach my $idx ( 0 .. $#$others ) { + my $rel_obj = $others->[$idx]; + if( !blessed $rel_obj ) { + $rel_obj = $new->__new_related_find_or_new_helper($key, $rel_obj); } - if ($rel_obj->in_storage) { - $rel_obj->throw_exception ('A multi relationship can not be pre-existing when doing multicreate. Something went wrong'); + if( $rel_obj->in_storage ) { + $type eq 'multi' + and $rel_obj->throw_exception( 'A multi relationship can not be pre-existing when doing multicreate. Something went wrong' ); + + $new->{_rel_in_storage}{$key} = 1; + $new->set_from_related( $key, $rel_obj ) if $type eq 'single'; } else { - MULTICREATE_DEBUG and - print STDERR "MC $new uninserted $key $rel_obj (${\($idx+1)} of $total)\n"; + my $tmp = $type eq 'multi' ? "(${\($idx+1)} of $total)" : ''; + MULTICREATE_DEBUG + and print STDERR "MC $new uninserted $key $rel_obj$tmp\n"; } - push(@objects, $rel_obj); + push( @objects, $rel_obj ); } - return @objects; + return $type eq 'multi' + ? \@objects + : shift @objects + ; }; + use Data::Dump qw/ pp /; + sub new { my ($class, $attrs) = @_; $class = ref $class if ref $class; @@ -219,7 +236,7 @@ sub new { @{$new->{_ignore_at_insert}={}}{@$col_from_rel} = (); } - my ($related,$inflated); + my( $postponed, $inflated ); foreach my $key (keys %$attrs) { if (ref $attrs->{$key} and ! is_literal_value($attrs->{$key}) ) { ## Can we extract this lot to use with update(_or .. ) ? @@ -228,40 +245,17 @@ sub new { my $info = $rsrc->relationship_info($key); my $acc_type = $info->{attrs}{accessor} || ''; if ($acc_type eq 'single') { - my $rel_obj = delete $attrs->{$key}; - if(!blessed $rel_obj) { - $rel_obj = $new->__new_related_find_or_new_helper($key, $rel_obj); - } - - if ($rel_obj->in_storage) { - $new->{_rel_in_storage}{$key} = 1; - $new->set_from_related($key, $rel_obj); - } else { - MULTICREATE_DEBUG and print STDERR "MC $new uninserted $key $rel_obj\n"; - } - - $related->{$key} = $rel_obj; + $postponed->{$key} = [ $acc_type, delete $attrs->{$key} ]; next; } elsif ($acc_type eq 'multi' && ref $attrs->{$key} eq 'ARRAY' ) { # We can add related (children) row *ONLY AFTER* main (parent) row is created!!! # So we postpone creation (see below) - $related->{$key} = delete $attrs->{$key}; + $postponed->{$key} = [ $acc_type, delete $attrs->{$key} ]; next; } elsif ($acc_type eq 'filter') { - ## 'filter' should disappear and get merged in with 'single' above! - my $rel_obj = delete $attrs->{$key}; - if(!blessed $rel_obj) { - $rel_obj = $new->__new_related_find_or_new_helper($key, $rel_obj); - } - if ($rel_obj->in_storage) { - $new->{_rel_in_storage}{$key} = 1; - } - else { - MULTICREATE_DEBUG and print STDERR "MC $new uninserted $key $rel_obj\n"; - } - $inflated->{$key} = $rel_obj; + $postponed->{$key} = [ $acc_type, delete $attrs->{$key} ]; next; } elsif ( @@ -277,8 +271,15 @@ sub new { } # After main (master) row's columns are stored (new row is created) # we can add related (children) rows - foreach my $key ( keys %$related ) { - $related->{$key} = [ $create_related_row->( $new, $key, $related->{$key} ) ]; + + my $related; + foreach my $key ( keys %$postponed ) { + if( $postponed->{$key}[0] ne 'filter' ) { + $related->{$key} = $create_related_rows->( $new, $key, $postponed->{$key}[0], $postponed->{$key}[1] ); + } + else { + $inflated->{$key} = $create_related_rows->( $new, $key, $postponed->{$key}[0], $postponed->{$key}[1] ); + } } $new->{_relationship_data} = $related if $related; From f199e7095dbe1b869a0c692a19cfab0fdbae34a4 Mon Sep 17 00:00:00 2001 From: Eugen Konkov Date: Thu, 14 May 2015 22:14:59 +0300 Subject: [PATCH 06/11] Removed duplicated code --- lib/DBIx/Class/Row.pm | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/lib/DBIx/Class/Row.pm b/lib/DBIx/Class/Row.pm index 6524ff7e2..b6bec34c4 100644 --- a/lib/DBIx/Class/Row.pm +++ b/lib/DBIx/Class/Row.pm @@ -244,20 +244,15 @@ sub new { unless $rsrc; my $info = $rsrc->relationship_info($key); my $acc_type = $info->{attrs}{accessor} || ''; - if ($acc_type eq 'single') { - $postponed->{$key} = [ $acc_type, delete $attrs->{$key} ]; - next; - } - elsif ($acc_type eq 'multi' && ref $attrs->{$key} eq 'ARRAY' ) { + if( $acc_type eq 'single' + || $acc_type eq 'multi' && ref $attrs->{$key} eq 'ARRAY' + || $acc_type eq 'filter' + ) { # We can add related (children) row *ONLY AFTER* main (parent) row is created!!! # So we postpone creation (see below) $postponed->{$key} = [ $acc_type, delete $attrs->{$key} ]; next; } - elsif ($acc_type eq 'filter') { - $postponed->{$key} = [ $acc_type, delete $attrs->{$key} ]; - next; - } elsif ( $rsrc->has_column($key) and From cb909f39c33c225ba72038e67fafc5ef27da14ce Mon Sep 17 00:00:00 2001 From: Eugen Konkov Date: Thu, 14 May 2015 22:16:38 +0300 Subject: [PATCH 07/11] $rsrc is not changed below. No need cyclic checks --- lib/DBIx/Class/Row.pm | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/DBIx/Class/Row.pm b/lib/DBIx/Class/Row.pm index b6bec34c4..be81386bf 100644 --- a/lib/DBIx/Class/Row.pm +++ b/lib/DBIx/Class/Row.pm @@ -236,12 +236,14 @@ sub new { @{$new->{_ignore_at_insert}={}}{@$col_from_rel} = (); } + + $new->throw_exception("Can't do multi-create without result source") + unless $rsrc; + my( $postponed, $inflated ); foreach my $key (keys %$attrs) { if (ref $attrs->{$key} and ! is_literal_value($attrs->{$key}) ) { ## Can we extract this lot to use with update(_or .. ) ? - $new->throw_exception("Can't do multi-create without result source") - unless $rsrc; my $info = $rsrc->relationship_info($key); my $acc_type = $info->{attrs}{accessor} || ''; if( $acc_type eq 'single' From b248152fc59bffe7101e68d10f57d61e6a669d59 Mon Sep 17 00:00:00 2001 From: Eugen Konkov Date: Thu, 14 May 2015 22:23:20 +0300 Subject: [PATCH 08/11] Remove Data::Dump --- lib/DBIx/Class/Row.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/DBIx/Class/Row.pm b/lib/DBIx/Class/Row.pm index be81386bf..3ba796726 100644 --- a/lib/DBIx/Class/Row.pm +++ b/lib/DBIx/Class/Row.pm @@ -213,7 +213,7 @@ my $create_related_rows = sub { ; }; - use Data::Dump qw/ pp /; + sub new { my ($class, $attrs) = @_; From 556e090f955e56cf78517565457c6b056dc1b10c Mon Sep 17 00:00:00 2001 From: Eugen Konkov Date: Thu, 14 May 2015 23:03:18 +0300 Subject: [PATCH 09/11] Add myself to AUTHORS --- AUTHORS | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS b/AUTHORS index 90f30335b..fca9f9066 100644 --- a/AUTHORS +++ b/AUTHORS @@ -110,6 +110,7 @@ jshirley: J. Shirley kaare: Kaare Rasmussen kd: Kieren Diment kentnl: Kent Fredric +KES777: Eugen Konkov kkane: Kevin L. Kane konobi: Scott McWhirter Lasse Makholm From d2106cf0f8cfa033c94e4616802fff637194440a Mon Sep 17 00:00:00 2001 From: Eugen Konkov Date: Thu, 21 May 2015 13:47:02 +0300 Subject: [PATCH 10/11] Parent row must exist before child row is created Test case for: http://search.cpan.org/~ribasushi/DBIx-Class-0.082820/lib/DBIx/Class/ResultSet.pm#create "Example of creating a new row and also creating rows in a related has_many or has_one resultse" When related row is autofilled by Related::new and depend on info of master/parent table this cause exception: died: Can't call method "name" on an undefined value at t/multi_create/create_related.t line 42, <> line 1. Because of master/parent row must be created before child/related row. --- t/multi_create/parent_children.t | 48 ++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 t/multi_create/parent_children.t diff --git a/t/multi_create/parent_children.t b/t/multi_create/parent_children.t new file mode 100644 index 000000000..f2caf62c3 --- /dev/null +++ b/t/multi_create/parent_children.t @@ -0,0 +1,48 @@ +use strict; +use warnings; + +use Test::More; +use Test::Exception; +use lib qw(t/lib); +use DBICTest; + +plan tests => 4; + +my $schema = DBICTest->init_schema(); + +lives_ok ( sub { + + my $artist_rs = $schema->resultset ('Artist'); + my $artist_count = $artist_rs->count(); + my $cd_rs = $schema->resultset ('CD'); + my $cd_count = $cd_rs->count(); + $artist_rs->create({ + name => 'parent child relation', + cds => [ {}, {} ], #CD's 'title' field are autofilled at CD::new + }); + + is ($artist_rs->count, $artist_count + 1, 'New artist was created'); + ok ($artist_rs->find ({name => 'parent child relation'}), 'Artist was created with correct name'); + + is ($cd_rs->count, $cd_count + 2, 'New cds were created'); + is ($cd_rs->search({name => 'parent child relation'})->count, 2, 'CDs were created with correct name'); + +}, 'Parent row must exist before child row is created'); + +1; + +package # hide from PAUSE + DBICTest::Schema::CD; + +sub new { + my ( $class, $attrs ) = @_; + +my $self = $class->next::method($attrs); + +$self->title( $self->artist->name ); + +return $self; +} + + +1; From 20f40a90efa6459f340f10b62be019add2276a14 Mon Sep 17 00:00:00 2001 From: Eugen Konkov Date: Thu, 21 May 2015 14:11:42 +0300 Subject: [PATCH 11/11] Revert "$rsrc is not changed below. No need cyclic checks" This reverts commit cb909f39c33c225ba72038e67fafc5ef27da14ce. --- lib/DBIx/Class/Row.pm | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/DBIx/Class/Row.pm b/lib/DBIx/Class/Row.pm index 3ba796726..e3727daba 100644 --- a/lib/DBIx/Class/Row.pm +++ b/lib/DBIx/Class/Row.pm @@ -236,14 +236,12 @@ sub new { @{$new->{_ignore_at_insert}={}}{@$col_from_rel} = (); } - - $new->throw_exception("Can't do multi-create without result source") - unless $rsrc; - my( $postponed, $inflated ); foreach my $key (keys %$attrs) { if (ref $attrs->{$key} and ! is_literal_value($attrs->{$key}) ) { ## Can we extract this lot to use with update(_or .. ) ? + $new->throw_exception("Can't do multi-create without result source") + unless $rsrc; my $info = $rsrc->relationship_info($key); my $acc_type = $info->{attrs}{accessor} || ''; if( $acc_type eq 'single'