Skip to content

Commit 7ea4884

Browse files
pentoingeniumed
authored and
adamsilverstein
committed
RTC: Fix notes not syncing between collaborative editors (#76873)
466db06 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. Co-authored-by: pento <pento@git.wordpress.org> Co-authored-by: ingeniumed <ingeniumed@git.wordpress.org>
1 parent 85928dc commit 7ea4884

3 files changed

Lines changed: 496 additions & 15 deletions

File tree

‎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: 262 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,17 @@ function createMockDoc( clientID = 1 ) {
7777
return { clientID, on: jest.fn(), off: jest.fn() };
7878
}
7979

80+
// Helper to extract the onDocUpdate callback registered via doc.on('updateV2', ...).
81+
function getOnDocUpdate( doc: ReturnType< typeof createMockDoc > ) {
82+
const call = doc.on.mock.calls.find(
83+
( args: unknown[] ) => args[ 0 ] === 'updateV2'
84+
);
85+
if ( ! call ) {
86+
throw new Error( 'onDocUpdate not registered' );
87+
}
88+
return call[ 1 ] as ( update: Uint8Array, origin: unknown ) => void;
89+
}
90+
8091
function createMockAwareness() {
8192
return {
8293
clientID: 1,
@@ -141,17 +152,6 @@ describe( 'polling-manager', () => {
141152
} );
142153

143154
describe( 'document size limit', () => {
144-
// Helper to extract the onDocUpdate callback registered via doc.on('updateV2', ...).
145-
function getOnDocUpdate( doc: ReturnType< typeof createMockDoc > ) {
146-
const call = doc.on.mock.calls.find(
147-
( args: unknown[] ) => args[ 0 ] === 'updateV2'
148-
);
149-
if ( ! call ) {
150-
throw new Error( 'onDocUpdate not registered' );
151-
}
152-
return call[ 1 ] as ( update: Uint8Array, origin: unknown ) => void;
153-
}
154-
155155
it( 'emits document-size-limit-exceeded error when an update exceeds the size limit', async () => {
156156
mockPostSyncUpdate.mockResolvedValue( syncResponse );
157157

@@ -552,6 +552,257 @@ 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 = getOnDocUpdate( collectionDoc );
726+
onDocUpdate( new Uint8Array( [ 1, 2, 3 ] ), 'local-origin' );
727+
728+
// Second poll: still no collaborators, collection room updates should be empty.
729+
mockPostSyncUpdate.mockResolvedValue( {
730+
rooms: [
731+
{
732+
room: 'primary-room',
733+
end_cursor: 2,
734+
awareness: { 1: { collaboratorInfo: { id: 100 } } },
735+
updates: [],
736+
},
737+
{
738+
room: 'collection-room',
739+
end_cursor: 2,
740+
awareness: {},
741+
updates: [],
742+
},
743+
],
744+
} );
745+
await jest.advanceTimersByTimeAsync( 4000 );
746+
747+
const secondCallPayload = mockPostSyncUpdate.mock.calls[ 1 ][ 0 ];
748+
const collectionRoomPoll2 = secondCallPayload.rooms.find(
749+
( r: { room: string } ) => r.room === 'collection-room'
750+
);
751+
expect( collectionRoomPoll2!.updates ).toEqual( [] );
752+
753+
// Third poll: collaborator joins — queues should be resumed.
754+
mockPostSyncUpdate.mockResolvedValue( {
755+
rooms: [
756+
{
757+
room: 'primary-room',
758+
end_cursor: 3,
759+
awareness: {
760+
1: { collaboratorInfo: { id: 100 } },
761+
2: { collaboratorInfo: { id: 200 } },
762+
},
763+
updates: [],
764+
},
765+
{
766+
room: 'collection-room',
767+
end_cursor: 3,
768+
awareness: {},
769+
updates: [],
770+
},
771+
],
772+
} );
773+
await jest.advanceTimersByTimeAsync( 4000 );
774+
775+
// Fourth poll: collection room should now send accumulated updates.
776+
mockPostSyncUpdate.mockResolvedValue( {
777+
rooms: [
778+
{
779+
room: 'primary-room',
780+
end_cursor: 4,
781+
awareness: {
782+
1: { collaboratorInfo: { id: 100 } },
783+
2: { collaboratorInfo: { id: 200 } },
784+
},
785+
updates: [],
786+
},
787+
{
788+
room: 'collection-room',
789+
end_cursor: 4,
790+
awareness: {},
791+
updates: [],
792+
},
793+
],
794+
} );
795+
await jest.advanceTimersByTimeAsync( 1000 );
796+
797+
const fourthCallPayload = mockPostSyncUpdate.mock.calls[ 3 ][ 0 ];
798+
const collectionRoomPoll4 = fourthCallPayload.rooms.find(
799+
( r: { room: string } ) => r.room === 'collection-room'
800+
);
801+
// Should include the initial sync_step1 update + the local update.
802+
expect( collectionRoomPoll4!.updates.length ).toBeGreaterThan( 0 );
803+
} );
804+
} );
805+
555806
describe( 'visibility change', () => {
556807
it( 'does not spawn a duplicate poll when a request is in-flight', () => {
557808
// Keep the first postSyncUpdate pending so we can simulate

0 commit comments

Comments
 (0)