Cherry pick/optimize excalidraw sync code

src/modules/exdraw/managers/excalidraw-manager.js

@@ -114,10 +114,18 @@ class ExcalidrawManager {
     }
     const sceneData = new ExcalidrawDocument(exdrawUuid, exdrawName, docContent);
     sceneData.setLastModifyUser({ username });
+
+    // Multiple operations may load the same cold document concurrently.
+    // Use the document that won the cache race instead of overwriting it.
+    const cachedDocument = this.documents.get(exdrawUuid);
+    if (cachedDocument) {
+      return cachedDocument.toJson();
+    }
+
     this.documents.set(exdrawUuid, sceneData);
 
     if (!result.data) {
-      sceneData.setMeta({need_save: true});
+      sceneData.setMeta({ need_save: true });
       await this.saveSceneDoc(exdrawUuid);
     }
 
@@ -140,7 +148,9 @@ class ExcalidrawManager {
 
     document.setMeta({ is_saving: true });
 
-    // Get save info
+    // Capture the version included in this save. Operations that arrive while
+    // the upload is in flight must keep the document dirty for a later save.
+    const savedVersion = document.version;
     const exdrawContent = document.toJson();
     const exdrawName = document.exdrawName;
 
@@ -165,17 +175,22 @@ class ExcalidrawManager {
     } finally {
       deleteDir(tempPath);
     }
-    document.setMeta({is_saving: false, need_save: false});
+    const hasNewerChanges = document.version !== savedVersion;
+    document.setMeta({
+      is_saving: false,
+      need_save: !saveFlag || hasNewerChanges,
+    });
 
     return Promise.resolve(saveFlag);
   };
 
   saveSceneDocToMemory = async (exdrawUuid, exdrawName, content, username) => {
-    const document = this.documents.get(exdrawUuid);
+    let document = this.documents.get(exdrawUuid);
     if (!document) {
       try {
         // Load the document before executing op to avoid the document not being loaded into the memory after disconnection and reconnection
-        await this.getDoc(exdrawUuid, exdrawName);
+        await this.getSceneDoc(exdrawUuid, exdrawName);
+        document = this.documents.get(exdrawUuid);
       } catch(e) {
         logger.error(`SOCKET_MESSAGE: Load ${exdrawName}(${exdrawUuid}) doc content error`);
         const result = {
@@ -186,6 +201,14 @@ class ExcalidrawManager {
       }
     }
 
+    if (!document) {
+      logger.error(`SOCKET_MESSAGE: Document ${exdrawName}(${exdrawUuid}) is not available after loading`);
+      return Promise.resolve({
+        success: false,
+        error_type: 'load_document_content_error',
+      });
+    }
+
     const { version: clientVersion} = content;
     const { version: serverVersion, elements } = document;
     if (serverVersion !== clientVersion) {
@@ -214,11 +237,12 @@ class ExcalidrawManager {
 
   execOperationsBySocket = async (params, exdrawName) => {
     const { doc_uuid: docUuid, version: clientVersion, user, elements } = params;
-    const document = this.documents.get(docUuid);
+    let document = this.documents.get(docUuid);
     if (!document) {
       try {
         // Load the document before executing op to avoid the document not being loaded into the memory after disconnection and reconnection
-        await this.getDoc(docUuid, exdrawName);
+        await this.getSceneDoc(docUuid, exdrawName);
+        document = this.documents.get(docUuid);
       } catch(e) {
         logger.error(`SOCKET_MESSAGE: Load ${exdrawName}(${docUuid}) doc content error`);
         const result = {
@@ -229,6 +253,14 @@ class ExcalidrawManager {
       }
     }
 
+    if (!document) {
+      logger.error(`SOCKET_MESSAGE: Document ${exdrawName}(${docUuid}) is not available after loading`);
+      return Promise.resolve({
+        success: false,
+        error_type: 'load_document_content_error',
+      });
+    }
+
     const { version: serverVersion } = document;
     if (serverVersion !== clientVersion) {
       const result = {
@@ -242,6 +274,14 @@ class ExcalidrawManager {
       return Promise.resolve(result);
     }
 
+    if (!Array.isArray(elements) || elements.length === 0) {
+      return Promise.resolve({
+        success: true,
+        updated: false,
+        version: document.version,
+      });
+    }
+
         // execute operations success
     let isExecuteSuccess = false;
     try {

src/modules/exdraw/wio/io-handler.js

@@ -2,6 +2,7 @@ import ExcalidrawManager from "../managers/excalidraw-manager";
 import UsersManager from "../managers/users-manager";
 import IOHelper from "./io-helper";
 import checkPermission from "./is-permission-valid";
+import logger from "../../../loggers";
 
 class ExdrawIOHandler {
 
@@ -24,25 +25,37 @@ class ExdrawIOHandler {
   onConnection(socket) {
     // todo permission check
     this.ioHelper.sendInitRoomToPrivate(socket.id);
-    socket.on('join-room', async (params) => {
-      // join room
-      const { doc_uuid: docUuid, user: userInfo } = params;
-      socket.join(docUuid);
-
-      const usersManager = UsersManager.getInstance();
-      if (!usersManager.getUser(docUuid, socket.id)) {
-        usersManager.addUser(docUuid, socket.id, userInfo);
+    socket.on('join-room', async (params, callback) => {
+      try {
+        const { doc_uuid: docUuid, user: userInfo } = params || {};
+        if (!docUuid || !userInfo) {
+          throw new Error('Invalid join-room parameters');
+        }
+
+        await socket.join(docUuid);
+
+        const usersManager = UsersManager.getInstance();
+        if (!usersManager.getUser(docUuid, socket.id)) {
+          usersManager.addUser(docUuid, socket.id, userInfo);
+        }
+
+        const users = usersManager.getDocUsers(docUuid);
+
+        // if (users.length === 1) {
+        // this.ioHelper.sendFirstInRoomMessage(socket.id);
+        // } else {
+        //   this.ioHelper.sendNewUserMessage(socket, docUuid);
+        // }
+
+        this.ioHelper.sendRoomUserChangeMessage(socket, docUuid, users);
+        callback && callback({ success: true });
+      } catch (error) {
+        logger.error('SOCKET_MESSAGE: Join room failed', error);
+        callback && callback({
+          success: false,
+          error_type: 'join_room_error',
+        });
       }
-
-      const users = usersManager.getDocUsers(docUuid);
-
-      // if (users.length === 1) {
-      // this.ioHelper.sendFirstInRoomMessage(socket.id);
-      // } else {
-      //   this.ioHelper.sendNewUserMessage(socket, docUuid);
-      // }
-
-      this.ioHelper.sendRoomUserChangeMessage(socket, docUuid, users);
     });
 
     socket.on('elements-updated', async (params, callback) => {

tests/managers/excalidraw-manager.test.js

@@ -0,0 +1,102 @@
+import ExcalidrawManager from '../../src/modules/exdraw/managers/excalidraw-manager';
+import ExcalidrawDocument from '../../src/modules/exdraw/models/excalidraw-document';
+import seaServerAPI from '../../src/modules/exdraw/api/sea-server-api';
+
+jest.mock('../../src/modules/exdraw/api/sea-server-api', () => ({
+  __esModule: true,
+  default: {
+    getSceneDownloadLink: jest.fn(),
+    getSceneContent: jest.fn(),
+    saveSceneContent: jest.fn(),
+  },
+}));
+
+jest.mock('../../src/loggers', () => ({
+  __esModule: true,
+  default: {
+    error: jest.fn(),
+    info: jest.fn(),
+    warn: jest.fn(),
+  },
+}));
+
+jest.mock('../../src/utils', () => ({
+  __esModule: true,
+  deleteDir: jest.fn(),
+  getErrorMessage: jest.fn(() => ({})),
+  errorHandle: jest.fn(),
+}));
+
+const createDeferred = () => {
+  let resolve;
+  const promise = new Promise((res) => {
+    resolve = res;
+  });
+  return { promise, resolve };
+};
+
+describe('ExcalidrawManager cold document loading and saving', () => {
+  let manager;
+
+  beforeEach(() => {
+    manager = ExcalidrawManager.getInstance();
+    manager.documents = new Map();
+    jest.clearAllMocks();
+    seaServerAPI.getSceneDownloadLink.mockResolvedValue({
+      data: { download_link: 'https://example.test/doc' },
+    });
+  });
+
+  it('uses the document already cached when concurrent cold loads finish', async () => {
+    const contentLoads = [];
+    seaServerAPI.getSceneContent.mockImplementation(() => {
+      const deferred = createDeferred();
+      contentLoads.push(deferred);
+      return deferred.promise;
+    });
+
+    const firstLoad = manager.getSceneDoc('doc-uuid', 'doc.excalidraw', 'user-a');
+    const secondLoad = manager.getSceneDoc('doc-uuid', 'doc.excalidraw', 'user-b');
+
+    await new Promise(resolve => setImmediate(resolve));
+    expect(contentLoads).toHaveLength(2);
+
+    contentLoads[0].resolve({
+      data: { version: 0, elements: [] },
+    });
+    await new Promise(resolve => setImmediate(resolve));
+
+    contentLoads[1].resolve({
+      data: { version: 0, elements: [] },
+    });
+
+    const [firstDocument, secondDocument] = await Promise.all([firstLoad, secondLoad]);
+    const cachedDocument = manager.documents.get('doc-uuid');
+
+    expect(cachedDocument.last_modify_user).toBe('user-a');
+    expect(firstDocument.last_modify_user).toBe('user-a');
+    expect(secondDocument.last_modify_user).toBe('user-a');
+  });
+
+  it('keeps the document dirty when an operation arrives during saving', async () => {
+    const document = new ExcalidrawDocument('doc-uuid', 'doc.excalidraw', {
+      version: 0,
+      elements: [],
+    });
+    document.setMeta({ need_save: true });
+    manager.documents.set('doc-uuid', document);
+
+    const save = createDeferred();
+    seaServerAPI.saveSceneContent.mockReturnValue(save.promise);
+
+    const savePromise = manager.saveSceneDoc('doc-uuid');
+    document.setValue([{ id: 'new-element' }], 1);
+    save.resolve({ data: {} });
+
+    expect(await savePromise).toBe(true);
+    expect(document.getMeta()).toEqual(expect.objectContaining({
+      is_saving: false,
+      need_save: true,
+    }));
+  });
+});

tests/wio-io-handler.test.js

@@ -0,0 +1,92 @@
+import ExdrawIOHandler from '../src/modules/exdraw/wio/io-handler';
+import IOHelper from '../src/modules/exdraw/wio/io-helper';
+import UsersManager from '../src/modules/exdraw/managers/users-manager';
+
+jest.mock('../src/modules/exdraw/wio/io-helper');
+jest.mock('../src/modules/exdraw/managers/users-manager');
+jest.mock('../src/loggers', () => ({
+  __esModule: true,
+  default: {
+    error: jest.fn(),
+  },
+}));
+
+describe('ExdrawIOHandler join-room', () => {
+  let ioHelper;
+  let usersManager;
+  let socket;
+  let handlers;
+
+  beforeEach(() => {
+    handlers = {};
+    ioHelper = {
+      sendInitRoomToPrivate: jest.fn(),
+      sendRoomUserChangeMessage: jest.fn(),
+    };
+    usersManager = {
+      getUser: jest.fn(),
+      addUser: jest.fn(),
+      getDocUsers: jest.fn().mockReturnValue([]),
+    };
+    socket = {
+      id: 'socket-id',
+      on: jest.fn((event, handler) => {
+        handlers[event] = handler;
+      }),
+      join: jest.fn().mockResolvedValue(undefined),
+    };
+    IOHelper.getInstance.mockReturnValue(ioHelper);
+    UsersManager.getInstance.mockReturnValue(usersManager);
+  });
+
+  it('returns a success acknowledgement after joining the room', async () => {
+    const callback = jest.fn();
+    const handler = new ExdrawIOHandler({});
+
+    handler.onConnection(socket);
+    await handlers['join-room']({
+      doc_uuid: 'doc-uuid',
+      user: { _username: 'user' },
+    }, callback);
+
+    expect(socket.join).toHaveBeenCalledWith('doc-uuid');
+    expect(ioHelper.sendRoomUserChangeMessage).toHaveBeenCalledWith(socket, 'doc-uuid', []);
+    expect(callback).toHaveBeenCalledWith({ success: true });
+  });
+
+  it('returns a failure acknowledgement when joining the room throws', async () => {
+    const callback = jest.fn();
+    socket.join.mockRejectedValue(new Error('adapter unavailable'));
+    const handler = new ExdrawIOHandler({});
+
+    handler.onConnection(socket);
+    await handlers['join-room']({
+      doc_uuid: 'doc-uuid',
+      user: { _username: 'user' },
+    }, callback);
+
+    expect(callback).toHaveBeenCalledWith({
+      success: false,
+      error_type: 'join_room_error',
+    });
+  });
+
+  it('returns a failure acknowledgement when user management throws', async () => {
+    const callback = jest.fn();
+    usersManager.addUser.mockImplementation(() => {
+      throw new Error('user manager unavailable');
+    });
+    const handler = new ExdrawIOHandler({});
+
+    handler.onConnection(socket);
+    await handlers['join-room']({
+      doc_uuid: 'doc-uuid',
+      user: { _username: 'user' },
+    }, callback);
+
+    expect(callback).toHaveBeenCalledWith({
+      success: false,
+      error_type: 'join_room_error',
+    });
+  });
+});