Prevent creation of new MANAGED entity instance by reloading REMOVED entity from database

[xificurk]

Fixes #6123, #9463 Replaces: #6126

Current behavior: EntityManager::remove() called with MANAGED entity schedules the entity for deletion at a later time, but immediately removes it from identity map.

The immediate removal from identity map is problematic. Until the changes are flushed, the entity may get loaded again from database - either directly as part of a query result, or indirectly through a reference from another entity. This leads to the creation of a new entity (proxy) instance that is then added to identity map, i.e. the UnitOfWork will have both - MANAGED entity instance in identity map, and REMOVED entity scheduled for deletion.

This leads to all sorts of unexpected behaviors, e.g.:

• unexpected results of === entity comparisons
• inconsistent persist behavior depending on which of the instances is passed (including indirect through cascade: persist)
• MANAGED entity instance remaining in identity map even after flushing the changes to database (executing the delete of the mapped row)

Proposed fix: Postpone the removal from identity map to the time when the scheduled deletes are executed.

This change caused failure of UnitOfWorkTest::testRemovedAndRePersistedEntitiesAreInTheIdentityMapAndAreNotGarbageCollected(), which directly asserted the problematic immediate removal from identity map. From the original PR #1338, it seems this is rather an unintended by-product than actually desired behavior. The test was introduced to ensure that REMOVED entity can become MANAGED again (and unscheduled from deletion) by re-persisting it. This functionality does not require the immediate removal from identity map. In fact, the immediate removal from identity map may break it as demonstrated in GH6123Test::testRemovedEntityCanBePersistedAgain().

~Although the PR targets 3.1.x branch, it would be definitely a good candidate for backporting.~ (changed to target 2.19.x)

[greg0ire]

Although the PR targets 3.1.x branch, it would be definitely a good candidate for backporting.

According to https://github.com/doctrine/orm/issues/11208, you should target 2.x rightaway

[xificurk]

@greg0ire I've overlooked that, thanks. PR is now rebased and targets 2.19.x.

[greg0ire]

Thanks @xificurk !