Skip to content

Commit

Permalink
Fix invalid SQL on paginated results.
Browse files Browse the repository at this point in the history
When a query with nested matching calls was reused (via pagination), the
bound EagerLoadable instances for the nested relations were re-applied
incorrectly. By cloning the eagerloader objects when preparing the
count() query we can side-step this issue from happening as state is no
longer shared between the two query executions.

Refs #10633
  • Loading branch information
markstory committed May 15, 2017
1 parent 129f528 commit 4489522
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 3 deletions.
14 changes: 14 additions & 0 deletions src/ORM/EagerLoader.php
Expand Up @@ -829,4 +829,18 @@ protected function _groupKeys($statement, $collectKeys)

return $keys;
}

/**
* Clone hook implementation
*
* Clone the _matching eager loader as well.
*
* @return void
*/
public function __clone()
{
if ($this->_matching) {
$this->_matching = clone $this->_matching;
}
}
}
1 change: 1 addition & 0 deletions src/ORM/Query.php
Expand Up @@ -765,6 +765,7 @@ public function cleanCopy()
$clone->formatResults(null, true);
$clone->setSelectTypeMap(new TypeMap());
$clone->decorateResults(null, true);
$clone->setEagerLoader(clone $this->getEagerLoader());

return $clone;
}
Expand Down
36 changes: 36 additions & 0 deletions tests/TestCase/ORM/QueryRegressionTest.php
Expand Up @@ -175,6 +175,42 @@ public function testEagerLoadingBelongsToManyList()
$table->find()->contain('Tags')->toArray();
}

/**
* Tests that eagerloading and hydration works for associations that have
* different aliases in the association and targetTable
*
* @return void
*/
public function testEagerLoadingNestedMatchingCalls()
{
$this->loadFixtures('Articles', 'Authors', 'Tags', 'ArticlesTags', 'AuthorsTags');
$articles = TableRegistry::get('Articles');
$articles->belongsToMany('Tags', [
'foreignKey' => 'article_id',
'targetForeignKey' => 'tag_id',
'joinTable' => 'articles_tags'
]);
$tags = TableRegistry::get('Tags');
$tags->belongsToMany('Authors', [
'foreignKey' => 'tag_id',
'targetForeignKey' => 'author_id',
'joinTable' => 'authors_tags'
]);

$query = $articles->find()
->matching('Tags', function ($q) {
return $q->matching('Authors', function ($q) {
return $q->where(['Authors.name' => 'larry']);
});
});
$this->assertEquals(3, $query->count());

$result = $query->first();
$this->assertInstanceOf(EntityInterface::class, $result);
$this->assertInstanceOf(EntityInterface::class, $result->_matchingData['Tags']);
$this->assertInstanceOf(EntityInterface::class, $result->_matchingData['Authors']);
}

/**
* Tests that duplicate aliases in contain() can be used, even when they would
* naturally be attached to the query instead of eagerly loaded. What should
Expand Down
14 changes: 11 additions & 3 deletions tests/TestCase/ORM/QueryTest.php
Expand Up @@ -2714,12 +2714,20 @@ public function testCleanCopy()
$query->offset(10)
->limit(1)
->order(['Articles.id' => 'DESC'])
->contain(['Comments']);
->contain(['Comments'])
->matching('Comments');
$copy = $query->cleanCopy();

$this->assertNotSame($copy, $query);
$this->assertNotSame($copy->eagerLoader(), $query->eagerLoader());
$this->assertNotEmpty($copy->eagerLoader()->contain());
$copyLoader = $copy->getEagerLoader();
$loader = $query->getEagerLoader();
$this->assertEquals($copyLoader, $loader, 'should be equal');
$this->assertNotSame($copyLoader, $loader, 'should be clones');
$this->assertNotSame(
$this->readAttribute($copyLoader, '_matching'),
$this->readAttribute($loader, '_matching'),
'should be clones'
);
$this->assertNull($copy->clause('offset'));
$this->assertNull($copy->clause('limit'));
$this->assertNull($copy->clause('order'));
Expand Down

0 comments on commit 4489522

Please sign in to comment.