Skip to content
Open
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 @@ -32,7 +32,7 @@ internal class AttachmentUrlValidator(private val attachmentHelper: AttachmentHe
return newMessages.map { newMessage -> updateValidAttachmentsUrl(newMessage, oldMessages[newMessage.id]) }
}

private fun updateValidAttachmentsUrl(newMessage: Message, oldMessage: Message?): Message {
internal fun updateValidAttachmentsUrl(newMessage: Message, oldMessage: Message?): Message {
return if (newMessage.attachments.isEmpty() || oldMessage == null) {
newMessage
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -398,8 +398,8 @@ internal class ChannelEventHandlerImpl(
}

private fun updateMessageWithReaction(message: Message) {
state.updateMessageById(message.id) { oldMessage ->
message.copy(ownReactions = oldMessage.ownReactions)
state.updateMessageFromEvent(message) { old, new ->
new.copy(ownReactions = old.ownReactions)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -313,20 +313,20 @@ internal class ChannelLogicImpl(
// User's window was already trimmed away from the latest (insideSearch set by
// trimNewestMessages, or a prior jump-to-message). Stay at current position;
// refresh the "jump to latest" cache with the server's current latest page.
state.upsertCachedLatestMessages(sortedMessages)
state.upsertCachedLatestMessages(sortedMessages, preserveAttachmentUrls = true)
}
hasGap(currentMessages, sortedMessages) -> {
// Incoming page is newer than the current window with no overlap. Inserting the
// incoming messages would create a fragmented list. Instead, treat the user's
// position as a mid-page: store the incoming as the "latest" cache and signal the UI.
state.upsertCachedLatestMessages(sortedMessages)
state.upsertCachedLatestMessages(sortedMessages, preserveAttachmentUrls = true)
state.setInsideSearch(true)
state.paginationManager.setEndOfNewerMessages(false)
}
else -> {
// Incoming messages are contiguous with (or overlap) the current window.
// Upsert preserves the user's scroll position while adding/updating messages.
state.upsertMessages(sortedMessages)
state.upsertMessages(sortedMessages, preserveAttachmentUrls = true)
state.paginationManager.setEndOfOlderMessages(channel.messages.size < messageLimit)
}
}
Expand Down Expand Up @@ -382,7 +382,7 @@ internal class ChannelLogicImpl(
}

query.isFilteringNewerMessages() -> {
// Loading newer messages - upsert
// Loading newer messages - append
state.upsertMessages(channel.messages)
state.trimOldestMessages()
val endReached = query.messagesLimit() > channel.messages.size
Expand All @@ -394,7 +394,7 @@ internal class ChannelLogicImpl(
}

query.filteringOlderMessages() -> {
// Loading older messages - prepend; ceiling does not change
// Loading older messages - prepend
state.upsertMessages(channel.messages)
state.trimNewestMessages()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ 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
import io.getstream.chat.android.client.internal.state.message.attachments.internal.AttachmentUrlValidator
import io.getstream.chat.android.client.internal.state.plugin.state.channel.internal.ChannelStateImpl.Companion.CACHED_LATEST_MESSAGES_LIMIT
import io.getstream.chat.android.client.internal.state.plugin.state.channel.internal.ChannelStateImpl.Companion.TRIM_BUFFER
import io.getstream.chat.android.client.internal.state.utils.internal.combineStates
Expand Down Expand Up @@ -88,6 +89,7 @@ internal class ChannelStateImpl(
private val liveLocations: StateFlow<List<Location>>,
private val messageLimit: Int?,
val paginationManager: MessagesPaginationManager = MessagesPaginationManagerImpl(),
private val attachmentUrlValidator: AttachmentUrlValidator = AttachmentUrlValidator(),
) : ChannelState {

override val cid: String = "$channelType:$channelId"
Expand Down Expand Up @@ -321,6 +323,8 @@ internal class ChannelStateImpl(
/**
* Upserts a single message into the current state.
* Uses optimized single-element upsert with binary search insertion.
* When updating an existing message, valid attachment URLs from the old message are preserved
* to prevent unnecessary image reloads caused by CDN signature changes.
*
* @param message The message to upsert.
*/
Expand All @@ -334,6 +338,7 @@ internal class ChannelStateImpl(
element = message,
idSelector = Message::id,
comparator = MESSAGE_COMPARATOR,
update = { old -> preserveAttachmentUrls(message, old) },
)
}
// Update can be called for "ephemeral" messages (ex. Shuffle Giphy)
Expand All @@ -343,6 +348,7 @@ internal class ChannelStateImpl(
element = message,
idSelector = Message::id,
comparator = MESSAGE_COMPARATOR,
update = { old -> preserveAttachmentUrls(message, old) },
)
}
}
Expand All @@ -356,8 +362,11 @@ internal class ChannelStateImpl(
* This is guaranteed when messages come from API responses or database queries.
*
* @param messages The list of messages to upsert (must be sorted by createdAt).
* @param preserveAttachmentUrls When `true`, valid attachment URLs from existing messages in state are
* preserved to prevent unnecessary image reloads caused by CDN signature changes. Defaults to `false`
* for pagination performance; set to `true` for reconnection/sync paths where messages may overlap.
*/
fun upsertMessages(messages: List<Message>) {
fun upsertMessages(messages: List<Message>, preserveAttachmentUrls: Boolean = false) {
val messagesToUpsert = messages.filterNot { shouldIgnoreUpsertion(it) }
if (messagesToUpsert.isEmpty()) return
for (message in messagesToUpsert) {
Expand All @@ -366,8 +375,13 @@ internal class ChannelStateImpl(
message.poll?.let { registerPollForMessage(it, message.id) }
}
_messages.update { current ->
val enriched = if (preserveAttachmentUrls) {
preserveAttachmentUrls(messagesToUpsert, current)
} else {
messagesToUpsert
}
current.mergeSorted(
other = messagesToUpsert,
other = enriched,
idSelector = Message::id,
comparator = MESSAGE_COMPARATOR,
)
Expand All @@ -377,7 +391,8 @@ internal class ChannelStateImpl(
/**
* Upserts a single message into the cached latest messages state.
* The cached messages are bounded to [CACHED_LATEST_MESSAGES_LIMIT] to prevent unbounded growth
* while the user is in search mode.
* while the user is in search mode. When updating an existing message, valid attachment URLs
* from the old message are preserved to prevent unnecessary image reloads.
*
* @param message The message to upsert.
*/
Expand All @@ -392,17 +407,22 @@ internal class ChannelStateImpl(
maxSize = CACHED_LATEST_MESSAGES_LIMIT,
idSelector = Message::id,
comparator = MESSAGE_COMPARATOR,
update = { old -> preserveAttachmentUrls(message, old) },
)
}
}

/**
* Updates a message in the current state. Does nothing if the message does not exist.
* Valid attachment URLs from the existing message are preserved to prevent unnecessary
* image reloads caused by CDN signature changes.
*
* @param message The message to update.
*/
fun updateMessage(message: Message) {
updateMessageById(message.id) { message }
updateMessageById(message.id) { old ->
preserveAttachmentUrls(message, old)
}
Comment on lines 422 to +425
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

The URL-preserved Message never escapes these helpers.

Both update paths only apply preserveAttachmentUrls(...) inside the internal lists. Callers still hold the raw event payload, so any secondary fan-out can reintroduce the re-signed imageUrl; MessageUpdatedEvent -> updateQuotedMessageReferences(...) is the current example, which means quoted reply previews can still flicker. Return the merged Message, or preserve again inside updateQuotedMessageReferences.

Also applies to: 457-459

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateImpl.kt`
around lines 422 - 425, The preserved-URL Message produced by
preserveAttachmentUrls(...) is only applied inside internal lists and never
returned to callers, so external code (e.g., updateQuotedMessageReferences) can
still see and propagate the original re-signed imageUrl; modify updateMessage
and the other update path (the one at lines indicated in the review) so that
updateMessageById(...) returns the merged/updated Message (apply
preserveAttachmentUrls and return that Message) and propagate that returned
Message out to callers, or alternatively call preserveAttachmentUrls again
inside updateQuotedMessageReferences(...) to ensure quoted/secondary fan-out
always receives the URL-preserved Message; reference the functions
updateMessage, updateMessageById, preserveAttachmentUrls, and
updateQuotedMessageReferences when making the change.

}

/**
Expand All @@ -419,6 +439,27 @@ internal class ChannelStateImpl(
_pinnedMessages.update { it.updateIf({ msg -> msg.id == id }, transform) }
}

/**
* Replaces an existing message with [eventMessage] while preserving valid attachment URLs
* from the old message, then applies the [enrich] function for caller-specific field merging.
*
* Use this for event-driven full-message replacements (e.g. reaction events) where the event
* payload may carry attachment URLs with different CDN signatures than the ones already in state.
*
* @param eventMessage The new message from the event payload.
* @param enrich A function that receives the old message and the URL-preserved new message,
* returning the final merged message. Typically used to preserve fields like `ownReactions`.
*/
fun updateMessageFromEvent(
eventMessage: Message,
enrich: (old: Message, new: Message) -> Message,
) {
updateMessageById(eventMessage.id) { old ->
val urlPreserved = preserveAttachmentUrls(eventMessage, old)
enrich(old, urlPreserved)
}
}

/**
* Hard deletes a message from the current state.
* Note: Soft deletes are handled via [updateMessage].
Expand Down Expand Up @@ -661,6 +702,8 @@ internal class ChannelStateImpl(

/**
* Adds pinned messages to the current pinned messages list.
* When updating an existing pinned message, valid attachment URLs from the old message
* are preserved to prevent unnecessary image reloads caused by CDN signature changes.
*
* @param pinnedMessages The list of pinned messages to add.
*/
Expand All @@ -679,6 +722,7 @@ internal class ChannelStateImpl(
element = pinnedMessage,
idSelector = Message::id,
comparator = compareBy { it.pinnedAt },
update = { old -> preserveAttachmentUrls(pinnedMessage, old) },
)
}
result
Expand Down Expand Up @@ -1411,14 +1455,24 @@ internal class ChannelStateImpl(
*
* Called during reconnection to refresh the "jump to latest" cache with the server's
* current latest page without disturbing the user's active scroll position.
*
* @param messages The list of messages to merge.
* @param preserveAttachmentUrls When `true`, valid attachment URLs from existing cached messages are
* preserved to prevent unnecessary image reloads caused by CDN signature changes. Defaults to `false`;
* set to `true` for reconnection/sync paths where messages may overlap.
*/
fun upsertCachedLatestMessages(messages: List<Message>) {
fun upsertCachedLatestMessages(messages: List<Message>, preserveAttachmentUrls: Boolean = false) {
if (messages.isEmpty()) return
val messagesToUpsert = messages.filterNot { shouldIgnoreUpsertion(it) }
if (messagesToUpsert.isEmpty()) return
_cachedLatestMessages.update { current ->
val enriched = if (preserveAttachmentUrls) {
preserveAttachmentUrls(messagesToUpsert, current)
} else {
messagesToUpsert
}
current.mergeSorted(
other = messagesToUpsert,
other = enriched,
idSelector = Message::id,
comparator = MESSAGE_COMPARATOR,
).takeLast(CACHED_LATEST_MESSAGES_LIMIT)
Expand Down Expand Up @@ -1556,6 +1610,25 @@ internal class ChannelStateImpl(
FROM_NEWEST,
}

/**
* Preserves valid attachment URLs from [oldMessage] onto [newMessage].
* Prevents unnecessary image reloads when the backend delivers events with different CDN signatures.
*/
private fun preserveAttachmentUrls(newMessage: Message, oldMessage: Message): Message =
attachmentUrlValidator.updateValidAttachmentsUrl(newMessage, oldMessage)

/**
* Preserves valid attachment URLs for a batch of messages.
* Builds a lookup map from [oldMessages] and enriches each message in [newMessages].
*/
private fun preserveAttachmentUrls(
newMessages: List<Message>,
oldMessages: List<Message>,
): List<Message> {
val oldById = oldMessages.associateBy(Message::id)
return attachmentUrlValidator.updateValidAttachmentsUrl(newMessages, oldById)
}

private companion object {
/**
* Hard limit for cached latest messages to prevent unbounded memory growth while in search mode.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,9 @@ internal fun <T, ID> List<T>.upsertSortedBounded(
maxSize: Int,
idSelector: (T) -> ID,
comparator: Comparator<in T>,
update: (old: T) -> T = { element },
): List<T> {
val result = upsertSorted(element, idSelector, comparator)
val result = upsertSorted(element, idSelector, comparator, update)
return if (result.size > maxSize) result.takeLast(maxSize) else result
}

Expand Down
Loading
Loading