Convert codebase to asynchronous code (reroll v2) (#363)

* Remove the synchronous functions and convert upload() and uploadNextChunk() (and upper-level resume(), pause(), cancel(), removeFile(), and retry()) to async
* Remove the need to call readFinished() from the `readFileFn` (only returns the data)
* Make FlowFile async-first and use a couple of Jasmine expectAsync()
* Maintain event flow consistency
* tests: Convert tests into async/await
* tests: added a test about async file-added hook
* doc: Adapt documentation

Co-authored-by: ilessiivi <[email protected]>
Co-authored-by: Raphaël Droz <[email protected]>

Author: 3 people

README.md

@@ -126,7 +126,7 @@ function, it will be passed a FlowFile, a FlowChunk and isTest boolean (Default:
 * `testChunks` Make a GET request to the server for each chunks to see if it already exists. If implemented on the server-side, this will allow for upload resumes even after a browser crash or even a computer restart. (Default: `true`)
 * `preprocess` Optional function to process each chunk before testing & sending. To the function it will be passed the chunk as parameter, and should call the `preprocessFinished` method on the chunk when finished. (Default: `null`)
 * `changeRawDataBeforeSend` Optional function to change Raw Data just before the XHR Request can be sent for each chunk. To the function, it will be passed the chunk and the data as a Parameter. Return the data which will be then sent to the XHR request without further modification. (Default: `null`). This is helpful when using FlowJS with [Google Cloud Storage](https://cloud.google.com/storage/docs/json_api/v1/how-tos/multipart-upload). Usage example can be seen [#276](https://github.com/flowjs/flow.js/pull/276). (For more, check issue [#170](https://github.com/flowjs/flow.js/issues/170)).
-* `initFileFn` Optional function to initialize the fileObject. To the function it will be passed a FlowFile and a FlowChunk arguments.
+* `initFileFn` Optional (asynchronous) function to initialize the fileObject. To the function it will be passed a FlowFile and a FlowChunk arguments.
 * `readFileFn` Optional function wrapping reading operation from the original file. To the function it will be passed the FlowFile, the startByte and endByte, the fileType and the FlowChunk.
 * `generateUniqueIdentifier` Override the function that generates unique identifiers for each file. (Default: `null`)
 * `maxChunkRetries` The maximum number of retries for a chunk before the upload is failed. Valid values are any positive integer and `undefined` for no limit. (Default: `0`)
@@ -167,24 +167,18 @@ parameter must be adjusted together with `progressCallbacksInterval` parameter.
     * `.off()` All events are removed.
     * `.off(event)` Remove all callbacks of specific event.
     * `.off(event, callback)` Remove specific callback of event. `callback` should be a `Function`.
-* `.upload()` Start or resume uploading.
-* `.pause()` Pause uploading.
-* `.resume()` Resume uploading.
-* `.cancel()` Cancel upload of all `FlowFile` objects and remove them from the list.
+* `.upload()` [async] Start or resume uploading.
+* `.pause()` [async] Pause uploading.
+* `.resume()` [async] Resume uploading.
+* `.cancel()` [asyc] Cancel upload of all `FlowFile` objects and remove them from the list.
 * `.progress()` Returns a float between 0 and 1 indicating the current upload progress of all files.
 * `.isUploading()` Returns a boolean indicating whether or not the instance is currently uploading anything.
-* `.addFile(file, event = null, initFileFn = undefined)` Add a HTML5 File object to the list of files.
+* `.addFile(file, event = null, initFileFn = undefined)` [async] Add a HTML5 File object to the list of files.
     * Accept the same `event` and `initFileFn` parameters thant `addFiles` which is used under the hood.
-* `.addFiles([files], event = null, initFileFn = undefined)` Add multiple File objects to the list of files.
+* `.addFiles([files], event = null, initFileFn = undefined)` [async] Add multiple File objects to the list of files and returns the promise of the corresponding FlowFiles.
     * `event` The optional event that trigger the addition (for internal purposes)
-    * `initFileFn` An override of Flow.initFileFn
-* `.asyncAddFile(file, event = null, initFileFn = undefined)` Add a HTML5 File object to the list of files.
-* `.asyncAddFiles([files], event = null, initFileFn = undefined)` Add multiple File objects to the list of files.
-    * `asyncAddFile` and `asyncAddFiles` rely on the same parameters than they non-async counterparts with one
-      difference: They accept an asynchronous `initFileFn` file function and return, in a promise, the corresponding FlowFiles.
-    * Note: Calling `asyncAddFile` or `asyncAddFiles` with no `initFileFn` being defined is aimed identical to there non-async
-      counterpart but this may change in the future [TBD].
-* `.removeFile(file)` Cancel upload of a specific `FlowFile` object on the list from the list.
+    * `initFileFn` An [async] override of Flow.initFileFn
+* `.removeFile(file)` [asyc] Cancel upload of a specific `FlowFile` object on the list from the list.
 * `.getFromUniqueIdentifier(uniqueIdentifier)` Look up a `FlowFile` object by its unique identifier.
 * `.getSize()` Returns the total size of the upload in bytes.
 * `.sizeUploaded()` Returns the total size uploaded of all files in bytes.
@@ -207,27 +201,16 @@ Events are native, synchronous and provide information about the internal state
 
 #### Processing hooks
 
-Hooks allows for either synchronous or asynchronous operations and allow altering the regular processing of the file(s) from addition to upload completion. 
-It's user responsability to use or not the `async` version of `(async)?addFile` and `(async)?addFiles` according to the behavior of its processing hooks.
- (Defining `async` callbacks for the `asyncAddFile(s)`)
-
+Hooks allows for either (possibly asynchronous) operations and allow altering the regular processing of the file(s) from addition to upload completion.
 * `file-added(<FlowFile> file, event) : null` This event is also called before file is added to upload queue and after it's been fully initialized. `event` is the browser `event` object from when the file was added.
 * `files-added([<FlowFile> files], event) : null` Same as `file-added`, but used for multiple file validation.
 * `files-submitted([<FlowFile> files], event) : null` Same as `files-added`, but happens after the file is added to upload queue. Can be used to start upload of currently added files.
 * `filter-file(<FlowFile> file, event) : boolean` The boolean return value decide whether this particular file must be processed or ignored.
 
 ### Hooks and events format
-- Events and hooks name are case-sensitive, snake-cased and return CustomEvent passed straight to `Flow.on()` callback.
+- Events and hooks name are case-sensitive, snake-cased.
+- In the case of events, a CustomEvent passed straight to callback passed to `Flow.on()`.
 - Sample use `flow.on('file-removed', ({detail: [file]}) => { ... });`
-- In an attempt of backward compatibility, some support of camelCase events exist:
-```
-flow.on('filesAdded', async (files, event) => { // v2 events prototype
-   if (files instanceof CustomEvent) { // Handle v3+ events
-      var [files, event] = files.detail;
-   }
-   // Do something with files
-});
-```
 
 ### FlowFile
 FlowFile constructor can be accessed in `Flow.FlowFile`.
@@ -250,11 +233,12 @@ FlowFile constructor can be accessed in `Flow.FlowFile`.
 
 * `.progress(relative)` Returns a float between 0 and 1 indicating the current upload progress of the file. If `relative` is `true`, the value is returned relative to all files in the Flow.js instance.
 * `.pause()` Pause uploading the file.
-* `.resume()` Resume uploading the file.
-* `.cancel()` Abort uploading the file and delete it from the list of files to upload.
-* `.retry()` Retry uploading the file.
-* `.bootstrap()` Rebuild the state of a `FlowFile` object, including reassigning chunks and XMLHttpRequest instances.
+* `.resume()` [async] Resume uploading the file.
+* `.cancel()` [async] Abort uploading the file and delete it from the list of files to upload.
+* `.retry()` [async] Retry uploading the file.
+* `.bootstrap()` [async / internal use only] Rebuild the state of a `FlowFile` object, including reassigning chunks and XMLHttpRequest instances.
 * `.isUploading()` Returns a boolean indicating whether file chunks is uploading.
+* `.isReading()` Returns a boolean indicating whether the file/stream is being read.
 * `.isComplete()` Returns a boolean indicating whether the file has completed uploading and received a server response.
 * `.sizeUploaded()` Returns size uploaded in bytes.
 * `.timeRemaining()` Returns remaining time to finish upload file in seconds. Accuracy is based on average speed. If speed is zero, time remaining will be equal to positive infinity `Number.POSITIVE_INFINITY`

src/AsyncFlowFile.js

@@ -5,7 +5,7 @@
 /**
  * Hooks exist to allow users to alter flow.js file processing. This is intended for users relying on a dropzone and other higher-level components
  * when the flow between `addFiles()` and `upload()` is hardly configurable.
- * Users calling `await flow.asyncAddFiles()` have more room for customization before calling `upload();` without having to rely upon hooks.
+ * Users calling `await flow.addFiles()` have more room for customization before calling `upload();` without having to rely upon hooks.
  *
  * Hooks can *alter* the parameters they receive (javascript pass-by-reference rules will apply).
  * For example, the `file-added` hook receives a `flowfile` parameter. `delete flowfile` or `flowfile = {}` have no effect
@@ -49,20 +49,18 @@ const EVENTS = [
  * This class:
  * - add EventListener support to an object.
  * - wrap EventListener attachment in order to ease their removal
- * - add the concept of processing hooks similar to native events (explained below) 
+ * - add the concept of processing hooks similar to native events (explained below)
  *
  * The file is organized in three parts:
  * 1. isHook, isFilter, isEvent, on, off
- *    wrap the above concept and offer an unified interface. Whether a callback
- *    apply to a hook or an event is determined by its name (and the "async" nature of
- *    the callback).
+ *    wrap the above concept and offer a unified interface. Whether a callback
+ *    apply to a hook or an event is determined by its name.
  *
  * 2. Events: addEventListener, removeEventListener and *emit()*
  *    apply to addition/removal/dispatching of *events*.
  *
  * 3. Hooks: addHook, hasHook, removeHook apply to addition/removal of *hooks*.
  *    - *hook()* trigger the hook execution.
- *    - *aHook()* is the async counterpart.
  */
 
 EventTarget.prototype._addEventListener = EventTarget.prototype.addEventListener;
@@ -77,8 +75,7 @@ export default class extends EventTarget {
    *
    * Each key is check against an hardcoded to list to defined whether:
    * - it's a "native" CustomEvent (dispatched asynchronously dirsregarding its value)
-   * - it's a known event (whether a "filter" or an "action", and in this case, whether
-   *   each callback is asynchronous or not.
+   * - it's a known event (whether a "filter" or an "action")
    * @type {}
    */
   constructor(hooks_events = {}) {
@@ -252,8 +249,7 @@ export default class extends EventTarget {
    * ### HOOKS ###
    */
   addHook(event, callback, options) {
-    var isAsync = callback.constructor.name === 'AsyncFunction',
-        target = isAsync ? this._asyncHooks : this._hooks;
+    var target = this._hooks;
     if (!target.hasOwnProperty(event)) {
       target[event] = [];
     }
@@ -262,7 +258,7 @@ export default class extends EventTarget {
 
   hasHook(async, events) {
     events = typeof events === 'string' ? [events] : events || [];
-    var target = async ? this._asyncHooks : this._hooks;
+    var target = this._hooks;
     for (let [k, v] of Object.entries(target)) {
       if (events.length > 0 && ! events.includes(k)) {
         continue;
@@ -288,55 +284,18 @@ export default class extends EventTarget {
 
     if (event && event != '*') {
       if (callback) {
-        var isAsync = callback.constructor.name === 'AsyncFunction',
-            target = isAsync ? this._asyncHooks : this._hooks;
+        var target = this._hooks;
         if (target.hasOwnProperty(event)) {
           arrayRemove(target[event], callback);
         }
       } else {
         delete this._hooks[event];
-        delete this._asyncHooks[event];
       }
     } else {
       this._hooks = {};
-      this._asyncHooks = {};
     }
   }
 
-  /**
-   * Run a synchronous hook (action or filter).
-   *
-   * @param {string} event event name
-   * @param {...} args arguments of a callback
-   *
-   * @return {bool} In the case of *filters*, indicates whether processing must continue.
-   * @return null   In the case of *actions*.
-   */
-  hook(name, ...args) {
-    let value,
-        preventDefault = false,
-        isFilter = this.isFilter(name),
-        callbacks = this._hooks[name] || [];
-
-    for (let callback of callbacks) {
-      // console.log(`[event] Fire hook "${name}"${args.length ? ' with ' + args.length + ' arguments' : ''}`);
-      value = callback.apply(this, args);
-      if (name === 'file-added' && value === false) {
-        console.warn('In Flow.js 3.x, file-added event is an action rather than a filter. Return value is ignored but removing the `file` property allows to skip an enqueued file.');
-      }
-
-      if (isFilter) {
-        // console.log(`[filter-event] ${event} returned:`, item.value);
-        preventDefault |= (value === false);
-      } else {
-        // Changes happen by reference. We ignore iterator.next().value.
-      }
-    }
-
-    this.emitCatchAll(name, ...args);
-    return isFilter ? !preventDefault : null;
-  }
-
   /**
    * Run an asynchronous hook (action or filter).
    *
@@ -346,19 +305,18 @@ export default class extends EventTarget {
    * @return {bool} In the case of *filters*, indicates whether processing must continue.
    * @return {mixed} In the case of *actions*: The first argument (possibly modified by hooks).
    */
-  async aHook(name, ...args) {
-    const calls = this._asyncHooks[name] || [],
-          isFilter = this.isFilter(name);
+  async hook(name, ...args) {
+    let calls = this._hooks[name] || [],
+        isFilter = this.isFilter(name),
+        returns = isFilter ? true : args[0];
 
-    if (! calls.length) {
-      return isFilter ? true : args[0];
+    if (calls.length) {
+      // console.log(`[event] Fire ${calls.length} async hook for "${name}"${args.length ? ' with ' + args.length + ' arguments' : ''}`);
+      const results = await Promise.all(calls.map(e => e.apply(this, args)));
+      returns = isFilter ? !results.includes(false) : results;
     }
 
-    // console.log(`[event] Fire ${calls.length} async hook for "${name}"${args.length ? ' with ' + args.length + ' arguments' : ''}`);
-    const returns = await Promise.all(calls.map(e => e.apply(this, args)));
-
     this.emitCatchAll(name, ...args);
-
-    return isFilter ? !returns.includes(false) : returns;
+    return returns;
   }
 }