From b7b72bf9f28da3de12ee88ab1dd8b519656b461b Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Fri, 30 Aug 2024 15:35:20 +0200 Subject: [PATCH 1/5] collect remnant read-only folders during discovery Signed-off-by: Matthieu Gallien --- src/libsync/discovery.cpp | 3 +++ src/libsync/discoveryphase.h | 1 + src/libsync/syncengine.cpp | 8 ++++++++ src/libsync/syncengine.h | 4 ++++ 4 files changed, 16 insertions(+) diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index ba24802cf2756..0c228ca807876 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -1832,12 +1832,14 @@ bool ProcessDirectoryJob::checkPermissions(const OCC::SyncFileItemPtr &item) #if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15 qCWarning(lcDisco) << "unexpected new folder in a read-only folder will be made read-write" << localPath; FileSystem::setFolderPermissions(localPath, FileSystem::FolderPermissions::ReadWrite); + emit _discoveryData->remnantReadOnlyFolderDiscovered(item); #endif return false; } else if (!item->isDirectory() && !perms.hasPermission(RemotePermissions::CanAddFile)) { qCWarning(lcDisco) << "checkForPermission: ERROR" << item->_file; item->_instruction = CSYNC_INSTRUCTION_ERROR; item->_errorString = tr("Not allowed because you don't have permission to add files in that folder"); + emit _discoveryData->remnantReadOnlyFolderDiscovered(item); return false; } break; @@ -2038,6 +2040,7 @@ int ProcessDirectoryJob::processSubJobs(int nbJobs) #if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15 qCWarning(lcDisco) << "unexpected new folder in a read-only folder will be made read-write" << localPath; FileSystem::setFolderPermissions(localPath, FileSystem::FolderPermissions::ReadWrite); + emit _discoveryData->remnantReadOnlyFolderDiscovered(_dirItem); #endif } diff --git a/src/libsync/discoveryphase.h b/src/libsync/discoveryphase.h index 89012df25f878..7d53f1336d698 100644 --- a/src/libsync/discoveryphase.h +++ b/src/libsync/discoveryphase.h @@ -357,6 +357,7 @@ class DiscoveryPhase : public QObject void addErrorToGui(const SyncFileItem::Status status, const QString &errorMessage, const QString &subject, const OCC::ErrorCategory category); + void remnantReadOnlyFolderDiscovered(const OCC::SyncFileItemPtr &item); private slots: void slotItemDiscovered(const OCC::SyncFileItemPtr &item); }; diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index 3f0cd9b3cd711..21d6b0c8b6bc3 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -630,6 +630,8 @@ void SyncEngine::startSync() _progressInfo->_status = ProgressInfo::Discovery; emit transmissionProgress(*_progressInfo); + _remnantReadOnlyFolders.clear(); + _discoveryPhase.reset(new DiscoveryPhase); _discoveryPhase->_leadingAndTrailingSpacesFilesAllowed = _leadingAndTrailingSpacesFilesAllowed; _discoveryPhase->_account = _account; @@ -683,6 +685,7 @@ void SyncEngine::startSync() connect(_discoveryPhase.data(), &DiscoveryPhase::finished, this, &SyncEngine::slotDiscoveryFinished); connect(_discoveryPhase.data(), &DiscoveryPhase::silentlyExcluded, _syncFileStatusTracker.data(), &SyncFileStatusTracker::slotAddSilentlyExcluded); + connect(_discoveryPhase.data(), &DiscoveryPhase::remnantReadOnlyFolderDiscovered, this, &SyncEngine::remnantReadOnlyFolderDiscovered); ProcessDirectoryJob *discoveryJob = nullptr; @@ -1486,6 +1489,11 @@ void SyncEngine::slotCleanupScheduledSyncTimers() } } +void SyncEngine::remnantReadOnlyFolderDiscovered(const SyncFileItemPtr &item) +{ + _remnantReadOnlyFolders.push_back(item); +} + void SyncEngine::slotUnscheduleFilesDelayedSync() { if (!_discoveryPhase || _discoveryPhase->_filesUnscheduleSync.empty()) { diff --git a/src/libsync/syncengine.h b/src/libsync/syncengine.h index ea9b3c21ad004..2887c4bc1f658 100644 --- a/src/libsync/syncengine.h +++ b/src/libsync/syncengine.h @@ -240,6 +240,8 @@ private slots: void slotUnscheduleFilesDelayedSync(); void slotCleanupScheduledSyncTimers(); + void remnantReadOnlyFolderDiscovered(const OCC::SyncFileItemPtr &item); + private: // Some files need a sync run to be executed at a specified time after // their status is scheduled to change (e.g. lock status will expire in @@ -404,6 +406,8 @@ private slots: QVector> _scheduledSyncTimers; SingleItemDiscoveryOptions _singleItemDiscoveryOptions; + + QList _remnantReadOnlyFolders; }; } From 155c61acbbaf32d5c4996b7b8b53c6204360fb8c Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Fri, 30 Aug 2024 16:51:09 +0200 Subject: [PATCH 2/5] let cancel sync and finish sync lambda be reusable methods will enable implementation of other ways to interrupt sync after discovery to get user feedback Signed-off-by: Matthieu Gallien --- src/libsync/syncengine.cpp | 246 ++++++++++++++++++++----------------- src/libsync/syncengine.h | 7 ++ 2 files changed, 138 insertions(+), 115 deletions(-) diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index 21d6b0c8b6bc3..eb114f553d240 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -801,102 +801,6 @@ void SyncEngine::slotDiscoveryFinished() _progressInfo->_status = ProgressInfo::Reconcile; emit transmissionProgress(*_progressInfo); - // qCInfo(lcEngine) << "Permissions of the root folder: " << _csync_ctx->remote.root_perms.toString(); - auto finish = [this]{ - auto databaseFingerprint = _journal->dataFingerprint(); - // If databaseFingerprint is empty, this means that there was no information in the database - // (for example, upgrading from a previous version, or first sync, or server not supporting fingerprint) - if (!databaseFingerprint.isEmpty() && _discoveryPhase - && _discoveryPhase->_dataFingerprint != databaseFingerprint) { - qCInfo(lcEngine) << "data fingerprint changed, assume restore from backup" << databaseFingerprint << _discoveryPhase->_dataFingerprint; - restoreOldFiles(_syncItems); - } - - if (_discoveryPhase->_anotherSyncNeeded && !_discoveryPhase->_filesNeedingScheduledSync.empty()) { - slotScheduleFilesDelayedSync(); - } else if (_discoveryPhase->_anotherSyncNeeded && _anotherSyncNeeded == NoFollowUpSync) { - _anotherSyncNeeded = ImmediateFollowUp; - } - - if (!_discoveryPhase->_filesUnscheduleSync.empty()) { - slotUnscheduleFilesDelayedSync(); - } - - if (_discoveryPhase->_hasDownloadRemovedItems && _discoveryPhase->_hasUploadErrorItems) { - for (const auto &item : qAsConst(_syncItems)) { - if (item->_instruction == CSYNC_INSTRUCTION_ERROR && item->_direction == SyncFileItem::Up) { - item->_instruction = CSYNC_INSTRUCTION_IGNORE; - } - } - _anotherSyncNeeded = ImmediateFollowUp; - } - - Q_ASSERT(std::is_sorted(_syncItems.begin(), _syncItems.end())); - - qCInfo(lcEngine) << "#### Reconcile (aboutToPropagate) #################################################### " << _stopWatch.addLapTime(QStringLiteral("Reconcile (aboutToPropagate)")) << "ms"; - - _localDiscoveryPaths.clear(); - - // To announce the beginning of the sync - emit aboutToPropagate(_syncItems); - - qCInfo(lcEngine) << "#### Reconcile (aboutToPropagate OK) #################################################### "<< _stopWatch.addLapTime(QStringLiteral("Reconcile (aboutToPropagate OK)")) << "ms"; - - // it's important to do this before ProgressInfo::start(), to announce start of new sync - _progressInfo->_status = ProgressInfo::Propagation; - emit transmissionProgress(*_progressInfo); - _progressInfo->startEstimateUpdates(); - - // post update phase script: allow to tweak stuff by a custom script in debug mode. - if (!qEnvironmentVariableIsEmpty("OWNCLOUD_POST_UPDATE_SCRIPT")) { - #ifndef NDEBUG - const QString script = qEnvironmentVariable("OWNCLOUD_POST_UPDATE_SCRIPT"); - - qCDebug(lcEngine) << "Post Update Script: " << script; - auto scriptArgs = script.split(QRegularExpression("\\s+"), Qt::SkipEmptyParts); - if (scriptArgs.size() > 0) { - const auto scriptExecutable = scriptArgs.takeFirst(); - QProcess::execute(scriptExecutable, scriptArgs); - } -#else - qCWarning(lcEngine) << "**** Attention: POST_UPDATE_SCRIPT installed, but not executed because compiled with NDEBUG"; - #endif - } - - // do a database commit - _journal->commit(QStringLiteral("post treewalk")); - - _propagator = QSharedPointer( - new OwncloudPropagator(_account, _localPath, _remotePath, _journal, _bulkUploadBlackList)); - _propagator->setSyncOptions(_syncOptions); - connect(_propagator.data(), &OwncloudPropagator::itemCompleted, - this, &SyncEngine::slotItemCompleted); - connect(_propagator.data(), &OwncloudPropagator::progress, - this, &SyncEngine::slotProgress); - connect(_propagator.data(), &OwncloudPropagator::finished, this, &SyncEngine::slotPropagationFinished, Qt::QueuedConnection); - connect(_propagator.data(), &OwncloudPropagator::seenLockedFile, this, &SyncEngine::seenLockedFile); - connect(_propagator.data(), &OwncloudPropagator::touchedFile, this, &SyncEngine::slotAddTouchedFile); - connect(_propagator.data(), &OwncloudPropagator::insufficientLocalStorage, this, &SyncEngine::slotInsufficientLocalStorage); - connect(_propagator.data(), &OwncloudPropagator::insufficientRemoteStorage, this, &SyncEngine::slotInsufficientRemoteStorage); - connect(_propagator.data(), &OwncloudPropagator::newItem, this, &SyncEngine::slotNewItem); - - // apply the network limits to the propagator - setNetworkLimits(_uploadLimit, _downloadLimit); - - deleteStaleDownloadInfos(_syncItems); - deleteStaleUploadInfos(_syncItems); - deleteStaleErrorBlacklistEntries(_syncItems); - _journal->commit(QStringLiteral("post stale entry removal")); - - // Emit the started signal only after the propagator has been set up. - if (_needsUpdate) - Q_EMIT started(); - - _propagator->start(std::move(_syncItems)); - - qCInfo(lcEngine) << "#### Post-Reconcile end #################################################### " << _stopWatch.addLapTime(QStringLiteral("Post-Reconcile Finished")) << "ms"; - }; - const auto displayDialog = ConfigFile().promptDeleteFiles() && !_syncOptions.isCmd(); if (!_hasNoneFiles && _hasRemoveFile && displayDialog) { qCInfo(lcEngine) << "All the files are going to be changed, asking the user"; @@ -907,27 +811,14 @@ void SyncEngine::slotDiscoveryFinished() } } - QPointer guard = new QObject(); - QPointer self = this; - auto callback = [this, self, finish, guard](bool cancel) -> void { - // use a guard to ensure its only called once... - // qpointer to self to ensure we still exist - if (!guard || !self) { - return; - } - guard->deleteLater(); - if (cancel) { - qCInfo(lcEngine) << "User aborted sync"; - finalize(false); - return; - } else { - finish(); - } - }; - emit aboutToRemoveAllFiles(side >= 0 ? SyncFileItem::Down : SyncFileItem::Up, callback); + promptUserBeforePropagation([this, side](auto &&callback){ + emit aboutToRemoveAllFiles(side >= 0 ? SyncFileItem::Down : SyncFileItem::Up, callback); + }); return; } - finish(); + + + finishSync(); } void SyncEngine::slotCleanPollsJobAborted(const QString &error, const ErrorCategory errorCategory) @@ -1104,6 +995,131 @@ void SyncEngine::restoreOldFiles(SyncFileItemVector &syncItems) } } +void SyncEngine::cancelSyncOrContinue(bool cancel) +{ + if (cancel) { + qCInfo(lcEngine) << "User aborted sync"; + finalize(false); + } else { + finishSync(); + } +} + +void SyncEngine::finishSync() +{ + auto databaseFingerprint = _journal->dataFingerprint(); + // If databaseFingerprint is empty, this means that there was no information in the database + // (for example, upgrading from a previous version, or first sync, or server not supporting fingerprint) + if (!databaseFingerprint.isEmpty() && _discoveryPhase + && _discoveryPhase->_dataFingerprint != databaseFingerprint) { + qCInfo(lcEngine) << "data fingerprint changed, assume restore from backup" << databaseFingerprint << _discoveryPhase->_dataFingerprint; + restoreOldFiles(_syncItems); + } + + if (_discoveryPhase && _discoveryPhase->_anotherSyncNeeded && !_discoveryPhase->_filesNeedingScheduledSync.empty()) { + slotScheduleFilesDelayedSync(); + } else if (_discoveryPhase && _discoveryPhase->_anotherSyncNeeded && _anotherSyncNeeded == NoFollowUpSync) { + _anotherSyncNeeded = ImmediateFollowUp; + } + + if (_discoveryPhase && !_discoveryPhase->_filesUnscheduleSync.empty()) { + slotUnscheduleFilesDelayedSync(); + } + + if (_discoveryPhase && _discoveryPhase->_hasDownloadRemovedItems && _discoveryPhase->_hasUploadErrorItems) { + for (const auto &item : qAsConst(_syncItems)) { + if (item->_instruction == CSYNC_INSTRUCTION_ERROR && item->_direction == SyncFileItem::Up) { + // item->_instruction = CSYNC_INSTRUCTION_IGNORE; + } + } + _anotherSyncNeeded = ImmediateFollowUp; + } + + Q_ASSERT(std::is_sorted(_syncItems.begin(), _syncItems.end())); + + qCInfo(lcEngine) << "#### Reconcile (aboutToPropagate) #################################################### " << _stopWatch.addLapTime(QStringLiteral("Reconcile (aboutToPropagate)")) << "ms"; + + _localDiscoveryPaths.clear(); + + // To announce the beginning of the sync + emit aboutToPropagate(_syncItems); + + qCInfo(lcEngine) << "#### Reconcile (aboutToPropagate OK) #################################################### "<< _stopWatch.addLapTime(QStringLiteral("Reconcile (aboutToPropagate OK)")) << "ms"; + + // it's important to do this before ProgressInfo::start(), to announce start of new sync + _progressInfo->_status = ProgressInfo::Propagation; + emit transmissionProgress(*_progressInfo); + _progressInfo->startEstimateUpdates(); + + // post update phase script: allow to tweak stuff by a custom script in debug mode. + if (!qEnvironmentVariableIsEmpty("OWNCLOUD_POST_UPDATE_SCRIPT")) { +#ifndef NDEBUG + const QString script = qEnvironmentVariable("OWNCLOUD_POST_UPDATE_SCRIPT"); + + qCDebug(lcEngine) << "Post Update Script: " << script; + auto scriptArgs = script.split(QRegularExpression("\\s+"), Qt::SkipEmptyParts); + if (scriptArgs.size() > 0) { + const auto scriptExecutable = scriptArgs.takeFirst(); + QProcess::execute(scriptExecutable, scriptArgs); + } +#else + qCWarning(lcEngine) << "**** Attention: POST_UPDATE_SCRIPT installed, but not executed because compiled with NDEBUG"; +#endif + } + + // do a database commit + _journal->commit(QStringLiteral("post treewalk")); + + _propagator = QSharedPointer( + new OwncloudPropagator(_account, _localPath, _remotePath, _journal, _bulkUploadBlackList)); + _propagator->setSyncOptions(_syncOptions); + connect(_propagator.data(), &OwncloudPropagator::itemCompleted, + this, &SyncEngine::slotItemCompleted); + connect(_propagator.data(), &OwncloudPropagator::progress, + this, &SyncEngine::slotProgress); + connect(_propagator.data(), &OwncloudPropagator::finished, this, &SyncEngine::slotPropagationFinished, Qt::QueuedConnection); + connect(_propagator.data(), &OwncloudPropagator::seenLockedFile, this, &SyncEngine::seenLockedFile); + connect(_propagator.data(), &OwncloudPropagator::touchedFile, this, &SyncEngine::slotAddTouchedFile); + connect(_propagator.data(), &OwncloudPropagator::insufficientLocalStorage, this, &SyncEngine::slotInsufficientLocalStorage); + connect(_propagator.data(), &OwncloudPropagator::insufficientRemoteStorage, this, &SyncEngine::slotInsufficientRemoteStorage); + connect(_propagator.data(), &OwncloudPropagator::newItem, this, &SyncEngine::slotNewItem); + + // apply the network limits to the propagator + setNetworkLimits(_uploadLimit, _downloadLimit); + + deleteStaleDownloadInfos(_syncItems); + deleteStaleUploadInfos(_syncItems); + deleteStaleErrorBlacklistEntries(_syncItems); + _journal->commit(QStringLiteral("post stale entry removal")); + + // Emit the started signal only after the propagator has been set up. + if (_needsUpdate) + Q_EMIT started(); + + _propagator->start(std::move(_syncItems)); + + qCInfo(lcEngine) << "#### Post-Reconcile end #################################################### " << _stopWatch.addLapTime(QStringLiteral("Post-Reconcile Finished")) << "ms"; +} + +template +void SyncEngine::promptUserBeforePropagation(T &&lambda) +{ + QPointer guard = new QObject(); + QPointer self = this; + auto callback = [this, self, guard](bool cancel) -> void { + // use a guard to ensure its only called once... + // qpointer to self to ensure we still exist + if (!guard || !self) { + return; + } + guard->deleteLater(); + + cancelSyncOrContinue(cancel); + }; + + lambda(callback); +} + void SyncEngine::slotAddTouchedFile(const QString &fn) { QElapsedTimer now; diff --git a/src/libsync/syncengine.h b/src/libsync/syncengine.h index 2887c4bc1f658..278c5569dfd4b 100644 --- a/src/libsync/syncengine.h +++ b/src/libsync/syncengine.h @@ -361,6 +361,13 @@ private slots: */ void restoreOldFiles(SyncFileItemVector &syncItems); + void cancelSyncOrContinue(bool cancel); + + void finishSync(); + + template + void promptUserBeforePropagation(T &&lambda); + // true if there is at least one file which was not changed on the server bool _hasNoneFiles = false; From a118c0ad1ed7137e2f65254a18cdeeda3eb2b170 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Tue, 3 Sep 2024 18:07:57 +0200 Subject: [PATCH 3/5] 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 5c7620c6a7fa0..52baa342f13b6 100644 --- 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 &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 e3bed9f7f92e8..c79153a86758f 100644 --- 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 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 eb114f553d240..9be7992a5b728 100644 --- 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 void SyncEngine::promptUserBeforePropagation(T &&lambda) { diff --git a/src/libsync/syncengine.h b/src/libsync/syncengine.h index 278c5569dfd4b..2d824f3318d4e 100644 --- a/src/libsync/syncengine.h +++ b/src/libsync/syncengine.h @@ -189,6 +189,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); @@ -365,6 +369,8 @@ private slots: void finishSync(); + void handleRemnantReadOnlyFolders(); + template void promptUserBeforePropagation(T &&lambda); diff --git a/test/testpermissions.cpp b/test/testpermissions.cpp index ee1ddbcdb7a9d..f334b22de5abc 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; } @@ -87,6 +97,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. @@ -424,6 +442,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. @@ -543,6 +568,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. @@ -591,6 +624,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"); @@ -643,6 +685,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"); @@ -660,6 +710,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"); @@ -678,6 +736,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"); @@ -714,6 +780,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"); @@ -774,6 +848,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"); @@ -804,6 +886,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"); @@ -836,6 +926,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"); @@ -871,6 +969,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"); From 34c25ecb4306610723b99e709f0e510cc959c092 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Mon, 2 Sep 2024 13:53:11 +0200 Subject: [PATCH 4/5] let FileSystem::removeRecursively be able to delete read-only folders Signed-off-by: Matthieu Gallien --- src/libsync/filesystem.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/libsync/filesystem.cpp b/src/libsync/filesystem.cpp index c05b29795401b..7ff29ca4c1e85 100644 --- a/src/libsync/filesystem.cpp +++ b/src/libsync/filesystem.cpp @@ -273,6 +273,12 @@ bool FileSystem::removeRecursively(const QString &path, const std::function= MAC_OS_X_VERSION_10_15 + const auto fileInfo = QFileInfo{di.filePath()}; + const auto parentFolderPath = fileInfo.dir().absolutePath(); + const auto parentPermissionsHandler = FileSystem::FilePermissionsRestore{parentFolderPath, FileSystem::FolderPermissions::ReadWrite}; +#endif removeOk = FileSystem::remove(di.filePath(), &removeError); if (removeOk) { if (onDeleted) @@ -289,6 +295,12 @@ bool FileSystem::removeRecursively(const QString &path, const std::function= MAC_OS_X_VERSION_10_15 + const auto fileInfo = QFileInfo{path}; + const auto parentFolderPath = fileInfo.dir().absolutePath(); + const auto parentPermissionsHandler = FileSystem::FilePermissionsRestore{parentFolderPath, FileSystem::FolderPermissions::ReadWrite}; + FileSystem::setFolderPermissions(path, FileSystem::FolderPermissions::ReadWrite); +#endif allRemoved = QDir().rmdir(path); if (allRemoved) { if (onDeleted) From 8faefc6fa28d9a4bf956ec074cc6f9b60975eb88 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Mon, 2 Sep 2024 22:50:26 +0200 Subject: [PATCH 5/5] delete invalid folders and restart sync automatically users should then notice that the sync errors are gone Signed-off-by: Matthieu Gallien --- src/gui/folder.cpp | 35 ++++++++++++++--------------------- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index 52baa342f13b6..67de7b3874394 100644 --- a/src/gui/folder.cpp +++ b/src/gui/folder.cpp @@ -1670,28 +1670,21 @@ void Folder::slotNeedToRemoveRemnantsReadOnlyFolders(const QList 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); + auto listOfFolders = QStringList{}; + for (const auto &oneFolder : folders) { + listOfFolders.push_back(oneFolder->_file); + } + + qCInfo(lcFolder()) << "will delete invalid read-only folders:" << listOfFolders.join(", "); + 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(); + for(const auto &oneFolder : folders) { + FileSystem::removeRecursively(localPath + oneFolder->_file); + } + callback(true); + setSyncPaused(false); + _lastEtag.clear(); + slotScheduleThisFolder(); } void Folder::removeLocalE2eFiles()