diff --git a/crates/matrix-sdk-base/CHANGELOG.md b/crates/matrix-sdk-base/CHANGELOG.md index 173574bce7f..dc875c7b61f 100644 --- a/crates/matrix-sdk-base/CHANGELOG.md +++ b/crates/matrix-sdk-base/CHANGELOG.md @@ -1,6 +1,8 @@ # unreleased - Replace the `Notification` type from Ruma in `SyncResponse` and `StateChanges` by a custom one +- The ambiguity maps in `SyncResponse` are moved to `JoinedRoom` and `LeftRoom` +- `AmbiguityCache` contains the room member's user ID # 0.7.0 diff --git a/crates/matrix-sdk-base/src/client.rs b/crates/matrix-sdk-base/src/client.rs index 528bda0776e..151f44b4bd9 100644 --- a/crates/matrix-sdk-base/src/client.rs +++ b/crates/matrix-sdk-base/src/client.rs @@ -849,6 +849,8 @@ impl BaseClient { let notification_count = new_info.unread_notifications.into(); room_info.update_notification_count(notification_count); + let ambiguity_changes = ambiguity_cache.changes.remove(&room_id).unwrap_or_default(); + new_rooms.join.insert( room_id, JoinedRoom::new( @@ -857,6 +859,7 @@ impl BaseClient { new_info.account_data.events, new_info.ephemeral.events, notification_count, + ambiguity_changes, ), ); @@ -901,10 +904,17 @@ impl BaseClient { self.handle_room_account_data(&room_id, &new_info.account_data.events, &mut changes) .await; + let ambiguity_changes = ambiguity_cache.changes.remove(&room_id).unwrap_or_default(); + changes.add_room(room_info); new_rooms.leave.insert( room_id, - LeftRoom::new(timeline, new_info.state.events, new_info.account_data.events), + LeftRoom::new( + timeline, + new_info.state.events, + new_info.account_data.events, + ambiguity_changes, + ), ); } @@ -961,7 +971,6 @@ impl BaseClient { presence: response.presence.events, account_data: response.account_data.events, to_device, - ambiguity_changes: AmbiguityChanges { changes: ambiguity_cache.changes }, notifications, }; diff --git a/crates/matrix-sdk-base/src/deserialized_responses.rs b/crates/matrix-sdk-base/src/deserialized_responses.rs index 9785e33d63a..9c73ea64703 100644 --- a/crates/matrix-sdk-base/src/deserialized_responses.rs +++ b/crates/matrix-sdk-base/src/deserialized_responses.rs @@ -14,7 +14,7 @@ //! SDK-specific variations of response types from Ruma. -use std::{collections::BTreeMap, fmt}; +use std::{collections::BTreeMap, fmt, iter}; pub use matrix_sdk_common::deserialized_responses::*; use ruma::{ @@ -34,9 +34,12 @@ use serde::Serialize; /// A change in ambiguity of room members that an `m.room.member` event /// triggers. -#[derive(Clone, Debug, Default)] +#[derive(Clone, Debug)] #[non_exhaustive] pub struct AmbiguityChange { + /// The user ID of the member that is contained in the state key of the + /// `m.room.member` event. + pub member_id: OwnedUserId, /// Is the member that is contained in the state key of the `m.room.member` /// event itself ambiguous because of the event. pub member_ambiguous: bool, @@ -46,6 +49,15 @@ pub struct AmbiguityChange { pub ambiguated_member: Option, } +impl AmbiguityChange { + /// Get an iterator over the user IDs listed in this `AmbiguityChange`. + pub fn user_ids(&self) -> impl Iterator { + iter::once(&*self.member_id) + .chain(self.disambiguated_member.as_deref()) + .chain(self.ambiguated_member.as_deref()) + } +} + /// Collection of ambiguity changes that room member events trigger. #[derive(Clone, Debug, Default)] #[non_exhaustive] diff --git a/crates/matrix-sdk-base/src/sliding_sync.rs b/crates/matrix-sdk-base/src/sliding_sync.rs index 28f7e9dd2fc..859f5118942 100644 --- a/crates/matrix-sdk-base/src/sliding_sync.rs +++ b/crates/matrix-sdk-base/src/sliding_sync.rs @@ -36,7 +36,6 @@ use crate::latest_event::{is_suitable_for_latest_event, LatestEvent, PossibleLat #[cfg(feature = "e2e-encryption")] use crate::RoomMemberships; use crate::{ - deserialized_responses::AmbiguityChanges, error::Result, read_receipts::{compute_unread_counts, PreviousEventsProvider}, rooms::RoomState, @@ -299,7 +298,6 @@ impl BaseClient { Ok(SyncResponse { rooms: new_rooms, - ambiguity_changes: AmbiguityChanges { changes: ambiguity_cache.changes }, notifications, // FIXME not yet supported by sliding sync. presence: Default::default(), @@ -415,6 +413,8 @@ impl BaseClient { let notification_count = room_data.unread_notifications.clone().into(); room_info.update_notification_count(notification_count); + let ambiguity_changes = ambiguity_cache.changes.remove(room_id).unwrap_or_default(); + match room_info.state() { RoomState::Joined => { // Ephemeral events are added separately, because we might not @@ -430,6 +430,7 @@ impl BaseClient { room_account_data.unwrap_or_default(), ephemeral, notification_count, + ambiguity_changes, )), None, None, @@ -443,6 +444,7 @@ impl BaseClient { timeline, raw_state_events, room_account_data.unwrap_or_default(), + ambiguity_changes, )), None, )), diff --git a/crates/matrix-sdk-base/src/store/ambiguity_map.rs b/crates/matrix-sdk-base/src/store/ambiguity_map.rs index 6cded30c2ed..25ce7366e21 100644 --- a/crates/matrix-sdk-base/src/store/ambiguity_map.rs +++ b/crates/matrix-sdk-base/src/store/ambiguity_map.rs @@ -118,6 +118,7 @@ impl AmbiguityCache { self.update(room_id, old_map, new_map); let change = AmbiguityChange { + member_id: member_event.state_key().clone(), disambiguated_member, ambiguated_member, member_ambiguous: ambiguous, diff --git a/crates/matrix-sdk-base/src/sync.rs b/crates/matrix-sdk-base/src/sync.rs index d629b47e337..28fddb3ee4c 100644 --- a/crates/matrix-sdk-base/src/sync.rs +++ b/crates/matrix-sdk-base/src/sync.rs @@ -27,13 +27,13 @@ use ruma::{ }, push::Action, serde::Raw, - OwnedRoomId, + OwnedEventId, OwnedRoomId, }; use serde::{Deserialize, Serialize}; use crate::{ debug::{DebugInvitedRoom, DebugListOfRawEvents, DebugListOfRawEventsNoId}, - deserialized_responses::{AmbiguityChanges, RawAnySyncOrStrippedTimelineEvent}, + deserialized_responses::{AmbiguityChange, RawAnySyncOrStrippedTimelineEvent}, }; /// Internal representation of a `/sync` response. @@ -50,8 +50,6 @@ pub struct SyncResponse { pub account_data: Vec>, /// Messages sent directly between devices. pub to_device: Vec>, - /// Collection of ambiguity changes that room member events trigger. - pub ambiguity_changes: AmbiguityChanges, /// New notifications per room. pub notifications: BTreeMap>, } @@ -63,7 +61,6 @@ impl fmt::Debug for SyncResponse { .field("rooms", &self.rooms) .field("account_data", &DebugListOfRawEventsNoId(&self.account_data)) .field("to_device", &DebugListOfRawEventsNoId(&self.to_device)) - .field("ambiguity_changes", &self.ambiguity_changes) .field("notifications", &self.notifications) .finish_non_exhaustive() } @@ -108,6 +105,11 @@ pub struct JoinedRoom { /// The ephemeral events in the room that aren't recorded in the timeline or /// state of the room. e.g. typing. pub ephemeral: Vec>, + /// Collection of ambiguity changes that room member events trigger. + /// + /// This is a map of event ID of the `m.room.member` event to the + /// details of the ambiguity change. + pub ambiguity_changes: BTreeMap, } #[cfg(not(tarpaulin_include))] @@ -119,6 +121,7 @@ impl fmt::Debug for JoinedRoom { .field("state", &DebugListOfRawEvents(&self.state)) .field("account_data", &DebugListOfRawEventsNoId(&self.account_data)) .field("ephemeral", &self.ephemeral) + .field("ambiguity_changes", &self.ambiguity_changes) .finish() } } @@ -130,8 +133,9 @@ impl JoinedRoom { account_data: Vec>, ephemeral: Vec>, unread_notifications: UnreadNotificationsCount, + ambiguity_changes: BTreeMap, ) -> Self { - Self { unread_notifications, timeline, state, account_data, ephemeral } + Self { unread_notifications, timeline, state, account_data, ephemeral, ambiguity_changes } } } @@ -167,6 +171,11 @@ pub struct LeftRoom { pub state: Vec>, /// The private data that this user has attached to this room. pub account_data: Vec>, + /// Collection of ambiguity changes that room member events trigger. + /// + /// This is a map of event ID of the `m.room.member` event to the + /// details of the ambiguity change. + pub ambiguity_changes: BTreeMap, } impl LeftRoom { @@ -174,8 +183,9 @@ impl LeftRoom { timeline: Timeline, state: Vec>, account_data: Vec>, + ambiguity_changes: BTreeMap, ) -> Self { - Self { timeline, state, account_data } + Self { timeline, state, account_data, ambiguity_changes } } } @@ -186,6 +196,7 @@ impl fmt::Debug for LeftRoom { .field("timeline", &self.timeline) .field("state", &DebugListOfRawEvents(&self.state)) .field("account_data", &DebugListOfRawEventsNoId(&self.account_data)) + .field("ambiguity_changes", &self.ambiguity_changes) .finish() } } diff --git a/crates/matrix-sdk-ui/src/event_graph.rs b/crates/matrix-sdk-ui/src/event_graph.rs index 2d3b3978dac..ad05d8f8213 100644 --- a/crates/matrix-sdk-ui/src/event_graph.rs +++ b/crates/matrix-sdk-ui/src/event_graph.rs @@ -45,13 +45,13 @@ use std::{collections::BTreeMap, fmt::Debug, sync::Arc}; use async_trait::async_trait; use matrix_sdk::{sync::RoomUpdate, Client, Room}; use matrix_sdk_base::{ - deserialized_responses::SyncTimelineEvent, + deserialized_responses::{AmbiguityChange, SyncTimelineEvent}, sync::{JoinedRoom, LeftRoom, Timeline}, }; use ruma::{ events::{AnyRoomAccountDataEvent, AnySyncEphemeralRoomEvent}, serde::Raw, - OwnedRoomId, RoomId, + OwnedEventId, OwnedRoomId, RoomId, }; use tokio::{ spawn, @@ -246,8 +246,13 @@ impl RoomEventGraphInner { } async fn handle_joined_room_update(&self, updates: JoinedRoom) -> Result<()> { - self.handle_timeline(updates.timeline, updates.ephemeral.clone(), updates.account_data) - .await?; + self.handle_timeline( + updates.timeline, + updates.ephemeral.clone(), + updates.account_data, + updates.ambiguity_changes, + ) + .await?; Ok(()) } @@ -256,6 +261,7 @@ impl RoomEventGraphInner { timeline: Timeline, ephemeral: Vec>, account_data: Vec>, + ambiguity_changes: BTreeMap, ) -> Result<()> { let room_id = self.room.room_id(); @@ -278,13 +284,15 @@ impl RoomEventGraphInner { prev_batch: timeline.prev_batch, ephemeral, account_data, + ambiguity_changes, }); Ok(()) } async fn handle_left_room_update(&self, updates: LeftRoom) -> Result<()> { - self.handle_timeline(updates.timeline, Vec::new(), Vec::new()).await?; + self.handle_timeline(updates.timeline, Vec::new(), Vec::new(), updates.ambiguity_changes) + .await?; Ok(()) } @@ -350,6 +358,7 @@ impl RoomEventGraphInner { prev_batch: None, account_data: Default::default(), ephemeral: Default::default(), + ambiguity_changes: Default::default(), }); Ok(()) @@ -374,5 +383,10 @@ pub enum RoomEventGraphUpdate { /// XXX: this is temporary, until read receipts are handled in the event /// graph ephemeral: Vec>, + /// Collection of ambiguity changes that room member events trigger. + /// + /// This is a map of event ID of the `m.room.member` event to the + /// details of the ambiguity change. + ambiguity_changes: BTreeMap, }, } diff --git a/crates/matrix-sdk-ui/src/timeline/builder.rs b/crates/matrix-sdk-ui/src/timeline/builder.rs index 2128e115cac..17368cb0929 100644 --- a/crates/matrix-sdk-ui/src/timeline/builder.rs +++ b/crates/matrix-sdk-ui/src/timeline/builder.rs @@ -193,6 +193,7 @@ impl TimelineBuilder { prev_batch, account_data, ephemeral, + ambiguity_changes, } => { trace!("Received new events"); @@ -211,9 +212,16 @@ impl TimelineBuilder { state: Default::default(), account_data, ephemeral, + ambiguity_changes: Default::default(), }; inner.handle_joined_room_update(update).await; + let member_ambiguity_changes = ambiguity_changes + .values() + .flat_map(|change| change.user_ids()) + .collect::>(); + inner.force_update_sender_profiles(&member_ambiguity_changes).await; + sync_response_notify.notify_waiters(); } } diff --git a/crates/matrix-sdk-ui/src/timeline/inner/mod.rs b/crates/matrix-sdk-ui/src/timeline/inner/mod.rs index ec328b6a195..348e6d91767 100644 --- a/crates/matrix-sdk-ui/src/timeline/inner/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/inner/mod.rs @@ -870,8 +870,8 @@ impl TimelineInner

{ }); } - pub(super) async fn update_sender_profiles(&self) { - trace!("Updating sender profiles"); + pub(super) async fn update_missing_sender_profiles(&self) { + trace!("Updating missing sender profiles"); let mut state = self.state.write().await; let mut entries = state.items.entries(); @@ -907,7 +907,52 @@ impl TimelineInner

{ } } - trace!("Done updating sender profiles"); + trace!("Done updating missing sender profiles"); + } + + /// Update the profiles of the given senders, even if they are ready. + pub(super) async fn force_update_sender_profiles(&self, sender_ids: &BTreeSet<&UserId>) { + trace!("Forcing update of sender profiles: {sender_ids:?}"); + + let mut state = self.state.write().await; + let mut entries = state.items.entries(); + while let Some(mut entry) = entries.next() { + let Some(event_item) = entry.as_event() else { continue }; + if !sender_ids.contains(event_item.sender()) { + continue; + } + + let event_id = event_item.event_id().map(debug); + let transaction_id = event_item.transaction_id().map(debug); + + match self.room_data_provider.profile_from_user_id(event_item.sender()).await { + Some(profile) => { + if matches!(event_item.sender_profile(), TimelineDetails::Ready(old_profile) if *old_profile == profile) + { + debug!(event_id, transaction_id, "Profile already up-to-date"); + } else { + trace!(event_id, transaction_id, "Updating profile"); + let updated_item = + event_item.with_sender_profile(TimelineDetails::Ready(profile)); + let new_item = entry.with_kind(updated_item); + ObservableVectorEntry::set(&mut entry, new_item); + } + } + None => { + if !event_item.sender_profile().is_unavailable() { + trace!(event_id, transaction_id, "Marking profile unavailable"); + let updated_item = + event_item.with_sender_profile(TimelineDetails::Unavailable); + let new_item = entry.with_kind(updated_item); + ObservableVectorEntry::set(&mut entry, new_item); + } else { + debug!(event_id, transaction_id, "Profile already marked unavailable"); + } + } + } + } + + trace!("Done forcing update of sender profiles"); } #[cfg(test)] diff --git a/crates/matrix-sdk-ui/src/timeline/mod.rs b/crates/matrix-sdk-ui/src/timeline/mod.rs index deeba42f791..e0743e77273 100644 --- a/crates/matrix-sdk-ui/src/timeline/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/mod.rs @@ -661,7 +661,7 @@ impl Timeline { self.inner.set_sender_profiles_pending().await; match self.room().sync_members().await { Ok(_) => { - self.inner.update_sender_profiles().await; + self.inner.update_missing_sender_profiles().await; } Err(e) => { self.inner.set_sender_profiles_error(Arc::new(e)).await; diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/subscribe.rs b/crates/matrix-sdk-ui/tests/integration/timeline/subscribe.rs index 72a8cf88342..a06f099b846 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/subscribe.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/subscribe.rs @@ -23,7 +23,7 @@ use matrix_sdk_test::{ async_test, sync_timeline_event, EventBuilder, GlobalAccountDataTestEvent, JoinedRoomBuilder, SyncResponseBuilder, ALICE, BOB, }; -use matrix_sdk_ui::timeline::{RoomExt, TimelineItemContent, VirtualTimelineItem}; +use matrix_sdk_ui::timeline::{RoomExt, TimelineDetails, TimelineItemContent, VirtualTimelineItem}; use ruma::{ event_id, events::room::{ @@ -335,3 +335,262 @@ async fn test_timeline_is_reset_when_a_user_is_ignored_or_unignored() { }); assert_pending!(timeline_stream); } + +#[async_test] +async fn test_profile_updates() { + let room_id = room_id!("!a98sd12bjh:example.org"); + let (client, server) = logged_in_client().await; + let sync_settings = SyncSettings::new().timeout(Duration::from_millis(3000)); + + let mut ev_builder = SyncResponseBuilder::new(); + ev_builder.add_joined_room(JoinedRoomBuilder::new(room_id)); + + mock_sync(&server, ev_builder.build_json_sync_response(), None).await; + let _response = client.sync_once(sync_settings.clone()).await.unwrap(); + server.reset().await; + + let room = client.get_room(room_id).unwrap(); + let timeline = room.timeline_builder().build().await.unwrap(); + let (_, timeline_stream) = timeline.subscribe().await; + pin_mut!(timeline_stream); + + let alice = "@alice:example.org"; + let bob = "@bob:example.org"; + + // Add users with unknown profile. + let event_1_id = event_id!("$YTQwYl2pl1"); + let event_2_id = event_id!("$YTQwYl2pl2"); + + ev_builder.add_joined_room( + JoinedRoomBuilder::new(room_id) + .add_timeline_event(sync_timeline_event!({ + "content": { + "body": "hello", + "msgtype": "m.text", + }, + "event_id": event_1_id, + "origin_server_ts": 152037280, + "sender": alice, + "type": "m.room.message", + })) + .add_timeline_event(sync_timeline_event!({ + "content": { + "body": "hello", + "msgtype": "m.text", + }, + "event_id": event_2_id, + "origin_server_ts": 152037280, + "sender": bob, + "type": "m.room.message", + })), + ); + + mock_sync(&server, ev_builder.build_json_sync_response(), None).await; + let _response = client.sync_once(sync_settings.clone()).await.unwrap(); + server.reset().await; + + assert_next_matches!(timeline_stream, VectorDiff::PushBack { value } => { + assert_matches!(value.as_virtual(), Some(VirtualTimelineItem::DayDivider(_))); + }); + + let item_1 = assert_next_matches!(timeline_stream, VectorDiff::PushBack { value } => value); + let event_1_item = item_1.as_event().unwrap(); + assert_eq!(event_1_item.event_id(), Some(event_1_id)); + assert_matches!(event_1_item.sender_profile(), TimelineDetails::Unavailable); + + let item_2 = assert_next_matches!(timeline_stream, VectorDiff::PushBack { value } => value); + let event_2_item = item_2.as_event().unwrap(); + assert_eq!(event_2_item.event_id(), Some(event_2_id)); + assert_matches!(event_2_item.sender_profile(), TimelineDetails::Unavailable); + + assert_pending!(timeline_stream); + + // Add profiles of users and other message. + let event_3_id = event_id!("$YTQwYl2pl3"); + let event_4_id = event_id!("$YTQwYl2pl4"); + let event_5_id = event_id!("$YTQwYl2pl5"); + + ev_builder.add_joined_room( + JoinedRoomBuilder::new(room_id) + .add_timeline_event(sync_timeline_event!({ + "content": { + "displayname": "Member", + "membership": "join" + }, + "event_id": event_3_id, + "origin_server_ts": 152037280, + "sender": bob, + "state_key": bob, + "type": "m.room.member", + })) + .add_timeline_event(sync_timeline_event!({ + "content": { + "displayname": "Alice", + "membership": "join" + }, + "event_id": event_4_id, + "origin_server_ts": 152037280, + "sender": alice, + "state_key": alice, + "type": "m.room.member", + })) + .add_timeline_event(sync_timeline_event!({ + "content": { + "body": "hello", + "msgtype": "m.text", + }, + "event_id": event_5_id, + "origin_server_ts": 152037280, + "sender": alice, + "type": "m.room.message", + })), + ); + + mock_sync(&server, ev_builder.build_json_sync_response(), None).await; + let _response = client.sync_once(sync_settings.clone()).await.unwrap(); + server.reset().await; + + // Read receipt change. + assert_next_matches!(timeline_stream, VectorDiff::Set { index: 2, value } => { + assert_eq!(value.as_event().unwrap().event_id(), Some(event_2_id)); + }); + + // The events are added. + let item_3 = assert_next_matches!(timeline_stream, VectorDiff::PushBack { value } => value); + let event_3_item = item_3.as_event().unwrap(); + assert_eq!(event_3_item.event_id(), Some(event_3_id)); + let profile = + assert_matches!(event_3_item.sender_profile(), TimelineDetails::Ready(profile) => profile); + assert_eq!(profile.display_name.as_deref(), Some("Member")); + assert!(!profile.display_name_ambiguous); + + // Read receipt change. + assert_next_matches!(timeline_stream, VectorDiff::Set { index: 1, value } => { + assert_eq!(value.as_event().unwrap().event_id(), Some(event_1_id)); + }); + + let item_4 = assert_next_matches!(timeline_stream, VectorDiff::PushBack { value } => value); + let event_4_item = item_4.as_event().unwrap(); + assert_eq!(event_4_item.event_id(), Some(event_4_id)); + let profile = + assert_matches!(event_4_item.sender_profile(), TimelineDetails::Ready(profile) => profile); + assert_eq!(profile.display_name.as_deref(), Some("Alice")); + assert!(!profile.display_name_ambiguous); + + // Read receipt change. + assert_next_matches!(timeline_stream, VectorDiff::Set { index: 4, value } => { + assert_eq!(value.as_event().unwrap().event_id(), Some(event_4_id)); + }); + + let item_5 = assert_next_matches!(timeline_stream, VectorDiff::PushBack { value } => value); + let event_5_item = item_5.as_event().unwrap(); + assert_eq!(event_5_item.event_id(), Some(event_5_id)); + let profile = + assert_matches!(event_5_item.sender_profile(), TimelineDetails::Ready(profile) => profile); + assert_eq!(profile.display_name.as_deref(), Some("Alice")); + assert!(!profile.display_name_ambiguous); + + // The profiles changed. + let item_1 = + assert_next_matches!(timeline_stream, VectorDiff::Set { index: 1, value } => value); + let event_1_item = item_1.as_event().unwrap(); + assert_eq!(event_1_item.event_id(), Some(event_1_id)); + let profile = + assert_matches!(event_1_item.sender_profile(), TimelineDetails::Ready(profile) => profile); + assert_eq!(profile.display_name.as_deref(), Some("Alice")); + assert!(!profile.display_name_ambiguous); + + let item_2 = + assert_next_matches!(timeline_stream, VectorDiff::Set { index: 2, value } => value); + let event_2_item = item_2.as_event().unwrap(); + assert_eq!(event_2_item.event_id(), Some(event_2_id)); + let profile = + assert_matches!(event_2_item.sender_profile(), TimelineDetails::Ready(profile) => profile); + assert_eq!(profile.display_name.as_deref(), Some("Member")); + assert!(!profile.display_name_ambiguous); + + assert_pending!(timeline_stream); + + // Change name to be ambiguous. + let event_6_id = event_id!("$YTQwYl2pl6"); + + ev_builder.add_joined_room(JoinedRoomBuilder::new(room_id).add_timeline_event( + sync_timeline_event!({ + "content": { + "displayname": "Member", + "membership": "join" + }, + "event_id": event_6_id, + "origin_server_ts": 152037280, + "sender": alice, + "state_key": alice, + "type": "m.room.member", + }), + )); + + mock_sync(&server, ev_builder.build_json_sync_response(), None).await; + let _response = client.sync_once(sync_settings.clone()).await.unwrap(); + server.reset().await; + + // Read receipt change. + assert_next_matches!(timeline_stream, VectorDiff::Set { index: 5, value } => { + assert_eq!(value.as_event().unwrap().event_id(), Some(event_5_id)); + }); + + // The event is added. + let item_6 = assert_next_matches!(timeline_stream, VectorDiff::PushBack { value } => value); + let event_6_item = item_6.as_event().unwrap(); + assert_eq!(event_6_item.event_id(), Some(event_6_id)); + let profile = + assert_matches!(event_6_item.sender_profile(), TimelineDetails::Ready(profile) => profile); + assert_eq!(profile.display_name.as_deref(), Some("Member")); + assert!(profile.display_name_ambiguous); + + // The profiles changed. + let item_1 = + assert_next_matches!(timeline_stream, VectorDiff::Set { index: 1, value } => value); + let event_1_item = item_1.as_event().unwrap(); + assert_eq!(event_1_item.event_id(), Some(event_1_id)); + let profile = + assert_matches!(event_1_item.sender_profile(), TimelineDetails::Ready(profile) => profile); + assert_eq!(profile.display_name.as_deref(), Some("Member")); + assert!(profile.display_name_ambiguous); + + let item_2 = + assert_next_matches!(timeline_stream, VectorDiff::Set { index: 2, value } => value); + let event_2_item = item_2.as_event().unwrap(); + assert_eq!(event_2_item.event_id(), Some(event_2_id)); + let profile = + assert_matches!(event_2_item.sender_profile(), TimelineDetails::Ready(profile) => profile); + assert_eq!(profile.display_name.as_deref(), Some("Member")); + assert!(profile.display_name_ambiguous); + + let item_3 = + assert_next_matches!(timeline_stream, VectorDiff::Set { index: 3, value } => value); + let event_3_item = item_3.as_event().unwrap(); + assert_eq!(event_3_item.event_id(), Some(event_3_id)); + let profile = + assert_matches!(event_3_item.sender_profile(), TimelineDetails::Ready(profile) => profile); + assert_eq!(profile.display_name.as_deref(), Some("Member")); + assert!(profile.display_name_ambiguous); + + let item_4 = + assert_next_matches!(timeline_stream, VectorDiff::Set { index: 4, value } => value); + let event_4_item = item_4.as_event().unwrap(); + assert_eq!(event_4_item.event_id(), Some(event_4_id)); + let profile = + assert_matches!(event_4_item.sender_profile(), TimelineDetails::Ready(profile) => profile); + assert_eq!(profile.display_name.as_deref(), Some("Member")); + assert!(profile.display_name_ambiguous); + + let item_5 = + assert_next_matches!(timeline_stream, VectorDiff::Set { index: 5, value } => value); + let event_5_item = item_5.as_event().unwrap(); + assert_eq!(event_5_item.event_id(), Some(event_5_id)); + let profile = + assert_matches!(event_5_item.sender_profile(), TimelineDetails::Ready(profile) => profile); + assert_eq!(profile.display_name.as_deref(), Some("Member")); + assert!(profile.display_name_ambiguous); + + assert_pending!(timeline_stream); +} diff --git a/crates/matrix-sdk/CHANGELOG.md b/crates/matrix-sdk/CHANGELOG.md index 9c7d99809ea..28c7167d458 100644 --- a/crates/matrix-sdk/CHANGELOG.md +++ b/crates/matrix-sdk/CHANGELOG.md @@ -5,6 +5,8 @@ Breaking changes: - Replace the `Notification` type from Ruma in `SyncResponse` and `Client::register_notification_handler` by a custom one - `Room::can_user_redact` and `Member::can_redact` are split between `*_redact_own` and `*_redact_other` +- The ambiguity maps in `SyncResponse` are moved to `JoinedRoom` and `LeftRoom` +- `AmbiguityCache` contains the room member's user ID # 0.7.0 diff --git a/crates/matrix-sdk/src/sync.rs b/crates/matrix-sdk/src/sync.rs index 5c845a5934f..85f1f6bb1dc 100644 --- a/crates/matrix-sdk/src/sync.rs +++ b/crates/matrix-sdk/src/sync.rs @@ -23,7 +23,6 @@ use std::{ pub use matrix_sdk_base::sync::*; use matrix_sdk_base::{ debug::{DebugInvitedRoom, DebugListOfRawEventsNoId}, - deserialized_responses::AmbiguityChanges, instant::Instant, sync::SyncResponse as BaseSyncResponse, }; @@ -51,32 +50,16 @@ pub struct SyncResponse { pub account_data: Vec>, /// Messages sent directly between devices. pub to_device: Vec>, - /// Collection of ambiguity changes that room member events trigger. - pub ambiguity_changes: AmbiguityChanges, /// New notifications per room. pub notifications: BTreeMap>, } impl SyncResponse { pub(crate) fn new(next_batch: String, base_response: BaseSyncResponse) -> Self { - let BaseSyncResponse { - rooms, - presence, - account_data, - to_device, - ambiguity_changes, - notifications, - } = base_response; - - Self { - next_batch, - rooms, - presence, - account_data, - to_device, - ambiguity_changes, - notifications, - } + let BaseSyncResponse { rooms, presence, account_data, to_device, notifications } = + base_response; + + Self { next_batch, rooms, presence, account_data, to_device, notifications } } } @@ -88,7 +71,6 @@ impl fmt::Debug for SyncResponse { .field("rooms", &self.rooms) .field("account_data", &DebugListOfRawEventsNoId(&self.account_data)) .field("to_device", &DebugListOfRawEventsNoId(&self.to_device)) - .field("ambiguity_changes", &self.ambiguity_changes) .field("notifications", &self.notifications) .finish_non_exhaustive() } @@ -164,14 +146,7 @@ impl Client { /// the event, room update and notification handlers. #[tracing::instrument(skip(self, response))] pub(crate) async fn handle_sync_response(&self, response: &BaseSyncResponse) -> Result<()> { - let BaseSyncResponse { - rooms, - presence, - account_data, - to_device, - ambiguity_changes: _, - notifications, - } = response; + let BaseSyncResponse { rooms, presence, account_data, to_device, notifications } = response; let now = Instant::now(); self.handle_sync_events(HandlerKind::GlobalAccountData, None, account_data).await?; @@ -189,8 +164,14 @@ impl Client { updates: room_info.clone(), }); - let JoinedRoom { unread_notifications: _, timeline, state, account_data, ephemeral } = - room_info; + let JoinedRoom { + unread_notifications: _, + timeline, + state, + account_data, + ephemeral, + ambiguity_changes: _, + } = room_info; let room = Some(&room); self.handle_sync_events(HandlerKind::RoomAccountData, room, account_data).await?; @@ -212,7 +193,7 @@ impl Client { updates: room_info.clone(), }); - let LeftRoom { timeline, state, account_data } = room_info; + let LeftRoom { timeline, state, account_data, ambiguity_changes: _ } = room_info; let room = Some(&room); self.handle_sync_events(HandlerKind::RoomAccountData, room, account_data).await?; diff --git a/crates/matrix-sdk/tests/integration/client.rs b/crates/matrix-sdk/tests/integration/client.rs index 355c93d409d..8e83a454903 100644 --- a/crates/matrix-sdk/tests/integration/client.rs +++ b/crates/matrix-sdk/tests/integration/client.rs @@ -8,7 +8,10 @@ use matrix_sdk::{ sync::RoomUpdate, }; use matrix_sdk_base::RoomState; -use matrix_sdk_test::{async_test, test_json, DEFAULT_TEST_ROOM_ID}; +use matrix_sdk_test::{ + async_test, sync_state_event, test_json, JoinedRoomBuilder, SyncResponseBuilder, + DEFAULT_TEST_ROOM_ID, +}; use ruma::{ api::client::{ directory::{ @@ -20,6 +23,7 @@ use ruma::{ }, assign, device_id, directory::Filter, + event_id, events::{ direct::DirectEventContent, room::{message::ImageMessageEventContent, ImageInfo, MediaSource}, @@ -30,6 +34,8 @@ use ruma::{ uint, user_id, OwnedUserId, }; use serde_json::{json, Value as JsonValue}; +use stream_assert::{assert_next_matches, assert_pending}; +use tokio_stream::wrappers::BroadcastStream; use wiremock::{ matchers::{header, method, path, path_regex}, Mock, Request, ResponseTemplate, @@ -898,3 +904,298 @@ async fn create_dm_error() { assert_eq!(client_api_error.status_code, 404); } + +#[async_test] +async fn ambiguity_changes() { + let (client, server) = logged_in_client().await; + + let example_id = user_id!("@example:localhost"); + let example_2_id = user_id!("@example2:localhost"); + let example_3_id = user_id!("@example3:localhost"); + + let mut updates = BroadcastStream::new(client.subscribe_to_room_updates(&DEFAULT_TEST_ROOM_ID)); + assert_pending!(updates); + + // Initial sync, adds 2 members. + mock_sync(&server, &*test_json::SYNC, None).await; + let response = client.sync_once(SyncSettings::default()).await.unwrap(); + server.reset().await; + + let room = client.get_room(&DEFAULT_TEST_ROOM_ID).unwrap(); + + let changes = &response.rooms.join.get(*DEFAULT_TEST_ROOM_ID).unwrap().ambiguity_changes; + + // A new member always triggers an ambiguity change. + let example_change = changes.get(event_id!("$151800140517rfvjc:localhost")).unwrap(); + assert_eq!(example_change.member_id, example_id); + assert!(!example_change.member_ambiguous); + assert_eq!(example_change.ambiguated_member, None); + assert_eq!(example_change.disambiguated_member, None); + + let example_2_change = changes.get(event_id!("$152034824468gOeNB:localhost")).unwrap(); + assert_eq!(example_2_change.member_id, example_2_id); + assert!(!example_2_change.member_ambiguous); + assert_eq!(example_2_change.ambiguated_member, None); + assert_eq!(example_2_change.disambiguated_member, None); + + let example = room.get_member_no_sync(example_id).await.unwrap().unwrap(); + assert!(!example.name_ambiguous()); + let example_2 = room.get_member_no_sync(example_2_id).await.unwrap().unwrap(); + assert!(!example_2.name_ambiguous()); + + let changes = assert_next_matches!(updates, Ok(RoomUpdate::Joined { updates, .. }) => updates.ambiguity_changes); + + let example_change = changes.get(event_id!("$151800140517rfvjc:localhost")).unwrap(); + assert_eq!(example_change.member_id, example_id); + assert!(!example_change.member_ambiguous); + assert_eq!(example_change.ambiguated_member, None); + assert_eq!(example_change.disambiguated_member, None); + + let example_2_change = changes.get(event_id!("$152034824468gOeNB:localhost")).unwrap(); + assert_eq!(example_2_change.member_id, example_2_id); + assert!(!example_2_change.member_ambiguous); + assert_eq!(example_2_change.ambiguated_member, None); + assert_eq!(example_2_change.disambiguated_member, None); + + // Add 1 member and set all 3 to the same display name. + let example_2_rename_1_event_id = event_id!("$example_2_rename_1"); + let example_3_join_event_id = event_id!("$example_3_join"); + + let mut sync_builder = SyncResponseBuilder::new(); + let joined_room = JoinedRoomBuilder::new(&DEFAULT_TEST_ROOM_ID).add_state_bulk([ + sync_state_event!({ + "content": { + "avatar_url": null, + "displayname": "example", + "membership": "join" + }, + "event_id": example_2_rename_1_event_id, + "origin_server_ts": 151800140, + "sender": example_2_id, + "state_key": example_2_id, + "type": "m.room.member", + }), + sync_state_event!({ + "content": { + "avatar_url": null, + "displayname": "example", + "membership": "join" + }, + "event_id": example_3_join_event_id, + "origin_server_ts": 151800140, + "sender": example_3_id, + "state_key": example_3_id, + "type": "m.room.member", + }), + ]); + sync_builder.add_joined_room(joined_room); + + mock_sync(&server, sync_builder.build_json_sync_response(), None).await; + let response = client.sync_once(SyncSettings::default()).await.unwrap(); + server.reset().await; + + let changes = &response.rooms.join.get(*DEFAULT_TEST_ROOM_ID).unwrap().ambiguity_changes; + + // First joined member made both members ambiguous. + let example_2_change = changes.get(example_2_rename_1_event_id).unwrap(); + assert_eq!(example_2_change.member_id, example_2_id); + assert!(example_2_change.member_ambiguous); + assert_eq!(example_2_change.ambiguated_member.as_deref(), Some(example_id)); + assert_eq!(example_2_change.disambiguated_member, None); + + // Second joined member only adds itself as ambiguous. + let example_3_change = changes.get(example_3_join_event_id).unwrap(); + assert_eq!(example_3_change.member_id, example_3_id); + assert!(example_3_change.member_ambiguous); + assert_eq!(example_3_change.ambiguated_member, None); + assert_eq!(example_3_change.disambiguated_member, None); + + let example = room.get_member_no_sync(example_id).await.unwrap().unwrap(); + assert!(example.name_ambiguous()); + let example_2 = room.get_member_no_sync(example_2_id).await.unwrap().unwrap(); + assert!(example_2.name_ambiguous()); + let example_3 = room.get_member_no_sync(example_3_id).await.unwrap().unwrap(); + assert!(example_3.name_ambiguous()); + + let changes = assert_next_matches!(updates, Ok(RoomUpdate::Joined { updates, .. }) => updates.ambiguity_changes); + + let example_2_change = changes.get(example_2_rename_1_event_id).unwrap(); + assert_eq!(example_2_change.member_id, example_2_id); + assert!(example_2_change.member_ambiguous); + assert_eq!(example_2_change.ambiguated_member.as_deref(), Some(example_id)); + assert_eq!(example_2_change.disambiguated_member, None); + + let example_3_change = changes.get(example_3_join_event_id).unwrap(); + assert_eq!(example_3_change.member_id, example_3_id); + assert!(example_3_change.member_ambiguous); + assert_eq!(example_3_change.ambiguated_member, None); + assert_eq!(example_3_change.disambiguated_member, None); + + // Rename example 2 to a unique name. + let example_2_rename_2_event_id = event_id!("$example_2_rename_2"); + + let joined_room = + JoinedRoomBuilder::new(&DEFAULT_TEST_ROOM_ID).add_state_bulk([sync_state_event!({ + "content": { + "avatar_url": null, + "displayname": "another example", + "membership": "join" + }, + "event_id": example_2_rename_2_event_id, + "origin_server_ts": 151800140, + "sender": example_2_id, + "state_key": example_2_id, + "type": "m.room.member", + })]); + sync_builder.add_joined_room(joined_room); + + mock_sync(&server, sync_builder.build_json_sync_response(), None).await; + let response = client.sync_once(SyncSettings::default()).await.unwrap(); + server.reset().await; + + let changes = &response.rooms.join.get(*DEFAULT_TEST_ROOM_ID).unwrap().ambiguity_changes; + + // example 2 is not ambiguous anymore. + let example_2_change = changes.get(example_2_rename_2_event_id).unwrap(); + assert_eq!(example_2_change.member_id, example_2_id); + assert!(!example_2_change.member_ambiguous); + assert_eq!(example_2_change.ambiguated_member, None); + assert_eq!(example_2_change.disambiguated_member, None); + + let example = room.get_member_no_sync(example_id).await.unwrap().unwrap(); + assert!(example.name_ambiguous()); + let example_2 = room.get_member_no_sync(example_2_id).await.unwrap().unwrap(); + assert!(!example_2.name_ambiguous()); + let example_3 = room.get_member_no_sync(example_3_id).await.unwrap().unwrap(); + assert!(example_3.name_ambiguous()); + + let changes = assert_next_matches!(updates, Ok(RoomUpdate::Joined { updates, .. }) => updates.ambiguity_changes); + + let example_2_change = changes.get(example_2_rename_2_event_id).unwrap(); + assert_eq!(example_2_change.member_id, example_2_id); + assert!(!example_2_change.member_ambiguous); + assert_eq!(example_2_change.ambiguated_member, None); + assert_eq!(example_2_change.disambiguated_member, None); + + // Rename example 3, using the same name as example 2. + let example_3_rename_event_id = event_id!("$example_3_rename"); + + let joined_room = + JoinedRoomBuilder::new(&DEFAULT_TEST_ROOM_ID).add_state_bulk([sync_state_event!({ + "content": { + "avatar_url": null, + "displayname": "another example", + "membership": "join" + }, + "event_id": example_3_rename_event_id, + "origin_server_ts": 151800140, + "sender": example_3_id, + "state_key": example_3_id, + "type": "m.room.member", + })]); + sync_builder.add_joined_room(joined_room); + + mock_sync(&server, sync_builder.build_json_sync_response(), None).await; + let response = client.sync_once(SyncSettings::default()).await.unwrap(); + server.reset().await; + + let changes = &response.rooms.join.get(*DEFAULT_TEST_ROOM_ID).unwrap().ambiguity_changes; + + // example 3 is now ambiguous with example 2, not example. + let example_3_change = changes.get(example_3_rename_event_id).unwrap(); + assert_eq!(example_3_change.member_id, example_3_id); + assert!(example_3_change.member_ambiguous); + assert_eq!(example_3_change.ambiguated_member.as_deref(), Some(example_2_id)); + assert_eq!(example_3_change.disambiguated_member.as_deref(), Some(example_id)); + + let example = room.get_member_no_sync(example_id).await.unwrap().unwrap(); + assert!(!example.name_ambiguous()); + let example_2 = room.get_member_no_sync(example_2_id).await.unwrap().unwrap(); + assert!(example_2.name_ambiguous()); + let example_3 = room.get_member_no_sync(example_3_id).await.unwrap().unwrap(); + assert!(example_3.name_ambiguous()); + + let changes = assert_next_matches!(updates, Ok(RoomUpdate::Joined { updates, .. }) => updates.ambiguity_changes); + + let example_3_change = changes.get(example_3_rename_event_id).unwrap(); + assert_eq!(example_3_change.member_id, example_3_id); + assert!(example_3_change.member_ambiguous); + assert_eq!(example_3_change.ambiguated_member.as_deref(), Some(example_2_id)); + assert_eq!(example_3_change.disambiguated_member.as_deref(), Some(example_id)); + + // Rename example, still using a unique name. + let example_rename_event_id = event_id!("$example_rename"); + + let joined_room = + JoinedRoomBuilder::new(&DEFAULT_TEST_ROOM_ID).add_state_bulk([sync_state_event!({ + "content": { + "avatar_url": null, + "displayname": "the first example", + "membership": "join" + }, + "event_id": example_rename_event_id, + "origin_server_ts": 151800140, + "sender": example_id, + "state_key": example_id, + "type": "m.room.member", + })]); + sync_builder.add_joined_room(joined_room); + + mock_sync(&server, sync_builder.build_json_sync_response(), None).await; + let response = client.sync_once(SyncSettings::default()).await.unwrap(); + server.reset().await; + + let changes = &response.rooms.join.get(*DEFAULT_TEST_ROOM_ID).unwrap().ambiguity_changes; + + // name change, even if still not ambiguous, triggers ambiguity change. + let example_change = changes.get(example_rename_event_id).unwrap(); + assert_eq!(example_change.member_id, example_id); + assert!(!example_change.member_ambiguous); + assert_eq!(example_change.ambiguated_member, None); + assert_eq!(example_change.disambiguated_member, None); + + let example = room.get_member_no_sync(example_id).await.unwrap().unwrap(); + assert!(!example.name_ambiguous()); + let example_2 = room.get_member_no_sync(example_2_id).await.unwrap().unwrap(); + assert!(example_2.name_ambiguous()); + let example_3 = room.get_member_no_sync(example_3_id).await.unwrap().unwrap(); + assert!(example_3.name_ambiguous()); + + let changes = assert_next_matches!(updates, Ok(RoomUpdate::Joined { updates, .. }) => updates.ambiguity_changes); + + let example_change = changes.get(example_rename_event_id).unwrap(); + assert_eq!(example_change.member_id, example_id); + assert!(!example_change.member_ambiguous); + assert_eq!(example_change.ambiguated_member, None); + assert_eq!(example_change.disambiguated_member, None); + + // Change avatar. + let example_avatar_event_id = event_id!("$example_avatar"); + + let joined_room = + JoinedRoomBuilder::new(&DEFAULT_TEST_ROOM_ID).add_state_bulk([sync_state_event!({ + "content": { + "avatar_url": "mxc://localhost/avatar", + "displayname": "the first example", + "membership": "join" + }, + "event_id": example_avatar_event_id, + "origin_server_ts": 151800140, + "sender": example_id, + "state_key": example_id, + "type": "m.room.member", + })]); + sync_builder.add_joined_room(joined_room); + + mock_sync(&server, sync_builder.build_json_sync_response(), None).await; + let response = client.sync_once(SyncSettings::default()).await.unwrap(); + server.reset().await; + + // Avatar change does not trigger ambiguity change. + assert!(response.rooms.join.get(*DEFAULT_TEST_ROOM_ID).unwrap().ambiguity_changes.is_empty()); + + let changes = assert_next_matches!(updates, Ok(RoomUpdate::Joined { updates, .. }) => updates.ambiguity_changes); + assert!(changes.is_empty()); + + assert_pending!(updates); +}