Skip to content

Commit

Permalink
detect remnants read-only folders to delete: try to delete them
Browse files Browse the repository at this point in the history 
Signed-off-by: Matthieu Gallien <[email protected]>
  • Loading branch information
@mgallien @backportbot
mgallien authored and backportbot[bot] committed Sep 12, 2024
1 parent a0096cf commit bbb36e3
Show file tree
Hide file tree
Showing 5 changed files with 157 additions and 0 deletions.
30 changes: 30 additions & 0 deletions src/gui/folder.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -1653,6 +1655,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";
Expand Down
4 changes: 4 additions & 0 deletions src/gui/folder.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,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
*
Expand Down
11 changes: 11 additions & 0 deletions src/libsync/syncengine.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -822,6 +822,10 @@ void SyncEngine::slotDiscoveryFinished()
return;
}

if (!_remnantReadOnlyFolders.isEmpty()) {
handleRemnantReadOnlyFolders();
return;
}

finishSync();
}
Expand Down Expand Up @@ -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 <typename T>
void SyncEngine::promptUserBeforePropagation(T &&lambda)
{
Expand Down
6 changes: 6 additions & 0 deletions src/libsync/syncengine.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,10 @@ public slots:
*/
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);

Expand Down Expand Up @@ -360,6 +364,8 @@ private slots:

void finishSync();

void handleRemnantReadOnlyFolders();

template <typename T>
void promptUserBeforePropagation(T &&lambda);

Expand Down
106 changes: 106 additions & 0 deletions test/testpermissions.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -85,6 +95,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.
Expand Down Expand Up @@ -401,6 +419,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.
Expand Down Expand Up @@ -520,6 +545,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.
Expand Down Expand Up @@ -568,6 +601,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");
Expand Down Expand Up @@ -620,6 +662,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");
Expand All @@ -637,6 +687,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");
Expand All @@ -655,6 +713,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");
Expand Down Expand Up @@ -691,6 +757,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");
Expand Down Expand Up @@ -751,6 +825,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");
Expand Down Expand Up @@ -781,6 +863,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");
Expand Down Expand Up @@ -813,6 +903,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");
Expand Down Expand Up @@ -848,6 +946,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");
Expand Down

0 comments on commit bbb36e3

Please sign in to comment.