Skip to content

Commit 905bd65

Browse files
pentopento
committed
RTC: Fix notes not syncing between collaborative editors
PR #76704 introduced `isPrimaryRoom` to only check the primary room for collaborator awareness, but accidentally restricted `updateQueue.resume()` to only the primary room queue. Non-primary rooms (like the `root/comment` collection room used for notes) had their update queues permanently paused, so CRDT save events were never sent to the server and notes never synced between editors. Fix by resuming all room queues when collaborators are detected on the primary room, preserving the optimisation of only checking the primary room for awareness.
1 parent ccc4743 commit 905bd65

File tree

3 files changed

+487
-4
lines changed

3 files changed

+487
-4
lines changed

‎packages/sync/src/providers/http-polling/polling-manager.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -447,15 +447,19 @@ function poll(): void {
447447
roomState.processAwarenessUpdate( room.awareness );
448448

449449
// If there is another collaborator on the primary entity,
450-
// resume the queue for the next poll and increase polling
451-
// frequency. We only check the primary room to avoid false
452-
// positives from shared collection rooms (e.g. taxonomy/category).
450+
// resume all room queues for the next poll and increase
451+
// polling frequency. We only check the primary room to
452+
// avoid false positives from shared collection rooms
453+
// (e.g. taxonomy/category), but resume all queues so
454+
// collection rooms (e.g. root/comment) can also sync.
453455
if (
454456
roomState.isPrimaryRoom &&
455457
Object.keys( room.awareness ).length > 1
456458
) {
457459
hasCollaborators = true;
458-
roomState.updateQueue.resume();
460+
roomStates.forEach( ( state ) => {
461+
state.updateQueue.resume();
462+
} );
459463
}
460464

461465
// Process each incoming update and collect any responses.

‎packages/sync/src/providers/http-polling/test/polling-manager.test.ts‎

Lines changed: 253 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -552,6 +552,259 @@ describe( 'polling-manager', () => {
552552
} );
553553
} );
554554

555+
describe( 'collaborator queue resumption', () => {
556+
it( 'resumes non-primary room queues when collaborators are detected on primary room', async () => {
557+
// First poll: primary room has collaborators, collection room has none.
558+
mockPostSyncUpdate.mockResolvedValue( {
559+
rooms: [
560+
{
561+
room: 'primary-room',
562+
end_cursor: 1,
563+
awareness: {
564+
1: { collaboratorInfo: { id: 100 } },
565+
2: { collaboratorInfo: { id: 200 } },
566+
},
567+
updates: [],
568+
},
569+
{
570+
room: 'collection-room',
571+
end_cursor: 1,
572+
awareness: {},
573+
updates: [],
574+
},
575+
],
576+
} );
577+
578+
// Register primary room first (becomes isPrimaryRoom).
579+
pollingManager.registerRoom( {
580+
room: 'primary-room',
581+
doc: createMockDoc( 1 ),
582+
awareness: createMockAwareness(),
583+
log: jest.fn(),
584+
onStatusChange: jest.fn(),
585+
onSync: jest.fn(),
586+
} );
587+
588+
pollingManager.registerRoom( {
589+
room: 'collection-room',
590+
doc: createMockDoc( 2 ),
591+
awareness: createMockAwareness(),
592+
log: jest.fn(),
593+
onStatusChange: jest.fn(),
594+
onSync: jest.fn(),
595+
} );
596+
597+
// First poll: detects collaborators on primary room, resumes all queues.
598+
await jest.advanceTimersByTimeAsync( 0 );
599+
600+
// Second poll: collection room queue should now be unpaused,
601+
// so its initial sync_step1 update should be included.
602+
mockPostSyncUpdate.mockResolvedValue( {
603+
rooms: [
604+
{
605+
room: 'primary-room',
606+
end_cursor: 2,
607+
awareness: {
608+
1: { collaboratorInfo: { id: 100 } },
609+
2: { collaboratorInfo: { id: 200 } },
610+
},
611+
updates: [],
612+
},
613+
{
614+
room: 'collection-room',
615+
end_cursor: 2,
616+
awareness: {},
617+
updates: [],
618+
},
619+
],
620+
} );
621+
622+
await jest.advanceTimersByTimeAsync( 1000 );
623+
624+
// The second call should include non-empty updates for the collection room.
625+
const secondCallPayload = mockPostSyncUpdate.mock.calls[ 1 ][ 0 ];
626+
const collectionRoom = secondCallPayload.rooms.find(
627+
( r: { room: string } ) => r.room === 'collection-room'
628+
);
629+
expect( collectionRoom!.updates.length ).toBeGreaterThan( 0 );
630+
} );
631+
632+
it( 'does not resume non-primary room queues when no collaborators are detected', async () => {
633+
// Only 1 client (self) — no collaborators.
634+
mockPostSyncUpdate.mockResolvedValue( {
635+
rooms: [
636+
{
637+
room: 'primary-room',
638+
end_cursor: 1,
639+
awareness: { 1: { collaboratorInfo: { id: 100 } } },
640+
updates: [],
641+
},
642+
{
643+
room: 'collection-room',
644+
end_cursor: 1,
645+
awareness: {},
646+
updates: [],
647+
},
648+
],
649+
} );
650+
651+
pollingManager.registerRoom( {
652+
room: 'primary-room',
653+
doc: createMockDoc( 1 ),
654+
awareness: createMockAwareness(),
655+
log: jest.fn(),
656+
onStatusChange: jest.fn(),
657+
onSync: jest.fn(),
658+
} );
659+
660+
pollingManager.registerRoom( {
661+
room: 'collection-room',
662+
doc: createMockDoc( 2 ),
663+
awareness: createMockAwareness(),
664+
log: jest.fn(),
665+
onStatusChange: jest.fn(),
666+
onSync: jest.fn(),
667+
} );
668+
669+
// First poll: no collaborators.
670+
await jest.advanceTimersByTimeAsync( 0 );
671+
672+
// Second poll: collection room queue should still be paused.
673+
await jest.advanceTimersByTimeAsync( 4000 );
674+
675+
const secondCallPayload = mockPostSyncUpdate.mock.calls[ 1 ][ 0 ];
676+
const collectionRoom = secondCallPayload.rooms.find(
677+
( r: { room: string } ) => r.room === 'collection-room'
678+
);
679+
expect( collectionRoom!.updates ).toEqual( [] );
680+
} );
681+
682+
it( 'sends accumulated collection room updates after collaborator detection', async () => {
683+
// First poll: no collaborators.
684+
mockPostSyncUpdate.mockResolvedValue( {
685+
rooms: [
686+
{
687+
room: 'primary-room',
688+
end_cursor: 1,
689+
awareness: { 1: { collaboratorInfo: { id: 100 } } },
690+
updates: [],
691+
},
692+
{
693+
room: 'collection-room',
694+
end_cursor: 1,
695+
awareness: {},
696+
updates: [],
697+
},
698+
],
699+
} );
700+
701+
const collectionDoc = createMockDoc( 2 );
702+
703+
pollingManager.registerRoom( {
704+
room: 'primary-room',
705+
doc: createMockDoc( 1 ),
706+
awareness: createMockAwareness(),
707+
log: jest.fn(),
708+
onStatusChange: jest.fn(),
709+
onSync: jest.fn(),
710+
} );
711+
712+
pollingManager.registerRoom( {
713+
room: 'collection-room',
714+
doc: collectionDoc,
715+
awareness: createMockAwareness(),
716+
log: jest.fn(),
717+
onStatusChange: jest.fn(),
718+
onSync: jest.fn(),
719+
} );
720+
721+
// First poll: no collaborators, queues stay paused.
722+
await jest.advanceTimersByTimeAsync( 0 );
723+
724+
// Simulate a local doc update on the collection room (e.g., a note was saved).
725+
const onDocUpdate = collectionDoc.on.mock.calls.find(
726+
( args: unknown[] ) => args[ 0 ] === 'updateV2'
727+
)![ 1 ] as ( update: Uint8Array, origin: unknown ) => void;
728+
onDocUpdate( new Uint8Array( [ 1, 2, 3 ] ), 'local-origin' );
729+
730+
// Second poll: still no collaborators, collection room updates should be empty.
731+
mockPostSyncUpdate.mockResolvedValue( {
732+
rooms: [
733+
{
734+
room: 'primary-room',
735+
end_cursor: 2,
736+
awareness: { 1: { collaboratorInfo: { id: 100 } } },
737+
updates: [],
738+
},
739+
{
740+
room: 'collection-room',
741+
end_cursor: 2,
742+
awareness: {},
743+
updates: [],
744+
},
745+
],
746+
} );
747+
await jest.advanceTimersByTimeAsync( 4000 );
748+
749+
const secondCallPayload = mockPostSyncUpdate.mock.calls[ 1 ][ 0 ];
750+
const collectionRoomPoll2 = secondCallPayload.rooms.find(
751+
( r: { room: string } ) => r.room === 'collection-room'
752+
);
753+
expect( collectionRoomPoll2!.updates ).toEqual( [] );
754+
755+
// Third poll: collaborator joins — queues should be resumed.
756+
mockPostSyncUpdate.mockResolvedValue( {
757+
rooms: [
758+
{
759+
room: 'primary-room',
760+
end_cursor: 3,
761+
awareness: {
762+
1: { collaboratorInfo: { id: 100 } },
763+
2: { collaboratorInfo: { id: 200 } },
764+
},
765+
updates: [],
766+
},
767+
{
768+
room: 'collection-room',
769+
end_cursor: 3,
770+
awareness: {},
771+
updates: [],
772+
},
773+
],
774+
} );
775+
await jest.advanceTimersByTimeAsync( 4000 );
776+
777+
// Fourth poll: collection room should now send accumulated updates.
778+
mockPostSyncUpdate.mockResolvedValue( {
779+
rooms: [
780+
{
781+
room: 'primary-room',
782+
end_cursor: 4,
783+
awareness: {
784+
1: { collaboratorInfo: { id: 100 } },
785+
2: { collaboratorInfo: { id: 200 } },
786+
},
787+
updates: [],
788+
},
789+
{
790+
room: 'collection-room',
791+
end_cursor: 4,
792+
awareness: {},
793+
updates: [],
794+
},
795+
],
796+
} );
797+
await jest.advanceTimersByTimeAsync( 1000 );
798+
799+
const fourthCallPayload = mockPostSyncUpdate.mock.calls[ 3 ][ 0 ];
800+
const collectionRoomPoll4 = fourthCallPayload.rooms.find(
801+
( r: { room: string } ) => r.room === 'collection-room'
802+
);
803+
// Should include the initial sync_step1 update + the local update.
804+
expect( collectionRoomPoll4!.updates.length ).toBeGreaterThan( 0 );
805+
} );
806+
} );
807+
555808
describe( 'visibility change', () => {
556809
it( 'does not spawn a duplicate poll when a request is in-flight', () => {
557810
// Keep the first postSyncUpdate pending so we can simulate

0 commit comments

Comments
 (0)