Collaboration: Harden storage layer, fix duplicate awareness rows, and expand test coverage

Author: josephfusco

src/wp-includes/collaboration.php

@@ -70,7 +70,7 @@ function wp_delete_old_collaboration_data() {
 		 */
 		$wpdb->query(
 			$wpdb->prepare(
-				"DELETE FROM {$wpdb->collaboration} WHERE type != 'awareness' AND date_gmt < %s",
+				"DELETE FROM {$wpdb->collaboration} WHERE date_gmt < %s",
 				gmdate( 'Y-m-d H:i:s', time() - WEEK_IN_SECONDS )
 			)
 		);
@@ -86,16 +86,10 @@ function wp_delete_old_collaboration_data() {
 		return;
 	}
 
-	/*
-	 * Clean up sync rows older than 7 days.
-	 *
-	 * The type != 'awareness' exclusion keeps awareness rows untouched —
-	 * they are cleaned up separately below. Future persistent types
-	 * (e.g. persisted_crdt_doc) may also need exclusion here.
-	 */
+	/* Clean up rows older than 7 days. */
 	$wpdb->query(
 		$wpdb->prepare(
-			"DELETE FROM {$wpdb->collaboration} WHERE type != 'awareness' AND date_gmt < %s",
+			"DELETE FROM {$wpdb->collaboration} WHERE date_gmt < %s",
 			gmdate( 'Y-m-d H:i:s', time() - WEEK_IN_SECONDS )
 		)
 	);

src/wp-includes/collaboration/class-wp-collaboration-table-storage.php

@@ -241,10 +241,7 @@ public function get_updates_after_cursor( string $room, int $cursor ): array {
 	}
 
 	/**
-	 * Removes updates from a room that are older than the given cursor.
-	 *
-	 * Uses a single atomic DELETE query, avoiding the race-prone
-	 * "delete all, re-add some" pattern.
+	 * Removes updates from a room up to and including the given cursor.
 	 *
 	 * @since 7.0.0
 	 *
@@ -254,9 +251,11 @@ public function get_updates_after_cursor( string $room, int $cursor ): array {
 	 * @param int    $cursor Remove updates up to and including this cursor.
 	 * @return bool True on success, false on failure.
 	 */
-	public function remove_updates_before_cursor( string $room, int $cursor ): bool {
+	public function remove_updates_up_to_cursor( string $room, int $cursor ): bool {
 		global $wpdb;
 
+		// Uses a single atomic DELETE query, avoiding the race-prone
+		// "delete all, re-add some" pattern.
 		$result = $wpdb->query(
 			$wpdb->prepare(
 				"DELETE FROM {$wpdb->collaboration} WHERE room = %s AND type != 'awareness' AND id <= %d",
@@ -271,10 +270,10 @@ public function remove_updates_before_cursor( string $room, int $cursor ): bool
 	/**
 	 * Sets awareness state for a given client in a room.
 	 *
-	 * Uses UPDATE-then-INSERT: tries to update the existing row first,
-	 * and only inserts if no row was updated. Each client writes only
-	 * its own row, eliminating the race condition inherent in shared-state
-	 * approaches.
+	 * Uses SELECT-then-UPDATE/INSERT: checks for an existing row by
+	 * primary key, then updates or inserts accordingly. Each client
+	 * writes only its own row, eliminating the race condition inherent
+	 * in shared-state approaches.
 	 *
 	 * After writing, the cached awareness entries for the room are updated
 	 * in-place so that subsequent get_awareness_state() calls from other
@@ -299,23 +298,26 @@ public function set_awareness_state( string $room, string $client_id, array $sta
 		$data = wp_json_encode( $state );
 		$now  = gmdate( 'Y-m-d H:i:s' );
 
-		/* Try UPDATE first. */
-		$updated = $wpdb->update(
-			$wpdb->collaboration,
-			array(
-				'user_id'  => $user_id,
-				'data'     => $data,
-				'date_gmt' => $now,
-			),
-			array(
-				'room'      => $room,
-				'type'      => 'awareness',
-				'client_id' => $client_id,
+		/* Check if a row already exists. */
+		$exists = $wpdb->get_var(
+			$wpdb->prepare(
+				"SELECT id FROM {$wpdb->collaboration} WHERE room = %s AND type = 'awareness' AND client_id = %s LIMIT 1",
+				$room,
+				$client_id
 			)
 		);
 
-		/* INSERT only if no existing row. */
-		if ( 0 === (int) $updated ) {
+		if ( $exists ) {
+			$result = $wpdb->update(
+				$wpdb->collaboration,
+				array(
+					'user_id'  => $user_id,
+					'data'     => $data,
+					'date_gmt' => $now,
+				),
+				array( 'id' => $exists )
+			);
+		} else {
 			$result = $wpdb->insert(
 				$wpdb->collaboration,
 				array(
@@ -327,11 +329,9 @@ public function set_awareness_state( string $room, string $client_id, array $sta
 					'date_gmt'  => $now,
 				)
 			);
+		}
 
-			if ( false === $result ) {
-				return false;
-			}
-		} elseif ( false === $updated ) {
+		if ( false === $result ) {
 			return false;
 		}
 

src/wp-includes/collaboration/class-wp-http-polling-collaboration-server.php

@@ -373,8 +373,10 @@ private function can_user_collaborate_on_entity_type( string $entity_kind, strin
 			return current_user_can( 'edit_comment', (int) $object_id );
 		}
 
-		// All the remaining checks are for collections. If an object ID is provided,
-		// reject the request.
+		/*
+		 * All the remaining checks are for collections. If an object ID is
+		 * provided, reject the request.
+		 */
 		if ( null !== $object_id ) {
 			return false;
 		}
@@ -389,9 +391,11 @@ private function can_user_collaborate_on_entity_type( string $entity_kind, strin
 			return current_user_can( $post_type_object->cap->edit_posts );
 		}
 
-		// Collection collaboration does not exchange entity data. It only signals if
-		// another user has updated an entity in the collection. Therefore, we only
-		// compare against an allow list of collection types.
+		/*
+		 * Collection collaboration does not exchange entity data. It only
+		 * signals if another user has updated an entity in the collection.
+		 * Therefore, we only compare against an allow list of collection types.
+		 */
 		$allowed_collection_entity_kinds = array(
 			'postType',
 			'root',
@@ -441,10 +445,12 @@ private function process_awareness_update( string $room, string $client_id, ?arr
 			$response[ $entry['client_id'] ] = $entry['state'];
 		}
 
-		// Other clients' states were decoded from the DB. Run the current
-		// client's state through the same encode/decode path so the response
-		// is consistent — wp_json_encode may normalize values (e.g. strip
-		// invalid UTF-8) that would otherwise differ on the next poll.
+		/*
+		 * Other clients' states were decoded from the DB. Run the current
+		 * client's state through the same encode/decode path so the response
+		 * is consistent — wp_json_encode may normalize values (e.g. strip
+		 * invalid UTF-8) that would otherwise differ on the next poll.
+		 */
 		if ( null !== $awareness_update ) {
 			$response[ $client_id ] = json_decode( wp_json_encode( $awareness_update ), true );
 		}
@@ -488,18 +494,20 @@ private function process_collaboration_update( string $room, string $client_id,
 				}
 
 				if ( ! $has_newer_compaction ) {
-					// Insert the compaction row before deleting old rows.
-					// Reversing the order closes a race window where a
-					// client joining with cursor=0 between the DELETE and
-					// INSERT would see an empty room for one poll cycle.
-					// The compaction row always has a higher ID than the
-					// deleted rows, so cursor-based filtering is unaffected.
+					/*
+					 * Insert the compaction row before deleting old rows.
+					 * Reversing the order closes a race window where a
+					 * client joining with cursor=0 between the DELETE and
+					 * INSERT would see an empty room for one poll cycle.
+					 * The compaction row always has a higher ID than the
+					 * deleted rows, so cursor-based filtering is unaffected.
+					 */
 					$insert_result = $this->add_update( $room, $client_id, $type, $data );
 					if ( is_wp_error( $insert_result ) ) {
 						return $insert_result;
 					}
 
-					if ( ! $this->storage->remove_updates_before_cursor( $room, $cursor ) ) {
+					if ( ! $this->storage->remove_updates_up_to_cursor( $room, $cursor ) ) {
 						global $wpdb;
 						$error_data = array( 'status' => 500 );
 						if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) {
@@ -515,8 +523,10 @@ private function process_collaboration_update( string $room, string $client_id,
 					return true;
 				}
 
-				// Reaching this point means there's a newer compaction, so we can
-				// silently ignore this one.
+				/*
+				 * Reaching this point means there's a newer compaction,
+				 * so we can silently ignore this one.
+				 */
 				return true;
 
 			case self::UPDATE_TYPE_SYNC_STEP1: