Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Include event_id when /state/:type[/:key]?format=event #15454

Open
MadLittleMods opened this issue Apr 17, 2023 · 3 comments
Open

Include event_id when /state/:type[/:key]?format=event #15454

MadLittleMods opened this issue Apr 17, 2023 · 3 comments
Labels
O-Occasional Affects or can be seen by some users regularly or most users rarely S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.

Comments

@MadLittleMods
Copy link
Contributor

MadLittleMods commented Apr 17, 2023

Problem

Currently when you GET /_matrix/client/v3/rooms/{room_id}/state/m.room.create?format=event (notice the event format), it doesn't include event_id which is kinda annoying to work with and reference.

Example:

{
    "content": {
        "creator": "@foo:bar",
        "room_version": "6"
    },
    "origin_server_ts": 1606978812896,
    "room_id": "!abc",
    "sender": "@foo:bar",
    "state_key": "",
    "type": "m.room.create",
    "unsigned": {
        "age_ts": 1606978812896
    }
}

Proposal

Add event_id to the output. There is no spec to go off of but seems like a non-breaking change we can do.


Spawning from a need in matrix-org/matrix-viewer#167 to get the event_id of the m.room.create event in the room.

Is there a better way to do this? Perhaps /_matrix/client/v3/rooms/{room_id}/messages?dir=f&limit=1

Dev notes

Relevant code:

if format == "event":
event = format_event_for_client_v2(data.get_dict())
return 200, event
elif format == "content":
return 200, data.get_dict()["content"]

Currently, we use format_event_for_client_v2(data.get_dict()) against the EventBase PDU JSON dictionary which doesn't include event_id. We could instead use serialize_event(data, self.clock.time_msec()) which would get us something more workable.

We could also alternatively, add event_id to the dictionary returned by EventBase.get_dict() but that probably has too many downstream side-effects like messing with the PDU JSON serialization stuff.

def get_dict(self) -> JsonDict:
d = dict(self._dict)
d.update({"signatures": self.signatures, "unsigned": dict(self.unsigned)})
return d


Separately, it would be good to standardize with the ?event_format=client|federation parameter used with other endpoints

@MadLittleMods MadLittleMods changed the title Include event_id when ?format=event Include event_id when /state/:type[/:key]?format=event Apr 17, 2023
@H-Shay H-Shay added T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements. O-Occasional Affects or can be seen by some users regularly or most users rarely S-Tolerable Minor significance, cosmetic issues, low or no impact to users. labels Apr 19, 2023
@H-Shay
Copy link
Contributor

H-Shay commented Apr 20, 2023

We discussed this in the backend meeting and the consensus was that it should be okay to go ahead and do this, with the caveat that the spec issue should be updated when it's done.

@DMRobertson
Copy link
Contributor

(The rationale for it being this way is that the event ID is not part of the event (since room v2); it's a form of derived data.)

@MadLittleMods
Copy link
Contributor Author

MadLittleMods commented Apr 21, 2023

Makes sense for how it could have ended up this way 👍

For the /state client API, I think it makes less sense since it's going to be hard and cumbersome to use Canonical JSON to calculate the event ID yourself on all platforms. Looks like there is a spec'ed ClientEvent format .

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
O-Occasional Affects or can be seen by some users regularly or most users rarely S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.
Projects
None yet
Development

No branches or pull requests

3 participants