Merge pull request #6780 from nextcloud/bugfix/groupfolder-rename

Check if sync item is actually a folder before processing it
nextcloud · Jul 5, 2024 · 25acde4 · 25acde4
2 parents e4c571c + 0160cec
commit 25acde4
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 15 deletions.
diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp
@@ -1409,7 +1409,6 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
         item->_errorString = tr("Moved to invalid target, restoring");
     }
 
-    //
     // If it's not a move it's just a local-NEW
     if (!isMove || (isE2eeMove && !isE2eeMoveOnlineOnlyItemWithCfApi)) {
         if (base.isE2eEncrypted()) {
@@ -1419,27 +1418,14 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
             item->_e2eEncryptionServerCapability = EncryptionStatusEnums::fromEndToEndEncryptionApiVersion(_discoveryData->_account->capabilities().clientSideEncryptionVersion());
         }
         postProcessLocalNew();
-        /*if (item->isDirectory() && item->_instruction == CSYNC_INSTRUCTION_NEW && item->_direction == SyncFileItem::Up
-            && _discoveryData->_account->capabilities().clientSideEncryptionVersion() >= 2.0) {
-            OCC::SyncJournalFileRecord rec;
-            _discoveryData->_statedb->findEncryptedAncestorForRecord(item->_file, &rec);
-            if (rec.isValid() && rec._e2eEncryptionStatus >= OCC::SyncJournalFileRecord::EncryptionStatus::EncryptedMigratedV2_0) {
-                qCDebug(lcDisco) << "Attempting to create a subfolder in top-level E2EE V2 folder. Ignoring.";
-                item->_instruction = CSYNC_INSTRUCTION_IGNORE;
-                item->_direction = SyncFileItem::None;
-                item->_status = SyncFileItem::NormalError;
-                item->_errorString = tr("Creating nested encrypted folders is not supported yet.");
-            }
-        }*/
-
         finalize();
         return;
     }
 
     // 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 isExternalStorage = base._remotePerm.hasPermission(RemotePermissions::IsMounted) && base.isDirectory();
     const auto movePerms = checkMovePermissions(base._remotePerm, originalPath, item->isDirectory());
     if (!movePerms.sourceOk || !movePerms.destinationOk || (serverHasMountRootProperty && isExternalStorage) || isE2eeMoveOnlineOnlyItemWithCfApi) {
         qCInfo(lcDisco) << "Move without permission to rename base file, "

diff --git a/test/testsyncmove.cpp b/test/testsyncmove.cpp
@@ -320,6 +320,45 @@ private slots:
         QCOMPARE(printDbData(fakeFolder.dbState()), printDbData(remoteInfo));
     }
 
+    void testLocalExternalStorageRenameDetection()
+    {
+        FakeFolder fakeFolder{{}};
+        fakeFolder.remoteModifier().mkdir("external-storage");
+        auto externalStorage = fakeFolder.remoteModifier().find("external-storage");
+        externalStorage->extraDavProperties = "<nc:is-mount-root>true</nc:is-mount-root>";
+        setAllPerm(externalStorage, RemotePermissions::fromServerString("WDNVCKRM"));
+        QVERIFY(fakeFolder.syncOnce());
+
+        OperationCounter operationCounter;
+        fakeFolder.setServerOverride(operationCounter.functor());
+
+        fakeFolder.localModifier().insert("external-storage/file", 100);
+        QVERIFY(fakeFolder.syncOnce());
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+        QCOMPARE(printDbData(fakeFolder.dbState()), printDbData(fakeFolder.remoteModifier()));
+        QCOMPARE(operationCounter.nPUT, 1);
+
+        const auto firstFileId = fakeFolder.remoteModifier().find("external-storage/file")->fileId;
+
+        fakeFolder.localModifier().rename("external-storage/file", "external-storage/file2");
+        QVERIFY(fakeFolder.syncOnce());
+        QCOMPARE(operationCounter.nMOVE, 1);
+
+        fakeFolder.localModifier().rename("external-storage/file2", "external-storage/file3");
+        QVERIFY(fakeFolder.syncOnce());
+        QCOMPARE(operationCounter.nMOVE, 2);
+
+        const auto renamedFileId = fakeFolder.remoteModifier().find("external-storage/file3")->fileId;
+
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+        QCOMPARE(printDbData(fakeFolder.dbState()), printDbData(fakeFolder.remoteModifier()));
+
+        QCOMPARE(fakeFolder.remoteModifier().find("external-storage/file"), nullptr);
+        QCOMPARE(fakeFolder.remoteModifier().find("external-storage/file2"), nullptr);
+        QVERIFY(fakeFolder.remoteModifier().find("external-storage/file3"));
+        QCOMPARE(firstFileId, renamedFileId);
+    }
+
     void testDuplicateFileId_data()
     {
         QTest::addColumn<QString>("prefix");