search

matrix-org / synapse

Synapse: Matrix homeserver written in Python/Twisted.
https://matrix-org.github.io/synapse
Apache License 2.0
11.83k stars 2.12k forks source link

Elide redundant member events in `/messages` response #11960

Closed ShadowJonathan closed 2 years ago

ShadowJonathan commented 2 years ago

https://github.com/matrix-org/synapse/blob/68acb0a29dcb03a0ecbcebdb95e09c5999598f42/synapse/handlers/pagination.py#L525

This is not mandated explicitly by the spec;

Rather than responses in the sequence sending duplicate membership events for these senders to the client, the server MAY assume that clients will remember membership events they have already been sent, and choose to skip sending membership events for members whose membership has not changed.

...however, this caused problems when conduit did not implement this similarly to synapse;

JS-SDK (Cinny and Element Web), Element Android, and Hydrogen all assume this behaviour is a must.

I believe it'd help with more correct spec adoption greatly if redundant membership was indeed removed, so that current clients are more aware of this optional spec behaviour.

erikjohnston commented 2 years ago

We should do this on Synapse to help with spec compliance. However, before we can land the change in Synapse we'll need the clients to be fixed first.

We should talk to the client teams to make sure fixing those bugs is on their radar before we spend time on this.

erikjohnston commented 2 years ago

First step is to figure out what is required to actually fix these TODOs, note that there are likely other TODOs around lazy loading of members in the pagination and sync code.

DMRobertson commented 2 years ago

Possibly related: https://github.com/matrix-org/matrix-spec/issues/942

ShadowJonathan commented 2 years ago

https://github.com/matrix-org/synapse/issues/5393 appears to be related, and/or the original issue for this.

richvdh commented 2 years ago

Duplicate of #5393