Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CollectionChangeSet.collection_was_cleared incorrect for dictionaries #6205

Closed
nirinchev opened this issue Jan 16, 2023 · 4 comments · Fixed by #6254
Closed

CollectionChangeSet.collection_was_cleared incorrect for dictionaries #6205

nirinchev opened this issue Jan 16, 2023 · 4 comments · Fixed by #6254
Assignees
Labels

Comments

@nirinchev
Copy link
Member

nirinchev commented Jan 16, 2023

In the .NET SDK we have a test calling Dictionary::remove_all and then verifying that the resulting notification has marked the collection as cleared (CollectionChangeSet.collection_was_cleared should be true). This works for lists and sets but not dictionaries.

@sync-by-unito
Copy link

sync-by-unito bot commented Jan 18, 2023

➤ ironage commented:

[~[email protected]] is this blocking you?

We emulate dictionary clear by removing all elements one by one because we didn't add an explicit clear instruction for notifications https://github.com/realm/realm-core/blob/master/src/realm/dictionary.cpp#L721

@nirinchev
Copy link
Member Author

Not blocking per se (we can always disable that assertion from the tests), just found it inconsistent and figured I'll file a ticket.

@ironage
Copy link
Contributor

ironage commented Jan 18, 2023

Fixing this inconsistency involves actually emitting the correct clear instruction to the replication system for both notifications and sync. This implies a change in behaviour for niche sync cases when resolving conflicting clear/add (instead of remove/add) but this could be considered a "fix" instead of a breaking change because that is the expected collection behaviour. However, because both the client and the server's OT semantics for clearing a dictionary are already in place, we should be able to just add this without bumping the sync protocol version.

@nicola-cab
Copy link
Member

nicola-cab commented Feb 21, 2023

When the next-major branch is merged, this issue will go away.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants