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

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
This commit is contained in:
Martin Brennan 2021-09-21 08:41:07 +10:00 committed by GitHub
parent 37a3bf9c11
commit 02f7035cbe
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 108 additions and 27 deletions

View File

@ -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)

View File

@ -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);

View File

@ -1,4 +1,5 @@
import { BasePlugin } from "@uppy/core";
import { Promise } from "rsvp";
import { warn } from "@ember/debug";
export class UppyPluginBase extends BasePlugin {
@ -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) {
@ -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) {
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);
}
}

View File

@ -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);
}
},
});

View File

@ -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++;
@ -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();
}
});
@ -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);
}
},
});

View File

@ -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,

View File

@ -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();