Starting to fix subquery strategy to accept paginated queries

cakephp · Sep 11, 2014 · ce31b72 · ce31b72
1 parent d64645a
commit ce31b72
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 81 deletions.
diff --git a/src/ORM/Association/SelectableAssociationTrait.php b/src/ORM/Association/SelectableAssociationTrait.php
@@ -15,6 +15,7 @@
 namespace Cake\ORM\Association;
 
 use Cake\Database\Expression\TupleComparison;
+use Cake\Database\Expression\IdentifierExpression;
 
 /**
  * Represents a type of association that that can be fetched using another query
@@ -80,10 +81,7 @@ protected function _buildQuery($options) {
 		$alias = $target->alias();
 		$key = $this->_linkField($options);
 		$filter = $options['keys'];
-
-		if ($options['strategy'] === $this::STRATEGY_SUBQUERY) {
-			$filter = $this->_buildSubquery($options['query']);
-		}
+		$useSubquery = $options['strategy'] === $this::STRATEGY_SUBQUERY;
 
 		$finder = isset($options['finder']) ? $options['finder'] : $this->finder();
 		list($finder, $opts) = $this->_extractFinder($finder);
@@ -92,7 +90,13 @@ protected function _buildQuery($options) {
 			->where($options['conditions'])
 			->eagerLoaded(true)
 			->hydrate($options['query']->hydrate());
-		$fetchQuery = $this->_addFilteringCondition($fetchQuery, $key, $filter);
+
+		if ($useSubquery) {
+			$filter = $this->_buildSubquery($options['query']);
+			$fetchQuery = $this->_addFilteringJoin($fetchQuery, $key, $filter);
+		} else {
+			$fetchQuery = $this->_addFilteringCondition($fetchQuery, $key, $filter);
+		}
 
 		if (!empty($options['fields'])) {
 			$fields = $fetchQuery->aliasFields($options['fields'], $alias);
@@ -119,6 +123,27 @@ protected function _buildQuery($options) {
 		return $fetchQuery;
 	}
 
+	public function _addFilteringJoin($query, $key, $subquery) {
+		$filter = [];
+		$aliasedTable = uniqid();
+
+		foreach ($subquery->clause('select') as $aliasedField => $field) {
+			$filter[] = new IdentifierExpression("$aliasedTable.$aliasedField");
+		}
+
+		if (is_array($key)) {
+			$conditions = $this->_createTupleCondition($query, $key, $filter, '=');
+		} else {
+			$filter = current($filter);
+		}
+
+		$conditions = isset($conditions) ? $conditions : $query->newExpr([$key => $filter]);
+		return $query->innerJoin(
+			[$aliasedTable => $subquery],
+			$conditions
+		);
+	}
+
 /**
  * Appends any conditions required to load the relevant set of records in the
  * target table query given a filter key and some filtering values.
@@ -130,17 +155,22 @@ protected function _buildQuery($options) {
  */
 	protected function _addFilteringCondition($query, $key, $filter) {
 		if (is_array($key)) {
-			$types = [];
-			$defaults = $query->defaultTypes();
-			foreach ($key as $k) {
-				if (isset($defaults[$k])) {
-					$types[] = $defaults[$k];
-				}
-			}
-			return $query->andWhere(new TupleComparison($key, $filter, $types, 'IN'));
+			$conditions = $this->_createTupleCondition($query, $key, $filter, 'IN');
 		}
 
-		return $query->andWhere([$key . ' IN' => $filter]);
+		$conditions = isset($conditions) ? $conditions : [$key . ' IN' => $filter];
+		return $query->andWhere($conditions);
+	}
+
+	protected function _createTupleCondition($query, $keys, $filter, $operator) {
+		$types = [];
+		$defaults = $query->defaultTypes();
+		foreach ($keys as $k) {
+			if (isset($defaults[$k])) {
+				$types[] = $defaults[$k];
+			}
+		}
+		return new TupleComparison($keys, $filter, $types, $operator);
 	}
 
 /**
@@ -161,9 +191,18 @@ protected abstract function _linkField($options);
  * @return \Cake\ORM\Query
  */
 	protected function _buildSubquery($query) {
-		$filterQuery = $query->cleanCopy();
+		$filterQuery = clone $query;
+		$filterQuery->autoFields(false);
+		$filterQuery->mapReduce(null, null, true);
+		$filterQuery->formatResults(null, true);
 		$filterQuery->contain([], true);
 
+		if (!$filterQuery->clause('limit')) {
+			$filterQuery->limit(null);
+			$filterQuery->order([], true);
+			$filterQuery->offset(null);
+		}
+
 		$joins = $filterQuery->join();
 		foreach ($joins as $i => $join) {
 			if (strtolower($join['type']) !== 'inner') {

diff --git a/tests/TestCase/ORM/Association/HasManyTest.php b/tests/TestCase/ORM/Association/HasManyTest.php
@@ -303,72 +303,6 @@ public function testEagerLoaderFieldsException() {
 		]);
 	}
 
-/**
- * Tests eager loading using subquery
- *
- * @return void
- */
-	public function testEagerLoaderSubquery() {
-		$config = [
-			'sourceTable' => $this->author,
-			'targetTable' => $this->article,
-		];
-		$association = new HasMany('Articles', $config);
-		$parent = (new Query(null, $this->author))
-			->join(['foo' => ['table' => 'foo', 'type' => 'inner', 'conditions' => []]])
-			->join(['bar' => ['table' => 'bar', 'type' => 'left', 'conditions' => []]]);
-
-		$query = $this->getMock(
-			'Cake\ORM\Query',
-			['all', 'where', 'andWhere', 'order', 'select', 'contain'],
-			[null, null]
-		);
-
-		$this->article->expects($this->once())->method('find')->with('all')
-			->will($this->returnValue($query));
-		$results = [
-			['id' => 1, 'title' => 'article 1', 'author_id' => 2],
-			['id' => 2, 'title' => 'article 2', 'author_id' => 1]
-		];
-		$query->expects($this->once())->method('all')
-			->will($this->returnValue($results));
-
-		$query->expects($this->at(0))->method('where')
-			->with([])
-			->will($this->returnSelf());
-		$query->expects($this->at(1))->method('where')
-			->with([])
-			->will($this->returnSelf());
-
-		$expected = clone $parent;
-		$joins = $expected->join();
-		unset($joins['bar']);
-		$expected
-			->contain([], true)
-			->select(['Authors__id' => 'Authors.id'], true)
-			->join($joins, [], true);
-		$query->expects($this->once())->method('andWhere')
-			->with(['Articles.author_id IN' => $expected])
-			->will($this->returnSelf());
-
-		$callable = $association->eagerLoader([
-			'query' => $parent, 'strategy' => HasMany::STRATEGY_SUBQUERY, 'keys' => []
-		]);
-		$row = ['Authors__id' => 1, 'username' => 'author 1'];
-		$result = $callable($row);
-		$row['Articles'] = [
-			['id' => 2, 'title' => 'article 2', 'author_id' => 1]
-		];
-		$this->assertEquals($row, $result);
-
-		$row = ['Authors__id' => 2, 'username' => 'author 2'];
-		$result = $callable($row);
-		$row['Articles'] = [
-			['id' => 1, 'title' => 'article 1', 'author_id' => 2]
-		];
-		$this->assertEquals($row, $result);
-	}
-
 /**
  * Tests that eager loader accepts a queryBuilder option
  *