orphan removal removes children, not orphans #5854

Open
opened 2026-01-22 15:19:56 +01:00 by admin · 20 comments
Owner

Originally created by @stijn-at-work on GitHub (Jan 21, 2018).

Originally assigned to: @stijn-at-work on GitHub.

Upgrade from doctrine/orm 2.5.14 to 2.6.0 in our Symfony based application caused the removal of our EnvironmentAccesses records. Before, the orphanRemoval=true annotation worked as expected and only removed orphans. After upgrade the orphanRemoval annotation removed all children, not only orphans.

User parent

    /**
     * @ORM\OneToMany(targetEntity="AccountsBundle\Entity\EnvironmentAccess", mappedBy="user", cascade={"persist", "remove"}, orphanRemoval=true)
     * @Expose
     * @Type("array<AccountsBundle\Entity\EnvironmentAccess>")
     * @var AccountsBundle\Entity\EnvironmentAccess[]
     */
    private $environmentAccess;

EnvironmentAccess child

    /**
     * @ORM\ManyToOne(targetEntity="AccountsBundle\Entity\User", inversedBy="environmentAccess")
     * @ORM\JoinColumn(name="user_id", referencedColumnName="id")
     */
    private $user;
Originally created by @stijn-at-work on GitHub (Jan 21, 2018). Originally assigned to: @stijn-at-work on GitHub. Upgrade from doctrine/orm 2.5.14 to 2.6.0 in our Symfony based application caused the removal of our EnvironmentAccesses records. Before, the orphanRemoval=true annotation worked as expected and only removed orphans. After upgrade the orphanRemoval annotation removed all children, not only orphans. **User parent** ``` /** * @ORM\OneToMany(targetEntity="AccountsBundle\Entity\EnvironmentAccess", mappedBy="user", cascade={"persist", "remove"}, orphanRemoval=true) * @Expose * @Type("array<AccountsBundle\Entity\EnvironmentAccess>") * @var AccountsBundle\Entity\EnvironmentAccess[] */ private $environmentAccess; ``` **EnvironmentAccess child** ``` /** * @ORM\ManyToOne(targetEntity="AccountsBundle\Entity\User", inversedBy="environmentAccess") * @ORM\JoinColumn(name="user_id", referencedColumnName="id") */ private $user; ```
admin added the BugRegression labels 2026-01-22 15:19:56 +01:00
Author
Owner

@Ocramius commented on GitHub (Jan 21, 2018):

What is the code causing the issue looking like?

Are any of the removed records removed and re-added to the association, by chance? If so, this is expected behaviour.

An example of how this can be triggered is needed in order to write an integration test.

See
https://github.com/doctrine/doctrine2/tree/2.6/tests/Doctrine/Tests/ORM/Functional/Ticket for examples of what would be needed here.

@Ocramius commented on GitHub (Jan 21, 2018): What is the code causing the issue looking like? Are any of the removed records removed and re-added to the association, by chance? If so, this is expected behaviour. An example of how this can be triggered is needed in order to write an integration test. See https://github.com/doctrine/doctrine2/tree/2.6/tests/Doctrine/Tests/ORM/Functional/Ticket for examples of what would be needed here.
Author
Owner

@stijn-at-work commented on GitHub (Jan 22, 2018):

There are no User records or EnvironmentAccess records removed and re-added.

The bug is gone when we remove 'orphanRemoval=true' from environmentAccess attribute.

Our hypotheses: a closer look at our (legacy) code shows that we have added both cascade="remove" and orphanRemoval=true instead of choosing for one of these mutually exclusive strategies.
Choosing for cascade="remove" strategy removes the bug.
Choosing for orphanRemoval=true does not though (!?).

EnvironmentAccesses are removed when we make a post or patch request to a User or EnvironmentAccess. e.g. POST [environment].localhost.com:8000/api/oauth/v2/token

@stijn-at-work commented on GitHub (Jan 22, 2018): There are no User records or EnvironmentAccess records removed and re-added. The bug is gone when we remove 'orphanRemoval=true' from environmentAccess attribute. Our hypotheses: a closer look at our (legacy) code shows that we have added both cascade="remove" and orphanRemoval=true instead of choosing for one of these mutually exclusive strategies. Choosing for cascade="remove" strategy removes the bug. Choosing for orphanRemoval=true does not though (!?). EnvironmentAccesses are removed when we make a post or patch request to a User or EnvironmentAccess. e.g. POST [environment].localhost.com:8000/api/oauth/v2/token
Author
Owner

@Ocramius commented on GitHub (Jan 22, 2018):

@stijin-at-work that doesn't help us find the bug. What we need is a reproducible test scenario that works in isolation. See https://github.com/doctrine/doctrine2/tree/2.6/tests/Doctrine/Tests/ORM/Functional/Ticket for examples.

@Ocramius commented on GitHub (Jan 22, 2018): @stijin-at-work that doesn't help us find the bug. What we need is a reproducible test scenario that works in isolation. See https://github.com/doctrine/doctrine2/tree/2.6/tests/Doctrine/Tests/ORM/Functional/Ticket for examples.
Author
Owner

@Padam87 commented on GitHub (May 9, 2018):

@Ocramius I can confirm the bug exists. Will investigate.

@Padam87 commented on GitHub (May 9, 2018): @Ocramius I can confirm the bug exists. Will investigate.
Author
Owner

@Padam87 commented on GitHub (May 9, 2018):

@Ocramius I believe the bug was introduced by the piece of code linked above.

At this point $orgValue is just an empty persistent collection. But thanks to this change it triggers a DELETE FROM table WHERE owner_id = x statement.

This only causes problems when orpanRemoval=true because of this part: 0b7d878cd3/lib/Doctrine/ORM/Persisters/Collection/OneToManyPersister.php (L35)

I would advise everyone to temporarily revert back to 2.5, because this can cause data loss.

@Padam87 commented on GitHub (May 9, 2018): @Ocramius I believe the bug was introduced by the piece of code linked above. At this point `$orgValue` is just an empty persistent collection. But thanks to this change it triggers a `DELETE FROM table WHERE owner_id = x` statement. This only causes problems when `orpanRemoval=true` because of this part: https://github.com/doctrine/doctrine2/blob/0b7d878cd340fed70e22404daa9656bb06cec74a/lib/Doctrine/ORM/Persisters/Collection/OneToManyPersister.php#L35 **I would advise everyone to temporarily revert back to 2.5, because this can cause data loss.**
Author
Owner

@Padam87 commented on GitHub (May 9, 2018):

#6976

@Padam87 commented on GitHub (May 9, 2018): #6976
Author
Owner

@Padam87 commented on GitHub (May 9, 2018):

https://github.com/symfony/symfony/issues/25937

@Padam87 commented on GitHub (May 9, 2018): https://github.com/symfony/symfony/issues/25937
Author
Owner

@Steveb-p commented on GitHub (Nov 26, 2018):

Looks like this hit me in my own app when updating to ORM 2.6. I've got a collection that is replaced by it's copy during form processing, and pre-existing entities are removed from database after redirect. It looks to me that replacing PersistentCollection instance with ArrayCollection triggers this effect, but will investigate further.

EDIT:
image

If I replace PersistentCollection with ArrayCollection in my setter, UnitOfWork's getScheduledCollectionUpdates() and getScheduledCollectionDeletions() report as above. Otherwise, if I simply check Collections against each other they appear as expected.

@Steveb-p commented on GitHub (Nov 26, 2018): Looks like this hit me in my own app when updating to ORM 2.6. I've got a collection that is replaced by it's copy during form processing, and pre-existing entities are removed from database after redirect. It looks to me that replacing `PersistentCollection` instance with `ArrayCollection` triggers this effect, but will investigate further. EDIT: ![image](https://user-images.githubusercontent.com/3183926/49019188-a96d5e80-f18d-11e8-80f3-2a59ac9bbc40.png) If I replace `PersistentCollection` with `ArrayCollection` in my setter, `UnitOfWork`'s `getScheduledCollectionUpdates()` and `getScheduledCollectionDeletions()` report as above. Otherwise, if I simply check Collections against each other they appear as expected.
Author
Owner

@Padam87 commented on GitHub (Nov 26, 2018):

@Steveb-p You should never replace collections, only manipulate them with add / remove.

This always was the preferred usage, but so far it had not caused any problems.

I linked the commit above which caused this change, but so far no one has acknowledged it as a BC break.

If you have a live system, revert back to 2.5 until you change your code to avoid data loss.

@Padam87 commented on GitHub (Nov 26, 2018): @Steveb-p You should never replace collections, only manipulate them with add / remove. This always was the preferred usage, but so far it had not caused any problems. I linked the commit above which caused this change, but so far no one has acknowledged it as a BC break. If you have a live system, revert back to 2.5 until you change your code to avoid data loss.
Author
Owner

@Steveb-p commented on GitHub (Nov 26, 2018):

@Padam87 Well, now I know :)
It didn't have any side-effects until this change. I couldn't find any explicit notices that mentioned that one should not replace Collection instances. In my case, I was using Collection's filter method to remove elements which went over a certain threshold and saved the result, overwriting the previous Collection, so it seemed like a good idea at the time ;)

@Steveb-p commented on GitHub (Nov 26, 2018): @Padam87 Well, now I know :) It didn't have any side-effects until this change. I couldn't find any explicit notices that mentioned that one should not replace `Collection` instances. In my case, I was using `Collection`'s `filter` method to remove elements which went over a certain threshold and saved the result, overwriting the previous `Collection`, so it seemed like a good idea at the time ;)
Author
Owner

@Padam87 commented on GitHub (Nov 26, 2018):

@Steveb-p Yeah, I didn't know before this either, it is not general knowledge at all.

I see it as a huge problem, because there are many ways a collection might get replaced. Your case is a perfect example, even a simple filter creates a new collection.

@Padam87 commented on GitHub (Nov 26, 2018): @Steveb-p Yeah, I didn't know before this either, it is not general knowledge at all. I see it as a huge problem, because there are many ways a collection might get replaced. Your case is a perfect example, even a simple filter creates a new collection.
Author
Owner

@mvannes commented on GitHub (Feb 6, 2019):

+1 in being affected by this issue, or at least very very closely related.

Mapping in the Entity 1

    /**
     * @ORM\ManyToOne(
     *     targetEntity="Entity",
     *     inversedBy="preferred_entity"
     * )
     * @ORM\JoinColumn(name="entity_id", referencedColumnName="id")
     */
    private $entity;

And mapping in entity 2

     /**
     * @ORM\OneToMany(
     *     targetEntity="PreferredEntity",
     *     mappedBy="entity",
     *     orphanRemoval=true
     * )
     */
    private $preferred_entity;

If the $entity field in Entity 1 is changed, orphan removal is triggered because the collection on the entity 2 side will be empty.

Reverting to a ~2.5 version fixes this.

@mvannes commented on GitHub (Feb 6, 2019): +1 in being affected by this issue, or at least very very closely related. Mapping in the Entity 1 ``` /** * @ORM\ManyToOne( * targetEntity="Entity", * inversedBy="preferred_entity" * ) * @ORM\JoinColumn(name="entity_id", referencedColumnName="id") */ private $entity; ``` And mapping in entity 2 ``` /** * @ORM\OneToMany( * targetEntity="PreferredEntity", * mappedBy="entity", * orphanRemoval=true * ) */ private $preferred_entity; ``` If the `$entity` field in Entity 1 is changed, orphan removal is triggered because the collection on the entity 2 side will be empty. Reverting to a `~2.5` version fixes this.
Author
Owner

@Padam87 commented on GitHub (Feb 6, 2019):

I don't think this BC break is gonna get acknowledged ever. It has been a year.

Revert to 2.5, and fix the usage of your collections.

The important thing is that you should not recreate or override your collections (orphaning a collection will trigger a delete statement for all entries), you must use the same object.

BAD

    public function setUsers($users): self
    {
        $this->users = new ArrayCollection(); // or []
        
        foreach ($users as $user) {
            $this->addUser($user);
        }

        return $this;
    }

GOOD

    public function setUsers($users): self
    {
        $this->users->clear();
        
        foreach ($users as $user) {
            $this->addUser($user);
        }

        return $this;
    }
@Padam87 commented on GitHub (Feb 6, 2019): I don't think this BC break is gonna get acknowledged ever. It has been a year. Revert to 2.5, and fix the usage of your collections. **The important thing is that you should not recreate or override your collections (orphaning a collection will trigger a delete statement for all entries), you must use the same object.** BAD ```php public function setUsers($users): self { $this->users = new ArrayCollection(); // or [] foreach ($users as $user) { $this->addUser($user); } return $this; } ``` GOOD ```php public function setUsers($users): self { $this->users->clear(); foreach ($users as $user) { $this->addUser($user); } return $this; } ```
Author
Owner

@mvannes commented on GitHub (Feb 6, 2019):

Yeah damn, saw the november comments and thought it was a very active issue still. But it's actually way older.
We'll be fixing it in the above way eventually. Luckily it only affects the Entity I was creating today in this exact way due to my specific use case of switching relations around a bit.
Should be a fun afternoon to fix in our codebase. Thanks for the examples @Padam87

@mvannes commented on GitHub (Feb 6, 2019): Yeah damn, saw the november comments and thought it was a very active issue still. But it's actually way older. We'll be fixing it in the above way eventually. Luckily it only affects the Entity I was creating today in this exact way due to my specific use case of switching relations around a bit. Should be a fun afternoon to fix in our codebase. Thanks for the examples @Padam87
Author
Owner

@damienfa commented on GitHub (Feb 23, 2020):

Is this the same issue ?
https://stackoverflow.com/questions/47037493/manytomany-relation-orphanremoval
I think it's really well explained this way.

@damienfa commented on GitHub (Feb 23, 2020): Is this the same issue ? https://stackoverflow.com/questions/47037493/manytomany-relation-orphanremoval I think it's really well explained this way.
Author
Owner

@Marco-Kawon commented on GitHub (Mar 31, 2020):

upvote ...

Caused data-loss on a production environment for about 2 months. 12 Entities affected. Was working fine for years until we upgraded to doctrine/orm 2.6 in February. Not even our tests catched it, because the entity still returned the relations with valid database IDs and data, but they were deleted already from database. Only after reload entity again from database, we noticed data is not there anymore.

Thanks to @Padam87 for pointing me to the right direction

@Marco-Kawon commented on GitHub (Mar 31, 2020): upvote ... Caused data-loss on a production environment for about 2 months. 12 Entities affected. Was working fine for years until we upgraded to doctrine/orm 2.6 in February. Not even our tests catched it, because the entity still returned the relations with valid database IDs and data, but they were deleted already from database. Only after reload entity again from database, we noticed data is not there anymore. Thanks to @Padam87 for pointing me to the right direction
Author
Owner

@ianef commented on GitHub (Sep 1, 2020):

This is also causing me a lot of grief, I've upgraded 2 projects to 2.6 recently and had to remove the orphanRemoval from all my entities. I use JSON deserialisation extensively in my projects.

I find it hard to understand why this issue has not seen any positive action from the maintainers. Loss of data is a very serious issue, ignoring this is not exactly a responsible approach and potentially leaves developers open to financial harm through loss of customers data.

@ianef commented on GitHub (Sep 1, 2020): This is also causing me a lot of grief, I've upgraded 2 projects to 2.6 recently and had to remove the orphanRemoval from all my entities. I use JSON deserialisation extensively in my projects. I find it hard to understand why this issue has not seen any positive action from the maintainers. Loss of data is a very serious issue, ignoring this is not exactly a responsible approach and potentially leaves developers open to financial harm through loss of customers data.
Author
Owner

@albe commented on GitHub (Oct 4, 2020):

We stumbled across this too.

What would work at least for the "easy" case of just replacing the collection with a new collection containing the same elements (or cloning), would be to add following code at https://github.com/doctrine/orm/blob/2.6/lib/Doctrine/ORM/UnitOfWork.php#L745:

if ($orgValue->toArray() === $actualValue->toArray()) {
    continue;
}

What would be the drawback of this? The equality check on the arrays should be relatively fast, no? Would that be viable @Ocramius ?

@albe commented on GitHub (Oct 4, 2020): We stumbled across this too. What would work at least for the "easy" case of just replacing the collection with a new collection containing the same elements (or cloning), would be to add following code at https://github.com/doctrine/orm/blob/2.6/lib/Doctrine/ORM/UnitOfWork.php#L745: ```php if ($orgValue->toArray() === $actualValue->toArray()) { continue; } ``` What would be the drawback of this? The equality check on the arrays should be relatively fast, no? Would that be viable @Ocramius ?
Author
Owner

@vcastro45 commented on GitHub (Jan 20, 2022):

Any news about it ? I use doctrine/orm 2.8.2 and I'm still facing the problem with orphanRemoval.
As same as @ianef , I use a lot of JSON Deserialization (Symfony as REST API) and it's very hard to deal with the removal of "orphanRemoval" from the code, and the solution suggested by @Padam87 (Yes, my entities are coming from JSON Deserialization, so even with this kind of setters, collections are "hard setted".

Downgrade to doctrine/orm 2.5 is no longer acceptable today.

@vcastro45 commented on GitHub (Jan 20, 2022): Any news about it ? I use doctrine/orm 2.8.2 and I'm still facing the problem with orphanRemoval. As same as @ianef , I use a lot of JSON Deserialization (Symfony as REST API) and it's very hard to deal with the removal of "orphanRemoval" from the code, and the solution suggested by @Padam87 (Yes, my entities are coming from JSON Deserialization, so even with this kind of setters, collections are "hard setted". Downgrade to doctrine/orm 2.5 is no longer acceptable today.
Author
Owner

@jperovic commented on GitHub (Apr 20, 2023):

Phew, I am working on upgrading a legacy project from Symfony 2 and this just exploded in my face. :( Fortunately, I noticed it in a test environment.

I upgraded from doctrine/orm 2.4 to 2.7.5 and there it was...

@jperovic commented on GitHub (Apr 20, 2023): Phew, I am working on upgrading a legacy project from Symfony 2 and this just exploded in my face. :( Fortunately, I noticed it in a test environment. I upgraded from `doctrine/orm` 2.4 to 2.7.5 and there it was...
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: doctrine/archived-orm#5854