[Pkg-owncloud-commits] [owncloud-client] 11/135: Selective sync: Skip excluded folders when reading db

Sandro Knauß hefee at debian.org
Sat Sep 9 14:28:24 UTC 2017


This is an automated email from the git hooks/post-receive script.

hefee pushed a commit to branch master
in repository owncloud-client.

commit c5a0ce5a43c5539b0b22ca3a5b81d327465adb5f
Author: Christian Kamm <mail at ckamm.de>
Date:   Mon May 15 14:46:09 2017 +0200

    Selective sync: Skip excluded folders when reading db
    
    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
---
 csync/src/csync_statedb.c   | 32 +++++++++++++++++++++++++++++++-
 src/libsync/syncjournaldb.h |  8 ++++++++
 test/testsyncengine.cpp     | 43 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 82 insertions(+), 1 deletion(-)

diff --git a/csync/src/csync_statedb.c b/csync/src/csync_statedb.c
index 95b8e78..16446f8 100644
--- a/csync/src/csync_statedb.c
+++ b/csync/src/csync_statedb.c
@@ -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 ) {
@@ -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. */
diff --git a/src/libsync/syncjournaldb.h b/src/libsync/syncjournaldb.h
index cd45726..72db1c4 100644
--- a/src/libsync/syncjournaldb.h
+++ b/src/libsync/syncjournaldb.h
@@ -136,6 +136,14 @@ public:
     /**
      * 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);
 
diff --git a/test/testsyncengine.cpp b/test/testsyncengine.cpp
index 2498860..17bf0f7 100644
--- a/test/testsyncengine.cpp
+++ b/test/testsyncengine.cpp
@@ -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"));
+                QVERIFY(local.find("parentFolder/subFolder/fileB.txt"));
+            }
+        }
+    }
+
     void abortAfterFailedMkdir() {
         QSKIP("Skip for 2.3");
         FakeFolder fakeFolder{FileInfo{}};

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-owncloud/owncloud-client.git



More information about the Pkg-owncloud-commits mailing list