DDC-3372: PersistentCollection clear function takes snapshot when the collection is cleared

[doctrinebot]

Jira issue originally created by user wardpeet:

I'm not sure if it's a bug or you guys have a reason why you do it. The problem occurs when you do a $coll->clear() it will clear the collection and whenever it's cleared the takeSnapshot is ran as well so our snapshot is empty so we lose the previous state of the collection.

    public function clear()
    {
        if ($this->initialized && $this->isEmpty()) {
            return;
        }

        $uow = $this->em->getUnitOfWork();

        if ($this->association['type'] & ClassMetadata::TO_MANY &&
            $this->association['orphanRemoval'] &&
            $this->owner) {
            // we need to initialize here, as orphan removal acts like implicit cascadeRemove,
            // hence for event listeners we need the objects in memory.
            $this->initialize();

            foreach ($this->coll as $element) {
                $uow->scheduleOrphanRemoval($element);
            }
        }

        $this->coll->clear();

        $this->initialized = true; // direct call, {@link initialize()} is too expensive

        if ($this->association['isOwningSide'] && $this->owner) {
            $this->changed();

            $uow->scheduleCollectionDeletion($this);

            $this->takeSnapshot();
        }
    }

[doctrinebot]

Comment created by @Ocramius:

Can you please abstract your problem into a test case?

[doctrinebot]

Comment created by wardpeet:

if that helps yes. The problem is that i'm not sure it's a bug. It might be the way you guys wanted it to be.
I'll create a test case might be better to understand.

[doctrinebot]

Comment created by @Ocramius:

It looks like a bug to me: the snapshot should be taken only when no snapshot was there upfront, as far as I know.

[doctrinebot]

Comment created by deatheriam:

I just got bitten by the same issue. I need to have access to all elements being cleared in a collection before it gets actually cleared, but because its internal collection gets cleared before PersistentCollection is able to take a snapshot, all these elements are lost.

[doctrinebot]

Comment created by deatheriam:

@ward Peeters, have you ever created the test case in question?

[doctrinebot]

Comment created by wardpeet:

No i haven't will make one today if you're not doing it. I made workaround for now.

[Ocramius]

@githoober please don't bump: provide a failing test case instead.

[Dis1092006]

Test case for ManyToManyBasicAssociationTest.php:

    public function testManyToManyCollectionTraceClearedItems()
    {
        $user = $this->addCmsUserGblancoWithGroups($groupCount = 10);

        $user->groups->clear();

        $this->assertEquals(
            $groupCount,
            count($user->groups->getDeleteDiff()),
            "There should be 10 groups in the snapshot of the collection"
        );
    }

[skylord123]

I just ran into this problem today. I have a relation that if it gets emptied the association shows no changes. I traced it down to this line within PersistentCollection:

orm/lib/Doctrine/ORM/PersistentCollection.php

Line 565 in 01187c9

$this->takeSnapshot();

Why do we take a snapshot when a collection is cleared? This doesn't make any sense to me.

#2272 seems to be the exact same issue.

[skylord123]

Okay I see the problem. We want clear() to be fast so we mark the collection for deletion, clear it in memory, then take a new snapshot to prevent the collection from iterating the elements and trying to remove them one by one after it has already been cleared as well as avoiding calculating these changes at all. Basically we do this for performance reasons.

Considering a snapshot is "A snapshot of the collection at the moment it was fetched from the database." (from the PHPDoc) I don't think we should be taking another snapshot during the PersistentCollection::clear() as this doesn't conform with this description. Currently this description gives users a false narrative (one that I fell for and by the looks of it many others).

I built an auditor that watches updates to various entities and logs what the changes are and who did them. For this reason I need the snapshot to be the collection the moment it was fetched otherwise when a relation gets emptied I get no log records.

I would love it if we could find a better way of keeping PersistentCollection::clear() fast without having to touch PersistentCollection snapshot. Especially considering Symfony forms automatically call ::clear() for relations when all options are de-selected (and there is no way to avoid this).

Any ideas?