Making sure ValuesExpression also casts values to expressions

src/Database/Expression/ValuesExpression.php

@@ -18,6 +18,7 @@ use Cake\Database\Exception;
 use Cake\Database\ExpressionInterface;
 use Cake\Database\Query;
 use Cake\Database\TypeMapTrait;
+use Cake\Database\Type\ExpressionTypeCasterTrait;
 use Cake\Database\ValueBinder;
 
 /**
@@ -32,6 +33,7 @@ class ValuesExpression implements ExpressionInterface
 {
 
     use TypeMapTrait;
+    use ExpressionTypeCasterTrait;
 
     /**
      * Array of values to insert.
@@ -55,6 +57,14 @@ class ValuesExpression implements ExpressionInterface
     protected $_query = false;
 
     /**
+     * Whether or not values have been casted to expressions
+     * already.
+     *
+     * @var string
+     */
+    protected $_castedExpressions = false;
+
+    /**
      * Constructor
      *
      * @param array $columns The list of columns that are going to be part of the values.
@@ -88,6 +98,7 @@ class ValuesExpression implements ExpressionInterface
             return;
         }
         $this->_values[] = $data;
+        $this->_castedExpressions = false;
     }
 
     /**
@@ -103,6 +114,7 @@ class ValuesExpression implements ExpressionInterface
             return $this->_columns;
         }
         $this->_columns = $cols;
+        $this->_castedExpressions = false;
         return $this;
     }
 
@@ -119,6 +131,7 @@ class ValuesExpression implements ExpressionInterface
             return $this->_values;
         }
         $this->_values = $values;
+        $this->_castedExpressions = false;
         return $this;
     }
 
@@ -150,23 +163,37 @@ class ValuesExpression implements ExpressionInterface
             return '';
         }
 
+        if (!$this->_castedExpressions) {
+            $this->_processExpressions();
+        }
+
         $i = 0;
         $defaults = array_fill_keys($this->_columns, null);
         $placeholders = [];
 
+        $types = [];
+        $typeMap = $this->typeMap();
+        foreach ($defaults as $col => $v) {
+            $types[$col] = $typeMap->type($col);
+        }
+
         foreach ($this->_values as $row) {
-            $row = array_merge($defaults, $row);
+            $row += $defaults;
             $rowPlaceholders = [];
-            foreach ($row as $column => $value) {
+
+            foreach ($this->_columns as $column) {
+                $value = $row[$column];
+
                 if ($value instanceof ExpressionInterface) {
                     $rowPlaceholders[] = '(' . $value->sql($generator) . ')';
                     continue;
                 }
-                $type = $this->typeMap()->type($column);
+
                 $placeholder = $generator->placeholder($i);
                 $rowPlaceholders[] = $placeholder;
-                $generator->bind($placeholder, $value, $type);
+                $generator->bind($placeholder, $value, $types[$column]);
             }
+
             $placeholders[] = implode(', ', $rowPlaceholders);
         }
 
@@ -192,6 +219,10 @@ class ValuesExpression implements ExpressionInterface
             return;
         }
 
+        if (!$this->_castedExpressions) {
+            $this->_processExpressions();
+        }
+
         foreach ($this->_values as $v) {
             if ($v instanceof ExpressionInterface) {
                 $v->traverse($visitor);
@@ -201,9 +232,41 @@ class ValuesExpression implements ExpressionInterface
             }
             foreach ($v as $column => $field) {
                 if ($field instanceof ExpressionInterface) {
+                    $visitor($field);
                     $field->traverse($visitor);
                 }
             }
         }
     }
+
+    /**
+     * Converts values that need to be casted to expressions
+     *
+     * @return void
+     */
+    protected function _processExpressions()
+    {
+        $types = [];
+        $typeMap = $this->typeMap();
+
+        foreach ($this->_columns as $c) {
+            if (!is_scalar($c)) {
+                continue;
+            }
+            $types[$c] = $typeMap->type($c);
+        }
+
+        $types = $this->_requiresToExpressionCasting($types);
+
+        if (empty($types)) {
+            return;
+        }
+
+        foreach ($this->_values as $row => $values) {
+            foreach ($types as $col => $type) {
+                $this->_values[$row][$col] = $type->toExpression($values[$col]);
+            }
+        }
+        $this->_castedExpressions = true;
+    }
 }

src/Database/Type/ExpressionTypeCasterTrait.php

@@ -55,4 +55,25 @@ trait ExpressionTypeCasterTrait
 
         return $converter->toExpression($value);
     }
+
+    /**
+     * Returns an array with the types that require values to
+     * be casted to expressions, out of the list of type names
+     * passed as parameter.
+     *
+     * @param array $types List of type names
+     * @return array
+     */
+    protected function _requiresToExpressionCasting($types)
+    {
+        $result  = [];
+        $types = array_filter($types);
+        foreach ($types as $k => $type) {
+            $object = Type::build($type);
+            if ($object instanceof ExpressionTypeInterface) {
+                $result[$k] = $object;
+            }
+        }
+        return $result;
+    }
 }

tests/TestCase/Database/ExpressionTypeCastingTest.php

@@ -22,6 +22,7 @@ use Cake\Database\Type;
 use Cake\Database\Expression\Comparison;
 use Cake\Database\Expression\BetweenExpression;
 use Cake\Database\Expression\CaseExpression;
+use Cake\Database\Expression\ValuesExpression;
 
 class TestType extends StringType implements ExpressionTypeInterface
 {
@@ -168,4 +169,33 @@ class ExpressionTypeCastingTest extends TestCase
         $result = array_sum($expressions);
         $this->assertEquals(1, $result, 'Missing expressions in the tree');
     }
+
+    /**
+     * Tests that values in ValuesExpression are converted to expressions correctly
+     *
+     * @return void
+     */
+    public function testValuesExpression()
+    {
+        $values = new ValuesExpression(['title'], ['title' => 'test']);
+        $values->add(['title' => 'foo']);
+        $values->add(['title' => 'bar']);
+
+        $binder = new ValueBinder;
+        $sql = $values->sql($binder);
+        $this->assertEquals(
+            ' VALUES ((CONCAT(:c0, :c1))), ((CONCAT(:c2, :c3)))',
+            $sql
+        );
+        $this->assertEquals('foo', $binder->bindings()[':c0']['value']);
+        $this->assertEquals('bar', $binder->bindings()[':c2']['value']);
+
+        $expressions = [];
+        $values->traverse(function ($exp) use (&$expressions) {
+            $expressions[] = $exp instanceof FunctionExpression ? 1 : 0;
+        });
+
+        $result = array_sum($expressions);
+        $this->assertEquals(2, $result, 'Missing expressions in the tree');
+    }
 }