From bbb36e3c13a4969d4af874fe10c63319c2908620 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Tue, 3 Sep 2024 18:07:57 +0200 Subject: [PATCH] detect remnants read-only folders to delete: try to delete them Signed-off-by: Matthieu Gallien --- src/gui/folder.cpp | 30 +++++++++++ src/gui/folder.h | 4 ++ src/libsync/syncengine.cpp | 11 ++++ src/libsync/syncengine.h | 6 +++ test/testpermissions.cpp | 106 +++++++++++++++++++++++++++++++++++++ 5 files changed, 157 insertions(+) diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index cee0a8d7204e..03422e90f1b6 100644 --- a/src/gui/folder.cpp +++ b/src/gui/folder.cpp @@ -100,6 +100,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); @@ -1653,6 +1655,34 @@ void Folder::slotAboutToRemoveAllFiles(SyncFileItem::Direction dir, std::functio msgBox->open(); } +void Folder::slotNeedToRemoveRemnantsReadOnlyFolders(const QList &folders, + const QString &localPath, + std::function 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 eea99e144778..8f2e5265a10d 100644 --- a/src/gui/folder.h +++ b/src/gui/folder.h @@ -333,6 +333,10 @@ public slots: // connected to the corresponding signals in the SyncEngine void slotAboutToRemoveAllFiles(OCC::SyncFileItem::Direction, std::function callback); + void slotNeedToRemoveRemnantsReadOnlyFolders(const QList &folders, + const QString &localPath, + std::function callback); + /** * Starts a sync operation * diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index 20c77814b603..592078dda140 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -822,6 +822,10 @@ void SyncEngine::slotDiscoveryFinished() return; } + if (!_remnantReadOnlyFolders.isEmpty()) { + handleRemnantReadOnlyFolders(); + return; + } finishSync(); } @@ -1106,6 +1110,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 void SyncEngine::promptUserBeforePropagation(T &&lambda) { diff --git a/src/libsync/syncengine.h b/src/libsync/syncengine.h index 174b0fcaae33..b0b26c5804bf 100644 --- a/src/libsync/syncengine.h +++ b/src/libsync/syncengine.h @@ -184,6 +184,10 @@ public slots: */ void aboutToRemoveAllFiles(OCC::SyncFileItem::Direction direction, std::function f); + void aboutToRemoveRemnantsReadOnlyFolders(const QList &folders, + const QString &localPath, + std::function f); + // A new folder was discovered and was not synced because of the confirmation feature void newBigFolder(const QString &folder, bool isExternal); @@ -360,6 +364,8 @@ private slots: void finishSync(); + void handleRemnantReadOnlyFolders(); + template void promptUserBeforePropagation(T &&lambda); diff --git a/test/testpermissions.cpp b/test/testpermissions.cpp index 1e2ab899b7ab..bdef17baee75 100644 --- 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; } @@ -85,6 +95,14 @@ private slots: FakeFolder fakeFolder{ FileInfo() }; QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + QObject::connect(&fakeFolder.syncEngine(), &SyncEngine::aboutToRemoveRemnantsReadOnlyFolders, + [&](const QList &folders, const QString &localPath, std::function 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. @@ -401,6 +419,13 @@ private slots: { FakeFolder fakeFolder{FileInfo{}}; + QObject::connect(&fakeFolder.syncEngine(), &SyncEngine::aboutToRemoveRemnantsReadOnlyFolders, + [&](const QList &folders, const QString &localPath, std::function 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. @@ -520,6 +545,14 @@ private slots: void testAllowedMoveForbiddenDelete() { FakeFolder fakeFolder{FileInfo{}}; + QObject::connect(&fakeFolder.syncEngine(), &SyncEngine::aboutToRemoveRemnantsReadOnlyFolders, + [&](const QList &folders, const QString &localPath, std::function 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. @@ -568,6 +601,15 @@ private slots: void testParentMoveNotAllowedChildrenRestored() { FakeFolder fakeFolder{FileInfo{}}; + + QObject::connect(&fakeFolder.syncEngine(), &SyncEngine::aboutToRemoveRemnantsReadOnlyFolders, + [&](const QList &folders, const QString &localPath, std::function 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"); @@ -620,6 +662,14 @@ private slots: { FakeFolder fakeFolder{FileInfo{}}; + QObject::connect(&fakeFolder.syncEngine(), &SyncEngine::aboutToRemoveRemnantsReadOnlyFolders, + [&](const QList &folders, const QString &localPath, std::function callback) { + for(const auto &oneFolder : folders) { + FileSystem::removeRecursively(localPath + oneFolder->_file); + } + callback(false); + }); + auto &remote = fakeFolder.remoteModifier(); remote.mkdir("readOnlyFolder"); @@ -637,6 +687,14 @@ private slots: { FakeFolder fakeFolder{FileInfo{}}; + QObject::connect(&fakeFolder.syncEngine(), &SyncEngine::aboutToRemoveRemnantsReadOnlyFolders, + [&](const QList &folders, const QString &localPath, std::function callback) { + for(const auto &oneFolder : folders) { + FileSystem::removeRecursively(localPath + oneFolder->_file); + } + callback(false); + }); + auto &remote = fakeFolder.remoteModifier(); remote.mkdir("readWriteFolder"); @@ -655,6 +713,14 @@ private slots: { FakeFolder fakeFolder{FileInfo{}}; + QObject::connect(&fakeFolder.syncEngine(), &SyncEngine::aboutToRemoveRemnantsReadOnlyFolders, + [&](const QList &folders, const QString &localPath, std::function callback) { + for(const auto &oneFolder : folders) { + FileSystem::removeRecursively(localPath + oneFolder->_file); + } + callback(false); + }); + auto &remote = fakeFolder.remoteModifier(); remote.mkdir("testFolder"); @@ -691,6 +757,14 @@ private slots: { FakeFolder fakeFolder{FileInfo{}}; + QObject::connect(&fakeFolder.syncEngine(), &SyncEngine::aboutToRemoveRemnantsReadOnlyFolders, + [&](const QList &folders, const QString &localPath, std::function callback) { + for(const auto &oneFolder : folders) { + FileSystem::removeRecursively(localPath + oneFolder->_file); + } + callback(false); + }); + auto &remote = fakeFolder.remoteModifier(); remote.mkdir("testFolder"); @@ -751,6 +825,14 @@ private slots: { FakeFolder fakeFolder{FileInfo{}}; + QObject::connect(&fakeFolder.syncEngine(), &SyncEngine::aboutToRemoveRemnantsReadOnlyFolders, + [&](const QList &folders, const QString &localPath, std::function callback) { + for(const auto &oneFolder : folders) { + FileSystem::removeRecursively(localPath + oneFolder->_file); + } + callback(false); + }); + auto &remote = fakeFolder.remoteModifier(); remote.mkdir("readOnlyFolder"); @@ -781,6 +863,14 @@ private slots: { FakeFolder fakeFolder{FileInfo{}}; + QObject::connect(&fakeFolder.syncEngine(), &SyncEngine::aboutToRemoveRemnantsReadOnlyFolders, + [&](const QList &folders, const QString &localPath, std::function callback) { + for(const auto &oneFolder : folders) { + FileSystem::removeRecursively(localPath + oneFolder->_file); + } + callback(false); + }); + auto &remote = fakeFolder.remoteModifier(); remote.mkdir("readOnlyFolder"); @@ -813,6 +903,14 @@ private slots: { FakeFolder fakeFolder{FileInfo{}}; + QObject::connect(&fakeFolder.syncEngine(), &SyncEngine::aboutToRemoveRemnantsReadOnlyFolders, + [&](const QList &folders, const QString &localPath, std::function callback) { + for(const auto &oneFolder : folders) { + FileSystem::removeRecursively(localPath + oneFolder->_file); + } + callback(false); + }); + auto &remote = fakeFolder.remoteModifier(); remote.mkdir("readOnlyFolder"); @@ -848,6 +946,14 @@ private slots: { FakeFolder fakeFolder{FileInfo{}}; + QObject::connect(&fakeFolder.syncEngine(), &SyncEngine::aboutToRemoveRemnantsReadOnlyFolders, + [&](const QList &folders, const QString &localPath, std::function callback) { + for(const auto &oneFolder : folders) { + FileSystem::removeRecursively(localPath + oneFolder->_file); + } + callback(false); + }); + auto &remote = fakeFolder.remoteModifier(); remote.mkdir("readOnlyFolder");