FEATURE: Uppy direct S3 multipart uploads in composer (#14051)

FEATURE: Uppy direct S3 multipart uploads in composer (#14051)

This pull request introduces the endpoints required, and the JavaScript functionality in the ComposerUppyUpload mixin, for direct S3 multipart uploads. There are four new endpoints in the uploads controller:

  • create-multipart.json - Creates the multipart upload in S3 along with an ExternalUploadStub record, storing information about the file in the same way as generate-presigned-put.json does for regular direct S3 uploads

  • batch-presign-multipart-parts.json - Takes a list of part numbers and the unique identifier for an ExternalUploadStub record, and generates the presigned URLs for those parts if the multipart upload still exists and if the user has permission to access that upload

  • complete-multipart.json - Completes the multipart upload in S3. Needs the full list of part numbers and their associated ETags which are returned when the part is uploaded to the presigned URL above. Only works if the user has permission to access the associated ExternalUploadStub record and the multipart upload still exists.

    After we confirm the upload is complete in S3, we go through the regular UploadCreator flow, the same as complete-external-upload.json, and promote the temporary upload S3 into a full Upload record, moving it to its final destination.

  • abort-multipart.json - Aborts the multipart upload on S3 and destroys the ExternalUploadStub record if the user has permission to access that upload.

Also added are a few new columns to ExternalUploadStub:

  • multipart - Whether or not this is a multipart upload
  • external_upload_identifier - The “upload ID” for an S3 multipart upload
  • filesize - The size of the file when the create-multipart.json or generate-presigned-put.json is called. This is used for validation.

When the user completes a direct S3 upload, either regular or multipart, we take the filesize that was captured when the ExternalUploadStub was first created and compare it with the final Content-Length size of the file where it is stored in S3. Then, if the two do not match, we throw an error, delete the file on S3, and ban the user from uploading files for N (default 5) minutes. This would only happen if the user uploads a different file than what they first specified, or in the case of multipart uploads uploaded larger chunks than needed. This is done to prevent abuse of S3 storage by bad actors.

Also included in this PR is an update to vendor/uppy.js. This has been built locally from the latest uppy source at @uppy/aws-s3-multipart: add support for presigned URL batching (#3056) · transloadit/uppy@d613b84 · GitHub. This must be done so that I can get my multipart upload changes into Discourse. When the Uppy team cuts a proper release, we can bump the package.json versions instead.

diff --git a/app/assets/javascripts/discourse-shims.js b/app/assets/javascripts/discourse-shims.js
index 71ce847..3f038c1 100644
--- a/app/assets/javascripts/discourse-shims.js
+++ b/app/assets/javascripts/discourse-shims.js
@@ -32,7 +32,7 @@ define("@popperjs/core", ["exports"], function (__exports__) {
 
 define("@uppy/core", ["exports"], function (__exports__) {
   __exports__.default = window.Uppy.Core;
-  __exports__.Plugin = window.Uppy.Plugin;
+  __exports__.BasePlugin = window.Uppy.BasePlugin;
 });
 
 define("@uppy/aws-s3", ["exports"], function (__exports__) {
diff --git a/app/assets/javascripts/discourse/app/lib/uppy-checksum-plugin.js b/app/assets/javascripts/discourse/app/lib/uppy-checksum-plugin.js
index 4cd0cea..3e74b36 100644
--- a/app/assets/javascripts/discourse/app/lib/uppy-checksum-plugin.js
+++ b/app/assets/javascripts/discourse/app/lib/uppy-checksum-plugin.js
@@ -1,8 +1,8 @@
-import { Plugin } from "@uppy/core";
+import { BasePlugin } from "@uppy/core";
 import { warn } from "@ember/debug";
 import { Promise } from "rsvp";
 
-export default class UppyChecksum extends Plugin {
+export default class UppyChecksum extends BasePlugin {
   constructor(uppy, opts) {
     super(uppy, opts);
     this.id = opts.id || "uppy-checksum";
diff --git a/app/assets/javascripts/discourse/app/lib/uppy-media-optimization-plugin.js b/app/assets/javascripts/discourse/app/lib/uppy-media-optimization-plugin.js
index 3ba4cc8..dcf3c31 100644
--- a/app/assets/javascripts/discourse/app/lib/uppy-media-optimization-plugin.js
+++ b/app/assets/javascripts/discourse/app/lib/uppy-media-optimization-plugin.js
@@ -1,8 +1,8 @@
-import { Plugin } from "@uppy/core";
+import { BasePlugin } from "@uppy/core";
 import { warn } from "@ember/debug";
 import { Promise } from "rsvp";
 
-export default class UppyMediaOptimization extends Plugin {
+export default class UppyMediaOptimization extends BasePlugin {
   constructor(uppy, opts) {
     super(uppy, opts);
     this.id = opts.id || "uppy-media-optimization";
@@ -30,7 +30,10 @@ export default class UppyMediaOptimization extends Plugin {
             id: "discourse.uppy-media-optimization",
           });
         } else {
-          this.uppy.setFileState(fileId, { data: optimizedFile });
+          this.uppy.setFileState(fileId, {
+            data: optimizedFile,
+            size: optimizedFile.size,
+          });
         }
         this.uppy.emit("preprocess-complete", this.pluginClass, file);
       })
diff --git a/app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js b/app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js
index d7b88d7..97cd755 100644
--- a/app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js
+++ b/app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js
@@ -1,10 +1,12 @@
 import Mixin from "@ember/object/mixin";
+import { ajax } from "discourse/lib/ajax";
 import { deepMerge } from "discourse-common/lib/object";
 import UppyChecksum from "discourse/lib/uppy-checksum-plugin";
 import UppyMediaOptimization from "discourse/lib/uppy-media-optimization-plugin";
 import Uppy from "@uppy/core";
 import DropTarget from "@uppy/drop-target";
 import XHRUpload from "@uppy/xhr-upload";
+import AwsS3Multipart from "@uppy/aws-s3-multipart";
 import { warn } from "@ember/debug";
 import I18n from "I18n";
 import getURL from "discourse-common/lib/get-url";
@@ -70,6 +72,7 @@ export default Mixin.create({
 
   _bindUploadTarget() {
     this.placeholders = {};
+    this._inProgressUploads = 0;
     this._preProcessorStatus = {};
     this.fileInputEl = document.getElementById("file-uploader");
     const isPrivateMessage = this.get("composer.privateMessage");
@@ -140,9 +143,12 @@ export default Mixin.create({
     // name for the preprocess-X events.
     this._trackPreProcessorStatus(UppyChecksum);
 
-    // TODO (martin) support for direct S3 uploads will come later, for now
-    // we just want the regular /uploads.json endpoint to work well
-    this._useXHRUploads();
+    // hidden setting like enable_experimental_image_uploader
+    if (this.siteSettings.enable_direct_s3_uploads) {
+      this._useS3MultipartUploads();
+    } else {
+      this._useXHRUploads();
+    }
 
     // TODO (martin) develop upload handler guidance and an API to use; will
     // likely be using uppy plugins for this
@@ -171,6 +177,7 @@ export default Mixin.create({
       });
 
       files.forEach((file) => {
+        this._inProgressUploads++;
         const placeholder = this._uploadPlaceholder(file);
         this.placeholders[file.id] = {
           uploadPlaceholder: placeholder,
@@ -199,14 +206,7 @@ export default Mixin.create({
       this.appEvents.trigger("composer:upload-success", file.name, upload);
     });
 
-    this._uppyInstance.on("upload-error", (file, error, response) => {
-      this._resetUpload(file, { removePlaceholder: true });
-
-      if (!this.userCancelled) {
-        displayErrorForUpload(response, this.siteSettings, file.name);
-        this.appEvents.trigger("composer:upload-error", file);
-      }
-    });
+    this._uppyInstance.on("upload-error", this._handleUploadError.bind(this));
 
     this._uppyInstance.on("complete", () => {
       this.appEvents.trigger("composer:all-uploads-complete");
@@ -235,6 +235,20 @@ export default Mixin.create({
     this._setupPreprocessing();
   },
 
+  _handleUploadError(file, error, response) {
+    this._inProgressUploads--;
+    this._resetUpload(file, { removePlaceholder: true });
+
+    if (!this.userCancelled) {
+      displayErrorForUpload(response || error, this.siteSettings, file.name);
+      this.appEvents.trigger("composer:upload-error", file);
+    }
+
+    if (this._inProgressUploads === 0) {
+      this._reset();
+    }
+  },
+
   _setupPreprocessing() {
     Object.keys(this.uploadProcessorActions).forEach((action) => {
       switch (action) {
@@ -343,6 +357,99 @@ export default Mixin.create({
     });
   },
 
+  _useS3MultipartUploads() {
+    const self = this;
+
+    this._uppyInstance.use(AwsS3Multipart, {
+      // controls how many simultaneous _chunks_ are uploaded, not files,
+      // which in turn controls the minimum number of chunks presigned
+      // in each batch (limit / 2)
+      //
+      // the default, and minimum, chunk size is 5mb. we can control the
+      // chunk size via getChunkSize(file), so we may want to increase
+      // the chunk size for larger files
+      limit: 10,
+
+      createMultipartUpload(file) {
+        return ajax("/uploads/create-multipart.json", {
+          type: "POST",
+          data: {
+            file_name: file.name,
+            file_size: file.size,
+            upload_type: file.meta.upload_type,
+          },
+          // uppy is inconsistent, an error here fires the upload-error event
+        }).then((data) => {
+          file.meta.unique_identifier = data.unique_identifier;
+          return {
+            uploadId: data.external_upload_identifier,
+            key: data.key,
+          };
+        });
+      },
+
+      prepareUploadParts(file, partData) {
+        return (
+          ajax("/uploads/batch-presign-multipart-parts.json", {
+            type: "POST",
+            data: {
+              part_numbers: partData.partNumbers,
+              unique_identifier: file.meta.unique_identifier,
+            },
+          })
+            .then((data) => {
+              return { presignedUrls: data.presigned_urls };
+            })
+            // uppy is inconsistent, an error here does not fire the upload-error event
+            .catch((err) => {
+              self._handleUploadError(file, err);
+            })
+        );
+      },
+
+      completeMultipartUpload(file, data) {
+        const parts = data.parts.map((part) => {
+          return { part_number: part.PartNumber, etag: part.ETag };
+        });
+        return ajax("/uploads/complete-multipart.json", {
+          type: "POST",

[... diff too long, it was truncated ...]

GitHub sha: d295a16dab994c45e33e1df8da46c337ebd6a917

This commit appears in #14051 which was approved by tgxworld. It was merged by martin.