Browse Source

Implement __clone() in Database\Query and remaining expressions

Move __clone() into Database\Query and implement deep cloning on all the
inner parts. This solves shared state in clones. Also implement
__clone() on the other expression objects that contain mutable state and
might be cloned with queries.

Refs #7533
Mark Story 10 years ago
parent
commit
426af95f2a

+ 14 - 0
src/Database/Expression/BetweenExpression.php

@@ -120,4 +120,18 @@ class BetweenExpression implements ExpressionInterface, FieldInterface
         $generator->bind($placeholder, $value, $type);
         return $placeholder;
     }
+
+    /**
+     * Do a deep clone of this expression.
+     *
+     * @return void
+     */
+    public function __clone()
+    {
+        foreach (['_field', '_from', '_to'] as $part) {
+            if ($this->{$part} instanceof ExpressionInterface) {
+                $this->{$part} = clone $this->{$part};
+            }
+        }
+    }
 }

+ 16 - 0
src/Database/Expression/Comparison.php

@@ -153,6 +153,22 @@ class Comparison implements ExpressionInterface, FieldInterface
     }
 
     /**
+     * Create a deep clone.
+     *
+     * Clones the field and value if they are expression objects.
+     *
+     * @return void
+     */
+    public function __clone()
+    {
+        foreach (['_value', '_field'] as $prop) {
+            if ($prop instanceof ExpressionInterface) {
+                $this->{$prop} = clone $this->{$prop};
+            }
+        }
+    }
+
+    /**
      * Returns a template and a placeholder for the value after registering it
      * with the placeholder $generator
      *

+ 12 - 0
src/Database/Expression/OrderClauseExpression.php

@@ -67,4 +67,16 @@ class OrderClauseExpression implements ExpressionInterface, FieldInterface
             $this->_field->traverse($visitor);
         }
     }
+
+    /**
+     * Create a deep clone of the order clause.
+     *
+     * @return void
+     */
+    public function __clone()
+    {
+        if ($this->_field instanceof ExpressionInterface) {
+            $this->_field = clone $this->_field;
+        }
+    }
 }

+ 12 - 0
src/Database/Expression/UnaryExpression.php

@@ -102,4 +102,16 @@ class UnaryExpression implements ExpressionInterface
             $callable($this->_value);
         }
     }
+
+    /**
+     * Perform a deep clone of the inner expression.
+     *
+     * @return void
+     */
+    public function __clone()
+    {
+        if ($this->_value instanceof ExpressionInterface) {
+            $this->_value = clone $this->_value;
+        }
+    }
 }

+ 31 - 0
src/Database/Query.php

@@ -1740,6 +1740,37 @@ class Query implements ExpressionInterface, IteratorAggregate
     }
 
     /**
+     * Do a deep clone on this object.
+     *
+     * Will clone all of the expression objects used in
+     * each of the clauses, as well as the valueBinder.
+     *
+     * @return void
+     */
+    public function __clone()
+    {
+        $this->_iterator = null;
+        if ($this->_valueBinder) {
+            $this->_valueBinder = clone $this->_valueBinder;
+        }
+        foreach ($this->_parts as $name => $part) {
+            if (empty($part)) {
+                continue;
+            }
+            if (is_array($part)) {
+                foreach ($part as $i => $piece) {
+                    if ($piece instanceof ExpressionInterface) {
+                        $this->_parts[$name][$i] = clone $piece;
+                    }
+                }
+            }
+            if ($part instanceof ExpressionInterface) {
+                $this->_parts[$name] = clone $part;
+            }
+        }
+    }
+
+    /**
      * Returns string representation of this query (complete SQL statement).
      *
      * @return string

+ 4 - 3
src/ORM/Query.php

@@ -675,9 +675,10 @@ class Query extends DatabaseQuery implements JsonSerializable, QueryInterface
      */
     public function __clone()
     {
-        $this->_iterator = null;
-        $this->eagerLoader(clone $this->eagerLoader());
-        $this->valueBinder(clone $this->valueBinder());
+        parent::__clone();
+        if ($this->_eagerLoader) {
+            $this->_eagerLoader = clone $this->_eagerLoader;
+        }
     }
 
     /**

+ 32 - 0
tests/TestCase/Database/QueryTest.php

@@ -3438,6 +3438,38 @@ class QueryTest extends TestCase
     }
 
     /**
+     * Test that cloning goes deep.
+     *
+     * @return void
+     */
+    public function testDeepClone()
+    {
+        $query = new Query($this->connection);
+        $query->select(['id', 'title' => $query->func()->concat(['title' => 'literal', 'test'])])
+            ->from('articles')
+            ->where(['Articles.id' => 1])
+            ->offset(10)
+            ->limit(1)
+            ->order(['Articles.id' => 'DESC']);
+        $dupe = clone $query;
+
+        $this->assertEquals($query->clause('where'), $dupe->clause('where'));
+        $this->assertNotSame($query->clause('where'), $dupe->clause('where'));
+        $dupe->where(['Articles.title' => 'thinger']);
+        $this->assertNotEquals($query->clause('where'), $dupe->clause('where'));
+
+        $this->assertNotSame(
+            $query->clause('select')['title'],
+            $dupe->clause('select')['title']
+        );
+        $this->assertEquals($query->clause('order'), $dupe->clause('order'));
+        $this->assertNotSame($query->clause('order'), $dupe->clause('order'));
+
+        $query->order(['Articles.title' => 'ASC']);
+        $this->assertNotEquals($query->clause('order'), $dupe->clause('order'));
+    }
+
+    /**
      * Assertion for comparing a table's contents with what is in it.
      *
      * @param string $table