List rooms improvements

[kilimnik]

I noticed that the fields, join_rules, guest_access and history_visibility can be null. So I wrapped them in an option.
The join_rules deserialization did not work with the old type.

I also added the missing room_type field.

[zecakeh]

How did you know that these fields can be null? Did you encounter such rooms in practice?

It is not clear which rooms are returned. All the rooms that the server know about? Even the ones where the server only has the stripped state because a user was invited to it? If that is the case there are more fields that should be optional.

There is not much documentation about it, but looking at the code they seem to be loaded directly from the database, and in the database schemas a lot of those fields can be null.

[kilimnik]

How did you know that these fields can be null? Did you encounter such rooms in practice?

Yes, I encountered multiple rooms on my homeserver having this fields as null.

It is not clear which rooms are returned. All the rooms that the server know about? Even the ones where the server only has the stripped state because a user was invited to it? If that is the case there are more fields that should be optional.

I'm not sure about which rooms are returned, I woud've guessed it's only rooms on the homeserver.

There is not much documentation about it, but looking at the code they seem to be loaded directly from the database, and in the database schemas a lot of those fields can be null.

I haven't looked at the underlying code, but if that's the case maybe we should put everything except for the room id as an option.

[jplatte]

@kilimnik you have to put an empty line between a quote and the next non-quite line, your comment does not render the intended way at least on GH mobile.

[zecakeh]

If I join the SQL request, with the PostgreSQL schemas and the JSON constructor.

If get this:

"room_id": text NOT NULL,
"name": text,
"canonical_alias": text,
"joined_members": integer NOT NULL,
"joined_local_members": integer NOT NULL,
"version": text,
"creator": text,
"encryption": text,
"federatable": boolean, // coerced to a boolean so NOT NULL
"public": boolean, // coerced to a boolean so NOT NULL
"join_rules": text,
"guest_access": text,
"history_visibility": text,
"state_events": integer NOT NULL,
"room_type": text,

So it looks like every field with a string value can be null, except the room_id. So only version needs to be changed now.

[zecakeh]

I have found these type definitions for the room details endpoint that corroborate my previous comment, given that it is constructed from the same SQL tables: https://github.com/element-hq/synapse/blob/24c4d82aeb1bd5ac15cc0614e243595404fb2009/synapse/storage/databases/main/room.py#L81-L102

[kilimnik]

I added the version field to be nullable as well, so now it should fully match the schema

[zecakeh]

Thanks, could you add an entry to the changelog? It might be a good thing to start using the same sections as in the main ruma crates ("Breaking changes", "Bug fixes", "Improvements") to make the impact of changes clearer.

[zecakeh]

Just a few nits in the changelog

[zecakeh]

Thanks!