Most of the code reviewed... Missing ResultSet->populate, ResultSet->find, Row->copy...
Daniel Ruoso [Wed, 2 Jun 2010 17:27:37 +0000 (17:27 +0000)]
lib/DBIx/Class/Relationship/Base.pm
lib/DBIx/Class/Relationship/ManyToMany.pm
lib/DBIx/Class/ResultSource.pm
t/lib/DBICTest/Schema/Artist.pm
t/lib/DBICTest/Schema/Track.pm

index 6c64754..c23ebdf 100644 (file)
@@ -263,7 +263,13 @@ sub related_resultset {
 
     # condition resolution may fail if an incomplete master-object prefetch
     # is encountered - that is ok during prefetch construction (not yet in_storage)
-    my $cond = try {
+
+    # if $rel_info->{cond} is a CODE, we might need to join from the
+    # current resultsource instead of just querying the target
+    # resultsource, in that case, the condition might provide an
+    # additional condition in order to avoid an unecessary join if
+    # that is at all possible.
+    my ($cond, $cond2) = try {
       $source->_resolve_condition( $rel_info->{cond}, $rel, $self )
     }
     catch {
@@ -300,10 +306,31 @@ sub related_resultset {
         }
       } @$cond ];
     } elsif (ref $cond eq 'HASH') {
-      foreach my $key (grep { ! /\./ } keys %$cond) {
-        $cond->{"me.$key"} = delete $cond->{$key};
+
+      # this is where we're going to check if we have an extended
+      # rel. In that case, we need to: 1) If there's a second
+      # condition, we use that instead.  2) If there is only one
+      # condition, we need to join the current resultsource and have
+      # additional conditions.
+      if (ref $rel_info->{cond} eq 'CODE') {
+        # this is an extended relationship.
+        if ($cond2) {
+          $cond = $cond2;
+        } else {
+          if (exists $attrs->{join} && $attrs->{join}) {
+            # it's a bit hard to find out what to do here.
+            $self->throw_exception('Extended relationship '.$rel.' with additional join not supported');
+          } else {
+            $attrs->{join} = $rel;
+          }
+        }
+      } else {
+        foreach my $key (grep { ! /\./ } keys %$cond) {
+          $cond->{"me.$key"} = delete $cond->{$key};
+        }
       }
     }
+
     $query = ($query ? { '-and' => [ $cond, $query ] } : $cond);
     $self->result_source->related_source($rel)->resultset->search(
       $query, $attrs
@@ -485,9 +512,16 @@ sub set_from_related {
     $self->throw_exception( "Object $f_obj isn't a ".$f_class )
       unless blessed $f_obj and $f_obj->isa($f_class);
   }
-  $self->set_columns(
-    $self->result_source->_resolve_condition(
-       $rel_info->{cond}, $f_obj, $rel));
+
+  # _resolve_condition might return two hashrefs, specially in the
+  # current case, since we know $f_object is an object.
+  my ($condref1, $condref2) = $self->result_source->_resolve_condition
+    ($rel_info->{cond}, $f_obj, $rel);
+
+  # if we get two condrefs, we need to use the second, otherwise we
+  # use the first.
+  $self->set_columns($condref2 ? $condref2 : $condref1);
+
   return 1;
 }
 
index 0b4ad56..6e507c7 100644 (file)
@@ -133,9 +133,20 @@ EOW
         unless blessed ($obj);
       my $rel_source = $self->search_related($rel)->result_source;
       my $cond = $rel_source->relationship_info($f_rel)->{cond};
-      my $link_cond = $rel_source->_resolve_condition(
-        $cond, $obj, $f_rel
-      );
+      my $link_cond;
+      if (ref $cond eq 'CODE') {
+        my ($cond1, $cond2) = $rel_source->_resolve_condition
+          ($cond, $obj, $f_rel);
+        if ($cond2) {
+          $link_cond = $cond2;
+        } else {
+          $self->throw_exception('Extended relationship '.$rel.
+                                 ' requires optimized version for ManyToMany.');
+        }
+      } else {
+        $link_cond = $rel_source->_resolve_condition
+          ($cond, $obj, $f_rel);
+      }
       $self->search_related($rel, $link_cond)->delete;
     };
 
index fecbbb7..9d5f0dc 100644 (file)
@@ -1568,12 +1568,7 @@ sub _resolve_condition {
       $self->throw_exception ('Unable to determine relationship name for condition resolution');
     }
 
-    $cond = $cond->(
-      $for,
-      ref $for ? 'me' : $as,
-      $self,
-      $rel,
-    );
+    return $cond->($for, ref $for ? 'me' : $as, $self, $rel, ref $for ? $for : undef);
 
   } elsif (ref $cond eq 'HASH') {
     my %ret;
index af6257c..f519fe4 100644 (file)
@@ -49,13 +49,18 @@ __PACKAGE__->has_many(
 __PACKAGE__->has_many(
     cds_80s => 'DBICTest::Schema::CD',
     sub {
-      my ( $me, $as, $self_rsrc, $rel ) = @_;
-      return {
-        "${as}.artist" => (ref $me ? $me->artistid : { '=' => \"${me}.artistid"}),
-        "${as}.year"   => { '>', "1979",
-                            '<', "1990" }
-      };
-    }
+      my ( $me_alias, $rel_alias, $me_result_source, $rel_name, $optional_me_object ) = @_;
+      return
+        ({ "${rel_alias}.artist"  => { '=' => \"${me_alias}.artistid"},
+           "${rel_alias}.year"    => { '>', "1979",
+                                       '<', "1990" }
+         },
+         $optional_me_object &&
+         { "${rel_alias}.artist" => $optional_me_object->artistid,
+           "${rel_alias}.year"   => { '>', "1979",
+                                      '<', "1990" }
+       });
+  }
 );
 
 
index f9cbcc9..c27ba98 100644 (file)
@@ -67,11 +67,15 @@ __PACKAGE__->might_have (
     'next_track',
     __PACKAGE__,
     sub {
-        my ( $me, $as, $self_rsrc, $rel_name ) = @_;
-        return {
-            "${as}.cd" => (ref $me ? $me->cd : { '=' => \"${me}.cd" }),
-            "${as}.position" => { '>', (ref $me ? $me->position : \"${me}.position" )},
-        };
+        my ( $me_alias, $rel_alias, $me_result_source, $rel_name, $optional_me_object ) = @_;
+        return
+          ({ "${rel_alias}.cd"       => { '=', \"${me_alias}.cd" },
+             "${rel_alias}.position" => { '>', \"${me_alias}.position" },
+           },
+           $optional_me_object &&
+           { "${rel_alias}.cd"       => $optional_me_object->cd,
+             "${rel_alias}.position" => { '>', $optional_me_object->position },
+           });
     },
 );