refactor(room_preview): make RoomPreview use the local known data only for joined rooms

When instantiating a room preview, previously it would try to just check if the room exists locally either as joined, invited, knocked, left, etc., and then retrieve the info we cached about it.

While this seems fine for most cases, it turns out for non-joined rooms, the info we have locally will **always** be the one we received when the invite/knock/leave action took place and it'll never be updated,
so we may have the case where we knock into a room, never receive a response, someone changes the join rule of the room to something else and we'll think about this room as a 'request to join' room until we clear the local cache.

To prevent that, we can only use the local data for joined rooms, which are constantly updated, and try to use the room summary API and other fallbacks for the rest, even if they're rooms known to us.
This commit is contained in:
Jorge Martín
2024-11-21 17:01:50 +01:00
committed by Jorge Martin Espinosa
parent 48fbda844f
commit 6b0987385e
3 changed files with 36 additions and 10 deletions

View File

@@ -1142,8 +1142,8 @@ impl Client {
self.base_client().get_room(room_id).map(|room| Room::new(self.clone(), room))
}
/// Gets the preview of a room, whether the current user knows it (because
/// they've joined/left/been invited to it) or not.
/// Gets the preview of a room, whether the current user has joined it or
/// not.
pub async fn get_room_preview(
&self,
room_or_alias_id: &RoomOrAliasId,
@@ -1155,10 +1155,16 @@ impl Client {
};
if let Some(room) = self.get_room(&room_id) {
return Ok(RoomPreview::from_known(&room).await);
// The cached data can only be trusted if the room is joined: for invite and
// knock rooms, no updates will be received for the rooms after the invite/knock
// action took place so we may have very out to date data for important fields
// such as `join_rule`
if room.state() == RoomState::Joined {
return Ok(RoomPreview::from_joined(&room).await);
}
}
RoomPreview::from_unknown(self, room_id, room_or_alias_id, via).await
RoomPreview::from_not_joined(self, room_id, room_or_alias_id, via).await
}
/// Resolve a room alias to a room id and a list of servers which know

View File

@@ -124,9 +124,8 @@ impl RoomPreview {
}
}
/// Create a room preview from a known room (i.e. one we've been invited to,
/// we've joined or we've left).
pub(crate) async fn from_known(room: &Room) -> Self {
/// Create a room preview from a known room we've joined.
pub(crate) async fn from_joined(room: &Room) -> Self {
let is_direct = room.is_direct().await.ok();
let display_name = room.compute_display_name().await.ok().map(|name| name.to_string());
@@ -142,7 +141,7 @@ impl RoomPreview {
}
#[instrument(skip(client))]
pub(crate) async fn from_unknown(
pub(crate) async fn from_not_joined(
client: &Client,
room_id: OwnedRoomId,
room_or_alias_id: &RoomOrAliasId,

View File

@@ -9,7 +9,9 @@ use matrix_sdk_test::{
};
#[cfg(feature = "experimental-sliding-sync")]
use ruma::{api::client::sync::sync_events::v5::response::Hero, assign};
use ruma::{owned_user_id, room_id, space::SpaceRoomJoinRule, RoomId};
use ruma::{
events::room::member::MembershipState, owned_user_id, room_id, space::SpaceRoomJoinRule, RoomId,
};
use serde_json::json;
use wiremock::{
matchers::{header, method, path_regex},
@@ -30,6 +32,14 @@ async fn test_room_preview_leave_invited() {
client.sync_once(SyncSettings::default()).await.unwrap();
server.reset().await;
mock_unknown_summary(
room_id,
None,
SpaceRoomJoinRule::Knock,
Some(MembershipState::Invite),
&server,
)
.await;
mock_leave(room_id, &server).await;
let room_preview = client.get_room_preview(room_id.into(), Vec::new()).await.unwrap();
@@ -52,6 +62,14 @@ async fn test_room_preview_leave_knocked() {
client.sync_once(SyncSettings::default()).await.unwrap();
server.reset().await;
mock_unknown_summary(
room_id,
None,
SpaceRoomJoinRule::Knock,
Some(MembershipState::Knock),
&server,
)
.await;
mock_leave(room_id, &server).await;
let room_preview = client.get_room_preview(room_id.into(), Vec::new()).await.unwrap();
@@ -91,7 +109,7 @@ async fn test_room_preview_leave_unknown_room_fails() {
let (client, server) = logged_in_client_with_server().await;
let room_id = room_id!("!room:localhost");
mock_unknown_summary(room_id, None, SpaceRoomJoinRule::Knock, &server).await;
mock_unknown_summary(room_id, None, SpaceRoomJoinRule::Knock, None, &server).await;
let room_preview = client.get_room_preview(room_id.into(), Vec::new()).await.unwrap();
assert!(room_preview.state.is_none());
@@ -142,6 +160,7 @@ async fn mock_unknown_summary(
room_id: &RoomId,
alias: Option<String>,
join_rule: SpaceRoomJoinRule,
membership: Option<MembershipState>,
server: &MockServer,
) {
let body = if let Some(alias) = alias {
@@ -152,6 +171,7 @@ async fn mock_unknown_summary(
"num_joined_members": 1,
"world_readable": true,
"join_rule": join_rule,
"membership": membership,
})
} else {
json!({
@@ -160,6 +180,7 @@ async fn mock_unknown_summary(
"num_joined_members": 1,
"world_readable": true,
"join_rule": join_rule,
"membership": membership,
})
};
Mock::given(method("GET"))