Skip to content

Commit

Permalink
DEV: Improve uppy plugin base and large file handling
Browse files Browse the repository at this point in the history
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 discourse/discourse-encrypt#141
  • Loading branch information
martin-brennan committed Sep 20, 2021
1 parent 37a3bf9 commit 44144a3
Show file tree
Hide file tree
Showing 7 changed files with 108 additions and 27 deletions.
11 changes: 9 additions & 2 deletions 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";
Expand Down Expand Up @@ -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)
Expand Down
Expand Up @@ -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);
Expand Down
42 changes: 38 additions & 4 deletions app/assets/javascripts/discourse/app/lib/uppy-plugin-base.js
@@ -1,4 +1,5 @@
import { BasePlugin } from "@uppy/core";
import { Promise } from "rsvp";
import { warn } from "@ember/debug";

export class UppyPluginBase extends BasePlugin {
Expand All @@ -8,7 +9,14 @@ 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) {
// eslint-disable-next-line no-console
console.log(`[${this.id}] ${msg}`);
}
}

_getFile(fileId) {
Expand Down Expand Up @@ -41,10 +49,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, 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);
}

_emitComplete(file) {
this.uppy.emit("preprocess-complete", this.id, file);
_skipAll(file) {
return this._emitAllComplete(file, true);
}
}
Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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 };
});
Expand All @@ -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;
});
},
Expand Down Expand Up @@ -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);
}
},
});
23 changes: 17 additions & 6 deletions app/assets/javascripts/discourse/app/mixins/extendable-uploader.js
Expand Up @@ -80,8 +80,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++;

Expand All @@ -90,16 +92,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();
}
});
Expand Down Expand Up @@ -168,4 +172,11 @@ export default Mixin.create({
}
}
},

_consoleDebug(msg) {
if (this.siteSettings.enable_upload_debug_mode) {
// eslint-disable-next-line no-console
console.log(msg);
}
},
});
2 changes: 2 additions & 0 deletions app/assets/javascripts/discourse/app/mixins/uppy-upload.js
Expand Up @@ -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,
Expand Down
Expand Up @@ -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,
},
};
}
Expand Down Expand Up @@ -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();
});
Expand All @@ -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();
});
Expand All @@ -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();
Expand Down

0 comments on commit 44144a3

Please sign in to comment.