Skip to content

Conversation

nikophil
Copy link
Member

@nikophil nikophil commented Sep 21, 2025

Fixing this was trickier than expected 😅

Since we cannot get the id values of the entity from the clone, we need to collect them at a place where we have access to the real object.

The first guess was to call Configuration::instance()->persistence()->getIdentifierValues($object) in PersistedObjectsTracker::proxifyObjects(). But this was messing with the container, during the kernel.reset phase. The problem is more or less the same than explained in #963 - BTW, I've fixed this issue, and created the test mentioned there. PHPUnit is missing an event before the tear down methods are called, so I didn't have a clean solution with this idea.

Second guess: let's collect the ids when we add the objects to the PersistedObjectsTracker! Problem: this is made in a "post instantiate" hook, so the ids are not generated at this time.

Third guess would be to make it in a "post persist" hook. But the problem here is that we perform an additional flush() when a post persist hook is used, this will have a performance impact.

The last solution I had was to collect the id values after each PersistManager::save(). This solution is not very satisfactory because now, the PersistenceManager knows about the PersistedObjectsTracker, but it is the only solution I found.

fixes #976
fixes #963
fixes #979

@nikophil nikophil force-pushed the fix/dont-use-clone-for-ids branch from e8404ee to 4672c87 Compare September 21, 2025 15:33
@nikophil nikophil marked this pull request as draft September 21, 2025 15:56
@nikophil nikophil force-pushed the fix/dont-use-clone-for-ids branch 5 times, most recently from 980fd5c to 7548c3d Compare September 21, 2025 17:31
@nikophil nikophil requested a review from kbond September 21, 2025 18:41
@nikophil nikophil marked this pull request as ready for review September 21, 2025 19:12
@nikophil nikophil force-pushed the fix/dont-use-clone-for-ids branch 2 times, most recently from 2354b6e to f73ecde Compare September 24, 2025 14:59
@nikophil nikophil force-pushed the fix/dont-use-clone-for-ids branch from f73ecde to ef603f8 Compare September 24, 2025 15:01
@nikophil nikophil merged commit 485746e into zenstruck:2.x Sep 24, 2025
73 checks passed
@nikophil nikophil deleted the fix/dont-use-clone-for-ids branch September 24, 2025 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
1 participant