Skip to content

RTC: Ensure a single canonical update log for collaborative edits. #11660

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dmsnell wants to merge 4 commits into
base: trunk
Choose a base branch
from
+300 −4
Open

RTC: Ensure a single canonical update log for collaborative edits. #11660

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
49f278e
RTC: Ensure a single canonical update log for collaborative edits.
Apr 27, 2026
9c9c36d
Merge remote-tracking branch 'upstream/trunk' into rtc/avoid-duplicat…
dmsnell May 6, 2026
37e8a55
Incorporate fix from wordpress/gutenberg#78053
dmsnell May 7, 2026
a3354ae
Update to match latest Gutenberg trunk
dmsnell May 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
124 changes: 121 additions & 3 deletions src/wp-includes/collaboration/class-wp-sync-post-meta-storage.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,14 +256,132 @@ private function get_storage_post_id( string $room ): ?int {
)
);

if ( is_int( $post_id ) ) {
self::$storage_post_ids[ $room_hash ] = $post_id;
return $post_id;
if ( is_int( $post_id ) && $post_id > 0 ) {
$canonical_post_id = $this->resolve_canonical_storage_post_id_after_insert( $room_hash, $post_id );
if ( null === $canonical_post_id ) {
return null;
}

self::$storage_post_ids[ $room_hash ] = $canonical_post_id;
return $canonical_post_id;
}

return null;
}

/**
* Resolves the canonical room storage post after inserting a new post.
*
* Two concurrent first writers can both miss the lookup above and create
* storage posts for the same room hash. Depending on the exact interleaving,
* WordPress may create either a duplicate exact slug or a suffixed slug.
* When this request receives a non-canonical post, redirect it to the
* canonical storage before any sync or awareness data is written.
*
* @since 7.0.0
*
* @param string $room_hash MD5 hash of the room identifier.
* @param int $inserted_post_id Post ID returned by wp_insert_post().
* @return int|null Canonical storage post ID.
*/
private function resolve_canonical_storage_post_id_after_insert( string $room_hash, int $inserted_post_id ): ?int {
$canonical_post_id = $this->find_canonical_storage_post_id( $room_hash );
if ( null === $canonical_post_id ) {
$canonical_post_id = $this->promote_storage_post_to_canonical_slug( $room_hash, $inserted_post_id );
}

if ( null === $canonical_post_id ) {
Copy link
Copy Markdown

@apermo apermo Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would still consider checking for intval() instead for null, but in these cases the case is not as strong as above.

Copy link
Copy Markdown
Member Author

@dmsnell dmsnell May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah. hm. the functions returning the $canonical_post_id are both typed to return int|null (unlike $wpdb->insert() etc…)

there should be no reasonable way to get to this point if it’s neither null nor int (strict-mode aside), so I think in this case it’s more or less equivalent, no?

the intval() emphasizes what it expects and the null-check emphasizes the fact that a post id might not exist.

wp_delete_post( $inserted_post_id, true );
return null;
}

if ( $inserted_post_id !== $canonical_post_id ) {
/*
* This request just created a duplicate empty storage post because
* another first writer won the exact-slug race. Delete only that
* just-created empty post and write this request's data to canonical
* storage.
*
* Do not merge or delete older duplicate storage posts here. A stale
* request may already hold a duplicate post ID, and MySQL advisory
* locks/raw transactions are not a reliable cross-server fence under
* HyperDB or database proxies. Future historical repair should be
* bounded and idempotent, or run out of band with primary-pinned
* verification and a grace period before deleting duplicates.
*/
wp_delete_post( $inserted_post_id, true );
}

return $canonical_post_id;
}

/**
* Finds the canonical storage post for a room hash.
*
* The canonical post is the oldest published storage post with the exact
* room hash slug. Suffixed slugs are repair candidates, not canonical.
*
* @since 7.0.0
*
* @param string $room_hash MD5 hash of the room identifier.
* @return int|null Canonical storage post ID.
*/
private function find_canonical_storage_post_id( string $room_hash ): ?int {
Copy link
Copy Markdown
Contributor

@t-hamano t-hamano May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function appears to always return null. I have submitted additional fixes in Gutenberg.

WordPress/gutenberg#78053

Copy link
Copy Markdown
Member Author

@dmsnell dmsnell May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @t-hamano — this slipped in though my partial application of a review comment. I apologize for missing that.

$posts = get_posts(
array(
'post_type' => self::POST_TYPE,
'posts_per_page' => 1,
'post_status' => 'publish',
'name' => $room_hash,
'fields' => 'ids',
'orderby' => 'ID',
'order' => 'ASC',
)
);

if ( empty( $posts ) ) {
return null;
}

return $posts[0];
}

/**
* Promotes a storage post to the canonical room slug.
*
* @since 7.0.0
*
* @param string $room_hash MD5 hash of the room identifier.
* @param int $post_id Post ID to promote.
* @return int|null Promoted post ID on success.
*/
private function promote_storage_post_to_canonical_slug( string $room_hash, int $post_id ): ?int {
global $wpdb;

/*
* @todo Could this be replaced by {@see wp_update_post()}? Could we experience
* a race with other posts having a different post type or post status?
*/
$result = $wpdb->update(
$wpdb->posts,
array( 'post_name' => $room_hash ),
array(
'ID' => $post_id,
'post_type' => self::POST_TYPE,
'post_status' => 'publish',
),
array( '%s' ),
array( '%d', '%s', '%s' )
);

if ( false === $result ) {
return null;
}

clean_post_cache( $post_id );
return $post_id;
}

/**
* Gets the number of updates stored for a given room.
*
Expand Down
180 changes: 179 additions & 1 deletion tests/phpunit/tests/collaboration/wpSyncPostMetaStorage.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,44 @@ public function set_up() {
parent::set_up();
update_option( 'wp_collaboration_enabled', 1 );

// Reset storage post ID cache to ensure clean state after transaction rollback.
$this->reset_storage_post_id_cache();
}

/**
* Resets the static room-to-storage-post cache.
*/
private function reset_storage_post_id_cache(): void {
$reflection = new ReflectionProperty( 'WP_Sync_Post_Meta_Storage', 'storage_post_ids' );
if ( PHP_VERSION_ID < 80100 ) {
$reflection->setAccessible( true );
}
$reflection->setValue( null, array() );
}

/**
* Gets active storage posts that could be used as room storage lineages.
*
* @param string $room Room identifier.
* @return array<int, object> Matching storage posts.
*/
private function get_storage_post_lineages( string $room ): array {
global $wpdb;

$room_hash = md5( $room );
return $wpdb->get_results(
$wpdb->prepare(
"SELECT ID, post_name FROM {$wpdb->posts}
WHERE post_type = %s
AND post_status = 'publish'
AND ( post_name = %s OR post_name LIKE %s )
ORDER BY ID ASC",
WP_Sync_Post_Meta_Storage::POST_TYPE,
$room_hash,
$wpdb->esc_like( $room_hash . '-' ) . '%'
)
);
}

/**
* Returns the room identifier for the test post.
*
Expand Down Expand Up @@ -704,4 +734,152 @@ public function __set( $name, $value ) {
'Concurrent update should survive compaction.'
);
}

/**
* @ticket 65138
*/
public function test_first_access_race_does_not_split_room_storage() {
$storage = new WP_Sync_Post_Meta_Storage();
$room = $this->get_room() . ':first-access-race';
$room_hash = md5( $room );
$update = array(
'type' => 'update',
'data' => base64_encode( 'first-access-race' ),
);

$did_inject = false;
$injected_post_id = 0;
$injector = static function ( $data ) use ( &$did_inject, &$injected_post_id, $room_hash ) {
if (
$did_inject ||
! is_array( $data ) ||
( $data['post_type'] ?? null ) !== WP_Sync_Post_Meta_Storage::POST_TYPE ||
( $data['post_name'] ?? null ) !== $room_hash
) {
return $data;
}

$did_inject = true;
$injected_post_id = wp_insert_post(
array(
'post_type' => WP_Sync_Post_Meta_Storage::POST_TYPE,
'post_status' => 'publish',
'post_title' => 'Sync Storage',
'post_name' => $room_hash,
)
);

return $data;
};

add_filter( 'wp_insert_post_data', $injector, 10, 1 );
try {
$this->assertTrue( $storage->add_update( $room, $update ) );
} finally {
remove_filter( 'wp_insert_post_data', $injector, 10 );
}

$this->assertTrue( $did_inject, 'Expected first-access race injection to occur.' );
$this->assertIsInt( $injected_post_id, 'Expected injected storage post to be created.' );
$this->assertGreaterThan( 0, $injected_post_id, 'Expected injected storage post to be created.' );

$lineages = $this->get_storage_post_lineages( $room );
$this->assertCount( 1, $lineages, 'First-access race split room storage.' );
$this->assertSame(
$room_hash,
$lineages[0]->post_name,
'The surviving storage lineage must use the exact room hash slug.'
);

$this->reset_storage_post_id_cache();
$fresh_storage = new WP_Sync_Post_Meta_Storage();
$updates = $fresh_storage->get_updates_after_cursor( $room, 0 );

$this->assertContains(
$update['data'],
wp_list_pluck( $updates, 'data' ),
'An acknowledged first-access update must be visible to a fresh storage instance.'
);
}

/**
* @ticket 65138
*/
public function test_duplicate_storage_merge_preserves_exact_room_slug() {
global $wpdb;

$storage = new WP_Sync_Post_Meta_Storage();
$room = $this->get_room() . ':suffix-id-before-exact';
$room_hash = md5( $room );

$suffixed_post_id = wp_insert_post(
array(
'post_type' => WP_Sync_Post_Meta_Storage::POST_TYPE,
'post_status' => 'publish',
'post_title' => 'Sync Storage',
'post_name' => $room_hash . '-2',
)
);
$this->assertIsInt( $suffixed_post_id );
$this->assertGreaterThan( 0, $suffixed_post_id );
$this->assertSame( $room_hash . '-2', get_post_field( 'post_name', $suffixed_post_id ) );

$suffixed_update = array(
'type' => 'update',
'data' => base64_encode( 'suffixed-lineage-update' ),
);
$this->assertNotFalse(
$wpdb->insert(
$wpdb->postmeta,
array(
'post_id' => $suffixed_post_id,
'meta_key' => WP_Sync_Post_Meta_Storage::SYNC_UPDATE_META_KEY,
'meta_value' => wp_json_encode( $suffixed_update ),
),
array( '%d', '%s', '%s' )
)
);

$exact_post_id = wp_insert_post(
array(
'post_type' => WP_Sync_Post_Meta_Storage::POST_TYPE,
'post_status' => 'publish',
'post_title' => 'Sync Storage',
'post_name' => $room_hash,
)
);
$this->assertIsInt( $exact_post_id );
$this->assertGreaterThan( $suffixed_post_id, $exact_post_id );
$this->assertSame( $room_hash, get_post_field( 'post_name', $exact_post_id ) );

$exact_update = array(
'type' => 'update',
'data' => base64_encode( 'exact-lineage-update' ),
);
$this->assertTrue( $storage->add_update( $room, $exact_update ) );

$lineages = $this->get_storage_post_lineages( $room );
$this->assertCount( 1, $lineages, 'Duplicate storage lineages should be merged.' );
$this->assertSame(
$room_hash,
$lineages[0]->post_name,
'Merging must keep the exact room hash slug even when a suffixed duplicate has the lower post ID.'
);

$this->reset_storage_post_id_cache();
$fresh_storage = new WP_Sync_Post_Meta_Storage();
$updates = $fresh_storage->get_updates_after_cursor( $room, 0 );
$update_data = wp_list_pluck( $updates, 'data' );

$this->assertContains(
$suffixed_update['data'],
$update_data,
'Merged storage must preserve updates from the suffixed lineage.'
);
$this->assertContains(
$exact_update['data'],
$update_data,
'Merged storage must preserve updates from the exact lineage.'
);
}
}
Loading