mirrors/nextcloud-desktop
3
0
Fork 0
mirror of https://github.com/nextcloud/desktop.git synced 2025-10-26 11:17:43 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: folder delete/new conflict will be "delete"

Browse Source 
should enable someone to delete a folder even if there is a new/delete
conflict happening for some other users

Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>
This commit is contained in:
Matthieu Gallien 2025-08-26 18:26:24 +02:00
parent 491e7cd55c
commit 1ec98ee46e
6 changed files with 100 additions and 74 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
src/libsync/discovery.cpp
View File

@ -913,6 +913,14 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo(const SyncFileItemPtr &it
// Unknown in db: new file on the server
Q_ASSERT(!dbEntry.isValid());
if (_discoveryData->recursiveCheckForDeletedParents(item->_file)) {
qCWarning(lcDisco) << "Removing local file inside a remotely deleted folder" << item->_file;
item->_instruction = CSYNC_INSTRUCTION_REMOVE;
item->_direction = SyncFileItem::Down;
emit _discoveryData->itemDiscovered(item);
return;
}
item->_instruction = CSYNC_INSTRUCTION_NEW;
item->_direction = SyncFileItem::Down;
item->_modtime = serverEntry.modtime;
@ -1425,6 +1433,14 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
return;
}
if (_discoveryData->recursiveCheckForDeletedParents(item->_file)) {
qCWarning(lcDisco) << "Removing local file inside a remotely deleted folder" << item->_file;
item->_instruction = CSYNC_INSTRUCTION_REMOVE;
item->_direction = SyncFileItem::Down;
emit _discoveryData->itemDiscovered(item);
return;
}
// New local file or rename
item->_instruction = CSYNC_INSTRUCTION_NEW;
item->_direction = SyncFileItem::Up;
@ -2155,25 +2171,6 @@ int ProcessDirectoryJob::processSubJobs(int nbJobs)
if (_queuedJobs.empty() && _runningJobs.empty() && _pendingAsyncJobs == 0) {
_pendingAsyncJobs = -1; // We're finished, we don't want to emit finished again
if (_dirItem) {
if (_childModified && _dirItem->_instruction == CSYNC_INSTRUCTION_REMOVE) {
// re-create directory that has modified contents
_dirItem->_instruction = CSYNC_INSTRUCTION_NEW;
const auto perms = !_rootPermissions.isNull() ? _rootPermissions
: _dirParentItem ? _dirParentItem->_remotePerm : _rootPermissions;
if (perms.isNull()) {
// No permissions set
} else if (_dirItem->isDirectory() && !perms.hasPermission(RemotePermissions::CanAddSubDirectories)) {
qCWarning(lcDisco) << "checkForPermission: Not allowed because you don't have permission to add subfolders to that folder: " << _dirItem->_file;
_dirItem->_instruction = CSYNC_INSTRUCTION_IGNORE;
_dirItem->_errorString = tr("Not allowed because you don't have permission to add subfolders to that folder");
const auto localPath = QString{_discoveryData->_localDir + _dirItem->_file};
emit _discoveryData->remnantReadOnlyFolderDiscovered(_dirItem);
}
_dirItem->_direction = _dirItem->_direction == SyncFileItem::Up ? SyncFileItem::Down : SyncFileItem::Up;
}
if (_childModified && _dirItem->_instruction == CSYNC_INSTRUCTION_TYPE_CHANGE && !_dirItem->isDirectory()) {
// Replacing a directory by a file is a conflict, if the directory had modified children
_dirItem->_instruction = CSYNC_INSTRUCTION_CONFLICT;

28
src/libsync/discoveryphase.cpp
View File

@ -24,6 +24,7 @@
#include <QLoggingCategory>
#include <QUrl>
#include <QFile>
#include <QDir>
#include <QFileInfo>
#include <QTextCodec>
#include <cstring>
@ -220,6 +221,33 @@ void DiscoveryPhase::enqueueDirectoryToDelete(const QString &path, ProcessDirect
}
}
bool DiscoveryPhase::recursiveCheckForDeletedParents(const QString &itemPath) const
{
const auto &allKeys = _deletedItem.keys();
qCDebug(lcDiscovery()) << allKeys.join(", ");
auto result = false;
const auto &pathElements = itemPath.split('/');
auto currentParentFolder = QString{};
for (const auto &onePathComponent : pathElements) {
if (!currentParentFolder.isEmpty()) {
currentParentFolder += '/';
}
currentParentFolder += onePathComponent;
qCDebug(lcDiscovery()) << "checks" << currentParentFolder << "for" << allKeys.join(", ");
if (_deletedItem.find(currentParentFolder) == _deletedItem.end()) {
continue;
}
qCDebug(lcDiscovery()) << "deleted parent found";
result = true;
break;
}
return result;
}
void DiscoveryPhase::markPermanentDeletionRequests()
{
// since we don't know in advance which files/directories need to be permanently deleted,

2
src/libsync/discoveryphase.h
View File

@ -272,6 +272,8 @@ class DiscoveryPhase : public QObject
void enqueueDirectoryToDelete(const QString &path, ProcessDirectoryJob* const directoryJob);
bool recursiveCheckForDeletedParents(const QString &itemPath) const;
/// contains files/folder names that are requested to be deleted permanently
QSet<QString> _permanentDeletionRequests;

27
test/testsynccfapi.cpp
View File

@ -1553,6 +1553,33 @@ private slots:
QVERIFY(fakeFolder.syncOnce());
QCOMPARE(fakeFolder.currentRemoteState(), fakeFolder.currentRemoteState());
}
void testSyncFolderNewDeleteConflictExpectDeletion()
{
FakeFolder fakeFolder{FileInfo{}};
setupVfs(fakeFolder);
fakeFolder.remoteModifier().mkdir("directory");
fakeFolder.remoteModifier().mkdir("directory/subdir");
fakeFolder.remoteModifier().insert("directory/file1");
fakeFolder.remoteModifier().insert("directory/file2");
fakeFolder.remoteModifier().insert("directory/file3");
fakeFolder.remoteModifier().insert("directory/subdir/fileTxt1.txt");
fakeFolder.remoteModifier().insert("directory/subdir/fileTxt2.txt");
fakeFolder.remoteModifier().insert("directory/subdir/fileTxt3.txt");
// perform an initial sync to ensure local and remote have the same state
QVERIFY(fakeFolder.syncOnce());
fakeFolder.remoteModifier().remove("directory");
fakeFolder.localModifier().mkdir("directory/subFolder");
fakeFolder.localModifier().insert("directory/file4");
fakeFolder.localModifier().insert("directory/subdir/fileTxt4.txt");
QVERIFY(fakeFolder.syncOnce());
const auto directoryItem = fakeFolder.remoteModifier().find("directory");
QCOMPARE(directoryItem, nullptr);
}
};
QTEST_GUILESS_MAIN(TestSyncCfApi)

4
test/testsyncdelete.cpp
View File

@ -43,11 +43,11 @@ private slots:
// A/a1 must be gone because the directory was removed on the server, but hello.txt must be there
QVERIFY(!fakeFolder.currentRemoteState().find("A/a1"));
QVERIFY(fakeFolder.currentRemoteState().find("A/hello.txt"));
QVERIFY(!fakeFolder.currentRemoteState().find("A/hello.txt"));
// Symmetry
QVERIFY(!fakeFolder.currentRemoteState().find("B/b1"));
QVERIFY(fakeFolder.currentRemoteState().find("B/hello.txt"));
QVERIFY(!fakeFolder.currentRemoteState().find("B/hello.txt"));
QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
}

78
test/testsyncengine.cpp
View File

@ -259,59 +259,6 @@ private slots:
QTest::newRow("delete") << false;
}
void testLocalDeleteWithReuploadForNewLocalFiles()
{
QFETCH(bool, moveToTrashEnabled);
FakeFolder fakeFolder{FileInfo{}};
auto syncOptions = fakeFolder.syncEngine().syncOptions();
syncOptions._moveFilesToTrash = moveToTrashEnabled;
fakeFolder.syncEngine().setSyncOptions(syncOptions);
// create folders hierarchy with some nested dirs and files
fakeFolder.localModifier().mkdir("A");
fakeFolder.localModifier().insert("A/existingfile_A.txt", 100);
fakeFolder.localModifier().mkdir("A/B");
fakeFolder.localModifier().insert("A/B/existingfile_B.data", 100);
fakeFolder.localModifier().mkdir("A/B/C");
fakeFolder.localModifier().mkdir("A/B/C/c1");
fakeFolder.localModifier().mkdir("A/B/C/c1/c2");
fakeFolder.localModifier().insert("A/B/C/c1/c2/existingfile_C2.md", 100);
QVERIFY(fakeFolder.syncOnce());
// make sure everything is uploaded
QVERIFY(fakeFolder.currentRemoteState().find("A/B/C/c1/c2"));
QVERIFY(fakeFolder.currentRemoteState().find("A/existingfile_A.txt"));
QVERIFY(fakeFolder.currentRemoteState().find("A/B/existingfile_B.data"));
QVERIFY(fakeFolder.currentRemoteState().find("A/B/C/c1/c2/existingfile_C2.md"));
QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
// remove a folder "A" on the server
fakeFolder.remoteModifier().remove("A");
// put new files and folders into a local folder "A"
fakeFolder.localModifier().insert("A/B/C/c1/c2/newfile.txt", 100);
fakeFolder.localModifier().insert("A/B/C/c1/c2/Readme.data", 100);
fakeFolder.localModifier().mkdir("A/B/C/c1/c2/newfiles");
fakeFolder.localModifier().insert("A/B/C/c1/c2/newfiles/newfile.txt", 100);
fakeFolder.localModifier().insert("A/B/C/c1/c2/newfiles/Readme.data", 100);
QVERIFY(fakeFolder.syncOnce());
// make sure new files and folders are uploaded (restored)
QVERIFY(fakeFolder.currentLocalState().find("A/B/C/c1/c2"));
QVERIFY(fakeFolder.currentLocalState().find("A/B/C/c1/c2/Readme.data"));
QVERIFY(fakeFolder.currentLocalState().find("A/B/C/c1/c2/newfiles/newfile.txt"));
QVERIFY(fakeFolder.currentLocalState().find("A/B/C/c1/c2/newfiles/Readme.data"));
// and the old files are removed
QVERIFY(!fakeFolder.currentLocalState().find("A/existingfile_A.txt"));
QVERIFY(!fakeFolder.currentLocalState().find("A/B/existingfile_B.data"));
QVERIFY(!fakeFolder.currentLocalState().find("A/B/C/c1/c2/existingfile_C2.md"));
QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
}
void testEmlLocalChecksum() {
FakeFolder fakeFolder{FileInfo{}};
fakeFolder.localModifier().insert("a1.eml", 64, 'A');
@ -2489,6 +2436,31 @@ private slots:
// last sync without changes, expect no files to be touched
syncAndExpectNoTouchedFiles();
}
void testSyncFolderNewDeleteConflictExpectDeletion()
{
FakeFolder fakeFolder{FileInfo{}};
fakeFolder.remoteModifier().mkdir("directory");
fakeFolder.remoteModifier().mkdir("directory/subdir");
fakeFolder.remoteModifier().insert("directory/file1");
fakeFolder.remoteModifier().insert("directory/file2");
fakeFolder.remoteModifier().insert("directory/file3");
fakeFolder.remoteModifier().insert("directory/subdir/fileTxt1.txt");
fakeFolder.remoteModifier().insert("directory/subdir/fileTxt2.txt");
fakeFolder.remoteModifier().insert("directory/subdir/fileTxt3.txt");
// perform an initial sync to ensure local and remote have the same state
QVERIFY(fakeFolder.syncOnce());
fakeFolder.remoteModifier().remove("directory");
fakeFolder.localModifier().mkdir("directory/subFolder");
fakeFolder.localModifier().insert("directory/file4");
fakeFolder.localModifier().insert("directory/subdir/fileTxt4.txt");
QVERIFY(fakeFolder.syncOnce());
const auto directoryItem = fakeFolder.remoteModifier().find("directory");
QCOMPARE(directoryItem, nullptr);
}
};
QTEST_GUILESS_MAIN(TestSyncEngine)