fix doubling of find_related condition

diff --git a/Changes b/Changes
index 2eb1ca3..4f1167c 100644 (file)
--- a/Changes
+++ b/Changes
@@ -2,6 +2,8 @@ Revision history for DBIx::Class
 
     * New Features / Changes
         - Allow schema cloning to mutate attributes
+        - DBIC now attempts more aggressive de-duplication of where
+          conditions on resultset chaining
 
     * Fixes
         - Fix issue where the query was becoming overly mangled when trying

diff --git a/Makefile.PL b/Makefile.PL
index d7c9816..90da52b 100644 (file)
--- a/Makefile.PL
+++ b/Makefile.PL
@@ -73,6 +73,7 @@ my $runtime_requires = {
   'Path::Class'              => '0.18',
   'Scope::Guard'             => '0.03',
   'SQL::Abstract'            => '1.72',
+  'Test::Deep'               => '0.108',
   'Try::Tiny'                => '0.04',
 
   # XS (or XS-dependent) libs

diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm
index 900355c..7f9174c 100644 (file)
--- a/lib/DBIx/Class/ResultSet.pm
+++ b/lib/DBIx/Class/ResultSet.pm
@@ -8,6 +8,7 @@ use DBIx::Class::Exception;
 use DBIx::Class::ResultSetColumn;
 use Scalar::Util qw/blessed weaken/;
 use Try::Tiny;
+use Test::Deep::NoTest 'eq_deeply';
 
 # not importing first() as it will clash with our own method
 use List::Util ();
@@ -529,17 +530,61 @@ sub _normalize_selection {
 
 sub _stack_cond {
   my ($self, $left, $right) = @_;
+
+  # collapse single element top-level conditions
+  # (single pass only, unlikely to need recursion)
+  for ($left, $right) {
+    if (ref $_ eq 'ARRAY') {
+      if (@$_ == 0) {
+        $_ = undef;
+      }
+      elsif (@$_ == 1) {
+        $_ = $_->[0];
+      }
+    }
+    elsif (ref $_ eq 'HASH') {
+      my ($first, $more) = keys %$_;
+
+      # empty hash
+      if (! defined $first) {
+        $_ = undef;
+      }
+      # one element hash
+      elsif (! defined $more) {
+        if ($first eq '-and' and ref $_->{'-and'} eq 'HASH') {
+          $_ = $_->{'-and'};
+        }
+        elsif ($first eq '-or' and ref $_->{'-or'} eq 'ARRAY') {
+          $_ = $_->{'-or'};
+        }
+      }
+    }
+  }
+
+  # merge hashes with weeding out of duplicates (simple cases only)
+  if (ref $left eq 'HASH' and ref $right eq 'HASH') {
+
+    # shallow copy to destroy
+    $right = { %$right };
+    for (grep { exists $right->{$_} } keys %$left) {
+      # the use of eq_deeply here is justified - the rhs of an
+      # expression can contain a lot of twisted weird stuff
+      delete $right->{$_} if eq_deeply( $left->{$_}, $right->{$_} );
+    }
+
+    $right = undef unless keys %$right;
+  }
+
+
   if (defined $left xor defined $right) {
     return defined $left ? $left : $right;
   }
-  elsif (defined $left) {
-    return { -and => [ map
-      { ref $_ eq 'ARRAY' ? [ -or => $_ ] : $_ }
-      ($left, $right)
-    ]};
+  elsif (! defined $left) {
+    return undef;
+  }
+  else {
+    return { -and => [ $left, $right ] };
   }
-
-  return undef;
 }
 
 =head2 search_literal

diff --git a/t/53lean_startup.t b/t/53lean_startup.t
index d54de0b..19e6945 100644 (file)
--- a/t/53lean_startup.t
+++ b/t/53lean_startup.t
@@ -44,6 +44,8 @@ BEGIN {
 
     Class::Accessor::Grouped
     Class::C3::Componentised
+
+    Test::Deep::NoTest
   /, $] < 5.010 ? 'MRO::Compat' : () };
 
   $test_hook = sub {

diff --git a/t/row/find_one_has_many.t b/t/row/find_one_has_many.t
new file mode 100644 (file)
index 0000000..92c5bd0
--- /dev/null
+++ b/t/row/find_one_has_many.t
@@ -0,0 +1,35 @@
+use strict;
+use warnings;
+
+use Test::More;
+use Test::Exception;
+use lib qw(t/lib);
+use DBICTest;
+use DBIC::DebugObj;
+use DBIC::SqlMakerTest;
+
+my $schema = DBICTest->init_schema();
+
+$schema->resultset('Artist')->delete;
+$schema->resultset('CD')->delete;
+
+my $artist  = $schema->resultset("Artist")->create({ artistid => 21, name => 'Michael Jackson', rank => 20 });
+my $cd = $artist->create_related('cds', { year => 1975, title => 'Compilation from 1975' });
+
+my ($sql, @bind);
+local $schema->storage->{debug} = 1;
+local $schema->storage->{debugobj} = DBIC::DebugObj->new(\$sql, \@bind);
+
+my $find_cd = $artist->find_related('cds',{title => 'Compilation from 1975'});
+
+s/^'//, s/'\z// for @bind; # why does DBIC::DebugObj not do this?
+
+is_same_sql_bind (
+  $sql,
+  \@bind,
+  'SELECT me.cdid, me.artist, me.title, me.year, me.genreid, me.single_track FROM cd me WHERE ( ( me.artist = ? AND me.title = ? ) ) ORDER BY year ASC',
+  [21, 'Compilation from 1975'],
+  'find_related only uses foreign key condition once',
+);
+
+done_testing;