Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -498,9 +498,14 @@ private function process_sync_update( string $room, int $client_id, int $cursor,
return $this->add_update( $room, $client_id, $type, $data );
}

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

case self::UPDATE_TYPE_SYNC_STEP1:
case self::UPDATE_TYPE_SYNC_STEP2:
Expand Down
36 changes: 28 additions & 8 deletions tests/phpunit/tests/rest-api/rest-sync-server.php
Original file line number Diff line number Diff line change
Expand Up @@ -936,7 +936,7 @@ public function test_sync_should_compact_is_false_for_non_compactor() {
$this->assertFalse( $data['rooms'][0]['should_compact'] );
}

public function test_sync_stale_compaction_succeeds_when_newer_compaction_exists() {
public function test_sync_stale_compaction_is_stored_as_update_when_newer_compaction_exists() {
wp_set_current_user( self::$editor_id );

$room = $this->get_post_room();
Expand Down Expand Up @@ -966,9 +966,14 @@ public function test_sync_stale_compaction_succeeds_when_newer_compaction_exists
)
);

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

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

// Verify the newer compaction is preserved and the stale one was not stored.
$response = $this->dispatch_sync(
/*
* Verify the newer compaction is preserved AND the stale compaction's
* bytes were persisted (now as type=update so subsequent compactions
* don't trip the has_newer_compaction check).
*/
$response = $this->dispatch_sync(
array(
$this->build_room( $room, 4, 0, array( 'user' => 'c4' ) ),
)
);
$update_data = wp_list_pluck( $response->get_data()['rooms'][0]['updates'], 'data' );
$updates = $response->get_data()['rooms'][0]['updates'];

$update_data = wp_list_pluck( $updates, 'data' );
$this->assertContains( 'Y29tcGFjdGVk', $update_data, 'The newer compaction should be preserved.' );
$this->assertNotContains( 'c3RhbGU=', $update_data, 'The stale compaction should not be stored.' );
$this->assertContains( 'c3RhbGU=', $update_data, 'The stale compaction bytes should be stored so client 3\'s operations propagate.' );

$stale_entry = null;
foreach ( $updates as $entry ) {
if ( 'c3RhbGU=' === $entry['data'] ) {
$stale_entry = $entry;
break;
}
}
$this->assertNotNull( $stale_entry, 'The stale compaction entry should be present in the room.' );
$this->assertSame( 'update', $stale_entry['type'], 'The stale compaction should be stored as type=update, not type=compaction.' );
}

/*
Expand Down
Loading