Merge pull request #6513 from nextcloud/bugfix/noid/fixPlausibleLastReadMessageGuard

fix(chat): treat a newest known message id of 0 as unknown

Author: mahibi

app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt

@@ -2426,8 +2426,16 @@ class ChatViewModel @AssistedInject constructor(
 
         private const val PLAUSIBLE_MESSAGE_ID_BUFFER = 10_000L
 
-        fun isPlausibleLastReadMessageId(messageId: Int, newestKnownRealMessageId: Long?): Boolean =
-            newestKnownRealMessageId == null || messageId <= newestKnownRealMessageId + PLAUSIBLE_MESSAGE_ID_BUFFER
+        /**
+         * A real server-assigned message id is always positive, so a null, zero or negative
+         * [newestKnownRealMessageId] is never a trustworthy ceiling to judge [messageId] against -
+         * e.g. a federated conversation's cached lastMessage can carry an id of 0 when it was
+         * never populated. Without a trustworthy ceiling, [messageId] is accepted unchecked here.
+         */
+        fun isPlausibleLastReadMessageId(messageId: Int, newestKnownRealMessageId: Long?): Boolean {
+            val trustworthyCeiling = newestKnownRealMessageId?.takeIf { it > 0 } ?: return true
+            return messageId <= trustworthyCeiling + PLAUSIBLE_MESSAGE_ID_BUFFER
+        }
     }
 
     sealed class OutOfOfficeUIState {

app/src/test/java/com/nextcloud/talk/chat/viewmodels/ChatViewModelTest.kt

@@ -61,6 +61,13 @@ class ChatViewModelTest {
         )
     }
 
+    @Test
+    fun `isPlausibleLastReadMessageId treats a newest known id of 0 as unknown`() {
+        // A federated conversation's cached lastMessage can carry an id of 0 - a real message id
+        // is never 0, so this must not be treated as a real ceiling near the start of the room.
+        assertTrue(ChatViewModel.isPlausibleLastReadMessageId(messageId = 136556, newestKnownRealMessageId = 0L))
+    }
+
     // The unread marker latch: the marker position must only be derived from the visible window
     // when the window provably reaches back to the unread boundary — otherwise a window of
     // only-unread messages (e.g. after a capped fetch of the newest messages) would place the