Browse Source

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
Mark Story 11 years ago
parent
commit
eab525c776

+ 22 - 10
src/ORM/Query.php

@@ -477,16 +477,28 @@ class Query extends DatabaseQuery implements JsonSerializable
      */
     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);
     }
 
     /**

+ 22 - 0
tests/TestCase/Controller/Component/PaginatorComponentTest.php

@@ -854,6 +854,28 @@ class PaginatorComponentTest extends TestCase
     }
 
     /**
+     * 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.
      *

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

@@ -2383,6 +2383,26 @@ class QueryTest extends TestCase
     }
 
     /**
+     * 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.
      *
      * @return void