Skip to content

Commit

Permalink
Ensure local discovery on selective sync changes
Browse files Browse the repository at this point in the history
As far as I'm aware local discovery can be skipped on folders that are
selective-sync blacklisted, so a local discovery is required when an
entry is removed from the blacklist.

Also rename
avoidReadFromDbOnNextSync() -> schedulePathForRemoteDiscovery()
since the old name might also imply it's not read from db in the local
discovery - which is not the case. Use Folder::
schedulePathForLocalDiscovery() for that.
  • Loading branch information
ckamm committed Feb 14, 2019
1 parent cf4ffd8 commit a627f37
Show file tree
Hide file tree
Showing 17 changed files with 43 additions and 25 deletions.
2 changes: 1 addition & 1 deletion src/cmd/cmd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ void selectiveSyncFixup(OCC::SyncJournalDb *journal, const QStringList &newList)
auto blackListSet = newList.toSet();
auto changes = (oldBlackListSet - blackListSet) + (blackListSet - oldBlackListSet);
foreach (const auto &it, changes) {
journal->avoidReadFromDbOnNextSync(it);
journal->schedulePathForRemoteDiscovery(it);
}

journal->setSelectiveSyncList(SyncJournalDb::SelectiveSyncBlackList, newList);
Expand Down
6 changes: 3 additions & 3 deletions src/common/syncjournaldb.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1752,10 +1752,10 @@ void SyncJournalDb::avoidRenamesOnNextSync(const QByteArray &path)

// We also need to remove the ETags so the update phase refreshes the directory paths
// on the next sync
avoidReadFromDbOnNextSync(path);
schedulePathForRemoteDiscovery(path);
}

void SyncJournalDb::avoidReadFromDbOnNextSync(const QByteArray &fileName)
void SyncJournalDb::schedulePathForRemoteDiscovery(const QByteArray &fileName)
{
QMutexLocker locker(&_mutex);

Expand Down Expand Up @@ -2009,7 +2009,7 @@ void SyncJournalDb::markVirtualFileForDownloadRecursively(const QByteArray &path
query.bindValue(1, path);
query.exec();

// We also must make sure we do not read the files from the database (same logic as in avoidReadFromDbOnNextSync)
// We also must make sure we do not read the files from the database (same logic as in schedulePathForRemoteDiscovery)
// This includes all the parents up to the root, but also all the directory within the selected dir.
static_assert(ItemTypeDirectory == 2, "");
query.prepare("UPDATE metadata SET md5='_invalid_' WHERE "
Expand Down
8 changes: 4 additions & 4 deletions src/common/syncjournaldb.h
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,8 @@ class OCSYNC_EXPORT SyncJournalDb : public QObject
* Any setFileRecord() call to affected directories before the next sync run will be
* adjusted to retain the invalid etag via _etagStorageFilter.
*/
void avoidReadFromDbOnNextSync(const QString &fileName) { avoidReadFromDbOnNextSync(fileName.toUtf8()); }
void avoidReadFromDbOnNextSync(const QByteArray &fileName);
void schedulePathForRemoteDiscovery(const QString &fileName) { schedulePathForRemoteDiscovery(fileName.toUtf8()); }
void schedulePathForRemoteDiscovery(const QByteArray &fileName);

/**
* Wipe _etagStorageFilter. Also done implicitly on close().
Expand All @@ -191,7 +191,7 @@ class OCSYNC_EXPORT SyncJournalDb : public QObject
/**
* Ensures full remote discovery happens on the next sync.
*
* Equivalent to calling avoidReadFromDbOnNextSync() for all files.
* Equivalent to calling schedulePathForRemoteDiscovery() for all files.
*/
void forceRemoteDiscoveryNextSync();

Expand Down Expand Up @@ -400,7 +400,7 @@ class OCSYNC_EXPORT SyncJournalDb : public QObject

/* Storing etags to these folders, or their parent folders, is filtered out.
*
* When avoidReadFromDbOnNextSync() is called some etags to _invalid_ in the
* When schedulePathForRemoteDiscovery() is called some etags to _invalid_ in the
* database. If this is done during a sync run, a later propagation job might
* undo that by writing the correct etag to the database instead. This filter
* will prevent this write and instead guarantee the _invalid_ etag stays in
Expand Down
3 changes: 2 additions & 1 deletion src/gui/accountsettings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,8 @@ void AccountSettings::slotEnableVfsCurrentFolder()
auto oldBlacklist = folder->journalDb()->getSelectiveSyncList(SyncJournalDb::SelectiveSyncBlackList, &ok);
folder->journalDb()->setSelectiveSyncList(SyncJournalDb::SelectiveSyncBlackList, {});
for (const auto &entry : oldBlacklist) {
folder->journalDb()->avoidReadFromDbOnNextSync(entry);
folder->journalDb()->schedulePathForRemoteDiscovery(entry);
folder->schedulePathForLocalDiscovery(entry);
}

// Change the folder vfs mode and load the plugin
Expand Down
10 changes: 8 additions & 2 deletions src/gui/folder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -590,8 +590,9 @@ void Folder::downloadVirtualFile(const QString &_relativepath)
if (record._type == ItemTypeVirtualFile) {
record._type = ItemTypeVirtualFileDownload;
_journal.setFileRecord(record);
// Make sure we go over that file during the discovery
_journal.avoidReadFromDbOnNextSync(relativepath);
// Make sure we go over that file during the discovery even if
// no actual remote discovery would be necessary
_journal.schedulePathForRemoteDiscovery(relativepath);
} else if (record._type == ItemTypeDirectory || relativepath.isEmpty()) {
_journal.markVirtualFileForDownloadRecursively(relativepath);
} else {
Expand Down Expand Up @@ -1127,6 +1128,11 @@ void Folder::slotNextSyncFullLocalDiscovery()
_timeSinceLastFullLocalDiscovery.invalidate();
}

void Folder::schedulePathForLocalDiscovery(const QString &relativePath)
{
_localDiscoveryTracker->addTouchedPath(relativePath.toUtf8());
}

void Folder::slotFolderConflicts(const QString &folder, const QStringList &conflictPaths)
{
if (folder != _definition.alias)
Expand Down
8 changes: 8 additions & 0 deletions src/gui/folder.h
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,14 @@ public slots:
/** Ensures that the next sync performs a full local discovery. */
void slotNextSyncFullLocalDiscovery();

/** Adds the path to the local discovery list
*
* A weaker version of slotNextSyncFullLocalDiscovery() that just
* schedules all parent and child items of the path for local
* discovery.
*/
void schedulePathForLocalDiscovery(const QString &relativePath);

private slots:
void slotSyncStarted();
void slotSyncFinished(bool);
Expand Down
6 changes: 4 additions & 2 deletions src/gui/folderstatusmodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -851,7 +851,8 @@ void FolderStatusModel::slotApplySelectiveSync()
//The part that changed should not be read from the DB on next sync because there might be new folders
// (the ones that are no longer in the blacklist)
foreach (const auto &it, changes) {
folder->journalDb()->avoidReadFromDbOnNextSync(it);
folder->journalDb()->schedulePathForRemoteDiscovery(it);
folder->schedulePathForLocalDiscovery(it);
}
// Also make sure we see the local file that had been ignored before
folder->slotNextSyncFullLocalDiscovery();
Expand Down Expand Up @@ -1164,7 +1165,8 @@ void FolderStatusModel::slotSyncAllPendingBigFolders()
// The part that changed should not be read from the DB on next sync because there might be new folders
// (the ones that are no longer in the blacklist)
foreach (const auto &it, undecidedList) {
folder->journalDb()->avoidReadFromDbOnNextSync(it);
folder->journalDb()->schedulePathForRemoteDiscovery(it);
folder->schedulePathForLocalDiscovery(it);
}
// Also make sure we see the local file that had been ignored before
folder->slotNextSyncFullLocalDiscovery();
Expand Down
3 changes: 2 additions & 1 deletion src/gui/selectivesyncdialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,8 @@ void SelectiveSyncDialog::accept()
auto blackListSet = blackList.toSet();
auto changes = (oldBlackListSet - blackListSet) + (blackListSet - oldBlackListSet);
foreach (const auto &it, changes) {
_folder->journalDb()->avoidReadFromDbOnNextSync(it);
_folder->journalDb()->schedulePathForRemoteDiscovery(it);
_folder->schedulePathForLocalDiscovery(it);
}
// Also make sure we see the local file that had been ignored before
_folder->slotNextSyncFullLocalDiscovery();
Expand Down
2 changes: 1 addition & 1 deletion src/gui/sharemanager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ static void updateFolder(const AccountPtr &account, const QString &path)
// Workaround the fact that the server does not invalidate the etags of parent directories
// when something is shared.
auto relative = path.midRef(f->remotePathTrailingSlash().length());
f->journalDb()->avoidReadFromDbOnNextSync(relative.toString());
f->journalDb()->schedulePathForRemoteDiscovery(relative.toString());

// Schedule a sync so it can update the remote permission flag and let the socket API
// know about the shared icon.
Expand Down
2 changes: 1 addition & 1 deletion src/libsync/propagatedownload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,7 @@ void PropagateDownloadFile::slotGetFinished()
// As a precaution against bugs that cause our database and the
// reality on the server to diverge, rediscover this folder on the
// next sync run.
propagator()->_journal->avoidReadFromDbOnNextSync(_item->_file);
propagator()->_journal->schedulePathForRemoteDiscovery(_item->_file);
}

QByteArray errorBody;
Expand Down
2 changes: 1 addition & 1 deletion src/libsync/propagateupload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ void PropagateUploadFileCommon::commonErrorHandling(AbstractNetworkJob *job)

// Maybe the bad etag is in the database, we need to clear the
// parent folder etag so we won't read from DB next sync.
propagator()->_journal->avoidReadFromDbOnNextSync(_item->_file);
propagator()->_journal->schedulePathForRemoteDiscovery(_item->_file);
propagator()->_anotherSyncNeeded = true;
}

Expand Down
2 changes: 1 addition & 1 deletion src/libsync/syncengine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ void SyncEngine::startSync()
}

// Functionality like selective sync might have set up etag storage
// filtering via avoidReadFromDbOnNextSync(). This *is* the next sync, so
// filtering via schedulePathForRemoteDiscovery(). This *is* the next sync, so
// undo the filter to allow this sync to retrieve and store the correct etags.
_journal->clearEtagStorageFilter();

Expand Down
4 changes: 2 additions & 2 deletions test/testselectivesync.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ private slots:

auto oldSync = fakeFolder.currentLocalState();
// syncing again should do the same
fakeFolder.syncEngine().journal()->avoidReadFromDbOnNextSync(QString("A/newBigDir"));
fakeFolder.syncEngine().journal()->schedulePathForRemoteDiscovery(QString("A/newBigDir"));
QVERIFY(fakeFolder.syncOnce());
QCOMPARE(fakeFolder.currentLocalState(), oldSync);
QCOMPARE(newBigFolder.count(), 1); // (since we don't have a real Folder, the files were not added to any list)
Expand All @@ -80,7 +80,7 @@ private slots:
// Simulate that we accept all files by seting a wildcard white list
fakeFolder.syncEngine().journal()->setSelectiveSyncList(SyncJournalDb::SelectiveSyncWhiteList,
QStringList() << QLatin1String("/"));
fakeFolder.syncEngine().journal()->avoidReadFromDbOnNextSync(QString("A/newBigDir"));
fakeFolder.syncEngine().journal()->schedulePathForRemoteDiscovery(QString("A/newBigDir"));
QVERIFY(fakeFolder.syncOnce());
QCOMPARE(newBigFolder.count(), 0);
QCOMPARE(sizeRequests.count(), 0);
Expand Down
2 changes: 1 addition & 1 deletion test/testsyncengine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ private slots:
// Remove subFolderA with selectiveSync:
fakeFolder.syncEngine().journal()->setSelectiveSyncList(SyncJournalDb::SelectiveSyncBlackList,
{"parentFolder/subFolderA/"});
fakeFolder.syncEngine().journal()->avoidReadFromDbOnNextSync(QByteArrayLiteral("parentFolder/subFolderA/"));
fakeFolder.syncEngine().journal()->schedulePathForRemoteDiscovery(QByteArrayLiteral("parentFolder/subFolderA/"));
auto getEtag = [&](const QByteArray &file) {
SyncJournalFileRecord rec;
fakeFolder.syncJournal().getFileRecord(file, &rec);
Expand Down
2 changes: 1 addition & 1 deletion test/testsyncjournaldb.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ private slots:
makeEntry("foodir/subdir/subsubdir/file", fileType);
makeEntry("foodir/subdir/otherdir", dirType);

_db.avoidReadFromDbOnNextSync(QByteArray("foodir/subdir"));
_db.schedulePathForRemoteDiscovery(QByteArray("foodir/subdir"));

// Direct effects of parent directories being set to _invalid_
QCOMPARE(getEtag("foodir"), invalidEtag);
Expand Down
2 changes: 1 addition & 1 deletion test/testsyncmove.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ private slots:
// Remove subFolderA with selectiveSync:
fakeFolder.syncEngine().journal()->setSelectiveSyncList(SyncJournalDb::SelectiveSyncBlackList,
{ "parentFolder/subFolderA/" });
fakeFolder.syncEngine().journal()->avoidReadFromDbOnNextSync(QByteArrayLiteral("parentFolder/subFolderA/"));
fakeFolder.syncEngine().journal()->schedulePathForRemoteDiscovery(QByteArrayLiteral("parentFolder/subFolderA/"));

fakeFolder.syncOnce();

Expand Down
4 changes: 2 additions & 2 deletions test/testsyncvirtualfiles.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ void triggerDownload(FakeFolder &folder, const QByteArray &path)
return;
record._type = ItemTypeVirtualFileDownload;
journal.setFileRecord(record);
journal.avoidReadFromDbOnNextSync(record._path);
journal.schedulePathForRemoteDiscovery(record._path);
}

void markForDehydration(FakeFolder &folder, const QByteArray &path)
Expand All @@ -56,7 +56,7 @@ void markForDehydration(FakeFolder &folder, const QByteArray &path)
return;
record._type = ItemTypeVirtualFileDehydration;
journal.setFileRecord(record);
journal.avoidReadFromDbOnNextSync(record._path);
journal.schedulePathForRemoteDiscovery(record._path);
}

QSharedPointer<Vfs> setupVfs(FakeFolder &folder)
Expand Down

0 comments on commit a627f37

Please sign in to comment.