ChangeSet building for embedManyDocuments fails to detect removes

[Jan0707]

When removing a document from an embedMany PersistentCollection, the changeSet for the parent document does not contain the property/field as changed, even though the document is successfully removed.

It does work however, if the property has been initialised before, e.g. by adding another document to the collection.

For better understanding, I have attached a unit test, reproducing the issue:

<?php

namespace Adticket\Elvis\CoreBundle\Tests\EventListener\Doctrine;

use Doctrine\Common\Collections\ArrayCollection;

use Doctrine\ODM\MongoDB\Event\OnFlushEventArgs;
use Doctrine\ODM\MongoDB\Events;
use Doctrine\ODM\MongoDB\Mapping\Annotations as MongoDB;
use Doctrine\ODM\MongoDB\PersistentCollection;
use Doctrine\ODM\MongoDB\Tests\BaseTest;

/**
 * Class DoctrineTest
 *
 * This test uses:
 * doctrine/mongodb-odm 1.0.4  Doctrine MongoDB Object Document Mapper
 * phpunit/phpunit      4.8.23 The PHP Unit Testing framework.
 */
class DoctrineTest extends BaseTest
{
    public function testBug()
    {
        $listener = new SimpleListener();

        $this->dm->getEventManager()->addEventListener(Events::onFlush, $listener);

        $document     = new Document();
        $document->id = new \MongoId();

        $document->embeddedCollection = new ArrayCollection();
        $document->embeddedCollection->add(new Embed('A'));

        $listener->onFlushCallable = function (OnFlushEventArgs $eventArgs) {
            $unitOfWork = $eventArgs->getDocumentManager()->getUnitOfWork();

            foreach ($unitOfWork->getScheduledDocumentUpserts() as $document) {
                $changeSet = $unitOfWork->getDocumentChangeSet($document);

                /** @var PersistentCollection $persistentCollection */
                $persistentCollection = $changeSet['embeddedCollection'][0];

                $this->assertEquals(1, count($persistentCollection->getInsertDiff()));
                /** @var Embed $newEmbed */
                $newEmbed = $persistentCollection->getInsertDiff()[0];
                $this->assertInstanceOf(Embed::class, $newEmbed);
                $this->assertEquals('A', $newEmbed->value);

            }
        };

        $id = (string) $document->id;

        $this->dm->persist($document);
        $this->dm->flush();
        $this->dm->clear();

        $document = $this->dm->getRepository(Document::class)->find($id);

        $document->embeddedCollection->add(new Embed('B'));

        $listener->onFlushCallable = function (OnFlushEventArgs $eventArgs) {
            $unitOfWork = $eventArgs->getDocumentManager()->getUnitOfWork();

            foreach ($unitOfWork->getScheduledDocumentUpdates() as $document) {
                $changeSet = $unitOfWork->getDocumentChangeSet($document);

                /** @var PersistentCollection $persistentCollection */
                $persistentCollection = $changeSet['embeddedCollection'][0];

                $this->assertEquals(1, count($persistentCollection->getInsertDiff()));
                // Using "1" as index here, as apparently the index from the original PersistentCollection is used
                /** @var Embed $newEmbed */
                $newEmbed = $persistentCollection->getInsertDiff()[1];
                $this->assertInstanceOf(Embed::class, $newEmbed);
                $this->assertEquals('B', $newEmbed->value);
            }
        };

        $this->dm->flush();
        $this->dm->clear();

        $document = $this->dm->getRepository(Document::class)->find($id);

        $document->embeddedCollection->removeElement($document->embeddedCollection->first());

//        This line is required for doctrine do detect the removal in the change set correctly
        $document->embeddedCollection->add(new Embed('C'));

        $listener->onFlushCallable = function (OnFlushEventArgs $eventArgs) {
            $unitOfWork = $eventArgs->getDocumentManager()->getUnitOfWork();

            foreach ($unitOfWork->getScheduledDocumentUpdates() as $document) {
                $changeSet = $unitOfWork->getDocumentChangeSet($document);

                // This will fail if the above comment remains commented out
                // The change ( = removal of first element ) in the embeddedCollection property / Persistent Collection will not be detected
                // Thus the index 'embeddedCollection' will be missing from the array

                /** @var PersistentCollection $persistentCollection */
                $persistentCollection = $changeSet['embeddedCollection'][0];

                $this->assertEquals(1, count($persistentCollection->getDeletedDocuments()));

                // Using "0" as index here, as apparently the index from the original PersistentCollection is used
                /** @var Embed $removedEmbed */
                $removedEmbed = $persistentCollection->getDeletedDocuments()[0];

                $this->assertInstanceOf(Embed::class, $removedEmbed);
                $this->assertEquals('A', $removedEmbed->value);
            }
        };

        $this->dm->flush();
        $this->dm->clear();
    }
}

/**
 * Class SimpleListener
 */
class SimpleListener
{
    /**
     * @var callable|null
     */
    public $onFlushCallable;

    /**
     * @param OnFlushEventArgs $eventArgs
     */
    public function onFlush(OnFlushEventArgs $eventArgs)
    {
        if ($this->onFlushCallable) {
            call_user_func($this->onFlushCallable, $eventArgs);
        }
    }
}

/**
 * @MongoDB\Document()
 */
class Document
{
    /**
     * @MongoDB\Id()
     *
     * @var \MongoId
     */
    public $id;

    /**
     * @MongoDB\EmbedMany(targetDocument="Adticket\Elvis\CoreBundle\Tests\EventListener\Doctrine\Embed")
     *
     * @var ArrayCollection|Embed[]
     */
    public $embeddedCollection;
}

/**
 * @MongoDB\EmbeddedDocument()
 */
class Embed
{
    /**
     * @MongoDB\String()
     *
     * @var string
     */
    public $value;

    /**
     * @param $value
     */
    public function __construct($value)
    {
        $this->value = $value;
    }
}

[malarzm]

@Jan0707 thanks for the test case! Is the error also reproducible on 1.1.x branch of ODM? Asking since 1.0.x is no longer supported

[Jan0707]

Using version ~1.1 solved the problem. Which is technically good news, but also sucks for us, as we cannot upgrade the version just yet in this specific project.

I will close this issue. Thanks for the help !

[malarzm]

@Jan0707 looking through 1.1.x changelog I'd guess this was relevant PR: #1437 if you'd like to backport it into fork of yours. Also the upgrade to 1.1.x is quite painless unless it's PHP version that is holding you back.