Skip to content
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

MSC3981: /relations recursion #3981

Merged
merged 25 commits into from
Jan 30, 2024
Merged
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
098b343
Initial Draft for MSC 3981: /relations recursion
justjanne Mar 19, 2023
abddab8
fix accidentally copy-pasted title
justjanne Mar 20, 2023
4f61e43
Remove unnecessarily specific sentence
justjanne Mar 22, 2023
381a355
Add warning to avoid unlimited recursion
justjanne Mar 22, 2023
bbcc81a
Clean-up links in MSC
justjanne Mar 28, 2023
f7e5ca3
Apply reviewer suggestions
justjanne Mar 28, 2023
ffb316d
More clarifications on examples
justjanne Mar 28, 2023
077f6b1
Address feedback on formatting
justjanne Apr 19, 2023
c638630
Address feedback on linking related specs
justjanne Apr 19, 2023
dad341e
Rephrase technical definition of the parameter
justjanne Apr 19, 2023
6d88fe8
Correct mistake in examples
justjanne Apr 19, 2023
123eef1
Rephrase paragraph on O(n+1) issue
justjanne Apr 19, 2023
65cab28
fix: correct mixup between event type and rel type
justjanne May 9, 2023
6baafd8
feat: add clarification for why this was needed
justjanne May 9, 2023
b6119c5
feat: add clarification for how it affects intermediate events
justjanne May 9, 2023
cfd4223
feat: add clarification for how it affects intermediate events
justjanne May 9, 2023
94561ad
Add /version unstable feature flag
weeman1337 May 11, 2023
f404254
feat: improve phrasing of unstable prefix section
justjanne May 11, 2023
03e8d68
feat: add clarification for how clients can make use of this
justjanne May 11, 2023
0e50bdb
Clarify unstable features map
dbkr Dec 13, 2023
a4d8e73
Attempt at resolving the discussion threads for 3981.
dbkr Dec 13, 2023
a027354
Add note that security is discussed elsewhere.
dbkr Jan 3, 2024
6ace9f4
Add recursion_depth response parameter.
dbkr Jan 3, 2024
c160d36
Note that recursion_depth is sent un-prefixed.
dbkr Jan 3, 2024
bdae577
Add summary of security discussions
dbkr Jan 16, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 63 additions & 0 deletions proposals/3981-relations-recursion.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# MSC3715: Add a pagination direction parameter to `/relations`
justjanne marked this conversation as resolved.
Show resolved Hide resolved

[MSC2675](https://github.com/matrix-org/matrix-doc/pull/2675) introduced the
`/relations` API to retrieve events which are directly related to a given event.

This API has been used as basis of many other features and MSCs since then,
including threads.

Threads was one of the first usages of this API that allowed nested relations -
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FTR I think this is only an issue with thread & reference relations?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is saying only thread & reference relations can have second-order relations? In which case I think probably (although I'd be surprised if references ever did) but I'm not sure it makes a huge difference here.

an event may have an m.reaction or m.replace relation to another event, which
in turn may have an m.thread relation to the thread root.
justjanne marked this conversation as resolved.
Show resolved Hide resolved

This forms a tree of relations, which is necessary for clients to retrieve to
be able to correctly display threads and determine the latest event of a thread
to be able to correctly send read receipts and determine the thread's
unread status.
justjanne marked this conversation as resolved.
Show resolved Hide resolved

justjanne marked this conversation as resolved.
Show resolved Hide resolved
## Proposal

It is proposed to add the `recurse` parameter to the `/relations` API.
justjanne marked this conversation as resolved.
Show resolved Hide resolved
justjanne marked this conversation as resolved.
Show resolved Hide resolved

> Whether to recursively include all nested relations of a given event.
justjanne marked this conversation as resolved.
Show resolved Hide resolved
>
> If this is set to true, it will return the entire subtree of events related
> to the specified event, directly or indirectly.
justjanne marked this conversation as resolved.
Show resolved Hide resolved
> If this is set to false, it will only return events directly related to the
> specified event.
>
> One of: `[true false]`.
justjanne marked this conversation as resolved.
Show resolved Hide resolved

In order to be backwards compatible with MSC2675 (and Synapse's legacy
implementation), the `recurse` parameter must be optional (defaulting to
`false`).
justjanne marked this conversation as resolved.
Show resolved Hide resolved

Regardless of the value of the `recurse` parameter, events will always be
returned in the same order as they would be by the `/messages` API.
justjanne marked this conversation as resolved.
Show resolved Hide resolved

If the API call specifies an `event_type` or `rel_type`, this filter will be
applied to nested relations just as it is applied to direct relations.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any relations that are allowed to be applied to events of that same relation? It doesn't matter too much now, just curious.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As in X relates to Y and Y relates to Z with the same rel type? Only replies I think. Either way I've clarified that a rel type filter + the recurse option is probably not useful for this reason.

justjanne marked this conversation as resolved.
Show resolved Hide resolved

## Potential issues

Naive implementations of recursive API endpoints frequently cause N+1 query
problems. Homeservers should take care to implementing this MSC to avoid
justjanne marked this conversation as resolved.
Show resolved Hide resolved
situations where a specifically crafted set of events and API calls could
amplify the load on the server unreasonably.

## Alternatives
justjanne marked this conversation as resolved.
Show resolved Hide resolved

1. Clients could fetch all relations recursively client-side, which would
increase network traffic and server load significantly.
2. A new, specialised endpoint could be created for threads, specifically
designed to present separate timelines that, in all other ways, would
behave identically to `/messages`
justjanne marked this conversation as resolved.
Show resolved Hide resolved

## Security considerations

None.
dbkr marked this conversation as resolved.
Show resolved Hide resolved

## Unstable prefix

Before standardization, `org.matrix.msc3981.recurse` should be used in place
of `recurse`.