diff options
author | Christian Kamm <mail@ckamm.de> | 2017-03-20 16:28:32 +0300 |
---|---|---|
committer | ckamm <mail@ckamm.de> | 2017-03-21 19:07:35 +0300 |
commit | f67989afea6c2c98b6a3d2f4ee12ca0b01df97c8 (patch) | |
tree | ab656e3473fce7bd8575365c06f366f63c3c7d08 | |
parent | 14fef4a0d3d259b538f59c8831cf65719c642f3c (diff) |
Wizards: Never propose an existing folder for syncing #5597
-rw-r--r-- | src/gui/folder.h | 7 | ||||
-rw-r--r-- | src/gui/folderman.cpp | 38 | ||||
-rw-r--r-- | src/gui/folderman.h | 15 | ||||
-rw-r--r-- | src/gui/folderwizard.cpp | 1 | ||||
-rw-r--r-- | src/gui/wizard/owncloudadvancedsetuppage.cpp | 20 | ||||
-rw-r--r-- | src/gui/wizard/owncloudadvancedsetuppage.h | 1 | ||||
-rw-r--r-- | test/testfolderman.cpp | 48 |
7 files changed, 121 insertions, 9 deletions
diff --git a/src/gui/folder.h b/src/gui/folder.h index c6739cfda..67026bd52 100644 --- a/src/gui/folder.h +++ b/src/gui/folder.h @@ -115,12 +115,15 @@ public: QString shortGuiLocalPath() const; /** - * local folder path + * canonical local folder path, always ends with / */ QString path() const; /** - * wrapper for QDir::cleanPath("Z:\\"), which returns "Z:\\", but we need "Z:" instead + * cleaned canonical folder path, like path() but never ends with a / + * + * Wrapper for QDir::cleanPath(path()) except for "Z:/", + * where it returns "Z:" instead of "Z:/". */ QString cleanPath() const; diff --git a/src/gui/folderman.cpp b/src/gui/folderman.cpp index 1a230a28c..6e4122c4e 100644 --- a/src/gui/folderman.cpp +++ b/src/gui/folderman.cpp @@ -1278,7 +1278,7 @@ QString FolderMan::statusToString( SyncResult::Status syncStatus, bool paused ) return folderMessage; } -QString FolderMan::checkPathValidityForNewFolder(const QString& path, const QUrl &serverUrl, bool forNewDirectory) +QString FolderMan::checkPathValidityForNewFolder(const QString& path, const QUrl &serverUrl, bool forNewDirectory) const { if (path.isEmpty()) { return tr("No valid folder selected!"); @@ -1371,6 +1371,42 @@ QString FolderMan::checkPathValidityForNewFolder(const QString& path, const QUrl } +QString FolderMan::findGoodPathForNewSyncFolder(const QString &basePath, const QUrl &serverUrl) const +{ + QString folder = basePath; + + // If the parent folder is a sync folder or contained in one, we can't + // possibly find a valid sync folder inside it. + // Example: Someone syncs their home directory. Then ~/foobar is not + // going to be an acceptable sync folder path for any value of foobar. + QString parentFolder = QFileInfo(folder).dir().canonicalPath(); + if (FolderMan::instance()->folderForPath(parentFolder)) { + // Any path with that parent is going to be unacceptable, + // so just keep it as-is. + return basePath; + } + + int attempt = 1; + forever { + const bool isGood = + !QFileInfo(folder).exists() + && FolderMan::instance()->checkPathValidityForNewFolder(folder, serverUrl).isEmpty(); + if (isGood) { + break; + } + + // Count attempts and give up eventually + attempt++; + if (attempt > 100) { + return basePath; + } + + folder = basePath + QString::number(attempt); + } + + return folder; +} + bool FolderMan::ignoreHiddenFiles() const { if (_folderMap.empty()) { diff --git a/src/gui/folderman.h b/src/gui/folderman.h index 7386335b4..2545c8838 100644 --- a/src/gui/folderman.h +++ b/src/gui/folderman.h @@ -124,11 +124,24 @@ public: * Check if @a path is a valid path for a new folder considering the already sync'ed items. * Make sure that this folder, or any subfolder is not sync'ed already. * + * Note that different accounts are allowed to sync to the same folder. + * * \a forNewDirectory is internal and is used for recursion. * * @returns an empty string if it is allowed, or an error if it is not allowed */ - QString checkPathValidityForNewFolder(const QString &path, const QUrl& serverUrl = QUrl(), bool forNewDirectory = false); + QString checkPathValidityForNewFolder(const QString &path, const QUrl& serverUrl = QUrl(), bool forNewDirectory = false) const; + + /** + * Attempts to find a non-existing, acceptable path for creating a new sync folder. + * + * Uses \a basePath as the baseline. It'll return this path if it's acceptable. + * + * Note that this can fail. If someone syncs ~ and \a basePath is ~/ownCloud, no + * subfolder of ~ would be a good candidate. When that happens \a basePath + * is returned. + */ + QString findGoodPathForNewSyncFolder(const QString &basePath, const QUrl &serverUrl) const; /** * While ignoring hidden files can theoretically be switched per folder, diff --git a/src/gui/folderwizard.cpp b/src/gui/folderwizard.cpp index 8a15c8739..2db147556 100644 --- a/src/gui/folderwizard.cpp +++ b/src/gui/folderwizard.cpp @@ -66,6 +66,7 @@ FolderWizardLocalPath::FolderWizardLocalPath(const AccountPtr& account) _ui.localFolderChooseBtn->setToolTip(tr("Click to select a local folder to sync.")); QString defaultPath = QDir::homePath() + QLatin1Char('/') + Theme::instance()->appName(); + defaultPath = FolderMan::instance()->findGoodPathForNewSyncFolder(defaultPath, account->url()); _ui.localFolderLineEdit->setText( QDir::toNativeSeparators( defaultPath ) ); _ui.localFolderLineEdit->setToolTip(tr("Enter the path to the local folder.")); diff --git a/src/gui/wizard/owncloudadvancedsetuppage.cpp b/src/gui/wizard/owncloudadvancedsetuppage.cpp index 99092e09e..3c9dd6979 100644 --- a/src/gui/wizard/owncloudadvancedsetuppage.cpp +++ b/src/gui/wizard/owncloudadvancedsetuppage.cpp @@ -107,6 +107,10 @@ void OwncloudAdvancedSetupPage::initializePage() _ui.lSelectiveSyncSizeLabel->setText(QString()); _ui.lSyncEverythingSizeLabel->setText(QString()); + // Update the local folder - this is not guaranteed to find a good one + QString goodLocalFolder = FolderMan::instance()->findGoodPathForNewSyncFolder(localFolder(), serverUrl()); + wizard()->setProperty("localFolder", goodLocalFolder); + // call to init label updateStatus(); @@ -138,13 +142,9 @@ void OwncloudAdvancedSetupPage::initializePage() void OwncloudAdvancedSetupPage::updateStatus() { const QString locFolder = localFolder(); - const QString url = static_cast<OwncloudWizard *>(wizard())->ocUrl(); - const QString user = static_cast<OwncloudWizard *>(wizard())->getCredentials()->user(); - QUrl serverUrl(url); - serverUrl.setUserName(user); // check if the local folder exists. If so, and if its not empty, show a warning. - QString errorStr = FolderMan::instance()->checkPathValidityForNewFolder(locFolder, serverUrl); + QString errorStr = FolderMan::instance()->checkPathValidityForNewFolder(locFolder, serverUrl()); _localFolderValid = errorStr.isEmpty(); QString t; @@ -197,6 +197,16 @@ void OwncloudAdvancedSetupPage::stopSpinner() _progressIndi->stopAnimation(); } +QUrl OwncloudAdvancedSetupPage::serverUrl() const +{ + const QString urlString = static_cast<OwncloudWizard *>(wizard())->ocUrl(); + const QString user = static_cast<OwncloudWizard *>(wizard())->getCredentials()->user(); + + QUrl url(urlString); + url.setUserName(user); + return url; +} + int OwncloudAdvancedSetupPage::nextId() const { return WizardCommon::Page_Result; diff --git a/src/gui/wizard/owncloudadvancedsetuppage.h b/src/gui/wizard/owncloudadvancedsetuppage.h index 45aadd2ab..5d1c82ed2 100644 --- a/src/gui/wizard/owncloudadvancedsetuppage.h +++ b/src/gui/wizard/owncloudadvancedsetuppage.h @@ -64,6 +64,7 @@ private: bool dataChanged(); void startSpinner(); void stopSpinner(); + QUrl serverUrl() const; Ui_OwncloudAdvancedSetupPage _ui; bool _checking; diff --git a/test/testfolderman.cpp b/test/testfolderman.cpp index e62437bdd..b6e6478f5 100644 --- a/test/testfolderman.cpp +++ b/test/testfolderman.cpp @@ -153,6 +153,54 @@ private slots: QSKIP("Test not supported with Qt4", SkipSingle); #endif } + + void testFindGoodPathForNewSyncFolder() + { +#if QT_VERSION >= QT_VERSION_CHECK(5, 1, 0) + // SETUP + + QTemporaryDir dir; + ConfigFile::setConfDir(dir.path()); // we don't want to pollute the user's config file + QVERIFY(dir.isValid()); + QDir dir2(dir.path()); + QVERIFY(dir2.mkpath("sub/ownCloud1/folder/f")); + QVERIFY(dir2.mkpath("ownCloud")); + QVERIFY(dir2.mkpath("ownCloud2")); + QVERIFY(dir2.mkpath("ownCloud2/foo")); + QVERIFY(dir2.mkpath("sub/free")); + QVERIFY(dir2.mkpath("free2/sub")); + QString dirPath = dir2.canonicalPath(); + + AccountPtr account = Account::create(); + QUrl url("http://example.de"); + HttpCredentialsTest *cred = new HttpCredentialsTest("testuser", "secret"); + account->setCredentials(cred); + account->setUrl( url ); + + AccountStatePtr newAccountState(new AccountState(account)); + FolderMan *folderman = FolderMan::instance(); + QCOMPARE(folderman, &_fm); + QVERIFY(folderman->addFolder(newAccountState.data(), folderDefinition(dirPath + "/sub/ownCloud/"))); + QVERIFY(folderman->addFolder(newAccountState.data(), folderDefinition(dirPath + "/ownCloud2/"))); + + // TEST + + QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath + "/oc", url), + QString(dirPath + "/oc")); + QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath + "/ownCloud", url), + QString(dirPath + "/ownCloud3")); + QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath + "/ownCloud2", url), + QString(dirPath + "/ownCloud22")); + QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath + "/ownCloud2/foo", url), + QString(dirPath + "/ownCloud2/foo")); + QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath + "/ownCloud2/bar", url), + QString(dirPath + "/ownCloud2/bar")); + QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath + "/sub", url), + QString(dirPath + "/sub2")); +#else + QSKIP("Test not supported with Qt4", SkipSingle); +#endif + } }; QTEST_APPLESS_MAIN(TestFolderMan) |