Scale back validation of the 'as' attribute (revert 95e41036)

[dbsrgits/DBIx-Class.git] / lib / DBIx / Class / ResultSet.pm

diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm
index 5dc6454..bfd6064 100644 (file)
--- a/lib/DBIx/Class/ResultSet.pm
+++ b/lib/DBIx/Class/ResultSet.pm
@@ -5,7 +5,7 @@ use warnings;
 use base qw/DBIx::Class/;
 use DBIx::Class::Carp;
 use DBIx::Class::ResultSetColumn;
-use Scalar::Util qw/blessed weaken/;
+use Scalar::Util qw/blessed weaken reftype/;
 use Try::Tiny;
 use Data::Compare (); # no imports!!! guard against insane architecture
 
@@ -25,6 +25,10 @@ use overload
         'bool'   => "_bool",
         fallback => 1;
 
+# this is real - CDBICompat overrides it with insanity
+# yes, prototype won't matter, but that's for now ;)
+sub _bool () { 1 }
+
 __PACKAGE__->mk_group_accessors('simple' => qw/_result_class result_source/);
 
 =head1 NAME
@@ -393,11 +397,11 @@ sub search_rs {
   my $cache;
   my %safe = (alias => 1, cache => 1);
   if ( ! List::Util::first { !$safe{$_} } keys %$call_attrs and (
-    ! defined $_[0]
+    ! defined $call_cond
       or
-    ref $_[0] eq 'HASH' && ! keys %{$_[0]}
+    ref $call_cond eq 'HASH' && ! keys %$call_cond
       or
-    ref $_[0] eq 'ARRAY' && ! @{$_[0]}
+    ref $call_cond eq 'ARRAY' && ! @$call_cond
   )) {
     $cache = $self->get_cache;
   }
@@ -1327,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;
@@ -1468,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};
   }
@@ -1682,9 +1691,6 @@ sub _count_subq_rs {
                   ->get_column ('count');
 }
 
-sub _bool {
-  return 1;
-}
 
 =head2 count_literal
 
@@ -2361,15 +2367,29 @@ sub new_result {
 
   my ($merged_cond, $cols_from_relations) = $self->_merge_with_rscond($values);
 
-  my %new = (
+  my $new = $self->result_class->new({
     %$merged_cond,
-    @$cols_from_relations
+    ( @$cols_from_relations
       ? (-cols_from_relations => $cols_from_relations)
-      : (),
+      : ()
+    ),
     -result_source => $self->result_source, # DO NOT REMOVE THIS, REQUIRED
-  );
+  });
 
-  return $self->result_class->new(\%new);
+  if (
+    reftype($new) eq 'HASH'
+      and
+    ! keys %$new
+      and
+    blessed($new)
+  ) {
+    carp_unique (sprintf (
+      "%s->new returned a blessed empty hashref - a strong indicator something is wrong with its inheritance chain",
+      $self->result_class,
+    ));
+  }
+
+  $new;
 }
 
 # _merge_with_rscond
@@ -3318,40 +3338,6 @@ sub _chain_relationship {
   return {%$attrs, from => $from, seen_join => $seen};
 }
 
-# FIXME - this needs to go live in Schema with the tree walker... or
-# something
-my $inflatemap_checker;
-$inflatemap_checker = sub {
-  my ($rsrc, $relpaths) = @_;
-
-  my $rels;
-
-  for (@$relpaths) {
-    $_ =~ /^ ( [^\.]+ ) \. (.+) $/x
-      or next;
-
-    push @{$rels->{$1}}, $2;
-  }
-
-  for my $rel (keys %$rels) {
-    my $rel_rsrc = try {
-      $rsrc->related_source ($rel)
-    } catch {
-    $rsrc->throw_exception(sprintf(
-      "Inflation into non-existent relationship '%s' of '%s' requested, "
-    . "check the inflation specification (columns/as) ending in '...%s.%s'",
-      $rel,
-      $rsrc->source_name,
-      $rel,
-      ( sort { length($a) <=> length ($b) } @{$rels->{$rel}} )[0],
-    ))};
-
-    $inflatemap_checker->($rel_rsrc, $rels->{$rel});
-  }
-
-  return;
-};
-
 sub _resolved_attrs {
   my $self = shift;
   return $self->{_attrs} if $self->{_attrs};
@@ -3423,14 +3409,6 @@ sub _resolved_attrs {
     }
   }
 
-  # validate the user-supplied 'as' chain
-  # folks get too confused by the (logical) exception message, need to
-  # go to some lengths to clarify the text
-  #
-  # FIXME - this needs to go live in Schema with the tree walker... or
-  # something
-  $inflatemap_checker->($source, \@as);
-
   $attrs->{select} = \@sel;
   $attrs->{as} = \@as;