From 2a529eef3c317192765822c606f337a2ae0aca8d Mon Sep 17 00:00:00 2001 From: alex-z Date: Thu, 8 Sep 2022 14:21:30 +0300 Subject: [PATCH] Make sure Folder is deleted from the list and the SyncJournalDB is closed for every folder of the account that has been removed. Signed-off-by: alex-z --- src/common/syncjournaldb.cpp | 4 ++- src/gui/folder.cpp | 8 ----- src/gui/folder.h | 2 -- src/gui/folderman.cpp | 16 +++++---- src/gui/folderman.h | 2 +- src/gui/wizard/owncloudadvancedsetuppage.cpp | 2 +- test/testfolderman.cpp | 34 ++++++++++---------- 7 files changed, 32 insertions(+), 36 deletions(-) diff --git a/src/common/syncjournaldb.cpp b/src/common/syncjournaldb.cpp index b5e4f97ca..a2caff63a 100644 --- a/src/common/syncjournaldb.cpp +++ b/src/common/syncjournaldb.cpp @@ -2425,7 +2425,9 @@ void SyncJournalDb::commitInternal(const QString &context, bool startTrans) SyncJournalDb::~SyncJournalDb() { - close(); + if (isOpen()) { + close(); + } } diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index e76383150..7229c1ba4 100644 --- a/src/gui/folder.cpp +++ b/src/gui/folder.cpp @@ -302,14 +302,6 @@ void Folder::setSyncPaused(bool paused) emit canSyncChanged(); } -void Folder::onAssociatedAccountRemoved() -{ - if (_vfs) { - _vfs->stop(); - _vfs->unregisterFolder(); - } -} - void Folder::setSyncState(SyncResult::Status state) { _syncResult.setStatus(state); diff --git a/src/gui/folder.h b/src/gui/folder.h index 02134d907..87e4e7a4c 100644 --- a/src/gui/folder.h +++ b/src/gui/folder.h @@ -206,8 +206,6 @@ public: */ virtual void wipeForRemoval(); - void onAssociatedAccountRemoved(); - void setSyncState(SyncResult::Status state); void setDirtyNetworkLimits(); diff --git a/src/gui/folderman.cpp b/src/gui/folderman.cpp index 314b70917..97d4514d2 100644 --- a/src/gui/folderman.cpp +++ b/src/gui/folderman.cpp @@ -945,11 +945,15 @@ void FolderMan::runEtagJobIfPossible(Folder *folder) void FolderMan::slotAccountRemoved(AccountState *accountState) { + QVector foldersToRemove; for (const auto &folder : qAsConst(_folderMap)) { if (folder->accountState() == accountState) { - folder->onAssociatedAccountRemoved(); + foldersToRemove.push_back(folder); } } + for (const auto &folder : qAsConst(foldersToRemove)) { + removeFolder(folder); + } } void FolderMan::slotRemoveFoldersForAccount(AccountState *accountState) @@ -1663,10 +1667,10 @@ QPair FolderMan::checkPathValidityForNew { QPair result; - QString recursiveValidity = checkPathValidityRecursive(path); + const auto recursiveValidity = checkPathValidityRecursive(path); if (!recursiveValidity.isEmpty()) { qCDebug(lcFolderMan) << path << recursiveValidity; - result.first = FolderMan::ErrorRecursiveValidity; + result.first = FolderMan::PathValidityResult::ErrorRecursiveValidity; result.second = recursiveValidity; return result; } @@ -1684,7 +1688,7 @@ QPair FolderMan::checkPathValidityForNew bool differentPaths = QString::compare(folderDir, userDir, cs) != 0; if (differentPaths && folderDir.startsWith(userDir, cs)) { - result.first = FolderMan::ErrorContainsFolder; + result.first = FolderMan::PathValidityResult::ErrorContainsFolder; result.second = tr("The local folder %1 already contains a folder used in a folder sync connection. " "Please pick another one!") .arg(QDir::toNativeSeparators(path)); @@ -1692,7 +1696,7 @@ QPair FolderMan::checkPathValidityForNew } if (differentPaths && userDir.startsWith(folderDir, cs)) { - result.first = FolderMan::ErrorContainedInFolder; + result.first = FolderMan::PathValidityResult::ErrorContainedInFolder; result.second = tr("The local folder %1 is already contained in a folder used in a folder sync connection. " "Please pick another one!") .arg(QDir::toNativeSeparators(path)); @@ -1708,7 +1712,7 @@ QPair FolderMan::checkPathValidityForNew folderUrl.setUserName(user); if (serverUrl == folderUrl) { - result.first = FolderMan::ErrorNonEmptyFolder; + result.first = FolderMan::PathValidityResult::ErrorNonEmptyFolder; result.second = tr("There is already a sync from the server to this local folder. " "Please pick another local folder!"); return result; diff --git a/src/gui/folderman.h b/src/gui/folderman.h index 495138b67..985fcb50b 100644 --- a/src/gui/folderman.h +++ b/src/gui/folderman.h @@ -63,7 +63,7 @@ class FolderMan : public QObject { Q_OBJECT public: - enum PathValidityResult { + enum class PathValidityResult { Valid, ErrorRecursiveValidity, ErrorContainsFolder, diff --git a/src/gui/wizard/owncloudadvancedsetuppage.cpp b/src/gui/wizard/owncloudadvancedsetuppage.cpp index c050253ed..ccd54edc4 100644 --- a/src/gui/wizard/owncloudadvancedsetuppage.cpp +++ b/src/gui/wizard/owncloudadvancedsetuppage.cpp @@ -260,7 +260,7 @@ void OwncloudAdvancedSetupPage::updateStatus() // check if the local folder exists. If so, and if its not empty, show a warning. const auto pathValidityCheckResult = FolderMan::instance()->checkPathValidityForNewFolder(locFolder, serverUrl()); auto errorStr = pathValidityCheckResult.second; - _localFolderValid = errorStr.isEmpty() || pathValidityCheckResult.first == FolderMan::ErrorNonEmptyFolder; + _localFolderValid = errorStr.isEmpty() || pathValidityCheckResult.first == FolderMan::PathValidityResult::ErrorNonEmptyFolder; QString t; diff --git a/test/testfolderman.cpp b/test/testfolderman.cpp index 0035ad728..e66049f57 100644 --- a/test/testfolderman.cpp +++ b/test/testfolderman.cpp @@ -65,14 +65,14 @@ private slots: // those should be allowed - // QString FolderMan::checkPathValidityForNewFolder(const QString& path, const QUrl &serverUrl, bool forNewDirectory) + // QString FolderMan::checkPathValidityForNewFolder(const QString& path, const QUrl &serverUrl, bool forNewDirectory).second QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/sub/free").second, QString()); QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/free2/").second, QString()); // Not an existing directory -> Ok QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/sub/bliblablu").second, QString()); QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/sub/free/bliblablu").second, QString()); - // QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/sub/bliblablu/some/more"), QString()); + // QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/sub/bliblablu/some/more").second, QString()); // A file -> Error QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/sub/file.txt").second.isNull()); @@ -104,33 +104,33 @@ private slots: QVERIFY(QFile::link(dirPath + "/sub/ownCloud1/folder", dirPath + "/link4")); // Ok - QVERIFY(folderman->checkPathValidityForNewFolder(dirPath + "/link1").isNull()); - QVERIFY(folderman->checkPathValidityForNewFolder(dirPath + "/link2/free").isNull()); + QVERIFY(folderman->checkPathValidityForNewFolder(dirPath + "/link1").second.isNull()); + QVERIFY(folderman->checkPathValidityForNewFolder(dirPath + "/link2/free").second.isNull()); // Not Ok - QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/link2").isNull()); + QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/link2").second.isNull()); // link 3 points to an existing sync folder. To make it fail, the account must be the same - QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/link3", url2).isNull()); + QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/link3", url2).second.isNull()); // while with a different account, this is fine - QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/link3", url3), QString()); + QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/link3", url3).second, QString()); - QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/link4").isNull()); - QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/link3/folder").isNull()); + QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/link4").second.isNull()); + QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/link3/folder").second.isNull()); // test some non existing sub path (error) - QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1/some/sub/path").isNull()); - QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/ownCloud2/blublu").isNull()); - QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1/folder/g/h").isNull()); - QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/link3/folder/neu_folder").isNull()); + QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1/some/sub/path").second.isNull()); + QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/ownCloud2/blublu").second.isNull()); + QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1/folder/g/h").second.isNull()); + QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/link3/folder/neu_folder").second.isNull()); // Subfolder of links - QVERIFY(folderman->checkPathValidityForNewFolder(dirPath + "/link1/subfolder").isNull()); - QVERIFY(folderman->checkPathValidityForNewFolder(dirPath + "/link2/free/subfolder").isNull()); + QVERIFY(folderman->checkPathValidityForNewFolder(dirPath + "/link1/subfolder").second.isNull()); + QVERIFY(folderman->checkPathValidityForNewFolder(dirPath + "/link2/free/subfolder").second.isNull()); // Should not have the rights - QVERIFY(!folderman->checkPathValidityForNewFolder("/").isNull()); - QVERIFY(!folderman->checkPathValidityForNewFolder("/usr/bin/somefolder").isNull()); + QVERIFY(!folderman->checkPathValidityForNewFolder("/").second.isNull()); + QVERIFY(!folderman->checkPathValidityForNewFolder("/usr/bin/somefolder").second.isNull()); #endif #ifdef Q_OS_WIN // drive-letter tests