Simplifying result nesting by alwasy passing the expected nest key.

This removes the hackery introduced for nesting results from an external query and crazy names like '___collection_'
cakephp · Apr 3, 2014 · 69918ec · 69918ec
1 parent 6eb0a15
commit 69918ec
Show file tree

Hide file tree

Showing 8 changed files with 37 additions and 85 deletions.
diff --git a/src/ORM/Association.php b/src/ORM/Association.php
@@ -447,15 +447,18 @@ public function attachTo(Query $query, array $options = []) {
  * source results.
  *
  * @param array $row
+ * @param string $nestKey The array key under which the results for this association
+ * should be found
  * @param boolean $joined Whether or not the row is a result of a direct join
  * with this association
  * @return array
  */
-	public function transformRow($row, $joined) {
+	public function transformRow($row, $nestKey, $joined) {
 		$sourceAlias = $this->source()->alias();
-		$targetAlias = $this->target()->alias();
+		$nestKey = $nestKey ?: $this->_name;
 		if (isset($row[$sourceAlias])) {
-			$row[$sourceAlias][$this->property()] = $row[$targetAlias];
+			$row[$sourceAlias][$this->property()] = $row[$nestKey];
+			unset($row[$nestKey]);
 		}
 		return $row;
 	}

diff --git a/src/ORM/Association/BelongsTo.php b/src/ORM/Association/BelongsTo.php
@@ -102,23 +102,6 @@ public function type() {
 		return self::MANY_TO_ONE;
 	}
 
-/**
- * {@inheritdoc}
- *
- */
-	public function transformRow($row, $joined) {
-		if ($this->strategy() === $this::STRATEGY_JOIN) {
-			return parent::transformRow($row, $joined);
-		}
-
-		$sourceAlias = $this->source()->alias();
-		$nestKey = $this->_nestingKey();
-		if (isset($row[$nestKey])) {
-			$row[$sourceAlias][$this->property()] = $row[$nestKey];
-		}
-		return $row;
-	}
-
 /**
  * Takes an entity from the source table and looks if there is a field
  * matching the property name for this association. The found entity will be

diff --git a/src/ORM/Association/BelongsToMany.php b/src/ORM/Association/BelongsToMany.php
@@ -32,7 +32,6 @@ class BelongsToMany extends Association {
 	use ExternalAssociationTrait {
 		_options as _externalOptions;
 		_addFilteringCondition as _addExternalConditions;
-		transformRow as protected _transformRow;
 	}
 
 /**
@@ -236,21 +235,17 @@ public function attachTo(Query $query, array $options = []) {
 	}
 
 /**
- * Correctly nests a result row associated values into the correct array keys inside the
- * source results.
+ * {@inheritdoc}
  *
- * @param array $row
- * @param boolean $joined Whether or not the row is a result of a direct join
- * with this association
- * @return array
  */
-	public function transformRow($row, $joined) {
+	public function transformRow($row, $nestKey, $joined) {
 		$alias = $this->junction()->alias();
 		if ($joined) {
 			$row[$this->target()->alias()][$this->_junctionProperty] = $row[$alias];
 			unset($row[$alias]);
 		}
-		return $this->_transformRow($row, $joined);
+
+		return parent::transformRow($row, $nestKey, $joined);
 	}
 
 /**

diff --git a/src/ORM/Association/ExternalAssociationTrait.php b/src/ORM/Association/ExternalAssociationTrait.php
@@ -77,30 +77,6 @@ public function sort($sort = null) {
 		return $this->_sort;
 	}
 
-/**
- * Correctly nests a result row associated values into the correct array keys inside the
- * source results.
- *
- * @param array $row
- * @param boolean $joined Whether or not the row is a result of a direct join
- * with this association
- * @return array
- */
-	public function transformRow($row, $joined) {
-		$sourceAlias = $this->source()->alias();
-		$targetAlias = $this->target()->alias();
-
-		$collectionAlias = $this->_name . '___collection_';
-		if (isset($row[$collectionAlias])) {
-			$values = $row[$collectionAlias];
-		} else {
-			$values = $row[$this->_name];
-		}
-
-		$row[$sourceAlias][$this->property()] = $values;
-		return $row;
-	}
-
 /**
  * Returns the default options to use for the eagerLoader
  *
@@ -130,16 +106,6 @@ protected function _buildResultMap($fetchQuery, $options) {
 		return $resultMap;
 	}
 
-/**
- * Returns the key under which the eagerLoader will put this association results
- *
- * @return void
- */
-	protected function _nestingKey() {
-		return $this->_name . '___collection_';
-	}
-
-
 /**
  * Parse extra options passed in the constructor.
  *

diff --git a/src/ORM/Association/SelectableAssociationTrait.php b/src/ORM/Association/SelectableAssociationTrait.php
@@ -50,7 +50,7 @@ public function eagerLoader(array $options) {
 			$fetchQuery = $queryBuilder($fetchQuery);
 		}
 		$resultMap = $this->_buildResultMap($fetchQuery, $options);
-		return $this->_resultInjector($fetchQuery, $resultMap);
+		return $this->_resultInjector($fetchQuery, $resultMap, $options);
 	}
 
 /**
@@ -62,7 +62,8 @@ protected function _defaultOptions() {
 		return [
 			'foreignKey' => $this->foreignKey(),
 			'conditions' => [],
-			'strategy' => $this->strategy()
+			'strategy' => $this->strategy(),
+			'nestKey' => $this->_name
 		];
 	}
 
@@ -199,7 +200,7 @@ protected abstract function _buildResultMap($fetchQuery, $options);
  * the corresponding target table results as value.
  * @return \Closure
  */
-	protected function _resultInjector($fetchQuery, $resultMap) {
+	protected function _resultInjector($fetchQuery, $resultMap, $options) {
 		$source = $this->source();
 		$sAlias = $source->alias();
 		$keys = $this->type() === $this::MANY_TO_ONE ?
@@ -211,7 +212,7 @@ protected function _resultInjector($fetchQuery, $resultMap) {
 			$sourceKeys[] = key($fetchQuery->aliasField($key, $sAlias));
 		}
 
-		$nestKey = $this->_nestingKey();
+		$nestKey = $options['nestKey'];
 		if (count($sourceKeys) > 1) {
 			return $this->_multiKeysInjector($resultMap, $sourceKeys, $nestKey);
 		}
@@ -225,15 +226,6 @@ protected function _resultInjector($fetchQuery, $resultMap) {
 		};
 	}
 
-/**
- * Returns the key under which the eagerLoader will put this association results
- *
- * @return string
- */
-	protected function _nestingKey() {
-		return $this->property();
-	}
-
 /**
  * Returns a callable to be used for each row in a query result set
  * for injecting the eager loaded rows when the matching needs to

diff --git a/src/ORM/EagerLoader.php b/src/ORM/EagerLoader.php
@@ -389,7 +389,12 @@ public function loadExternal($query, $statement) {
 
 			$keys = isset($collected[$alias]) ? $collected[$alias] : null;
 			$f = $meta['instance']->eagerLoader(
-				$meta['config'] + ['query' => $query, 'contain' => $contain, 'keys' => $keys]
+				$meta['config'] + [
+					'query' => $query,
+					'contain' => $contain,
+					'keys' => $keys,
+					'nestKey' => $meta['aliasPath']
+				]
 			);
 			$statement = new CallbackStatement($statement, $driver, $f);
 		}

diff --git a/src/ORM/ResultSet.php b/src/ORM/ResultSet.php
@@ -309,7 +309,8 @@ protected function _calculateAssociationMap() {
 					'alias' => $assoc,
 					'instance' => $meta['instance'],
 					'canBeJoined' => $meta['canBeJoined'],
-					'entityClass' => $meta['instance']->target()->entityClass()
+					'entityClass' => $meta['instance']->target()->entityClass(),
+					'nestKey' => $meta['canBeJoined'] ? $assoc : $meta['aliasPath']
 				];
 				if (!empty($meta['associations'])) {
 					$visitor($meta['associations']);
@@ -351,7 +352,7 @@ protected function _groupResult($row) {
 			$table = $defaultAlias;
 			$field = $key;
 
-			if (strpos($key, '___collection_') !== false) {
+			if (isset($this->_associationMap[$key])) {
 				$results[$key] = $value;
 				continue;
 			}
@@ -383,9 +384,10 @@ protected function _groupResult($row) {
 			'markNew' => false,
 			'guard' => false
 		];
+
 		foreach (array_reverse($this->_associationMap) as $assoc) {
-			$alias = $assoc['alias'];
-			if (!isset($results[$alias]) && !isset($results[$alias . '___collection_'])) {
+			$alias = $assoc['nestKey'];
+			if (!isset($results[$alias])) {
 				continue;
 			}
 
@@ -404,7 +406,7 @@ protected function _groupResult($row) {
 				$results[$alias] = $entity;
 			}
 
-			$results = $instance->transformRow($results, $assoc['canBeJoined']);
+			$results = $instance->transformRow($results, $alias, $assoc['canBeJoined']);
 		}
 
 		foreach ($presentAliases as $alias => $present) {

diff --git a/tests/TestCase/ORM/QueryRegressionTest.php b/tests/TestCase/ORM/QueryRegressionTest.php
@@ -72,10 +72,16 @@ public function testDuplicateAttachableAliases() {
 
 		$table = TableRegistry::get('Articles');
 		$table->belongsTo('Authors');
-		$table->hasOne('Things');
+		$table->hasOne('Things', ['propertyName' => 'articles_tag']);
 
-		$table->Authors->target()->hasOne('Stuff', ['foreignKey' => 'id']);
-		$table->Things->target()->belongsTo('Stuff', ['foreignKey' => 'tag_id']);
+		$table->Authors->target()->hasOne('Stuff', [
+			'foreignKey' => 'id',
+			'propertyName' => 'favorite_tag'
+		]);
+		$table->Things->target()->belongsTo('Stuff', [
+			'foreignKey' => 'tag_id',
+			'propertyName' => 'foo']
+		);
 
 		$results = $table->find()
 			->contain(['Authors.Stuff', 'Things.Stuff'])