Merge branch 'excalidraw-master'

zsviczian · Oct 30, 2024 · cd1ad09 · cd1ad09
2 parents b3f2c8a + 7910778
commit cd1ad09
Show file tree

Hide file tree

Showing 24 changed files with 1,678 additions and 1,270 deletions.
diff --git a/excalidraw-app/collab/Collab.tsx b/excalidraw-app/collab/Collab.tsx
@@ -1,6 +1,7 @@
 import throttle from "lodash.throttle";
 import { PureComponent } from "react";
 import type {
+  BinaryFileData,
   ExcalidrawImperativeAPI,
   SocketId,
 } from "../../packages/excalidraw/types";
@@ -9,6 +10,7 @@ import { APP_NAME, ENV, EVENT } from "../../packages/excalidraw/constants";
 import type { ImportedDataState } from "../../packages/excalidraw/data/types";
 import type {
   ExcalidrawElement,
+  FileId,
   InitializedExcalidrawImageElement,
   OrderedExcalidrawElement,
 } from "../../packages/excalidraw/element/types";
@@ -157,14 +159,37 @@ class Collab extends PureComponent<CollabProps, CollabState> {
           throw new AbortError();
         }
 
-        return saveFilesToFirebase({
+        const { savedFiles, erroredFiles } = await saveFilesToFirebase({
           prefix: `${FIREBASE_STORAGE_PREFIXES.collabFiles}/${roomId}`,
           files: await encodeFilesForUpload({
             files: addedFiles,
             encryptionKey: roomKey,
             maxBytes: FILE_UPLOAD_MAX_BYTES,
           }),
         });
+
+        return {
+          savedFiles: savedFiles.reduce(
+            (acc: Map<FileId, BinaryFileData>, id) => {
+              const fileData = addedFiles.get(id);
+              if (fileData) {
+                acc.set(id, fileData);
+              }
+              return acc;
+            },
+            new Map(),
+          ),
+          erroredFiles: erroredFiles.reduce(
+            (acc: Map<FileId, BinaryFileData>, id) => {
+              const fileData = addedFiles.get(id);
+              if (fileData) {
+                acc.set(id, fileData);
+              }
+              return acc;
+            },
+            new Map(),
+          ),
+        };
       },
     });
     this.excalidrawAPI = props.excalidrawAPI;
@@ -394,7 +419,7 @@ class Collab extends PureComponent<CollabProps, CollabState> {
       .filter((element) => {
         return (
           isInitializedImageElement(element) &&
-          !this.fileManager.isFileHandled(element.fileId) &&
+          !this.fileManager.isFileTracked(element.fileId) &&
           !element.isDeleted &&
           (opts.forceFetchFiles
             ? element.status !== "pending" ||

diff --git a/excalidraw-app/data/FileManager.ts b/excalidraw-app/data/FileManager.ts
@@ -16,14 +16,26 @@ import type {
   BinaryFiles,
 } from "../../packages/excalidraw/types";
 
+type FileVersion = Required<BinaryFileData>["version"];
+
 export class FileManager {
   /** files being fetched */
   private fetchingFiles = new Map<ExcalidrawImageElement["fileId"], true>();
+  private erroredFiles_fetch = new Map<
+    ExcalidrawImageElement["fileId"],
+    true
+  >();
   /** files being saved */
-  private savingFiles = new Map<ExcalidrawImageElement["fileId"], true>();
+  private savingFiles = new Map<
+    ExcalidrawImageElement["fileId"],
+    FileVersion
+  >();
   /* files already saved to persistent storage */
-  private savedFiles = new Map<ExcalidrawImageElement["fileId"], true>();
-  private erroredFiles = new Map<ExcalidrawImageElement["fileId"], true>();
+  private savedFiles = new Map<ExcalidrawImageElement["fileId"], FileVersion>();
+  private erroredFiles_save = new Map<
+    ExcalidrawImageElement["fileId"],
+    FileVersion
+  >();
 
   private _getFiles;
   private _saveFiles;
@@ -37,28 +49,37 @@ export class FileManager {
       erroredFiles: Map<FileId, true>;
     }>;
     saveFiles: (data: { addedFiles: Map<FileId, BinaryFileData> }) => Promise<{
-      savedFiles: Map<FileId, true>;
-      erroredFiles: Map<FileId, true>;
+      savedFiles: Map<FileId, BinaryFileData>;
+      erroredFiles: Map<FileId, BinaryFileData>;
     }>;
   }) {
     this._getFiles = getFiles;
     this._saveFiles = saveFiles;
   }
 
   /**
-   * returns whether file is already saved or being processed
+   * returns whether file is saved/errored, or being processed
    */
-  isFileHandled = (id: FileId) => {
+  isFileTracked = (id: FileId) => {
     return (
       this.savedFiles.has(id) ||
-      this.fetchingFiles.has(id) ||
       this.savingFiles.has(id) ||
-      this.erroredFiles.has(id)
+      this.fetchingFiles.has(id) ||
+      this.erroredFiles_fetch.has(id) ||
+      this.erroredFiles_save.has(id)
+    );
+  };
+
+  isFileSavedOrBeingSaved = (file: BinaryFileData) => {
+    const fileVersion = this.getFileVersion(file);
+    return (
+      this.savedFiles.get(file.id) === fileVersion ||
+      this.savingFiles.get(file.id) === fileVersion
     );
   };
 
-  isFileSaved = (id: FileId) => {
-    return this.savedFiles.has(id);
+  getFileVersion = (file: BinaryFileData) => {
+    return file.version ?? 1;
   };
 
   saveFiles = async ({
@@ -71,13 +92,16 @@ export class FileManager {
     const addedFiles: Map<FileId, BinaryFileData> = new Map();
 
     for (const element of elements) {
+      const fileData =
+        isInitializedImageElement(element) && files[element.fileId];
+
       if (
-        isInitializedImageElement(element) &&
-        files[element.fileId] &&
-        !this.isFileHandled(element.fileId)
+        fileData &&
+        // NOTE if errored during save, won't retry due to this check
+        !this.isFileSavedOrBeingSaved(fileData)
       ) {
         addedFiles.set(element.fileId, files[element.fileId]);
-        this.savingFiles.set(element.fileId, true);
+        this.savingFiles.set(element.fileId, this.getFileVersion(fileData));
       }
     }
 
@@ -86,8 +110,12 @@ export class FileManager {
         addedFiles,
       });
 
-      for (const [fileId] of savedFiles) {
-        this.savedFiles.set(fileId, true);
+      for (const [fileId, fileData] of savedFiles) {
+        this.savedFiles.set(fileId, this.getFileVersion(fileData));
+      }
+
+      for (const [fileId, fileData] of erroredFiles) {
+        this.erroredFiles_save.set(fileId, this.getFileVersion(fileData));
       }
 
       return {
@@ -121,10 +149,10 @@ export class FileManager {
       const { loadedFiles, erroredFiles } = await this._getFiles(ids);
 
       for (const file of loadedFiles) {
-        this.savedFiles.set(file.id, true);
+        this.savedFiles.set(file.id, this.getFileVersion(file));
       }
       for (const [fileId] of erroredFiles) {
-        this.erroredFiles.set(fileId, true);
+        this.erroredFiles_fetch.set(fileId, true);
       }
 
       return { loadedFiles, erroredFiles };
@@ -160,7 +188,7 @@ export class FileManager {
   ): element is InitializedExcalidrawImageElement => {
     return (
       isInitializedImageElement(element) &&
-      this.isFileSaved(element.fileId) &&
+      this.savedFiles.has(element.fileId) &&
       element.status === "pending"
     );
   };
@@ -169,7 +197,8 @@ export class FileManager {
     this.fetchingFiles.clear();
     this.savingFiles.clear();
     this.savedFiles.clear();
-    this.erroredFiles.clear();
+    this.erroredFiles_fetch.clear();
+    this.erroredFiles_save.clear();
   }
 }
 

diff --git a/excalidraw-app/data/LocalData.ts b/excalidraw-app/data/LocalData.ts
@@ -183,8 +183,8 @@ export class LocalData {
       );
     },
     async saveFiles({ addedFiles }) {
-      const savedFiles = new Map<FileId, true>();
-      const erroredFiles = new Map<FileId, true>();
+      const savedFiles = new Map<FileId, BinaryFileData>();
+      const erroredFiles = new Map<FileId, BinaryFileData>();
 
       // before we use `storage` event synchronization, let's update the flag
       // optimistically. Hopefully nothing fails, and an IDB read executed
@@ -195,10 +195,10 @@ export class LocalData {
         [...addedFiles].map(async ([id, fileData]) => {
           try {
             await set(id, fileData, filesStore);
-            savedFiles.set(id, true);
+            savedFiles.set(id, fileData);
           } catch (error: any) {
             console.error(error);
-            erroredFiles.set(id, true);
+            erroredFiles.set(id, fileData);
           }
         }),
       );

diff --git a/excalidraw-app/data/firebase.ts b/excalidraw-app/data/firebase.ts
@@ -177,8 +177,8 @@ export const saveFilesToFirebase = async ({
 }) => {
   const firebase = await loadFirebaseStorage();
 
-  const erroredFiles = new Map<FileId, true>();
-  const savedFiles = new Map<FileId, true>();
+  const erroredFiles: FileId[] = [];
+  const savedFiles: FileId[] = [];
 
   await Promise.all(
     files.map(async ({ id, buffer }) => {
@@ -194,9 +194,9 @@ export const saveFilesToFirebase = async ({
               cacheControl: `public, max-age=${FILE_CACHE_MAX_AGE_SEC}`,
             },
           );
-        savedFiles.set(id, true);
+        savedFiles.push(id);
       } catch (error: any) {
-        erroredFiles.set(id, true);
+        erroredFiles.push(id);
       }
     }),
   );