From 8a52d6f2a3113d68f3adaa93ea48749206daf18e Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Tue, 16 Jul 2024 12:45:35 +0200 Subject: [PATCH] notable room updates: have a change to the unread-marker event cause a notable update Also log out why an account data event couldn't be deserialized. --- crates/matrix-sdk-base/src/client.rs | 68 +++++++++----- crates/matrix-sdk-base/src/rooms/normal.rs | 59 ++++++++++-- crates/matrix-sdk-base/src/sliding_sync.rs | 92 ++++++++++++++++++- .../src/room_list_service/room_list.rs | 3 +- 4 files changed, 192 insertions(+), 30 deletions(-) diff --git a/crates/matrix-sdk-base/src/client.rs b/crates/matrix-sdk-base/src/client.rs index fa4e910fe..360df762d 100644 --- a/crates/matrix-sdk-base/src/client.rs +++ b/crates/matrix-sdk-base/src/client.rs @@ -568,6 +568,7 @@ impl BaseClient { room_id: &RoomId, events: &[Raw], changes: &mut StateChanges, + room_info_notable_updates: &mut BTreeMap, ) { // Small helper to make the code easier to read. // @@ -577,9 +578,9 @@ impl BaseClient { room_id: &RoomId, changes: &mut StateChanges, client: &BaseClient, - on_room_info: F, + mut on_room_info: F, ) where - F: Fn(&mut RoomInfo), + F: FnMut(&mut RoomInfo), { // `StateChanges` has the `RoomInfo`. if let Some(room_info) = changes.room_infos.get_mut(room_id) { @@ -601,24 +602,39 @@ impl BaseClient { // Handle new events. for raw_event in events { - if let Ok(event) = raw_event.deserialize() { - changes.add_room_account_data(room_id, event.clone(), raw_event.clone()); + match raw_event.deserialize() { + Ok(event) => { + changes.add_room_account_data(room_id, event.clone(), raw_event.clone()); - match event { - AnyRoomAccountDataEvent::MarkedUnread(event) => { - on_room_info(room_id, changes, self, |room_info| { - room_info.base_info.is_marked_unread = event.content.unread; - }); + match event { + AnyRoomAccountDataEvent::MarkedUnread(event) => { + on_room_info(room_id, changes, self, |room_info| { + if room_info.base_info.is_marked_unread != event.content.unread { + // Notify the room list about a manual read marker change if the + // value's changed. + room_info_notable_updates + .entry(room_id.to_owned()) + .or_default() + .insert(RoomInfoNotableUpdateReasons::UNREAD_MARKER); + } + + room_info.base_info.is_marked_unread = event.content.unread; + }); + } + + AnyRoomAccountDataEvent::Tag(event) => { + on_room_info(room_id, changes, self, |room_info| { + room_info.base_info.handle_notable_tags(&event.content.tags); + }); + } + + // Nothing. + _ => {} } + } - AnyRoomAccountDataEvent::Tag(event) => { - on_room_info(room_id, changes, self, |room_info| { - room_info.base_info.handle_notable_tags(&event.content.tags); - }); - } - - // Nothing. - _ => {} + Err(err) => { + warn!("unable to deserialize account data event: {err}"); } } } @@ -947,8 +963,13 @@ impl BaseClient { // Save the new `RoomInfo`. changes.add_room(room_info); - self.handle_room_account_data(&room_id, &new_info.account_data.events, &mut changes) - .await; + self.handle_room_account_data( + &room_id, + &new_info.account_data.events, + &mut changes, + &mut Default::default(), + ) + .await; // `Self::handle_room_account_data` might have updated the `RoomInfo`. Let's // fetch it again. @@ -1036,8 +1057,13 @@ impl BaseClient { // Save the new `RoomInfo`. changes.add_room(room_info); - self.handle_room_account_data(&room_id, &new_info.account_data.events, &mut changes) - .await; + self.handle_room_account_data( + &room_id, + &new_info.account_data.events, + &mut changes, + &mut Default::default(), + ) + .await; let ambiguity_changes = ambiguity_cache.changes.remove(&room_id).unwrap_or_default(); diff --git a/crates/matrix-sdk-base/src/rooms/normal.rs b/crates/matrix-sdk-base/src/rooms/normal.rs index f47a2d6e0..b0a891b54 100644 --- a/crates/matrix-sdk-base/src/rooms/normal.rs +++ b/crates/matrix-sdk-base/src/rooms/normal.rs @@ -99,6 +99,9 @@ bitflags! { /// A read receipt has changed. const READ_RECEIPT = 0b0000_0100; + + /// The user-controlled unread marker value has changed. + const UNREAD_MARKER = 0b0000_1000; } } @@ -1853,8 +1856,18 @@ mod tests { // When the new tag is handled and applied. let mut changes = StateChanges::default(); - client.handle_room_account_data(room_id, &[tag_raw], &mut changes).await; - client.apply_changes(&changes, Default::default()); + let mut notable_reasons_updates = Default::default(); + client + .handle_room_account_data( + room_id, + &[tag_raw], + &mut changes, + &mut notable_reasons_updates, + ) + .await; + + assert!(notable_reasons_updates.is_empty()); + client.apply_changes(&changes, notable_reasons_updates); // The `RoomInfo` is getting notified. assert_ready!(room_info_subscriber); @@ -1872,7 +1885,18 @@ mod tests { })) .unwrap() .cast(); - client.handle_room_account_data(room_id, &[tag_raw], &mut changes).await; + + let mut notable_reasons_updates = Default::default(); + client + .handle_room_account_data( + room_id, + &[tag_raw], + &mut changes, + &mut notable_reasons_updates, + ) + .await; + + assert!(notable_reasons_updates.is_empty()); client.apply_changes(&changes, Default::default()); // The `RoomInfo` is getting notified. @@ -1927,8 +1951,18 @@ mod tests { // When the new tag is handled and applied. let mut changes = StateChanges::default(); - client.handle_room_account_data(room_id, &[tag_raw], &mut changes).await; - client.apply_changes(&changes, Default::default()); + let mut notable_reasons_updates = Default::default(); + client + .handle_room_account_data( + room_id, + &[tag_raw], + &mut changes, + &mut notable_reasons_updates, + ) + .await; + + assert!(notable_reasons_updates.is_empty()); + client.apply_changes(&changes, notable_reasons_updates); // The `RoomInfo` is getting notified. assert_ready!(room_info_subscriber); @@ -1946,8 +1980,19 @@ mod tests { })) .unwrap() .cast(); - client.handle_room_account_data(room_id, &[tag_raw], &mut changes).await; - client.apply_changes(&changes, Default::default()); + + let mut notable_reasons_updates = Default::default(); + client + .handle_room_account_data( + room_id, + &[tag_raw], + &mut changes, + &mut notable_reasons_updates, + ) + .await; + + assert!(notable_reasons_updates.is_empty()); + client.apply_changes(&changes, notable_reasons_updates); // The `RoomInfo` is getting notified. assert_ready!(room_info_subscriber); diff --git a/crates/matrix-sdk-base/src/sliding_sync.rs b/crates/matrix-sdk-base/src/sliding_sync.rs index 51238a363..4f29c5430 100644 --- a/crates/matrix-sdk-base/src/sliding_sync.rs +++ b/crates/matrix-sdk-base/src/sliding_sync.rs @@ -231,7 +231,13 @@ impl BaseClient { // Handle room account data for (room_id, raw) in &rooms_account_data { - self.handle_room_account_data(room_id, raw, &mut changes).await; + self.handle_room_account_data( + room_id, + raw, + &mut changes, + &mut room_info_notable_updates, + ) + .await; if let Some(room) = self.store.room(room_id) { match room.state() { @@ -1938,6 +1944,90 @@ mod tests { ); } + #[async_test] + async fn test_unread_marker_can_trigger_a_notable_update_reason() { + // Given a logged-in client, + let client = logged_in_base_client(None).await; + let mut room_info_notable_update_stream = client.room_info_notable_update_receiver(); + + // When I receive a sliding sync response containing a new room, + let room_id = room_id!("!r:e.uk"); + let room = v4::SlidingSyncRoom::new(); + let response = response_with_room(room_id, room); + client.process_sliding_sync(&response, &()).await.expect("Failed to process sync"); + + // Then a room info notable update is NOT received. + assert_matches!( + room_info_notable_update_stream.recv().await, + Ok(RoomInfoNotableUpdate { room_id: received_room_id, reasons: received_reasons }) => { + assert_eq!(received_room_id, room_id); + assert!(!received_reasons.contains(RoomInfoNotableUpdateReasons::UNREAD_MARKER)); + } + ); + + // When I receive a sliding sync response containing one update about an unread + // marker, + let room_id = room_id!("!r:e.uk"); + let room_account_data_events = vec![Raw::from_json_string( + json!({ + "type": "com.famedly.marked_unread", + "event_id": "$1", + "content": { "unread": true }, + "sender": client.session_meta().unwrap().user_id, + "origin_server_ts": 12344445, + }) + .to_string(), + ) + .unwrap()]; + let mut response = response_with_room(room_id, v4::SlidingSyncRoom::new()); + response.extensions.account_data.rooms.insert(room_id.to_owned(), room_account_data_events); + + client.process_sliding_sync(&response, &()).await.expect("Failed to process sync"); + + // Then a room info notable update is received. + assert_matches!( + room_info_notable_update_stream.recv().await, + Ok(RoomInfoNotableUpdate { room_id: received_room_id, reasons: received_reasons }) => { + assert_eq!(received_room_id, room_id); + assert!(received_reasons.contains(RoomInfoNotableUpdateReasons::UNREAD_MARKER), "{received_reasons:?}"); + } + ); + + // But getting it again won't trigger a new notable update… + client.process_sliding_sync(&response, &()).await.expect("Failed to process sync"); + + assert_matches!( + room_info_notable_update_stream.recv().await, + Ok(RoomInfoNotableUpdate { room_id: received_room_id, reasons: received_reasons }) => { + assert_eq!(received_room_id, room_id); + assert!(!received_reasons.contains(RoomInfoNotableUpdateReasons::UNREAD_MARKER)); + } + ); + + // …Unless its value changes! + let room_account_data_events = vec![Raw::from_json_string( + json!({ + "type": "com.famedly.marked_unread", + "event_id": "$1", + "content": { "unread": false }, + "sender": client.session_meta().unwrap().user_id, + "origin_server_ts": 12344445, + }) + .to_string(), + ) + .unwrap()]; + response.extensions.account_data.rooms.insert(room_id.to_owned(), room_account_data_events); + client.process_sliding_sync(&response, &()).await.expect("Failed to process sync"); + + assert_matches!( + room_info_notable_update_stream.recv().await, + Ok(RoomInfoNotableUpdate { room_id: received_room_id, reasons: received_reasons }) => { + assert_eq!(received_room_id, room_id); + assert!(received_reasons.contains(RoomInfoNotableUpdateReasons::UNREAD_MARKER)); + } + ); + } + async fn choose_event_to_cache(events: &[SyncTimelineEvent]) -> Option { let room = make_room(); let mut room_info = room.clone_info(); diff --git a/crates/matrix-sdk-ui/src/room_list_service/room_list.rs b/crates/matrix-sdk-ui/src/room_list_service/room_list.rs index bd21b8387..a3245231d 100644 --- a/crates/matrix-sdk-ui/src/room_list_service/room_list.rs +++ b/crates/matrix-sdk-ui/src/room_list_service/room_list.rs @@ -219,7 +219,8 @@ fn merge_stream_and_receiver( // We are interested by these _reasons_. if reasons.contains(NotableUpdate::LATEST_EVENT) || reasons.contains(NotableUpdate::RECENCY_TIMESTAMP) || - reasons.contains(NotableUpdate::READ_RECEIPT) { + reasons.contains(NotableUpdate::READ_RECEIPT) || + reasons.contains(NotableUpdate::UNREAD_MARKER) { // Emit a `VectorDiff::Set` for the specific rooms. if let Some(index) = raw_current_values.iter().position(|room| room.room_id() == update.room_id) { let update = VectorDiff::Set { index, value: raw_current_values[index].clone() };