Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Optimizing duplicate aliases detection by making it per branch
Also throwing exception when it is not possible to solve the duplicate
aliases issue
  • Loading branch information
lorenzo committed Apr 4, 2014
1 parent 6c86e86 commit 8978f18
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 10 deletions.
49 changes: 39 additions & 10 deletions src/ORM/EagerLoader.php
Expand Up @@ -164,7 +164,7 @@ public function normalized(Table $repository) {
$repository,
$alias,
$options,
[]
['root' => null]
);
}

Expand Down Expand Up @@ -312,7 +312,7 @@ protected function _normalizeContain(Table $parent, $alias, $options, $paths) {
);
}

$paths += ['aliasPath' => '', 'propertyPath' => ''];
$paths += ['aliasPath' => '', 'propertyPath' => '', 'root' => $alias];
$paths['aliasPath'] .= '.' . $alias;
$paths['propertyPath'] .= '.' . $instance->property();

Expand All @@ -324,17 +324,16 @@ protected function _normalizeContain(Table $parent, $alias, $options, $paths) {
'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);
Expand All @@ -343,6 +342,36 @@ protected function _normalizeContain(Table $parent, $alias, $options, $paths) {
return $config;
}

/**
* 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.
Expand Down
19 changes: 19 additions & 0 deletions tests/TestCase/ORM/QueryTest.php
Expand Up @@ -1923,4 +1923,23 @@ public function testRepeatedAssociationAliases($strategy) {
$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();
}

}

0 comments on commit 8978f18

Please sign in to comment.