Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Stabilize support for MSC2675: /relations endpoint #11753

Open
7 of 8 tasks
clokep opened this issue Jan 14, 2022 · 5 comments
Open
7 of 8 tasks

Stabilize support for MSC2675: /relations endpoint #11753

clokep opened this issue Jan 14, 2022 · 5 comments
Assignees
Labels
A-Threads Threaded messages T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.

Comments

@clokep
Copy link
Member

clokep commented Jan 14, 2022

MSC2675 is currently in FCP (to complete tomorrow, barring any concerns). We have a few bits of work to do to stabilize support in Synapse for it:

  • Fix-up redaction support:
    • Ensure that redacted relations are not bundled nor returned from /relations.
    • Support fetching relations of a redacted event. (Currently this returns nothing.)
    • Bundle aggregations for a redacted event.
  • Properly handle relations for ignored users in:
    • /relations
    • Bundled aggregations.
  • Support accepting pagination tokens from /sync and /messages.
    • Clean-up backwards compatibility with the old pagination token format.
  • Add the v1 prefix for /relations.
  • Remove the unstable prefix for /relations.

Those were all the differences that I found by reading through it, but there could be more.

@clokep clokep added the T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. label Jan 14, 2022
@clokep
Copy link
Member Author

clokep commented Feb 8, 2022

It is not explicit in the MSC, but it seems we also don't return any bundled aggregations for a redacted event. We likely should since that would match the behavior of being able to call /relations on it.

@clokep
Copy link
Member Author

clokep commented Feb 8, 2022

Another piece of this that seems broken is that currently you cannot paginate this API using the results of a /sync or /messages API.

@clokep clokep self-assigned this Feb 8, 2022
@clokep clokep added the A-Threads Threaded messages label Feb 8, 2022
@clokep
Copy link
Member Author

clokep commented Feb 8, 2022

Another piece of this that seems broken is that currently you cannot paginate this API using the results of a /sync or /messages API.

This is due to the usage of RelationPaginationToken instead of StreamToken by the returned APIs. I think a way forward would be to:

  • Switch the APIs to accept a serialized StreamToken or RelationPaginationToken (in which case we generate a StreamToken with only room_key filled in from the parsed RelationPaginationToken).
  • Update the generated tokens to be serialied StreamTokens.

This all seems fairly straightforward (although finicky), but the similar code in the PaginationHandler does some rather complicated bits with topological ordering.

@clokep
Copy link
Member Author

clokep commented Apr 11, 2022

This is done, minus removing support for the unstable /relations endpoint, which I think needs to wait for the next spec release.

@clokep
Copy link
Member Author

clokep commented Jan 23, 2023

This is done, minus removing support for the unstable /relations endpoint, which I think needs to wait for the next spec release.

I think Matrix v1.3 included this endpoint, so we can probably finish up this work now. Also see #14104.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Threads Threaded messages T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.
Projects
None yet
Development

No branches or pull requests

1 participant