Skip to content

Commit 6a6952a

Browse files
committed
Fix: Fetch message from API when marking as delivered by ID
When `markMessageAsDelivered` is called with a `messageId`, the SDK now fetches the message from the local repository. If the message is not found locally, it falls back to fetching it from the API. This ensures that a delivery receipt can be sent for a push notification even if the message is not yet in the local database.
1 parent cf17138 commit 6a6952a

File tree

5 files changed

+86
-34
lines changed

5 files changed

+86
-34
lines changed

stream-chat-android-client/src/main/java/io/getstream/chat/android/client/ChatClient.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,7 @@ internal constructor(
338338
MessageReceiptManager(
339339
now = now,
340340
getCurrentUser = ::getCurrentUser,
341-
channelRepository = repositoryFacade,
341+
repositoryFacade = repositoryFacade,
342342
messageReceiptRepository = repository,
343343
api = api,
344344
)

stream-chat-android-client/src/main/java/io/getstream/chat/android/client/notifications/ChatNotifications.kt

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ import io.getstream.chat.android.client.notifications.handler.NotificationConfig
2828
import io.getstream.chat.android.client.notifications.handler.NotificationHandler
2929
import io.getstream.chat.android.core.internal.coroutines.DispatcherProvider
3030
import io.getstream.chat.android.models.Device
31-
import io.getstream.chat.android.models.Message
3231
import io.getstream.chat.android.models.PushMessage
3332
import io.getstream.chat.android.models.User
3433
import io.getstream.log.taggedLogger
@@ -110,11 +109,7 @@ internal class ChatNotificationsImpl(
110109

111110
pushNotificationReceivedListener.onPushNotificationReceived(pushMessage.channelType, pushMessage.channelId)
112111

113-
val message = Message(
114-
id = pushMessage.messageId,
115-
cid = "${pushMessage.channelType}:${pushMessage.channelId}",
116-
)
117-
scope.launch { chatClient.messageReceiptManager.markMessageAsDelivered(message) }
112+
scope.launch { chatClient.messageReceiptManager.markMessageAsDelivered(pushMessage.messageId) }
118113

119114
if (notificationConfig.shouldShowNotificationOnPush() && !handler.onPushMessage(pushMessage)) {
120115
handlePushMessage(pushMessage)

stream-chat-android-client/src/main/java/io/getstream/chat/android/client/receipts/MessageReceiptManager.kt

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import io.getstream.chat.android.client.extensions.getCreatedAtOrThrow
2323
import io.getstream.chat.android.client.extensions.internal.NEVER
2424
import io.getstream.chat.android.client.extensions.internal.lastMessage
2525
import io.getstream.chat.android.client.extensions.userRead
26-
import io.getstream.chat.android.client.persistance.repository.ChannelRepository
26+
import io.getstream.chat.android.client.persistance.repository.RepositoryFacade
2727
import io.getstream.chat.android.client.persistence.repository.MessageReceiptRepository
2828
import io.getstream.chat.android.client.utils.message.isDeleted
2929
import io.getstream.chat.android.models.Channel
@@ -40,7 +40,7 @@ import java.util.Date
4040
internal class MessageReceiptManager(
4141
private val now: () -> Date,
4242
private val getCurrentUser: () -> User?,
43-
private val channelRepository: ChannelRepository,
43+
private val repositoryFacade: RepositoryFacade,
4444
private val messageReceiptRepository: MessageReceiptRepository,
4545
private val api: ChatApi,
4646
) {
@@ -91,7 +91,7 @@ internal class MessageReceiptManager(
9191

9292
if (!currentUser.isDeliveryReceiptsEnabled) return
9393

94-
val channel = getChannel(message.cid) ?: run {
94+
val channel = retrieveChannel(message.cid) ?: run {
9595
logger.w { "[markMessageAsDelivered] Channel ${message.cid} not found" }
9696
return
9797
}
@@ -101,14 +101,33 @@ internal class MessageReceiptManager(
101101
}
102102
}
103103

104-
private suspend fun getChannel(cid: String): Channel? =
105-
channelRepository.selectChannel(cid)
106-
?: run {
107-
val (channelType, channelId) = cid.cidToTypeAndId()
108-
val request = QueryChannelRequest()
109-
api.queryChannel(channelType, channelId, request)
110-
.await().getOrNull()
111-
}
104+
/**
105+
* Request to mark the message with the given id as delivered.
106+
*
107+
* @see [markChannelsAsDelivered] for the conditions to mark a message as delivered.
108+
*/
109+
suspend fun markMessageAsDelivered(messageId: String) {
110+
val message = retrieveMessage(messageId) ?: run {
111+
logger.w { "[markMessageAsDelivered] Message $messageId not found" }
112+
return
113+
}
114+
115+
markMessageAsDelivered(message)
116+
}
117+
118+
private suspend fun retrieveChannel(cid: String): Channel? =
119+
repositoryFacade.selectChannel(cid) ?: run {
120+
val (channelType, channelId) = cid.cidToTypeAndId()
121+
val request = QueryChannelRequest()
122+
api.queryChannel(channelType, channelId, request)
123+
.await().getOrNull()
124+
}
125+
126+
private suspend fun retrieveMessage(id: String): Message? =
127+
repositoryFacade.selectMessage(id) ?: run {
128+
api.getMessage(id)
129+
.await().getOrNull()
130+
}
112131

113132
private suspend fun markMessagesAsDelivered(messages: List<Message>) {
114133
if (messages.isEmpty()) {

stream-chat-android-client/src/test/java/io/getstream/chat/android/client/notifications/ChatNotificationsImplTest.kt

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ import io.getstream.chat.android.client.notifications.handler.NotificationConfig
2727
import io.getstream.chat.android.client.notifications.handler.NotificationHandler
2828
import io.getstream.chat.android.client.randomPushMessage
2929
import io.getstream.chat.android.client.receipts.MessageReceiptManager
30-
import io.getstream.chat.android.models.Message
3130
import io.getstream.chat.android.models.PushMessage
3231
import kotlinx.coroutines.CoroutineScope
3332
import kotlinx.coroutines.test.UnconfinedTestDispatcher
@@ -69,11 +68,7 @@ internal class ChatNotificationsImplTest {
6968

7069
sut.onPushMessage(pushMessage)
7170

72-
val message = Message(
73-
id = pushMessage.messageId,
74-
cid = "${pushMessage.channelType}:${pushMessage.channelId}",
75-
)
76-
fixture.verifyMarkMessageAsDeliveredCalled(message)
71+
fixture.verifyMarkMessageAsDeliveredCalled(messageId = pushMessage.messageId)
7772
}
7873

7974
@Test
@@ -132,8 +127,8 @@ internal class ChatNotificationsImplTest {
132127
notificationConfig = config
133128
}
134129

135-
fun verifyMarkMessageAsDeliveredCalled(message: Message) {
136-
verifyBlocking(mockMessageReceiptManager) { markMessageAsDelivered(message) }
130+
fun verifyMarkMessageAsDeliveredCalled(messageId: String) {
131+
verifyBlocking(mockMessageReceiptManager) { markMessageAsDelivered(messageId) }
137132
}
138133

139134
fun get(): ChatNotificationsImpl {

stream-chat-android-client/src/test/java/io/getstream/chat/android/client/receipts/MessageReceiptManagerTest.kt

Lines changed: 51 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import io.getstream.chat.android.DeliveryReceipts
2020
import io.getstream.chat.android.PrivacySettings
2121
import io.getstream.chat.android.client.api.ChatApi
2222
import io.getstream.chat.android.client.extensions.internal.NEVER
23-
import io.getstream.chat.android.client.persistance.repository.ChannelRepository
23+
import io.getstream.chat.android.client.persistance.repository.RepositoryFacade
2424
import io.getstream.chat.android.client.persistence.repository.MessageReceiptRepository
2525
import io.getstream.chat.android.models.User
2626
import io.getstream.chat.android.randomChannel
@@ -64,10 +64,10 @@ internal class MessageReceiptManagerTest {
6464
}
6565

6666
@Test
67-
fun `store message delivery receipt when channel is found from API`() = runTest {
67+
fun `fetch channel from API when channel is not found from repository`() = runTest {
6868
val message = DeliverableMessage
6969
val fixture = Fixture()
70-
.givenChannelNotFromRepository()
70+
.givenChannelNotFoundFromRepository()
7171
val sut = fixture.get()
7272

7373
sut.markMessageAsDelivered(message)
@@ -83,11 +83,44 @@ internal class MessageReceiptManagerTest {
8383
fixture.verifyUpsertMessageReceiptsCalled(receipts = receipts)
8484
}
8585

86+
@Test
87+
fun `fetch message from API when message is not found from repository`() = runTest {
88+
val message = DeliverableMessage
89+
val fixture = Fixture()
90+
.givenMessageNotFoundFromRepository()
91+
val sut = fixture.get()
92+
93+
sut.markMessageAsDelivered(messageId = message.id)
94+
95+
val receipts = listOf(
96+
MessageReceipt(
97+
messageId = message.id,
98+
type = MessageReceipt.TYPE_DELIVERY,
99+
createdAt = Now,
100+
cid = message.cid,
101+
),
102+
)
103+
fixture.verifyUpsertMessageReceiptsCalled(receipts = receipts)
104+
}
105+
106+
@Test
107+
fun `should skip storing message delivery receipt when message is not found`() = runTest {
108+
val message = DeliverableMessage
109+
val fixture = Fixture()
110+
.givenMessageNotFoundFromRepository()
111+
.givenMessageNotFoundFromApi()
112+
val sut = fixture.get()
113+
114+
sut.markMessageAsDelivered(messageId = message.id)
115+
116+
fixture.verifyUpsertMessageReceiptsCalled(never())
117+
}
118+
86119
@Test
87120
fun `should skip storing message delivery receipt when channel is not found`() = runTest {
88121
val message = DeliverableMessage
89122
val fixture = Fixture()
90-
.givenChannelNotFromRepository()
123+
.givenChannelNotFoundFromRepository()
91124
.givenChannelNotFoundFromApi()
92125
val sut = fixture.get()
93126

@@ -301,8 +334,9 @@ internal class MessageReceiptManagerTest {
301334

302335
private class Fixture {
303336
private var getCurrentUser: () -> User? = { CurrentUser }
304-
private val mockChannelRepository = mock<ChannelRepository> {
337+
private val mockRepositoryFacade = mock<RepositoryFacade> {
305338
onBlocking { selectChannel(DeliverableChannel.cid) } doReturn DeliverableChannel
339+
onBlocking { selectMessage(DeliverableMessage.id) } doReturn DeliverableMessage
306340
}
307341
private val mockMessageReceiptRepository = mock<MessageReceiptRepository>()
308342
private val mockChatApi = mock<ChatApi> {
@@ -313,14 +347,19 @@ internal class MessageReceiptManagerTest {
313347
query = any(),
314348
)
315349
} doReturn DeliverableChannel.asCall()
350+
on { getMessage(messageId = DeliverableMessage.id) } doReturn DeliverableMessage.asCall()
316351
}
317352

318353
fun givenCurrentUser(user: User?) = apply {
319354
getCurrentUser = { user }
320355
}
321356

322-
fun givenChannelNotFromRepository() = apply {
323-
wheneverBlocking { mockChannelRepository.selectChannel(cid = any()) } doReturn null
357+
fun givenChannelNotFoundFromRepository() = apply {
358+
wheneverBlocking { mockRepositoryFacade.selectChannel(cid = any()) } doReturn null
359+
}
360+
361+
fun givenMessageNotFoundFromRepository() = apply {
362+
wheneverBlocking { mockRepositoryFacade.selectMessage(messageId = any()) } doReturn null
324363
}
325364

326365
fun givenChannelNotFoundFromApi() = apply {
@@ -333,6 +372,10 @@ internal class MessageReceiptManagerTest {
333372
} doReturn mock<Error>().asCall()
334373
}
335374

375+
fun givenMessageNotFoundFromApi() = apply {
376+
wheneverBlocking { mockChatApi.getMessage(messageId = any()) } doReturn mock<Error>().asCall()
377+
}
378+
336379
fun verifyUpsertMessageReceiptsCalled(
337380
mode: VerificationMode = times(1),
338381
receipts: List<MessageReceipt>? = null,
@@ -345,7 +388,7 @@ internal class MessageReceiptManagerTest {
345388
fun get() = MessageReceiptManager(
346389
now = { Now },
347390
getCurrentUser = getCurrentUser,
348-
channelRepository = mockChannelRepository,
391+
repositoryFacade = mockRepositoryFacade,
349392
messageReceiptRepository = mockMessageReceiptRepository,
350393
api = mockChatApi,
351394
)

0 commit comments

Comments
 (0)