Closed mislavjakopovic closed 1 month ago
@mislavjakopovic Could you please provide a test case to demonstrate the BC break? I think there is some important detail missing from the example snippets you've provided, because removing (or not) entity from identity map should not affect the contents of loaded collection.
@xificurk thank you for your reply. I will check it (might take some time as its quite complex) and try to come up with a test case repo.
After going through code I found that the issue is more complex.
Closing this one in favor of https://github.com/doctrine/orm/issues/11448
@xificurk I'd appreciate if you can take a look.
BC Break Report
Summary
The problem arose couple days ago when we updated our project
doctrine/orm 2.19.4 => 2.19.5
. For the purposes of explaining I will simplify examples here. Let's assume we have a following entity:Now we want to keep only children with even IDs and apply some more logic as for example change their names. Below follows what we do in our service layer.
Previous Behavior
Current Behavior
Workarounds
We've managed to find two possible ways how to fix this problem.
First one being that we access entity manager's
UnitOfWork
and do a direct call towardremoveFromIdentityMap
, but since this function is marked as internal, this doesn't seem like a good solution.Second approach is to just do explicit call on
$entityManager->flush()
Another approach might be to manually re-set the fields of our parent again to array containing only odd children, but with this we're losing already implemented functionality and elegance which comes from having entity manager.
Solution
With keeping the current ORM code, obvious solution would be to refactor our app code to have
flush()
call after everyremove()
. However, with this approach having entity manager and flush seems to start to lose sense.The changes we make via entity manager should be reflected inside an
identityMap
and therefore accessible in entity manager straight away.It is my understanding that flush was always there for committing those in memory changes to database. Not for refreshing current state of our objects. With current approach, think of it like this, would we ever introduce having
flush
call behind everypersist
?Root Cause
https://github.com/doctrine/orm/pull/11428
In this pull request a change was introduced where we would remove entity from
identityMap
only upon executing deletion queries (UnitOfWork::executeDeletions
) rather than straight away (UnitOfWork::scheduleForDelete
)I do agree with points raised there by @xificurk. Every developer using Doctrine has met with rogue
Proxy
object sooner or later due to issues he tried to fix there (and I'd like to really thank him for that), but I believe that solution should be more discussed, looked at closer and potentially reverted.