Merge branch 'fix/#2511-#2609-correct-identifier-and-data-type-collis…

…ions-in-list-type-extraction' into 2.5 Close #2511 Close #2609
doctrine · Jan 19, 2017 · 0d2f818 · 0d2f818
2 parents 79f74d8 + 7068f20
commit 0d2f818
Show file tree

Hide file tree

Showing 2 changed files with 146 additions and 22 deletions.
diff --git a/lib/Doctrine/DBAL/Connection.php b/lib/Doctrine/DBAL/Connection.php
@@ -580,18 +580,20 @@ public function delete($tableExpression, array $identifier, array $types = array
             throw InvalidArgumentException::fromEmptyCriteria();
         }
 
-        $this->connect();
-
+        $columnList = array();
         $criteria = array();
+        $paramValues = array();
 
-        foreach (array_keys($identifier) as $columnName) {
+        foreach ($identifier as $columnName => $value) {
+            $columnList[] = $columnName;
             $criteria[] = $columnName . ' = ?';
+            $paramValues[] = $value;
         }
 
         return $this->executeUpdate(
             'DELETE FROM ' . $tableExpression . ' WHERE ' . implode(' AND ', $criteria),
-            array_values($identifier),
-            is_string(key($types)) ? $this->extractTypeValues($identifier, $types) : $types
+            $paramValues,
+            is_string(key($types)) ? $this->extractTypeValues($columnList, $types) : $types
         );
     }
 
@@ -649,24 +651,31 @@ public function getTransactionIsolation()
      */
     public function update($tableExpression, array $data, array $identifier, array $types = array())
     {
-        $this->connect();
+        $columnList = array();
         $set = array();
+        $criteria = array();
+        $paramValues = array();
 
         foreach ($data as $columnName => $value) {
+            $columnList[] = $columnName;
             $set[] = $columnName . ' = ?';
+            $paramValues[] = $value;
         }
 
-        if (is_string(key($types))) {
-            $types = $this->extractTypeValues(array_merge($data, $identifier), $types);
+        foreach ($identifier as $columnName => $value) {
+            $columnList[] = $columnName;
+            $criteria[] = $columnName . ' = ?';
+            $paramValues[] = $value;
         }
 
-        $params = array_merge(array_values($data), array_values($identifier));
+        if (is_string(key($types))) {
+            $types = $this->extractTypeValues($columnList, $types);
+        }
 
         $sql  = 'UPDATE ' . $tableExpression . ' SET ' . implode(', ', $set)
-                . ' WHERE ' . implode(' = ? AND ', array_keys($identifier))
-                . ' = ?';
+                . ' WHERE ' . implode(' AND ', $criteria);
 
-        return $this->executeUpdate($sql, $params, $types);
+        return $this->executeUpdate($sql, $paramValues, $types);
     }
 
     /**
@@ -688,29 +697,39 @@ public function insert($tableExpression, array $data, array $types = array())
             return $this->executeUpdate('INSERT INTO ' . $tableExpression . ' ()' . ' VALUES ()');
         }
 
+        $columnList = array();
+        $paramPlaceholders = array();
+        $paramValues = array();
+
+        foreach ($data as $columnName => $value) {
+            $columnList[] = $columnName;
+            $paramPlaceholders[] = '?';
+            $paramValues[] = $value;
+        }
+
         return $this->executeUpdate(
-            'INSERT INTO ' . $tableExpression . ' (' . implode(', ', array_keys($data)) . ')' .
-            ' VALUES (' . implode(', ', array_fill(0, count($data), '?')) . ')',
-            array_values($data),
-            is_string(key($types)) ? $this->extractTypeValues($data, $types) : $types
+            'INSERT INTO ' . $tableExpression . ' (' . implode(', ', $columnList) . ')' .
+            ' VALUES (' . implode(', ', $paramPlaceholders) . ')',
+            $paramValues,
+            is_string(key($types)) ? $this->extractTypeValues($columnList, $types) : $types
         );
     }
 
     /**
-     * Extract ordered type list from two associate key lists of data and types.
+     * Extract ordered type list from an ordered column list and type map.
      *
-     * @param array $data
+     * @param array $columnList
      * @param array $types
      *
      * @return array
      */
-    private function extractTypeValues(array $data, array $types)
+    private function extractTypeValues(array $columnList, array $types)
     {
         $typeValues = array();
 
-        foreach ($data as $k => $_) {
-            $typeValues[] = isset($types[$k])
-                ? $types[$k]
+        foreach ($columnList as $columnIndex => $columnName) {
+            $typeValues[] = isset($types[$columnName])
+                ? $types[$columnName]
                 : \PDO::PARAM_STR;
         }
 

diff --git a/tests/Doctrine/Tests/DBAL/ConnectionTest.php b/tests/Doctrine/Tests/DBAL/ConnectionTest.php
@@ -300,6 +300,111 @@ public function testEmptyInsert()
         $conn->insert('footable', array());
     }
 
+    /**
+     * @group DBAL-2511
+     */
+    public function testUpdateWithDifferentColumnsInDataAndIdentifiers()
+    {
+        $driverMock = $this->getMock('Doctrine\DBAL\Driver');
+
+        $driverMock->expects($this->any())
+            ->method('connect')
+            ->will($this->returnValue(new DriverConnectionMock()));
+
+        $conn = $this->getMockBuilder('Doctrine\DBAL\Connection')
+            ->setMethods(array('executeUpdate'))
+            ->setConstructorArgs(array(array('platform' => new Mocks\MockPlatform()), $driverMock))
+            ->getMock();
+
+        $conn->expects($this->once())
+            ->method('executeUpdate')
+            ->with(
+                'UPDATE TestTable SET text = ?, is_edited = ? WHERE id = ? AND name = ?',
+                [
+                    'some text',
+                    true,
+                    1,
+                    'foo',
+                ],
+                [
+                    'string',
+                    'boolean',
+                    'integer',
+                    'string',
+                ]
+            );
+
+        $conn->update(
+            'TestTable',
+            [
+                'text' => 'some text',
+                'is_edited' => true,
+            ],
+            [
+                'id' => 1,
+                'name' => 'foo',
+            ],
+            [
+                'text' => 'string',
+                'is_edited' => 'boolean',
+                'id' => 'integer',
+                'name' => 'string',
+            ]
+        );
+    }
+
+    /**
+     * @group DBAL-2511
+     */
+    public function testUpdateWithSameColumnInDataAndIdentifiers()
+    {
+        $driverMock = $this->getMock('Doctrine\DBAL\Driver');
+
+        $driverMock->expects($this->any())
+            ->method('connect')
+            ->will($this->returnValue(new DriverConnectionMock()));
+
+        $conn = $this->getMockBuilder('Doctrine\DBAL\Connection')
+            ->setMethods(array('executeUpdate'))
+            ->setConstructorArgs(array(array('platform' => new Mocks\MockPlatform()), $driverMock))
+            ->getMock();
+
+        $conn->expects($this->once())
+            ->method('executeUpdate')
+            ->with(
+                'UPDATE TestTable SET text = ?, is_edited = ? WHERE id = ? AND is_edited = ?',
+                [
+                    'some text',
+                    true,
+                    1,
+                    false,
+                ],
+                [
+                    'string',
+                    'boolean',
+                    'integer',
+                    'boolean',
+                ]
+            );
+
+        $conn->update(
+            'TestTable',
+            [
+                'text' => 'some text',
+                'is_edited' => true,
+            ],
+            [
+                'id' => 1,
+                'is_edited' => false,
+            ],
+            [
+                'text' => 'string',
+                'is_edited' => 'boolean',
+                'id' => 'integer',
+            ]
+        );
+    }
+
     public function testFetchAssoc()
     {
         $statement = 'SELECT * FROM foo WHERE bar = ?';