Since v2.19.5 $em->remove($child) doesn't work if called prior to creating another child and $em->persist($parent) #7369

Closed
opened 2026-01-22 15:50:45 +01:00 by admin · 2 comments
Owner

Originally created by @mislavjakopovic on GitHub (May 10, 2024).

BC Break Report

Q A
BC Break yes
Version 2.19.5

Summary

Following scenario produces a breaking change after upgrading to doctrine/orm: "2.19.5":

  1. Create a Parent and Child entity which are linked via OneToMany relation. Parent relation is configured to cascade: ['persist', 'remove']
  2. Delete a Child entity via $entityManager->remove($child)
  3. Retrieve a Parent entity which is linked to a deleted Child via f.e. $parentRepository->find()
  4. Create a new Child entity and link it to retrieved Parent entity
  5. Persist that Parent entity $entityManager->persist($parent)

Persisting a Parent entity directly instead of Child is not the best approach, but afaik there is nowhere written that
it is forbidden. I believe there are some use cases where this can actually be quite practical, instead of having persist child on 10 places in code, a simple persist of parent works here more elegant. It is not the best pattern, but I believe it is a valid behavior, otherwise we wouldn't be even allowed to call persist on already loaded entity manually.

The issue comes from a change where we would remove entity from identityMap only upon executing deletion queries (UnitOfWork::executeDeletions) rather than straight away (UnitOfWork::scheduleForDelete): https://github.com/doctrine/orm/pull/11428

Example

// Remove all entries with odd IDs
foreach ($this->pageRepository->findAll() as $page) {
    if ($page->getId() % 2) {
        $this->entityManager->remove($page);
    }
}

foreach ($this->bookRepository->findAll() as $book) {
    $book->setName('New Book Name');

    // The loop here is not required since the same bug would occur with just ONE new entity,
    // here we're just creating more of them to showcase a possible real world example
    for ($i = 0; $i < 5; $i++) {
        $newPage = new Page();
        $newPage->setTitle('New Page Name');
        $book->addPage($newPage);
    }
    $this->entityManager->persist($book);
}

$this->entityManager->flush();

Previous Behavior (v2.19.4)

After calling $entityManager->flush() Child entity was deleted and a new Child was created and assigned to Parent.

Current Behavior (v2.19.5)

After calling $entityManager->flush() Child entity is not deleted and a new Child is created and assigned to parent (remove had no effect)

How to recreate

Made a test case repository https://github.com/mislavjakopovic/doctrine-orm-issue-11448/ with example code which can be ran quickly via docker-compose.

Workarounds

In doctrine/orm: "2.19.5" for above remove() to have effect we need to either:

  • place a flush() call right after remove()
  • or persist($newPage), not persist(book)
  • or remove call to persist(book)
Originally created by @mislavjakopovic on GitHub (May 10, 2024). ### BC Break Report | Q | A |------------ | ------ | BC Break | yes | Version | 2.19.5 #### Summary Following scenario produces a breaking change after upgrading to `doctrine/orm: "2.19.5"`: 1. Create a Parent and Child entity which are linked via `OneToMany` relation. Parent relation is configured to `cascade: ['persist', 'remove']` 2. Delete a Child entity via `$entityManager->remove($child)` 3. Retrieve a Parent entity which is linked to a deleted Child via f.e. `$parentRepository->find()` 4. Create a new Child entity and link it to retrieved Parent entity 5. Persist that Parent entity `$entityManager->persist($parent)` Persisting a Parent entity directly instead of Child is not the best approach, but afaik there is nowhere written that it is forbidden. I believe there are some use cases where this can actually be quite practical, instead of having persist child on 10 places in code, a simple persist of parent works here more elegant. It is not the best pattern, but I believe it is a valid behavior, otherwise we wouldn't be even allowed to call persist on already loaded entity manually. The issue comes from a change where we would remove entity from `identityMap` only upon executing deletion queries (`UnitOfWork::executeDeletions`) rather than straight away (`UnitOfWork::scheduleForDelete`): https://github.com/doctrine/orm/pull/11428 #### Example ```php // Remove all entries with odd IDs foreach ($this->pageRepository->findAll() as $page) { if ($page->getId() % 2) { $this->entityManager->remove($page); } } foreach ($this->bookRepository->findAll() as $book) { $book->setName('New Book Name'); // The loop here is not required since the same bug would occur with just ONE new entity, // here we're just creating more of them to showcase a possible real world example for ($i = 0; $i < 5; $i++) { $newPage = new Page(); $newPage->setTitle('New Page Name'); $book->addPage($newPage); } $this->entityManager->persist($book); } $this->entityManager->flush(); ``` #### Previous Behavior (v2.19.4) After calling `$entityManager->flush()` Child entity was deleted and a new Child was created and assigned to Parent. #### Current Behavior (v2.19.5) After calling `$entityManager->flush()` Child entity is not deleted and a new Child is created and assigned to parent (remove had no effect) #### How to recreate Made a test case repository https://github.com/mislavjakopovic/doctrine-orm-issue-11448/ with example code which can be ran quickly via `docker-compose`. #### Workarounds In `doctrine/orm: "2.19.5"` for above `remove()` to have effect we need to either: - place a `flush()` call right after `remove()` - or `persist($newPage)`, not `persist(book)` - or remove call to `persist(book)`
admin closed this issue 2026-01-22 15:50:45 +01:00
Author
Owner

@xificurk commented on GitHub (May 12, 2024):

Thanks for the example repo. The main issue is that the Page entity is removed without updating its existing association to Book entity, i.e. the removed Page is still contained in Book::$pages collection.

Why the Page rows are deleted from database on <=2.19.4 :

When Page entity is removed, Book::$pages collection is not updated, but it is also not initialized. The collection gets initialized only after Book::addPage() call. Because of the original bug (fixed in https://github.com/doctrine/orm/pull/11428) the collection will contain a references to new instances of removed entities, and because they are in managed state, they will be ignored by cascading persist($book) call. The flush results in inconsistent state, where odd-id Page entities are deleted from database, but their instances are kept in identity map and Book::$pages collections. See PR with expanded output that iterates over the contents of Book::$pages: https://github.com/mislavjakopovic/doctrine-orm-issue-11448/pull/3

Furthermore, the entities are deleted from database only thanks to the fact that Book::$pages collections are not initialized at the moment of remove() call. If those collections are initialized before that, they will not get deleted: https://github.com/mislavjakopovic/doctrine-orm-issue-11448/pull/2

The proper fix is to take care of any existing associations before removing the entity: https://github.com/mislavjakopovic/doctrine-orm-issue-11448/pull/1

@xificurk commented on GitHub (May 12, 2024): Thanks for the example repo. The main issue is that the `Page` entity is removed without updating its existing association to `Book` entity, i.e. the removed `Page` is still contained in `Book::$pages` collection. Why the Page rows are deleted from database on <=2.19.4 : When `Page` entity is removed, `Book::$pages` collection is not updated, but it is also not initialized. The collection gets initialized only after [Book::addPage()](https://github.com/mislavjakopovic/doctrine-orm-issue-11448/blob/master/app/src/Command/TestCaseCommand.php#L58) call. Because of the original bug (fixed in https://github.com/doctrine/orm/pull/11428) the collection will contain a references to new instances of removed entities, and because they are in `managed` state, they will be ignored by cascading `persist($book)` call. The flush results in inconsistent state, where odd-id `Page` entities are deleted from database, but their instances are kept in identity map and `Book::$pages` collections. See PR with expanded output that iterates over the contents of `Book::$pages`: https://github.com/mislavjakopovic/doctrine-orm-issue-11448/pull/3 Furthermore, the entities are deleted from database only thanks to the fact that `Book::$pages` collections are not initialized at the moment of `remove()` call. If those collections are initialized before that, they will not get deleted: https://github.com/mislavjakopovic/doctrine-orm-issue-11448/pull/2 The proper fix is to take care of any existing associations before removing the entity: https://github.com/mislavjakopovic/doctrine-orm-issue-11448/pull/1
Author
Owner

@greg0ire commented on GitHub (May 28, 2024):

Closing since developers are responsible for maintaining both sides of the association. I think that's stated somewhere in the docs.

@greg0ire commented on GitHub (May 28, 2024): Closing since developers are responsible for maintaining both sides of the association. I think that's stated somewhere in the docs.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: doctrine/archived-orm#7369