From: Luke Saunders <luke.saunders@gmail.com>
Date: Mon, 19 Jun 2006 09:58:00 +0000 (+0000)
Subject: changed join merging behaviour. some code cleanup.
X-Git-Tag: v0.07002~75^2~101
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=e083fb1e7cadd3aab6ee13e6bcc89ea2d16a5320;p=dbsrgits%2FDBIx-Class.git

changed join merging behaviour. some code cleanup.
---

diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm
index a216589..6b57676 100644
--- a/lib/DBIx/Class/ResultSet.pm
+++ b/lib/DBIx/Class/ResultSet.pm
@@ -165,9 +165,10 @@ sub search_rs {
   my $our_attrs = ($attrs->{_parent_attrs})
     ? { %{$attrs->{_parent_attrs}} }
       : { %{$self->{attrs}} };
+  delete($attrs->{_parent_attrs}) if(exists $attrs->{_parent_attrs});  
   my $having = delete $our_attrs->{having};
 
-  # XXX this is getting messy
+  # XXX should only maintain _live_join_stack and generate _live_join_h from that  
   if ($attrs->{_live_join_stack}) {
     my $live_join = $attrs->{_live_join_stack};
     foreach (reverse @{$live_join}) {
@@ -175,7 +176,7 @@ sub search_rs {
     }
   }
 
-  # merge new attrs into old
+  # merge new attrs into inherited
   foreach my $key (qw/join prefetch/) {
     next unless (exists $attrs->{$key});
     if ($attrs->{_live_join_stack} || $our_attrs->{_live_join_stack}) {
@@ -195,12 +196,12 @@ sub search_rs {
   }
 
   $our_attrs->{join} = $self->_merge_attr(
-    $our_attrs->{join}, $attrs->{_live_join_h}, 1
+    $our_attrs->{join}, $attrs->{_live_join_h}
   ) if ($attrs->{_live_join_h});
 
   if (defined $our_attrs->{prefetch}) {
     $our_attrs->{join} = $self->_merge_attr(
-      $our_attrs->{join}, $our_attrs->{prefetch}, 1
+      $our_attrs->{join}, $our_attrs->{prefetch}
     );
   }
 
@@ -244,7 +245,6 @@ sub search_rs {
 
   my $rs = (ref $self)->new($self->result_source, $new_attrs);
   $rs->{_parent_rs} = $self->{_parent_rs} if ($self->{_parent_rs});
-       #XXX - hack to pass through parent of related resultsets
 
   unless (@_) {                 # no search, effectively just a clone
     my $rows = $self->get_cache;
@@ -252,7 +252,6 @@ sub search_rs {
       $rs->set_cache($rows);
     }
   }
-  
   return $rs;
 }
 
@@ -791,45 +790,12 @@ sub _resolve {
         push(@{$attrs->{from}}, $source->resolve_join($p, $attrs->{alias}))
           unless $seen{$p};
       }
-
-      # we're about to resolve_join on the current class, so we need to bring
-      # the joins (which are from the original class) to the right level
-      # XXX the below alg is ridiculous
-      if ($attrs->{_live_join_stack}) {
-      STACK:
-        foreach (@{$attrs->{_live_join_stack}}) {
-          if (ref $p eq 'HASH') {
-            if (exists $p->{$_}) {
-              $p = $p->{$_};
-            } else {
-              $p = undef;
-              last STACK;
-            }
-          } elsif (ref $p eq 'ARRAY') {
-            foreach my $pe (@{$p}) {
-              if ($pe eq $_) {
-                $p = undef;
-                last STACK;
-              }
-              next unless(ref $pe eq 'HASH');
-              next unless(exists $pe->{$_});
-              $p = $pe->{$_};
-              next STACK;
-            }                                           
-            $p = undef;
-            last STACK;
-          } else {
-            $p = undef;
-            last STACK;
-          }
-        }
-      }
-                
+      # bring joins back to level of current class
+      $p = $self->_reduce_joins($p, $attrs) if ($attrs->{_live_join_stack});
       if ($p) {
         my @prefetch = $self->result_source->resolve_prefetch(
           $p, $attrs->{alias}, {}, \@pre_order, $collapse
         );
-                
         push(@{$attrs->{select}}, map { $_->[0] } @prefetch);
         push(@{$attrs->{as}}, map { $_->[1] } @prefetch);
       }
@@ -841,13 +807,13 @@ sub _resolve {
 }
 
 sub _merge_attr {
-  my ($self, $a, $b, $is_prefetch) = @_;
+  my ($self, $a, $b) = @_;
     
   return $b unless $a;
   if (ref $b eq 'HASH' && ref $a eq 'HASH') {
     foreach my $key (keys %{$b}) {
       if (exists $a->{$key}) {
-        $a->{$key} = $self->_merge_attr($a->{$key}, $b->{$key}, $is_prefetch);
+        $a->{$key} = $self->_merge_attr($a->{$key}, $b->{$key});
       } else {
         $a->{$key} = $b->{$key};
       }
@@ -862,12 +828,12 @@ sub _merge_attr {
     foreach ($a, $b) {
       foreach my $element (@{$_}) {
         if (ref $element eq 'HASH') {
-          $hash = $self->_merge_attr($hash, $element, $is_prefetch);
+          $hash = $self->_merge_attr($hash, $element);
         } elsif (ref $element eq 'ARRAY') {
           $array = [@{$array}, @{$element}];
         } else {        
-          if (($b == $_) && $is_prefetch) {
-            $self->_merge_array($array, $element, $is_prefetch);
+          if ($b == $_) {
+            $self->_merge_array($array, $element);
           } else {
             push(@{$array}, $element);
           }
@@ -899,6 +865,37 @@ sub _merge_array {
   }
 }
 
+# bring the joins (which are from the original class) to the level
+# of the current class so that we can resolve them properly
+sub _reduce_joins {
+  my ($self, $p, $attrs) = @_;
+
+ STACK:
+  foreach (@{$attrs->{_live_join_stack}}) {
+    if (ref $p eq 'HASH') {
+      if (exists $p->{$_}) {
+        $p = $p->{$_};
+      } else {
+        return undef;
+      }
+    } elsif (ref $p eq 'ARRAY') {
+      foreach my $pe (@{$p}) {
+        if ($pe eq $_) {
+          return undef;
+        }
+        if ((ref $pe eq 'HASH') && (exists $pe->{$_})) {
+          $p = $pe->{$_};
+          next STACK;
+        }
+      }                                           
+      return undef;
+    } else {
+      return undef;
+    }
+  }
+  return $p;
+}
+
 sub _construct_object {
   my ($self, @row) = @_;
   my @as = @{ $self->{_attrs}{as} };
diff --git a/t/90join_torture.t b/t/90join_torture.t
index d2cde4f..fac8535 100644
--- a/t/90join_torture.t
+++ b/t/90join_torture.t
@@ -4,10 +4,10 @@ use warnings;
 use Test::More;
 use lib qw(t/lib);
 use DBICTest;
-
+use Data::Dumper;
 my $schema = DBICTest->init_schema();
 
-plan tests => 14;
+plan tests => 17;
 
 my @rs1a_results = $schema->resultset("Artist")->search_related('cds', {title => 'Forkful of bees'}, {order_by => 'title'});
 is($rs1a_results[0]->title, 'Forkful of bees', "bare field conditions okay after search related");
@@ -67,9 +67,18 @@ my $prod_map_rs = $schema->resultset("Artist")->find(1)->cds->search_related('cd
 ok($prod_map_rs->next->producer, 'search related with prefetch okay');
 
 my $stupid = $schema->resultset("Artist")->search_related('artist_undirected_maps', {}, { prefetch => 'artist1' })->search_related('mapped_artists')->search_related('cds', {'cds.cdid' => '2'}, { prefetch => 'tracks' });
-#use Data::Dumper; warn Dumper($stupid->{attrs});
 
 my $cd_final = $schema->resultset("Artist")->search_related('artist_undirected_maps', {}, { prefetch => 'artist1' })->search_related('mapped_artists')->search_related('cds', {'cds.cdid' => '2'}, { prefetch => 'tracks' })->first;
 is($cd_final->cdid, '2', 'bonkers search_related-with-join-midway okay');
 
+# should end up with cds and cds_2 joined
+my $merge_rs_1 = $schema->resultset("Artist")->search({ 'cds_2.cdid' => '2' }, { join => ['cds', 'cds'] });
+is(scalar(@{$merge_rs_1->{attrs}->{join}}), 2, 'both joins kept');
+ok($merge_rs_1->next, 'query on double joined rel runs okay');
+
+# should only end up with cds joined
+my $merge_rs_2 = $schema->resultset("Artist")->search({ }, { join => 'cds' })->search({ 'cds.cdid' => '2' }, { join => 'cds' });
+is(scalar(@{$merge_rs_2->{attrs}->{join}}), 1, 'only one join kept when inherited');
+my $merge_rs_2_cd = $merge_rs_2->next;
+
 1;