Autosave plugin not ignoring transparent type events.

[asimkt]

The current autosave calls the Promise for almost all change:data events. This is not needed as some changes may be from the Collaboration Mode, (ie, changes from other users). This will cause in collaboration mode:

• Unwanted API calls for every connected user.
• Unwanted API calls for other ignorable events.

Here is the diff that solved my problem:

diff --git a/node_modules/@ckeditor/ckeditor5-autosave/src/autosave.js b/node_modules/@ckeditor/ckeditor5-autosave/src/autosave.js
index 004c0a7..20720ef 100644
--- a/node_modules/@ckeditor/ckeditor5-autosave/src/autosave.js
+++ b/node_modules/@ckeditor/ckeditor5-autosave/src/autosave.js
@@ -153,11 +153,17 @@ export default class Autosave extends Plugin {
 
 		this._pendingActions = editor.plugins.get( PendingActions );
 
-		this.listenTo( doc, 'change:data', () => {
+		this.listenTo( doc, 'change:data', (_e, batch) => {
 			if ( !this._saveCallbacks.length ) {
 				return;
 			}
 
+      // If the change is from another collaborative session from another user
+      // don't autosave.
+      if ( batch.type === 'transparent') {
+        return;
+      }
+
 			if ( this.state == 'synchronized' ) {
 				this._action = this._pendingActions.add( t( 'Saving changes' ) );
 				this.state = 'waiting';

This issue body was partially generated by patch-package.

[scofalik]

This is not a safe change, unfortunately. There are several places where we use transparent batches that should be handled by the autosave plugin. Most notably - the undo feature. I wanted to write that we could discuss and refactor what types of batches we need (batch types are one of firsts concepts so we thought about them before we had some of the use cases we have now). However, honestly, autosave should be fired always when editor content changes and this situation might be the only exception - which is due to limiting number of requests, not because incorrect data would be saved.

Unwanted API calls for every connected user.

Maybe it would be possible to use cloudDocumentVersion property from RealTimeCollaborationClient plugin? This property is always raised whenever new data from collaboration server is received. Maybe caching and comparing to the previous value in the autosave callback would solve this issue? I am not sure if that would work - that's just an idea I have.

Unwanted API calls for other ignorable events.

What kind of events do you mean?

[asimkt]

What kind of events do you mean?

Actually, I don't know. I was under the impression that transparent events can be safely ignored.

cloudDocumentVersion

Isn't the value change to everyone connected and everyone will call the API right if we compare it with the old one? I think if we let the server decide whether to overwrite the old data, yes, cloudDocumentVersion may be a good candidate.

[scofalik]

To solve this problem, we need to identify which changes are remote. It is difficult at the moment, as we don't give direct tools to check whether given batch comes from a remote client or from the local client. It was somewhat intentional, as we wanted to treat all changes equal but maybe it will be a good idea to implement an easy way to check the source of the changes. OTOH, even remote changes can trigger some local changes (due to postfixers, etc.) so this is not always a black-and-white situation.

I wanted to use cloudDocumentVersion to check whether local operations have been performed. This property is increased only when new operations are received by the client. So, in autosave callback, if cloudDocumentVersion is same as in the last callback, but local document version is higher - then it means that local changes have been applied. If cloudDocumentVersion is higher by the same amount as version then it means that only remote changes have been applied.

EDIT: ☝️  the above solution has additional difficulty - autosave callbacks are debounced while cloudDocumentVersion is also updated after local changes are successfully sent to the server. Hence, I think that the below solution is even better.

But I have even better idea now. As I said, we need to identify whether the changes in given batch have been all remote. And we can do that by scanning operations in that batch and checking their author. You can call Users#getOperationAuthor( operation ) to get the author of the operation and compare it with local user (Users#me). If all operations in a batch are created by non-local user, then the batch is completely remote and autosave is not needed.

I'd implement the final solution as a micro-plugin, which would:

1. Add a callback to model document change:data event. The callback receives batch as a parameter.
2. In the callback, check all batch.operations and see if the batch is local or remote. If local, set some property in your plugin to true (let's call it shouldSave).
3. Your autosave callback then receives editor instance, so it can load your micro-plugin and can check shouldSave property. If it is true, you can call your autosave callback as usual but remember to set shouldSave to false. If it was false - do nothing.

Maybe this will solve your issue until we implement a good solution for the problem?

[asimkt]

It will. thanks. I need a way to control the autosave call from the outside of the CKE engine.( ie as a consumer ).

[asimkt]

Thanks people