From: Peter Rabbitson <ribasushi@cpan.org>
Date: Fri, 19 Apr 2013 12:32:06 +0000 (+0200)
Subject: Explicitly disallow redirection of has_many columns to main object
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=1e1cea3877c13cf9dcccefe2ba7ac9f089e2d4bc;p=dbsrgits%2FDBIx-Class-Historic.git

Explicitly disallow redirection of has_many columns to main object

This can never work in practice, but it is too obscure to have a user catch it
---

diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm
index af1cbe6..df7b701 100644
--- a/lib/DBIx/Class/ResultSet.pm
+++ b/lib/DBIx/Class/ResultSet.pm
@@ -1346,6 +1346,36 @@ sub _construct_results {
 
   return undef unless @{$rows||[]};
 
+  # sanity check - people are too clever for their own good
+  if ($attrs->{collapse} and my $aliastypes = $attrs->{_sqlmaker_select_args}[3]{_aliastypes} ) {
+
+    my $multiplied_selectors;
+    for my $sel_alias ( grep { $_ ne $attrs->{alias} } keys %{ $aliastypes->{selecting} } ) {
+      if (
+        $aliastypes->{multiplying}{$sel_alias}
+          or
+        scalar grep { $aliastypes->{multiplying}{(values %$_)[0]} } @{ $aliastypes->{selecting}{$sel_alias}{-parents} }
+      ) {
+        $multiplied_selectors->{$_} = 1 for values %{$aliastypes->{selecting}{$sel_alias}{-seen_columns}}
+      }
+    }
+
+    for my $i (0 .. $#{$attrs->{as}} ) {
+      my $sel = $attrs->{select}[$i];
+
+      if (ref $sel eq 'SCALAR') {
+        $sel = $$sel;
+      }
+      elsif( ref $sel eq 'REF' and ref $$sel eq 'ARRAY' ) {
+        $sel = $$sel->[0];
+      }
+
+      $self->throw_exception(
+        'Result collapse not possible - selection from a has_many source redirected to the main object'
+      ) if ($multiplied_selectors->{$sel} and $attrs->{as}[$i] !~ /\./);
+    }
+  }
+
   my @extra_collapser_args;
   if ($attrs->{collapse} and ! $did_fetch_all ) {
 
@@ -3523,9 +3553,6 @@ sub _resolved_attrs {
   if ( List::Util::first { $_ =~ /\./ } @{$attrs->{as}} ) {
     $attrs->{_related_results_construction} = 1;
   }
-  else {
-    $attrs->{collapse} = 0;
-  }
 
   # run through the resulting joinstructure (starting from our current slot)
   # and unset collapse if proven unnesessary
diff --git a/t/prefetch/manual.t b/t/prefetch/manual.t
index 1ad2253..97f45c9 100644
--- a/t/prefetch/manual.t
+++ b/t/prefetch/manual.t
@@ -220,29 +220,6 @@ lives_ok { my $dummy = $rs;  warnings_exist {
 ], 'expected_warnings'
 } 'traversing prefetch chain with empty intermediates works';
 
-TODO: {
-local $TODO = 'this does not work at all, need to promote rsattrs to an object on its own';
-# make sure has_many column redirection does not do weird stuff when collapse is requested
-for my $pref_args (
-  { prefetch => 'cds'},
-  { collapse => 1 }
-) {
-  for my $col_and_join_args (
-    { '+columns' => { 'cd_title' => 'cds_2.title' }, join => [ 'cds', 'cds' ] },
-    { '+columns' => { 'cd_title' => 'cds.title' }, join => 'cds', }
-  ) {
-
-    my $weird_rs = $schema->resultset('Artist')->search({}, {
-      %$col_and_join_args, %$pref_args,
-    });
-
-    for (qw/next all first/) {
-      throws_ok { $weird_rs->$_ } qr/not yet determined exception text/;
-    }
-  }
-}
-}
-
 # multi-has_many with underdefined root, with rather random order
 $rs = $schema->resultset ('CD')->search ({}, {
   join => [ 'tracks', { single_track => { cd => { artist => { cds => 'tracks' } } } }  ],
diff --git a/t/resultset/inflatemap_abuse.t b/t/resultset/inflatemap_abuse.t
index 1be2f69..1645ca1 100644
--- a/t/resultset/inflatemap_abuse.t
+++ b/t/resultset/inflatemap_abuse.t
@@ -70,4 +70,28 @@ throws_ok
   'Correct exception on illegal ::Row inflation attempt'
 ;
 
+# make sure has_many column redirection does not do weird stuff when collapse is requested
+for my $pref_args (
+  { prefetch => 'cds'},
+  { collapse => 1 }
+) {
+  for my $col_and_join_args (
+    { '+columns' => { 'cd_title' => 'cds_2.title' }, join => [ 'cds', 'cds' ] },
+    { '+columns' => { 'cd_title' => 'cds.title' }, join => 'cds' },
+    { '+columns' => { 'cd_gr_name' => 'genre.name' }, join => { cds => 'genre' } },
+  ) {
+    for my $call (qw(next all first)) {
+
+      my $weird_rs = $s->resultset('Artist')->search({}, {
+        %$col_and_join_args, %$pref_args,
+      });
+
+      throws_ok
+        { $weird_rs->$call }
+        qr/\QResult collapse not possible - selection from a has_many source redirected to the main object/
+      for (1,2);
+    }
+  }
+}
+
 done_testing;