Merge branch 'backport/ticket/6699' into 2.5

Backporting: doctrine#6705
lcobucci · Nov 24, 2017 · c811c3a · c811c3a
2 parents 049470c + aba62ed
commit c811c3a
Show file tree

Hide file tree

Showing 5 changed files with 211 additions and 33 deletions.
diff --git a/lib/Doctrine/ORM/AbstractQuery.php b/lib/Doctrine/ORM/AbstractQuery.php
@@ -325,14 +325,14 @@ public function getParameters()
     public function getParameter($key)
     {
         $filteredParameters = $this->parameters->filter(
-            function ($parameter) use ($key)
-            {
-                // Must not be identical because of string to integer conversion
-                return ($key == $parameter->getName());
+            function (Query\Parameter $parameter) use ($key) {
+                $parameterName = $parameter->getName();
+
+                return $key === $parameterName || (string) $key === (string) $parameterName;
             }
         );
 
-        return count($filteredParameters) ? $filteredParameters->first() : null;
+        return ! $filteredParameters->isEmpty() ? $filteredParameters->first() : null;
     }
 
     /**
@@ -373,17 +373,10 @@ public function setParameters($parameters)
      */
     public function setParameter($key, $value, $type = null)
     {
-        $filteredParameters = $this->parameters->filter(
-            function ($parameter) use ($key)
-            {
-                // Must not be identical because of string to integer conversion
-                return ($key == $parameter->getName());
-            }
-        );
+        $existingParameter = $this->getParameter($key);
 
-        if (count($filteredParameters)) {
-            $parameter = $filteredParameters->first();
-            $parameter->setValue($value, $type);
+        if ($existingParameter !== null) {
+            $existingParameter->setValue($value, $type);
 
             return $this;
         }

diff --git a/lib/Doctrine/ORM/QueryBuilder.php b/lib/Doctrine/ORM/QueryBuilder.php
@@ -529,24 +529,15 @@ public function getRootEntities()
      */
     public function setParameter($key, $value, $type = null)
     {
-        $filteredParameters = $this->parameters->filter(
-            function ($parameter) use ($key)
-            {
-                // Must not be identical because of string to integer conversion
-                return ($key == $parameter->getName());
-            }
-        );
+        $existingParameter = $this->getParameter($key);
 
-        if (count($filteredParameters)) {
-            $parameter = $filteredParameters->first();
-            $parameter->setValue($value, $type);
+        if ($existingParameter !== null) {
+            $existingParameter->setValue($value, $type);
 
             return $this;
         }
 
-        $parameter = new Query\Parameter($key, $value, $type);
-
-        $this->parameters->add($parameter);
+        $this->parameters->add(new Query\Parameter($key, $value, $type));
 
         return $this;
     }
@@ -609,14 +600,14 @@ public function getParameters()
     public function getParameter($key)
     {
         $filteredParameters = $this->parameters->filter(
-            function ($parameter) use ($key)
-            {
-                // Must not be identical because of string to integer conversion
-                return ($key == $parameter->getName());
+            function (Query\Parameter $parameter) use ($key) {
+                $parameterName = $parameter->getName();
+
+                return $key === $parameterName || (string) $key === (string) $parameterName;
             }
         );
 
-        return count($filteredParameters) ? $filteredParameters->first() : null;
+        return ! $filteredParameters->isEmpty() ? $filteredParameters->first() : null;
     }
 
     /**

diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH6699Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH6699Test.php
@@ -0,0 +1,55 @@
+<?php
+
+namespace Doctrine\Tests\ORM\Functional\Ticket;
+
+use Doctrine\Tests\OrmFunctionalTestCase;
+
+/**
+ * @group 6699
+ */
+final class GH6699Test extends OrmFunctionalTestCase
+{
+    protected function setUp()
+    {
+        $this->useModelSet('cms');
+
+        parent::setUp();
+    }
+
+    public function testMixedParametersWithZeroNumber()
+    {
+        $query = $this->_em->createQueryBuilder()
+                           ->select('u')
+                           ->from('Doctrine\Tests\Models\CMS\CmsUser', 'u')
+                           ->andWhere('u.username = :username')
+                           ->andWhere('u.id = ?0')
+                           ->getQuery();
+
+        $query->setParameter('username', 'bar');
+        $query->setParameter(0, 0);
+
+        $query->execute();
+
+        self::assertCount(2, $query->getParameters());
+        self::assertSame(0, $query->getParameter(0)->getValue());
+        self::assertSame('bar', $query->getParameter('username')->getValue());
+    }
+
+    public function testMixedParametersWithZeroNumberOnQueryBuilder()
+    {
+        $query = $this->_em->createQueryBuilder()
+                           ->select('u')
+                           ->from('Doctrine\Tests\Models\CMS\CmsUser', 'u')
+                           ->andWhere('u.username = :username')
+                           ->andWhere('u.id = ?0')
+                           ->setParameter('username', 'bar')
+                           ->setParameter(0, 0)
+                           ->getQuery();
+
+        $query->execute();
+
+        self::assertCount(2, $query->getParameters());
+        self::assertSame(0, $query->getParameter(0)->getValue());
+        self::assertSame('bar', $query->getParameter('username')->getValue());
+    }
+}
diff --git a/tests/Doctrine/Tests/ORM/Query/QueryTest.php b/tests/Doctrine/Tests/ORM/Query/QueryTest.php
@@ -197,4 +197,66 @@ public function testDefaultQueryHints()
         $q2 = clone $query;
         $this->assertSame($config->getDefaultQueryHints(), $q2->getHints());
     }
+
+    /**
+     * @group 6699
+     */
+    public function testGetParameterTypeJuggling()
+    {
+        $query = $this->_em->createQuery('select u from Doctrine\Tests\Models\CMS\CmsUser u where u.id = ?0');
+
+        $query->setParameter(0, 0);
+
+        self::assertCount(1, $query->getParameters());
+        self::assertSame(0, $query->getParameter(0)->getValue());
+        self::assertSame(0, $query->getParameter('0')->getValue());
+    }
+
+    /**
+     * @group 6699
+     */
+    public function testSetParameterWithNameZeroIsNotOverridden()
+    {
+        $query = $this->_em->createQuery('select u from Doctrine\Tests\Models\CMS\CmsUser u where u.id != ?0 and u.username = :name');
+
+        $query->setParameter(0, 0);
+        $query->setParameter('name', 'Doctrine');
+
+        self::assertCount(2, $query->getParameters());
+        self::assertSame(0, $query->getParameter('0')->getValue());
+        self::assertSame('Doctrine', $query->getParameter('name')->getValue());
+    }
+
+    /**
+     * @group 6699
+     */
+    public function testSetParameterWithNameZeroDoesNotOverrideAnotherParameter()
+    {
+        $query = $this->_em->createQuery('select u from Doctrine\Tests\Models\CMS\CmsUser u where u.id != ?0 and u.username = :name');
+
+        $query->setParameter('name', 'Doctrine');
+        $query->setParameter(0, 0);
+
+        self::assertCount(2, $query->getParameters());
+        self::assertSame(0, $query->getParameter(0)->getValue());
+        self::assertSame('Doctrine', $query->getParameter('name')->getValue());
+    }
+
+    /**
+     * @group 6699
+     */
+    public function testSetParameterWithTypeJugglingWorks()
+    {
+        $query = $this->_em->createQuery('select u from Doctrine\Tests\Models\CMS\CmsUser u where u.id != ?0 and u.username = :name');
+
+        $query->setParameter('0', 1);
+        $query->setParameter('name', 'Doctrine');
+        $query->setParameter(0, 2);
+        $query->setParameter('0', 3);
+
+        self::assertCount(2, $query->getParameters());
+        self::assertSame(3, $query->getParameter(0)->getValue());
+        self::assertSame(3, $query->getParameter('0')->getValue());
+        self::assertSame('Doctrine', $query->getParameter('name')->getValue());
+    }
 }
diff --git a/tests/Doctrine/Tests/ORM/QueryBuilderTest.php b/tests/Doctrine/Tests/ORM/QueryBuilderTest.php
@@ -1172,4 +1172,81 @@ public function testGetAllAliasesWithJoins()
 
         $this->assertEquals(['u', 'g'], $aliases);
     }
+
+    /**
+     * @group 6699
+     */
+    public function testGetParameterTypeJuggling()
+    {
+        $builder = $this->_em->createQueryBuilder()
+                             ->select('u')
+                             ->from('Doctrine\Tests\Models\CMS\CmsUser', 'u')
+                             ->where('u.id = ?0');
+
+        $builder->setParameter(0, 0);
+
+        self::assertCount(1, $builder->getParameters());
+        self::assertSame(0, $builder->getParameter(0)->getValue());
+        self::assertSame(0, $builder->getParameter('0')->getValue());
+    }
+
+    /**
+     * @group 6699
+     */
+    public function testSetParameterWithNameZeroIsNotOverridden()
+    {
+        $builder = $this->_em->createQueryBuilder()
+                             ->select('u')
+                             ->from('Doctrine\Tests\Models\CMS\CmsUser', 'u')
+                             ->where('u.id != ?0')
+                             ->andWhere('u.username = :name');
+
+        $builder->setParameter(0, 0);
+        $builder->setParameter('name', 'Doctrine');
+
+        self::assertCount(2, $builder->getParameters());
+        self::assertSame(0, $builder->getParameter('0')->getValue());
+        self::assertSame('Doctrine', $builder->getParameter('name')->getValue());
+    }
+
+    /**
+     * @group 6699
+     */
+    public function testSetParameterWithNameZeroDoesNotOverrideAnotherParameter()
+    {
+        $builder = $this->_em->createQueryBuilder()
+                             ->select('u')
+                             ->from('Doctrine\Tests\Models\CMS\CmsUser', 'u')
+                             ->where('u.id != ?0')
+                             ->andWhere('u.username = :name');
+
+        $builder->setParameter('name', 'Doctrine');
+        $builder->setParameter(0, 0);
+
+        self::assertCount(2, $builder->getParameters());
+        self::assertSame(0, $builder->getParameter(0)->getValue());
+        self::assertSame('Doctrine', $builder->getParameter('name')->getValue());
+    }
+
+    /**
+     * @group 6699
+     */
+    public function testSetParameterWithTypeJugglingWorks()
+    {
+        $builder = $this->_em->createQueryBuilder()
+                             ->select('u')
+                             ->from('Doctrine\Tests\Models\CMS\CmsUser', 'u')
+                             ->where('u.id != ?0')
+                             ->andWhere('u.username = :name');
+
+        $builder->setParameter('0', 1);
+        $builder->setParameter('name', 'Doctrine');
+        $builder->setParameter(0, 2);
+        $builder->setParameter('0', 3);
+
+        self::assertCount(2, $builder->getParameters());
+        self::assertSame(3, $builder->getParameter(0)->getValue());
+        self::assertSame(3, $builder->getParameter('0')->getValue());
+        self::assertSame('Doctrine', $builder->getParameter('name')->getValue());
+    }
 }