Skip to content

Commit 18bfc50

Browse files
committed
fix: sync engine readonly folder file deletion
This is for #7797 and #10099 Signed-off-by: Markus Goetz <markus@woboq.com> Assisted-by: ClaudeCode:claude-opus-4-8
1 parent a80ec7c commit 18bfc50

4 files changed

Lines changed: 25 additions & 8 deletions

File tree

src/gui/tray/usermodel.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1390,6 +1390,11 @@ void User::processCompletedSyncItem(const Folder *folder, const SyncFileItemPtr
13901390
activity._links = {buttonActivityLink};
13911391
}
13921392
_activityModel->addErrorToActivityList(activity, ActivityListModel::ErrorType::SyncError);
1393+
} else if (!item->_errorString.isEmpty()) {
1394+
// The item was ignored for a concrete reason (e.g. it lives in a read-only
1395+
// folder and cannot be uploaded). Surface it passively in the Not-synced list
1396+
// so the user can see why it did not sync.
1397+
_activityModel->addErrorToActivityList(activity, ActivityListModel::ErrorType::SyncError);
13931398
}
13941399
}
13951400
}

src/libsync/discovery.cpp

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1637,6 +1637,14 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
16371637
return;
16381638
}
16391639

1640+
// The move target is in a read-only folder so it can't be uploaded, but its data is safe
1641+
// at the source (restored below). Emitting remnantReadOnlyFolderDiscovered() schedules this
1642+
// local copy for deletion at the end of the sync, removing the duplicate. A genuinely new
1643+
// local file never reaches this move branch and is kept by checkPermissions (#7797/#10099).
1644+
if (!localEntry.isVirtualFile && item->_instruction == CSYNC_INSTRUCTION_IGNORE) {
1645+
emit _discoveryData->remnantReadOnlyFolderDiscovered(item);
1646+
}
1647+
16401648
// Here we know the new location can't be uploaded: must prevent the source delete.
16411649
// Two cases: either the source item was already processed or not.
16421650
auto wasDeletedOnClient = _discoveryData->findAndCancelDeletedJob(originalPath);
@@ -2017,13 +2025,15 @@ bool ProcessDirectoryJob::checkPermissions(const OCC::SyncFileItemPtr &item)
20172025
qCWarning(lcDisco) << "checkForPermission: Not allowed because you don't have permission to add subfolders to that folder:" << item->_file;
20182026
item->_instruction = CSYNC_INSTRUCTION_IGNORE;
20192027
item->_errorString = tr("Not allowed because you don't have permission to add subfolders to that folder");
2020-
emit _discoveryData->remnantReadOnlyFolderDiscovered(item);
2028+
// Do NOT schedule the new local folder for deletion: it may contain data the user just
2029+
// created and that exists nowhere else. (see read-only folder regression, issues #7797/#10099).
20212030
return false;
20222031
} else if (!item->isDirectory() && !perms.hasPermission(RemotePermissions::CanAddFile)) {
20232032
qCWarning(lcDisco) << "checkForPermission: Not allowed because you don't have permission to add files in that folder:" << item->_file;
20242033
item->_instruction = CSYNC_INSTRUCTION_IGNORE;
20252034
item->_errorString = tr("Not allowed because you don't have permission to add files in that folder");
2026-
emit _discoveryData->remnantReadOnlyFolderDiscovered(item);
2035+
// Do NOT schedule the new local file for deletion: it may be data the user just created
2036+
// and that exists nowhere else. (see read-only folder regression, issues #7797/#10099).
20272037
return false;
20282038
}
20292039
break;

test/CMakeLists.txt

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,7 @@ nextcloud_add_test(Blacklist)
7878
nextcloud_add_test(LocalDiscovery)
7979
nextcloud_add_test(RemoteDiscovery)
8080

81-
if (NOT APPLE)
82-
nextcloud_add_test(Permissions)
83-
endif()
81+
nextcloud_add_test(Permissions)
8482

8583
nextcloud_add_test(SelectiveSync)
8684
nextcloud_add_test(DatabaseError)

test/testpermissions.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -297,9 +297,12 @@ private slots:
297297
currentLocalState = fakeFolder.currentLocalState();
298298

299299
//6.
300-
// The file should not exist on the remote, and not be there
301-
QVERIFY(!currentLocalState.find("readonlyDirectory_PERM_MG_/newFile_PERM_GWDNV_.data"));
300+
// The file cannot be uploaded to the read-only folder, but it MUST be kept locally:
301+
// it is local-only data and silently deleting it would lose it (issues #7797/#10099).
302+
QVERIFY(currentLocalState.find("readonlyDirectory_PERM_MG_/newFile_PERM_GWDNV_.data"));
302303
QVERIFY(!fakeFolder.currentRemoteState().find("readonlyDirectory_PERM_MG_/newFile_PERM_GWDNV_.data"));
304+
// Remove the kept local-only file so the following sections can compare local and remote.
305+
removeReadOnly("readonlyDirectory_PERM_MG_/newFile_PERM_GWDNV_.data");
303306
// Both side should still be the same
304307
QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
305308

@@ -379,7 +382,8 @@ private slots:
379382
QVERIFY(currentLocalState.find("readonlyDirectory_PERM_MG_/subdir_PERM_CKG_" ));
380383
// including contents
381384
QVERIFY(currentLocalState.find("readonlyDirectory_PERM_MG_/subdir_PERM_CKG_/subsubdir_PERM_CKDNVG_/normalFile_PERM_GWVND_.data" ));
382-
// new no longer exists
385+
// new no longer exists: it was a move of an existing (server-side) item into a read-only
386+
// folder, so the local copy is cleaned up while its data is restored at the source above.
383387
QVERIFY(!currentLocalState.find("readonlyDirectory_PERM_MG_/newname_PERM_CK_/subsubdir_PERM_CKDNVG_/normalFile_PERM_GWVND_.data" ));
384388
// but is not on server: should have been locally removed
385389
QVERIFY(!currentLocalState.find("readonlyDirectory_PERM_MG_/newname_PERM_CK_"));

0 commit comments

Comments
 (0)