Browse Source

Optimizing duplicate aliases detection by making it per branch

Also throwing exception when it is not possible to solve the duplicate
aliases issue
Jose Lorenzo Rodriguez 12 years ago
parent
commit
8978f18741
2 changed files with 58 additions and 10 deletions
  1. 39 10
      src/ORM/EagerLoader.php
  2. 19 0
      tests/TestCase/ORM/QueryTest.php

+ 39 - 10
src/ORM/EagerLoader.php

@@ -164,7 +164,7 @@ class EagerLoader {
 				$repository,
 				$alias,
 				$options,
-				[]
+				['root' => null]
 			);
 		}
 
@@ -312,7 +312,7 @@ class EagerLoader {
 			);
 		}
 
-		$paths += ['aliasPath' => '', 'propertyPath' => ''];
+		$paths += ['aliasPath' => '', 'propertyPath' => '', 'root' => $alias];
 		$paths['aliasPath'] .= '.' . $alias;
 		$paths['propertyPath'] .= '.' . $instance->property();
 
@@ -324,17 +324,16 @@ class EagerLoader {
 			'instance' => $instance,
 			'config' => array_diff_key($options, $extra),
 			'aliasPath' => trim($paths['aliasPath'], '.'),
-			'propertyPath' => trim($paths['propertyPath'], '.'),
+			'propertyPath' => trim($paths['propertyPath'], '.')
 		];
-
 		$config['canBeJoined'] = $instance->canBeJoined($config['config']);
-		$canChange = $config['canBeJoined'] && empty($config['config']['matching']);
-		if ($canChange && !empty($this->_aliasList[$alias])) {
-			$config['canBeJoined'] = false;
-			$config['config']['strategy'] = $instance::STRATEGY_SELECT;
-		}
+		$config = $this->_correctStrategy($alias, $config, $paths['root']);
 
-		$this->_aliasList[$alias][] = $paths['aliasPath'];
+		if ($config['canBeJoined']) {
+			$this->_aliasList[$paths['root']][$alias] = true;
+		} else {
+			$paths['root'] = $config['aliasPath'];
+		}
 
 		foreach ($extra as $t => $assoc) {
 			$config['associations'][$t] = $this->_normalizeContain($table, $t, $assoc, $paths);
@@ -344,6 +343,36 @@ class EagerLoader {
 	}
 
 /**
+ * Changes the association fetching strategy if required because of duplicate
+ * under the same direct associations chain
+ *
+ * @param string $alias the name of the association to evaluate
+ * @param array $config The association config
+ * @param string $root An string representing the root association that started
+ * the direct chain this alias is in
+ * @return array The modified association config
+ * @throws \RuntimeException if a duplicate association in the same chain is detected
+ * but is not possible to change the strategy due to conflicting settings
+ */
+	protected function _correctStrategy($alias, $config, $root) {
+		if (!$config['canBeJoined'] || empty($this->_aliasList[$root][$alias])) {
+			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;
+
+		return $config;
+	}
+
+/**
  * Helper function used to compile a list of all associations that can be
  * joined in the query.
  *

+ 19 - 0
tests/TestCase/ORM/QueryTest.php

@@ -1923,4 +1923,23 @@ class QueryTest extends TestCase {
 		$this->assertNotEmpty($results[2]['article']);
 	}
 
+/**
+ * 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();
+	}
+
 }