mirror of
https://github.com/doctrine/orm.git
synced 2026-03-23 22:42:18 +01:00
No preUpdate event if removing elements from PersistentCollection results in empty collection #7019
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @s-bernstein on GitHub (Aug 4, 2022).
Summary
Removing elements from a PersistentCollection will trigger a preUpdate event, except if the resulting collection becomes empty.
Current behavior
Updating an entity through changing a PersistentCollection triggers a preUpdate event, if the resulting collection is not empty.
Expected behavior
Updating an entity through changing a PersistentCollection should always result in a preUpdate event being triggert, even if the resulting collection becomes empty.
How to reproduce
Have an entity with a many-to-many association represented by a PersistentCollection. Remove all elements from said collection and flush.
Possible problem
During debugging I realized, that removing all elements from a PersistentCollection doesn't set the dirty flag, and therefore the UnitOfWork::computeChangeSet() does not enter the last if-Statement (beginning line 821). Manually setting the dirty flag in the debugger will trigger the preUpdate event.
Some more debugging revealed, that SonataAdmin is calling PersistentCollection::clear() to remove all elements, which will set the dirty flag until the subsequent call of PersistentCollection::takeSnapshot() in PersistenCollection:clear() line 553 resets the dirty flag.
@derrabus commented on GitHub (Aug 7, 2022):
Would you be able to prepare a test case that reproduces your problem? And since you've already debugged the issue, would you be able to work on a fix?
@s-bernstein commented on GitHub (Aug 9, 2022):
@derrabus Regarding your questions: First, I might be able to create a test but haven't looked into it. Also I'm not quite sure if this can be tested with a unit test or if this needs a functional test.
For the second question: Well, I can fix it - I actually have identified two places, where this could be fixed - but I have no understanding of all the implications this fixes will have. As described before, calling PersistentCollection::clear() will reset the dirty flag on the collection by calling PersistentCollection::takeSnapshot(). I suppose, whoever written this part had a good reason for it. I don't know what side effects will arise, if this behaviour is changed - although this seems to me the logical place because resetting the dirty flag looks like a logical error for me.
The other place would be in UnitOfWork::computeChangeSet(), but this involves changing an already long-ish if statement and feels a little forced.
@s-bernstein commented on GitHub (Aug 9, 2022):
OK, I have a PR (#9985) to fix this problem - hopefully...
@s-bernstein commented on GitHub (Aug 18, 2022):
Any chance this one will proceed in the near future?
@s-bernstein commented on GitHub (Sep 27, 2022):
Anything new?
@SherinBloemendaal commented on GitHub (Oct 31, 2022):
Any update? We have the same problem
@mathieu-ducrot commented on GitHub (Apr 19, 2023):
Hi everyone, we also encounter this behavior while implementing a history system for a project which is based on the doctrine preUpdate event, and indeed we actually can't log this behavior (all elements of the PersistentCollection has been removed).
It would really be nice if some core team of doctrine could check what @s-bernstein did to know if fixing this dirty flag for that behavior have some side effects.
@s-bernstein commented on GitHub (Apr 20, 2023):
I'm using my supplied patch since August 2022 with no ill effects identified since.