famedly / matrix-dart-sdk

Matrix SDK written in pure Dart.
GNU Affero General Public License v3.0
62 stars 31 forks source link

Room name/avatar replaced with older value after historical events emitted #1772

Open PhantomRay opened 6 months ago

PhantomRay commented 6 months ago

Checklist

In which Project did the bug appear?

Matrix dart SDK

On which platform did the bug appear?

Android, iOS

SDK Version

0.27

Describe the problem caused by this bug

Room info replaced after timeline. timeline.requestHistory(). From this point onwards, the room's state is changed in memory and local database.

Steps To Reproduce

  1. Sync rooms' data as usual
  2. use timeline.requestHistory for a particular room
  3. when the older events contain old room state like display name and avatar, it will override room's current value in memory and local database
### Tasks
krille-chan commented 6 months ago

Hello @PhantomRay can you try this again with 0.27.0? We have refactored the logic there how to handle incoming events

PhantomRay commented 6 months ago

Thanks a lot @krille-chan. I cannot test it directly in my app. But can certain use your demo app.

We rely on modified matrix api lite which supports jwt.

Hopefully you guys can support it very soon. Only a few lines of code.

PhantomRay commented 6 months ago

In fact, I will try to integrate jwt feature to 0.27 now.

PhantomRay commented 6 months ago

@krille-chan I just tested it in fluffychat with 0.27.0, the issue still remains.

PhantomRay commented 6 months ago

Hi @krille-chan could you please treat this bug as high priority? Because once it happens, re-launch the app will show incorrect room name and avatar.

PhantomRay commented 6 months ago

Thanks @krille-chan Much appreciated.

krille-chan commented 6 months ago

@PhantomRay I've tried to reproduce the bug in this way:

  1. Create a new room in FluffyChat and invite another test account
  2. Close the app
  3. From the other test account change the room name and send more than 10 messages
  4. Restart FluffyChat and load history because we received limited timeline
  5. Room Name was still correct 🤔

Do you have better way how to reproduce this?

PhantomRay commented 6 months ago

I think my scenario might be different to yours, because you must be editing data using client API. In my case, Matrix is integrated with our own system. We rely on admin API to push display name and avatar changes to matrix.

We use PUT /_synapse/admin/v2/users/<user_id> and change only displayname and avatar_url.

But either way, this shouldn't happen.

PhantomRay commented 6 months ago

Btw, I'm currently testing direct chat (1 to1) only.

nico-famedly commented 4 months ago

This should be fixed in 0.29.10, please reopen, if you can still reproduce it with that.

PhantomRay commented 4 months ago

Thanks a lot!

PhantomRay commented 4 months ago

In 0.29.10, it's still not fixed. Just to re-iterate, both of the following are overridden by older events:

nico-famedly commented 4 months ago

Are you sure this isn't just an effect of not having cleared the cache? Does this affect new sessions or existing ones?

PhantomRay commented 4 months ago

Our test is comprehensive including deleting the app.

Checked your recent code, noticed that you put some checks on event types. However I think that's not enough because we also have to compare the events' timestamps. If it's older than the time when room was loaded, or user's info previously set, then ignore.

nico-famedly commented 4 months ago

The checks are on the event update types. We should only apply the state updates, if the update is a forward update, i.e. when there is a new sync response. Could you describe in more detail, where you are seeing the wrong state updates, i.e. what steps you are doing like jumping to some timeline location and then paginating forward or similar?

Timestamps would be incorrect, but we really should only be writing event updates to the database in the sync loop and we might still be missing some cases, where they get written outside of that loop?

PhantomRay commented 4 months ago

Steps To Reproduce

  1. create a direct chat room
  2. update user name/avatar
  3. send 50 or more messages
  4. change the user's name/avatar again
  5. send another 50+ messages
  6. sign out
  7. Sync rooms' data as usual (at this point, the room's state would reflect the latest data correctly)
  8. enter the room again and use timeline.requestHistory till the last message.
  9. then you would see that room's current value in memory and local database has been changed, including the user's info.
nico-famedly commented 4 months ago

Thank you for the report, I will try to reproduce that.