CollaboraOnline / online

Collabora Online is a collaborative online office suite based on LibreOffice technology. This is also the source for the Collabora Office apps for iOS and Android.
https://collaboraonline.com
Other
1.77k stars 679 forks source link

Keep session open if document contains unsaved changes instead of saving it to the WOPI-host #1206

Closed TijZwa closed 3 years ago

TijZwa commented 3 years ago

At this moment, if a users edits a document and closes the browser, the document is saved to storage with a WOPI call. I think it is better when the document stays open at the backend, preserving the changes. If the user connects back to Collabora (by opening the URL to this document) the session resumes, showing the unsaved changes while the file on disk stays unchanged (because the user didn't explicitly saved the changes.)

I think this is a better way because a lot of protocols work like this. If I disconnect a VNC or RDP session, everything stays exactly the same until I connect again. Also tools like Notepad++ act like this; no save action, no data is written to disk.

What do you think about this?

mmeeks commented 3 years ago

Well - of course, we auto-save a lot too quite regularly. If there is another user editing of course we leave the document around, we could/did? have a timeout to keep a document with no users around in case someone re-connects (we sometimes have to stall people if we are tearing down a document while they re-connect eg. pressing the 'reload' button).

It would be easy enough to add a new timeout to loolwsd.xml - and/or wsd/DocumentBroker.cpp to keep a DocumentBroker alive with no ClientSessions around for some timeout (of course at a resource cost). Patches appreciated - checkout DocumentBroker::pollThread() and the conditions to exit:

    else if (_sessions.empty() && (isLoaded() || _markToDestroy))
    {
        // If all sessions have been removed, no reason to linger.
        LOG_INF("Terminating dead DocumentBroker for docKey [" << getDocKey() << "].");
        stop("dead");
    }

could be tweaked with a timeout (would need some testing).

Or do you want to disable autosave too ? I guess that could already be done by bumping the existing autosave_duration_secs waaay high =)

TijZwa commented 3 years ago

Thanks. I've been thinking about this and found another way around it. I can use the header X-LOOL-WOPI-IsExitSave on my Wopi host and cache unsaved changes there, so we dont need a lingering DocumentBroker.

I will go in that direction.