From ef322cc39bd8fe8f4e29885e725f623d3d0bd14b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Commaille?= Date: Sat, 27 Jan 2024 15:43:30 +0100 Subject: [PATCH 1/6] base: Add room member ID to AmbiguityChange MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Makes AmbiguityChange easier to use, especially outside of SyncResponse, where we might not have the m.room.member event. Signed-off-by: Kévin Commaille --- crates/matrix-sdk-base/src/deserialized_responses.rs | 5 ++++- crates/matrix-sdk-base/src/store/ambiguity_map.rs | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/matrix-sdk-base/src/deserialized_responses.rs b/crates/matrix-sdk-base/src/deserialized_responses.rs index 9785e33d63a..c37a664ac5f 100644 --- a/crates/matrix-sdk-base/src/deserialized_responses.rs +++ b/crates/matrix-sdk-base/src/deserialized_responses.rs @@ -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, 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, From bcf1ee408b1487beac6673a875500d2dde5a8e54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Commaille?= Date: Sat, 27 Jan 2024 15:43:56 +0100 Subject: [PATCH 2/6] sdk: Add integration test for ambiguity changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Kévin Commaille --- crates/matrix-sdk/tests/integration/client.rs | 241 +++++++++++++++++- 1 file changed, 240 insertions(+), 1 deletion(-) diff --git a/crates/matrix-sdk/tests/integration/client.rs b/crates/matrix-sdk/tests/integration/client.rs index 355c93d409d..702e0612f3c 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}, @@ -898,3 +902,238 @@ 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"); + + // 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.ambiguity_changes.changes.get(*DEFAULT_TEST_ROOM_ID).unwrap(); + + // 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()); + + // 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.ambiguity_changes.changes.get(*DEFAULT_TEST_ROOM_ID).unwrap(); + + // 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()); + + // 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.ambiguity_changes.changes.get(*DEFAULT_TEST_ROOM_ID).unwrap(); + + // 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()); + + // 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.ambiguity_changes.changes.get(*DEFAULT_TEST_ROOM_ID).unwrap(); + + // 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()); + + // 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.ambiguity_changes.changes.get(*DEFAULT_TEST_ROOM_ID).unwrap(); + + // 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()); + + // 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.ambiguity_changes.changes.get(*DEFAULT_TEST_ROOM_ID).is_none()); +} From f0d722099f22e41298c051effc4e3935853ef0ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Commaille?= Date: Sat, 27 Jan 2024 16:19:16 +0100 Subject: [PATCH 3/6] sdk: Allow to receive ambiguity changes per-room. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Kévin Commaille --- crates/matrix-sdk/CHANGELOG.md | 2 + crates/matrix-sdk/src/sync.rs | 57 ++++++++++++++--- crates/matrix-sdk/tests/integration/client.rs | 62 +++++++++++++++++++ 3 files changed, 111 insertions(+), 10 deletions(-) diff --git a/crates/matrix-sdk/CHANGELOG.md b/crates/matrix-sdk/CHANGELOG.md index 9c7d99809ea..563cfaa07f9 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` +- `RoomUpdate` also contains the ambiguity changes of a `Room` with `AmbiguityCache` containing 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..be40128e116 100644 --- a/crates/matrix-sdk/src/sync.rs +++ b/crates/matrix-sdk/src/sync.rs @@ -23,7 +23,7 @@ use std::{ pub use matrix_sdk_base::sync::*; use matrix_sdk_base::{ debug::{DebugInvitedRoom, DebugListOfRawEventsNoId}, - deserialized_responses::AmbiguityChanges, + deserialized_responses::{AmbiguityChange, AmbiguityChanges}, instant::Instant, sync::SyncResponse as BaseSyncResponse, }; @@ -31,7 +31,7 @@ use ruma::{ api::client::sync::sync_events::{self, v3::InvitedRoom}, events::{presence::PresenceEvent, AnyGlobalAccountDataEvent, AnyToDeviceEvent}, serde::Raw, - OwnedRoomId, RoomId, + OwnedEventId, OwnedRoomId, RoomId, }; use tracing::{debug, error, warn}; @@ -103,6 +103,11 @@ pub enum RoomUpdate { room: Room, /// Updates to the room. updates: LeftRoom, + /// 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, }, /// Updates to a room the user is currently in. Joined { @@ -110,6 +115,11 @@ pub enum RoomUpdate { room: Room, /// Updates to the room. updates: JoinedRoom, + /// 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, }, /// Updates to a room the user is invited to. Invited { @@ -117,6 +127,11 @@ pub enum RoomUpdate { room: Room, /// Updates to the room. updates: InvitedRoom, + /// 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, }, } @@ -124,16 +139,23 @@ pub enum RoomUpdate { impl fmt::Debug for RoomUpdate { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - Self::Left { room, updates } => { - f.debug_struct("Left").field("room", room).field("updates", updates).finish() - } - Self::Joined { room, updates } => { - f.debug_struct("Joined").field("room", room).field("updates", updates).finish() - } - Self::Invited { room, updates } => f + Self::Left { room, updates, ambiguity_changes } => f + .debug_struct("Left") + .field("room", room) + .field("updates", updates) + .field("ambiguity_changes", ambiguity_changes) + .finish(), + Self::Joined { room, updates, ambiguity_changes } => f + .debug_struct("Joined") + .field("room", room) + .field("updates", updates) + .field("ambiguity_changes", ambiguity_changes) + .finish(), + Self::Invited { room, updates, ambiguity_changes } => f .debug_struct("Invited") .field("room", room) .field("updates", &DebugInvitedRoom(updates)) + .field("ambiguity_changes", ambiguity_changes) .finish(), } } @@ -169,7 +191,7 @@ impl Client { presence, account_data, to_device, - ambiguity_changes: _, + ambiguity_changes, notifications, } = response; @@ -187,6 +209,11 @@ impl Client { self.send_room_update(room_id, || RoomUpdate::Joined { room: room.clone(), updates: room_info.clone(), + ambiguity_changes: ambiguity_changes + .changes + .get(room_id) + .cloned() + .unwrap_or_default(), }); let JoinedRoom { unread_notifications: _, timeline, state, account_data, ephemeral } = @@ -210,6 +237,11 @@ impl Client { self.send_room_update(room_id, || RoomUpdate::Left { room: room.clone(), updates: room_info.clone(), + ambiguity_changes: ambiguity_changes + .changes + .get(room_id) + .cloned() + .unwrap_or_default(), }); let LeftRoom { timeline, state, account_data } = room_info; @@ -229,6 +261,11 @@ impl Client { self.send_room_update(room_id, || RoomUpdate::Invited { room: room.clone(), updates: room_info.clone(), + ambiguity_changes: ambiguity_changes + .changes + .get(room_id) + .cloned() + .unwrap_or_default(), }); let invite_state = &room_info.invite_state.events; diff --git a/crates/matrix-sdk/tests/integration/client.rs b/crates/matrix-sdk/tests/integration/client.rs index 702e0612f3c..e8be3437b23 100644 --- a/crates/matrix-sdk/tests/integration/client.rs +++ b/crates/matrix-sdk/tests/integration/client.rs @@ -34,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, @@ -911,6 +913,9 @@ async fn ambiguity_changes() { 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(); @@ -938,6 +943,20 @@ async fn ambiguity_changes() { 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 { ambiguity_changes, .. }) => 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"); @@ -998,6 +1017,20 @@ async fn ambiguity_changes() { 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 { ambiguity_changes, .. }) => 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"); @@ -1036,6 +1069,14 @@ async fn ambiguity_changes() { 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 { ambiguity_changes, .. }) => 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"); @@ -1074,6 +1115,14 @@ async fn ambiguity_changes() { 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 { ambiguity_changes, .. }) => 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"); @@ -1112,6 +1161,14 @@ async fn ambiguity_changes() { 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 { ambiguity_changes, .. }) => 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"); @@ -1136,4 +1193,9 @@ async fn ambiguity_changes() { // Avatar change does not trigger ambiguity change. assert!(response.ambiguity_changes.changes.get(*DEFAULT_TEST_ROOM_ID).is_none()); + + let changes = assert_next_matches!(updates, Ok(RoomUpdate::Joined { ambiguity_changes, .. }) => ambiguity_changes); + assert!(changes.is_empty()); + + assert_pending!(updates); } From bc6c458371f2b550ad176ce02f55512b29e917b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Commaille?= Date: Sat, 27 Jan 2024 17:56:16 +0100 Subject: [PATCH 4/6] ui: Update user profile when ambiguity changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Kévin Commaille --- .../src/deserialized_responses.rs | 11 +- crates/matrix-sdk-ui/src/timeline/builder.rs | 16 +- .../matrix-sdk-ui/src/timeline/inner/mod.rs | 51 +++- crates/matrix-sdk-ui/src/timeline/mod.rs | 2 +- .../tests/integration/timeline/subscribe.rs | 261 +++++++++++++++++- 5 files changed, 333 insertions(+), 8 deletions(-) diff --git a/crates/matrix-sdk-base/src/deserialized_responses.rs b/crates/matrix-sdk-base/src/deserialized_responses.rs index c37a664ac5f..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::{ @@ -49,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-ui/src/timeline/builder.rs b/crates/matrix-sdk-ui/src/timeline/builder.rs index 1795272617b..c37cfff57aa 100644 --- a/crates/matrix-sdk-ui/src/timeline/builder.rs +++ b/crates/matrix-sdk-ui/src/timeline/builder.rs @@ -172,11 +172,23 @@ impl TimelineBuilder { trace!("Handling a room update"); match update { - RoomUpdate::Left { updates, .. } => { + RoomUpdate::Left { updates, ambiguity_changes, .. } => { inner.handle_sync_timeline(updates.timeline).await; + + let member_ambiguity_changes = ambiguity_changes + .values() + .flat_map(|change| change.user_ids()) + .collect::>(); + inner.force_update_sender_profiles(&member_ambiguity_changes).await; } - RoomUpdate::Joined { updates, .. } => { + RoomUpdate::Joined { updates, ambiguity_changes, .. } => { inner.handle_joined_room_update(updates).await; + + let member_ambiguity_changes = ambiguity_changes + .values() + .flat_map(|change| change.user_ids()) + .collect::>(); + inner.force_update_sender_profiles(&member_ambiguity_changes).await; } RoomUpdate::Invited { .. } => { warn!("Room is in invited state, can't build or update its timeline"); diff --git a/crates/matrix-sdk-ui/src/timeline/inner/mod.rs b/crates/matrix-sdk-ui/src/timeline/inner/mod.rs index cc95198926d..29f01277750 100644 --- a/crates/matrix-sdk-ui/src/timeline/inner/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/inner/mod.rs @@ -876,8 +876,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(); @@ -913,7 +913,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 ccd0c657996..711999c1f99 100644 --- a/crates/matrix-sdk-ui/src/timeline/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/mod.rs @@ -659,7 +659,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 13fa29f2cb1..aaa36fff2ca 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; + 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); +} From 3599e578e2008338c0b9b94f6c51ee1ba6e434d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Commaille?= Date: Mon, 29 Jan 2024 16:46:02 +0100 Subject: [PATCH 5/6] base: Move ambiguity changes maps from SyncResponse to {Left/Joined}Room MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Kévin Commaille --- crates/matrix-sdk-base/src/client.rs | 13 ++- crates/matrix-sdk-base/src/sliding_sync.rs | 6 +- crates/matrix-sdk-base/src/sync.rs | 25 +++-- crates/matrix-sdk-ui/src/timeline/builder.rs | 11 +- crates/matrix-sdk/src/sync.rs | 100 ++++-------------- crates/matrix-sdk/tests/integration/client.rs | 24 ++--- 6 files changed, 74 insertions(+), 105 deletions(-) diff --git a/crates/matrix-sdk-base/src/client.rs b/crates/matrix-sdk-base/src/client.rs index 0d0ae7df3bd..f916659d684 100644 --- a/crates/matrix-sdk-base/src/client.rs +++ b/crates/matrix-sdk-base/src/client.rs @@ -848,6 +848,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( @@ -856,6 +858,7 @@ impl BaseClient { new_info.account_data.events, new_info.ephemeral.events, notification_count, + ambiguity_changes, ), ); @@ -900,10 +903,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, + ), ); } @@ -959,7 +969,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/sliding_sync.rs b/crates/matrix-sdk-base/src/sliding_sync.rs index 9a6bb0b5d6b..caeb55c2de8 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(), @@ -408,6 +406,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 @@ -423,6 +423,7 @@ impl BaseClient { room_account_data.unwrap_or_default(), ephemeral, notification_count, + ambiguity_changes, )), None, None, @@ -436,6 +437,7 @@ impl BaseClient { timeline, raw_state_events, room_account_data.unwrap_or_default(), + ambiguity_changes, )), None, )), 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/timeline/builder.rs b/crates/matrix-sdk-ui/src/timeline/builder.rs index c37cfff57aa..032175577b0 100644 --- a/crates/matrix-sdk-ui/src/timeline/builder.rs +++ b/crates/matrix-sdk-ui/src/timeline/builder.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::{collections::BTreeSet, sync::Arc}; +use std::{collections::BTreeSet, mem, sync::Arc}; use eyeball::SharedObservable; use futures_util::{pin_mut, StreamExt}; @@ -172,16 +172,19 @@ impl TimelineBuilder { trace!("Handling a room update"); match update { - RoomUpdate::Left { updates, ambiguity_changes, .. } => { + RoomUpdate::Left { updates, .. } => { inner.handle_sync_timeline(updates.timeline).await; - let member_ambiguity_changes = ambiguity_changes + let member_ambiguity_changes = updates + .ambiguity_changes .values() .flat_map(|change| change.user_ids()) .collect::>(); inner.force_update_sender_profiles(&member_ambiguity_changes).await; } - RoomUpdate::Joined { updates, ambiguity_changes, .. } => { + RoomUpdate::Joined { mut updates, .. } => { + let ambiguity_changes = mem::take(&mut updates.ambiguity_changes); + inner.handle_joined_room_update(updates).await; let member_ambiguity_changes = ambiguity_changes diff --git a/crates/matrix-sdk/src/sync.rs b/crates/matrix-sdk/src/sync.rs index be40128e116..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::{AmbiguityChange, AmbiguityChanges}, instant::Instant, sync::SyncResponse as BaseSyncResponse, }; @@ -31,7 +30,7 @@ use ruma::{ api::client::sync::sync_events::{self, v3::InvitedRoom}, events::{presence::PresenceEvent, AnyGlobalAccountDataEvent, AnyToDeviceEvent}, serde::Raw, - OwnedEventId, OwnedRoomId, RoomId, + OwnedRoomId, RoomId, }; use tracing::{debug, error, warn}; @@ -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() } @@ -103,11 +85,6 @@ pub enum RoomUpdate { room: Room, /// Updates to the room. updates: LeftRoom, - /// 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, }, /// Updates to a room the user is currently in. Joined { @@ -115,11 +92,6 @@ pub enum RoomUpdate { room: Room, /// Updates to the room. updates: JoinedRoom, - /// 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, }, /// Updates to a room the user is invited to. Invited { @@ -127,11 +99,6 @@ pub enum RoomUpdate { room: Room, /// Updates to the room. updates: InvitedRoom, - /// 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, }, } @@ -139,23 +106,16 @@ pub enum RoomUpdate { impl fmt::Debug for RoomUpdate { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - Self::Left { room, updates, ambiguity_changes } => f - .debug_struct("Left") - .field("room", room) - .field("updates", updates) - .field("ambiguity_changes", ambiguity_changes) - .finish(), - Self::Joined { room, updates, ambiguity_changes } => f - .debug_struct("Joined") - .field("room", room) - .field("updates", updates) - .field("ambiguity_changes", ambiguity_changes) - .finish(), - Self::Invited { room, updates, ambiguity_changes } => f + Self::Left { room, updates } => { + f.debug_struct("Left").field("room", room).field("updates", updates).finish() + } + Self::Joined { room, updates } => { + f.debug_struct("Joined").field("room", room).field("updates", updates).finish() + } + Self::Invited { room, updates } => f .debug_struct("Invited") .field("room", room) .field("updates", &DebugInvitedRoom(updates)) - .field("ambiguity_changes", ambiguity_changes) .finish(), } } @@ -186,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?; @@ -209,15 +162,16 @@ impl Client { self.send_room_update(room_id, || RoomUpdate::Joined { room: room.clone(), updates: room_info.clone(), - ambiguity_changes: ambiguity_changes - .changes - .get(room_id) - .cloned() - .unwrap_or_default(), }); - 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?; @@ -237,14 +191,9 @@ impl Client { self.send_room_update(room_id, || RoomUpdate::Left { room: room.clone(), updates: room_info.clone(), - ambiguity_changes: ambiguity_changes - .changes - .get(room_id) - .cloned() - .unwrap_or_default(), }); - 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?; @@ -261,11 +210,6 @@ impl Client { self.send_room_update(room_id, || RoomUpdate::Invited { room: room.clone(), updates: room_info.clone(), - ambiguity_changes: ambiguity_changes - .changes - .get(room_id) - .cloned() - .unwrap_or_default(), }); let invite_state = &room_info.invite_state.events; diff --git a/crates/matrix-sdk/tests/integration/client.rs b/crates/matrix-sdk/tests/integration/client.rs index e8be3437b23..8e83a454903 100644 --- a/crates/matrix-sdk/tests/integration/client.rs +++ b/crates/matrix-sdk/tests/integration/client.rs @@ -923,7 +923,7 @@ async fn ambiguity_changes() { let room = client.get_room(&DEFAULT_TEST_ROOM_ID).unwrap(); - let changes = response.ambiguity_changes.changes.get(*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(); @@ -943,7 +943,7 @@ async fn ambiguity_changes() { 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 { ambiguity_changes, .. }) => ambiguity_changes); + 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); @@ -994,7 +994,7 @@ async fn ambiguity_changes() { let response = client.sync_once(SyncSettings::default()).await.unwrap(); server.reset().await; - let changes = response.ambiguity_changes.changes.get(*DEFAULT_TEST_ROOM_ID).unwrap(); + 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(); @@ -1017,7 +1017,7 @@ async fn ambiguity_changes() { 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 { ambiguity_changes, .. }) => ambiguity_changes); + 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); @@ -1053,7 +1053,7 @@ async fn ambiguity_changes() { let response = client.sync_once(SyncSettings::default()).await.unwrap(); server.reset().await; - let changes = response.ambiguity_changes.changes.get(*DEFAULT_TEST_ROOM_ID).unwrap(); + 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(); @@ -1069,7 +1069,7 @@ async fn ambiguity_changes() { 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 { ambiguity_changes, .. }) => ambiguity_changes); + 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); @@ -1099,7 +1099,7 @@ async fn ambiguity_changes() { let response = client.sync_once(SyncSettings::default()).await.unwrap(); server.reset().await; - let changes = response.ambiguity_changes.changes.get(*DEFAULT_TEST_ROOM_ID).unwrap(); + 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(); @@ -1115,7 +1115,7 @@ async fn ambiguity_changes() { 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 { ambiguity_changes, .. }) => ambiguity_changes); + 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); @@ -1145,7 +1145,7 @@ async fn ambiguity_changes() { let response = client.sync_once(SyncSettings::default()).await.unwrap(); server.reset().await; - let changes = response.ambiguity_changes.changes.get(*DEFAULT_TEST_ROOM_ID).unwrap(); + 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(); @@ -1161,7 +1161,7 @@ async fn ambiguity_changes() { 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 { ambiguity_changes, .. }) => ambiguity_changes); + 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); @@ -1192,9 +1192,9 @@ async fn ambiguity_changes() { server.reset().await; // Avatar change does not trigger ambiguity change. - assert!(response.ambiguity_changes.changes.get(*DEFAULT_TEST_ROOM_ID).is_none()); + assert!(response.rooms.join.get(*DEFAULT_TEST_ROOM_ID).unwrap().ambiguity_changes.is_empty()); - let changes = assert_next_matches!(updates, Ok(RoomUpdate::Joined { ambiguity_changes, .. }) => ambiguity_changes); + let changes = assert_next_matches!(updates, Ok(RoomUpdate::Joined { updates, .. }) => updates.ambiguity_changes); assert!(changes.is_empty()); assert_pending!(updates); From 8cef7a54d9174f7546273b688dc662eb4260fc93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Commaille?= Date: Wed, 31 Jan 2024 18:49:07 +0100 Subject: [PATCH 6/6] Update changelogs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Kévin Commaille --- crates/matrix-sdk-base/CHANGELOG.md | 2 ++ crates/matrix-sdk/CHANGELOG.md | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) 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/CHANGELOG.md b/crates/matrix-sdk/CHANGELOG.md index 563cfaa07f9..28c7167d458 100644 --- a/crates/matrix-sdk/CHANGELOG.md +++ b/crates/matrix-sdk/CHANGELOG.md @@ -5,8 +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` -- `RoomUpdate` also contains the ambiguity changes of a `Room` with `AmbiguityCache` containing the - room member's user ID. +- The ambiguity maps in `SyncResponse` are moved to `JoinedRoom` and `LeftRoom` +- `AmbiguityCache` contains the room member's user ID # 0.7.0