Wizards: Never propose an existing folder for syncing #5597

diff --git a/src/gui/folder.h b/src/gui/folder.h
index c6739cfdaf6d021a3b417d13955267e1dc820f7d..67026bd52ed30c0ec92f12d9acc2513afa97b054 100644 (file)
--- 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 1a230a28ca6a417da319a00a09d2028e1debf619..6e4122c4e8e4bda3442d848f5b090eaadf50ad65 100644 (file)
--- 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 7386335b480d633acf552bf3bd5ecf795e44d82f..2545c88387ae1e857814265f97979d4eb1879604 100644 (file)
--- 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 8a15c873947ab1257548641a6ea20ad9452e794f..2db147556a87520e50b8eabc1a9cacf93faa859c 100644 (file)
--- 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 99092e09e19e50b438f4b04e4a5b79c5a5a78c65..3c9dd69793ac111f68c70130c722cd5a9f5f8c87 100644 (file)
--- 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 45aadd2ab9f3ac2bb24feb483dd9f3365cf77acb..5d1c82ed219e01cdebacabefadb1823e49694f8a 100644 (file)
--- 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 e62437bdded48e31107d97d512c835637d194b10..b6e6478f5ae8bb92aae90171984e7a516a820e14 100644 (file)
--- a/test/testfolderman.cpp
+++ b/test/testfolderman.cpp
@@ -151,6 +151,54 @@ private slots:
         QVERIFY(!folderman->checkPathValidityForNewFolder("/usr/bin/somefolder").isNull());
 #else
         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
     }
 };