Skip to content

Commit 3b11171

Browse files
committed
REST API/RTC: Fix divergence when two offline users reconnect.
Prevent Real-Time Collaboration sessions from becoming out of sync when an offline editor returns to an online state. The change ensures that the updates are merged rather than discarded when each client is at a different cursor after reconnecting or experiencing slow network conditions. Props alecgeatches, peterwilsoncc, maxschmeling, joefusco. See #64622. git-svn-id: https://develop.svn.wordpress.org/trunk@62298 602fd350-edb4-49c9-b593-d223f7449a82
1 parent a129457 commit 3b11171

2 files changed

Lines changed: 36 additions & 11 deletions

File tree

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -498,9 +498,14 @@ private function process_sync_update( string $room, int $client_id, int $cursor,
498498
return $this->add_update( $room, $client_id, $type, $data );
499499
}
500500

501-
// Reaching this point means there's a newer compaction, so we can
502-
// silently ignore this one.
503-
return true;
501+
/*
502+
* A newer compaction already advanced the cursor, but we
503+
* can not safely drop an update. The incoming bytes still encode
504+
* operations other clients may not have seen, so store them as a
505+
* regular update. Y.applyUpdateV2 merges state-as-update blobs
506+
* idempotently, so overlap with the existing compaction is safe.
507+
*/
508+
return $this->add_update( $room, $client_id, self::UPDATE_TYPE_UPDATE, $data );
504509

505510
case self::UPDATE_TYPE_SYNC_STEP1:
506511
case self::UPDATE_TYPE_SYNC_STEP2:

tests/phpunit/tests/rest-api/rest-sync-server.php

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -936,7 +936,7 @@ public function test_sync_should_compact_is_false_for_non_compactor() {
936936
$this->assertFalse( $data['rooms'][0]['should_compact'] );
937937
}
938938

939-
public function test_sync_stale_compaction_succeeds_when_newer_compaction_exists() {
939+
public function test_sync_stale_compaction_is_stored_as_update_when_newer_compaction_exists() {
940940
wp_set_current_user( self::$editor_id );
941941

942942
$room = $this->get_post_room();
@@ -966,9 +966,14 @@ public function test_sync_stale_compaction_succeeds_when_newer_compaction_exists
966966
)
967967
);
968968

969-
// Client 3 sends a stale compaction at cursor 0. The server should find
970-
// client 2's compaction in the updates after cursor 0 and silently discard
971-
// this one.
969+
/*
970+
* Client 3 sends a stale compaction at cursor 0 (mirroring two offline
971+
* clients that reconnect from the same baseline cursor). The server
972+
* cannot run remove_updates_before_cursor because client 2 has already
973+
* advanced the frontier, but the bytes must still be stored as a
974+
* regular update so client 3's operations can propagate to other
975+
* clients via Yjs state-as-update merging.
976+
*/
972977
$stale_compaction = array(
973978
'type' => 'compaction',
974979
'data' => 'c3RhbGU=',
@@ -981,16 +986,31 @@ public function test_sync_stale_compaction_succeeds_when_newer_compaction_exists
981986

982987
$this->assertSame( 200, $response->get_status() );
983988

984-
// Verify the newer compaction is preserved and the stale one was not stored.
985-
$response = $this->dispatch_sync(
989+
/*
990+
* Verify the newer compaction is preserved AND the stale compaction's
991+
* bytes were persisted (now as type=update so subsequent compactions
992+
* don't trip the has_newer_compaction check).
993+
*/
994+
$response = $this->dispatch_sync(
986995
array(
987996
$this->build_room( $room, 4, 0, array( 'user' => 'c4' ) ),
988997
)
989998
);
990-
$update_data = wp_list_pluck( $response->get_data()['rooms'][0]['updates'], 'data' );
999+
$updates = $response->get_data()['rooms'][0]['updates'];
9911000

1001+
$update_data = wp_list_pluck( $updates, 'data' );
9921002
$this->assertContains( 'Y29tcGFjdGVk', $update_data, 'The newer compaction should be preserved.' );
993-
$this->assertNotContains( 'c3RhbGU=', $update_data, 'The stale compaction should not be stored.' );
1003+
$this->assertContains( 'c3RhbGU=', $update_data, 'The stale compaction bytes should be stored so client 3\'s operations propagate.' );
1004+
1005+
$stale_entry = null;
1006+
foreach ( $updates as $entry ) {
1007+
if ( 'c3RhbGU=' === $entry['data'] ) {
1008+
$stale_entry = $entry;
1009+
break;
1010+
}
1011+
}
1012+
$this->assertNotNull( $stale_entry, 'The stale compaction entry should be present in the room.' );
1013+
$this->assertSame( 'update', $stale_entry['type'], 'The stale compaction should be stored as type=update, not type=compaction.' );
9941014
}
9951015

9961016
/*

0 commit comments

Comments
 (0)