Merge pull request #7940 from doctrine/GH-7864-ExtraLazyRemoveElement

[GH-7864] Bugfix in PersistentCollection::removeElement for EXTRA_LAZY.

Author: beberlei

docs/en/tutorials/extra-lazy-associations.rst

@@ -35,6 +35,15 @@ With extra lazy collections you can now not only add entities to large collectio
 easily using a combination of ``count`` and ``slice``.
 
 
+.. warning::
+
+   ``removeElement`` directly issued DELETE queries to the database from
+   version 2.4.0 to 2.7.0.  This circumvents the flush operation and might run
+   outside a transactional boundary if you don't create one yourself. We
+   consider this a critical bug in the assumptio of how the ORM works and
+   reverted ``removeElement`` EXTRA_LAZY behavior in 2.7.1.
+
+
 Enabling Extra-Lazy Associations
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

lib/Doctrine/ORM/Cache/Persister/Collection/AbstractCollectionPersister.php

@@ -243,20 +243,6 @@ public function get(PersistentCollection $collection, $index)
         return $this->persister->get($collection, $index);
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function removeElement(PersistentCollection $collection, $element)
-    {
-        if ($persisterResult = $this->persister->removeElement($collection, $element)) {
-            $this->evictCollectionCache($collection);
-            $this->evictElementCache($this->sourceEntity->rootEntityName, $collection->getOwner());
-            $this->evictElementCache($this->targetEntity->rootEntityName, $element);
-        }
-
-        return $persisterResult;
-    }
-
     /**
      * {@inheritdoc}
      */

lib/Doctrine/ORM/PersistentCollection.php

@@ -367,16 +367,6 @@ public function remove($key)
      */
     public function removeElement($element)
     {
-        if ( ! $this->initialized && $this->association['fetch'] === ClassMetadata::FETCH_EXTRA_LAZY) {
-            if ($this->collection->contains($element)) {
-                return $this->collection->removeElement($element);
-            }
-
-            $persister = $this->em->getUnitOfWork()->getCollectionPersister($this->association);
-
-            return $persister->removeElement($this, $element);
-        }
-
         $removed = parent::removeElement($element);
 
         if ( ! $removed) {

lib/Doctrine/ORM/Persisters/Collection/CollectionPersister.php

@@ -90,16 +90,6 @@ public function contains(PersistentCollection $collection, $element);
      */
     public function containsKey(PersistentCollection $collection, $key);
 
-    /**
-     * Removes an element.
-     *
-     * @param \Doctrine\ORM\PersistentCollection $collection
-     * @param object                             $element
-     *
-     * @return mixed
-     */
-    public function removeElement(PersistentCollection $collection, $element);
-
     /**
      * Gets an element by key.
      *

lib/Doctrine/ORM/Persisters/Collection/ManyToManyPersister.php

@@ -211,22 +211,6 @@ public function contains(PersistentCollection $collection, $element)
         return (bool) $this->conn->fetchColumn($sql, $params, 0, $types);
     }
 
-    /**
-     * {@inheritDoc}
-     */
-    public function removeElement(PersistentCollection $collection, $element)
-    {
-        if ( ! $this->isValidEntityState($element)) {
-            return false;
-        }
-
-        list($quotedJoinTable, $whereClauses, $params, $types) = $this->getJoinTableRestrictions($collection, $element, false);
-
-        $sql = 'DELETE FROM ' . $quotedJoinTable . ' WHERE ' . implode(' AND ', $whereClauses);
-
-        return (bool) $this->conn->executeUpdate($sql, $params, $types);
-    }
-
     /**
      * {@inheritDoc}
      */

lib/Doctrine/ORM/Persisters/Collection/OneToManyPersister.php

@@ -166,28 +166,6 @@ public function contains(PersistentCollection $collection, $element)
         return $persister->exists($element, $criteria);
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function removeElement(PersistentCollection $collection, $element)
-    {
-        $mapping = $collection->getMapping();
-
-        if ( ! $mapping['orphanRemoval']) {
-            // no-op: this is not the owning side, therefore no operations should be applied
-            return false;
-        }
-
-        if ( ! $this->isValidEntityState($element)) {
-            return false;
-        }
-
-        return $this
-            ->uow
-            ->getEntityPersister($mapping['targetEntity'])
-            ->delete($element);
-    }
-
     /**
      * {@inheritdoc}
      */

tests/Doctrine/Tests/ORM/Cache/Persister/Collection/AbstractCollectionPersisterTest.php

@@ -227,23 +227,6 @@ public function testInvokeContainsKey()
         $this->assertFalse($persister->containsKey($collection, 0));
     }
 
-    public function testInvokeRemoveElement()
-    {
-        $entity     = new State("Foo");
-        $element    = new State("Bar");
-        $persister  = $this->createPersisterDefault();
-        $collection = $this->createCollection($entity);
-
-        $this->em->getUnitOfWork()->registerManaged($entity, ['id'=>1], ['id'=>1, 'name'=>'Foo']);
-
-        $this->collectionPersister->expects($this->once())
-            ->method('removeElement')
-            ->with($this->equalTo($collection), $this->equalTo($element))
-            ->will($this->returnValue(false));
-
-        $this->assertFalse($persister->removeElement($collection, $element));
-    }
-
     public function testInvokeGet()
     {
         $entity     = new State("Foo");