Merge pull request #303 from nextcloud/upstream/issue/6322

SyncJournal: Don't use LIKE with paths
This commit is contained in:
Roeland Jago Douma 2018-05-18 13:56:25 +02:00 committed by GitHub
commit b7d5aff901
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 153 additions and 19 deletions

View file

@ -33,6 +33,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, "nextcloud.sync.database", QtInfoMsg)
@ -572,14 +579,13 @@ bool SyncJournalDb::checkConnect()
// database instead
_getFilesBelowPathQuery.reset(new SqlQuery(_db));
if (_getFilesBelowPathQuery->prepare(
GET_FILE_RECORD_QUERY
" WHERE path > (?1||'/') AND path < (?1||'0')"
// We want to ensure that the contents of a directory are sorted
// directly behind the directory itself. Without this ORDER BY
// an ordering like foo, foo-2, foo/file would be returned.
// With the trailing /, we get foo-2, foo, foo/file. This property
// is used in fill_tree_from_db().
" ORDER BY path||'/' ASC")) {
GET_FILE_RECORD_QUERY
" WHERE " IS_PREFIX_PATH_OF("?1", "path") " ORDER BY path||'/' ASC")) {
return sqlFail("prepare _getFilesBelowPathQuery", *_getFilesBelowPathQuery);
}
@ -656,7 +662,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);
}
@ -1907,9 +1913,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
@ -1919,25 +1924,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: ItemTypeDirectory == 2
query.prepare("UPDATE metadata SET md5='_invalid_' WHERE ?1 LIKE(path||'/%') AND type == 2;");
query.bindValue(1, fileName);
// Note: CSYNC_FTW_TYPE_DIR == 2
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()

View file

@ -161,15 +161,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()); }
@ -292,6 +293,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;

View file

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

View file

@ -205,6 +205,121 @@ private slots:
QVERIFY(!_db.conflictRecord(record.path).isValid());
}
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;
};