Skip to content

Commit b3836a7

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 8baff2e commit b3836a7

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
@@ -1631,6 +1631,14 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
16311631
return;
16321632
}
16331633

1634+
// The move target is in a read-only folder so it can't be uploaded, but its data is safe
1635+
// at the source (restored below). Emitting remnantReadOnlyFolderDiscovered() schedules this
1636+
// local copy for deletion at the end of the sync, removing the duplicate. A genuinely new
1637+
// local file never reaches this move branch and is kept by checkPermissions (#7797/#10099).
1638+
if (!localEntry.isVirtualFile && item->_instruction == CSYNC_INSTRUCTION_IGNORE) {
1639+
emit _discoveryData->remnantReadOnlyFolderDiscovered(item);
1640+
}
1641+
16341642
// Here we know the new location can't be uploaded: must prevent the source delete.
16351643
// Two cases: either the source item was already processed or not.
16361644
auto wasDeletedOnClient = _discoveryData->findAndCancelDeletedJob(originalPath);
@@ -2011,13 +2019,15 @@ bool ProcessDirectoryJob::checkPermissions(const OCC::SyncFileItemPtr &item)
20112019
qCWarning(lcDisco) << "checkForPermission: Not allowed because you don't have permission to add subfolders to that folder:" << item->_file;
20122020
item->_instruction = CSYNC_INSTRUCTION_IGNORE;
20132021
item->_errorString = tr("Not allowed because you don't have permission to add subfolders to that folder");
2014-
emit _discoveryData->remnantReadOnlyFolderDiscovered(item);
2022+
// Do NOT schedule the new local folder for deletion: it may contain data the user just
2023+
// created and that exists nowhere else. (see read-only folder regression, issues #7797/#10099).
20152024
return false;
20162025
} else if (!item->isDirectory() && !perms.hasPermission(RemotePermissions::CanAddFile)) {
20172026
qCWarning(lcDisco) << "checkForPermission: Not allowed because you don't have permission to add files in that folder:" << item->_file;
20182027
item->_instruction = CSYNC_INSTRUCTION_IGNORE;
20192028
item->_errorString = tr("Not allowed because you don't have permission to add files in that folder");
2020-
emit _discoveryData->remnantReadOnlyFolderDiscovered(item);
2029+
// Do NOT schedule the new local file for deletion: it may be data the user just created
2030+
// and that exists nowhere else. (see read-only folder regression, issues #7797/#10099).
20212031
return false;
20222032
}
20232033
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)