From a8dcea6931367b855f34baeabb4ae66cf2d59278 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Commaille?= Date: Tue, 20 May 2025 09:30:30 +0200 Subject: [PATCH] refactor(sdk): Make Room::set_unread_flag() a no-op if the unread flag already has the wanted value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Kévin Commaille --- crates/matrix-sdk-ui/src/timeline/mod.rs | 10 ++-- crates/matrix-sdk/CHANGELOG.md | 2 + crates/matrix-sdk/src/room/mod.rs | 17 ++++--- .../tests/integration/room/joined.rs | 47 +++++++++++++++---- 4 files changed, 55 insertions(+), 21 deletions(-) diff --git a/crates/matrix-sdk-ui/src/timeline/mod.rs b/crates/matrix-sdk-ui/src/timeline/mod.rs index de725ab5e..bb8b3feee 100644 --- a/crates/matrix-sdk-ui/src/timeline/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/mod.rs @@ -545,7 +545,7 @@ impl Timeline { "not sending receipt, because we already cover the event with a previous receipt" ); - if thread == ReceiptThread::Unthreaded && self.room().is_marked_unread() { + if thread == ReceiptThread::Unthreaded { // Unset the read marker. self.room().set_unread_flag(false).await?; } @@ -608,7 +608,7 @@ impl Timeline { if !receipts.is_empty() { self.room().send_multiple_receipts(receipts).await?; - } else if self.room().is_marked_unread() { + } else { self.room().set_unread_flag(false).await?; } @@ -632,10 +632,8 @@ impl Timeline { } else { trace!("can't mark room as read because there's no latest event id"); - if self.room().is_marked_unread() { - // Unset the read marker. - self.room().set_unread_flag(false).await?; - } + // Unset the read marker. + self.room().set_unread_flag(false).await?; Ok(false) } diff --git a/crates/matrix-sdk/CHANGELOG.md b/crates/matrix-sdk/CHANGELOG.md index 542656d37..ba791d109 100644 --- a/crates/matrix-sdk/CHANGELOG.md +++ b/crates/matrix-sdk/CHANGELOG.md @@ -53,6 +53,8 @@ All notable changes to this project will be documented in this file. `get_all_rooms`, `get_rooms`, `get_number_of_rooms`, and `FrozenSlidingSync` methods and type have been removed. ([#5047](https://github.com/matrix-org/matrix-rust-sdk/pull/5047)) +- `Room::set_unread_flag()` is now a no-op if the unread flag already has the wanted value. + ([#5055](https://github.com/matrix-org/matrix-rust-sdk/pull/5055)) ## [0.11.0] - 2025-04-11 diff --git a/crates/matrix-sdk/src/room/mod.rs b/crates/matrix-sdk/src/room/mod.rs index 5b0fb4578..c887ab233 100644 --- a/crates/matrix-sdk/src/room/mod.rs +++ b/crates/matrix-sdk/src/room/mod.rs @@ -1626,8 +1626,7 @@ impl Room { .read_receipt_deduplicated_handler .run((request_key, event_id.clone()), async { // We will unset the unread flag if we send an unthreaded receipt. - let unset_unread_flag = - thread == ReceiptThread::Unthreaded && self.is_marked_unread(); + let is_unthreaded = thread == ReceiptThread::Unthreaded; let mut request = create_receipt::v3::Request::new( self.room_id().to_owned(), @@ -1638,7 +1637,7 @@ impl Room { self.client.send(request).await?; - if unset_unread_flag { + if is_unthreaded { self.set_unread_flag(false).await?; } @@ -1671,9 +1670,7 @@ impl Room { self.client.send(request).await?; - if self.is_marked_unread() { - self.set_unread_flag(false).await?; - } + self.set_unread_flag(false).await?; Ok(()) } @@ -3165,7 +3162,15 @@ impl Room { /// Set a flag on the room to indicate that the user has explicitly marked /// it as (un)read. + /// + /// This is a no-op if [`Room::is_marked_unread()`] returns the same value + /// as `unread`. pub async fn set_unread_flag(&self, unread: bool) -> Result<()> { + if self.is_marked_unread() == unread { + // The request is not necessary. + return Ok(()); + } + let user_id = self.client.user_id().ok_or(Error::AuthenticationRequired)?; let content = MarkedUnreadEventContent::new(unread); diff --git a/crates/matrix-sdk/tests/integration/room/joined.rs b/crates/matrix-sdk/tests/integration/room/joined.rs index 97b9457c0..f14a0cb08 100644 --- a/crates/matrix-sdk/tests/integration/room/joined.rs +++ b/crates/matrix-sdk/tests/integration/room/joined.rs @@ -176,20 +176,49 @@ async fn test_unban_user() { async fn test_mark_as_unread() { let server = MatrixMockServer::new().await; let client = server.client_builder().build().await; - - server - .mock_set_room_account_data(RoomAccountDataEventType::MarkedUnread) - .ok() - .expect(2) - .mount() - .await; + let room_id = room_id!("!test:example.org"); // Initial sync with our test room. - let room = server.sync_joined_room(&client, room_id!("!test:example.org")).await; + let room = server.sync_joined_room(&client, room_id).await; + assert!(!room.is_marked_unread()); - room.set_unread_flag(true).await.unwrap(); + // Setting the room as unread makes a request. + { + let _guard = server + .mock_set_room_account_data(RoomAccountDataEventType::MarkedUnread) + .ok() + .expect(1) + .mount_as_scoped() + .await; + room.set_unread_flag(true).await.unwrap(); + } + // Unsetting the room as unread is a no-op. room.set_unread_flag(false).await.unwrap(); + + // Now we mark the room as unread. + server + .sync_room( + &client, + JoinedRoomBuilder::new(room_id) + .add_account_data(RoomAccountDataTestEvent::MarkedUnread), + ) + .await; + assert!(room.is_marked_unread()); + + // Unsetting the room as unread makes a request. + { + let _guard = server + .mock_set_room_account_data(RoomAccountDataEventType::MarkedUnread) + .ok() + .expect(1) + .mount_as_scoped() + .await; + room.set_unread_flag(false).await.unwrap(); + } + + // Setting the room as unread is a no-op. + room.set_unread_flag(true).await.unwrap(); } #[async_test]