nextcloud / text

📑 Collaborative document editing using Markdown
GNU Affero General Public License v3.0
552 stars 91 forks source link

Sync endpoint always sets up the file sytem #1184

Open juliusknorr opened 3 years ago

juliusknorr commented 3 years ago

The sync endpoint actually always sets up the filesystem for the user due to the current way we check if the file has changed outside:

text_sync_file_session

We mainly need to obtain the mtime and etag of the file to detect if it was changed or if it should be autosaved and ideally we only need to go to the filesystem layer in those cases. However I currently don't see a proper way of only obtaining those details without querying the filecache directly, which we could do since we have the file id. Alternatively we can probably introduce our own state handling by hooking into the postWrite hook and basically marking text documents as changed outside then.

@rullzer Do you see any other way? I'd probably prefer just querying the filecache directly from a implementation perspective but the hook feels a bit cleaner.

rullzer commented 3 years ago

So the hook would be triggering an event that the text app can react on to obtain the info right? It indeed feels cleaner.

The problem I see is that you do a lot of potential work if there are files being changed and you have to somehow react to them.

While text is presumably only running on a very limited set of files.

We could of course just only do this for systems with a proper cache setup?

So we store a list of filid => mtime in the cache. Then if a write event happens we check the cache for the fileid if it is not there we ignore if it is we update the mtime?

Raudius commented 1 year ago

From what I can tell this is still relevant \OCA\Text\Service\DocumentService::getFileById, so will leave this open for now

juliusknorr commented 1 year ago

Kind of related to #1042 as well