diff --git a/apps/flipcash/features/balance/src/main/kotlin/com/flipcash/app/balance/internal/WalletViewModel.kt b/apps/flipcash/features/balance/src/main/kotlin/com/flipcash/app/balance/internal/WalletViewModel.kt index d471933e78..2f1a212393 100644 --- a/apps/flipcash/features/balance/src/main/kotlin/com/flipcash/app/balance/internal/WalletViewModel.kt +++ b/apps/flipcash/features/balance/src/main/kotlin/com/flipcash/app/balance/internal/WalletViewModel.kt @@ -21,6 +21,7 @@ import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.filterIsInstance +import kotlinx.coroutines.flow.filterNotNull import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.mapNotNull @@ -77,6 +78,12 @@ internal class WalletViewModel @Inject constructor( * short-circuit the wait: if there is already activity to draw, there is nothing to * mistake for a new account — and neither is a held balance, which is a live read of the * account rather than of the cache. + * + * The two caches are separate and settle separately. The `feedSyncState` clause covers the + * activity feed; the *chat* cache backing the tip milestone is covered by + * [onboardingItems] staying null, because the milestone flow withholds an answer until its + * own history has hydrated. A held balance deliberately does not short-circuit that one: + * it says nothing about whether the account has tipped. */ val isAwaitingActivity: Boolean get() = onboardingItems == null || @@ -137,14 +144,21 @@ internal class WalletViewModel @Inject constructor( chatCoordinator.hasEverTipped(), tokenCoordinator.hasAnyBalance, ) { hasReceivedMoney, hasTipped, holdsBalance -> - Event.OnOnboardingItemsUpdated( - items = listOf( - TutorialItem.AddMoney(isCompleted = hasReceivedMoney || holdsBalance), - TutorialItem.ScanTipCard(isCompleted = hasTipped), - ), - holdsBalance = holdsBalance, - ) + // A null tip milestone means the chat cache has not been reconciled yet, and an + // un-hydrated cache reports every account as never having tipped. Emitting nothing + // keeps [State.onboardingItems] null, which holds the whole tab on its spinner rather + // than drawing "Scan a Tip Card" as outstanding to someone who already did it. + hasTipped?.let { + Event.OnOnboardingItemsUpdated( + items = listOf( + TutorialItem.AddMoney(isCompleted = hasReceivedMoney || holdsBalance), + TutorialItem.ScanTipCard(isCompleted = it), + ), + holdsBalance = holdsBalance, + ) + } } + .filterNotNull() .onEach { dispatchEvent(it) } .launchIn(viewModelScope) } diff --git a/apps/flipcash/features/balance/src/test/kotlin/com/flipcash/app/balance/internal/WalletMilestoneGatingTest.kt b/apps/flipcash/features/balance/src/test/kotlin/com/flipcash/app/balance/internal/WalletMilestoneGatingTest.kt new file mode 100644 index 0000000000..e902fa8dda --- /dev/null +++ b/apps/flipcash/features/balance/src/test/kotlin/com/flipcash/app/balance/internal/WalletMilestoneGatingTest.kt @@ -0,0 +1,109 @@ +package com.flipcash.app.balance.internal + +import androidx.arch.core.executor.testing.InstantTaskExecutorRule +import com.flipcash.app.analytics.StubFlipcashAnalytics +import com.flipcash.app.balance.internal.components.TutorialItem +import com.flipcash.app.core.MainCoroutineRule +import com.flipcash.app.core.dispatchers.TestDispatchers +import com.flipcash.app.funding.PurchaseMethodController +import com.flipcash.app.tokens.TokenCoordinator +import com.flipcash.app.userflags.UserFlagsCoordinator +import com.flipcash.services.user.UserManager +import com.flipcash.shared.chat.ChatCoordinator +import com.flipcash.shared.transactionhistory.ActivityFeedCoordinator +import io.mockk.every +import io.mockk.mockk +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runTest +import org.junit.Rule +import org.junit.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.test.assertTrue + +/** + * The tip milestone is read off the chat cache, which reports every account as never having tipped + * until its history has been reconciled. The wallet must not draw that: [WalletViewModel.State. + * onboardingItems] stays null — holding the tab on its loading state — until the chat coordinator + * commits to an answer. + * + * A held balance deliberately does not release the wait here, unlike for the activity feed: it is + * evidence about money, not about tipping. + */ +@OptIn(ExperimentalCoroutinesApi::class) +class WalletMilestoneGatingTest { + + @get:Rule + val instantExecutorRule = InstantTaskExecutorRule() + + @get:Rule + var mainCoroutineRule = MainCoroutineRule(UnconfinedTestDispatcher()) + + private val hasEverTipped = MutableStateFlow(null) + + private val chatCoordinator: ChatCoordinator = mockk(relaxed = true) { + every { hasEverTipped() } returns hasEverTipped + } + private val feedCoordinator: ActivityFeedCoordinator = mockk(relaxed = true) { + every { hasEverReceivedMoney() } returns flowOf(true) + } + private val tokenCoordinator: TokenCoordinator = mockk(relaxed = true) { + every { hasAnyBalance } returns flowOf(true) + } + + private lateinit var dispatchers: TestDispatchers + + private fun createViewModel() = WalletViewModel( + userManager = mockk(relaxed = true), + userFlags = mockk(relaxed = true), + dispatchers = dispatchers, + purchaseMethodController = mockk(relaxed = true), + analytics = StubFlipcashAnalytics(), + chatCoordinator = chatCoordinator, + feedCoordinator = feedCoordinator, + tokenCoordinator = tokenCoordinator, + ) + + @Test + fun `milestones are withheld while the tip milestone is unknown`() = + runTest(mainCoroutineRule.dispatcher) { + dispatchers = TestDispatchers(testScheduler) + + val vm = createViewModel() + advanceUntilIdle() + + assertNull( + vm.stateFlow.value.onboardingItems, + "an un-reconciled chat cache reports every account as never having tipped", + ) + assertTrue( + vm.stateFlow.value.isAwaitingActivity, + "the tab must keep loading rather than draw a milestone it cannot yet answer", + ) + } + + @Test + fun `milestones are published once the tip milestone resolves`() = + runTest(mainCoroutineRule.dispatcher) { + dispatchers = TestDispatchers(testScheduler) + + val vm = createViewModel() + advanceUntilIdle() + hasEverTipped.value = true + advanceUntilIdle() + + val items = vm.stateFlow.value.onboardingItems + assertNotNull(items) + assertEquals( + listOf(true, true), + items.map { it.isCompleted }, + "both milestones read complete: the account holds a balance and has tipped", + ) + assertTrue(items.any { it is TutorialItem.ScanTipCard }) + } +} diff --git a/apps/flipcash/shared/authentication/build.gradle.kts b/apps/flipcash/shared/authentication/build.gradle.kts index 8f80b903dc..0362d2ab58 100644 --- a/apps/flipcash/shared/authentication/build.gradle.kts +++ b/apps/flipcash/shared/authentication/build.gradle.kts @@ -12,6 +12,7 @@ dependencies { implementation(libs.androidx.datastore) implementation(project(":apps:flipcash:shared:appsettings")) + implementation(project(":apps:flipcash:shared:chat")) implementation(project(":apps:flipcash:shared:contacts")) implementation(project(":apps:flipcash:shared:persistence:provider")) implementation(project(":apps:flipcash:shared:push")) diff --git a/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/AuthManager.kt b/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/AuthManager.kt index f07255df63..b725f009f7 100644 --- a/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/AuthManager.kt +++ b/apps/flipcash/shared/authentication/src/main/kotlin/com/flipcash/app/auth/AuthManager.kt @@ -7,6 +7,7 @@ import com.flipcash.app.auth.internal.credentials.PassphraseCredentialManager import com.flipcash.app.contacts.ContactCoordinator import com.flipcash.app.featureflags.FeatureFlagController import com.flipcash.app.persistence.PersistenceProvider +import com.flipcash.shared.chat.ChatCoordinator import com.flipcash.app.push.PushTokenProvider import com.flipcash.app.tokens.TokenCoordinator import com.flipcash.app.userflags.UserFlagsCoordinator @@ -54,6 +55,7 @@ class AuthManager @Inject constructor( private val userFlags: UserFlagsCoordinator, private val profileCoordinator: ProfileCoordinator, private val contactCoordinator: ContactCoordinator, + private val chatCoordinator: ChatCoordinator, private val networkObserver: NetworkConnectivityListener, private val dispatchers: DispatcherProvider, // private val analytics: AnalyticsService, @@ -315,6 +317,11 @@ class AuthManager @Inject constructor( //todo: add account deletion // Wipe server contact set before logout while the session can still authenticate. contactCoordinator.clearServerContactSet() + // Deletion is the only path that erases the local chat cache. Plain logout and account + // switching leave it — the database is per-account, so it never has to be emptied to keep + // accounts apart, and keeping it lets a re-login reconcile rather than refetch. That makes + // this the one caller responsible for the data actually going away. + chatCoordinator.clearCache() return logout() } diff --git a/apps/flipcash/shared/authentication/src/test/kotlin/com/flipcash/app/auth/AuthManagerTest.kt b/apps/flipcash/shared/authentication/src/test/kotlin/com/flipcash/app/auth/AuthManagerTest.kt index d9dfee698a..e883733d94 100644 --- a/apps/flipcash/shared/authentication/src/test/kotlin/com/flipcash/app/auth/AuthManagerTest.kt +++ b/apps/flipcash/shared/authentication/src/test/kotlin/com/flipcash/app/auth/AuthManagerTest.kt @@ -11,6 +11,7 @@ import com.flipcash.app.persistence.PersistenceProvider import com.flipcash.app.push.PushTokenProvider import com.flipcash.app.tokens.TokenCoordinator import com.flipcash.app.userflags.UserFlagsCoordinator +import com.flipcash.shared.chat.ChatCoordinator import com.flipcash.shared.profile.ProfileCoordinator import com.flipcash.services.controllers.AccountController import com.flipcash.services.controllers.ProfileController @@ -65,6 +66,7 @@ class AuthManagerTest { private val userFlags: UserFlagsCoordinator = mockk(relaxed = true) private val profileCoordinator: ProfileCoordinator = mockk(relaxed = true) private val contactCoordinator: ContactCoordinator = mockk(relaxed = true) + private val chatCoordinator: ChatCoordinator = mockk(relaxed = true) private val userManagerState = MutableStateFlow(UserManager.State()) private val networkConnectivityListener = mockk(relaxed = true) @@ -108,6 +110,7 @@ class AuthManagerTest { userFlags = userFlags, profileCoordinator = profileCoordinator, contactCoordinator = contactCoordinator, + chatCoordinator = chatCoordinator, dispatchers = dispatchers, networkObserver = networkConnectivityListener, ) @@ -164,6 +167,27 @@ class AuthManagerTest { verify { userFlags.clearAll() } } + @Test + fun `logout leaves the chat cache in place`() = runTest { + coEvery { credentialManager.logout() } returns Result.success(Unit) + + authManager.logout() + + // The Room file is named from the account entropy, so signing out never has to erase chat + // history to keep accounts apart — and keeping it lets a re-login reconcile what it already + // has instead of rebuilding from one message per chat. + coVerify(exactly = 0) { chatCoordinator.clearCache() } + } + + @Test + fun `deleteAndLogout erases the chat cache`() = runTest { + coEvery { credentialManager.logout() } returns Result.success(Unit) + + authManager.deleteAndLogout() + + coVerify(exactly = 1) { chatCoordinator.clearCache() } + } + @Test fun `non-soft login sets softLoginDisabled flag`() = runTest { val entropy = "dGVzdGVudHJvcHkxMjM0NQ==" diff --git a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/ChatCoordinator.kt b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/ChatCoordinator.kt index 9ac375aa14..5af1d838b8 100644 --- a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/ChatCoordinator.kt +++ b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/ChatCoordinator.kt @@ -120,8 +120,19 @@ interface MessagingOperations { /** Observes all messages in [chatId] as a flat list. */ fun observeMessages(chatId: ChatId): Flow> - /** True once the user has ever sent a tip (a Cash message with verb TIPPED) — onboarding milestone. */ - fun hasEverTipped(): Flow + /** + * Whether the user has ever sent a tip — a Cash message with verb TIPPED — or `null` while the + * answer is not yet trustworthy. + * + * The read is of a local cache, so an absent TIPPED message means "never tipped" only once that + * cache is known to be complete; before then it is indistinguishable from history that has not + * arrived. Onboarding is the caller, and it draws a tutorial off the answer, so it needs the + * third state rather than a `false` it has to guess about. Resolves to `true`/`false` once + * [ChatHydrationState] leaves [ChatHydrationState.Unknown] — including on + * [ChatHydrationState.Unavailable], so an unreachable server ends the wait instead of extending + * it forever. + */ + fun hasEverTipped(): Flow /** Observes messages in [chatId] via Paging 3, with remote-mediated page loads. */ fun observeMessagesPaged(chatId: ChatId): Flow> @@ -166,8 +177,27 @@ interface ChatCoordinator : FeedOperations, EventStreamOperations, DmChatResolve /** Full observable snapshot of chat state (feed, typing, reactions, active chat). */ val state: StateFlow - /** Tears down all connections, clears persisted data, and resets in-memory state. */ - suspend fun reset() + /** + * Closes connections, cancels jobs, and drops in-memory state. + * + * Deliberately leaves the persisted cache alone. The Room database is per-account + * (`FlipcashDatabase.init` names the file from the account entropy), so signing out does not + * have to erase anything to keep the next account's data separate — logging in swaps to a + * different file. Keeping the cache lets a re-login reconcile what it already has via the + * feed sync's catch-up instead of rebuilding from nothing, which is what the "send a tip" + * onboarding milestone was silently losing. + * + * @see clearCache for the one caller that does want the data gone. + */ + suspend fun teardown() + + /** + * Erases this account's persisted chat history — metadata, messages, and members. + * + * Account deletion only. Ordinary logout and account switching go through [teardown] and + * keep the cache. + */ + suspend fun clearCache() } class NoDmChatInitializedException(e164: String) : Exception("No DM chat for $e164") diff --git a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/ChatState.kt b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/ChatState.kt index d9c021b875..f4a010780d 100644 --- a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/ChatState.kt +++ b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/ChatState.kt @@ -11,6 +11,7 @@ data class ChatState( val typingIndicators: Map> = emptyMap(), val reactionOverlays: Map> = emptyMap(), val feedSyncState: FeedSyncState = FeedSyncState.Idle, + val historyHydration: ChatHydrationState = ChatHydrationState.Unknown, val activeChat: ChatId? = null, ) @@ -30,3 +31,24 @@ enum class FeedSyncState { Synced, Error, } + +/** + * How far the *message* cache has got in reconciling itself with the server for the signed-in user. + * + * Distinct from [FeedSyncState], which reports only the conversation-list fetch. That fetch marks + * itself [FeedSyncState.Synced] as soon as the metadata lands, before the per-chat history backfill + * it schedules has run — so it cannot answer "is this account's chat history here yet". Callers that + * read chat history as evidence need that second question: the wallet's "send a tip" milestone looks + * for an outgoing TIPPED message, and an absent one only means the user has never tipped once the + * cache is known to be complete. + */ +enum class ChatHydrationState { + /** No sync has finished a full pass this session — an absent message proves nothing. */ + Unknown, + + /** A sync succeeded and every catch-up it scheduled has run: an absent message really is absent. */ + Hydrated, + + /** A sync completed without success. Callers should stop waiting; the next sync will retry. */ + Unavailable, +} diff --git a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/RealChatCoordinator.kt b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/RealChatCoordinator.kt index b25f995535..3ba5b7d263 100644 --- a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/RealChatCoordinator.kt +++ b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/RealChatCoordinator.kt @@ -144,7 +144,13 @@ class RealChatCoordinator @Inject constructor( is FeedSyncDelegate.Event.LoadMessages -> messagingDelegate.loadMessages(event.chatId) is FeedSyncDelegate.Event.DeltaSyncNeeded -> - eventStreamDelegate.performDeltaSync(event.chatId) + eventStreamDelegate.performDeltaSync(event.chatId, event.afterSequence) + // Arrives after every catch-up item above it, because this is one sequential + // collector over a FIFO channel. Anything reading chat history as evidence — + // the wallet's "send a tip" milestone — waits for this rather than for the + // feed sync, which reports itself synced before the backfill is scheduled. + FeedSyncDelegate.Event.CatchUpComplete -> + feedDelegate.markHistoryHydrated() } }.launchIn(scope) @@ -197,7 +203,7 @@ class RealChatCoordinator @Inject constructor( // region ChatCoordinator - override suspend fun reset() { + override suspend fun teardown() { eventStreamDelegate.stopHeartbeat() eventStreamDelegate.close() feedDelegate.cancelJobs() @@ -205,9 +211,13 @@ class RealChatCoordinator @Inject constructor( stateHolder.reset() eventStreamDelegate.clearAll() cluster.value = null - messagingDelegate.clear() supervisorJob.cancel() - trace(tag = TAG, message = "reset complete", type = TraceType.Process) + trace(tag = TAG, message = "teardown complete", type = TraceType.Process) + } + + override suspend fun clearCache() { + messagingDelegate.clear() + trace(tag = TAG, message = "cache cleared", type = TraceType.Process) } // endregion diff --git a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/EventStreamDelegate.kt b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/EventStreamDelegate.kt index 7377693d5a..e740da77d9 100644 --- a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/EventStreamDelegate.kt +++ b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/EventStreamDelegate.kt @@ -185,8 +185,17 @@ class EventStreamDelegate @Inject constructor( heartbeatJob = null } - internal suspend fun performDeltaSync(chatId: ChatId) { - val afterSequence = metadataDataSource.getLatestEventSequence(chatId) + /** + * Backfills a chat from [afterSequence] onward. + * + * Callers that have already written the server's sequence to `chat_metadata` before asking for + * a delta must pass the sequence the cache held *beforehand* — reading it here would return + * the value they just wrote and ask the server for everything after its own latest event, + * which is always nothing. The live gap-fill path has no such write in front of it and omits + * the argument. + */ + internal suspend fun performDeltaSync(chatId: ChatId, afterSequence: Long? = null) { + val afterSequence = afterSequence ?: metadataDataSource.getLatestEventSequence(chatId) trace(tag = TAG, message = "Delta sync for $chatId from sequence $afterSequence", type = TraceType.Process) try { diff --git a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/FeedSyncDelegate.kt b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/FeedSyncDelegate.kt index da7cb2951b..b4c1e6e6e2 100644 --- a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/FeedSyncDelegate.kt +++ b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/FeedSyncDelegate.kt @@ -10,6 +10,7 @@ import com.flipcash.services.models.chat.ChatMember import com.flipcash.services.models.chat.ChatMetadata import com.flipcash.services.models.chat.ChatType import com.flipcash.services.models.chat.PointerType +import com.flipcash.shared.chat.ChatHydrationState import com.flipcash.shared.chat.ChatSummary import com.flipcash.shared.chat.FeedOperations import com.flipcash.shared.chat.FeedSyncState @@ -62,7 +63,24 @@ class FeedSyncDelegate @Inject constructor( sealed interface Event { data class LoadMessages(val chatId: ChatId) : Event - data class DeltaSyncNeeded(val chatId: ChatId) : Event + /** + * @param afterSequence the sequence the local cache held *before* this sync overwrote it; + * the delta must be requested from there, not from the row we just wrote. + */ + data class DeltaSyncNeeded(val chatId: ChatId, val afterSequence: Long) : Event + + /** + * Emitted last by every successful sync, after any catch-up above it. + * + * [events] is a FIFO channel routed by a single sequential collector in + * [RealChatCoordinator][com.flipcash.shared.chat.internal.RealChatCoordinator], so by the + * time this is handled every catch-up item ahead of it has finished its suspend call and + * committed its writes. That ordering is the whole reason it exists: it is what makes + * [markHistoryHydrated] safe to call, and it cannot be replaced by watching + * [FeedSyncState][com.flipcash.shared.chat.FeedSyncState], which flips to `Synced` before + * the catch-up is even scheduled. + */ + data object CatchUpComplete : Event } private val _events = Channel(Channel.UNLIMITED) @@ -153,6 +171,16 @@ class FeedSyncDelegate @Inject constructor( syncJob = scope.launch { performFeedSync() } } + /** + * Marks the message cache reconciled with the server. + * + * Called by the coordinator when it routes [Event.CatchUpComplete], not by the sync itself — + * the sync only *schedules* the backfill, and hydration is about that backfill having run. + */ + internal fun markHistoryHydrated() { + stateHolder.update { it.copy(historyHydration = ChatHydrationState.Hydrated) } + } + internal fun cancelJobs() { syncJob?.cancel() feedObserverJob?.cancel() @@ -188,10 +216,35 @@ class FeedSyncDelegate @Inject constructor( Result.success(contact.chats + (tip?.chats ?: emptyList())) } + /** What the local cache held for a chat before a sync wrote to it. */ + private data class CachedChatState( + val hasMessages: Boolean, + val latestEventSequence: Long, + ) + private suspend fun performFeedSync() { stateHolder.update { it.copy(feedSyncState = FeedSyncState.Syncing) } fetchCombinedFeed() .onSuccess { chats -> + // Snapshot the cache before writing to it. The writes below stamp the server's + // latestEventSequence onto every metadata row and give every chat at least one + // message, so the catch-up checks at the end of this sync — which are reads of + // exactly those two things — would otherwise always find the chat current and + // never fire. + // + // Both branches being inert had the same consequence: after a re-login, where + // logout has cleared the chat cache (ChatCoordinator.reset), a chat is left + // holding only the one message the feed carried. Anything derived from chat + // history then reads as if the rest never happened — the wallet's "send a tip" + // milestone looks for an outgoing TIPPED message and re-shows the new-user + // tutorial to an account that has already tipped. + val cached = chats.associate { chat -> + chat.chatId to CachedChatState( + hasMessages = messageDataSource.hasMessages(chat.chatId), + latestEventSequence = metadataDataSource.getLatestEventSequence(chat.chatId), + ) + } + metadataDataSource.upsert(chats) for (chat in chats) { @@ -205,20 +258,40 @@ class FeedSyncDelegate @Inject constructor( trace(tag = TAG, message = "Feed synced: ${chats.size} chats", type = TraceType.Process) for (chat in chats) { + val before = cached[chat.chatId] ?: continue if (chat.latestEventSequence > 0) { - val localSeq = metadataDataSource.getLatestEventSequence(chat.chatId) - if (localSeq > 0 && localSeq < chat.latestEventSequence) { - _events.send(Event.DeltaSyncNeeded(chat.chatId)) + if (before.latestEventSequence > 0 && + before.latestEventSequence < chat.latestEventSequence + ) { + _events.send( + Event.DeltaSyncNeeded(chat.chatId, before.latestEventSequence) + ) continue } } - if (!messageDataSource.hasMessages(chat.chatId)) { + if (!before.hasMessages) { _events.send(Event.LoadMessages(chat.chatId)) } } + + _events.send(Event.CatchUpComplete) } .onFailure { error -> - stateHolder.update { it.copy(feedSyncState = FeedSyncState.Error) } + stateHolder.update { state -> + state.copy( + feedSyncState = FeedSyncState.Error, + // Don't downgrade a hydration that already succeeded: a later failure means + // this sync missed, not that the cache stopped being trustworthy. Moving off + // Unknown at all matters though — callers waiting on hydration have to stop + // waiting when the server is unreachable, or the wallet spins forever + // offline. + historyHydration = if (state.historyHydration == ChatHydrationState.Unknown) { + ChatHydrationState.Unavailable + } else { + state.historyHydration + }, + ) + } trace(tag = TAG, message = "Feed sync failed: ${error.message}", type = TraceType.Error) } } diff --git a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/MessagingDelegate.kt b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/MessagingDelegate.kt index c75d48d142..fcf3b26197 100644 --- a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/MessagingDelegate.kt +++ b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/MessagingDelegate.kt @@ -1,4 +1,4 @@ -@file:OptIn(ExperimentalPagingApi::class) +@file:OptIn(ExperimentalPagingApi::class, ExperimentalCoroutinesApi::class) package com.flipcash.shared.chat.internal.delegates @@ -23,12 +23,15 @@ import com.flipcash.services.models.chat.MessageContent import com.flipcash.services.models.chat.MessagePointer import com.flipcash.services.models.chat.PointerType import com.flipcash.services.models.chat.TypingState +import com.flipcash.shared.chat.ChatHydrationState import com.flipcash.shared.chat.MessagingOperations import com.flipcash.shared.chat.internal.ChatStateHolder import com.flipcash.services.user.UserManager import com.getcode.opencode.model.core.ID +import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.map import javax.inject.Inject import javax.inject.Singleton @@ -94,7 +97,32 @@ class MessagingDelegate @Inject constructor( return messageDataSource.observeMessages(chatId) } - override fun hasEverTipped(): Flow = messageDataSource.hasEverTipped() + override fun hasEverTipped(): Flow = + stateHolder.state + .map { it.historyHydration } + .distinctUntilChanged() + .flatMapLatest { hydration -> + // Re-subscribing on the hydration change re-runs the query, so the first value a + // caller sees after a backfill is a fresh read. `combine`-ing the two flows instead + // would race: the emission carrying "hydrated" would carry the *pre*-backfill answer + // with it, and the caller would act on `false` a beat before Room's invalidation + // published `true` — the same flash of wrong state, just narrower. + messageDataSource.hasEverTipped().map { tipped -> + when { + // A TIPPED message in the cache is proof regardless of hydration, and + // answering straight away is what keeps a warm cache — the normal case now + // that logout no longer wipes it — from waiting on a round-trip. + tipped -> true + // Absence proves nothing yet. Null rather than false: the caller has to be + // able to tell "has not tipped" from "we have not looked". + hydration == ChatHydrationState.Unknown -> null + else -> false + } + } + } + // The re-subscribe repeats the current answer whenever hydration moves, which for a + // cache that already held the tip is the same `true` twice over. + .distinctUntilChanged() override fun observeMessagesPaged(chatId: ChatId): Flow> { return Pager( diff --git a/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ChatCoordinatorEagerBalanceTest.kt b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ChatCoordinatorEagerBalanceTest.kt index 2a6025e0ee..33542d4316 100644 --- a/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ChatCoordinatorEagerBalanceTest.kt +++ b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ChatCoordinatorEagerBalanceTest.kt @@ -177,7 +177,7 @@ class ChatCoordinatorEagerBalanceTest { runCurrent() coVerify(exactly = 1) { tokenCoordinator.add(mint, amount) } - coordinator.reset() + coordinator.teardown() } @Test @@ -188,7 +188,7 @@ class ChatCoordinatorEagerBalanceTest { runCurrent() coVerify(exactly = 0) { tokenCoordinator.add(any(), any()) } - coordinator.reset() + coordinator.teardown() } @Test @@ -199,7 +199,7 @@ class ChatCoordinatorEagerBalanceTest { runCurrent() coVerify(exactly = 0) { tokenCoordinator.add(any(), any()) } - coordinator.reset() + coordinator.teardown() } @Test @@ -217,7 +217,7 @@ class ChatCoordinatorEagerBalanceTest { coVerify(exactly = 1) { tokenCoordinator.add(mint, amount1) } coVerify(exactly = 1) { tokenCoordinator.add(mintB, amount2) } - coordinator.reset() + coordinator.teardown() } @Test @@ -231,6 +231,6 @@ class ChatCoordinatorEagerBalanceTest { runCurrent() coVerify(exactly = 1) { tokenCoordinator.add(any(), any()) } - coordinator.reset() + coordinator.teardown() } } diff --git a/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ChatCoordinatorEventsTest.kt b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ChatCoordinatorEventsTest.kt index 1a83953299..3a44e72b1b 100644 --- a/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ChatCoordinatorEventsTest.kt +++ b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ChatCoordinatorEventsTest.kt @@ -186,7 +186,7 @@ class ChatCoordinatorEventsTest { messages.size == 1 && messages[0].messageId == 1L }) } - coordinator.reset() + coordinator.teardown() } @Test @@ -209,7 +209,7 @@ class ChatCoordinatorEventsTest { messages.size == 1 && messages[0].messageId == 42L }) } - coordinator.reset() + coordinator.teardown() } @Test @@ -238,7 +238,7 @@ class ChatCoordinatorEventsTest { messages.size == 2 }) } - coordinator.reset() + coordinator.teardown() } // endregion @@ -262,7 +262,7 @@ class ChatCoordinatorEventsTest { runCurrent() coVerify { metadataDataSource.updateLatestEventSequence(chatId, 2L) } - coordinator.reset() + coordinator.teardown() } @Test @@ -290,7 +290,7 @@ class ChatCoordinatorEventsTest { // Cursor should advance to 1 (contiguous), not 3 coVerify { metadataDataSource.updateLatestEventSequence(chatId, 1L) } coVerify(exactly = 0) { metadataDataSource.updateLatestEventSequence(chatId, 3L) } - coordinator.reset() + coordinator.teardown() } @Test @@ -322,7 +322,7 @@ class ChatCoordinatorEventsTest { // After filling the gap, cursor should advance to 3 coVerify { metadataDataSource.updateLatestEventSequence(chatId, 3L) } - coordinator.reset() + coordinator.teardown() } // endregion @@ -359,7 +359,7 @@ class ChatCoordinatorEventsTest { assertEquals(1, summary.reactions.size) assertEquals("\uD83D\uDE00", summary.reactions[0].emoji.value) assertEquals(1L, summary.reactions[0].count) - coordinator.reset() + coordinator.teardown() } @Test @@ -407,7 +407,7 @@ class ChatCoordinatorEventsTest { assertEquals(1, reactions.size) assertEquals(3L, reactions[0].count) // stayed at 3, stale update rejected assertEquals(5L, reactions[0].sequence) - coordinator.reset() + coordinator.teardown() } @Test @@ -453,7 +453,7 @@ class ChatCoordinatorEventsTest { val reactions = coordinator.state.value.reactionOverlays[chatId]?.get(1L)?.reactions assertNotNull(reactions) assertTrue(reactions.isEmpty()) - coordinator.reset() + coordinator.teardown() } @Test @@ -489,7 +489,7 @@ class ChatCoordinatorEventsTest { val reactions = coordinator.state.value.reactionOverlays[chatId]?.get(1L)?.reactions assertNotNull(reactions) assertEquals(2, reactions.size) - coordinator.reset() + coordinator.teardown() } // endregion @@ -503,7 +503,7 @@ class ChatCoordinatorEventsTest { runCurrent() // Logout cancels the coordinator's supervisor job (and thus its scope). - coordinator.reset() + coordinator.teardown() runCurrent() // Re-login in the same process. Before the fix, the scope stayed cancelled, @@ -522,7 +522,7 @@ class ChatCoordinatorEventsTest { messages.size == 1 && messages[0].messageId == 7L }) } - coordinator.reset() + coordinator.teardown() } // endregion diff --git a/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ChatCoordinatorTeardownTest.kt b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ChatCoordinatorTeardownTest.kt new file mode 100644 index 0000000000..b8c8fc21c9 --- /dev/null +++ b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ChatCoordinatorTeardownTest.kt @@ -0,0 +1,135 @@ +package com.flipcash.shared.chat + +import com.flipcash.app.core.dispatchers.TestDispatchers +import com.flipcash.app.persistence.sources.ChatMemberDataSource +import com.flipcash.app.persistence.sources.ChatMessageDataSource +import com.flipcash.app.persistence.sources.ChatMetadataDataSource +import com.flipcash.app.persistence.sources.ContactDataSource +import com.flipcash.app.tokens.TokenCoordinator +import com.flipcash.services.controllers.ChatController +import com.flipcash.services.controllers.ChatMessagingController +import com.flipcash.services.controllers.EventStreamingController +import com.flipcash.services.models.chat.ChatUpdate +import com.flipcash.services.user.UserManager +import com.flipcash.shared.chat.internal.ChatIdGenerator +import com.flipcash.shared.chat.internal.ChatStateHolder +import com.flipcash.shared.chat.internal.RealChatCoordinator +import com.flipcash.shared.chat.internal.delegates.DmChatResolverDelegate +import com.flipcash.shared.chat.internal.delegates.EventStreamDelegate +import com.flipcash.shared.chat.internal.delegates.FeedSyncDelegate +import com.flipcash.shared.chat.internal.delegates.MessagingDelegate +import com.getcode.utils.network.NetworkConnectivityListener +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.every +import io.mockk.mockk +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.channels.Channel +import kotlinx.coroutines.flow.receiveAsFlow +import kotlinx.coroutines.test.TestCoroutineScheduler +import kotlinx.coroutines.test.runTest +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +/** + * Signing out and erasing the account are separate acts, and only the second one should cost the + * user their cached chat history. + * + * The Room database is per-account — [com.flipcash.app.persistence.FlipcashDatabase.init] names the + * file from the account entropy — so a logout never has to empty a table to keep the next account's + * data separate; the next login opens a different file. Emptying it anyway is what left a re-login + * unable to see that the user had ever sent a tip. + */ +@OptIn(ExperimentalCoroutinesApi::class) +@RunWith(RobolectricTestRunner::class) +class ChatCoordinatorTeardownTest { + + private lateinit var metadataDataSource: ChatMetadataDataSource + private lateinit var messageDataSource: ChatMessageDataSource + private lateinit var memberDataSource: ChatMemberDataSource + private lateinit var coordinator: RealChatCoordinator + + @Before + fun setUp() { + val userManager = mockk(relaxed = true) + every { userManager.accountId } returns listOf(1, 2, 3) + + val eventStreamingController = mockk(relaxed = true) + every { eventStreamingController.chatUpdates } returns + Channel(Channel.UNLIMITED).receiveAsFlow() + + val chatController = mockk(relaxed = true) + coEvery { chatController.getDmChatFeed(any(), any()) } returns + Result.failure(RuntimeException("not needed")) + + metadataDataSource = mockk(relaxed = true) + messageDataSource = mockk(relaxed = true) + memberDataSource = mockk(relaxed = true) + val messagingController = mockk(relaxed = true) + val stateHolder = ChatStateHolder() + + coordinator = RealChatCoordinator( + feedDelegate = FeedSyncDelegate( + chatController = chatController, + metadataDataSource = metadataDataSource, + messageDataSource = messageDataSource, + memberDataSource = memberDataSource, + stateHolder = stateHolder, + userManager = userManager, + ), + eventStreamDelegate = EventStreamDelegate( + eventStreamingController = eventStreamingController, + messagingController = messagingController, + metadataDataSource = metadataDataSource, + messageDataSource = messageDataSource, + memberDataSource = memberDataSource, + tokenCoordinator = mockk(relaxed = true), + userManager = userManager, + stateHolder = stateHolder, + analytics = mockk(relaxed = true), + exchange = mockk(relaxed = true), + ), + dmChatResolverDelegate = DmChatResolverDelegate( + chatIdGenerator = ChatIdGenerator(), + userManager = userManager, + contactDataSource = mockk(relaxed = true), + memberDataSource = memberDataSource, + ), + messagingDelegate = MessagingDelegate( + chatController = chatController, + messagingController = messagingController, + metadataDataSource = metadataDataSource, + messageDataSource = messageDataSource, + memberDataSource = memberDataSource, + notificationManager = mockk(relaxed = true), + userManager = userManager, + stateHolder = stateHolder, + analytics = mockk(relaxed = true), + ), + stateHolder = stateHolder, + userManager = userManager, + networkObserver = mockk(relaxed = true), + dispatchers = TestDispatchers(TestCoroutineScheduler()), + ) + } + + @Test + fun `teardown leaves the persisted chat cache intact`() = runTest { + coordinator.teardown() + + coVerify(exactly = 0) { messageDataSource.clear() } + coVerify(exactly = 0) { metadataDataSource.clear() } + coVerify(exactly = 0) { memberDataSource.clear() } + } + + @Test + fun `clearCache erases metadata, messages, and members`() = runTest { + coordinator.clearCache() + + coVerify(exactly = 1) { metadataDataSource.clear() } + coVerify(exactly = 1) { messageDataSource.clear() } + coVerify(exactly = 1) { memberDataSource.clear() } + } +} diff --git a/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ChatHistoryHydrationTest.kt b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ChatHistoryHydrationTest.kt new file mode 100644 index 0000000000..d8255c5c91 --- /dev/null +++ b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ChatHistoryHydrationTest.kt @@ -0,0 +1,254 @@ +package com.flipcash.shared.chat + +import com.flipcash.app.core.dispatchers.TestDispatchers +import com.flipcash.app.persistence.sources.ChatMemberDataSource +import com.flipcash.app.persistence.sources.ChatMessageDataSource +import com.flipcash.app.persistence.sources.ChatMetadataDataSource +import com.flipcash.app.persistence.sources.ContactDataSource +import com.flipcash.app.tokens.TokenCoordinator +import com.flipcash.services.controllers.ChatController +import com.flipcash.services.controllers.ChatMessagingController +import com.flipcash.services.controllers.EventStreamingController +import com.flipcash.services.models.chat.ChatFeedPage +import com.flipcash.services.models.chat.ChatId +import com.flipcash.services.models.chat.ChatMessage +import com.flipcash.services.models.chat.ChatMetadata +import com.flipcash.services.models.chat.ChatType +import com.flipcash.services.models.chat.ChatUpdate +import com.flipcash.services.models.chat.MessageContent +import com.flipcash.services.user.UserManager +import com.flipcash.shared.chat.internal.ChatIdGenerator +import com.flipcash.shared.chat.internal.ChatStateHolder +import com.flipcash.shared.chat.internal.RealChatCoordinator +import com.flipcash.shared.chat.internal.delegates.DmChatResolverDelegate +import com.flipcash.shared.chat.internal.delegates.EventStreamDelegate +import com.flipcash.shared.chat.internal.delegates.FeedSyncDelegate +import com.flipcash.shared.chat.internal.delegates.MessagingDelegate +import com.getcode.opencode.model.accounts.AccountCluster +import com.getcode.utils.network.NetworkConnectivityListener +import io.mockk.coEvery +import io.mockk.every +import io.mockk.mockk +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.channels.Channel +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.flow.receiveAsFlow +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.test.runTest +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import kotlin.test.assertEquals +import kotlin.time.Instant + +/** + * The "send a tip" onboarding milestone is read off the local chat cache, so an absent TIPPED + * message is only evidence once that cache is known to be complete. These tests hold the milestone + * to that: it withholds an answer until the sync's catch-up has actually run, rather than reporting + * the cold-start `false` that re-shows the tutorial to an account which has already tipped. + * + * The feed sync alone cannot be that signal — it reports itself `Synced` after writing the + * conversation list, before the per-chat backfill it schedules has run. + */ +@OptIn(ExperimentalCoroutinesApi::class) +@RunWith(RobolectricTestRunner::class) +class ChatHistoryHydrationTest { + + private val chatId = ChatId("aabbccdd") + private val selfId = listOf(1, 2, 3) + private val otherId = listOf(4, 5, 6) + + /** Stands in for the Room query behind the milestone; the backfill flips it. */ + private val tippedInCache = MutableStateFlow(false) + + private val messagingController = mockk(relaxed = true) + private val chatController = mockk(relaxed = true) + + private fun message(messageId: Long) = ChatMessage( + messageId = messageId, + senderId = otherId, + content = listOf(MessageContent.Text("hi")), + timestamp = Instant.fromEpochSeconds(1_000 + messageId), + unreadSeq = messageId, + eventSequence = messageId, + ) + + private fun metadata() = ChatMetadata( + chatId = chatId, + type = ChatType.TIP_DM, + members = emptyList(), + lastMessage = message(20), + lastActivity = Instant.fromEpochSeconds(1_000), + latestEventSequence = 0, + ) + + private fun TestScope.buildCoordinator(): RealChatCoordinator { + val userManager = mockk(relaxed = true) + every { userManager.accountId } returns selfId + + val eventStreamingController = mockk(relaxed = true) + every { eventStreamingController.chatUpdates } returns + Channel(Channel.UNLIMITED).receiveAsFlow() + + val messageDataSource = mockk(relaxed = true) + every { messageDataSource.hasEverTipped() } returns tippedInCache + // A cold cache: nothing has messages, so the sync schedules a catch-up for every chat. + coEvery { messageDataSource.hasMessages(any()) } returns false + + val metadataDataSource = mockk(relaxed = true) + val memberDataSource = mockk(relaxed = true) + val stateHolder = ChatStateHolder() + + return RealChatCoordinator( + feedDelegate = FeedSyncDelegate( + chatController = chatController, + metadataDataSource = metadataDataSource, + messageDataSource = messageDataSource, + memberDataSource = memberDataSource, + stateHolder = stateHolder, + userManager = userManager, + ), + eventStreamDelegate = EventStreamDelegate( + eventStreamingController = eventStreamingController, + messagingController = messagingController, + metadataDataSource = metadataDataSource, + messageDataSource = messageDataSource, + memberDataSource = memberDataSource, + tokenCoordinator = mockk(relaxed = true), + userManager = userManager, + stateHolder = stateHolder, + analytics = mockk(relaxed = true), + exchange = mockk(relaxed = true), + ), + dmChatResolverDelegate = DmChatResolverDelegate( + chatIdGenerator = ChatIdGenerator(), + userManager = userManager, + contactDataSource = mockk(relaxed = true), + memberDataSource = memberDataSource, + ), + messagingDelegate = MessagingDelegate( + chatController = chatController, + messagingController = messagingController, + metadataDataSource = metadataDataSource, + messageDataSource = messageDataSource, + memberDataSource = memberDataSource, + notificationManager = mockk(relaxed = true), + userManager = userManager, + stateHolder = stateHolder, + analytics = mockk(relaxed = true), + ), + stateHolder = stateHolder, + userManager = userManager, + networkObserver = mockk(relaxed = true), + dispatchers = TestDispatchers(testScheduler), + ) + } + + private fun feedReturns(chats: List) { + coEvery { chatController.getDmChatFeed(ChatType.CONTACT_DM, any()) } returns + Result.success(ChatFeedPage(emptyList(), null, false)) + coEvery { chatController.getDmChatFeed(ChatType.TIP_DM, any()) } returns + Result.success(ChatFeedPage(chats, null, false)) + } + + private fun TestScope.collectMilestone(coordinator: RealChatCoordinator): List { + val values = mutableListOf() + coordinator.hasEverTipped().onEach { values += it }.launchIn(backgroundScope) + runCurrent() + return values + } + + /** + * Signs in and lets the session's work run to completion, then tears it down. + * + * The teardown is not incidental: signing in starts a heartbeat that delays forever, and + * [runTest] drains the scheduler once the body returns — on the coordinator's own scope, which + * is not [TestScope.backgroundScope] and so outlives the body. Left running, that loop spins + * virtual time indefinitely and the test never ends. + */ + private suspend fun TestScope.session(coordinator: RealChatCoordinator, body: () -> Unit) { + coordinator.onUserLoggedIn(mockk(relaxed = true)) + runCurrent() + // Unconditional: a failing assertion that skipped the teardown would leave the heartbeat + // running, and the resulting spin would bury the assertion under a test that never returns. + try { + body() + } finally { + coordinator.teardown() + } + } + + /** + * The reported bug, at the read side: an account that has tipped signs in, the cache is cold, + * and the milestone must never say `false` on the way to `true`. Anything that reports `false` + * first draws the tutorial for a frame. + */ + @Test + fun `the milestone withholds an answer until the backfill has run`() = runTest { + feedReturns(listOf(metadata())) + // The backfill is what surfaces the tip: it is older than the chat's last message, so the + // feed sync's own write could never have revealed it. + coEvery { messagingController.getMessages(chatId) } coAnswers { + tippedInCache.value = true + Result.success(listOf(message(20))) + } + val coordinator = buildCoordinator() + val values = collectMilestone(coordinator) + + assertEquals(listOf(null), values, "nothing has been reconciled yet") + + session(coordinator) { + assertEquals( + listOf(null, true), + values, + "the milestone must resolve straight from unknown to tipped, never through false", + ) + } + } + + /** Absence is real once the catch-up has run; the caller has to be released to draw. */ + @Test + fun `the milestone reports false once the cache is reconciled`() = runTest { + feedReturns(listOf(metadata())) + coEvery { messagingController.getMessages(chatId) } returns Result.success(emptyList()) + val coordinator = buildCoordinator() + val values = collectMilestone(coordinator) + + session(coordinator) { + assertEquals(listOf(null, false), values) + } + } + + /** + * A warm cache is the normal case now that signing out keeps chat history, and proof of a tip + * cannot go stale — so it answers without waiting for a round-trip. + */ + @Test + fun `a cached tip answers before any sync`() = runTest { + tippedInCache.value = true + feedReturns(emptyList()) + val coordinator = buildCoordinator() + + assertEquals(listOf(true), collectMilestone(coordinator)) + coordinator.teardown() + } + + /** + * Offline, the answer is still not trustworthy — but waiting on a server that cannot be reached + * would hold the wallet on its spinner indefinitely, which is worse than a stale milestone. + */ + @Test + fun `an unreachable server ends the wait rather than extending it`() = runTest { + coEvery { chatController.getDmChatFeed(any(), any()) } returns + Result.failure(RuntimeException("offline")) + val coordinator = buildCoordinator() + val values = collectMilestone(coordinator) + + session(coordinator) { + assertEquals(listOf(null, false), values) + } + } +} diff --git a/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/FeedSyncCatchUpTest.kt b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/FeedSyncCatchUpTest.kt new file mode 100644 index 0000000000..741b1b6ce7 --- /dev/null +++ b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/FeedSyncCatchUpTest.kt @@ -0,0 +1,228 @@ +package com.flipcash.shared.chat + +import com.flipcash.app.persistence.sources.ChatMemberDataSource +import com.flipcash.app.persistence.sources.ChatMessageDataSource +import com.flipcash.app.persistence.sources.ChatMetadataDataSource +import com.flipcash.services.controllers.ChatController +import com.flipcash.services.models.chat.ChatFeedPage +import com.flipcash.services.models.chat.ChatId +import com.flipcash.services.models.chat.ChatMessage +import com.flipcash.services.models.chat.ChatMetadata +import com.flipcash.services.models.chat.ChatType +import com.flipcash.services.models.chat.MessageContent +import com.flipcash.services.user.UserManager +import com.flipcash.shared.chat.internal.ChatStateHolder +import com.flipcash.shared.chat.internal.delegates.FeedSyncDelegate +import io.mockk.coEvery +import io.mockk.mockk +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.test.runTest +import org.junit.Test +import kotlin.test.assertEquals +import kotlin.time.Instant + +/** + * A feed sync writes each chat's `lastMessage` and the server's `latestEventSequence` before it + * decides whether that chat needs catching up. These tests hold the decision to what the cache held + * *before* the sync wrote to it. + * + * The user-visible failure: after a re-login the chat cache is empty (logout clears it via + * `ChatCoordinator.reset`), the sync repopulates one message per chat, and anything derived from + * chat history — the "send a tip" onboarding milestone reads an outgoing TIPPED message out of it — + * silently reads as if it never happened. + */ +@OptIn(ExperimentalCoroutinesApi::class) +class FeedSyncCatchUpTest { + + private val selfId = listOf(1, 2, 3) + private val otherId = listOf(4, 5, 6) + private val chatId = ChatId("aabbccdd") + + private fun message( + messageId: Long, + senderId: List, + eventSequence: Long = messageId, + ) = ChatMessage( + messageId = messageId, + senderId = senderId, + content = listOf(MessageContent.Text("hi")), + timestamp = Instant.fromEpochSeconds(1_000 + messageId), + unreadSeq = messageId, + eventSequence = eventSequence, + ) + + private fun metadata( + lastMessage: ChatMessage?, + latestEventSequence: Long = 0, + ) = ChatMetadata( + chatId = chatId, + type = ChatType.TIP_DM, + members = emptyList(), + lastMessage = lastMessage, + lastActivity = Instant.fromEpochSeconds(1_000), + latestEventSequence = latestEventSequence, + ) + + /** + * Stands in for the per-user Room database: what the sync writes is what a later read sees. + * Relaxed mocks would answer `false`/`0` regardless of the writes, which is exactly the coupling + * under test. + */ + private class Harness(chats: List, seededChatsWithMessages: Set = emptySet()) { + val chatsWithMessages = seededChatsWithMessages.toMutableSet() + val storedSequences = mutableMapOf() + + val messageDataSource = mockk(relaxed = true).also { source -> + coEvery { source.upsert(any(), any()) } answers { + chatsWithMessages += firstArg() + } + coEvery { source.hasMessages(any()) } answers { firstArg() in chatsWithMessages } + } + + val metadataDataSource = mockk(relaxed = true).also { source -> + coEvery { source.upsert(any>()) } answers { + firstArg>().forEach { storedSequences[it.chatId] = it.latestEventSequence } + } + coEvery { source.getLatestEventSequence(any()) } answers { + storedSequences[firstArg()] ?: 0L + } + } + + val chatController = mockk(relaxed = true).also { controller -> + coEvery { controller.getDmChatFeed(ChatType.CONTACT_DM, any()) } returns + Result.success(ChatFeedPage(emptyList(), null, false)) + coEvery { controller.getDmChatFeed(ChatType.TIP_DM, any()) } returns + Result.success(ChatFeedPage(chats, null, false)) + } + + val delegate = FeedSyncDelegate( + chatController = chatController, + metadataDataSource = metadataDataSource, + messageDataSource = messageDataSource, + memberDataSource = mockk(relaxed = true), + stateHolder = ChatStateHolder(), + userManager = mockk(relaxed = true), + ) + + fun sync(scope: TestScope): List { + val events = mutableListOf() + delegate.events.onEach { events += it }.launchIn(scope.backgroundScope) + delegate.initialize(scope.backgroundScope) + delegate.syncFeed() + scope.runCurrent() + return events + } + } + + @Test + fun `cold cache loads a chat's history even when the feed carried its last message`() = runTest { + // The tip the user sent is not the newest message in the chat — the other party replied + // after it — so the feed's lastMessage alone can never prove the tip happened. + val harness = Harness(chats = listOf(metadata(lastMessage = message(20, otherId)))) + + val events = harness.sync(this) + + assertEquals( + listOf(FeedSyncDelegate.Event.LoadMessages(chatId), FeedSyncDelegate.Event.CatchUpComplete), + events, + "a chat with no cached history must be caught up, not judged by the row the sync just wrote", + ) + } + + @Test + fun `warm cache does not refetch history`() = runTest { + val harness = Harness( + chats = listOf(metadata(lastMessage = message(20, otherId))), + seededChatsWithMessages = setOf(chatId), + ) + + val events = harness.sync(this) + + assertEquals( + listOf(FeedSyncDelegate.Event.CatchUpComplete), + events, + "a chat already backed by cached history needs no catch-up, only the terminal marker", + ) + } + + @Test + fun `a local sequence behind the server's triggers a delta sync`() = runTest { + val harness = Harness( + chats = listOf(metadata(lastMessage = message(20, otherId), latestEventSequence = 99)), + seededChatsWithMessages = setOf(chatId), + ).apply { storedSequences[chatId] = 42 } + + val events = harness.sync(this) + + assertEquals( + listOf( + FeedSyncDelegate.Event.DeltaSyncNeeded(chatId, afterSequence = 42), + FeedSyncDelegate.Event.CatchUpComplete, + ), + events, + "the gap must be measured against the sequence the cache held before the sync overwrote it", + ) + } + + /** + * The event carrying its own `afterSequence` is the whole point: the delta consumer reads + * `chat_metadata` when not given one, and by then this sync has already stamped the server's + * sequence onto that row — so a re-read would ask the server for everything after its own + * latest event and get nothing back. + */ + @Test + fun `the delta request starts from the pre-sync sequence, not the row the sync wrote`() = runTest { + val harness = Harness( + chats = listOf(metadata(lastMessage = message(20, otherId), latestEventSequence = 99)), + seededChatsWithMessages = setOf(chatId), + ).apply { storedSequences[chatId] = 42 } + + val event = harness.sync(this) + .filterIsInstance() + .single() + + assertEquals( + 99L, + harness.metadataDataSource.getLatestEventSequence(chatId), + "precondition: the sync has overwritten the stored sequence with the server's", + ) + assertEquals( + 42L, + event.afterSequence, + "the delta must be requested from 42, or the backfill silently fetches nothing", + ) + } + + /** + * The marker has to be last, not merely present: hydration is declared when it is routed, and a + * marker that overtook a pending catch-up would declare the cache complete while the backfill + * that completes it is still outstanding. + */ + @Test + fun `the catch-up marker is the last event of a successful sync`() = runTest { + val harness = Harness(chats = listOf(metadata(lastMessage = message(20, otherId)))) + + val events = harness.sync(this) + + assertEquals(FeedSyncDelegate.Event.CatchUpComplete, events.last()) + } + + @Test + fun `a failed sync emits no marker`() = runTest { + val harness = Harness(chats = emptyList()) + coEvery { harness.chatController.getDmChatFeed(ChatType.CONTACT_DM, any()) } returns + Result.failure(RuntimeException("offline")) + + val events = harness.sync(this) + + assertEquals( + emptyList(), + events, + "nothing was reconciled, so nothing may report itself reconciled", + ) + } +} diff --git a/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ReceivedCounterTest.kt b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ReceivedCounterTest.kt index 0fed87915e..218ee64734 100644 --- a/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ReceivedCounterTest.kt +++ b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ReceivedCounterTest.kt @@ -205,7 +205,7 @@ class ReceivedCounterTest { coVerify(exactly = 1) { analytics.incrementReceivedCounter(Analytics.ReceivedCounter.Tips, 1.0) } coVerify(exactly = 1) { analytics.incrementReceivedCounter(Analytics.ReceivedCounter.Messages, 1.0) } coVerify(exactly = 1) { analytics.incrementReceivedCounter(Analytics.ReceivedCounter.TipsValue, 5.0) } - coordinator.reset() + coordinator.teardown() } @Test @@ -215,7 +215,7 @@ class ReceivedCounterTest { coVerify(exactly = 1) { analytics.incrementReceivedCounter(Analytics.ReceivedCounter.Messages, 1.0) } coVerify(exactly = 0) { analytics.incrementReceivedCounter(Analytics.ReceivedCounter.Tips, any()) } - coordinator.reset() + coordinator.teardown() } @Test @@ -224,7 +224,7 @@ class ReceivedCounterTest { deliver(tipMessage(messageId = 1L, senderId = selfId)) coVerify(exactly = 0) { analytics.incrementReceivedCounter(any(), any()) } - coordinator.reset() + coordinator.teardown() } @Test @@ -236,7 +236,7 @@ class ReceivedCounterTest { deliver(msg) // gap fill / reconnect replays the same message coVerify(exactly = 1) { analytics.incrementReceivedCounter(Analytics.ReceivedCounter.Tips, 1.0) } - coordinator.reset() + coordinator.teardown() } @Test @@ -246,7 +246,7 @@ class ReceivedCounterTest { deliver(textMessage(messageId = 10L, senderId = otherId)) coVerify(exactly = 0) { analytics.incrementReceivedCounter(any(), any()) } - coordinator.reset() + coordinator.teardown() } @Test @@ -256,7 +256,7 @@ class ReceivedCounterTest { deliver(tipMessage(messageId = 1L, senderId = otherId, amount = cad)) coVerify(exactly = 1) { analytics.incrementReceivedCounter(Analytics.ReceivedCounter.TipsValue, 5.0) } - coordinator.reset() + coordinator.teardown() } @Test @@ -268,6 +268,6 @@ class ReceivedCounterTest { coVerify(exactly = 1) { analytics.incrementReceivedCounter(Analytics.ReceivedCounter.Tips, 1.0) } coVerify(exactly = 0) { analytics.incrementReceivedCounter(Analytics.ReceivedCounter.TipsValue, any()) } - coordinator.reset() + coordinator.teardown() } } diff --git a/apps/flipcash/shared/session/src/main/kotlin/com/flipcash/app/session/internal/RealSessionController.kt b/apps/flipcash/shared/session/src/main/kotlin/com/flipcash/app/session/internal/RealSessionController.kt index 5d72de19be..b32fcc16cc 100644 --- a/apps/flipcash/shared/session/src/main/kotlin/com/flipcash/app/session/internal/RealSessionController.kt +++ b/apps/flipcash/shared/session/src/main/kotlin/com/flipcash/app/session/internal/RealSessionController.kt @@ -185,7 +185,7 @@ class RealSessionController @Inject constructor( stopPolling() depositDelegate.cancelSweep() scope.launch { contactCoordinator.reset() } - scope.launch { chatCoordinator.reset() } + scope.launch { chatCoordinator.teardown() } stateHolder.reset() }