fix: translate mangled keys before hydrating from a snapshot - #1149
fix: translate mangled keys before hydrating from a snapshot#1149kabylixx wants to merge 3 commits into
Conversation
`hydrateFromSnapshot()` fed an `(array) $object` cast straight to the hydrators, but the cast mangles non-public property names while the hydrators expect the convention built by `VarExporter\Hydrator`: a plain name for anything writable in the object's own scope, `"\0Scope\0name"` only for a private property declared by another class. When every key resolves to the object's own scope, the hydrator takes its un-grouped fast path and uses the mangled name verbatim, failing with `Error: Cannot access property starting with "\0"`.
362cde8 to
8840d2f
Compare
nikophil
left a comment
There was a problem hiding this comment.
thanks for the fix, I'm just wondering if there is not a solution out of the box using deepclone (and its polyfill) directly
| * @test | ||
| */ | ||
| #[Test] | ||
| public function can_hydrate_from_snapshot_with_public_property_only(): void |
There was a problem hiding this comment.
any chance that you also reproduce the problem in a functional test please? in \Zenstruck\Foundry\Tests\Integration\Persistence\AutoRefreshTestCase
Existing fixtures cannot reproduce the failure: they all inherit a public `$id` from a mapped superclass, and any plain or foreign-scope key in the snapshot is enough to take the hydrator's grouping path, where mangled names are resolved. Adds an entity and a document declaring every property themselves, privately, which is the shape that triggers the un-grouped fast path.
|
hey @kabylixx I think the problem is in symfony/polyfill-deepclone I'll propose a fix there |
|
Makes sense — and I agree the polyfill is inconsistent with itself here: its grouping path resolves Two things that might matter for a fix landing there:
Happy to reduce this PR to just the tests if you'd rather fix it upstream — they fail against the current polyfill and would guard against a regression wherever the fix ends up living. Just say the word. |
|
Hi! what I'm planning to do is:
does that seem OK to you? I'd like to prevent monkey patching deepclone in Foundry 😅 |
The root cause is fixed in symfony/polyfill-deepclone, so translating the keys here would only work around it. The tests stay and fail until a polyfill release carries the fix.
|
Sounds good to me — and I'd rather not monkey-patch deepclone from Foundry either. I've dropped |
Fixes #1148.
hydrateFromSnapshot()feeds an(array) $objectcast straight todeepclone_hydrate(), but the two use different key conventions: the cast mangles non-public names ("\0Class\0name"for a private one,"\0*\0name"for a protected one) while the hydrators expect whatVarExporter\Hydratorbuilds — a plain name for anything writable in the object's own scope, and"\0Scope\0name"only for a private property declared by another class.When every key resolves to the object's own scope, the hydrator takes its un-grouped fast path and uses the mangled name verbatim, which fails with
Error: Cannot access property starting with "\0". In practice this hits any autorefreshed object that gets deleted:autorefresh()then takes the "object no longer exists" branch and restores the snapshot.This translates the keys before hydrating. Two details worth noting:
"\0": a property name never contains one, but an anonymous class name does (class@anonymous\0/path/to/file.php:7$0), soexplode("\0", $key, 3)mis-splits those;substr/strrpos, becausemb_str_functionsrewrites them to theirmb_*counterparts, which count characters and would make byte-level mangling encoding-dependent.Tests: 2 of the 4 added cases fail before the patch (own private, own protected — the first one on an anonymous class, so the split above is covered). The other 2 guard the paths that already worked: public-only, and inherited properties including a private shadowed across scopes.