Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix empty message list #5086

Merged
merged 6 commits into from
Nov 24, 2023
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
## stream-chat-android-client
### 🐞 Fixed
- Fixed audio recording not being uploaded [#5066](https://github.com/GetStream/stream-chat-android/pull/5066)
- All sent messages are initialized with a non-null `createdLocallyAt` property. [#5086](https://github.com/GetStream/stream-chat-android/pull/5086)

### ⬆️ Improved

Expand All @@ -39,6 +40,7 @@
### 🐞 Fixed

### ⬆️ Improved
- Fix issue on the pagination process when querying a channel by filling the messages list gap. [#5086](https://github.com/GetStream/stream-chat-android/pull/5086)

### ✅ Added

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1933,6 +1933,9 @@ public final class io/getstream/chat/android/client/extensions/FlowExtensions {

public final class io/getstream/chat/android/client/extensions/MessageExtensionsKt {
public static final fun enrichWithCid (Lio/getstream/chat/android/models/Message;Ljava/lang/String;)Lio/getstream/chat/android/models/Message;
public static final fun getCreatedAtOrDefault (Lio/getstream/chat/android/models/Message;Ljava/util/Date;)Ljava/util/Date;
public static final fun getCreatedAtOrNull (Lio/getstream/chat/android/models/Message;)Ljava/util/Date;
public static final fun getCreatedAtOrThrow (Lio/getstream/chat/android/models/Message;)Ljava/util/Date;
}

public final class io/getstream/chat/android/client/extensions/SharedPreferencesKt {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1693,20 +1693,23 @@ internal constructor(
message: Message,
isRetrying: Boolean = false,
): Call<Message> {
return CoroutineCall(userScope) {
val debugger = clientDebugger.debugSendMessage(channelType, channelId, message, isRetrying)
debugger.onStart(message)
sendAttachments(channelType, channelId, message, isRetrying, debugger)
.flatMapSuspend { newMessage ->
debugger.onSendStart(newMessage)
doSendMessage(channelType, channelId, newMessage).also { result ->
debugger.onSendStop(result, newMessage)
debugger.onStop(result, newMessage)
}
return message.copy(createdLocallyAt = message.createdLocallyAt ?: Date())
.let { processedMessage ->
CoroutineCall(userScope) {
val debugger = clientDebugger.debugSendMessage(channelType, channelId, processedMessage, isRetrying)
debugger.onStart(processedMessage)
sendAttachments(channelType, channelId, processedMessage, isRetrying, debugger)
.flatMapSuspend { newMessage ->
debugger.onSendStart(newMessage)
doSendMessage(channelType, channelId, newMessage).also { result ->
debugger.onSendStop(result, newMessage)
debugger.onStop(result, newMessage)
}
}
}.share(userScope) {
SendMessageIdentifier(channelType, channelId, processedMessage.id)
}
}.share(userScope) {
SendMessageIdentifier(channelType, channelId, message.id)
}
}
}

private suspend fun doSendMessage(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,25 @@ public fun Message.updateMessageOnlineState(isOnline: Boolean): Message {
updatedLocallyAt = Date(),
)
}

/**
* @return when the message was created or throw an exception.
*/
public fun Message.getCreatedAtOrThrow(): Date {
val created = getCreatedAtOrNull()
return checkNotNull(created) { "a message needs to have a non null value for either createdAt or createdLocallyAt" }
}

/**
* @return when the message was created or null.
*/
public fun Message.getCreatedAtOrNull(): Date? {
return createdAt ?: createdLocallyAt
}

/**
* @return when the message was created or `default`.
*/
public fun Message.getCreatedAtOrDefault(default: Date): Date {
return getCreatedAtOrNull() ?: default
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import androidx.compose.ui.Modifier
import androidx.compose.ui.res.painterResource
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.dp
import io.getstream.chat.android.client.extensions.getCreatedAtOrThrow
import io.getstream.chat.android.compose.R
import io.getstream.chat.android.compose.previewdata.PreviewMessageData
import io.getstream.chat.android.compose.ui.theme.ChatTheme
Expand All @@ -34,7 +35,6 @@ import io.getstream.chat.android.models.Channel
import io.getstream.chat.android.models.Message
import io.getstream.chat.android.models.SyncStatus
import io.getstream.chat.android.models.User
import io.getstream.chat.android.ui.common.utils.extensions.getCreatedAtOrThrow

/**
* Shows a delivery status indicator for a particular message.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ public class MessagesViewModelFactory(
mediaRecorder = mediaRecorder,
fileToUri = fileToUriConverter,
channelId = channelId,
messageLimit = messageLimit,
maxAttachmentCount = maxAttachmentCount,
maxAttachmentSize = maxAttachmentSize,
messageId = messageId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,7 @@ internal class MessageComposerViewModelTest {
fileToUri = { it.path },
maxAttachmentCount = maxAttachmentCount,
maxAttachmentSize = maxAttachmentSize,
messageLimit = 30,
),
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ import io.getstream.chat.android.client.events.UserPresenceChangedEvent
import io.getstream.chat.android.client.events.UserStartWatchingEvent
import io.getstream.chat.android.client.events.UserStopWatchingEvent
import io.getstream.chat.android.client.events.UserUpdatedEvent
import io.getstream.chat.android.client.extensions.getCreatedAtOrDefault
import io.getstream.chat.android.client.extensions.getCreatedAtOrNull
import io.getstream.chat.android.client.extensions.internal.NEVER
import io.getstream.chat.android.client.extensions.internal.applyPagination
import io.getstream.chat.android.client.persistance.repository.RepositoryFacade
import io.getstream.chat.android.client.query.pagination.AnyChannelPaginationRequest
Expand All @@ -80,6 +83,8 @@ import io.getstream.log.StreamLog
import io.getstream.log.taggedLogger
import io.getstream.result.Error
import io.getstream.result.Result
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.launch
import java.util.Date

/**
Expand All @@ -95,6 +100,7 @@ internal class ChannelLogic(
private val repos: RepositoryFacade,
private val userPresence: Boolean,
private val channelStateLogic: ChannelStateLogic,
private val coroutineScope: CoroutineScope,
) {

private val mutableState: ChannelMutableState = channelStateLogic.writeChannelState()
Expand Down Expand Up @@ -192,12 +198,11 @@ internal class ChannelLogic(
request: WatchChannelRequest,
): Result<Channel> {
logger.d { "[runChannelQuery] #$src; request: $request" }
val loadedMessages = mutableState.messageList.value
val offlineChannel = runChannelQueryOffline(request)

val onlineResult =
ChatClient.instance()
.queryChannel(mutableState.channelType, mutableState.channelId, request, skipOnRequest = true)
.await()
val onlineResult = runChannelQueryOnline(request)
.onSuccess { fillTheGap(request.messagesLimit(), loadedMessages, it.messages) }

return when {
onlineResult is Result.Success -> onlineResult
Expand All @@ -206,6 +211,68 @@ internal class ChannelLogic(
}
}

/**
* Query the API and return a channel object.
*
* @param request The request object for the query.
*/
private suspend fun runChannelQueryOnline(request: WatchChannelRequest): Result<Channel> =
ChatClient.instance()
.queryChannel(mutableState.channelType, mutableState.channelId, request, skipOnRequest = true)
.await()

/**
* Fills the gap between the loaded messages and the requested messages.
* This is used to keep the messages sorted by date and avoid gaps in the pagination.
*
* @param messageLimit The limit of messages inside the channel that should be requested.
* @param loadedMessages The list of messages that were loaded before the request.
* @param requestedMessages The list of messages that were loaded by the previous request.
*/
private fun fillTheGap(
messageLimit: Int,
loadedMessages: List<Message>,
requestedMessages: List<Message>,
) {
if (loadedMessages.isEmpty() || requestedMessages.isEmpty() || messageLimit <= 0) return
coroutineScope.launch {
val loadedMessageIds = loadedMessages
.filter { it.getCreatedAtOrNull() != null }
.sortedBy { it.getCreatedAtOrDefault(NEVER) }
.map { it.id }
val requestedMessageIds = requestedMessages
.filter { it.getCreatedAtOrNull() != null }
.sortedBy { it.getCreatedAtOrDefault(NEVER) }
.map { it.id }
val intersection = loadedMessageIds.intersect(requestedMessageIds.toSet())
val loadedMessagesOlderDate = loadedMessages.minOf { it.getCreatedAtOrDefault(Date()) }
val loadedMessagesNewerDate = loadedMessages.maxOf { it.getCreatedAtOrDefault(NEVER) }
val requestedMessagesOlderDate = requestedMessages.minOf { it.getCreatedAtOrDefault(Date()) }
val requestedMessagesNewerDate = requestedMessages.maxOf { it.getCreatedAtOrDefault(NEVER) }
if (intersection.isEmpty()) {
when {
loadedMessagesOlderDate > requestedMessagesNewerDate ->
runChannelQueryOnline(
newerWatchChannelRequest(
messageLimit,
requestedMessageIds.last(),
),
)

loadedMessagesNewerDate < requestedMessagesOlderDate ->
runChannelQueryOnline(
olderWatchChannelRequest(
messageLimit,
requestedMessageIds.first(),
),
)

else -> null
}?.onSuccess { fillTheGap(messageLimit, loadedMessages, it.messages) }
}
}
}

private suspend fun runChannelQueryOffline(request: QueryChannelRequest): Channel? {
/* It is not possible to guarantee that the next page of newer messages or the page surrounding a certain
* message is the same as the one on backend, so we force the backend usage */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ internal class LogicRegistry internal constructor(
repos = repos,
userPresence = userPresence,
channelStateLogic = stateLogic,
coroutineScope = coroutineScope,
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ internal class WhenHandleEvent : SynchronizedCoroutineTest {
repos,
false,
channelStateLogic,
testCoroutines.scope,
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1251,8 +1251,6 @@ public final class io/getstream/chat/android/ui/common/utils/extensions/MessageF
}

public final class io/getstream/chat/android/ui/common/utils/extensions/MessageKt {
public static final fun getCreatedAtOrNull (Lio/getstream/chat/android/models/Message;)Ljava/util/Date;
public static final fun getCreatedAtOrThrow (Lio/getstream/chat/android/models/Message;)Ljava/util/Date;
public static final fun isModerationFailed (Lio/getstream/chat/android/models/Message;Lio/getstream/chat/android/models/User;)Z
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ public class MessageComposerController(
private val chatClient: ChatClient = ChatClient.instance(),
private val mediaRecorder: StreamMediaRecorder,
private val fileToUri: (File) -> String,
private val messageLimit: Int,
private val maxAttachmentCount: Int = AttachmentConstants.MAX_ATTACHMENTS_COUNT,
private val maxAttachmentSize: Long = AttachmentConstants.MAX_UPLOAD_FILE_SIZE,
private val messageId: String? = null,
Expand Down Expand Up @@ -359,7 +360,6 @@ public class MessageComposerController(
}

private fun observeChannelState(): Flow<ChannelState> {
val messageLimit = if (messageId != null) 0 else DefaultMessageLimit
logger.d { "[observeChannelState] cid: $channelId, messageId: $messageId, messageLimit: $messageLimit" }
return chatClient.watchChannelAsState(
cid = channelId,
Expand Down Expand Up @@ -968,11 +968,6 @@ public class MessageComposerController(
*/
private val CommandPattern = Pattern.compile("^/[a-z]*$")

/**
* The default limit for messages count in requests.
*/
private const val DefaultMessageLimit: Int = 30

private const val OneSecond = 1000L

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@

package io.getstream.chat.android.ui.common.feature.messages.list

import io.getstream.chat.android.client.extensions.getCreatedAtOrDefault
import io.getstream.chat.android.client.extensions.getCreatedAtOrNull
import io.getstream.chat.android.client.extensions.internal.NEVER
import io.getstream.chat.android.models.Message
import io.getstream.chat.android.ui.common.utils.extensions.getCreatedAtOrThrow

/**
* A SAM designed to evaluate if a date separator should be added between messages.
Expand Down Expand Up @@ -78,7 +79,12 @@ public fun interface DateSeparatorHandler {
message: Message,
separatorTimeMillis: Long,
): Boolean {
return (message.getCreatedAtOrThrow().time - (previousMessage?.getCreatedAtOrThrow()?.time ?: NEVER.time)) >
return (
message.getCreatedAtOrDefault(NEVER).time - (
previousMessage?.getCreatedAtOrNull()?.time
?: NEVER.time
)
) >
separatorTimeMillis
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import io.getstream.chat.android.client.ChatClient
import io.getstream.chat.android.client.channel.state.ChannelState
import io.getstream.chat.android.client.errors.extractCause
import io.getstream.chat.android.client.extensions.cidToTypeAndId
import io.getstream.chat.android.client.extensions.getCreatedAtOrDefault
import io.getstream.chat.android.client.extensions.getCreatedAtOrNull
import io.getstream.chat.android.client.extensions.internal.wasCreatedAfter
import io.getstream.chat.android.client.setup.state.ClientState
import io.getstream.chat.android.client.utils.message.isDeleted
Expand Down Expand Up @@ -89,7 +91,6 @@ import io.getstream.chat.android.ui.common.state.messages.list.SystemMessageItem
import io.getstream.chat.android.ui.common.state.messages.list.ThreadDateSeparatorItemState
import io.getstream.chat.android.ui.common.state.messages.list.TypingItemState
import io.getstream.chat.android.ui.common.state.messages.list.stringify
import io.getstream.chat.android.ui.common.utils.extensions.getCreatedAtOrThrow
import io.getstream.chat.android.ui.common.utils.extensions.onFirst
import io.getstream.chat.android.ui.common.utils.extensions.shouldShowMessageFooter
import io.getstream.log.TaggedLogger
Expand Down Expand Up @@ -384,9 +385,6 @@ public class MessageListController(
}

private fun observeChannelState(): StateFlow<ChannelState?> {
// Using 0 as limit will skip insertion of new messages if we are loading a specific message when
// opening the MessageList screen. Prevents race condition where wrong messages might get loaded.
val messageLimit = if (messageId != null) 0 else messageLimit
JcMinarro marked this conversation as resolved.
Show resolved Hide resolved
logger.d { "[observeChannelState] cid: $cid, messageId: $messageId, messageLimit: $messageLimit" }
return chatClient.watchChannelAsState(
cid = cid,
Expand Down Expand Up @@ -719,7 +717,9 @@ public class MessageListController(
)

if (shouldAddDateSeparator) {
groupedMessages.add(DateSeparatorItemState(message.getCreatedAtOrThrow()))
message.getCreatedAtOrNull()?.let { createdAt ->
groupedMessages.add(DateSeparatorItemState(createdAt))
}
}

if (message.isSystem() || (message.isError() && !message.isModerationBounce())) {
Expand Down Expand Up @@ -754,7 +754,9 @@ public class MessageListController(
}

if (shouldAddDateSeparatorInEmptyThread) {
groupedMessages.add(DateSeparatorItemState(message.getCreatedAtOrThrow()))
message.getCreatedAtOrNull()?.let { createdAt ->
groupedMessages.add(DateSeparatorItemState(createdAt))
}
}

if (isThreadWithNoReplies) {
Expand All @@ -764,7 +766,7 @@ public class MessageListController(
if (index == 0 && isInThread && isThreadWithNoReplies) {
groupedMessages.add(
ThreadDateSeparatorItemState(
date = message.createdAt ?: message.createdLocallyAt ?: Date(),
date = message.getCreatedAtOrDefault(Date()),
replyCount = message.replyCount,
),
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import io.getstream.chat.android.core.internal.InternalStreamChatApi
import io.getstream.chat.android.models.Message
import io.getstream.chat.android.models.MessageModerationAction
import io.getstream.chat.android.models.User
import java.util.Date

/**
* @return if the message was sent by current user.
Expand All @@ -32,21 +31,6 @@ public fun Message.isMine(chatClient: ChatClient): Boolean = chatClient.clientSt
@InternalStreamChatApi
public fun Message.isMine(currentUser: User?): Boolean = currentUser?.id == user.id

/**
* @return when the message was created or throw an exception.
*/
public fun Message.getCreatedAtOrThrow(): Date {
val created = createdAt ?: createdLocallyAt
return checkNotNull(created) { "a message needs to have a non null value for either createdAt or createdLocallyAt" }
}

/**
* @return when the message was created or null.
*/
public fun Message.getCreatedAtOrNull(): Date? {
return createdAt ?: createdLocallyAt
}

/**
* @return if the message failed at moderation or not.
*/
Expand Down
Loading
Loading