matrix-sdk: High-level room api

[jsparber]

This adds four new structs: room::{Common, Joined, Left, Invited}

• Common: Contains methods that are shared between the different Room states and is the base for other room types. This struct contains a clone of matrix_sdk::Client and derefs to the matrix_sdk_base::Room so that the user can access the underlying data directly.
• Joined, Left, Invited: Contains methods that are available for each room state respectively rooms, they all deref to room::Common

Fixes: #165, #144

[jsparber]

@poljar i think this is ready for a review. I will still work on making the methods return more useful data instead of the raw matrix response.

[poljar]

Looks mostly good, some methods need to be shuffled around to different structs.

[jsparber]

@poljar i think i fixed all issues.

[poljar]

Please add the examples back and I think we can merge this.

[poljar]

Is the plan to leave room::Common public and let users construct the higher level rooms manually? I think I liked the enum way more, not that we need to change it in this PR, just asking for the future.

[jsparber]

Joined, Invited and Leftderef toCommonand therefore they obtain automatically all methods fromCommon`, which is a nice feature to have and therefore it need to stay public.
I like your idea of using rusts type safety to enforce the state of a room, but the state isn't the type of a room but just a state which can change add any time with or without the sdk's knowledge. I think we can and should do it for methods (a.k.a, server requests), but not for the underlying data since the data stays valid but possible server requests change based on the state.

[jsparber]

i would implement try_from to allow the user to move between Common and the other types.

[poljar]

That's the same API as having the new() constructors for Joined, Invited, and Left. Users need to provide their own type safety by constructing the higher level types themselves. What's the advantage of this vs having get_room() return an enum over the different room types? Note that the enum should dereference into Common as well.

As far as I can tell using the try_from method people need to construct their own room types, while using an enum they need to deconstruct the enum. For me it's more intuitive that the type safety is provided by the lib vs me having to wrap types manually to provide additional type safety.

I think we can and should do it for methods (a.k.a, server requests), but not for the underlying data since the data stays valid but possible server requests change based on the state.

The underlying data is already presented in a common way, no? We got this when we removed the different room types from the base crate.

[jsparber]

That's the same API as having the new() constructors for Joined, Invited, and Left. Users need to provide their own type safety by constructing the higher level types themselves. What's the advantage of this vs having get_room() return an enum over the different room types? Note that the enum should dereference into Common as well.

I'm fine with that as long it derefs to Common. What addition does the enum bring?

As far as I can tell using the try_from method people need to construct their own room types, while using an enum they need to deconstruct the enum. For me it's more intuitive that the type safety is provided by the lib vs me having to wrap types manually to provide additional type safety.

I agree, but the lib can only create the illusion of type safety which i think is worse then no type safety at all. Imo many people use multiple clients for matrix, so they can change the type of a room on different clients, hence the developer still needs to handles wrong room types.

The underlying data is already presented in a common way, no? We got this when we removed the different room types from the base crate.

If the enum derefs to Common then yes.

[poljar]

I'm fine with that as long it derefs to Common. What addition does the enum bring?

I mentioned that, a more intuitive interface, at least it seems that way to me. Let's check the simple case of sending a message. People will either search for:

• How to get a room object out of the client
• The send_message() method.

Ok great, if they get a room object they get a Common room, now they'll search for the send_message() method and discover that they have the wrong room type, now they are confused. I think the same thing applies if they search for the send() method first.

If we instead provide an enum that destructures into the more specific room types they need to discover the get_room() method and discover that the type that can send a message is already there, they just need to destructure it.

I find it very unintuitive to get a room type, which I then need to manually wrap with another type to send messages.

If the enum derefs to Common then yes.

I don't see a reason why we wouldn't want this, considering that the whole point of the different types from the base layer was so we can access local data using a common type.

[jsparber]

I think this is a really specific use cases. In this use case i think we should point the user to use get_joined_room() which does return the correct type and then they can immediately send a message.

I don't have any real issue with the enum. I think I failed to explain the point why i think that we need to allow users to move between room types and allow them to construct there own Joined, Invited and Left from a Common.
Consider the case where the user holds an room::Joined and then the state changes from joined to left via a sync. So now the room::Joined isn't valid anymore and the user needs to change the struct they are holding. If the user isn't allowed to construct a room::Left from a room::Joined, the only way they can obtain the new and correct struct is by going through the store via client.get_left_room(room_id), which requires the user to hold the client struct just to use it in case they need to updated the room struct they are holding. Additionally, IMO the user should never have to use the room_id once they obtained the a room.

[poljar]

I think this is a really specific use cases. In this use case i think we should point the user to use get_joined_room() which does return the correct type and then they can immediately send a message.

I don't have any real issue with the enum. I think I failed to explain the point why i think that we need to allow users to move between room types and allow them to construct there own Joined, Invited and Left from a Common.

In that case lets go for the enum for now at least.

Consider the case where the user holds an room::Joined and then the state changes from joined to left via a sync. So now the room::Joined isn't valid anymore and the user needs to change the struct they are holding. If the user isn't allowed to construct a room::Left from a room::Joined, the only way they can obtain the new and correct struct is by going through the store via client.get_left_room(room_id), which requires the user to hold the client struct just to use it in case they need to updated the room struct they are holding. Additionally, IMO the user should never have to use the room_id once they obtained the a room.

How would you notify people that they need to change their room types around if not by room_id? I'm not sure it's a big deal to require people to hold on to a Client object, it's clone-able for a reason and if it does turn out to be annoying we can let people get a Client object out of a Joined, the higher level rooms all hold on to a Client object.

I feel like it's worse that you have to manually check the type of the room and then construct a higher level object with the correct type then if they have to use the room_id.

We could even provide a method that goes from Joined -> Enum and corrects the type and then you destructure again into your Left or whatever. That way there's no need to check the room id, no need to have a client object, no need check the room type except you need to destructure it.

[jsparber]

We could even provide a method that goes from Joined -> Enum and corrects the type and then you destructure again into your Left or whatever. That way there's no need to check the room id, no need to have a client object, no need check the room type except you need to destructure it.

I like it. 👍

[poljar]

Great, let's go with that route then.

[poljar]

One example is still missing. Add it back and we can merge.