Improve handling of pending and local-only messages#6252
Open
VelikovPetar wants to merge 30 commits intov7from
Open
Improve handling of pending and local-only messages#6252VelikovPetar wants to merge 30 commits intov7from
VelikovPetar wants to merge 30 commits intov7from
Conversation
Co-Authored-By: Claude <noreply@anthropic.com>
…ck_from_background
…ck_from_background
…ck_from_background
…fter_coming_back_from_background' into bug/AND-1113_fix_channel_jumps_after_coming_back_from_background
…ck_from_background
- 8 @disabled @test stubs covering all SyncStatus values (SYNC_NEEDED, IN_PROGRESS, AWAITING_ATTACHMENTS, FAILED_PERMANENTLY) plus type-based cases (ephemeral/error with COMPLETED) and negative cases (COMPLETED/system) - All stubs disabled with "Wave 1 — implement isLocalOnly() first" message - Satisfies PRES-05 Wave 0 requirement: test class resolvable by Gradle --tests "*.MessageIsLocalOnlyTest" without "no tests found" failure
…reservingLocalOnly() - 12 @disabled @test stubs covering survival cases (FAILED_PERMANENTLY, ephemeral, AWAITING_ATTACHMENTS, pending edit SYNC_NEEDED), collision (server wins), window floor (below-floor excluded, boundary included), empty page (null floor = all local-only), no-DB fallback, DB+state union, COMPLETED exclusion, and DB seed full-replace semantics - Extends ChannelStateImplTestBase to reuse channelState fixture - All stubs disabled with "Wave 2 — implement setMessagesPreservingLocalOnly() first" - Satisfies PRES-01/PRES-04 Wave 0 requirement: test class resolvable by Gradle --tests "*.ChannelStateImplPreservationTest" without build failure
- Create MessageLocalOnlyExt.kt with internal fun Message.isLocalOnly() - Covers SYNC_NEEDED, IN_PROGRESS, AWAITING_ATTACHMENTS, FAILED_PERMANENTLY, type==ephemeral, and type==error; returns false for COMPLETED+regular/system - Remove @disabled from all 8 MessageIsLocalOnlyTest stubs and fill real assertions - All 8 tests pass
- Add selectLocalOnlyMessagesForChannel to MessageRepository interface - NoOpMessageRepository overrides it returning emptyList() - MessageDao.selectLocalOnlyForChannel @query without default args (Room constraint) - DatabaseMessageRepository implements with explicit SyncStatus int constants and MessageType string constants, maps entities via existing toMessage() - RepositoryFacade picks up new method automatically via 'by messageRepository' delegation
…n to 102 - ChannelEntity gains oldestLoadedDate: Date? = null as last field (floor for local-only message window; written in Phase 1, read in Phase 2) - ChatDatabase version bumped 101 -> 102 - fallbackToDestructiveMigration() already in place — no migration script needed - No public API surface changes; no legacy path touched
- 12 test cases covering all preservation scenarios - Cases: FAILED_PERMANENTLY/ephemeral/AWAITING_ATTACHMENTS survival, ID collision (server wins), window floor filtering (below excluded, above included, boundary included), null floor (all local-only included), no-DB fallback, state+DB union dedup, COMPLETED not preserved, setMessages full-replace semantics unchanged - Tests fail to compile until implementation is added (TDD RED phase)
…Impl
- Adds internal fun setMessagesPreservingLocalOnly(incoming, localOnlyFromDb, windowFloor)
immediately after setMessages in ChannelStateImpl
- Uses _messages.update { } (CAS atomic read-modify-write, no value assignment)
- Step 1: gathers local-only from current in-memory state via isLocalOnly() predicate
- Step 2: unions with localOnlyFromDb, deduplicates by ID
- Step 3: server wins on ID collision (local-only with matching incoming ID dropped)
- Step 4: applies windowFloor filter (null = include all; createdAt < floor = exclude)
- Step 5: filters incoming through existing shouldIgnoreUpsertion() guard
- Step 6: merges and sorts with existing MESSAGE_COMPARATOR
- Step 7: syncs quotedMessagesMap and poll indexes (mirrors setMessages post-loop)
- setMessages is NOT modified (retains full-replace semantics for DB-seed path)
- All 12 ChannelStateImplPreservationTest cases pass; all 8 MessageIsLocalOnlyTest pass
…ry layer - ChannelDao: add updateOldestLoadedDate @query targeting oldestLoadedDate column - ChannelRepository: add updateOldestLoadedDateForChannel interface method - NoOpChannelRepository: add no-op override returning Unit - DatabaseChannelRepository: implement via channelDao.updateOldestLoadedDate - RepositoryFacade picks up new method automatically via ChannelRepository delegation
…l call sites - updateMessages: change signature to suspend, add localOnlyFromDb and windowFloor params - Three setMessages call sites in updateMessages replaced with setMessagesPreservingLocalOnly - DB-seed path (updateDataForChannel/shouldRefreshMessages=true) unchanged at state.setMessages - onQueryChannelResult: prefetch localOnlyFromDb and derive windowFloor in coroutineScope.launch - windowFloor derived from min(channel.messages.createdAt); null when incoming is empty - Floor persistence: repository.updateOldestLoadedDateForChannel called when windowFloor != null - ChannelLogicImplTest: add PreservationCallSites nested class with 4 new test cases - ChannelLogicImplTest: update 3 existing call-site tests to verify setMessagesPreservingLocalOnly - ChannelLogicLegacyImpl untouched
- Add single-column SELECT @query for oldestLoadedDate column - Method returns Date? matching ChannelEntity field nullability - Placed immediately after updateOldestLoadedDate for co-location
- Add interface method to ChannelRepository returning Date? - Add no-op override returning null in NoOpChannelRepository - Add DB implementation delegating to channelDao.selectOldestLoadedDate - RepositoryFacade picks up via ChannelRepository by channelsRepository delegation
- Updated filteringOlderMessages and isFilteringNewerMessages tests to verify setMessagesPreservingLocalOnly replaces upsertMessages - Added new tests for endReached=true path (null floor), reconnect path (isChannelsStateUpdate=false), and else-upsert branch in updateDataForChannel - Updated DB-seed tests to pass isChannelsStateUpdate=true explicitly - Added isNull import for Mockito RED phase: 8 tests failing as expected
…ches - filteringOlderMessages branch: replace upsertMessages with setMessagesPreservingLocalOnly(messages, localOnlyFromDb, windowFloor) - isFilteringNewerMessages endReached=false: replace upsertMessages with setMessagesPreservingLocalOnly(messages, localOnlyFromDb, windowFloor) - isFilteringNewerMessages endReached=true: replace upsertMessages with setMessagesPreservingLocalOnly(messages, localOnlyFromDb, null) — null floor signals no-ceiling (reached latest messages) - trimOldestMessages / trimNewestMessages calls preserved in both branches GREEN: pagination preservation tests pass (Task 1 complete)
- Prefetch localOnlyFromDb and persistedFloor from repository at top of messageLimit > 0 block - shouldRefreshMessages branch: isChannelsStateUpdate=true uses setMessages (DB-seed; local-only already in DB), isChannelsStateUpdate=false uses setMessagesPreservingLocalOnly (SyncManager reconnect) - else upsert branch: replaced upsertMessages with setMessagesPreservingLocalOnly using persistedFloor - insideSearch and hasGap branches unchanged (route to cache, not active messages) - Removed TODO(Phase 2) comment from updateMessages — floor is now read in updateDataForChannel GREEN: all 92 tests pass — full trigger coverage complete
- 4 tests verifying filteringOlderMessages and isFilteringNewerMessages branches call setMessagesPreservingLocalOnly (not upsertMessages) - end-reached test verifies null windowFloor (isNull()) - mid-page test verifies advanceNewestLoadedDate still called
- 3 tests covering updateDataForChannel reconnect path, DB-seed path, and else-upsert branch - reconnect (isChannelsStateUpdate=false) verifies setMessagesPreservingLocalOnly - DB-seed (isChannelsStateUpdate=true) verifies setMessages full-replace (regression guard) - else-upsert branch verifies setMessagesPreservingLocalOnly, never upsertMessages - Full testDebugUnitTest suite: BUILD SUCCESSFUL, zero failures
…anches Pagination branches (filteringOlderMessages, isFilteringNewerMessages) were incorrectly calling setMessagesPreservingLocalOnly which *replaces* the entire message list with only the new page. Instead they must *merge* the new page into existing state so prior pagination results are preserved. Add upsertMessagesPreservingLocalOnly to ChannelStateImpl — identical to setMessagesPreservingLocalOnly except step 6 keeps existingServer messages alongside incoming + local-only, preventing page drops on pagination. Also fix endReached branch to pass windowFloor instead of null: the floor must not reset when we reach the newest end; it stays at the oldest loaded point across the full pagination session. Tests updated to assert upsertMessagesPreservingLocalOnly for pagination paths and setMessagesPreservingLocalOnly for set/replace paths only. Fix flaky else-branch test to use ID overlap for deterministic hasGap=false. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
Contributor
|
DB Entities have been updated. Do we need to upgrade DB Version? |
Contributor
SDK Size Comparison 📏
|
WalkthroughThis pull request refactors the message pagination and state management system. It introduces a new MessagesPaginationManager to centralize pagination logic, adds local-only message handling, removes the legacy PendingMessagesManager, updates database schema to version 102, and refactors ChannelLogicImpl and ChannelStateImpl to use the new pagination patterns throughout. Changes
Sequence DiagramsequenceDiagram
participant Client
participant QueryListener as QueryChannelListener
participant Logic as ChannelLogicImpl
participant Manager as MessagesPaginationManager
participant State as ChannelStateImpl
participant Database as MessageDatabase
participant Server as Server API
Client->>QueryListener: onQueryChannelRequest()
QueryListener->>Logic: updateStateFromDatabase()
Logic->>Database: selectLocalOnlyMessagesForChannel()
Database-->>Logic: local-only messages
Logic->>State: setLocalOnlyMessages()
Logic->>State: setOldestMessage()
QueryListener->>Manager: begin(query)
Manager->>Manager: determine pagination mode<br/>(LESS_THAN/GREATER_THAN/AROUND_ID)
Manager->>Manager: update loading flags
Logic->>Server: queryChannel(request)
Server-->>Logic: Result<Channel>
Logic->>Manager: end(query, result)
Manager->>Manager: update pagination bounds<br/>(oldestMessage/newestMessage)
Manager->>Manager: set hasLoadedAll flags
Logic->>State: updateDataForChannel()<br/>(with new messages)
State->>State: merge messages from three sources<br/>(server + local-only + pending)
State->>State: filter by pagination bounds
State-->>Client: emit updated messages flow
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can disable the changed files summary in the walkthrough.Disable the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateImplTestBase.kt (1)
92-104: MakecreateLocalOnlyMessageflexible for both local-only paths.Right now the helper is local-only only because of
type = EPHEMERAL, whilesyncStatus = COMPLETED. Making status/type configurable improves test clarity and coverage for sync-status-based local-only behavior.🧪 Suggested refactor
-protected fun createLocalOnlyMessage(index: Int, timestamp: Long): Message = +protected fun createLocalOnlyMessage( + index: Int, + timestamp: Long, + syncStatus: SyncStatus = SyncStatus.IN_PROGRESS, + type: MessageType = MessageType.EPHEMERAL, +): Message = randomMessage( id = "local_only_$index", cid = CID, createdAt = Date(timestamp), createdLocallyAt = null, - syncStatus = SyncStatus.COMPLETED, - type = MessageType.EPHEMERAL, + syncStatus = syncStatus, + type = type, parentId = null, showInChannel = true, shadowed = false, deletedAt = null, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateImplTestBase.kt` around lines 92 - 104, The helper createLocalOnlyMessage is hardcoded to use type = MessageType.EPHEMERAL and syncStatus = SyncStatus.COMPLETED, which prevents testing the other local-only path; update createLocalOnlyMessage to accept optional parameters for message type and sync status (e.g., messageType: MessageType = MessageType.EPHEMERAL, syncStatus: SyncStatus = SyncStatus.COMPLETED) and pass them through to the call to randomMessage so tests can construct messages that are local-only via either type or syncStatus.stream-chat-android-client/src/main/java/io/getstream/chat/android/client/persistance/repository/MessageRepository.kt (1)
203-212: Add thread-expectation notes to the new public API KDoc.The new API KDoc is helpful, but it should also state threading/dispatcher expectations for callers.
✍️ Suggested KDoc update
/** * Returns all messages for [cid] that are local-only: syncStatus is one of * SYNC_NEEDED, IN_PROGRESS, AWAITING_ATTACHMENTS, FAILED_PERMANENTLY, or type is * "ephemeral" or "error". Used by the preservation mechanism before a server response * replaces the active message window. + * + * Threading: safe to call from any coroutine context; implementations may hit local storage. + * State notes: result may include messages not present in the active in-memory window. * * `@param` cid The channel ID (format "type:id"). * `@return` List of local-only messages, unordered. */As per coding guidelines: "Document public APIs with KDoc, including thread expectations and state notes".
🤖 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/persistance/repository/MessageRepository.kt` around lines 203 - 212, Update the KDoc for the public suspend function selectLocalOnlyMessagesForChannel to include thread/dispatcher expectations and state notes: explicitly state that although it is suspendable it performs local database access and should be called from a background coroutine (e.g., Dispatchers.IO or a non-main thread), whether it is safe to call from main/UI, and that it returns an unordered snapshot of local-only messages (no network calls, no mutation of state). Place this note in the function KDoc so callers know the expected coroutine context and the function’s side-effect and ordering guarantees.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/offline/repository/database/internal/ChatDatabase.kt`:
- Line 91: The Room database was bumped to version = 102 but getDatabase() still
relies on fallbackToDestructiveMigration, which will erase user data on upgrade;
add an explicit Migration object for 101→102 and register it via
addMigrations(...) on the Room.databaseBuilder in getDatabase() (or remove
fallbackToDestructiveMigration and supply all needed migrations), ensuring the
Migration handles the new draft message & pagination bound schema changes
declared in ChatDatabase (version = 102).
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/MessagesPaginationState.kt`:
- Around line 193-200: When updating pagination state in MessagesPaginationState
(inside the block that computes hasLoadedAllPreviousMessages and calls
current.copy), avoid clearing the existing floor when the older-page response is
an empty list: compute a newOldestMessage that uses current.oldestMessage if
messages.isEmpty(), otherwise use the response oldestMessage, and pass that
newOldestMessage into current.copy (instead of unconditionally setting
oldestMessage to the response value). Keep the existing
hasLoadedAllPreviousMessages logic (messages.size < query.messagesLimit()) and
leave the other flags (isLoadingNextMessages, isLoadingPreviousMessages,
isLoadingMiddleMessages) unchanged.
In
`@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/channel/internal/ChannelLogicImplTest.kt`:
- Around line 686-699: The test fixture returns messages in ascending order but
the real MessageDao.messagesForChannel returns messages in DESC order, so change
the stubbed messages used in ChannelLogicImplTest (the list passed to
whenever(repository.selectMessagesForChannel(...))) to be ordered DESC (newest
first) so that paginationManager.setOldestMessage is asserted against the
correct item; locate the stub in the test that sets
whenever(repository.selectMessagesForChannel(any(), any())) and reverse or
reorder the messages list (used with randomMessage id "m1"/"m2") so
messages.first() represents the newest and messages.last() the oldest to match
MessageDao.messagesForChannel behavior referenced by updateStateFromDatabase and
paginationManager.setOldestMessage.
In
`@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateImplPendingMessagesTest.kt`:
- Line 145: In ChannelStateImplPendingMessagesTest, the fixture named ceiling is
created with id = "floor" which is confusing; update the call to randomMessage
that constructs the ceiling variable to use a matching id (e.g., id = "ceiling")
so the variable name and message id align for clearer test output and easier
debugging when assertions fail.
---
Nitpick comments:
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/persistance/repository/MessageRepository.kt`:
- Around line 203-212: Update the KDoc for the public suspend function
selectLocalOnlyMessagesForChannel to include thread/dispatcher expectations and
state notes: explicitly state that although it is suspendable it performs local
database access and should be called from a background coroutine (e.g.,
Dispatchers.IO or a non-main thread), whether it is safe to call from main/UI,
and that it returns an unordered snapshot of local-only messages (no network
calls, no mutation of state). Place this note in the function KDoc so callers
know the expected coroutine context and the function’s side-effect and ordering
guarantees.
In
`@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateImplTestBase.kt`:
- Around line 92-104: The helper createLocalOnlyMessage is hardcoded to use type
= MessageType.EPHEMERAL and syncStatus = SyncStatus.COMPLETED, which prevents
testing the other local-only path; update createLocalOnlyMessage to accept
optional parameters for message type and sync status (e.g., messageType:
MessageType = MessageType.EPHEMERAL, syncStatus: SyncStatus =
SyncStatus.COMPLETED) and pass them through to the call to randomMessage so
tests can construct messages that are local-only via either type or syncStatus.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 95d85e40-860e-4ea3-95ad-7ad586800194
📒 Files selected for processing (23)
stream-chat-android-client/api/stream-chat-android-client.apistream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/offline/repository/database/internal/ChatDatabase.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/offline/repository/domain/message/internal/DatabaseMessageRepository.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/offline/repository/domain/message/internal/MessageDao.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/listener/internal/QueryChannelListenerState.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/channel/internal/ChannelLogicImpl.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateImpl.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/MessagesPaginationState.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/PendingMessagesManager.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/persistance/repository/MessageRepository.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/persistance/repository/noop/NoOpChannelRepository.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/persistance/repository/noop/NoOpMessageRepository.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/utils/message/MessageUtils.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/channel/controller/attachment/UploadAttachmentsIntegrationTests.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/channel/internal/ChannelLogicImplTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateImplLocalOnlyMessagesTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateImplMessagesTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateImplNonChannelStatesTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateImplPendingMessagesTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateImplTestBase.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/MessagesPaginationManagerImplTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/PendingMessagesManagerTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/utils/message/MessageUtilsTest.kt
💤 Files with no reviewable changes (3)
- stream-chat-android-client/src/main/java/io/getstream/chat/android/client/persistance/repository/noop/NoOpChannelRepository.kt
- stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/PendingMessagesManager.kt
- stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/PendingMessagesManagerTest.kt
.../getstream/chat/android/client/internal/offline/repository/database/internal/ChatDatabase.kt
Show resolved
Hide resolved
.../chat/android/client/internal/state/plugin/state/channel/internal/MessagesPaginationState.kt
Show resolved
Hide resolved
...eam/chat/android/client/internal/state/plugin/logic/channel/internal/ChannelLogicImplTest.kt
Outdated
Show resolved
Hide resolved
...d/client/internal/state/plugin/state/channel/internal/ChannelStateImplPendingMessagesTest.kt
Outdated
Show resolved
Hide resolved
- Fix ChannelLogicImplTest: stub messages now ordered DESC (newest first) to match MessageDao.messagesForChannel behaviour; assertion updated to verify setOldestMessage receives the oldest message (m1/1000ms). - Fix ChannelStateImplPendingMessagesTest: ceiling fixture id corrected from "floor" to "ceiling" to avoid confusion when assertions fail. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Goal
Improve the handling of pending and local-only messages (ephemeral, error, failed, in-progress) so they are never lost when a server response replaces the active message window, and so that the pagination state accurately tracks the currently loaded range to filter these messages correctly.
Note: Review after #6234 is merged.
Implementation
Local-only message preservation
Introduces the concept of "local-only messages" — messages that are only known to the client and are never returned by the server after the initial send attempt. This covers:
SYNC_NEEDED,IN_PROGRESS,AWAITING_ATTACHMENTS,FAILED_PERMANENTLYephemeral(e.g. Giphy previews) orerrorA new
selectLocalOnlyMessagesForChannel(cid)method is added toMessageRepository, backed by a new DAO query (selectBySyncStatusOrTypeForChannel) and a database migration to version 102. WhenupdateStateFromDatabaseis called, these messages are loaded from the DB and stored separately inChannelStateImpl.localOnlyMessages. They are filtered by the current pagination window (floor/ceiling) before being merged into the publicmessagesflow, alongside regular messages.Pagination state refactoring (
MessagesPaginationManager)Replaces the five ad-hoc
MutableStateFlowfields for loading/end-of-page state inChannelStateImpl(_loading,_loadingOlderMessages,_loadingNewerMessages,_endOfOlderMessages,_endOfNewerMessages) with a singleMessagesPaginationStatedata class managed by a newMessagesPaginationManagerinterface (MessagesPaginationManagerImpl). The manager tracks:oldestMessage/newestMessage— the current pagination window (floor and ceiling)hasLoadedAllPreviousMessages/hasLoadedAllNextMessages— whether the ends have been reachedisLoadingPreviousMessages/isLoadingNextMessages/isLoadingMiddleMessages— loading indicatorsisInWindow(message)— helper to check if a message falls within the currently loaded rangebegin(query)andend(query, result)are called around each paginated channel query to atomically transition loading states.ChannelLogicImpl.setPaginationDirectionandonQueryChannelResultare updated to delegate to the manager. The removedPendingMessagesManagerdate-range logic (advanceOldestLoadedDate,setNewestLoadedDate,advanceNewestLoadedDate) is superseded by the manager's window tracking.Call-order fix in
QueryChannelListenerStatesetPaginationDirectionis now called beforeupdateStateFromDatabase, ensuring the pagination state (oldest message floor) is initialized from the DB before the window filter is applied to local-only messages.🎨 UI Changes
Add relevant videos
local-only-before.mp4
local-only-after.mp4
Testing
The changes are covered by unit tests across three test classes:
ChannelStateImplLocalOnlyMessagesTest(new) — 14 tests covering visibility, floor/ceiling window filtering, upsert (ephemeral path), and delete cleanup for local-only messages.ChannelStateImplMessagesTest— extended with tests forupsertCachedLatestMessagesand the gap-detection /updateDataForChannelscenarios (refresh, contiguous upsert, gap → cache, insideSearch → cache).ChannelLogicImplTest— updated to mockMessagesPaginationManagerand verify delegation; added two new tests forupdateStateFromDatabasecoveringsetOldestMessagewith messages andsetOldestMessage(null)for empty results.🎉 GIF
Please provide a suitable gif that describes your work on this pull request
Summary by CodeRabbit
New Features
Bug Fixes
Refactor