Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RoomInfo loses content, m.room.create not stored in raw format #247

Closed
johannescpk opened this issue May 25, 2021 · 4 comments
Closed

RoomInfo loses content, m.room.create not stored in raw format #247

johannescpk opened this issue May 25, 2021 · 4 comments

Comments

@johannescpk
Copy link
Contributor

johannescpk commented May 25, 2021

Currently the m.room.create event isn't stored in its raw format. Given the content of that event might change over time it should be stored in its raw format.

Currently that's a bug affecting space rooms where the content type is lost

Related hotfix commit from @MTRNord: MTRNord@85c6d76

@johannescpk johannescpk changed the title RoomInfo loses content, m.room.create not stored in raw format RoomInfo loses content, m.room.create not stored in raw format May 25, 2021
@johannescpk johannescpk mentioned this issue May 25, 2021
2 tasks
@poljar
Copy link
Contributor

poljar commented May 25, 2021

Currently the m.room.create event isn't stored in its raw format. Given the content of that event might change over time it should be stored in its raw format.

Is this true? As far as I can tell we're storing everything in Raw format nowadays, relevant code is over here: https://github.com/matrix-org/matrix-rust-sdk/blob/master/matrix_sdk_base/src/client.rs#L476.

This mentions RoomInfo but the point of this struct is to only store certain important fields of important state events, important here means what is important to display a room.

If new fields should be added to the RoomInfo the relevant event can be fetched from the state store, where we do have the full Raw event, and a migration can be performed that way. Not that I'm suggesting to do a migration currently, considering that the state store format isn't yet stable.

@MTRNord
Copy link
Contributor

MTRNord commented May 25, 2021

Hm I only saw state events being stored in the store. Is that including the m.room.create event? And if so what would be the best way to upstream spaces/add them to RoomInfo (where I think they make the most sense) while not needing a store reset?

@poljar
Copy link
Contributor

poljar commented May 25, 2021

Well m.room.create is a state event like any other, so I would say we store all state events the same way. We do store additionally state stuff in the RoomInfo which is kept minimal so it can stay in memory. That way you can render a room list in your client without the need to access storage.

Here's some proof that m.room.create is actually stored:

cargo run --example state_inspector --features="sled_state_store" ~/.weechat/matrix-rust/localhost/ get-state '!GttWlUVQWSLJlXiTMc:morpheus.localhost' 'm.room.create'
    Finished dev [unoptimized + debuginfo] target(s) in 0.07s
    Running `/home/poljar/werk/matrix/nio-rust/target/debug/examples/state_inspector /home/poljar/.weechat/matrix-rust/localhost/ get-state ''\!'GttWlUVQWSLJlXiTMc:morpheus.localhost' m.room.create`
Some(
    Raw::<ruma_events::enums::AnySyncStateEvent> {
        json: RawValue({
            "type":"m.room.create",
            "sender":"@example:morpheus.localhost",
            "content":{
                "room_version":"5",
                "creator":"@example:morpheus.localhost"
            },
            "state_key":"",
            "origin_server_ts":1591626808369,
            "unsigned":{
                "age":29959107097
            },
            "event_id":"$Rywa1ExYalO5m-jwjplTc4emJOhxcifAdD2nqowW2ow"
        }),
    },
)

As mentioned, migrations for this need to load the m.room.create state event and modify the RoomInfo, though I suggest you to just break the store and be done with it.

@johannescpk
Copy link
Contributor Author

Thanks for the clarification, should've taken a closer look, I guess this can be closed then since adding new fields to RoomInfo is a different story

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants