Merge pull request #384 from FabioBatSilva/DDC1885

[DDC-1885] Fix quotes in many to many persisters

lib/Doctrine/ORM/Persisters/BasicEntityPersister.php

@@ -858,7 +858,11 @@ class BasicEntityPersister
         if ($assoc['isOwningSide']) {
             $quotedJoinTable = $this->quoteStrategy->getJoinTableName($assoc, $sourceClass, $this->_platform);
 
-            foreach ($assoc['relationToSourceKeyColumns'] as $relationKeyColumn => $sourceKeyColumn) {
+            foreach ($assoc['joinTable']['joinColumns'] as $joinColumn) {
+                $relationKeyColumn  = $joinColumn['name'];
+                $sourceKeyColumn    = $joinColumn['referencedColumnName'];
+                $quotedKeyColumn    = $this->quoteStrategy->getJoinColumnName($joinColumn, $sourceClass, $this->_platform);
+
                 if ($sourceClass->containsForeignIdentifier) {
                     $field = $sourceClass->getFieldForColumn($sourceKeyColumn);
                     $value = $sourceClass->reflFields[$field]->getValue($sourceEntity);
@@ -868,9 +872,9 @@ class BasicEntityPersister
                         $value = $value[$this->_em->getClassMetadata($sourceClass->associationMappings[$field]['targetEntity'])->identifier[0]];
                     }
 
-                    $criteria[$quotedJoinTable . "." . $relationKeyColumn] = $value;
+                    $criteria[$quotedJoinTable . "." . $quotedKeyColumn] = $value;
                 } else if (isset($sourceClass->fieldNames[$sourceKeyColumn])) {
-                    $criteria[$quotedJoinTable . "." . $relationKeyColumn] = $sourceClass->reflFields[$sourceClass->fieldNames[$sourceKeyColumn]]->getValue($sourceEntity);
+                    $criteria[$quotedJoinTable . "." . $quotedKeyColumn] = $sourceClass->reflFields[$sourceClass->fieldNames[$sourceKeyColumn]]->getValue($sourceEntity);
                 } else {
                     throw MappingException::joinColumnMustPointToMappedField(
                         $sourceClass->name, $sourceKeyColumn
@@ -882,7 +886,11 @@ class BasicEntityPersister
             $quotedJoinTable = $this->quoteStrategy->getJoinTableName($owningAssoc, $sourceClass, $this->_platform);
 
             // TRICKY: since the association is inverted source and target are flipped
-            foreach ($owningAssoc['relationToTargetKeyColumns'] as $relationKeyColumn => $sourceKeyColumn) {
+            foreach ($owningAssoc['joinTable']['inverseJoinColumns'] as $joinColumn) {
+                $relationKeyColumn  = $joinColumn['name'];
+                $sourceKeyColumn    = $joinColumn['referencedColumnName'];
+                $quotedKeyColumn    = $this->quoteStrategy->getJoinColumnName($joinColumn, $sourceClass, $this->_platform);
+
                 if ($sourceClass->containsForeignIdentifier) {
                     $field = $sourceClass->getFieldForColumn($sourceKeyColumn);
                     $value = $sourceClass->reflFields[$field]->getValue($sourceEntity);
@@ -892,9 +900,9 @@ class BasicEntityPersister
                         $value = $value[$this->_em->getClassMetadata($sourceClass->associationMappings[$field]['targetEntity'])->identifier[0]];
                     }
 
-                    $criteria[$quotedJoinTable . "." . $relationKeyColumn] = $value;
+                    $criteria[$quotedJoinTable . "." . $quotedKeyColumn] = $value;
                 } else if (isset($sourceClass->fieldNames[$sourceKeyColumn])) {
-                    $criteria[$quotedJoinTable . "." . $relationKeyColumn] = $sourceClass->reflFields[$sourceClass->fieldNames[$sourceKeyColumn]]->getValue($sourceEntity);
+                    $criteria[$quotedJoinTable . "." . $quotedKeyColumn] = $sourceClass->reflFields[$sourceClass->fieldNames[$sourceKeyColumn]]->getValue($sourceEntity);
                 } else {
                     throw MappingException::joinColumnMustPointToMappedField(
                         $sourceClass->name, $sourceKeyColumn
@@ -1144,31 +1152,27 @@ class BasicEntityPersister
      */
     protected function _getSelectManyToManyJoinSQL(array $manyToMany)
     {
-        if ($manyToMany['isOwningSide']) {
+        $conditions         = array();
-            $owningAssoc = $manyToMany;
+        $association        = $manyToMany;
-            $joinClauses = $manyToMany['relationToTargetKeyColumns'];
+        $sourceTableAlias   = $this->_getSQLTableAlias($this->_class->name);
-        } else {
+
-            $owningAssoc = $this->_em->getClassMetadata($manyToMany['targetEntity'])->associationMappings[$manyToMany['mappedBy']];
+        if ( ! $manyToMany['isOwningSide']) {
-            $joinClauses = $owningAssoc['relationToSourceKeyColumns'];
+            $targetEntity   = $this->_em->getClassMetadata($manyToMany['targetEntity']);
+            $association    = $targetEntity->associationMappings[$manyToMany['mappedBy']];
         }
 
-        $joinTableName = $this->quoteStrategy->getJoinTableName($owningAssoc, $this->_class, $this->_platform);
+        $joinTableName  = $this->quoteStrategy->getJoinTableName($association, $this->_class, $this->_platform);
-        $joinSql = '';
+        $joinColumns    = ($manyToMany['isOwningSide'])
+            ? $association['joinTable']['inverseJoinColumns']
+            : $association['joinTable']['joinColumns'];
 
-        foreach ($joinClauses as $joinTableColumn => $sourceColumn) {
+        foreach ($joinColumns as $joinColumn) {
-            if ($joinSql != '') $joinSql .= ' AND ';
+            $quotedSourceColumn = $this->quoteStrategy->getJoinColumnName($joinColumn, $this->_class, $this->_platform);
-
+            $quotedTargetColumn = $this->quoteStrategy->getReferencedJoinColumnName($joinColumn, $this->_class, $this->_platform);
-            if ($this->_class->containsForeignIdentifier && ! isset($this->_class->fieldNames[$sourceColumn])) {
+            $conditions[]       = $sourceTableAlias . '.' . $quotedTargetColumn . ' = ' . $joinTableName . '.' . $quotedSourceColumn;
-                $quotedColumn = $sourceColumn; // join columns cannot be quoted
-            } else {
-                $quotedColumn = $this->quoteStrategy->getColumnName($this->_class->fieldNames[$sourceColumn], $this->_class, $this->_platform);
-            }
-
-            $joinSql .= $this->_getSQLTableAlias($this->_class->name) . '.' . $quotedColumn . ' = '
-                      . $joinTableName . '.' . $joinTableColumn;
         }
 
-        return ' INNER JOIN ' . $joinTableName . ' ON ' . $joinSql;
+        return ' INNER JOIN ' . $joinTableName . ' ON ' . implode(' AND ', $conditions);
     }
 
     /**

lib/Doctrine/ORM/Persisters/ManyToManyPersister.php

@@ -40,11 +40,20 @@ class ManyToManyPersister extends AbstractCollectionPersister
      */
     protected function _getDeleteRowSQL(PersistentCollection $coll)
     {
+        $columns = array();
         $mapping = $coll->getMapping();
         $class   = $this->_em->getClassMetadata(get_class($coll->getOwner()));
 
+        foreach ($mapping['joinTable']['joinColumns'] as $joinColumn) {
+            $columns[] = $this->quoteStrategy->getJoinColumnName($joinColumn, $class, $this->platform);
+        }
+
+        foreach ($mapping['joinTable']['inverseJoinColumns'] as $joinColumn) {
+            $columns[] = $this->quoteStrategy->getJoinColumnName($joinColumn, $class, $this->platform);
+        }
+
         return 'DELETE FROM ' . $this->quoteStrategy->getJoinTableName($mapping, $class, $this->platform)
-             . ' WHERE ' . implode(' = ? AND ', $mapping['joinTableColumns']) . ' = ?';
+             . ' WHERE ' . implode(' = ? AND ', $columns) . ' = ?';
     }
 
     /**
@@ -76,11 +85,19 @@ class ManyToManyPersister extends AbstractCollectionPersister
      */
     protected function _getInsertRowSQL(PersistentCollection $coll)
     {
+        $columns    = array();
         $mapping    = $coll->getMapping();
-        $columns    = $mapping['joinTableColumns'];
         $class      = $this->_em->getClassMetadata(get_class($coll->getOwner()));
         $joinTable  = $this->quoteStrategy->getJoinTableName($mapping, $class, $this->platform);
 
+        foreach ($mapping['joinTable']['joinColumns'] as $joinColumn) {
+            $columns[] = $this->quoteStrategy->getJoinColumnName($joinColumn, $class, $this->platform);
+        }
+
+        foreach ($mapping['joinTable']['inverseJoinColumns'] as $joinColumn) {
+            $columns[] = $this->quoteStrategy->getJoinColumnName($joinColumn, $class, $this->platform);
+        }
+
         return 'INSERT INTO ' . $joinTable . ' (' . implode(', ', $columns) . ')'
              . ' VALUES (' . implode(', ', array_fill(0, count($columns), '?')) . ')';
     }
@@ -147,12 +164,17 @@ class ManyToManyPersister extends AbstractCollectionPersister
      */
     protected function _getDeleteSQL(PersistentCollection $coll)
     {
-        $class      = $this->_em->getClassMetadata(get_class($coll->getOwner()));
+        $columns    = array();
         $mapping    = $coll->getMapping();
+        $class      = $this->_em->getClassMetadata(get_class($coll->getOwner()));
         $joinTable  = $this->quoteStrategy->getJoinTableName($mapping, $class, $this->platform);
 
+        foreach ($mapping['joinTable']['joinColumns'] as $joinColumn) {
+            $columns[] = $this->quoteStrategy->getJoinColumnName($joinColumn, $class, $this->platform);
+        }
+
         return 'DELETE FROM ' . $joinTable
-             . ' WHERE ' . implode(' = ? AND ', array_keys($mapping['relationToSourceKeyColumns'])) . ' = ?';
+             . ' WHERE ' . implode(' = ? AND ', $columns) . ' = ?';
     }
 
     /**
@@ -190,41 +212,42 @@ class ManyToManyPersister extends AbstractCollectionPersister
      */
     public function count(PersistentCollection $coll)
     {
-        $mapping = $filterMapping = $coll->getMapping();
+        $conditions     = array();
-        $class   = $this->_em->getClassMetadata($mapping['sourceEntity']);
+        $params         = array();
-        $id      = $this->_em->getUnitOfWork()->getEntityIdentifier($coll->getOwner());
+        $mapping        = $coll->getMapping();
+        $association    = $mapping;
+        $class          = $this->_em->getClassMetadata($mapping['sourceEntity']);
+        $id             = $this->_em->getUnitOfWork()->getEntityIdentifier($coll->getOwner());
 
-        if ($mapping['isOwningSide']) {
+        if ( ! $mapping['isOwningSide']) {
-            $joinColumns = $mapping['relationToSourceKeyColumns'];
+            $targetEntity   = $this->_em->getClassMetadata($mapping['targetEntity']);
-        } else {
+            $association    = $targetEntity->associationMappings[$mapping['mappedBy']];
-            $mapping = $this->_em->getClassMetadata($mapping['targetEntity'])->associationMappings[$mapping['mappedBy']];
-            $joinColumns = $mapping['relationToTargetKeyColumns'];
         }
 
-        $whereClauses = array();
+        $joinColumns = ( ! $mapping['isOwningSide'])
-        $params  = array();
+            ? $association['joinTable']['inverseJoinColumns']
+            : $association['joinTable']['joinColumns'];
 
-        foreach ($mapping['joinTableColumns'] as $joinTableColumn) {
+        foreach ($joinColumns as $joinColumn) {
-            if ( ! isset($joinColumns[$joinTableColumn])) {
+            $columnName     = $this->quoteStrategy->getJoinColumnName($joinColumn, $class, $this->platform);
-                continue;
+            $referencedName = $joinColumn['referencedColumnName'];
-            }
+            $conditions[]   = $columnName . ' = ?';
-
+            $params[]       = ($class->containsForeignIdentifier)
-            $whereClauses[] = $joinTableColumn . ' = ?';
+                ? $id[$class->getFieldForColumn($referencedName)]
-
+                : $id[$class->fieldNames[$referencedName]];
-            $params[] = ($class->containsForeignIdentifier)
-                ? $id[$class->getFieldForColumn($joinColumns[$joinTableColumn])]
-                : $id[$class->fieldNames[$joinColumns[$joinTableColumn]]];
         }
 
-        list($joinTargetEntitySQL, $filterSql) = $this->getFilterSql($filterMapping);
+        $joinTableName = $this->quoteStrategy->getJoinTableName($association, $class, $this->platform);
+        list($joinTargetEntitySQL, $filterSql) = $this->getFilterSql($mapping);
+
         if ($filterSql) {
-            $whereClauses[] = $filterSql;
+            $conditions[] = $filterSql;
         }
 
         $sql = 'SELECT COUNT(*)'
-            . ' FROM ' . $this->quoteStrategy->getJoinTableName($mapping, $class, $this->platform) . ' t'
+            . ' FROM ' . $joinTableName . ' t'
             . $joinTargetEntitySQL
-            . ' WHERE ' . implode(' AND ', $whereClauses);
+            . ' WHERE ' . implode(' AND ', $conditions);
 
         return $this->_conn->fetchColumn($sql, $params);
     }

tests/Doctrine/Tests/Models/Quote/User.php

@@ -34,7 +34,7 @@ class User
     public $address;
 
     /**
-     * @ManyToMany(targetEntity="Group", inversedBy="users", cascade={"persist"})
+     * @ManyToMany(targetEntity="Group", inversedBy="users", cascade={"all"})
      * @JoinTable(name="`quote-users-groups`",
      *      joinColumns={
      *          @JoinColumn(
@@ -52,6 +52,25 @@ class User
      */
     public $groups;
 
+    /**
+     * @ManyToMany(targetEntity="Group", inversedBy="users", cascade={"all"}, fetch="EXTRA_LAZY")
+     * @JoinTable(name="`quote-extra-lazy-users-groups`",
+     *      joinColumns={
+     *          @JoinColumn(
+     *              name="`user-id`",
+     *              referencedColumnName="`user-id`"
+     *          )
+     *      },
+     *      inverseJoinColumns={
+     *          @JoinColumn(
+     *              name="`group-id`",
+     *              referencedColumnName="`group-id`"
+     *          )
+     *      }
+     * )
+     */
+    public $extraLazyGroups;
+
     public function __construct()
     {
         $this->phones = new ArrayCollection;

tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1885Test.php

@@ -0,0 +1,173 @@
+<?php
+
+namespace Doctrine\Tests\ORM\Functional\Ticket;
+
+use Doctrine\Tests\Models\Quote\Group;
+use Doctrine\Tests\Models\Quote\User;
+
+require_once __DIR__ . '/../../../TestInit.php';
+
+/**
+ * @group DDC-1845
+ * @group DDC-1885
+ */
+class DDC1885Test extends \Doctrine\Tests\OrmFunctionalTestCase
+{
+
+    /**
+     * @var \Doctrine\Tests\Models\Quote\User
+     */
+    private $user;
+
+    protected function setUp()
+    {
+        parent::setUp();
+
+        try {
+            $this->_schemaTool->createSchema(array(
+                $this->_em->getClassMetadata('Doctrine\Tests\Models\Quote\User'),
+                $this->_em->getClassMetadata('Doctrine\Tests\Models\Quote\Group'),
+                $this->_em->getClassMetadata('Doctrine\Tests\Models\Quote\Address'),
+            ));
+        } catch(\Exception $e) {
+        }
+
+        $user           = new User();
+        $user->name     = "FabioBatSilva";
+        $user->email    = "fabio.bat.silva@gmail.com";
+        $user->groups[] = new Group('G 1');
+        $user->groups[] = new Group('G 2');
+        $this->user     = $user;
+
+        // Create
+        $this->_em->persist($user);
+        $this->_em->flush();
+        $this->_em->clear();
+        
+    }
+
+    public function testCreateRetreaveUpdateDelete()
+    {
+        $user   = $this->user;
+        $g1     = $user->getGroups()->get(0);
+        $g2     = $user->getGroups()->get(1);
+
+        $u1Id   = $user->id;
+        $g1Id   = $g1->id;
+        $g2Id   = $g2->id;
+
+        // Retreave
+        $user = $this->_em->find('Doctrine\Tests\Models\Quote\User', $u1Id);
+
+        $this->assertInstanceOf('Doctrine\Tests\Models\Quote\User', $user);
+        $this->assertEquals('FabioBatSilva', $user->name);
+        $this->assertEquals($u1Id, $user->id);
+        
+        $this->assertCount(2, $user->groups);
+
+        $g1 = $user->getGroups()->get(0);
+        $g2 = $user->getGroups()->get(1);
+
+        $this->assertInstanceOf('Doctrine\Tests\Models\Quote\Group', $g1);
+        $this->assertInstanceOf('Doctrine\Tests\Models\Quote\Group', $g2);
+
+        $g1->name = 'Bar 11';
+        $g2->name = 'Foo 22';
+
+        // Update
+        $this->_em->persist($user);
+        $this->_em->flush();
+        $this->_em->clear();
+
+        $user = $this->_em->find('Doctrine\Tests\Models\Quote\User', $u1Id);
+
+        $this->assertInstanceOf('Doctrine\Tests\Models\Quote\User', $user);
+        $this->assertEquals('FabioBatSilva', $user->name);
+        $this->assertEquals($u1Id, $user->id);
+
+        // Delete
+        $this->_em->remove($user);
+        
+        $this->_em->flush();
+        $this->_em->clear();
+
+        $this->assertNull($this->_em->find('Doctrine\Tests\Models\Quote\User', $u1Id));
+        $this->assertNull($this->_em->find('Doctrine\Tests\Models\Quote\Group', $g1Id));
+        $this->assertNull($this->_em->find('Doctrine\Tests\Models\Quote\Group', $g2Id));
+    }
+
+    public function testRemoveItem()
+    {
+        $user   = $this->user;
+        $u1Id   = $user->id;
+        $user   = $this->_em->find('Doctrine\Tests\Models\Quote\User', $u1Id);
+
+        $this->assertInstanceOf('Doctrine\Tests\Models\Quote\User', $user);
+        $this->assertEquals('FabioBatSilva', $user->name);
+        $this->assertEquals($u1Id, $user->id);
+
+        $this->assertCount(2, $user->groups);
+        $this->assertInstanceOf('Doctrine\Tests\Models\Quote\Group', $user->getGroups()->get(0));
+        $this->assertInstanceOf('Doctrine\Tests\Models\Quote\Group', $user->getGroups()->get(1));
+
+        $user->getGroups()->remove(0);
+
+        // Update
+        $this->_em->persist($user);
+        $this->_em->flush();
+        $this->_em->clear();
+
+        $user = $this->_em->find('Doctrine\Tests\Models\Quote\User', $u1Id);
+
+        $this->assertInstanceOf('Doctrine\Tests\Models\Quote\User', $user);
+        $this->assertEquals('FabioBatSilva', $user->name);
+        $this->assertEquals($u1Id, $user->id);
+
+        $this->assertCount(1, $user->getGroups());
+    }
+
+    public function testClearAll()
+    {
+        $user   = $this->user;
+        $u1Id   = $user->id;
+        $user   = $this->_em->find('Doctrine\Tests\Models\Quote\User', $u1Id);
+
+        $this->assertInstanceOf('Doctrine\Tests\Models\Quote\User', $user);
+        $this->assertEquals('FabioBatSilva', $user->name);
+        $this->assertEquals($u1Id, $user->id);
+
+        $this->assertCount(2, $user->groups);
+        $this->assertInstanceOf('Doctrine\Tests\Models\Quote\Group', $user->getGroups()->get(0));
+        $this->assertInstanceOf('Doctrine\Tests\Models\Quote\Group', $user->getGroups()->get(1));
+
+        $user->getGroups()->clear();
+
+        // Update
+        $this->_em->persist($user);
+        $this->_em->flush();
+        $this->_em->clear();
+
+        $user = $this->_em->find('Doctrine\Tests\Models\Quote\User', $u1Id);
+
+        $this->assertInstanceOf('Doctrine\Tests\Models\Quote\User', $user);
+        $this->assertEquals('FabioBatSilva', $user->name);
+        $this->assertEquals($u1Id, $user->id);
+
+        $this->assertCount(0, $user->getGroups());
+    }
+
+    public function testCountExtraLazy()
+    {
+        $user   = $this->user;
+        $u1Id   = $user->id;
+        $user   = $this->_em->find('Doctrine\Tests\Models\Quote\User', $u1Id);
+
+        $this->assertInstanceOf('Doctrine\Tests\Models\Quote\User', $user);
+        $this->assertEquals('FabioBatSilva', $user->name);
+        $this->assertEquals($u1Id, $user->id);
+
+        $this->assertCount(0, $user->extraLazyGroups);
+        $this->assertInstanceOf('Doctrine\Tests\Models\Quote\Group', $user->getGroups()->get(0));
+        $this->assertInstanceOf('Doctrine\Tests\Models\Quote\Group', $user->getGroups()->get(1));
+    }
+}