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

[SlidingSync] unique room identifiers #1977

Closed
Tracked by #1911
stefanceriu opened this issue May 26, 2023 · 4 comments
Closed
Tracked by #1911

[SlidingSync] unique room identifiers #1977

stefanceriu opened this issue May 26, 2023 · 4 comments
Assignees

Comments

@stefanceriu
Copy link
Member

Given certain sliding sync operations we can end up with multiple "invalid" rooms with the same identifier. This is a problem for SwiftUI that expects all items to be unique. We're currently trying to work around this swift side by trying to assign custom identifiers to whatever rooms are duplicated. The downside is that SwiftUI won't be able to infer differences in the room and instead just replace the whole cell.

It would be great if the new RoomList API can take this into consideration

@Hywan Hywan changed the title RoomList API: unique room identifiers [SlidingSync] unique room identifiers May 26, 2023
@Hywan
Copy link
Member

Hywan commented May 26, 2023

Thank for the issue,

How do you get invalidated rooms? When a room is marked as Filled, it has a room ID. When a room is marked as Invalidated, the same room ID is used. So it means that, at some points, you already have Filled rooms with duplicated IDs. Can you confirm that?

@stefanceriu
Copy link
Member Author

I cannot confirm or deny that we get duplicated filled rooms. We get duplicates atm but I can't tell whether it's because we merge together 2 different lists that update at different times or because this is actually a problem.
I'm only talking about invalid rooms here

Let's imagine the following scenario: you start off with the following room list

Invalid(a)
Invalid(b)
Invalid(c)
---------> Visible Range
Filled(d)
Filled(e)
---------> Visible Range
Invalid(f)
Invalid(g)

and let assume d and e get new messages and move to the top of the list

Invalid(d)
Invalid(e)
Invalid(c)
---------> Visible Range
Filled(b)
Filled(c)
---------> Visible Range
Invalid(f)
Invalid(g)

This is what I believe happens and you end up with duplicated IDs between a filled and invalid room.

On the other hand this might yet again be a side effect of us using 2 different lists so I'm definitiely willing to wait until we adopt the RoomList API and see if it's actually a problem

@Hywan
Copy link
Member

Hywan commented May 26, 2023

I don't want to set the problem on you, but it's more likely that the problem is happening on your “merge 2 lists” algorithm.

Let's see with the real RoomList API yeah, #1911.

@Hywan Hywan mentioned this issue Jun 1, 2023
62 tasks
@Hywan
Copy link
Member

Hywan commented Jun 15, 2023

Ensured by #1911 (and already ensured by SlidingSync) since we don't do weird merges between SlidingSyncLists. Confirmed that it was a problem on matrix_sdk_ffi consumers, not from the SDK itself. I'm closing the issue.

@Hywan Hywan closed this as completed Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants