From: Peter Rabbitson Date: Sat, 3 Nov 2012 17:12:06 +0000 (+0100) Subject: Fix API mismatch between new_related() and new_result() (RT#78336) X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=81e4dc3d93646a11bee973606a31be412f23cd5f;p=dbsrgits%2FDBIx-Class-Historic.git Fix API mismatch between new_related() and new_result() (RT#78336) Almost 7 years ago a refactor in fea3d045 fed an (undocumented) $attrs parameter from new_related() to new_result(), while new_result() never expected (and ignored) said parameter. Since this is an undocumented feature, which nobody complained about for all this time - document extensively and kill it with fire. --- diff --git a/Changes b/Changes index c865454..db5545f 100644 --- a/Changes +++ b/Changes @@ -6,6 +6,8 @@ Revision history for DBIx::Class * Fixes - Fix unique constraint violations in Ordered.pm blanket movement (RT#79773, rolls back short-sighted 5e6fde33e) + - Fix API mismatch between new_result() and new_related() (originally + broken by fea3d045) - Fix test failure on perl 5.8 0.08203 2012-10-18 diff --git a/lib/DBIx/Class/Relationship/Base.pm b/lib/DBIx/Class/Relationship/Base.pm index 6cfe28c..eda461d 100644 --- a/lib/DBIx/Class/Relationship/Base.pm +++ b/lib/DBIx/Class/Relationship/Base.pm @@ -571,7 +571,7 @@ on it. =cut sub new_related { - my ($self, $rel, $values, $attrs) = @_; + my ($self, $rel, $values) = @_; # FIXME - this is a bad position for this (also an identical copy in # set_from_related), but I have no saner way to hook, and I absolutely @@ -600,8 +600,7 @@ sub new_related { } } - my $row = $self->search_related($rel)->new($values, $attrs); - return $row; + return $self->search_related($rel)->new_result($values); } =head2 create_related diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm index eb6e7d7..3112cb5 100644 --- a/lib/DBIx/Class/ResultSet.pm +++ b/lib/DBIx/Class/ResultSet.pm @@ -207,11 +207,23 @@ automatically get one from e.g. a L called in scalar context: my $rs = $schema->resultset('CD')->search({ title => '100th Window' }); -IMPORTANT: If called on an object, proxies to new_result instead so +=over + +=item WARNING + +If called on an object, proxies to L instead, so my $cd = $schema->resultset('CD')->new({ title => 'Spoon' }); -will return a CD object, not a ResultSet. +will return a CD object, not a ResultSet, and is equivalent to: + + my $cd = $schema->resultset('CD')->new_result({ title => 'Spoon' }); + +Please also keep in mind that many internals call C directly, +so overloading this method with the idea of intercepting new result object +creation B. See also warning pertaining to L. + +=back =cut @@ -2308,7 +2320,11 @@ Passes the hashref of input on to L. sub new_result { my ($self, $values) = @_; - $self->throw_exception( "new_result needs a hash" ) + + $self->throw_exception( "new_result takes only one argument - a hashref of values" ) + if @_ > 2; + + $self->throw_exception( "new_result expects a hashref" ) unless (ref $values eq 'HASH'); my ($merged_cond, $cols_from_relations) = $self->_merge_with_rscond($values); @@ -2643,7 +2659,8 @@ it is a simple shortcut for C<< $self->new_result($attrs)->insert >>, a lot of the internals simply never call it, so your override will be bypassed more often than not. Override either L or L depending on how early in the -L process you need to intervene. +L process you need to intervene. See also warning pertaining to +L. =back diff --git a/t/cdbi/09-has_many.t b/t/cdbi/09-has_many.t index 7b2a336..c2939ef 100644 --- a/t/cdbi/09-has_many.t +++ b/t/cdbi/09-has_many.t @@ -45,7 +45,7 @@ eval { my $pj = Film->add_to_actors(\%pj_data) }; like $@, qr/class/, "add_to_actors must be object method"; eval { my $pj = $btaste->add_to_actors(%pj_data) }; -like $@, qr/needs/, "add_to_actors takes hash"; +like $@, qr/expects a hashref/, "add_to_actors takes hash"; ok( my $pj = $btaste->add_to_actors(