Selective sync: Skip excluded folders when reading db #5772
Conversation
|
@ogoffart Yes, testing and documentation are still missing. |
|
@ckamm Could you put this in the 2.3 branch instead? |
|
Added a test that failed before and passes now - as well as some documentation and retargeted for 2.3. |
When a new folder becomes selective-sync excluded, we already mark it and all its parent folders with _invalid_ etags to force rediscovery. That's not enough however. Later calls to csync_statedb_get_below_path could still pull data about the excluded files into the remote tree. That lead to incorrect behavior, such as uploads happening for folders that had been explicitly excluded from sync. To fix the problem, statedb_get_below_path is adjusted to not read the data about excluded folders from the database. Currently we can't wipe this data from the database outright because we need it to determine whether the files in the excluded folder can be wiped away or not. See owncloud/enterprise#1965
|
Will it be in tomorrow's nightlies? |
|
@michaelstingl I've merged it now, should be in the nightlies. @guruz ? |
| // The local state should still have subFolderA | ||
| auto local = fakeFolder.currentLocalState(); | ||
| QVERIFY(local.find("parentFolder/subFolder")); | ||
| QVERIFY(local.find("parentFolder/subFolder/fileA.txt")); |
There was a problem hiding this comment.
Why does it still have fileA.txt ?
That looks like a bug to me.
There was a problem hiding this comment.
@ogoffart I thought that was intentional. Currently it either deletes the whole folder -- or doesn't do anything about it. Was the originally intended behavior to remove all files that should be safe to remove?
The culprit is checkForPermissions, which sets all subitems to IGNORED.
When a new folder becomes selective-sync excluded, we already mark it
and all its parent folders with invalid etags to force rediscovery.
That's not enough however. Later calls to csync_statedb_get_below_path
could still pull data about the excluded files into the remote tree.
That lead to incorrect behavior, such as uploads happening for folders
that had been explicitly excluded from sync.
To fix the problem, statedb_get_below_path is adjusted to not read the
data about excluded folders from the database.
Currently we can't wipe this data from the database outright because we
need it to determine whether the files in the excluded folder can be
wiped away or not.
See owncloud/enterprise#1965