From b76450cfa9345ef9b5d615811827bc5515e73f24 Mon Sep 17 00:00:00 2001 From: chriszarate Date: Fri, 20 Mar 2026 11:44:51 -0600 Subject: [PATCH 1/4] Use prepared queries instead of `*_post_meta` functions --- .../class-wp-sync-post-meta-storage.php | 85 +++++++++++++++++-- 1 file changed, 80 insertions(+), 5 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 ae8a54cc81d94..c1f5253ee1e4d 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 @@ -74,14 +74,34 @@ class WP_Sync_Post_Meta_Storage implements WP_Sync_Storage { * @return bool True on success, false on failure. */ public function add_update( string $room, $update ): bool { + global $wpdb; + $post_id = $this->get_storage_post_id( $room ); if ( null === $post_id ) { return false; } - $meta_id = add_post_meta( $post_id, self::SYNC_UPDATE_META_KEY, $update, false ); + // 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). + // + // This operation corresponds to this function call (single=false): + // add_post_meta( $post_id, self::SYNC_UPDATE_META_KEY, $update, false ); + $meta_value = wp_unslash( $update ); + $meta_value = sanitize_meta( self::SYNC_UPDATE_META_KEY, $meta_value, 'post', self::POST_TYPE ); + $meta_value = maybe_serialize( $meta_value ); + + $result = $wpdb->insert( + $wpdb->postmeta, + array( + 'post_id' => $post_id, + 'meta_key' => self::SYNC_UPDATE_META_KEY, + 'meta_value' => $meta_value, + ), + array( '%d', '%s', '%s' ) + ); - return (bool) $meta_id; + return (bool) $result; } /** @@ -93,12 +113,27 @@ public function add_update( string $room, $update ): bool { * @return array Awareness state. */ public function get_awareness_state( string $room ): array { + global $wpdb; + $post_id = $this->get_storage_post_id( $room ); if ( null === $post_id ) { return array(); } - $awareness = get_post_meta( $post_id, self::AWARENESS_META_KEY, true ); + // Use direct database operation to avoid updating the post meta cache. + $meta_value = $wpdb->get_var( + $wpdb->prepare( + "SELECT meta_value FROM $wpdb->postmeta WHERE post_id = %d AND meta_key = %s", + $post_id, + self::AWARENESS_META_KEY + ) + ); + + if ( null === $meta_value ) { + return array(); + } + + $awareness = maybe_unserialize( $meta_value ); if ( ! is_array( $awareness ) ) { return array(); @@ -117,13 +152,53 @@ public function get_awareness_state( string $room ): array { * @return bool True on success, false on failure. */ public function set_awareness_state( string $room, array $awareness ): bool { + global $wpdb; + $post_id = $this->get_storage_post_id( $room ); if ( null === $post_id ) { return false; } - // update_post_meta returns false if the value is the same as the existing value. - update_post_meta( $post_id, wp_slash( self::AWARENESS_META_KEY ), wp_slash( $awareness ) ); + // 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). + // + // This operation corresponds to this function call: + // update_post_meta( $post_id, self::AWARENESS_META_KEY, $awareness ); + $meta_value = wp_unslash( $awareness ); + $meta_value = sanitize_meta( self::AWARENESS_META_KEY, $meta_value, 'post', self::POST_TYPE ); + $meta_value = maybe_serialize( $meta_value ); + + $meta_id = $wpdb->get_var( + $wpdb->prepare( + "SELECT meta_id FROM $wpdb->postmeta WHERE post_id = %d AND meta_key = %s", + $post_id, + self::AWARENESS_META_KEY + ) + ); + + if ( $meta_id ) { + $wpdb->update( + $wpdb->postmeta, + array( 'meta_value' => $meta_value ), + array( 'meta_id' => $meta_id ), + array( '%s' ), + array( '%d' ) + ); + } else { + $wpdb->insert( + $wpdb->postmeta, + array( + 'post_id' => $post_id, + 'meta_key' => self::AWARENESS_META_KEY, + 'meta_value' => $meta_value, + ), + array( '%d', '%s', '%s' ) + ); + } + + // Return true even if the database operation fails, since this operation is + // not critical for collaboration state. return true; } From bfb5f12236aa0e1bbef40316f2083e0ed255f98b Mon Sep 17 00:00:00 2001 From: chriszarate Date: Fri, 20 Mar 2026 14:24:15 -0600 Subject: [PATCH 2/4] Add unit tests to assert post meta cache is preserved --- .../wpSyncPostMetaStorageCache.php | 296 ++++++++++++++++++ 1 file changed, 296 insertions(+) create mode 100644 tests/phpunit/tests/collaboration/wpSyncPostMetaStorageCache.php diff --git a/tests/phpunit/tests/collaboration/wpSyncPostMetaStorageCache.php b/tests/phpunit/tests/collaboration/wpSyncPostMetaStorageCache.php new file mode 100644 index 0000000000000..ac65ddbf8539a --- /dev/null +++ b/tests/phpunit/tests/collaboration/wpSyncPostMetaStorageCache.php @@ -0,0 +1,296 @@ +user->create( array( 'role' => 'editor' ) ); + self::$post_id = $factory->post->create( array( 'post_author' => self::$editor_id ) ); + update_option( 'wp_collaboration_enabled', 1 ); + } + + public static function wpTearDownAfterClass() { + self::delete_user( self::$editor_id ); + delete_option( 'wp_collaboration_enabled' ); + wp_delete_post( self::$post_id, true ); + } + + 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. + $reflection = new ReflectionProperty( 'WP_Sync_Post_Meta_Storage', 'storage_post_ids' ); + if ( PHP_VERSION_ID < 80100 ) { + $reflection->setAccessible( true ); + } + $reflection->setValue( null, array() ); + } + + /** + * Returns the room identifier for the test post. + * + * @return string Room identifier. + */ + private function get_room(): string { + return 'postType/post:' . self::$post_id; + } + + /** + * Creates the storage post for the room and returns its ID. + * + * Adds a seed update to trigger storage post creation, then looks up + * the resulting post ID. + * + * @param WP_Sync_Post_Meta_Storage $storage Storage instance. + * @param string $room Room identifier. + * @return int Storage post ID. + */ + private function create_storage_post( WP_Sync_Post_Meta_Storage $storage, string $room ): int { + $storage->add_update( + $room, + array( + 'type' => 'update', + 'data' => 'seed', + ) + ); + + $posts = get_posts( + array( + 'post_type' => 'wp_sync_storage', + 'posts_per_page' => 1, + 'post_status' => 'publish', + 'name' => md5( $room ), + 'fields' => 'ids', + ) + ); + + $storage_post_id = array_first( $posts ); + $this->assertIsInt( $storage_post_id ); + + return $storage_post_id; + } + + /** + * Primes the post meta object cache for a given post and returns the cached value. + * + * @param int $post_id Post ID. + * @return array Cached meta data. + */ + private function prime_and_get_meta_cache( int $post_id ): array { + update_meta_cache( 'post', array( $post_id ) ); + + $cached = wp_cache_get( $post_id, 'post_meta' ); + $this->assertNotFalse( $cached, 'Post meta cache should be primed.' ); + + return $cached; + } + + /* + * Write operations must not invalidate the post meta object cache. + */ + + public function test_add_update_does_not_invalidate_post_meta_cache() { + $storage = new WP_Sync_Post_Meta_Storage(); + $room = $this->get_room(); + $storage_post_id = $this->create_storage_post( $storage, $room ); + $cached_before = $this->prime_and_get_meta_cache( $storage_post_id ); + + $storage->add_update( + $room, + array( + 'type' => 'update', + 'data' => 'new', + ) + ); + + $cached_after = wp_cache_get( $storage_post_id, 'post_meta' ); + $this->assertSame( + $cached_before, + $cached_after, + 'add_update() must not invalidate the post meta cache.' + ); + } + + public function test_set_awareness_state_insert_does_not_invalidate_post_meta_cache() { + $storage = new WP_Sync_Post_Meta_Storage(); + $room = $this->get_room(); + $storage_post_id = $this->create_storage_post( $storage, $room ); + $cached_before = $this->prime_and_get_meta_cache( $storage_post_id ); + + // First call triggers an INSERT (no existing awareness row). + $storage->set_awareness_state( $room, array( 1 => array( 'name' => 'Test' ) ) ); + + $cached_after = wp_cache_get( $storage_post_id, 'post_meta' ); + $this->assertSame( + $cached_before, + $cached_after, + 'set_awareness_state() INSERT path must not invalidate the post meta cache.' + ); + } + + public function test_set_awareness_state_update_does_not_invalidate_post_meta_cache() { + $storage = new WP_Sync_Post_Meta_Storage(); + $room = $this->get_room(); + $storage_post_id = $this->create_storage_post( $storage, $room ); + + // Create initial awareness row (INSERT path). + $storage->set_awareness_state( $room, array( 1 => array( 'name' => 'Initial' ) ) ); + + // Prime cache after the insert. + $cached_before = $this->prime_and_get_meta_cache( $storage_post_id ); + + // Second call triggers an UPDATE (existing awareness row). + $storage->set_awareness_state( $room, array( 1 => array( 'name' => 'Updated' ) ) ); + + $cached_after = wp_cache_get( $storage_post_id, 'post_meta' ); + $this->assertSame( + $cached_before, + $cached_after, + 'set_awareness_state() UPDATE path must not invalidate the post meta cache.' + ); + } + + public function test_remove_updates_before_cursor_does_not_invalidate_post_meta_cache() { + $storage = new WP_Sync_Post_Meta_Storage(); + $room = $this->get_room(); + $storage_post_id = $this->create_storage_post( $storage, $room ); + + // Get a cursor after the seed update. + $storage->get_updates_after_cursor( $room, 0 ); + $cursor = $storage->get_cursor( $room ); + + $cached_before = $this->prime_and_get_meta_cache( $storage_post_id ); + + $storage->remove_updates_before_cursor( $room, $cursor ); + + $cached_after = wp_cache_get( $storage_post_id, 'post_meta' ); + $this->assertSame( + $cached_before, + $cached_after, + 'remove_updates_before_cursor() must not invalidate the post meta cache.' + ); + } + + /* + * Write operations must not update the 'posts' last_changed cache marker. + */ + + public function test_add_update_does_not_update_posts_last_changed() { + $storage = new WP_Sync_Post_Meta_Storage(); + $room = $this->get_room(); + $this->create_storage_post( $storage, $room ); + + $last_changed_before = wp_cache_get_last_changed( 'posts' ); + + $storage->add_update( + $room, + array( + 'type' => 'update', + 'data' => 'new', + ) + ); + + $this->assertSame( + $last_changed_before, + wp_cache_get_last_changed( 'posts' ), + 'add_update() must not update posts last_changed.' + ); + } + + public function test_set_awareness_state_does_not_update_posts_last_changed() { + $storage = new WP_Sync_Post_Meta_Storage(); + $room = $this->get_room(); + $this->create_storage_post( $storage, $room ); + + $last_changed_before = wp_cache_get_last_changed( 'posts' ); + + $storage->set_awareness_state( $room, array( 1 => array( 'name' => 'Test' ) ) ); + + $this->assertSame( + $last_changed_before, + wp_cache_get_last_changed( 'posts' ), + 'set_awareness_state() must not update posts last_changed.' + ); + } + + public function test_remove_updates_before_cursor_does_not_update_posts_last_changed() { + $storage = new WP_Sync_Post_Meta_Storage(); + $room = $this->get_room(); + $this->create_storage_post( $storage, $room ); + + $storage->get_updates_after_cursor( $room, 0 ); + $cursor = $storage->get_cursor( $room ); + + $last_changed_before = wp_cache_get_last_changed( 'posts' ); + + $storage->remove_updates_before_cursor( $room, $cursor ); + + $this->assertSame( + $last_changed_before, + wp_cache_get_last_changed( 'posts' ), + 'remove_updates_before_cursor() must not update posts last_changed.' + ); + } + + /* + * Read operations must not prime the post meta object cache. + */ + + public function test_get_awareness_state_does_not_prime_post_meta_cache() { + $storage = new WP_Sync_Post_Meta_Storage(); + $room = $this->get_room(); + $storage_post_id = $this->create_storage_post( $storage, $room ); + + // Populate awareness so there is data to read. + $storage->set_awareness_state( $room, array( 1 => array( 'name' => 'Test' ) ) ); + + // Clear any existing cache. + wp_cache_delete( $storage_post_id, 'post_meta' ); + $this->assertFalse( + wp_cache_get( $storage_post_id, 'post_meta' ), + 'Post meta cache should be empty before read.' + ); + + $storage->get_awareness_state( $room ); + + $this->assertFalse( + wp_cache_get( $storage_post_id, 'post_meta' ), + 'get_awareness_state() must not prime the post meta cache.' + ); + } + + public function test_get_updates_after_cursor_does_not_prime_post_meta_cache() { + $storage = new WP_Sync_Post_Meta_Storage(); + $room = $this->get_room(); + $storage_post_id = $this->create_storage_post( $storage, $room ); + + // Clear any existing cache. + wp_cache_delete( $storage_post_id, 'post_meta' ); + $this->assertFalse( + wp_cache_get( $storage_post_id, 'post_meta' ), + 'Post meta cache should be empty before read.' + ); + + $storage->get_updates_after_cursor( $room, 0 ); + + $this->assertFalse( + wp_cache_get( $storage_post_id, 'post_meta' ), + 'get_updates_after_cursor() must not prime the post meta cache.' + ); + } +} From a0f1fdd0b923e3c214c8ffdc6525f5b930f008ef Mon Sep 17 00:00:00 2001 From: chriszarate Date: Sat, 21 Mar 2026 10:08:37 -0600 Subject: [PATCH 3/4] Remove unnecessary unslashing and use JSON encoding --- .../class-wp-sync-post-meta-storage.php | 64 +++++++------------ .../wpSyncPostMetaStorageCache.php | 44 +++++++++++++ .../tests/rest-api/rest-sync-server.php | 15 +++-- 3 files changed, 77 insertions(+), 46 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 c1f5253ee1e4d..96068f43cf0aa 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 @@ -30,7 +30,7 @@ class WP_Sync_Post_Meta_Storage implements WP_Sync_Storage { * @since 7.0.0 * @var string */ - const AWARENESS_META_KEY = 'wp_sync_awareness'; + const AWARENESS_META_KEY = 'wp_sync_awareness_state'; /** * Meta key for sync updates. @@ -38,7 +38,7 @@ class WP_Sync_Post_Meta_Storage implements WP_Sync_Storage { * @since 7.0.0 * @var string */ - const SYNC_UPDATE_META_KEY = 'wp_sync_update'; + const SYNC_UPDATE_META_KEY = 'wp_sync_update_data'; /** * Cache of cursors by room. @@ -84,24 +84,15 @@ 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). - // - // This operation corresponds to this function call (single=false): - // add_post_meta( $post_id, self::SYNC_UPDATE_META_KEY, $update, false ); - $meta_value = wp_unslash( $update ); - $meta_value = sanitize_meta( self::SYNC_UPDATE_META_KEY, $meta_value, 'post', self::POST_TYPE ); - $meta_value = maybe_serialize( $meta_value ); - - $result = $wpdb->insert( + return (bool) $wpdb->insert( $wpdb->postmeta, array( 'post_id' => $post_id, 'meta_key' => self::SYNC_UPDATE_META_KEY, - 'meta_value' => $meta_value, + 'meta_value' => wp_json_encode( $update ), ), array( '%d', '%s', '%s' ) ); - - return (bool) $result; } /** @@ -133,7 +124,7 @@ public function get_awareness_state( string $room ): array { return array(); } - $awareness = maybe_unserialize( $meta_value ); + $awareness = json_decode( $meta_value, true ); if ( ! is_array( $awareness ) ) { return array(); @@ -159,16 +150,7 @@ public function set_awareness_state( string $room, array $awareness ): bool { return false; } - // 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). - // - // This operation corresponds to this function call: - // update_post_meta( $post_id, self::AWARENESS_META_KEY, $awareness ); - $meta_value = wp_unslash( $awareness ); - $meta_value = sanitize_meta( self::AWARENESS_META_KEY, $meta_value, 'post', self::POST_TYPE ); - $meta_value = maybe_serialize( $meta_value ); - + // Use direct database operation to avoid updating the post meta cache. $meta_id = $wpdb->get_var( $wpdb->prepare( "SELECT meta_id FROM $wpdb->postmeta WHERE post_id = %d AND meta_key = %s", @@ -177,29 +159,28 @@ public function set_awareness_state( string $room, array $awareness ): 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). if ( $meta_id ) { - $wpdb->update( + return (bool) $wpdb->update( $wpdb->postmeta, - array( 'meta_value' => $meta_value ), + array( 'meta_value' => wp_json_encode( $awareness ) ), array( 'meta_id' => $meta_id ), array( '%s' ), array( '%d' ) ); - } else { - $wpdb->insert( - $wpdb->postmeta, - array( - 'post_id' => $post_id, - 'meta_key' => self::AWARENESS_META_KEY, - 'meta_value' => $meta_value, - ), - array( '%d', '%s', '%s' ) - ); } - // Return true even if the database operation fails, since this operation is - // not critical for collaboration state. - return true; + return (bool) $wpdb->insert( + $wpdb->postmeta, + array( + 'post_id' => $post_id, + 'meta_key' => self::AWARENESS_META_KEY, + 'meta_value' => wp_json_encode( $awareness ), + ), + array( '%d', '%s', '%s' ) + ); } /** @@ -336,7 +317,10 @@ public function get_updates_after_cursor( string $room, int $cursor ): array { $updates = array(); foreach ( $rows as $row ) { - $updates[] = maybe_unserialize( $row->meta_value ); + $decoded = json_decode( $row->meta_value, true ); + if ( null !== $decoded ) { + $updates[] = $decoded; + } } return $updates; diff --git a/tests/phpunit/tests/collaboration/wpSyncPostMetaStorageCache.php b/tests/phpunit/tests/collaboration/wpSyncPostMetaStorageCache.php index ac65ddbf8539a..32f1f0023b79c 100644 --- a/tests/phpunit/tests/collaboration/wpSyncPostMetaStorageCache.php +++ b/tests/phpunit/tests/collaboration/wpSyncPostMetaStorageCache.php @@ -274,6 +274,50 @@ public function test_get_awareness_state_does_not_prime_post_meta_cache() { ); } + public function test_get_updates_after_cursor_drops_malformed_json() { + global $wpdb; + + $storage = new WP_Sync_Post_Meta_Storage(); + $room = $this->get_room(); + $storage_post_id = $this->create_storage_post( $storage, $room ); + + // Advance cursor past the seed update from create_storage_post(). + $storage->get_updates_after_cursor( $room, 0 ); + $cursor = $storage->get_cursor( $room ); + + // Insert a valid update. + $valid_update = array( + 'type' => 'update', + 'data' => 'dGVzdA==', + ); + $this->assertTrue( $storage->add_update( $room, $valid_update ) ); + + // Insert a malformed JSON row directly into the database. + $wpdb->insert( + $wpdb->postmeta, + array( + 'post_id' => $storage_post_id, + 'meta_key' => WP_Sync_Post_Meta_Storage::SYNC_UPDATE_META_KEY, + 'meta_value' => '{invalid json', + ), + array( '%d', '%s', '%s' ) + ); + + // Insert another valid update after the malformed one. + $valid_update_2 = array( + 'type' => 'sync_step1', + 'data' => 'c3RlcDE=', + ); + $this->assertTrue( $storage->add_update( $room, $valid_update_2 ) ); + + $updates = $storage->get_updates_after_cursor( $room, $cursor ); + + // The malformed row should be dropped; only the valid updates should appear. + $this->assertCount( 2, $updates ); + $this->assertSame( $valid_update, $updates[0] ); + $this->assertSame( $valid_update_2, $updates[1] ); + } + public function test_get_updates_after_cursor_does_not_prime_post_meta_cache() { $storage = new WP_Sync_Post_Meta_Storage(); $room = $this->get_room(); diff --git a/tests/phpunit/tests/rest-api/rest-sync-server.php b/tests/phpunit/tests/rest-api/rest-sync-server.php index d9a1c47e945fd..b899c77418077 100644 --- a/tests/phpunit/tests/rest-api/rest-sync-server.php +++ b/tests/phpunit/tests/rest-api/rest-sync-server.php @@ -667,11 +667,14 @@ private function inject_update(): void { $this->did_inject = true; - add_post_meta( - $this->storage_post_id, - WP_Sync_Post_Meta_Storage::SYNC_UPDATE_META_KEY, - $this->injected_update, - false + $this->wpdb->insert( + $this->wpdb->postmeta, + array( + 'post_id' => $this->storage_post_id, + 'meta_key' => WP_Sync_Post_Meta_Storage::SYNC_UPDATE_META_KEY, + 'meta_value' => wp_json_encode( $this->injected_update ), + ), + array( '%d', '%s', '%s' ) ); } @@ -934,7 +937,7 @@ public function query( $query ) { array( 'post_id' => $this->storage_post_id, 'meta_key' => WP_Sync_Post_Meta_Storage::SYNC_UPDATE_META_KEY, - 'meta_value' => maybe_serialize( $this->concurrent_update ), + 'meta_value' => wp_json_encode( $this->concurrent_update ), ), array( '%d', '%s', '%s' ) ); From 0e73dcdd434c22f2932beca9116149cae64b58d7 Mon Sep 17 00:00:00 2001 From: chriszarate Date: Sat, 21 Mar 2026 10:50:23 -0600 Subject: [PATCH 4/4] Consolidate post meta storage tests --- ...ageCache.php => wpSyncPostMetaStorage.php} | 288 +++++++++++++++++- .../tests/rest-api/rest-sync-server.php | 277 ----------------- 2 files changed, 274 insertions(+), 291 deletions(-) rename tests/phpunit/tests/collaboration/{wpSyncPostMetaStorageCache.php => wpSyncPostMetaStorage.php} (55%) diff --git a/tests/phpunit/tests/collaboration/wpSyncPostMetaStorageCache.php b/tests/phpunit/tests/collaboration/wpSyncPostMetaStorage.php similarity index 55% rename from tests/phpunit/tests/collaboration/wpSyncPostMetaStorageCache.php rename to tests/phpunit/tests/collaboration/wpSyncPostMetaStorage.php index 32f1f0023b79c..76a63cb391c1a 100644 --- a/tests/phpunit/tests/collaboration/wpSyncPostMetaStorageCache.php +++ b/tests/phpunit/tests/collaboration/wpSyncPostMetaStorage.php @@ -1,10 +1,9 @@ get_room(); + $storage_post_id = $this->create_storage_post( $storage, $room ); + + // Clear any existing cache. + wp_cache_delete( $storage_post_id, 'post_meta' ); + $this->assertFalse( + wp_cache_get( $storage_post_id, 'post_meta' ), + 'Post meta cache should be empty before read.' + ); + + $storage->get_updates_after_cursor( $room, 0 ); + + $this->assertFalse( + wp_cache_get( $storage_post_id, 'post_meta' ), + 'get_updates_after_cursor() must not prime the post meta cache.' + ); + } + + /* + * Data integrity tests. + */ + public function test_get_updates_after_cursor_drops_malformed_json() { global $wpdb; @@ -318,23 +341,260 @@ public function test_get_updates_after_cursor_drops_malformed_json() { $this->assertSame( $valid_update_2, $updates[1] ); } - public function test_get_updates_after_cursor_does_not_prime_post_meta_cache() { + /* + * Race-condition tests. + * + * These use a $wpdb proxy to inject concurrent writes between internal + * query steps, verifying that the cursor-bounded query window prevents + * data loss. + */ + + public function test_cursor_does_not_skip_update_inserted_during_fetch_window() { + global $wpdb; + $storage = new WP_Sync_Post_Meta_Storage(); $room = $this->get_room(); $storage_post_id = $this->create_storage_post( $storage, $room ); - // Clear any existing cache. - wp_cache_delete( $storage_post_id, 'post_meta' ); - $this->assertFalse( - wp_cache_get( $storage_post_id, 'post_meta' ), - 'Post meta cache should be empty before read.' + $seed_update = array( + 'client_id' => 1, + 'type' => 'update', + 'data' => 'c2VlZA==', + ); + + $this->assertTrue( $storage->add_update( $room, $seed_update ) ); + + $initial_updates = $storage->get_updates_after_cursor( $room, 0 ); + $baseline_cursor = $storage->get_cursor( $room ); + + // The seed from create_storage_post() plus the one we just added. + $this->assertGreaterThan( 0, $baseline_cursor ); + + $injected_update = array( + 'client_id' => 9999, + 'type' => 'update', + 'data' => base64_encode( 'injected-during-fetch' ), ); + $original_wpdb = $wpdb; + $proxy_wpdb = new class( $original_wpdb, $storage_post_id, $injected_update ) { + private $wpdb; + private $storage_post_id; + private $injected_update; + public $postmeta; + public $did_inject = false; + + public function __construct( $wpdb, int $storage_post_id, array $injected_update ) { + $this->wpdb = $wpdb; + $this->storage_post_id = $storage_post_id; + $this->injected_update = $injected_update; + $this->postmeta = $wpdb->postmeta; + } + + // phpcs:disable WordPress.DB.PreparedSQL.NotPrepared -- Proxy forwards fully prepared core queries. + public function prepare( ...$args ) { + return $this->wpdb->prepare( ...$args ); + } + + public function get_row( $query = null, $output = OBJECT, $y = 0 ) { + $result = $this->wpdb->get_row( $query, $output, $y ); + + $this->maybe_inject_after_sync_query( $query ); + + return $result; + } + + public function get_var( $query = null, $x = 0, $y = 0 ) { + $result = $this->wpdb->get_var( $query, $x, $y ); + + $this->maybe_inject_after_sync_query( $query ); + + return $result; + } + + public function get_results( $query = null, $output = OBJECT ) { + return $this->wpdb->get_results( $query, $output ); + } + // phpcs:enable WordPress.DB.PreparedSQL.NotPrepared + + public function __call( $name, $arguments ) { + return $this->wpdb->$name( ...$arguments ); + } + + public function __get( $name ) { + return $this->wpdb->$name; + } + + public function __set( $name, $value ) { + $this->wpdb->$name = $value; + } + + private function inject_update(): void { + if ( $this->did_inject ) { + return; + } + + $this->did_inject = true; + + $this->wpdb->insert( + $this->wpdb->postmeta, + array( + 'post_id' => $this->storage_post_id, + 'meta_key' => WP_Sync_Post_Meta_Storage::SYNC_UPDATE_META_KEY, + 'meta_value' => wp_json_encode( $this->injected_update ), + ), + array( '%d', '%s', '%s' ) + ); + } + + private function maybe_inject_after_sync_query( $query ): void { + if ( $this->did_inject || ! is_string( $query ) ) { + return; + } + + $targets_postmeta = false !== strpos( $query, $this->postmeta ); + $targets_post_id = 1 === preg_match( '/\bpost_id\s*=\s*' . (int) $this->storage_post_id . '\b/', $query ); + $targets_meta_key = 1 === preg_match( + "/\bmeta_key\s*=\s*'" . preg_quote( WP_Sync_Post_Meta_Storage::SYNC_UPDATE_META_KEY, '/' ) . "'/", + $query + ); + + if ( $targets_postmeta && $targets_post_id && $targets_meta_key ) { + $this->inject_update(); + } + } + }; + + $wpdb = $proxy_wpdb; + try { + $race_updates = $storage->get_updates_after_cursor( $room, $baseline_cursor ); + $race_cursor = $storage->get_cursor( $room ); + } finally { + $wpdb = $original_wpdb; + } + + $this->assertTrue( $proxy_wpdb->did_inject, 'Expected race-window update injection to occur.' ); + $this->assertEmpty( $race_updates ); + $this->assertSame( $baseline_cursor, $race_cursor ); + + $follow_up_updates = $storage->get_updates_after_cursor( $room, $race_cursor ); + $follow_up_cursor = $storage->get_cursor( $room ); + + $this->assertCount( 1, $follow_up_updates ); + $this->assertSame( $injected_update, $follow_up_updates[0] ); + $this->assertGreaterThan( $race_cursor, $follow_up_cursor ); + } + + public function test_compaction_does_not_delete_update_inserted_during_delete() { + global $wpdb; + + $storage = new WP_Sync_Post_Meta_Storage(); + $room = $this->get_room(); + $storage_post_id = $this->create_storage_post( $storage, $room ); + + // Seed three updates so there's something to compact. + for ( $i = 1; $i <= 3; $i++ ) { + $this->assertTrue( + $storage->add_update( + $room, + array( + 'client_id' => $i, + 'type' => 'update', + 'data' => base64_encode( "seed-$i" ), + ) + ) + ); + } + + // Capture the cursor after all seeds are in place. $storage->get_updates_after_cursor( $room, 0 ); + $compaction_cursor = $storage->get_cursor( $room ); + $this->assertGreaterThan( 0, $compaction_cursor ); - $this->assertFalse( - wp_cache_get( $storage_post_id, 'post_meta' ), - 'get_updates_after_cursor() must not prime the post meta cache.' + $concurrent_update = array( + 'client_id' => 9999, + 'type' => 'update', + 'data' => base64_encode( 'arrived-during-compaction' ), + ); + + $original_wpdb = $wpdb; + $proxy_wpdb = new class( $original_wpdb, $storage_post_id, $concurrent_update ) { + private $wpdb; + private $storage_post_id; + private $concurrent_update; + public $did_inject = false; + + public function __construct( $wpdb, int $storage_post_id, array $concurrent_update ) { + $this->wpdb = $wpdb; + $this->storage_post_id = $storage_post_id; + $this->concurrent_update = $concurrent_update; + } + + // phpcs:disable WordPress.DB.PreparedSQL.NotPrepared -- Proxy forwards fully prepared core queries. + public function prepare( ...$args ) { + return $this->wpdb->prepare( ...$args ); + } + + public function query( $query ) { + $result = $this->wpdb->query( $query ); + + // After the DELETE executes, inject a concurrent update via + // raw SQL through the real $wpdb to avoid metadata cache + // interactions while the proxy is active. + if ( ! $this->did_inject + && is_string( $query ) + && 0 === strpos( $query, "DELETE FROM {$this->wpdb->postmeta}" ) + && false !== strpos( $query, "post_id = {$this->storage_post_id}" ) + ) { + $this->did_inject = true; + $this->wpdb->insert( + $this->wpdb->postmeta, + array( + 'post_id' => $this->storage_post_id, + 'meta_key' => WP_Sync_Post_Meta_Storage::SYNC_UPDATE_META_KEY, + 'meta_value' => wp_json_encode( $this->concurrent_update ), + ), + array( '%d', '%s', '%s' ) + ); + } + + return $result; + } + // phpcs:enable WordPress.DB.PreparedSQL.NotPrepared + + public function __call( $name, $arguments ) { + return $this->wpdb->$name( ...$arguments ); + } + + public function __get( $name ) { + return $this->wpdb->$name; + } + + public function __set( $name, $value ) { + $this->wpdb->$name = $value; + } + }; + + // Run compaction through the proxy so the concurrent update + // is injected immediately after the DELETE executes. + $wpdb = $proxy_wpdb; + try { + $result = $storage->remove_updates_before_cursor( $room, $compaction_cursor ); + } finally { + $wpdb = $original_wpdb; + } + + $this->assertTrue( $result ); + $this->assertTrue( $proxy_wpdb->did_inject, 'Expected concurrent update injection to occur.' ); + + // The concurrent update must survive the compaction delete. + $updates = $storage->get_updates_after_cursor( $room, 0 ); + + $update_data = wp_list_pluck( $updates, 'data' ); + $this->assertContains( + $concurrent_update['data'], + $update_data, + 'Concurrent update should survive compaction.' ); } } diff --git a/tests/phpunit/tests/rest-api/rest-sync-server.php b/tests/phpunit/tests/rest-api/rest-sync-server.php index b899c77418077..7a04226ced8c9 100644 --- a/tests/phpunit/tests/rest-api/rest-sync-server.php +++ b/tests/phpunit/tests/rest-api/rest-sync-server.php @@ -565,157 +565,6 @@ public function test_sync_total_updates_increments() { $this->assertSame( 3, $data['rooms'][0]['total_updates'] ); } - public function test_sync_cursor_does_not_skip_update_inserted_during_fetch_window() { - global $wpdb; - - wp_set_current_user( self::$editor_id ); - - $room = $this->get_post_room(); - $storage = new WP_Sync_Post_Meta_Storage(); - - $seed_update = array( - 'client_id' => 1, - 'type' => 'update', - 'data' => 'c2VlZA==', - ); - - $this->assertTrue( $storage->add_update( $room, $seed_update ) ); - - $initial_updates = $storage->get_updates_after_cursor( $room, 0 ); - $baseline_cursor = $storage->get_cursor( $room ); - - $this->assertCount( 1, $initial_updates ); - $this->assertSame( $seed_update, $initial_updates[0] ); - $this->assertGreaterThan( 0, $baseline_cursor ); - - $storage_posts = get_posts( - array( - 'post_type' => WP_Sync_Post_Meta_Storage::POST_TYPE, - 'posts_per_page' => 1, - 'post_status' => 'publish', - 'name' => md5( $room ), - 'fields' => 'ids', - ) - ); - $storage_post_id = array_first( $storage_posts ); - - $this->assertIsInt( $storage_post_id ); - - $injected_update = array( - 'client_id' => 9999, - 'type' => 'update', - 'data' => base64_encode( 'injected-during-fetch' ), - ); - - $original_wpdb = $wpdb; - $proxy_wpdb = new class( $original_wpdb, $storage_post_id, $injected_update ) { - private $wpdb; - private $storage_post_id; - private $injected_update; - public $postmeta; - public $did_inject = false; - - public function __construct( $wpdb, int $storage_post_id, array $injected_update ) { - $this->wpdb = $wpdb; - $this->storage_post_id = $storage_post_id; - $this->injected_update = $injected_update; - $this->postmeta = $wpdb->postmeta; - } - - // phpcs:disable WordPress.DB.PreparedSQL.NotPrepared -- Proxy forwards fully prepared core queries. - public function prepare( ...$args ) { - return $this->wpdb->prepare( ...$args ); - } - - public function get_row( $query = null, $output = OBJECT, $y = 0 ) { - $result = $this->wpdb->get_row( $query, $output, $y ); - - $this->maybe_inject_after_sync_query( $query ); - - return $result; - } - - public function get_var( $query = null, $x = 0, $y = 0 ) { - $result = $this->wpdb->get_var( $query, $x, $y ); - - $this->maybe_inject_after_sync_query( $query ); - - return $result; - } - - public function get_results( $query = null, $output = OBJECT ) { - return $this->wpdb->get_results( $query, $output ); - } - // phpcs:enable WordPress.DB.PreparedSQL.NotPrepared - - public function __call( $name, $arguments ) { - return $this->wpdb->$name( ...$arguments ); - } - - public function __get( $name ) { - return $this->wpdb->$name; - } - - public function __set( $name, $value ) { - $this->wpdb->$name = $value; - } - - private function inject_update(): void { - if ( $this->did_inject ) { - return; - } - - $this->did_inject = true; - - $this->wpdb->insert( - $this->wpdb->postmeta, - array( - 'post_id' => $this->storage_post_id, - 'meta_key' => WP_Sync_Post_Meta_Storage::SYNC_UPDATE_META_KEY, - 'meta_value' => wp_json_encode( $this->injected_update ), - ), - array( '%d', '%s', '%s' ) - ); - } - - private function maybe_inject_after_sync_query( $query ): void { - if ( $this->did_inject || ! is_string( $query ) ) { - return; - } - - $targets_postmeta = false !== strpos( $query, $this->postmeta ); - $targets_post_id = 1 === preg_match( '/\bpost_id\s*=\s*' . (int) $this->storage_post_id . '\b/', $query ); - $targets_meta_key = 1 === preg_match( - "/\bmeta_key\s*=\s*'" . preg_quote( WP_Sync_Post_Meta_Storage::SYNC_UPDATE_META_KEY, '/' ) . "'/", - $query - ); - - if ( $targets_postmeta && $targets_post_id && $targets_meta_key ) { - $this->inject_update(); - } - } - }; - - $wpdb = $proxy_wpdb; - try { - $race_updates = $storage->get_updates_after_cursor( $room, $baseline_cursor ); - $race_cursor = $storage->get_cursor( $room ); - } finally { - $wpdb = $original_wpdb; - } - - $this->assertTrue( $proxy_wpdb->did_inject, 'Expected race-window update injection to occur.' ); - $this->assertEmpty( $race_updates ); - $this->assertSame( $baseline_cursor, $race_cursor ); - - $follow_up_updates = $storage->get_updates_after_cursor( $room, $race_cursor ); - $follow_up_cursor = $storage->get_cursor( $room ); - - $this->assertCount( 1, $follow_up_updates ); - $this->assertSame( $injected_update, $follow_up_updates[0] ); - $this->assertGreaterThan( $race_cursor, $follow_up_cursor ); - } - /* * Compaction tests. */ @@ -857,132 +706,6 @@ public function test_sync_stale_compaction_succeeds_when_newer_compaction_exists $this->assertNotContains( 'c3RhbGU=', $update_data, 'The stale compaction should not be stored.' ); } - public function test_sync_compaction_does_not_delete_update_inserted_during_delete() { - global $wpdb; - - wp_set_current_user( self::$editor_id ); - - $room = $this->get_post_room(); - $storage = new WP_Sync_Post_Meta_Storage(); - - // Seed three updates so there's something to compact. - for ( $i = 1; $i <= 3; $i++ ) { - $this->assertTrue( - $storage->add_update( - $room, - array( - 'client_id' => $i, - 'type' => 'update', - 'data' => base64_encode( "seed-$i" ), - ) - ) - ); - } - - // Capture the cursor after all seeds are in place. - $storage->get_updates_after_cursor( $room, 0 ); - $compaction_cursor = $storage->get_cursor( $room ); - $this->assertGreaterThan( 0, $compaction_cursor ); - - $storage_posts = get_posts( - array( - 'post_type' => WP_Sync_Post_Meta_Storage::POST_TYPE, - 'posts_per_page' => 1, - 'post_status' => 'publish', - 'name' => md5( $room ), - 'fields' => 'ids', - ) - ); - $storage_post_id = array_first( $storage_posts ); - $this->assertIsInt( $storage_post_id ); - - $concurrent_update = array( - 'client_id' => 9999, - 'type' => 'update', - 'data' => base64_encode( 'arrived-during-compaction' ), - ); - - $original_wpdb = $wpdb; - $proxy_wpdb = new class( $original_wpdb, $storage_post_id, $concurrent_update ) { - private $wpdb; - private $storage_post_id; - private $concurrent_update; - public $did_inject = false; - - public function __construct( $wpdb, int $storage_post_id, array $concurrent_update ) { - $this->wpdb = $wpdb; - $this->storage_post_id = $storage_post_id; - $this->concurrent_update = $concurrent_update; - } - - // phpcs:disable WordPress.DB.PreparedSQL.NotPrepared -- Proxy forwards fully prepared core queries. - public function prepare( ...$args ) { - return $this->wpdb->prepare( ...$args ); - } - - public function query( $query ) { - $result = $this->wpdb->query( $query ); - - // After the DELETE executes, inject a concurrent update via - // raw SQL through the real $wpdb to avoid metadata cache - // interactions while the proxy is active. - if ( ! $this->did_inject - && is_string( $query ) - && 0 === strpos( $query, "DELETE FROM {$this->wpdb->postmeta}" ) - && false !== strpos( $query, "post_id = {$this->storage_post_id}" ) - ) { - $this->did_inject = true; - $this->wpdb->insert( - $this->wpdb->postmeta, - array( - 'post_id' => $this->storage_post_id, - 'meta_key' => WP_Sync_Post_Meta_Storage::SYNC_UPDATE_META_KEY, - 'meta_value' => wp_json_encode( $this->concurrent_update ), - ), - array( '%d', '%s', '%s' ) - ); - } - - return $result; - } - // phpcs:enable WordPress.DB.PreparedSQL.NotPrepared - - public function __call( $name, $arguments ) { - return $this->wpdb->$name( ...$arguments ); - } - - public function __get( $name ) { - return $this->wpdb->$name; - } - - public function __set( $name, $value ) { - $this->wpdb->$name = $value; - } - }; - - // Run compaction through the proxy so the concurrent update - // is injected immediately after the DELETE executes. - $wpdb = $proxy_wpdb; - try { - $result = $storage->remove_updates_before_cursor( $room, $compaction_cursor ); - } finally { - $wpdb = $original_wpdb; - } - - $this->assertTrue( $result ); - $this->assertTrue( $proxy_wpdb->did_inject, 'Expected concurrent update injection to occur.' ); - - // The concurrent update must survive the compaction delete. - $updates = $storage->get_updates_after_cursor( $room, 0 ); - - $update_data = wp_list_pluck( $updates, 'data' ); - $this->assertContains( - $concurrent_update['data'], - $update_data, - 'Concurrent update should survive compaction.' - ); - } - /* * Awareness tests. */