nextcloud / desktop

💻 Desktop sync client for Nextcloud
https://nextcloud.com/install/#install-clients
GNU General Public License v2.0
2.97k stars 781 forks source link

[Bug]: client triggered deletions - partly-synced nested directories delete also directories on the server that are not configured to sync #6948

Open vacy opened 1 month ago

vacy commented 1 month ago

⚠️ Before submitting, please verify the following: ⚠️

Bug description

I sometimes had a suspision that nextcloud sync feels a little flaky but i way never able to put my finger on it until now. lets assume we have the folder structure below.

It looks like nextcloud desktop client is removing hidden not synced files and directories on the server that the client has not seen.

Steps to reproduce

prepare structure

Entries with

* are selected to be synced
x is synced because parent folder selected
- is not configured to sync AND not seen by the user

bash: mkdir -p dir-something/dir-foo/dir-subfoo dir-something/dir-bar/dir-subbar

dir-something/
        -- dir-bar
           -- file-bar
           -- dir-subbar
        ** dir-foo
           x file-foo
           -- dir-subfoo

folder-config

sync

So the client goes and syncs as configured in the structure above

two examples

1st way: delete "dir-something"

because you cleaned up stuff and just want to remove dir-something... easy right?

but it also deletes the not synced directories on the server

        |-- dir-bar
            -- file-bar
            -- dir-subbar
            -- dir-subfoo

dir-something/ -- dir-bar -- file-bar -- dir-subbar ** dir-foo x file-foo -- dir-subfoo

While you only expected to remove:

dir-something/
        `** dir-foo
            `x file-foo

2nd way: delete "dir-foo"

because you cleaned up stuff and just wanted to remove dir-foo... easy right?

but it also deletes the not synced directories on the server

           -- dir-subfoo

While you only expected to remove:

        ** dir-foo
           x file-foo

Expected behavior

I would expect the desktop client to only delete files and folders that a seen by the user. In my opinion it needs a workaround to have the nextcloud client handle "unknown non-empty directories" to remove them from the sync config of the client.

But the data needs to stay on the server. does it?

Which files are affected by this bug

procedure provided

Operating system

Linux

Which version of the operating system you are running.

fedora

Package

Distro package manager

Nextcloud Server version

28.0.8

Nextcloud Desktop Client version

3.13.2

Is this bug present after an update or on a fresh install?

Fresh desktop client install

Are you using the Nextcloud Server Encryption module?

Encryption is Disabled

Are you using an external user-backend?

vacy commented 1 month ago

started a discussion here: https://help.nextcloud.com/t/desktop-client-triggered-deletions-are-deleting-not-synced-data/199356

also drafted a workflow to fix this.

serverFolders = fetchServerFolders($deletePath)
bool keepParentFolder=false
for each in serverFolders
do
   if (isSynced($each)) { # folder synced
        delete($each) # deletes local and on server

  }
  if (not isSynced($each) { # folder not synced
        keepParentFolder=true
  }
done

# finally time to decide about parentFolder
if ($keepParentFolder == true) {
   removeParentFolder(unsubscribe)
}

if ($keepParentFolder == false) {
   removeParentFolder(delete)
}
mgallien commented 2 weeks ago

I think there is a misunderstanding about what deleting a folder means.

I understand that this may be confusing in some way but if you delete dir-something, you are telling Nextcloud that you want to delete it. The same would happen in the other direction if you had deleted on the server. Sure on the server you are supposed to see the whole directory listing and on the client that may be different when using selective sync.

What if the user really wants to delete the whole dir-something ? Should we bother him with a sync warning and letting him say: "Yes I really want to delete the folder I deleted".

Ultimately, I have no strong opinion to oppose to any change there. I just fear that the expectations between users may vary and that one solution may be good for some and bad for other people.