From 9873a942e3d2df367717f119c591492c690d291d Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Mon, 6 Sep 2021 08:22:50 +1000 Subject: [PATCH] DEV: Add addComposerUploadPreProcessor to plugin-api (#14222) This new interface will be used explicitly to add upload preprocessors in the form of uppy plugins. These will be run for each upload in the composer (dependent on the logic of the plugin itself), before the UppyChecksum plugin is finally run. Since discourse-encrypt uses the existing addComposerUploadHandler API for essentially preprocessing an upload and not uploading it to a different place, it will be the first plugin to use this interface, along with the register-media-optimization-upload-processor initializer in core. Related https://github.com/discourse/discourse-encrypt/pull/131. --- .../app/components/composer-editor.js | 19 ++++++ .../discourse/app/controllers/composer.js | 12 +--- ...ter-media-optimization-upload-processor.js | 39 ++++++++---- .../discourse/app/lib/plugin-api.js | 34 +++++++++++ .../app/mixins/composer-upload-uppy.js | 60 ++++++++++++------- .../app/services/media-optimization-worker.js | 8 ++- .../discourse/tests/helpers/qunit-helpers.js | 2 + 7 files changed, 131 insertions(+), 43 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/composer-editor.js b/app/assets/javascripts/discourse/app/components/composer-editor.js index eb2d0798538..bc351f69270 100644 --- a/app/assets/javascripts/discourse/app/components/composer-editor.js +++ b/app/assets/javascripts/discourse/app/components/composer-editor.js @@ -3,6 +3,7 @@ import { authorizesAllExtensions, authorizesOneOrMoreImageExtensions, } from "discourse/lib/uploads"; +import { BasePlugin } from "@uppy/core"; import { resolveAllShortUrls } from "pretty-text/upload-short-url"; import { caretPosition, @@ -61,6 +62,23 @@ export function cleanUpComposerUploadProcessor() { uploadProcessorActions = {}; } +let uploadPreProcessors = []; +export function addComposerUploadPreProcessor(pluginClass, optionsResolverFn) { + if (!(pluginClass.prototype instanceof BasePlugin)) { + throw new Error( + "Composer upload preprocessors must inherit from the Uppy BasePlugin class." + ); + } + + uploadPreProcessors.push({ + pluginClass, + optionsResolverFn, + }); +} +export function cleanUpComposerUploadPreProcessor() { + uploadPreProcessors = []; +} + let uploadMarkdownResolvers = []; export function addComposerUploadMarkdownResolver(resolver) { uploadMarkdownResolvers.push(resolver); @@ -79,6 +97,7 @@ export default Component.extend(ComposerUpload, { uploadMarkdownResolvers, uploadProcessorActions, uploadProcessorQueue, + uploadPreProcessors, uploadHandlers, @discourseComputed("composer.requiredCategoryMissing") diff --git a/app/assets/javascripts/discourse/app/controllers/composer.js b/app/assets/javascripts/discourse/app/controllers/composer.js index 43dad347e20..39418656ce1 100644 --- a/app/assets/javascripts/discourse/app/controllers/composer.js +++ b/app/assets/javascripts/discourse/app/controllers/composer.js @@ -1,5 +1,4 @@ import Composer, { SAVE_ICONS, SAVE_LABELS } from "discourse/models/composer"; -import { warn } from "@ember/debug"; import Controller, { inject as controller } from "@ember/controller"; import EmberObject, { action, computed } from "@ember/object"; import { alias, and, or, reads } from "@ember/object/computed"; @@ -285,17 +284,10 @@ export default Controller.extend({ return option; }, - @discourseComputed("model.isEncrypted") - composerComponent(isEncrypted) { + @discourseComputed() + composerComponent() { const defaultComposer = "composer-editor"; if (this.siteSettings.enable_experimental_composer_uploader) { - if (isEncrypted) { - warn( - "Uppy cannot be used for composer uploads until upload handlers are developed, falling back to composer-editor.", - { id: "composer" } - ); - return defaultComposer; - } return "composer-editor-uppy"; } return defaultComposer; diff --git a/app/assets/javascripts/discourse/app/initializers/register-media-optimization-upload-processor.js b/app/assets/javascripts/discourse/app/initializers/register-media-optimization-upload-processor.js index 1a2e4276f6e..7b6bfbff653 100644 --- a/app/assets/javascripts/discourse/app/initializers/register-media-optimization-upload-processor.js +++ b/app/assets/javascripts/discourse/app/initializers/register-media-optimization-upload-processor.js @@ -1,4 +1,8 @@ -import { addComposerUploadProcessor } from "discourse/components/composer-editor"; +import { + addComposerUploadPreProcessor, + addComposerUploadProcessor, +} from "discourse/components/composer-editor"; +import UppyMediaOptimization from "discourse/lib/uppy-media-optimization-plugin"; export default { name: "register-media-optimization-upload-processor", @@ -6,15 +10,30 @@ export default { initialize(container) { let siteSettings = container.lookup("site-settings:main"); if (siteSettings.composer_media_optimization_image_enabled) { - addComposerUploadProcessor( - { action: "optimizeJPEG" }, - { - optimizeJPEG: (data, opts) => - container - .lookup("service:media-optimization-worker") - .optimizeImage(data, opts), - } - ); + if (!siteSettings.enable_experimental_composer_uploader) { + addComposerUploadProcessor( + { action: "optimizeJPEG" }, + { + optimizeJPEG: (data, opts) => + container + .lookup("service:media-optimization-worker") + .optimizeImage(data, opts), + } + ); + } else { + addComposerUploadPreProcessor( + UppyMediaOptimization, + ({ isMobileDevice }) => { + return { + optimizeFn: (data, opts) => + container + .lookup("service:media-optimization-worker") + .optimizeImage(data, opts), + runParallel: !isMobileDevice, + }; + } + ); + } } }, }; diff --git a/app/assets/javascripts/discourse/app/lib/plugin-api.js b/app/assets/javascripts/discourse/app/lib/plugin-api.js index 64299bcd5a8..f4aa7897fb3 100644 --- a/app/assets/javascripts/discourse/app/lib/plugin-api.js +++ b/app/assets/javascripts/discourse/app/lib/plugin-api.js @@ -1,6 +1,7 @@ import ComposerEditor, { addComposerUploadHandler, addComposerUploadMarkdownResolver, + addComposerUploadPreProcessor, addComposerUploadProcessor, } from "discourse/components/composer-editor"; import { @@ -1027,6 +1028,39 @@ class PluginApi { addComposerUploadProcessor(queueItem, actionItem); } + /** + * Registers a pre-processor for file uploads in the form + * of an Uppy preprocessor plugin. + * + * See https://uppy.io/docs/writing-plugins/ for the Uppy + * documentation, but other examples of preprocessors in core + * can be found in UppyMediaOptimization and UppyChecksum. + * + * Useful for transforming to-be uploaded files client-side. + * + * Example: + * + * api.addComposerUploadPreProcessor(UppyMediaOptimization, ({ composerModel, composerElement, capabilities, isMobileDevice }) => { + * return { + * composerModel, + * composerElement, + * capabilities, + * isMobileDevice, + * someOption: true, + * someFn: () => {}, + * }; + * }); + * + * @param {BasePlugin} pluginClass The uppy plugin class to use for the preprocessor. + * @param {Function} optionsResolverFn This function should return an object which is passed into the constructor + * of the uppy plugin as the options argument. The object passed to the function + * contains references to the composer model, element, the capabilities of the + * browser, and isMobileDevice. + */ + addComposerUploadPreProcessor(pluginClass, optionsResolverFn) { + addComposerUploadPreProcessor(pluginClass, optionsResolverFn); + } + /** * Registers a function to generate Markdown after a file has been uploaded. * 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 788a4416eec..d081f20393e 100644 --- a/app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js +++ b/app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js @@ -2,7 +2,6 @@ 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"; @@ -228,14 +227,8 @@ export default Mixin.create({ } }); - this._setupPreprocessing(); - - // It is important that the UppyChecksum preprocessor is the last one to - // be added; the preprocessors are run in order and since other preprocessors - // may modify the file (e.g. the UppyMediaOptimization one), we need to - // checksum once we are sure the file data has "settled". - this._uppyInstance.use(UppyChecksum, { capabilities: this.capabilities }); - this._uppyInstance.use(DropTarget, { target: this.element }); + this._setupPreProcessors(); + this._setupUIPlugins(); // TODO (martin) Need a more automatic way to do this for preprocessor // plugins like UppyChecksum and UppyMediaOptimization so people don't @@ -261,20 +254,31 @@ export default Mixin.create({ } }, - _setupPreprocessing() { - Object.keys(this.uploadProcessorActions).forEach((action) => { - switch (action) { - case "optimizeJPEG": - this._uppyInstance.use(UppyMediaOptimization, { - optimizeFn: this.uploadProcessorActions[action], - runParallel: !this.site.isMobileDevice, - }); - this._trackPreProcessorStatus(UppyMediaOptimization); - break; - } + _setupPreProcessors() { + this.uploadPreProcessors.forEach(({ pluginClass, optionsResolverFn }) => { + this._uppyInstance.use( + pluginClass, + optionsResolverFn({ + composerModel: this.composerModel, + composerElement: this.composerElement, + capabilities: this.capabilities, + isMobileDevice: this.site.isMobileDevice, + }) + ); + this._trackPreProcessorStatus(pluginClass); }); + // It is important that the UppyChecksum preprocessor is the last one to + // be added; the preprocessors are run in order and since other preprocessors + // may modify the file (e.g. the UppyMediaOptimization one), we need to + // checksum once we are sure the file data has "settled". + this._uppyInstance.use(UppyChecksum, { capabilities: this.capabilities }); + this._uppyInstance.on("preprocess-progress", (pluginClass, file) => { + this._debugLog( + `[${pluginClass}] processing file ${file.name} (${file.id})` + ); + this._preProcessorStatus[pluginClass].activeProcessing++; let placeholderData = this.placeholders[file.id]; placeholderData.processingPlaceholder = `[${I18n.t( @@ -292,6 +296,10 @@ export default Mixin.create({ }); this._uppyInstance.on("preprocess-complete", (pluginClass, file) => { + this._debugLog( + `[${pluginClass}] completed processing file ${file.name} (${file.id})` + ); + let placeholderData = this.placeholders[file.id]; this.appEvents.trigger( `${this.eventPrefix}:replace-text`, @@ -313,6 +321,7 @@ export default Mixin.create({ isProcessingUpload: false, isCancellable: true, }); + this._debugLog("All upload preprocessors complete."); this.appEvents.trigger( `${this.eventPrefix}:uploads-preprocessing-complete` ); @@ -321,6 +330,10 @@ export default Mixin.create({ }); }, + _setupUIPlugins() { + this._uppyInstance.use(DropTarget, { target: this.element }); + }, + _uploadFilenamePlaceholder(file) { const filename = this._filenamePlaceholder(file); @@ -593,4 +606,11 @@ export default Mixin.create({ showUploadSelector(toolbarEvent) { this.send("showUploadSelector", toolbarEvent); }, + + _debugLog(message) { + if (this.siteSettings.enable_upload_debug_mode) { + // eslint-disable-next-line no-console + console.log(message); + } + }, }); diff --git a/app/assets/javascripts/discourse/app/services/media-optimization-worker.js b/app/assets/javascripts/discourse/app/services/media-optimization-worker.js index f2e82584761..37f238776eb 100644 --- a/app/assets/javascripts/discourse/app/services/media-optimization-worker.js +++ b/app/assets/javascripts/discourse/app/services/media-optimization-worker.js @@ -17,9 +17,11 @@ export default class MediaOptimizationWorkerService extends Service { } stopWorker() { - this.logIfDebug("Stopping media-optimization-worker..."); - this.worker.terminate(); - this.worker = null; + if (this.worker) { + this.logIfDebug("Stopping media-optimization-worker..."); + this.worker.terminate(); + this.worker = null; + } } ensureAvailiableWorker() { diff --git a/app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js b/app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js index 6b72aec9425..04e981f7c65 100644 --- a/app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js +++ b/app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js @@ -46,6 +46,7 @@ import { clearNavItems } from "discourse/models/nav-item"; import { cleanUpComposerUploadHandler, cleanUpComposerUploadMarkdownResolver, + cleanUpComposerUploadPreProcessor, cleanUpComposerUploadProcessor, } from "discourse/components/composer-editor"; import { resetLastEditNotificationClick } from "discourse/models/post-stream"; @@ -288,6 +289,7 @@ export function acceptance(name, optionsOrCallback) { cleanUpComposerUploadHandler(); cleanUpComposerUploadProcessor(); cleanUpComposerUploadMarkdownResolver(); + cleanUpComposerUploadPreProcessor(); resetLastEditNotificationClick(); app._runInitializer("instanceInitializers", (initName, initializer) => { if (initializer && initializer.teardown) {