-
Notifications
You must be signed in to change notification settings - Fork 433
MSC3571: Relation aggregation pagination #3571
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
Open
bwindels
wants to merge
2
commits into
main
Choose a base branch
from
bwindels/aggregation-pagination
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,129 @@ | ||
| # MSC3571: Aggregation pagination | ||
|
|
||
| MSC 2675 introduced aggregating relations on the server. The goal of bundled aggregations is to be more bandwidth efficient. For relations that aggregate to an array with many entries, we might actually end up sending too much data still when bundling. | ||
|
|
||
| This MSC proposes a pagination mechanism for aggregations so we don't need to bundle all entries for array aggregations. | ||
|
|
||
| Both MSC 2677 and MSC MSC 3267 would benefit from the pagination mechanism, hence proposing it in a separate MSC rather than including it in either of those. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. MSC2677 ended up not including an aggregation. |
||
|
|
||
| ## Proposal | ||
|
|
||
| For relation types that aggregate to an array, only a first few entries of the array are bundled. A pagination token is also provided to fetch subsequent aggregation array entries with a new `/aggregations` endpoint. The pagination tries to follow Matrix's defined pagination idiom where possibly; using `next_batch` and `chunk` - respectively giving a pagination token if there are | ||
| more aggregations, and an array of elements in the list. | ||
|
|
||
| Only the first page | ||
| is bundled; pagination of subsequent pages happens through the `/aggregations` | ||
| API that is defined in this MSC. The maximum amount of aggregations bundled | ||
| before the list is truncated is determined freely by the server. | ||
|
|
||
| Note that the client *can* determine the page size when calling | ||
| `/aggregations` through the `limit` request parameter, the offset is solely | ||
| determined by the `next_batch` token. | ||
|
|
||
| For instance, the below example shows an event with five bundled relations: | ||
| one replace, one reference and three thumbsup reaction annotations, | ||
| with more aggregated reactions available to paginate in | ||
| through `/aggregations` as `next_batch` is present. | ||
|
|
||
| These are just non-normative examples of what the aggregation for these | ||
| relation types could look like, and their MSCs might end up with | ||
| a different shape, so take these with a grain of salt. | ||
|
|
||
| ```json | ||
| { | ||
| ..., | ||
| "unsigned": { | ||
| "m.relations": { | ||
| "m.replace": { | ||
| "event_id": "$edit_event_id", | ||
| "origin_server_ts": 1562763768320, | ||
| "sender": "@bruno1:localhost" | ||
| }, | ||
| "m.reference": { | ||
| "chunk": [ | ||
| { | ||
| "event_id": "$some_event_id" | ||
| } | ||
| ], | ||
| }, | ||
| "m.annotation": { | ||
| "chunk": [ | ||
| { | ||
| "type": "m.reaction", | ||
| "key": "👍", | ||
| "origin_server_ts": 1562763768320, | ||
| "count": 3 | ||
| } | ||
| "next_batch": "abc123", | ||
| ] | ||
| } | ||
| } | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
|
|
||
| ### Querying aggregations | ||
|
|
||
| The `/aggregations` API lets you iterate over aggregations for the relations | ||
| of a given event. | ||
|
Comment on lines
+68
to
+69
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
|
|
||
| To iterate over the aggregations for an event (optionally filtering by | ||
| relation type and relation event type): | ||
|
|
||
| ``` | ||
| GET /_matrix/client/v1/rooms/{room_id}/aggregations/{event_id}/{rel_type}[/{event_type}][?from=token][&limit=amount] | ||
| ``` | ||
|
|
||
| ```json | ||
| { | ||
| "chunk": [ | ||
| { | ||
| "type": "m.reaction", | ||
| "key": "👍", | ||
| "count": 5, | ||
| } | ||
| ], | ||
| "next_batch": "some_token", | ||
| } | ||
| ``` | ||
|
|
||
| Again, this is a non-normative example of the aggregation for an | ||
| `m.annotation` relation type. | ||
|
|
||
| Note that endpoint does not have any trailing slashes: `GET /_matrix/client/v1/rooms/{roomID}/aggregations/$abc/m.reaction/` | ||
| would return aggregations of relations with an *empty* `event_type`, which is nonsensical. | ||
|
|
||
| The `from` and `limit` query parameters are used for pagination, and work | ||
| just like described for the `/messages` endpoint. | ||
|
|
||
| Trying to iterate over a relation type which does not use an aggregation key | ||
| (eg. `m.replace` and `m.reference`) should fail with 400 and error | ||
| `M_INVALID_REL_TYPE`. | ||
|
|
||
| ### Redactions | ||
|
|
||
| Redacted relations should not be taken into consideration in | ||
| aggregations returned from `/aggregations`. | ||
|
|
||
| Requesting `/aggregations` on a redacted event should | ||
| still return any existing aggregations. | ||
| This is in line with other APIs like `/context` and `/messages`. | ||
|
|
||
| ## Limitations | ||
|
|
||
| ### Event type based aggregation and filtering won't work well in encrypted rooms | ||
|
|
||
| The `/aggregations` endpoint allows filtering by event type, | ||
| which for encrypted rooms will be `m.room.encrypted`, rendering this filtering | ||
| less useful for encrypted rooms. | ||
|
|
||
| ## Prefix | ||
|
|
||
| While this MSC is not considered stable, the endpoints become: | ||
|
|
||
| - `GET /_matrix/client/unstable/rooms/{roomID}/aggregations/{eventID}/{relationType}[/{eventType}]` | ||
uhoreg marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| None of the newly introduced identifiers should use a prefix though, as this MSC | ||
| tries to document relation support already being used in | ||
| the wider matrix ecosystem. | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We write MSC numbers without a space between the "MSC" and the number. Also, please wrap your lines.