From 1f88052a8e6067c4191193d9dd95240296e70d70 Mon Sep 17 00:00:00 2001 From: Camila Ayres Date: Tue, 4 Jun 2024 20:30:32 +0200 Subject: [PATCH 1/5] Fix #3144: check for promptDeleteAllFiles config setting before emitting signal to display warning. The signal is not connected to any slot when running nextcloudcmd. The callback to finish the sync was never being called because the check for the config was done in the slot, which was never called. Signed-off-by: Camila Ayres --- src/gui/folder.cpp | 8 +------- src/libsync/syncengine.cpp | 4 ++-- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index aa755fdf1e6a0..66e194d16580b 100644 --- a/src/gui/folder.cpp +++ b/src/gui/folder.cpp @@ -1628,12 +1628,6 @@ bool Folder::virtualFilesEnabled() const void Folder::slotAboutToRemoveAllFiles(SyncFileItem::Direction dir, std::function callback) { - ConfigFile cfgFile; - if (!cfgFile.promptDeleteFiles()) { - callback(false); - return; - } - const QString msg = dir == SyncFileItem::Down ? tr("All files in the sync folder \"%1\" folder were deleted on the server.\n" "These deletes will be synchronized to your local sync folder, making such files " "unavailable unless you have a right to restore. \n" @@ -1644,7 +1638,7 @@ void Folder::slotAboutToRemoveAllFiles(SyncFileItem::Direction dir, std::functio "Are you sure you want to sync those actions with the server?\n" "If this was an accident and you decide to keep your files, they will be re-synced from the server."); auto msgBox = new QMessageBox(QMessageBox::Warning, tr("Remove All Files?"), - msg.arg(shortGuiLocalPath()), QMessageBox::NoButton); + msg.arg(shortGuiLocalPath()), QMessageBox::NoButton); msgBox->setAttribute(Qt::WA_DeleteOnClose); msgBox->setWindowFlags(msgBox->windowFlags() | Qt::WindowStaysOnTopHint); msgBox->addButton(tr("Remove all files"), QMessageBox::DestructiveRole); diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index 8f8dcecec31ca..2ce9966d889b4 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -898,10 +898,10 @@ void SyncEngine::slotDiscoveryFinished() qCInfo(lcEngine) << "#### Post-Reconcile end #################################################### " << _stopWatch.addLapTime(QStringLiteral("Post-Reconcile Finished")) << "ms"; }; - if (!_hasNoneFiles && _hasRemoveFile) { + if (!_hasNoneFiles && _hasRemoveFile && ConfigFile().promptDeleteFiles()) { qCInfo(lcEngine) << "All the files are going to be changed, asking the user"; int side = 0; // > 0 means more deleted on the server. < 0 means more deleted on the client - foreach (const auto &it, _syncItems) { + for (const auto &it : _syncItems) { if (it->_instruction == CSYNC_INSTRUCTION_REMOVE) { side += it->_direction == SyncFileItem::Down ? 1 : -1; } From b3ff87b73e959ec3a302173d6f5eaa358a1c86d4 Mon Sep 17 00:00:00 2001 From: Camila Ayres Date: Tue, 4 Jun 2024 20:33:47 +0200 Subject: [PATCH 2/5] Make warning text before deleting all files more clear and concise. Signed-off-by: Camila Ayres --- src/gui/folder.cpp | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index 66e194d16580b..5ac1e63e5d009 100644 --- a/src/gui/folder.cpp +++ b/src/gui/folder.cpp @@ -1628,21 +1628,14 @@ bool Folder::virtualFilesEnabled() const void Folder::slotAboutToRemoveAllFiles(SyncFileItem::Direction dir, std::function callback) { - const QString msg = dir == SyncFileItem::Down ? tr("All files in the sync folder \"%1\" folder were deleted on the server.\n" - "These deletes will be synchronized to your local sync folder, making such files " - "unavailable unless you have a right to restore. \n" - "If you decide to restore the files, they will be re-synced with the server if you have rights to do so.\n" - "If you decide to delete the files, they will be unavailable to you, unless you are the owner.") - : tr("All the files in your local sync folder \"%1\" were deleted. These deletes will be " - "synchronized with your server, making such files unavailable unless restored.\n" - "Are you sure you want to sync those actions with the server?\n" - "If this was an accident and you decide to keep your files, they will be re-synced from the server."); - auto msgBox = new QMessageBox(QMessageBox::Warning, tr("Remove All Files?"), - msg.arg(shortGuiLocalPath()), QMessageBox::NoButton); + const QString msg = dir == SyncFileItem::Down ? tr("All files in the server folder \"%1\" were deleted.\n\nIf you restore the files, they will be uploaded again to the server.") + : tr("All files in the local folder \"%1\" were deleted.\n\nIf you restore the files, they will be downloaded again from the server."); + auto msgBox = new QMessageBox(QMessageBox::Warning, tr("Remove all files?"), + msg.arg(shortGuiLocalPath()), QMessageBox::NoButton); msgBox->setAttribute(Qt::WA_DeleteOnClose); msgBox->setWindowFlags(msgBox->windowFlags() | Qt::WindowStaysOnTopHint); - msgBox->addButton(tr("Remove all files"), QMessageBox::DestructiveRole); - QPushButton *keepBtn = msgBox->addButton(tr("Keep files"), QMessageBox::AcceptRole); + msgBox->addButton(tr("Proceed to remove all files"), QMessageBox::DestructiveRole); + QPushButton *keepBtn = msgBox->addButton(tr("Restore files"), QMessageBox::AcceptRole); bool oldPaused = syncPaused(); setSyncPaused(true); connect(msgBox, &QMessageBox::finished, this, [msgBox, keepBtn, callback, oldPaused, this] { From d7c8365b05a3ba4c01c380efc9d6736e4ab268c2 Mon Sep 17 00:00:00 2001 From: Camila Ayres Date: Tue, 4 Jun 2024 20:40:44 +0200 Subject: [PATCH 3/5] Use readable variable names. Signed-off-by: Camila Ayres --- src/cmd/cmd.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/cmd/cmd.cpp b/src/cmd/cmd.cpp index 2764f7255d19f..2a1be09c701ce 100644 --- a/src/cmd/cmd.cpp +++ b/src/cmd/cmd.cpp @@ -529,10 +529,10 @@ int main(int argc, char **argv) selectiveSyncFixup(&db, selectiveSyncList); } - SyncOptions opt; - opt.fillFromEnvironmentVariables(); - opt.verifyChunkSizes(); - SyncEngine engine(account, options.source_dir, opt, folder, &db); + SyncOptions syncOptions; + syncOptions.fillFromEnvironmentVariables(); + syncOptions.verifyChunkSizes(); + SyncEngine engine(account, options.source_dir, syncOptions, folder, &db); engine.setIgnoreHiddenFiles(options.ignoreHiddenFiles); engine.setNetworkLimits(options.uplimit, options.downlimit); QObject::connect(&engine, &SyncEngine::finished, From 7b9b7bc3b69debbf6ab9e45c150cacd37903f7b9 Mon Sep 17 00:00:00 2001 From: Camila Ayres Date: Tue, 25 Jun 2024 20:39:09 +0200 Subject: [PATCH 4/5] Add sync option for nextcloudcmd. Signed-off-by: Camila Ayres --- src/cmd/cmd.cpp | 1 + src/libsync/syncengine.cpp | 3 ++- src/libsync/syncoptions.cpp | 11 +++++++++++ src/libsync/syncoptions.h | 7 ++++++- 4 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/cmd/cmd.cpp b/src/cmd/cmd.cpp index 2a1be09c701ce..3c940ee2e9adc 100644 --- a/src/cmd/cmd.cpp +++ b/src/cmd/cmd.cpp @@ -532,6 +532,7 @@ int main(int argc, char **argv) SyncOptions syncOptions; syncOptions.fillFromEnvironmentVariables(); syncOptions.verifyChunkSizes(); + syncOptions.setIsCmd(true); SyncEngine engine(account, options.source_dir, syncOptions, folder, &db); engine.setIgnoreHiddenFiles(options.ignoreHiddenFiles); engine.setNetworkLimits(options.uplimit, options.downlimit); diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index 2ce9966d889b4..3470504e2709f 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -898,7 +898,8 @@ void SyncEngine::slotDiscoveryFinished() qCInfo(lcEngine) << "#### Post-Reconcile end #################################################### " << _stopWatch.addLapTime(QStringLiteral("Post-Reconcile Finished")) << "ms"; }; - if (!_hasNoneFiles && _hasRemoveFile && ConfigFile().promptDeleteFiles()) { + const auto displayDialog = ConfigFile().promptDeleteFiles() && !_syncOptions.isCmd(); + if (!_hasNoneFiles && _hasRemoveFile && displayDialog) { qCInfo(lcEngine) << "All the files are going to be changed, asking the user"; int side = 0; // > 0 means more deleted on the server. < 0 means more deleted on the client for (const auto &it : _syncItems) { diff --git a/src/libsync/syncoptions.cpp b/src/libsync/syncoptions.cpp index c11d3a9bba2b1..1585317a859ea 100644 --- a/src/libsync/syncoptions.cpp +++ b/src/libsync/syncoptions.cpp @@ -21,6 +21,7 @@ using namespace OCC; SyncOptions::SyncOptions() : _vfs(new VfsOff) + , _isCmd(false) { } @@ -91,3 +92,13 @@ void SyncOptions::setPathPattern(const QString &pattern) _fileRegex.setPatternOptions(Utility::fsCasePreserving() ? QRegularExpression::CaseInsensitiveOption : QRegularExpression::NoPatternOption); _fileRegex.setPattern(pattern); } + +void SyncOptions::setIsCmd(const bool isCmd) +{ + _isCmd = isCmd; +} + +bool SyncOptions::isCmd() const +{ + return _isCmd; +} diff --git a/src/libsync/syncoptions.h b/src/libsync/syncoptions.h index 092ac6fb6f1c6..91ebf4f01531e 100644 --- a/src/libsync/syncoptions.h +++ b/src/libsync/syncoptions.h @@ -93,7 +93,6 @@ class OWNCLOUDSYNC_EXPORT SyncOptions */ void verifyChunkSizes(); - /** A regular expression to match file names * If no pattern is provided the default is an invalid regular expression. */ @@ -109,6 +108,10 @@ class OWNCLOUDSYNC_EXPORT SyncOptions */ void setPathPattern(const QString &pattern); + /** sync had been started via nextcloudcmd command line */ + [[nodiscard]] bool isCmd() const; + void setIsCmd(const bool isCmd); + private: /** * Only sync files that match the expression @@ -118,6 +121,8 @@ class OWNCLOUDSYNC_EXPORT SyncOptions qint64 _minChunkSize = chunkV2MinChunkSize; qint64 _maxChunkSize = chunkV2MaxChunkSize; + + bool _isCmd = false; }; } From 92390cc754f819e27228a6eb0c6fecbb9c2fab7c Mon Sep 17 00:00:00 2001 From: Camila Ayres Date: Tue, 25 Jun 2024 22:12:14 +0200 Subject: [PATCH 5/5] Add tests to remove all files when sycing with or without nextcloudcmd. Signed-off-by: Camila Ayres --- test/testsyncengine.cpp | 69 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/test/testsyncengine.cpp b/test/testsyncengine.cpp index 1fc3345445376..e9aa0d41cb8d8 100644 --- a/test/testsyncengine.cpp +++ b/test/testsyncengine.cpp @@ -1910,6 +1910,75 @@ private slots: QVERIFY(fakeFolder.syncOnce()); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); } + + void testRemoveAllFilesWithNextcloudCmd() + { + FakeFolder fakeFolder{FileInfo{}}; + auto nextcloudCmdSyncOptions = fakeFolder.syncEngine().syncOptions(); + nextcloudCmdSyncOptions.setIsCmd(true); + fakeFolder.syncEngine().setSyncOptions(nextcloudCmdSyncOptions); + ConfigFile().setPromptDeleteFiles(true); + QSignalSpy displayDialogSignal(&fakeFolder.syncEngine(), &SyncEngine::aboutToRemoveAllFiles); + + fakeFolder.remoteModifier().mkdir("folder"); + fakeFolder.remoteModifier().insert("folder/file1"); + fakeFolder.remoteModifier().insert("folder/file2"); + fakeFolder.remoteModifier().insert("folder/file3"); + fakeFolder.remoteModifier().mkdir("folder2"); + fakeFolder.remoteModifier().insert("file1"); + fakeFolder.remoteModifier().insert("file2"); + fakeFolder.remoteModifier().insert("file3"); + + QVERIFY(fakeFolder.syncOnce()); + + fakeFolder.remoteModifier().remove("folder"); + fakeFolder.remoteModifier().remove("folder2"); + fakeFolder.remoteModifier().remove("file1"); + fakeFolder.remoteModifier().remove("file2"); + fakeFolder.remoteModifier().remove("file3"); + + QVERIFY(fakeFolder.syncOnce()); + // the signal to display the dialog should not be emitted + QCOMPARE(displayDialogSignal.count(), 0); + QCOMPARE(fakeFolder.remoteModifier().find("folder"), nullptr); + QCOMPARE(fakeFolder.remoteModifier().find("folder2"), nullptr); + QCOMPARE(fakeFolder.remoteModifier().find("file1"), nullptr); + } + + void testRemoveAllFilesWithoutNextcloudCmd() + { + FakeFolder fakeFolder{FileInfo{}}; + auto nextcloudCmdSyncOptions = fakeFolder.syncEngine().syncOptions(); + nextcloudCmdSyncOptions.setIsCmd(false); + fakeFolder.syncEngine().setSyncOptions(nextcloudCmdSyncOptions); + ConfigFile().setPromptDeleteFiles(true); + QSignalSpy displayDialogSignal(&fakeFolder.syncEngine(), &SyncEngine::aboutToRemoveAllFiles); + + fakeFolder.remoteModifier().mkdir("folder"); + fakeFolder.remoteModifier().insert("folder/file1"); + fakeFolder.remoteModifier().insert("folder/file2"); + fakeFolder.remoteModifier().insert("folder/file3"); + fakeFolder.remoteModifier().mkdir("folder2"); + fakeFolder.remoteModifier().insert("file1"); + fakeFolder.remoteModifier().insert("file2"); + fakeFolder.remoteModifier().insert("file3"); + + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + + fakeFolder.remoteModifier().remove("folder"); + fakeFolder.remoteModifier().remove("folder2"); + fakeFolder.remoteModifier().remove("file1"); + fakeFolder.remoteModifier().remove("file2"); + fakeFolder.remoteModifier().remove("file3"); + + QVERIFY(fakeFolder.syncOnce()); + // the signal to show the dialog should be emitted + QCOMPARE(displayDialogSignal.count(), 1); + QCOMPARE(fakeFolder.remoteModifier().find("folder"), nullptr); + QCOMPARE(fakeFolder.remoteModifier().find("folder2"), nullptr); + QCOMPARE(fakeFolder.remoteModifier().find("file1"), nullptr); + } }; QTEST_GUILESS_MAIN(TestSyncEngine)