refactor(sdk): Make Room::set_unread_flag() a no-op if the unread flag already has the wanted value

Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>

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)
         }

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
 

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);

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]