From: Peter Rabbitson <ribasushi@cpan.org>
Date: Tue, 29 Jul 2014 01:06:09 +0000 (+0200)
Subject: Properly throw on FC with find (it can never work anyway)
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=7ed4b48f691b78a3d832266d3a253a4d5c6a4837;p=dbsrgits%2FDBIx-Class-Historic.git

Properly throw on FC with find (it can never work anyway)

This is the proper solution for RT#95054
---

diff --git a/Changes b/Changes
index fc72d6b..6024523 100644
--- a/Changes
+++ b/Changes
@@ -5,6 +5,8 @@ Revision history for DBIx::Class
           like the rest of DBIC
         - DBIC::FilterColumn "from_storage" handler is now invoked on NULLs
           returned from storage
+        - find() now throws an exception if some of the supplied values are
+          managed by DBIC::FilterColumn (RT#95054)
         - Custom condition relationships are now invoked with a slightly
           different signature (existing coderefs will continue to work)
         - Add extra custom condition coderef attribute 'foreign_values'
diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm
index d9d8959..2a08b65 100644
--- a/lib/DBIx/Class/ResultSet.pm
+++ b/lib/DBIx/Class/ResultSet.pm
@@ -870,7 +870,7 @@ sub find {
     # relationship
   }
   else {
-    my (@unique_queries, %seen_column_combinations);
+    my (@unique_queries, %seen_column_combinations, $ci, @fc_exceptions);
 
     # no key was specified - fall down to heuristics mode:
     # run through all unique queries registered on the resultset, and
@@ -887,17 +887,25 @@ sub find {
         join "\x00", sort $rsrc->unique_constraint_columns($c_name)
       }++;
 
-      push @unique_queries, try {
-        $self->result_source->_minimal_valueset_satisfying_constraint(
-          constraint_name => $c_name,
-          values => ($self->_merge_with_rscond($call_cond))[0],
-        ),
-      } || ();
+      try {
+        push @unique_queries, $self->_qualify_cond_columns(
+          $self->result_source->_minimal_valueset_satisfying_constraint(
+            constraint_name => $c_name,
+            values => ($self->_merge_with_rscond($call_cond))[0],
+            columns_info => ($ci ||= $self->result_source->columns_info),
+          ),
+          $alias
+        );
+      }
+      catch {
+        push @fc_exceptions, $_ if $_ =~ /\bFilterColumn\b/;
+      };
     }
 
-    $final_cond = @unique_queries
-      ? [ map { $self->_qualify_cond_columns($_, $alias) } @unique_queries ]
-      : $self->_non_unique_find_fallback ($call_cond, $attrs)
+    $final_cond =
+        @unique_queries   ? \@unique_queries
+      : @fc_exceptions    ? $self->throw_exception(join "; ", map { $_ =~ /(.*) at .+ line \d+$/s } @fc_exceptions )
+      :                     $self->_non_unique_find_fallback ($call_cond, $attrs)
     ;
   }
 
diff --git a/lib/DBIx/Class/ResultSource.pm b/lib/DBIx/Class/ResultSource.pm
index 47fa905..262e2d8 100644
--- a/lib/DBIx/Class/ResultSource.pm
+++ b/lib/DBIx/Class/ResultSource.pm
@@ -1556,6 +1556,8 @@ sub _minimal_valueset_satisfying_constraint {
   my $self = shift;
   my $args = { ref $_[0] eq 'HASH' ? %{ $_[0] } : @_ };
 
+  $args->{columns_info} ||= $self->columns_info;
+
   my $vals = $self->storage->_extract_fixed_condition_columns(
     $args->{values},
     ($args->{carp_on_nulls} ? 'consider_nulls' : undef ),
@@ -1572,6 +1574,12 @@ sub _minimal_valueset_satisfying_constraint {
     else {
       $cols->{present}{$col} = 1;
     }
+
+    $cols->{fc}{$col} = 1 if (
+      ! ( $cols->{missing} || {})->{$col}
+        and
+      $args->{columns_info}{$col}{_filter_info}
+    );
   }
 
   $self->throw_exception( sprintf ( "Unable to satisfy requested constraint '%s', missing values for column(s): %s",
@@ -1579,6 +1587,12 @@ sub _minimal_valueset_satisfying_constraint {
     join (', ', map { "'$_'" } sort keys %{$cols->{missing}} ),
   ) ) if $cols->{missing};
 
+  $self->throw_exception( sprintf (
+    "Unable to satisfy requested constraint '%s', FilterColumn values not usable for column(s): %s",
+    $args->{constraint_name},
+    join (', ', map { "'$_'" } sort keys %{$cols->{fc}}),
+  )) if $cols->{fc};
+
   if (
     $cols->{undefined}
       and
diff --git a/t/row/filter_column.t b/t/row/filter_column.t
index 26631b4..10ac1a4 100644
--- a/t/row/filter_column.t
+++ b/t/row/filter_column.t
@@ -259,4 +259,18 @@ throws_ok { DBICTest::Schema::Artist->filter_column( charfield => {} ) }
   'Correctly throws exception for empty attributes'
 ;
 
+FC_ON_PK_TEST: {
+
+  DBICTest::Schema::Artist->filter_column(artistid => {
+    filter_to_storage => sub { $_[1] * 100 },
+    filter_from_storage => sub { $_[1] - 100 },
+  });
+
+  for my $key ('', 'primary') {
+    throws_ok {
+      $schema->resultset('Artist')->find_or_create({ artistid => 42 }, { $key ? ( key => $key ) : () });
+    } qr/\QUnable to satisfy requested constraint 'primary', FilterColumn values not usable for column(s): 'artistid'/;
+  }
+}
+
 done_testing;