From 098b3436b9cd3561c92ec2a35456ababca787a9a Mon Sep 17 00:00:00 2001 From: Janne Mareike Koschinski Date: Sun, 19 Mar 2023 11:06:54 +0100 Subject: [PATCH 01/25] Initial Draft for MSC 3981: /relations recursion Signed-off-by: Janne Mareike Koschinski --- proposals/3981-relations-recursion.md | 63 +++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 proposals/3981-relations-recursion.md diff --git a/proposals/3981-relations-recursion.md b/proposals/3981-relations-recursion.md new file mode 100644 index 00000000000..c6b687b8d81 --- /dev/null +++ b/proposals/3981-relations-recursion.md @@ -0,0 +1,63 @@ +# MSC3715: Add a pagination direction parameter to `/relations` + +[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 - +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. + +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. + +## Proposal + +It is proposed to add the `recurse` parameter to the `/relations` API. + +> Whether to recursively include all nested relations of a given event. +> +> If this is set to true, it will return the entire subtree of events related +> to the specified event, directly or indirectly. +> If this is set to false, it will only return events directly related to the +> specified event. +> +> One of: `[true false]`. + +In order to be backwards compatible with MSC2675 (and Synapse's legacy +implementation), the `recurse` parameter must be optional (defaulting to +`false`). + +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. + +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. + +## Potential issues + +Naive implementations of recursive API endpoints frequently cause N+1 query +problems. Homeservers should take care to implementing this MSC to avoid +situations where a specifically crafted set of events and API calls could +amplify the load on the server unreasonably. + +## Alternatives + +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` + +## Security considerations + +None. + +## Unstable prefix + +Before standardization, `org.matrix.msc3981.recurse` should be used in place +of `recurse`. From abddab858cc6d7af236eb8d636b25ae92160a09f Mon Sep 17 00:00:00 2001 From: Janne Mareike Koschinski Date: Mon, 20 Mar 2023 09:06:25 +0100 Subject: [PATCH 02/25] fix accidentally copy-pasted title Co-authored-by: Travis Ralston --- proposals/3981-relations-recursion.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/3981-relations-recursion.md b/proposals/3981-relations-recursion.md index c6b687b8d81..f50733fcecb 100644 --- a/proposals/3981-relations-recursion.md +++ b/proposals/3981-relations-recursion.md @@ -1,4 +1,4 @@ -# MSC3715: Add a pagination direction parameter to `/relations` +# MSC3981: `/relations` recursion [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. From 4f61e4389e6d1923632664682637b76b859d2373 Mon Sep 17 00:00:00 2001 From: Janne Mareike Koschinski Date: Wed, 22 Mar 2023 12:22:13 +0100 Subject: [PATCH 03/25] Remove unnecessarily specific sentence Co-authored-by: Patrick Cloke --- proposals/3981-relations-recursion.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/proposals/3981-relations-recursion.md b/proposals/3981-relations-recursion.md index f50733fcecb..2ffb677b66d 100644 --- a/proposals/3981-relations-recursion.md +++ b/proposals/3981-relations-recursion.md @@ -28,9 +28,8 @@ It is proposed to add the `recurse` parameter to the `/relations` API. > > One of: `[true false]`. -In order to be backwards compatible with MSC2675 (and Synapse's legacy -implementation), the `recurse` parameter must be optional (defaulting to -`false`). +In order to be backwards compatible the `recurse` parameter must be +optional (defaulting to `false`). 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. From 381a355ddfafe37730328453a62ee6927cc015cf Mon Sep 17 00:00:00 2001 From: Janne Mareike Koschinski Date: Wed, 22 Mar 2023 12:23:12 +0100 Subject: [PATCH 04/25] Add warning to avoid unlimited recursion Co-authored-by: Patrick Cloke --- proposals/3981-relations-recursion.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/proposals/3981-relations-recursion.md b/proposals/3981-relations-recursion.md index 2ffb677b66d..2bf409f7abe 100644 --- a/proposals/3981-relations-recursion.md +++ b/proposals/3981-relations-recursion.md @@ -26,6 +26,9 @@ It is proposed to add the `recurse` parameter to the `/relations` API. > If this is set to false, it will only return events directly related to the > specified event. > +> It is recommended that at least 3 relations are traversed, implementations +> should be careful to not infinitely recurse. +> > One of: `[true false]`. In order to be backwards compatible the `recurse` parameter must be From bbcc81a12511e8b048eccacb146536428617384c Mon Sep 17 00:00:00 2001 From: Janne Mareike Koschinski Date: Wed, 29 Mar 2023 00:42:43 +0200 Subject: [PATCH 05/25] Clean-up links in MSC --- proposals/3981-relations-recursion.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/proposals/3981-relations-recursion.md b/proposals/3981-relations-recursion.md index 2bf409f7abe..a7715166b7f 100644 --- a/proposals/3981-relations-recursion.md +++ b/proposals/3981-relations-recursion.md @@ -1,7 +1,7 @@ # MSC3981: `/relations` recursion -[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. +[MSC2675] 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. @@ -54,6 +54,7 @@ amplify the load on the server unreasonably. 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` +3. Twitter-style threads (see [MSC2836]) ## Security considerations @@ -63,3 +64,7 @@ None. Before standardization, `org.matrix.msc3981.recurse` should be used in place of `recurse`. + +[MSC2675]: https://github.com/matrix-org/matrix-spec-proposals/pull/2675 +[MSC2836]: https://github.com/matrix-org/matrix-spec-proposals/pull/2836 +[MSC3771]: https://github.com/matrix-org/matrix-spec-proposals/pull/3771 From f7e5ca3eb80c8b55cb5ad90ddbfee6de7922c161 Mon Sep 17 00:00:00 2001 From: Janne Mareike Koschinski Date: Wed, 29 Mar 2023 00:43:00 +0200 Subject: [PATCH 06/25] Apply reviewer suggestions --- proposals/3981-relations-recursion.md | 53 +++++++++++++++++++++++---- 1 file changed, 46 insertions(+), 7 deletions(-) diff --git a/proposals/3981-relations-recursion.md b/proposals/3981-relations-recursion.md index a7715166b7f..c1b495e3db1 100644 --- a/proposals/3981-relations-recursion.md +++ b/proposals/3981-relations-recursion.md @@ -7,13 +7,13 @@ 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 - -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. +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. -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. +This forms a tree of relations, which so far could only be traversed +efficiently in hierarchical, but not in chronological order. Yet, for some +functionality – e.g., to determine which event a read receipt should +reference as per [MSC3771] – chronological order is necessary. ## Proposal @@ -34,8 +34,13 @@ It is proposed to add the `recurse` parameter to the `/relations` API. In order to be backwards compatible the `recurse` parameter must be optional (defaulting to `false`). +In this situation, topological ordering is intended to refer to the same +ordering that would be applied to the events when requested via the `/messages` +api given the same `dir` parameter. + 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. +returned in topological ordering. Pagination and limiting shall also apply to +topological ordering. 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. @@ -55,11 +60,45 @@ amplify the load on the server unreasonably. designed to present separate timelines that, in all other ways, would behave identically to `/messages` 3. Twitter-style threads (see [MSC2836]) +4. Alternatively a `depth` parameter could have been specified, as in [MSC2836] + We believe that a customizable depth would add unnecessary constraints to + server implementers, as different server implementations may have different + performance considerations and may choose different limits. Additionally, + the maximum currently achievable depth is still low enough to avoid this + becoming an issue. ## Security considerations None. +## Examples + +Given the following graph: + +```mermaid +flowchart RL + subgraph Thread + G + E-->D + B-->A + end + B-.->|m.thread|A + G-.->|m.thread|A + E-.->|m.reaction|B + D-.->|m.edit|A + G-->F-->E + D-->C-->B +``` + +`/messages` with `dir=f` would return +`[A, B, C, D, E, F, G]` + +`/relations` on event `A` with `rel_type=m.thread` and `dir=f` would return +`[A, B, G]`. + +`/relations` on event `A` with `recurse=true` and `dir=f` would return +`[A, B, D, E, G]`. + ## Unstable prefix Before standardization, `org.matrix.msc3981.recurse` should be used in place From ffb316d337389cf00d4cf2cd9c93f179b2b949cc Mon Sep 17 00:00:00 2001 From: Janne Mareike Koschinski Date: Wed, 29 Mar 2023 01:05:43 +0200 Subject: [PATCH 07/25] More clarifications on examples --- proposals/3981-relations-recursion.md | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/proposals/3981-relations-recursion.md b/proposals/3981-relations-recursion.md index c1b495e3db1..69dd2db4d69 100644 --- a/proposals/3981-relations-recursion.md +++ b/proposals/3981-relations-recursion.md @@ -90,14 +90,17 @@ flowchart RL D-->C-->B ``` -`/messages` with `dir=f` would return -`[A, B, C, D, E, F, G]` +`/messages` with `dir=f` would +return `[A, B, C, D, E, F, G]`. -`/relations` on event `A` with `rel_type=m.thread` and `dir=f` would return -`[A, B, G]`. +`/relations` on event `A` with `rel_type=m.thread` and `dir=f` would +return `[A, B, G]`. -`/relations` on event `A` with `recurse=true` and `dir=f` would return -`[A, B, D, E, G]`. +`/relations` on event `A` with `recurse=true` and `dir=f` would +return `[A, B, D, E, G]`. + +`/relations` on event `A` with `recurse=true`, `dir=b` and `limit=2` would +return `[G, E]`. ## Unstable prefix From 077f6b15730a7714fe8a14a6d0737107364ea6fa Mon Sep 17 00:00:00 2001 From: Janne Mareike Koschinski Date: Wed, 19 Apr 2023 12:21:04 +0200 Subject: [PATCH 08/25] Address feedback on formatting Signed-off-by: Janne Mareike Koschinski --- proposals/3981-relations-recursion.md | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/proposals/3981-relations-recursion.md b/proposals/3981-relations-recursion.md index 69dd2db4d69..e9d47d47662 100644 --- a/proposals/3981-relations-recursion.md +++ b/proposals/3981-relations-recursion.md @@ -10,14 +10,15 @@ Threads was one of the first usages of this API that allowed nested relations - 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. -This forms a tree of relations, which so far could only be traversed +This forms a tree of relations, which can currently only be traversed efficiently in hierarchical, but not in chronological order. Yet, for some functionality – e.g., to determine which event a read receipt should reference as per [MSC3771] – chronological order is necessary. ## Proposal -It is proposed to add the `recurse` parameter to the `/relations` API. +It is proposed to add the `recurse` parameter to the `/relations` API, defined +as follows: > Whether to recursively include all nested relations of a given event. > @@ -26,7 +27,7 @@ It is proposed to add the `recurse` parameter to the `/relations` API. > If this is set to false, it will only return events directly related to the > specified event. > -> It is recommended that at least 3 relations are traversed, implementations +> It is recommended that at least 3 relations are traversed. Implementations > should be careful to not infinitely recurse. > > One of: `[true false]`. @@ -58,9 +59,9 @@ amplify the load on the server unreasonably. 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` -3. Twitter-style threads (see [MSC2836]) -4. Alternatively a `depth` parameter could have been specified, as in [MSC2836] + behave identically to `/messages`. +3. Twitter-style threads (see [MSC2836]). +4. Alternatively a `depth` parameter could have been specified, as in [MSC2836]. We believe that a customizable depth would add unnecessary constraints to server implementers, as different server implementations may have different performance considerations and may choose different limits. Additionally, From c63863050bb5c95400e9fb18ff94a458889d730a Mon Sep 17 00:00:00 2001 From: Janne Mareike Koschinski Date: Wed, 19 Apr 2023 12:22:06 +0200 Subject: [PATCH 09/25] Address feedback on linking related specs Signed-off-by: Janne Mareike Koschinski --- proposals/3981-relations-recursion.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/proposals/3981-relations-recursion.md b/proposals/3981-relations-recursion.md index e9d47d47662..3df7e6e3f6c 100644 --- a/proposals/3981-relations-recursion.md +++ b/proposals/3981-relations-recursion.md @@ -1,13 +1,13 @@ # MSC3981: `/relations` recursion -[MSC2675] introduced the `/relations` API to retrieve events which are directly -related to a given event. +The [`/relations`] API allows clients to retrieve events which directly relate +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 - -an event may have an `m.reaction` or `m.replace` relation to another event, +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. This forms a tree of relations, which can currently only be traversed @@ -108,6 +108,8 @@ return `[G, E]`. Before standardization, `org.matrix.msc3981.recurse` should be used in place of `recurse`. -[MSC2675]: https://github.com/matrix-org/matrix-spec-proposals/pull/2675 [MSC2836]: https://github.com/matrix-org/matrix-spec-proposals/pull/2836 [MSC3771]: https://github.com/matrix-org/matrix-spec-proposals/pull/3771 +[`/relations`]: https://spec.matrix.org/v1.6/client-server-api/#get_matrixclientv1roomsroomidrelationseventid +[`m.reaction`]: https://github.com/matrix-org/matrix-spec-proposals/pull/2677 +[`m.replace`]: https://spec.matrix.org/v1.6/client-server-api/#event-replacements From dad341e377516169759a86fe1ef7d5712b79331a Mon Sep 17 00:00:00 2001 From: Janne Mareike Koschinski Date: Wed, 19 Apr 2023 12:54:02 +0200 Subject: [PATCH 10/25] Rephrase technical definition of the parameter Signed-off-by: Janne Mareike Koschinski --- proposals/3981-relations-recursion.md | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/proposals/3981-relations-recursion.md b/proposals/3981-relations-recursion.md index 3df7e6e3f6c..601086b3aa2 100644 --- a/proposals/3981-relations-recursion.md +++ b/proposals/3981-relations-recursion.md @@ -20,15 +20,16 @@ reference as per [MSC3771] – chronological order is necessary. It is proposed to add the `recurse` parameter to the `/relations` API, defined as follows: -> Whether to recursively include all nested relations of a given event. +> Whether to include events which relate only indirectly to the given event. +> +> If set to false, only events which have direct a relation with the given +> event will be included. +> +> If set to true, all events which relate to the given event, or relate to +> events that relate to the given event, will be included. > -> If this is set to true, it will return the entire subtree of events related -> to the specified event, directly or indirectly. -> If this is set to false, it will only return events directly related to the -> specified event. -> -> It is recommended that at least 3 relations are traversed. Implementations -> should be careful to not infinitely recurse. +> It is recommended that at least 3 levels of relationships are traversed. +> Implementations should be careful to not infinitely recurse. > > One of: `[true false]`. From 6d88fe8defe437edcf158d613447be7935af0a60 Mon Sep 17 00:00:00 2001 From: Janne Mareike Koschinski Date: Wed, 19 Apr 2023 12:58:03 +0200 Subject: [PATCH 11/25] Correct mistake in examples Signed-off-by: Janne Mareike Koschinski --- proposals/3981-relations-recursion.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/proposals/3981-relations-recursion.md b/proposals/3981-relations-recursion.md index 601086b3aa2..70860d0ca99 100644 --- a/proposals/3981-relations-recursion.md +++ b/proposals/3981-relations-recursion.md @@ -96,10 +96,10 @@ flowchart RL return `[A, B, C, D, E, F, G]`. `/relations` on event `A` with `rel_type=m.thread` and `dir=f` would -return `[A, B, G]`. +return `[B, G]`. `/relations` on event `A` with `recurse=true` and `dir=f` would -return `[A, B, D, E, G]`. +return `[B, D, E, G]`. `/relations` on event `A` with `recurse=true`, `dir=b` and `limit=2` would return `[G, E]`. From 123eef1fe394aa49fbaf22b67d053364121bf7ce Mon Sep 17 00:00:00 2001 From: Janne Mareike Koschinski Date: Wed, 19 Apr 2023 13:23:12 +0200 Subject: [PATCH 12/25] Rephrase paragraph on O(n+1) issue Signed-off-by: Janne Mareike Koschinski --- proposals/3981-relations-recursion.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/proposals/3981-relations-recursion.md b/proposals/3981-relations-recursion.md index 70860d0ca99..8e2bc290101 100644 --- a/proposals/3981-relations-recursion.md +++ b/proposals/3981-relations-recursion.md @@ -49,10 +49,12 @@ applied to nested relations just as it is applied to direct relations. ## Potential issues -Naive implementations of recursive API endpoints frequently cause N+1 query -problems. Homeservers should take care to implementing this MSC to avoid -situations where a specifically crafted set of events and API calls could -amplify the load on the server unreasonably. +Naive implementations might be tempted to provide support for this parameter +through a thin shim which is functionally identical to the client doing +separate recursive `/relations` requests itself. This is ill-advised. + +Such an implementation would, given a specifically crafted set of events, +allow a client to cause unreasonable load. ## Alternatives From 65cab28654df4d2df75284a4039aaae888a596d1 Mon Sep 17 00:00:00 2001 From: Janne Mareike Koschinski Date: Tue, 9 May 2023 16:13:26 +0200 Subject: [PATCH 13/25] fix: correct mixup between event type and rel type Co-authored-by: Patrick Cloke --- proposals/3981-relations-recursion.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/3981-relations-recursion.md b/proposals/3981-relations-recursion.md index 8e2bc290101..88a84f7887b 100644 --- a/proposals/3981-relations-recursion.md +++ b/proposals/3981-relations-recursion.md @@ -88,7 +88,7 @@ flowchart RL end B-.->|m.thread|A G-.->|m.thread|A - E-.->|m.reaction|B + E-.->|m.annotation|B D-.->|m.edit|A G-->F-->E D-->C-->B From 6baafd8280bb45a55bce8fadfafe82f4749cc174 Mon Sep 17 00:00:00 2001 From: Janne Mareike Koschinski Date: Tue, 9 May 2023 16:24:41 +0200 Subject: [PATCH 14/25] feat: add clarification for why this was needed Signed-off-by: Janne Mareike Koschinski --- proposals/3981-relations-recursion.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/proposals/3981-relations-recursion.md b/proposals/3981-relations-recursion.md index 88a84f7887b..10fc6f1266d 100644 --- a/proposals/3981-relations-recursion.md +++ b/proposals/3981-relations-recursion.md @@ -15,6 +15,12 @@ efficiently in hierarchical, but not in chronological order. Yet, for some functionality – e.g., to determine which event a read receipt should reference as per [MSC3771] – chronological order is necessary. +Previously, clients would be unable to obtain a consistent ordering of +events in threads or related to threads. +Workarounds such as sending a separate `/relations` request per individual +message in each thread are only able to approximate the actual ordering, +as they rely on timestamps. + ## Proposal It is proposed to add the `recurse` parameter to the `/relations` API, defined @@ -106,6 +112,9 @@ return `[B, D, E, G]`. `/relations` on event `A` with `recurse=true`, `dir=b` and `limit=2` would return `[G, E]`. +`/relations` on event `A` with `rel_type=m.annotation`, +`event_type=m.reaction` and `recurse=true` would return `[G, E]`. + ## Unstable prefix Before standardization, `org.matrix.msc3981.recurse` should be used in place From b6119c57b234c75e53ab69adf97a9bd244b0a7db Mon Sep 17 00:00:00 2001 From: Janne Mareike Koschinski Date: Tue, 9 May 2023 16:28:44 +0200 Subject: [PATCH 15/25] feat: add clarification for how it affects intermediate events Signed-off-by: Janne Mareike Koschinski --- proposals/3981-relations-recursion.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/proposals/3981-relations-recursion.md b/proposals/3981-relations-recursion.md index 10fc6f1266d..d4abd568242 100644 --- a/proposals/3981-relations-recursion.md +++ b/proposals/3981-relations-recursion.md @@ -50,8 +50,10 @@ Regardless of the value of the `recurse` parameter, events will always be returned in topological ordering. Pagination and limiting shall also apply to topological ordering. -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. +Filters specified via `event_type` or `rel_type` will only be applied to the +leaves of the graph formed by the related events, even if their only relation +to the original given event is through an intermediate event that does not +match these filters. ## Potential issues From cfd4223485a26172e81c018b9bf480c90f708126 Mon Sep 17 00:00:00 2001 From: Janne Mareike Koschinski Date: Tue, 9 May 2023 16:41:29 +0200 Subject: [PATCH 16/25] feat: add clarification for how it affects intermediate events Signed-off-by: Janne Mareike Koschinski --- proposals/3981-relations-recursion.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/proposals/3981-relations-recursion.md b/proposals/3981-relations-recursion.md index d4abd568242..61beaf5cd9a 100644 --- a/proposals/3981-relations-recursion.md +++ b/proposals/3981-relations-recursion.md @@ -50,10 +50,10 @@ Regardless of the value of the `recurse` parameter, events will always be returned in topological ordering. Pagination and limiting shall also apply to topological ordering. -Filters specified via `event_type` or `rel_type` will only be applied to the -leaves of the graph formed by the related events, even if their only relation -to the original given event is through an intermediate event that does not -match these filters. +Filters specified via `event_type` or `rel_type` will be applied to leaves +and intermediate events on the graph formed by the related events. +Events that would match the filter, but whose only relation to the original +given event is through a non-matching intermediate event, will not be included. ## Potential issues From 94561adaa6cf2e4be18104d059087079985291ed Mon Sep 17 00:00:00 2001 From: Michael Weimann Date: Thu, 11 May 2023 08:52:17 +0200 Subject: [PATCH 17/25] Add /version unstable feature flag --- proposals/3981-relations-recursion.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/proposals/3981-relations-recursion.md b/proposals/3981-relations-recursion.md index 61beaf5cd9a..4508f2f6afc 100644 --- a/proposals/3981-relations-recursion.md +++ b/proposals/3981-relations-recursion.md @@ -122,6 +122,11 @@ return `[G, E]`. Before standardization, `org.matrix.msc3981.recurse` should be used in place of `recurse`. +## Unstable feature in `/version` + +Homeservers which support this MSC should indicate it by adding `org.matrix.msc3981` and +`org.matrix.msc3981.stable` in the response to `GET /_matrix/client/versions` requests. + [MSC2836]: https://github.com/matrix-org/matrix-spec-proposals/pull/2836 [MSC3771]: https://github.com/matrix-org/matrix-spec-proposals/pull/3771 [`/relations`]: https://spec.matrix.org/v1.6/client-server-api/#get_matrixclientv1roomsroomidrelationseventid From f40425456a60d6dbc66c02ea6f82857e648d8531 Mon Sep 17 00:00:00 2001 From: Janne Mareike Koschinski Date: Thu, 11 May 2023 12:54:00 +0200 Subject: [PATCH 18/25] feat: improve phrasing of unstable prefix section Signed-off-by: Janne Mareike Koschinski --- proposals/3981-relations-recursion.md | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/proposals/3981-relations-recursion.md b/proposals/3981-relations-recursion.md index 4508f2f6afc..3c933c6d3f7 100644 --- a/proposals/3981-relations-recursion.md +++ b/proposals/3981-relations-recursion.md @@ -119,13 +119,24 @@ return `[G, E]`. ## Unstable prefix -Before standardization, `org.matrix.msc3981.recurse` should be used in place -of `recurse`. +### While the MSC is not yet part of a spec version -## Unstable feature in `/version` +During this period, to detect server support, clients should check for the +presence of the `org.matrix.msc3981` flag in unstable_features on `/versions`. -Homeservers which support this MSC should indicate it by adding `org.matrix.msc3981` and -`org.matrix.msc3981.stable` in the response to `GET /_matrix/client/versions` requests. +Clients are also required to use `org.matrix.msc3981.recurse` in place +of `recurse` at this time. + +### Once the MSC is in a spec version + +Once this MSC becomes a part of a spec version, clients should rely on the +presence of the spec version that supports this MSC in the `/version` response +to determine support. + +Servers are encouraged to keep the `org.matrix.msc3827` flag around for a +reasonable amount of time to help smooth over the transition for clients. +"Reasonable" is intentionally left as an implementation detail, however the MSC +process currently recommends at most 2 months from the date of spec release. [MSC2836]: https://github.com/matrix-org/matrix-spec-proposals/pull/2836 [MSC3771]: https://github.com/matrix-org/matrix-spec-proposals/pull/3771 From 03e8d68670c013d09fb9791ce0d1d56455f31af4 Mon Sep 17 00:00:00 2001 From: Janne Mareike Koschinski Date: Thu, 11 May 2023 16:03:29 +0200 Subject: [PATCH 19/25] feat: add clarification for how clients can make use of this Signed-off-by: Janne Mareike Koschinski --- proposals/3981-relations-recursion.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/proposals/3981-relations-recursion.md b/proposals/3981-relations-recursion.md index 3c933c6d3f7..5008f936b63 100644 --- a/proposals/3981-relations-recursion.md +++ b/proposals/3981-relations-recursion.md @@ -21,6 +21,10 @@ Workarounds such as sending a separate `/relations` request per individual message in each thread are only able to approximate the actual ordering, as they rely on timestamps. +If a homeserver announces support for this feature, a client can stop this +fallback behavior and instead just append the recurse parameter to the initial +request. + ## Proposal It is proposed to add the `recurse` parameter to the `/relations` API, defined From 0e50bdbc61e103175e91dc7991b574a7e244a4c8 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 13 Dec 2023 12:56:03 +0000 Subject: [PATCH 20/25] Clarify unstable features map Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> --- proposals/3981-relations-recursion.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/proposals/3981-relations-recursion.md b/proposals/3981-relations-recursion.md index 5008f936b63..8fc2d074ff2 100644 --- a/proposals/3981-relations-recursion.md +++ b/proposals/3981-relations-recursion.md @@ -126,7 +126,8 @@ return `[G, E]`. ### While the MSC is not yet part of a spec version During this period, to detect server support, clients should check for the -presence of the `org.matrix.msc3981` flag in unstable_features on `/versions`. +presence of the `org.matrix.msc3981` flag in the `unstable_features` map +on [`/versions`](https://spec.matrix.org/v1.7/client-server-api/#get_matrixclientversions). Clients are also required to use `org.matrix.msc3981.recurse` in place of `recurse` at this time. From a4d8e737710fa8e7f5da1b6184f9fb2c26a58c27 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 13 Dec 2023 15:20:35 +0000 Subject: [PATCH 21/25] Attempt at resolving the discussion threads for 3981. Largely removes the talk about topological vs chronological ordering as I don't really see what it was trying to express either, other than the fact that a client fetching relations by recursing manually would give no ordering information, which is what I've reduced it to. --- proposals/3981-relations-recursion.md | 51 ++++++++++++--------------- 1 file changed, 22 insertions(+), 29 deletions(-) diff --git a/proposals/3981-relations-recursion.md b/proposals/3981-relations-recursion.md index 8fc2d074ff2..2863baf8b45 100644 --- a/proposals/3981-relations-recursion.md +++ b/proposals/3981-relations-recursion.md @@ -10,27 +10,23 @@ Threads was one of the first usages of this API that allowed nested relations - 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. -This forms a tree of relations, which can currently only be traversed -efficiently in hierarchical, but not in chronological order. Yet, for some -functionality – e.g., to determine which event a read receipt should -reference as per [MSC3771] – chronological order is necessary. +This forms a tree of relations. A client wanting to display a thread will want +to display reactions and edits to messages in the thread, and will therefore need +the second-order related events in addition to just the events with a direct thread +relation to the root. -Previously, clients would be unable to obtain a consistent ordering of -events in threads or related to threads. -Workarounds such as sending a separate `/relations` request per individual -message in each thread are only able to approximate the actual ordering, -as they rely on timestamps. - -If a homeserver announces support for this feature, a client can stop this -fallback behavior and instead just append the recurse parameter to the initial -request. +Clients can recursively perform the /relations queries on each event but this is +very slow and does not give the client any information on how the events are ordered +for the purpose of sending read receipts. ## Proposal It is proposed to add the `recurse` parameter to the `/relations` API, defined as follows: -> Whether to include events which relate only indirectly to the given event. +> Whether to additionally include events which only relate indirectly to the +> given event, +> ie. events related to the root events via one or more direct relationships. > > If set to false, only events which have direct a relation with the given > event will be included. @@ -39,34 +35,31 @@ as follows: > events that relate to the given event, will be included. > > It is recommended that at least 3 levels of relationships are traversed. -> Implementations should be careful to not infinitely recurse. +> Implementations may perform more but should be careful to not infinitely recurse. > > One of: `[true false]`. In order to be backwards compatible the `recurse` parameter must be optional (defaulting to `false`). -In this situation, topological ordering is intended to refer to the same -ordering that would be applied to the events when requested via the `/messages` -api given the same `dir` parameter. - Regardless of the value of the `recurse` parameter, events will always be -returned in topological ordering. Pagination and limiting shall also apply to -topological ordering. +returned in topological ordering, ie. the same order in which the `/messages` API +would return them (given the same `dir` parameter). -Filters specified via `event_type` or `rel_type` will be applied to leaves -and intermediate events on the graph formed by the related events. -Events that would match the filter, but whose only relation to the original -given event is through a non-matching intermediate event, will not be included. +Filters specified via `event_type` or `rel_type` will be applied to all events +returned, whether direct or indirect relations. Events that would match the filter, +but whose only relation to the original given event is through a non-matching +intermediate event, will not be included. This means that supplying a `rel_type` +parameter of `m.thread` is not appropriate for fetching all events in a thread since +relations to the threaded events would be filtered out. For this purpose, clients should +omit the `rel_type` parameter and perform any necessary filtering on the client side. ## Potential issues Naive implementations might be tempted to provide support for this parameter through a thin shim which is functionally identical to the client doing -separate recursive `/relations` requests itself. This is ill-advised. - -Such an implementation would, given a specifically crafted set of events, -allow a client to cause unreasonable load. +separate recursive `/relations` requests itself. However this would allow a +client to craft a set of events that would cause unreasonable load. ## Alternatives From a02735413535fc341b3fba47fdd7ce5c56c48d9f Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 3 Jan 2024 10:30:58 +0000 Subject: [PATCH 22/25] Add note that security is discussed elsewhere. Co-authored-by: Travis Ralston --- proposals/3981-relations-recursion.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/3981-relations-recursion.md b/proposals/3981-relations-recursion.md index 2863baf8b45..1af30688872 100644 --- a/proposals/3981-relations-recursion.md +++ b/proposals/3981-relations-recursion.md @@ -78,7 +78,7 @@ client to craft a set of events that would cause unreasonable load. ## Security considerations -None. +Security considerations are discussed inline throughout this proposal. ## Examples From 6ace9f4db8c72f616c66631d8b05617570139b2d Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 3 Jan 2024 13:47:51 +0000 Subject: [PATCH 23/25] Add recursion_depth response parameter. --- proposals/3981-relations-recursion.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/proposals/3981-relations-recursion.md b/proposals/3981-relations-recursion.md index 1af30688872..0f338b49feb 100644 --- a/proposals/3981-relations-recursion.md +++ b/proposals/3981-relations-recursion.md @@ -46,6 +46,21 @@ Regardless of the value of the `recurse` parameter, events will always be returned in topological ordering, ie. the same order in which the `/messages` API would return them (given the same `dir` parameter). +It is also proposed to add a response parameter, `recursion_depth` to the response +which gives the actual depth limit the server used in its recursion. This key is mandatory if +the `recurse` parameter was passed and is absent otherwise. eg: + +```json +{ + "chunk": [...], + "recursion_depth": 3 +} +``` + +Note that there no way in this MSC for a client to affect how much the server recurses. +If the client decides that the server's recursion level is insufficient, it could, for example, +perform the recursion manually, or disable whatever feature requires more recursion. + Filters specified via `event_type` or `rel_type` will be applied to all events returned, whether direct or indirect relations. Events that would match the filter, but whose only relation to the original given event is through a non-matching From c160d3606f0adb8c7641856fa1e75297279d03a2 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 3 Jan 2024 14:08:41 +0000 Subject: [PATCH 24/25] Note that recursion_depth is sent un-prefixed. --- proposals/3981-relations-recursion.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/proposals/3981-relations-recursion.md b/proposals/3981-relations-recursion.md index 0f338b49feb..914ee3a6110 100644 --- a/proposals/3981-relations-recursion.md +++ b/proposals/3981-relations-recursion.md @@ -140,6 +140,9 @@ on [`/versions`](https://spec.matrix.org/v1.7/client-server-api/#get_matrixclien Clients are also required to use `org.matrix.msc3981.recurse` in place of `recurse` at this time. +`recursion_depth` is always used un-namespaced (it would only ever be sent +if the client had already sent the recurse parameter). + ### Once the MSC is in a spec version Once this MSC becomes a part of a spec version, clients should rely on the From bdae5772768bbbd142f58ac0be0ea2a807c2f25d Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 16 Jan 2024 11:43:37 +0000 Subject: [PATCH 25/25] Add summary of security discussions --- proposals/3981-relations-recursion.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/proposals/3981-relations-recursion.md b/proposals/3981-relations-recursion.md index 914ee3a6110..9a7ae79b643 100644 --- a/proposals/3981-relations-recursion.md +++ b/proposals/3981-relations-recursion.md @@ -93,7 +93,11 @@ client to craft a set of events that would cause unreasonable load. ## Security considerations -Security considerations are discussed inline throughout this proposal. +Security considerations are discussed inline throughout this proposal. To summarise: + * Allowing a client to control recursion depth could allow a client to cause outsize + load on the server if the server doesn't check the recursion depth. + * Naive server implementations could allow a client to craft a set of events that would + cause high load. ## Examples