From eab525c7760b16d2583ba3ff50a566f4891bfd8b Mon Sep 17 00:00:00 2001 From: Mark Story Date: Fri, 24 Apr 2015 21:57:05 -0400 Subject: [PATCH] Fix bound values being dropped when a clone is made. Use the clone hook to clear out a query's state. This also moves most of the logic into `__clone`. I'm not sure why I didn't do this earlier, but it makes sense to try and have `clone` be safe. Refs #6384 --- src/ORM/Query.php | 32 +++++++++++++------ .../Component/PaginatorComponentTest.php | 22 +++++++++++++ tests/TestCase/ORM/QueryTest.php | 20 ++++++++++++ 3 files changed, 64 insertions(+), 10 deletions(-) diff --git a/src/ORM/Query.php b/src/ORM/Query.php index 4af5d3402e7..8fa1825a912 100644 --- a/src/ORM/Query.php +++ b/src/ORM/Query.php @@ -477,16 +477,28 @@ public function applyOptions(array $options) */ public function cleanCopy() { - $query = clone $this; - $query->triggerBeforeFind(); - $query->autoFields(false); - $query->eagerLoader(clone $this->eagerLoader()); - $query->limit(null); - $query->order([], true); - $query->offset(null); - $query->mapReduce(null, null, true); - $query->formatResults(null, true); - return $query; + return clone $this; + } + + /** + * Object clone hook. + * + * Destroys the clones inner iterator and clones the value binder, and eagerloader instances. + * + * @return void + */ + public function __clone() + { + $this->_iterator = null; + $this->triggerBeforeFind(); + $this->eagerLoader(clone $this->eagerLoader()); + $this->valueBinder(clone $this->valueBinder()); + $this->autoFields(false); + $this->limit(null); + $this->order([], true); + $this->offset(null); + $this->mapReduce(null, null, true); + $this->formatResults(null, true); } /** diff --git a/tests/TestCase/Controller/Component/PaginatorComponentTest.php b/tests/TestCase/Controller/Component/PaginatorComponentTest.php index 01c4516909c..c0619896a28 100644 --- a/tests/TestCase/Controller/Component/PaginatorComponentTest.php +++ b/tests/TestCase/Controller/Component/PaginatorComponentTest.php @@ -853,6 +853,28 @@ public function testPaginateQuery() $this->Paginator->paginate($query, $settings); } + /** + * test paginate() with bind() + * + * @return void + */ + public function testPaginateQueryWithBindValue() + { + $this->loadFixtures('Posts'); + $table = TableRegistry::get('PaginatorPosts'); + $query = $table->find() + ->where(['PaginatorPosts.author_id BETWEEN :start AND :end']) + ->bind(':start', 1) + ->bind(':end', 2); + + $results = $this->Paginator->paginate($query, []); + + $result = $results->toArray(); + $this->assertCount(2, $result); + $this->assertEquals('First Post', $result[0]->title); + $this->assertEquals('Third Post', $result[1]->title); + } + /** * Tests that passing a query object with a limit clause set will * overwrite it with the passed defaults. diff --git a/tests/TestCase/ORM/QueryTest.php b/tests/TestCase/ORM/QueryTest.php index d57ba6061a2..220b101ee59 100644 --- a/tests/TestCase/ORM/QueryTest.php +++ b/tests/TestCase/ORM/QueryTest.php @@ -2382,6 +2382,26 @@ public function testCleanCopy() $this->assertNull($copy->clause('order')); } + /** + * test that cleanCopy retains bindings + * + * @return void + */ + public function testCleanCopyRetainsBindings() + { + $table = TableRegistry::get('Articles'); + $query = $table->find(); + $query->offset(10) + ->limit(1) + ->where(['Articles.id BETWEEN :start AND :end']) + ->order(['Articles.id' => 'DESC']) + ->bind(':start', 1) + ->bind(':end', 2); + $copy = $query->cleanCopy(); + + $this->assertNotEmpty($copy->valueBinder()->bindings()); + } + /** * test that cleanCopy makes a cleaned up clone with a beforeFind. *