diff --git a/packages/ckeditor5-autosave/src/autosave.js b/packages/ckeditor5-autosave/src/autosave.js index daf0909d567..7b361bbe2ba 100644 --- a/packages/ckeditor5-autosave/src/autosave.js +++ b/packages/ckeditor5-autosave/src/autosave.js @@ -88,6 +88,7 @@ export default class Autosave extends Plugin { * * error &ndash When the provided save method will throw an error. This state immediately changes to the `saving` state and * the save method will be called again in the short period of time. * + * @readonly * @member {'synchronized'|'waiting'|'saving'} #state */ this.set( 'state', 'synchronized' ); @@ -109,6 +110,17 @@ export default class Autosave extends Plugin { */ this._lastDocumentVersion = editor.model.document.version; + /** + * Promise used for asynchronous save calls. + * + * Created to handle the autosave call to an external data source. It resolves when that call is finished. It is re-used if + * save is called before the promise has resolved. It is set to `null` if there is no call in progress. + * + * @type {Promise|null} + * @private + */ + this._savePromise = null; + /** * DOM emitter. * @@ -126,17 +138,29 @@ export default class Autosave extends Plugin { this._config = config; /** - * An action that will be added to pending action manager for actions happening in that plugin. + * Editor's pending actions manager. * * @private - * @member {Object} #_action + * @member {module:core/pendingactions~PendingActions} #_pendingActions */ + this._pendingActions = editor.plugins.get( PendingActions ); /** - * Editor's pending actions manager. + * Informs whether there should be another autosave callback performed, immediately after current autosave callback finishes. + * + * This is set to `true` when there's a save request while autosave callback is already being processed and the model has changed + * since the last save. * * @private - * @member {module:core/pendingactions~PendingActions} #_pendingActions + * @type {Boolean} + */ + this._makeImmediateSave = false; + + /** + * An action that will be added to pending action manager for actions happening in that plugin. + * + * @private + * @member {Object} #_action */ } @@ -146,25 +170,25 @@ export default class Autosave extends Plugin { init() { const editor = this.editor; const doc = editor.model.document; - const t = editor.t; - - this._pendingActions = editor.plugins.get( PendingActions ); // Add the listener only after the editor is initialized to prevent firing save callback on data init. this.listenTo( editor, 'ready', () => { - this.listenTo( doc, 'change:data', () => { + this.listenTo( doc, 'change:data', ( evt, batch ) => { if ( !this._saveCallbacks.length ) { return; } - if ( this.state == 'synchronized' ) { - this._action = this._pendingActions.add( t( 'Saving changes' ) ); - this.state = 'waiting'; + if ( !batch.isLocal ) { + return; + } - this._debouncedSave(); + if ( this.state === 'synchronized' ) { + this.state = 'waiting'; + // Set pending action already when we are waiting for the autosave callback. + this._setPendingAction(); } - else if ( this.state == 'waiting' ) { + if ( this.state === 'waiting' ) { this._debouncedSave(); } @@ -200,11 +224,15 @@ export default class Autosave extends Plugin { } /** - * Calls autosave plugin callback and cancels any delayed callbacks that may have been already triggered. + * Immediately calls autosave callback. All previously queued (debounced) callbacks are cleared. If there is already an autosave + * callback in progress, then the requested save will be performed immediately after the current callback finishes. + * + * @returns {Promise} A promise that will be resolved when the autosave callback is finished. */ save() { this._debouncedSave.cancel(); - this._save(); + + return this._save(); } /** @@ -222,39 +250,87 @@ export default class Autosave extends Plugin { * It waits for the result and then removes the created pending action. * * @private + * @returns {Promise} A promise that will be resolved when the autosave callback is finished. */ _save() { + if ( this._savePromise ) { + this._makeImmediateSave = this.editor.model.document.version > this._lastDocumentVersion; + + return this._savePromise; + } + + // Make sure there is a pending action (in case if `_save()` was called through manual `save()` call). + this._setPendingAction(); + this.state = 'saving'; this._lastDocumentVersion = this.editor.model.document.version; - // Wait one promise cycle to be sure that save callbacks are not called - // inside a conversion or when the editor's state changes. - Promise.resolve() + // Wait one promise cycle to be sure that save callbacks are not called inside a conversion or when the editor's state changes. + this._savePromise = Promise.resolve() + // Make autosave callback. .then( () => Promise.all( this._saveCallbacks.map( cb => cb( this.editor ) ) ) ) - // In case of an error re-try the save later and throw the original error. - // Being in the `saving` state ensures that the debounced save action - // won't be delayed further by the `change:data` event listener. + // When the autosave callback is finished, always clear `this._savePromise`, no matter if it was successful or not. + .finally( () => { + this._savePromise = null; + } ) + // If the save was successful we have three scenarios: + // + // 1. If a save was requested when an autosave callback was already processed, we need to immediately call + // another autosave callback. In this case, `this._savePromise` won't be resolved until the next callback is done. + // 2. Otherwise, if changes happened to the model, make a delayed autosave callback (like the change just happened). + // 3. If no changes happened to the model, return to the `synchronized` state. + .then( () => { + if ( this._makeImmediateSave ) { + this._makeImmediateSave = false; + + // Start another autosave callback. Return a promise that will be resolved after the new autosave callback. + // This way promises returned by `_save()` won't be resolved until all changes are saved. + // + // If `save()` was called when another (most often automatic) autosave callback was already processed, + // the promise returned by `save()` call will be resolved only after new changes has been saved. + // + // Note that it would not work correctly if `this._savePromise` is not cleared. + return this._save(); + } else { + if ( this.editor.model.document.version > this._lastDocumentVersion ) { + this.state = 'waiting'; + this._debouncedSave(); + } else { + this.state = 'synchronized'; + this._pendingActions.remove( this._action ); + this._action = null; + } + } + } ) + // In case of an error retry the autosave callback after a delay (and also throw the original error). .catch( err => { + // Change state to `error` so that listeners handling autosave error can be called. this.state = 'error'; - // Change immediately to the `saving` state so the `change:state` event will be fired. + // Then, immediately change to the `saving` state as described above. + // Being in the `saving` state ensures that the autosave callback won't be delayed further by the `change:data` listener. this.state = 'saving'; this._debouncedSave(); throw err; - } ) - .then( () => { - if ( this.editor.model.document.version > this._lastDocumentVersion ) { - this.state = 'waiting'; - this._debouncedSave(); - } else { - this.state = 'synchronized'; - this._pendingActions.remove( this._action ); - this._action = null; - } } ); + + return this._savePromise; + } + + /** + * Creates a pending action if it is not set already. + * + * @private + */ + _setPendingAction() { + const t = this.editor.t; + + if ( !this._action ) { + this._action = this._pendingActions.add( t( 'Saving changes' ) ); + } } /** diff --git a/packages/ckeditor5-autosave/tests/autosave.js b/packages/ckeditor5-autosave/tests/autosave.js index c36cb9c4eb3..99e51ad4d0a 100644 --- a/packages/ckeditor5-autosave/tests/autosave.js +++ b/packages/ckeditor5-autosave/tests/autosave.js @@ -635,7 +635,7 @@ describe( 'Autosave', () => { } ); } ); - it( 'should handle a situration when the save callback throws an error', () => { + it( 'should handle a situation when the save callback throws an error', () => { const pendingActions = editor.plugins.get( PendingActions ); const successServerActionSpy = sinon.spy(); const serverActionStub = sinon.stub(); @@ -673,6 +673,22 @@ describe( 'Autosave', () => { sinon.assert.calledOnce( successServerActionSpy ); } ); } ); + + it( 'should ignore non-local changes', () => { + autosave.adapter = { + save: sinon.spy() + }; + + editor.model.enqueueChange( { isLocal: false }, writer => { + writer.insertElement( 'paragraph', null, editor.model.document.getRoot(), 0 ); + } ); + + sinon.clock.tick( 2000 ); + + return runPromiseCycles().then( () => { + sinon.assert.notCalled( autosave.adapter.save ); + } ); + } ); } ); describe( 'save()', () => { @@ -688,7 +704,11 @@ describe( 'Autosave', () => { .create( element, { plugins: [ Autosave, Paragraph ], autosave: { - save: spy + save: async () => { + await wait( 100 ); + + spy(); + } }, initialData: '

Foo

' } ) @@ -704,28 +724,74 @@ describe( 'Autosave', () => { return editor.destroy(); } ); - it( 'shout not call autosave callback if nothing changed', () => { - expect( spy.called ).to.be.false; + it( 'should call autosave callback and return a promise resolved when the callback is finished', async () => { + const promise = autosave.save(); - autosave.save(); + // Wait for autosave's inner promise delayer. After this callback has been called. + await runPromiseCycles(); - expect( spy.called ).to.be.false; + // "Wait" for the callback to finish. + sinon.clock.tick( 100 ); + + // Wait for autosave callback promise to resolve. + await promise; + + expect( spy.calledOnce ).to.be.true; + } ); + + it( 'should use one autosave call and one promise if called multiple times', async () => { + const promise = autosave.save(); + const promiseB = autosave.save(); + const promiseC = autosave.save(); + + expect( promiseB ).to.equal( promise ); + expect( promiseC ).to.equal( promise ); + + // Wait for autosave's inner promise delayer. After this callback has been called. + await runPromiseCycles(); + + // "Wait" for the callback to finish. + sinon.clock.tick( 100 ); + + // Wait for autosave callback promise to resolve. + await promise; + + expect( spy.calledOnce ).to.be.true; } ); - it( 'should call autosave callback', () => { + it( 'should use two autosave calls and one promise if called another time after model changed', async () => { + const promise = autosave.save(); + editor.model.change( writer => { writer.setSelection( writer.createRangeIn( editor.model.document.getRoot().getChild( 0 ) ) ); editor.model.insertContent( writer.createText( 'foo' ) ); } ); - autosave.save(); + const promiseB = autosave.save(); - return Promise.resolve().then( () => { - expect( spy.calledOnce ).to.be.true; - } ); + expect( promise ).to.equal( promiseB ); + + // Wait for autosave's inner promise delayer. After this callback has been called. + await runPromiseCycles(); + + // "Wait" for the first callback to finish. + sinon.clock.tick( 100 ); + + // Wait for autosave callback promise to resolve. + await runPromiseCycles(); + + expect( spy.calledOnce ).to.be.true; + + // "Wait" for the second callback to finish. + sinon.clock.tick( 100 ); + + // Wait for autosave callback promise to resolve. + await promise; + + expect( spy.calledTwice ).to.be.true; } ); - it( 'should cancel delayed autosave callback', () => { + it( 'should cancel delayed autosave callback', async () => { editor.model.change( writer => { writer.setSelection( writer.createRangeIn( editor.model.document.getRoot().getChild( 0 ) ) ); editor.model.insertContent( writer.createText( 'foo' ) ); @@ -733,11 +799,67 @@ describe( 'Autosave', () => { autosave.save(); - sinon.clock.tick( 1000 ); + // Wait for autosave's inner promise delayer. + await runPromiseCycles(); - return Promise.resolve().then( () => { - expect( spy.calledOnce ).to.be.true; + // "Wait" for the `save()` callback to finish. + sinon.clock.tick( 100 ); + + // Wait for autosave callback promise to resolve. + await runPromiseCycles(); + + // "Wait" for the debounce to finish. + sinon.clock.tick( 2000 ); + + // Wait for autosave's inner promise delayer. + await runPromiseCycles(); + + // "Wait" for the autosave callback to finish. + sinon.clock.tick( 100 ); + + // Wait for autosave callback promise to resolve. + await runPromiseCycles(); + + expect( spy.calledOnce ).to.be.true; + } ); + + it( 'should return a promise resolved when the autosave for save() call is finished (another callback in progress)', async () => { + editor.model.change( writer => { + writer.setSelection( writer.createRangeIn( editor.model.document.getRoot().getChild( 0 ) ) ); + editor.model.insertContent( writer.createText( 'foo' ) ); + } ); + + // "Wait" for the debounce to finish. + sinon.clock.tick( 2000 ); + + // Wait for autosave's inner promise delayer. + await runPromiseCycles(); + + editor.model.change( writer => { + editor.model.insertContent( writer.createText( 'foo' ) ); } ); + + // Call `save()` before earlier callback finishes. + const promise = autosave.save(); + + expect( spy.called ).to.be.false; + + // "Wait" for the callback to finish. + sinon.clock.tick( 100 ); + + // Wait for autosave callback promise to resolve. + await runPromiseCycles(); + + // First callback (debounced) has finished. + expect( spy.calledOnce ).to.be.true; + + // "Wait" for the second callback to finish. + sinon.clock.tick( 100 ); + + await promise; + + // First callback has finished. + expect( spy.calledTwice ).to.be.true; } ); } );