PersistentCollection not written to originalEntityData after flush

[achterberatung]

Bug Report

Q	A
BC Break	?
Version	2.7.0,2.6.4

Summary

When flushing an Entity with ManyToMany-association, originalEntityData is written new (of course). But the (old) PersistentCollection is not added to the new originalEntityData. Changing the collection inside the entity let doctrine think, the collection is all new, resulting in duplicate key errors from database.

Current behavior

After a flush, doctrine tries to insert elements of a ManyToMany-association even if they are already in the database.

How to reproduce

Given two Entities:

/**
  *@ORM\Entity
  */
class MainEntity {

    public function __construct () {
        $this->second = new ArrayCollection();
    }

   /**
     * @ORM\Column(name="id", type="integer", nullable=false)
     * @ORM\Id
     * @ORM\GeneratedValue(strategy="IDENTITY")
     */
    protected $id;

    /**
     * @ORM\ManyToMany(targetEntity="SecondEntity", fetch="LAZY")
     */
    protected $second;

    /**
     * @ORM\Column(name="text", type="string", length=1000, nullable=true)
     */
    protected $text;

with getters and setters...

}

/**
  *@ORM\Entity
  */
class SecondEntity {

    /**
     *
     * @ORM\Column(name="id", type="integer", nullable=false)
     * @ORM\Id
     * @ORM\GeneratedValue(strategy="IDENTITY")
     */
    protected $id;

}

and corresponding repositories you can reproduce the Bug.

First initialize some Data with given $em, $mainRepo and $secondRepo:

$s1 = new SecondEntity();
$s2 = new SecondEntity();
$em->persist($s1);
$em->persist($s2);

$main = new MainEntity();
$main->getSecond()->add($s1);
$main->getSecond()->add($s2);
$em->persist($main);
$em->flush();

Now (i've done it in a second request) reproduce the bug:

$main = $mainRepo->find(1);
$main->setText("test");
$em->flush();

$s1 = $secondRepo->find(1);
$newCollection = new ArrayCollection();
$newCollection->add($s1);
$main->setSecond($newCollection);
$em->flush();

Results in:

  An exception occurred while executing 'INSERT INTO doctrine_bug_main_x_second (main_id, second_id) VALUES (?, ?)' with params [1, 1]:
web_1  | 
web_1  | SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '1-1' for key 'PRIMARY'
web_1  |        /var/www/html/core/framework/composer-vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/AbstractMySQLDriver.php:55
web_1  |        /var/www/html/core/framework/composer-vendor/doctrine/dbal/lib/Doctrine/DBAL/DBALException.php : 169
web_1  |                Doctrine\DBAL\Driver\AbstractMySQLDriver->convertException()
web_1  |        /var/www/html/core/framework/composer-vendor/doctrine/dbal/lib/Doctrine/DBAL/DBALException.php : 145
web_1  |                Doctrine\DBAL\DBALException::wrapException()
web_1  |        /var/www/html/core/framework/composer-vendor/doctrine/dbal/lib/Doctrine/DBAL/Connection.php : 1063
web_1  |                Doctrine\DBAL\DBALException::driverExceptionDuringQuery()
web_1  |        /var/www/html/core/framework/composer-vendor/doctrine/orm/lib/Doctrine/ORM/Persisters/Collection/ManyToManyPersister.php : 86
web_1  |                Doctrine\DBAL\Connection->executeUpdate()
web_1  |        /var/www/html/core/framework/composer-vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php : 404
web_1  |                Doctrine\ORM\Persisters\Collection\ManyToManyPersister->update()
web_1  |        /var/www/html/core/framework/composer-vendor/doctrine/orm/lib/Doctrine/ORM/EntityManager.php : 359
web_1  |                Doctrine\ORM\UnitOfWork->commit()
web_1  |        /var/www/html/core/framework/composer-vendor/doctrine/orm/lib/Doctrine/ORM/Decorator/EntityManagerDecorator.php : 188
web_1  |                Doctrine\ORM\EntityManager->flush()

Expected behavior

also ignored collection have to get into originalEntityData.

My Quickfix in UnitOfWork:

public function computeChangeSet(ClassMetadata $class, $entity)
{
...
$actualData = [];
//New array for ignored Collections
$ignoredCollectionData = [];
foreach ($class->reflFields as $name => $refProp) {
   ...
   if ($value->getOwner() === $entity) {
      //Add the Collection to the Array
      $ignoredCollectionData[$name] = $value;
      continue;
   }
...
if ($changeSet) {
  $this->entityChangeSets[$oid]   = $changeSet;
  //add also ignored collections
  $this->originalEntityData[$oid] = $actualData + $ignoredCollectionData;
  $this->entityUpdates[$oid]      = $entity;
}
...and again a few lines later

[lcobucci]

The problem is that you're overriding the persistent collection, which is not the intended usage for that. Have you tried this (on the second run):

$main = $mainRepo->find(1);
$main->setText("test");
$em->flush();

$s1 = $secondRepo->find(1);
$main->getSecond()->clear();
$main->getSecond()->add($s1);
$em->flush();

[SenseException]

@achterberatung Did the answer help with your problem? If you consider this done, we could close this issue.

[achterberatung]

@lcobucci Unfortunately our real code of course is a little bit more complicated. We have a separation of the doctrine entity and our data-framework. So since we are using doctrine, we had the collections in ManyToX-Associations generated new without knowledge of the original collection. And this never was a problem, until we needed to do a double flush because of including some legacy business functions, which are directly interacting with the database. Refactor that would be much more effort and a break of our separation concept, so patching doctrine the way i showed is much more suitable for us.

@SenseException Maybe we are misusing doctrine to reproduce it, but my bug ticket shall show, that after a flush the originalEntityData is by definition in a faulty state, because elements are missing. When first loading the PersistentCollection, it is written to originalEntityData, but by doing a flush, they are just sortet out, because they already know their parent entity. Any other data is still written to originalEntityData. And everything works well, if I put these ignored collections in an array and appending them to the new originalEntityData.

If you do not consider as a bug, maybe we can convert it to a feature request?

[SenseException]

I'm not sure if I understand it right. Can you please provide us a PR with a failing test closer to your use case so we can evaluate if this is an edge case or a possible feature request? It looks like the first example doesn't really cover the problem you have for a suitable fix.