Remove OC10SyncRoot (#12422)

* Remove OC10SyncRoot

Also simplify the path validity checks.

* Fix review comments

- remove static from methods accessing the `FolderMan` instance
- renamed method for clarity
- improve entry point method documentation

Also use `tr()` calls instead of `FolderMan::tr()` for member methods.

* Improve doc for `findExistingFolderAndCheckValidity`

---------

Co-authored-by: modSpike <lisa.reese@kiteworks.com>

Author: erikjv

src/gui/folderman.cpp

@@ -761,7 +761,7 @@ void FolderMan::saveFolder(Folder *folder)
     saveFolder(folder, settings);
 }
 
-Folder *FolderMan::folderForPath(const QString &path, QString *relativePath)
+Folder *FolderMan::folderForPath(const QString &path, QString *relativePath) const
 {
     QString absolutePath = QDir::cleanPath(path) + QLatin1Char('/');
 
@@ -912,29 +912,40 @@ static QString canonicalPath(const QString &path)
 
 static QString checkPathForSyncRootMarkingRecursive(const QString &path, FolderMan::NewFolderType folderType, const QUuid &accountUuid)
 {
+    // First do basic checks if this path is usable
+    const QFileInfo selectedPathInfo(path);
+    if (!selectedPathInfo.isDir()) {
+        return FolderMan::tr("The selected path is not a folder.");
+    }
+
+    if (numberOfSyncJournals(selectedPathInfo.filePath()) != 0) {
+        return FolderMan::tr("The folder %1 is used in a folder sync connection.").arg(QDir::toNativeSeparators(selectedPathInfo.filePath()));
+    }
+
+    // Check for sync root markings in this folder
     std::pair<QString, QUuid> existingTags = Utility::getDirectorySyncRootMarkings(path);
     if (!existingTags.first.isEmpty()) {
         if (existingTags.first != Theme::instance()->orgDomainName()) {
             // another application uses this as spaces root folder
-            return FolderMan::tr("Folder '%1' is already in use by application %2!").arg(path, existingTags.first);
+            return FolderMan::tr("Folder '%1' is already in use by application %2.").arg(path, existingTags.first);
         }
 
         // Looks good, it's our app, let's check the account tag:
         switch (folderType) {
         case FolderMan::NewFolderType::SpacesFolder:
             if (existingTags.second == accountUuid) {
-                // Nice, that's what we like, the sync root for our account in our app. No error.
+                // Nice, that's what we like, the sync root for our account in our app. No error, and we don't need to check the parent folders.
                 return {};
             }
             [[fallthrough]];
-        case FolderMan::NewFolderType::OC10SyncRoot:
-            [[fallthrough]];
         case FolderMan::NewFolderType::SpacesSyncRoot:
             // It's our application but we don't want to create a spaces folder, so it must be another space root
             return FolderMan::tr("Folder '%1' is already in use by another account.").arg(path);
         }
     }
 
+    // This folder is ok, but check the parent folders if they are used
+
     QString parent = QFileInfo(path).path();
     if (parent == path) { // root dir, stop recursing
         return {};
@@ -943,51 +954,7 @@ static QString checkPathForSyncRootMarkingRecursive(const QString &path, FolderM
     return checkPathForSyncRootMarkingRecursive(parent, folderType, accountUuid);
 }
 
-QString FolderMan::checkPathValidityRecursive(const QString &path, FolderMan::NewFolderType folderType, const QUuid &accountUuid)
-{
-    if (path.isEmpty()) {
-        return FolderMan::tr("No valid folder selected!");
-    }
-
-#ifdef Q_OS_WIN
-    Utility::NtfsPermissionLookupRAII ntfs_perm;
-#endif
-
-    auto pathLengthCheck = Folder::checkPathLength(path);
-    if (!pathLengthCheck) {
-        return pathLengthCheck.error();
-    }
-
-    const QFileInfo selectedPathInfo(path);
-    if (!selectedPathInfo.exists()) {
-        const QString parentPath = selectedPathInfo.path();
-        if (parentPath != path) {
-            return checkPathValidityRecursive(parentPath, folderType, accountUuid);
-        }
-        return FolderMan::tr("The selected path does not exist!");
-    }
-
-    if (numberOfSyncJournals(selectedPathInfo.filePath()) != 0) {
-        return FolderMan::tr("The folder %1 is used in a folder sync connection!").arg(QDir::toNativeSeparators(selectedPathInfo.filePath()));
-    }
-
-    // At this point we know there is no syncdb in the parent hierarchy, check for spaces sync root.
-
-    if (!selectedPathInfo.isDir()) {
-        return FolderMan::tr("The selected path is not a folder!");
-    }
-
-    if (!selectedPathInfo.isWritable()) {
-        return FolderMan::tr("You have no permission to write to the selected folder!");
-    }
-
-    return checkPathForSyncRootMarkingRecursive(path, folderType, accountUuid);
-}
-
 /*
- * OC10 folder:
- *  - sync root not in syncdb folder
- *  - sync root not in spaces root
  * with spaces:
  *  - spaces sync root not in syncdb folder
  *  - spaces sync root not in another spaces sync root
@@ -996,41 +963,71 @@ QString FolderMan::checkPathValidityRecursive(const QString &path, FolderMan::Ne
  *  - space *can* be in sync root
  *  - space not in spaces sync root of other account (check with account uuid)
  */
-QString FolderMan::checkPathValidityForNewFolder(const QString &path, NewFolderType folderType, const QUuid &accountUuid) const
+QString FolderMan::checkPathValidity(const QString &path, NewFolderType folderType, const QUuid &accountUuid) const
 {
-    // check if the local directory isn't used yet in another ownCloud sync
+    // Do checks against existing folders. This catches cases where:
+    // - the user deleted a folder from the filesystem, but there is still a folder sync connection in the configuration
+    // - check if this folder contains other folders for spaces
+    // If so, error out.
     const auto cs = Utility::fsCaseSensitivity();
-
     const QString userDir = QDir::cleanPath(canonicalPath(path)) + QLatin1Char('/');
     for (auto f : folders()) {
         const QString folderDir = QDir::cleanPath(canonicalPath(f->path())) + QLatin1Char('/');
-
         if (QString::compare(folderDir, userDir, cs) == 0) {
-            return tr("There is already a sync from the server to this local folder. "
-                      "Please pick another local folder!");
+            return tr("There is already a sync from the server to this local folder.");
         }
         if (FileSystem::isChildPathOf(folderDir, userDir)) {
-            return tr("The local folder %1 already contains a folder used in a folder sync connection. "
-                      "Please pick another local folder!")
+            return tr("The local folder %1 already contains a folder used in a folder sync connection.")
                 .arg(QDir::toNativeSeparators(path));
         }
 
         if (FileSystem::isChildPathOf(userDir, folderDir)) {
-            return tr("The local folder %1 is already contained in a folder used in a folder sync connection. "
-                      "Please pick another local folder!")
+            return tr("The local folder %1 is already contained in a folder used in a folder sync connection.")
                 .arg(QDir::toNativeSeparators(path));
         }
     }
 
-    const auto result = checkPathValidityRecursive(path, folderType, accountUuid);
-    if (!result.isEmpty()) {
-        return tr("%1 Please pick another local folder!").arg(result);
+    // Now run filesystem based checks
+    return findExistingFolderAndCheckValidity(path, folderType, accountUuid);
+}
+
+QString FolderMan::findExistingFolderAndCheckValidity(const QString &path, NewFolderType folderType, const QUuid &accountUuid)
+{
+    if (path.isEmpty()) {
+        return tr("No valid folder selected.");
     }
-    return {};
+
+#ifdef Q_OS_WIN
+    Utility::NtfsPermissionLookupRAII ntfs_perm;
+#endif
+
+    auto pathLengthCheck = Folder::checkPathLength(path);
+    if (!pathLengthCheck) {
+        return pathLengthCheck.error();
+    }
+
+    // If this is a new folder, recurse up to the first parent that exists, to see if we can use that to create a new folder
+    const QFileInfo selectedPathInfo(path);
+    if (!selectedPathInfo.exists()) {
+        const QString parentPath = selectedPathInfo.path();
+        if (parentPath != path) {
+            return findExistingFolderAndCheckValidity(parentPath, folderType, accountUuid);
+        }
+        return tr("The selected path does not exist.");
+    }
+
+    // At this point we have an existing folder, so check if we can create files/folders here.
+    // Note: we don't need to write to any parent, so we don't need to check writablility when traversing the parents.
+    if (!selectedPathInfo.isWritable()) {
+        return tr("You have no permission to write to the selected folder.");
+    }
+
+    // Now check if none of the parents is already used.
+    return checkPathForSyncRootMarkingRecursive(canonicalPath(path), folderType, accountUuid);
 }
 
 QString FolderMan::findGoodPathForNewSyncFolder(
-    const QString &basePath, const QString &newFolder, FolderMan::NewFolderType folderType, const QUuid &accountUuid)
+    const QString &basePath, const QString &newFolder, FolderMan::NewFolderType folderType, const QUuid &accountUuid) const
 {
     OC_ASSERT(!accountUuid.isNull() || folderType == FolderMan::NewFolderType::SpacesSyncRoot);
 
@@ -1041,7 +1038,7 @@ QString FolderMan::findGoodPathForNewSyncFolder(
     // 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.
-    if (FolderMan::instance()->folderForPath(QFileInfo(normalisedPath).canonicalPath())) {
+    if (folderForPath(QFileInfo(normalisedPath).canonicalPath())) {
         // Any path with that parent is going to be unacceptable,
         // so just keep it as-is.
         return canonicalPath(normalisedPath);
@@ -1050,7 +1047,7 @@ QString FolderMan::findGoodPathForNewSyncFolder(
     {
         QString folder = normalisedPath;
         for (int attempt = 2; attempt <= 100; ++attempt) {
-            if (!QFileInfo::exists(folder) && FolderMan::instance()->checkPathValidityForNewFolder(folder, folderType, accountUuid).isEmpty()) {
+            if (!QFileInfo::exists(folder) && checkPathValidity(folder, folderType, accountUuid).isEmpty()) {
                 return canonicalPath(folder);
             }
             folder = normalisedPath + QStringLiteral(" (%1)").arg(attempt);

src/gui/folderman.h

@@ -85,7 +85,6 @@ class OWNCLOUDGUI_EXPORT FolderMan : public QObject
      * Or in case of a space folder, that if the new folder is in a Space sync root, it is the sync root of the same account.
      */
     enum class NewFolderType {
-        OC10SyncRoot, // todo: #43
         SpacesSyncRoot,
         SpacesFolder,
     };
@@ -130,8 +129,22 @@ class OWNCLOUDGUI_EXPORT FolderMan : public QObject
 
     static QString suggestSyncFolder(NewFolderType folderType, const QUuid &accountUuid);
 
-
-    static QString checkPathValidityRecursive(const QString &path, FolderMan::NewFolderType folderType, const QUuid &accountUuid);
+    /**
+     * @brief Check a path for a new spaces sync root or spaces folder for validity.
+     *
+     * @param path The path to check
+     * @param folderType The kind of folder that is to be created (Folder or Spaces sync root)
+     * @param the account UUID for the which the folder is created
+     * @return an error string if there is a problem, or an null string if the path is valid
+     *
+     * Checks:
+     *  - spaces sync root not in a syncdb folder (a space folder)
+     *  - spaces sync root not in another spaces sync root
+     *  - space folder not in a syncdb folder (another space folder)
+     *  - space folder *can* be in sync root, if:
+     *  - space folder not in a spaces sync root of other account (check with account uuid) or (possibly branded) client
+     */
+    QString checkPathValidity(const QString &path, NewFolderType folderType, const QUuid &accountUuid) const;
 
     static std::unique_ptr<FolderMan> createInstance();
 
@@ -203,7 +216,7 @@ class OWNCLOUDGUI_EXPORT FolderMan : public QObject
      * Optionally, the path relative to the found folder is returned in
      * relativePath.
      */
-    Folder *folderForPath(const QString &path, QString *relativePath = nullptr);
+    Folder *folderForPath(const QString &path, QString *relativePath = nullptr) const;
 
     /**
      * Ensures that a given directory does not contain a sync journal file.
@@ -215,14 +228,6 @@ class OWNCLOUDGUI_EXPORT FolderMan : public QObject
 
     SocketApi *socketApi();
 
-    /**
-     * 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.
-     *
-     * @returns an empty string if it is allowed, or an error if it is not allowed
-     */
-    QString checkPathValidityForNewFolder(const QString &path, NewFolderType folderType, const QUuid &accountUuid) const;
-
     /**
      * Attempts to find a non-existing, acceptable path for creating a new sync folder.
      *
@@ -232,7 +237,7 @@ class OWNCLOUDGUI_EXPORT FolderMan : public QObject
      * subfolder of ~ would be a good candidate. When that happens \a basePath
      * is returned.
      */
-    static QString findGoodPathForNewSyncFolder(const QString &basePath, const QString &newFolder, NewFolderType folderType, const QUuid &accountUuid);
+    QString findGoodPathForNewSyncFolder(const QString &basePath, const QString &newFolder, NewFolderType folderType, const QUuid &accountUuid) const;
 
     bool ignoreHiddenFiles() const;
     void setIgnoreHiddenFiles(bool ignore);
@@ -457,6 +462,16 @@ private Q_SLOTS:
     // pair this with _socketApi->slotUnregisterPath(folder);
     void registerFolderWithSocketApi(Folder *folder);
 
+    // Helper for `checkPathValidity`. It first checks if the folder `path` exists, and if not recusively checks its parent. When a folder
+    // is found, it checks for sync root markers. See the documentation of `checkPathValidity` for when a path is valid. If the path
+    // is valid, a null-string is returned. When a path is invalid, an error string is returned.
+    //
+    // For example: start with /x/y/z
+    // First check is if z exists. If not, retry with /x/y.
+    // Now if /x/y exists, check for markers wether this is used as a spaces folder, or a sync root for another account. If it is, return an error message. If
+    // not, check /x. When a path is valid, the null string is returned.
+    static QString findExistingFolderAndCheckValidity(const QString &path, NewFolderType folderType, const QUuid &accountUuid);
+
     QString _folderConfigPath;
     bool _ignoreHiddenFiles = true;
 

src/gui/folderwizard/folderwizard.cpp

@@ -59,13 +59,12 @@ QString FolderWizardPrivate::formatWarnings(const QStringList &warnings, bool is
     return ret;
 }
 
-// todo: #43
 QString FolderWizardPrivate::defaultSyncRoot() const
 {
     // this should never happen when we have set up the account using spaces - there is ALWAYS a default root when spaces are in play
     // and they are always in play so this check is bogus. todo: #43
     if (!_account->hasDefaultSyncRoot()) {
-        const auto folderType = FolderMan::NewFolderType::SpacesSyncRoot; // todo: #43 : FolderMan::NewFolderType::OC10SyncRoot;
+        const auto folderType = FolderMan::NewFolderType::SpacesSyncRoot;
         return FolderMan::suggestSyncFolder(folderType, _account->uuid());
     } else {
         return _account->defaultSyncRoot();
@@ -102,7 +101,7 @@ FolderWizardPrivate::FolderWizardPrivate(FolderWizard *q, Account *account)
 
 QString FolderWizardPrivate::initialLocalPath() const
 {
-    return FolderMan::findGoodPathForNewSyncFolder(
+    return FolderMan::instance()->findGoodPathForNewSyncFolder(
         defaultSyncRoot(), _spacesPage->currentSpace()->displayName(), FolderMan::NewFolderType::SpacesSyncRoot, _account->uuid());
 }
 

src/gui/folderwizard/folderwizardlocalpath.cpp

@@ -65,7 +65,7 @@ bool FolderWizardLocalPath::isComplete() const
 {
     auto folderType = FolderMan::NewFolderType::SpacesFolder;
     auto accountUuid = folderWizardPrivate()->uuid();
-    QString errorStr = FolderMan::instance()->checkPathValidityForNewFolder(localPath(), folderType, accountUuid);
+    QString errorStr = FolderMan::instance()->checkPathValidity(localPath(), folderType, accountUuid);
 
     bool isOk = errorStr.isEmpty();
     QStringList warnStrings;

src/gui/newaccountwizard/advancedsettingspagecontroller.cpp

@@ -226,7 +226,7 @@ bool AdvancedSettingsPageController::validateSyncRoot(const QString &rootPath)
         return false;
     }
 
-    QString invalidPathErrorMessage = FolderMan::checkPathValidityRecursive(rootPath, FolderMan::NewFolderType::SpacesSyncRoot, {});
+    QString invalidPathErrorMessage = FolderMan::instance()->checkPathValidity(rootPath, FolderMan::NewFolderType::SpacesSyncRoot, {});
     if (!invalidPathErrorMessage.isEmpty()) {
         _errorField->setText(errorMessageTemplate.arg(rootPath, invalidPathErrorMessage));
         return false;