refactor!(sdk): make the join_rule and related getters optional

The join rule state event can be missing from a room state. In this
case, it's an API footgun to return a default value; instead, we should
return none and let the caller decide what to do with missing
information.
This commit is contained in:
Benjamin Bouvier
2025-06-24 14:29:51 +02:00
parent 0722ed9d8f
commit bc9192f818
9 changed files with 55 additions and 37 deletions

View File

@@ -35,7 +35,6 @@ pub struct NotificationRoomInfo {
pub joined_members_count: u64,
pub is_encrypted: Option<bool>,
pub is_direct: bool,
pub is_public: bool,
}
#[derive(uniffi::Record)]
@@ -74,11 +73,10 @@ impl NotificationItem {
display_name: item.room_computed_display_name,
avatar_url: item.room_avatar_url,
canonical_alias: item.room_canonical_alias,
join_rule: item.room_join_rule.try_into().ok(),
join_rule: item.room_join_rule.map(TryInto::try_into).transpose().ok().flatten(),
joined_members_count: item.joined_members_count,
is_encrypted: item.is_room_encrypted,
is_direct: item.is_direct_message_room,
is_public: item.is_room_public,
},
is_noisy: item.is_noisy,
has_mention: item.has_mention,

View File

@@ -116,7 +116,10 @@ impl Room {
self.inner.is_direct().await.unwrap_or(false)
}
pub fn is_public(&self) -> bool {
/// Whether the room can be publicly joined or not, based on its join rule.
///
/// Can return `None` if the join rule state event is missing.
pub fn is_public(&self) -> Option<bool> {
self.inner.is_public()
}

View File

@@ -26,7 +26,11 @@ pub struct RoomInfo {
topic: Option<String>,
avatar_url: Option<String>,
is_direct: bool,
is_public: bool,
/// Whether the room is public or not, based on the join rules.
///
/// Can be `None` if the join rules state event is not available for this
/// room.
is_public: Option<bool>,
is_space: bool,
/// If present, it means the room has been archived/upgraded.
successor_room: Option<SuccessorRoom>,
@@ -77,10 +81,15 @@ impl RoomInfo {
let pinned_event_ids =
room.pinned_event_ids().unwrap_or_default().iter().map(|id| id.to_string()).collect();
let join_rule = room.join_rule().try_into();
if let Err(e) = &join_rule {
warn!("Failed to parse join rule: {e:?}");
}
let join_rule = room
.join_rule()
.map(TryInto::try_into)
.transpose()
.inspect_err(|err| {
warn!("Failed to parse join rule: {err}");
})
.ok()
.flatten();
let power_levels = RoomPowerLevels::new(
room.power_levels().await.map_err(matrix_sdk::Error::from)?,
@@ -135,7 +144,7 @@ impl RoomInfo {
num_unread_notifications: room.num_unread_notifications(),
num_unread_mentions: room.num_unread_mentions(),
pinned_event_ids,
join_rule: join_rule.ok(),
join_rule,
history_visibility: room.history_visibility_or_default().try_into()?,
power_levels: Arc::new(power_levels),
})

View File

@@ -37,8 +37,9 @@ impl RoomPreview {
membership: info.state.map(|state| state.into()),
join_rule: info
.join_rule
.clone()
.try_into()
.as_ref()
.map(TryInto::try_into)
.transpose()
.map_err(|_| anyhow::anyhow!("unhandled SpaceRoomJoinRule kind"))?,
is_direct: info.is_direct,
heroes: info
@@ -114,17 +115,17 @@ pub struct RoomPreviewInfo {
/// The membership state for the current user, if known.
pub membership: Option<Membership>,
/// The join rule for this room (private, public, knock, etc.).
pub join_rule: JoinRule,
pub join_rule: Option<JoinRule>,
/// Whether the room is direct or not, if known.
pub is_direct: Option<bool>,
/// Room heroes.
pub heroes: Option<Vec<RoomHero>>,
}
impl TryFrom<SpaceRoomJoinRule> for JoinRule {
impl TryFrom<&SpaceRoomJoinRule> for JoinRule {
type Error = ();
fn try_from(join_rule: SpaceRoomJoinRule) -> Result<Self, ()> {
fn try_from(join_rule: &SpaceRoomJoinRule) -> Result<Self, ()> {
Ok(match join_rule {
SpaceRoomJoinRule::Invite => JoinRule::Invite,
SpaceRoomJoinRule::Knock => JoinRule::Knock,

View File

@@ -340,13 +340,15 @@ impl Room {
}
/// Is the room considered to be public.
pub fn is_public(&self) -> bool {
matches!(self.join_rule(), JoinRule::Public)
///
/// May return `None` if the join rule event is not available.
pub fn is_public(&self) -> Option<bool> {
self.inner.read().join_rule().map(|join_rule| matches!(join_rule, JoinRule::Public))
}
/// Get the join rule policy of this room.
pub fn join_rule(&self) -> JoinRule {
self.inner.read().join_rule().clone()
/// Get the join rule policy of this room, if available.
pub fn join_rule(&self) -> Option<JoinRule> {
self.inner.read().join_rule().cloned()
}
/// Get the maximum power level that this room contains.

View File

@@ -867,13 +867,12 @@ impl RoomInfo {
}
}
/// Returns the join rule for this room.
///
/// Defaults to `Public`, if missing.
pub fn join_rule(&self) -> &JoinRule {
/// Return the join rule for this room, if the `m.room.join_rules` event is
/// available.
pub fn join_rule(&self) -> Option<&JoinRule> {
match &self.base_info.join_rules {
Some(MinimalStateEvent::Original(ev)) => &ev.content.join_rule,
_ => &JoinRule::Public,
Some(MinimalStateEvent::Original(ev)) => Some(&ev.content.join_rule),
_ => None,
}
}

View File

@@ -910,11 +910,11 @@ pub struct NotificationItem {
/// Room canonical alias.
pub room_canonical_alias: Option<String>,
/// Room join rule.
pub room_join_rule: JoinRule,
///
/// Set to `None` if the join rule for this room is not available.
pub room_join_rule: Option<JoinRule>,
/// Is this room encrypted?
pub is_room_encrypted: Option<bool>,
/// Is this a public room?
pub is_room_public: bool,
/// Is this room considered a direct message?
pub is_direct_message_room: bool,
/// Numbers of members who joined the room.
@@ -1010,7 +1010,6 @@ impl NotificationItem {
room_canonical_alias: room.canonical_alias().map(|c| c.to_string()),
room_join_rule: room.join_rule(),
is_direct_message_room: room.is_direct().await?,
is_room_public: room.is_public(),
is_room_encrypted: room
.latest_encryption_state()
.await
@@ -1024,6 +1023,13 @@ impl NotificationItem {
Ok(item)
}
/// Returns whether this room is public or not, based on the join rule.
///
/// Maybe return `None` if the join rule is not available.
pub fn is_public(&self) -> Option<bool> {
self.room_join_rule.as_ref().map(|rule| matches!(rule, JoinRule::Public))
}
}
/// An error for the [`NotificationClient`].

View File

@@ -64,7 +64,7 @@ pub struct RoomPreview {
pub room_type: Option<RoomType>,
/// What's the join rule for this room?
pub join_rule: SpaceRoomJoinRule,
pub join_rule: Option<SpaceRoomJoinRule>,
/// Is the room world-readable (i.e. is its history_visibility set to
/// world_readable)?
@@ -102,7 +102,7 @@ impl RoomPreview {
topic: room_info.topic().map(ToOwned::to_owned),
avatar_url: room_info.avatar_url().map(ToOwned::to_owned),
room_type: room_info.room_type().cloned(),
join_rule: match room_info.join_rule() {
join_rule: room_info.join_rule().map(|rule| match rule {
JoinRule::Invite => SpaceRoomJoinRule::Invite,
JoinRule::Knock => SpaceRoomJoinRule::Knock,
JoinRule::Private => SpaceRoomJoinRule::Private,
@@ -114,7 +114,7 @@ impl RoomPreview {
// private (a cautious choice).
SpaceRoomJoinRule::Private
}
},
}),
is_world_readable: room_info
.history_visibility()
.map(|vis| *vis == HistoryVisibility::WorldReadable),
@@ -287,7 +287,7 @@ impl RoomPreview {
num_joined_members: response.num_joined_members.into(),
num_active_members,
room_type: response.room_type,
join_rule: response.join_rule,
join_rule: Some(response.join_rule),
is_world_readable: Some(response.world_readable),
state,
is_direct,
@@ -375,14 +375,14 @@ async fn search_for_room_preview_in_room_directory(
num_active_members: None,
// Assume it's a room
room_type: None,
join_rule: match room_description.join_rule {
join_rule: Some(match room_description.join_rule {
PublicRoomJoinRule::Public => SpaceRoomJoinRule::Public,
PublicRoomJoinRule::Knock => SpaceRoomJoinRule::Knock,
PublicRoomJoinRule::_Custom(rule) => SpaceRoomJoinRule::_Custom(rule),
_ => {
panic!("Unexpected PublicRoomJoinRule {:?}", room_description.join_rule)
}
},
}),
is_world_readable: Some(room_description.is_world_readable),
state: None,
is_direct: None,

View File

@@ -1228,7 +1228,7 @@ fn assert_room_preview(preview: &RoomPreview, room_alias: &str) {
assert_eq!(preview.avatar_url.as_ref().unwrap(), mxc_uri!("mxc://localhost/alice"));
assert_eq!(preview.num_joined_members, 1);
assert!(preview.room_type.is_none());
assert_eq!(preview.join_rule, SpaceRoomJoinRule::Invite);
assert_eq!(preview.join_rule, Some(SpaceRoomJoinRule::Invite));
assert!(preview.is_world_readable.unwrap());
}