OrphanRemoval doesn't work if set new collection without explicit remove() #5463

New Issue

admin · 2026-01-22T15:08:18+01:00

admin commented

2026-01-22 15:08:18 +01:00

Originally created by @benbor on GitHub (Mar 15, 2017).

I've relations OneToMany and I want to bulk update all tags (I want to delete few tags and add other few) for one post

$post = new Post();// for example with id = 1
$post->tags->add(new Tag()); // for example with id = 1
$post->tags->add(new Tag()); // for example with id = 2

$em->persist($post);
$em->flush();
$em->clear();

$post = $repository->findById(1);
$collection = new ArrayCollection(); //new collection contain
$collection->add (new Tag(1)); // one old tag with id  = 1
$collection->add (new Tag()); // and contain one new tag (after flush id will be 3)
//as you can see tag#3 removed
$post->tags = $collection;

$em->flush();

Expected:
Doctrine saves Post#1 with Tag#1 (tag witch doesn't changed), Tag#3 (new tag)

Actual:
Doctrine saves Post#1 with Tag#1, Tag#2, Tag#3

My hot fix:
before set new collection I explicit remove tags, which should be removed

 $em->clear();

$post = $repository->findById(1);

$collection = new ArrayCollection(); 
$collection->add (new Tag(1)); 
$collection->add (new Tag()); 

//my hot fix
foreach ($post->tag as $originTag) {
       if (!$collection->contains($originTag)) {
            $post->tag->removeElement($originTag); //explicit remove 
       }
}

$post->tags = $collection;

$em->flush();

And all works fine! Creating and deleting works fine. (Editing another fields like "name" also works fine)

What is My question? Why doctrine can't do it self? I'm sure orphanRemoval option should delete it self.

My entities:

/**
* @ORM\Entity()
*/
class Post
{
//id
 /**
    * 
    * @Type("ArrayCollection<Tag>")
    * @ORM\OneToMany(targetEntity="Tag", mappedBy="post", cascade={"all"}, orphanRemoval=true)
    */
   public $tags;
}

/**
* @ORM\Entity
*/
class Tag
{  
//id
   /**
    * @ORM\ManyToOne(targetEntity="Post", inversedBy="tags")
    * @ORM\JoinColumn(name="post_id", referencedColumnName="id", nullable=true)
    *
    */
   public $post;
}

Originally created by @benbor on GitHub (Mar 15, 2017). I've relations OneToMany and I want to bulk update all tags (I want to delete few tags and add other few) for one post ```php $post = new Post();// for example with id = 1 $post->tags->add(new Tag()); // for example with id = 1 $post->tags->add(new Tag()); // for example with id = 2 $em->persist($post); $em->flush(); $em->clear(); $post = $repository->findById(1); $collection = new ArrayCollection(); //new collection contain $collection->add (new Tag(1)); // one old tag with id = 1 $collection->add (new Tag()); // and contain one new tag (after flush id will be 3) //as you can see tag#3 removed $post->tags = $collection; $em->flush(); ``` **Expected:** Doctrine saves Post#1 with Tag#1 (tag witch doesn't changed), Tag#3 (new tag) **Actual:** Doctrine saves Post#1 with Tag#1, Tag#2, Tag#3 **My hot fix:** before set new collection I explicit remove tags, which should be removed ```php $em->clear(); $post = $repository->findById(1); $collection = new ArrayCollection(); $collection->add (new Tag(1)); $collection->add (new Tag()); //my hot fix foreach ($post->tag as $originTag) { if (!$collection->contains($originTag)) { $post->tag->removeElement($originTag); //explicit remove } } $post->tags = $collection; $em->flush(); ``` And all works fine! Creating and deleting works fine. (Editing another fields like "name" also works fine) What is My question? Why doctrine can't do it self? I'm sure orphanRemoval option should delete it self. My entities: ```php /** * @ORM\Entity() */ class Post { //id /** * * @Type("ArrayCollection<Tag>") * @ORM\OneToMany(targetEntity="Tag", mappedBy="post", cascade={"all"}, orphanRemoval=true) */ public $tags; } /** * @ORM\Entity */ class Tag { //id /** * @ORM\ManyToOne(targetEntity="Post", inversedBy="tags") * @ORM\JoinColumn(name="post_id", referencedColumnName="id", nullable=true) * */ public $post; } ```

admin commented

2026-01-22 15:08:19 +01:00

@Ocramius commented on GitHub (Mar 15, 2017):

Replacing collection instances is not really fully supported, although the bug looks legit. Calling clear() on the original collection would probably cause chaos though...

@Ocramius commented on GitHub (Mar 15, 2017): Replacing collection instances is not really fully supported, although the bug looks legit. Calling `clear()` on the original collection would probably cause chaos though...

admin commented

2026-01-22 15:08:20 +01:00

@benbor commented on GitHub (Mar 15, 2017):

@Ocramius method clear() didn't help me.

// ...
$post->tags->clear();
$post->tags = $collection;
// ...

It saves only Post#1 and doesn't save Tags at all .

@benbor commented on GitHub (Mar 15, 2017): @Ocramius method `clear()` didn't help me. ```php // ... $post->tags->clear(); $post->tags = $collection; // ... ``` It saves only Post#1 and doesn't save Tags at all .

admin commented

2026-01-22 15:08:20 +01:00

@alsma commented on GitHub (Mar 16, 2017):

I've also met similar issue today:

$newTag = new Tag();
$post->tags->add($newTag);
$post->tags->clear();
$post->tags->add($newTag)

new tag is scheduled for orphan removal, and not inserted

@alsma commented on GitHub (Mar 16, 2017): I've also met similar issue today: ``` $newTag = new Tag(); $post->tags->add($newTag); $post->tags->clear(); $post->tags->add($newTag) ``` new tag is scheduled for orphan removal, and not inserted

admin commented

2026-01-22 15:08:20 +01:00

@lcobucci commented on GitHub (Mar 16, 2017):

As @Ocramius said, replacing the collection is not supported (and it would never work since the information regarding orphan removals is on PersistentCollection class). However Collection#clear() + Collection#add() does the job, I just ran this test on master and everything works:

$post = new Post();
$post->tags->add(new Tag($post, 'a'));
$post->tags->add(new Tag($post, 'b'));

$this->_em->persist($post);
$this->_em->flush();
$this->_em->clear();

$tags = $this->_em->getRepository(Tag::class)->findAll();

self::assertCount(2, $tags);
self::assertEquals('a', $tags[0]->name);
self::assertEquals('b', $tags[1]->name);

$post = $this->_em->find(Post::class, 1);
$post->tags->clear();
$post->tags->add(new Tag($post, 'c'));

$this->_em->persist($post);
$this->_em->flush();
$this->_em->clear();

$tags = $this->_em->getRepository(Tag::class)->findAll();

self::assertCount(1, $tags);
self::assertEquals('c', $tags[0]->name);

@lcobucci commented on GitHub (Mar 16, 2017): As @Ocramius said, replacing the collection is not supported (and it would never work since the [information regarding orphan removals is on `PersistentCollection` class](https://github.com/doctrine/doctrine2/blob/1aa02f9afcb374d7d3060f960fa8ce096bf71369/lib/Doctrine/ORM/PersistentCollection.php#L354)). However `Collection#clear()` + `Collection#add()` does the job, I just ran this test on `master` and everything works: ```php $post = new Post(); $post->tags->add(new Tag($post, 'a')); $post->tags->add(new Tag($post, 'b')); $this->_em->persist($post); $this->_em->flush(); $this->_em->clear(); $tags = $this->_em->getRepository(Tag::class)->findAll(); self::assertCount(2, $tags); self::assertEquals('a', $tags[0]->name); self::assertEquals('b', $tags[1]->name); $post = $this->_em->find(Post::class, 1); $post->tags->clear(); $post->tags->add(new Tag($post, 'c')); $this->_em->persist($post); $this->_em->flush(); $this->_em->clear(); $tags = $this->_em->getRepository(Tag::class)->findAll(); self::assertCount(1, $tags); self::assertEquals('c', $tags[0]->name); ```

admin commented

2026-01-22 15:08:21 +01:00

@lcobucci commented on GitHub (Mar 16, 2017):

I do agree that replacing the whole collection is something nice to have but I'd say that it should be only supported/considered on v3 after the refactor that @guilhermeblanco is doing

@lcobucci commented on GitHub (Mar 16, 2017): I do agree that replacing the whole collection is something nice to have but I'd say that it should be only supported/considered on v3 after the refactor that @guilhermeblanco is doing

admin commented

2026-01-22 15:08:22 +01:00

@benbor commented on GitHub (Mar 16, 2017):

@lcobucci thanks for the answer.

Your test isn't full for my case. If you will add few already exists Tags after $post->tags->clear() they will be lost. Bug reproduced only for last stable branch(at now 2.5), for master branch it behavior already fixed and everything works well.

Would be great to fix it case for stable branch too.

Gist with my test. I specified the test environment in comments.

@benbor commented on GitHub (Mar 16, 2017): @lcobucci thanks for the answer. Your test isn't full for my case. If you will add few already exists Tags after `$post->tags->clear()` they will be lost. Bug reproduced only for last stable branch(at now 2.5), for `master` branch it behavior already fixed and everything works well. Would be great to fix it case for stable branch too. [Gist with my test](https://gist.github.com/benbor/37df518fbf6120c64a1aa07249175734). I specified the test environment in comments.

admin commented

2026-01-22 15:08:22 +01:00

@guilhermeblanco commented on GitHub (Mar 16, 2017):

@benbor Not an easy fix to be backported, as it introduces a BC break...
The commit that fixes this is here: 1587aac4ff

This should be part of 2.6 but I cannot classify this as a "bug" of 2.5, since it introduces a new behavior to OneToMany.

@guilhermeblanco commented on GitHub (Mar 16, 2017): @benbor Not an easy fix to be backported, as it introduces a BC break... The commit that fixes this is here: https://github.com/doctrine/doctrine2/commit/1587aac4ff6b0753ddd5f8b8d4558b6b40096057 This should be part of 2.6 but I cannot classify this as a "bug" of 2.5, since it introduces a new behavior to OneToMany.

admin commented

2026-01-22 15:08:22 +01:00

@benbor commented on GitHub (Mar 17, 2017):

@guilhermeblanco from my point of view old tag losing after clear() is pure "bug"

        $post = $this->_em->find(Post::class, 1);
        $tagA = $this->_em->find(Tag::class, 1);
        $post->tags->clear();
        $post->tags->add($tagA);
        $post->tags->add(new Tag($post, 'c'));
        $this->_em->persist($post);
        $this->_em->flush();
        $this->_em->clear();

        $tags = $this->_em->getRepository(Tag::class)->findAll();
        self::assertCount(2, $tags);
        self::assertEquals('a', $tags[0]->name);
        self::assertEquals('c', $tags[1]->name);

Why this case doesn't work for version 2.5 ? imho its obvious bug.

@benbor commented on GitHub (Mar 17, 2017): @guilhermeblanco from my point of view old tag losing after clear() is pure "bug" ```php $post = $this->_em->find(Post::class, 1); $tagA = $this->_em->find(Tag::class, 1); $post->tags->clear(); $post->tags->add($tagA); $post->tags->add(new Tag($post, 'c')); $this->_em->persist($post); $this->_em->flush(); $this->_em->clear(); $tags = $this->_em->getRepository(Tag::class)->findAll(); self::assertCount(2, $tags); self::assertEquals('a', $tags[0]->name); self::assertEquals('c', $tags[1]->name); ``` Why this case doesn't work for version 2.5 ? imho its obvious bug.

admin commented

2026-01-22 15:08:23 +01:00

@wadjeroudi commented on GitHub (Jan 12, 2018):

"I do agree that replacing the whole collection is something nice to have but I'd say that it should be only supported/considered on v3 after the refactor that @guilhermeblanco is doing"
@lcobucci Is it still planned for v3 ?

The issue should be closed as 2.6 has been released ?

@wadjeroudi commented on GitHub (Jan 12, 2018): "I do agree that replacing the whole collection is something nice to have but I'd say that it should be only supported/considered on v3 after the refactor that @guilhermeblanco is doing" @lcobucci Is it still planned for v3 ? The issue should be closed as 2.6 has been released ?

admin commented

2026-01-22 15:08:24 +01:00

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

If someone wants to implement that, they'd need to pick up the work themselves. Starting from a test would already be great.

The core team already has enough WIP areas for now.

@Ocramius commented on GitHub (Jan 12, 2018): If someone wants to implement that, they'd need to pick up the work themselves. Starting from a test would already be great. The core team already has enough WIP areas for now.

admin commented

2026-01-22 15:08:24 +01:00

@wadjeroudi commented on GitHub (Jan 12, 2018):

@Ocramius in my opinion the orphanRemoval is a weird implementation to do a simple task : remove element.
The "natural" and "logical" way to remove an entity from a collection should be just to call remove/removeElement from this collection.
Without setting the parent entity to null.

Is there a possible implementation of this behaviour and would it be accepted ? If yes, I may try to implement it.

@wadjeroudi commented on GitHub (Jan 12, 2018): @Ocramius in my opinion the orphanRemoval is a weird implementation to do a simple task : remove element. The "natural" and "logical" way to remove an entity from a collection should be just to call remove/removeElement from this collection. Without setting the parent entity to null. Is there a possible implementation of this behaviour and would it be accepted ? If yes, I may try to implement it.

admin commented

2026-01-22 15:08:24 +01:00

@NavyCoat commented on GitHub (Jan 12, 2018):

@wadjeroudi I'm not the "oldest" developer and in my first days/months :), Doctrine looks like big and strange piece of code that do a lot of magic. And it has strange thing. For example this removing elements behaviour. For me as user of this package, I expected it will remove element :P

@NavyCoat commented on GitHub (Jan 12, 2018): @wadjeroudi I'm not the "oldest" developer and in my first days/months :), Doctrine looks like big and strange piece of code that do a lot of magic. And it has strange thing. For example this removing elements behaviour. For me as user of this package, I expected it will remove element :P

admin commented

2026-01-22 15:08:25 +01:00

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

As already mentioned, write a test case first. Discussion should start from the test expectations.

@Ocramius commented on GitHub (Jan 12, 2018): As already mentioned, write a test case first. Discussion should start from the test expectations.

admin commented

2026-01-22 15:08:26 +01:00

@mattsah commented on GitHub (Apr 11, 2019):

I'm not sure if this is related to this. But we had a long working piece of software that would replace collections with new collections outright. The new collection might have a mix of the same entities as the old, some removed, or even some new ones added. Or, if nothing changed, the new collection might have exactly the same entities.

At some point (still trying to determine what version, currently on 2.6.3) this stopped working. We were receiving an exception because doctrine was trying to remove the related record and it was still being referenced by other entities. We were not changing any of the associated entities, they were simply being added to a new collection, and the collection was being replaced outright.

From what I can tell doctrine is seeing the entities as orphaned because of this and it was trying to remove the entities. Thankfully, this was causing an exception due to other relations and foreign key constraints. When I set orphan removal to false, everything worked as expected again (but obviously I wouldn't expected the orphans to be removed).

@mattsah commented on GitHub (Apr 11, 2019): I'm not sure if this is related to this. But we had a long working piece of software that would replace collections with new collections outright. The new collection might have a mix of the same entities as the old, some removed, or even some new ones added. Or, if nothing changed, the new collection might have **exactly the same entities**. At some point (still trying to determine what version, currently on 2.6.3) this stopped working. We were receiving an exception because doctrine was trying to remove the related record and it was still being referenced by other entities. We were not changing any of the associated entities, they were simply being added to a new collection, and the collection was being replaced outright. From what I can tell doctrine is seeing the entities as orphaned because of this and it was trying to remove the entities. Thankfully, this was causing an exception due to other relations and foreign key constraints. When I set orphan removal to false, everything worked as expected again (but obviously I wouldn't expected the orphans to be removed).

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#5463