Merge pull request #6571 from nextcloud/revertIsPlausibleLastReadMessageChecks

revert isPlausibleLastReadMessageId check

Author: AndyScherzinger

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

@@ -1845,17 +1845,6 @@ class ChatViewModel @AssistedInject constructor(
         Log.d(TAG, "advanceLocalLastReadMessageIfNeeded, messageId: $messageId")
         Log.d(TAG, "advanceLocalLastReadMessageIfNeeded, localLastReadMessage: $localLastReadMessage")
 
-        val newestKnownRealMessageId = _uiState.value.conversation?.lastMessage?.id
-
-        if (!isPlausibleLastReadMessageId(messageId, newestKnownRealMessageId)) {
-            logger.w(
-                TAG,
-                "advanceLocalLastReadMessageIfNeeded, messageId ($messageId) is implausibly higher than the " +
-                    "conversation's newest known message id ($newestKnownRealMessageId). We won't advance."
-            )
-            return
-        }
-
         if (localLastReadMessage < messageId && -1 < messageId) {
             Log.d(TAG, "advanceLocalLastReadMessageIfNeeded, setting localLastReadMessage to $messageId")
             localLastReadMessage = messageId
@@ -2487,18 +2476,6 @@ class ChatViewModel @AssistedInject constructor(
         // cycle (up to 2 minutes later) before its "sent, not yet confirmed" spinner clears.
         private val POST_UPLOAD_FETCH_RETRY_DELAYS_MS = listOf(1_000L, 1_500L, 2_000L, 3_000L, 4_000L, 5_000L, 5_000L)
         private const val LOCAL_PREVIEW_GRACE_PERIOD_MS = 15_000L
-        private const val PLAUSIBLE_MESSAGE_ID_BUFFER = 10_000L
-
-        /**
-         * 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

@@ -11,63 +11,12 @@ import com.nextcloud.talk.chat.ui.model.ChatMessageUi
 import com.nextcloud.talk.chat.ui.model.MessageStatusIcon
 import com.nextcloud.talk.chat.ui.model.MessageTypeContent
 import org.junit.Assert.assertEquals
-import org.junit.Assert.assertFalse
 import org.junit.Assert.assertNull
-import org.junit.Assert.assertTrue
 import org.junit.Test
 import java.time.LocalDate
 
 class ChatViewModelTest {
 
-    @Test
-    fun `isPlausibleLastReadMessageId returns true when there is no known real message id yet`() {
-        assertTrue(ChatViewModel.isPlausibleLastReadMessageId(messageId = 158761, newestKnownRealMessageId = null))
-    }
-
-    @Test
-    fun `isPlausibleLastReadMessageId returns true for the newest known real message id itself`() {
-        assertTrue(ChatViewModel.isPlausibleLastReadMessageId(messageId = 158761, newestKnownRealMessageId = 158761L))
-    }
-
-    @Test
-    fun `isPlausibleLastReadMessageId returns true for an older message id`() {
-        assertTrue(ChatViewModel.isPlausibleLastReadMessageId(messageId = 100, newestKnownRealMessageId = 158761L))
-    }
-
-    @Test
-    fun `isPlausibleLastReadMessageId returns true within the buffer above the newest known message id`() {
-        assertTrue(
-            ChatViewModel.isPlausibleLastReadMessageId(
-                messageId = 158761 + 10_000,
-                newestKnownRealMessageId = 158761L
-            )
-        )
-    }
-
-    @Test
-    fun `isPlausibleLastReadMessageId returns false just beyond the buffer above the newest known message id`() {
-        assertFalse(
-            ChatViewModel.isPlausibleLastReadMessageId(
-                messageId = 158761 + 10_000 + 1,
-                newestKnownRealMessageId = 158761L
-            )
-        )
-    }
-
-    @Test
-    fun `isPlausibleLastReadMessageId rejects a hash-derived placeholder id far beyond the real message id space`() {
-        assertFalse(
-            ChatViewModel.isPlausibleLastReadMessageId(messageId = 1_963_726_147, newestKnownRealMessageId = 158761L)
-        )
-    }
-
-    @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