[BUG] External file changes are not detected if file is overwritten by move

[jkhsjdhjs]

Issue Description

QOwnNotes doesn't detect modifications of notes if they're synced by the Nextcloud desktop client or more generally, if they're performed by a move operation. This was already discussed in #3366, but now I finally had some time to investigate this issue further.

Nextcloud Desktop File Synchronization

When the Nextcloud desktop client syncs changes to an already-existing file, it doesn't overwrite it directly.
Instead, it first creates a temporary file .<name-of-note>.~<unique-id>, e.g. .Welcome to QOwnNotes.md.~dce5e2c.
All remote changes are written to this file first. Only afterwards, the original note file is overwritten by a move to finalize the synchronization.

So why are these changes not detected by QOwnNotes?

QOwnNotes File Change Detection

QOwnNotes uses two kinds of filesystem watchers: a directory watcher and a file watcher:

QOwnNotes/src/mainwindow.cpp

Lines 637 to 653 in 025e685

	void MainWindow::connectFileWatcher(bool delayed) {
	if (!delayed) {
	connect(&noteDirectoryWatcher, &QFileSystemWatcher::directoryChanged, this,
	&MainWindow::notesDirectoryWasModified, Qt::UniqueConnection);
	connect(&noteDirectoryWatcher, &QFileSystemWatcher::fileChanged, this,
	&MainWindow::notesWereModified, Qt::UniqueConnection);
	} else {
	// In some cases, there are delayed signals coming in which we don't want to handle
	// so reconnect with delay
	QTimer::singleShot(300, this, [this] {
	connect(&noteDirectoryWatcher, &QFileSystemWatcher::directoryChanged, this,
	&MainWindow::notesDirectoryWasModified, Qt::UniqueConnection);
	connect(&noteDirectoryWatcher, &QFileSystemWatcher::fileChanged, this,
	&MainWindow::notesWereModified, Qt::UniqueConnection);
	});
	}
	}

The directory watcher is attached to the note directory and the file watcher to every note.
On a directory event (i.e. the creation of a new file), the notesDirectoryWasModified callback is called.
If a file is changed, notesWereModified is called instead.

Control Flow during Synchronization

If the Nextcloud client syncs a file, QOwnNotes' notesDirectoryWasModified callback is run first, since the Nextcloud desktop client first creates the temporary file. This callback also contains a workaround to call notesWereModified for the currently active note. However, since the Nextcloud client at this point only created the temporary file, the currently active note hasn't been modified yet. Thus, notesWereModified returns early. Afterwards, the temporary note file is modified by the Nextcloud sync client, which triggers notesDirectoryWasModified again for some reason, with the same result: no changes were detected because no changes to the actual note were performed yet.

Finally, the Nextcloud sync client moves the temporary note to the location of the actual note, thereby overwriting it. This triggers notesDirectoryWasModified again, but it still doesn't detect any changes, although they would be detectable by now. This is because before calling notesWereModified, notesDirectoryWasModified first rebuilds and reloads the note directory list, which results in the changes not being detected for some reason.

QOwnNotes/src/mainwindow.cpp

Lines 3112 to 3123 in 025e685

	// rebuild and reload the notes directory list
	buildNotesIndexAndLoadNoteDirectoryList();
	// check if the current note was modified
	// this fixes not detected external note changes of the current note if the
	// event for the change in the current note comes after the event that the
	// note folder was modified
	QString noteFileName = currentNote.getFileName();
	if (!noteFileName.isEmpty()) {
	// Use the full path, like a filesystem watcher would, instead of just the file-name
	notesWereModified(currentNote.fullNoteFilePath());
	}

Moving the call to buildNotesIndexAndLoadNoteDirectoryList() after notesWereModified() makes the detection functional. However, I'm not sure if this has additional unwanted side-effects.

This final move operation also triggers notesWereModified directly, because in addition to the notes directory, files are also watched. However, since the note index has been reloaded and rebuilt, no changes are detected here as well.

Another issue results from how Qt handles file replacements. Nextcloud overwriting the note file via a move operation destroys the original file's inode. This causes QFileSystemWatcher to silently detach from the note. The watcher is never reattached to the new inode, meaning subsequent modifications to that note are completely ignored.

Summary

In summary, there are two important issues with the change detection the way it's currently implemented:

1. Rebuilding the notes index breaks the change detection
2. File watchers aren't reattached if files are overwritten

As I don't know much about the inner workings of the note index, I don't have a suggestion for a fix there.
However, the second issue could be fixed by re-adding files to the noteDirectoryWatcher after they have been overwritten.

Related Issues

#3366
#3428 @ohnemich

[pbek]

Well researched issue. 😉

However, the second issue could be fixed by re-adding files to the noteDirectoryWatcher after they have been overwritten.

The main issue here is that for the QFileSystemWatcher to notify you about changes of a file, you actively need to subscribe to it for that file. And if that file goes away, the watcher goes away too.

And a secondary issue is that you may not always get notifications anyway, because of OS limitations.

Moving the call to buildNotesIndexAndLoadNoteDirectoryList() after notesWereModified() makes the detection functional.

Did you test that? I'm not sure why it should... And of course this will have side effects. 🤔

[jkhsjdhjs]

The main issue here is that for the QFileSystemWatcher to notify you about changes of a file, you actively need to subscribe to it for that file. And if that file goes away, the watcher goes away too.

Yep, but the watcher can easily be reattached, since the directory is monitored anyway. The problem is, that this is currently not implemented.

And a secondary issue is that you may not always get notifications anyway, because of OS limitations.

I'm not aware of that (on Linux at least). In my testing I always got inotify events, they just weren't handled by QOwnNotes since buildNotesIndexAndLoadNoteDirectoryList() is called before notesWereModified(), blocking the detection for some reason. Furthermore, subsequent events are not received by QOwnNotes at all since the watcher is not reattached to new files.

Did you test that? I'm not sure why it should... And of course this will have side effects. 🤔

Yep! I tested all of the statements in my bug report. I'm also not sure why running buildNotesIndexAndLoadNoteDirectoryList() first would mess with the detection, but yes, that's the case.

[pbek]

I can reproduce your issue only if the note you change via sync is not the current note. If it is the current note, then I get an instant update.
But moving the statement as you suggested didn't fix that for me.

[jkhsjdhjs]

Hmm, that's odd. I only tested with the currently active note for this bug report.

So what's the difference between our setups?
I'm on Arch Linux (Linux 6.18.9-arch1-2) with Nextcloud desktop 4.0.6 on an ext4 filesystem. As for QOwnNotes, I used the latest git master in debug mode during testing.

[pbek]

And I guess the note will not be updated, because the sync client starts to write the temporary file, triggers MainWindow::notesDirectoryWasModified and then when buildNotesIndexAndLoadNoteDirectoryList is done the changed note gets moved and doesn't trigger anything else. 🤔

[jkhsjdhjs]

when buildNotesIndexAndLoadNoteDirectoryList is done the changed note gets moved and doesn't trigger anything else. 🤔

The moved note still triggers the filesystem watcher, but I could see that notesWereModified does not detect any changes in the file, even though the file did in fact change.

[pbek]

So what's the difference between our setups?

NixOS Unstable on ZFS. Kernel 6.18.12. Nextcloud desktop 4.0.6. QOwnNotes from main branch with debug.

[jkhsjdhjs]

@ohnemich Maybe you could also post your setup here? My current guess is that there's a difference on how the events are emitted on different filesystems. Maybe I'll have time to test on ZFS in the next days.

[pbek]

Maybe you could also post your setup here?

On what part of my setup are you interested in?

[pbek]

26.2.10

• Fixed Nextcloud desktop client sync changes to non-current note files not being picked up by QOwnNotes
  (for #3468)
  • The Nextcloud client uses a temp-file + atomic rename strategy to update note files; the rename
    typically completes before the directoryChanged signal handler runs, so the temp file is already
    gone when QOwnNotes looks for it — the deferred re-index was therefore never triggered
  • The notes directory watcher now always schedules a deferred re-index (1 second after the directory
    change event) so the final on-disk state of all note files is captured regardless of whether a
    temp file is visible at the time the event fires
  • The note index rebuild also refreshes the note list when an existing note's content was updated
    on disk, not only when notes were added or removed

Sets see how many side effects that change has... 😊