Tighten up value inferrence in relationship resolution
[dbsrgits/DBIx-Class.git] / lib / DBIx / Class / ResultSource.pm
index c598b1d..8b01a88 100644 (file)
@@ -4,7 +4,7 @@ package DBIx::Class::ResultSource;
 #
 # Some of the methods defined here will be around()-ed by code at the
 # end of ::ResultSourceProxy. The reason for this strange arrangement
-# is that the list of around()s of methods in this # class depends
+# is that the list of around()s of methods in this class depends
 # directly on the list of may-not-be-defined-yet methods within
 # ::ResultSourceProxy itself.
 # If this sounds terrible - it is. But got to work with what we have.
@@ -22,6 +22,7 @@ use DBIx::Class::_Util qw(
   refdesc emit_loud_diag
 );
 use DBIx::Class::SQLMaker::Util qw( normalize_sqla_condition extract_equality_conditions );
+use DBIx::Class::ResultSource::FromSpec::Util 'fromspec_columns_info';
 use SQL::Abstract 'is_literal_value';
 use Devel::GlobalDestruction;
 use Scalar::Util qw( blessed weaken isweak refaddr );
@@ -31,6 +32,13 @@ use DBIx::Class::ResultSet;
 
 use namespace::clean;
 
+# This global is present for the afaik nonexistent, but nevertheless possible
+# case of folks using stock ::ResultSet with a completely custom Result-class
+# hierarchy, not derived from DBIx::Class::Row at all
+# Instead of patching stuff all over the place - this would be one convenient
+# place to override things if need be
+our $__expected_result_class_isa = 'DBIx::Class::Row';
+
 my @hashref_attributes = qw(
   source_info resultset_attributes
   _columns _unique_constraints _relationships
@@ -2265,16 +2273,19 @@ sub _resolve_relationship_condition {
 
   $args->{require_join_free_condition} ||= !!$args->{infer_values_based_on};
 
-  $self->throw_exception( "Argument 'self_result_object' must be an object inheriting from DBIx::Class::Row" )
+  $self->throw_exception( "Argument 'self_result_object' must be an object inheriting from '$__expected_result_class_isa'" )
     if (
       exists $args->{self_result_object}
         and
-      ( ! defined blessed $args->{self_result_object} or ! $args->{self_result_object}->isa('DBIx::Class::Row') )
+      (
+        ! defined blessed $args->{self_result_object}
+          or
+        ! $args->{self_result_object}->isa( $__expected_result_class_isa )
+      )
     )
   ;
 
   my $rel_rsrc = $self->related_source($args->{rel_name});
-  my $storage = $self->schema->storage;
 
   if (exists $args->{foreign_values}) {
 
@@ -2284,8 +2295,8 @@ sub _resolve_relationship_condition {
     }
     elsif (defined blessed $args->{foreign_values}) {
 
-      $self->throw_exception( "Objects supplied as 'foreign_values' ($args->{foreign_values}) must inherit from DBIx::Class::Row" )
-        unless $args->{foreign_values}->isa('DBIx::Class::Row');
+      $self->throw_exception( "Objects supplied as 'foreign_values' ($args->{foreign_values}) must inherit from '$__expected_result_class_isa'" )
+        unless $args->{foreign_values}->isa( $__expected_result_class_isa );
 
       carp_unique(
         "Objects supplied as 'foreign_values' ($args->{foreign_values}) "
@@ -2399,11 +2410,9 @@ sub _resolve_relationship_condition {
       ) for keys %$jfc;
 
       (
-        length ref $_
-          and
         defined blessed($_)
           and
-        $_->isa('DBIx::Class::Row')
+        $_->isa( $__expected_result_class_isa )
           and
         $self->throw_exception (
           "The join-free condition returned for $exception_rel_id may not "
@@ -2486,12 +2495,12 @@ sub _resolve_relationship_condition {
         $ret = $subconds[0];
       }
       else {
-        # we are discarding inferred values here... likely incorrect...
-        # then again - the entire thing is an OR, so we *can't* use them anyway
         for my $subcond ( @subconds ) {
           $self->throw_exception('Either all or none of the OR-condition members must resolve to a join-free condition')
             if ( $ret and ( $ret->{join_free_condition} xor $subcond->{join_free_condition} ) );
 
+          # we are discarding inferred_values from individual 'OR' branches here
+          # see @nonvalues checks below
           $subcond->{$_} and push @{$ret->{$_}}, $subcond->{$_} for (qw(condition join_free_condition));
         }
       }
@@ -2526,31 +2535,43 @@ sub _resolve_relationship_condition {
 
     my $jfc_eqs = extract_equality_conditions( $jfc, 'consider_nulls' );
 
-    if (keys %$jfc_eqs) {
-
-      for (keys %$jfc) {
+    for (keys %$jfc) {
+      if( $_ =~ /^-/ ) {
+        push @nonvalues, { $_ => $jfc->{$_} };
+      }
+      else {
         # $jfc is fully qualified by definition
-        my ($col) = $_ =~ /\.(.+)/;
+        my ($col) = $_ =~ /\.(.+)/ or carp_unique(
+          'Internal error - extract_equality_conditions() returned a '
+        . "non-fully-qualified key '$_'. *Please* file a bugreport "
+        . "including your definition of $exception_rel_id"
+        );
 
         if (exists $jfc_eqs->{$_} and ($jfc_eqs->{$_}||'') ne UNRESOLVABLE_CONDITION) {
           $ret->{inferred_values}{$col} = $jfc_eqs->{$_};
         }
         elsif ( !$args->{infer_values_based_on} or ! exists $args->{infer_values_based_on}{$col} ) {
-          push @nonvalues, $col;
+          push @nonvalues, { $_ => $jfc->{$_} };
         }
       }
-
-      # all or nothing
-      delete $ret->{inferred_values} if @nonvalues;
     }
+
+    # all or nothing
+    delete $ret->{inferred_values} if @nonvalues;
   }
 
   # did the user explicitly ask
   if ($args->{infer_values_based_on}) {
 
     $self->throw_exception(sprintf (
-      "Unable to complete value inferrence - custom $exception_rel_id returns conditions instead of values for column(s): %s",
-      map { "'$_'" } @nonvalues
+      "Unable to complete value inferrence - $exception_rel_id results in expression(s) instead of definitive values: %s",
+      do {
+        # FIXME - used for diag only, but still icky
+        my $sqlm = $self->schema->storage->sql_maker;
+        local $sqlm->{quote_char};
+        local $sqlm->{_dequalify_idents} = 1;
+        ($sqlm->_recurse_where({ -and => \@nonvalues }))[0]
+      }
     )) if @nonvalues;
 
 
@@ -2574,7 +2595,7 @@ sub _resolve_relationship_condition {
       # there is no way to know who is right and who is left in a cref
       # therefore a full blown resolution call, and figure out the
       # direction a bit further below
-      $colinfos ||= $storage->_resolve_column_info([
+      $colinfos ||= fromspec_columns_info([
         { -alias => $args->{self_alias}, -rsrc => $self },
         { -alias => $args->{foreign_alias}, -rsrc => $rel_rsrc },
       ]);