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 beac2f093..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) @@ -1659,12 +1663,16 @@ static QString canonicalPath(const QString &path) return selFile.canonicalFilePath(); } -QString FolderMan::checkPathValidityForNewFolder(const QString &path, const QUrl &serverUrl) const +QPair FolderMan::checkPathValidityForNewFolder(const QString &path, const QUrl &serverUrl) const { - QString recursiveValidity = checkPathValidityRecursive(path); + QPair result; + + const auto recursiveValidity = checkPathValidityRecursive(path); if (!recursiveValidity.isEmpty()) { qCDebug(lcFolderMan) << path << recursiveValidity; - return recursiveValidity; + result.first = FolderMan::PathValidityResult::ErrorRecursiveValidity; + result.second = recursiveValidity; + return result; } // check if the local directory isn't used yet in another ownCloud sync @@ -1680,15 +1688,19 @@ QString FolderMan::checkPathValidityForNewFolder(const QString &path, const QUrl bool differentPaths = QString::compare(folderDir, userDir, cs) != 0; if (differentPaths && folderDir.startsWith(userDir, cs)) { - return tr("The local folder %1 already contains a folder used in a folder sync connection. " - "Please pick another one!") - .arg(QDir::toNativeSeparators(path)); + 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)); + return result; } if (differentPaths && userDir.startsWith(folderDir, cs)) { - return tr("The local folder %1 is already contained in a folder used in a folder sync connection. " + 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)); + return result; } // if both pathes are equal, the server url needs to be different @@ -1700,13 +1712,15 @@ QString FolderMan::checkPathValidityForNewFolder(const QString &path, const QUrl folderUrl.setUserName(user); if (serverUrl == folderUrl) { - return tr("There is already a sync from the server to this local folder. " + 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; } } } - return QString(); + return result; } QString FolderMan::findGoodPathForNewSyncFolder(const QString &basePath, const QUrl &serverUrl) const @@ -1728,7 +1742,7 @@ QString FolderMan::findGoodPathForNewSyncFolder(const QString &basePath, const Q forever { const bool isGood = !QFileInfo(folder).exists() - && FolderMan::instance()->checkPathValidityForNewFolder(folder, serverUrl).isEmpty(); + && FolderMan::instance()->checkPathValidityForNewFolder(folder, serverUrl).second.isEmpty(); if (isGood) { break; } diff --git a/src/gui/folderman.h b/src/gui/folderman.h index 689d102d1..985fcb50b 100644 --- a/src/gui/folderman.h +++ b/src/gui/folderman.h @@ -63,6 +63,14 @@ class FolderMan : public QObject { Q_OBJECT public: + enum class PathValidityResult { + Valid, + ErrorRecursiveValidity, + ErrorContainsFolder, + ErrorContainedInFolder, + ErrorNonEmptyFolder + }; + ~FolderMan() override; static FolderMan *instance(); @@ -134,9 +142,9 @@ public: * * Note that different accounts are allowed to sync to the same folder. * - * @returns an empty string if it is allowed, or an error if it is not allowed + * @returns an empty string and PathValidityResult::Valid if it is allowed, or an error if it is not allowed */ - QString checkPathValidityForNewFolder(const QString &path, const QUrl &serverUrl = QUrl()) const; + QPair checkPathValidityForNewFolder(const QString &path, const QUrl &serverUrl = QUrl()) const; /** * Attempts to find a non-existing, acceptable path for creating a new sync folder. diff --git a/src/gui/folderwizard.cpp b/src/gui/folderwizard.cpp index b10752621..cb4fa2a13 100644 --- a/src/gui/folderwizard.cpp +++ b/src/gui/folderwizard.cpp @@ -98,8 +98,8 @@ bool FolderWizardLocalPath::isComplete() const QUrl serverUrl = _account->url(); serverUrl.setUserName(_account->credentials()->user()); - QString errorStr = FolderMan::instance()->checkPathValidityForNewFolder( - QDir::fromNativeSeparators(_ui.localFolderLineEdit->text()), serverUrl); + const auto errorStr = FolderMan::instance()->checkPathValidityForNewFolder( + QDir::fromNativeSeparators(_ui.localFolderLineEdit->text()), serverUrl).second; bool isOk = errorStr.isEmpty(); diff --git a/src/gui/wizard/owncloudadvancedsetuppage.cpp b/src/gui/wizard/owncloudadvancedsetuppage.cpp index e0818dfc9..ccd54edc4 100644 --- a/src/gui/wizard/owncloudadvancedsetuppage.cpp +++ b/src/gui/wizard/owncloudadvancedsetuppage.cpp @@ -258,8 +258,9 @@ void OwncloudAdvancedSetupPage::updateStatus() const QString locFolder = localFolder(); // check if the local folder exists. If so, and if its not empty, show a warning. - QString errorStr = FolderMan::instance()->checkPathValidityForNewFolder(locFolder, serverUrl()); - _localFolderValid = errorStr.isEmpty(); + const auto pathValidityCheckResult = FolderMan::instance()->checkPathValidityForNewFolder(locFolder, serverUrl()); + auto errorStr = pathValidityCheckResult.second; + _localFolderValid = errorStr.isEmpty() || pathValidityCheckResult.first == FolderMan::PathValidityResult::ErrorNonEmptyFolder; QString t; diff --git a/test/testfolderman.cpp b/test/testfolderman.cpp index 7a2b3135e..e66049f57 100644 --- a/test/testfolderman.cpp +++ b/test/testfolderman.cpp @@ -65,17 +65,17 @@ 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"), QString()); - QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/free2/"), QString()); + 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"), QString()); - QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/sub/free/bliblablu"), QString()); - // QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/sub/bliblablu/some/more"), QString()); + 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").second, QString()); // A file -> Error - QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/sub/file.txt").isNull()); + QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/sub/file.txt").second.isNull()); // There are folders configured in those folders, url needs to be taken into account: -> ERROR QUrl url2(url); @@ -83,18 +83,18 @@ private slots: url2.setUserName(user); // The following both fail because they refer to the same account (user and url) - QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1", url2).isNull()); - QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/ownCloud2/", url2).isNull()); + QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1", url2).second.isNull()); + QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/ownCloud2/", url2).second.isNull()); // Now it will work because the account is different QUrl url3("http://anotherexample.org"); url3.setUserName("dummy"); - QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1", url3), QString()); - QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/ownCloud2/", url3), QString()); + QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1", url3).second, QString()); + QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/ownCloud2/", url3).second, QString()); - QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath).isNull()); - QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1/folder").isNull()); - QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1/folder/f").isNull()); + QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath).second.isNull()); + QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1/folder").second.isNull()); + QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1/folder/f").second.isNull()); #ifndef Q_OS_WIN // no links on windows, no permissions // make a bunch of links @@ -104,56 +104,56 @@ 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 if (!QFileInfo("v:/").exists()) { - QVERIFY(!folderman->checkPathValidityForNewFolder("v:").isNull()); - QVERIFY(!folderman->checkPathValidityForNewFolder("v:/").isNull()); - QVERIFY(!folderman->checkPathValidityForNewFolder("v:/foo").isNull()); + QVERIFY(!folderman->checkPathValidityForNewFolder("v:").second.isNull()); + QVERIFY(!folderman->checkPathValidityForNewFolder("v:/").second.isNull()); + QVERIFY(!folderman->checkPathValidityForNewFolder("v:/foo").second.isNull()); } if (QFileInfo("c:/").isWritable()) { - QVERIFY(folderman->checkPathValidityForNewFolder("c:").isNull()); - QVERIFY(folderman->checkPathValidityForNewFolder("c:/").isNull()); - QVERIFY(folderman->checkPathValidityForNewFolder("c:/foo").isNull()); + QVERIFY(folderman->checkPathValidityForNewFolder("c:").second.isNull()); + QVERIFY(folderman->checkPathValidityForNewFolder("c:/").second.isNull()); + QVERIFY(folderman->checkPathValidityForNewFolder("c:/foo").second.isNull()); } #endif // Invalid paths - QVERIFY(!folderman->checkPathValidityForNewFolder("").isNull()); + QVERIFY(!folderman->checkPathValidityForNewFolder("").second.isNull()); // REMOVE ownCloud2 from the filesystem, but keep a folder sync'ed to it. QDir(dirPath + "/ownCloud2/").removeRecursively(); - QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/ownCloud2/blublu").isNull()); - QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/ownCloud2/sub/subsub/sub").isNull()); + QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/ownCloud2/blublu").second.isNull()); + QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/ownCloud2/sub/subsub/sub").second.isNull()); } void testFindGoodPathForNewSyncFolder()