event properties in API responses and AS API duplicated as top-level field

[joepie91]

When calling the /messages endpoint (also /search, /context, etc) each event in the chunk will have a number of top-level properties that duplicate those within unsigned:

• age (see also: age logic for events is broken #8429)
• redacted_because
• replaces_state
• invite_room_state
• knock_room_state.
• prev_content (though this one was specced until MSC3442)
• user_id (duplicates sender)

These top-level properties are not a part of the spec.

(Edited by @richvdh to make the list of affected properties clear)

[joepie91]

Same happens for replaces_state (related: matrix-org/matrix-spec#274). Does the Synapse team prefer separate issues for every incorrect field, or is it okay to group them together by "type of error" like this?

[clokep]

@joepie91 I think grouping them together is OK, as long as it is clear what properties are incorrect on what endpoints.

Hopefully most of these are easy to fix, as long as clients aren't depending on them.

[richvdh]

as long as clients aren't depending on them.

well, therein lies the rub.

[joepie91]

Another case: redacted_because.

[richvdh]

The top-level property is not a part of the spec.

I don't think the unsigned.age property is part of the spec either, is it? It's listed in some of the examples, but never actually defined? (see: matrix-org/matrix-spec-proposals#684)

See also: #8429

[joepie91]

I don't think the unsigned.age property is part of the spec either, is it?

Yeah, I think you're correct. I'm not sure why I phrased it like that.

[richvdh]

as long as clients aren't depending on them.

well, therein lies the rub.

Thinking about this a bit harder: any client that wants to use these properties, and be compatible with /sync, must be using the values from unsigned rather than the top-level properties... so I'd be reasonably confident removing them from the top level.

[richvdh]

For anyone thinking of looking at this: the code that does the copying is at https://github.com/matrix-org/synapse/blob/v1.43.0/synapse/events/utils.py#L255-L265. Note that that code also includes prev_content which is a slightly more complex case (see matrix-org/matrix-spec-proposals#877). I'd also be inclined to get rid of the unspecced user_id property.