Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Selective sync: Skip excluded folders when reading db #5772

Merged
merged 1 commit into from
May 16, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 31 additions & 1 deletion csync/src/csync_statedb.c
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ int csync_statedb_get_below_path( CSYNC *ctx, const char *path ) {
* In other words, anything that is between path+'/' and path+'0',
* (because '0' follows '/' in ascii)
*/
const char *below_path_query = "SELECT " METADATA_COLUMNS " FROM metadata WHERE path > (?||'/') AND path < (?||'0')";
const char *below_path_query = "SELECT " METADATA_COLUMNS " FROM metadata WHERE path > (?||'/') AND path < (?||'0') ORDER BY path||'/' ASC";
SQLITE_BUSY_HANDLED(sqlite3_prepare_v2(ctx->statedb.db, below_path_query, -1, &stmt, NULL));
ctx->statedb.lastReturnValue = rc;
if( rc != SQLITE_OK ) {
Expand All @@ -462,6 +462,36 @@ int csync_statedb_get_below_path( CSYNC *ctx, const char *path ) {

rc = _csync_file_stat_from_metadata_table( &st, stmt);
if( st ) {
/* When selective sync is used, the database may have subtrees with a parent
* whose etag (md5) is _invalid_. These are ignored and shall not appear in the
* remote tree.
* Sometimes folders that are not ignored by selective sync get marked as
* _invalid_, but that is not a problem as the next discovery will retrieve
* their correct etags again and we don't run into this case.
*/
if( c_streq(st->etag, "_invalid_") ) {
CSYNC_LOG(CSYNC_LOG_PRIORITY_TRACE, "%s selective sync excluded", st->path);
char *skipbase = c_strdup(st->path);
skipbase[st->pathlen] = '/';
int skiplen = st->pathlen + 1;

/* Skip over all entries with the same base path. Note that this depends
* strongly on the ordering of the retrieved items. */
do {
csync_file_stat_free(st);
rc = _csync_file_stat_from_metadata_table( &st, stmt);
if( st && strncmp(st->path, skipbase, skiplen) != 0 ) {
break;
}
CSYNC_LOG(CSYNC_LOG_PRIORITY_TRACE, "%s selective sync excluded because the parent is", st->path);
} while( rc == SQLITE_ROW );

/* End of data? */
if( rc != SQLITE_ROW || !st ) {
continue;
}
}

/* Check for exclusion from the tree.
* Note that this is only a safety net in case the ignore list changes
* without a full remote discovery being triggered. */
Expand Down
8 changes: 8 additions & 0 deletions src/libsync/syncjournaldb.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,14 @@ class OWNCLOUDSYNC_EXPORT SyncJournalDb : public QObject
/**
* Make sure that on the next sync, fileName is not read from the DB but uses the PROPFIND to
* get the info from the server
*
* Specifically, this sets the md5 field of fileName and all its parents to _invalid_.
* That causes a metadata difference and a resulting discovery from the remote for the
* affected folders.
*
* Since folders in the selective sync list will not be rediscovered (csync_ftw,
* _csync_detect_update skip them), the _invalid_ marker will stay and it. And any
* child items in the db will be ignored when reading a remote tree from the database.
*/
void avoidReadFromDbOnNextSync(const QString& fileName);

Expand Down
43 changes: 43 additions & 0 deletions test/testsyncengine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,49 @@ private slots:
}
}

void testSelectiveSyncBug() {
// issue owncloud/enterprise#1965: files from selective-sync ignored
// folders are uploaded anyway is some circumstances.
FakeFolder fakeFolder{FileInfo{ QString(), {
FileInfo { QStringLiteral("parentFolder"), {
FileInfo{ QStringLiteral("subFolder"), {
{ QStringLiteral("fileA.txt"), 400 },
{ QStringLiteral("fileB.txt"), 400, 'o' }
}}
}}
}}};

QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
auto expectedServerState = fakeFolder.currentRemoteState();

// Remove subFolder with selectiveSync:
fakeFolder.syncEngine().journal()->setSelectiveSyncList(SyncJournalDb::SelectiveSyncBlackList,
{"parentFolder/subFolder/"});
fakeFolder.syncEngine().journal()->avoidReadFromDbOnNextSync("parentFolder/subFolder/");

// But touch a local file before the next sync, such that the local folder
// can't be removed
fakeFolder.localModifier().setContents("parentFolder/subFolder/fileB.txt", 'n');

// Several follow-up syncs don't change the state at all,
// in particular the remote state doesn't change and fileB.txt
// isn't uploaded.

for (int i = 0; i < 3; ++i) {
fakeFolder.syncOnce();

{
// Nothing changed on the server
QCOMPARE(fakeFolder.currentRemoteState(), expectedServerState);
// The local state should still have subFolderA
auto local = fakeFolder.currentLocalState();
QVERIFY(local.find("parentFolder/subFolder"));
QVERIFY(local.find("parentFolder/subFolder/fileA.txt"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does it still have fileA.txt ?
That looks like a bug to me.

Copy link
Contributor Author

@ckamm ckamm May 17, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

---> #5783

QVERIFY(local.find("parentFolder/subFolder/fileB.txt"));
}
}
}

void abortAfterFailedMkdir() {
QSKIP("Skip for 2.3");
FakeFolder fakeFolder{FileInfo{}};
Expand Down