Potential bugs in /relations endpoint

[matrixbot]

This issue has been migrated from #13862.

---

The /relations endpoint is similar to /messages, but essentially filters over a subset of events related to a specific event.

The core code that figures out which events to return from /relations is synapse.handlers.relations.RelationsHandler.get_relations, while for /messages it is synapse.handlers.pagination.PaginationHandler.get_messages, these then call down into similar datastore methods.

There's some subtle (and not so subtle) differences between them.

Differences which might cause bugs:

• I think both start at the "current" stream token if no from_token is given, but get_messages calculates this, while get_relations leaves it implicit and just searches to the edge of the table.
• get_messages takes a "pagination" lock -- should this be shared with get_relations?
• get_messages tries harder to not show users things from when they were not in the room.
• get_messages might backfill.
• get_messages fetches 2 * limit and then filters with _filter_results which is something to do with a multi-worker setup. get_relations does not do this.

Interface differenes (I don't think these are bugs, but are just confusing when trying to compare the two methods):

• get_messages always returns a "next" StreamToken and then sometimes doesn't use it; get_relations does not return one when there's no more data.
  • Not really a bug, but this means that get_messages will sometimes return a stream token when there's no additional data, e.g. if the last page has exactly limit entries in it. This will (potentially) cause an extra roundtrip from the client which could easily be avoided in most cases.
• from_token is optional in get_relations (see first bullet point above -- I suspect this logic of finding the proper starting point should be shared).
• Both implementations have a bunch of shared code (e.g. figuring out the from and to boundaries, calling generate_pagination_where_clause, generating the "next" token).

The final bit of this is that we likely don't test this endpoint as thoroughly as /messages. There's a start to some integration tests at matrix-org/complement#467, but they're very simplistic.