From: Peter Rabbitson <ribasushi@cpan.org>
Date: Sun, 24 Feb 2013 16:56:53 +0000 (+0100)
Subject: Clarify result_class tweaking on active/cached cursors
X-Git-Tag: v0.08242~16
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=704a0f8ec36f8bd8e273f5ddb553b2f67ea83f06;p=dbsrgits%2FDBIx-Class.git

Clarify result_class tweaking on active/cached cursors

We need this to simplify logic of result_class type caching
---

diff --git a/Changes b/Changes
index c010a79..04dbd47 100644
--- a/Changes
+++ b/Changes
@@ -1,5 +1,11 @@
 Revision history for DBIx::Class
 
+    * New Features / Changes
+        - Changing the result_class of a ResultSet in progress is now
+          explicitly forbidden. The behavior was undefined before, and
+          would result in wildly differing outcomes depending on $rs
+          attributes.
+
 0.08209 2013-03-01 12:56 (UTC)
     * New Features / Changes
         - Debugging aid - warn on invalid result objects created by what
diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm
index c3156ab..6c94721 100644
--- a/lib/DBIx/Class/ResultSet.pm
+++ b/lib/DBIx/Class/ResultSet.pm
@@ -1331,7 +1331,7 @@ sub _construct_objects {
 
       # instead of looping over ->next, use ->all in stealth mode
       # *without* calling a ->reset afterwards
-      # FIXME - encapsulation breach, got to be a better way
+      # FIXME ENCAPSULATION - encapsulation breach, cursor method additions pending
       if (! $cursor->{_done}) {
         $rows = [ ($rows ? @$rows : ()), $cursor->all ];
         $cursor->{_done} = 1;
@@ -1472,14 +1472,19 @@ sub result_class {
   my ($self, $result_class) = @_;
   if ($result_class) {
 
-    unless (ref $result_class) { # don't fire this for an object
-      $self->ensure_class_loaded($result_class);
+    # don't fire this for an object
+    $self->ensure_class_loaded($result_class)
+      unless ref($result_class);
+
+    if ($self->get_cache) {
+      carp_unique('Changing the result_class of a ResultSet instance with cached results is a noop - the cache contents will not be altered');
+    }
+    # FIXME ENCAPSULATION - encapsulation breach, cursor method additions pending
+    elsif ($self->{cursor} && $self->{cursor}{_pos}) {
+      $self->throw_exception('Changing the result_class of a ResultSet instance with an active cursor is not supported');
     }
+
     $self->_result_class($result_class);
-    # THIS LINE WOULD BE A BUG - this accessor specifically exists to
-    # permit the user to set result class on one result set only; it only
-    # chains if provided to search()
-    #$self->{attrs}{result_class} = $result_class if ref $self;
 
     delete $self->{_result_inflator};
   }
diff --git a/t/97result_class.t b/t/97result_class.t
index fe2efe3..faff994 100644
--- a/t/97result_class.t
+++ b/t/97result_class.t
@@ -2,14 +2,13 @@ use strict;
 use warnings;
 
 use Test::More;
+use Test::Warn;
 use Test::Exception;
 use lib qw(t/lib);
 use DBICTest;
 
 my $schema = DBICTest->init_schema();
 
-plan tests => 12;
-
 {
   my $cd_rc = $schema->resultset("CD")->result_class;
 
@@ -61,3 +60,41 @@ plan tests => 12;
   isa_ok(eval{ $cd_rs->find(1) }, $cd_rc, 'Inflated into correct cd result_class');
   isa_ok(eval{ $cd_rs->search({ cdid => 1 })->first }, $cd_rc, 'Inflated into correct cd result_class');
 }
+
+{
+  my $rs = $schema->resultset('Artist')->search(
+    { 'cds.title' => 'Spoonful of bees' },
+    { prefetch => 'cds', result_class => 'DBIx::Class::ResultClass::HashRefInflator' },
+  );
+
+  is ($rs->result_class, 'DBIx::Class::ResultClass::HashRefInflator', 'starting with correct resultclass');
+
+  $rs->result_class('DBICTest::Artist');
+  is ($rs->result_class, 'DBICTest::Artist', 'resultclass changed');
+
+  my $art = $rs->next;
+  is (ref $art, 'DBICTest::Artist', 'Correcty blessed output');
+
+  throws_ok
+    { $rs->result_class('IWillExplode') }
+    qr/\QChanging the result_class of a ResultSet instance with an active cursor is not supported/,
+    'Throws on result class change in progress'
+  ;
+
+  my $cds = $art->cds;
+
+  warnings_exist
+    { $cds->result_class('IWillExplode') }
+    qr/\QChanging the result_class of a ResultSet instance with cached results is a noop/,
+    'Warning on noop result_class change'
+  ;
+
+  is ($cds->result_class, 'IWillExplode', 'class changed anyway');
+
+  # even though the original was HRI (at $rs definition time above)
+  # we lost the control over the *prefetched* object result class
+  # when we handed the root object creation to ::Row::inflate_result
+  is( ref $cds->next, 'DBICTest::CD', 'Correctly inflated prefetched result');
+}
+
+done_testing;
diff --git a/t/inflate/hri.t b/t/inflate/hri.t
index d027e26..eece6df 100644
--- a/t/inflate/hri.t
+++ b/t/inflate/hri.t
@@ -40,6 +40,7 @@ my $schema = DBICTest->init_schema();
       [ sort $rs->result_source->columns ],
       'returned correct columns',
     );
+    $hri_rs->reset;
 
     $cd = $hri_rs->find ({cdid => 1});
     is_deeply ( $cd, $datahashref1, 'first/find return the same thing (result_class attr propagates)');