Skip to content

Commit

Permalink
ffi: make the name method sync again
Browse files Browse the repository at this point in the history
Also:

- rename `display_name` to `computed_display_name` in several places,
and reflect that change into a few callers
- simplify slightly the `computed_display_name()` method
  • Loading branch information
bnjbvr committed May 1, 2024
1 parent a3061eb commit 90bed18
Show file tree
Hide file tree
Showing 11 changed files with 78 additions and 44 deletions.
7 changes: 5 additions & 2 deletions bindings/matrix-sdk-ffi/src/room.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,11 @@ impl Room {
Ok(Timeline::new(timeline))
}

pub async fn display_name(&self) -> Result<String, ClientError> {
Ok(self.inner.display_name().await?.to_string())
/// Returns the room's name from the state event if available, otherwise
/// compute a room name based on the room's nature (DM or not) and number of
/// members.
pub fn computed_display_name(&self) -> Result<String, ClientError> {
Ok(RUNTIME.block_on(self.inner.computed_display_name())?.to_string())
}

pub async fn is_encrypted(&self) -> Result<bool, ClientError> {
Expand Down
7 changes: 5 additions & 2 deletions bindings/matrix-sdk-ffi/src/room_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -490,8 +490,11 @@ impl RoomListItem {
self.inner.id().to_string()
}

async fn name(&self) -> Option<String> {
self.inner.name().await
/// Returns the room's name from the state event if available, otherwise
/// compute a room name based on the room's nature (DM or not) and number of
/// members.
fn computed_display_name(&self) -> Option<String> {
RUNTIME.block_on(self.inner.computed_display_name())
}

fn avatar_url(&self) -> Option<String> {
Expand Down
2 changes: 1 addition & 1 deletion crates/matrix-sdk-base/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1635,7 +1635,7 @@ mod tests {
let room = client.get_room(room_id).expect("Room not found");
assert_eq!(room.state(), RoomState::Invited);
assert_eq!(
room.display_name().await.expect("fetching display name failed"),
room.computed_display_name().await.expect("fetching display name failed"),
DisplayName::Calculated("Kyra".to_owned())
);
}
Expand Down
74 changes: 48 additions & 26 deletions crates/matrix-sdk-base/src/rooms/normal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ impl Room {
/// The display name is calculated according to [this algorithm][spec].
///
/// [spec]: <https://matrix.org/docs/spec/client_server/latest#calculating-the-display-name-for-a-room>
pub async fn display_name(&self) -> StoreResult<DisplayName> {
pub async fn computed_display_name(&self) -> StoreResult<DisplayName> {
self.calculate_name().await
}

Expand Down Expand Up @@ -602,25 +602,26 @@ impl Room {
inner.summary.clone()
};

let is_own_member = |m: &RoomMember| m.user_id() == &*self.own_user_id;
let is_own_user_id = |u: &str| u == self.own_user_id().as_str();
let own_user_id = self.own_user_id().as_str();

let members: Vec<RoomMember> = if summary.heroes.is_empty() {
self.members(RoomMemberships::ACTIVE)
.await?
.into_iter()
.filter(|u| !is_own_member(u))
.filter(|u| u.user_id() != own_user_id)
.take(5)
.collect()
} else {
let members: Vec<_> =
stream::iter(summary.heroes.iter().filter(|u| !is_own_user_id(u)))
.filter_map(|u| async move {
let user_id = UserId::parse(u.as_str()).ok()?;
self.get_member(&user_id).await.transpose()
})
.collect()
.await;
let members: Vec<_> = stream::iter(summary.heroes.iter())
.filter_map(|u| async move {
let user_id = UserId::parse(u.as_str()).ok()?;
if user_id == own_user_id {
return None;
}
self.get_member(&user_id).await.transpose()
})
.collect()
.await;

let members: StoreResult<Vec<_>> = members.into_iter().collect();

Expand Down Expand Up @@ -1720,32 +1721,41 @@ mod tests {
#[async_test]
async fn test_display_name_for_joined_room_is_empty_if_no_info() {
let (_, room) = make_room(RoomState::Joined);
assert_eq!(room.display_name().await.unwrap(), DisplayName::Empty);
assert_eq!(room.computed_display_name().await.unwrap(), DisplayName::Empty);
}

#[async_test]
async fn test_display_name_for_joined_room_uses_canonical_alias_if_available() {
let (_, room) = make_room(RoomState::Joined);
room.inner
.update(|info| info.base_info.canonical_alias = Some(make_canonical_alias_event()));
assert_eq!(room.display_name().await.unwrap(), DisplayName::Aliased("test".to_owned()));
assert_eq!(
room.computed_display_name().await.unwrap(),
DisplayName::Aliased("test".to_owned())
);
}

#[async_test]
async fn test_display_name_for_joined_room_prefers_name_over_alias() {
let (_, room) = make_room(RoomState::Joined);
room.inner
.update(|info| info.base_info.canonical_alias = Some(make_canonical_alias_event()));
assert_eq!(room.display_name().await.unwrap(), DisplayName::Aliased("test".to_owned()));
assert_eq!(
room.computed_display_name().await.unwrap(),
DisplayName::Aliased("test".to_owned())
);
room.inner.update(|info| info.base_info.name = Some(make_name_event()));
// Display name wasn't cached when we asked for it above, and name overrides
assert_eq!(room.display_name().await.unwrap(), DisplayName::Named("Test Room".to_owned()));
assert_eq!(
room.computed_display_name().await.unwrap(),
DisplayName::Named("Test Room".to_owned())
);
}

#[async_test]
async fn test_display_name_for_invited_room_is_empty_if_no_info() {
let (_, room) = make_room(RoomState::Invited);
assert_eq!(room.display_name().await.unwrap(), DisplayName::Empty);
assert_eq!(room.computed_display_name().await.unwrap(), DisplayName::Empty);
}

#[async_test]
Expand All @@ -1758,26 +1768,35 @@ mod tests {
});
room.inner.update(|info| info.base_info.name = Some(room_name));

assert_eq!(room.display_name().await.unwrap(), DisplayName::Empty);
assert_eq!(room.computed_display_name().await.unwrap(), DisplayName::Empty);
}

#[async_test]
async fn test_display_name_for_invited_room_uses_canonical_alias_if_available() {
let (_, room) = make_room(RoomState::Invited);
room.inner
.update(|info| info.base_info.canonical_alias = Some(make_canonical_alias_event()));
assert_eq!(room.display_name().await.unwrap(), DisplayName::Aliased("test".to_owned()));
assert_eq!(
room.computed_display_name().await.unwrap(),
DisplayName::Aliased("test".to_owned())
);
}

#[async_test]
async fn test_display_name_for_invited_room_prefers_name_over_alias() {
let (_, room) = make_room(RoomState::Invited);
room.inner
.update(|info| info.base_info.canonical_alias = Some(make_canonical_alias_event()));
assert_eq!(room.display_name().await.unwrap(), DisplayName::Aliased("test".to_owned()));
assert_eq!(
room.computed_display_name().await.unwrap(),
DisplayName::Aliased("test".to_owned())
);
room.inner.update(|info| info.base_info.name = Some(make_name_event()));
// Display name wasn't cached when we asked for it above, and name overrides
assert_eq!(room.display_name().await.unwrap(), DisplayName::Named("Test Room".to_owned()));
assert_eq!(
room.computed_display_name().await.unwrap(),
DisplayName::Named("Test Room".to_owned())
);
}

fn make_canonical_alias_event() -> MinimalStateEvent<RoomCanonicalAliasEventContent> {
Expand Down Expand Up @@ -1817,7 +1836,7 @@ mod tests {

room.inner.update_if(|info| info.update_summary(&summary));
assert_eq!(
room.display_name().await.unwrap(),
room.computed_display_name().await.unwrap(),
DisplayName::Calculated("Matthew".to_owned())
);
}
Expand All @@ -1839,7 +1858,7 @@ mod tests {
store.save_changes(&changes).await.unwrap();

assert_eq!(
room.display_name().await.unwrap(),
room.computed_display_name().await.unwrap(),
DisplayName::Calculated("Matthew".to_owned())
);
}
Expand Down Expand Up @@ -1869,7 +1888,7 @@ mod tests {

room.inner.update_if(|info| info.update_summary(&summary));
assert_eq!(
room.display_name().await.unwrap(),
room.computed_display_name().await.unwrap(),
DisplayName::Calculated("Matthew".to_owned())
);
}
Expand All @@ -1894,7 +1913,7 @@ mod tests {
store.save_changes(&changes).await.unwrap();

assert_eq!(
room.display_name().await.unwrap(),
room.computed_display_name().await.unwrap(),
DisplayName::Calculated("Matthew".to_owned())
);
}
Expand Down Expand Up @@ -1923,7 +1942,10 @@ mod tests {
store.save_changes(&changes).await.unwrap();

room.inner.update_if(|info| info.update_summary(&summary));
assert_eq!(room.display_name().await.unwrap(), DisplayName::EmptyWas("Matthew".to_owned()));
assert_eq!(
room.computed_display_name().await.unwrap(),
DisplayName::EmptyWas("Matthew".to_owned())
);
}

#[test]
Expand Down
2 changes: 1 addition & 1 deletion crates/matrix-sdk-base/src/sliding_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1272,7 +1272,7 @@ mod tests {
// Then the room's name is just exactly what the server supplied
let client_room = client.get_room(room_id).expect("No room found");
assert_eq!(
client_room.display_name().await.unwrap().to_string(),
client_room.computed_display_name().await.unwrap().to_string(),
"This came from the server"
);
}
Expand Down
2 changes: 1 addition & 1 deletion crates/matrix-sdk-ui/src/notification_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -715,7 +715,7 @@ impl NotificationItem {
sender_display_name,
sender_avatar_url,
is_sender_name_ambiguous,
room_display_name: room.display_name().await?.to_string(),
room_display_name: room.computed_display_name().await?.to_string(),
room_avatar_url: room.avatar_url().map(|s| s.to_string()),
room_canonical_alias: room.canonical_alias().map(|c| c.to_string()),
is_direct_message_room: room.is_direct().await?,
Expand Down
6 changes: 3 additions & 3 deletions crates/matrix-sdk-ui/src/room_list_service/room.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,9 @@ impl Room {
self.inner.room.room_id()
}

/// Get the name of the room if it exists.
pub async fn name(&self) -> Option<String> {
Some(self.inner.room.display_name().await.ok()?.to_string())
/// Get a computed room name for the room.
pub async fn computed_display_name(&self) -> Option<String> {
Some(self.inner.room.computed_display_name().await.ok()?.to_string())
}

/// Get the underlying [`matrix_sdk::Room`].
Expand Down
6 changes: 3 additions & 3 deletions crates/matrix-sdk-ui/tests/integration/room_list_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2260,15 +2260,15 @@ async fn test_room() -> Result<(), Error> {
let room0 = room_list.room(room_id_0).await?;

// Room has received a name from sliding sync.
assert_eq!(room0.name().await, Some("Room #0".to_owned()));
assert_eq!(room0.computed_display_name().await, Some("Room #0".to_owned()));

// Room has received an avatar from sliding sync.
assert_eq!(room0.avatar_url(), Some(mxc_uri!("mxc://homeserver/media").to_owned()));

let room1 = room_list.room(room_id_1).await?;

// Room has not received a name from sliding sync, then it's calculated.
assert_eq!(room1.name().await, Some("Empty Room".to_owned()));
assert_eq!(room1.computed_display_name().await, Some("Empty Room".to_owned()));

// Room has not received an avatar from sliding sync, then it's calculated, but
// there is nothing to calculate from, so there is no URL.
Expand Down Expand Up @@ -2303,7 +2303,7 @@ async fn test_room() -> Result<(), Error> {
};

// Room has _now_ received a name from sliding sync!
assert_eq!(room1.name().await, Some("Room #1".to_owned()));
assert_eq!(room1.computed_display_name().await, Some("Room #1".to_owned()));

// Room has _now_ received an avatar URL from sliding sync!
assert_eq!(room1.avatar_url(), Some(mxc_uri!("mxc://homeserver/other-media").to_owned()));
Expand Down
12 changes: 9 additions & 3 deletions crates/matrix-sdk/tests/integration/room/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,10 @@ async fn calculate_room_names_from_summary() {
let _response = client.sync_once(sync_settings).await.unwrap();
let room = client.get_room(&DEFAULT_TEST_ROOM_ID).unwrap();

assert_eq!(DisplayName::Calculated("example2".to_owned()), room.display_name().await.unwrap());
assert_eq!(
DisplayName::Calculated("example2".to_owned()),
room.computed_display_name().await.unwrap()
);
}

#[async_test]
Expand All @@ -72,7 +75,10 @@ async fn room_names() {
assert_eq!(client.rooms().len(), 1);
let room = client.get_room(&DEFAULT_TEST_ROOM_ID).unwrap();

assert_eq!(DisplayName::Aliased("tutorial".to_owned()), room.display_name().await.unwrap());
assert_eq!(
DisplayName::Aliased("tutorial".to_owned()),
room.computed_display_name().await.unwrap()
);

mock_sync(&server, &*test_json::INVITE_SYNC, Some(sync_token.clone())).await;

Expand All @@ -83,7 +89,7 @@ async fn room_names() {

assert_eq!(
DisplayName::Named("My Room Name".to_owned()),
invited_room.display_name().await.unwrap()
invited_room.computed_display_name().await.unwrap()
);
}

Expand Down
2 changes: 1 addition & 1 deletion examples/oidc_cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -880,7 +880,7 @@ async fn on_room_message(event: OriginalSyncRoomMessageEvent, room: Room) {
}
let MessageType::Text(text_content) = &event.content.msgtype else { return };

let room_name = match room.display_name().await {
let room_name = match room.computed_display_name().await {
Ok(room_name) => room_name.to_string(),
Err(error) => {
println!("Error getting room display name: {error}");
Expand Down
2 changes: 1 addition & 1 deletion examples/persist_session/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ async fn on_room_message(event: OriginalSyncRoomMessageEvent, room: Room) {
}
let MessageType::Text(text_content) = &event.content.msgtype else { return };

let room_name = match room.display_name().await {
let room_name = match room.computed_display_name().await {
Ok(room_name) => room_name.to_string(),
Err(error) => {
println!("Error getting room display name: {error}");
Expand Down

0 comments on commit 90bed18

Please sign in to comment.