diff --git a/playwright/e2e/collaboration.spec.ts b/playwright/e2e/collaboration.spec.ts index 432922fb..5216a754 100644 --- a/playwright/e2e/collaboration.spec.ts +++ b/playwright/e2e/collaboration.spec.ts @@ -3,53 +3,365 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ import { expect } from '@playwright/test' +import type { Page } from '@playwright/test' import { test } from '../support/fixtures/random-user' import { addTextElement, createWhiteboard, newLoggedInPage, + openWhiteboardById, openFilesApp, - openWhiteboardFromFiles, - fetchBoardContent, - getBoardAuth, + resolveFileIdByDav, } from '../support/utils' +type CapturedSceneMessage = { + transport: 'room' | 'direct' + type: string + syncAll: boolean + elementsCount: number +} + +type ReceivedSceneMessage = { + type: string + syncAll: boolean + elementsCount: number +} + +type CollaborationSocketHook = { + connected?: boolean + emit: (eventName: string, ...args: unknown[]) => unknown +} + +type WhiteboardTestHooks = { + collaborationStore?: { + getState?: () => { + socket?: CollaborationSocketHook + isInRoom?: boolean + } + } + excalidrawStore?: { + getState?: () => { + excalidrawAPI?: { + getSceneElementsIncludingDeleted?: () => Array<{ + type?: string + text?: string + isDeleted?: boolean + }> + } + } + } + emittedSceneMessages?: CapturedSceneMessage[] + receivedSceneMessages?: ReceivedSceneMessage[] + sceneEmitSpyInstalled?: boolean + sceneReceiveSpyInstalled?: boolean +} + +type WhiteboardTestWindow = Window & { + __whiteboardTest?: boolean + __whiteboardTestHooks?: WhiteboardTestHooks +} + +async function enableWhiteboardTestHooks(page: Page) { + await page.addInitScript(() => { + const win = window as WhiteboardTestWindow + win.__whiteboardTest = true + win.__whiteboardTestHooks = win.__whiteboardTestHooks || {} + win.__whiteboardTestHooks.emittedSceneMessages = [] + }) + await page.evaluate(() => { + const win = window as WhiteboardTestWindow + win.__whiteboardTest = true + win.__whiteboardTestHooks = win.__whiteboardTestHooks || {} + win.__whiteboardTestHooks.emittedSceneMessages = win.__whiteboardTestHooks.emittedSceneMessages || [] + }) +} + +async function waitForCollaborationReady(page: Page) { + await page.waitForFunction(() => { + const win = window as WhiteboardTestWindow + const store = win.__whiteboardTestHooks?.collaborationStore + const state = store?.getState?.() + return Boolean(state?.socket?.connected && state?.isInRoom) + }, { timeout: 30000 }) +} + +async function installSceneEmitSpy(page: Page) { + await waitForCollaborationReady(page) + await page.evaluate(() => { + const win = window as WhiteboardTestWindow + win.__whiteboardTestHooks = win.__whiteboardTestHooks || {} + const hooks = win.__whiteboardTestHooks + const emittedSceneMessages = hooks.emittedSceneMessages || [] + hooks.emittedSceneMessages = emittedSceneMessages + if (hooks.sceneEmitSpyInstalled) { + return + } + + const store = hooks.collaborationStore + const socket = store?.getState?.().socket + if (!socket) { + throw new Error('Collaboration socket not available') + } + + const decodePayload = (payload: unknown) => { + if (typeof payload === 'string') { + return payload + } + if (payload instanceof ArrayBuffer) { + return new TextDecoder().decode(new Uint8Array(payload)) + } + if (ArrayBuffer.isView(payload)) { + return new TextDecoder().decode( + new Uint8Array(payload.buffer, payload.byteOffset, payload.byteLength), + ) + } + return '' + } + + const originalEmit = socket.emit.bind(socket) + socket.emit = (eventName: string, ...args: unknown[]) => { + if (eventName === 'server-broadcast' || eventName === 'server-direct-broadcast') { + const decoded = decodePayload(eventName === 'server-direct-broadcast' ? args[2] : args[1]) + if (decoded) { + try { + const parsed = JSON.parse(decoded) + if (parsed?.type === 'SCENE_INIT' || parsed?.type === 'SCENE_UPDATE') { + emittedSceneMessages.push({ + transport: eventName === 'server-direct-broadcast' ? 'direct' : 'room', + type: parsed.type, + syncAll: parsed.payload?.syncAll === true, + elementsCount: Array.isArray(parsed.payload?.elements) + ? parsed.payload.elements.length + : 0, + }) + } + } catch { + // Ignore frames that are not JSON scene payloads. + } + } + } + return originalEmit(eventName, ...args) + } + + hooks.sceneEmitSpyInstalled = true + }) +} + +async function clearCapturedSceneMessages(page: Page) { + await page.evaluate(() => { + const win = window as WhiteboardTestWindow + if (win.__whiteboardTestHooks) { + win.__whiteboardTestHooks.emittedSceneMessages = [] + } + }) +} + +async function getCapturedSceneMessages(page: Page): Promise { + return page.evaluate(() => { + const win = window as WhiteboardTestWindow + return win.__whiteboardTestHooks?.emittedSceneMessages || [] + }) +} + +async function installSceneReceiveSpy(page: Page) { + await waitForCollaborationReady(page) + await page.evaluate(() => { + const win = window as WhiteboardTestWindow + win.__whiteboardTestHooks = win.__whiteboardTestHooks || {} + const hooks = win.__whiteboardTestHooks + const receivedSceneMessages = hooks.receivedSceneMessages || [] + hooks.receivedSceneMessages = receivedSceneMessages + if (hooks.sceneReceiveSpyInstalled) { + return + } + + const store = hooks.collaborationStore + const socket = store?.getState?.().socket + if (!socket) { + throw new Error('Collaboration socket not available') + } + + const decodePayload = (payload: unknown) => { + if (typeof payload === 'string') { + return payload + } + if (payload instanceof ArrayBuffer) { + return new TextDecoder().decode(new Uint8Array(payload)) + } + if (ArrayBuffer.isView(payload)) { + return new TextDecoder().decode( + new Uint8Array(payload.buffer, payload.byteOffset, payload.byteLength), + ) + } + return '' + } + + socket.onAny((eventName: string, ...args: unknown[]) => { + if (eventName !== 'client-broadcast') { + return + } + + const decoded = decodePayload(args[0]) + if (!decoded) { + return + } + + try { + const parsed = JSON.parse(decoded) + if (parsed?.type === 'SCENE_INIT' || parsed?.type === 'SCENE_UPDATE') { + receivedSceneMessages.push({ + type: parsed.type, + syncAll: parsed.payload?.syncAll === true, + elementsCount: Array.isArray(parsed.payload?.elements) + ? parsed.payload.elements.length + : 0, + }) + } + } catch { + // Ignore frames that are not JSON scene payloads. + } + }) + + hooks.sceneReceiveSpyInstalled = true + }) +} + +async function clearReceivedSceneMessages(page: Page) { + await page.evaluate(() => { + const win = window as WhiteboardTestWindow + if (win.__whiteboardTestHooks) { + win.__whiteboardTestHooks.receivedSceneMessages = [] + } + }) +} + +async function getReceivedSceneMessages(page: Page): Promise { + return page.evaluate(() => { + const win = window as WhiteboardTestWindow + return win.__whiteboardTestHooks?.receivedSceneMessages || [] + }) +} + +async function resolveBoardFileId(page: Page, boardName: string): Promise { + await expect.poll(async () => resolveFileIdByDav(page, boardName), { + timeout: 30000, + intervals: [500], + }).not.toBeNull() + + const fileId = await resolveFileIdByDav(page, boardName) + if (!fileId) { + throw new Error(`Failed to resolve file id for board: ${boardName}`) + } + return fileId +} + test.beforeEach(async ({ page }) => { await openFilesApp(page) }) -test('whiteboard changes sync across sessions', async ({ page, browser, user }) => { +test('whiteboard changes sync across sessions', async ({ page, browser }) => { test.setTimeout(90000) const boardName = `Collab board ${Date.now()}` + await enableWhiteboardTestHooks(page) await createWhiteboard(page, { name: boardName }) await addTextElement(page, 'First session text') + const fileId = await resolveBoardFileId(page, boardName) + await installSceneReceiveSpy(page) + await clearReceivedSceneMessages(page) - let auth - try { - auth = await getBoardAuth(page) - } catch { - const saveResp = await page.waitForResponse((response) => response.request().method() === 'PUT' && response.url().includes('/apps/whiteboard/'), { timeout: 60000 }) - const authHeader = saveResp.request().headers()['authorization'] || '' - const apiPath = new URL(saveResp.url()).pathname.replace('/index.php/', '') - const fileId = Number(apiPath.split('/').pop()) - auth = { fileId, jwt: authHeader } - } + const pageB = await newLoggedInPage(page, browser) + await enableWhiteboardTestHooks(pageB) + await openWhiteboardById(pageB, fileId) - await expect.poll(async () => JSON.stringify(await fetchBoardContent(page, auth)), { - timeout: 20000, - interval: 500, - }).toContain('First session text') + await addTextElement(pageB, 'Second session text') + await page.waitForFunction(() => { + const win = window as WhiteboardTestWindow + const messages = win.__whiteboardTestHooks?.receivedSceneMessages || [] + return messages.some((message: ReceivedSceneMessage) => ( + message.type === 'SCENE_UPDATE' + && message.syncAll === false + && message.elementsCount === 1 + )) + }, { timeout: 30000 }) + + const receivedMessages = await getReceivedSceneMessages(page) + expect(receivedMessages.some((message) => ( + message.type === 'SCENE_UPDATE' + && message.syncAll === false + && message.elementsCount === 1 + ))).toBe(true) + + await pageB.close() +}) + +test('incremental scene sync sends only changed elements after targeted bootstrap', async ({ page, browser }) => { + test.setTimeout(120000) + const boardName = `Incremental collab board ${Date.now()}` + const bootstrapText = 'Incremental bootstrap text' + const deltaText = 'Incremental delta text' + + await enableWhiteboardTestHooks(page) + await createWhiteboard(page, { name: boardName }) + await addTextElement(page, bootstrapText) + const fileId = await resolveBoardFileId(page, boardName) + await installSceneEmitSpy(page) + await clearCapturedSceneMessages(page) + await installSceneReceiveSpy(page) + await clearReceivedSceneMessages(page) const pageB = await newLoggedInPage(page, browser) - await openWhiteboardFromFiles(pageB, boardName) - const fetchContent = async (targetPage) => JSON.stringify(await fetchBoardContent(targetPage, auth)) + await enableWhiteboardTestHooks(pageB) + await openWhiteboardById(pageB, fileId) - await expect.poll(async () => fetchContent(pageB), { timeout: 20000, interval: 500 }).toContain('First session text') + await page.waitForFunction(() => { + const win = window as WhiteboardTestWindow + const messages = win.__whiteboardTestHooks?.emittedSceneMessages || [] + return messages.some((message: CapturedSceneMessage) => ( + message.transport === 'direct' + && message.type === 'SCENE_INIT' + )) + }, { timeout: 30000 }) - await addTextElement(pageB, 'Second session text') + const bootstrapMessages = await getCapturedSceneMessages(page) + expect(bootstrapMessages.some((message) => ( + message.transport === 'direct' + && message.type === 'SCENE_INIT' + ))).toBe(true) + expect(bootstrapMessages.some((message) => ( + message.transport === 'room' + && message.type === 'SCENE_INIT' + ))).toBe(false) + + await installSceneEmitSpy(pageB) + await clearCapturedSceneMessages(pageB) + + await addTextElement(pageB, deltaText) + + await pageB.waitForFunction(() => { + const win = window as WhiteboardTestWindow + const messages = win.__whiteboardTestHooks?.emittedSceneMessages || [] + return messages.some((message: { type?: string, syncAll?: boolean, elementsCount?: number }) => ( + message.type === 'SCENE_UPDATE' + && message.syncAll === false + && message.elementsCount === 1 + )) + }, { timeout: 30000 }) + + const messages = await getCapturedSceneMessages(pageB) + const incrementalMessages = messages.filter((message) => message.type === 'SCENE_UPDATE') + const receivedMessages = await getReceivedSceneMessages(page) - await expect.poll(async () => fetchContent(page), { timeout: 30000, interval: 500 }).toContain('Second session text') + expect(incrementalMessages.length).toBeGreaterThan(0) + expect(incrementalMessages.some((message) => message.syncAll)).toBe(false) + expect(incrementalMessages.every((message) => message.transport === 'room')).toBe(true) + expect(messages.some((message) => message.type === 'SCENE_INIT')).toBe(false) + expect(incrementalMessages.every((message) => message.elementsCount === 1)).toBe(true) + expect(receivedMessages.some((message) => ( + message.type === 'SCENE_UPDATE' + && message.syncAll === false + && message.elementsCount === 1 + ))).toBe(true) await pageB.close() }) diff --git a/playwright/support/utils.ts b/playwright/support/utils.ts index c2847ed5..13595288 100644 --- a/playwright/support/utils.ts +++ b/playwright/support/utils.ts @@ -345,7 +345,7 @@ export async function openWhiteboardById( await waitForCanvas(page) } -async function resolveFileIdByDav(page: Page, name: string): Promise { +export async function resolveFileIdByDav(page: Page, name: string): Promise { const origin = new URL(await page.url()).origin const userResponse = await page.request.get(`${origin}/ocs/v2.php/cloud/user?format=json`, { headers: { 'OCS-APIREQUEST': 'true' }, diff --git a/src/App.tsx b/src/App.tsx index daa742c9..221f83ca 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -11,7 +11,7 @@ import { translate as t } from '@nextcloud/l10n' import { loadState } from '@nextcloud/initial-state' import { Excalidraw as ExcalidrawComponent, useHandleLibrary, Sidebar, isElementLink } from '@nextcloud/excalidraw' import '@excalidraw/excalidraw/index.css' -import type { LibraryItems } from '@nextcloud/excalidraw/dist/types/excalidraw/types' +import type { AppState, BinaryFiles, LibraryItems } from '@nextcloud/excalidraw/dist/types/excalidraw/types' import { useExcalidrawStore } from './stores/useExcalidrawStore' import { useWhiteboardConfigStore } from './stores/useWhiteboardConfigStore' import { useThemeHandling } from './hooks/useThemeHandling' @@ -439,12 +439,12 @@ export default function App({ return Array.from({ length: 40 }, () => Math.floor(Math.random() * 16).toString(16)).join('') }, [maxImageSizeBytes, maxImageSizeMb]) - const handleOnChange = useCallback(() => { + const handleOnChange = useCallback((elements: readonly ExcalidrawElement[], appState: AppState, files: BinaryFiles) => { if (isVersionPreview) { return } if (!excalidrawAPI || !normalizedFileId || isLoading) return - onChangeSync() + onChangeSync(elements, appState, files) }, [excalidrawAPI, normalizedFileId, isLoading, onChangeSync, isVersionPreview]) const canvasActions = useMemo(() => { diff --git a/src/hooks/useCollaboration.ts b/src/hooks/useCollaboration.ts index 054c6504..de2f1892 100644 --- a/src/hooks/useCollaboration.ts +++ b/src/hooks/useCollaboration.ts @@ -14,7 +14,7 @@ import type { Collaborator, ExcalidrawImageElement, } from '@excalidraw/excalidraw/types/types' -import { restoreElements } from '@nextcloud/excalidraw' +import { CaptureUpdateAction, restoreElements } from '@nextcloud/excalidraw' import { loadState } from '@nextcloud/initial-state' import { mergeElementsWithMetadata } from '../utils/mergeElementsWithMetadata' import { io } from 'socket.io-client' @@ -26,11 +26,12 @@ import { sanitizeAppStateForSync } from '../utils/sanitizeAppState' import { useShallow } from 'zustand/react/shallow' import { throttle, debounce } from 'lodash' import { db } from '../database/db' -import { computeElementVersionHash } from '../utils/syncSceneData' +import { buildBroadcastedElementVersions, computeElementVersionHash, mergeSceneElements } from '../utils/syncSceneData' import type { ClientToServerEvents, CollaborationSocket, ServerToClientEvents } from '../types/collaboration' enum BroadcastType { SceneInit = 'SCENE_INIT', // Incoming scene data from others + SceneUpdate = 'SCENE_UPDATE', // Incremental scene data from others SceneRestore = 'SCENE_RESTORE', // Force replace scene from authoritative source MouseLocation = 'MOUSE_LOCATION', // Incoming cursor data ImageAdd = 'IMAGE_ADD', // Incoming image data from others @@ -42,7 +43,7 @@ const CURSOR_UPDATE_DELAY = 50 export function useCollaboration() { const joinedRoomRef = useRef(null) - const pendingSceneUpdateRef = useRef(null) + const pendingSceneUpdatesRef = useRef([]) const pendingImageUpdatesRef = useRef>(new Map()) const pendingSceneReplaceRef = useRef<{ elements: ExcalidrawElement[] @@ -81,6 +82,9 @@ export function useCollaboration() { setStatus, setSocket, setDedicatedSyncer, + replaceBroadcastedElementVersions, + mergeBroadcastedElementVersions, + resetSceneSyncState, incrementAuthFailure, clearAuthError, resetStore, // Use resetStore for full cleanup @@ -90,6 +94,9 @@ export function useCollaboration() { setStatus: state.setStatus, setSocket: state.setSocket, setDedicatedSyncer: state.setDedicatedSyncer, + replaceBroadcastedElementVersions: state.replaceBroadcastedElementVersions, + mergeBroadcastedElementVersions: state.mergeBroadcastedElementVersions, + resetSceneSyncState: state.resetSceneSyncState, incrementAuthFailure: state.incrementAuthFailure, clearAuthError: state.clearAuthError, resetStore: state.resetStore, @@ -98,41 +105,58 @@ export function useCollaboration() { ) // --- Remote Update Handlers --- + const requestMissingImages = useCallback((remoteElements: readonly ExcalidrawElement[]) => { + if (!excalidrawAPI) return + + const currentFiles = excalidrawAPI.getFiles() + const currentSocket = useCollaborationStore.getState().socket + + if (!currentSocket?.connected || !fileId) { + return + } + + const missingImages = remoteElements + .filter(el => el.type === 'image' + && (el as ExcalidrawImageElement).fileId + && !currentFiles[(el as ExcalidrawImageElement).fileId]) + + missingImages.forEach(el => { + const imageId = (el as ExcalidrawImageElement).fileId + console.log(`[Collaboration] Requesting missing image: ${imageId}`) + currentSocket.emit('image-get', `${fileId}`, imageId) + }) + }, [excalidrawAPI, fileId]) + const reconcileAndApplyRemoteElements = useCallback( (remoteElements: readonly ExcalidrawElement[]) => { if (!excalidrawAPI) return try { - // Restore and reconcile elements const restoredRemoteElements = restoreElements(remoteElements, null) const localElements = excalidrawAPI.getSceneElementsIncludingDeleted() || [] const appState = excalidrawAPI.getAppState() const reconciledElements = mergeElementsWithMetadata(localElements, restoredRemoteElements, appState) - excalidrawAPI.updateScene({ elements: reconciledElements }) - - // Request any missing images - const currentFiles = excalidrawAPI.getFiles() - const currentSocket = useCollaborationStore.getState().socket - - if (currentSocket?.connected && fileId) { - // Find image elements with missing file data - const missingImages = restoredRemoteElements - .filter(el => el.type === 'image' - && (el as ExcalidrawImageElement).fileId - && !currentFiles[(el as ExcalidrawImageElement).fileId]) - - // Request each missing image - missingImages.forEach(el => { - const imageId = (el as ExcalidrawImageElement).fileId - console.log(`[Collaboration] Requesting missing image: ${imageId}`) - currentSocket.emit('image-get', `${fileId}`, imageId) - }) + const localSceneHash = computeElementVersionHash(localElements) + const reconciledHash = computeElementVersionHash(reconciledElements) + const remoteVersions = buildBroadcastedElementVersions(restoredRemoteElements) + + if (localSceneHash === reconciledHash) { + mergeBroadcastedElementVersions(remoteVersions) + requestMissingImages(restoredRemoteElements) + return } + + mergeBroadcastedElementVersions(remoteVersions) + excalidrawAPI.updateScene({ + elements: reconciledElements, + captureUpdate: CaptureUpdateAction.NEVER, + }) + requestMissingImages(restoredRemoteElements) } catch (error) { console.error('[Collaboration] Error reconciling remote elements:', error) } }, - [excalidrawAPI, fileId], + [excalidrawAPI, mergeBroadcastedElementVersions, requestMissingImages], ) const handleRemoteImageAdd = useCallback( @@ -158,7 +182,14 @@ export function useCollaboration() { const queueSceneUpdate = useCallback( (remoteElements: readonly ExcalidrawElement[]) => { if (!excalidrawAPI) { - pendingSceneUpdateRef.current = remoteElements + const restoredRemoteElements = restoreElements(remoteElements, null) + pendingSceneUpdatesRef.current = pendingSceneUpdatesRef.current.length === 0 + ? [restoredRemoteElements] + : [mergeSceneElements( + pendingSceneUpdatesRef.current[0], + restoredRemoteElements, + {} as AppState, + )] return } @@ -191,6 +222,7 @@ export function useCollaboration() { } try { + replaceBroadcastedElementVersions(buildBroadcastedElementVersions(payload.elements)) excalidrawAPI.resetScene() const currentAppState = excalidrawAPI.getAppState() @@ -204,6 +236,7 @@ export function useCollaboration() { excalidrawAPI.updateScene({ elements: payload.elements, appState: mergedAppState, + captureUpdate: CaptureUpdateAction.NEVER, }) const filesArray = Object.values(payload.files || {}).filter( @@ -217,7 +250,7 @@ export function useCollaboration() { console.error('[Collaboration] Error applying restored scene:', error) } }, - [excalidrawAPI], + [excalidrawAPI, replaceBroadcastedElementVersions], ) useEffect(() => { @@ -231,10 +264,11 @@ export function useCollaboration() { applySceneReplacement(payload) } - if (pendingSceneUpdateRef.current) { - const latestScene = pendingSceneUpdateRef.current - pendingSceneUpdateRef.current = null - reconcileAndApplyRemoteElements(latestScene) + if (pendingSceneUpdatesRef.current.length > 0) { + const [queuedScene] = pendingSceneUpdatesRef.current.splice(0) + if (queuedScene) { + reconcileAndApplyRemoteElements(queuedScene) + } } if (pendingImageUpdatesRef.current.size > 0) { @@ -247,10 +281,11 @@ export function useCollaboration() { }, [excalidrawAPI, handleRemoteImageAdd, reconcileAndApplyRemoteElements, applySceneReplacement]) useEffect(() => { - pendingSceneUpdateRef.current = null + pendingSceneUpdatesRef.current = [] pendingImageUpdatesRef.current.clear() pendingSceneReplaceRef.current = null - }, [fileId]) + resetSceneSyncState() + }, [fileId, resetSceneSyncState]) // --- Collaborator State Management --- const updateCollaboratorsState = useCallback( @@ -516,8 +551,6 @@ export function useCollaboration() { debouncedJoinRoom(currentSocket, roomIdStr) }, [fileId, debouncedJoinRoom]) // Dependencies read via store state - let lastElementsString = null - const handleClientBroadcast = useCallback( async (data: ArrayBuffer) => { try { @@ -549,7 +582,7 @@ export function useCollaboration() { const scrollToContent = payload.scrollToContent ?? true // Clear pending queue state since we have an authoritative snapshot - pendingSceneUpdateRef.current = null + pendingSceneUpdatesRef.current = [] pendingImageUpdatesRef.current.clear() if (excalidrawAPI) { @@ -591,16 +624,11 @@ export function useCollaboration() { break } case BroadcastType.SceneInit: + case BroadcastType.SceneUpdate: if (Array.isArray(decoded.payload?.elements)) { - const elementsString = JSON.stringify(decoded.payload.elements) - if (elementsString === lastElementsString) { - console.warn('[Collaboration] Received identical SceneInit payload, skipping update') - break - } queueSceneUpdate(decoded.payload.elements) - lastElementsString = JSON.stringify(decoded.payload.elements) } else { - console.warn('[Collaboration] Invalid SceneInit payload:', decoded.payload) + console.warn('[Collaboration] Invalid scene payload:', decoded.payload) } break case BroadcastType.MouseLocation: @@ -667,12 +695,23 @@ export function useCollaboration() { // If we are the syncer, broadcast all our images to the new user const { isDedicatedSyncer } = useCollaborationStore.getState() if (isDedicatedSyncer && excalidrawAPI) { - console.log(`[Collaboration] Broadcasting images to new user: ${data.userName}`) - - const files = excalidrawAPI.getFiles() const socket = useCollaborationStore.getState().socket - if (!socket || !socket.connected || !fileId) return + if (!socket || !socket.connected || !fileId || socket.id === data.socketId) return + + console.log(`[Collaboration] Broadcasting scene and images to new user: ${data.userName}`) + const sceneElements = excalidrawAPI.getSceneElementsIncludingDeleted() + + const sceneData = { + type: BroadcastType.SceneInit, + payload: { + elements: sceneElements, + }, + } + const sceneBuffer = new TextEncoder().encode(JSON.stringify(sceneData)) + socket.emit('server-direct-broadcast', `${fileId}`, data.socketId, sceneBuffer, []) + + const files = excalidrawAPI.getFiles() // Broadcast each image file Object.entries(files).forEach(([, file]) => { @@ -680,7 +719,7 @@ export function useCollaboration() { const fileData = { type: BroadcastType.ImageAdd, payload: { file } } const fileJson = JSON.stringify(fileData) const fileBuffer = new TextEncoder().encode(fileJson) - socket.emit('server-broadcast', `${fileId}`, fileBuffer, []) + socket.emit('server-direct-broadcast', `${fileId}`, data.socketId, fileBuffer, []) } }) } diff --git a/src/hooks/useComment.tsx b/src/hooks/useComment.tsx index 70190d30..259f88f1 100644 --- a/src/hooks/useComment.tsx +++ b/src/hooks/useComment.tsx @@ -8,12 +8,13 @@ import { createRoot } from 'react-dom/client' import { mdiCommentOutline, mdiAccount } from '@mdi/js' import { useExcalidrawStore } from '../stores/useExcalidrawStore' import { useShallow } from 'zustand/react/shallow' -import { viewportCoordsToSceneCoords, convertToExcalidrawElements } from '@nextcloud/excalidraw' +import { viewportCoordsToSceneCoords, convertToExcalidrawElements, newElementWith } from '@nextcloud/excalidraw' import { generateUrl } from '@nextcloud/router' import { getCurrentUser } from '@nextcloud/auth' import { CommentPopover } from '../components/CommentPopover' import { renderToolbarButton } from '../components/ToolbarButton' import { getRelativeTime } from '../utils/time' +import { updateElementCustomData } from '../utils/updateElementCustomData' import './useComment.scss' import { t } from '@nextcloud/l10n' @@ -329,10 +330,16 @@ export function useComment(props?: UseCommentProps) { const elements = excalidrawAPI.getSceneElementsIncludingDeleted() const updatedElements = elements.map((el: unknown) => { if (isCommentElement(el) && el.customData?.commentThread?.id === threadId) { - return { - ...el, - ...updater(el.customData.commentThread), + const patch = updater(el.customData.commentThread) + + if (patch.customData && typeof patch.customData === 'object') { + return updateElementCustomData(el, (customData) => ({ + ...customData, + ...(patch.customData as Record), + })) } + + return newElementWith(el, patch) } return el }) diff --git a/src/hooks/useSync.ts b/src/hooks/useSync.ts index 68265add..5f2ee98c 100644 --- a/src/hooks/useSync.ts +++ b/src/hooks/useSync.ts @@ -5,22 +5,27 @@ import { useCallback, useEffect, useMemo, useRef } from 'react' import { throttle } from 'lodash' +import { hashString } from '@nextcloud/excalidraw' +import type { ExcalidrawElement } from '@nextcloud/excalidraw/dist/types/excalidraw/element/types' +import type { AppState, BinaryFiles } from '@nextcloud/excalidraw/dist/types/excalidraw/types' +import { generateUrl } from '@nextcloud/router' +import { useShallow } from 'zustand/react/shallow' import { useWhiteboardConfigStore } from '../stores/useWhiteboardConfigStore' import { useSyncStore, logSyncResult } from '../stores/useSyncStore' import { useExcalidrawStore } from '../stores/useExcalidrawStore' import { useJWTStore } from '../stores/useJwtStore' import { useCollaborationStore } from '../stores/useCollaborationStore' -import { generateUrl } from '@nextcloud/router' -import { useShallow } from 'zustand/react/shallow' import logger from '../utils/logger' import { sanitizeAppStateForSync } from '../utils/sanitizeAppState' -import type { ExcalidrawElement } from '@excalidraw/excalidraw/types/element/types' -import type { BinaryFiles } from '@excalidraw/excalidraw/types/types' import type { CollaborationSocket } from '../types/collaboration' import type { WorkerInboundMessage } from '../types/protocol' +import { + buildBroadcastedElementVersions, + planIncrementalSceneSync, +} from '../utils/syncSceneData' enum SyncMessageType { - SceneInit = 'SCENE_INIT', + SceneUpdate = 'SCENE_UPDATE', ImageAdd = 'IMAGE_ADD', MouseLocation = 'MOUSE_LOCATION', ViewportUpdate = 'VIEWPORT_UPDATE', @@ -33,6 +38,12 @@ const SERVER_API_SYNC_DELAY = 10000 const WEBSOCKET_SYNC_DELAY = 500 const CURSOR_SYNC_DELAY = 50 +type SceneSnapshot = { + elements: readonly ExcalidrawElement[] + files: BinaryFiles + appState: Partial +} + export function useSync() { const { fileId, isReadOnly } = useWhiteboardConfigStore( useShallow(state => ({ @@ -75,7 +86,40 @@ export function useSync() { })), ) - // --- Worker Initialization --- + const latestSnapshotRef = useRef({ + elements: [], + files: {} as BinaryFiles, + appState: {}, + }) + const prevSyncedFilesRef = useRef>({}) + const isSyncerRef = useRef(isDedicatedSyncer) + + const captureSnapshot = useCallback(( + elements?: readonly ExcalidrawElement[], + appState?: AppState, + files?: BinaryFiles, + ): SceneSnapshot | null => { + if (!excalidrawAPI) { + return null + } + + const snapshotElements = elements ?? excalidrawAPI.getSceneElementsIncludingDeleted() + const snapshotAppState = sanitizeAppStateForSync(appState ?? excalidrawAPI.getAppState()) + const snapshotFiles = (files ?? excalidrawAPI.getFiles()) as BinaryFiles + + latestSnapshotRef.current = { + elements: snapshotElements, + files: snapshotFiles, + appState: snapshotAppState, + } + + return latestSnapshotRef.current + }, [excalidrawAPI]) + + const getLatestSnapshot = useCallback(() => { + return captureSnapshot() ?? latestSnapshotRef.current + }, [captureSnapshot]) + useEffect(() => { initializeWorker() return () => { @@ -83,42 +127,47 @@ export function useSync() { } }, [initializeWorker, terminateWorker]) - // Keep track of previously synced files to avoid resending unchanged files - const prevSyncedFilesRef = useRef>({}) - - // Reset prevSyncedFilesRef when fileId changes to prevent leakage across files useEffect(() => { prevSyncedFilesRef.current = {} - }, [fileId]) // Depends on fileId from the hook scope + useCollaborationStore.getState().resetSceneSyncState() + }, [fileId]) - // --- Sync Logic --- + useEffect(() => { + if (isDedicatedSyncer !== isSyncerRef.current) { + logger.debug('[Sync] SYNCER STATUS:', isDedicatedSyncer ? 'DESIGNATED AS SYNCER' : 'NOT SYNCER') + isSyncerRef.current = isDedicatedSyncer + } + }, [isDedicatedSyncer]) - // Saves current state to IndexedDB const doSyncToLocal = useCallback(async () => { if (!isWorkerReady || !worker || !fileId || !excalidrawAPI || isReadOnly) { return } try { - const elements = excalidrawAPI.getSceneElementsIncludingDeleted() as readonly ExcalidrawElement[] - const appState = excalidrawAPI.getAppState() - const files = excalidrawAPI.getFiles() as BinaryFiles - const filteredAppState = sanitizeAppStateForSync(appState) + const snapshot = getLatestSnapshot() + if (!snapshot) { + return + } - const message: WorkerInboundMessage = { type: 'SYNC_TO_LOCAL', fileId, elements, files, appState: filteredAppState } + const message: WorkerInboundMessage = { + type: 'SYNC_TO_LOCAL', + fileId, + elements: snapshot.elements, + files: snapshot.files, + appState: snapshot.appState, + } worker.postMessage(message) logSyncResult('local', { status: 'syncing' }) } catch (error) { logger.error('[Sync] Local sync failed:', error) logSyncResult('local', { status: 'error', error: error instanceof Error ? error.message : String(error) }) } - }, [isWorkerReady, worker, fileId, excalidrawAPI, isReadOnly]) + }, [isWorkerReady, worker, fileId, excalidrawAPI, isReadOnly, getLatestSnapshot]) - // Saves current state to the Nextcloud server API const doSyncToServerAPI = useCallback(async (forceSync = false) => { logger.debug('[Sync] doSyncToServerAPI called', { forceSync, isDedicatedSyncer, collabStatus }) - // Allow force sync for final save, otherwise check normal conditions if (!forceSync && (!isWorkerReady || !worker || !fileId || !excalidrawAPI || !isDedicatedSyncer || isReadOnly || collabStatus !== 'online')) { logger.debug('[Sync] Skipping server sync - normal conditions not met', { isWorkerReady, worker: !!worker, fileId, excalidrawAPI: !!excalidrawAPI, isDedicatedSyncer, isReadOnly, collabStatus, @@ -126,7 +175,6 @@ export function useSync() { return } - // For force sync, only check minimum requirements if (forceSync && (!isWorkerReady || !worker || !fileId || !excalidrawAPI || isReadOnly)) { logger.debug('[Sync] Skipping forced server sync - minimum requirements not met', { isWorkerReady, worker: !!worker, fileId, excalidrawAPI: !!excalidrawAPI, isReadOnly, @@ -135,7 +183,6 @@ export function useSync() { } logSyncResult('server', { status: 'syncing API' }) - logger.debug('[Sync] Sending SYNC_TO_SERVER message to worker') try { const jwt = await getJWT() @@ -144,16 +191,18 @@ export function useSync() { throw new Error('FileId changed during server sync preparation.') } - const elements = excalidrawAPI.getSceneElementsIncludingDeleted() as readonly ExcalidrawElement[] - const files = excalidrawAPI.getFiles() as BinaryFiles + const snapshot = getLatestSnapshot() + if (!snapshot) { + return + } const message: WorkerInboundMessage = { type: 'SYNC_TO_SERVER', fileId, url: generateUrl(`apps/whiteboard/${fileId}`), jwt, - elements, - files, + elements: snapshot.elements, + files: snapshot.files, } worker.postMessage(message) @@ -162,76 +211,90 @@ export function useSync() { logger.error('[Sync] Server API sync failed:', error) logSyncResult('server', { status: 'error API', error: error instanceof Error ? error.message : String(error) }) } - }, [isWorkerReady, worker, fileId, excalidrawAPI, isDedicatedSyncer, isReadOnly, collabStatus, getJWT]) - - // Simple hash function for file content - const hashFileContent = (content: string): string => { - if (!content) return '' - const len = content.length - const start = content.substring(0, 20) - const end = content.substring(Math.max(0, len - 20)) - return `${len}:${start}:${end}` - } - - // Syncs scene and files via WebSocket + }, [isWorkerReady, worker, fileId, excalidrawAPI, isDedicatedSyncer, isReadOnly, collabStatus, getJWT, getLatestSnapshot]) + const doSyncViaWebSocket = useCallback(async () => { - if (!fileId || !excalidrawAPI || !socket || collabStatus !== 'online' || isReadOnly) { + if (!fileId || !socket || collabStatus !== 'online' || isReadOnly) { return } try { - const elements = excalidrawAPI.getSceneElementsIncludingDeleted() as readonly ExcalidrawElement[] - const files = excalidrawAPI.getFiles() as BinaryFiles + const snapshot = getLatestSnapshot() + if (!snapshot) { + return + } - // 1. Send Scene - const sceneData = { type: SyncMessageType.SceneInit, payload: { elements } } - const sceneJson = JSON.stringify(sceneData) - const sceneBuffer = new TextEncoder().encode(sceneJson) - socket.emit(SyncMessageType.ServerBroadcast, `${fileId}`, sceneBuffer, []) + const { elements, files } = snapshot + const sceneSyncPlan = planIncrementalSceneSync({ + elements, + broadcastedElementVersions: useCollaborationStore.getState().broadcastedElementVersions, + lastSceneHash: useCollaborationStore.getState().lastSceneHash, + }) + let syncedElementsCount = 0 + + if (sceneSyncPlan.type === 'broadcast') { + const sceneData = { + type: SyncMessageType.SceneUpdate, + payload: { + elements: sceneSyncPlan.sceneElements, + }, + } + const sceneBuffer = new TextEncoder().encode(JSON.stringify(sceneData)) + socket.emit(SyncMessageType.ServerBroadcast, `${fileId}`, sceneBuffer, []) + + useCollaborationStore.getState().replaceBroadcastedElementVersions( + buildBroadcastedElementVersions(elements), + ) + useCollaborationStore.getState().setLastSceneHash(sceneSyncPlan.sceneHash) + syncedElementsCount = sceneSyncPlan.sceneElements.length + } else if (sceneSyncPlan.type === 'advance') { + useCollaborationStore.getState().replaceBroadcastedElementVersions( + sceneSyncPlan.broadcastedElementVersions, + ) + useCollaborationStore.getState().setLastSceneHash(sceneSyncPlan.sceneHash) + } - // 2. Send only new or changed files if (files && Object.keys(files).length > 0) { - const currentFileHashes: Record = {} + const currentFileHashes: Record = {} for (const fileIdKey in files) { const file = files[fileIdKey] if (!file?.dataURL) continue - const currentHash = hashFileContent(file.dataURL) + const currentHash = hashString(file.dataURL) currentFileHashes[fileIdKey] = currentHash if (prevSyncedFilesRef.current[fileIdKey] !== currentHash) { const fileData = { type: SyncMessageType.ImageAdd, payload: { file } } - const fileJson = JSON.stringify(fileData) - const fileBuffer = new TextEncoder().encode(fileJson) + const fileBuffer = new TextEncoder().encode(JSON.stringify(fileData)) socket.emit(SyncMessageType.ServerBroadcast, `${fileId}`, fileBuffer, []) } } prevSyncedFilesRef.current = currentFileHashes - logSyncResult('websocket', { status: 'sync success', elementsCount: elements.length }) } else { - logSyncResult('websocket', { status: 'sync success', elementsCount: elements.length }) prevSyncedFilesRef.current = {} } + + logSyncResult('websocket', { status: 'sync success', elementsCount: syncedElementsCount }) } catch (error) { logger.error('[Sync] WebSocket sync failed:', error) logSyncResult('websocket', { status: 'sync error', error: error instanceof Error ? error.message : String(error) }) } - }, [fileId, excalidrawAPI, socket, collabStatus, isReadOnly]) + }, [fileId, socket, collabStatus, isReadOnly, getLatestSnapshot]) const throttledSyncToLocal = useMemo(() => - // Use both leading and trailing edge executions to ensure changes are saved immediately and after delay throttle(doSyncToLocal, LOCAL_SYNC_DELAY, { leading: true, trailing: true }) , [doSyncToLocal]) const throttledSyncToServerAPI = useMemo(() => - // Use both leading and trailing edge executions for server sync throttle(doSyncToServerAPI, SERVER_API_SYNC_DELAY, { leading: true, trailing: true }) , [doSyncToServerAPI]) const throttledSyncViaWebSocket = useMemo(() => - // Use both leading and trailing edge executions for WebSocket sync - throttle(doSyncViaWebSocket, WEBSOCKET_SYNC_DELAY, { leading: true, trailing: true }) + throttle(() => { + doSyncViaWebSocket().catch((error) => { + logger.error('[Sync] Throttled WebSocket sync failed:', error) + }) + }, WEBSOCKET_SYNC_DELAY, { leading: true, trailing: true }) , [doSyncViaWebSocket]) - // --- Cursor Sync --- const doSyncCursors = useCallback( (payload: { pointer: { x: number; y: number; tool: 'pointer' | 'laser' } @@ -250,8 +313,7 @@ export function useSync() { selectedElementIds: excalidrawAPI.getAppState().selectedElementIds, }, } - const json = JSON.stringify(data) - const encodedBuffer = new TextEncoder().encode(json) + const encodedBuffer = new TextEncoder().encode(JSON.stringify(data)) socket.emit(SyncMessageType.ServerVolatileBroadcast, `${fileId}`, encodedBuffer) logSyncResult('cursor', { status: 'sync success' }) } catch (error) { @@ -262,7 +324,6 @@ export function useSync() { [fileId, excalidrawAPI, socket, collabStatus], ) - // --- Viewport Sync --- const lastBroadcastedViewportRef = useRef({ scrollX: 0, scrollY: 0, zoom: 1 }) const doSyncViewport = useCallback( @@ -274,14 +335,12 @@ export function useSync() { const { scrollX, scrollY, zoom } = appState const lastViewport = lastBroadcastedViewportRef.current - // Only broadcast if viewport has changed significantly if ( Math.abs(scrollX - lastViewport.scrollX) > 5 || Math.abs(scrollY - lastViewport.scrollY) > 5 || Math.abs(zoom.value - lastViewport.zoom) > 0.01 ) { try { - // Get current user ID for viewport tracking const { getJWT, parseJwt } = useJWTStore.getState() const jwt = await getJWT() const jwtPayload = jwt ? parseJwt(jwt) : null @@ -296,8 +355,7 @@ export function useSync() { zoom: zoom.value, }, } - const json = JSON.stringify(data) - const encodedBuffer = new TextEncoder().encode(json) + const encodedBuffer = new TextEncoder().encode(JSON.stringify(data)) socket.emit(SyncMessageType.ServerVolatileBroadcast, `${fileId}`, encodedBuffer) lastBroadcastedViewportRef.current = { scrollX, scrollY, zoom: zoom.value } @@ -317,32 +375,33 @@ export function useSync() { throttle(doSyncViewport, CURSOR_SYNC_DELAY, { leading: true, trailing: true }) , [doSyncViewport]) - // --- Event Handlers --- - const onChange = useCallback(() => { - // Update cached state immediately on every change - if (excalidrawAPI) { - const elements = excalidrawAPI.getSceneElementsIncludingDeleted() - const files = excalidrawAPI.getFiles() - cachedStateRef.current = { elements, files } + const onChange = useCallback(( + elements?: readonly ExcalidrawElement[], + appState?: AppState, + files?: BinaryFiles, + ) => { + const snapshot = captureSnapshot(elements, appState, files) + if (snapshot) { + latestSnapshotRef.current = snapshot } throttledSyncToLocal() throttledSyncToServerAPI() throttledSyncViaWebSocket() - // Sync viewport changes - if (excalidrawAPI) { - const appState = excalidrawAPI.getAppState() + if (appState) { throttledSyncViewport(appState) + } else if (excalidrawAPI) { + throttledSyncViewport(excalidrawAPI.getAppState()) } logger.debug('[Sync] Changes detected, triggered sync operations') - }, [throttledSyncToLocal, throttledSyncToServerAPI, throttledSyncViaWebSocket, throttledSyncViewport, excalidrawAPI]) + }, [captureSnapshot, throttledSyncToLocal, throttledSyncToServerAPI, throttledSyncViaWebSocket, throttledSyncViewport, excalidrawAPI]) const onPointerUpdate = useCallback( (payload: { - pointersMap: Map, - pointer: { x: number; y: number; tool: 'laser' | 'pointer' }, + pointersMap: Map + pointer: { x: number; y: number; tool: 'laser' | 'pointer' } button: 'down' | 'up' }) => { if (payload.pointersMap.size < 2) { @@ -352,30 +411,12 @@ export function useSync() { [throttledSyncCursors], ) - // Capture syncer state immediately to avoid closure issues - const isSyncerRef = useRef(isDedicatedSyncer) - useEffect(() => { - if (isDedicatedSyncer !== isSyncerRef.current) { - // eslint-disable-next-line no-console - console.log('[Sync] SYNCER STATUS:', isDedicatedSyncer ? 'DESIGNATED AS SYNCER' : 'NOT SYNCER') - isSyncerRef.current = isDedicatedSyncer - } - }, [isDedicatedSyncer]) - - // Cache the latest state for final sync - update on EVERY change - const cachedStateRef = useRef<{ elements: readonly ExcalidrawElement[]; files: BinaryFiles }>({ elements: [], files: {} as BinaryFiles }) - - // Direct sync when leaving - synchronous to ensure it completes const doFinalServerSync = useCallback(() => { if (!fileId || !isSyncerRef.current) { return } - // eslint-disable-next-line no-console - console.log('[Sync] Executing final sync on page leave') - try { - // Get JWT from store - it's stored in tokens[fileId] const jwtState = useJWTStore.getState() const jwt = jwtState.tokens[fileId] @@ -383,29 +424,20 @@ export function useSync() { return } - // Use CACHED state instead of trying to get it now (might be cleared already) - const { elements, files } = cachedStateRef.current - // eslint-disable-next-line no-console - console.log('[Sync] Using cached state with', elements.length, 'elements') - + const snapshot = latestSnapshotRef.current const url = generateUrl(`apps/whiteboard/${fileId}`) const data = JSON.stringify({ - data: { elements, files: files || {} }, + data: { elements: snapshot.elements, files: snapshot.files || {} }, }) - // Use synchronous XMLHttpRequest (works in beforeunload) const xhr = new XMLHttpRequest() - xhr.open('PUT', url, false) // false = synchronous + xhr.open('PUT', url, false) xhr.setRequestHeader('Content-Type', 'application/json') xhr.setRequestHeader('X-Requested-With', 'XMLHttpRequest') xhr.setRequestHeader('Authorization', `Bearer ${jwt}`) - xhr.send(data) - // eslint-disable-next-line no-console - console.log('[Sync] Final sync done, status:', xhr.status) } catch (error) { - // eslint-disable-next-line no-console console.error('[Sync] Final sync failed:', error) } }, [fileId]) @@ -413,23 +445,22 @@ export function useSync() { useEffect(() => { const handleBeforeUnload = () => { if (excalidrawAPI && !isReadOnly && isSyncerRef.current) { - // eslint-disable-next-line no-console - console.log('[Sync] Page unloading - syncing as dedicated syncer') - // Cancel any pending throttled trailing call FIRST throttledSyncToLocal.cancel() throttledSyncToServerAPI.cancel() - // Call the unthrottled versions directly - doSyncToLocal() + doSyncToLocal().catch((error) => { + logger.error('[Sync] Final local sync on unload failed:', error) + }) doFinalServerSync() } } - // Also handle visibility change as backup for mobile/tabs const handleVisibilityChange = () => { if (document.visibilityState === 'hidden' && isSyncerRef.current && excalidrawAPI && !isReadOnly) { throttledSyncToLocal.cancel() throttledSyncToServerAPI.cancel() - doSyncToLocal() + doSyncToLocal().catch((error) => { + logger.error('[Sync] Final local sync on visibility change failed:', error) + }) doFinalServerSync() } } @@ -437,22 +468,19 @@ export function useSync() { window.addEventListener('beforeunload', handleBeforeUnload) document.addEventListener('visibilitychange', handleVisibilityChange) - // Cleanup function for component unmount return () => { window.removeEventListener('beforeunload', handleBeforeUnload) document.removeEventListener('visibilitychange', handleVisibilityChange) - // If we're the dedicated syncer and unmounting, do a final sync if (isSyncerRef.current && excalidrawAPI && !isReadOnly) { - // Cancel pending throttled calls throttledSyncToLocal.cancel() throttledSyncToServerAPI.cancel() - // Do final syncs - doSyncToLocal() + doSyncToLocal().catch((error) => { + logger.error('[Sync] Final local sync on cleanup failed:', error) + }) doFinalServerSync() } - // Cancel all throttled functions to prevent them from running after unmount throttledSyncToLocal.cancel() throttledSyncToServerAPI.cancel() throttledSyncViaWebSocket.cancel() diff --git a/src/stores/useCollaborationStore.ts b/src/stores/useCollaborationStore.ts index e14e88db..c16fd417 100644 --- a/src/stores/useCollaborationStore.ts +++ b/src/stores/useCollaborationStore.ts @@ -18,37 +18,40 @@ interface AuthErrorState { message: string | null consecutiveFailures: number lastFailureTime: number | null - isPersistent: boolean // True when we've detected a persistent auth issue (likely JWT secret mismatch) + isPersistent: boolean } interface CollaborationStore { status: CollaborationConnectionStatus socket: CollaborationSocket | null - isDedicatedSyncer: boolean // Is this client responsible for syncing to server/broadcasting? + isDedicatedSyncer: boolean authError: AuthErrorState - followedUserId: string | null // User ID being followed for viewport synchronization - isInRoom: boolean // Whether the socket has joined the current room + followedUserId: string | null + isInRoom: boolean + lastSceneHash: number | null + broadcastedElementVersions: Record - // Presentation state - presenterId: string | null // User ID of current presenter - isPresentationMode: boolean // Whether presentation mode is active in the room - isPresenting: boolean // Whether current user is presenting - presentationStartTime: number | null // When presentation started - autoFollowPresenter: boolean // Whether to automatically follow presenter (can be disabled by user) + presenterId: string | null + isPresentationMode: boolean + isPresenting: boolean + presentationStartTime: number | null + autoFollowPresenter: boolean votings: Voting[] - // Actions setStatus: (status: CollaborationConnectionStatus) => void setSocket: (socket: CollaborationSocket | null) => void setDedicatedSyncer: (isSyncer: boolean) => void setIsInRoom: (inRoom: boolean) => void + setLastSceneHash: (hash: number | null) => void + replaceBroadcastedElementVersions: (versions: Record) => void + mergeBroadcastedElementVersions: (versions: Record) => void + resetSceneSyncState: () => void setAuthError: (error: Partial) => void incrementAuthFailure: (errorType: AuthErrorType, message: string) => void clearAuthError: () => void resetStore: () => void - // Presentation actions setPresentationState: (state: { presenterId?: string | null isPresentationMode?: boolean @@ -57,7 +60,6 @@ interface CollaborationStore { }) => void setAutoFollowPresenter: (autoFollow: boolean) => void - // Voting actions addVoting: (voting: Voting) => void updateVoting: (voting: Voting) => void setVotings: (votings: Voting[]) => void @@ -71,15 +73,35 @@ const initialAuthErrorState: AuthErrorState = { isPersistent: false, } -const initialState: Omit = { +const initialState: Omit< +CollaborationStore, +| 'setStatus' +| 'setSocket' +| 'setDedicatedSyncer' +| 'setIsInRoom' +| 'setLastSceneHash' +| 'replaceBroadcastedElementVersions' +| 'mergeBroadcastedElementVersions' +| 'resetSceneSyncState' +| 'setAuthError' +| 'incrementAuthFailure' +| 'clearAuthError' +| 'resetStore' +| 'setPresentationState' +| 'setAutoFollowPresenter' +| 'addVoting' +| 'updateVoting' +| 'setVotings' +> = { status: 'offline', socket: null, isDedicatedSyncer: false, authError: initialAuthErrorState, followedUserId: null, isInRoom: false, + lastSceneHash: null, + broadcastedElementVersions: {}, - // Presentation initial state presenterId: null, isPresentationMode: false, isPresenting: false, @@ -89,9 +111,8 @@ const initialState: Omit()((set) => ({ ...initialState, @@ -100,6 +121,20 @@ export const useCollaborationStore = create()((set) => ({ setSocket: (socket) => set({ socket }), setDedicatedSyncer: (isSyncer) => set({ isDedicatedSyncer: isSyncer }), setIsInRoom: (inRoom) => set({ isInRoom: inRoom }), + setLastSceneHash: (hash) => set({ lastSceneHash: hash }), + replaceBroadcastedElementVersions: (versions) => set({ broadcastedElementVersions: versions }), + mergeBroadcastedElementVersions: (versions) => set((state) => ({ + broadcastedElementVersions: Object.entries(versions).reduce>((nextVersions, [id, version]) => { + nextVersions[id] = nextVersions[id] === undefined + ? version + : Math.max(nextVersions[id], version) + return nextVersions + }, { ...state.broadcastedElementVersions }), + })), + resetSceneSyncState: () => set({ + lastSceneHash: null, + broadcastedElementVersions: {}, + }), setAuthError: (error) => set((state) => ({ authError: { ...state.authError, ...error }, @@ -108,8 +143,6 @@ export const useCollaborationStore = create()((set) => ({ incrementAuthFailure: (errorType, message) => set((state) => { const now = Date.now() const newFailureCount = state.authError.consecutiveFailures + 1 - - // Determine if this is a persistent issue (likely JWT secret mismatch) const isPersistent = newFailureCount >= MAX_AUTH_FAILURES && (state.authError.lastFailureTime === null || now - state.authError.lastFailureTime < PERSISTENT_FAILURE_THRESHOLD) @@ -129,7 +162,6 @@ export const useCollaborationStore = create()((set) => ({ resetStore: () => set(initialState), - // Presentation actions setPresentationState: (state) => set((currentState) => ({ ...currentState, ...state, @@ -137,7 +169,6 @@ export const useCollaborationStore = create()((set) => ({ setAutoFollowPresenter: (autoFollow) => set({ autoFollowPresenter: autoFollow }), - // Voting actions addVoting: (voting) => set((state) => ({ votings: [...state.votings, voting], })), diff --git a/src/stores/useExcalidrawStore.ts b/src/stores/useExcalidrawStore.ts index ebf44394..40e773e1 100644 --- a/src/stores/useExcalidrawStore.ts +++ b/src/stores/useExcalidrawStore.ts @@ -14,11 +14,41 @@ interface ExcalidrawStore { scrollToContent: () => void } +type WhiteboardTestHooks = { + excalidrawStore?: { + getState?: () => { + excalidrawAPI: ExcalidrawImperativeAPI | null + } + } +} + +declare global { + interface Window { + __whiteboardTest?: boolean + __whiteboardTestHooks?: WhiteboardTestHooks & Record + } +} + +const attachTestHooks = () => { + if (typeof window === 'undefined' || !window.__whiteboardTest) { + return + } + + window.__whiteboardTestHooks = window.__whiteboardTestHooks || {} + window.__whiteboardTestHooks.excalidrawStore = useExcalidrawStore +} + export const useExcalidrawStore = create((set, get) => ({ excalidrawAPI: null, - setExcalidrawAPI: (api: ExcalidrawImperativeAPI) => set({ excalidrawAPI: api }), - resetExcalidrawAPI: () => set({ excalidrawAPI: null }), + setExcalidrawAPI: (api: ExcalidrawImperativeAPI) => { + set({ excalidrawAPI: api }) + attachTestHooks() + }, + resetExcalidrawAPI: () => { + set({ excalidrawAPI: null }) + attachTestHooks() + }, scrollToContent: () => { const { excalidrawAPI } = get() if (!excalidrawAPI) return @@ -31,3 +61,5 @@ export const useExcalidrawStore = create((set, get) => ({ }) }, })) + +attachTestHooks() diff --git a/src/types/collaboration.ts b/src/types/collaboration.ts index 335c3380..f76ac663 100644 --- a/src/types/collaboration.ts +++ b/src/types/collaboration.ts @@ -65,6 +65,7 @@ export interface ServerToClientEvents { export interface ClientToServerEvents { 'join-room': (roomId: string) => void 'server-broadcast': (roomId: string, payload: ArrayBuffer | Uint8Array, iv: ArrayBuffer | number[] | []) => void + 'server-direct-broadcast': (roomId: string, targetSocketId: string, payload: ArrayBuffer | Uint8Array, iv: ArrayBuffer | number[] | []) => void 'server-volatile-broadcast': (roomId: string, payload: Uint8Array) => void 'image-get': (roomId: string, id: string) => void 'request-presenter-viewport': (payload: { fileId: string }) => void diff --git a/src/utils/mergeElementsWithMetadata.ts b/src/utils/mergeElementsWithMetadata.ts index 06e6a8da..a572d4e1 100644 --- a/src/utils/mergeElementsWithMetadata.ts +++ b/src/utils/mergeElementsWithMetadata.ts @@ -43,13 +43,13 @@ export function mergeElementsWithMetadata( const whiteboardElement = element as WhiteboardElement const remoteElement = remoteElementsMap.get(element.id) const localElement = localElementsMap.get(element.id) + const customData = { ...(whiteboardElement.customData || {}) } + let hasCustomDataChanges = false // If remote element has creator info, preserve it if (remoteElement?.customData?.creator) { - if (!whiteboardElement.customData) { - whiteboardElement.customData = {} - } - whiteboardElement.customData.creator = remoteElement.customData.creator + customData.creator = remoteElement.customData.creator + hasCustomDataChanges = true } // If remote element has lastModifiedBy info, check if it's newer @@ -58,43 +58,45 @@ export function mergeElementsWithMetadata( const localModTime = localElement?.customData?.lastModifiedBy?.createdAt || 0 if (remoteModTime > localModTime) { - if (!whiteboardElement.customData) { - whiteboardElement.customData = {} - } - whiteboardElement.customData.lastModifiedBy = remoteElement.customData.lastModifiedBy + customData.lastModifiedBy = remoteElement.customData.lastModifiedBy + hasCustomDataChanges = true } } // If local element had creator info but remote doesn't, preserve local if (localElement?.customData?.creator && !whiteboardElement.customData?.creator) { - if (!whiteboardElement.customData) { - whiteboardElement.customData = {} - } - whiteboardElement.customData.creator = localElement.customData.creator + customData.creator = localElement.customData.creator + hasCustomDataChanges = true } // Preserve table-specific custom data from whichever version won reconciliation // This ensures tableHtml, isTable, and tableLock are not lost const sourceElement = remoteElement || localElement if (sourceElement?.customData) { - if (!whiteboardElement.customData) { - whiteboardElement.customData = {} - } - // Preserve table metadata if (sourceElement.customData.isTable !== undefined) { - whiteboardElement.customData.isTable = sourceElement.customData.isTable + customData.isTable = sourceElement.customData.isTable + hasCustomDataChanges = true } if (sourceElement.customData.tableHtml !== undefined) { - whiteboardElement.customData.tableHtml = sourceElement.customData.tableHtml + customData.tableHtml = sourceElement.customData.tableHtml + hasCustomDataChanges = true } // Preserve or clear lock status from the source element if ('tableLock' in sourceElement.customData) { - whiteboardElement.customData.tableLock = sourceElement.customData.tableLock + customData.tableLock = sourceElement.customData.tableLock + hasCustomDataChanges = true } } - return whiteboardElement + if (!hasCustomDataChanges) { + return whiteboardElement + } + + return { + ...whiteboardElement, + customData, + } }) return finalElements diff --git a/src/utils/syncSceneData.ts b/src/utils/syncSceneData.ts index 505fbabd..bcaeea0b 100644 --- a/src/utils/syncSceneData.ts +++ b/src/utils/syncSceneData.ts @@ -3,8 +3,9 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ -import type { ExcalidrawElement } from '@excalidraw/excalidraw/types/element/types' -import type { AppState } from '@excalidraw/excalidraw/types/types' +import type { ExcalidrawElement } from '@nextcloud/excalidraw/dist/types/excalidraw/element/types' +import type { AppState } from '@nextcloud/excalidraw/dist/types/excalidraw/types' +import { hashElementsVersion } from '@nextcloud/excalidraw' import { isObject } from 'lodash' /** @@ -14,34 +15,91 @@ import { isObject } from 'lodash' export const computeElementVersionHash = ( elements: readonly ExcalidrawElement[], ): number => { - let hash = 5381 // djb2 starting point + return hashElementsVersion(elements) +} - // Special case: empty elements array should have a unique hash - // This ensures that when all elements are deleted, the hash changes - if (elements.length === 0) { - return 1 // Special hash for empty array - } +export const buildBroadcastedElementVersions = ( + elements: readonly ExcalidrawElement[], +): Record => { + return elements.reduce>((versions, element) => { + versions[element.id] = element.version + return versions + }, {}) +} - // Count deleted elements to ensure hash changes when elements are deleted - let deletedCount = 0 +export const mergeBroadcastedElementVersions = ( + currentVersions: Record, + elements: readonly ExcalidrawElement[], +): Record => { + const nextVersions = { ...currentVersions } - for (const el of elements) { - // Combine version, nonce, and deletion status into the hash - // Using prime numbers helps distribute values - hash = (hash * 33) ^ (el.version || 0) - hash = (hash * 33) ^ (el.versionNonce || 0) + elements.forEach((element) => { + const currentVersion = nextVersions[element.id] + nextVersions[element.id] = currentVersion === undefined + ? element.version + : Math.max(currentVersion, element.version) + }) - // Track deletion status - if (el.isDeleted) { - deletedCount++ - hash = (hash * 33) ^ 1 // Include isDeleted status - } + return nextVersions +} + +export const getIncrementalSceneElements = ( + elements: readonly ExcalidrawElement[], + broadcastedElementVersions: Record, +): readonly ExcalidrawElement[] => { + return elements.filter((element) => broadcastedElementVersions[element.id] !== element.version) +} + +type SceneSyncPlanNoop = { + type: 'noop' +} + +type SceneSyncPlanAdvance = { + type: 'advance' + sceneHash: number + broadcastedElementVersions: Record +} + +type SceneSyncPlanBroadcast = { + type: 'broadcast' + sceneHash: number + sceneElements: readonly ExcalidrawElement[] + broadcastedElementVersions: Record +} + +export type IncrementalSceneSyncPlan = SceneSyncPlanNoop | SceneSyncPlanAdvance | SceneSyncPlanBroadcast + +export const planIncrementalSceneSync = ({ + elements, + broadcastedElementVersions, + lastSceneHash, +}: { + elements: readonly ExcalidrawElement[] + broadcastedElementVersions: Record + lastSceneHash: number | null +}): IncrementalSceneSyncPlan => { + const sceneHash = computeElementVersionHash(elements) + + if (lastSceneHash === sceneHash) { + return { type: 'noop' } } - // Include deleted count in the hash to ensure it changes when elements are deleted - hash = (hash * 33) ^ deletedCount + const incrementalElements = getIncrementalSceneElements(elements, broadcastedElementVersions) - return hash >>> 0 // Ensure positive integer + if (incrementalElements.length === 0) { + return { + type: 'advance', + sceneHash, + broadcastedElementVersions: buildBroadcastedElementVersions(elements), + } + } + + return { + type: 'broadcast', + sceneHash, + sceneElements: incrementalElements, + broadcastedElementVersions: buildBroadcastedElementVersions(incrementalElements), + } } /** diff --git a/src/utils/tableLocking.ts b/src/utils/tableLocking.ts index 9dd02de4..7de1980d 100644 --- a/src/utils/tableLocking.ts +++ b/src/utils/tableLocking.ts @@ -7,6 +7,7 @@ import { showError } from '@nextcloud/dialogs' import { t } from '@nextcloud/l10n' import type { ExcalidrawImperativeAPI } from '@nextcloud/excalidraw/dist/types/excalidraw/types' import type { ExcalidrawImageElement } from '@nextcloud/excalidraw/dist/types/excalidraw/element/types' +import { updateElementCustomData } from './updateElementCustomData' const LOCK_TIMEOUT_MS = 5 * 60 * 1000 // 5 minutes @@ -91,14 +92,10 @@ export function setLockOnElement( if (idx === -1) return // Update the element with the new lock state - elements[idx] = { - ...elements[idx], - customData: { - ...elements[idx].customData, - // Set tableLock to the provided value, or explicitly undefined to clear - ...(lock ? { tableLock: lock } : { tableLock: undefined }), - }, - } + elements[idx] = updateElementCustomData(elements[idx], (customData) => ({ + ...customData, + ...(lock ? { tableLock: lock } : { tableLock: undefined }), + })) // Trigger onChange which syncs to other users excalidrawAPI.updateScene({ elements }) } diff --git a/src/utils/updateElementCustomData.ts b/src/utils/updateElementCustomData.ts new file mode 100644 index 00000000..2c60e405 --- /dev/null +++ b/src/utils/updateElementCustomData.ts @@ -0,0 +1,20 @@ +/** + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import { newElementWith } from '@nextcloud/excalidraw' +import type { ExcalidrawElement } from '@nextcloud/excalidraw/dist/types/excalidraw/element/types' + +export function updateElementCustomData( + element: T, + updater: (currentCustomData: Record) => Record, +): T { + const currentCustomData = (element.customData && typeof element.customData === 'object') + ? element.customData as Record + : {} + + return newElementWith(element, { + customData: updater(currentCustomData), + } as Partial) as T +} diff --git a/tests/integration/multinode-redis.spec.mjs b/tests/integration/multinode-redis.spec.mjs index ada882aa..7eea409b 100644 --- a/tests/integration/multinode-redis.spec.mjs +++ b/tests/integration/multinode-redis.spec.mjs @@ -417,6 +417,54 @@ describe('Multi node websocket cluster with redis streams', () => { }) }) + it('delivers direct scene broadcasts across nodes only to the targeted socket', async () => { + const roomID = 'room-direct-scene' + const senderToken = buildToken(roomID, { id: 'sender', name: 'Sender', displayName: 'Sender' }) + const targetToken = buildToken(roomID, { id: 'target', name: 'Target', displayName: 'Target' }) + const observerToken = buildToken(roomID, { id: 'observer', name: 'Observer', displayName: 'Observer' }) + + const senderSocket = createSocket(serverAUrl, senderToken) + await waitFor(senderSocket, 'connect') + senderSocket.emit('join-room', roomID) + await waitFor(senderSocket, 'sync-designate') + + const targetSocket = createSocket(serverBUrl, targetToken) + await waitFor(targetSocket, 'connect') + targetSocket.emit('join-room', roomID) + await waitFor(targetSocket, 'sync-designate') + + const observerSocket = createSocket(serverBUrl, observerToken) + await waitFor(observerSocket, 'connect') + observerSocket.emit('join-room', roomID) + await waitFor(observerSocket, 'sync-designate') + + const payload = new TextEncoder().encode(JSON.stringify({ + type: 'SCENE_INIT', + payload: { + elements: [{ id: 'shape-1' }], + }, + })) + const targetMessagePromise = waitFor(targetSocket, 'client-broadcast') + const observerMessages = [] + observerSocket.on('client-broadcast', (...args) => { + observerMessages.push(args) + }) + + senderSocket.emit('server-direct-broadcast', roomID, targetSocket.id, payload, []) + + const receivedPayload = await Promise.race([ + targetMessagePromise, + new Promise((_resolve, reject) => + setTimeout(() => reject(new Error('Timeout waiting for targeted client-broadcast event')), 2000), + ), + ]) + const decodedPayload = JSON.parse(new TextDecoder().decode(receivedPayload)) + + expect(decodedPayload.type).toBe('SCENE_INIT') + await new Promise(resolve => setTimeout(resolve, 200)) + expect(observerMessages).toHaveLength(0) + }) + it('broadcasts a presentation stop when the presenter node shuts down', async () => { const roomID = 'room-presenter-shutdown' const presenterToken = buildToken(roomID, { id: 'shutdown-presenter', name: 'Presenter', displayName: 'Presenter' }) diff --git a/tests/integration/socket.spec.mjs b/tests/integration/socket.spec.mjs index 963b7238..125cb9ec 100644 --- a/tests/integration/socket.spec.mjs +++ b/tests/integration/socket.spec.mjs @@ -204,4 +204,155 @@ describe('Socket handling', () => { newSocket.disconnect() }) + it('clears the old syncer when the room empties', async () => { + const roomID = 456 + + const bobSocket = io(Config.NEXTCLOUD_URL, { + auth: { + token: jwt.sign( + { + roomID, + user: { id: 'bob-user', name: 'Bob' }, + }, + Config.JWT_SECRET_KEY, + ), + }, + }) + + await waitFor(bobSocket, 'connect') + const bobDesignationPromise = waitFor(bobSocket, 'sync-designate') + bobSocket.emit('join-room', roomID) + const bobDesignation = await bobDesignationPromise + expect(bobDesignation.isSyncer).toBe(true) + + bobSocket.disconnect() + await new Promise(resolve => setTimeout(resolve, 100)) + + const adminSocket = io(Config.NEXTCLOUD_URL, { + auth: { + token: jwt.sign( + { + roomID, + user: { id: 'admin-user', name: 'Admin' }, + }, + Config.JWT_SECRET_KEY, + ), + }, + }) + + await waitFor(adminSocket, 'connect') + const adminDesignationPromise = waitFor(adminSocket, 'sync-designate') + adminSocket.emit('join-room', roomID) + const adminDesignation = await adminDesignationPromise + expect(adminDesignation.isSyncer).toBe(true) + + const bobReconnectSocket = io(Config.NEXTCLOUD_URL, { + auth: { + token: jwt.sign( + { + roomID, + user: { id: 'bob-user', name: 'Bob' }, + }, + Config.JWT_SECRET_KEY, + ), + }, + }) + + await waitFor(bobReconnectSocket, 'connect') + const bobReconnectDesignationPromise = waitFor(bobReconnectSocket, 'sync-designate') + bobReconnectSocket.emit('join-room', roomID) + const bobReconnectDesignation = await bobReconnectDesignationPromise + expect(bobReconnectDesignation.isSyncer).toBe(false) + + bobReconnectSocket.disconnect() + adminSocket.disconnect() + }) + + it('direct scene broadcasts reach only the targeted socket', async () => { + const roomID = 789 + const senderSocket = io(Config.NEXTCLOUD_URL, { + auth: { + token: jwt.sign( + { + roomID, + user: { id: 'sender-user', name: 'Sender' }, + }, + Config.JWT_SECRET_KEY, + ), + }, + }) + const targetSocket = io(Config.NEXTCLOUD_URL, { + auth: { + token: jwt.sign( + { + roomID, + user: { id: 'target-user', name: 'Target' }, + }, + Config.JWT_SECRET_KEY, + ), + }, + }) + const observerSocket = io(Config.NEXTCLOUD_URL, { + auth: { + token: jwt.sign( + { + roomID, + user: { id: 'observer-user', name: 'Observer' }, + }, + Config.JWT_SECRET_KEY, + ), + }, + }) + + await Promise.all([ + waitFor(senderSocket, 'connect'), + waitFor(targetSocket, 'connect'), + waitFor(observerSocket, 'connect'), + ]) + + const senderDesignationPromise = waitFor(senderSocket, 'sync-designate') + const targetDesignationPromise = waitFor(targetSocket, 'sync-designate') + const observerDesignationPromise = waitFor(observerSocket, 'sync-designate') + + senderSocket.emit('join-room', roomID) + targetSocket.emit('join-room', roomID) + observerSocket.emit('join-room', roomID) + + await Promise.all([ + senderDesignationPromise, + targetDesignationPromise, + observerDesignationPromise, + ]) + + const payload = new TextEncoder().encode(JSON.stringify({ + type: 'SCENE_INIT', + payload: { + elements: [{ id: 'shape-1' }], + }, + })) + const targetMessagePromise = waitFor(targetSocket, 'client-broadcast') + const observerMessages = [] + observerSocket.on('client-broadcast', (...args) => { + observerMessages.push(args) + }) + + senderSocket.emit('server-direct-broadcast', `${roomID}`, targetSocket.id, payload, []) + + const receivedPayload = await Promise.race([ + targetMessagePromise, + new Promise((_resolve, reject) => + setTimeout(() => reject(new Error('Timeout waiting for targeted client-broadcast event')), 2000), + ), + ]) + const decodedPayload = new TextDecoder().decode(receivedPayload) + + expect(decodedPayload).toContain('"type":"SCENE_INIT"') + await new Promise(resolve => setTimeout(resolve, 200)) + expect(observerMessages).toHaveLength(0) + + senderSocket.disconnect() + targetSocket.disconnect() + observerSocket.disconnect() + }) + }) diff --git a/tests/integration/syncSceneData.spec.mjs b/tests/integration/syncSceneData.spec.mjs new file mode 100644 index 00000000..cadd33eb --- /dev/null +++ b/tests/integration/syncSceneData.spec.mjs @@ -0,0 +1,144 @@ +import { describe, expect, it, vi } from 'vitest' + +vi.mock('@nextcloud/excalidraw', () => ({ + hashElementsVersion: vi.fn((elements = []) => elements.reduce( + (hash, element, index) => hash + ((element.versionNonce ?? element.version ?? 0) * (index + 1)), + 0, + )), +})) + +const { + buildBroadcastedElementVersions, + getIncrementalSceneElements, + mergeBroadcastedElementVersions, + planIncrementalSceneSync, +} = await import('../../src/utils/syncSceneData.ts') + +function makeElement(id, version = 1, versionNonce = version * 100) { + return { + id, + version, + versionNonce, + } +} + +describe('syncSceneData incremental selection', () => { + it('selects only the changed element from a large scene', () => { + const originalElements = Array.from({ length: 45 }, (_value, index) => + makeElement(`element-${index + 1}`), + ) + const broadcastedVersions = buildBroadcastedElementVersions(originalElements) + const updatedElements = originalElements.map((element, index) => + index === 17 + ? makeElement(element.id, element.version + 1) + : element, + ) + + const incrementalElements = getIncrementalSceneElements( + updatedElements, + broadcastedVersions, + ) + + expect(incrementalElements).toHaveLength(1) + expect(incrementalElements[0]).toMatchObject({ + id: 'element-18', + version: 2, + }) + }) + + it('selects newly added elements without resending the full scene', () => { + const originalElements = Array.from({ length: 45 }, (_value, index) => + makeElement(`element-${index + 1}`), + ) + const broadcastedVersions = buildBroadcastedElementVersions(originalElements) + const updatedElements = [ + ...originalElements, + makeElement('element-46'), + ] + + const incrementalElements = getIncrementalSceneElements( + updatedElements, + broadcastedVersions, + ) + + expect(incrementalElements).toHaveLength(1) + expect(incrementalElements[0]).toMatchObject({ + id: 'element-46', + version: 1, + }) + }) + + it('merges remote versions without downgrading already synced elements', () => { + const syncedVersions = { + E1: 5, + E2: 3, + } + + const mergedVersions = mergeBroadcastedElementVersions( + syncedVersions, + [ + makeElement('E1', 4), + makeElement('E2', 6), + ], + ) + + expect(mergedVersions).toEqual({ + E1: 5, + E2: 6, + }) + }) + + it('skips scene broadcast and advances sync markers after remote-only changes', () => { + const elements = [ + makeElement('E1', 4, 444), + makeElement('E2', 5, 555), + ] + const plan = planIncrementalSceneSync({ + elements, + broadcastedElementVersions: { + E1: 4, + E2: 5, + }, + lastSceneHash: 123, + }) + + expect(plan).toEqual({ + type: 'advance', + sceneHash: 1554, + broadcastedElementVersions: { + E1: 4, + E2: 5, + }, + }) + }) + + it('broadcasts only unsent local edits after remote versions are merged', () => { + const lastSentElements = [ + makeElement('E1', 3, 101), + makeElement('E2', 5, 202), + ] + const reconciledElements = [ + makeElement('E1', 4, 303), + makeElement('E2', 6, 404), + ] + const syncedVersions = mergeBroadcastedElementVersions( + buildBroadcastedElementVersions(lastSentElements), + [makeElement('E1', 4, 303)], + ) + + const plan = planIncrementalSceneSync({ + elements: reconciledElements, + broadcastedElementVersions: syncedVersions, + lastSceneHash: 505, + }) + + expect(plan).toEqual({ + type: 'broadcast', + sceneHash: 1111, + sceneElements: [makeElement('E2', 6, 404)], + broadcastedElementVersions: { + E2: 6, + }, + }) + }) +}) diff --git a/tests/integration/tableLocking.spec.mjs b/tests/integration/tableLocking.spec.mjs index 576c2994..5a2f17f1 100644 --- a/tests/integration/tableLocking.spec.mjs +++ b/tests/integration/tableLocking.spec.mjs @@ -4,9 +4,33 @@ */ import { describe, it, expect, vi, beforeEach } from 'vitest' -import { isLockExpired, setLockOnElement, tryAcquireLock, releaseLock } from '../../src/utils/tableLocking.ts' -import * as auth from '@nextcloud/auth' -import * as dialogs from '@nextcloud/dialogs' + +vi.mock('@nextcloud/excalidraw', () => ({ + newElementWith: vi.fn((element, updates = {}, force = false) => { + let didChange = false + for (const [key, value] of Object.entries(updates)) { + if (typeof value === 'undefined') { + continue + } + if (element[key] === value && (typeof value !== 'object' || value === null)) { + continue + } + didChange = true + } + + if (!didChange && !force) { + return element + } + + return { + ...element, + ...updates, + updated: 1, + version: (element.version ?? 0) + 1, + versionNonce: 1, + } + }), +})) // Mock the Nextcloud modules vi.mock('@nextcloud/auth', () => ({ @@ -20,6 +44,10 @@ vi.mock('@nextcloud/dialogs', () => ({ showError: vi.fn(), })) +const { isLockExpired, setLockOnElement, tryAcquireLock, releaseLock } = await import('../../src/utils/tableLocking.ts') +const auth = await import('@nextcloud/auth') +const dialogs = await import('@nextcloud/dialogs') + describe('tableLocking utilities', () => { describe('isLockExpired', () => { it('should return true for undefined lock', () => { diff --git a/vite.config.ts b/vite.config.ts index 459a2af6..e09d841a 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -37,7 +37,14 @@ const AppConfig = createAppConfig({ manualChunks: { vendor: ['react', 'react-dom'], }, - // assetFileNames: 'js/[name]-[hash].[ext]', + chunkFileNames: 'js/[name]-[hash].chunk.mjs', + assetFileNames: (assetInfo) => { + const assetName = assetInfo.name || '' + if (assetName.endsWith('.css')) { + return 'css/[name]-[hash][extname]' + } + return 'dist/[name]-[hash][extname]' + }, }, }, }, @@ -46,6 +53,8 @@ const AppConfig = createAppConfig({ rollupOptions: { output: { entryFileNames: 'js/[name]-[hash].js', + chunkFileNames: 'js/[name]-[hash].chunk.js', + assetFileNames: 'dist/[name]-[hash][extname]', }, }, }, diff --git a/websocket_server/Services/RoomLifecycleService.js b/websocket_server/Services/RoomLifecycleService.js index 1fbc2f92..11e7cfd0 100644 --- a/websocket_server/Services/RoomLifecycleService.js +++ b/websocket_server/Services/RoomLifecycleService.js @@ -161,17 +161,18 @@ export default class RoomLifecycleService { } } - async onDisconnecting(socket, rooms, { shuttingDown = false } = {}) { + async onDisconnecting(socket, rooms, { shuttingDown = false, socketData: initialSocketData = null } = {}) { if (shuttingDown) { return } + + const socketData = initialSocketData || await this.sessionStore.getSocketData(socket.id) + const userId = socketData?.user?.id + const userName = socketData?.user?.name || 'Unknown' + for (const roomID of rooms) { if (roomID === socket.id) continue - const socketData = await this.sessionStore.getSocketData(socket.id) - const userId = socketData?.user?.id - const userName = socketData?.user?.name || 'Unknown' - console.log(`[${roomID}] User ${userName} disconnecting`) const currentSyncer = await this.cluster.getRoomSyncer(roomID) @@ -194,7 +195,7 @@ export default class RoomLifecycleService { const presentationSession = await this.presentationState.getPresentationSession(roomID) if (presentationSession) { - const isPublicSharingUser = userId.startsWith('shared_') + const isPublicSharingUser = typeof userId === 'string' && userId.startsWith('shared_') let shouldEndPresentation = false if (presentationSession.presenterId === userId) { diff --git a/websocket_server/Services/SocketService.js b/websocket_server/Services/SocketService.js index 2f43f9d7..6e91ab11 100644 --- a/websocket_server/Services/SocketService.js +++ b/websocket_server/Services/SocketService.js @@ -501,6 +501,7 @@ export default class SocketService { const events = { 'join-room': this.joinRoomHandler, 'server-broadcast': this.serverBroadcastHandler, + 'server-direct-broadcast': this.serverDirectBroadcastHandler, 'server-volatile-broadcast': this.serverVolatileBroadcastHandler, 'image-get': this.imageGetHandler, 'check-recording-availability': this.checkRecordingAvailabilityHandler, @@ -536,8 +537,10 @@ export default class SocketService { const rooms = Array.from(socket.rooms).filter( (room) => room !== socket.id, ) - this.safeSocketHandler(socket, () => - this.disconnectingHandler(socket, rooms), + this.safeSocketHandler(socket, async () => { + const socketData = await this.sessionStore.getSocketData(socket.id) + return this.disconnectingHandler(socket, rooms, socketData) + }, ) }) } @@ -554,6 +557,10 @@ export default class SocketService { return this.viewportController.serverBroadcast(socket, roomID, encryptedData, iv) } + async serverDirectBroadcastHandler(socket, roomID, targetSocketId, encryptedData, iv) { + return this.viewportController.serverDirectBroadcast(socket, roomID, targetSocketId, encryptedData, iv) + } + async serverVolatileBroadcastHandler(socket, roomID, encryptedData) { return this.viewportController.serverVolatileBroadcast(socket, roomID, encryptedData) } @@ -582,14 +589,17 @@ export default class SocketService { } } - async disconnectingHandler(socket, rooms) { + async disconnectingHandler(socket, rooms, socketData) { if (this.shuttingDown) { return } - await this.stopRecordingOnDisconnect(socket, rooms) + await this.stopRecordingOnDisconnect(socket, rooms, socketData) - return this.roomLifecycleController.onDisconnecting(socket, rooms, { shuttingDown: this.shuttingDown }) + return this.roomLifecycleController.onDisconnecting(socket, rooms, { + shuttingDown: this.shuttingDown, + socketData, + }) } cancelPendingRecordingStop(roomID, userId) { @@ -700,8 +710,8 @@ export default class SocketService { }) } - async stopRecordingOnDisconnect(socket, rooms) { - const socketData = await this.sessionStore.getSocketData(socket.id) + async stopRecordingOnDisconnect(socket, rooms, initialSocketData = null) { + const socketData = initialSocketData || await this.sessionStore.getSocketData(socket.id) if (!socketData?.user?.id) { return } diff --git a/websocket_server/Services/ViewportService.js b/websocket_server/Services/ViewportService.js index f7d0718c..bb0bc3f7 100644 --- a/websocket_server/Services/ViewportService.js +++ b/websocket_server/Services/ViewportService.js @@ -24,6 +24,20 @@ export default class ViewportService { socket.broadcast.to(roomID).emit('client-broadcast', encryptedData, iv) } + async serverDirectBroadcast(socket, roomID, targetSocketId, encryptedData, iv) { + const isReadOnly = await this.sessionStore.isReadOnly(socket.id) + if (!socket.rooms.has(roomID) || isReadOnly) return + + const targetSockets = await this.io.in(targetSocketId).fetchSockets() + const targetSocket = targetSockets[0] + + if (!targetSocket || !targetSocket.rooms.has(roomID)) { + return + } + + this.io.to(targetSocketId).emit('client-broadcast', encryptedData, iv) + } + async serverVolatileBroadcast(socket, roomID, encryptedData) { if (!socket.rooms.has(roomID)) return