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

[mislavjakopovic]

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)

[xificurk]

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

[greg0ire]

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