Skip to content

Commit 4633018

Browse files
authored
fix: deduplicate edit clients (#260)
1 parent e55b61b commit 4633018

File tree

9 files changed

+276
-24
lines changed

9 files changed

+276
-24
lines changed
Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
import * as awarenessProtocol from 'y-protocols/awareness';
2+
3+
import { TestTool } from '../../../support/page-utils';
4+
import { PageSelectors } from '../../../support/selectors';
5+
import { testLog } from '../../../support/test-helpers';
6+
7+
import { avatarTestUtils } from './avatar-test-utils';
8+
9+
const { generateRandomEmail, setupBeforeEach, signInAndWaitForApp, imports } = avatarTestUtils;
10+
const { AvatarSelectors, dbUtils } = imports;
11+
12+
type TestWindow = Window & {
13+
__APPFLOWY_AWARENESS_MAP__?: Record<string, awarenessProtocol.Awareness>;
14+
};
15+
16+
const openFirstPageAndTriggerAwareness = (): void => {
17+
TestTool.expandSpace(0);
18+
cy.wait(1000);
19+
20+
PageSelectors.names().should('be.visible', { timeout: 10000 });
21+
PageSelectors.names().first().then(($page) => {
22+
cy.wrap($page).click({ force: true });
23+
});
24+
25+
cy.wait(2000);
26+
27+
cy.get('[contenteditable="true"]').then(($editors) => {
28+
if ($editors.length === 0) {
29+
return;
30+
}
31+
32+
let editorFound = false;
33+
34+
$editors.each((index, el) => {
35+
const $el = Cypress.$(el);
36+
37+
38+
if (!$el.attr('data-testid')?.includes('title') && !$el.hasClass('editor-title')) {
39+
cy.wrap(el).click({ force: true });
40+
cy.wait(500);
41+
cy.wrap(el).type(' ', { force: true });
42+
editorFound = true;
43+
return false;
44+
}
45+
});
46+
47+
if (!editorFound) {
48+
cy.wrap($editors.last()).click({ force: true });
49+
cy.wait(500);
50+
cy.wrap($editors.last()).type(' ', { force: true });
51+
}
52+
});
53+
54+
cy.wait(1500);
55+
};
56+
57+
describe('Avatar Awareness Dedupe', () => {
58+
beforeEach(() => {
59+
setupBeforeEach();
60+
});
61+
62+
it('should show one header avatar for same user across multiple awareness clients', function () {
63+
const testEmail = generateRandomEmail();
64+
65+
signInAndWaitForApp(testEmail).then(() => {
66+
testLog.info('Step 1: Open a document and trigger local awareness');
67+
openFirstPageAndTriggerAwareness();
68+
69+
AvatarSelectors.headerAvatars().should('have.length', 1);
70+
71+
dbUtils.getCurrentUserUuid().then((userUuid) => {
72+
expect(userUuid, 'Current user UUID should be available').to.be.a('string').and.not.be.empty;
73+
74+
testLog.info('Step 2: Inject two remote awareness clients for the same user UUID');
75+
cy.window().then((win) => {
76+
const testWindow = win as TestWindow;
77+
78+
expect(testWindow.__APPFLOWY_AWARENESS_MAP__, 'Awareness map test hook should be exposed').to.exist;
79+
const awarenessMap = testWindow.__APPFLOWY_AWARENESS_MAP__!;
80+
81+
const pathMatch = win.location.pathname.match(/\/app\/[^/]+\/([^/?]+)/);
82+
const currentViewId = pathMatch?.[1];
83+
84+
expect(currentViewId, 'Current viewId should be parsed from URL').to.be.a('string').and.not.be.empty;
85+
86+
const awareness = currentViewId ? awarenessMap[currentViewId] : undefined;
87+
const fallbackAwareness = awareness || Object.values(awarenessMap)[0];
88+
89+
expect(fallbackAwareness, 'Awareness instance for active view').to.exist;
90+
if (!fallbackAwareness) {
91+
throw new Error('Awareness instance for active view is missing');
92+
}
93+
94+
const targetAwareness = fallbackAwareness;
95+
96+
const nowSeconds = Math.floor(Date.now() / 1000);
97+
const remoteClientA = 990001;
98+
const remoteClientB = 990002;
99+
100+
const stateA = {
101+
version: 1,
102+
timestamp: nowSeconds,
103+
user: {
104+
uid: 1000001,
105+
device_id: 'cypress-device-a',
106+
},
107+
metadata: JSON.stringify({
108+
user_name: 'Same User',
109+
cursor_color: '#ff6600',
110+
selection_color: '#ff660040',
111+
user_avatar: '',
112+
user_uuid: userUuid,
113+
}),
114+
};
115+
const stateB = {
116+
version: 1,
117+
timestamp: nowSeconds + 1,
118+
user: {
119+
uid: 2000002,
120+
device_id: 'cypress-device-b',
121+
},
122+
metadata: JSON.stringify({
123+
user_name: 'Same User',
124+
cursor_color: '#ff6600',
125+
selection_color: '#ff660040',
126+
user_avatar: '',
127+
user_uuid: userUuid,
128+
}),
129+
};
130+
131+
const states = new Map<number, Record<string, unknown>>([
132+
[remoteClientA, stateA],
133+
[remoteClientB, stateB],
134+
]);
135+
136+
// encodeAwarenessUpdate requires clocks in meta for each client id.
137+
targetAwareness.meta.set(remoteClientA, { clock: 1, lastUpdated: Date.now() });
138+
targetAwareness.meta.set(remoteClientB, { clock: 1, lastUpdated: Date.now() });
139+
140+
const update = awarenessProtocol.encodeAwarenessUpdate(targetAwareness, [remoteClientA, remoteClientB], states);
141+
142+
awarenessProtocol.applyAwarenessUpdate(targetAwareness, update, 'cypress');
143+
});
144+
145+
testLog.info('Step 3: Verify header keeps one avatar for the same user');
146+
cy.wait(1000);
147+
AvatarSelectors.headerAvatars().should('have.length', 1);
148+
});
149+
});
150+
});
151+
});

src/application/awareness/dispatch.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import dayjs from 'dayjs';
22
import { debounce } from 'lodash-es';
3-
import { useCallback, useMemo, useRef } from 'react';
3+
import { useCallback, useEffect, useMemo, useRef } from 'react';
44
import { Editor } from 'slate';
55
import { Awareness } from 'y-protocols/awareness';
66

@@ -14,6 +14,7 @@ import { convertSlateSelectionToAwareness, generateUserColors } from './utils';
1414
// User information parameters for awareness synchronization
1515
export interface UserAwarenessParams {
1616
uid: number;
17+
user_uuid?: string;
1718
device_id: string;
1819
user_name: string;
1920
cursor_color: string;
@@ -35,6 +36,7 @@ export function useDispatchUserAwareness(awareness?: Awareness) {
3536
cursor_color: userParams.cursor_color,
3637
selection_color: userParams.selection_color,
3738
user_avatar: userParams.user_avatar || '',
39+
user_uuid: userParams.user_uuid,
3840
};
3941

4042
const awarenessState: AwarenessState = {
@@ -85,6 +87,7 @@ export function useDispatchCursorAwareness(awareness?: Awareness) {
8587
cursor_color: userParams.cursor_color,
8688
selection_color: userParams.selection_color,
8789
user_avatar: userParams.user_avatar || '',
90+
user_uuid: userParams.user_uuid,
8891
};
8992

9093
const awarenessState: AwarenessState = {
@@ -112,6 +115,12 @@ export function useDispatchCursorAwareness(awareness?: Awareness) {
112115
return debounce(syncCursor, 100);
113116
}, [syncCursor]);
114117

118+
useEffect(() => {
119+
return () => {
120+
debounceSyncCursor.cancel();
121+
};
122+
}, [debounceSyncCursor]);
123+
115124
const dispatchCursor = useCallback(
116125
(userParams: UserAwarenessParams, editor?: Editor) => {
117126
if (!awareness || !editor) return;
@@ -169,6 +178,7 @@ export function useDispatchClearAwareness(awareness?: Awareness) {
169178
cursor_color: generateUserColors(currentUser?.name || '').cursor_color,
170179
selection_color: generateUserColors(currentUser?.name || '').selection_color,
171180
user_avatar: userAvatar,
181+
user_uuid: currentUser?.uuid,
172182
}),
173183
});
174184

src/application/awareness/selector.ts

Lines changed: 53 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,43 @@ import { convertAwarenessSelection } from './utils';
1212
// Re-export types for backward compatibility
1313
export type { AwarenessMetadata, AwarenessSelection, AwarenessState, AwarenessUser, Cursor } from './types';
1414

15+
const getAwarenessIdentityKey = (uuid: string | undefined, uid: number, deviceId: string) => {
16+
if (uuid) {
17+
return `uuid:${uuid}`;
18+
}
19+
20+
if (Number.isFinite(uid)) {
21+
return `uid:${uid}`;
22+
}
23+
24+
return `device:${deviceId}`;
25+
};
26+
27+
const getNormalizedMetadata = (rawMetadata?: string): AwarenessMetadata | null => {
28+
if (!rawMetadata) {
29+
return null;
30+
}
31+
32+
try {
33+
const parsed = JSON.parse(rawMetadata) as Record<string, unknown>;
34+
35+
if (!parsed || typeof parsed !== 'object') {
36+
return null;
37+
}
38+
39+
return {
40+
user_name: typeof parsed.user_name === 'string' ? parsed.user_name : '',
41+
cursor_color: typeof parsed.cursor_color === 'string' ? parsed.cursor_color : '',
42+
selection_color: typeof parsed.selection_color === 'string' ? parsed.selection_color : '',
43+
user_avatar: typeof parsed.user_avatar === 'string' ? parsed.user_avatar : '',
44+
user_uuid: typeof parsed.user_uuid === 'string' ? parsed.user_uuid : undefined,
45+
};
46+
} catch (error) {
47+
Log.warn('Failed to parse awareness metadata', { error });
48+
return null;
49+
}
50+
};
51+
1552
export function useUsersSelector(awareness?: Awareness) {
1653
const [users, setUsers] = useState<AwarenessUser[]>([]);
1754

@@ -27,10 +64,15 @@ export function useUsersSelector(awareness?: Awareness) {
2764
}
2865

2966
const uid = Number(state.user.uid);
30-
const meta = JSON.parse(state.metadata || '{}') as AwarenessMetadata;
67+
const meta = getNormalizedMetadata(state.metadata);
68+
69+
if (!meta) {
70+
return;
71+
}
3172

3273
users.push({
3374
uid,
75+
uuid: meta.user_uuid,
3476
name: meta.user_name,
3577
timestamp: state.timestamp,
3678
device_id: state.user.device_id,
@@ -42,7 +84,7 @@ export function useUsersSelector(awareness?: Awareness) {
4284
setUsers(
4385
uniqBy(
4486
users.sort((a, b) => b.timestamp - a.timestamp),
45-
'uid'
87+
(user) => getAwarenessIdentityKey(user.uuid, user.uid, user.device_id)
4688
).filter((user) => !!user.name)
4789
);
4890
};
@@ -60,6 +102,7 @@ export function useRemoteSelectionsSelector(awareness?: Awareness) {
60102
const [cursors, setCursors] = useState<Cursor[]>([]);
61103
const editor = useSlate();
62104
const service = useService();
105+
const localDeviceId = service?.getDeviceId();
63106

64107
useEffect(() => {
65108
const renderCursors = () => {
@@ -72,11 +115,13 @@ export function useRemoteSelectionsSelector(awareness?: Awareness) {
72115
}
73116

74117
const uid = Number(state.user.uid);
75-
const meta = JSON.parse(state.metadata || '{}') as AwarenessMetadata;
118+
const meta = getNormalizedMetadata(state.metadata);
76119

77-
const deviceId = service?.getDeviceId();
120+
if (!meta) {
121+
return;
122+
}
78123

79-
if (deviceId === state.user.device_id) {
124+
if (localDeviceId && localDeviceId === state.user.device_id) {
80125
return;
81126
}
82127

@@ -85,6 +130,7 @@ export function useRemoteSelectionsSelector(awareness?: Awareness) {
85130

86131
cursors.push({
87132
uid,
133+
uuid: meta.user_uuid,
88134
name: meta.user_name,
89135
cursorColor: meta.cursor_color,
90136
selectionColor: meta.selection_color,
@@ -100,14 +146,14 @@ export function useRemoteSelectionsSelector(awareness?: Awareness) {
100146
setCursors(
101147
uniqBy(
102148
cursors.sort((a, b) => b.timestamp - a.timestamp),
103-
'uid'
149+
(cursor) => getAwarenessIdentityKey(cursor.uuid, cursor.uid, cursor.deviceId)
104150
)
105151
);
106152
};
107153

108154
awareness?.on('change', renderCursors);
109155
return () => awareness?.off('change', renderCursors);
110-
}, [awareness, service]);
156+
}, [awareness, localDeviceId]);
111157

112158
const cursorsWithBaseRange = useMemo(() => {
113159
if (!cursors.length || !editor || !editor.children[0]) return [];

src/application/awareness/types.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ export interface AwarenessSelection {
1313

1414
export interface AwarenessUser {
1515
uid: number;
16+
uuid?: string;
1617
name: string;
1718
timestamp: number;
1819
device_id: string;
@@ -25,6 +26,7 @@ export interface AwarenessMetadata {
2526
cursor_color: string;
2627
selection_color: string;
2728
user_avatar: string;
29+
user_uuid?: string;
2830
}
2931

3032
export interface AwarenessState {
@@ -40,6 +42,7 @@ export interface AwarenessState {
4042

4143
export interface Cursor {
4244
uid: number;
45+
uuid?: string;
4346
deviceId: string;
4447
name: string;
4548
cursorColor: string;
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import { nanoid } from 'nanoid';
2+
3+
const DEVICE_ID_STORAGE_KEY = 'x-device-id';
4+
5+
let memoizedDeviceId: string | undefined;
6+
7+
/**
8+
* Returns a stable device ID for the current browser profile.
9+
* Falls back to an in-memory ID if localStorage is unavailable.
10+
*/
11+
export function getOrCreateDeviceId(): string {
12+
if (memoizedDeviceId) {
13+
return memoizedDeviceId;
14+
}
15+
16+
if (typeof window !== 'undefined') {
17+
try {
18+
const stored = window.localStorage.getItem(DEVICE_ID_STORAGE_KEY);
19+
20+
if (stored) {
21+
memoizedDeviceId = stored;
22+
return stored;
23+
}
24+
25+
const created = nanoid(8);
26+
27+
window.localStorage.setItem(DEVICE_ID_STORAGE_KEY, created);
28+
memoizedDeviceId = created;
29+
return created;
30+
} catch {
31+
// Fall back to memory-only device id below.
32+
}
33+
}
34+
35+
memoizedDeviceId = nanoid(8);
36+
return memoizedDeviceId;
37+
}

0 commit comments

Comments
 (0)