From 7045bed7850b5f50e86e82e8ccfe55157329dce0 Mon Sep 17 00:00:00 2001 From: Dan Luu Date: Mon, 27 Apr 2026 16:55:25 -0500 Subject: [PATCH] RTC: Ensure a single canonical update log for collaborative edits. Trac ticket: Core-65138 Backport of https://github.com/WordPress/gutenberg/pull/77675 A race condition on opening an editor session allows for Core to create duplicate post meta for sync storage. This creates two copies of the document which the editors operate on independently, leading to a mismatch between the sessions. In this patch, when such a duplicate storage row is detected, a canonical version of the post meta is chosen (the one with the lowest id viz. the oldest one) and the duplicate is merged into it. This should ensure that all edit sessions for a given post use the same synchronized backing store. Co-authored-by: Dennis Snell --- .../class-wp-sync-post-meta-storage.php | 205 +++++++++++++++++- .../collaboration/wpSyncPostMetaStorage.php | 174 ++++++++++++++- 2 files changed, 371 insertions(+), 8 deletions(-) diff --git a/src/wp-includes/collaboration/class-wp-sync-post-meta-storage.php b/src/wp-includes/collaboration/class-wp-sync-post-meta-storage.php index 658a9b65539dd..36081f4d943c9 100644 --- a/src/wp-includes/collaboration/class-wp-sync-post-meta-storage.php +++ b/src/wp-includes/collaboration/class-wp-sync-post-meta-storage.php @@ -86,7 +86,7 @@ public function add_update( string $room, $update ): bool { // Use direct database operation to avoid cache invalidation performed by // post meta functions (`wp_cache_set_posts_last_changed()` and direct // `wp_cache_delete()` calls). - return (bool) $wpdb->insert( + $result = $wpdb->insert( $wpdb->postmeta, array( 'post_id' => $post_id, @@ -95,6 +95,13 @@ public function add_update( string $room, $update ): bool { ), array( '%d', '%s', '%s' ) ); + + if ( $result ) { + $room_hash = md5( $room ); + self::$storage_post_ids[ $room_hash ] = $this->merge_duplicate_storage_posts( $room_hash, $post_id ); + } + + return (bool) $result; } /** @@ -153,7 +160,8 @@ public function get_awareness_state( string $room ): array { public function set_awareness_state( string $room, array $awareness ): bool { global $wpdb; - $post_id = $this->get_storage_post_id( $room ); + $room_hash = md5( $room ); + $post_id = $this->get_storage_post_id( $room ); if ( null === $post_id ) { return false; } @@ -174,16 +182,22 @@ public function set_awareness_state( string $room, array $awareness ): bool { ); if ( $meta_id ) { - return (bool) $wpdb->update( + $result = $wpdb->update( $wpdb->postmeta, array( 'meta_value' => wp_json_encode( $awareness ) ), array( 'meta_id' => $meta_id ), array( '%s' ), array( '%d' ) ); + + if ( false !== $result ) { + self::$storage_post_ids[ $room_hash ] = $this->merge_duplicate_storage_posts( $room_hash, $post_id ); + } + + return false !== $result; } - return (bool) $wpdb->insert( + $result = $wpdb->insert( $wpdb->postmeta, array( 'post_id' => $post_id, @@ -192,6 +206,12 @@ public function set_awareness_state( string $room, array $awareness ): bool { ), array( '%d', '%s', '%s' ) ); + + if ( $result ) { + self::$storage_post_ids[ $room_hash ] = $this->merge_duplicate_storage_posts( $room_hash, $post_id ); + } + + return (bool) $result; } /** @@ -256,14 +276,185 @@ 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 that happens, merge everything back into one canonical lineage. + * + * @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 ) { + wp_delete_post( $inserted_post_id, true ); + return null; + } + + return $this->merge_duplicate_storage_posts( $room_hash, $canonical_post_id ); + } + + /** + * Merges duplicate storage posts created by a first-access race. + * + * @since 7.0.0 + * + * @param string $room_hash MD5 hash of the room identifier. + * @param int $canonical_post_id Preferred post ID that should own the room. + * @return int Canonical storage post ID. + */ + private function merge_duplicate_storage_posts( string $room_hash, int $canonical_post_id ): int { + global $wpdb; + + $storage_post_ids = $this->get_storage_post_ids_for_room_hash( $room_hash ); + if ( empty( $storage_post_ids ) ) { + return $canonical_post_id; + } + + $exact_post_id = $this->find_canonical_storage_post_id( $room_hash ); + if ( null === $exact_post_id ) { + $canonical_post_id = in_array( $canonical_post_id, $storage_post_ids, true ) ? $canonical_post_id : (int) $storage_post_ids[0]; + $promoted_post_id = $this->promote_storage_post_to_canonical_slug( $room_hash, $canonical_post_id ); + if ( null === $promoted_post_id ) { + return $canonical_post_id; + } + + $canonical_post_id = $promoted_post_id; + $storage_post_ids = $this->get_storage_post_ids_for_room_hash( $room_hash ); + } else { + $canonical_post_id = $exact_post_id; + } + + foreach ( $storage_post_ids as $duplicate_id ) { + if ( $canonical_post_id === $duplicate_id ) { + continue; + } + + $move_result = $wpdb->update( + $wpdb->postmeta, + array( 'post_id' => $canonical_post_id ), + array( 'post_id' => $duplicate_id ), + array( '%d' ), + array( '%d' ) + ); + + if ( false === $move_result ) { + continue; + } + + wp_delete_post( $duplicate_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 { + global $wpdb; + + $post_id = $wpdb->get_var( + $wpdb->prepare( + "SELECT ID FROM {$wpdb->posts} WHERE post_type = %s AND post_status = 'publish' AND post_name = %s ORDER BY ID ASC LIMIT 1", + self::POST_TYPE, + $room_hash + ) + ); + + return is_numeric( $post_id ) ? (int) $post_id : null; + } + + /** + * 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; + + $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; + } + + /** + * Lists storage posts belonging to a room hash, including suffixed duplicates. + * + * @since 7.0.0 + * + * @param string $room_hash MD5 hash of the room identifier. + * @return array Storage post IDs. + */ + private function get_storage_post_ids_for_room_hash( string $room_hash ): array { + global $wpdb; + + $post_ids = $wpdb->get_col( + $wpdb->prepare( + "SELECT ID FROM {$wpdb->posts} + WHERE post_type = %s + AND post_status = 'publish' + AND ( post_name = %s OR post_name LIKE %s ) + ORDER BY ID ASC", + self::POST_TYPE, + $room_hash, + $wpdb->esc_like( $room_hash . '-' ) . '%' + ) + ); + + return array_map( 'intval', $post_ids ); + } + /** * Gets the number of updates stored for a given room. * diff --git a/tests/phpunit/tests/collaboration/wpSyncPostMetaStorage.php b/tests/phpunit/tests/collaboration/wpSyncPostMetaStorage.php index 8286fa643b45e..e8ec29fa60977 100644 --- a/tests/phpunit/tests/collaboration/wpSyncPostMetaStorage.php +++ b/tests/phpunit/tests/collaboration/wpSyncPostMetaStorage.php @@ -32,7 +32,13 @@ 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 ); @@ -40,6 +46,30 @@ public function set_up() { $reflection->setValue( null, array() ); } + /** + * Gets active storage posts that could be used as room storage lineages. + * + * @param string $room Room identifier. + * @return array 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. * @@ -704,4 +734,146 @@ public function __set( $name, $value ) { 'Concurrent update should survive compaction.' ); } + + 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.' + ); + } + + 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.' + ); + } }