From 0d70690ec8fa37902b3fdb48e7d605f25bd1cc72 Mon Sep 17 00:00:00 2001 From: DanielJGeiger <1852529+DanielJGeiger@users.noreply.github.com> Date: Mon, 9 May 2022 08:38:44 -0500 Subject: [PATCH] fix: Don't save deleted ExcalidrawElements to Firebase (#5108) Co-authored-by: dwelle --- src/excalidraw-app/app_constants.ts | 1 + src/excalidraw-app/collab/CollabWrapper.tsx | 20 ++++------ src/excalidraw-app/collab/Portal.tsx | 8 +++- src/excalidraw-app/data/firebase.ts | 44 ++++++++++++++------- src/excalidraw-app/data/index.ts | 28 ++++++++++++- 5 files changed, 70 insertions(+), 31 deletions(-) diff --git a/src/excalidraw-app/app_constants.ts b/src/excalidraw-app/app_constants.ts index be291380..cbead768 100644 --- a/src/excalidraw-app/app_constants.ts +++ b/src/excalidraw-app/app_constants.ts @@ -6,6 +6,7 @@ export const LOAD_IMAGES_TIMEOUT = 500; export const SYNC_FULL_SCENE_INTERVAL_MS = 20000; export const SYNC_BROWSER_TABS_TIMEOUT = 50; export const CURSOR_SYNC_TIMEOUT = 33; // ~30fps +export const DELETED_ELEMENT_TIMEOUT = 24 * 60 * 60 * 1000; // 1 day export const FILE_UPLOAD_MAX_BYTES = 3 * 1024 * 1024; // 3 MiB // 1 year (https://stackoverflow.com/a/25201898/927631) diff --git a/src/excalidraw-app/collab/CollabWrapper.tsx b/src/excalidraw-app/collab/CollabWrapper.tsx index 1d5647a3..c3786a44 100644 --- a/src/excalidraw-app/collab/CollabWrapper.tsx +++ b/src/excalidraw-app/collab/CollabWrapper.tsx @@ -30,7 +30,9 @@ import { generateCollaborationLinkData, getCollaborationLink, getCollabServer, + getSyncableElements, SocketUpdateDataSource, + SyncableExcalidrawElement, } from "../data"; import { isSavedToFirebase, @@ -50,7 +52,6 @@ import { t } from "../../i18n"; import { UserIdleState } from "../../types"; import { IDLE_THRESHOLD, ACTIVE_THRESHOLD } from "../../constants"; import { trackEvent } from "../../analytics"; -import { isInvisiblySmallElement } from "../../element"; import { encodeFilesForUpload, FileManager, @@ -202,7 +203,7 @@ class CollabWrapper extends PureComponent { }; private beforeUnload = withBatchedUpdates((event: BeforeUnloadEvent) => { - const syncableElements = this.getSyncableElements( + const syncableElements = getSyncableElements( this.getSceneElementsIncludingDeleted(), ); @@ -232,7 +233,7 @@ class CollabWrapper extends PureComponent { }); saveCollabRoomToFirebase = async ( - syncableElements: readonly ExcalidrawElement[], + syncableElements: readonly SyncableExcalidrawElement[], ) => { try { const savedData = await saveToFirebase( @@ -262,7 +263,7 @@ class CollabWrapper extends PureComponent { this.loadImageFiles.cancel(); this.saveCollabRoomToFirebase( - this.getSyncableElements( + getSyncableElements( this.excalidrawAPI.getSceneElementsIncludingDeleted(), ), ); @@ -413,7 +414,7 @@ class CollabWrapper extends PureComponent { commitToHistory: true, }); - this.saveCollabRoomToFirebase(this.getSyncableElements(elements)); + this.saveCollabRoomToFirebase(getSyncableElements(elements)); } // fallback in case you're not alone in the room but still don't receive @@ -749,7 +750,7 @@ class CollabWrapper extends PureComponent { queueSaveToFirebase = throttle(() => { if (this.portal.socketInitialized) { this.saveCollabRoomToFirebase( - this.getSyncableElements( + getSyncableElements( this.excalidrawAPI.getSceneElementsIncludingDeleted(), ), ); @@ -775,13 +776,6 @@ class CollabWrapper extends PureComponent { }); }; - isSyncableElement = (element: ExcalidrawElement) => { - return element.isDeleted || !isInvisiblySmallElement(element); - }; - - getSyncableElements = (elements: readonly ExcalidrawElement[]) => - elements.filter((element) => this.isSyncableElement(element)); - /** PRIVATE. Use `this.getContextValue()` instead. */ private contextValue: CollabAPI | null = null; diff --git a/src/excalidraw-app/collab/Portal.tsx b/src/excalidraw-app/collab/Portal.tsx index 043ba6d8..3a60e1df 100644 --- a/src/excalidraw-app/collab/Portal.tsx +++ b/src/excalidraw-app/collab/Portal.tsx @@ -1,4 +1,8 @@ -import { SocketUpdateData, SocketUpdateDataSource } from "../data"; +import { + isSyncableElement, + SocketUpdateData, + SocketUpdateDataSource, +} from "../data"; import CollabWrapper from "./CollabWrapper"; @@ -143,7 +147,7 @@ class Portal { !this.broadcastedElementVersions.has(element.id) || element.version > this.broadcastedElementVersions.get(element.id)!) && - this.collab.isSyncableElement(element) + isSyncableElement(element) ) { acc.push({ ...element, diff --git a/src/excalidraw-app/data/firebase.ts b/src/excalidraw-app/data/firebase.ts index 31a9dfb3..018bc3c4 100644 --- a/src/excalidraw-app/data/firebase.ts +++ b/src/excalidraw-app/data/firebase.ts @@ -13,6 +13,7 @@ import { decompressData } from "../../data/encode"; import { encryptData, decryptData } from "../../data/encryption"; import { MIME_TYPES } from "../../constants"; import { reconcileElements } from "../collab/reconciliation"; +import { getSyncableElements, SyncableExcalidrawElement } from "."; // private // ----------------------------------------------------------------------------- @@ -127,7 +128,18 @@ const decryptElements = async ( return JSON.parse(decodedData); }; -const firebaseSceneVersionCache = new WeakMap(); +class FirebaseSceneVersionCache { + private static cache = new WeakMap(); + static get = (socket: SocketIOClient.Socket) => { + return FirebaseSceneVersionCache.cache.get(socket); + }; + static set = ( + socket: SocketIOClient.Socket, + elements: readonly SyncableExcalidrawElement[], + ) => { + FirebaseSceneVersionCache.cache.set(socket, getSceneVersion(elements)); + }; +} export const isSavedToFirebase = ( portal: Portal, @@ -136,7 +148,7 @@ export const isSavedToFirebase = ( if (portal.socket && portal.roomId && portal.roomKey) { const sceneVersion = getSceneVersion(elements); - return firebaseSceneVersionCache.get(portal.socket) === sceneVersion; + return FirebaseSceneVersionCache.get(portal.socket) === sceneVersion; } // if no room exists, consider the room saved so that we don't unnecessarily // prevent unload (there's nothing we could do at that point anyway) @@ -181,7 +193,7 @@ export const saveFilesToFirebase = async ({ const createFirebaseSceneDocument = async ( firebase: ResolutionType, - elements: readonly ExcalidrawElement[], + elements: readonly SyncableExcalidrawElement[], roomKey: string, ) => { const sceneVersion = getSceneVersion(elements); @@ -197,7 +209,7 @@ const createFirebaseSceneDocument = async ( export const saveToFirebase = async ( portal: Portal, - elements: readonly ExcalidrawElement[], + elements: readonly SyncableExcalidrawElement[], appState: AppState, ) => { const { roomId, roomKey, socket } = portal; @@ -229,18 +241,18 @@ export const saveToFirebase = async ( transaction.set(docRef, sceneDocument); return { - sceneVersion: sceneDocument.sceneVersion, + elements, reconciledElements: null, }; } const prevDocData = snapshot.data() as FirebaseStoredScene; - const prevElements = await decryptElements(prevDocData, roomKey); + const prevElements = getSyncableElements( + await decryptElements(prevDocData, roomKey), + ); - const reconciledElements = reconcileElements( - elements, - prevElements, - appState, + const reconciledElements = getSyncableElements( + reconcileElements(elements, prevElements, appState), ); const sceneDocument = await createFirebaseSceneDocument( @@ -251,14 +263,14 @@ export const saveToFirebase = async ( transaction.update(docRef, sceneDocument); return { + elements, reconciledElements, - sceneVersion: sceneDocument.sceneVersion, }; }); - firebaseSceneVersionCache.set(socket, savedData.sceneVersion); + FirebaseSceneVersionCache.set(socket, savedData.elements); - return savedData; + return { reconciledElements: savedData.reconciledElements }; }; export const loadFromFirebase = async ( @@ -275,10 +287,12 @@ export const loadFromFirebase = async ( return null; } const storedScene = doc.data() as FirebaseStoredScene; - const elements = await decryptElements(storedScene, roomKey); + const elements = getSyncableElements( + await decryptElements(storedScene, roomKey), + ); if (socket) { - firebaseSceneVersionCache.set(socket, getSceneVersion(elements)); + FirebaseSceneVersionCache.set(socket, elements); } return restoreElements(elements, null); diff --git a/src/excalidraw-app/data/index.ts b/src/excalidraw-app/data/index.ts index d59b7eb7..705347fc 100644 --- a/src/excalidraw-app/data/index.ts +++ b/src/excalidraw-app/data/index.ts @@ -7,6 +7,7 @@ import { import { serializeAsJSON } from "../../data/json"; import { restore } from "../../data/restore"; import { ImportedDataState } from "../../data/types"; +import { isInvisiblySmallElement } from "../../element/sizeHelpers"; import { isInitializedImageElement } from "../../element/typeChecks"; import { ExcalidrawElement, FileId } from "../../element/types"; import { t } from "../../i18n"; @@ -17,10 +18,35 @@ import { UserIdleState, } from "../../types"; import { bytesToHexString } from "../../utils"; -import { FILE_UPLOAD_MAX_BYTES, ROOM_ID_BYTES } from "../app_constants"; +import { + DELETED_ELEMENT_TIMEOUT, + FILE_UPLOAD_MAX_BYTES, + ROOM_ID_BYTES, +} from "../app_constants"; import { encodeFilesForUpload } from "./FileManager"; import { saveFilesToFirebase } from "./firebase"; +export type SyncableExcalidrawElement = ExcalidrawElement & { + _brand: "SyncableExcalidrawElement"; +}; + +export const isSyncableElement = ( + element: ExcalidrawElement, +): element is SyncableExcalidrawElement => { + if (element.isDeleted) { + if (element.updated > Date.now() - DELETED_ELEMENT_TIMEOUT) { + return true; + } + return false; + } + return !isInvisiblySmallElement(element); +}; + +export const getSyncableElements = (elements: readonly ExcalidrawElement[]) => + elements.filter((element) => + isSyncableElement(element), + ) as SyncableExcalidrawElement[]; + const BACKEND_V2_GET = process.env.REACT_APP_BACKEND_V2_GET_URL; const BACKEND_V2_POST = process.env.REACT_APP_BACKEND_V2_POST_URL;