Handle folder removals on sync key resend
ClosedPublic
Actions

Authored by mollekopf on Apr 30 2025, 2:56 PM.

Project Tags

None

Referenced Files

	F117373962: D5223.1774819179.diff
	Sat, Mar 28, 9:19 PM

Subscribers

machniak

Details

Reviewers

machniak

Group Reviewers

Syncroton Developers

Commits

rSa0b1473db15d: Handle folder removals on sync key resend

Summary

We used to not re-report a folder removal on sync key resend,
which could potentially be a source of an endless sync loop:

Clients misses folder removal
Client sends sync request with removed folder, triggering a hierarchy changed status
Client issues a FolderSync with latest known sync key, which no longer reports the folder removal.

We now mark the folder as deleted instead, so we can report the change
again.

Diff Detail

Repository

rS syncroton

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

mollekopf requested review of this revision.Apr 30 2025, 2:56 PM

mollekopf created this revision.

Harbormaster completed remote builds in B53385: Diff 15024.Apr 30 2025, 2:56 PM

I'm aware that we're not actually taking the is_deleted flag into account anywhere, but I haven't found a place where it matters yet.

Wouldn't it be easier to remember which folder is missing in Sync/Ping (e.g. using cache) and then in FolderSync treat it as deleted?

In D5223#66996, @machniak wrote:

Wouldn't it be easier to remember which folder is missing in Sync/Ping (e.g. using cache) and then in FolderSync treat it as deleted?

That would seem worse to me because it relies on a Sync/Ping in the middle for a correct FolderSync response, when FolderSync on it's own should just provide the correct response.
I suppose in principle we could store the removed id's as part of the SyncKey data, and then make sure we always include that if a synckey is resent, but the current solution seems relatively simple to me (and it's the same approach that we use already for syncroton_content).