[IMP] image: do not swallow exception and remove GC

doc/extending/plugin.md

@@ -211,15 +211,3 @@ class UserPlugin extends CorePlugin {
   }
 }
 ```
-
-**Delete external resources**
-
-If your custom plugin stores data on an external server, it may be wise to delete
-unused data if it is deleted from the spreadsheet.
-
-Naively cleaning the server when the data is deleted from the spreadsheet doesn't
-work well because the data is still in used in the history stack!
-
-When the history stack is cleared (because the spreadsheet is snapshotted),
-`garbageCollectExternalResources` method is called on each plugin. Implement this
-method to clean unused external resources.

packages/o-spreadsheet-engine/src/model.ts

@@ -245,7 +245,6 @@ export class Model extends EventBus<any> implements CommandDispatcher {
       const startSnapshot = performance.now();
       console.debug("Snapshot requested");
       this.session.snapshot(this.exportData());
-      this.garbageCollectExternalResources();
       console.debug("Snapshot taken in", performance.now() - startSnapshot, "ms");
     }
     console.debug("Model created in", performance.now() - start, "ms");
@@ -664,12 +663,6 @@ export class Model extends EventBus<any> implements CommandDispatcher {
 
     return getXLSX(data);
   }
-
-  garbageCollectExternalResources() {
-    for (const plugin of this.corePlugins) {
-      plugin.garbageCollectExternalResources();
-    }
-  }
 }
 
 function createCommand(type: string, payload: any = {}): Command {

packages/o-spreadsheet-engine/src/plugins/core/image.ts

@@ -1,5 +1,5 @@
 import { FIGURE_ID_SPLITTER } from "../../constants";
-import { deepCopy, isDefined } from "../../helpers/misc";
+import { deepCopy } from "../../helpers/misc";
 import { CommandResult, CoreCommand } from "../../types/commands";
 import { FigureSize } from "../../types/figure";
 import { FileStore } from "../../types/files";
@@ -84,18 +84,6 @@ export class ImagePlugin extends CorePlugin<ImageState> implements ImageState {
     }
   }
 
-  /**
-   * Delete unused images from the file store
-   */
-  garbageCollectExternalResources() {
-    const images = new Set(this.getAllImages().map((image) => image.path));
-    for (const path of this.syncedImages) {
-      if (!images.has(path)) {
-        this.fileStore?.delete(path);
-      }
-    }
-  }
-
   // ---------------------------------------------------------------------------
   // Getters
   // ---------------------------------------------------------------------------
@@ -172,12 +160,4 @@ export class ImagePlugin extends CorePlugin<ImageState> implements ImageState {
       sheet.images = [...sheet.images, ...images];
     }
   }
-
-  private getAllImages(): Image[] {
-    const images: Image[] = [];
-    for (const sheetId in this.images) {
-      images.push(...Object.values(this.images[sheetId] || {}).filter(isDefined));
-    }
-    return images;
-  }
 }

packages/o-spreadsheet-engine/src/plugins/core_plugin.ts

@@ -63,10 +63,4 @@ export class CorePlugin<State = any>
    * @param sheetName couple of old and new sheet names to adapt ranges pointing to that sheet
    */
   adaptRanges(applyChange: ApplyRangeChange, sheetId: UID, sheetName: AdaptSheetName): void {}
-
-  /**
-   * Implement this method to clean unused external resources, such as images
-   * stored on a server which have been deleted.
-   */
-  garbageCollectExternalResources() {}
 }

packages/o-spreadsheet-engine/src/types/files.ts

@@ -12,11 +12,6 @@ export interface FileStore {
    */
   upload(file: File): Promise<FilePath>;
 
-  /**
-   * Delete a file from the server
-   */
-  delete(filePath: FilePath): Promise<void>;
-
   /**
    * get File from the server
    */

src/actions/menu_items_actions.ts

@@ -14,7 +14,6 @@ import {
   ClipboardMIMEType,
   ClipboardPasteOptions,
 } from "@odoo/o-spreadsheet-engine/types/clipboard";
-import { Image } from "@odoo/o-spreadsheet-engine/types/image";
 import { SpreadsheetChildEnv } from "@odoo/o-spreadsheet-engine/types/spreadsheet_env";
 import { getPivotTooBigErrorMessage } from "../../packages/o-spreadsheet-engine/src/components/translations_terms";
 import { CellPopoverStore } from "../components/popover";
@@ -520,23 +519,12 @@ export const REINSERT_STATIC_PIVOT_CHILDREN = (env: SpreadsheetChildEnv) =>
 //------------------------------------------------------------------------------
 // Image
 //------------------------------------------------------------------------------
-async function requestImage(env: SpreadsheetChildEnv): Promise<Image | undefined> {
-  try {
-    return await env.imageProvider!.requestImage();
-  } catch {
-    env.raiseError(_t("An unexpected error occurred during the image transfer"));
-    return;
-  }
-}
 
 export const CREATE_IMAGE = async (env: SpreadsheetChildEnv) => {
   if (env.imageProvider) {
     const sheetId = env.model.getters.getActiveSheetId();
     const figureId = env.model.uuidGenerator.smallUuid();
-    const image = await requestImage(env);
-    if (!image) {
-      return;
-    }
+    const image = await env.imageProvider.requestImage();
     const size = getMaxFigureSize(env.model.getters, image.size);
     const { col, row, offset } = centerFigurePosition(env.model.getters, size);
     env.model.dispatch("CREATE_IMAGE", {

tests/__mocks__/mock_file_store.ts

@@ -6,8 +6,6 @@ export class FileStore implements FileStoreInterface {
     return `file/${this.fileId++}`;
   }
 
-  async delete() {}
-
   async getFile(fileUrl) {
     return new File([], "mock", { type: "image/png" });
   }