detect remnants read-only folders to delete: try to delete them

Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>

diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp
index 5c7620c6a7fa06b2c6d76fbd17ab1f06cb8b1160..52baa342f13b6c3fc2cd3a87a70718e168c94cd7 100644 (file)
--- a/src/gui/folder.cpp
+++ b/src/gui/folder.cpp
@@ -101,6 +101,8 @@ Folder::Folder(const FolderDefinition &definition,
 
     connect(_engine.data(), &SyncEngine::aboutToRemoveAllFiles,
         this, &Folder::slotAboutToRemoveAllFiles);
+    connect(_engine.data(), &SyncEngine::aboutToRemoveRemnantsReadOnlyFolders,
+            this, &Folder::slotNeedToRemoveRemnantsReadOnlyFolders);
     connect(_engine.data(), &SyncEngine::transmissionProgress, this, &Folder::slotTransmissionProgress);
     connect(_engine.data(), &SyncEngine::itemCompleted,
         this, &Folder::slotItemCompleted);
@@ -1664,6 +1666,34 @@ void Folder::slotAboutToRemoveAllFiles(SyncFileItem::Direction dir, std::functio
     msgBox->open();
 }
 
+void Folder::slotNeedToRemoveRemnantsReadOnlyFolders(const QList<SyncFileItemPtr> &folders,
+                                                     const QString &localPath,
+                                                     std::function<void (bool)> callback)
+{
+    const auto msg = tr("Do you want to clean up remnant read-only folders left over from previous failed synchronization attempts.");
+    auto msgBox = new QMessageBox(QMessageBox::Question, tr("Remove remnant invalid folders?"),
+                                  msg, QMessageBox::NoButton);
+    msgBox->setAttribute(Qt::WA_DeleteOnClose);
+    msgBox->setWindowFlags(msgBox->windowFlags() | Qt::WindowStaysOnTopHint);
+    msgBox->addButton(tr("Proceed to remove remnant folders"), QMessageBox::AcceptRole);
+    const auto keepBtn = msgBox->addButton(tr("Do nothing"), QMessageBox::RejectRole);
+    setSyncPaused(true);
+    connect(msgBox, &QMessageBox::finished, this, [msgBox, keepBtn, callback, folders, localPath, this] {
+        const bool cancel = msgBox->clickedButton() == keepBtn;
+        if (!cancel) {
+            for(const auto &oneFolder : folders) {
+                FileSystem::removeRecursively(localPath + oneFolder->_file);
+            }
+        }
+        callback(cancel);
+        if (cancel) {
+            setSyncPaused(true);
+        }
+    });
+    connect(this, &Folder::destroyed, msgBox, &QMessageBox::deleteLater);
+    msgBox->open();
+}
+
 void Folder::removeLocalE2eFiles()
 {
     qCDebug(lcFolder) << "Removing local E2EE files";

diff --git a/src/gui/folder.h b/src/gui/folder.h
index e3bed9f7f92e82828190f103dcfeecb16d934fcf..c79153a86758f02414ac1b28d96bc0cb95a20ee6 100644 (file)
--- a/src/gui/folder.h
+++ b/src/gui/folder.h
@@ -335,6 +335,10 @@ public slots:
     // connected to the corresponding signals in the SyncEngine
     void slotAboutToRemoveAllFiles(OCC::SyncFileItem::Direction, std::function<void(bool)> callback);
 
+    void slotNeedToRemoveRemnantsReadOnlyFolders(const QList<SyncFileItemPtr> &folders,
+                                                 const QString &localPath,
+                                                 std::function<void(bool)> callback);
+
     /**
       * Starts a sync operation
       *

diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp
index eb114f553d2400266846c35bbe3de22acc0dbecf..9be7992a5b7288de77dcc293ff2644ed546ba322 100644 (file)
--- a/src/libsync/syncengine.cpp
+++ b/src/libsync/syncengine.cpp
@@ -817,6 +817,10 @@ void SyncEngine::slotDiscoveryFinished()
         return;
     }
 
+    if (!_remnantReadOnlyFolders.isEmpty()) {
+        handleRemnantReadOnlyFolders();
+        return;
+    }
 
     finishSync();
 }
@@ -1101,6 +1105,13 @@ void SyncEngine::finishSync()
     qCInfo(lcEngine) << "#### Post-Reconcile end #################################################### " << _stopWatch.addLapTime(QStringLiteral("Post-Reconcile Finished")) << "ms";
 }
 
+void SyncEngine::handleRemnantReadOnlyFolders()
+{
+    promptUserBeforePropagation([this](auto &&callback) {
+                                    emit aboutToRemoveRemnantsReadOnlyFolders(_remnantReadOnlyFolders, _localPath, callback);
+                                });
+}
+
 template <typename T>
 void SyncEngine::promptUserBeforePropagation(T &&lambda)
 {

diff --git a/src/libsync/syncengine.h b/src/libsync/syncengine.h
index 278c5569dfd4bc422c6ac3671c811414b035eaa8..2d824f3318d4e73c0bf8202fef5c89e690e82801 100644 (file)
--- a/src/libsync/syncengine.h
+++ b/src/libsync/syncengine.h
@@ -189,6 +189,10 @@ signals:
      */
     void aboutToRemoveAllFiles(OCC::SyncFileItem::Direction direction, std::function<void(bool)> f);
 
+    void aboutToRemoveRemnantsReadOnlyFolders(const QList<SyncFileItemPtr> &folders,
+                                              const QString &localPath,
+                                              std::function<void(bool)> f);
+
     // A new folder was discovered and was not synced because of the confirmation feature
     void newBigFolder(const QString &folder, bool isExternal);
 
@@ -365,6 +369,8 @@ private:
 
     void finishSync();
 
+    void handleRemnantReadOnlyFolders();
+
     template <typename T>
     void promptUserBeforePropagation(T &&lambda);
 

diff --git a/test/testpermissions.cpp b/test/testpermissions.cpp
index ee1ddbcdb7a9d71cc1060e351ba80662d46073eb..f334b22de5abc7b952315bd4a8ec83316ee03238 100644 (file)
--- a/test/testpermissions.cpp
+++ b/test/testpermissions.cpp
@@ -59,12 +59,22 @@ SyncFileItemPtr findDiscoveryItem(const SyncFileItemVector &spy, const QString &
 bool itemInstruction(const ItemCompletedSpy &spy, const QString &path, const SyncInstructions instr)
 {
     auto item = spy.findItem(path);
+    const auto checkHelper = [item, instr]() {
+        QCOMPARE(item->_instruction, instr);
+    };
+
+    checkHelper();
     return item->_instruction == instr;
 }
 
 bool discoveryInstruction(const SyncFileItemVector &spy, const QString &path, const SyncInstructions instr)
 {
     auto item = findDiscoveryItem(spy, path);
+    const auto checkHelper = [item, instr]() {
+        QCOMPARE(item->_instruction, instr);
+    };
+
+    checkHelper();
     return item->_instruction == instr;
 }
 
@@ -87,6 +97,14 @@ private slots:
         FakeFolder fakeFolder{ FileInfo() };
         QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
 
+        QObject::connect(&fakeFolder.syncEngine(), &SyncEngine::aboutToRemoveRemnantsReadOnlyFolders,
+                         [&](const QList<SyncFileItemPtr> &folders, const QString &localPath, std::function<void(bool)> callback) {
+                             qDebug() << "aboutToRemoveRemnantsReadOnlyFolders called";
+                             Q_UNUSED(folders);
+                             Q_UNUSED(localPath);
+                             callback(false);
+                         });
+
         // Some of this test depends on the order of discovery. With threading
         // that order becomes effectively random, but we want to make sure to test
         // all cases and thus disable threading.
@@ -424,6 +442,13 @@ private slots:
     {
         FakeFolder fakeFolder{FileInfo{}};
 
+        QObject::connect(&fakeFolder.syncEngine(), &SyncEngine::aboutToRemoveRemnantsReadOnlyFolders,
+                         [&](const QList<SyncFileItemPtr> &folders, const QString &localPath, std::function<void(bool)> callback) {
+                             Q_UNUSED(folders)
+                             Q_UNUSED(localPath)
+                             callback(false);
+                         });
+
         // Some of this test depends on the order of discovery. With threading
         // that order becomes effectively random, but we want to make sure to test
         // all cases and thus disable threading.
@@ -543,6 +568,14 @@ private slots:
     void testAllowedMoveForbiddenDelete() {
          FakeFolder fakeFolder{FileInfo{}};
 
+        QObject::connect(&fakeFolder.syncEngine(), &SyncEngine::aboutToRemoveRemnantsReadOnlyFolders,
+                         [&](const QList<SyncFileItemPtr> &folders, const QString &localPath, std::function<void(bool)> callback) {
+                             for(const auto &oneFolder : folders) {
+                                 FileSystem::removeRecursively(localPath + oneFolder->_file);
+                             }
+                             callback(false);
+                         });
+
         // Some of this test depends on the order of discovery. With threading
         // that order becomes effectively random, but we want to make sure to test
         // all cases and thus disable threading.
@@ -591,6 +624,15 @@ private slots:
     void testParentMoveNotAllowedChildrenRestored()
     {
         FakeFolder fakeFolder{FileInfo{}};
+
+        QObject::connect(&fakeFolder.syncEngine(), &SyncEngine::aboutToRemoveRemnantsReadOnlyFolders,
+                         [&](const QList<SyncFileItemPtr> &folders, const QString &localPath, std::function<void(bool)> callback) {
+                             for(const auto &oneFolder : folders) {
+                                 FileSystem::removeRecursively(localPath + oneFolder->_file);
+                             }
+                             callback(false);
+                         });
+
         auto &lm = fakeFolder.localModifier();
         auto &rm = fakeFolder.remoteModifier();
         rm.mkdir("forbidden-move");
@@ -643,6 +685,14 @@ private slots:
     {
         FakeFolder fakeFolder{FileInfo{}};
 
+        QObject::connect(&fakeFolder.syncEngine(), &SyncEngine::aboutToRemoveRemnantsReadOnlyFolders,
+                         [&](const QList<SyncFileItemPtr> &folders, const QString &localPath, std::function<void(bool)> callback) {
+                             for(const auto &oneFolder : folders) {
+                                 FileSystem::removeRecursively(localPath + oneFolder->_file);
+                             }
+                             callback(false);
+                         });
+
         auto &remote = fakeFolder.remoteModifier();
 
         remote.mkdir("readOnlyFolder");
@@ -660,6 +710,14 @@ private slots:
     {
         FakeFolder fakeFolder{FileInfo{}};
 
+        QObject::connect(&fakeFolder.syncEngine(), &SyncEngine::aboutToRemoveRemnantsReadOnlyFolders,
+                         [&](const QList<SyncFileItemPtr> &folders, const QString &localPath, std::function<void(bool)> callback) {
+                             for(const auto &oneFolder : folders) {
+                                 FileSystem::removeRecursively(localPath + oneFolder->_file);
+                             }
+                             callback(false);
+                         });
+
         auto &remote = fakeFolder.remoteModifier();
 
         remote.mkdir("readWriteFolder");
@@ -678,6 +736,14 @@ private slots:
     {
         FakeFolder fakeFolder{FileInfo{}};
 
+        QObject::connect(&fakeFolder.syncEngine(), &SyncEngine::aboutToRemoveRemnantsReadOnlyFolders,
+                         [&](const QList<SyncFileItemPtr> &folders, const QString &localPath, std::function<void(bool)> callback) {
+                             for(const auto &oneFolder : folders) {
+                                 FileSystem::removeRecursively(localPath + oneFolder->_file);
+                             }
+                             callback(false);
+                         });
+
         auto &remote = fakeFolder.remoteModifier();
 
         remote.mkdir("testFolder");
@@ -714,6 +780,14 @@ private slots:
     {
         FakeFolder fakeFolder{FileInfo{}};
 
+        QObject::connect(&fakeFolder.syncEngine(), &SyncEngine::aboutToRemoveRemnantsReadOnlyFolders,
+                         [&](const QList<SyncFileItemPtr> &folders, const QString &localPath, std::function<void(bool)> callback) {
+                             for(const auto &oneFolder : folders) {
+                                 FileSystem::removeRecursively(localPath + oneFolder->_file);
+                             }
+                             callback(false);
+                         });
+
         auto &remote = fakeFolder.remoteModifier();
 
         remote.mkdir("testFolder");
@@ -774,6 +848,14 @@ private slots:
     {
         FakeFolder fakeFolder{FileInfo{}};
 
+        QObject::connect(&fakeFolder.syncEngine(), &SyncEngine::aboutToRemoveRemnantsReadOnlyFolders,
+                         [&](const QList<SyncFileItemPtr> &folders, const QString &localPath, std::function<void(bool)> callback) {
+                             for(const auto &oneFolder : folders) {
+                                 FileSystem::removeRecursively(localPath + oneFolder->_file);
+                             }
+                             callback(false);
+                         });
+
         auto &remote = fakeFolder.remoteModifier();
 
         remote.mkdir("readOnlyFolder");
@@ -804,6 +886,14 @@ private slots:
     {
         FakeFolder fakeFolder{FileInfo{}};
 
+        QObject::connect(&fakeFolder.syncEngine(), &SyncEngine::aboutToRemoveRemnantsReadOnlyFolders,
+                         [&](const QList<SyncFileItemPtr> &folders, const QString &localPath, std::function<void(bool)> callback) {
+                             for(const auto &oneFolder : folders) {
+                                 FileSystem::removeRecursively(localPath + oneFolder->_file);
+                             }
+                             callback(false);
+                         });
+
         auto &remote = fakeFolder.remoteModifier();
 
         remote.mkdir("readOnlyFolder");
@@ -836,6 +926,14 @@ private slots:
     {
         FakeFolder fakeFolder{FileInfo{}};
 
+        QObject::connect(&fakeFolder.syncEngine(), &SyncEngine::aboutToRemoveRemnantsReadOnlyFolders,
+                         [&](const QList<SyncFileItemPtr> &folders, const QString &localPath, std::function<void(bool)> callback) {
+                             for(const auto &oneFolder : folders) {
+                                 FileSystem::removeRecursively(localPath + oneFolder->_file);
+                             }
+                             callback(false);
+                         });
+
         auto &remote = fakeFolder.remoteModifier();
 
         remote.mkdir("readOnlyFolder");
@@ -871,6 +969,14 @@ private slots:
     {
         FakeFolder fakeFolder{FileInfo{}};
 
+        QObject::connect(&fakeFolder.syncEngine(), &SyncEngine::aboutToRemoveRemnantsReadOnlyFolders,
+                         [&](const QList<SyncFileItemPtr> &folders, const QString &localPath, std::function<void(bool)> callback) {
+                             for(const auto &oneFolder : folders) {
+                                 FileSystem::removeRecursively(localPath + oneFolder->_file);
+                             }
+                             callback(false);
+                         });
+
         auto &remote = fakeFolder.remoteModifier();
 
         remote.mkdir("readOnlyFolder");