[Pkg-owncloud-commits] [owncloud-client] 83/94: SyncJournal: Don't use LIKE with paths
Sandro Knauß
hefee at debian.org
Thu Mar 29 11:12:17 UTC 2018
This is an automated email from the git hooks/post-receive script.
hefee pushed a commit to branch upstream
in repository owncloud-client.
commit 73062e21a390fd5bf8e3cd9269f524121dd5bf0d
Author: Christian Kamm <mail at ckamm.de>
Date: Wed Feb 7 13:05:41 2018 +0100
SyncJournal: Don't use LIKE with paths
Paths can contain the wildcards % and _ and that would lead to odd
behavior.
This patch also clarifies the behavior of avoidReadFromDbOnNextSync()
which previously dependend on whether "foo/bar" or "foo/bar/" was
passed as input.
Possibly affects #6322
---
src/common/syncjournaldb.cpp | 33 ++++++++-----
src/common/syncjournaldb.h | 11 +++--
test/testsyncengine.cpp | 8 +++
test/testsyncjournaldb.cpp | 115 +++++++++++++++++++++++++++++++++++++++++++
4 files changed, 151 insertions(+), 16 deletions(-)
diff --git a/src/common/syncjournaldb.cpp b/src/common/syncjournaldb.cpp
index e5b2f2d..27ac268 100644
--- a/src/common/syncjournaldb.cpp
+++ b/src/common/syncjournaldb.cpp
@@ -32,6 +32,13 @@
#include "common/c_jhash.h"
+// SQL expression to check whether path.startswith(prefix + '/')
+// Note: '/' + 1 == '0'
+#define IS_PREFIX_PATH_OF(prefix, path) \
+ "(" path " > (" prefix "||'/') AND " path " < (" prefix "||'0'))"
+#define IS_PREFIX_PATH_OR_EQUAL(prefix, path) \
+ "(" path " == " prefix " OR " IS_PREFIX_PATH_OF(prefix, path) ")"
+
namespace OCC {
Q_LOGGING_CATEGORY(lcDb, "sync.database", QtInfoMsg)
@@ -550,7 +557,7 @@ bool SyncJournalDb::checkConnect()
_getFilesBelowPathQuery.reset(new SqlQuery(_db));
if (_getFilesBelowPathQuery->prepare(
GET_FILE_RECORD_QUERY
- " WHERE path > (?1||'/') AND path < (?1||'0') ORDER BY path||'/' ASC")) {
+ " WHERE " IS_PREFIX_PATH_OF("?1", "path") " ORDER BY path||'/' ASC")) {
return sqlFail("prepare _getFilesBelowPathQuery", *_getFilesBelowPathQuery);
}
@@ -620,7 +627,7 @@ bool SyncJournalDb::checkConnect()
}
_deleteFileRecordRecursively.reset(new SqlQuery(_db));
- if (_deleteFileRecordRecursively->prepare("DELETE FROM metadata WHERE path LIKE(?||'/%')")) {
+ if (_deleteFileRecordRecursively->prepare("DELETE FROM metadata WHERE " IS_PREFIX_PATH_OF("?1", "path"))) {
return sqlFail("prepare _deleteFileRecordRecursively", *_deleteFileRecordRecursively);
}
@@ -1780,9 +1787,8 @@ void SyncJournalDb::avoidRenamesOnNextSync(const QByteArray &path)
}
SqlQuery query(_db);
- query.prepare("UPDATE metadata SET fileid = '', inode = '0' WHERE path == ?1 OR path LIKE(?2||'/%')");
+ query.prepare("UPDATE metadata SET fileid = '', inode = '0' WHERE " IS_PREFIX_PATH_OR_EQUAL("?1", "path"));
query.bindValue(1, path);
- query.bindValue(2, path);
query.exec();
// We also need to remove the ETags so the update phase refreshes the directory paths
@@ -1792,25 +1798,28 @@ void SyncJournalDb::avoidRenamesOnNextSync(const QByteArray &path)
void SyncJournalDb::avoidReadFromDbOnNextSync(const QByteArray &fileName)
{
- // 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
- // We achieve that by clearing the etag of the parents directory recursively
-
QMutexLocker locker(&_mutex);
if (!checkConnect()) {
return;
}
+ // Remove trailing slash
+ auto argument = fileName;
+ if (argument.endsWith('/'))
+ argument.chop(1);
+
SqlQuery query(_db);
// This query will match entries for which the path is a prefix of fileName
// Note: CSYNC_FTW_TYPE_DIR == 2
- query.prepare("UPDATE metadata SET md5='_invalid_' WHERE ?1 LIKE(path||'/%') AND type == 2;");
- query.bindValue(1, fileName);
+ query.prepare("UPDATE metadata SET md5='_invalid_' WHERE " IS_PREFIX_PATH_OR_EQUAL("path", "?1") " AND type == 2;");
+ query.bindValue(1, argument);
query.exec();
- // Prevent future overwrite of the etag for this sync
- _avoidReadFromDbOnNextSyncFilter.append(fileName);
+ // Prevent future overwrite of the etags of this folder and all
+ // parent folders for this sync
+ argument.append('/');
+ _avoidReadFromDbOnNextSyncFilter.append(argument);
}
void SyncJournalDb::forceRemoteDiscoveryNextSync()
diff --git a/src/common/syncjournaldb.h b/src/common/syncjournaldb.h
index 6baa138..8634697 100644
--- a/src/common/syncjournaldb.h
+++ b/src/common/syncjournaldb.h
@@ -158,15 +158,16 @@ public:
void setSelectiveSyncList(SelectiveSyncListType type, const QStringList &list);
/**
- * 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
+ * Make sure that on the next sync fileName and its parents are discovered from the server.
*
- * Specifically, this sets the md5 field of fileName and all its parents to _invalid_.
+ * That means its metadata and, if it's a directory, its direct contents.
+ *
+ * Specifically, etag (md5 field) of fileName and all its parents are set 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
+ * _csync_detect_update skip them), the _invalid_ marker will stay. And any
* child items in the db will be ignored when reading a remote tree from the database.
*/
void avoidReadFromDbOnNextSync(const QString &fileName) { avoidReadFromDbOnNextSync(fileName.toUtf8()); }
@@ -268,6 +269,8 @@ private:
/* This is the list of paths we called avoidReadFromDbOnNextSync on.
* It means that they should not be written to the DB in any case since doing
* that would write the etag and would void the purpose of avoidReadFromDbOnNextSync
+ *
+ * The contained paths have a trailing /.
*/
QList<QByteArray> _avoidReadFromDbOnNextSyncFilter;
diff --git a/test/testsyncengine.cpp b/test/testsyncengine.cpp
index 2250938..a7c0456 100644
--- a/test/testsyncengine.cpp
+++ b/test/testsyncengine.cpp
@@ -172,6 +172,14 @@ private slots:
fakeFolder.syncEngine().journal()->setSelectiveSyncList(SyncJournalDb::SelectiveSyncBlackList,
{"parentFolder/subFolderA/"});
fakeFolder.syncEngine().journal()->avoidReadFromDbOnNextSync(QByteArrayLiteral("parentFolder/subFolderA/"));
+ auto getEtag = [&](const QByteArray &file) {
+ SyncJournalFileRecord rec;
+ fakeFolder.syncJournal().getFileRecord(file, &rec);
+ return rec._etag;
+ };
+ QVERIFY(getEtag("parentFolder") == "_invalid_");
+ QVERIFY(getEtag("parentFolder/subFolderA") == "_invalid_");
+ QVERIFY(getEtag("parentFolder/subFolderA/subsubFolder") != "_invalid_");
// But touch local file before the next sync, such that the local folder
// can't be removed
diff --git a/test/testsyncjournaldb.cpp b/test/testsyncjournaldb.cpp
index cb86a16..e9fa385 100644
--- a/test/testsyncjournaldb.cpp
+++ b/test/testsyncjournaldb.cpp
@@ -183,6 +183,121 @@ private slots:
QCOMPARE(record.numericFileId(), QByteArray("123456789"));
}
+ void testAvoidReadFromDbOnNextSync()
+ {
+ auto invalidEtag = QByteArray("_invalid_");
+ auto initialEtag = QByteArray("etag");
+ auto makeEntry = [&](const QByteArray &path, int type) {
+ SyncJournalFileRecord record;
+ record._path = path;
+ record._type = type;
+ record._etag = initialEtag;
+ _db.setFileRecord(record);
+ };
+ auto getEtag = [&](const QByteArray &path) {
+ SyncJournalFileRecord record;
+ _db.getFileRecord(path, &record);
+ return record._etag;
+ };
+
+ makeEntry("foodir", 2);
+ makeEntry("otherdir", 2);
+ makeEntry("foo%", 2); // wildcards don't apply
+ makeEntry("foodi_", 2); // wildcards don't apply
+ makeEntry("foodir/file", 0);
+ makeEntry("foodir/subdir", 2);
+ makeEntry("foodir/subdir/file", 0);
+ makeEntry("foodir/otherdir", 2);
+ makeEntry("fo", 2); // prefix, but does not match
+ makeEntry("foodir/sub", 2); // prefix, but does not match
+ makeEntry("foodir/subdir/subsubdir", 2);
+ makeEntry("foodir/subdir/subsubdir/file", 0);
+ makeEntry("foodir/subdir/otherdir", 2);
+
+ _db.avoidReadFromDbOnNextSync(QByteArray("foodir/subdir"));
+
+ // Direct effects of parent directories being set to _invalid_
+ QCOMPARE(getEtag("foodir"), invalidEtag);
+ QCOMPARE(getEtag("foodir/subdir"), invalidEtag);
+ QCOMPARE(getEtag("foodir/subdir/subsubdir"), initialEtag);
+
+ QCOMPARE(getEtag("foodir/file"), initialEtag);
+ QCOMPARE(getEtag("foodir/subdir/file"), initialEtag);
+ QCOMPARE(getEtag("foodir/subdir/subsubdir/file"), initialEtag);
+
+ QCOMPARE(getEtag("fo"), initialEtag);
+ QCOMPARE(getEtag("foo%"), initialEtag);
+ QCOMPARE(getEtag("foodi_"), initialEtag);
+ QCOMPARE(getEtag("otherdir"), initialEtag);
+ QCOMPARE(getEtag("foodir/otherdir"), initialEtag);
+ QCOMPARE(getEtag("foodir/sub"), initialEtag);
+ QCOMPARE(getEtag("foodir/subdir/otherdir"), initialEtag);
+
+ // Indirect effects: setFileRecord() calls filter etags
+ initialEtag = "etag2";
+
+ makeEntry("foodir", 2);
+ QCOMPARE(getEtag("foodir"), invalidEtag);
+ makeEntry("foodir/subdir", 2);
+ QCOMPARE(getEtag("foodir/subdir"), invalidEtag);
+ makeEntry("foodir/subdir/subsubdir", 2);
+ QCOMPARE(getEtag("foodir/subdir/subsubdir"), initialEtag);
+ makeEntry("fo", 2);
+ QCOMPARE(getEtag("fo"), initialEtag);
+ makeEntry("foodir/sub", 2);
+ QCOMPARE(getEtag("foodir/sub"), initialEtag);
+ }
+
+ void testRecursiveDelete()
+ {
+ auto makeEntry = [&](const QByteArray &path) {
+ SyncJournalFileRecord record;
+ record._path = path;
+ _db.setFileRecord(record);
+ };
+
+ QByteArrayList elements;
+ elements
+ << "foo"
+ << "foo/file"
+ << "bar"
+ << "moo"
+ << "moo/file"
+ << "foo%bar"
+ << "foo bla bar/file"
+ << "fo_"
+ << "fo_/file";
+ for (auto elem : elements)
+ makeEntry(elem);
+
+ auto checkElements = [&]() {
+ bool ok = true;
+ for (auto elem : elements) {
+ SyncJournalFileRecord record;
+ _db.getFileRecord(elem, &record);
+ if (!record.isValid()) {
+ qWarning() << "Missing record: " << elem;
+ ok = false;
+ }
+ }
+ return ok;
+ };
+
+ _db.deleteFileRecord("moo", true);
+ elements.removeAll("moo");
+ elements.removeAll("moo/file");
+ QVERIFY(checkElements());
+
+ _db.deleteFileRecord("fo_", true);
+ elements.removeAll("fo_");
+ elements.removeAll("fo_/file");
+ QVERIFY(checkElements());
+
+ _db.deleteFileRecord("foo%bar", true);
+ elements.removeAll("foo%bar");
+ QVERIFY(checkElements());
+ }
+
private:
SyncJournalDb _db;
};
--
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