Merge pull request doctrine#5668 from petitchevalroux/many-to-many-cr…

…iteria-fixes Many to many criteria fixes
jaspernbrouwer · Feb 16, 2016 · d814ad7 · d814ad7
2 parents 02eaf6a + 9461839
commit d814ad7
Show file tree

Hide file tree

Showing 9 changed files with 362 additions and 22 deletions.
diff --git a/lib/Doctrine/ORM/Persisters/Collection/ManyToManyPersister.php b/lib/Doctrine/ORM/Persisters/Collection/ManyToManyPersister.php
@@ -236,36 +236,52 @@ public function loadCriteria(PersistentCollection $collection, Criteria $criteri
         $mapping       = $collection->getMapping();
         $owner         = $collection->getOwner();
         $ownerMetadata = $this->em->getClassMetadata(get_class($owner));
+        $id            = $this->uow->getEntityIdentifier($owner);
+        $targetClass   = $this->em->getClassMetadata($mapping['targetEntity']);
+        $onConditions  = $this->getOnConditionSQL($mapping);
         $whereClauses  = $params = array();
 
-        foreach ($mapping['relationToSourceKeyColumns'] as $key => $value) {
+        if ( ! $mapping['isOwningSide']) {
+            $associationSourceClass = $targetClass;
+            $mapping = $targetClass->associationMappings[$mapping['mappedBy']];
+            $sourceRelationMode = 'relationToTargetKeyColumns';
+        } else {
+            $associationSourceClass = $ownerMetadata;
+            $sourceRelationMode = 'relationToSourceKeyColumns';
+        }
+
+        foreach ($mapping[$sourceRelationMode] as $key => $value) {
             $whereClauses[] = sprintf('t.%s = ?', $key);
-            $params[]       = $ownerMetadata->getFieldValue($owner, $value);
+            $params[] = $ownerMetadata->containsForeignIdentifier
+                ? $id[$ownerMetadata->getFieldForColumn($value)]
+                : $id[$ownerMetadata->fieldNames[$value]];
         }
 
         $parameters = $this->expandCriteriaParameters($criteria);
 
         foreach ($parameters as $parameter) {
             list($name, $value) = $parameter;
-            $whereClauses[]     = sprintf('te.%s = ?', $name);
+            $field = $this->quoteStrategy->getColumnName($name, $targetClass, $this->platform);
+            $whereClauses[]     = sprintf('te.%s = ?', $field);
             $params[]           = $value;
         }
 
-        $mapping      = $collection->getMapping();
-        $targetClass  = $this->em->getClassMetadata($mapping['targetEntity']);
         $tableName    = $this->quoteStrategy->getTableName($targetClass, $this->platform);
-        $joinTable    = $this->quoteStrategy->getJoinTableName($mapping, $ownerMetadata, $this->platform);
-        $onConditions = $this->getOnConditionSQL($mapping);
+        $joinTable    = $this->quoteStrategy->getJoinTableName($mapping, $associationSourceClass, $this->platform);
 
         $rsm = new Query\ResultSetMappingBuilder($this->em);
-        $rsm->addRootEntityFromClassMetadata($mapping['targetEntity'], 'te');
+        $rsm->addRootEntityFromClassMetadata($targetClass->name, 'te');
 
         $sql = 'SELECT ' . $rsm->generateSelectClause()
             . ' FROM ' . $tableName . ' te'
             . ' JOIN ' . $joinTable  . ' t ON'
             . implode(' AND ', $onConditions)
             . ' WHERE ' . implode(' AND ', $whereClauses);
 
+        $sql .= $this->getOrderingSql($criteria, $targetClass);
+
+        $sql .= $this->getLimitSql($criteria);
+
         $stmt = $this->conn->executeQuery($sql, $params);
 
         return $this
@@ -728,4 +744,43 @@ private function expandCriteriaParameters(Criteria $criteria)
 
         return $types;
     }
+
+    /**
+     * @param Criteria $criteria
+     * @param ClassMetadata $targetClass
+     * @return string
+     */
+    private function getOrderingSql(Criteria $criteria, ClassMetadata $targetClass)
+    {
+        $orderings = $criteria->getOrderings();
+        if ($orderings) {
+            $orderBy = [];
+            foreach ($orderings as $name => $direction) {
+                $field = $this->quoteStrategy->getColumnName(
+                    $name,
+                    $targetClass,
+                    $this->platform
+                );
+                $orderBy[] = $field . ' ' . $direction;
+            }
+
+            return ' ORDER BY ' . implode(', ', $orderBy);
+        }
+        return '';
+    }
+
+    /**
+     * @param Criteria $criteria
+     * @return string
+     * @throws \Doctrine\DBAL\DBALException
+     */
+    private function getLimitSql(Criteria $criteria)
+    {
+        $limit  = $criteria->getMaxResults();
+        $offset = $criteria->getFirstResult();
+        if ($limit !== null || $offset !== null) {
+            return $this->platform->modifyLimitQuery('', $limit, $offset);
+        }
+        return '';
+    }
 }
diff --git a/tests/Doctrine/Tests/Models/CMS/CmsTag.php b/tests/Doctrine/Tests/Models/CMS/CmsTag.php
@@ -0,0 +1,48 @@
+<?php
+/*
+ * To change this template, choose Tools | Templates
+ * and open the template in the editor.
+ */
+
+namespace Doctrine\Tests\Models\CMS;
+
+/**
+ * Description of CmsTag
+ *
+ * @Entity
+ * @Table(name="cms_tags")
+ */
+class CmsTag
+{
+    /**
+     * @Id
+     * @Column(type="integer")
+     * @GeneratedValue
+     */
+    public $id;
+    /**
+     * @Column(length=50, name="tag_name", nullable=true)
+     */
+    public $name;
+    /**
+     * @ManyToMany(targetEntity="CmsUser", mappedBy="tags")
+     */
+    public $users;
+
+    public function setName($name) {
+        $this->name = $name;
+    }
+
+    public function getName() {
+        return $this->name;
+    }
+
+    public function addUser(CmsUser $user) {
+        $this->users[] = $user;
+    }
+
+    public function getUsers() {
+        return $this->users;
+    }
+}
+
diff --git a/tests/Doctrine/Tests/Models/CMS/CmsUser.php b/tests/Doctrine/Tests/Models/CMS/CmsUser.php
@@ -162,6 +162,14 @@ class CmsUser
      *      )
      */
     public $groups;
+    /**
+     * @ManyToMany(targetEntity="CmsTag", inversedBy="users", cascade={"all"})
+     * @JoinTable(name="cms_users_tags",
+     *      joinColumns={@JoinColumn(name="user_id", referencedColumnName="id")},
+     *      inverseJoinColumns={@JoinColumn(name="tag_id", referencedColumnName="id")}
+     *      )
+     */
+    public $tags;
 
     public $nonPersistedProperty;
 
@@ -171,6 +179,7 @@ public function __construct() {
         $this->phonenumbers = new ArrayCollection;
         $this->articles = new ArrayCollection;
         $this->groups = new ArrayCollection;
+        $this->tags = new ArrayCollection;
     }
 
     public function getId() {
@@ -217,6 +226,15 @@ public function getGroups() {
         return $this->groups;
     }
 
+    public function addTag(CmsTag $tag) {
+        $this->tags[] = $tag;
+        $tag->addUser($this);
+    }
+
+    public function getTags() {
+        return $this->tags;
+    }
+
     public function removePhonenumber($index) {
         if (isset($this->phonenumbers[$index])) {
             $ph = $this->phonenumbers[$index];

diff --git a/tests/Doctrine/Tests/ORM/Functional/DatabaseDriverTest.php b/tests/Doctrine/Tests/ORM/Functional/DatabaseDriverTest.php
@@ -117,15 +117,18 @@ public function testDetectManyToManyTables()
             $this->markTestSkipped('Platform does not support foreign keys.');
         }
 
-        $metadatas = $this->extractClassMetadata(array("CmsUsers", "CmsGroups"));
+        $metadatas = $this->extractClassMetadata(array("CmsUsers", "CmsGroups", "CmsTags"));
 
         $this->assertArrayHasKey('CmsUsers', $metadatas, 'CmsUsers entity was not detected.');
         $this->assertArrayHasKey('CmsGroups', $metadatas, 'CmsGroups entity was not detected.');
+        $this->assertArrayHasKey('CmsTags', $metadatas, 'CmsTags entity was not detected.');
 
-        $this->assertEquals(2, count($metadatas['CmsUsers']->associationMappings));
+        $this->assertEquals(3, count($metadatas['CmsUsers']->associationMappings));
         $this->assertArrayHasKey('group', $metadatas['CmsUsers']->associationMappings);
         $this->assertEquals(1, count($metadatas['CmsGroups']->associationMappings));
         $this->assertArrayHasKey('user', $metadatas['CmsGroups']->associationMappings);
+        $this->assertEquals(1, count($metadatas['CmsTags']->associationMappings));
+        $this->assertArrayHasKey('user', $metadatas['CmsGroups']->associationMappings);
     }
 
     public function testIgnoreManyToManyTableWithoutFurtherForeignKeyDetails()

diff --git a/tests/Doctrine/Tests/ORM/Functional/ManyToManyBasicAssociationTest.php b/tests/Doctrine/Tests/ORM/Functional/ManyToManyBasicAssociationTest.php
@@ -3,6 +3,7 @@
 namespace Doctrine\Tests\ORM\Functional;
 
 use Doctrine\Common\Collections\Criteria;
+use Doctrine\Tests\Models\CMS\CmsTag;
 use Doctrine\Tests\Models\CMS\CmsUser,
     Doctrine\Tests\Models\CMS\CmsGroup,
     Doctrine\Common\Collections\ArrayCollection;
@@ -377,6 +378,154 @@ public function testClearBeforeLazyLoad()
         $this->assertEquals(0, count($user->groups));
     }
 
+    /**
+     * @group DDC-3952
+     */
+    public function testManyToManyOrderByIsNotIgnored()
+    {
+        $user = $this->addCmsUserGblancoWithGroups(1);
+
+        $group1 = new CmsGroup;
+        $group2 = new CmsGroup;
+        $group3 = new CmsGroup;
+
+        $group1->name = 'C';
+        $group2->name = 'A';
+        $group3->name = 'B';
+
+        $user->addGroup($group1);
+        $user->addGroup($group2);
+        $user->addGroup($group3);
+
+        $this->_em->persist($user);
+        $this->_em->flush();
+
+        $this->_em->clear();
+
+        $user = $this->_em->find(get_class($user), $user->id);
+
+        $criteria = Criteria::create()
+            ->orderBy(['name' => Criteria::ASC]);
+
+        $this->assertEquals(
+            ['A', 'B', 'C', 'Developers_0'],
+            $user
+                ->getGroups()
+                ->matching($criteria)
+                ->map(function (CmsGroup $group) {
+                    return $group->getName();
+                })
+                ->toArray()
+        );
+    }
+
+    /**
+     * @group DDC-3952
+     */
+    public function testManyToManyOrderByHonorsFieldNameColumnNameAliases()
+    {
+        $user = new CmsUser;
+        $user->name = 'Guilherme';
+        $user->username = 'gblanco';
+        $user->status = 'developer';
+
+        $tag1 = new CmsTag;
+        $tag2 = new CmsTag;
+        $tag3 = new CmsTag;
+
+        $tag1->name = 'C';
+        $tag2->name = 'A';
+        $tag3->name = 'B';
+
+        $user->addTag($tag1);
+        $user->addTag($tag2);
+        $user->addTag($tag3);
+
+        $this->_em->persist($user);
+        $this->_em->flush();
+
+        $this->_em->clear();
+
+        $user = $this->_em->find(get_class($user), $user->id);
+
+        $criteria = Criteria::create()
+            ->orderBy(['name' => Criteria::ASC]);
+
+        $this->assertEquals(
+            ['A', 'B', 'C'],
+            $user
+                ->getTags()
+                ->matching($criteria)
+                ->map(function (CmsTag $tag) {
+                    return $tag->getName();
+                })
+                ->toArray()
+        );
+    }
+
+    public function testMatchingWithLimit()
+    {
+        $user = $this->addCmsUserGblancoWithGroups(2);
+        $this->_em->clear();
+
+        $user = $this->_em->find(get_class($user), $user->id);
+
+        $groups = $user->groups;
+        $this->assertFalse($user->groups->isInitialized(), "Pre-condition: lazy collection");
+
+        $criteria = Criteria::create()->setMaxResults(1);
+        $result   = $groups->matching($criteria);
+
+        $this->assertCount(1, $result);
+
+        $this->assertFalse($user->groups->isInitialized(), "Post-condition: matching does not initialize collection");
+    }
+
+    public function testMatchingWithOffset()
+    {
+        $user = $this->addCmsUserGblancoWithGroups(2);
+        $this->_em->clear();
+
+        $user = $this->_em->find(get_class($user), $user->id);
+
+        $groups = $user->groups;
+        $this->assertFalse($user->groups->isInitialized(), "Pre-condition: lazy collection");
+
+        $criteria = Criteria::create()->setFirstResult(1);
+        $result   = $groups->matching($criteria);
+
+        $this->assertCount(1, $result);
+
+        $firstGroup = $result->first();
+        $this->assertEquals('Developers_1', $firstGroup->name);
+
+        $this->assertFalse($user->groups->isInitialized(), "Post-condition: matching does not initialize collection");
+    }
+
+    public function testMatchingWithLimitAndOffset()
+    {
+        $user = $this->addCmsUserGblancoWithGroups(5);
+        $this->_em->clear();
+
+        $user = $this->_em->find(get_class($user), $user->id);
+
+        $groups = $user->groups;
+        $this->assertFalse($user->groups->isInitialized(), "Pre-condition: lazy collection");
+
+        $criteria = Criteria::create()->setFirstResult(1)->setMaxResults(3);
+        $result   = $groups->matching($criteria);
+
+        $this->assertCount(3, $result);
+
+        $firstGroup = $result->first();
+        $this->assertEquals('Developers_1', $firstGroup->name);
+
+        $lastGroup = $result->last();
+        $this->assertEquals('Developers_3', $lastGroup->name);
+
+        $this->assertFalse($user->groups->isInitialized(), "Post-condition: matching does not initialize collection");
+    }
+
     public function testMatching()
     {
         $user = $this->addCmsUserGblancoWithGroups(2);