Merge pull request #5410 from cakephp/3.0-better-matching

3.0 - Allow matching() and contain() on the same alias

src/ORM/Association.php

@@ -664,20 +664,26 @@ abstract class Association {
  * @return void
  */
 	protected function _bindNewAssociations($query, $surrogate, $options) {
-		$contain = $surrogate->contain();
+		$loader = $surrogate->eagerLoader();
+		$contain = $loader->contain();
+		$matching = $loader->matching();
 		$target = $this->_targetTable;
 
-		if (!$contain) {
+		if (!$contain && !$matching) {
 			return;
 		}
 
-		$loader = $surrogate->eagerLoader();
 		$loader->attachAssociations($query, $target, $options['includeFields']);
-		$newBinds = [];
+		$newContain = [];
 		foreach ($contain as $alias => $value) {
-			$newBinds[$options['aliasPath'] . '.' . $alias] = $value;
+			$newContain[$options['aliasPath'] . '.' . $alias] = $value;
+		}
+
+		$query->contain($newContain);
+
+		foreach ($matching as $alias => $value) {
+			$query->matching($options['aliasPath'] . '.' . $alias, $value['queryBuilder']);
 		}
-		$query->contain($newBinds);
 	}
 
 /**

src/ORM/Association/BelongsToMany.php

@@ -233,9 +233,9 @@ class BelongsToMany extends Association {
 		unset($options['queryBuilder']);
 		$options = ['conditions' => [$cond]] + compact('includeFields');
 		$options['foreignKey'] = $this->targetForeignKey();
-		$this->_targetTable
-			->association($junction->alias())
-			->attachTo($query, $options);
+		$assoc = $this->_targetTable->association($junction->alias());
+		$assoc->attachTo($query, $options);
+		$query->eagerLoader()->addToJoinsMap($junction->alias(), $assoc, true);
 	}
 
 /**
@@ -868,7 +868,9 @@ class BelongsToMany extends Association {
 /**
  * Auxiliary function to construct a new Query object to return all the records
  * in the target table that are associated to those specified in $options from
- * the source table
+ * the source table.
+ *
+ * This is used for eager loading records on the target table based on conditions.
  *
  * @param array $options options accepted by eagerLoader()
  * @return \Cake\ORM\Query
@@ -888,8 +890,14 @@ class BelongsToMany extends Association {
 			]
 		];
 
-		$joins = $matching + $joins;
-		$query->join($joins, [], true)->matching($name);
+		$assoc = $this->target()->association($name);
+		$query
+			->join($matching + $joins, [], true)
+			->autoFields($query->clause('select') === [])
+			->select($query->aliasFields((array)$assoc->foreignKey(), $name));
+
+		$query->eagerLoader()->addToJoinsMap($name, $assoc);
+		$assoc->attachTo($query);
 		return $query;
 	}
 

src/ORM/EagerLoader.php

@@ -16,6 +16,7 @@ namespace Cake\ORM;
 
 use Cake\Database\Statement\BufferedStatement;
 use Cake\Database\Statement\CallbackStatement;
+use Cake\ORM\Association;
 use Cake\ORM\Query;
 use Cake\ORM\Table;
 use Closure;
@@ -78,6 +79,21 @@ class EagerLoader {
 	protected $_aliasList = [];
 
 /**
+ * Another EagerLoader instance that will be used for 'matching' associations.
+ *
+ * @var \Cake\ORM\EagerLoader
+ */
+	protected $_matching;
+
+/**
+ * A map of table aliases pointing to the association objects they represent
+ * for the query.
+ *
+ * @var array
+ */
+	protected $_joinsMap = [];
+
+/**
  * Sets the list of associations that should be eagerly loaded along for a
  * specific table using when a query is provided. The list of associated tables
  * passed to this method must have been previously set as associations using the
@@ -121,12 +137,23 @@ class EagerLoader {
  * parameter, this will translate in setting all those associations with the
  * `matching` option.
  *
- * @param string $assoc A single association or a dot separated path of associations.
+ * If called with no arguments it will return the current tree of associations to
+ * be matched.
+ *
+ * @param string|null $assoc A single association or a dot separated path of associations.
  * @param callable|null $builder the callback function to be used for setting extra
  * options to the filtering query
  * @return array The resulting containments array
  */
-	public function matching($assoc, callable $builder = null) {
+	public function matching($assoc = null, callable $builder = null) {
+		if ($this->_matching === null) {
+			$this->_matching = new self();
+		}
+
+		if ($assoc === null) {
+			return $this->_matching->contain();
+		}
+
 		$assocs = explode('.', $assoc);
 		$last = array_pop($assocs);
 		$containments = [];
@@ -138,7 +165,7 @@ class EagerLoader {
 		}
 
 		$pointer[$last] = ['queryBuilder' => $builder, 'matching' => true];
-		return $this->contain($containments);
+		return $this->_matching->contain($containments);
 	}
 
 /**
@@ -242,7 +269,7 @@ class EagerLoader {
  * @return void
  */
 	public function attachAssociations(Query $query, Table $repository, $includeFields) {
-		if (empty($this->_containments)) {
+		if (empty($this->_containments) && $this->_matching === null) {
 			return;
 		}
 
@@ -270,7 +297,8 @@ class EagerLoader {
  */
 	public function attachableAssociations(Table $repository) {
 		$contain = $this->normalized($repository);
-		return $this->_resolveJoins($contain);
+		$matching = $this->_matching ? $this->_matching->normalized($repository) : [];
+		return $this->_resolveJoins($contain, $matching);
 	}
 
 /**
@@ -290,7 +318,7 @@ class EagerLoader {
 		}
 
 		$contain = $this->normalized($repository);
-		$this->_resolveJoins($contain);
+		$this->_resolveJoins($contain, []);
 		return $this->_loadExternal;
 	}
 
@@ -391,13 +419,6 @@ class EagerLoader {
 			return $config;
 		}
 
-		if (!empty($config['config']['matching'])) {
-			throw new \RuntimeException(sprintf(
-				'Cannot use "matching" on "%s" as there is another association with the same alias',
-				$alias
-			));
-		}
-
 		$config['canBeJoined'] = false;
 		$config['config']['strategy'] = $config['instance']::STRATEGY_SELECT;
 	}
@@ -406,16 +427,23 @@ class EagerLoader {
  * Helper function used to compile a list of all associations that can be
  * joined in the query.
  *
- * @param array $associations list of associations for $source
+ * @param array $associations list of associations from which to obtain joins.
+ * @param array $matching list of associations that should be forcibly joined.
  * @return array
  */
-	protected function _resolveJoins($associations) {
+	protected function _resolveJoins($associations, $matching = []) {
 		$result = [];
+		foreach ($matching as $table => $options) {
+			$result[$table] = $options;
+			$result += $this->_resolveJoins($options['associations'], []);
+		}
 		foreach ($associations as $table => $options) {
-			if ($options['canBeJoined']) {
+			$inMatching = isset($matching[$table]);
+			if (!$inMatching && $options['canBeJoined']) {
 				$result[$table] = $options;
-				$result += $this->_resolveJoins($options['associations']);
+				$result += $this->_resolveJoins($options['associations'], $inMatching ? $mathching[$table] : []);
 			} else {
+				$options['canBeJoined'] = false;
 				$this->_loadExternal[] = $options;
 			}
 		}
@@ -463,6 +491,71 @@ class EagerLoader {
 	}
 
 /**
+ * Returns an array having as keys a dotted path of associations that participate
+ * in this eager loader. The values of the array will contain the following keys
+ *
+ * - alias: The association alias
+ * - instance: The association instance
+ * - canBeJoined: Whether or not the association will be loaded using a JOIN
+ * - entityClass: The entity that should be used for hydrating the results
+ * - nestKey: A dotted path that can be used to correctly insert the data into the results.
+ * - mathcing: Whether or not it is an association loaded through `matching()`.
+ *
+ * @param \Cake\ORM\Table $table The table containing the association that
+ * will be normalized
+ * @return array
+ */
+	public function associationsMap($table) {
+		$map = [];
+
+		if (!$this->matching() && !$this->contain() && empty($this->_joinsMap)) {
+			return $map;
+		}
+
+		$visitor = function ($level, $matching = false) use (&$visitor, &$map) {
+			foreach ($level as $assoc => $meta) {
+				$map[] = [
+					'alias' => $assoc,
+					'instance' => $meta['instance'],
+					'canBeJoined' => $meta['canBeJoined'],
+					'entityClass' => $meta['instance']->target()->entityClass(),
+					'nestKey' => $meta['canBeJoined'] ? $assoc : $meta['aliasPath'],
+					'matching' => isset($meta['matching']) ? $meta['matching'] : $matching
+				];
+				if ($meta['canBeJoined'] && !empty($meta['associations'])) {
+					$visitor($meta['associations'], $matching);
+				}
+			}
+		};
+		$visitor($this->_matching->normalized($table), true);
+		$visitor($this->normalized($table));
+		$visitor($this->_joinsMap);
+		return $map;
+	}
+
+/**
+ * Registers a table alias, typically loaded as a join in a query, as belonging to
+ * an association. This helps hydrators know what to do with the columns coming
+ * from such joined table.
+ *
+ * @param string $alias The table alias as it appears in the query.
+ * @param \Cake\ORM\Association $assoc The association object the alias represents;
+ * will be normalized
+ * @param bool $asMatching Whether or not this join results should be treated as a
+ * 'matching' association.
+ * @return void
+ */
+	public function addToJoinsMap($alias, Association $assoc, $asMatching = false) {
+		$this->_joinsMap[$alias] = [
+			'aliasPath' => $alias,
+			'instance' => $assoc,
+			'canBeJoined' => true,
+			'matching' => $asMatching,
+			'associations' => []
+		];
+	}
+
+/**
  * Helper function used to return the keys from the query records that will be used
  * to eagerly load associations.
  *

src/ORM/Query.php

@@ -796,12 +796,14 @@ class Query extends DatabaseQuery implements JsonSerializable {
  * {@inheritDoc}
  */
 	public function __debugInfo() {
+		$eagerLoader = $this->eagerLoader();
 		return parent::__debugInfo() + [
 			'hydrate' => $this->_hydrate,
 			'buffered' => $this->_useBufferedResults,
 			'formatters' => count($this->_formatters),
 			'mapReducers' => count($this->_mapReduce),
-			'contain' => $this->contain(),
+			'contain' => $eagerLoader->contain(),
+			'matching' => $eagerLoader->matching(),
 			'extraOptions' => $this->_options,
 			'repository' => $this->_repository
 		];

src/ORM/ResultSet.php

@@ -14,6 +14,7 @@
  */
 namespace Cake\ORM;
 
+use Cake\Collection\Collection;
 use Cake\Collection\CollectionTrait;
 use Cake\Database\Exception;
 use Cake\Database\Type;
@@ -68,11 +69,19 @@ class ResultSet implements ResultSetInterface {
 	protected $_defaultTable;
 
 /**
- * List of associations that should be eager loaded
+ * List of associations that should be placed under the `_matchingData`
+ * result key.
  *
  * @var array
  */
-	protected $_associationMap = [];
+	protected $_matchingMap = [];
+
+/**
+ * List of associations that should be eager loaded.
+ *
+ * @var array
+ */
+	protected $_containMap = [];
 
 /**
  * Map of fields that are fetched from the statement with
@@ -271,28 +280,14 @@ class ResultSet implements ResultSetInterface {
  * @return void
  */
 	protected function _calculateAssociationMap() {
-		$contain = $this->_query->eagerLoader()->normalized($this->_defaultTable);
-		if (!$contain) {
-			return;
-		}
-
-		$map = [];
-		$visitor = function ($level) use (&$visitor, &$map) {
-			foreach ($level as $assoc => $meta) {
-				$map[$meta['aliasPath']] = [
-					'alias' => $assoc,
-					'instance' => $meta['instance'],
-					'canBeJoined' => $meta['canBeJoined'],
-					'entityClass' => $meta['instance']->target()->entityClass(),
-					'nestKey' => $meta['canBeJoined'] ? $assoc : $meta['aliasPath']
-				];
-				if ($meta['canBeJoined'] && !empty($meta['associations'])) {
-					$visitor($meta['associations']);
-				}
-			}
-		};
-		$visitor($contain, []);
-		$this->_associationMap = $map;
+		$map = $this->_query->eagerLoader()->associationsMap($this->_defaultTable);
+		$this->_matchingMap = (new Collection($map))
+			->match(['matching' => true])
+			->compile();
+
+		$this->_containMap = (new Collection(array_reverse($map)))
+			->match(['matching' => false])
+			->compile();
 	}
 
 /**
@@ -322,11 +317,43 @@ class ResultSet implements ResultSetInterface {
 	protected function _groupResult($row) {
 		$defaultAlias = $this->_defaultTable->alias();
 		$results = $presentAliases = [];
+		$options = [
+			'useSetters' => false,
+			'markClean' => true,
+			'markNew' => false,
+			'guard' => false
+		];
+
+		foreach ($this->_matchingMap as $matching) {
+			foreach ($row as $key => $value) {
+				if (strpos($key, $matching['alias'] . '__') !== 0) {
+					continue;
+				}
+				list($table, $field) = explode('__', $key);
+				$results['_matchingData'][$table][$field] = $value;
+				unset($row[$key]);
+			}
+			if (empty($results['_matchingData'][$matching['alias']])) {
+				continue;
+			}
+
+			$results['_matchingData'][$matching['alias']] = $this->_castValues(
+				$matching['instance']->target(),
+				$results['_matchingData'][$matching['alias']]
+			);
+
+			if ($this->_hydrate) {
+				$entity = new $matching['entityClass']($results['_matchingData'][$matching['alias']], $options);
+				$entity->clean();
+				$results['_matchingData'][$matching['alias']] = $entity;
+			}
+		}
+
 		foreach ($row as $key => $value) {
 			$table = $defaultAlias;
 			$field = $key;
 
-			if (isset($this->_associationMap[$key])) {
+			if ($value !== null && !is_scalar($value)) {
 				$results[$key] = $value;
 				continue;
 			}
@@ -354,14 +381,7 @@ class ResultSet implements ResultSetInterface {
 		}
 		unset($presentAliases[$defaultAlias]);
 
-		$options = [
-			'useSetters' => false,
-			'markClean' => true,
-			'markNew' => false,
-			'guard' => false
-		];
-
-		foreach (array_reverse($this->_associationMap) as $assoc) {
+		foreach ($this->_containMap as $assoc) {
 			$alias = $assoc['nestKey'];
 			$instance = $assoc['instance'];
 
@@ -395,6 +415,7 @@ class ResultSet implements ResultSetInterface {
 				$entity->clean();
 				$results[$alias] = $entity;
 			}
+
 			$results = $instance->transformRow($results, $alias, $assoc['canBeJoined']);
 		}
 
@@ -405,6 +426,10 @@ class ResultSet implements ResultSetInterface {
 			$results[$defaultAlias][$alias] = $results[$alias];
 		}
 
+		if (isset($results['_matchingData'])) {
+			$results[$defaultAlias]['_matchingData'] = $results['_matchingData'];
+		}
+
 		$options['source'] = $defaultAlias;
 		$results = $results[$defaultAlias];
 		if ($this->_hydrate && !($results instanceof Entity)) {

tests/TestCase/ORM/QueryTest.php

@@ -637,12 +637,14 @@ class QueryTest extends TestCase {
 			[
 				'id' => 3,
 				'name' => 'larry',
-				'articles' => [
-					'id' => 2,
-					'title' => 'Second Article',
-					'body' => 'Second Article Body',
-					'author_id' => 3,
-					'published' => 'Y',
+				'_matchingData' => [
+					'articles' => [
+						'id' => 2,
+						'title' => 'Second Article',
+						'body' => 'Second Article Body',
+						'author_id' => 3,
+						'published' => 'Y',
+					]
 				]
 			]
 		];
@@ -678,10 +680,12 @@ class QueryTest extends TestCase {
 				'title' => 'Second Article',
 				'body' => 'Second Article Body',
 				'published' => 'Y',
-				'tags' => [
-					'id' => 3,
-					'name' => 'tag3',
-					'_joinData' => ['article_id' => 2, 'tag_id' => 3]
+				'_matchingData' => [
+					'Tags' => [
+						'id' => 3,
+						'name' => 'tag3',
+					],
+					'ArticlesTags' => ['article_id' => 2, 'tag_id' => 3]
 				]
 			]
 		];
@@ -701,10 +705,12 @@ class QueryTest extends TestCase {
 				'body' => 'First Article Body',
 				'author_id' => 1,
 				'published' => 'Y',
-				'tags' => [
-					'id' => 2,
-					'name' => 'tag2',
-					'_joinData' => ['article_id' => 1, 'tag_id' => 2]
+				'_matchingData' => [
+					'Tags' => [
+						'id' => 2,
+						'name' => 'tag2',
+					],
+					'ArticlesTags' => ['article_id' => 1, 'tag_id' => 2]
 				]
 			]
 		];
@@ -734,17 +740,22 @@ class QueryTest extends TestCase {
 			[
 				'id' => 1,
 				'name' => 'mariano',
-				'articles' => [
-					'id' => 1,
-					'title' => 'First Article',
-					'body' => 'First Article Body',
-					'author_id' => 1,
-					'published' => 'Y',
+				'_matchingData' => [
 					'tags' => [
 						'id' => 2,
 						'name' => 'tag2',
-						'_joinData' => ['article_id' => 1, 'tag_id' => 2]
-					]
+					],
+					'articles' => [
+						'id' => 1,
+						'author_id' => 1,
+						'title' => 'First Article',
+						'body' => 'First Article Body',
+						'published' => 'Y'
+					],
+					'ArticlesTags' => [
+							'article_id' => 1,
+							'tag_id' => 2
+						]
 				]
 			]
 		];
@@ -1863,7 +1874,7 @@ class QueryTest extends TestCase {
 
 		$results = $query->toArray();
 		$this->assertCount(1, $results);
-		$this->assertEquals('tag3', $results[0]->articles->articles_tags->tag->name);
+		$this->assertEquals('tag3', $results[0]->_matchingData['tags']->name);
 	}
 
 /**
@@ -1902,7 +1913,8 @@ class QueryTest extends TestCase {
 			'buffered' => false,
 			'formatters' => 1,
 			'mapReducers' => 1,
-			'contain' => [
+			'contain' => [],
+			'matching' => [
 				'articles' => [
 					'queryBuilder' => null,
 					'matching' => true
@@ -1992,25 +2004,6 @@ class QueryTest extends TestCase {
 	}
 
 /**
- * Tests that it is not allowed to use matching on an association
- * that is already added to containments.
- *
- * @expectedException \RuntimeException
- * @expectedExceptionMessage Cannot use "matching" on "Authors" as there is another association with the same alias
- * @return void
- */
-	public function testConflitingAliases() {
-		$table = TableRegistry::get('ArticlesTags');
-		$table->belongsTo('Articles')->target()->belongsTo('Authors');
-		$table->belongsTo('Tags');
-		$table->Tags->target()->hasOne('Authors');
-		$table->find()
-			->contain(['Articles.Authors'])
-			->matching('Tags.Authors')
-			->all();
-	}
-
-/**
  * Tests that a hasOne association using the select strategy will still have the
  * key present in the results when no match is found
  *
@@ -2274,4 +2267,51 @@ class QueryTest extends TestCase {
 		$this->assertNull($articles[2]->author);
 	}
 
+/**
+ * Tests that it is possible to call matching and contain on the same
+ * association.
+ *
+ * @return void
+ */
+	public function testMatchingWithContain() {
+		$query = new Query($this->connection, $this->table);
+		$table = TableRegistry::get('authors');
+		$table->hasMany('articles');
+		TableRegistry::get('articles')->belongsToMany('tags');
+
+		$result = $query->repository($table)
+			->select()
+			->matching('articles.tags', function ($q) {
+				return $q->where(['tags.id' => 2]);
+			})
+			->contain('articles')
+			->first();
+
+		$this->assertEquals(1, $result->id);
+		$this->assertCount(2, $result->articles);
+		$this->assertEquals(2, $result->_matchingData['tags']->id);
+	}
+
+/**
+ * Tests that it is possible to call matching and contain on the same
+ * association with only one level of depth.
+ *
+ * @return void
+ */
+	public function testNotSoFarMatchingWithContainOnTheSameAssociation() {
+		$table = TableRegistry::get('articles');
+		$table->belongsToMany('tags');
+
+		$result = $table->find()
+			->matching('tags', function ($q) {
+				return $q->where(['tags.id' => 2]);
+			})
+			->contain('tags')
+			->first();
+
+		$this->assertEquals(1, $result->id);
+		$this->assertCount(2, $result->tags);
+		$this->assertEquals(2, $result->_matchingData['tags']->id);
+	}
+
 }