From 0b5dab51e7e9e01d3f5cfc3b6c288ad83cb49c55 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Mon, 4 Feb 2019 22:14:50 +0000 Subject: [PATCH 01/46] a very WIP draft of an aggregations MSC to replace #441 --- proposals/1849-aggregations.md | 153 +++++++++++++++++++++++++++++++++ 1 file changed, 153 insertions(+) create mode 100644 proposals/1849-aggregations.md diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md new file mode 100644 index 00000000000..0536d873320 --- /dev/null +++ b/proposals/1849-aggregations.md @@ -0,0 +1,153 @@ +# Proposal for aggregations via m.relates_to + +> WIP WIP WIP WIP WIP WIP WIP WIP WIP WIP +> +> Checking this in so i can link to it, even though it's very much an informal draft + +Today, replies looks like: + +```json +"type": "m.room.message", +"contents": { + "m.relates_to": { + "m.in_reply_to": { + "event_id": "$another:event.com" + } + } +} +``` + +`m.relates_to` is the signal to the server that the fields within describe aggregation operations. + +This is a bit clunky as for other types of relations, we end up duplicating the "m.in_reply_to" +type between the event type and the relationship type. +So instead, perhaps we should only specify a relationship type if strictly needed, e.g.: + +```json +"type": "m.room.message", +"contents": { + "m.relates_to": { + "event_id": "$another:event.com", + "type": "m.reply" + } +} +``` + +and + +```json +"type": "m.reaction", +"contents": { + "m.text": "πŸ‘", + "m.relates_to": { + "event_id": "$another:event.com", + } +} +``` + +Problem: + * Should the payload be inside or outside the E2E payload? + * For reaction data to be useful for server-side calculated reputation work they have to be available outside E2E. But perhaps we should be doing reputation analysis clientside instead? + * Folks who publish reputation data could deliberately do so aggregated, and outside E2E. They could also lie through their teeth, but they can do that anyway. + * We will never be able to do smart aggregations over federation if these are E2E encrypted, as the server won't know that 5 x πŸ‘ == 5πŸ‘. + +And then the server just blitzes through `m.relates_to.event_id` and builds up all the relationships based on that field. +This is kinda similar to pik's proposal below, but without the JSON schema. + +## CS API considerations + +Then, whenever the event is served down the CS API, we inline all the relationships for a given event (modulo filters)? + +For edits, we'd want the most recent relation (by default) +For reactions, we want all the reaction objects (or ideally their sum?) +For replies, we don't want the original at all; the client can load it if needed via /context. + +Do we send the events down via normal pagination? Or do we inline them? + We should probably do both: updates come in via pagination, but when doing a limited sync, + we should probably include them as inline as a basis. + +This in turn is related to LL members: ideally we wouldn't re-load LL members from scratch after +a limited sync, but just load the difference somehow. Similarly, we don't want to do a full snapshot +of all reactions after a limited sync, but just the base delta that changed. Perhaps this is okay, though: +limited syncs can just include the number of reactions. + +This is similar to how redactions are calculated today. We just build up a table of which events reference +which other events, and expand out the 'join' when syncing. + +So: + +```json +"type": "m.room.message", +"event_id": "$another:event.com", +"contents": { + "m.text": "I have an excellent idea", +}, +"relations": [ + { + "type": "m.reaction", + "event_id": "$reaction:alice.com", + "sender": "@alice:alice.com", + "contents": { + "m.text": "πŸ‘", + } + }, + { + "type": "m.reaction", + "event_id": "$reaction2:bob.com", + "sender": "@bob:bob.com", + "contents": { + "m.text": "πŸ‘Ž", + } + }, +] +``` + +## Federation considerations + +In general, no special considerations are needed for federation; relational events are just sent as needed over federation +same as any other event type. + +We need some mechanism to know whether we have synced all the relational events that exist for a given event, however. +Perhaps we should inline them into the original events, as we do on the CS API. + +In future, we might want to aggregate relational events (e.g. send a summary of the events, rather than the +actual original events). This requires the payload to be non-E2E encrypted, and would also require some kind of +challenge-response mechanism to prove that the summary is accurate to the recipients (a ZK mechanism of some kind). +In some ways this is a subset of the more general problem of how we can efficiently send summaries of rooms and even +room state over federation without having to send all the events up front. + +## Historical context + +pik's MSC441 has: + +Define the JSON schema for the aggregation event, so the server can work out which fields should be aggregated. + +```json +"type": "m.room._aggregation.emoticon", +"contents": { + "emoticon": "::smile::", + "msgtype": "?", + "target_id": "$another:event.com" +} +``` + +These would then aggregated, based on target_id, and returned as annotations on the source event in an +`aggregation_data` field: + +```json +"contents": { + ... + "aggregation_data": { + "m.room._aggregation.emoticon": { + "aggregation_data": [ + { + "emoticon": "::smile::", + "event_id": "$14796538949JTYis:pik-test", + "sender": "@pik:pik-test" + } + ], + "latest_event_id": "$14796538949JTYis:pik-test" + } + } +} +``` \ No newline at end of file From e03457d7884a748b4de937b257903f92ee5f80f5 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Fri, 5 Apr 2019 04:06:28 +0100 Subject: [PATCH 02/46] incorporate notes from sync with erik --- proposals/1849-aggregations.md | 102 ++++++++++++++++++++++++++------- 1 file changed, 82 insertions(+), 20 deletions(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index 0536d873320..9e01e8d81b2 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -1,8 +1,8 @@ # Proposal for aggregations via m.relates_to > WIP WIP WIP WIP WIP WIP WIP WIP WIP WIP -> -> Checking this in so i can link to it, even though it's very much an informal draft + +A very rough WIP set of notes on how relations could work in Matrix. Today, replies looks like: @@ -45,15 +45,11 @@ and } ``` -Problem: - * Should the payload be inside or outside the E2E payload? - * For reaction data to be useful for server-side calculated reputation work they have to be available outside E2E. But perhaps we should be doing reputation analysis clientside instead? - * Folks who publish reputation data could deliberately do so aggregated, and outside E2E. They could also lie through their teeth, but they can do that anyway. - * We will never be able to do smart aggregations over federation if these are E2E encrypted, as the server won't know that 5 x πŸ‘ == 5πŸ‘. - And then the server just blitzes through `m.relates_to.event_id` and builds up all the relationships based on that field. This is kinda similar to pik's proposal below, but without the JSON schema. +`event_id` should take either a string or a list of strings, to support relation DAGs (needed for ordering edits) + ## CS API considerations Then, whenever the event is served down the CS API, we inline all the relationships for a given event (modulo filters)? @@ -62,17 +58,14 @@ For edits, we'd want the most recent relation (by default) For reactions, we want all the reaction objects (or ideally their sum?) For replies, we don't want the original at all; the client can load it if needed via /context. -Do we send the events down via normal pagination? Or do we inline them? - We should probably do both: updates come in via pagination, but when doing a limited sync, - we should probably include them as inline as a basis. +We should send the aggregated event down during normal pagination, +as well as the individual relations down incrementally during sync. -This in turn is related to LL members: ideally we wouldn't re-load LL members from scratch after -a limited sync, but just load the difference somehow. Similarly, we don't want to do a full snapshot -of all reactions after a limited sync, but just the base delta that changed. Perhaps this is okay, though: -limited syncs can just include the number of reactions. +After a limited sync, we should send a fresh aggregated event rather +than try to calculate a delta. This is similar to how redactions are calculated today. We just build up a table of which events reference -which other events, and expand out the 'join' when syncing. +which other events, and expand them out when syncing. So: @@ -102,13 +95,82 @@ So: ] ``` +## Sending relations + +```json +PUT /_matrix/client/r0/rooms/{roomId}/send_relation/{parent_id}/{eventType}/{txnId} +{ + "m.text": "πŸ‘", +} +``` + +N.B. that the server then gets to populate out the m.relates_to field itself, +adding the `parent_id` as one parent, but also adding any other dangling relations-dag extremitie. + +## Receiving relations + +TODO: + +/sync +/messages +/context + +## Pagination considerations + +How do we handle 20K edits in a row? + * we need to paginate 'vertically' somehow + +How do we handle a message with 20K different emojis? + * we need to paginate 'horizontally' somehow - return the 10 most popular emojis? + +Do relations automatically give us threads somehow? + * No; we will at the least need to define how to permalink to a relation then and then paginate around it. + +## Edge cases + +XXX: What happens when you react to an edit? + * You should be able to, but the reaction should be attributed to the edit (or its contents) rather than the message as a whole. + * So how do you aggregate? + +How do you handle racing edits? + * The edits could form a DAG of relations for robustness. + * Tie-break between forward DAG extremities based on origin_ts + * m.relates_to should be able to take an array of event_ids. + * ...or do we just always tiebreak on origin_ts, and rely on a social problem for it not to be abused? + * problem is that other relation types might well need a more robust way of ordering. + +Redactions + * Redacting an edited event in the UI should redact the original; the client will need to redact the original event to make this happen. + * Clients could also try to expand the relations and redact those too if they wanted to, but in practice the server shouldn't send down relations to redacted messages, so it's overkill. + * You can also redact specific relations if needed (e.g. to remove a reaction from ever happening) + * If you redact an relation, we keep the relation DAG (and solve that metadata leak alongside our others) + +What does it mean to call /context on a relation? + * We should probably just return the root event for now, and then refine it in future for threading? + +## E2E considerations + +In E2E rooms: + * The payload should be encrypted. This means that we can't sum emoji reactions serverside; + they'll have to be passed around one by one. Given E2E rooms tend to be smaller, this is + hopefully not a major problem. We could reduce bandwidth by reusing the same key to + encrypt the relations as the original message. + * This means that reputation data can't be calculated serverside for E2E rooms however. + * It might be okay to calculate it clientside? Or we could special-case reputation data to not be E2E? + * The m.relates_to field however should not be encrypted, so that the server can use it for + performing aggregations where possible (e.g. returning only the most recent edit). + ## Federation considerations In general, no special considerations are needed for federation; relational events are just sent as needed over federation -same as any other event type. - -We need some mechanism to know whether we have synced all the relational events that exist for a given event, however. -Perhaps we should inline them into the original events, as we do on the CS API. +same as any other event type - aggregated onto the original event if needed. + +XXX: We have a problem with resynchronising relations after a gap in federation. +We have no way of knowing that an edit happened in the gap to one of the events +we already have. So, we'll show inconsistent data until we backfill the gap. + * We could write this off as a limitation. + * Or we could make relations a DAG, so we can spot holes at the next relation, and + go walk the DAG to pull in the missing relations? In future, we might want to aggregate relational events (e.g. send a summary of the events, rather than the actual original events). This requires the payload to be non-E2E encrypted, and would also require some kind of From 8567149438dbc37fb80d4d664f94f712d4f24a55 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Fri, 5 Apr 2019 10:49:22 +0100 Subject: [PATCH 03/46] shower thoughts --- proposals/1849-aggregations.md | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index 9e01e8d81b2..1fbdf391ad5 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -45,14 +45,26 @@ and } ``` +or if building a relation DAG (for ordering for edits, and for pulling in missing relations after a federation outage): + +```json +"type": "m.reaction", +"contents": { + "m.text": "πŸ‘", + "m.relates_to": { + "event_id": ["$another:event.com", "$another2:event.com"] + } +} +``` + And then the server just blitzes through `m.relates_to.event_id` and builds up all the relationships based on that field. This is kinda similar to pik's proposal below, but without the JSON schema. -`event_id` should take either a string or a list of strings, to support relation DAGs (needed for ordering edits) +XXX: `event_id` should take either a string or a list of strings, to support relation DAGs (needed for ordering edits) ## CS API considerations -Then, whenever the event is served down the CS API, we inline all the relationships for a given event (modulo filters)? +Then, whenever the event is served down the CS API, we inline the relations for a given event (modulo filters)? For edits, we'd want the most recent relation (by default) For reactions, we want all the reaction objects (or ideally their sum?) @@ -97,8 +109,8 @@ So: ## Sending relations -```json PUT /_matrix/client/r0/rooms/{roomId}/send_relation/{parent_id}/{eventType}/{txnId} +```json { "m.text": "πŸ‘", } @@ -111,9 +123,9 @@ adding the `parent_id` as one parent, but also adding any other dangling relatio TODO: -/sync -/messages -/context + * /sync + * /messages + * /context ## Pagination considerations @@ -169,8 +181,11 @@ XXX: We have a problem with resynchronising relations after a gap in federation. We have no way of knowing that an edit happened in the gap to one of the events we already have. So, we'll show inconsistent data until we backfill the gap. * We could write this off as a limitation. - * Or we could make relations a DAG, so we can spot holes at the next relation, and - go walk the DAG to pull in the missing relations? + * Or we could make *ALL* relations a DAG, so we can spot holes at the next relation, and + go walk the DAG to pull in the missing relations? Then, the next relation for an event + could pull in any of the missing relations. + * Could we also ask the server, after a gap, to provide all the relations which happened during the gap to events whose root preceeded the gap. + * "Give me all relations which happened between this set of forward-extremities when I lost sync, and the point i've rejoined the DAG, for events which preceeded the gap"? In future, we might want to aggregate relational events (e.g. send a summary of the events, rather than the actual original events). This requires the payload to be non-E2E encrypted, and would also require some kind of From ed2a3a0d8e921c5632e2f6571d1a76770a983d97 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Fri, 5 Apr 2019 23:53:00 +0100 Subject: [PATCH 04/46] more notes from today's sync with erik --- proposals/1849-aggregations.md | 84 ++++++++++++++++++++++++---------- 1 file changed, 60 insertions(+), 24 deletions(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index 1fbdf391ad5..cb9d3d61a21 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -45,23 +45,9 @@ and } ``` -or if building a relation DAG (for ordering for edits, and for pulling in missing relations after a federation outage): - -```json -"type": "m.reaction", -"contents": { - "m.text": "πŸ‘", - "m.relates_to": { - "event_id": ["$another:event.com", "$another2:event.com"] - } -} -``` - And then the server just blitzes through `m.relates_to.event_id` and builds up all the relationships based on that field. This is kinda similar to pik's proposal below, but without the JSON schema. -XXX: `event_id` should take either a string or a list of strings, to support relation DAGs (needed for ordering edits) - ## CS API considerations Then, whenever the event is served down the CS API, we inline the relations for a given event (modulo filters)? @@ -70,17 +56,23 @@ For edits, we'd want the most recent relation (by default) For reactions, we want all the reaction objects (or ideally their sum?) For replies, we don't want the original at all; the client can load it if needed via /context. +Proposal: start off syncing the relations individually, rather than summing reactions. +For edits, the server can be smart and send the most recent msg. + We should send the aggregated event down during normal pagination, as well as the individual relations down incrementally during sync. After a limited sync, we should send a fresh aggregated event rather -than try to calculate a delta. +than try to calculate a delta (possibly minus the contents field of +the original event) This is similar to how redactions are calculated today. We just build up a table of which events reference which other events, and expand them out when syncing. So: +Without summing: + ```json "type": "m.room.message", "event_id": "$another:event.com", @@ -90,23 +82,53 @@ So: "relations": [ { "type": "m.reaction", - "event_id": "$reaction:alice.com", - "sender": "@alice:alice.com", "contents": { + "event_id": "$reaction:alice.com", + "sender": "@alice:alice.com", "m.text": "πŸ‘", } }, { "type": "m.reaction", - "event_id": "$reaction2:bob.com", - "sender": "@bob:bob.com", "contents": { + "event_id": "$reaction:alice.com", + "sender": "@alice:alice.com", "m.text": "πŸ‘Ž", } }, + // etc... ] ``` +With summing, in future: + +```json +"type": "m.room.message", +"event_id": "$another:event.com", +"contents": { + "m.text": "I have an excellent idea", +}, +"relations": [ + { + "type": "m.reaction", + "contents": { + "m.text": "πŸ‘", + "count": 120, + } + }, + { + "type": "m.reaction", + "contents": { + "m.text": "πŸ‘Ž", + "count": 42, + } + }, +] +``` + +XXX: if we are doing sums, we need to worry about races between aggregated events +and the individual relation deltas in /sync responss. + ## Sending relations PUT /_matrix/client/r0/rooms/{roomId}/send_relation/{parent_id}/{eventType}/{txnId} @@ -140,16 +162,21 @@ Do relations automatically give us threads somehow? ## Edge cases -XXX: What happens when you react to an edit? +What happens when you react to an edit? * You should be able to, but the reaction should be attributed to the edit (or its contents) rather than the message as a whole. * So how do you aggregate? + * Suggestion: edits gather their own reactions, and the clients should display the reactions on the most recent edit. + * This provides a social pressure to get your edits in quickly before there are many reactions, otherwise the reactions will get lost. + * And it avoids us randomly aggregating reactions to potentially very different contents of messages. How do you handle racing edits? * The edits could form a DAG of relations for robustness. * Tie-break between forward DAG extremities based on origin_ts * m.relates_to should be able to take an array of event_ids. - * ...or do we just always tiebreak on origin_ts, and rely on a social problem for it not to be abused? - * problem is that other relation types might well need a more robust way of ordering. + * hard to see who is responsible for linearising the DAG when receiving. Nasty for the client to do it, but the server would have to buffer, meaning relations could get stuck if an event in the DAG is unavailable. + * ...or do we just always order by on origin_ts, and rely on a social problem for it not to be abused? + * problem is that other relation types might well need a more robust way of ordering. XXX: can we think of any? + * could add the DAG in later if it's really needed? Redactions * Redacting an edited event in the UI should redact the original; the client will need to redact the original event to make this happen. @@ -177,15 +204,24 @@ In E2E rooms: In general, no special considerations are needed for federation; relational events are just sent as needed over federation same as any other event type - aggregated onto the original event if needed. -XXX: We have a problem with resynchronising relations after a gap in federation. +We have a problem with resynchronising relations after a gap in federation: We have no way of knowing that an edit happened in the gap to one of the events we already have. So, we'll show inconsistent data until we backfill the gap. * We could write this off as a limitation. * Or we could make *ALL* relations a DAG, so we can spot holes at the next relation, and go walk the DAG to pull in the missing relations? Then, the next relation for an event - could pull in any of the missing relations. + could pull in any of the missing relations. Socially this probably doesn't work as reactions + will likely drop-off over time, so by the time your server comes back there won't be any more + reactions pulling the missing ones in. * Could we also ask the server, after a gap, to provide all the relations which happened during the gap to events whose root preceeded the gap. * "Give me all relations which happened between this set of forward-extremities when I lost sync, and the point i've rejoined the DAG, for events which preceeded the gap"? + * Would be hard to auth all the relations which this api coughed up. + * We could auth them based only the auth events of the relation, except we lose the context of the nearby DAG which we'd have if it was a normal backfilled event. + * As a result it would be easier for a server to retrospectively lie about events on behalf of its users. + * This probably isn't the end of the world, plus it's more likely to be consistent than if we leave a gap. + * i.e. it's better to consistent with a small chance of being maliciously wrong, than inconsistent with a guaranteed chance of being innocently wrong. + * We'd need to worry about pagination. + * This is probably the best solution, but can also be added as a v2. In future, we might want to aggregate relational events (e.g. send a summary of the events, rather than the actual original events). This requires the payload to be non-E2E encrypted, and would also require some kind of From 07050a415db8b027669d0118d4ca3492c28f64c6 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 30 Apr 2019 14:21:07 +0100 Subject: [PATCH 05/46] Update with new proposal --- proposals/1849-aggregations.md | 352 +++++++++++++++++++-------------- 1 file changed, 200 insertions(+), 152 deletions(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index cb9d3d61a21..4fcb8639eb6 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -1,6 +1,6 @@ # Proposal for aggregations via m.relates_to -> WIP WIP WIP WIP WIP WIP WIP WIP WIP WIP +> WIP WIP WIP WIP WIP WIP WIP WIP WIP WIP A very rough WIP set of notes on how relations could work in Matrix. @@ -17,223 +17,271 @@ Today, replies looks like: } ``` -`m.relates_to` is the signal to the server that the fields within describe aggregation operations. +`m.relates_to` is the signal to the server that the fields within describe +aggregation operations. -This is a bit clunky as for other types of relations, we end up duplicating the "m.in_reply_to" -type between the event type and the relationship type. -So instead, perhaps we should only specify a relationship type if strictly needed, e.g.: +We would like to add support for other types of relations, including message +editing and reactions. -```json -"type": "m.room.message", -"contents": { - "m.relates_to": { - "event_id": "$another:event.com", - "type": "m.reply" - } -} -``` -and +## Types of relations -```json -"type": "m.reaction", -"contents": { - "m.text": "πŸ‘", - "m.relates_to": { - "event_id": "$another:event.com", - } -} -``` +There are three broad types of relations: annotations, replacements and +references. -And then the server just blitzes through `m.relates_to.event_id` and builds up all the relationships based on that field. -This is kinda similar to pik's proposal below, but without the JSON schema. +Annotations are things like reactions, which should be displayed alongside the +original event. These should support be aggregated so that e.g. if twenty people +"likes" an event we can bundle the twenty events together when sending the +original event to clients. Another usage of an annotation is e.g. for bots, who +could use annotations to report the success/failure or progress of a command. -## CS API considerations +Replacements are essentially edits, and indicate that instead of giving clients +the original event they should be handed the replacement event instead. Clients +should be able to request all replacements of an event, i.e. the "edit history". -Then, whenever the event is served down the CS API, we inline the relations for a given event (modulo filters)? +References things like replies, where a later event refers to an earlier event +in some way. The server should include references when sending an event to the +client so they can display the number of replies, and navigate easily to them. -For edits, we'd want the most recent relation (by default) -For reactions, we want all the reaction objects (or ideally their sum?) -For replies, we don't want the original at all; the client can load it if needed via /context. +These types effect how the server bundles the related events with the original, +and so the type must be known to servers when handling relations. However, the +exact semantics of a particular relation only needs to be known by clients. This +means that if we include the relation type in the related event we can use the +event type to easily add new types of e.g annotations without requiring server +side support. -Proposal: start off syncing the relations individually, rather than summing reactions. -For edits, the server can be smart and send the most recent msg. -We should send the aggregated event down during normal pagination, -as well as the individual relations down incrementally during sync. +## Aggregating and paginating relations -After a limited sync, we should send a fresh aggregated event rather -than try to calculate a delta (possibly minus the contents field of -the original event) +In large rooms an event may end up having a large number of related events, and +so we do not want to have to include all relations when sending the event to the +client. How we limit depends on the relation type. -This is similar to how redactions are calculated today. We just build up a table of which events reference -which other events, and expand them out when syncing. +Annotations are grouped by their event type and an "aggregation key", and the +top N groups with the highest number is included in the event. For example, +reactions would be implemented as a `m.reaction` with aggration key of e.g. +`πŸ‘`. -So: + TODO: Should we include anything other than event type, aggregation key and + count? -Without summing: + +Replacements replace the original event, and so no aggregation is required. +Though care must be taken by the server to ensure that if there are multiple +replacement events it consistently chooses the same one as all other servers. +The replacement event should also include a reference to the original event ID +so that clients can tell that the message has been edited. + +For references the original event should include the list of `type` and +`event_id` of the earliest N references. + + TODO: Do we need the type? Do we want to differentiate between replies and + other types of references? This assumes the type of the related event gives + some hint to clients. + +In each case where we limit what is included there should be a corresponding API +to paginate the full sets of events. Annotations would need APIs for both +fetching more groups and fetching events in a group. + + +## Event format + +All the information about the relation is put under `m.relates_to` key. + +A reply would look something like: ```json -"type": "m.room.message", -"event_id": "$another:event.com", -"contents": { - "m.text": "I have an excellent idea", -}, -"relations": [ - { - "type": "m.reaction", - "contents": { - "event_id": "$reaction:alice.com", - "sender": "@alice:alice.com", - "m.text": "πŸ‘", - } - }, - { - "type": "m.reaction", - "contents": { - "event_id": "$reaction:alice.com", - "sender": "@alice:alice.com", - "m.text": "πŸ‘Ž", +{ + "type": "m.room.message", + "contents": { + "m.relates_to": { + "type": "m.reply", + "event_id": "$some_event_id" } - }, - // etc... -] + } +} ``` -With summing, in future: +And a reaction might look like the following, where we define for `m.reaction` +that the aggregation key is the unicode reaction itself. ```json -"type": "m.room.message", -"event_id": "$another:event.com", -"contents": { - "m.text": "I have an excellent idea", -}, -"relations": [ - { - "type": "m.reaction", - "contents": { - "m.text": "πŸ‘", - "count": 120, - } - }, - { - "type": "m.reaction", - "contents": { - "m.text": "πŸ‘Ž", - "count": 42, +{ + "type": "m.reaction", + "contents": { + "m.relates_to": { + "type": "m.annotation", + "event_id": "$some_event_id", + "aggregation_key": "πŸ‘" } - }, -] + } +} ``` -XXX: if we are doing sums, we need to worry about races between aggregated events -and the individual relation deltas in /sync responss. + TODO: This limits an event to only having one relation, on the assumption + that there are no use cases and that it will make life simpler. -## Sending relations -PUT /_matrix/client/r0/rooms/{roomId}/send_relation/{parent_id}/{eventType}/{txnId} +An event that has relations might look something like: + ```json { - "m.text": "πŸ‘", + ..., + "unsigned": { + "m.relations": { + "m.annotation": [ + { + "type": "m.reaction", + "aggregation_key": "πŸ‘", + "count": 3 + } + ], + "m.reference": { + "chunk": [ + { + "type": "m.room.message", + "event_id": "$some_event" + } + ], + "limited": false, + "count": 1 + } + } + } } ``` -N.B. that the server then gets to populate out the m.relates_to field itself, -adding the `parent_id` as one parent, but also adding any other dangling relations-dag extremitie. -## Receiving relations +## End to end encryption -TODO: +Since the server bundles related events the relation information must not be +encrypted. - * /sync - * /messages - * /context +For aggregations of annotations there are two options: -## Pagination considerations +1. Don't group together annotations and have the aggregation_key encrypted, so + as to not leak how someone reacted (though server would still see that they + did). +2. In some way encrypt the `aggregation_key`, with the properties that different + users and clients reacting in the same way to the same event produce the same + `aggregation_key`, but isn't something the server can calculate and is + different between different events (to stop statistical analysis). Clients + also need to be able to go from encrypted `aggregation_key` to the actual + reaction. -How do we handle 20K edits in a row? - * we need to paginate 'vertically' somehow + One suggestion here was to use the decryption key of the event as a base for + a shared secret. -How do we handle a message with 20K different emojis? - * we need to paginate 'horizontally' somehow - return the 10 most popular emojis? -Do relations automatically give us threads somehow? - * No; we will at the least need to define how to permalink to a relation then and then paginate around it. +## CS API + +Sending a related event uses an equivalent of the normal send API (with an +equivalent `PUT` API): + +``` +POST /_matrix/client/r0/rooms/{roomId}/send_relation/{parent_id}/{relation_type}/{event_type} +{ + // event contents +} +``` + +Whenever an event that has relations is sent to the client, e.g. pagination, +event search etc, the server bundles the relations into the event as per above. + +The `parent_id` is: + - For annotations the event being displayed (which may be an edit) + - For replaces/edits the original event (not previous edits) + - For references should be the original event (?) + +The same happens in the sync API, howevr the client will need to handle new +relations themselves when they come down incremental sync. + ## Edge cases What happens when you react to an edit? - * You should be able to, but the reaction should be attributed to the edit (or its contents) rather than the message as a whole. + * You should be able to, but the reaction should be attributed to the edit (or + its contents) rather than the message as a whole. * So how do you aggregate? - * Suggestion: edits gather their own reactions, and the clients should display the reactions on the most recent edit. - * This provides a social pressure to get your edits in quickly before there are many reactions, otherwise the reactions will get lost. - * And it avoids us randomly aggregating reactions to potentially very different contents of messages. + * Suggestion: edits gather their own reactions, and the clients should display + the reactions on the most recent edit. + * This provides a social pressure to get your edits in quickly before there + are many reactions, otherwise the reactions will get lost. + * And it avoids us randomly aggregating reactions to potentially very + different contents of messages. How do you handle racing edits? * The edits could form a DAG of relations for robustness. * Tie-break between forward DAG extremities based on origin_ts - * m.relates_to should be able to take an array of event_ids. - * hard to see who is responsible for linearising the DAG when receiving. Nasty for the client to do it, but the server would have to buffer, meaning relations could get stuck if an event in the DAG is unavailable. - * ...or do we just always order by on origin_ts, and rely on a social problem for it not to be abused? - * problem is that other relation types might well need a more robust way of ordering. XXX: can we think of any? + * this should be different from the target event_id in the relations, to + make it easier to know what is being replaced. + * hard to see who is responsible for linearising the DAG when receiving. + Nasty for the client to do it, but the server would have to buffer, + meaning relations could get stuck if an event in the DAG is unavailable. + * ...or do we just always order by on origin_ts, and rely on a social problem + for it not to be abused? + * problem is that other relation types might well need a more robust way of + ordering. XXX: can we think of any? * could add the DAG in later if it's really needed? Redactions - * Redacting an edited event in the UI should redact the original; the client will need to redact the original event to make this happen. - * Clients could also try to expand the relations and redact those too if they wanted to, but in practice the server shouldn't send down relations to redacted messages, so it's overkill. - * You can also redact specific relations if needed (e.g. to remove a reaction from ever happening) - * If you redact an relation, we keep the relation DAG (and solve that metadata leak alongside our others) + * Redacting an edited event in the UI should redact the original; the client + will need to redact the original event to make this happen. + * Is this not problematic when trying to remove illegal content from servers? + * Clients could also try to expand the relations and redact those too if they + wanted to, but in practice the server shouldn't send down relations to + redacted messages, so it's overkill. + * You can also redact specific relations if needed (e.g. to remove a reaction + from ever happening) + * If you redact an relation, we keep the relation DAG (and solve that metadata + leak alongside our others) What does it mean to call /context on a relation? - * We should probably just return the root event for now, and then refine it in future for threading? - -## E2E considerations - -In E2E rooms: - * The payload should be encrypted. This means that we can't sum emoji reactions serverside; - they'll have to be passed around one by one. Given E2E rooms tend to be smaller, this is - hopefully not a major problem. We could reduce bandwidth by reusing the same key to - encrypt the relations as the original message. - * This means that reputation data can't be calculated serverside for E2E rooms however. - * It might be okay to calculate it clientside? Or we could special-case reputation data to not be E2E? - * The m.relates_to field however should not be encrypted, so that the server can use it for - performing aggregations where possible (e.g. returning only the most recent edit). + * We should probably just return the root event for now, and then refine it in + future for threading? ## Federation considerations -In general, no special considerations are needed for federation; relational events are just sent as needed over federation -same as any other event type - aggregated onto the original event if needed. +In general, no special considerations are needed for federation; relational +events are just sent as needed over federation same as any other event type - +aggregated onto the original event if needed. We have a problem with resynchronising relations after a gap in federation: We have no way of knowing that an edit happened in the gap to one of the events we already have. So, we'll show inconsistent data until we backfill the gap. * We could write this off as a limitation. - * Or we could make *ALL* relations a DAG, so we can spot holes at the next relation, and - go walk the DAG to pull in the missing relations? Then, the next relation for an event - could pull in any of the missing relations. Socially this probably doesn't work as reactions - will likely drop-off over time, so by the time your server comes back there won't be any more - reactions pulling the missing ones in. - * Could we also ask the server, after a gap, to provide all the relations which happened during the gap to events whose root preceeded the gap. - * "Give me all relations which happened between this set of forward-extremities when I lost sync, and the point i've rejoined the DAG, for events which preceeded the gap"? + * Or we could make *ALL* relations a DAG, so we can spot holes at the next + relation, and go walk the DAG to pull in the missing relations? Then, the + next relation for an event could pull in any of the missing relations. + Socially this probably doesn't work as reactions will likely drop-off over + time, so by the time your server comes back there won't be any more reactions + pulling the missing ones in. + * Could we also ask the server, after a gap, to provide all the relations which + happened during the gap to events whose root preceeded the gap. + * "Give me all relations which happened between this set of + forward-extremities when I lost sync, and the point i've rejoined the DAG, + for events which preceeded the gap"? * Would be hard to auth all the relations which this api coughed up. - * We could auth them based only the auth events of the relation, except we lose the context of the nearby DAG which we'd have if it was a normal backfilled event. - * As a result it would be easier for a server to retrospectively lie about events on behalf of its users. - * This probably isn't the end of the world, plus it's more likely to be consistent than if we leave a gap. - * i.e. it's better to consistent with a small chance of being maliciously wrong, than inconsistent with a guaranteed chance of being innocently wrong. + * We could auth them based only the auth events of the relation, except we + lose the context of the nearby DAG which we'd have if it was a normal + backfilled event. + * As a result it would be easier for a server to retrospectively lie about + events on behalf of its users. + * This probably isn't the end of the world, plus it's more likely to be + consistent than if we leave a gap. + * i.e. it's better to consistent with a small chance of being maliciously + wrong, than inconsistent with a guaranteed chance of being innocently + wrong. * We'd need to worry about pagination. * This is probably the best solution, but can also be added as a v2. -In future, we might want to aggregate relational events (e.g. send a summary of the events, rather than the -actual original events). This requires the payload to be non-E2E encrypted, and would also require some kind of -challenge-response mechanism to prove that the summary is accurate to the recipients (a ZK mechanism of some kind). -In some ways this is a subset of the more general problem of how we can efficiently send summaries of rooms and even -room state over federation without having to send all the events up front. ## Historical context pik's MSC441 has: -Define the JSON schema for the aggregation event, so the server can work out which fields should be aggregated. +Define the JSON schema for the aggregation event, so the server can work out +which fields should be aggregated. ```json "type": "m.room._aggregation.emoticon", @@ -244,8 +292,8 @@ Define the JSON schema for the aggregation event, so the server can work out whi } ``` -These would then aggregated, based on target_id, and returned as annotations on the source event in an -`aggregation_data` field: +These would then aggregated, based on target_id, and returned as annotations on +the source event in an `aggregation_data` field: ```json "contents": { @@ -263,4 +311,4 @@ These would then aggregated, based on target_id, and returned as annotations on } } } -``` \ No newline at end of file +``` From 25b879b9ce62ba85b24149c6126b72fac3f932f5 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 30 Apr 2019 14:34:49 +0100 Subject: [PATCH 06/46] fixup --- proposals/1849-aggregations.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index 4fcb8639eb6..0ee8956e440 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -95,7 +95,7 @@ A reply would look something like: "type": "m.room.message", "contents": { "m.relates_to": { - "type": "m.reply", + "type": "m.references", "event_id": "$some_event_id" } } @@ -189,9 +189,10 @@ Whenever an event that has relations is sent to the client, e.g. pagination, event search etc, the server bundles the relations into the event as per above. The `parent_id` is: - - For annotations the event being displayed (which may be an edit) - - For replaces/edits the original event (not previous edits) - - For references should be the original event (?) + + * For annotations the event being displayed (which may be an edit) + * For replaces/edits the original event (not previous edits) + * For references should be the original event (?) The same happens in the sync API, howevr the client will need to handle new relations themselves when they come down incremental sync. From f50de1fd90850c8be6ef0fe744cb6e37b8c7570b Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 30 Apr 2019 14:56:54 +0100 Subject: [PATCH 07/46] Add some notes about extended annotations --- proposals/1849-aggregations.md | 73 +++++++++++++++++++++++++++++++++- 1 file changed, 72 insertions(+), 1 deletion(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index 0ee8956e440..6274164369b 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -194,7 +194,7 @@ The `parent_id` is: * For replaces/edits the original event (not previous edits) * For references should be the original event (?) -The same happens in the sync API, howevr the client will need to handle new +The same happens in the sync API, however the client will need to handle new relations themselves when they come down incremental sync. @@ -277,6 +277,77 @@ we already have. So, we'll show inconsistent data until we backfill the gap. * This is probably the best solution, but can also be added as a v2. +## Extended annotation use case + +In future it might be useful to be able to annotate events with more +information, some examples include: + + * Annotate commit/PR notification messages with their associated CI state, e.g. + pending/passed/failed. + * If a user issues a command to a bot, e.g. `!deploy-site` the bot could + annotate that event with current state, like "acknowledged", + "redeploying...", "success", "failed", etc. + * Other use cases...? + +However, this doesn't really work with the proposed grouping, as the aggregation +key wouldn't contain the right information needed to display it (unlike for +reactions). + +One way to potentially support this is to include the events (or a subset of the +event) when grouping, so that clients have enough information to render them. +However this dramatically inceases the size of the parent event if we bundle the +full events inside, even if limit the number we bundle in. To reduce the +overhead the annotation event could include a `m.summary` field which gets +included. + +This would look something like the following, where the annotation is: + +```json +{ + "type": "m.bot_command_response", + "content": { + "m.summary": { + "state": "success", + }, + "m.relates_to": { + "type": "m.annotation", + "annotation_key": "" + } + } +} +``` + +and gets bundled into an event like: + +```json +{ + "unsigned": { + "m.relations": { + "m.annotation": [ + { + "type": "m.bot_command_response", + "aggregation_key": "", + "count": 1, + "chunk": [ + { + "state": "success", + } + ], + "limited": false, + } + ] + } + } +} +``` + +This is something that could be added later on. A few issues with this are: + + * How does this work with end to end? How do we encrypt the `m.summary`? + * We would end up including old annotations that had been superceded, should + these be done via edits instead? + + ## Historical context pik's MSC441 has: From ebb25e09c849861e9a28399aaa3df1a78d8c652e Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Tue, 30 Apr 2019 16:51:00 +0100 Subject: [PATCH 08/46] further updates from RLing with Erik --- proposals/1849-aggregations.md | 57 +++++++++++++++++++++++++++------- 1 file changed, 45 insertions(+), 12 deletions(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index 6274164369b..a67c1e41b38 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -94,6 +94,7 @@ A reply would look something like: { "type": "m.room.message", "contents": { + "body": "i <3 shelties", "m.relates_to": { "type": "m.references", "event_id": "$some_event_id" @@ -103,7 +104,7 @@ A reply would look something like: ``` And a reaction might look like the following, where we define for `m.reaction` -that the aggregation key is the unicode reaction itself. +that the aggregation `key` is the unicode reaction itself. ```json { @@ -112,7 +113,7 @@ that the aggregation key is the unicode reaction itself. "m.relates_to": { "type": "m.annotation", "event_id": "$some_event_id", - "aggregation_key": "πŸ‘" + "key": "πŸ‘" } } } @@ -132,7 +133,7 @@ An event that has relations might look something like: "m.annotation": [ { "type": "m.reaction", - "aggregation_key": "πŸ‘", + "key": "πŸ‘", "count": 3 } ], @@ -159,18 +160,18 @@ encrypted. For aggregations of annotations there are two options: -1. Don't group together annotations and have the aggregation_key encrypted, so +1. Don't group together annotations and have the aggregation `key` encrypted, so as to not leak how someone reacted (though server would still see that they did). -2. In some way encrypt the `aggregation_key`, with the properties that different +2. In some way encrypt the aggregation `key`, with the properties that different users and clients reacting in the same way to the same event produce the same - `aggregation_key`, but isn't something the server can calculate and is + `key`, but isn't something the server can calculate and is different between different events (to stop statistical analysis). Clients - also need to be able to go from encrypted `aggregation_key` to the actual + also need to be able to go from encrypted `key` to the actual reaction. - One suggestion here was to use the decryption key of the event as a base for - a shared secret. + One suggestion here was to use the message key of the event to encrypt the + aggregation `key`. ## CS API @@ -197,6 +198,24 @@ The `parent_id` is: The same happens in the sync API, however the client will need to handle new relations themselves when they come down incremental sync. +##Β Pagination + +We need to paginate over: + * The relations of a given event, via /messages? or /context? or something else? + * For replacements (i.e. edits) we get a paginated list of all edits on the source event + * Should permalinks point to the most recent revision of a given edit, or the original one? + * Permalinks should capture the event ID that the sender is viewing at that point (which might be an edit ID) + * The receiver should resolve this ID to the source event ID, and then display the most recent version that event. + * Groups of annotations + * Need to paginate across the different groups (i.e. how many different reactions of different types did it get?) + * List all the reactions individually per group for this message + * List all the reactions full stop for this message (same as paginating over replacements) + * References (i.e. threads of replies) + * We don't bundle contents in the references (at least for now); instead we just follow the event IDs to stitch the right events back together. + * We could include a count? + * We just provide the event IDs (to keep it nice and normalised) in a dict; we can denormalise it later for performance if needed by including the event type or whatever. We could include event_type if it was useful to say "5 replies to this message", except given event types are not just m.room.message (in future), it wouldn't be very useful to say "3 image replies and 2 msg replies". + +TODO: WHAT IS THE API SHAPE ERIK? ## Edge cases @@ -224,6 +243,9 @@ How do you handle racing edits? * problem is that other relation types might well need a more robust way of ordering. XXX: can we think of any? * could add the DAG in later if it's really needed? + * the abuse vector is for a malicious moderator to edit a message with origin_ts + of MAX_INT. the mitigation is to redact such malicious messages, although this + does mean the original message ends up being vandalised... :/ Redactions * Redacting an edited event in the UI should redact the original; the client @@ -276,6 +298,15 @@ we already have. So, we'll show inconsistent data until we backfill the gap. * We'd need to worry about pagination. * This is probably the best solution, but can also be added as a v2. +##Β Security considerations + +When using reactions for voting purposes we might well want to anonymise the +reactor, at least from other users if not server admins, to avoid retribution problems. +This gives an unfair advantage to people who run their own servers however and +can cheat and deanonymise (and publish) reactor details. + +Or in a MSC1228 world... we could let users join the room under an anonymous +persona from a big public server in order to vote? ## Extended annotation use case @@ -311,7 +342,7 @@ This would look something like the following, where the annotation is: }, "m.relates_to": { "type": "m.annotation", - "annotation_key": "" + "key": "" } } } @@ -326,11 +357,13 @@ and gets bundled into an event like: "m.annotation": [ { "type": "m.bot_command_response", - "aggregation_key": "", + "key": "", "count": 1, "chunk": [ { - "state": "success", + "m.summary": { + "state": "success", + }, } ], "limited": false, From a9d2efa2a4456661ac0ea6ed10168ed01ea9952c Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 30 Apr 2019 17:28:06 +0100 Subject: [PATCH 09/46] Fix spacing --- proposals/1849-aggregations.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index a67c1e41b38..c1eb36347ca 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -198,7 +198,7 @@ The `parent_id` is: The same happens in the sync API, however the client will need to handle new relations themselves when they come down incremental sync. -##Β Pagination +## Pagination We need to paginate over: * The relations of a given event, via /messages? or /context? or something else? From c47391e3d8df048c80301ed0e292e1b5015fb4da Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 1 May 2019 15:57:50 +0100 Subject: [PATCH 10/46] Add proposed pagination API --- proposals/1849-aggregations.md | 65 +++++++++++++++++++++++++++++++++- 1 file changed, 64 insertions(+), 1 deletion(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index c1eb36347ca..0b3836fc6c3 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -215,7 +215,70 @@ We need to paginate over: * We could include a count? * We just provide the event IDs (to keep it nice and normalised) in a dict; we can denormalise it later for performance if needed by including the event type or whatever. We could include event_type if it was useful to say "5 replies to this message", except given event types are not just m.room.message (in future), it wouldn't be very useful to say "3 image replies and 2 msg replies". -TODO: WHAT IS THE API SHAPE ERIK? +### API + +We provide two API endpoints, one to paginate relations based in normal +topological order, the second to paginate aggregated annotations. + + +Standard pagination API looks like the following, where you can optionally +specify relation and event type to filter by. The `dir` param allows client to +paginate forwards or backwards. + +``` +GET /_matrix/client/r0/rooms/{roomID}/relations/{eventID}[/{relationType}[/{eventType}]] +``` + +```json +{ + "chunk": [ ... ], + "next_batch": "some_token", + "prev_batch": "some_token" +} +``` + +Group pagination API operates in two modes, the first is to paginate the groups +themselves (order by count): + +``` +GET /_matrix/client/r0/rooms/{roomID}/annotations/{eventID}[/{eventType}] +``` + +```json +{ + "chunk": [ + { + "type": "m.reaction", + "key": "πŸ‘", + "count": 5, + } + ], + "next_batch": "some_token", + "prev_batch": "some_token" +} +``` + +The second paginates within a group, in normal topological order: + +``` +GET /_matrix/client/r0/rooms/{roomID}/annotations/{eventID}/{eventType}/{key} +``` + +```json +{ + "chunk": [ + { + "type": "m.reaction", + "sender": "..", + "content": { ... } + }, + ... + ], + "next_batch": "some_token", + "prev_batch": "some_token" +} +``` + ## Edge cases From 1b243347872de35f59fac14e79a296fe582f4072 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 1 May 2019 16:01:17 +0100 Subject: [PATCH 11/46] Flesh out the pagination API a bit --- proposals/1849-aggregations.md | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index 0b3836fc6c3..1854acd460a 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -220,10 +220,13 @@ We need to paginate over: We provide two API endpoints, one to paginate relations based in normal topological order, the second to paginate aggregated annotations. +Both APIs behave in a similar way to `/messages`, except using `next_batch` and +`prev_batch` names (in line with `/sync` API). Clients can start paginating +either from the earliest or latest events using the `dir` param. + Standard pagination API looks like the following, where you can optionally -specify relation and event type to filter by. The `dir` param allows client to -paginate forwards or backwards. +specify relation and event type to filter by. ``` GET /_matrix/client/r0/rooms/{roomID}/relations/{eventID}[/{relationType}[/{eventType}]] @@ -231,14 +234,20 @@ GET /_matrix/client/r0/rooms/{roomID}/relations/{eventID}[/{relationType}[/{even ```json { - "chunk": [ ... ], + "chunk": [ + { + "type": "m.reaction", + "sender": "...", + "content": { } + } + ], "next_batch": "some_token", "prev_batch": "some_token" } ``` -Group pagination API operates in two modes, the first is to paginate the groups -themselves (order by count): +The aggregated pagination API operates in two modes, the first is to paginate +the groups themselves (order by count): ``` GET /_matrix/client/r0/rooms/{roomID}/annotations/{eventID}[/{eventType}] @@ -269,10 +278,9 @@ GET /_matrix/client/r0/rooms/{roomID}/annotations/{eventID}/{eventType}/{key} "chunk": [ { "type": "m.reaction", - "sender": "..", - "content": { ... } + "sender": "...", + "content": { } }, - ... ], "next_batch": "some_token", "prev_batch": "some_token" From a12db5b4788eac29667b711a48f23ad1cafd698e Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Wed, 1 May 2019 17:47:11 +0100 Subject: [PATCH 12/46] following RL review --- proposals/1849-aggregations.md | 39 ++++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index 1854acd460a..cae260febd7 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -111,7 +111,7 @@ that the aggregation `key` is the unicode reaction itself. "type": "m.reaction", "contents": { "m.relates_to": { - "type": "m.annotation", + "rel_type": "m.annotation", "event_id": "$some_event_id", "key": "πŸ‘" } @@ -130,13 +130,17 @@ An event that has relations might look something like: ..., "unsigned": { "m.relations": { - "m.annotation": [ - { - "type": "m.reaction", - "key": "πŸ‘", - "count": 3 - } - ], + "m.annotation": { + "chunk": [ + { + "type": "m.reaction", + "key": "πŸ‘", + "count": 3 + } + ], + "limited": false, + "count": 1 + }, "m.reference": { "chunk": [ { @@ -152,6 +156,7 @@ An event that has relations might look something like: } ``` +ERIK, WE NEED AN EDIT EXAMPLE O:-) ## End to end encryption @@ -247,12 +252,18 @@ GET /_matrix/client/r0/rooms/{roomID}/relations/{eventID}[/{relationType}[/{even ``` The aggregated pagination API operates in two modes, the first is to paginate -the groups themselves (order by count): +the groups themselves ``` -GET /_matrix/client/r0/rooms/{roomID}/annotations/{eventID}[/{eventType}] +GET /_matrix/client/r0/rooms/{roomID}/aggregations/{eventID}[/{relationType}][/{eventType}][?filter=id] ``` +We use the filter to specify/override how to aggregate the relations, and we use the same filter shape +to inform /sync how we want to be receiving our bundled relations. By default: + * rel_type of m.annotations == group by count, and order by count desc + * rel_type of m.replaces == we just get the most recent message, no bundles. + * rel_type of m.references == we get the IDs of the events replying to us, and the total count of replies to this msg + ```json { "chunk": [ @@ -270,7 +281,13 @@ GET /_matrix/client/r0/rooms/{roomID}/annotations/{eventID}[/{eventType}] The second paginates within a group, in normal topological order: ``` -GET /_matrix/client/r0/rooms/{roomID}/annotations/{eventID}/{eventType}/{key} +GET /_matrix/client/r0/rooms/{roomID}/aggregations/{eventID}/${relationType}/{eventType}/{key} +``` + +e.g. + +``` +GET /_matrix/client/r0/rooms/!asd:matrix.org/aggregations/$1cd23476/m.annotation/m.reaction/πŸ‘ ``` ```json From 3255ae0c7972b96ab42b545fab13a6cf784222c1 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 2 May 2019 16:12:21 +0100 Subject: [PATCH 13/46] Add edit example --- proposals/1849-aggregations.md | 50 ++++++++++++++++++++++++++-------- 1 file changed, 39 insertions(+), 11 deletions(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index cae260febd7..05f200bfa48 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -96,7 +96,7 @@ A reply would look something like: "contents": { "body": "i <3 shelties", "m.relates_to": { - "type": "m.references", + "type": "m.reference", "event_id": "$some_event_id" } } @@ -122,6 +122,21 @@ that the aggregation `key` is the unicode reaction itself. TODO: This limits an event to only having one relation, on the assumption that there are no use cases and that it will make life simpler. +An edit would be: + +```json +{ + "type": "m.room.message", + "contents": { + "body": "Hello! I'm an edit", + "msgtype": "m.text", + "m.relates_to": { + "rel_type": "m.replace", + "event_id": "$some_event_id", + } + } +} +``` An event that has relations might look something like: @@ -206,19 +221,30 @@ relations themselves when they come down incremental sync. ## Pagination We need to paginate over: - * The relations of a given event, via /messages? or /context? or something else? + * The relations of a given event, via /messages? or /context? or something + else? * For replacements (i.e. edits) we get a paginated list of all edits on the source event - * Should permalinks point to the most recent revision of a given edit, or the original one? - * Permalinks should capture the event ID that the sender is viewing at that point (which might be an edit ID) - * The receiver should resolve this ID to the source event ID, and then display the most recent version that event. + * Should permalinks point to the most recent revision of a given edit, or + the original one? + * Permalinks should capture the event ID that the sender is viewing at that + point (which might be an edit ID) + * The receiver should resolve this ID to the source event ID, and then + display the most recent version that event. * Groups of annotations - * Need to paginate across the different groups (i.e. how many different reactions of different types did it get?) + * Need to paginate across the different groups (i.e. how many different + reactions of different types did it get?) * List all the reactions individually per group for this message * List all the reactions full stop for this message (same as paginating over replacements) * References (i.e. threads of replies) - * We don't bundle contents in the references (at least for now); instead we just follow the event IDs to stitch the right events back together. + * We don't bundle contents in the references (at least for now); instead we + just follow the event IDs to stitch the right events back together. * We could include a count? - * We just provide the event IDs (to keep it nice and normalised) in a dict; we can denormalise it later for performance if needed by including the event type or whatever. We could include event_type if it was useful to say "5 replies to this message", except given event types are not just m.room.message (in future), it wouldn't be very useful to say "3 image replies and 2 msg replies". + * We just provide the event IDs (to keep it nice and normalised) in a dict; we + can denormalise it later for performance if needed by including the event + type or whatever. We could include event_type if it was useful to say "5 + replies to this message", except given event types are not just + m.room.message (in future), it wouldn't be very useful to say "3 image + replies and 2 msg replies". ### API @@ -258,11 +284,13 @@ the groups themselves GET /_matrix/client/r0/rooms/{roomID}/aggregations/{eventID}[/{relationType}][/{eventType}][?filter=id] ``` -We use the filter to specify/override how to aggregate the relations, and we use the same filter shape -to inform /sync how we want to be receiving our bundled relations. By default: +We use the filter to specify/override how to aggregate the relations, and we use +the same filter shape to inform /sync how we want to be receiving our bundled +relations. By default: * rel_type of m.annotations == group by count, and order by count desc * rel_type of m.replaces == we just get the most recent message, no bundles. - * rel_type of m.references == we get the IDs of the events replying to us, and the total count of replies to this msg + * rel_type of m.references == we get the IDs of the events replying to us, and + the total count of replies to this msg ```json { From e0f165ebbe5bcfbd590686118aea8bd50b083851 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 2 May 2019 16:13:58 +0100 Subject: [PATCH 14/46] Example has been added --- proposals/1849-aggregations.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index 05f200bfa48..38e89f1e691 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -171,8 +171,6 @@ An event that has relations might look something like: } ``` -ERIK, WE NEED AN EDIT EXAMPLE O:-) - ## End to end encryption Since the server bundles related events the relation information must not be From d38d7a7d3649bbeede4aaae4a1779a045764ea67 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Mon, 13 May 2019 00:18:30 +0100 Subject: [PATCH 15/46] try to converge towards a proper reviewable draft --- proposals/1849-aggregations.md | 189 ++++++++++++++++++++++++--------- 1 file changed, 137 insertions(+), 52 deletions(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index 38e89f1e691..a66b07429d8 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -1,8 +1,8 @@ # Proposal for aggregations via m.relates_to -> WIP WIP WIP WIP WIP WIP WIP WIP WIP WIP +This is WIP, but converging on a final draft which should soon be ready for review -A very rough WIP set of notes on how relations could work in Matrix. +## Overview Today, replies looks like: @@ -18,28 +18,64 @@ Today, replies looks like: ``` `m.relates_to` is the signal to the server that the fields within describe -aggregation operations. +an aggregation operation. We would like to add support for other types of relations, including message editing and reactions. +We take the opportunity to simplify m.relates_to to avoid giving the impression +that relation types are mixins, and we define new relation types to describe the +different classes of aggregations. + +```json +"type": "m.room.message", +"contents": { + "m.relates_to": { + "rel_type": "m.reference", + "event_id": "$another:event.com" + } +} +``` + + FIXME: how *do* we specify multiple relations between two events, then, + if the only way we can define a relation is attached to the 'object'? + Answer: If we have a relation type that applies to two existing events + in retrospect, then it would be an event in its own right (with rel_type m.reference). + Otherwise, we can save time by applying the m.reference to the object directly. + + FIXME: don't we need a way to specify more specific relation types than + m.reference etc? The difference between a reply and a quote and a thread + etc? + + FIXME: should we have a contents in the relation itself? + + TODO: given we're changing the shape, should we rename the new type as + `m.relation` or something, to distinguish from the old `m.relates_to` + type? + + FIXME: Or should we jump straight to m.reference, m.annotation, m.replace + as top level mixin types? + +Relation events are then aggregated together based on the behaviour implied by +their `rel_type`, and bundled appropriately their target event when you /sync. +Additional APIs are available to send relations and paginate them. ## Types of relations There are three broad types of relations: annotations, replacements and references. -Annotations are things like reactions, which should be displayed alongside the -original event. These should support be aggregated so that e.g. if twenty people -"likes" an event we can bundle the twenty events together when sending the + * Annotations are things like reactions, which should be displayed alongside the +original event. These should support aggregations so that e.g. if twenty people +"like" an event we can bundle the twenty events together when sending the original event to clients. Another usage of an annotation is e.g. for bots, who could use annotations to report the success/failure or progress of a command. -Replacements are essentially edits, and indicate that instead of giving clients + * Replacements are essentially edits, and indicate that instead of giving clients the original event they should be handed the replacement event instead. Clients should be able to request all replacements of an event, i.e. the "edit history". -References things like replies, where a later event refers to an earlier event + * References are things like replies, where a later event refers to an earlier event in some way. The server should include references when sending an event to the client so they can display the number of replies, and navigate easily to them. @@ -50,7 +86,6 @@ means that if we include the relation type in the related event we can use the event type to easily add new types of e.g annotations without requiring server side support. - ## Aggregating and paginating relations In large rooms an event may end up having a large number of related events, and @@ -59,20 +94,24 @@ client. How we limit depends on the relation type. Annotations are grouped by their event type and an "aggregation key", and the top N groups with the highest number is included in the event. For example, -reactions would be implemented as a `m.reaction` with aggration key of e.g. +reactions would be implemented as a `m.reaction` with aggregation key of e.g. `πŸ‘`. TODO: Should we include anything other than event type, aggregation key and count? - Replacements replace the original event, and so no aggregation is required. -Though care must be taken by the server to ensure that if there are multiple -replacement events it consistently chooses the same one as all other servers. +Care must be taken by the server to ensure that if there are multiple +replacement events, the server must consistently choose the same one as all other servers. The replacement event should also include a reference to the original event ID so that clients can tell that the message has been edited. -For references the original event should include the list of `type` and +Permalinks to edited events should capture the event ID that the sender is viewing +at that point (which might be an edit ID). The client viewing the permalink +should resolve this ID to the source event ID, and then display the most recent +version of that event. + +For references, the original event should include the list of `type` and `event_id` of the earliest N references. TODO: Do we need the type? Do we want to differentiate between replies and @@ -83,7 +122,6 @@ In each case where we limit what is included there should be a corresponding API to paginate the full sets of events. Annotations would need APIs for both fetching more groups and fetching events in a group. - ## Event format All the information about the relation is put under `m.relates_to` key. @@ -96,7 +134,7 @@ A reply would look something like: "contents": { "body": "i <3 shelties", "m.relates_to": { - "type": "m.reference", + "rel_type": "m.reference", "event_id": "$some_event_id" } } @@ -138,7 +176,7 @@ An edit would be: } ``` -An event that has relations might look something like: +An event that has relations bundled alongside it then looks like: ```json { @@ -160,7 +198,7 @@ An event that has relations might look something like: "chunk": [ { "type": "m.room.message", - "event_id": "$some_event" + "event_id": "$some_event_id" } ], "limited": false, @@ -171,14 +209,18 @@ An event that has relations might look something like: } ``` + FIXME: why `m.relations`? + FIXME: this is asymmetric with the rel_types shape we use to send them; + is this a problem? + ## End to end encryption -Since the server bundles related events the relation information must not be -encrypted. +Since the server bundles related events, the relation information must not be +encrypted end-to-end. For aggregations of annotations there are two options: -1. Don't group together annotations and have the aggregation `key` encrypted, so +1. Don't group together annotations, and have the aggregation `key` encrypted, so as to not leak how someone reacted (though server would still see that they did). 2. In some way encrypt the aggregation `key`, with the properties that different @@ -188,7 +230,7 @@ For aggregations of annotations there are two options: also need to be able to go from encrypted `key` to the actual reaction. - One suggestion here was to use the message key of the event to encrypt the + One suggestion here is to use the message key of the parent event to encrypt the aggregation `key`. @@ -204,39 +246,32 @@ POST /_matrix/client/r0/rooms/{roomId}/send_relation/{parent_id}/{relation_type} } ``` -Whenever an event that has relations is sent to the client, e.g. pagination, +Whenever an event that has relations is sent to the client, e.g. sync, pagination, event search etc, the server bundles the relations into the event as per above. The `parent_id` is: * For annotations the event being displayed (which may be an edit) * For replaces/edits the original event (not previous edits) - * For references should be the original event (?) + * For references should be the event being referenced -The same happens in the sync API, however the client will need to handle new -relations themselves when they come down incremental sync. +For the sync API, clients need to be aware of both bundled relations as well as +incremental standalone relation events in the sync response. ## Pagination -We need to paginate over: - * The relations of a given event, via /messages? or /context? or something - else? - * For replacements (i.e. edits) we get a paginated list of all edits on the source event - * Should permalinks point to the most recent revision of a given edit, or - the original one? - * Permalinks should capture the event ID that the sender is viewing at that - point (which might be an edit ID) - * The receiver should resolve this ID to the source event ID, and then - display the most recent version that event. - * Groups of annotations +Our requirements that we need to paginate over: + * The relations of a given event, via a new `/relations` API. + * For replacements (i.e. edits) we get a paginated list of all edits on the source event. + * For annotations (i.e. reactions) we get the full list of reactions for the source event. + * Groups of annotations, via a new `/aggregations` API. * Need to paginate across the different groups (i.e. how many different reactions of different types did it get?) * List all the reactions individually per group for this message - * List all the reactions full stop for this message (same as paginating over replacements) * References (i.e. threads of replies) * We don't bundle contents in the references (at least for now); instead we just follow the event IDs to stitch the right events back together. - * We could include a count? + * We could include a count of the number of references to a given event. * We just provide the event IDs (to keep it nice and normalised) in a dict; we can denormalise it later for performance if needed by including the event type or whatever. We could include event_type if it was useful to say "5 @@ -253,9 +288,9 @@ Both APIs behave in a similar way to `/messages`, except using `next_batch` and `prev_batch` names (in line with `/sync` API). Clients can start paginating either from the earliest or latest events using the `dir` param. - Standard pagination API looks like the following, where you can optionally -specify relation and event type to filter by. +specify relation and event type to filter by. It lists all the relations +in topological order. ``` GET /_matrix/client/r0/rooms/{roomID}/relations/{eventID}[/{relationType}[/{eventType}]] @@ -276,20 +311,23 @@ GET /_matrix/client/r0/rooms/{roomID}/relations/{eventID}[/{relationType}[/{even ``` The aggregated pagination API operates in two modes, the first is to paginate -the groups themselves +the groups themselves, returning aggregated results: ``` GET /_matrix/client/r0/rooms/{roomID}/aggregations/{eventID}[/{relationType}][/{eventType}][?filter=id] ``` -We use the filter to specify/override how to aggregate the relations, and we use -the same filter shape to inform /sync how we want to be receiving our bundled -relations. By default: - * rel_type of m.annotations == group by count, and order by count desc - * rel_type of m.replaces == we just get the most recent message, no bundles. - * rel_type of m.references == we get the IDs of the events replying to us, and +By default, the aggregation behaviour is defined by the relation type: + * rel_type of m.annotation == group by count, and order by count desc + * rel_type of m.replace == we just get the most recent message, no bundles. + * rel_type of m.reference == we get the IDs of the events replying to us, and the total count of replies to this msg +In future, we could use a filter to specify/override how to aggregate the relations, +which would then also be used to inform /sync how we want to receive our bundled +relations. (However, we really need to be better understand how to do custom +relation types first...) + ```json { "chunk": [ @@ -304,7 +342,7 @@ relations. By default: } ``` -The second paginates within a group, in normal topological order: +The second mode of operation is to paginate within a group, in normal topological order: ``` GET /_matrix/client/r0/rooms/{roomID}/aggregations/{eventID}/${relationType}/{eventType}/{key} @@ -330,7 +368,6 @@ GET /_matrix/client/r0/rooms/!asd:matrix.org/aggregations/$1cd23476/m.annotation } ``` - ## Edge cases What happens when you react to an edit? @@ -361,6 +398,9 @@ How do you handle racing edits? of MAX_INT. the mitigation is to redact such malicious messages, although this does mean the original message ends up being vandalised... :/ +How do you remove a reaction? + * You redact it. + Redactions * Redacting an edited event in the UI should redact the original; the client will need to redact the original event to make this happen. @@ -390,7 +430,7 @@ we already have. So, we'll show inconsistent data until we backfill the gap. * Or we could make *ALL* relations a DAG, so we can spot holes at the next relation, and go walk the DAG to pull in the missing relations? Then, the next relation for an event could pull in any of the missing relations. - Socially this probably doesn't work as reactions will likely drop-off over + Socially this probably doesn't work as reactions will likely drop off over time, so by the time your server comes back there won't be any more reactions pulling the missing ones in. * Could we also ask the server, after a gap, to provide all the relations which @@ -420,7 +460,8 @@ This gives an unfair advantage to people who run their own servers however and can cheat and deanonymise (and publish) reactor details. Or in a MSC1228 world... we could let users join the room under an anonymous -persona from a big public server in order to vote? +persona from a big public server in order to vote? However, such anonymous personae +would lack any reputation data. ## Extended annotation use case @@ -494,6 +535,50 @@ This is something that could be added later on. A few issues with this are: * We would end up including old annotations that had been superceded, should these be done via edits instead? +## Tradeoffs + +### Aggregation extensibility + +We have a choice between freeform aggregation functions a la SQL (SUM, MAX, ARRAY_AGG) +rather than defining rel_types like m.annotation, m.replace, m.reference respectively. + +It's unclear how we are meant to define custom relations or extend these rel types. + +### Event shape + +Shape of + +```json +"contents": { + "m.relates_to": { + "m.reference": { + "event_id": "$another:event.com" + } + } +} +``` + +versus + +```json +"contents": { + "m.relates_to": { + "rel_type": "m.reference", + "event_id": "$another:event.com" + } +} +``` + +The reasons to go with rel_type is: + * we don't need the extra indirection to let multiple relations apply to a given pair of + events, as that should be expressed as separate relation events. + * if we want 'adverbs' to apply to 'verbs' in the subject-verb-object triples which + relations form, then we apply it as mixins to the relation data itself rather than trying + to construct subject-verb-verb-object sentences. + * so, we should pick a simpler shape rather than inheriting the mistakes of m.in_reply_to + and we have to keep ugly backwards compatibility around for m.in_reply_to + but we can entirely separately worry about migrating replies to new-style-aggregations in future + perhaps at the same time as doing threads. ## Historical context From 08de48466456a26c82af34be36d0882a181aed78 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Mon, 13 May 2019 12:36:33 +0100 Subject: [PATCH 16/46] yet more notes on yet more edge cases --- proposals/1849-aggregations.md | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index a66b07429d8..8d6f2cee82c 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -397,6 +397,10 @@ How do you handle racing edits? * the abuse vector is for a malicious moderator to edit a message with origin_ts of MAX_INT. the mitigation is to redact such malicious messages, although this does mean the original message ends up being vandalised... :/ + * Conclusion: let's do it for origin_ts as a first cut, but use event shapes which + could be switched to DAG in future is/as needed. Good news is that it only + affects the server implementation; the clients can trust the server to linearise + correctly. How do you remove a reaction? * You redact it. @@ -417,6 +421,32 @@ What does it mean to call /context on a relation? * We should probably just return the root event for now, and then refine it in future for threading? +Should we enforce that each user can only send one type of reaction to a msg? + * Yes. We can do that when sending at CS API as the first cut + * But what do we use as a response? We can do a 400 with an error code that tells us + why - i.e. that it's because you can't react multiple times. + * We mandate txn IDs to provide idempotency. + * (Or can we rely on including our own reactions in bundles to tell whether + are doublecounting our own reactions or not?) + * However, we need to be able to handle bad servers who send duplicate events anyway. + * The only way to do this will be at SS API, and refuse to accept duplicatee + events. + +Should we always include our own reactions in a bundle, to make it easier to redact them, +or to show the UI in the right state? + * Probably, but this can be a future refinement. + * ...but might be needed for imposing one type of reaction per msg. + +Should we stop reactions being sent by the normal /send API? + +What can we edit? + * Only non-state events for now. + * We can't change event types, or anything else which is in an E2E payload + +How do diffs work on edits if you are missing intermediary edits? + * We just have to ensure that the UI for visualising diffs makes it clear + that diffs could span multiple edits rather than strictly be per-edit-event. + ## Federation considerations In general, no special considerations are needed for federation; relational From e78e7ad2f043b36564ad80ff373693a640fb6aca Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Mon, 13 May 2019 15:16:48 +0100 Subject: [PATCH 17/46] Add a coherent overview; remove a bunch of FIXMEs --- proposals/1849-aggregations.md | 95 +++++++++++++++++++++++----------- 1 file changed, 66 insertions(+), 29 deletions(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index 8d6f2cee82c..448878fc758 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -1,9 +1,57 @@ # Proposal for aggregations via m.relates_to -This is WIP, but converging on a final draft which should soon be ready for review - ## Overview +This proposal introduces the concept of relations, which can be used to associate +new information with an existing event. Relations are events which have an `m.relates_to` +mixin in their contents, and the new information they convey is expressed in their +usual event `type` and `contents`. + +Clients send relations using the new `/send_relation` API. + +Clients receive relations as normal events in /sync (aka 'unbundled relations'), +or may also be aggregated together by the server, and presented as +a 'bundle' attached to the original event. + +Bundles of relations for a given event are +paginated to prevent overloading the client with relations, and may be traversed by +via the new `/relations` API (which iterates over all relations for an event) or the +new `/aggregations` API (which iterates over the groups of relations, or the relations +within a group). + +Three types of relations are defined, each defining different behaviour when aggregated: + + * m.annotation - lets you define an event which annotates an existing event. + When aggregated, groups events together based on `key` and returns a `count`. (aka SQL's COUNT) + These are primarily intended for handling reactions. + + * m.replace - lets you define an event which replaces an existing event. + When aggregated, returns the most recent replacement event. (aka SQL's MAX) + These are primarily intended for handling edits. + + * m.reference - lets you define an event which references an existing event. + When aggregated, currently doesn't do anything special, but in future could bundle + chains of references (i.e. threads). + These are primarily intended for handling replies (and in future threads). + +This model has been designed for scenarios where the relationship is known between +two events at the point that the 2nd event is sent. Therefore, extensible info about +the relationship is intended to be stored in the 2nd event, rather than the relation +itself. For instance, to distinguish different types of references (in_reply_to v. refers +v. cites v. quotes) you would look at the fields of the 2nd event. Alternatively, one +could add fields to the `m.relates_to` object. + + XXX: do we want to support multiple parents for a m.reference event, if a given event + references differernt parents in differernt ways? + +In future, it may be desirable to send relationship events which link together two +events retrospectively - e.g. an `m.duplicate` event with an `m.link` relation type +might be a way to flag that existing 2 events are somehow duplicates of each other. +However, this would be defined as an entirely different relation type of `m.link`, +which might bundle together both referenced events when aggregated. + +## Context + Today, replies looks like: ```json @@ -24,7 +72,8 @@ We would like to add support for other types of relations, including message editing and reactions. We take the opportunity to simplify m.relates_to to avoid giving the impression -that relation types are mixins, and we define new relation types to describe the +that relation types are mixins and that you can send multiple different type of +relations for a given event, and we define new relation types to describe the different classes of aggregations. ```json @@ -37,24 +86,14 @@ different classes of aggregations. } ``` - FIXME: how *do* we specify multiple relations between two events, then, - if the only way we can define a relation is attached to the 'object'? - Answer: If we have a relation type that applies to two existing events - in retrospect, then it would be an event in its own right (with rel_type m.reference). - Otherwise, we can save time by applying the m.reference to the object directly. - - FIXME: don't we need a way to specify more specific relation types than - m.reference etc? The difference between a reply and a quote and a thread - etc? - - FIXME: should we have a contents in the relation itself? - TODO: given we're changing the shape, should we rename the new type as `m.relation` or something, to distinguish from the old `m.relates_to` type? FIXME: Or should we jump straight to m.reference, m.annotation, m.replace - as top level mixin types? + as top level mixin types? Erik would prefer not to, as grouping them all + under `m.relates_to` makes it very clear that they should not be E2E encrypted + etc. In fact, we could even move this outside of `contents`? Relation events are then aggregated together based on the behaviour implied by their `rel_type`, and bundled appropriately their target event when you /sync. @@ -62,7 +101,7 @@ Additional APIs are available to send relations and paginate them. ## Types of relations -There are three broad types of relations: annotations, replacements and +This proposal defines three types of relations: annotations, replacements and references. * Annotations are things like reactions, which should be displayed alongside the @@ -209,10 +248,6 @@ An event that has relations bundled alongside it then looks like: } ``` - FIXME: why `m.relations`? - FIXME: this is asymmetric with the rel_types shape we use to send them; - is this a problem? - ## End to end encryption Since the server bundles related events, the relation information must not be @@ -447,6 +482,16 @@ How do diffs work on edits if you are missing intermediary edits? * We just have to ensure that the UI for visualising diffs makes it clear that diffs could span multiple edits rather than strictly be per-edit-event. +What happens when we edit a reply? + * We just send an m.replace which refers to the m.reference target; nothing + special is needed. i.e. you cannot change who the event is replying to. + +Do we need to support retrospective references? + * For something like "m.duplicate" to retrospectively declare that one event + dupes another, we might need to bundle two-levels deep (subject+ref and then + ref+target). We can cross this bridge when we get there though, as a 4th + aggregation type + ## Federation considerations In general, no special considerations are needed for federation; relational @@ -567,13 +612,6 @@ This is something that could be added later on. A few issues with this are: ## Tradeoffs -### Aggregation extensibility - -We have a choice between freeform aggregation functions a la SQL (SUM, MAX, ARRAY_AGG) -rather than defining rel_types like m.annotation, m.replace, m.reference respectively. - -It's unclear how we are meant to define custom relations or extend these rel types. - ### Event shape Shape of @@ -587,7 +625,6 @@ Shape of } } ``` - versus ```json From dacae1f22ad7c15d141e03a43ace730f17c31d77 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Mon, 13 May 2019 15:18:00 +0100 Subject: [PATCH 18/46] Add TOC --- proposals/1849-aggregations.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index 448878fc758..8f4aadde603 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -1,5 +1,21 @@ # Proposal for aggregations via m.relates_to + - [Overview](#overview) + - [Context](#context) + - [Types of relations](#types-of-relations) + - [Aggregating and paginating relations](#aggregating-and-paginating-relations) + - [Event format](#event-format) + - [End to end encryption](#end-to-end-encryption) + - [CS API](#cs-api) + - [Pagination](#pagination) + - [API](#api) + - [Edge cases](#edge-cases) + - [Federation considerations](#federation-considerations) + - [Extended annotation use case](#extended-annotation-use-case) + - [Tradeoffs](#tradeoffs) + - [Event shape](#event-shape) + - [Historical context](#historical-context) + ## Overview This proposal introduces the concept of relations, which can be used to associate From 5375a23353d20b3c6f401c34de31806bf8224167 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Mon, 13 May 2019 15:21:54 +0100 Subject: [PATCH 19/46] markdown tweaks --- proposals/1849-aggregations.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index 8f4aadde603..d534a4df86c 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -37,15 +37,15 @@ within a group). Three types of relations are defined, each defining different behaviour when aggregated: - * m.annotation - lets you define an event which annotates an existing event. + * `m.annotation` - lets you define an event which annotates an existing event. When aggregated, groups events together based on `key` and returns a `count`. (aka SQL's COUNT) These are primarily intended for handling reactions. - * m.replace - lets you define an event which replaces an existing event. + * `m.replace` - lets you define an event which replaces an existing event. When aggregated, returns the most recent replacement event. (aka SQL's MAX) These are primarily intended for handling edits. - * m.reference - lets you define an event which references an existing event. + * `m.reference` - lets you define an event which references an existing event. When aggregated, currently doesn't do anything special, but in future could bundle chains of references (i.e. threads). These are primarily intended for handling replies (and in future threads). @@ -369,9 +369,9 @@ GET /_matrix/client/r0/rooms/{roomID}/aggregations/{eventID}[/{relationType}][/{ ``` By default, the aggregation behaviour is defined by the relation type: - * rel_type of m.annotation == group by count, and order by count desc - * rel_type of m.replace == we just get the most recent message, no bundles. - * rel_type of m.reference == we get the IDs of the events replying to us, and + * rel_type of `m.annotation` == group by count, and order by count desc + * rel_type of `m.replace` == we just get the most recent message, no bundles. + * rel_type of `m.reference` == we get the IDs of the events replying to us, and the total count of replies to this msg In future, we could use a filter to specify/override how to aggregate the relations, @@ -622,7 +622,7 @@ and gets bundled into an event like: This is something that could be added later on. A few issues with this are: - * How does this work with end to end? How do we encrypt the `m.summary`? + * How does this work with E2EE? How do we encrypt the `m.summary`? * We would end up including old annotations that had been superceded, should these be done via edits instead? @@ -652,7 +652,7 @@ versus } ``` -The reasons to go with rel_type is: +The reasons to go with `rel_type` is: * we don't need the extra indirection to let multiple relations apply to a given pair of events, as that should be expressed as separate relation events. * if we want 'adverbs' to apply to 'verbs' in the subject-verb-object triples which From 0f7cf5ea8be02c86616042d7d3400fefbd75b9a4 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Mon, 13 May 2019 15:31:14 +0100 Subject: [PATCH 20/46] typos --- proposals/1849-aggregations.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index d534a4df86c..622566b6fd7 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -38,7 +38,7 @@ within a group). Three types of relations are defined, each defining different behaviour when aggregated: * `m.annotation` - lets you define an event which annotates an existing event. - When aggregated, groups events together based on `key` and returns a `count`. (aka SQL's COUNT) + When aggregated, groups events together based on `key` and returns a `count`. (aka SQL's COUNT) These are primarily intended for handling reactions. * `m.replace` - lets you define an event which replaces an existing event. @@ -121,7 +121,7 @@ This proposal defines three types of relations: annotations, replacements and references. * Annotations are things like reactions, which should be displayed alongside the -original event. These should support aggregations so that e.g. if twenty people +original event. These should support being aggregated so that e.g. if twenty peoples "like" an event we can bundle the twenty events together when sending the original event to clients. Another usage of an annotation is e.g. for bots, who could use annotations to report the success/failure or progress of a command. From 4bb8d6bdbb101a67a7ba8c5cf88f31f6576b2227 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Mon, 13 May 2019 15:32:54 +0100 Subject: [PATCH 21/46] s/contents/content/g --- proposals/1849-aggregations.md | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index 622566b6fd7..c1a9428937b 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -72,7 +72,7 @@ Today, replies looks like: ```json "type": "m.room.message", -"contents": { +"content": { "m.relates_to": { "m.in_reply_to": { "event_id": "$another:event.com" @@ -94,7 +94,7 @@ different classes of aggregations. ```json "type": "m.room.message", -"contents": { +"content": { "m.relates_to": { "rel_type": "m.reference", "event_id": "$another:event.com" @@ -186,7 +186,7 @@ A reply would look something like: ```json { "type": "m.room.message", - "contents": { + "content": { "body": "i <3 shelties", "m.relates_to": { "rel_type": "m.reference", @@ -202,7 +202,7 @@ that the aggregation `key` is the unicode reaction itself. ```json { "type": "m.reaction", - "contents": { + "content": { "m.relates_to": { "rel_type": "m.annotation", "event_id": "$some_event_id", @@ -220,7 +220,7 @@ An edit would be: ```json { "type": "m.room.message", - "contents": { + "content": { "body": "Hello! I'm an edit", "msgtype": "m.text", "m.relates_to": { @@ -633,7 +633,7 @@ This is something that could be added later on. A few issues with this are: Shape of ```json -"contents": { +"content": { "m.relates_to": { "m.reference": { "event_id": "$another:event.com" @@ -644,7 +644,7 @@ Shape of versus ```json -"contents": { +"content": { "m.relates_to": { "rel_type": "m.reference", "event_id": "$another:event.com" @@ -672,7 +672,7 @@ which fields should be aggregated. ```json "type": "m.room._aggregation.emoticon", -"contents": { +"content": { "emoticon": "::smile::", "msgtype": "?", "target_id": "$another:event.com" @@ -683,7 +683,7 @@ These would then aggregated, based on target_id, and returned as annotations on the source event in an `aggregation_data` field: ```json -"contents": { +"content": { ... "aggregation_data": { "m.room._aggregation.emoticon": { From 60a3d61a6b170bb2a2b9cbf1814b2ad057f938a5 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Mon, 13 May 2019 15:54:40 +0100 Subject: [PATCH 22/46] fix wording --- proposals/1849-aggregations.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index c1a9428937b..17f81a3298b 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -472,7 +472,7 @@ What does it mean to call /context on a relation? * We should probably just return the root event for now, and then refine it in future for threading? -Should we enforce that each user can only send one type of reaction to a msg? +Should we enforce that each user can only send one of each type of reaction to a msg? * Yes. We can do that when sending at CS API as the first cut * But what do we use as a response? We can do a 400 with an error code that tells us why - i.e. that it's because you can't react multiple times. From 117ae9713dce5a97d9b7c866f644b180c406460a Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Mon, 13 May 2019 18:36:59 +0100 Subject: [PATCH 23/46] Update proposals/1849-aggregations.md Co-Authored-By: Travis Ralston --- proposals/1849-aggregations.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index 17f81a3298b..b53a32c6ea1 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -21,7 +21,7 @@ This proposal introduces the concept of relations, which can be used to associate new information with an existing event. Relations are events which have an `m.relates_to` mixin in their contents, and the new information they convey is expressed in their -usual event `type` and `contents`. +usual event `type` and `content`. Clients send relations using the new `/send_relation` API. From 8627fb77f07993cad3af633134d8ad9920e8b604 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Mon, 13 May 2019 18:39:01 +0100 Subject: [PATCH 24/46] Update proposals/1849-aggregations.md Co-Authored-By: Travis Ralston --- proposals/1849-aggregations.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index b53a32c6ea1..29a51c7f619 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -58,7 +58,7 @@ v. cites v. quotes) you would look at the fields of the 2nd event. Alternativel could add fields to the `m.relates_to` object. XXX: do we want to support multiple parents for a m.reference event, if a given event - references differernt parents in differernt ways? + references different parents in different ways? In future, it may be desirable to send relationship events which link together two events retrospectively - e.g. an `m.duplicate` event with an `m.link` relation type From 2dde2c1a3392cb7fc9e0d8c1e0c4fdbab1b376e1 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Tue, 14 May 2019 16:05:29 +0100 Subject: [PATCH 25/46] add m.new_content --- proposals/1849-aggregations.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index 29a51c7f619..2ba0e754ab0 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -221,8 +221,12 @@ An edit would be: { "type": "m.room.message", "content": { - "body": "Hello! I'm an edit", + "body": "s/foo/bar/", "msgtype": "m.text", + "m.new_content": { + "body": "Hello! My name is bar", + "msgtype": "m.text", + }, "m.relates_to": { "rel_type": "m.replace", "event_id": "$some_event_id", @@ -508,6 +512,10 @@ Do we need to support retrospective references? ref+target). We can cross this bridge when we get there though, as a 4th aggregation type +What power level do you need to be able to edit other people's messages, and how +does it fit in with fedeation event auth rules? + * 50, by default? + ## Federation considerations In general, no special considerations are needed for federation; relational From adb240d31b919e3afcc2f6ed3adfe6fa8946232f Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Thu, 27 Jun 2019 20:31:06 +0100 Subject: [PATCH 26/46] WIP commit of rewrite to MSC1849 --- proposals/1849-aggregations.md | 256 ++++++++++++++++----------------- 1 file changed, 127 insertions(+), 129 deletions(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index 2ba0e754ab0..ae1e34b8171 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -1,147 +1,138 @@ -# Proposal for aggregations via m.relates_to - - - [Overview](#overview) - - [Context](#context) - - [Types of relations](#types-of-relations) - - [Aggregating and paginating relations](#aggregating-and-paginating-relations) - - [Event format](#event-format) - - [End to end encryption](#end-to-end-encryption) - - [CS API](#cs-api) - - [Pagination](#pagination) - - [API](#api) - - [Edge cases](#edge-cases) - - [Federation considerations](#federation-considerations) - - [Extended annotation use case](#extended-annotation-use-case) - - [Tradeoffs](#tradeoffs) - - [Event shape](#event-shape) - - [Historical context](#historical-context) - -## Overview - -This proposal introduces the concept of relations, which can be used to associate -new information with an existing event. Relations are events which have an `m.relates_to` -mixin in their contents, and the new information they convey is expressed in their -usual event `type` and `content`. - -Clients send relations using the new `/send_relation` API. +# Proposal for aggregations via relations -Clients receive relations as normal events in /sync (aka 'unbundled relations'), -or may also be aggregated together by the server, and presented as -a 'bundle' attached to the original event. +##Β Problem -Bundles of relations for a given event are -paginated to prevent overloading the client with relations, and may be traversed by -via the new `/relations` API (which iterates over all relations for an event) or the -new `/aggregations` API (which iterates over the groups of relations, or the relations -within a group). +It's common to want to send events in Matrix which relate to existing events - +for instance, reactions, edits and even replies/threads. + +Clients typically need to track the related events alongside the original +event they relate to, in order to correctly display them. For instance, +reaction events need to aggregated together by summing and be shown next to +the event they react to; edits need to be aggregated together by replacing the +original event and subsequent edits; replies need to be indented after the +message they respond to, etc. -Three types of relations are defined, each defining different behaviour when aggregated: +It is possible to treat relations as normal events and aggregate them +clientside, but to do so comprehensively could be very resource intensive, as +the client would need to spider all possible events in a room to find +relationships and maintain an correct view. - * `m.annotation` - lets you define an event which annotates an existing event. - When aggregated, groups events together based on `key` and returns a `count`. (aka SQL's COUNT) - These are primarily intended for handling reactions. +Instead, this proposal seeks to solve this problem by: + * Defining a standard shape for defining events which relate to other events + * Defining APIs to let the server calculate the aggregations on behalf of the + client, and so bundle the related events with the original event where + appropriate. - * `m.replace` - lets you define an event which replaces an existing event. - When aggregated, returns the most recent replacement event. (aka SQL's MAX) - These are primarily intended for handling edits. +## Proposal - * `m.reference` - lets you define an event which references an existing event. - When aggregated, currently doesn't do anything special, but in future could bundle - chains of references (i.e. threads). - These are primarily intended for handling replies (and in future threads). +This proposal introduces the concept of relations, which can be used to +associate new information with an existing event. -This model has been designed for scenarios where the relationship is known between -two events at the point that the 2nd event is sent. Therefore, extensible info about -the relationship is intended to be stored in the 2nd event, rather than the relation -itself. For instance, to distinguish different types of references (in_reply_to v. refers -v. cites v. quotes) you would look at the fields of the 2nd event. Alternatively, one -could add fields to the `m.relates_to` object. +Relations are any event which have an `m.relationship` mixin in their +contents. The `m.relationship` field must include a `rel_type` field that +gives the type of relationship being defined, and the `event_id` field that +gives the event which is the target of the relation. All the information about +the relationship lives under the `m.relationship` key. - XXX: do we want to support multiple parents for a m.reference event, if a given event - references different parents in different ways? +If it helps, you can think of relations as a "subject verb object" triple, +where the subject is the relation event itself; the verb is the `rel_type` +field of the `m.relationship` and the object is the `event_id` field. -In future, it may be desirable to send relationship events which link together two -events retrospectively - e.g. an `m.duplicate` event with an `m.link` relation type -might be a way to flag that existing 2 events are somehow duplicates of each other. -However, this would be defined as an entirely different relation type of `m.link`, -which might bundle together both referenced events when aggregated. +We consciously do not support multiple different relations within a single event, +in order to keep the API simple, and in the absence of identifiable use cases. +Instead, one would send multiple events, each with its own `m.relationship` +defined. -## Context +### Relation types -Today, replies looks like: +This proposal defines three `rel_type`s, each which provide different behaviour +when aggregated: + + * `m.annotation` - Intended primarily for handling emoji reactions, these let + you define an event which annotates an existing event. The annotations are + typically presented alongside the event. When aggregated, it groups events + together based on their `key` and returns a `count`. Another usage of an + annotation is e.g. for bots, who could use annotations to report the + success/failure or progress of a command. + +For example, an `m.reaction` event which `annotates` an existing event with a πŸ‘ +looks like: ```json -"type": "m.room.message", -"content": { - "m.relates_to": { - "m.in_reply_to": { - "event_id": "$another:event.com" +{ + "type": "m.reaction", + "content": { + "m.relationship": { + "rel_type": "m.annotation", + "event_id": "$some_event_id", + "key": "πŸ‘" } } } ``` -`m.relates_to` is the signal to the server that the fields within describe -an aggregation operation. + * `m.replace` - Intended primarily for handling edits, these let you define + an event which replaces an existing event. When aggregated, returns the + most recent replacement event. The replacement event must contain an + `m.new_content` which defines the replacement content (allowing the normal + `body` fields to be used for a fallback for clients who do not understand + replacement events). -We would like to add support for other types of relations, including message -editing and reactions. +For instance, an `m.room.message` which `replaces` an existing event looks like: -We take the opportunity to simplify m.relates_to to avoid giving the impression -that relation types are mixins and that you can send multiple different type of -relations for a given event, and we define new relation types to describe the -different classes of aggregations. +```json +{ + "type": "m.room.message", + "content": { + "body": "s/foo/bar/", + "msgtype": "m.text", + "m.new_content": { + "body": "Hello! My name is bar", + "msgtype": "m.text", + }, + "m.relates_to": { + "rel_type": "m.replace", + "event_id": "$some_event_id", + } + } +} +``` + + * `m.reference` - Intended in future for handling replies and threading, + these let you define an event which references an existing event. When + aggregated, currently doesn't do anything special, but in future could + bundle chains of references (i.e. threads). These do not yet replace + `m.relates_to`-style replies however. + +For instance, an `m.room.message` which `references` an existing event +would look like: ```json "type": "m.room.message", "content": { - "m.relates_to": { + "body": "i <3 shelties", + "m.relationship": { "rel_type": "m.reference", - "event_id": "$another:event.com" + "event_id": "$another_event_id" } } ``` - TODO: given we're changing the shape, should we rename the new type as - `m.relation` or something, to distinguish from the old `m.relates_to` - type? - - FIXME: Or should we jump straight to m.reference, m.annotation, m.replace - as top level mixin types? Erik would prefer not to, as grouping them all - under `m.relates_to` makes it very clear that they should not be E2E encrypted - etc. In fact, we could even move this outside of `contents`? - -Relation events are then aggregated together based on the behaviour implied by -their `rel_type`, and bundled appropriately their target event when you /sync. -Additional APIs are available to send relations and paginate them. +Different subtypes of references could be defined through additional fields on +the `m.relationship` object, to distinguish between replies, threads, etc. +This MSC doesn't attempt to define these subtypes. -## Types of relations + XXX: do we want to support multiple parents for a m.reference event, if a given event + references different parents in different ways? -This proposal defines three types of relations: annotations, replacements and -references. +### Sending relations - * Annotations are things like reactions, which should be displayed alongside the -original event. These should support being aggregated so that e.g. if twenty peoples -"like" an event we can bundle the twenty events together when sending the -original event to clients. Another usage of an annotation is e.g. for bots, who -could use annotations to report the success/failure or progress of a command. - * Replacements are essentially edits, and indicate that instead of giving clients -the original event they should be handed the replacement event instead. Clients -should be able to request all replacements of an event, i.e. the "edit history". - * References are things like replies, where a later event refers to an earlier event -in some way. The server should include references when sending an event to the -client so they can display the number of replies, and navigate easily to them. -These types effect how the server bundles the related events with the original, -and so the type must be known to servers when handling relations. However, the -exact semantics of a particular relation only needs to be known by clients. This -means that if we include the relation type in the related event we can use the -event type to easily add new types of e.g annotations without requiring server -side support. +### Aggregating and paginating relations -## Aggregating and paginating relations +Relations may be sent to clients In large rooms an event may end up having a large number of related events, and so we do not want to have to include all relations when sending the event to the @@ -212,29 +203,8 @@ that the aggregation `key` is the unicode reaction itself. } ``` - TODO: This limits an event to only having one relation, on the assumption - that there are no use cases and that it will make life simpler. - An edit would be: -```json -{ - "type": "m.room.message", - "content": { - "body": "s/foo/bar/", - "msgtype": "m.text", - "m.new_content": { - "body": "Hello! My name is bar", - "msgtype": "m.text", - }, - "m.relates_to": { - "rel_type": "m.replace", - "event_id": "$some_event_id", - } - } -} -``` - An event that has relations bundled alongside it then looks like: ```json @@ -423,6 +393,34 @@ GET /_matrix/client/r0/rooms/!asd:matrix.org/aggregations/$1cd23476/m.annotation } ``` +### Client Server API + + +To receive relations: + +Relation events are then aggregated together based on the behaviour implied by +their `rel_type`, and bundled appropriately their target event when you /sync. +Additional APIs are available to send relations and paginate them. + +To send relations: + + +Clients receive relations as normal events in /sync (aka 'unbundled relations'), +or may also be aggregated together by the server, and presented as +a 'bundle' attached to the original event. + +Bundles of relations for a given event are +paginated to prevent overloading the client with relations, and may be traversed by +via the new `/relations` API (which iterates over all relations for an event) or the +new `/aggregations` API (which iterates over the groups of relations, or the relations +within a group). + + +### E2E Encryption + + + + ## Edge cases What happens when you react to an edit? From a6c8b6541d389d793beb2712629b6473f8dc7188 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Thu, 27 Jun 2019 20:31:41 +0100 Subject: [PATCH 27/46] WIP commit of rewrite to MSC1849 --- proposals/1849-aggregations.md | 126 +++++++++++++-------------------- 1 file changed, 50 insertions(+), 76 deletions(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index ae1e34b8171..2f89f101e99 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -76,7 +76,7 @@ looks like: most recent replacement event. The replacement event must contain an `m.new_content` which defines the replacement content (allowing the normal `body` fields to be used for a fallback for clients who do not understand - replacement events). + replacement events). The newest replacement is determined... FIXME For instance, an `m.room.message` which `replaces` an existing event looks like: @@ -127,6 +127,55 @@ This MSC doesn't attempt to define these subtypes. ### Sending relations +Related events are normal Matrix events, but it's possible that the server may need to +process them before sending them into a room (for instance, if we ever use a DAG to +define the ordering of an m.relations + +Similar to membership events, related events may be sent either by the normal + +Sending a related event uses an equivalent of the normal send API (with an +equivalent `PUT` API): + +``` +POST /_matrix/client/r0/rooms/{roomId}/send_relation/{parent_id}/{relation_type}/{event_type} +{ + // event contents +} +``` + + +### Receiving relations + +```json +{ + ..., + "unsigned": { + "m.relations": { + "m.annotation": { + "chunk": [ + { + "type": "m.reaction", + "key": "πŸ‘", + "count": 3 + } + ], + "limited": false, + "count": 1 + }, + "m.reference": { + "chunk": [ + { + "type": "m.room.message", + "event_id": "$some_event_id" + } + ], + "limited": false, + "count": 1 + } + } + } +} +``` @@ -168,75 +217,9 @@ In each case where we limit what is included there should be a corresponding API to paginate the full sets of events. Annotations would need APIs for both fetching more groups and fetching events in a group. -## Event format -All the information about the relation is put under `m.relates_to` key. -A reply would look something like: -```json -{ - "type": "m.room.message", - "content": { - "body": "i <3 shelties", - "m.relates_to": { - "rel_type": "m.reference", - "event_id": "$some_event_id" - } - } -} -``` - -And a reaction might look like the following, where we define for `m.reaction` -that the aggregation `key` is the unicode reaction itself. - -```json -{ - "type": "m.reaction", - "content": { - "m.relates_to": { - "rel_type": "m.annotation", - "event_id": "$some_event_id", - "key": "πŸ‘" - } - } -} -``` - -An edit would be: - -An event that has relations bundled alongside it then looks like: - -```json -{ - ..., - "unsigned": { - "m.relations": { - "m.annotation": { - "chunk": [ - { - "type": "m.reaction", - "key": "πŸ‘", - "count": 3 - } - ], - "limited": false, - "count": 1 - }, - "m.reference": { - "chunk": [ - { - "type": "m.room.message", - "event_id": "$some_event_id" - } - ], - "limited": false, - "count": 1 - } - } - } -} -``` ## End to end encryption @@ -261,15 +244,6 @@ For aggregations of annotations there are two options: ## CS API -Sending a related event uses an equivalent of the normal send API (with an -equivalent `PUT` API): - -``` -POST /_matrix/client/r0/rooms/{roomId}/send_relation/{parent_id}/{relation_type}/{event_type} -{ - // event contents -} -``` Whenever an event that has relations is sent to the client, e.g. sync, pagination, event search etc, the server bundles the relations into the event as per above. From 081ea3d8e4d022e36a2ce96f5fe63aeab30aaf71 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Fri, 5 Jul 2019 01:07:14 +0100 Subject: [PATCH 28/46] write up the new gappy sync stuff --- proposals/1849-aggregations.md | 301 +++++++++++++++++++++------------ 1 file changed, 189 insertions(+), 112 deletions(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index 2f89f101e99..c6ab57de25d 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -50,10 +50,10 @@ when aggregated: * `m.annotation` - Intended primarily for handling emoji reactions, these let you define an event which annotates an existing event. The annotations are - typically presented alongside the event. When aggregated, it groups events - together based on their `key` and returns a `count`. Another usage of an - annotation is e.g. for bots, who could use annotations to report the - success/failure or progress of a command. + typically presented alongside the event in the timeline. When aggregated, + it groups events together based on their `key` and returns a `count`. + Another usage of an annotation is e.g. for bots, who could use annotations + to report the success/failure or progress of a command. For example, an `m.reaction` event which `annotates` an existing event with a πŸ‘ looks like: @@ -73,10 +73,10 @@ looks like: * `m.replace` - Intended primarily for handling edits, these let you define an event which replaces an existing event. When aggregated, returns the - most recent replacement event. The replacement event must contain an - `m.new_content` which defines the replacement content (allowing the normal - `body` fields to be used for a fallback for clients who do not understand - replacement events). The newest replacement is determined... FIXME + most recent replacement event (as determined by `origin_server_ts`). The + replacement event must contain an `m.new_content` which defines the + replacement content (allowing the normal `body` fields to be used for a + fallback for clients who do not understand replacement events). For instance, an `m.room.message` which `replaces` an existing event looks like: @@ -98,6 +98,18 @@ For instance, an `m.room.message` which `replaces` an existing event looks like: } ``` + Permalinks to edited events should capture the event ID that the sender is viewing + at that point (which might be an edit ID). The client viewing the permalink + should resolve this ID to the source event ID, and then display the most recent + version of that event. + + XXX: in future we may wish to consider ordering replacements (or relations + in general) via a DAG rather than using `origin_server_ts` to determine + ordering - particularly to mitigate potential abuse of edits applied by + moderators. Whatever, Care must be taken by the server to ensure that if + there are multiple replacement events, the server must consistently choose + the same one as all other servers. + * `m.reference` - Intended in future for handling replies and threading, these let you define an event which references an existing event. When aggregated, currently doesn't do anything special, but in future could @@ -122,19 +134,26 @@ Different subtypes of references could be defined through additional fields on the `m.relationship` object, to distinguish between replies, threads, etc. This MSC doesn't attempt to define these subtypes. - XXX: do we want to support multiple parents for a m.reference event, if a given event - references different parents in different ways? + XXX: do we want to support multiple parents for a m.reference event, if a + given event references different parents in different ways? ### Sending relations -Related events are normal Matrix events, but it's possible that the server may need to -process them before sending them into a room (for instance, if we ever use a DAG to -define the ordering of an m.relations +Related events are normal Matrix events, and can be sent by the normal /send +API. -Similar to membership events, related events may be sent either by the normal +The server should postprocess relations if needed before sending +them into a room, for instance, if we ever use a DAG to define the ordering of +`m.replaces` relations, the server would be responsible for specifying the +parent pointers on the DAG. -Sending a related event uses an equivalent of the normal send API (with an -equivalent `PUT` API): +If the user tries to send the same annotation multiple times for the same +event `type` (e.g. `m.reaction`) and aggregation `key` (e.g. πŸ‘) then the +server should respond with 403 and error FIXME. + +Similar to membership events, a convenience API is also provided to highlight +that the server may post-process the event, and whose URL structures the +semantics of the relation being sent more clearly: ``` POST /_matrix/client/r0/rooms/{roomId}/send_relation/{parent_id}/{relation_type}/{event_type} @@ -143,9 +162,85 @@ POST /_matrix/client/r0/rooms/{roomId}/send_relation/{parent_id}/{relation_type} } ``` +The `parent_id` is: + + * For annotations the event being displayed (which may be an edit) + * For replaces/edits the original event (not previous edits) + * For references should be the event being referenced + +An idempotent version is available as normal by using PUT as the HTTP method +and appending a transaction ID to the URL. ### Receiving relations +#### Unbundled relation events + +Relations are received during non-gappy incremental syncs as normal discrete +Matrix events. These are called "unbundled relation events". + +There is one special case: `unsigned.count` is provided on annotation events, +calculated by the server to provide the current absolute count of the given +annotation key as of that point of the event, to avoid the client having to +accurately track the absolute value itself. + + XXX: this special case isn't implemented in Synapse yet + +For instance, an incremental sync might include the following: + +```json +{ + "type": "m.reaction", + "sender": "@matthew:matrix.org", + "content": { + "m.relationship": { + "rel_type": "m.annotation", + "event_id": "$some_event_id", + "key": "πŸ‘" + } + }, + "unsigned": { + "count": 1234, + } +} +``` + +...to indicate that Matthew just thumbsupped a given event, bringing the current +total to 1234 thumbsups. + +#### Bundled relations + +Other than during non-gappy incremental syncs, an aggregate view of relation +events should be bundled into the unsigned data of the event they relate to, +rather than sending un-bundled individual relation events. This is called a +bundled relation (or bundled aggregation), and by sending a summary of the +aggregations, avoids us having to always send lots of individual unbundled +relation events individually to the client. + +Any API which receives events should bundle relations (apart from non-gappy +incremental syncs), for instance: initial sync, gappy incremental sync, +/messages and /context. + +The bundled relations are grouped according to their `rel_type`, and then +paginated within each group using Matrix's normal pagination idiom of `count`, +`limited` and `chunk` fields - respectively giving the total number of +elements in the list, whether that list has been truncated, and an array of +elements in the list. + +The format of the aggregated value in the bundle depends on the relation type: + + * `m.annotation` aggregations provide the `type` of the relation event, the + aggregation `key`, and the `count` of the number of annotations of that + `type` and `key` which reference that event. + * `m.replace` relations do not appear in bundled aggregations at all, as they + instead replace the original event returned to the client (returning the most + recent version of that event). + * `m.reference` list the `event_id` and event `type` of the events which + reference that event. + +For instance, the below example shows an event +with four bundled relations; 3 thumbsup reaction annotations and one +reference. + ```json { ..., @@ -177,87 +272,76 @@ POST /_matrix/client/r0/rooms/{roomId}/send_relation/{parent_id}/{relation_type} } ``` +#### Handling limited (gappy) syncs + +For the special case of a gappy incremental sync, many reaction events may +have occurred during the gap. It would be inefficient to send each one +individually to the client, but it would also be inefficient to send all +possible bundled aggregations to the client too. + +The simplest thing a client can do is to just throw away its history for a +room on seeing a gappy incremental sync, and then re-paginate the history of +the room using /messages in order to get a consistent view of the relations +which may have changed during the gap. However, this is quite inefficient, +and prohibits the client from persisting multiple sections of timeline for a +given room. + +Alternatively, the server tells the client the event IDs of events which +predate the gap which received reactions during the gap. This means that the +client can invalidate its copy of those events (if any) and then requery them +(including their bundled relations) from the server if/when needed. + +The server does this with the new `stale_relations` field of each room object +in the sync response. The `stale_relations` field lists all the event ids +prior to the gap which had updated relations during the gap. The event ids +are grouped by relation type, and limited to N entries for efficiency. N +should be 100. If the number of events with stale relations exceeds N, the +list is marked as `limited` as per the normal Matrix pagination model. We do +not include events referenced by `m.reference` as stale, in favour of more +sophisticated pagination techniques in future. For instance: +```json +"!roomid:matrix.org": { + "account_data": {}, + "ephemeral": {}, + "state": {}, + "summary": {}, + "timeline": {}, + "unread_notifications": {}, + "stale_events": { + "m.annotations": { + "chunk": [ + "$12345676321:matrix.org", + "$12345321432:matrix.org", + ], + "limited": false + } + } +} +``` -### Aggregating and paginating relations - -Relations may be sent to clients - -In large rooms an event may end up having a large number of related events, and -so we do not want to have to include all relations when sending the event to the -client. How we limit depends on the relation type. - -Annotations are grouped by their event type and an "aggregation key", and the -top N groups with the highest number is included in the event. For example, -reactions would be implemented as a `m.reaction` with aggregation key of e.g. -`πŸ‘`. - - TODO: Should we include anything other than event type, aggregation key and - count? - -Replacements replace the original event, and so no aggregation is required. -Care must be taken by the server to ensure that if there are multiple -replacement events, the server must consistently choose the same one as all other servers. -The replacement event should also include a reference to the original event ID -so that clients can tell that the message has been edited. - -Permalinks to edited events should capture the event ID that the sender is viewing -at that point (which might be an edit ID). The client viewing the permalink -should resolve this ID to the source event ID, and then display the most recent -version of that event. - -For references, the original event should include the list of `type` and -`event_id` of the earliest N references. - - TODO: Do we need the type? Do we want to differentiate between replies and - other types of references? This assumes the type of the related event gives - some hint to clients. - -In each case where we limit what is included there should be a corresponding API -to paginate the full sets of events. Annotations would need APIs for both -fetching more groups and fetching events in a group. - - - - - -## End to end encryption - -Since the server bundles related events, the relation information must not be -encrypted end-to-end. - -For aggregations of annotations there are two options: - -1. Don't group together annotations, and have the aggregation `key` encrypted, so - as to not leak how someone reacted (though server would still see that they - did). -2. In some way encrypt the aggregation `key`, with the properties that different - users and clients reacting in the same way to the same event produce the same - `key`, but isn't something the server can calculate and is - different between different events (to stop statistical analysis). Clients - also need to be able to go from encrypted `key` to the actual - reaction. - - One suggestion here is to use the message key of the parent event to encrypt the - aggregation `key`. - - -## CS API +This shows that in the gappy sync response, a given room has two events prior +to the gap which received new annotations during the gap. Therefore if the +client has cached a local copy of those events, it should invalidate them, and +subsequently refresh them as needed. +To refresh events, we need an API to load arbitrary events from the room in +bulk, which the CS API doesn't currently provide. We propose extending GET +`{roomId}/event/{eventId}` to accept a list of event IDs on the URL, e.g: -Whenever an event that has relations is sent to the client, e.g. sync, pagination, -event search etc, the server bundles the relations into the event as per above. +`GET /_matrix/client/r0/rooms/{roomId}/event/{eventId},{eventId},{eventId}` -The `parent_id` is: +...which returns an array of events with the given IDs. - * For annotations the event being displayed (which may be an edit) - * For replaces/edits the original event (not previous edits) - * For references should be the event being referenced + XXX: Synapse hasn't implemented any of this section yet. -For the sync API, clients need to be aware of both bundled relations as well as -incremental standalone relation events in the sync response. +#### Paginating aggregations -## Pagination +Bundles of relations for a given event are +paginated to prevent overloading the client with relations, and may be traversed by +via the new `/relations` API (which iterates over all relations for an event) or the +new `/aggregations` API (which iterates over the groups of relations, or the relations +within a group). Our requirements that we need to paginate over: * The relations of a given event, via a new `/relations` API. @@ -278,8 +362,6 @@ Our requirements that we need to paginate over: m.room.message (in future), it wouldn't be very useful to say "3 image replies and 2 msg replies". -### API - We provide two API endpoints, one to paginate relations based in normal topological order, the second to paginate aggregated annotations. @@ -367,32 +449,27 @@ GET /_matrix/client/r0/rooms/!asd:matrix.org/aggregations/$1cd23476/m.annotation } ``` -### Client Server API - - -To receive relations: - -Relation events are then aggregated together based on the behaviour implied by -their `rel_type`, and bundled appropriately their target event when you /sync. -Additional APIs are available to send relations and paginate them. - -To send relations: - - -Clients receive relations as normal events in /sync (aka 'unbundled relations'), -or may also be aggregated together by the server, and presented as -a 'bundle' attached to the original event. +## End to end encryption -Bundles of relations for a given event are -paginated to prevent overloading the client with relations, and may be traversed by -via the new `/relations` API (which iterates over all relations for an event) or the -new `/aggregations` API (which iterates over the groups of relations, or the relations -within a group). +FIXME +Since the server bundles related events, the relation information must not be +encrypted end-to-end. -### E2E Encryption +For aggregations of annotations there are two options: +1. Don't group together annotations, and have the aggregation `key` encrypted, so + as to not leak how someone reacted (though server would still see that they + did). +2. In some way encrypt the aggregation `key`, with the properties that different + users and clients reacting in the same way to the same event produce the same + `key`, but isn't something the server can calculate and is + different between different events (to stop statistical analysis). Clients + also need to be able to go from encrypted `key` to the actual + reaction. + One suggestion here is to use the message key of the parent event to encrypt the + aggregation `key`. ## Edge cases From 9c90cc1db3391e072000f886aabed5a24f16ff7b Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Fri, 5 Jul 2019 01:13:28 +0100 Subject: [PATCH 29/46] s/stale_relations/stale_events/ --- proposals/1849-aggregations.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index c6ab57de25d..dc054ab165c 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -277,7 +277,7 @@ reference. For the special case of a gappy incremental sync, many reaction events may have occurred during the gap. It would be inefficient to send each one individually to the client, but it would also be inefficient to send all -possible bundled aggregations to the client too. +possible bundled aggregations to the client. The simplest thing a client can do is to just throw away its history for a room on seeing a gappy incremental sync, and then re-paginate the history of @@ -291,8 +291,8 @@ predate the gap which received reactions during the gap. This means that the client can invalidate its copy of those events (if any) and then requery them (including their bundled relations) from the server if/when needed. -The server does this with the new `stale_relations` field of each room object -in the sync response. The `stale_relations` field lists all the event ids +The server does this with the new `stale_events` field of each room object +in the sync response. The `stale_events` field lists all the event ids prior to the gap which had updated relations during the gap. The event ids are grouped by relation type, and limited to N entries for efficiency. N should be 100. If the number of events with stale relations exceeds N, the From 3f3d60e656fae3b38f9dbcda0bfd26dc7a8af700 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Tue, 16 Jul 2019 05:57:34 -0700 Subject: [PATCH 30/46] rewrite the paginations section --- proposals/1849-aggregations.md | 111 ++++++++++++++------------------- 1 file changed, 47 insertions(+), 64 deletions(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index dc054ab165c..8c3a80bed58 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -237,6 +237,12 @@ The format of the aggregated value in the bundle depends on the relation type: * `m.reference` list the `event_id` and event `type` of the events which reference that event. + XXX: An alternative approach could be to (also?) use a filter to + specify/override how to aggregate custom relation types, which would then + also be used to inform /sync how we want to receive our bundled relations. + However, we really need to be better understand how to do custom relation + types first... + For instance, the below example shows an event with four bundled relations; 3 thumbsup reaction annotations and one reference. @@ -337,41 +343,16 @@ bulk, which the CS API doesn't currently provide. We propose extending GET #### Paginating aggregations -Bundles of relations for a given event are -paginated to prevent overloading the client with relations, and may be traversed by -via the new `/relations` API (which iterates over all relations for an event) or the -new `/aggregations` API (which iterates over the groups of relations, or the relations -within a group). - -Our requirements that we need to paginate over: - * The relations of a given event, via a new `/relations` API. - * For replacements (i.e. edits) we get a paginated list of all edits on the source event. - * For annotations (i.e. reactions) we get the full list of reactions for the source event. - * Groups of annotations, via a new `/aggregations` API. - * Need to paginate across the different groups (i.e. how many different - reactions of different types did it get?) - * List all the reactions individually per group for this message - * References (i.e. threads of replies) - * We don't bundle contents in the references (at least for now); instead we - just follow the event IDs to stitch the right events back together. - * We could include a count of the number of references to a given event. - * We just provide the event IDs (to keep it nice and normalised) in a dict; we - can denormalise it later for performance if needed by including the event - type or whatever. We could include event_type if it was useful to say "5 - replies to this message", except given event types are not just - m.room.message (in future), it wouldn't be very useful to say "3 image - replies and 2 msg replies". - -We provide two API endpoints, one to paginate relations based in normal -topological order, the second to paginate aggregated annotations. - -Both APIs behave in a similar way to `/messages`, except using `next_batch` and -`prev_batch` names (in line with `/sync` API). Clients can start paginating -either from the earliest or latest events using the `dir` param. - -Standard pagination API looks like the following, where you can optionally -specify relation and event type to filter by. It lists all the relations -in topological order. +A single event can have lots of associated relations, and we do not want to +overload the client by including them all in a bundle. Instead, we provide two +new APIs in order to paginate over the relations, which behave in a similar +way to `/messages`, except using `next_batch` and `prev_batch` names (in line +with `/sync` API). Clients can start paginating either from the earliest or +latest events using the `dir` param. + +The `/relations` API lets you iterate over all the unbundled relations +associated with an event in standard topological order. You can optionally +filter by a given type of relation and event type: ``` GET /_matrix/client/r0/rooms/{roomID}/relations/{eventID}[/{relationType}[/{eventType}]] @@ -391,24 +372,14 @@ GET /_matrix/client/r0/rooms/{roomID}/relations/{eventID}[/{relationType}[/{even } ``` -The aggregated pagination API operates in two modes, the first is to paginate -the groups themselves, returning aggregated results: +The `/aggregations` API lets you iterate over bundled relations, and within them. + +To iterate over the bundled relations for an event (optionally filtering by relation type and target event type): ``` GET /_matrix/client/r0/rooms/{roomID}/aggregations/{eventID}[/{relationType}][/{eventType}][?filter=id] ``` -By default, the aggregation behaviour is defined by the relation type: - * rel_type of `m.annotation` == group by count, and order by count desc - * rel_type of `m.replace` == we just get the most recent message, no bundles. - * rel_type of `m.reference` == we get the IDs of the events replying to us, and - the total count of replies to this msg - -In future, we could use a filter to specify/override how to aggregate the relations, -which would then also be used to inform /sync how we want to receive our bundled -relations. (However, we really need to be better understand how to do custom -relation types first...) - ```json { "chunk": [ @@ -423,7 +394,10 @@ relation types first...) } ``` -The second mode of operation is to paginate within a group, in normal topological order: +To iterate over the unbundled relations within a specific bundled relation, you +use the following API form, identifying the bundle based on its `key` +(therefore this only applies to `m.annotation`, as it is the only current +`rel_type` which groups relations via `key`) ``` GET /_matrix/client/r0/rooms/{roomID}/aggregations/{eventID}/${relationType}/{eventType}/{key} @@ -451,26 +425,35 @@ GET /_matrix/client/r0/rooms/!asd:matrix.org/aggregations/$1cd23476/m.annotation ## End to end encryption -FIXME +Since the server has to be able to bundle related events, structural +information about relations cannot be encrypted end-to-end, and so the +`m.relates_to` field should not be included in the ciphertext. -Since the server bundles related events, the relation information must not be -encrypted end-to-end. +For instance, an encrypted `m.replace` looks like this: -For aggregations of annotations there are two options: +```json +{ + "content": { + "algorithm": "m.megolm.v1.aes-sha2", + "ciphertext": "AwgBErA....", + "device_id": "NBHOQUBWME", + "m.relates_to": { + "event_id": "$15632219072753999gNDqf:matrix.org", + "rel_type": "m.replace" + }, + "sender_key": "rt/7v+UV9cw9PzXEVk7gjLe8kLxuu/e3075PgPi9XVU", + "session_id": "q9Okpk4fK+SqSPvTBbhWPZt39LrTEj8vuQdcIK/iYa4" + }, + "sender": "@matthew:matrix.org", + "type": "m.room.encrypted", +} +``` -1. Don't group together annotations, and have the aggregation `key` encrypted, so - as to not leak how someone reacted (though server would still see that they - did). -2. In some way encrypt the aggregation `key`, with the properties that different - users and clients reacting in the same way to the same event produce the same - `key`, but isn't something the server can calculate and is - different between different events (to stop statistical analysis). Clients - also need to be able to go from encrypted `key` to the actual - reaction. +For `m.annotation` relations, the annotation SHOULD be encrypted, encrypting the `key` field using the same message key as the original message. However, while transition to this system, reactions may be sent entirely unencrypted in an E2E room. - One suggestion here is to use the message key of the parent event to encrypt the - aggregation `key`. + FIXME: spec how these look. +## Redactions ## Edge cases From d98d5b37089b1d9f31870287b0ff2f66907f2837 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Tue, 30 Jul 2019 01:25:33 +0200 Subject: [PATCH 31/46] finish rewriting MSC1849, in theory, incorporating all review so far --- proposals/1849-aggregations.md | 309 ++++++++++++++++++++++++++------- 1 file changed, 246 insertions(+), 63 deletions(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index 8c3a80bed58..7673ca01579 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -28,12 +28,17 @@ Instead, this proposal seeks to solve this problem by: This proposal introduces the concept of relations, which can be used to associate new information with an existing event. -Relations are any event which have an `m.relationship` mixin in their +Relations are any event which have an `m.relationship` field in their contents. The `m.relationship` field must include a `rel_type` field that gives the type of relationship being defined, and the `event_id` field that gives the event which is the target of the relation. All the information about the relationship lives under the `m.relationship` key. + FIXME: in practice, clients have ended up using `m.relates_to` rather than + `m.relationship`, based on an earlier version of this MSC. It's unclear + whether to migrate the clients to `m.relationship` or give up and stick with + `m.relates_to`. + If it helps, you can think of relations as a "subject verb object" triple, where the subject is the relation event itself; the verb is the `rel_type` field of the `m.relationship` and the object is the `event_id` field. @@ -45,7 +50,7 @@ defined. ### Relation types -This proposal defines three `rel_type`s, each which provide different behaviour +This proposal defines three `rel_type`s, each of which provide different behaviour when aggregated: * `m.annotation` - Intended primarily for handling emoji reactions, these let @@ -90,7 +95,7 @@ For instance, an `m.room.message` which `replaces` an existing event looks like: "body": "Hello! My name is bar", "msgtype": "m.text", }, - "m.relates_to": { + "m.relationship": { "rel_type": "m.replace", "event_id": "$some_event_id", } @@ -120,12 +125,14 @@ For instance, an `m.room.message` which `references` an existing event would look like: ```json -"type": "m.room.message", -"content": { - "body": "i <3 shelties", - "m.relationship": { - "rel_type": "m.reference", - "event_id": "$another_event_id" +{ + "type": "m.room.message", + "content": { + "body": "i <3 shelties", + "m.relationship": { + "rel_type": "m.reference", + "event_id": "$another_event_id" + } } } ``` @@ -229,14 +236,22 @@ elements in the list. The format of the aggregated value in the bundle depends on the relation type: * `m.annotation` aggregations provide the `type` of the relation event, the - aggregation `key`, and the `count` of the number of annotations of that - `type` and `key` which reference that event. - * `m.replace` relations do not appear in bundled aggregations at all, as they - instead replace the original event returned to the client (returning the most - recent version of that event). + aggregation `key`, the `origin_server_ts` of the first reaction to that event, + and the `count` of the number of annotations of that `type` and `key` which + reference that event. + * `m.replace` aggregations provide the most recent edited version of the event + in the main event body, and then in the bundle itself there are keys for + `event_id` (the id of the original event at the root of the sequence of edits). + `origin_server_ts` (for when it was edited) and `sender` for who did the edit. + This allows the client to identify the message as an edit. * `m.reference` list the `event_id` and event `type` of the events which reference that event. + XXX: shouldn't the origin_server_ts and sender of an edit event already tell you + who sent it and when? Why do we also have it on the bundle data? + + XXX: Does synapse send the most recent version of an edited event when bundling? + XXX: An alternative approach could be to (also?) use a filter to specify/override how to aggregate custom relation types, which would then also be used to inform /sync how we want to receive our bundled relations. @@ -257,6 +272,7 @@ reference. { "type": "m.reaction", "key": "πŸ‘", + "origin_server_ts": 1564435280, "count": 3 } ], @@ -278,6 +294,10 @@ reference. } ``` + FIXME: the server needs to generate the `origin_server_ts` of the first + reaction in a given group, to allow clients which want to do chronological + ordering to do so. + #### Handling limited (gappy) syncs For the special case of a gappy incremental sync, many reaction events may @@ -350,7 +370,7 @@ way to `/messages`, except using `next_batch` and `prev_batch` names (in line with `/sync` API). Clients can start paginating either from the earliest or latest events using the `dir` param. -The `/relations` API lets you iterate over all the unbundled relations +The `/relations` API lets you iterate over all the **unbundled** relations associated with an event in standard topological order. You can optionally filter by a given type of relation and event type: @@ -372,9 +392,16 @@ GET /_matrix/client/r0/rooms/{roomID}/relations/{eventID}[/{relationType}[/{even } ``` -The `/aggregations` API lets you iterate over bundled relations, and within them. + FIXME: we need to spell out that this API should return the original message + when paginating over m.replaces relations for a given message. Synapse currently + looks to include this as an `original_event` field alongside `chunk` on all relations, + which feels very redundant when we only need it for edits. Either we specialcase it + for edits, or we just have the client go call /event to grab the contents of the original? -To iterate over the bundled relations for an event (optionally filtering by relation type and target event type): +The `/aggregations` API lets you iterate over **bundled** relations, and within them. + +To iterate over the bundled relations for an event (optionally filtering by +relation type and target event type): ``` GET /_matrix/client/r0/rooms/{roomID}/aggregations/{eventID}[/{relationType}][/{eventType}][?filter=id] @@ -386,6 +413,7 @@ GET /_matrix/client/r0/rooms/{roomID}/aggregations/{eventID}[/{relationType}][/{ { "type": "m.reaction", "key": "πŸ‘", + "origin_server_ts": 1564435280, "count": 5, } ], @@ -394,10 +422,16 @@ GET /_matrix/client/r0/rooms/{roomID}/aggregations/{eventID}[/{relationType}][/{ } ``` + FIXME: what should we expect to see for bundled `m.replace` if anything? + Synapse currently returns an empty chunk for an event with subsequent edits, + while you might expect to receive the most recent edit. Similarly, what do + you get for `m.reference`? Should it be an array of event_ids for replies + to this msg? + To iterate over the unbundled relations within a specific bundled relation, you use the following API form, identifying the bundle based on its `key` (therefore this only applies to `m.annotation`, as it is the only current -`rel_type` which groups relations via `key`) +`rel_type` which groups relations via `key`). ``` GET /_matrix/client/r0/rooms/{roomID}/aggregations/{eventID}/${relationType}/{eventType}/{key} @@ -427,7 +461,7 @@ GET /_matrix/client/r0/rooms/!asd:matrix.org/aggregations/$1cd23476/m.annotation Since the server has to be able to bundle related events, structural information about relations cannot be encrypted end-to-end, and so the -`m.relates_to` field should not be included in the ciphertext. +`m.relationship` field should not be included in the ciphertext. For instance, an encrypted `m.replace` looks like this: @@ -437,7 +471,7 @@ For instance, an encrypted `m.replace` looks like this: "algorithm": "m.megolm.v1.aes-sha2", "ciphertext": "AwgBErA....", "device_id": "NBHOQUBWME", - "m.relates_to": { + "m.relationship": { "event_id": "$15632219072753999gNDqf:matrix.org", "rel_type": "m.replace" }, @@ -449,19 +483,180 @@ For instance, an encrypted `m.replace` looks like this: } ``` -For `m.annotation` relations, the annotation SHOULD be encrypted, encrypting the `key` field using the same message key as the original message. However, while transition to this system, reactions may be sent entirely unencrypted in an E2E room. +For `m.annotation` relations, the annotation SHOULD be encrypted, encrypting +the `key` field using the same message key as the original message. However, +while transition to this system, reactions may be sent entirely unencrypted in +an E2E room. + +### Thoughts on encrypting the aggregation key + +XXX: this hasn't been locked down yet, and at this rate will form a different +MSC entirely, for E2E-safe reactions. But until that point, let's gather it +here: + +@jryans said: + +We have agreed IRL that encrypted aggregations should include additional +metadata so that clients can clearly distinguish unencrypted and encrypted +aggregation keys for presentation. In addition, the proposal might want to +clarify that e2e rooms might have both unencrypted and encrypted aggregation +keys (just like it's currently possible to send unencrypted regular messages +to an e2e room). + +@ara4n said: + +How about "alg": "A256CTR" which then matches how we encrypt attachments for +E2E. Given the aggregation keys are kindasorta mini encrypted attachments, +this doesn't seem unreasonable. Someone needs to doublecheck whether CTR is +the right mode, however, and if so what we do about IVs and hashes etc. + +@jryans said: + +The core idea is that the aggregation key will be encrypted with the same +message key as the original message. Maybe copying encryption metadata from +the original message is the right way to indicate that on the key? For +example: + +```json +{ + "type": "m.reaction", + "content": { + "m.relationship": { + "algorithm": "m.megolm.v1.aes-sha2", + "session_id": "", + "rel_type": "m.annotation", + "event_id": "$some_event_id", + "key": "" + } + } +} +``` - FIXME: spec how these look. +We likely also want to define that reaction should be left as `m.reaction` in +an encrypted room (rather than becoming `m.room.encrypted` as least Riot Web +does at the moment) because: + +* `m.room.encrypted` triggers default push rules for DM rooms +* `m.room.encrypted` is a bit silly for reactions since the relation data (the + interesting bit of the event) is already lifted out to clear text anyhow +* `m.room.encrypted` prevents server-side bundling for reactions which + currently checks for the `m.reaction` type + +@uhoreg said: + +I don't think the encryption mode matters too much. Megolm encrypts using +CBC, which is probably fine for short data like this. The main advantage of +CTR over CBC is that CTR can encrypt in parallel, which shouldn't be a huge +deal in this case. So I think the main question for whether we use CTR or CBC +is whether we want to be consistent with Megolm or with encrypted attachments. + I'd suggest that it's better to be consistent with Megolm in this case, +especially if the encryption/decryption will be handled in libolm (see below). + +As far as reusing the same encryption key from the original message for +encrypting the reaction, I think it's going to be a bit complicated. + +First of all, the session ID is somewhat tied to the sending user. At least +in matrix-js-sdk, looking up an incoming session ID requires the sender's +curve25519 key, which in this case would mean the sender of the original +message, and not the sender of the reaction. This can be figured out in one +of two ways: either you add the original sender's key to the `m.relationship` +data, or else get the client to look it up from the original event. (And if +you're looking up the sender's key from the original event, you could also +look up the session ID as well.) + +Secondly, if we just follow the megolm format, the encrypted result includes a +signature from the sender's ed25519 key. Again, this would be the sender of +the original event, and not the reaction, and obviously the sender of the +reaction will not be able to form a correct signature (unless they're also the +sender of the original event, or manage to crack the sender's key). We could +instead replace it with a signature from the reaction sender's ed25519 key, +but then of course the reaction keys won't be the same between senders. We +could drop the signature entirely, but that means that ((unless I'm missing +something) a homeserver admin who can read the original message could forge a +reaction from one of their users. So we may need to add a signature somewhere +else. + +Third, using the exact same key for encrypting the reaction as the original +message opens up a potential known-plaintext attack. AES (and any other +modern cipher) should be immune to known-plaintext, but if we can easily use a +different key, then we may want to do that anyways. One way that this can be +done is by deriving the aes key/hmac key/aes IV using HKDF(0, R_i, rel_type, +80), rather than HKDF(0, R_i, "MEGOLM_KEYS", 80). + +The main thing on the olm side is that, no matter what we do, we we'll need to +add some functions to libolm in order to do this. We could: + +1. add an olm function to get the decryption key for a particular message, and +let the client author handle the encryption/decryption themselves. This would +also include +[formatting](https://gitlab.matrix.org/matrix-org/olm/blob/master/docs/megolm.md#message-format) +the encrypted reaction key themselves. The advantage to this is that it would +be a fairly generic function, not specific to reactions, but overall, I think +this would probably be too annoying for a lot of client authors, and probably +wouldn't be used outside of reaction anyways. + +2. add olm functions to handle the encryption and decryption entirely. This +would probably end up being functions that are very specific to reactions, +which would be :( , but would probably result in client authors not hating us +(more). + +This, of course, is all going to be a load of fun to spec. :P ## Redactions +Relations may be redacted like any other event. In the case of `m.reaction` +this removes the reaction from the message. In the case of `m.replace` it +removes the edit version. In the case of `m.reference` it removes the +referencing event. + +In the UI, the act of redacting an edited message in the timeline should +redact the *all* versions of that message. It can do this by redacting the +original msg, while ensuring that clients locally discard any edits to a +redacted message on receiving a redaction. + + XXX: does Riot actually do this? + +The `m.relationship`.`rel_type` field should be preserved over redactions, so +that clients can distinguish redacted edits from normal redacted messages. + + FIXME: synapse doesn't do this yet + +##Β Local echo + +XXX: do we need to spell out how to handle local echo considerations? + +Remember to let users edit unsent messages (as this is a common case for +rapidly fixing a typo in a msg which is still in flight!) + ## Edge cases +How do you stop people reacting more than once with the same key? + 1. You error with 400 (M_UNKNOWN) if they try to react twice with the same key, locally + 2. You flatten duplicate reactions received over federation from the same user + when calculating your local aggregations + 3. You don't pass duplicate reactions received over federation to your local user. + 4. XXX: does synapse do 2 & 3 yet? + +Can you edit a reaction? + * It feels reasonable to say "if you want to edit a reaction, redact it and resend". + `rel_type` is immutable, much like `type`. + +Can you react to a reaction? + * Yes, at the protocol level. But you shouldn't expect clients to do anything + useful with it. + +Can you reply (via m.references) to a reaction/edit? + * Yes, at the protocol level. But you shouldn't expect clients to do anything + useful with it. + * Replying to a reaction should be treated like a normal message and have the + reply behaviour ignored. + * Replying to an edit should be treated in the UI as if you had replied to + the original message. + What happens when you react to an edit? * You should be able to, but the reaction should be attributed to the edit (or its contents) rather than the message as a whole. - * So how do you aggregate? - * Suggestion: edits gather their own reactions, and the clients should display + * Edits gather their own reactions, and the clients should display the reactions on the most recent edit. * This provides a social pressure to get your edits in quickly before there are many reactions, otherwise the reactions will get lost. @@ -489,46 +684,21 @@ How do you handle racing edits? affects the server implementation; the clients can trust the server to linearise correctly. -How do you remove a reaction? - * You redact it. - -Redactions - * Redacting an edited event in the UI should redact the original; the client - will need to redact the original event to make this happen. - * Is this not problematic when trying to remove illegal content from servers? - * Clients could also try to expand the relations and redact those too if they - wanted to, but in practice the server shouldn't send down relations to - redacted messages, so it's overkill. - * You can also redact specific relations if needed (e.g. to remove a reaction - from ever happening) - * If you redact an relation, we keep the relation DAG (and solve that metadata - leak alongside our others) +Which message types are reactable? + * Any. But perhaps we should provide some UI best practice guidelines: + * `m.room.message` must be reactable + * `m.sticker` too + * ...but anything else may not be rendered. What does it mean to call /context on a relation? * We should probably just return the root event for now, and then refine it in future for threading? - -Should we enforce that each user can only send one of each type of reaction to a msg? - * Yes. We can do that when sending at CS API as the first cut - * But what do we use as a response? We can do a 400 with an error code that tells us - why - i.e. that it's because you can't react multiple times. - * We mandate txn IDs to provide idempotency. - * (Or can we rely on including our own reactions in bundles to tell whether - are doublecounting our own reactions or not?) - * However, we need to be able to handle bad servers who send duplicate events anyway. - * The only way to do this will be at SS API, and refuse to accept duplicatee - events. - -Should we always include our own reactions in a bundle, to make it easier to redact them, -or to show the UI in the right state? - * Probably, but this can be a future refinement. - * ...but might be needed for imposing one type of reaction per msg. - -Should we stop reactions being sent by the normal /send API? + * XXX: what does synapse do here? What can we edit? * Only non-state events for now. * We can't change event types, or anything else which is in an E2E payload + * We can't change relation types either. How do diffs work on edits if you are missing intermediary edits? * We just have to ensure that the UI for visualising diffs makes it clear @@ -537,6 +707,13 @@ How do diffs work on edits if you are missing intermediary edits? What happens when we edit a reply? * We just send an m.replace which refers to the m.reference target; nothing special is needed. i.e. you cannot change who the event is replying to. + * The edited reply should ditch the fallback representation of the reply itself + however from `m.new_content` (specifically the `` tag in the + HTML, and the unparseable chevron prefixed text in the plaintext), as we + can assume that any client which can handle edits can also display replies + natively. + + XXX: make Riot do this Do we need to support retrospective references? * For something like "m.duplicate" to retrospectively declare that one event @@ -548,6 +725,12 @@ What power level do you need to be able to edit other people's messages, and how does it fit in with fedeation event auth rules? * 50, by default? +"Editing other people's messages is evil; we shouldn't allow it" + * Sorry, we have to bridge with systems which support cross-user edits. + * When it happens, we should make it super clear in the timeline that a message + was edited by a specific user. + * We do not recommend that native Matrix clients expose this as a feature. + ## Federation considerations In general, no special considerations are needed for federation; relational @@ -614,7 +797,7 @@ One way to potentially support this is to include the events (or a subset of the event) when grouping, so that clients have enough information to render them. However this dramatically inceases the size of the parent event if we bundle the full events inside, even if limit the number we bundle in. To reduce the -overhead the annotation event could include a `m.summary` field which gets +overhead the annotation event could include a `m.result` field which gets included. This would look something like the following, where the annotation is: @@ -623,10 +806,10 @@ This would look something like the following, where the annotation is: { "type": "m.bot_command_response", "content": { - "m.summary": { + "m.result": { "state": "success", }, - "m.relates_to": { + "m.relationship": { "type": "m.annotation", "key": "" } @@ -647,7 +830,7 @@ and gets bundled into an event like: "count": 1, "chunk": [ { - "m.summary": { + "m.result": { "state": "success", }, } @@ -662,7 +845,7 @@ and gets bundled into an event like: This is something that could be added later on. A few issues with this are: - * How does this work with E2EE? How do we encrypt the `m.summary`? + * How does this work with E2EE? How do we encrypt the `m.result`? * We would end up including old annotations that had been superceded, should these be done via edits instead? @@ -674,7 +857,7 @@ Shape of ```json "content": { - "m.relates_to": { + "m.relationship": { "m.reference": { "event_id": "$another:event.com" } @@ -685,7 +868,7 @@ versus ```json "content": { - "m.relates_to": { + "m.relationship": { "rel_type": "m.reference", "event_id": "$another:event.com" } From fa7c338a42389bb378cd2e52c1e62654e7fb0c8d Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Tue, 30 Jul 2019 01:37:51 +0200 Subject: [PATCH 32/46] fix typos --- proposals/1849-aggregations.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index 7673ca01579..228e99dab63 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -1,6 +1,6 @@ # Proposal for aggregations via relations -##Β Problem +## Problem It's common to want to send events in Matrix which relate to existing events - for instance, reactions, edits and even replies/threads. @@ -151,7 +151,7 @@ API. The server should postprocess relations if needed before sending them into a room, for instance, if we ever use a DAG to define the ordering of -`m.replaces` relations, the server would be responsible for specifying the +`m.replace` relations, the server would be responsible for specifying the parent pointers on the DAG. If the user tries to send the same annotation multiple times for the same @@ -393,7 +393,7 @@ GET /_matrix/client/r0/rooms/{roomID}/relations/{eventID}[/{relationType}[/{even ``` FIXME: we need to spell out that this API should return the original message - when paginating over m.replaces relations for a given message. Synapse currently + when paginating over `m.replace` relations for a given message. Synapse currently looks to include this as an `original_event` field alongside `chunk` on all relations, which feels very redundant when we only need it for edits. Either we specialcase it for edits, or we just have the client go call /event to grab the contents of the original? From 1409119da2440925b618601a5528f68e6c415b36 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Tue, 30 Jul 2019 12:42:24 +0200 Subject: [PATCH 33/46] Update proposals/1849-aggregations.md Co-Authored-By: Travis Ralston --- proposals/1849-aggregations.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index 228e99dab63..d934ef28c0a 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -228,7 +228,7 @@ incremental syncs), for instance: initial sync, gappy incremental sync, /messages and /context. The bundled relations are grouped according to their `rel_type`, and then -paginated within each group using Matrix's normal pagination idiom of `count`, +paginated within each group using Matrix's defined pagination idiom of `count`, `limited` and `chunk` fields - respectively giving the total number of elements in the list, whether that list has been truncated, and an array of elements in the list. From e8fde9dcceeee6cb05b6df4a07c8110a7ed53c1e Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Tue, 30 Jul 2019 13:26:21 +0200 Subject: [PATCH 34/46] Update proposals/1849-aggregations.md Co-Authored-By: Travis Ralston --- proposals/1849-aggregations.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index d934ef28c0a..62c9776defc 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -318,7 +318,7 @@ client can invalidate its copy of those events (if any) and then requery them (including their bundled relations) from the server if/when needed. The server does this with the new `stale_events` field of each room object -in the sync response. The `stale_events` field lists all the event ids +in the sync response. The `stale_events` field lists all the event IDs prior to the gap which had updated relations during the gap. The event ids are grouped by relation type, and limited to N entries for efficiency. N should be 100. If the number of events with stale relations exceeds N, the From 4d236c67ffc60406370e2da2b9afb58334129d52 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Tue, 30 Jul 2019 13:31:15 +0200 Subject: [PATCH 35/46] incorporate some of the PR review --- proposals/1849-aggregations.md | 42 ++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index 62c9776defc..266f5a6095a 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -7,7 +7,7 @@ for instance, reactions, edits and even replies/threads. Clients typically need to track the related events alongside the original event they relate to, in order to correctly display them. For instance, -reaction events need to aggregated together by summing and be shown next to +reaction events need to be aggregated together by summing and be shown next to the event they react to; edits need to be aggregated together by replacing the original event and subsequent edits; replies need to be indented after the message they respond to, etc. @@ -163,12 +163,18 @@ that the server may post-process the event, and whose URL structures the semantics of the relation being sent more clearly: ``` -POST /_matrix/client/r0/rooms/{roomId}/send_relation/{parent_id}/{relation_type}/{event_type} +PUT /_matrix/client/r0/rooms/{roomId}/send_relation/{parent_id}/{relation_type}/{event_type}/{txn_id}?key=%F0%9F%91%8D { // event contents } ``` + XXX: Erik: why do we need the `key` as a querystring param here rather + than just fishing it out of the event contents when needed? + + XXX: Erik: are you okay to kill the POST form of this, given we apparently + killed the POST form of /send back in 2015 in https://github.com/matrix-org/matrix-doc/commit/5e30b5b8d74cdfbd764175fd735b3c39d652453e + The `parent_id` is: * For annotations the event being displayed (which may be an edit) @@ -185,7 +191,7 @@ and appending a transaction ID to the URL. Relations are received during non-gappy incremental syncs as normal discrete Matrix events. These are called "unbundled relation events". -There is one special case: `unsigned.count` is provided on annotation events, +There is one special case: `unsigned.annotation_count` is provided on annotation events, calculated by the server to provide the current absolute count of the given annotation key as of that point of the event, to avoid the client having to accurately track the absolute value itself. @@ -206,7 +212,7 @@ For instance, an incremental sync might include the following: } }, "unsigned": { - "count": 1234, + "annotation_count": 1234, } } ``` @@ -250,17 +256,14 @@ The format of the aggregated value in the bundle depends on the relation type: XXX: shouldn't the origin_server_ts and sender of an edit event already tell you who sent it and when? Why do we also have it on the bundle data? - XXX: Does synapse send the most recent version of an edited event when bundling? - XXX: An alternative approach could be to (also?) use a filter to specify/override how to aggregate custom relation types, which would then also be used to inform /sync how we want to receive our bundled relations. However, we really need to be better understand how to do custom relation types first... -For instance, the below example shows an event -with four bundled relations; 3 thumbsup reaction annotations and one -reference. +For instance, the below example shows an event with five bundled relations; +three thumbsup reaction annotations, one replace, and one reference. ```json { @@ -272,7 +275,7 @@ reference. { "type": "m.reaction", "key": "πŸ‘", - "origin_server_ts": 1564435280, + "origin_server_ts": 1562763768320, "count": 3 } ], @@ -288,6 +291,11 @@ reference. ], "limited": false, "count": 1 + }, + "m.replace": { + "event_id": "$original_event_id", + "origin_server_ts": 1562763768320, // why do we need this? it should be the same as the main event + "sender": "@bruno1:localhost"// why do we need this? it should be the same as the main event } } } @@ -300,10 +308,10 @@ reference. #### Handling limited (gappy) syncs -For the special case of a gappy incremental sync, many reaction events may -have occurred during the gap. It would be inefficient to send each one -individually to the client, but it would also be inefficient to send all -possible bundled aggregations to the client. +For the special case of a gappy incremental sync, many relations (particularly +reactions) may have occurred during the gap. It would be inefficient to send +each one individually to the client, but it would also be inefficient to send +all possible bundled aggregations to the client. The simplest thing a client can do is to just throw away its history for a room on seeing a gappy incremental sync, and then re-paginate the history of @@ -313,13 +321,13 @@ and prohibits the client from persisting multiple sections of timeline for a given room. Alternatively, the server tells the client the event IDs of events which -predate the gap which received reactions during the gap. This means that the +predate the gap which received relations during the gap. This means that the client can invalidate its copy of those events (if any) and then requery them (including their bundled relations) from the server if/when needed. The server does this with the new `stale_events` field of each room object in the sync response. The `stale_events` field lists all the event IDs -prior to the gap which had updated relations during the gap. The event ids +prior to the gap which had updated relations during the gap. The event IDs are grouped by relation type, and limited to N entries for efficiency. N should be 100. If the number of events with stale relations exceeds N, the list is marked as `limited` as per the normal Matrix pagination model. We do @@ -413,7 +421,7 @@ GET /_matrix/client/r0/rooms/{roomID}/aggregations/{eventID}[/{relationType}][/{ { "type": "m.reaction", "key": "πŸ‘", - "origin_server_ts": 1564435280, + "origin_server_ts": 1562763768320, "count": 5, } ], From f63b20aeac3c380105693385e46082d4948aef0d Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Tue, 30 Jul 2019 14:20:44 +0200 Subject: [PATCH 36/46] remaining review --- proposals/1849-aggregations.md | 42 ++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index 266f5a6095a..3c0b0e2e5dc 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -56,7 +56,7 @@ when aggregated: * `m.annotation` - Intended primarily for handling emoji reactions, these let you define an event which annotates an existing event. The annotations are typically presented alongside the event in the timeline. When aggregated, - it groups events together based on their `key` and returns a `count`. + it groups events together based on their `key` and `type` and returns a `count`. Another usage of an annotation is e.g. for bots, who could use annotations to report the success/failure or progress of a command. @@ -156,7 +156,9 @@ parent pointers on the DAG. If the user tries to send the same annotation multiple times for the same event `type` (e.g. `m.reaction`) and aggregation `key` (e.g. πŸ‘) then the -server should respond with 403 and error FIXME. +server should respond with 400 and error M_INVALID_REL_TYPE. + + XXX: currently synapse returns 400 and `{"errcode":"M_UNKNOWN","error":"Can't send same reaction twice"}` Similar to membership events, a convenience API is also provided to highlight that the server may post-process the event, and whose URL structures the @@ -181,8 +183,7 @@ The `parent_id` is: * For replaces/edits the original event (not previous edits) * For references should be the event being referenced -An idempotent version is available as normal by using PUT as the HTTP method -and appending a transaction ID to the URL. +Any trailing slashes on the endpoint should be ignored. ### Receiving relations @@ -247,7 +248,7 @@ The format of the aggregated value in the bundle depends on the relation type: reference that event. * `m.replace` aggregations provide the most recent edited version of the event in the main event body, and then in the bundle itself there are keys for - `event_id` (the id of the original event at the root of the sequence of edits). + `event_id` (the ID of the original event at the root of the sequence of edits). `origin_server_ts` (for when it was edited) and `sender` for who did the edit. This allows the client to identify the message as an edit. * `m.reference` list the `event_id` and event `type` of the events which @@ -369,7 +370,7 @@ bulk, which the CS API doesn't currently provide. We propose extending GET XXX: Synapse hasn't implemented any of this section yet. -#### Paginating aggregations +#### Paginating relations and aggregations A single event can have lots of associated relations, and we do not want to overload the client by including them all in a bundle. Instead, we provide two @@ -400,6 +401,8 @@ GET /_matrix/client/r0/rooms/{roomID}/relations/{eventID}[/{relationType}[/{even } ``` +Any trailing slashes on the endpoint should be ignored. + FIXME: we need to spell out that this API should return the original message when paginating over `m.replace` relations for a given message. Synapse currently looks to include this as an `original_event` field alongside `chunk` on all relations, @@ -430,6 +433,8 @@ GET /_matrix/client/r0/rooms/{roomID}/aggregations/{eventID}[/{relationType}][/{ } ``` +Any trailing slashes on the endpoint should be ignored. + FIXME: what should we expect to see for bundled `m.replace` if anything? Synapse currently returns an empty chunk for an event with subsequent edits, while you might expect to receive the most recent edit. Similarly, what do @@ -492,9 +497,9 @@ For instance, an encrypted `m.replace` looks like this: ``` For `m.annotation` relations, the annotation SHOULD be encrypted, encrypting -the `key` field using the same message key as the original message. However, -while transition to this system, reactions may be sent entirely unencrypted in -an E2E room. +the `key` field using the same message key as the original message as per +below. However, while transitioning to this system, reactions may be sent +entirely unencrypted in an E2E room. ### Thoughts on encrypting the aggregation key @@ -624,8 +629,9 @@ redacted message on receiving a redaction. XXX: does Riot actually do this? -The `m.relationship`.`rel_type` field should be preserved over redactions, so -that clients can distinguish redacted edits from normal redacted messages. +The `m.relationship`.`rel_type` and `m.relationship`.`event_id` fields should +be preserved over redactions, so that clients can distinguish redacted edits +from normal redacted messages, and maintain reply ordering. FIXME: synapse doesn't do this yet @@ -639,7 +645,7 @@ rapidly fixing a typo in a msg which is still in flight!) ## Edge cases How do you stop people reacting more than once with the same key? - 1. You error with 400 (M_UNKNOWN) if they try to react twice with the same key, locally + 1. You error with 400 (M_INVALID_REL_TYPE) if they try to react twice with the same key, locally 2. You flatten duplicate reactions received over federation from the same user when calculating your local aggregations 3. You don't pass duplicate reactions received over federation to your local user. @@ -717,7 +723,8 @@ What happens when we edit a reply? special is needed. i.e. you cannot change who the event is replying to. * The edited reply should ditch the fallback representation of the reply itself however from `m.new_content` (specifically the `` tag in the - HTML, and the unparseable chevron prefixed text in the plaintext), as we + HTML, and the chevron prefixed text in the plaintext which we don't know + whether to parse as we don't know whether this is a reply or not), as we can assume that any client which can handle edits can also display replies natively. @@ -733,6 +740,9 @@ What power level do you need to be able to edit other people's messages, and how does it fit in with fedeation event auth rules? * 50, by default? + XXX: Synapse doesn't impose this currently - it lets anyone send an edit, + but then filters them out of bundled data. + "Editing other people's messages is evil; we shouldn't allow it" * Sorry, we have to bridge with systems which support cross-user edits. * When it happens, we should make it super clear in the timeline that a message @@ -776,10 +786,12 @@ we already have. So, we'll show inconsistent data until we backfill the gap. ##Β Security considerations -When using reactions for voting purposes we might well want to anonymise the +If using reactions for upvoting/downvoting purposes we would almost certainly want to anonymise the reactor, at least from other users if not server admins, to avoid retribution problems. This gives an unfair advantage to people who run their own servers however and -can cheat and deanonymise (and publish) reactor details. +can cheat and deanonymise (and publish) reactor details. In practice, reactions may +not be best used for upvote/downvote as at the unbundled level they are intrinsically +private data. Or in a MSC1228 world... we could let users join the room under an anonymous persona from a big public server in order to vote? However, such anonymous personae From b22868876c25f16db3fc39a61f65b538ced4f47f Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Tue, 30 Jul 2019 14:43:45 +0200 Subject: [PATCH 37/46] clarify `m.replace` bundle structure --- proposals/1849-aggregations.md | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index 3c0b0e2e5dc..3542e80ead6 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -247,16 +247,16 @@ The format of the aggregated value in the bundle depends on the relation type: and the `count` of the number of annotations of that `type` and `key` which reference that event. * `m.replace` aggregations provide the most recent edited version of the event - in the main event body, and then in the bundle itself there are keys for - `event_id` (the ID of the original event at the root of the sequence of edits). - `origin_server_ts` (for when it was edited) and `sender` for who did the edit. - This allows the client to identify the message as an edit. + in the main event contents, but with the metadata (sender & ts) of the + original event. Then in the bundle itself there are keys for `event_id` + (the ID of the original event at the root of the sequence of edits). + `origin_server_ts` (for when it was edited) and `sender` for who did the + edit. This allows the client to identify the message as an edit, and easily + tell who authored the edit and when (given the event itself tracks the + original message's metadata). * `m.reference` list the `event_id` and event `type` of the events which reference that event. - XXX: shouldn't the origin_server_ts and sender of an edit event already tell you - who sent it and when? Why do we also have it on the bundle data? - XXX: An alternative approach could be to (also?) use a filter to specify/override how to aggregate custom relation types, which would then also be used to inform /sync how we want to receive our bundled relations. @@ -295,8 +295,8 @@ three thumbsup reaction annotations, one replace, and one reference. }, "m.replace": { "event_id": "$original_event_id", - "origin_server_ts": 1562763768320, // why do we need this? it should be the same as the main event - "sender": "@bruno1:localhost"// why do we need this? it should be the same as the main event + "origin_server_ts": 1562763768320, + "sender": "@bruno1:localhost" } } } @@ -437,9 +437,11 @@ Any trailing slashes on the endpoint should be ignored. FIXME: what should we expect to see for bundled `m.replace` if anything? Synapse currently returns an empty chunk for an event with subsequent edits, - while you might expect to receive the most recent edit. Similarly, what do - you get for `m.reference`? Should it be an array of event_ids for replies - to this msg? + while you might expect to receive the most recent bundled aggregation data + for an edit. Similarly, what do you get for `m.reference`? Should it be an + array of event_ids for replies to this msg? Alternatively, should we just + ignore anything other than annotations, given the API is only meaningful for + annotations? To iterate over the unbundled relations within a specific bundled relation, you use the following API form, identifying the bundle based on its `key` From fb4446f0e50a2ecaf298d23a4769b2bc469b01f1 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Wed, 31 Jul 2019 17:05:18 +0200 Subject: [PATCH 38/46] wording tweaks --- proposals/1849-aggregations.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index 3542e80ead6..b0f3f4b2918 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -499,9 +499,9 @@ For instance, an encrypted `m.replace` looks like this: ``` For `m.annotation` relations, the annotation SHOULD be encrypted, encrypting -the `key` field using the same message key as the original message as per -below. However, while transitioning to this system, reactions may be sent -entirely unencrypted in an E2E room. +the `key` field using the same message key as the original message as per the +following section. However, while transitioning to this system, reactions may +be sent entirely unencrypted in an E2E room. ### Thoughts on encrypting the aggregation key @@ -619,9 +619,9 @@ This, of course, is all going to be a load of fun to spec. :P ## Redactions -Relations may be redacted like any other event. In the case of `m.reaction` -this removes the reaction from the message. In the case of `m.replace` it -removes the edit version. In the case of `m.reference` it removes the +Relations may be redacted like any other event. In the case of `m.annotation` +this removes the annotation from the message. In the case of `m.replace` it +removes that edit revision. In the case of `m.reference` it removes the referencing event. In the UI, the act of redacting an edited message in the timeline should From 6225aae6285da3fba29220a0d47878995a48ee0b Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Wed, 31 Jul 2019 17:08:59 +0200 Subject: [PATCH 39/46] Apply suggestions from code review Co-Authored-By: J. Ryan Stinnett --- proposals/1849-aggregations.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index b0f3f4b2918..c1a36d5c20f 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -60,7 +60,7 @@ when aggregated: Another usage of an annotation is e.g. for bots, who could use annotations to report the success/failure or progress of a command. -For example, an `m.reaction` event which `annotates` an existing event with a πŸ‘ +For example, an `m.reaction` event which annotates an existing event with a πŸ‘ looks like: ```json @@ -83,7 +83,7 @@ looks like: replacement content (allowing the normal `body` fields to be used for a fallback for clients who do not understand replacement events). -For instance, an `m.room.message` which `replaces` an existing event looks like: +For instance, an `m.room.message` which replaces an existing event looks like: ```json { @@ -121,7 +121,7 @@ For instance, an `m.room.message` which `replaces` an existing event looks like: bundle chains of references (i.e. threads). These do not yet replace `m.relates_to`-style replies however. -For instance, an `m.room.message` which `references` an existing event +For instance, an `m.room.message` which references an existing event would look like: ```json From 6751de67e0b96da6eba58a04c4a65ddaea6baf36 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Thu, 1 Aug 2019 12:05:10 +0200 Subject: [PATCH 40/46] PR review --- proposals/1849-aggregations.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index c1a36d5c20f..1364cdecf12 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -624,8 +624,11 @@ this removes the annotation from the message. In the case of `m.replace` it removes that edit revision. In the case of `m.reference` it removes the referencing event. +Trying to call `/relations` or `/aggregations` on a redacted message must return +a 404. + In the UI, the act of redacting an edited message in the timeline should -redact the *all* versions of that message. It can do this by redacting the +remove the message entirely from the timeline. It can do this by redacting the original msg, while ensuring that clients locally discard any edits to a redacted message on receiving a redaction. @@ -637,6 +640,10 @@ from normal redacted messages, and maintain reply ordering. FIXME: synapse doesn't do this yet +When a specific revision of an event is redacted, the client should manually +refresh the parent event via `/events` to grab whatever the replacement +revision is. + ##Β Local echo XXX: do we need to spell out how to handle local echo considerations? From eab778df26d426b7033ee40d18d792fbc437c7ad Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Thu, 1 Aug 2019 12:36:07 +0200 Subject: [PATCH 41/46] clarify ignored users --- proposals/1849-aggregations.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index 1364cdecf12..2aff54faebb 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -676,6 +676,12 @@ Can you reply (via m.references) to a reaction/edit? * Replying to an edit should be treated in the UI as if you had replied to the original message. +How do you handle ignored users? + * Information about relations sent from ignored users must never be sent to + the client, either in bundled or unbundled form. This is to let you block + someone from harassing you with emoji reactions (or using edits as a + side-channel to harass you). + What happens when you react to an edit? * You should be able to, but the reaction should be attributed to the edit (or its contents) rather than the message as a whole. From a63d27ecabfe959f354eed64d8d570aa419f6494 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Thu, 1 Aug 2019 12:53:57 +0200 Subject: [PATCH 42/46] incorporate erik feedback --- proposals/1849-aggregations.md | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index 2aff54faebb..b01f16b6651 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -165,18 +165,12 @@ that the server may post-process the event, and whose URL structures the semantics of the relation being sent more clearly: ``` -PUT /_matrix/client/r0/rooms/{roomId}/send_relation/{parent_id}/{relation_type}/{event_type}/{txn_id}?key=%F0%9F%91%8D +PUT /_matrix/client/r0/rooms/{roomId}/send_relation/{parent_id}/{relation_type}/{event_type}/{txn_id}[?key=%F0%9F%91%8D] { // event contents } ``` - XXX: Erik: why do we need the `key` as a querystring param here rather - than just fishing it out of the event contents when needed? - - XXX: Erik: are you okay to kill the POST form of this, given we apparently - killed the POST form of /send back in 2015 in https://github.com/matrix-org/matrix-doc/commit/5e30b5b8d74cdfbd764175fd735b3c39d652453e - The `parent_id` is: * For annotations the event being displayed (which may be an edit) From 4e8d370a28e365ed2380d7857d4b514f5e1b065f Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Thu, 1 Aug 2019 13:03:50 +0200 Subject: [PATCH 43/46] POST /event --- proposals/1849-aggregations.md | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index b01f16b6651..0f7b24c4952 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -341,7 +341,7 @@ sophisticated pagination techniques in future. For instance: "m.annotations": { "chunk": [ "$12345676321:matrix.org", - "$12345321432:matrix.org", + "$12345321432:matrix.org" ], "limited": false } @@ -358,7 +358,15 @@ To refresh events, we need an API to load arbitrary events from the room in bulk, which the CS API doesn't currently provide. We propose extending GET `{roomId}/event/{eventId}` to accept a list of event IDs on the URL, e.g: -`GET /_matrix/client/r0/rooms/{roomId}/event/{eventId},{eventId},{eventId}` +`POST /_matrix/client/r0/rooms/{roomId}/event` +```json +{ + "event_ids": [ + "$12345676321:matrix.org", + "$12345321432:matrix.org" + ] +} +``` ...which returns an array of events with the given IDs. From e8162331007512950f4303b89148b082d7d96dae Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Thu, 1 Aug 2019 13:24:02 +0200 Subject: [PATCH 44/46] clarify errors on /aggregations --- proposals/1849-aggregations.md | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index 0f7b24c4952..911d49a0ea1 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -437,13 +437,9 @@ GET /_matrix/client/r0/rooms/{roomID}/aggregations/{eventID}[/{relationType}][/{ Any trailing slashes on the endpoint should be ignored. - FIXME: what should we expect to see for bundled `m.replace` if anything? - Synapse currently returns an empty chunk for an event with subsequent edits, - while you might expect to receive the most recent bundled aggregation data - for an edit. Similarly, what do you get for `m.reference`? Should it be an - array of event_ids for replies to this msg? Alternatively, should we just - ignore anything other than annotations, given the API is only meaningful for - annotations? +Trying to iterate over an relation type which does not use an aggregation key +(i.e. `m.replace` and `m.reference`) should fail with 400 and error +M_INVALID_REL_TYPE. To iterate over the unbundled relations within a specific bundled relation, you use the following API form, identifying the bundle based on its `key` From 4a96865ef671459f52f144d84db35fa334731198 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Thu, 1 Aug 2019 13:49:05 +0200 Subject: [PATCH 45/46] add bruno's local echo considerations --- proposals/1849-aggregations.md | 44 ++++++++++++++++++++++++++++++---- 1 file changed, 40 insertions(+), 4 deletions(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index 911d49a0ea1..3f5663e1ca6 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -644,10 +644,46 @@ revision is. ##Β Local echo -XXX: do we need to spell out how to handle local echo considerations? - -Remember to let users edit unsent messages (as this is a common case for -rapidly fixing a typo in a msg which is still in flight!) +As clients only receive unbundled events through /sync, they need to locally +aggregate these unbundled events for their parent event, on top of any +server-side aggregation that might have already happened, to get a complete +picture of the aggregated relations for a given parent event, as a client +might not be aware of all relations for an event. Local aggregation should +thus also take the `m.relation` data in the `unsigned` of the parent event +into account if it has been sent already. The aggregation algorithm is the +same as the one described here for the server. + +For the best possible user experience, clients should also include unsent +relations into the local aggregation. When adding a relation to the send +queue, clients should locally aggregate it into the relations of the parent +event, ideally regardless of the parent event having an `event_id` already or +still being pending. If the client gives up on sending the relation for some +reason, the relation should be de-aggregated from the relations of the parent +event. If the client offers the user a possibility of manually retrying to +send the relation, it should be re-aggregated when the user does so. + +De-aggregating a relation refers to rerunning the aggregation for a given +parent event while not considering the de-aggregated event any more. + +Upon receiving the remote echo for any relations, a client is likely to remove +the pending event from the send queue. Here, it should also de-aggregate the +pending event from the parent event's relations, and re-aggregate the received +remote event from `/sync` to make sure the local aggregation happens with the +same event data as on the server. + +When adding a redaction for a relation to the send queue, the relation +referred to should be de-aggregated from the relations of the target of the +relation. Similar to a relation, when the sending of the redaction fails or +is cancelled, the relation should be aggregated again. + +To support creating relations for pending events, clients will need a way for +events to relate to one another before the `event_id` of the parent event is +known. When the parent event receives its remote echo, the target event id +(`m.relationship`.`event_id`) of any relations in the send queue will need to be +set the newly received `event_id`. + +Particularly, please remember to let users edit unsent messages (as this is a +common case for rapidly fixing a typo in a msg which is still in flight!) ## Edge cases From e57033a8abb9f6e34d2c3a53aaabf2b7d0333e53 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Thu, 1 Aug 2019 15:31:00 +0200 Subject: [PATCH 46/46] arbitrarily restrict trailing slashes for now https://github.com/matrix-org/matrix-doc/pull/1849#discussion_r309690954 --- proposals/1849-aggregations.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/proposals/1849-aggregations.md b/proposals/1849-aggregations.md index 3f5663e1ca6..7492db41521 100644 --- a/proposals/1849-aggregations.md +++ b/proposals/1849-aggregations.md @@ -177,7 +177,7 @@ The `parent_id` is: * For replaces/edits the original event (not previous edits) * For references should be the event being referenced -Any trailing slashes on the endpoint should be ignored. +The endpoint does not have any trailing slashes. ### Receiving relations @@ -403,7 +403,7 @@ GET /_matrix/client/r0/rooms/{roomID}/relations/{eventID}[/{relationType}[/{even } ``` -Any trailing slashes on the endpoint should be ignored. +The endpoint does not have any trailing slashes. FIXME: we need to spell out that this API should return the original message when paginating over `m.replace` relations for a given message. Synapse currently @@ -435,7 +435,7 @@ GET /_matrix/client/r0/rooms/{roomID}/aggregations/{eventID}[/{relationType}][/{ } ``` -Any trailing slashes on the endpoint should be ignored. +The endpoint does not have any trailing slashes. Trying to iterate over an relation type which does not use an aggregation key (i.e. `m.replace` and `m.reference`) should fail with 400 and error