DEV: Improve uppy plugin base and large file handling (#14383)

We want to be able to skip plugins from doing any work under
certain conditions, and to be able raise their own errors if
a file being uploaded is completely incompatible with the concept
of the plugin if it is enabled. For example, the UppyChecksum plugin
is happy to skip hashing large files, but the UppyUploadEncrypt
plugin from discourse-encrypt relies on the file being encrypted
to do anything with the upload, so it is considered a blocking
error if the user uploads a file that is too large.

This improves the base functions available in uppy-plugin-base and
extendable-uploader to handle this, as well as introducing a
HUGE_FILE_THRESHOLD_BYTES variable which represents 100MB in bytes,
matching the ExternalUploadManager::DOWNLOAD_LIMIT on the
server side.

discourse-encrypt to take advantage of this new functionality will
follow in https://github.com/discourse/discourse-encrypt/pull/141

app/assets/javascripts/discourse/app/lib/uppy-checksum-plugin.js

@@ -1,5 +1,6 @@
 import { UploadPreProcessorPlugin } from "discourse/lib/uppy-plugin-base";
 import { Promise } from "rsvp";
+import { HUGE_FILE_THRESHOLD_BYTES } from "discourse/mixins/uppy-upload";
 
 export default class UppyChecksum extends UploadPreProcessorPlugin {
   static pluginId = "uppy-checksum";
@@ -34,14 +35,20 @@ export default class UppyChecksum extends UploadPreProcessorPlugin {
 
   _generateChecksum(fileIds) {
     if (!this._canUseSubtleCrypto()) {
-      return Promise.resolve();
+      return this._skipAll(fileIds, true);
     }
 
     let promises = fileIds.map((fileId) => {
       let file = this._getFile(fileId);
-
       this._emitProgress(file);
 
+      if (file.size > HUGE_FILE_THRESHOLD_BYTES) {
+        this._consoleWarn(
+          "The file provided is too large to checksum, skipping."
+        );
+        return this._skip(file);
+      }
+
       return file.data.arrayBuffer().then((arrayBuffer) => {
         return window.crypto.subtle
           .digest("SHA-1", arrayBuffer)

app/assets/javascripts/discourse/app/lib/uppy-media-optimization-plugin.js

@@ -22,17 +22,19 @@ export default class UppyMediaOptimization extends UploadPreProcessorPlugin {
 
     return this.optimizeFn(file, { stopWorkerOnError: !this.runParallel })
       .then((optimizedFile) => {
+        let skipped = false;
         if (!optimizedFile) {
           this._consoleWarn(
-            "Nothing happened, possible error or other restriction."
+            "Nothing happened, possible error or other restriction, or the file format is not a valid one for compression."
           );
+          skipped = true;
         } else {
           this._setFileState(fileId, {
             data: optimizedFile,
             size: optimizedFile.size,
           });
         }
-        this._emitComplete(file);
+        this._emitComplete(file, skipped);
       })
       .catch((err) => {
         this._consoleWarn(err);

app/assets/javascripts/discourse/app/lib/uppy-plugin-base.js

@@ -1,5 +1,6 @@
 import { BasePlugin } from "@uppy/core";
-import { warn } from "@ember/debug";
+import { Promise } from "rsvp";
+import { debug, warn } from "@ember/debug";
 
 export class UppyPluginBase extends BasePlugin {
   constructor(uppy, opts) {
@@ -8,7 +9,13 @@ export class UppyPluginBase extends BasePlugin {
   }
 
   _consoleWarn(msg) {
-    warn(msg, { id: `discourse.${this.id}` });
+    warn(`[${this.id}] ${msg}`, { id: `discourse.${this.id}` });
+  }
+
+  _consoleDebug(msg) {
+    if (this.siteSettings?.enable_upload_debug_mode) {
+      debug(`[${this.id}] ${msg}`, { id: `discourse.${this.id}` });
+    }
   }
 
   _getFile(fileId) {
@@ -41,10 +48,36 @@ export class UploadPreProcessorPlugin extends UppyPluginBase {
   }
 
   _emitProgress(file) {
-    this.uppy.emit("preprocess-progress", this.id, file);
+    this.uppy.emit("preprocess-progress", file, null, this.id);
   }
 
-  _emitComplete(file) {
-    this.uppy.emit("preprocess-complete", this.id, file);
+  _emitComplete(file, skipped = false) {
+    this.uppy.emit("preprocess-complete", file, skipped, this.id);
+    return Promise.resolve();
+  }
+
+  _emitAllComplete(fileIds, skipped = false) {
+    fileIds.forEach((fileId) => {
+      let file = this._getFile(fileId);
+      this._emitComplete(file, skipped);
+    });
+    return Promise.resolve();
+  }
+
+  _emitError(file, errorMessage) {
+    // the error message is stored twice; once to show in a displayErrorForUpload
+    // modal, and on the .message property to show in the uppy logs
+    this.uppy.emit("upload-error", file, {
+      errors: [errorMessage],
+      message: `[${this.id}] ${errorMessage}`,
+    });
+  }
+
+  _skip(file) {
+    return this._emitComplete(file, true);
+  }
+
+  _skipAll(file) {
+    return this._emitAllComplete(file, true);
   }
 }

app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js

@@ -382,6 +382,8 @@ export default Mixin.create(ExtendableUploader, {
       limit: 10,
 
       createMultipartUpload(file) {
+        self._uppyInstance.emit("create-multipart", file.id);
+
         const data = {
           file_name: file.name,
           file_size: file.size,
@@ -402,6 +404,8 @@ export default Mixin.create(ExtendableUploader, {
           data,
           // uppy is inconsistent, an error here fires the upload-error event
         }).then((responseData) => {
+          self._uppyInstance.emit("create-multipart-success", file.id);
+
           file.meta.unique_identifier = responseData.unique_identifier;
           return {
             uploadId: responseData.external_upload_identifier,
@@ -430,6 +434,7 @@ export default Mixin.create(ExtendableUploader, {
       },
 
       completeMultipartUpload(file, data) {
+        self._uppyInstance.emit("complete-multipart", file.id);
         const parts = data.parts.map((part) => {
           return { part_number: part.PartNumber, etag: part.ETag };
         });
@@ -442,6 +447,7 @@ export default Mixin.create(ExtendableUploader, {
           }),
           // uppy is inconsistent, an error here fires the upload-error event
         }).then((responseData) => {
+          self._uppyInstance.emit("complete-multipart-success", file.id);
           return responseData;
         });
       },
@@ -556,11 +562,4 @@ export default Mixin.create(ExtendableUploader, {
   showUploadSelector(toolbarEvent) {
     this.send("showUploadSelector", toolbarEvent);
   },
-
-  _debugLog(message) {
-    if (this.siteSettings.enable_upload_debug_mode) {
-      // eslint-disable-next-line no-console
-      console.log(message);
-    }
-  },
 });

app/assets/javascripts/discourse/app/mixins/extendable-uploader.js

@@ -1,4 +1,5 @@
 import Mixin from "@ember/object/mixin";
+import { debug } from "@ember/debug";
 
 /**
  * Use this mixin with any component that needs to upload files or images
@@ -80,8 +81,10 @@ export default Mixin.create({
   //
   // See: https://uppy.io/docs/writing-plugins/#Progress-events
   _onPreProcessProgress(callback) {
-    this._uppyInstance.on("preprocess-progress", (pluginId, file) => {
-      this._debugLog(`[${pluginId}] processing file ${file.name} (${file.id})`);
+    this._uppyInstance.on("preprocess-progress", (file, progress, pluginId) => {
+      this._consoleDebug(
+        `[${pluginId}] processing file ${file.name} (${file.id})`
+      );
 
       this._preProcessorStatus[pluginId].activeProcessing++;
 
@@ -90,16 +93,18 @@ export default Mixin.create({
   },
 
   _onPreProcessComplete(callback, allCompleteCallback) {
-    this._uppyInstance.on("preprocess-complete", (pluginId, file) => {
-      this._debugLog(
-        `[${pluginId}] completed processing file ${file.name} (${file.id})`
+    this._uppyInstance.on("preprocess-complete", (file, skipped, pluginId) => {
+      this._consoleDebug(
+        `[${pluginId}] ${skipped ? "skipped" : "completed"} processing file ${
+          file.name
+        } (${file.id})`
       );
 
       callback(file);
 
       this._completePreProcessing(pluginId, (allComplete) => {
         if (allComplete) {
-          this._debugLog("All upload preprocessors complete.");
+          this._consoleDebug("[uppy] All upload preprocessors complete!");
           allCompleteCallback();
         }
       });
@@ -168,4 +173,10 @@ export default Mixin.create({
       }
     }
   },
+
+  _consoleDebug(msg) {
+    if (this.siteSettings.enable_upload_debug_mode) {
+      debug(msg, { id: "discourse.extendable-uploader" });
+    }
+  },
 });

app/assets/javascripts/discourse/app/mixins/uppy-upload.js

@@ -16,6 +16,8 @@ import UppyChecksum from "discourse/lib/uppy-checksum-plugin";
 import { on } from "discourse-common/utils/decorators";
 import { warn } from "@ember/debug";
 
+export const HUGE_FILE_THRESHOLD_BYTES = 104_857_600; // 100MB
+
 export default Mixin.create({
   uploading: false,
   uploadProgress: 0,

app/assets/javascripts/discourse/tests/unit/lib/uppy-checksum-plugin-test.js

@@ -10,10 +10,17 @@ class FakeUppy {
       "uppy-test/file/vv2/xvejg5w/blah/png-1d-1d-2v-1d-1e-image/jpeg-9043429-1624921727764": {
         meta: {},
         data: createFile("test1.png"),
+        size: 1024,
       },
       "uppy-test/file/blah1/ads37x2/blah1/png-1d-1d-2v-1d-1e-image/jpeg-99999-1837921727764": {
         meta: {},
         data: createFile("test2.png"),
+        size: 2048,
+      },
+      "uppy-test/file/mnb3/jfhrg43x/blah3/png-1d-1d-2v-1d-1e-image/jpeg-111111-1837921727764": {
+        meta: {},
+        data: createFile("test2.png"),
+        size: 209715200,
       },
     };
   }
@@ -62,8 +69,8 @@ module("Unit | Utility | UppyChecksum Plugin", function () {
     plugin.uppy.preprocessors[0]([fileId]).then(() => {
       assert.equal(
         plugin.uppy.emitted.length,
-        0,
-        "no events were fired by the checksum plugin because it returned early"
+        1,
+        "only the complete event was fired by the checksum plugin because it skipped the file"
       );
       done();
     });
@@ -85,8 +92,8 @@ module("Unit | Utility | UppyChecksum Plugin", function () {
     plugin.uppy.preprocessors[0]([fileId]).then(() => {
       assert.equal(
         plugin.uppy.emitted.length,
-        0,
-        "no events were fired by the checksum plugin because it returned early"
+        1,
+        "only the complete event was fired by the checksum plugin because it skipped the file"
       );
       done();
     });
@@ -106,13 +113,32 @@ module("Unit | Utility | UppyChecksum Plugin", function () {
     plugin.uppy.preprocessors[0]([fileId]).then(() => {
       assert.equal(
         plugin.uppy.emitted.length,
-        0,
-        "no events were fired by the checksum plugin because it returned early"
+        1,
+        "only the complete event was fired by the checksum plugin because it skipped the file"
       );
       done();
     });
   });
 
+  test("it does nothing if the file is > 100MB", function (assert) {
+    const capabilities = {};
+    const fakeUppy = new FakeUppy();
+    const plugin = new UppyChecksum(fakeUppy, {
+      capabilities,
+    });
+    plugin.install();
+    const done = assert.async();
+
+    const fileId =
+      "uppy-test/file/mnb3/jfhrg43x/blah3/png-1d-1d-2v-1d-1e-image/jpeg-111111-1837921727764";
+    plugin.uppy.preprocessors[0]([fileId]).then(() => {
+      assert.equal(plugin.uppy.emitted[0].event, "preprocess-progress");
+      assert.equal(plugin.uppy.emitted[1].event, "preprocess-complete");
+      assert.equal(plugin.uppy.getFile(fileId).meta.sha1_checksum, null);
+      done();
+    });
+  });
+
   test("it gets a sha1 hash of each file and adds it to the file meta", function (assert) {
     const capabilities = {};
     const fakeUppy = new FakeUppy();