| Subject: | new_related() uses a strange interface, killing Moosified resultset classes |
Hello,
DBIC::Relationship::Base::new_related() creates the new object by calling
$self->search_related($rel)->new($values)
instead of
$self->search_related($rel)->new_result($values)
This is semantically strange (using ResultSet->new to create a Row?) and breaks Moose-
based custom ResultSet classes. Unmoosed classes work because ResultSet detects when the
first arg to new() is $self and proxies the call to new_result(). Moose munges the arguments
to new() so that it receives ($class, $values, $attrs) instead of ($self, $values, $attrs). new()
then calls ->isa() on $values and dies.
I changed new() to new_result() as above and both my code and the DBIC test suite passed
without failures.
To see if any other code in DBIx::Class used the $rs_obj->new pattern, I updated the
proxying code to die. Two test files, t/52leaks.t and t/60core.t, use that syntax but nothing
else does. I don't know if $rs->new() can be changed, since it's a published interface (albeit
old, circa 2006), but I don't think it should be used internally.
I've attached two patches. The first changes the ->new() calls in Base.pm, 52leaks.t, and
60core.t to new_result(). The second adds a test to 60core.t to make sure that $rs->new()
returns a DBIx::Class::Row object, if you wish to keep this behavior.
Thank you for your time and for maintaining DBIC!
Cheers,
Fitz Elliott
| Subject: | 0002-test-that-new-on-a-ResultSet-object-returns-a-Row.patch |
From 02c8390379638e8281b6bb389fb59f43d050f438 Mon Sep 17 00:00:00 2001
From: Fitz Elliott <fitz.elliott@gmail.com>
Date: Thu, 12 Jul 2012 17:26:59 -0400
Subject: [PATCH 2/2] test that ->new() on a ResultSet object returns a Row
diff --git a/t/60core.t b/t/60core.t
index 81a98f1..9a90906 100644
--- a/t/60core.t
+++ b/t/60core.t
@@ -553,6 +553,14 @@ lives_ok (sub { my $newlink = $newbook->link}, "stringify to false value doesn't
);
}
+# test to make sure that calling ->new() on a resultset object gives
+# us a row object
+{
+ my $new_artist = $schema->resultset('Artist')->new({});
+ isa_ok( $new_artist, 'DBIx::Class::Row', '$rs->new gives a row object' );
+}
+
+
# make sure we got rid of the compat shims
SKIP: {
skip "Remove in 0.082", 3 if $DBIx::Class::VERSION < 0.082;
--
1.7.11.1
| Subject: | 0001-replace-calls-to-rs-new-with-rs-new_result.patch |
From be4e5a3aba05759b35c55c86afdbda79564401fd Mon Sep 17 00:00:00 2001
From: Fitz Elliott <fitz.elliott@gmail.com>
Date: Thu, 12 Jul 2012 17:07:31 -0400
Subject: [PATCH 1/2] replace calls to $rs->new() with $rs->new_result()
When new() is called on an instantiated ResultSet object, the call is
proxied to new_result(). This patch replaces those new() calls with
direct calls to new_result(), so that the intent is clearer and to
avoid breaking Moose-based resultsets. Moose munges the arguments to
new(), replacing $self with $class bypassing the proxy call.
diff --git a/lib/DBIx/Class/Relationship/Base.pm b/lib/DBIx/Class/Relationship/Base.pm
index 6cfe28c..7267702 100644
--- a/lib/DBIx/Class/Relationship/Base.pm
+++ b/lib/DBIx/Class/Relationship/Base.pm
@@ -600,7 +600,7 @@ sub new_related {
}
}
- my $row = $self->search_related($rel)->new($values, $attrs);
+ my $row = $self->search_related($rel)->new_result($values, $attrs);
return $row;
}
diff --git a/t/52leaks.t b/t/52leaks.t
index 7b51dc4..724b42f 100644
--- a/t/52leaks.t
+++ b/t/52leaks.t
@@ -173,7 +173,7 @@ my @compose_ns_classes;
# dbh_do codepath
my ($rs_bind_circref, $cond_rowobj) = $schema->storage->dbh_do ( sub {
- my $row = $_[0]->schema->resultset('Artist')->new({});
+ my $row = $_[0]->schema->resultset('Artist')->new_result({});
my $rs = $_[0]->schema->resultset('Artist')->search({
name => $row, # this is deliberately bogus, see FIXME below!
});
diff --git a/t/60core.t b/t/60core.t
index edf5758..81a98f1 100644
--- a/t/60core.t
+++ b/t/60core.t
@@ -191,7 +191,7 @@ is($cd->title, 'Spoonful of bees', 'Correct CD returned with include');
is($cd->get_column('artist_name'), 'Caterwauler McCrae', 'Additional column returned');
# update_or_insert
-$new = $schema->resultset("Track")->new( {
+$new = $schema->resultset("Track")->new_result( {
trackid => 100,
cd => 1,
title => 'Insert or Update',
--
1.7.11.1