Sync: Fix awareness state JSON serialization format#75919
Sync: Fix awareness state JSON serialization format#75919
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
pkevan
added
[Feature] Real-time Collaboration
| // Skip entries with unexpected structure. | ||
| if ( ! is_array( $entry ) || ! isset( $entry['client_id'], $entry['state'], $entry['updated_at'] ) ) { | ||
| continue; |
There was a problem hiding this comment.
Why not handle this with REST type validation?
There was a problem hiding this comment.
didn't think of this - probably makes more sense than here.
|
I'm curious, under what conditions does this surface? The request should always include the user's own awareness state and guarantee at least one member. |
I came across this while testing something unrelated, but since it's a REST API request, any input can be sent (with the appropriate permissions) outside of its use within the editor. I guess we could also restrict it only within the editor, but this felt simpler. |
pkevan
force-pushed
the
pkevan/fix-awareness-format
branch
from
2bfd9cb to
3ee7671
Compare
Cast awareness state to (object) at the REST input handling layer in
handle_request, ensuring empty JSON objects {} are not lost to PHP's
json_decode array conversion. Update process_awareness_update to accept
?object instead of ?array.
Also cast state values to (object) in the response map for backward
compatibility with existing stored entries, and cast the outer awareness
map to (object) to handle the empty-map case.
Add defensive validation of existing entries from storage to skip any
with missing required keys.
Move awareness entry validation (client_id, state, updated_at) into the REST route schema definition alongside existing properties. Use rest_validate_value_from_schema to validate stored entries, and add a sanitize_callback to cast awareness to object for correct JSON serialization of empty states.
pkevan
force-pushed
the
pkevan/fix-awareness-format
branch
from
4006b99 to
1013abe
Compare
|
Flaky tests detected in ad9826e. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/22583593097
|
| 'type' => 'integer', | ||
| ), | ||
| ), | ||
| 'additionalProperties' => false, |
There was a problem hiding this comment.
Is it ok to omit this and give ourselves some additional flexibility?
|
|
||
| foreach ( $existing_awareness as $entry ) { | ||
| // Skip entries that don't match the expected schema. | ||
| if ( is_wp_error( rest_validate_value_from_schema( $entry, $this->awareness_entry_schema ) ) ) { |
There was a problem hiding this comment.
Is this necessary now that we have validation on input? If it is, do we need to add it other places?
| $response = array(); | ||
| foreach ( $updated_awareness as $entry ) { | ||
| $response[ $entry['client_id'] ] = $entry['state']; | ||
| $response[ $entry['client_id'] ] = (object) $entry['state']; |
|
@pkevan Is this PR still relevant? I haven't heard any bug reports around this. |
Depends on your perspective 😁 0 bugs were ever reported on this. |
|
Ok, can you close it or move it forward? It will need a separate Core backport (can't piggyback on 10894 since that's closed). |
|
Closing for now - can revisit if it come up again. |
2 participants
What?
Ensures awareness state values are cast to
stdClassobjects before storage and in response maps, preventing JSON serialization issues.Why?
When clients send empty awareness objects
{}, PHP'sjson_decode($body, true)converts them to empty arrays[]. Without casting, these empty arrays serialize back as[]instead of{}, breaking the TypeScript type contract (LocalAwarenessState = object | null). This fix ensures proper round-tripping through PHP for both empty and non-empty awareness states.How?
Cast awareness state values to
(object)at three points: when storing new entries, when building the response map, and when assigning the awareness map to the response. Also added defensive validation to skip entries from storage with missing required keys.Testing
Collaborative editing with multiple users should properly sync awareness state (cursor positions, presence) without type errors. The fix handles both empty and populated awareness states correctly during JSON serialization/deserialization.