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 45 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
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
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. |
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.
src/wp-includes/collaboration/class-wp-http-polling-sync-server.php
Outdated
Show resolved
Hide resolved
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; | |
| } |
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
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
There was a problem hiding this comment.
I've implemented this in a separate branch that can be merged in this one or separately into trunk:
src/wp-includes/post.php
Outdated
There was a problem hiding this comment.
Could we avoid registering the post type if the enable_real_time_collaboration setting is not enabled?
db3746e to
3d55d27
Compare
d3d6c7b to
20cfe6c
Compare
|
One of my first questions, which was already asked by @TimothyBJacobs, is related to the use of a single Unless I'm misreading it, when In testing with 3 clients, I'm seeing thousands of post meta function calls within minutes. I think some of that could be alleviated by giving each room its own |
That's great feedback, thank you. I'll explore using separate posts in a new PR based on this one. |
|
@mindctrl, I've implemented your suggestion in this PR. cc @TimothyBJacobs. Now merged into this PR |
westonruter
left a comment
There was a problem hiding this comment.
I increased PHPStan level from 8 to 9 and found a couple other nits:
------ ---------------------------------------------------------------------------------------------------------------------------------------------
Line class-wp-sync-post-meta-storage.php
------ ---------------------------------------------------------------------------------------------------------------------------------------------
104 Cannot access offset 'timestamp' on mixed.
🪪 offsetAccess.nonOffsetAccessible
at src/wp-includes/collaboration/class-wp-sync-post-meta-storage.php:104
104 Cannot access offset 'value' on mixed.
🪪 offsetAccess.nonOffsetAccessible
at src/wp-includes/collaboration/class-wp-sync-post-meta-storage.php:104
110 Method WP_Sync_Post_Meta_Storage::get_all_updates() should return array<int, array{timestamp: int, value: mixed}> but returns array<mixed>.
🪪 return.type
at src/wp-includes/collaboration/class-wp-sync-post-meta-storage.php:110
134 Method WP_Sync_Post_Meta_Storage::get_awareness_state() should return array<int, mixed> but returns array<mixed, mixed>.
🪪 return.type
at src/wp-includes/collaboration/class-wp-sync-post-meta-storage.php:134
------ ---------------------------------------------------------------------------------------------------------------------------------------------
[ERROR] Found 4 errors
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