Fix API mismatch between new_related() and new_result() (RT#78336)
Peter Rabbitson [Sat, 3 Nov 2012 17:12:06 +0000 (18:12 +0100)]
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.

Changes
lib/DBIx/Class/Relationship/Base.pm
lib/DBIx/Class/ResultSet.pm
t/cdbi/09-has_many.t

diff --git a/Changes b/Changes
index c865454..db5545f 100644 (file)
--- 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
index 6cfe28c..eda461d 100644 (file)
@@ -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
index eb6e7d7..3112cb5 100644 (file)
@@ -207,11 +207,23 @@ automatically get one from e.g. a L</search> 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</new_result> 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<new_result> directly,
+so overloading this method with the idea of intercepting new result object
+creation B<will not work>. See also warning pertaining to L</create>.
+
+=back
 
 =cut
 
@@ -2308,7 +2320,11 @@ Passes the hashref of input on to L<DBIx::Class::Row/new>.
 
 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<new|DBIx::Class::Row/new>
 or L<insert|DBIx::Class::Row/insert> depending on how early in the
-L</create> process you need to intervene.
+L</create> process you need to intervene. See also warning pertaining to
+L</new>.
 
 =back
 
index 7b2a336..c2939ef 100644 (file)
@@ -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(