Real-time collaboration: Add new REST endpoints, setting, and registered post meta#10894
Real-time collaboration: Add new REST endpoints, setting, and registered post meta#10894chriszarate wants to merge 29 commits intoWordPress:trunkfrom
Conversation
|
Hi @chriszarate! 👋 Thank you for your contribution to WordPress! 💖 It looks like this is your first pull request to No one monitors this repository for new pull requests. Pull requests must be attached to a Trac ticket to be considered for inclusion in WordPress Core. To attach a pull request to a Trac ticket, please include the ticket's full URL in your pull request description. Pull requests are never merged on GitHub. The WordPress codebase continues to be managed through the SVN repository that this GitHub repository mirrors. Please feel free to open pull requests to work on any contribution you are making. More information about how GitHub pull requests can be used to contribute to WordPress can be found in the Core Handbook. Please include automated tests. Including tests in your pull request is one way to help your patch be considered faster. To learn about WordPress' test suites, visit the Automated Testing page in the handbook. If you have not had a chance, please review the Contribute with Code page in the WordPress Core Handbook. The Developer Hub also documents the various coding standards that are followed:
Thank you, |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
|
Is there any tests we can add for the default provider and new functionality? |
|
Can we also provide some documentation on how to override the provider given it's known limitation listed above? |
TimothyBJacobs
left a comment
There was a problem hiding this comment.
Why isn't each room a post?
src/wp-includes/collaboration/class-wp-sync-post-meta-storage.php
Outdated
Show resolved
Hide resolved
src/wp-includes/collaboration/class-wp-http-polling-sync-server.php
Outdated
Show resolved
Hide resolved
src/wp-includes/collaboration/class-wp-http-polling-sync-server.php
Outdated
Show resolved
Hide resolved
src/wp-includes/collaboration/class-wp-http-polling-sync-server.php
Outdated
Show resolved
Hide resolved
| * @param bool $is_compactor True if this client is nominated to perform compaction. | ||
| * @return array Response data for this room. | ||
| */ | ||
| private function get_updates_after( string $room, int $client_id, int $cursor, bool $is_compactor ): array { |
There was a problem hiding this comment.
This seems like it should be a responsibility of the storage layer. If I want to swap with something that can retrieve results ordered by time, we shouldn't have to load all updates always to do that.
There was a problem hiding this comment.
I've updated the storage interface and class to take this approach in 9e5cf96. It was a little trickier than I thought, because compaction is an external concern and shouldn't be implemented by storage.
src/wp-includes/collaboration/class-wp-http-polling-sync-server.php
Outdated
Show resolved
Hide resolved
No strong reason. Mostly to avoid creating a large number of posts. Each room corresponds to a synced CRDT document (representing a WordPress entity). Currently supported entity types:
In the future, this could be expanded to include all database entities, including individual taxonomy terms. We might also create rooms to share awareness data at the page or screen level. We intend to enforce limits on this default provider, but probably applied to simultaneous collaborators and not on the overall number of synced entities. Sharing a single post felt prudent but happy to reconsider! I know |
src/wp-includes/collaboration/class-wp-http-polling-sync-server.php
Outdated
Show resolved
Hide resolved
src/wp-includes/collaboration/class-wp-http-polling-sync-server.php
Outdated
Show resolved
Hide resolved
src/wp-includes/collaboration/class-wp-sync-post-meta-storage.php
Outdated
Show resolved
Hide resolved
src/wp-includes/collaboration/class-wp-sync-post-meta-storage.php
Outdated
Show resolved
Hide resolved
src/wp-includes/collaboration/class-wp-sync-post-meta-storage.php
Outdated
Show resolved
Hide resolved
src/wp-includes/collaboration/class-wp-sync-post-meta-storage.php
Outdated
Show resolved
Hide resolved
src/wp-includes/collaboration/class-wp-sync-post-meta-storage.php
Outdated
Show resolved
Hide resolved
src/wp-includes/collaboration/class-wp-sync-post-meta-storage.php
Outdated
Show resolved
Hide resolved
src/wp-includes/collaboration/class-wp-sync-post-meta-storage.php
Outdated
Show resolved
Hide resolved
9e5cf96 to
2db6675
Compare
src/wp-includes/collaboration/class-wp-http-polling-sync-server.php
Outdated
Show resolved
Hide resolved
src/wp-includes/collaboration/class-wp-http-polling-sync-server.php
Outdated
Show resolved
Hide resolved
src/wp-includes/collaboration/class-wp-sync-post-meta-storage.php
Outdated
Show resolved
Hide resolved
|
Fix one of the failing unit tests here: chriszarate#1 since I couldn't PR against this. |
fb2d847 to
92152e8
Compare
…ted values The test_get_items test in WP_Test_REST_Settings_Controller hardcodes the expected list of settings exposed via the REST API. The new enable_real_time_collaboration setting was registered with show_in_rest but not added to this expected list, causing the test to fail.
92152e8 to
f83467c
Compare
|
Note that I've removed the commit that updates Gutenberg to the required version to allow unit tests to pass. The testing instructions have been updated accordingly. |
|
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 Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @gclapps0612-cmd. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
This comment was marked as spam.
This comment was marked as spam.
|
|
||
| $entity_kind = $type_parts[0]; | ||
| $entity_name = $object_parts[0]; | ||
| $object_id = isset( $object_parts[1] ) ? (int) $object_parts[1] : null; |
There was a problem hiding this comment.
Since the object ID is a string, right?
| $object_id = isset( $object_parts[1] ) ? (int) $object_parts[1] : null; | |
| $object_id = $object_parts[1] ?? null; |
| if ( is_wp_error( $merged_awareness ) ) { | ||
| return $merged_awareness; | ||
| } |
There was a problem hiding this comment.
The process_awareness_update method never returns a WP_Error, yeah?
| if ( is_wp_error( $merged_awareness ) ) { | |
| return $merged_awareness; | |
| } |
| * All updates are stored persistently. | ||
| */ | ||
| return $this->add_update( $room, $client_id, $type, $data ); | ||
| break; |
| * @param int $client_id Client identifier. | ||
| * @param string $type Update type (sync_step1, sync_step2, update, compaction). | ||
| * @param string $data Base64-encoded update data. | ||
| * @return bool|WP_Error True on success, WP_Error on storage failure. |
There was a problem hiding this comment.
| * @return bool|WP_Error True on success, WP_Error on storage failure. | |
| * @return true|WP_Error True on success, WP_Error on storage failure. |
| * @param int $cursor Return updates after this cursor. | ||
| * @param bool $is_compactor True if this client is nominated to perform compaction. | ||
| * @return array{ | ||
| * compaction_request: array|null, |
There was a problem hiding this comment.
This satisfies the following PHPStan issue:
phpstan: Method WP_HTTP_Polling_Sync_Server::get_updates() return type has no value type specified in iterable type array.
| * compaction_request: array|null, | |
| * compaction_request: mixed[]|null, |
There was a problem hiding this comment.
Actually, this is slightly better:
| * compaction_request: array|null, | |
| * compaction_request: array<int, mixed>|null, |
It is more explicit, and it satisfies PhpStorm's static analysis complaint as well:
Can be replaced with 'array'
| * | ||
| * @param string $room Room identifier. | ||
| * @param int $cursor Return updates after this cursor. | ||
| * @return array Sync updates. |
There was a problem hiding this comment.
This satisfies a PHPStan issue:
phpstan: Method WP_Sync_Storage::get_updates_after_cursor() return type has no value type specified in iterable type array.
| * @return array Sync updates. | |
| * @return mixed[] Sync updates. |
There was a problem hiding this comment.
| * @return array Sync updates. | |
| * @return array<int, mixed> Sync updates. |
| * | ||
| * @param string $room Room identifier. | ||
| * @param int $cursor Return updates after this cursor. | ||
| * @return array Sync updates. |
There was a problem hiding this comment.
This fixes a PHPStan complaint:
phpstan: Method WP_Sync_Post_Meta_Storage::get_updates_after_cursor() return type has no value type specified in iterable type array.
| * @return array Sync updates. | |
| * @return mixed[] Sync updates. |
There was a problem hiding this comment.
| * @return array Sync updates. | |
| * @return array<int, mixed> Sync updates. |
| 'wp_notes_notify' => 1, | ||
|
|
||
| // 7.0.0 | ||
| 'enable_real_time_collaboration' => 0, |
There was a problem hiding this comment.
| 'enable_real_time_collaboration' => 0, | |
| 'enable_real_time_collaboration' => 1, |
switch on for beta1
| 'type' => 'boolean', | ||
| 'description' => __( 'Enable Real-Time Collaboration' ), | ||
| 'sanitize_callback' => 'rest_sanitize_boolean', | ||
| 'default' => false, |
There was a problem hiding this comment.
| 'default' => false, | |
| 'default' => true, |
| "use_smilies": true, | ||
| "default_category": 1, | ||
| "default_post_format": "0", | ||
| "enable_real_time_collaboration": false, |
There was a problem hiding this comment.
| "enable_real_time_collaboration": false, | |
| "enable_real_time_collaboration": true, |
Description
Trac ticket: https://core.trac.wordpress.org/ticket/64622
In Gutenberg, we have added support for real-time collaboration using CRDT documents (via the Yjs library). This work has suggested the following additions to WordPress:
A default "sync provider" based on HTTP polling that allows collaborators to share updates with each other. Previously, we relied on WebRTC connections between collaborators for this purpose, but it proved unreliable under many network conditions.
A new registered post meta that allows Gutenberg to persist CRDT documents alongside posts.
A new Writing setting that allows users to opt-in to real-time collaboration.
A behavior change to autosaves is needed. When the the original author is editing a draft post (post_status == 'draft' OR 'auto-draft') and they hold the post lock, the autosave targets the actual post instead of an autosave revision. This puts the post data and the persisted CRDT document out of sync and leads to duplicate updates. When real-time collaboration is enabled, all collaborators must autosave in the same way.
This PR provides a proposed implementation of the changes above. This corresponding Gutenberg PR moves the work from the
experimentaldirectory tolib/compat:WordPress/gutenberg#75366
Cumulative work to add this functionality can be found using this label:
https://github.com/WordPress/gutenberg/issues?q=label%3A%22%5BFeature%5D%20Real-time%20Collaboration%22%20is%3Apr
Testing instructions
package.jsonto131239e64048aa0f107512129e8b6771c54ba855