Other (autosave): Autosave plugin will now ignore changes coming from…

… remote clients during real-time collaboration.

Feature (autosave): `Autosave#save()` will now return a promise that is resolved when the autosave callback has finished.

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;
 
+		/**
+		 * Promised 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.
 		 *
@@ -138,6 +150,7 @@ export default class Autosave extends Plugin {
 		 * @private
 		 * @member {module:core/pendingactions~PendingActions} #_pendingActions
 		 */
+		this._pendingActions = editor.plugins.get( PendingActions );
 	}
 
 	/**
@@ -146,25 +159,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 +213,14 @@ 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.
+	 *
+	 * @returns {Promise} A promise that will be resolved when the autosave callback is finished.
 	 */
 	save() {
 		this._debouncedSave.cancel();
-		this._save();
+
+		return this._save();
 	}
 
 	/**
@@ -224,12 +240,19 @@ export default class Autosave extends Plugin {
 	 * @private
 	 */
 	_save() {
+		if ( this._savePromise ) {
+			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()
+		this._savePromise = Promise.resolve()
 			.then( () => Promise.all(
 				this._saveCallbacks.map( cb => cb( this.editor ) )
 			) )
@@ -254,7 +277,25 @@ export default class Autosave extends Plugin {
 					this._pendingActions.remove( this._action );
 					this._action = null;
 				}
+			} )
+			.finally( () => {
+				this._savePromise = 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' ) );
+		}
 	}
 
 	/**

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,7 @@ describe( 'Autosave', () => {
 				.create( element, {
 					plugins: [ Autosave, Paragraph ],
 					autosave: {
-						save: spy
+						save: async () => { spy(); }
 					},
 					initialData: '<p>Foo</p>'
 				} )
@@ -704,23 +720,26 @@ describe( 'Autosave', () => {
 			return editor.destroy();
 		} );
 
-		it( 'shout not call autosave callback if nothing changed', () => {
-			expect( spy.called ).to.be.false;
-
-			autosave.save();
-
-			expect( spy.called ).to.be.false;
-		} );
-
-		it( 'should call autosave callback', () => {
+		it( 'should call autosave callback and return a promise that is resolved when the autosave callback is finished', () => {
 			editor.model.change( writer => {
 				writer.setSelection( writer.createRangeIn( editor.model.document.getRoot().getChild( 0 ) ) );
 				editor.model.insertContent( writer.createText( 'foo' ) );
 			} );
 
-			autosave.save();
+			const promise = autosave.save();
 
-			return Promise.resolve().then( () => {
+			return promise.then( () => {
+				expect( spy.calledOnce ).to.be.true;
+			} );
+		} );
+
+		it( 'should use one autosave call and one promise if called multiple times', () => {
+			const promiseA = autosave.save();
+			const promiseB = autosave.save();
+
+			expect( promiseA ).to.equal( promiseB );
+
+			return promiseA.then( () => {
 				expect( spy.calledOnce ).to.be.true;
 			} );
 		} );
@@ -733,7 +752,7 @@ describe( 'Autosave', () => {
 
 			autosave.save();
 
-			sinon.clock.tick( 1000 );
+			sinon.clock.tick( 2000 );
 
 			return Promise.resolve().then( () => {
 				expect( spy.calledOnce ).to.be.true;