diff --git a/.github/workflows/e2e-test-cron.yml b/.github/workflows/e2e-test-cron.yml index 206393464f5..077be3e252a 100644 --- a/.github/workflows/e2e-test-cron.yml +++ b/.github/workflows/e2e-test-cron.yml @@ -94,7 +94,12 @@ jobs: if: failure() with: name: logs_${{ env.ANDROID_API_LEVEL }} - path: fastlane/stream-chat-test-mock-server/logs/* + # The allure results carry the retry rule's failure artifacts (logcat, + # screenshot, hierarchy per failed attempt), which TestOps deduplicates + # away when the same test passes on another API level. + path: | + fastlane/stream-chat-test-mock-server/logs/* + fastlane/allure-results/* slack: permissions: actions: write diff --git a/.github/workflows/e2e-test.yml b/.github/workflows/e2e-test.yml index 0425407808d..ecc34c1f79f 100644 --- a/.github/workflows/e2e-test.yml +++ b/.github/workflows/e2e-test.yml @@ -11,6 +11,15 @@ on: type: string required: true default: main + api_level: + description: 'Android API level to run on' + type: string + required: false + default: '36' + test_class: + description: 'Fully qualified test class to run on its own. Every batch job then runs it, giving independent samples of a flaky test.' + type: string + required: false concurrency: group: ${{ github.workflow }}-${{ github.ref }} @@ -70,7 +79,7 @@ jobs: - batch: 2 fail-fast: false env: - ANDROID_API_LEVEL: 36 + ANDROID_API_LEVEL: ${{ inputs.api_level || '36' }} LAUNCH_ID: ${{ needs.allure_testops_launch.outputs.launch_id }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 @@ -90,7 +99,7 @@ jobs: profile: pixel arch : x86_64 emulator-options: -no-snapshot-save -no-window -no-audio -no-boot-anim -gpu swiftshader_indirect -camera-back none -camera-front none - script: bundle exec fastlane run_e2e_test batch:${{ matrix.batch }} batch_count:${{ strategy.job-total }} mock_server_branch:${{ inputs.mock_server_branch }} + script: bundle exec fastlane run_e2e_test batch:${{ matrix.batch }} batch_count:${{ strategy.job-total }} mock_server_branch:${{ inputs.mock_server_branch }} ${{ inputs.test_class && format('test_class:{0}', inputs.test_class) || '' }} - name: Allure TestOps Upload if: ${{ env.LAUNCH_ID != '' && (success() || failure()) }} run: bundle exec fastlane allure_upload @@ -109,4 +118,9 @@ jobs: timeout-minutes: 10 with: name: logs_${{ matrix.batch }} - path: fastlane/stream-chat-test-mock-server/logs/* + # The allure results carry the retry rule's failure artifacts (logcat, + # screenshot, hierarchy per failed attempt), which TestOps deduplicates + # away when the same test passes in another batch. + path: | + fastlane/stream-chat-test-mock-server/logs/* + fastlane/allure-results/* diff --git a/fastlane/Fastfile b/fastlane/Fastfile index cbd61f60b70..355874b6fca 100644 --- a/fastlane/Fastfile +++ b/fastlane/Fastfile @@ -107,8 +107,18 @@ lane :run_e2e_test do |options| backend_test_class = 'io.getstream.chat.android.compose.tests.BackendTests' + targeted_class = options[:test_class].to_s + # The value is interpolated into a shell command, so only a class name is accepted, + # optionally with a single method. A typo also fails here instead of in the shell. + UI.user_error!("Invalid test_class: #{targeted_class}") unless + targeted_class.empty? || targeted_class.match?(/\A[\w.$]+(#\w+)?\z/) + if options[:use_backend] test_filter = "-e class #{backend_test_class}" + elsif !targeted_class.empty? + # Targeted run: a single class and no batching, so every batch job runs it and gives an + # independent sample. Used to reproduce a failure that only shows on one API level. + test_filter = "-e class #{targeted_class}" else run_tests_in_batches = batch_tests( batch: options[:batch], diff --git a/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/channel/internal/ChannelEventHandlerImpl.kt b/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/channel/internal/ChannelEventHandlerImpl.kt index 20bcfbbcc40..1f9740cebd3 100644 --- a/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/channel/internal/ChannelEventHandlerImpl.kt +++ b/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/channel/internal/ChannelEventHandlerImpl.kt @@ -340,7 +340,7 @@ internal class ChannelEventHandlerImpl( } } - is NotificationMarkUnreadEvent -> state.updateRead(event.toChannelUserRead()) + is NotificationMarkUnreadEvent -> state.replaceRead(event.toChannelUserRead()) is MessageDeliveredEvent -> state.updateDelivered(event.toChannelUserRead()) // Invitation events is NotificationInviteAcceptedEvent -> { diff --git a/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/channel/internal/legacy/ChannelStateLogic.kt b/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/channel/internal/legacy/ChannelStateLogic.kt index ffe0aaf7b0b..822a2277203 100644 --- a/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/channel/internal/legacy/ChannelStateLogic.kt +++ b/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/channel/internal/legacy/ChannelStateLogic.kt @@ -25,6 +25,7 @@ import io.getstream.chat.android.client.events.HasChannel import io.getstream.chat.android.client.events.TypingStartEvent import io.getstream.chat.android.client.events.UserStartWatchingEvent import io.getstream.chat.android.client.events.UserStopWatchingEvent +import io.getstream.chat.android.client.extensions.getCreatedAtOrDefault import io.getstream.chat.android.client.extensions.internal.NEVER import io.getstream.chat.android.client.internal.state.message.attachments.internal.AttachmentUrlValidator import io.getstream.chat.android.client.internal.state.plugin.logic.channel.internal.SearchLogic @@ -925,17 +926,22 @@ internal class ChannelStateLogic( processedMessageIds.put(message.id, true) return } - // Skip update if the event is outdated + // Skip update for messages the user has already read. The read dates only advance + // with server-confirmed values, so a replayed event for a message older than the + // last read must not count again. val currentRead = mutableState.read.value - if (currentRead != null && currentRead.lastReceivedEventDate.after(eventReceivedDate)) { + if (currentRead != null && !message.getCreatedAtOrDefault(Date()).after(currentRead.lastRead)) { processedMessageIds.put(message.id, true) return } - // Update the unread count + // Update the unread count. Replays are excluded by processedMessageIds and the + // last read date; the event clock must not gate the count, because a queued event + // can legitimately carry an older date than an already processed one. The clock + // only moves forward, so an out of order event cannot regress it. currentRead?.let { updateRead( it.copy( - lastReceivedEventDate = eventReceivedDate, + lastReceivedEventDate = maxOf(it.lastReceivedEventDate, eventReceivedDate), unreadMessages = it.unreadMessages.inc(), ), ) diff --git a/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateImpl.kt b/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateImpl.kt index bf8f608eef6..7c479eb0045 100644 --- a/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateImpl.kt +++ b/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateImpl.kt @@ -63,6 +63,7 @@ import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.update import java.util.Date import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.atomic.AtomicInteger import kotlin.math.max /** @@ -167,7 +168,8 @@ internal class ChannelStateImpl( pending.filter { pagination.isInWindow(it) } } - private val logger by taggedLogger("ChannelStateImpl") + private val seq = seqGenerator.incrementAndGet() + private val logger by taggedLogger("ChannelStateImpl-$seq") override val repliedMessage: StateFlow = _repliedMessage.asStateFlow() @@ -1054,9 +1056,28 @@ internal class ChannelStateImpl( } else { reads } + val before = read.value?.unreadMessages _reads.update { current -> current + readsToUpsert.associateBy(ChannelUserRead::getUserId) } + val after = read.value?.unreadMessages + if (before != after) { + val incoming = reads.firstOrNull { it.getUserId() == currentUser.value?.id }?.unreadMessages + logger.d { "[updateReads] cid: $cid; unreadMessages: $before -> $after; incoming: $incoming" } + } + } + + /** + * Replaces the read state of [read]'s user with no arbitration against the local one. + * + * For read states the server reports explicitly, where the reported position and count are + * the truth even when the position moves backwards, as a mark unread does. [updateReads] + * cannot be used for those, because it protects the local count against stale payloads. + */ + fun replaceRead(read: ChannelUserRead) { + val previous = _reads.value[read.getUserId()]?.unreadMessages + logger.d { "[replaceRead] cid: $cid; unreadMessages: $previous -> ${read.unreadMessages}" } + _reads.update { current -> current + (read.getUserId() to read) } } /** @@ -1067,63 +1088,62 @@ internal class ChannelStateImpl( * @param message The new message that triggered the event. */ fun updateCurrentUserRead(eventReceivedDate: Date, message: Message) { - // Skip update if the message was already processed - val isProcessed = processedMessageIds[message.id] == true - if (isProcessed) { - return - } - // Skip update if the channel is muted - val isMuted = muted.value - if (isMuted) { - processedMessageIds.put(message.id, true) - return - } - // Skip update for thread replies not shown in channel - val isThreadReplyNotInChannel = message.parentId != null && !message.showInChannel - if (isThreadReplyNotInChannel) { - processedMessageIds.put(message.id, true) - return - } - // Skip update for messages from current user - val isFromCurrentUser = message.user.id == currentUser.value?.id - if (isFromCurrentUser) { - processedMessageIds.put(message.id, true) - return - } - // Skip update for messages from muted users - val isFromMutedUser = mutedUsers.value.any { it.target?.id == message.user.id } - if (isFromMutedUser) { - processedMessageIds.put(message.id, true) + if (processedMessageIds[message.id] == true) { + logger.d { "[updateCurrentUserRead] skipped msgId(${message.id}): already processed" } return } - // Skip update for messages from shadow banned users - if (message.shadowed) { + val currentRead = read.value + val skipReason = unreadCountSkipReason(currentRead, message) + if (skipReason != null) { + logger.d { "[updateCurrentUserRead] skipped msgId(${message.id}): $skipReason" } processedMessageIds.put(message.id, true) return } - // Skip update for silent messages - if (message.silent) { + if (currentRead == null) { + logger.d { "[updateCurrentUserRead] skipped msgId(${message.id}): no read state for the current user" } processedMessageIds.put(message.id, true) return } - // Skip update if the event is outdated - val currentRead = read.value - if (currentRead != null && currentRead.lastReceivedEventDate.after(eventReceivedDate)) { - processedMessageIds.put(message.id, true) - return + // Written straight into the map instead of through updateRead: this is the local count, + // not an incoming read, so it must not pass through the merge that arbitrates external + // reads, and the read-modify-write has to be atomic to survive a burst of events. The + // clock only moves forward, so an out of order event cannot regress it. + val userId = currentRead.getUserId() + _reads.update { current -> + val existing = current[userId] ?: currentRead + current + ( + userId to existing.copy( + lastReceivedEventDate = maxOf(existing.lastReceivedEventDate, eventReceivedDate), + unreadMessages = existing.unreadMessages.inc(), + ) + ) } - // Update the unread count - currentRead?.let { - updateRead( - it.copy( - lastReceivedEventDate = eventReceivedDate, - unreadMessages = it.unreadMessages.inc(), - ), - ) + logger.d { + "[updateCurrentUserRead] counted msgId(${message.id}); unreadMessages: ${read.value?.unreadMessages}" } processedMessageIds.put(message.id, true) } + /** + * Why [message] does not increment the current user's unread count, or `null` when it does. + * + * The returned reason is logged, so a wrong count can be traced to the exact condition that + * suppressed it. + */ + private fun unreadCountSkipReason(currentRead: ChannelUserRead?, message: Message): String? = when { + muted.value -> "channel is muted" + message.parentId != null && !message.showInChannel -> "thread reply not shown in channel" + message.user.id == currentUser.value?.id -> "own message" + mutedUsers.value.any { it.target?.id == message.user.id } -> "author is muted" + message.shadowed -> "message is shadowed" + message.silent -> "message is silent" + // The read dates only advance with server-confirmed values, so a replayed event for a + // message older than the last read must not count again. + currentRead != null && !message.getCreatedAtOrDefault(Date()).after(currentRead.lastRead) -> + "already read: createdAt(${message.createdAt}) <= lastRead(${currentRead.lastRead})" + else -> null + } + /** * Marks the channel as read for the current user if the following conditions are met: * 1. Read events are enabled in the channel configuration. @@ -1149,12 +1169,11 @@ internal class ChannelStateImpl( return true } return if (lastMessage.id != currentUserRead.lastReadMessageId) { - // The last message is different from the last read message, we can mark the channel as read - val updatedRead = currentUserRead.copy( - lastReceivedEventDate = lastMessage.getCreatedAtOrDefault(Date()), - lastRead = lastMessage.getCreatedAtOrDefault(Date()), - unreadMessages = 0, - ) + // Zero the count optimistically, but leave the read dates untouched: they stay + // anchored to what the server confirmed, and the server's own message.read echo + // advances them. Stamping them with the newest loaded message's date here made + // messages that were still queued behind this call count as already seen. + val updatedRead = currentUserRead.copy(unreadMessages = 0) _reads.update { current -> current + (updatedRead.getUserId() to updatedRead) } @@ -1614,12 +1633,22 @@ internal class ChannelStateImpl( localRead: ChannelUserRead, serverRead: ChannelUserRead, ): ChannelUserRead { - return if (localRead.lastReceivedEventDate.after(serverRead.lastReceivedEventDate)) { - // Local state is more recent, preserve it but merge other fields from server + // The read position is the only thing an incoming read can tell us that the local state + // does not already know: the count between two read positions is maintained locally, one + // increment per new message event. So the incoming count is taken only when the position + // moved forward, which is what happens when the channel is read elsewhere. At the same or + // an earlier position the incoming count may be stale, as with the channel list's own copy + // of the channel, so the local count is kept. A mark unread moves the position backwards on + // purpose and its count is authoritative, which is why it is applied by [replaceRead] + // instead of going through this arbitration. Note + // that lastReceivedEventDate cannot be used to decide this: the server does not send it, + // it is synthesised while mapping, so comparing it against a locally advanced clock is + // meaningless and lets a stale snapshot (for example the channel list's own copy of the + // channel) overwrite a count that is correct. + return if (!serverRead.lastRead.after(localRead.lastRead)) { logger.d { - "[updateReads] Local read state is more recent, preserving: " + - "local.lastReceivedEventDate=${localRead.lastReceivedEventDate}, " + - "server.lastReceivedEventDate=${serverRead.lastReceivedEventDate}, " + + "[updateReads] keeping the local count: " + + "local.lastRead=${localRead.lastRead}, server.lastRead=${serverRead.lastRead}, " + "local.unreadMessages=${localRead.unreadMessages}, " + "server.unreadMessages=${serverRead.unreadMessages}" } @@ -1632,7 +1661,7 @@ internal class ChannelStateImpl( // lastReceivedEventDate and unreadMessages are preserved from local (not set in copy) ) } else { - // Server data is more recent, use it + // The read position moved forward, so the incoming count describes it best. serverRead } } @@ -1675,6 +1704,9 @@ internal class ChannelStateImpl( } private companion object { + /** Distinguishes instances of the same channel in the logs. */ + private val seqGenerator = AtomicInteger() + /** * Hard limit for cached latest messages to prevent unbounded memory growth while in search mode. * When the user is viewing messages around a specific message (e.g., from a deep link or search), diff --git a/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateLegacyImpl.kt b/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateLegacyImpl.kt index 09dcb908279..900ddc0f088 100644 --- a/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateLegacyImpl.kt +++ b/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateLegacyImpl.kt @@ -17,7 +17,6 @@ package io.getstream.chat.android.client.internal.state.plugin.state.channel.internal import io.getstream.chat.android.client.channel.state.ChannelState -import io.getstream.chat.android.client.extensions.getCreatedAtOrDefault import io.getstream.chat.android.client.extensions.getCreatedAtOrNull import io.getstream.chat.android.client.extensions.internal.updateUsers import io.getstream.chat.android.client.extensions.internal.wasCreatedAfter @@ -611,15 +610,12 @@ internal class ChannelStateLegacyImpl( currentUserRead .takeIf { it.lastReadMessageId != lastMessage.id || it.unreadMessages > 0 } ?.let { - upsertReads( - listOf( - it.copy( - lastReceivedEventDate = lastMessage.getCreatedAtOrDefault(Date()), - lastRead = lastMessage.getCreatedAtOrDefault(Date()), - unreadMessages = 0, - ), - ), - ) + // Zero the count optimistically, but leave the read dates untouched: + // they stay anchored to what the server confirmed, and the server's + // own message.read echo advances them. Stamping them with the newest + // loaded message's date here made messages that were still queued + // behind this call count as already seen. + upsertReads(listOf(it.copy(unreadMessages = 0))) true } } diff --git a/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/channel/internal/ChannelEventHandlerImplTest.kt b/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/channel/internal/ChannelEventHandlerImplTest.kt index 8f4a88e3081..87ef8b60780 100644 --- a/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/channel/internal/ChannelEventHandlerImplTest.kt +++ b/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/channel/internal/ChannelEventHandlerImplTest.kt @@ -1025,12 +1025,15 @@ internal class ChannelEventHandlerImplTest { } @Test - fun `When NotificationMarkUnreadEvent is handled, Then read is updated`() { + fun `When NotificationMarkUnreadEvent is handled, Then the read is replaced`() { + // Replaced rather than merged: a mark unread moves the read position backwards on purpose, + // and updateRead would keep the local count instead of the one the server reported. val event = randomNotificationMarkUnreadEvent(cid = cid) handler.handle(event) - verify(state).updateRead(event.toChannelUserRead()) + verify(state).replaceRead(event.toChannelUserRead()) + verify(state, never()).updateRead(any()) } @Test diff --git a/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/channel/internal/legacy/ChannelStateLogicTest.kt b/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/channel/internal/legacy/ChannelStateLogicTest.kt index 595140e3bca..735edd35764 100644 --- a/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/channel/internal/legacy/ChannelStateLogicTest.kt +++ b/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/channel/internal/legacy/ChannelStateLogicTest.kt @@ -63,6 +63,7 @@ import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.RegisterExtension import org.mockito.kotlin.any +import org.mockito.kotlin.argumentCaptor import org.mockito.kotlin.doAnswer import org.mockito.kotlin.doReturn import org.mockito.kotlin.eq @@ -669,19 +670,20 @@ internal class ChannelStateLogicTest { } @Test - fun `Given event is outdated, When updateCurrentUserRead is called, Then unread count is not updated`() { - // given - val initialUnreadCount = 5 + fun `Given event is older than the read state clock, When updateCurrentUserRead is called, Then the message still counts and the clock does not regress`() { + // Marking read stamps the read state with the newest loaded message's date, so a + // queued event can legitimately carry an older date while its message was never + // counted. Only processedMessageIds may exclude it. val initialChannelUserRead = randomChannelUserRead( user = user, - lastReceivedEventDate = Date(100L), // Current read state is at 100L - unreadMessages = initialUnreadCount, + lastReceivedEventDate = Date(100L), + unreadMessages = 0, lastRead = Date(10L), lastReadMessageId = randomString(), ) _read.value = initialChannelUserRead - val eventDate = Date(50L) // Event is older than current read state + val eventDate = Date(50L) // Older than the read state clock, newer than lastRead val newMessage = randomMessage( user = randomUser(id = "anotherUserId"), createdAt = eventDate, @@ -694,7 +696,11 @@ internal class ChannelStateLogicTest { channelStateLogic.updateCurrentUserRead(eventDate, newMessage) // then - verify(mutableState, times(0)).upsertReads(any()) + val captor = argumentCaptor>() + verify(mutableState, times(1)).upsertReads(captor.capture()) + val updatedRead = captor.firstValue.single() + updatedRead.unreadMessages `should be equal to` 1 + updatedRead.lastReceivedEventDate `should be equal to` Date(100L) } @Test diff --git a/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateImplReadReceiptsTest.kt b/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateImplReadReceiptsTest.kt index 58b8969dc66..ba1da1b985e 100644 --- a/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateImplReadReceiptsTest.kt +++ b/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateImplReadReceiptsTest.kt @@ -240,7 +240,7 @@ internal class ChannelStateImplReadReceiptsTest : ChannelStateImplTestBase() { val initialRead = createRead( user = currentUser, unreadMessages = 2, - lastRead = Date(1000), + lastRead = Date(500), lastReceivedEventDate = Date(1000), ) channelState.updateRead(initialRead) @@ -258,7 +258,7 @@ internal class ChannelStateImplReadReceiptsTest : ChannelStateImplTestBase() { val initialRead = createRead( user = currentUser, unreadMessages = 0, - lastRead = Date(1000), + lastRead = Date(500), lastReceivedEventDate = Date(1000), ) channelState.updateRead(initialRead) @@ -277,7 +277,7 @@ internal class ChannelStateImplReadReceiptsTest : ChannelStateImplTestBase() { val initialRead = createRead( user = currentUser, unreadMessages = 0, - lastRead = Date(1000), + lastRead = Date(500), lastReceivedEventDate = Date(1000), ) channelState.updateRead(initialRead) @@ -296,7 +296,7 @@ internal class ChannelStateImplReadReceiptsTest : ChannelStateImplTestBase() { val initialRead = createRead( user = currentUser, unreadMessages = 0, - lastRead = Date(1000), + lastRead = Date(500), lastReceivedEventDate = Date(1000), ) channelState.updateRead(initialRead) @@ -315,7 +315,7 @@ internal class ChannelStateImplReadReceiptsTest : ChannelStateImplTestBase() { val initialRead = createRead( user = currentUser, unreadMessages = 0, - lastRead = Date(1000), + lastRead = Date(500), lastReceivedEventDate = Date(1000), ) channelState.updateRead(initialRead) @@ -333,7 +333,7 @@ internal class ChannelStateImplReadReceiptsTest : ChannelStateImplTestBase() { val initialRead = createRead( user = currentUser, unreadMessages = 0, - lastRead = Date(1000), + lastRead = Date(500), lastReceivedEventDate = Date(1000), ) channelState.updateRead(initialRead) @@ -351,7 +351,7 @@ internal class ChannelStateImplReadReceiptsTest : ChannelStateImplTestBase() { val initialRead = createRead( user = currentUser, unreadMessages = 0, - lastRead = Date(1000), + lastRead = Date(500), lastReceivedEventDate = Date(1000), ) channelState.updateRead(initialRead) @@ -379,7 +379,7 @@ internal class ChannelStateImplReadReceiptsTest : ChannelStateImplTestBase() { val initialRead = createRead( user = currentUser, unreadMessages = 0, - lastRead = Date(1000), + lastRead = Date(500), lastReceivedEventDate = Date(1000), ) stateWithMutedUsers.updateRead(initialRead) @@ -396,7 +396,7 @@ internal class ChannelStateImplReadReceiptsTest : ChannelStateImplTestBase() { val initialRead = createRead( user = currentUser, unreadMessages = 0, - lastRead = Date(1000), + lastRead = Date(500), lastReceivedEventDate = Date(1000), ) channelState.updateRead(initialRead) @@ -414,7 +414,7 @@ internal class ChannelStateImplReadReceiptsTest : ChannelStateImplTestBase() { val initialRead = createRead( user = currentUser, unreadMessages = 0, - lastRead = Date(1000), + lastRead = Date(500), lastReceivedEventDate = Date(1000), ) channelState.updateRead(initialRead) @@ -427,21 +427,24 @@ internal class ChannelStateImplReadReceiptsTest : ChannelStateImplTestBase() { } @Test - fun `updateCurrentUserRead should skip outdated events`() = runTest { - // given + fun `updateCurrentUserRead should count events older than the read state clock without regressing it`() = runTest { + // Marking read stamps the read state with the newest loaded message's date, so a + // queued event can legitimately carry an older date while its message was never + // counted. Only processedMessageIds may exclude it. val initialRead = createRead( user = currentUser, unreadMessages = 0, - lastRead = Date(1000), - lastReceivedEventDate = Date(5000), // recent event date + lastRead = Date(500), + lastReceivedEventDate = Date(5000), ) channelState.updateRead(initialRead) val otherUser = randomUser(id = "other_user") - val message = createMessage(1, user = otherUser) + val message = createMessage(1, timestamp = 3000, user = otherUser) // when - event date is older than current read's lastReceivedEventDate channelState.updateCurrentUserRead(Date(3000), message) // then - assertEquals(0, channelState.read.value?.unreadMessages) + assertEquals(1, channelState.read.value?.unreadMessages) + assertEquals(Date(5000), channelState.read.value?.lastReceivedEventDate) } @Test @@ -462,7 +465,7 @@ internal class ChannelStateImplReadReceiptsTest : ChannelStateImplTestBase() { val initialRead = createRead( user = currentUser, unreadMessages = 0, - lastRead = Date(1000), + lastRead = Date(500), lastReceivedEventDate = Date(1000), ) channelState.updateRead(initialRead) @@ -558,8 +561,10 @@ internal class ChannelStateImplReadReceiptsTest : ChannelStateImplTestBase() { } @Test - fun `markRead should update lastRead to last message createdAt`() = runTest { - // given + fun `markRead should keep the read dates untouched`() = runTest { + // The read dates only advance with server-confirmed values (query responses and + // read events); stamping them optimistically made messages that were still queued + // behind the mark-read call count as already seen. channelState.setChannelConfig(Config(readEventsEnabled = true)) val message1 = createMessage(1, timestamp = 1000) val message2 = createMessage(2, timestamp = 2000) @@ -568,6 +573,7 @@ internal class ChannelStateImplReadReceiptsTest : ChannelStateImplTestBase() { user = currentUser, unreadMessages = 2, lastRead = Date(500), + lastReceivedEventDate = Date(500), lastReadMessageId = null, ) channelState.updateRead(read) @@ -575,7 +581,9 @@ internal class ChannelStateImplReadReceiptsTest : ChannelStateImplTestBase() { channelState.markRead() // then val currentRead = channelState.read.value - assertEquals(Date(2000), currentRead?.lastRead) + assertEquals(0, currentRead?.unreadMessages) + assertEquals(Date(500), currentRead?.lastRead) + assertEquals(Date(500), currentRead?.lastReceivedEventDate) } } diff --git a/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateLegacyImplTest.kt b/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateLegacyImplTest.kt index 367b635b423..119b97ef476 100644 --- a/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateLegacyImplTest.kt +++ b/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateLegacyImplTest.kt @@ -637,8 +637,9 @@ internal class ChannelStateLegacyImplTest { val actual = channelState.markChannelAsRead() assertEquals(true, actual) - assertEquals(lastMessage.createdLocallyAt, channelState.read.value?.lastReceivedEventDate) - assertEquals(lastMessage.createdLocallyAt, channelState.read.value?.lastRead) + // The read dates stay untouched: they only advance with server-confirmed values. + assertEquals(readState.lastReceivedEventDate, channelState.read.value?.lastReceivedEventDate) + assertEquals(readState.lastRead, channelState.read.value?.lastRead) assertEquals(0, channelState.read.value?.unreadMessages) } @@ -683,8 +684,9 @@ internal class ChannelStateLegacyImplTest { val result = channelState.markChannelAsRead() assertEquals(true, result) - assertEquals(lastMessage.createdLocallyAt, channelState.read.value?.lastReceivedEventDate) - assertEquals(lastMessage.createdLocallyAt, channelState.read.value?.lastRead) + // The read dates stay untouched: they only advance with server-confirmed values. + assertEquals(readState.lastReceivedEventDate, channelState.read.value?.lastReceivedEventDate) + assertEquals(readState.lastRead, channelState.read.value?.lastRead) assertEquals(0, channelState.read.value?.unreadMessages) } diff --git a/stream-chat-android-compose-sample/src/androidTestE2eDebug/kotlin/io/getstream/chat/android/compose/robots/UserRobotMessageListAsserts.kt b/stream-chat-android-compose-sample/src/androidTestE2eDebug/kotlin/io/getstream/chat/android/compose/robots/UserRobotMessageListAsserts.kt index 54c3c768032..46dd859379c 100644 --- a/stream-chat-android-compose-sample/src/androidTestE2eDebug/kotlin/io/getstream/chat/android/compose/robots/UserRobotMessageListAsserts.kt +++ b/stream-chat-android-compose-sample/src/androidTestE2eDebug/kotlin/io/getstream/chat/android/compose/robots/UserRobotMessageListAsserts.kt @@ -48,6 +48,7 @@ import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNotEquals import org.junit.Assert.assertTrue +import java.util.regex.Pattern /** * Asserts the selector's visibility with a bounded wait: waits for it to appear when @@ -337,7 +338,11 @@ fun UserRobot.assertScrollToFirstUnreadButton(unreadCount: Int? = null, isDispla unreadCount, unreadCount, ) - assertTrue(By.text(expectedText).waitDisplayed()) + // Comparing against the rendered count so a failure reports the actual text. The + // count is the only text inside the button and carries no test tag of its own. + val countText = By.text(Pattern.compile(".+")) + .hasAncestor(MessageListPage.MessageList.scrollToFirstUnreadButton) + assertEquals(expectedText, countText.waitForText(expectedText)) } return this }