move join inference cleverness into a role
Matt S Trout [Tue, 19 Nov 2013 05:16:43 +0000 (05:16 +0000)]
lib/DBIx/Class/ResultSet.pm
lib/DBIx/Class/ResultSet/Role/DQMethods.pm [new file with mode: 0644]
lib/DBIx/Class/ResultSet/WithDQMethods.pm [new file with mode: 0644]
t/dq/join.t
t/prefetch/with_limit.t

index 346eb39..6019d7c 100644 (file)
@@ -399,14 +399,7 @@ sub search_rs {
   }
 
   if (blessed($call_cond) and $call_cond->isa('Data::Query::ExprBuilder')) {
-    my ($mapped_expr, $extra_join)
-      = $self->_remap_identifiers($call_cond->{expr});
-    $call_cond = \$mapped_expr;
-    if (@$extra_join) {
-      $self->throw_exception("Can't handle join-requiring DQ expr when join attribute specified")
-        if $call_attrs->{join};
-      $call_attrs->{join} = $extra_join;
-    }
+    $call_cond = \$call_cond->{expr};
   }
 
   # see if we can keep the cache (no $rs changes)
@@ -500,44 +493,6 @@ sub search_rs {
   return $rs;
 }
 
-sub _remap_identifiers {
-  my ($self, $dq) = @_;
-  my $map = {};
-  my $attrs = $self->_resolved_attrs;
-  foreach my $j ( @{$attrs->{from}}[1 .. $#{$attrs->{from}} ] ) {
-    next unless $j->[0]{-alias};
-    next unless $j->[0]{-join_path};
-    my $p = $map;
-    $p = $p->{$_} ||= {} for map { keys %$_ } @{$j->[0]{-join_path}};
-    $p->{''} = $j->[0]{-alias};
-  }
-
-  my $seen_join = { %{$attrs->{seen_join}||{}} };
-  my $storage = $self->result_source->storage;
-  my @need_join;
-  my $mapped = map_dq_tree {
-    return $_ unless is_Identifier;
-    my @el = @{$_->{elements}};
-    my $last = pop @el;
-    unless (@el) {
-      return Identifier($attrs->{alias}, $last);
-    }
-    my $p = $map;
-    $p = $p->{$_} ||= {} for @el;
-    if (my $alias = $p->{''}) {
-      return Identifier($alias, $last);
-    }
-    my $need = my $j = {};
-    $j = $j->{$_} = {} for @el;
-    push @need_join, $need;
-    my $alias = $storage->relname_to_table_alias(
-      $el[-1], ++$seen_join->{$el[-1]}
-    );
-    return Identifier($alias, $last);
-  } $dq;
-  return ($mapped, \@need_join);
-}
-
 my $dark_sel_dumper;
 sub _normalize_selection {
   my ($self, $attrs) = @_;
diff --git a/lib/DBIx/Class/ResultSet/Role/DQMethods.pm b/lib/DBIx/Class/ResultSet/Role/DQMethods.pm
new file mode 100644 (file)
index 0000000..77f9b90
--- /dev/null
@@ -0,0 +1,68 @@
+package DBIx::Class::ResultSet::Role::DQMethods;
+
+use Data::Query::ExprHelpers;
+use Safe::Isa;
+use Moo::Role;
+
+sub _dq_converter {
+  shift->result_source->schema->storage->sql_maker->converter;
+}
+
+sub where {
+  my ($self, $where) = @_;
+  if ($where->$_isa('Data::Query::ExprBuilder')) {
+    return $self->_apply_dq_where($where->{expr});
+  } elsif (ref($where) eq 'HASH') {
+    return $self->_apply_dq_where(
+             $self->_dq_converter->_where_to_dq($where)
+           );
+  }
+  die "Argument to ->where must be ExprBuilder or SQL::Abstract hashref, got: "
+      .(defined($where) ? $where : 'undef');
+}
+
+sub _apply_dq_where {
+  my ($self, $expr) = @_;
+  my ($mapped, $need_join) = $self->_remap_identifiers($expr);
+  $self->search_rs(\$mapped, { join => $need_join });
+}
+
+sub _remap_identifiers {
+  my ($self, $dq) = @_;
+  my $map = {};
+  my $attrs = $self->_resolved_attrs;
+  foreach my $j ( @{$attrs->{from}}[1 .. $#{$attrs->{from}} ] ) {
+    next unless $j->[0]{-alias};
+    next unless $j->[0]{-join_path};
+    my $p = $map;
+    $p = $p->{$_} ||= {} for map { keys %$_ } @{$j->[0]{-join_path}};
+    $p->{''} = $j->[0]{-alias};
+  }
+
+  my $seen_join = { %{$attrs->{seen_join}||{}} };
+  my $storage = $self->result_source->storage;
+  my @need_join;
+  my $mapped = map_dq_tree {
+    return $_ unless is_Identifier;
+    my @el = @{$_->{elements}};
+    my $last = pop @el;
+    unless (@el) {
+      return Identifier($attrs->{alias}, $last);
+    }
+    my $p = $map;
+    $p = $p->{$_} ||= {} for @el;
+    if (my $alias = $p->{''}) {
+      return Identifier($alias, $last);
+    }
+    my $need = my $j = {};
+    $j = $j->{$_} = {} for @el;
+    push @need_join, $need;
+    my $alias = $storage->relname_to_table_alias(
+      $el[-1], ++$seen_join->{$el[-1]}
+    );
+    return Identifier($alias, $last);
+  } $dq;
+  return ($mapped, \@need_join);
+}
+
+1;
diff --git a/lib/DBIx/Class/ResultSet/WithDQMethods.pm b/lib/DBIx/Class/ResultSet/WithDQMethods.pm
new file mode 100644 (file)
index 0000000..652d187
--- /dev/null
@@ -0,0 +1,28 @@
+package DBIx::Class::ResultSet::WithDQMethods;
+
+use Scalar::Util qw(blessed);
+use Moo;
+use Moo::Object;
+use namespace::clean;
+
+extends 'DBIx::Class::ResultSet';
+
+with 'DBIx::Class::ResultSet::Role::DQMethods';
+
+sub BUILDARGS {
+  if (@_ <= 3 and blessed($_[1])) { # ->new($source, $attrs?)
+    return $_[2]||{};
+  }
+  return Moo::Object::BUILDARGS(@_);
+}
+
+sub FOREIGNBUILDARGS {
+  if (@_ <= 3 and blessed($_[1])) { # ->new($source, $attrs?)
+    return ($_[1], $_[2]);
+  }
+  my $args = Moo::Object::BUILDARGS(@_);
+  my $source = delete $args->{result_source};
+  return ($source, $args);
+}
+
+1;
index 099e672..125a0a0 100644 (file)
@@ -32,13 +32,16 @@ is($mccrae->cds2->count, 3, 'CDs returned from expr join');
 
 is($mccrae->cds2_pre2k->count, 2, 'CDs returned from expr w/cond');
 
+$schema->source($_)->resultset_class('DBIx::Class::ResultSet::WithDQMethods')
+  for qw(CD Tag);
+
 my $cds = $schema->resultset('CD')
-                 ->search(expr { $_->artist->name eq 'Caterwauler McCrae' });
+                 ->where(expr { $_->artist->name eq 'Caterwauler McCrae' });
 
 is($cds->count, 3, 'CDs via join injection');
 
 my $tags = $schema->resultset('Tag')
-                  ->search(expr { $_->cd->artist->name eq 'Caterwauler McCrae' });
+                  ->where(expr { $_->cd->artist->name eq 'Caterwauler McCrae' });
 
 is($tags->count, 5, 'Tags via two step join injection');
 
index 480dc40..ade0a7f 100644 (file)
@@ -9,6 +9,7 @@ use lib qw(t/lib);
 use DBICTest;
 use DBIC::SqlMakerTest;
 use DBIx::Class::SQLMaker::LimitDialects;
+use Data::Query::ExprDeclare;
 
 my $ROWS = DBIx::Class::SQLMaker::LimitDialects->__rows_bindtype;
 
@@ -155,7 +156,7 @@ throws_ok (
   }, qr/A required group_by clause could not be constructed automatically/,
 ) || exit;
 
-my $artist = $use_prefetch->search({'cds.title' => $artist_many_cds->cds->first->title })->next;
+my $artist = $use_prefetch->search(expr { $_->cds->title eq $artist_many_cds->cds->first->title })->next;
 is($artist->cds->count, 1, "count on search limiting prefetched has_many");
 
 # try with double limit