From: Christian Kamm Date: Thu, 27 Jun 2019 13:47:04 +0000 (+0200) Subject: VfsSuffix: Wipe stale pin states #7273 X-Git-Tag: archive/raspbian/3.16.7-1_deb13u1+rpi1~1^2~12^2~21^2~468^2~224 X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=5acb157a7ee82dd5122b606e58af78cf360e9d43;p=nextcloud-desktop.git VfsSuffix: Wipe stale pin states #7273 Previously the pin states of deleted files stayed in the 'flags' database and could be inadvertently reused when a new file with the same name appeared. Now they are deleted. To make this work right, the meaning of the 'path' column in the 'flags' table was changed: Previously it never had the .owncloud file suffix. Now it's the same as in metadata.path. This takes the safe parts from #7274 for inclusion in 2.6. The more elaborate database schema changes (why use 'path' the join the two tables in the first place?) shall go into master. --- diff --git a/src/common/syncjournaldb.cpp b/src/common/syncjournaldb.cpp index 0645ad463..c301ddccf 100644 --- a/src/common/syncjournaldb.cpp +++ b/src/common/syncjournaldb.cpp @@ -1656,6 +1656,16 @@ bool SyncJournalDb::deleteStaleErrorBlacklistEntries(const QSet &keep) return deleteBatch(delQuery, superfluousPaths, "blacklist"); } +void SyncJournalDb::deleteStaleFlagsEntries() +{ + QMutexLocker locker(&_mutex); + if (!checkConnect()) + return; + + SqlQuery delQuery("DELETE FROM flags WHERE path != '' AND path NOT IN (SELECT path from metadata);", _db); + delQuery.exec(); +} + int SyncJournalDb::errorBlackListEntryCount() { int re = 0; diff --git a/src/common/syncjournaldb.h b/src/common/syncjournaldb.h index 9ef1e7d72..aa4b615ba 100644 --- a/src/common/syncjournaldb.h +++ b/src/common/syncjournaldb.h @@ -141,6 +141,9 @@ public: SyncJournalErrorBlacklistRecord errorBlacklistEntry(const QString &); bool deleteStaleErrorBlacklistEntries(const QSet &keep); + /// Delete flags table entries that have no metadata correspondent + void deleteStaleFlagsEntries(); + void avoidRenamesOnNextSync(const QString &path) { avoidRenamesOnNextSync(path.toUtf8()); } void avoidRenamesOnNextSync(const QByteArray &path); void setPollInfo(const PollInfo &); diff --git a/src/common/vfs.cpp b/src/common/vfs.cpp index 745f66a3b..efec96639 100644 --- a/src/common/vfs.cpp +++ b/src/common/vfs.cpp @@ -77,14 +77,16 @@ bool Vfs::setPinStateInDb(const QString &folderPath, PinState state) Optional Vfs::pinStateInDb(const QString &folderPath) { - return _setupParams.journal->internalPinStates().effectiveForPath(folderPath.toUtf8()); + auto pin = _setupParams.journal->internalPinStates().effectiveForPath(folderPath.toUtf8()); + return pin; } -Vfs::AvailabilityResult Vfs::availabilityInDb(const QString &folderPath, const QString &pinPath) +Vfs::AvailabilityResult Vfs::availabilityInDb(const QString &folderPath) { - auto pin = _setupParams.journal->internalPinStates().effectiveForPathRecursive(pinPath.toUtf8()); + auto path = folderPath.toUtf8(); + auto pin = _setupParams.journal->internalPinStates().effectiveForPathRecursive(path); // not being able to retrieve the pin state isn't too bad - auto hydrationStatus = _setupParams.journal->hasHydratedOrDehydratedFiles(folderPath.toUtf8()); + auto hydrationStatus = _setupParams.journal->hasHydratedOrDehydratedFiles(path); if (!hydrationStatus) return AvailabilityError::DbError; diff --git a/src/common/vfs.h b/src/common/vfs.h index 916c009a6..e587efd36 100644 --- a/src/common/vfs.h +++ b/src/common/vfs.h @@ -257,8 +257,7 @@ protected: // Db-backed pin state handling. Derived classes may use it to implement pin states. bool setPinStateInDb(const QString &folderPath, PinState state); Optional pinStateInDb(const QString &folderPath); - // sadly for virtual files the path in the metadata table can differ from path in 'flags' - AvailabilityResult availabilityInDb(const QString &folderPath, const QString &pinPath); + AvailabilityResult availabilityInDb(const QString &folderPath); // the parameters passed to start() VfsSetupParams _setupParams; diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index 5b5feb9d8..88a1e59de 100644 --- a/src/gui/folder.cpp +++ b/src/gui/folder.cpp @@ -617,12 +617,9 @@ void Folder::implicitlyHydrateFile(const QString &relativepath) // Change the file's pin state if it's contradictory to being hydrated // (suffix-virtual file's pin state is stored at the hydrated path) - QString pinPath = relativepath; - if (_vfs->mode() == Vfs::WithSuffix && pinPath.endsWith(_vfs->fileSuffix())) - pinPath.chop(_vfs->fileSuffix().size()); - const auto pin = _vfs->pinState(pinPath); + const auto pin = _vfs->pinState(relativepath); if (pin && *pin == PinState::OnlineOnly) { - _vfs->setPinState(pinPath, PinState::Unspecified); + _vfs->setPinState(relativepath, PinState::Unspecified); } // Add to local discovery diff --git a/src/gui/socketapi.cpp b/src/gui/socketapi.cpp index 6f3b534e9..32f295ea3 100644 --- a/src/gui/socketapi.cpp +++ b/src/gui/socketapi.cpp @@ -726,8 +726,7 @@ void SocketApi::command_MAKE_AVAILABLE_LOCALLY(const QString &filesArg, SocketLi continue; // Update the pin state on all items - auto pinPath = data.folderRelativePathNoVfsSuffix(); - data.folder->vfs().setPinState(pinPath, PinState::AlwaysLocal); + data.folder->vfs().setPinState(data.folderRelativePath, PinState::AlwaysLocal); // Trigger sync data.folder->schedulePathForLocalDiscovery(data.folderRelativePath); @@ -746,8 +745,7 @@ void SocketApi::command_MAKE_ONLINE_ONLY(const QString &filesArg, SocketListener continue; // Update the pin state on all items - auto pinPath = data.folderRelativePathNoVfsSuffix(); - data.folder->vfs().setPinState(pinPath, PinState::OnlineOnly); + data.folder->vfs().setPinState(data.folderRelativePath, PinState::OnlineOnly); // Trigger sync data.folder->schedulePathForLocalDiscovery(data.folderRelativePath); diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index 1d75772dd..e891c666f 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -1468,13 +1468,7 @@ void ProcessDirectoryJob::setupDbPinStateActions(SyncJournalFileRecord &record) if (!isVfsWithSuffix()) return; - QByteArray pinPath = record._path; - if (record.isVirtualFile()) { - const auto suffix = _discoveryData->_syncOptions._vfs->fileSuffix().toUtf8(); - if (pinPath.endsWith(suffix)) - pinPath.chop(suffix.size()); - } - auto pin = _discoveryData->_statedb->internalPinStates().rawForPath(pinPath); + auto pin = _discoveryData->_statedb->internalPinStates().rawForPath(record._path); if (!pin || *pin == PinState::Inherited) pin = _pinState; diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp index 92fa83951..5bd245c49 100644 --- a/src/libsync/propagatedownload.cpp +++ b/src/libsync/propagatedownload.cpp @@ -1027,6 +1027,13 @@ void PropagateDownloadFile::downloadFinished() qCDebug(lcPropagateDownload) << "Download of previous virtual file finished" << fn; QFile::remove(fn); propagator()->_journal->deleteFileRecord(virtualFile); + + // Move the pin state to the new location + auto pin = propagator()->_journal->internalPinStates().rawForPath(_item->_file.toUtf8()); + if (pin && *pin != PinState::Inherited) { + vfs->setPinState(virtualFile, *pin); + vfs->setPinState(_item->_file, PinState::Inherited); + } } } diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index d5bcd0cc6..a552cf223 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -842,6 +842,7 @@ void SyncEngine::slotPropagationFinished(bool success) conflictRecordMaintenance(); + _journal->deleteStaleFlagsEntries(); _journal->commit("All Finished.", false); // Send final progress information even if no diff --git a/src/libsync/vfs/suffix/vfs_suffix.cpp b/src/libsync/vfs/suffix/vfs_suffix.cpp index 5a2d829c8..474e7b77e 100644 --- a/src/libsync/vfs/suffix/vfs_suffix.cpp +++ b/src/libsync/vfs/suffix/vfs_suffix.cpp @@ -18,6 +18,7 @@ #include "syncfileitem.h" #include "filesystem.h" +#include "common/syncjournaldb.h" namespace OCC { @@ -81,6 +82,13 @@ void VfsSuffix::dehydratePlaceholder(const SyncFileItem &item) SyncFileItem virtualItem(item); virtualItem._file = item._renameTarget; createPlaceholder(virtualItem); + + // Move the item's pin state + auto pin = _setupParams.journal->internalPinStates().rawForPath(item._file.toUtf8()); + if (pin && *pin != PinState::Inherited) { + setPinState(item._renameTarget, *pin); + setPinState(item._file, PinState::Inherited); + } } void VfsSuffix::convertToPlaceholder(const QString &, const SyncFileItem &, const QString &) @@ -107,11 +115,7 @@ bool VfsSuffix::statTypeVirtualFile(csync_file_stat_t *stat, void *) Vfs::AvailabilityResult VfsSuffix::availability(const QString &folderPath) { - const auto suffix = fileSuffix(); - QString pinPath = folderPath; - if (pinPath.endsWith(suffix)) - pinPath.chop(suffix.size()); - return availabilityInDb(folderPath, pinPath); + return availabilityInDb(folderPath); } } // namespace OCC diff --git a/test/testsyncvirtualfiles.cpp b/test/testsyncvirtualfiles.cpp index d4efc9304..48e8c0e5d 100644 --- a/test/testsyncvirtualfiles.cpp +++ b/test/testsyncvirtualfiles.cpp @@ -1072,7 +1072,7 @@ private slots: triggerDownload(fakeFolder, "unspec/file1"); setPin("local/file2", PinState::OnlineOnly); - setPin("online/file2", PinState::AlwaysLocal); + setPin("online/file2.owncloud", PinState::AlwaysLocal); QVERIFY(fakeFolder.syncOnce()); QCOMPARE(*vfs->availability("unspec"), VfsItemAvailability::AllHydrated); @@ -1120,7 +1120,7 @@ private slots: QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); // root is unspecified - QCOMPARE(*vfs->pinState("file1"), PinState::Unspecified); + QCOMPARE(*vfs->pinState("file1.owncloud"), PinState::Unspecified); QCOMPARE(*vfs->pinState("local/file1"), PinState::AlwaysLocal); QCOMPARE(*vfs->pinState("online/file1"), PinState::Unspecified); QCOMPARE(*vfs->pinState("unspec/file1"), PinState::Unspecified); @@ -1148,6 +1148,17 @@ private slots: QCOMPARE(*vfs->pinState("onlinerenamed2/file1rename"), PinState::Unspecified); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + + // When a file is deleted and later a new file has the same name, the old pin + // state isn't preserved. + QCOMPARE(*vfs->pinState("onlinerenamed2/file1rename"), PinState::Unspecified); + fakeFolder.remoteModifier().remove("onlinerenamed2/file1rename"); + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(*vfs->pinState("onlinerenamed2/file1rename"), PinState::OnlineOnly); + fakeFolder.remoteModifier().insert("onlinerenamed2/file1rename"); + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(*vfs->pinState("onlinerenamed2/file1rename"), PinState::OnlineOnly); + QCOMPARE(*vfs->pinState("onlinerenamed2/file1rename.owncloud"), PinState::OnlineOnly); } };