From 4d2f8bb82cd3811e0fa6b54218a5bf55d11f5f8d Mon Sep 17 00:00:00 2001 From: JoFrost <20685007+JoFrost@users.noreply.github.com> Date: Mon, 30 Mar 2026 11:59:43 +0200 Subject: [PATCH] feat(base): improve profile updates for left/banned users (#6097) This avoids profile updates when a user decides to leave on their own (in which case their profile is kept as is), and when a user is banned (in which case their profile is removed). Signed-off-by: JoFrost <20685007+JoFrost@users.noreply.github.com> --- .../src/response_processors/profiles.rs | 16 +- .../tests/integration/timeline/profiles.rs | 167 +++++++++++++++++- 2 files changed, 179 insertions(+), 4 deletions(-) diff --git a/crates/matrix-sdk-base/src/response_processors/profiles.rs b/crates/matrix-sdk-base/src/response_processors/profiles.rs index 1ae1548b3..5f9981564 100644 --- a/crates/matrix-sdk-base/src/response_processors/profiles.rs +++ b/crates/matrix-sdk-base/src/response_processors/profiles.rs @@ -32,7 +32,10 @@ pub fn upsert_or_delete( // Senders can fake the profile easily so we keep track of profiles that the // member set themselves to avoid having confusing profile changes when a // member gets kicked/banned. - if event.state_key() == event.sender() { + // We don't want to update the profile if the member is leaving the room, as the + // server may return a dummy empty profile along the leave event. We want to + // keep the last known profile in that case. + if event.state_key() == event.sender() && *event.membership() != MembershipState::Leave { context .state_changes .profiles @@ -41,8 +44,8 @@ pub fn upsert_or_delete( .insert(event.sender().to_owned(), event.into()); } - if *event.membership() == MembershipState::Invite { - // Remove any profile previously stored for the invited user. + if matches!(*event.membership(), MembershipState::Invite | MembershipState::Ban) { + // Remove any profile previously stored for the invited/banned user. // // A room member could have joined the room and left it later; in that case, the // server may return a dummy, empty profile along the `leave` event. We @@ -54,5 +57,12 @@ pub fn upsert_or_delete( .entry(room_id.to_owned()) .or_default() .push(event.state_key().clone()); + + // Address the edge case of "in-flight" profiles. If an earlier event in + // this same sync batch inserted a profile (for example user joined then got + // banned by an admin of a room), we MUST NOT reinsert it. + if let Some(room_profiles) = context.state_changes.profiles.get_mut(room_id) { + room_profiles.remove(event.state_key()); + } } } diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/profiles.rs b/crates/matrix-sdk-ui/tests/integration/timeline/profiles.rs index 5a1b7ef56..4ef67d4eb 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/profiles.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/profiles.rs @@ -22,7 +22,8 @@ use matrix_sdk::{ use matrix_sdk_common::executor::spawn; use matrix_sdk_test::{ ALICE, BOB, CAROL, DEFAULT_TEST_ROOM_ID, JoinedRoomBuilder, SyncResponseBuilder, async_test, - event_factory::EventFactory, mocks::mock_encryption_state, + event_factory::{EventFactory, PreviousMembership}, + mocks::mock_encryption_state, }; use matrix_sdk_ui::timeline::{RoomExt, TimelineDetails}; use ruma::events::room::member::MembershipState; @@ -34,6 +35,170 @@ use wiremock::{ use crate::mock_sync; +#[async_test] +async fn test_user_profile_after_being_banned() { + let (client, server) = logged_in_client_with_server().await; + let sync_settings = + SyncSettings::new().timeout(Duration::from_millis(3000)).token(SyncToken::NoToken); + + let f = EventFactory::new(); + let mut sync_builder = SyncResponseBuilder::new(); + sync_builder.add_joined_room(JoinedRoomBuilder::new(&DEFAULT_TEST_ROOM_ID)); + + mock_sync(&server, sync_builder.build_json_sync_response(), None).await; + let _response = client.sync_once(sync_settings.clone()).await.unwrap(); + server.reset().await; + + mock_encryption_state(&server, false).await; + + let room = client.get_room(&DEFAULT_TEST_ROOM_ID).unwrap(); + let timeline = Arc::new(room.timeline().await.unwrap()); + + // Build a simple timeline with Bob joining the room, Alice accepting an invite, + // sending some messages, and then getting banned by Bob. Alice's profile should + // be unavailable after the ban, while Bob's profile should be unaffected. + sync_builder.add_joined_room( + JoinedRoomBuilder::new(&DEFAULT_TEST_ROOM_ID) + // Bob joins the room + .add_timeline_event( + f.member(&BOB) + .display_name("Bob") + .avatar_url("mxc://123".into()) + .membership(MembershipState::Join), + ) + // Alice accepts an invite into the room + .add_timeline_event( + f.member(&ALICE) + .display_name("*profanities*") + .avatar_url("mxc://456".into()) + .membership(MembershipState::Join) + .previous(MembershipState::Invite), + ) + // Bob sends a text message + .add_timeline_event(f.text_msg("hey!").sender(&BOB)) + // Alice send some insults and gets banned + .add_timeline_event(f.text_msg("*insults*").sender(&ALICE)) + .add_timeline_event(f.text_msg("Nope. You are out.").sender(&BOB)) + .add_timeline_event( + f.member(&ALICE).membership(MembershipState::Ban).sender(&BOB).previous( + PreviousMembership::new(MembershipState::Join) + .display_name("*profanities*") + .avatar_url("mxc://456".into()), + ), + ), + ); + + mock_sync(&server, sync_builder.build_json_sync_response(), None).await; + let _response = client.sync_once(sync_settings.clone()).await.unwrap(); + + let timeline_items = timeline.items().await; + // Date divider + 6 events + assert_eq!(timeline_items.len(), 7); + + // Alice's profile, and therefore display name, should be empty, + // as she has been banned. The SDK **should** return an empty + // profile for banned users. + // We are only checking Alice's profile here, as we check Bob's profile in a + // separate test + let alice_event = timeline_items[4].as_event().unwrap(); + let alice_profile = alice_event.sender_profile(); + // Was this event sent by Alice? + assert_eq!(alice_event.sender(), *ALICE); + match alice_profile { + TimelineDetails::Ready(profile) => { + assert_eq!(profile.display_name, None); + assert_eq!(profile.avatar_url, None); + } + _ => panic!("Expected Alice's profile to be ready"), + } +} + +#[async_test] +async fn test_user_profile_after_leaving() { + let (client, server) = logged_in_client_with_server().await; + let sync_settings = + SyncSettings::new().timeout(Duration::from_millis(3000)).token(SyncToken::NoToken); + + let f = EventFactory::new(); + let mut sync_builder = SyncResponseBuilder::new(); + sync_builder.add_joined_room(JoinedRoomBuilder::new(&DEFAULT_TEST_ROOM_ID)); + + mock_sync(&server, sync_builder.build_json_sync_response(), None).await; + let _response = client.sync_once(sync_settings.clone()).await.unwrap(); + server.reset().await; + + mock_encryption_state(&server, false).await; + + let room = client.get_room(&DEFAULT_TEST_ROOM_ID).unwrap(); + let timeline = Arc::new(room.timeline().await.unwrap()); + + // Build a simple timeline with Bob joining the room, Alice accepting an invite + // and sending a message, Bob sending a message, and Alice leaving + sync_builder.add_joined_room( + JoinedRoomBuilder::new(&DEFAULT_TEST_ROOM_ID) + // Bob joins the room + .add_timeline_event( + f.member(&BOB) + .display_name("Bob") + .avatar_url("mxc://...".into()) + .membership(MembershipState::Join), + ) + // Alice accepts an invite into the room + .add_timeline_event( + f.member(&ALICE) + .display_name("Alice") + .avatar_url("mxc://...".into()) + .membership(MembershipState::Join) + .previous(MembershipState::Invite), + ) + // Bob sends a text message + .add_timeline_event(f.text_msg("hey!").sender(&BOB)) + // Alice send a message and leaves the room + .add_timeline_event(f.text_msg("bye now").sender(&ALICE)) + .add_timeline_event( + f.member(&ALICE).membership(MembershipState::Leave).previous( + PreviousMembership::new(MembershipState::Join) + .display_name("Alice") + .avatar_url("mxc://...".into()), + ), + ), + ); + + mock_sync(&server, sync_builder.build_json_sync_response(), None).await; + let _response = client.sync_once(sync_settings.clone()).await.unwrap(); + + let timeline_items = timeline.items().await; + // Date divider + 5 events + assert_eq!(timeline_items.len(), 6); + + // Alice's profile, and therefore display name, should be still available after + // she left the room. + let alice_event = timeline_items[4].as_event().unwrap(); + let alice_profile = alice_event.sender_profile(); + // Was this event sent by Alice? + assert_eq!(alice_event.sender(), *ALICE); + match alice_profile { + TimelineDetails::Ready(profile) => { + assert_eq!(profile.display_name, Some("Alice".to_owned())); + assert_eq!(profile.avatar_url, Some("mxc://...".into())); + } + _ => panic!("Expected Alice's profile to be ready"), + } + + // Bob's profile should also be available + let bob_event = timeline_items[3].as_event().unwrap(); + let bob_profile = bob_event.sender_profile(); + // Was this event sent by Bob? + assert_eq!(bob_event.sender(), *BOB); + match bob_profile { + TimelineDetails::Ready(profile) => { + assert_eq!(profile.display_name, Some("Bob".to_owned())); + assert_eq!(profile.avatar_url, Some("mxc://...".into())); + } + _ => panic!("Expected Bob's profile to be ready"), + } +} + #[async_test] async fn test_update_sender_profiles() { let (client, server) = logged_in_client_with_server().await;