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

New Issue

admin · 2026-01-22T14:36:26+01:00

admin commented

2026-01-22 14:36:26 +01:00

Originally created by @doctrinebot on GitHub (Nov 6, 2014).

Originally assigned to: @beberlei on GitHub.

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();
        }
    }

Originally created by @doctrinebot on GitHub (Nov 6, 2014). Originally assigned to: @beberlei on GitHub. 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. ``` php 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(); } } ```

admin added the Bug label 2026-01-22 14:36:26 +01:00

admin commented

2026-01-22 14:36:28 +01:00

@doctrinebot commented on GitHub (Nov 7, 2014):

Comment created by @ocramius:

Can you please abstract your problem into a test case?

@doctrinebot commented on GitHub (Nov 7, 2014): Comment created by @ocramius: Can you please abstract your problem into a test case?

admin commented

2026-01-22 14:36:28 +01:00

@doctrinebot commented on GitHub (Nov 7, 2014):

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 commented on GitHub (Nov 7, 2014): 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.

admin commented

2026-01-22 14:36:28 +01:00

@doctrinebot commented on GitHub (Nov 7, 2014):

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 commented on GitHub (Nov 7, 2014): 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.

admin commented

2026-01-22 14:36:28 +01:00

@doctrinebot commented on GitHub (May 27, 2015):

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 commented on GitHub (May 27, 2015): 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.

admin commented

2026-01-22 14:36:28 +01:00

@doctrinebot commented on GitHub (May 27, 2015):

Comment created by deatheriam:

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

@doctrinebot commented on GitHub (May 27, 2015): Comment created by deatheriam: @Ward Peeters, have you ever created the test case in question?

admin commented

2026-01-22 14:36:28 +01:00

@doctrinebot commented on GitHub (May 27, 2015):

Comment created by wardpeet:

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

@doctrinebot commented on GitHub (May 27, 2015): Comment created by wardpeet: No i haven't will make one today if you're not doing it. I made workaround for now.

admin commented

2026-01-22 14:36:29 +01:00

@githoober commented on GitHub (Dec 7, 2018):

Bump.

@githoober commented on GitHub (Dec 7, 2018): Bump.

admin commented

2026-01-22 14:36:30 +01:00

@Ocramius commented on GitHub (Dec 7, 2018):

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

@Ocramius commented on GitHub (Dec 7, 2018): @githoober please don't bump: provide a failing test case instead.

admin commented

2026-01-22 14:36:30 +01:00

@Dis1092006 commented on GitHub (Dec 17, 2018):

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"
        );
    }

@Dis1092006 commented on GitHub (Dec 17, 2018): Test case for [ManyToManyBasicAssociationTest.php](https://github.com/doctrine/doctrine2/blob/master/tests/Doctrine/Tests/ORM/Functional/ManyToManyBasicAssociationTest.php): ```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" ); } ```

admin commented

2026-01-22 14:36:30 +01:00

@skylord123 commented on GitHub (Nov 17, 2020):

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:
01187c9260/lib/Doctrine/ORM/PersistentCollection.php (L565)

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 commented on GitHub (Nov 17, 2020): 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`: https://github.com/doctrine/orm/blob/01187c9260cd085529ddd1273665217cae659640/lib/Doctrine/ORM/PersistentCollection.php#L565 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.

admin commented

2026-01-22 14:36:30 +01:00

@skylord123 commented on GitHub (Nov 17, 2020):

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?

@skylord123 commented on GitHub (Nov 17, 2020): 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?

Sign in to join this conversation.

Branches Tags

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: doctrine/archived-orm#4166