Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -209,9 +209,9 @@ public fun Message.isModerationError(currentUserId: String?): Boolean = isMine(c
(isError() && isModerationBounce())

/**
* @return If the message exists only on this client and is not persisted server-side: it is
* still in flight, failed to send, or has a type the server never persists (ephemeral previews,
* error messages such as rejected or moderation-bounced sends).
* @return If the message is not part of the server's message list: it is unsynced, or has a type
* the server leaves out of message queries and read state (ephemeral previews, error messages such
* as rejected or moderation-bounced sends).
*/
@InternalStreamChatApi
public fun Message.isLocalOnly(): Boolean =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import io.getstream.chat.android.client.utils.message.isDeleted
import io.getstream.chat.android.client.utils.message.isError
import io.getstream.chat.android.client.utils.message.isGiphy
import io.getstream.chat.android.client.utils.message.isLocalOnly
import io.getstream.chat.android.client.utils.message.isMine
import io.getstream.chat.android.client.utils.message.isModerationBounce
import io.getstream.chat.android.client.utils.message.isModerationError
import io.getstream.chat.android.client.utils.message.isSystem
Expand Down Expand Up @@ -1717,19 +1718,17 @@ public class MessageListController(
* Marks that the last message in the list as read. This also sets the unread count to 0.
*/
private fun markLastMessageReadInternal() {
val itemState = messagesState.messageItems.lastOrNull { messageItem ->
messageItem is HasMessageListItemState
} as? HasMessageListItemState
val message = itemState?.message
val messageItems = messagesState.messageItems.filterIsInstance<HasMessageListItemState>()
val message = messageItems.lastOrNull()?.message
val messageId = message?.id
val messageText = message?.text
logger.d { "[markLastMessageRead] cid: $cid, msgId($isInThread): $messageId, msgText: \"$messageText\"" }

// Marking read while our own local-only message is at the bottom makes a channel the
// server sees as empty emit message.read with no last_read_message_id.
// Marking read with nothing the server tracks makes it emit message.read with no
// last_read_message_id.
val currentUserId = clientState.user.value?.id
if (message != null && message.user.id == currentUserId && message.isLocalOnly()) {
logger.v { "[markLastMessageRead] cid: $cid; rejected[$isInThread] (own local-only): $messageId" }
if (messageItems.none { it.message.isInServerReadState(currentUserId) }) {
logger.v { "[markLastMessageRead] cid: $cid; rejected[$isInThread] (no server-side message)" }
return
}

Expand All @@ -1747,6 +1746,11 @@ public class MessageListController(
}
}

// The server keeps our own local-only messages out of its read state, and silent and shadowed
// ones from anyone, so none of them can resolve a mark-read call.
private fun Message.isInServerReadState(currentUserId: String?): Boolean =
!(isMine(currentUserId) && isLocalOnly()) && !silent && !shadowed

private fun markChannelAsRead() {
val (channelType, channelId) = cid.cidToTypeAndId()
chatClient.markRead(channelType, channelId).enqueue(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -351,9 +351,9 @@ internal class MessageListControllerTests {
fun `When repetitive markLastMessageRead calls appear only single API call should be sent`() = runTest {
val chatClient: ChatClient = mock()
val messages = arrayListOf(
randomMessage(id = "1", syncStatus = SyncStatus.COMPLETED),
randomMessage(id = "2", syncStatus = SyncStatus.COMPLETED),
randomMessage(id = "3", syncStatus = SyncStatus.COMPLETED),
randomMessage(id = "1", syncStatus = SyncStatus.COMPLETED, silent = false),
randomMessage(id = "2", syncStatus = SyncStatus.COMPLETED, silent = false),
randomMessage(id = "3", syncStatus = SyncStatus.COMPLETED, silent = false),
)
val messagesState = MutableStateFlow(messages)
val controller = Fixture(chatClient = chatClient)
Expand Down Expand Up @@ -381,7 +381,7 @@ internal class MessageListControllerTests {
fun `When current user's last message is COMPLETED markLastMessageRead should invoke markRead`() = runTest {
val chatClient: ChatClient = mock()
val messagesState = MutableStateFlow(
listOf(randomMessage(id = "1", user = user1, syncStatus = SyncStatus.COMPLETED)),
listOf(randomMessage(id = "1", user = user1, syncStatus = SyncStatus.COMPLETED, silent = false)),
)
val controller = Fixture(chatClient = chatClient)
.givenCurrentUser()
Expand All @@ -401,7 +401,7 @@ internal class MessageListControllerTests {
fun `When current user's last message is not COMPLETED markLastMessageRead should not invoke markRead`() = runTest {
val chatClient: ChatClient = mock()
val messagesState = MutableStateFlow(
listOf(randomMessage(id = "1", user = user1, syncStatus = SyncStatus.IN_PROGRESS)),
listOf(randomMessage(id = "1", user = user1, syncStatus = SyncStatus.IN_PROGRESS, silent = false)),
)
val controller = Fixture(chatClient = chatClient)
.givenCurrentUser()
Expand All @@ -418,16 +418,20 @@ internal class MessageListControllerTests {
}

@Test
fun `When current user's last message is a rejected error echo markLastMessageRead should not invoke markRead`() =
fun `When the channel holds only our own rejected error echo markLastMessageRead should not invoke markRead`() =
runTest {
// A send into a frozen channel returns 201 with a type "error" echo which is stored
// as COMPLETED, but the server never persists it. The peer message makes sure the
// error one is the item the gate sees, and not filtered out.
// as COMPLETED, while the server keeps it out of its read state.
val chatClient: ChatClient = mock()
val messagesState = MutableStateFlow(
listOf(
randomMessage(id = "1", user = user2, type = MessageType.REGULAR, syncStatus = SyncStatus.COMPLETED),
randomMessage(id = "2", user = user1, type = MessageType.ERROR, syncStatus = SyncStatus.COMPLETED),
randomMessage(
id = "1",
user = user1,
type = MessageType.ERROR,
syncStatus = SyncStatus.COMPLETED,
silent = false,
),
),
)
val controller = Fixture(chatClient = chatClient)
Expand All @@ -445,12 +449,71 @@ internal class MessageListControllerTests {
}

@Test
fun `When current user's last message is ephemeral markLastMessageRead should not invoke markRead`() = runTest {
fun `When the channel holds only our own ephemeral message markLastMessageRead should not invoke markRead`() =
runTest {
val chatClient: ChatClient = mock()
val messagesState = MutableStateFlow(
listOf(
randomMessage(
id = "1",
user = user1,
type = MessageType.EPHEMERAL,
syncStatus = SyncStatus.COMPLETED,
silent = false,
),
),
)
val controller = Fixture(chatClient = chatClient)
.givenCurrentUser()
.givenChannelQuery()
.givenMarkRead()
.givenChannelState(messagesState = messagesState)
.get()

controller.markLastMessageRead()
delay(1000)

verify(chatClient, times(0)).markRead(any(), any())
controller.lastSeenMessageId.shouldBeNull()
}

@Test
fun `When a server-side message precedes our own error echo markLastMessageRead should invoke markRead`() = runTest {
// The server has messages of its own to mark read, so it resolves the read state itself.
val chatClient: ChatClient = mock()
val messagesState = MutableStateFlow(
listOf(
randomMessage(id = "1", user = user2, type = MessageType.REGULAR, syncStatus = SyncStatus.COMPLETED),
randomMessage(id = "2", user = user1, type = MessageType.EPHEMERAL, syncStatus = SyncStatus.COMPLETED),
randomMessage(id = "1", user = user2, type = MessageType.REGULAR, syncStatus = SyncStatus.COMPLETED, silent = false),
randomMessage(id = "2", user = user1, type = MessageType.ERROR, syncStatus = SyncStatus.COMPLETED, silent = false),
),
)
val controller = Fixture(chatClient = chatClient)
.givenCurrentUser()
.givenChannelQuery()
.givenMarkRead()
.givenChannelState(messagesState = messagesState)
.get()

controller.markLastMessageRead()
delay(1000)

verify(chatClient, times(1)).markRead(eq(CHANNEL_TYPE), eq(CHANNEL_ID))
controller.lastSeenMessageId `should be equal to` "2"
}

@Test
fun `When the channel holds only a silent message markLastMessageRead should not invoke markRead`() = runTest {
// A silent message does not mark a channel unread, so the server has nothing to resolve.
val chatClient: ChatClient = mock()
val messagesState = MutableStateFlow(
listOf(
randomMessage(
id = "1",
user = user2,
type = MessageType.REGULAR,
syncStatus = SyncStatus.COMPLETED,
silent = true,
),
),
)
val controller = Fixture(chatClient = chatClient)
Expand All @@ -467,6 +530,58 @@ internal class MessageListControllerTests {
controller.lastSeenMessageId.shouldBeNull()
}

@Test
fun `When the channel holds only a shadowed message markLastMessageRead should not invoke markRead`() = runTest {
val chatClient: ChatClient = mock()
val messagesState = MutableStateFlow(
listOf(
randomMessage(
id = "1",
user = user1,
type = MessageType.REGULAR,
syncStatus = SyncStatus.COMPLETED,
silent = false,
shadowed = true,
),
),
)
val controller = Fixture(chatClient = chatClient)
.givenCurrentUser()
.givenChannelQuery()
.givenMarkRead()
.givenChannelState(messagesState = messagesState)
.get()

controller.markLastMessageRead()
delay(1000)

verify(chatClient, times(0)).markRead(any(), any())
controller.lastSeenMessageId.shouldBeNull()
}

@Test
fun `When a silent message follows a tracked one markLastMessageRead should invoke markRead`() = runTest {
val chatClient: ChatClient = mock()
val messagesState = MutableStateFlow(
listOf(
randomMessage(id = "1", user = user2, type = MessageType.REGULAR, syncStatus = SyncStatus.COMPLETED, silent = false),
randomMessage(id = "2", user = user2, type = MessageType.REGULAR, syncStatus = SyncStatus.COMPLETED, silent = true),
),
)
val controller = Fixture(chatClient = chatClient)
.givenCurrentUser()
.givenChannelQuery()
.givenMarkRead()
.givenChannelState(messagesState = messagesState)
.get()

controller.markLastMessageRead()
delay(1000)

verify(chatClient, times(1)).markRead(eq(CHANNEL_TYPE), eq(CHANNEL_ID))
controller.lastSeenMessageId `should be equal to` "2"
}

@Test
fun `When the message list is empty markLastMessageRead should not invoke markRead`() = runTest {
val chatClient: ChatClient = mock()
Expand All @@ -489,8 +604,8 @@ internal class MessageListControllerTests {
val chatClient: ChatClient = mock()
val messagesState = MutableStateFlow(
listOf(
randomMessage(id = "1", user = user1, type = MessageType.ERROR, syncStatus = SyncStatus.COMPLETED),
randomMessage(id = "2", user = user1, type = MessageType.REGULAR, syncStatus = SyncStatus.COMPLETED),
randomMessage(id = "1", user = user1, type = MessageType.ERROR, syncStatus = SyncStatus.COMPLETED, silent = false),
randomMessage(id = "2", user = user1, type = MessageType.REGULAR, syncStatus = SyncStatus.COMPLETED, silent = false),
),
)
val controller = Fixture(chatClient = chatClient)
Expand All @@ -513,7 +628,7 @@ internal class MessageListControllerTests {
// class default — the gate must not block them on that.
val chatClient: ChatClient = mock()
val messagesState = MutableStateFlow(
listOf(randomMessage(id = "1", user = user2, syncStatus = SyncStatus.IN_PROGRESS)),
listOf(randomMessage(id = "1", user = user2, syncStatus = SyncStatus.IN_PROGRESS, silent = false)),
)
val controller = Fixture(chatClient = chatClient)
.givenCurrentUser()
Expand Down Expand Up @@ -1454,6 +1569,8 @@ internal class MessageListControllerTests {
type = type,
text = text,
syncStatus = syncStatus,
// randomMessage randomises silent, which the mark-read gate keys on.
silent = false,
createdAt = nowDate,
updatedAt = nowDate,
deletedAt = null,
Expand Down
Loading