Fix check for creation of a spaces folder in a spaces root

Fixes: #11516

src/gui/folderman.cpp

@@ -720,22 +720,42 @@ static QString canonicalPath(const QString &path)
     return selFile.canonicalFilePath();
 }
 
-static QString checkPathForSyncRootMarkingRecursive(const QString &path)
+static QString checkPathForSyncRootMarkingRecursive(const QString &path, FolderMan::NewFolderType folderType, const QUuid &accountUuid)
 {
-    auto existingTag = Utility::getDirectorySyncRootMarking(path);
-    if (!existingTag.isEmpty()) {
-        return FolderMan::tr("Folder '%1' is already in use by application %2!").arg(path, existingTag);
+    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);
+        }
+
+        // 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.
+                return {};
+            } else {
+                return FolderMan::tr("Folder '%1' is already in use by another account.").arg(path);
+            }
+
+        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);
+        }
     }
 
     QString parent = QFileInfo(path).path();
     if (parent == path) { // root dir, stop recursing
         return {};
     }
 
-    return checkPathForSyncRootMarkingRecursive(parent);
+    return checkPathForSyncRootMarkingRecursive(parent, folderType, accountUuid);
 }
 
-QString FolderMan::checkPathValidityRecursive(const QString &path)
+QString FolderMan::checkPathValidityRecursive(const QString &path, FolderMan::NewFolderType folderType, const QUuid &accountUuid)
 {
     if (path.isEmpty()) {
         return FolderMan::tr("No valid folder selected!");
@@ -750,31 +770,45 @@ QString FolderMan::checkPathValidityRecursive(const QString &path)
         return pathLenghtCheck.error();
     }
 
-    const QFileInfo selFile(path);
-    if (numberOfSyncJournals(selFile.filePath()) != 0) {
-        return FolderMan::tr("The folder %1 is used in a folder sync connection!").arg(QDir::toNativeSeparators(selFile.filePath()));
-    }
-
-    if (!selFile.exists()) {
-        const QString parentPath = selFile.path();
+    const QFileInfo selectedPathInfo(path);
+    if (!selectedPathInfo.exists()) {
+        const QString parentPath = selectedPathInfo.path();
         if (parentPath != path) {
-            return checkPathValidityRecursive(parentPath);
+            return checkPathValidityRecursive(parentPath, folderType, accountUuid);
         }
         return FolderMan::tr("The selected path does not exist!");
     }
 
-    if (!selFile.isDir()) {
+    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 hyrarchy, check for spaces sync root.
+
+    if (!selectedPathInfo.isDir()) {
         return FolderMan::tr("The selected path is not a folder!");
     }
 
-    if (!selFile.isWritable()) {
+    if (!selectedPathInfo.isWritable()) {
         return FolderMan::tr("You have no permission to write to the selected folder!");
     }
 
-    return checkPathForSyncRootMarkingRecursive(path);
+    return checkPathForSyncRootMarkingRecursive(path, folderType, accountUuid);
 }
 
-QString FolderMan::checkPathValidityForNewFolder(const QString &path) const
+/*
+ * 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
+ *
+ *  - space not in syncdb folder
+ *  - 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
 {
     // check if the local directory isn't used yet in another ownCloud sync
     const auto cs = Utility::fsCaseSensitivity();
@@ -789,25 +823,25 @@ QString FolderMan::checkPathValidityForNewFolder(const QString &path) const
         }
         if (FileSystem::isChildPathOf(folderDir, userDir)) {
             return tr("The local folder %1 already contains a folder used in a folder sync connection. "
-                      "Please pick another one!")
+                      "Please pick another local folder!")
                 .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 one!")
+                      "Please pick another local folder!")
                 .arg(QDir::toNativeSeparators(path));
         }
     }
 
-    const auto result = checkPathValidityRecursive(path);
+    const auto result = checkPathValidityRecursive(path, folderType, accountUuid);
     if (!result.isEmpty()) {
-        return tr("%1 Please pick another one!").arg(result);
+        return tr("%1 Please pick another local folder!").arg(result);
     }
     return {};
 }
 
-QString FolderMan::findGoodPathForNewSyncFolder(const QString &basePath, const QString &newFolder)
+QString FolderMan::findGoodPathForNewSyncFolder(const QString &basePath, const QString &newFolder, FolderMan::NewFolderType folderType)
 {
     // reserve extra characters to allow appending of a number
     const QString normalisedPath = FileSystem::createPortableFileName(basePath, FileSystem::pathEscape(newFolder), std::string_view(" (100)").size());
@@ -825,8 +859,8 @@ QString FolderMan::findGoodPathForNewSyncFolder(const QString &basePath, const Q
     {
         QString folder = normalisedPath;
         for (int attempt = 2; attempt <= 100; ++attempt) {
-            if (!QFileInfo::exists(folder)
-                && FolderMan::instance()->checkPathValidityForNewFolder(folder).isEmpty()) {
+            // HvR: is this correct?
+            if (!QFileInfo::exists(folder) && FolderMan::instance()->checkPathValidityForNewFolder(folder, folderType, {}).isEmpty()) {
                 return canonicalPath(folder);
             }
             folder = normalisedPath + QStringLiteral(" (%1)").arg(attempt);
@@ -933,13 +967,13 @@ Folder *FolderMan::addFolderFromFolderWizardResult(const AccountStatePtr &accoun
     return f;
 }
 
-QString FolderMan::suggestSyncFolder(const QUrl &server, const QString &displayName)
+QString FolderMan::suggestSyncFolder(const QUrl &server, const QString &displayName, NewFolderType folderType)
 {
     QString folderName = tr("%1 - %2@%3").arg(Theme::instance()->appName(), displayName, server.host());
     if (!Theme::instance()->multiAccount()) {
         folderName = Theme::instance()->appName();
     }
-    return FolderMan::instance()->findGoodPathForNewSyncFolder(QDir::homePath(), folderName);
+    return FolderMan::instance()->findGoodPathForNewSyncFolder(QDir::homePath(), folderName, folderType);
 }
 
 bool FolderMan::prepareFolder(const QString &folder)

src/gui/folderman.h

@@ -83,10 +83,20 @@ class FolderMan : public QObject
 {
     Q_OBJECT
 public:
-    static QString suggestSyncFolder(const QUrl &server, const QString &displayName);
+    /**
+     * For a new folder, the type guides what kind of checks are done to ensure the new folder is not embedded in an existing one.
+     * 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,
+        SpacesSyncRoot,
+        SpacesFolder,
+    };
+
+    static QString suggestSyncFolder(const QUrl &server, const QString &displayName, NewFolderType folderType);
     [[nodiscard]] static bool prepareFolder(const QString &folder);
 
-    static QString checkPathValidityRecursive(const QString &path);
+    static QString checkPathValidityRecursive(const QString &path, FolderMan::NewFolderType folderType, const QUuid &accountUuid);
 
     static std::unique_ptr<FolderMan> createInstance();
     ~FolderMan() override;
@@ -177,7 +187,7 @@ class FolderMan : public QObject
      *
      * @returns an empty string if it is allowed, or an error if it is not allowed
      */
-    QString checkPathValidityForNewFolder(const QString &path) const;
+    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.
@@ -188,7 +198,7 @@ class 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);
+    static QString findGoodPathForNewSyncFolder(const QString &basePath, const QString &newFolder, NewFolderType folderType);
 
     /**
      * While ignoring hidden files can theoretically be switched per folder,

src/gui/folderwizard/folderwizard.cpp

@@ -63,7 +63,8 @@ QString FolderWizardPrivate::formatWarnings(const QStringList &warnings, bool is
 QString FolderWizardPrivate::defaultSyncRoot() const
 {
     if (!_account->account()->hasDefaultSyncRoot()) {
-        return FolderMan::suggestSyncFolder(_account->account()->url(), _account->account()->davDisplayName());
+        auto folderType = _account->supportsSpaces() ? FolderMan::NewFolderType::SpacesSyncRoot : FolderMan::NewFolderType::OC10SyncRoot;
+        return FolderMan::suggestSyncFolder(_account->account()->url(), _account->account()->davDisplayName(), folderType);
     } else {
         return _account->account()->defaultSyncRoot();
     }
@@ -96,12 +97,13 @@ FolderWizardPrivate::FolderWizardPrivate(FolderWizard *q, const AccountStatePtr
 QString FolderWizardPrivate::initialLocalPath() const
 {
     if (_account->supportsSpaces()) {
-        return FolderMan::findGoodPathForNewSyncFolder(defaultSyncRoot(), _spacesPage->selectedSpaceData(Spaces::SpacesModel::Columns::Name).toString());
+        return FolderMan::findGoodPathForNewSyncFolder(
+            defaultSyncRoot(), _spacesPage->selectedSpaceData(Spaces::SpacesModel::Columns::Name).toString(), FolderMan::NewFolderType::SpacesSyncRoot);
     }
 
     // Split default sync root:
     const QFileInfo path(defaultSyncRoot());
-    return FolderMan::findGoodPathForNewSyncFolder(path.path(), path.fileName());
+    return FolderMan::findGoodPathForNewSyncFolder(path.path(), path.fileName(), FolderMan::NewFolderType::OC10SyncRoot);
 }
 
 QString FolderWizardPrivate::remotePath() const

src/gui/folderwizard/folderwizardlocalpath.cpp

@@ -63,7 +63,10 @@ QString FolderWizardLocalPath::localPath() const
 
 bool FolderWizardLocalPath::isComplete() const
 {
-    QString errorStr = FolderMan::instance()->checkPathValidityForNewFolder(localPath());
+    // HvR: is this correct?
+    auto folderType = folderWizardPrivate()->accountState()->supportsSpaces() ? FolderMan::NewFolderType::SpacesFolder : FolderMan::NewFolderType::OC10SyncRoot;
+    auto accountUuid = folderWizardPrivate()->accountState()->account()->uuid();
+    QString errorStr = FolderMan::instance()->checkPathValidityForNewFolder(localPath(), folderType, accountUuid);
 
     bool isOk = errorStr.isEmpty();
     QStringList warnStrings;

src/gui/guiutility.cpp

@@ -39,6 +39,11 @@ const QString dirTag()
 {
     return QStringLiteral("com.owncloud.spaces.app");
 }
+
+const QString uuidTag()
+{
+    return QStringLiteral("com.owncloud.spaces.account-uuid");
+}
 } // anonymous namespace
 
 using namespace OCC;
@@ -104,25 +109,39 @@ bool Utility::internetConnectionIsMetered()
     return false;
 }
 
-void Utility::markDirectoryAsSyncRoot(const QString &path)
+void Utility::markDirectoryAsSyncRoot(const QString &path, const QUuid &accountUuid)
 {
-    Q_ASSERT(getDirectorySyncRootMarking(path).isEmpty());
+    Q_ASSERT(getDirectorySyncRootMarkings(path).first.isEmpty());
+    Q_ASSERT(getDirectorySyncRootMarkings(path).second.isNull());
 
-    auto result = FileSystem::Tags::set(path, dirTag(), Theme::instance()->orgDomainName().toUtf8());
-    if (!result) {
-        qCWarning(lcGuiUtility) << QStringLiteral("Failed to set tag on '%1': %2").arg(path, result.error())
+    auto result1 = FileSystem::Tags::set(path, dirTag(), Theme::instance()->orgDomainName().toUtf8());
+    if (!result1) {
+        qCWarning(lcGuiUtility) << QStringLiteral("Failed to set tag on '%1': %2").arg(path, result1.error())
 #ifdef Q_OS_WIN
                                 << QStringLiteral("(filesystem %1)").arg(FileSystem::fileSystemForPath(path))
 #endif // Q_OS_WIN
             ;
+        return;
+    }
+
+    auto result2 = FileSystem::Tags::set(path, uuidTag(), accountUuid.toString().toUtf8());
+    if (!result2) {
+        qCWarning(lcGuiUtility) << QStringLiteral("Failed to set tag on '%1': %2").arg(path, result2.error())
+#ifdef Q_OS_WIN
+                                << QStringLiteral("(filesystem %1)").arg(FileSystem::fileSystemForPath(path))
+#endif // Q_OS_WIN
+            ;
+        return;
     }
 }
 
-QString Utility::getDirectorySyncRootMarking(const QString &path)
+std::pair<QString, QUuid> Utility::getDirectorySyncRootMarkings(const QString &path)
 {
-    auto existingValue = FileSystem::Tags::get(path, dirTag());
-    if (existingValue.has_value()) {
-        return QString::fromUtf8(existingValue.value());
+    auto existingDirTag = FileSystem::Tags::get(path, dirTag());
+    auto existingUuidTag = FileSystem::Tags::get(path, uuidTag());
+
+    if (existingDirTag.has_value() && existingUuidTag.has_value()) {
+        return {QString::fromUtf8(existingDirTag.value()), QUuid::fromString(QString::fromUtf8(existingUuidTag.value()))};
     }
 
     return {};
@@ -133,4 +152,7 @@ void Utility::unmarkDirectoryAsSyncRoot(const QString &path)
     if (!FileSystem::Tags::remove(path, dirTag())) {
         qCWarning(lcGuiUtility) << "Failed to remove tag on" << path;
     }
+    if (!FileSystem::Tags::remove(path, uuidTag())) {
+        qCWarning(lcGuiUtility) << "Failed to remove uuid tag on" << path;
+    }
 }

src/gui/guiutility.h

@@ -53,8 +53,8 @@ namespace Utility {
 
     bool internetConnectionIsMetered();
 
-    void markDirectoryAsSyncRoot(const QString &path);
-    QString getDirectorySyncRootMarking(const QString &path);
+    void markDirectoryAsSyncRoot(const QString &path, const QUuid &accountUuid);
+    std::pair<QString, QUuid> getDirectorySyncRootMarkings(const QString &path);
     void unmarkDirectoryAsSyncRoot(const QString &path);
 } // namespace Utility
 } // namespace OCC

src/gui/newwizard/setupwizardaccountbuilder.cpp

@@ -141,7 +141,7 @@ AccountPtr SetupWizardAccountBuilder::build()
         if (!QFileInfo::exists(_defaultSyncTargetDir)) {
             OC_ASSERT(QDir().mkpath(_defaultSyncTargetDir));
         }
-        Utility::markDirectoryAsSyncRoot(_defaultSyncTargetDir);
+        Utility::markDirectoryAsSyncRoot(_defaultSyncTargetDir, newAccountPtr->uuid());
     }
 
     return newAccountPtr;

src/gui/newwizard/states/accountconfiguredsetupwizardstate.cpp

@@ -53,7 +53,9 @@ AccountConfiguredSetupWizardState::AccountConfiguredSetupWizardState(SetupWizard
         return _context->accountBuilder().serverUrl();
     }();
 
-    QString defaultSyncTargetDir = FolderMan::suggestSyncFolder(urlToSuggestSyncFolderFor, _context->accountBuilder().displayName());
+    // We need some sync root, either for spaces, or for OC10. It's never a Space folder.
+    auto folderType = FolderMan::NewFolderType::SpacesSyncRoot;
+    QString defaultSyncTargetDir = FolderMan::suggestSyncFolder(urlToSuggestSyncFolderFor, _context->accountBuilder().displayName(), folderType);
     QString syncTargetDir = _context->accountBuilder().syncTargetDir();
 
     if (syncTargetDir.isEmpty()) {
@@ -86,7 +88,8 @@ void AccountConfiguredSetupWizardState::evaluatePage()
             return;
         }
 
-        QString invalidPathErrorMessage = FolderMan::checkPathValidityRecursive(syncTargetDir);
+        // Doesn't matter wether it's a spaces sync root or a oc10 sync root
+        QString invalidPathErrorMessage = FolderMan::checkPathValidityRecursive(syncTargetDir, FolderMan::NewFolderType::SpacesSyncRoot, {});
         if (!invalidPathErrorMessage.isEmpty()) {
             Q_EMIT evaluationFailed(errorMessageTemplate.arg(invalidPathErrorMessage));
             return;

src/gui/owncloudgui.cpp

@@ -93,7 +93,7 @@ void setUpInitialSyncFolder(AccountStatePtr accountStatePtr, bool useVfs)
                     Utility::setupFavLink(localDir);
                     for (const auto *space : spaces) {
                         const QString name = space->displayName();
-                        const QString folderName = FolderMan::instance()->findGoodPathForNewSyncFolder(localDir, name);
+                        const QString folderName = FolderMan::instance()->findGoodPathForNewSyncFolder(localDir, name, FolderMan::NewFolderType::SpacesFolder);
                         auto folder = addFolder(folderName, {}, QUrl(space->drive().getRoot().getWebDavUrl()), space->drive().getRoot().getId(), name);
                         folder->setPriority(space->priority());
                         // save the new priority