Merge pull request #6578 from nextcloud/bugfix/allowRenamingFoldersIn…

…sideGroupFolders properly compute if a folder is top level or child extern mounted
nextcloud · Mar 27, 2024 · 6c03f76 · 6c03f76
2 parents b6ff0c5 + 55034f7
commit 6c03f76
Show file tree

Hide file tree

Showing 16 changed files with 114 additions and 26 deletions.
diff --git a/VERSION.cmake b/VERSION.cmake
@@ -13,6 +13,10 @@ set(NEXTCLOUD_SERVER_VERSION_SECURE_FILEDROP_MIN_SUPPORTED_MAJOR 26)
 set(NEXTCLOUD_SERVER_VERSION_SECURE_FILEDROP_MIN_SUPPORTED_MINOR 0)
 set(NEXTCLOUD_SERVER_VERSION_SECURE_FILEDROP_MIN_SUPPORTED_PATCH 0)
 
+set(NEXTCLOUD_SERVER_VERSION_MOUNT_ROOT_PROPERTY_SUPPORTED_MAJOR 28)
+set(NEXTCLOUD_SERVER_VERSION_MOUNT_ROOT_PROPERTY_SUPPORTED_MINOR 0)
+set(NEXTCLOUD_SERVER_VERSION_MOUNT_ROOT_PROPERTY_SUPPORTED_PATCH 3)
+
 if ( NOT DEFINED MIRALL_VERSION_SUFFIX )
     set( MIRALL_VERSION_SUFFIX "git") #e.g. beta1, beta2, rc1
 endif( NOT DEFINED MIRALL_VERSION_SUFFIX )

diff --git a/src/common/remotepermissions.cpp b/src/common/remotepermissions.cpp
@@ -17,10 +17,15 @@
  */
 
 #include "remotepermissions.h"
+
+#include <QLoggingCategory>
+
 #include <cstring>
 
 namespace OCC {
 
+Q_LOGGING_CATEGORY(lcRemotePermissions, "nextcloud.sync.remotepermissions", QtInfoMsg)
+
 static const char letters[] = " WDNVCKRSMm";
 
 
@@ -68,11 +73,43 @@ RemotePermissions RemotePermissions::fromDbValue(const QByteArray &value)
     return perm;
 }
 
-RemotePermissions RemotePermissions::fromServerString(const QString &value)
+template <typename T>
+RemotePermissions RemotePermissions::internalFromServerString(const QString &value,
+                                                              const T&otherProperties,
+                                                              MountedPermissionAlgorithm algorithm)
 {
     RemotePermissions perm;
     perm.fromArray(value.utf16());
+
+    if (algorithm == MountedPermissionAlgorithm::WildGuessMountedSubProperty) {
+        return perm;
+    }
+
+    if ((otherProperties.contains(QStringLiteral("is-mount-root")) && otherProperties.value(QStringLiteral("is-mount-root")) == QStringLiteral("false") && perm.hasPermission(RemotePermissions::IsMounted)) ||
+        (!otherProperties.contains(QStringLiteral("is-mount-root")) && perm.hasPermission(RemotePermissions::IsMounted))) {
+        /* All the entries in a external storage have 'M' in their permission. However, for all
+           purposes in the desktop client, we only need to know about the mount points.
+           So replace the 'M' by a 'm' for every sub entries in an external storage */
+        perm.unsetPermission(RemotePermissions::IsMounted);
+        perm.setPermission(RemotePermissions::IsMountedSub);
+        qCInfo(lcRemotePermissions()) << otherProperties.value(QStringLiteral("permissions")) << "replacing M permissions by m for subfolders inside a group folder";
+    }
+
     return perm;
 }
 
+RemotePermissions RemotePermissions::fromServerString(const QString &value,
+                                                      MountedPermissionAlgorithm algorithm,
+                                                      const QMap<QString, QString> &otherProperties)
+{
+    return internalFromServerString(value, otherProperties, algorithm);
+}
+
+RemotePermissions RemotePermissions::fromServerString(const QString &value,
+                                                      MountedPermissionAlgorithm algorithm,
+                                                      const QVariantMap &otherProperties)
+{
+    return internalFromServerString(value, otherProperties, algorithm);
+}
+
 } // namespace OCC
diff --git a/src/common/remotepermissions.h b/src/common/remotepermissions.h
@@ -59,6 +59,11 @@ class OCSYNC_EXPORT RemotePermissions
         PermissionsCount = IsMountedSub
     };
 
+    enum class MountedPermissionAlgorithm {
+        UseMountRootProperty,
+        WildGuessMountedSubProperty,
+    };
+
     /// null permissions
     RemotePermissions() = default;
 
@@ -72,7 +77,14 @@ class OCSYNC_EXPORT RemotePermissions
     static RemotePermissions fromDbValue(const QByteArray &);
 
     /// read a permissions string received from the server, never null
-    static RemotePermissions fromServerString(const QString &);
+    static RemotePermissions fromServerString(const QString &value,
+                                              MountedPermissionAlgorithm algorithm = MountedPermissionAlgorithm::WildGuessMountedSubProperty,
+                                              const QMap<QString, QString> &otherProperties = {});
+
+    /// read a permissions string received from the server, never null
+    static RemotePermissions fromServerString(const QString &value,
+                                              MountedPermissionAlgorithm algorithm,
+                                              const QVariantMap &otherProperties = {});
 
     [[nodiscard]] bool hasPermission(Permissions p) const
     {
@@ -101,6 +113,13 @@ class OCSYNC_EXPORT RemotePermissions
     {
         return dbg << p.toString();
     }
+
+private:
+
+    template <typename T>
+    static RemotePermissions internalFromServerString(const QString &value,
+                                                      const T&otherProperties,
+                                                      MountedPermissionAlgorithm algorithm);
 };
 
 

diff --git a/src/gui/editlocallyjob.cpp b/src/gui/editlocallyjob.cpp
@@ -213,7 +213,8 @@ void EditLocallyJob::fetchRemoteFileParentInfo()
                                   QByteArrayLiteral("http://owncloud.org/ns:size"),
                                   QByteArrayLiteral("http://owncloud.org/ns:id"),
                                   QByteArrayLiteral("http://owncloud.org/ns:permissions"),
-                                  QByteArrayLiteral("http://owncloud.org/ns:checksums")};
+                                  QByteArrayLiteral("http://owncloud.org/ns:checksums"),
+                                  QByteArrayLiteral("http://nextcloud.org/ns:is-mount-root")};
 
     job->setProperties(props);
     connect(job, &LsColJob::directoryListingIterated, this, &EditLocallyJob::slotDirectoryListingIterated);
@@ -545,7 +546,9 @@ void EditLocallyJob::slotDirectoryListingIterated(const QString &name, const QMa
         const auto cleanName = nameWithoutDavPath.startsWith(remoteFolderPathWithoutLeadingSlash)
             ? nameWithoutDavPath.mid(remoteFolderPathWithoutLeadingSlash.size()) : nameWithoutDavPath;
         disconnect(job, &LsColJob::directoryListingIterated, this, &EditLocallyJob::slotDirectoryListingIterated);
-        _fileParentItem = SyncFileItem::fromProperties(cleanName, properties);
+        _fileParentItem = SyncFileItem::fromProperties(cleanName,
+                                                       properties,
+                                                       _accountState->account()->serverHasMountRootProperty() ? RemotePermissions::MountedPermissionAlgorithm::UseMountRootProperty : RemotePermissions::MountedPermissionAlgorithm::WildGuessMountedSubProperty);
     }
 }
 

diff --git a/src/gui/folderstatusmodel.cpp b/src/gui/folderstatusmodel.cpp
@@ -616,6 +616,7 @@ void FolderStatusModel::fetchMore(const QModelIndex &parent)
     auto props = QList<QByteArray>() << "resourcetype"
                                      << "http://owncloud.org/ns:size"
                                      << "http://owncloud.org/ns:permissions"
+                                     << "http://nextcloud.org/ns:is-mount-root"
                                      << "http://owncloud.org/ns:fileid";
     if (_accountState->account()->capabilities().clientSideEncryptionAvailable()) {
         props << "http://nextcloud.org/ns:is-encrypted";

diff --git a/src/gui/invalidfilenamedialog.cpp b/src/gui/invalidfilenamedialog.cpp
@@ -117,7 +117,7 @@ InvalidFilenameDialog::~InvalidFilenameDialog() = default;
 void InvalidFilenameDialog::checkIfAllowedToRename()
 {
     const auto propfindJob = new PropfindJob(_account, QDir::cleanPath(_folder->remotePath() + _originalFileName));
-    propfindJob->setProperties({ "http://owncloud.org/ns:permissions" });
+    propfindJob->setProperties({"http://owncloud.org/ns:permissions", "http://nextcloud.org/ns:is-mount-root"});
     connect(propfindJob, &PropfindJob::result, this, &InvalidFilenameDialog::onPropfindPermissionSuccess);
     connect(propfindJob, &PropfindJob::finishedWithError, this, &InvalidFilenameDialog::onPropfindPermissionError);
     propfindJob->start();

diff --git a/src/gui/shellextensionsserver.cpp b/src/gui/shellextensionsserver.cpp
@@ -169,7 +169,7 @@ void ShellExtensionsServer::processCustomStateRequest(QLocalSocket *socket, cons
     }));
 
     auto *const lsColJob = new LsColJob(folder->accountState()->account(), QDir::cleanPath(folder->remotePath() + lsColJobPath));
-    lsColJob->setProperties({QByteArrayLiteral("http://owncloud.org/ns:share-types"), QByteArrayLiteral("http://owncloud.org/ns:permissions")});
+    lsColJob->setProperties({QByteArrayLiteral("http://owncloud.org/ns:share-types"), QByteArrayLiteral("http://owncloud.org/ns:permissions"), QByteArrayLiteral("http://nextcloud.org/ns:is-mount-root")});
 
     const auto folderAlias = customStateRequestInfo.folderAlias;
 

diff --git a/src/libsync/account.cpp b/src/libsync/account.cpp
@@ -731,6 +731,17 @@ int Account::serverVersionInt() const
         components.value(2).toInt());
 }
 
+bool Account::serverHasMountRootProperty() const
+{
+    if (serverVersionInt() == 0) {
+        return false;
+    }
+
+    return serverVersionInt() >= Account::makeServerVersion(NEXTCLOUD_SERVER_VERSION_MOUNT_ROOT_PROPERTY_SUPPORTED_MAJOR,
+                                                            NEXTCLOUD_SERVER_VERSION_MOUNT_ROOT_PROPERTY_SUPPORTED_MINOR,
+                                                            NEXTCLOUD_SERVER_VERSION_MOUNT_ROOT_PROPERTY_SUPPORTED_PATCH);
+}
+
 bool Account::serverVersionUnsupported() const
 {
     if (serverVersionInt() == 0) {

diff --git a/src/libsync/account.h b/src/libsync/account.h
@@ -247,6 +247,8 @@ class OWNCLOUDSYNC_EXPORT Account : public QObject
      */
     [[nodiscard]] int serverVersionInt() const;
 
+    [[nodiscard]] bool serverHasMountRootProperty() const;
+
     static constexpr int makeServerVersion(const int majorVersion, const int minorVersion, const int patchVersion) {
         return (majorVersion << 16) + (minorVersion << 8) + patchVersion;
     };

diff --git a/src/libsync/caseclashconflictsolver.cpp b/src/libsync/caseclashconflictsolver.cpp
@@ -220,7 +220,7 @@ void CaseClashConflictSolver::processLeadingOrTrailingSpacesError(const QString
 void CaseClashConflictSolver::checkIfAllowedToRename()
 {
     const auto propfindJob = new PropfindJob(_account, QDir::cleanPath(remoteTargetFilePath()));
-    propfindJob->setProperties({ "http://owncloud.org/ns:permissions" });
+    propfindJob->setProperties({"http://owncloud.org/ns:permissions", "http://nextcloud.org/ns:is-mount-root"});
     connect(propfindJob, &PropfindJob::result, this, &CaseClashConflictSolver::onPropfindPermissionSuccess);
     connect(propfindJob, &PropfindJob::finishedWithError, this, &CaseClashConflictSolver::onPropfindPermissionError);
     propfindJob->start();

diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp
@@ -1438,14 +1438,18 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
 
     // Check local permission if we are allowed to put move the file here
     // Technically we should use the permissions from the server, but we'll assume it is the same
+    const auto serverHasMountRootProperty = _discoveryData->_account->serverHasMountRootProperty();
     const auto isExternalStorage = base._remotePerm.hasPermission(RemotePermissions::IsMounted);
     const auto movePerms = checkMovePermissions(base._remotePerm, originalPath, item->isDirectory());
-    if (!movePerms.sourceOk || !movePerms.destinationOk || isExternalStorage || isE2eeMoveOnlineOnlyItemWithCfApi) {
+    if (!movePerms.sourceOk || !movePerms.destinationOk || (serverHasMountRootProperty && isExternalStorage) || isE2eeMoveOnlineOnlyItemWithCfApi) {
         qCInfo(lcDisco) << "Move without permission to rename base file, "
                         << "source:" << movePerms.sourceOk
                         << ", target:" << movePerms.destinationOk
                         << ", targetNew:" << movePerms.destinationNewOk
-                        << ", isExternalStorage:" << isExternalStorage;
+                        << ", isExternalStorage:" << isExternalStorage
+                        << ", serverHasMountRootProperty:" << serverHasMountRootProperty
+                        << ", base._remotePerm:" << base._remotePerm.toString()
+                        << ", base.path():" << base.path();
 
         // If we can create the destination, do that.
         // Permission errors on the destination will be handled by checkPermissions later.

diff --git a/src/libsync/discoveryphase.cpp b/src/libsync/discoveryphase.cpp
@@ -419,6 +419,7 @@ void DiscoverySingleDirectoryJob::start()
               << "http://nextcloud.org/ns:lock-time"
               << "http://nextcloud.org/ns:lock-timeout";
     }
+    props << "http://nextcloud.org/ns:is-mount-root";
 
     lsColJob->setProperties(props);
 
@@ -458,7 +459,7 @@ SyncFileItem::EncryptionStatus DiscoverySingleDirectoryJob::requiredEncryptionSt
     return _encryptionStatusRequired;
 }
 
-static void propertyMapToRemoteInfo(const QMap<QString, QString> &map, RemoteInfo &result)
+static void propertyMapToRemoteInfo(const QMap<QString, QString> &map, RemotePermissions::MountedPermissionAlgorithm algorithm, RemoteInfo &result)
 {
     for (auto it = map.constBegin(); it != map.constEnd(); ++it) {
         QString property = it.key();
@@ -490,7 +491,7 @@ static void propertyMapToRemoteInfo(const QMap<QString, QString> &map, RemoteInf
         } else if (property == "dDC") {
             result.directDownloadCookies = value;
         } else if (property == "permissions") {
-            result.remotePerm = RemotePermissions::fromServerString(value);
+            result.remotePerm = RemotePermissions::fromServerString(value, algorithm, map);
         } else if (property == "checksums") {
             result.checksumHeader = findBestChecksum(value.toUtf8());
         } else if (property == "share-types" && !value.isEmpty()) {
@@ -560,7 +561,10 @@ void DiscoverySingleDirectoryJob::directoryListingIteratedSlot(const QString &fi
         // The first entry is for the folder itself, we should process it differently.
         _ignoredFirst = true;
         if (map.contains("permissions")) {
-            auto perm = RemotePermissions::fromServerString(map.value("permissions"));
+            auto perm = RemotePermissions::fromServerString(map.value("permissions"),
+                                                            _account->serverHasMountRootProperty() ? RemotePermissions::MountedPermissionAlgorithm::UseMountRootProperty : RemotePermissions::MountedPermissionAlgorithm::WildGuessMountedSubProperty,
+                                                            map);
+            qCInfo(lcDiscovery()) << file << map.value("permissions") << map;
             emit firstDirectoryPermissions(perm);
             _isExternalStorage = perm.hasPermission(RemotePermissions::IsMounted);
         }
@@ -585,22 +589,17 @@ void DiscoverySingleDirectoryJob::directoryListingIteratedSlot(const QString &fi
             _size = map.value("size").toInt();
         }
     } else {
-
         RemoteInfo result;
         int slash = file.lastIndexOf('/');
         result.name = file.mid(slash + 1);
         result.size = -1;
-        propertyMapToRemoteInfo(map, result);
+        propertyMapToRemoteInfo(map,
+                                _account->serverHasMountRootProperty() ? RemotePermissions::MountedPermissionAlgorithm::UseMountRootProperty : RemotePermissions::MountedPermissionAlgorithm::WildGuessMountedSubProperty,
+                                result);
         if (result.isDirectory)
             result.size = 0;
 
-        if (_isExternalStorage && result.remotePerm.hasPermission(RemotePermissions::IsMounted)) {
-            /* All the entries in a external storage have 'M' in their permission. However, for all
-               purposes in the desktop client, we only need to know about the mount points.
-               So replace the 'M' by a 'm' for every sub entries in an external storage */
-            result.remotePerm.unsetPermission(RemotePermissions::IsMounted);
-            result.remotePerm.setPermission(RemotePermissions::IsMountedSub);
-        }
+        qCInfo(lcDiscovery()) << file << map.value("permissions") << result.remotePerm.toString() << map;
         _results.push_back(std::move(result));
     }
 

diff --git a/src/libsync/propagateremotemkdir.cpp b/src/libsync/propagateremotemkdir.cpp
@@ -139,10 +139,13 @@ void PropagateRemoteMkdir::finalizeMkColJob(QNetworkReply::NetworkError err, con
 
     propagator()->_activeJobList.append(this);
     auto propfindJob = new PropfindJob(propagator()->account(), jobPath, this);
-    propfindJob->setProperties({QByteArrayLiteral("http://owncloud.org/ns:share-types"), QByteArrayLiteral("http://owncloud.org/ns:permissions")});
+    propfindJob->setProperties({QByteArrayLiteral("http://owncloud.org/ns:share-types"), QByteArrayLiteral("http://owncloud.org/ns:permissions"), QByteArrayLiteral("http://nextcloud.org/ns:is-mount-root")});
     connect(propfindJob, &PropfindJob::result, this, [this, jobPath](const QVariantMap &result){
         propagator()->_activeJobList.removeOne(this);
-        _item->_remotePerm = RemotePermissions::fromServerString(result.value(QStringLiteral("permissions")).toString());
+        _item->_remotePerm = RemotePermissions::fromServerString(result.value(QStringLiteral("permissions")).toString(),
+                                                                 propagator()->account()->serverHasMountRootProperty() ? RemotePermissions::MountedPermissionAlgorithm::UseMountRootProperty : RemotePermissions::MountedPermissionAlgorithm::WildGuessMountedSubProperty,
+                                                                 result);
+
         _item->_sharedByMe = !result.value(QStringLiteral("share-types")).toString().isEmpty();
         _item->_isShared = _item->_remotePerm.hasPermission(RemotePermissions::IsShared) || _item->_sharedByMe;
         _item->_lastShareStateFetchedTimestamp = QDateTime::currentMSecsSinceEpoch();
@@ -231,6 +234,7 @@ void PropagateRemoteMkdir::slotMkcolJobFinished()
 
     _item->_fileId = _job->reply()->rawHeader("OC-FileId");
 
+    qCInfo(lcPropagateRemoteMkdir()) << "mkcol job error string:" << _item->_errorString << _job->errorString();
     _item->_errorString = _job->errorString();
 
     const auto jobHttpReasonPhraseString = _job->reply()->attribute(QNetworkRequest::HttpReasonPhraseAttribute).toString();

diff --git a/src/libsync/syncfileitem.cpp b/src/libsync/syncfileitem.cpp
@@ -169,7 +169,7 @@ SyncFileItemPtr SyncFileItem::fromSyncJournalFileRecord(const SyncJournalFileRec
     return item;
 }
 
-SyncFileItemPtr SyncFileItem::fromProperties(const QString &filePath, const QMap<QString, QString> &properties)
+SyncFileItemPtr SyncFileItem::fromProperties(const QString &filePath, const QMap<QString, QString> &properties, RemotePermissions::MountedPermissionAlgorithm algorithm)
 {
     SyncFileItemPtr item(new SyncFileItem);
     item->_file = filePath;
@@ -182,7 +182,7 @@ SyncFileItemPtr SyncFileItem::fromProperties(const QString &filePath, const QMap
     item->_fileId = properties.value(QStringLiteral("id")).toUtf8();
 
     if (properties.contains(QStringLiteral("permissions"))) {
-        item->_remotePerm = RemotePermissions::fromServerString(properties.value("permissions"));
+        item->_remotePerm = RemotePermissions::fromServerString(properties.value("permissions"), algorithm, properties);
     }
 
     if (!properties.value(QStringLiteral("share-types")).isEmpty()) {

diff --git a/src/libsync/syncfileitem.h b/src/libsync/syncfileitem.h
@@ -133,7 +133,7 @@ class OWNCLOUDSYNC_EXPORT SyncFileItem
 
     /** Creates a basic SyncFileItem from remote properties
      */
-    [[nodiscard]] static SyncFileItemPtr fromProperties(const QString &filePath, const QMap<QString, QString> &properties);
+    [[nodiscard]] static SyncFileItemPtr fromProperties(const QString &filePath, const QMap<QString, QString> &properties, RemotePermissions::MountedPermissionAlgorithm algorithm);
 
 
     SyncFileItem()

diff --git a/version.h.in b/version.h.in
@@ -45,4 +45,8 @@ constexpr int NEXTCLOUD_SERVER_VERSION_SECURE_FILEDROP_MIN_SUPPORTED_MAJOR = @NE
 constexpr int NEXTCLOUD_SERVER_VERSION_SECURE_FILEDROP_MIN_SUPPORTED_MINOR = @NEXTCLOUD_SERVER_VERSION_SECURE_FILEDROP_MIN_SUPPORTED_MINOR@;
 constexpr int NEXTCLOUD_SERVER_VERSION_SECURE_FILEDROP_MIN_SUPPORTED_PATCH = @NEXTCLOUD_SERVER_VERSION_SECURE_FILEDROP_MIN_SUPPORTED_PATCH@;
 
+constexpr int NEXTCLOUD_SERVER_VERSION_MOUNT_ROOT_PROPERTY_SUPPORTED_MAJOR = @NEXTCLOUD_SERVER_VERSION_MOUNT_ROOT_PROPERTY_SUPPORTED_MAJOR@;
+constexpr int NEXTCLOUD_SERVER_VERSION_MOUNT_ROOT_PROPERTY_SUPPORTED_MINOR = @NEXTCLOUD_SERVER_VERSION_MOUNT_ROOT_PROPERTY_SUPPORTED_MINOR@;
+constexpr int NEXTCLOUD_SERVER_VERSION_MOUNT_ROOT_PROPERTY_SUPPORTED_PATCH = @NEXTCLOUD_SERVER_VERSION_MOUNT_ROOT_PROPERTY_SUPPORTED_PATCH@;
+
 #endif // VERSION_H