fix(room): make the room previews more reliable

bindings/matrix-sdk-ffi/src/room_preview.rs

@@ -92,7 +92,7 @@ pub struct RoomPreviewInfo {
     /// The room type (space, custom) or nothing, if it's a regular room.
     pub room_type: RoomType,
     /// Is the history world-readable for this room?
-    pub is_history_world_readable: bool,
+    pub is_history_world_readable: Option<bool>,
     /// The membership state for the current user, if known.
     pub membership: Option<Membership>,
     /// The join rule for this room (private, public, knock, etc.).

crates/matrix-sdk-base/src/client.rs

@@ -1459,10 +1459,14 @@ impl BaseClient {
     pub async fn share_room_key(&self, room_id: &RoomId) -> Result<Vec<Arc<ToDeviceRequest>>> {
         match self.olm_machine().await.as_ref() {
             Some(o) => {
-                let (history_visibility, settings) = self
-                    .get_room(room_id)
-                    .map(|r| (r.history_visibility(), r.encryption_settings()))
-                    .unwrap_or((HistoryVisibility::Joined, None));
+                let Some(room) = self.get_room(room_id) else {
+                    return Err(Error::InsufficientData);
+                };
+
+                let history_visibility = room.history_visibility_or_default();
+                let Some(room_encryption_event) = room.encryption_settings() else {
+                    return Err(Error::EncryptionNotEnabled);
+                };
 
                 // Don't share the group session with members that are invited
                 // if the history visibility is set to `Joined`
@@ -1474,9 +1478,8 @@ impl BaseClient {
 
                 let members = self.store.get_user_ids(room_id, filter).await?;
 
-                let settings = settings.ok_or(Error::EncryptionNotEnabled)?;
                 let settings = EncryptionSettings::new(
-                    settings,
+                    room_encryption_event,
                     history_visibility,
                     self.room_key_recipient_strategy.clone(),
                 );

crates/matrix-sdk-base/src/rooms/normal.rs

@@ -487,8 +487,14 @@ impl Room {
     }
 
     /// Get the history visibility policy of this room.
-    pub fn history_visibility(&self) -> HistoryVisibility {
-        self.inner.read().history_visibility().clone()
+    pub fn history_visibility(&self) -> Option<HistoryVisibility> {
+        self.inner.read().history_visibility().cloned()
+    }
+
+    /// Get the history visibility policy of this room, or a sensible default if
+    /// the event is missing.
+    pub fn history_visibility_or_default(&self) -> HistoryVisibility {
+        self.inner.read().history_visibility_or_default().clone()
     }
 
     /// Is the room considered to be public.
@@ -1522,11 +1528,24 @@ impl RoomInfo {
 
     /// Returns the history visibility for this room.
     ///
-    /// Defaults to `WorldReadable`, if missing.
-    pub fn history_visibility(&self) -> &HistoryVisibility {
+    /// Returns None if the event was never seen during sync.
+    pub fn history_visibility(&self) -> Option<&HistoryVisibility> {
+        match &self.base_info.history_visibility {
+            Some(MinimalStateEvent::Original(ev)) => Some(&ev.content.history_visibility),
+            _ => None,
+        }
+    }
+
+    /// Returns the history visibility for this room, or a sensible default.
+    ///
+    /// Returns `Shared`, the default specified by the [spec], when the event is
+    /// missing.
+    ///
+    /// [spec]: https://spec.matrix.org/latest/client-server-api/#server-behaviour-7
+    pub fn history_visibility_or_default(&self) -> &HistoryVisibility {
         match &self.base_info.history_visibility {
             Some(MinimalStateEvent::Original(ev)) => &ev.content.history_visibility,
-            _ => &HistoryVisibility::WorldReadable,
+            _ => &HistoryVisibility::Shared,
         }
     }
 

crates/matrix-sdk-ui/src/room_list_service/mod.rs

@@ -89,12 +89,15 @@ const DEFAULT_REQUIRED_STATE: &[(StateEventType, &str)] = &[
     (StateEventType::RoomPowerLevels, ""),
     (StateEventType::CallMember, "*"),
     (StateEventType::RoomJoinRules, ""),
+    // Those two events are required to properly compute room previews.
+    (StateEventType::RoomCreate, ""),
+    (StateEventType::RoomHistoryVisibility, ""),
 ];
 
 /// The default `required_state` constant value for sliding sync room
 /// subscriptions that must be added to `DEFAULT_REQUIRED_STATE`.
 const DEFAULT_ROOM_SUBSCRIPTION_EXTRA_REQUIRED_STATE: &[(StateEventType, &str)] =
-    &[(StateEventType::RoomCreate, ""), (StateEventType::RoomPinnedEvents, "")];
+    &[(StateEventType::RoomPinnedEvents, "")];
 
 /// The default `timeline_limit` value when used with room subscriptions.
 const DEFAULT_ROOM_SUBSCRIPTION_TIMELINE_LIMIT: u32 = 20;

crates/matrix-sdk-ui/tests/integration/room_list_service.rs

@@ -358,6 +358,8 @@ async fn test_sync_all_states() -> Result<(), Error> {
                         ["m.room.power_levels", ""],
                         ["org.matrix.msc3401.call.member", "*"],
                         ["m.room.join_rules", ""],
+                        ["m.room.create", ""],
+                        ["m.room.history_visibility", ""],
                     ],
                     "include_heroes": true,
                     "filters": {
@@ -2224,6 +2226,7 @@ async fn test_room_subscription() -> Result<(), Error> {
                         ["org.matrix.msc3401.call.member", "*"],
                         ["m.room.join_rules", ""],
                         ["m.room.create", ""],
+                        ["m.room.history_visibility", ""],
                         ["m.room.pinned_events", ""],
                     ],
                     "timeline_limit": 20,
@@ -2263,6 +2266,7 @@ async fn test_room_subscription() -> Result<(), Error> {
                         ["org.matrix.msc3401.call.member", "*"],
                         ["m.room.join_rules", ""],
                         ["m.room.create", ""],
+                        ["m.room.history_visibility", ""],
                         ["m.room.pinned_events", ""],
                     ],
                     "timeline_limit": 20,

crates/matrix-sdk/src/room/mod.rs

@@ -613,7 +613,7 @@ impl Room {
     fn are_events_visible(&self) -> bool {
         if let RoomState::Invited = self.inner.state() {
             return matches!(
-                self.inner.history_visibility(),
+                self.inner.history_visibility_or_default(),
                 HistoryVisibility::WorldReadable | HistoryVisibility::Invited
             );
         }

crates/matrix-sdk/src/room_preview.rs

@@ -68,7 +68,7 @@ pub struct RoomPreview {
 
     /// Is the room world-readable (i.e. is its history_visibility set to
     /// world_readable)?
-    pub is_world_readable: bool,
+    pub is_world_readable: Option<bool>,
 
     /// Has the current user been invited/joined/left this room?
     ///
@@ -115,7 +115,9 @@ impl RoomPreview {
                     SpaceRoomJoinRule::Private
                 }
             },
-            is_world_readable: *room_info.history_visibility() == HistoryVisibility::WorldReadable,
+            is_world_readable: room_info
+                .history_visibility()
+                .map(|vis| *vis == HistoryVisibility::WorldReadable),
             num_joined_members,
             num_active_members,
             state,
@@ -266,7 +268,7 @@ impl RoomPreview {
             num_active_members,
             room_type: response.room_type,
             join_rule: response.join_rule,
-            is_world_readable: response.world_readable,
+            is_world_readable: Some(response.world_readable),
             state,
             is_direct,
             heroes: cached_room.map(|r| r.heroes()),
@@ -361,7 +363,7 @@ async fn search_for_room_preview_in_room_directory(
                     panic!("Unexpected PublicRoomJoinRule {:?}", room_description.join_rule)
                 }
             },
-            is_world_readable: room_description.is_world_readable,
+            is_world_readable: Some(room_description.is_world_readable),
             state: None,
             is_direct: None,
             heroes: None,

testing/matrix-sdk-integration-testing/src/tests/e2ee.rs

@@ -672,11 +672,14 @@ async fn test_failed_members_response() -> Result<()> {
 
     bob.sync_once().await?;
 
-    // Cause a failure of a sync_members request by asking for members before
-    // joining. Since this is a private DM room, it will fail with a 401, as
-    // we're not authorized to look at state history.
+    // Although we haven't joined the room yet, logic in `sync_members` looks at the
+    // room's visibility first; since it may be unknown for this room, from the
+    // point of view of Bob, it'll be assumed to be the default, aka shared. As
+    // a result, `sync_members()` doesn't even spawn a network request, and
+    // silently ignores the request.
+
     let result = bob.get_room(alice_room.room_id()).unwrap().sync_members().await;
-    assert!(result.is_err());
+    assert!(result.is_ok());
 
     bob.get_room(alice_room.room_id()).unwrap().join().await?;
 

testing/matrix-sdk-integration-testing/src/tests/sliding_sync/room.rs

@@ -1204,7 +1204,7 @@ fn assert_room_preview(preview: &RoomPreview, room_alias: &str) {
     assert_eq!(preview.num_joined_members, 1);
     assert!(preview.room_type.is_none());
     assert_eq!(preview.join_rule, SpaceRoomJoinRule::Invite);
-    assert!(preview.is_world_readable);
+    assert!(preview.is_world_readable.unwrap());
 }
 
 async fn get_room_preview_with_room_state(