From d29e840bc6eb1d1f51bcf0900c905036adbeda93 Mon Sep 17 00:00:00 2001 From: Tilman Vatteroth Date: Mon, 8 May 2023 17:23:28 +0200 Subject: [PATCH] fix(realtime): allow realtime user status updates from users that have read-only access Signed-off-by: Tilman Vatteroth --- .../realtime-user-status-adapter.spec.ts | 285 +++++++++++++++--- .../realtime-user-status-adapter.ts | 26 +- .../src/message-transporters/realtime-user.ts | 2 +- .../receive-remote-cursor-view-plugin.ts | 19 +- 4 files changed, 276 insertions(+), 56 deletions(-) diff --git a/backend/src/realtime/realtime-note/realtime-user-status-adapter.spec.ts b/backend/src/realtime/realtime-note/realtime-user-status-adapter.spec.ts index 8d25ecb0d..a2202e290 100644 --- a/backend/src/realtime/realtime-note/realtime-user-status-adapter.spec.ts +++ b/backend/src/realtime/realtime-note/realtime-user-status-adapter.spec.ts @@ -119,6 +119,13 @@ describe('realtime user status adapter', () => { username: null, displayName: guestDisplayName, }, + { + active: true, + cursor: null, + displayName: clientDeclineUsername, + styleIndex: 4, + username: clientDeclineUsername, + }, ], }, }; @@ -180,6 +187,13 @@ describe('realtime user status adapter', () => { username: null, displayName: guestDisplayName, }, + { + active: true, + cursor: null, + displayName: clientDeclineUsername, + styleIndex: 4, + username: clientDeclineUsername, + }, ], }, }; @@ -212,6 +226,55 @@ describe('realtime user status adapter', () => { username: clientLoggedIn2Username, displayName: clientLoggedIn2Username, }, + { + active: true, + cursor: null, + displayName: clientDeclineUsername, + styleIndex: 4, + username: clientDeclineUsername, + }, + ], + }, + }; + + const expectedMessage5: Message = { + type: MessageType.REALTIME_USER_STATE_SET, + payload: { + ownUser: { + displayName: clientDeclineUsername, + styleIndex: 4, + }, + users: [ + { + active: true, + cursor: { + from: newFrom, + to: newTo, + }, + styleIndex: 0, + username: clientLoggedIn1Username, + displayName: clientLoggedIn1Username, + }, + { + active: true, + cursor: { + from: 0, + to: 0, + }, + styleIndex: 1, + username: clientLoggedIn2Username, + displayName: clientLoggedIn2Username, + }, + { + active: true, + cursor: { + from: 0, + to: 0, + }, + displayName: guestDisplayName, + styleIndex: 2, + username: null, + }, ], }, }; @@ -226,7 +289,10 @@ describe('realtime user status adapter', () => { expectedMessage3, ); expect(clientNotReadySendMessageSpy).toHaveBeenCalledTimes(0); - expect(clientDeclineSendMessageSpy).toHaveBeenCalledTimes(1); + expect(clientDeclineSendMessageSpy).toHaveBeenNthCalledWith( + 1, + expectedMessage5, + ); }); it('will inform other clients about removed client', () => { @@ -256,6 +322,13 @@ describe('realtime user status adapter', () => { username: null, displayName: guestDisplayName, }, + { + active: true, + cursor: null, + displayName: clientDeclineUsername, + styleIndex: 4, + username: clientDeclineUsername, + }, ], }, }; @@ -278,6 +351,45 @@ describe('realtime user status adapter', () => { username: clientLoggedIn1Username, displayName: clientLoggedIn1Username, }, + { + active: true, + cursor: null, + displayName: clientDeclineUsername, + styleIndex: 4, + username: clientDeclineUsername, + }, + ], + }, + }; + + const expectedMessage5: Message = { + type: MessageType.REALTIME_USER_STATE_SET, + payload: { + ownUser: { + displayName: clientDeclineUsername, + styleIndex: 4, + }, + users: [ + { + active: true, + cursor: { + from: 0, + to: 0, + }, + styleIndex: 0, + username: clientLoggedIn1Username, + displayName: clientLoggedIn1Username, + }, + { + active: true, + cursor: { + from: 0, + to: 0, + }, + displayName: guestDisplayName, + styleIndex: 2, + username: null, + }, ], }, }; @@ -292,7 +404,10 @@ describe('realtime user status adapter', () => { expectedMessage3, ); expect(clientNotReadySendMessageSpy).toHaveBeenCalledTimes(0); - expect(clientDeclineSendMessageSpy).toHaveBeenCalledTimes(1); + expect(clientDeclineSendMessageSpy).toHaveBeenNthCalledWith( + 1, + expectedMessage5, + ); }); it('will inform other clients about inactivity and reactivity', () => { @@ -338,7 +453,14 @@ describe('realtime user status adapter', () => { }, styleIndex: 2, username: null, - displayName: 'Virtuous Mockingbird', + displayName: guestDisplayName, + }, + { + active: true, + cursor: null, + displayName: clientDeclineUsername, + styleIndex: 4, + username: clientDeclineUsername, }, ], }, @@ -350,7 +472,7 @@ describe('realtime user status adapter', () => { payload: { ownUser: { styleIndex: 2, - displayName: 'Virtuous Mockingbird', + displayName: guestDisplayName, }, users: [ { @@ -373,6 +495,56 @@ describe('realtime user status adapter', () => { username: clientLoggedIn2Username, displayName: clientLoggedIn2Username, }, + { + active: true, + cursor: null, + displayName: clientDeclineUsername, + styleIndex: 4, + username: clientDeclineUsername, + }, + ], + }, + }; + + const expectedInactivityMessage5: Message = + { + type: MessageType.REALTIME_USER_STATE_SET, + payload: { + ownUser: { + styleIndex: 4, + displayName: clientDeclineUsername, + }, + users: [ + { + active: false, + cursor: { + from: 0, + to: 0, + }, + styleIndex: 0, + username: clientLoggedIn1Username, + displayName: clientLoggedIn1Username, + }, + { + active: true, + cursor: { + from: 0, + to: 0, + }, + styleIndex: 1, + username: clientLoggedIn2Username, + displayName: clientLoggedIn2Username, + }, + { + active: true, + cursor: { + from: 0, + to: 0, + }, + displayName: guestDisplayName, + styleIndex: 2, + username: null, + }, ], }, }; @@ -387,7 +559,10 @@ describe('realtime user status adapter', () => { expectedInactivityMessage3, ); expect(clientNotReadySendMessageSpy).toHaveBeenCalledTimes(0); - expect(clientDeclineSendMessageSpy).toHaveBeenCalledTimes(1); + expect(clientDeclineSendMessageSpy).toHaveBeenNthCalledWith( + 1, + expectedInactivityMessage5, + ); clientLoggedIn1 .getTransporter() @@ -408,7 +583,10 @@ describe('realtime user status adapter', () => { expectedInactivityMessage3, ); expect(clientNotReadySendMessageSpy).toHaveBeenCalledTimes(0); - expect(clientDeclineSendMessageSpy).toHaveBeenCalledTimes(1); + expect(clientDeclineSendMessageSpy).toHaveBeenNthCalledWith( + 1, + expectedInactivityMessage5, + ); clientLoggedIn1 .getTransporter() @@ -446,7 +624,14 @@ describe('realtime user status adapter', () => { }, styleIndex: 2, username: null, - displayName: 'Virtuous Mockingbird', + displayName: guestDisplayName, + }, + { + active: true, + cursor: null, + displayName: clientDeclineUsername, + styleIndex: 4, + username: clientDeclineUsername, }, ], }, @@ -458,7 +643,7 @@ describe('realtime user status adapter', () => { payload: { ownUser: { styleIndex: 2, - displayName: 'Virtuous Mockingbird', + displayName: guestDisplayName, }, users: [ { @@ -481,6 +666,56 @@ describe('realtime user status adapter', () => { username: clientLoggedIn2Username, displayName: clientLoggedIn2Username, }, + { + active: true, + cursor: null, + displayName: clientDeclineUsername, + styleIndex: 4, + username: clientDeclineUsername, + }, + ], + }, + }; + + const expectedReactivityMessage5: Message = + { + type: MessageType.REALTIME_USER_STATE_SET, + payload: { + ownUser: { + styleIndex: 4, + displayName: clientDeclineUsername, + }, + users: [ + { + active: true, + cursor: { + from: 0, + to: 0, + }, + styleIndex: 0, + username: clientLoggedIn1Username, + displayName: clientLoggedIn1Username, + }, + { + active: true, + cursor: { + from: 0, + to: 0, + }, + styleIndex: 1, + username: clientLoggedIn2Username, + displayName: clientLoggedIn2Username, + }, + { + active: true, + cursor: { + from: 0, + to: 0, + }, + displayName: guestDisplayName, + styleIndex: 2, + username: null, + }, ], }, }; @@ -495,7 +730,10 @@ describe('realtime user status adapter', () => { expectedReactivityMessage3, ); expect(clientNotReadySendMessageSpy).toHaveBeenCalledTimes(0); - expect(clientDeclineSendMessageSpy).toHaveBeenCalledTimes(2); + expect(clientDeclineSendMessageSpy).toHaveBeenNthCalledWith( + 1, + expectedReactivityMessage5, + ); clientLoggedIn1 .getTransporter() @@ -516,30 +754,9 @@ describe('realtime user status adapter', () => { expectedReactivityMessage3, ); expect(clientNotReadySendMessageSpy).toHaveBeenCalledTimes(0); - expect(clientDeclineSendMessageSpy).toHaveBeenCalledTimes(2); - }); - - it('will ignore updates from read only clients', () => { - expect(clientLoggedIn1SendMessageSpy).toHaveBeenCalledTimes(0); - expect(clientLoggedIn2SendMessageSpy).toHaveBeenCalledTimes(0); - expect(clientGuestSendMessageSpy).toHaveBeenCalledTimes(0); - expect(clientNotReadySendMessageSpy).toHaveBeenCalledTimes(0); - expect(clientDeclineSendMessageSpy).toHaveBeenCalledTimes(0); - - clientDecline - .getTransporter() - .emit(MessageType.REALTIME_USER_SINGLE_UPDATE, { - type: MessageType.REALTIME_USER_SINGLE_UPDATE, - payload: { - from: 0, - to: 1234, - }, - }); - - expect(clientLoggedIn1SendMessageSpy).toHaveBeenCalledTimes(0); - expect(clientLoggedIn2SendMessageSpy).toHaveBeenCalledTimes(0); - expect(clientGuestSendMessageSpy).toHaveBeenCalledTimes(0); - expect(clientNotReadySendMessageSpy).toHaveBeenCalledTimes(0); - expect(clientDeclineSendMessageSpy).toHaveBeenCalledTimes(0); + expect(clientDeclineSendMessageSpy).toHaveBeenNthCalledWith( + 1, + expectedReactivityMessage5, + ); }); }); diff --git a/backend/src/realtime/realtime-note/realtime-user-status-adapter.ts b/backend/src/realtime/realtime-note/realtime-user-status-adapter.ts index 2fe83db08..e925587a1 100644 --- a/backend/src/realtime/realtime-note/realtime-user-status-adapter.ts +++ b/backend/src/realtime/realtime-note/realtime-user-status-adapter.ts @@ -41,10 +41,12 @@ export class RealtimeUserStatusAdapter { styleIndex: this.findLeastUsedStyleIndex( this.createStyleIndexToCountMap(realtimeNote), ), - cursor: { - from: 0, - to: 0, - }, + cursor: !this.acceptCursorUpdateProvider() + ? null + : { + from: 0, + to: 0, + }, }; } @@ -53,10 +55,10 @@ export class RealtimeUserStatusAdapter { const transporterMessagesListener = connection.getTransporter().on( MessageType.REALTIME_USER_SINGLE_UPDATE, (message: Message) => { - if (this.acceptCursorUpdateProvider()) { - this.realtimeUser.cursor = message.payload; - this.sendRealtimeUserStatusUpdateEvent(connection); - } + this.realtimeUser.cursor = this.acceptCursorUpdateProvider() + ? message.payload + : null; + this.sendRealtimeUserStatusUpdateEvent(connection); }, { objectify: true }, ) as Listener; @@ -83,10 +85,7 @@ export class RealtimeUserStatusAdapter { const realtimeUserSetActivityListener = connection.getTransporter().on( MessageType.REALTIME_USER_SET_ACTIVITY, (message: Message) => { - if ( - !this.acceptCursorUpdateProvider() || - this.realtimeUser.active === message.payload.active - ) { + if (this.realtimeUser.active === message.payload.active) { return; } this.realtimeUser.active = message.payload.active; @@ -115,9 +114,6 @@ export class RealtimeUserStatusAdapter { const realtimeUser = receivingClient.getRealtimeUserStateAdapter().realtimeUser; const realtimeUsers = this.collectAllConnectionsExcept(receivingClient) - .filter((client) => - client.getRealtimeUserStateAdapter().acceptCursorUpdateProvider(), - ) .map((client) => client.getRealtimeUserStateAdapter().realtimeUser) .filter((realtimeUser) => realtimeUser !== null); diff --git a/commons/src/message-transporters/realtime-user.ts b/commons/src/message-transporters/realtime-user.ts index b6d22fe63..c6de286b7 100644 --- a/commons/src/message-transporters/realtime-user.ts +++ b/commons/src/message-transporters/realtime-user.ts @@ -9,7 +9,7 @@ export interface RealtimeUser { username: string | null active: boolean styleIndex: number - cursor: RemoteCursor + cursor: RemoteCursor | null } export interface RemoteCursor { diff --git a/frontend/src/components/editor-page/editor-pane/codemirror-extensions/remote-cursors/receive-remote-cursor-view-plugin.ts b/frontend/src/components/editor-page/editor-pane/codemirror-extensions/remote-cursors/receive-remote-cursor-view-plugin.ts index 7656b4e4e..21f3bfef3 100644 --- a/frontend/src/components/editor-page/editor-pane/codemirror-extensions/remote-cursors/receive-remote-cursor-view-plugin.ts +++ b/frontend/src/components/editor-page/editor-pane/codemirror-extensions/remote-cursors/receive-remote-cursor-view-plugin.ts @@ -20,12 +20,19 @@ export class ReceiveRemoteCursorViewPlugin implements PluginValue { this.listener = messageTransporter.on( MessageType.REALTIME_USER_STATE_SET, ({ payload }) => { - const cursors: RemoteCursor[] = payload.users.map((user) => ({ - from: user.cursor.from, - to: user.cursor.to, - displayName: user.displayName, - styleIndex: user.styleIndex - })) + const cursors = payload.users + .map((user) => { + if (!user.cursor) { + return undefined + } + return { + from: user.cursor.from, + to: user.cursor.to, + displayName: user.displayName, + styleIndex: user.styleIndex + } + }) + .filter((value) => value !== undefined) as RemoteCursor[] view.dispatch({ effects: [remoteCursorUpdateEffect.of(cursors)] })