Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Commit

Permalink
Merge pull request #88 from ckeditor/t/87
Browse files Browse the repository at this point in the history 
Fix: `FileLoader` now accepts `Promise` instead of a `File` instance. Closes #87.

BREAKING CHANGE: The `FileLoader.file` property was changed to a getter which returns a native `Promise` instance instead of a `File` instance. The returned promise resolves to a `File` instance.
  • Loading branch information
@jodator
jodator authored Jan 7, 2019
2 parents 6bd50dd + 041f22b commit 62a8c69
Show file tree
Hide file tree
Showing 3 changed files with 406 additions and 63 deletions.
2 changes: 1 addition & 1 deletion src/filereader.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export default class FileReader {
* Reads the provided file.
*
* @param {File} file Native File object.
* @returns {Promise} Returns a promise that will be resolved with file's content.
* @returns {Promise.<String>} Returns a promise that will be resolved with file's content.
* The promise will be rejected in case of an error or when the reading process is aborted.
*/
read( file ) {
Expand Down
154 changes: 117 additions & 37 deletions src/filerepository.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,14 @@ export default class FileRepository extends Plugin {
this.loaders.on( 'add', () => this._updatePendingAction() );
this.loaders.on( 'remove', () => this._updatePendingAction() );

/**
* Loaders mappings used to retrieve loaders references.
*
* @private
* @member {Map<File|Promise, FileLoader>} #_loadersMap
*/
this._loadersMap = new Map();

/**
* Reference to a pending action registered in a {@link module:core/pendingactions~PendingActions} plugin
* while upload is in progress. When there is no upload then value is `null`.
Expand Down Expand Up @@ -120,32 +128,26 @@ export default class FileRepository extends Plugin {
}

/**
* Returns the loader associated with specified file.
* Returns the loader associated with specified file or promise.
*
* To get loader by id use `fileRepository.loaders.get( id )`.
*
* @param {File} file Native file handle.
* @param {File|Promise.<File>} fileOrPromise Native file or promise handle.
* @returns {module:upload/filerepository~FileLoader|null}
*/
getLoader( file ) {
for ( const loader of this.loaders ) {
if ( loader.file == file ) {
return loader;
}
}

return null;
getLoader( fileOrPromise ) {
return this._loadersMap.get( fileOrPromise ) || null;
}

/**
* Creates a loader instance for the given file.
*
* Requires {@link #createUploadAdapter} factory to be defined.
*
* @param {File} file Native File object.
* @param {File|Promise.<File>} fileOrPromise Native File object or native Promise object which resolves to a File.
* @returns {module:upload/filerepository~FileLoader|null}
*/
createLoader( file ) {
createLoader( fileOrPromise ) {
if ( !this.createUploadAdapter ) {
/**
* You need to enable an upload adapter in order to be able to upload files.
Expand Down Expand Up @@ -174,10 +176,17 @@ export default class FileRepository extends Plugin {
return null;
}

const loader = new FileLoader( file );
loader._adapter = this.createUploadAdapter( loader );
const loader = new FileLoader( Promise.resolve( fileOrPromise ), this.createUploadAdapter );

this.loaders.add( loader );
this._loadersMap.set( fileOrPromise, loader );

// Store also file => loader mapping so loader can be retrieved by file instance returned upon Promise resolution.
if ( fileOrPromise instanceof Promise ) {
loader.file.then( file => {
this._loadersMap.set( file, loader );
} );
}

loader.on( 'change:uploaded', () => {
let aggregatedUploaded = 0;
Expand Down Expand Up @@ -207,15 +216,21 @@ export default class FileRepository extends Plugin {
/**
* Destroys the given loader.
*
* @param {File|module:upload/filerepository~FileLoader} fileOrLoader File associated with that loader or loader
* itself.
* @param {File|Promise|module:upload/filerepository~FileLoader} fileOrPromiseOrLoader File or Promise associated
* with that loader or loader itself.
*/
destroyLoader( fileOrLoader ) {
const loader = fileOrLoader instanceof FileLoader ? fileOrLoader : this.getLoader( fileOrLoader );
destroyLoader( fileOrPromiseOrLoader ) {
const loader = fileOrPromiseOrLoader instanceof FileLoader ? fileOrPromiseOrLoader : this.getLoader( fileOrPromiseOrLoader );

loader._destroy();

this.loaders.remove( loader );

this._loadersMap.forEach( ( value, key ) => {
if ( value === loader ) {
this._loadersMap.delete( key );
}
} );
}

/**
Expand Down Expand Up @@ -252,10 +267,10 @@ class FileLoader {
/**
* Creates a new instance of `FileLoader`.
*
* @param {File} file A native file instance.
* @param {module:upload/filerepository~UploadAdapter} adapter
* @param {Promise.<File>} filePromise A promise which resolves to a file instance.
* @param {Function} uploadAdapterCreator The function which returns {@link module:upload/filerepository~UploadAdapter} instance.
*/
constructor( file, adapter ) {
constructor( filePromise, uploadAdapterCreator ) {
/**
* Unique id of FileLoader instance.
*
Expand All @@ -265,20 +280,20 @@ class FileLoader {
this.id = uid();

/**
* A `File` instance associated with this file loader.
* Additional wrapper over the initial file promise passed to this loader.
*
* @readonly
* @member {File}
* @private
* @member {module:upload/filerepository~FilePromiseWrapper}
*/
this.file = file;
this._filePromiseWrapper = this._createFilePromiseWrapper( filePromise );

/**
* Adapter instance associated with this file loader.
*
* @private
* @member {module:upload/filerepository~UploadAdapter}
*/
this._adapter = adapter;
this._adapter = uploadAdapterCreator( this );

/**
* FileReader used by FileLoader.
Expand Down Expand Up @@ -354,6 +369,28 @@ class FileLoader {
this.set( 'uploadResponse', null );
}

/**
* A `Promise` which resolves to a `File` instance associated with this file loader.
*
* @type {Promise.<File|null>}
*/
get file() {
if ( !this._filePromiseWrapper ) {
// Loader was destroyed, return promise which resolves to null.
return Promise.resolve( null );
} else {
// The `this._filePromiseWrapper.promise` is chained and not simply returned to handle a case when:
//
// * The `loader.file.then( ... )` is called by external code (returned promise is pending).
// * Then `loader._destroy()` is called (call is synchronous) which destroys the `loader`.
// * Promise returned by the first `loader.file.then( ... )` call is resolved.
//
// Returning `this._filePromiseWrapper.promise` will still resolve to a `File` instance so there
// is an additional check needed in the chain to see if `loader` was destroyed in the meantime.
return this._filePromiseWrapper.promise.then( file => this._filePromiseWrapper ? file : null );
}
}

/**
* Reads file using {@link module:upload/filereader~FileReader}.
*
Expand All @@ -372,7 +409,7 @@ class FileLoader {
* }
* } );
*
* @returns {Promise} Returns promise that will be resolved with read data. Promise will be rejected if error
* @returns {Promise.<String>} Returns promise that will be resolved with read data. Promise will be rejected if error
* occurs or if read process is aborted.
*/
read() {
Expand All @@ -382,7 +419,8 @@ class FileLoader {

this.status = 'reading';

return this._reader.read( this.file )
return this._filePromiseWrapper.promise
.then( file => this._reader.read( file ) )
.then( data => {
this.status = 'idle';

Expand All @@ -395,7 +433,7 @@ class FileLoader {
}

this.status = 'error';
throw this._reader.error;
throw this._reader.error ? this._reader.error : err;
} );
}

Expand All @@ -416,7 +454,7 @@ class FileLoader {
* }
* } );
*
* @returns {Promise} Returns promise that will be resolved with response data. Promise will be rejected if error
* @returns {Promise.<Object>} Returns promise that will be resolved with response data. Promise will be rejected if error
* occurs or if read process is aborted.
*/
upload() {
Expand All @@ -426,7 +464,8 @@ class FileLoader {

this.status = 'uploading';

return this._adapter.upload()
return this._filePromiseWrapper.promise
.then( () => this._adapter.upload() )
.then( data => {
this.uploadResponse = data;
this.status = 'idle';
Expand All @@ -450,11 +489,11 @@ class FileLoader {
const status = this.status;
this.status = 'aborted';

if ( status == 'reading' ) {
if ( !this._filePromiseWrapper.isFulfilled ) {
this._filePromiseWrapper.rejecter( 'aborted' );
} else if ( status == 'reading' ) {
this._reader.abort();
}

if ( status == 'uploading' && this._adapter.abort ) {
} else if ( status == 'uploading' && this._adapter.abort ) {
this._adapter.abort();
}

Expand All @@ -467,11 +506,41 @@ class FileLoader {
* @private
*/
_destroy() {
this._filePromiseWrapper = undefined;
this._reader = undefined;
this._adapter = undefined;
this.data = undefined;
this.uploadResponse = undefined;
this.file = undefined;
}

/**
* Wraps a given file promise into another promise giving additional
* control (resolving, rejecting, checking if fulfilled) over it.
*
* @private
* @param filePromise The initial file promise to be wrapped.
* @returns {module:upload/filerepository~FilePromiseWrapper}
*/
_createFilePromiseWrapper( filePromise ) {
const wrapper = {};

wrapper.promise = new Promise( ( resolve, reject ) => {
wrapper.resolver = resolve;
wrapper.rejecter = reject;
wrapper.isFulfilled = false;

filePromise
.then( file => {
wrapper.isFulfilled = true;
resolve( file );
} )
.catch( err => {
wrapper.isFulfilled = true;
reject( err );
} );
} );

return wrapper;
}
}

Expand Down Expand Up @@ -515,7 +584,7 @@ mix( FileLoader, ObservableMixin );
* {@link module:upload/filerepository~FileRepository#createUploadAdapter createUploadAdapter method}.
*
* @method module:upload/filerepository~UploadAdapter#upload
* @returns {Promise} Promise that should be resolved when data is uploaded.
* @returns {Promise.<Object>} Promise that should be resolved when data is uploaded.
*/

/**
Expand All @@ -527,3 +596,14 @@ mix( FileLoader, ObservableMixin );
*
* @method module:upload/filerepository~UploadAdapter#abort
*/

/**
* Object returned by {@link module:upload/filerepository~FileLoader#_createFilePromiseWrapper} method
* to add more control over the initial file promise passed to {@link module:upload/filerepository~FileLoader}.
*
* @typedef {Object} module:upload/filerepository~FilePromiseWrapper
* @property {Promise.<File>} promise Wrapper promise which can be chained for further processing.
* @property {Function} resolver Resolves the promise when called.
* @property {Function} rejecter Rejects the promise when called.
* @property {Boolean} isFulfilled Whether original promise is already fulfilled.
*/
Loading

0 comments on commit 62a8c69

Please sign in to comment.