You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Following scenario produces a breaking change after upgrading to doctrine/orm: "2.19.5":
Create a Parent and Child entity which are linked via OneToMany relation. Parent relation is configured to cascade: ['persist', 'remove']
Delete a Child entity via $entityManager->remove($child)
Retrieve a Parent entity which is linked to a deleted Child via f.e. $parentRepository->find()
Create a new Child entity and link it to retrieved Parent entity
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): #11428
Example
// Remove all entries with odd IDsforeach ($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 = newPage();
$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)
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 #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: mislavjakopovic/doctrine-orm-issue-11448#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: mislavjakopovic/doctrine-orm-issue-11448#2
BC Break Report
Summary
Following scenario produces a breaking change after upgrading to
doctrine/orm: "2.19.5"
:OneToMany
relation. Parent relation is configured tocascade: ['persist', 'remove']
$entityManager->remove($child)
$parentRepository->find()
$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
): #11428Example
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 aboveremove()
to have effect we need to either:flush()
call right afterremove()
persist($newPage)
, notpersist(book)
persist(book)
The text was updated successfully, but these errors were encountered: