diff --git a/client/auth/data/impl/build.gradle.kts b/client/auth/data/impl/build.gradle.kts index 7d898dfea..839cef224 100644 --- a/client/auth/data/impl/build.gradle.kts +++ b/client/auth/data/impl/build.gradle.kts @@ -13,6 +13,7 @@ kotlin { implementation(libs.supabase.storage) implementation(libs.supabase.functions) } + commonTest.dependencies { implementation(libs.ktor.client.mock) } jvmMain.dependencies { implementation(libs.ktor.client.cio) } androidMain.dependencies { implementation(libs.ktor.client.cio) } appleMain.dependencies { implementation(libs.ktor.client.darwin) } diff --git a/client/auth/data/impl/src/commonMain/kotlin/com/plusmobileapps/chefmate/auth/data/impl/ExpiredTokenRetry.kt b/client/auth/data/impl/src/commonMain/kotlin/com/plusmobileapps/chefmate/auth/data/impl/ExpiredTokenRetry.kt new file mode 100644 index 000000000..b24b97c5c --- /dev/null +++ b/client/auth/data/impl/src/commonMain/kotlin/com/plusmobileapps/chefmate/auth/data/impl/ExpiredTokenRetry.kt @@ -0,0 +1,95 @@ +package com.plusmobileapps.chefmate.auth.data.impl + +import co.touchlab.kermit.Logger +import io.ktor.client.plugins.api.Send +import io.ktor.client.plugins.api.createClientPlugin +import io.ktor.http.HttpHeaders +import io.ktor.http.HttpStatusCode +import io.ktor.http.encodedPath +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock + +/** + * Renews the access token and replays the request whenever Supabase rejects one as unauthorized. + * + * The SDK's auto-refresh is a single in-process timer, and outside Android nothing re-arms it when + * the machine sleeps through its window. A 401 is the only signal that doesn't depend on that timer + * — or on the client clock being right — so it's the one worth acting on. + * + * This sits below every repository, which matters: the sync paths swallow per-item failures to keep + * one bad row from stopping a whole reconcile, so a 401 surfaced any higher would be caught and + * dropped before anything could react to it. Down here the request simply succeeds on the retry. + */ +internal class ExpiredTokenRetryConfig { + /** + * Renews the session and returns a usable token, or null if it couldn't be renewed — in which + * case the original 401 stands rather than a second doomed request being sent. + * + * Receives the token the rejected request carried, so an implementation can tell "nobody has + * refreshed yet" from "someone already did". See [ConcurrentRefreshGuard]. + */ + var refreshToken: suspend (usedToken: String?) -> String? = { null } +} + +internal val ExpiredTokenRetry = + createClientPlugin("ExpiredTokenRetry", ::ExpiredTokenRetryConfig) { + val refreshToken = pluginConfig.refreshToken + on(Send) { request -> + val call = proceed(request) + if (call.response.status != HttpStatusCode.Unauthorized) return@on call + // The token endpoint answers a dead refresh token with a 401 of its own. Retrying that + // is how one expired session turns into an infinite refresh loop. + if (request.url.encodedPath.startsWith(AUTH_PATH)) return@on call + + val usedToken = request.headers[HttpHeaders.Authorization]?.removePrefix(BEARER_PREFIX) + val freshToken = refreshToken(usedToken) ?: return@on call + // Replace rather than append: bearerAuth() would add a second Authorization header. + request.headers[HttpHeaders.Authorization] = BEARER_PREFIX + freshToken + proceed(request) + } + } + +/** + * Collapses a burst of 401s into a single refresh. + * + * A full sync fires many requests at once, so a dead token produces many simultaneous 401s. Letting + * each one refresh independently is not merely wasteful: Supabase rotates the refresh token on + * every use, so parallel refreshes look like token replay and can invalidate the session outright — + * taking out the sign-in this code is trying to rescue. + * + * Callers hand over the token their request actually used. If the current token has moved on, some + * other caller already refreshed and that result is reused; otherwise this one refreshes, under a + * lock, while the rest wait. + */ +internal class ConcurrentRefreshGuard( + private val currentToken: () -> String?, + private val refresh: suspend () -> Unit, +) { + private val mutex = Mutex() + + suspend fun tokenAfterRefresh(usedToken: String?): String? = mutex.withLock { + val current = currentToken() + if (current != null && current != usedToken) return@withLock current + val refreshed = + try { + refresh() + true + } catch (e: CancellationException) { + throw e + } catch (t: Throwable) { + Logger.w(throwable = t, tag = TAG) { + "Refresh after a 401 failed; leaving the request to fail" + } + false + } + if (refreshed) currentToken() else null + } + + private companion object { + const val TAG = "ConcurrentRefreshGuard" + } +} + +private const val AUTH_PATH = "/auth/v1" +private const val BEARER_PREFIX = "Bearer " diff --git a/client/auth/data/impl/src/commonMain/kotlin/com/plusmobileapps/chefmate/auth/data/impl/SupabaseAuthenticationRepository.kt b/client/auth/data/impl/src/commonMain/kotlin/com/plusmobileapps/chefmate/auth/data/impl/SupabaseAuthenticationRepository.kt index 1d13ee566..131ca33d5 100644 --- a/client/auth/data/impl/src/commonMain/kotlin/com/plusmobileapps/chefmate/auth/data/impl/SupabaseAuthenticationRepository.kt +++ b/client/auth/data/impl/src/commonMain/kotlin/com/plusmobileapps/chefmate/auth/data/impl/SupabaseAuthenticationRepository.kt @@ -22,9 +22,18 @@ import io.github.jan.supabase.functions.functions import kotlin.coroutines.CoroutineContext import kotlin.io.encoding.Base64 import kotlin.io.encoding.ExperimentalEncodingApi +import kotlin.time.Clock +import kotlin.time.Duration.Companion.minutes +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.channels.BufferOverflow +import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.SharedFlow import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asSharedFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.launch import kotlinx.serialization.json.Json @@ -37,6 +46,7 @@ import kotlinx.serialization.json.jsonObject import kotlinx.serialization.json.jsonPrimitive import kotlinx.serialization.json.put +@OptIn(ExperimentalCoroutinesApi::class) @Inject @SingleIn(AppScope::class) @ContributesBinding(AppScope::class) @@ -44,11 +54,22 @@ class SupabaseAuthenticationRepository( private val supabaseClient: SupabaseClient, @Main private val mainContext: CoroutineContext, ) : AuthenticationRepository { - private val scope = CoroutineScope(mainContext) + // SupervisorJob so a throw inside the session collector below can't tear the scope down and + // freeze the auth state for the rest of the process' life. + private val scope = CoroutineScope(mainContext + SupervisorJob()) private val _state = MutableStateFlow(AuthState.Unauthenticated) override val state: StateFlow = _state.asStateFlow() + private val _authenticatedSessions = + MutableSharedFlow( + replay = 1, + extraBufferCapacity = 1, + onBufferOverflow = BufferOverflow.DROP_OLDEST, + ) + override val authenticatedSessions: SharedFlow = + _authenticatedSessions.asSharedFlow() + init { // Mirror the Supabase session state into our AuthState. The NotAuthenticated branch no // longer auto-bootstraps an anonymous session — callers that need a session call @@ -62,18 +83,34 @@ class SupabaseAuthenticationRepository( val user = sessionStatus.session.user val isAnon = isAnonymousJwt(sessionStatus.session.accessToken) user?.let { - _state.value = - AuthState.Authenticated(it.toChefMateUser(isAnonymous = isAnon)) + val chefMateUser = it.toChefMateUser(isAnonymous = isAnon) + _state.value = AuthState.Authenticated(chefMateUser) + // Fires on every refresh, not only the first sign-in — this is the + // signal sync triggers hang off, since [state] dedupes the equal + // Authenticated value a refresh produces. + _authenticatedSessions.tryEmit(chefMateUser) } } is SessionStatus.NotAuthenticated -> { + // Drop the replayed session so a repository constructed after sign-out + // doesn't immediately sync as the signed-out user. + _authenticatedSessions.resetReplayCache() if (_state.value !is AuthState.AwaitingEmailVerification) { _state.value = AuthState.Unauthenticated } } - is SessionStatus.Initializing, + is SessionStatus.Initializing -> { + // Keep current state during initialization + } is SessionStatus.RefreshFailure -> { - // Keep current state during initialization or refresh failures + // Keep the last known state: the SDK retries on its own, and + // [ExpiredTokenRetry] repairs the token on the next 401. Logged + // because it is otherwise invisible — the app keeps looking signed in + // while every request fails on an expired JWT. + Logger.w(tag = TAG) { + "Supabase session refresh failed; keeping the last known auth state. " + + "Requests will fail until the token recovers." + } } } } @@ -92,6 +129,22 @@ class SupabaseAuthenticationRepository( } } + override suspend fun refreshSessionIfNeeded(): Boolean { + val session = supabaseClient.auth.currentSessionOrNull() ?: return false + if (Clock.System.now() < session.expiresAt - EXPIRY_MARGIN) return true + return try { + // Besides handing back a live token, this re-arms the SDK's auto-refresh job — which + // is the thing that actually died when the machine slept through its timer. + supabaseClient.auth.refreshCurrentSession() + true + } catch (e: CancellationException) { + throw e + } catch (t: Throwable) { + Logger.w(throwable = t, tag = TAG) { "Forced session refresh failed" } + false + } + } + override suspend fun signInWithEmailAndPassword(email: String, password: String): Result = try { supabaseClient.auth.signInWith(Email) { @@ -288,5 +341,11 @@ class SupabaseAuthenticationRepository( private companion object { const val TAG = "SupabaseAuthenticationRepository" + + /** + * Refresh this far ahead of expiry so a sync that starts with a barely-valid token doesn't + * have it expire mid-run. + */ + val EXPIRY_MARGIN = 5.minutes } } diff --git a/client/auth/data/impl/src/commonMain/kotlin/com/plusmobileapps/chefmate/auth/data/impl/SupabaseModule.kt b/client/auth/data/impl/src/commonMain/kotlin/com/plusmobileapps/chefmate/auth/data/impl/SupabaseModule.kt index 00738aeeb..cab70cd4e 100644 --- a/client/auth/data/impl/src/commonMain/kotlin/com/plusmobileapps/chefmate/auth/data/impl/SupabaseModule.kt +++ b/client/auth/data/impl/src/commonMain/kotlin/com/plusmobileapps/chefmate/auth/data/impl/SupabaseModule.kt @@ -8,7 +8,9 @@ import dev.zacsweers.metro.ContributesTo import dev.zacsweers.metro.Provides import dev.zacsweers.metro.SingleIn import io.github.jan.supabase.SupabaseClient +import io.github.jan.supabase.annotations.SupabaseInternal import io.github.jan.supabase.auth.Auth +import io.github.jan.supabase.auth.auth import io.github.jan.supabase.createSupabaseClient import io.github.jan.supabase.functions.Functions import io.github.jan.supabase.postgrest.Postgrest @@ -18,6 +20,10 @@ import io.github.jan.supabase.storage.Storage @SingleIn(AppScope::class) @ContributesTo(AppScope::class) interface SupabaseModule { + // httpConfig is marked internal by supabase-kt, but it is the only hook that reaches the + // shared HttpClient every plugin sends through — which is exactly the layer a 401 retry belongs + // at. + @OptIn(SupabaseInternal::class) @SingleIn(AppScope::class) @Provides fun provideSupabaseClient(environmentProvider: EnvironmentProvider): SupabaseClient { @@ -33,12 +39,29 @@ interface SupabaseModule { Environment.PROD, Environment.FAKE -> BuildConfig.SUPABASE_PROD_URL to BuildConfig.SUPABASE_PROD_KEY } + + // [ExpiredTokenRetry] has to refresh the session of the very client it is being installed + // into, so it reads the client back out of this holder, which is filled the moment + // createSupabaseClient returns — always before anything can issue a request through it. + var client: SupabaseClient? = null + val refreshGuard = + ConcurrentRefreshGuard( + currentToken = { client?.auth?.currentAccessTokenOrNull() }, + refresh = { client?.auth?.refreshCurrentSession() }, + ) + return createSupabaseClient(supabaseUrl = url, supabaseKey = key) { - install(Auth) - install(Postgrest) - install(Realtime) - install(Storage) - install(Functions) - } + install(Auth) + install(Postgrest) + install(Realtime) + install(Storage) + install(Functions) + httpConfig { + install(ExpiredTokenRetry) { + refreshToken = { usedToken -> refreshGuard.tokenAfterRefresh(usedToken) } + } + } + } + .also { client = it } } } diff --git a/client/auth/data/impl/src/commonTest/kotlin/com/plusmobileapps/chefmate/auth/data/impl/ConcurrentRefreshGuardTest.kt b/client/auth/data/impl/src/commonTest/kotlin/com/plusmobileapps/chefmate/auth/data/impl/ConcurrentRefreshGuardTest.kt new file mode 100644 index 000000000..22c53e66f --- /dev/null +++ b/client/auth/data/impl/src/commonTest/kotlin/com/plusmobileapps/chefmate/auth/data/impl/ConcurrentRefreshGuardTest.kt @@ -0,0 +1,67 @@ +@file:Suppress("FunctionName") + +package com.plusmobileapps.chefmate.auth.data.impl + +import io.kotest.matchers.shouldBe +import kotlin.test.Test +import kotlinx.coroutines.async +import kotlinx.coroutines.awaitAll +import kotlinx.coroutines.delay +import kotlinx.coroutines.test.runTest + +class ConcurrentRefreshGuardTest { + + private var token: String? = "stale" + private var refreshCalls = 0 + + private val guard = + ConcurrentRefreshGuard( + currentToken = { token }, + refresh = { + refreshCalls += 1 + // A real refresh is a network round trip. Suspending here is what makes the + // burst test meaningful: without the lock, every waiting caller would sail past + // the "has someone already refreshed?" check while this one is still in flight. + delay(10) + token = "fresh-$refreshCalls" + }, + ) + + @Test + fun refreshes_when_the_caller_used_the_current_token() = runTest { + guard.tokenAfterRefresh(usedToken = "stale") shouldBe "fresh-1" + + refreshCalls shouldBe 1 + } + + @Test + fun reuses_a_refresh_another_caller_already_did() = runTest { + token = "already-fresh" + + guard.tokenAfterRefresh(usedToken = "stale") shouldBe "already-fresh" + + refreshCalls shouldBe 0 + } + + @Test + fun a_burst_of_401s_on_the_same_token_causes_one_refresh() = runTest { + // Supabase rotates the refresh token on every use, so parallel refreshes read as replay + // and can invalidate the session — the sign-in this code exists to rescue. + val results = List(8) { async { guard.tokenAfterRefresh(usedToken = "stale") } }.awaitAll() + + refreshCalls shouldBe 1 + results.distinct() shouldBe listOf("fresh-1") + } + + @Test + fun returns_null_when_the_refresh_fails() = runTest { + val failing = + ConcurrentRefreshGuard( + currentToken = { token }, + refresh = { throw RuntimeException("offline") }, + ) + + // Not the stale token: handing that back would only buy a second 401. + failing.tokenAfterRefresh(usedToken = "stale") shouldBe null + } +} diff --git a/client/auth/data/impl/src/commonTest/kotlin/com/plusmobileapps/chefmate/auth/data/impl/ExpiredTokenRetryTest.kt b/client/auth/data/impl/src/commonTest/kotlin/com/plusmobileapps/chefmate/auth/data/impl/ExpiredTokenRetryTest.kt new file mode 100644 index 000000000..413c48b6f --- /dev/null +++ b/client/auth/data/impl/src/commonTest/kotlin/com/plusmobileapps/chefmate/auth/data/impl/ExpiredTokenRetryTest.kt @@ -0,0 +1,96 @@ +@file:Suppress("FunctionName") + +package com.plusmobileapps.chefmate.auth.data.impl + +import io.kotest.matchers.shouldBe +import io.ktor.client.HttpClient +import io.ktor.client.engine.mock.MockEngine +import io.ktor.client.engine.mock.respond +import io.ktor.client.request.bearerAuth +import io.ktor.client.request.get +import io.ktor.http.HttpHeaders +import io.ktor.http.HttpStatusCode +import kotlin.test.Test +import kotlinx.coroutines.test.runTest + +class ExpiredTokenRetryTest { + + private val sentTokens = mutableListOf() + private var refreshCalls = 0 + + @Test + fun replays_the_request_with_a_fresh_token_after_a_401() = runTest { + val client = clientRespondingWith(HttpStatusCode.Unauthorized, HttpStatusCode.OK) + + val response = + client.get("https://project.supabase.co/rest/v1/recipes") { + bearerAuth("stale-token") + } + + response.status shouldBe HttpStatusCode.OK + refreshCalls shouldBe 1 + sentTokens shouldBe listOf("stale-token", "fresh-token") + } + + @Test + fun leaves_a_successful_request_alone() = runTest { + val client = clientRespondingWith(HttpStatusCode.OK) + + client.get("https://project.supabase.co/rest/v1/recipes") { bearerAuth("good-token") } + + refreshCalls shouldBe 0 + sentTokens shouldBe listOf("good-token") + } + + @Test + fun does_not_retry_the_auth_endpoint() = runTest { + // A dead refresh token makes /auth/v1/token answer 401. Refreshing in response to that + // would call the same endpoint again — the loop this guard exists to prevent. + val client = clientRespondingWith(HttpStatusCode.Unauthorized, HttpStatusCode.OK) + + val response = + client.get("https://project.supabase.co/auth/v1/token") { + bearerAuth("stale-token") + } + + response.status shouldBe HttpStatusCode.Unauthorized + refreshCalls shouldBe 0 + sentTokens shouldBe listOf("stale-token") + } + + @Test + fun lets_the_401_stand_when_the_session_cannot_be_renewed() = runTest { + val client = + clientRespondingWith(HttpStatusCode.Unauthorized, HttpStatusCode.OK) { + refreshCalls += 1 + null + } + + val response = + client.get("https://project.supabase.co/rest/v1/recipes") { + bearerAuth("stale-token") + } + + // One doomed request beats two: the retry is skipped entirely. + response.status shouldBe HttpStatusCode.Unauthorized + refreshCalls shouldBe 1 + sentTokens shouldBe listOf("stale-token") + } + + private fun clientRespondingWith( + vararg statuses: HttpStatusCode, + refresh: suspend (usedToken: String?) -> String? = { + refreshCalls += 1 + "fresh-token" + }, + ): HttpClient { + var index = 0 + val engine = MockEngine { request -> + sentTokens += request.headers[HttpHeaders.Authorization]?.removePrefix("Bearer ") + respond(content = "", status = statuses[index++.coerceAtMost(statuses.lastIndex)]) + } + return HttpClient(engine) { + install(ExpiredTokenRetry) { refreshToken = refresh } + } + } +} diff --git a/client/auth/data/public/src/commonMain/kotlin/com/plusmobileapps/chefmate/auth/data/AuthenticationRepository.kt b/client/auth/data/public/src/commonMain/kotlin/com/plusmobileapps/chefmate/auth/data/AuthenticationRepository.kt index 069817829..57fcc0d42 100644 --- a/client/auth/data/public/src/commonMain/kotlin/com/plusmobileapps/chefmate/auth/data/AuthenticationRepository.kt +++ b/client/auth/data/public/src/commonMain/kotlin/com/plusmobileapps/chefmate/auth/data/AuthenticationRepository.kt @@ -1,11 +1,38 @@ package com.plusmobileapps.chefmate.auth.data +import kotlinx.coroutines.flow.SharedFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.serialization.Serializable interface AuthenticationRepository { val state: StateFlow + /** + * Emits every time a usable session becomes available: the initial sign-in *and* every silent + * token refresh after it. + * + * [state] can't carry this. A refresh produces an [AuthState.Authenticated] equal to the one + * already there, and `StateFlow` drops equal values — so a process that stays alive for days + * (the desktop app) never learns that a dead token came back, and anything stranded by it stays + * unsynced until the app is restarted. Sync triggers observe this instead of [state]. + * + * The current session is replayed to late subscribers, so a repository constructed after + * sign-in still syncs. The replay cache is cleared on sign-out so one constructed afterwards + * never sees a stale user. + */ + val authenticatedSessions: SharedFlow + + /** + * Answers "is this session going to work?", renewing an expired or nearly-expired token to find + * out. A comfortably-valid token is left alone. + * + * Individual requests don't need this — a 401 repairs the token at the transport layer and the + * request is replayed. This exists for the one caller that wants to know *before* committing to + * a sync, so a session it can't revive can be reported to the user instead of turning into a + * pile of swallowed failures. + */ + suspend fun refreshSessionIfNeeded(): Boolean + /** * Idempotently ensures a Supabase session exists. Returns immediately if already authenticated * (anonymous or real). Otherwise lazily signs in anonymously — that's the only path callers diff --git a/client/auth/data/testing/src/commonMain/kotlin/com/plusmobileapps/chefmate/auth/data/testing/FakeAuthenticationRepository.kt b/client/auth/data/testing/src/commonMain/kotlin/com/plusmobileapps/chefmate/auth/data/testing/FakeAuthenticationRepository.kt index 94e49c40f..bb46db2da 100644 --- a/client/auth/data/testing/src/commonMain/kotlin/com/plusmobileapps/chefmate/auth/data/testing/FakeAuthenticationRepository.kt +++ b/client/auth/data/testing/src/commonMain/kotlin/com/plusmobileapps/chefmate/auth/data/testing/FakeAuthenticationRepository.kt @@ -5,13 +5,26 @@ import com.plusmobileapps.chefmate.auth.data.AuthenticationRepository import com.plusmobileapps.chefmate.auth.data.ChefMateUser import com.plusmobileapps.chefmate.auth.data.OtpFlow import com.plusmobileapps.chefmate.auth.data.SignUpResult +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.channels.BufferOverflow +import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.SharedFlow import kotlinx.coroutines.flow.StateFlow +@OptIn(ExperimentalCoroutinesApi::class) class FakeAuthenticationRepository : AuthenticationRepository { private val _state = MutableStateFlow(AuthState.Unauthenticated) override val state: StateFlow = _state + private val _authenticatedSessions = + MutableSharedFlow( + replay = 1, + extraBufferCapacity = 1, + onBufferOverflow = BufferOverflow.DROP_OLDEST, + ) + override val authenticatedSessions: SharedFlow = _authenticatedSessions + var signInResult: Result = Result.success(Unit) var signUpResult: Result = Result.success(SignUpResult.Success) var sendPasswordResetResult: Result = Result.success(Unit) @@ -40,16 +53,21 @@ class FakeAuthenticationRepository : AuthenticationRepository { var lastResendOtpFlow: OtpFlow? = null private set + var refreshSessionResult: Boolean = true + + var refreshSessionCallCount: Int = 0 + private set + fun setState(state: AuthState) { - _state.value = state + emitState(state) } fun setAuthenticated(user: ChefMateUser = fakeUser()) { - _state.value = AuthState.Authenticated(user) + emitState(AuthState.Authenticated(user)) } fun setAnonymous(userId: String = "anon-test-id") { - _state.value = + emitState( AuthState.Authenticated( ChefMateUser( userId = userId, @@ -59,6 +77,25 @@ class FakeAuthenticationRepository : AuthenticationRepository { isAnonymous = true, ) ) + ) + } + + /** + * Emits a fresh session for the currently-signed-in user without changing [state] — what a + * silent token refresh looks like to collectors. `StateFlow` swallows the identical + * `Authenticated` value, so this is the only way sync triggers hear about it. + */ + fun emitSessionRefresh() { + val user = (_state.value as? AuthState.Authenticated)?.user ?: return + _authenticatedSessions.tryEmit(user) + } + + private fun emitState(state: AuthState) { + _state.value = state + when (state) { + is AuthState.Authenticated -> _authenticatedSessions.tryEmit(state.user) + else -> _authenticatedSessions.resetReplayCache() + } } override suspend fun ensureSession(): Result { @@ -77,7 +114,12 @@ class FakeAuthenticationRepository : AuthenticationRepository { ): Result = signUpResult override suspend fun signOut() { - _state.value = AuthState.Unauthenticated + emitState(AuthState.Unauthenticated) + } + + override suspend fun refreshSessionIfNeeded(): Boolean { + refreshSessionCallCount += 1 + return refreshSessionResult } override suspend fun updateProfile(displayName: String, avatarUrl: String?): Result { @@ -86,7 +128,7 @@ class FakeAuthenticationRepository : AuthenticationRepository { return updateProfileResult.also { result -> if (result.isSuccess) { (_state.value as? AuthState.Authenticated)?.let { authenticated -> - _state.value = + emitState( AuthState.Authenticated( authenticated.user.copy( userName = displayName, @@ -94,6 +136,7 @@ class FakeAuthenticationRepository : AuthenticationRepository { avatarUrl ?: authenticated.user.userProfileImageUrl, ) ) + ) } } } @@ -112,7 +155,7 @@ class FakeAuthenticationRepository : AuthenticationRepository { override suspend fun verifyEmailOtp(email: String, token: String, flow: OtpFlow): Result { lastVerifyOtpFlow = flow return verifyEmailOtpResult.also { result -> - if (result.isSuccess) _state.value = AuthState.Authenticated(fakeUser(email)) + if (result.isSuccess) emitState(AuthState.Authenticated(fakeUser(email))) } } diff --git a/client/composeApp/build.gradle.kts b/client/composeApp/build.gradle.kts index 9b8626952..9a87fbca6 100644 --- a/client/composeApp/build.gradle.kts +++ b/client/composeApp/build.gradle.kts @@ -178,6 +178,13 @@ kotlin { dependencies { implementation(compose.desktop.uiTestJUnit4) implementation(compose.desktop.currentOs) + // Plain-logic tests for app-scoped coordinators. Kept out of commonTest so the + // Android instrumented variant doesn't have to carry the fakes. + implementation(projects.client.util.testing) + implementation(projects.client.recipe.data.testing) + implementation(projects.client.recipebook.data.testing) + implementation(projects.client.grocery.data.testing) + implementation(projects.client.meal.data.testing) } } val androidInstrumentedTest by getting { diff --git a/client/composeApp/src/commonMain/kotlin/com/plusmobileapps/chefmate/ApplicationComponent.kt b/client/composeApp/src/commonMain/kotlin/com/plusmobileapps/chefmate/ApplicationComponent.kt index 6c6d52390..b734ef065 100644 --- a/client/composeApp/src/commonMain/kotlin/com/plusmobileapps/chefmate/ApplicationComponent.kt +++ b/client/composeApp/src/commonMain/kotlin/com/plusmobileapps/chefmate/ApplicationComponent.kt @@ -4,6 +4,7 @@ import com.plusmobileapps.chefmate.auth.data.AuthenticationRepository import com.plusmobileapps.chefmate.di.OnboardingRepository import com.plusmobileapps.chefmate.recipe.core.root.RecipeRootBloc import com.plusmobileapps.chefmate.root.RootBloc +import com.plusmobileapps.chefmate.sync.SyncCoordinator import com.plusmobileapps.chefmate.toast.ToastService import com.russhwolf.settings.Settings @@ -20,4 +21,10 @@ interface ApplicationComponent { val onboardingRepository: OnboardingRepository val settings: Settings val toastService: ToastService + + /** + * Reconciles every repository on demand. Desktop drives this from window focus and a periodic + * tick, since a process that stays up for days has no launch to fall back on. + */ + val syncCoordinator: SyncCoordinator } diff --git a/client/composeApp/src/commonMain/kotlin/com/plusmobileapps/chefmate/sync/SyncCoordinator.kt b/client/composeApp/src/commonMain/kotlin/com/plusmobileapps/chefmate/sync/SyncCoordinator.kt new file mode 100644 index 000000000..96bb284d8 --- /dev/null +++ b/client/composeApp/src/commonMain/kotlin/com/plusmobileapps/chefmate/sync/SyncCoordinator.kt @@ -0,0 +1,96 @@ +package com.plusmobileapps.chefmate.sync + +import com.plusmobileapps.chefmate.auth.data.AuthState +import com.plusmobileapps.chefmate.auth.data.AuthenticationRepository +import com.plusmobileapps.chefmate.di.AppScope +import com.plusmobileapps.chefmate.di.IO +import com.plusmobileapps.chefmate.grocery.data.GroceryRepository +import com.plusmobileapps.chefmate.meal.data.MealPlanRepository +import com.plusmobileapps.chefmate.recipe.data.RecipeRepository +import com.plusmobileapps.chefmate.recipebook.data.RecipeBookRepository +import com.plusmobileapps.chefmate.util.DateTimeUtil +import dev.zacsweers.metro.Inject +import dev.zacsweers.metro.SingleIn +import kotlin.coroutines.CoroutineContext +import kotlin.time.Duration.Companion.minutes +import kotlin.time.Instant +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock +import kotlinx.coroutines.withContext + +/** + * Runs a full reconcile across every syncing repository, in dependency order. + * + * Repositories sync themselves whenever a session arrives or the user taps sync, which is enough on + * mobile — the OS tears the process down and the next launch reconciles. A desktop process instead + * stays up for days, so this exists to give that process something to call when it has reason to + * believe it fell behind (the window regaining focus, a periodic tick). See + * `AuthenticationRepository.refreshSessionIfNeeded` for what "fell behind" usually means. + */ +@Inject +@SingleIn(AppScope::class) +class SyncCoordinator( + private val authRepository: AuthenticationRepository, + private val recipeBookRepository: RecipeBookRepository, + private val recipeRepository: RecipeRepository, + private val groceryRepository: GroceryRepository, + private val mealPlanRepository: MealPlanRepository, + private val dateTimeUtil: DateTimeUtil, + @IO private val ioContext: CoroutineContext, +) { + + private val mutex = Mutex() + private var lastAttemptAt: Instant? = null + + /** + * Reconciles every repository, unless a run already happened within [MIN_INTERVAL] — focus + * events in particular can arrive in bursts as the user moves between windows. Pass [force] to + * bypass that (a user-initiated sync should never be swallowed). + * + * Throttling counts failed attempts too, so a machine that's genuinely offline isn't retried on + * every alt-tab. The per-screen sync buttons stay available as an immediate escape hatch. + */ + suspend fun syncAll(force: Boolean = false): SyncOutcome = mutex.withLock { + if (authRepository.state.value !is AuthState.Authenticated) return SyncOutcome.SignedOut + + val now = dateTimeUtil.now + val last = lastAttemptAt + if (!force && last != null && now - last < MIN_INTERVAL) return SyncOutcome.Throttled + lastAttemptAt = now + + // Nothing below can succeed on a dead token, and this is the moment we're best placed + // to revive it — a wake-from-sleep is exactly when the SDK's refresh timer has slipped. + if (!authRepository.refreshSessionIfNeeded()) return SyncOutcome.SessionExpired + + withContext(ioContext) { + // Books before recipes: a recipe resolves its book by remote id, so a book that + // hasn't been pushed yet would strand the recipes filed under it. + recipeBookRepository.syncAllUnsynced() + recipeRepository.syncAllUnsynced() + groceryRepository.syncAllUnsynced() + mealPlanRepository.syncAllUnsynced() + } + SyncOutcome.Synced + } + + private companion object { + val MIN_INTERVAL = 1.minutes + } +} + +/** Why a [SyncCoordinator.syncAll] call did or didn't do any work. */ +enum class SyncOutcome { + Synced, + + /** Nothing to sync — no one is signed in. */ + SignedOut, + + /** A run happened recently enough that this one was skipped. */ + Throttled, + + /** + * The access token is dead and couldn't be renewed, so syncing was not attempted. Worth + * surfacing: from the user's side this is indistinguishable from the app quietly doing nothing. + */ + SessionExpired, +} diff --git a/client/composeApp/src/jvmMain/kotlin/com/plusmobileapps/chefmate/main.kt b/client/composeApp/src/jvmMain/kotlin/com/plusmobileapps/chefmate/main.kt index 17798899c..529007ce4 100644 --- a/client/composeApp/src/jvmMain/kotlin/com/plusmobileapps/chefmate/main.kt +++ b/client/composeApp/src/jvmMain/kotlin/com/plusmobileapps/chefmate/main.kt @@ -4,6 +4,7 @@ package com.plusmobileapps.chefmate import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.material3.SnackbarDuration import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.collectAsState @@ -16,6 +17,7 @@ import androidx.compose.ui.input.key.Key import androidx.compose.ui.input.key.KeyEventType import androidx.compose.ui.input.key.key import androidx.compose.ui.input.key.type +import androidx.compose.ui.platform.LocalWindowInfo import androidx.compose.ui.res.painterResource import androidx.compose.ui.unit.DpSize import androidx.compose.ui.unit.dp @@ -24,6 +26,8 @@ import androidx.compose.ui.window.WindowPlacement import androidx.compose.ui.window.WindowPosition import androidx.compose.ui.window.application import androidx.compose.ui.window.rememberWindowState +import chefmate.client.ui.public.generated.resources.Res +import chefmate.client.ui.public.generated.resources.sync_session_expired import com.arkivanov.decompose.DefaultComponentContext import com.arkivanov.essenty.backhandler.BackDispatcher import com.arkivanov.essenty.lifecycle.LifecycleRegistry @@ -32,15 +36,20 @@ import com.plusmobileapps.chefmate.deeplink.DeepLinkCoordinator import com.plusmobileapps.chefmate.deeplink.SchemeRegistrar import com.plusmobileapps.chefmate.deeplink.SingleInstance import com.plusmobileapps.chefmate.root.DeepLink +import com.plusmobileapps.chefmate.sync.SyncOutcome +import com.plusmobileapps.chefmate.text.ResourceString import com.plusmobileapps.chefmate.ui.LocalRecipeWindowOpener import com.plusmobileapps.chefmate.ui.RecipeWindowOpener import com.plusmobileapps.chefmate.ui.theme.ChefMateTheme import com.plusmobileapps.chefmate.update.DesktopUpdater import com.plusmobileapps.chefmate.update.UpdateBanner import java.awt.Desktop +import kotlin.time.Duration.Companion.minutes import kotlin.time.ExperimentalTime import kotlinx.coroutines.FlowPreview +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.debounce +import kotlinx.coroutines.flow.filter private const val KEY_WINDOW_WIDTH = "window.width" private const val KEY_WINDOW_HEIGHT = "window.height" @@ -48,6 +57,12 @@ private const val KEY_WINDOW_X = "window.x" private const val KEY_WINDOW_Y = "window.y" private const val KEY_WINDOW_PLACEMENT = "window.placement" +/** + * How often an idle window reconciles with the remote. The other targets get this for free — the OS + * kills the process and the next launch syncs — but this one can sit open for days. + */ +private val SYNC_HEARTBEAT = 15.minutes + @OptIn(ExperimentalTime::class, FlowPreview::class) fun main(args: Array) { // Windows/Linux spawn a fresh process for every `chefmate://…` open. If another instance is @@ -187,6 +202,24 @@ fun main(args: Array) { window.requestFocus() } } + // Coming back to the window is the strongest hint that the machine woke up, and a + // slept-through token refresh is exactly what leaves this process unable to sync. + // The coordinator throttles, so a burst of focus changes costs nothing. + val windowInfo = LocalWindowInfo.current + LaunchedEffect(windowInfo) { + snapshotFlow { windowInfo.isWindowFocused } + .filter { it } + .collect { appComponent.syncAndReport() } + } + + // Backstop for a window left focused and untouched for hours. + LaunchedEffect(Unit) { + while (true) { + delay(SYNC_HEARTBEAT) + appComponent.syncAndReport() + } + } + val windowOpener = remember { RecipeWindowOpener(recipeWindows::open) } Box(modifier = Modifier.fillMaxSize()) { // Only desktop provides this, which is what turns the recipe list's right-click @@ -212,6 +245,20 @@ fun main(args: Array) { } } +/** + * Reconciles, and says so when it can't. A dead session is otherwise indistinguishable from the app + * quietly doing nothing — the exact failure that used to end with the user force-quitting. + * Throttled runs stay silent, so a flurry of focus changes can't turn into a flurry of snackbars. + */ +private suspend fun ApplicationComponent.syncAndReport() { + if (syncCoordinator.syncAll() == SyncOutcome.SessionExpired) { + toastService.show( + message = ResourceString(Res.string.sync_session_expired), + duration = SnackbarDuration.Long, + ) + } +} + private data class WindowSnapshot( val placement: WindowPlacement, val size: DpSize, diff --git a/client/composeApp/src/jvmTest/kotlin/com/plusmobileapps/chefmate/sync/SyncCoordinatorTest.kt b/client/composeApp/src/jvmTest/kotlin/com/plusmobileapps/chefmate/sync/SyncCoordinatorTest.kt new file mode 100644 index 000000000..4e1a0da7e --- /dev/null +++ b/client/composeApp/src/jvmTest/kotlin/com/plusmobileapps/chefmate/sync/SyncCoordinatorTest.kt @@ -0,0 +1,116 @@ +@file:Suppress("FunctionName") +@file:OptIn(ExperimentalCoroutinesApi::class) + +package com.plusmobileapps.chefmate.sync + +import com.plusmobileapps.chefmate.auth.data.testing.FakeAuthenticationRepository +import com.plusmobileapps.chefmate.grocery.data.testing.FakeGroceryRepository +import com.plusmobileapps.chefmate.meal.data.testing.FakeMealPlanRepository +import com.plusmobileapps.chefmate.recipe.data.testing.FakeRecipeRepository +import com.plusmobileapps.chefmate.recipebook.data.testing.FakeRecipeBookRepository +import com.plusmobileapps.chefmate.util.testing.FakeDateTimeUtil +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.time.Duration.Companion.minutes +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.runTest + +class SyncCoordinatorTest { + + private val testDispatcher = UnconfinedTestDispatcher() + private val fakeAuth = FakeAuthenticationRepository() + private val dateTimeUtil = FakeDateTimeUtil() + private val recipes = FakeRecipeRepository() + private val books = FakeRecipeBookRepository() + private val groceries = FakeGroceryRepository() + private val meals = FakeMealPlanRepository() + + private val coordinator = + SyncCoordinator( + authRepository = fakeAuth, + recipeBookRepository = books, + recipeRepository = recipes, + groceryRepository = groceries, + mealPlanRepository = meals, + dateTimeUtil = dateTimeUtil, + ioContext = testDispatcher, + ) + + @Test + fun syncAll_reconciles_every_repository_when_signed_in() = + runTest(testDispatcher) { + fakeAuth.setAuthenticated() + + assertEquals(SyncOutcome.Synced, coordinator.syncAll()) + + assertEquals(1, books.syncAllUnsyncedCallCount) + assertEquals(1, recipes.syncAllUnsyncedCallCount) + assertEquals(1, groceries.syncAllUnsyncedCallCount) + assertEquals(1, meals.syncAllUnsyncedCallCount) + } + + @Test + fun syncAll_does_nothing_when_signed_out() = + runTest(testDispatcher) { + assertEquals(SyncOutcome.SignedOut, coordinator.syncAll()) + + assertEquals(0, recipes.syncAllUnsyncedCallCount) + } + + @Test + fun syncAll_refreshes_the_session_before_syncing() = + runTest(testDispatcher) { + fakeAuth.setAuthenticated() + + coordinator.syncAll() + + assertEquals(1, fakeAuth.refreshSessionCallCount) + } + + @Test + fun syncAll_skips_syncing_when_the_session_cannot_be_revived() = + runTest(testDispatcher) { + fakeAuth.setAuthenticated() + fakeAuth.refreshSessionResult = false + + assertEquals(SyncOutcome.SessionExpired, coordinator.syncAll()) + + // Every call would fail on the dead token anyway; reporting it beats failing silently. + assertEquals(0, recipes.syncAllUnsyncedCallCount) + } + + @Test + fun syncAll_throttles_a_burst_of_triggers() = + runTest(testDispatcher) { + fakeAuth.setAuthenticated() + + assertEquals(SyncOutcome.Synced, coordinator.syncAll()) + assertEquals(SyncOutcome.Throttled, coordinator.syncAll()) + + assertEquals(1, recipes.syncAllUnsyncedCallCount) + } + + @Test + fun syncAll_runs_again_once_the_throttle_window_passes() = + runTest(testDispatcher) { + fakeAuth.setAuthenticated() + coordinator.syncAll() + + dateTimeUtil.fakeNow = dateTimeUtil.fakeNow + 2.minutes + + assertEquals(SyncOutcome.Synced, coordinator.syncAll()) + assertEquals(2, recipes.syncAllUnsyncedCallCount) + } + + @Test + fun syncAll_with_force_ignores_the_throttle() = + runTest(testDispatcher) { + fakeAuth.setAuthenticated() + coordinator.syncAll() + + assertEquals(SyncOutcome.Synced, coordinator.syncAll(force = true)) + + assertEquals(2, recipes.syncAllUnsyncedCallCount) + } +} diff --git a/client/grocery/data/impl/src/commonMain/kotlin/com/plusmobileapps/chefmate/grocery/data/impl/GroceryRepositoryImpl.kt b/client/grocery/data/impl/src/commonMain/kotlin/com/plusmobileapps/chefmate/grocery/data/impl/GroceryRepositoryImpl.kt index 83c082b3a..93bfc2f06 100644 --- a/client/grocery/data/impl/src/commonMain/kotlin/com/plusmobileapps/chefmate/grocery/data/impl/GroceryRepositoryImpl.kt +++ b/client/grocery/data/impl/src/commonMain/kotlin/com/plusmobileapps/chefmate/grocery/data/impl/GroceryRepositoryImpl.kt @@ -77,14 +77,19 @@ class GroceryRepositoryImpl( private var realtimeUserId: String? = null init { + // Every session — the first sign-in and every silent token refresh after it. A refresh + // means anything stranded by the expired token can finally be pushed, and on desktop + // (a process that stays up for days) that is the only automatic retry there is. scope.launch { + authRepository.authenticatedSessions.collect { user -> + syncWithRemote(user.userId) + startRealtimeSync(user.userId) + } + } + scope.launch { + // Sign-out only has to tear the subscription down; the sync side is driven above. authRepository.state.collect { state -> - if (state is AuthState.Authenticated) { - syncWithRemote(state.user.userId) - startRealtimeSync(state.user.userId) - } else { - stopRealtimeSync() - } + if (state !is AuthState.Authenticated) stopRealtimeSync() } } } diff --git a/client/grocery/data/impl/src/commonTest/kotlin/com/plusmobileapps/chefmate/grocery/data/impl/GroceryRepositoryImplTest.kt b/client/grocery/data/impl/src/commonTest/kotlin/com/plusmobileapps/chefmate/grocery/data/impl/GroceryRepositoryImplTest.kt index df67d2a9c..0c7850e35 100644 --- a/client/grocery/data/impl/src/commonTest/kotlin/com/plusmobileapps/chefmate/grocery/data/impl/GroceryRepositoryImplTest.kt +++ b/client/grocery/data/impl/src/commonTest/kotlin/com/plusmobileapps/chefmate/grocery/data/impl/GroceryRepositoryImplTest.kt @@ -782,6 +782,49 @@ class GroceryRepositoryImplTest { } } + @Test + fun session_refresh_re_runs_sync_without_an_auth_state_change() = + runTest(testDispatcher) { + repository.ensureDefaultList() + val remoteListId = "remote-list-session-refresh" + fakeRemote.remoteLists["user-1"] = + mutableListOf( + RemoteGroceryList( + id = remoteListId, + name = "My Grocery List", + ownerId = "user-1", + ) + ) + fakeAuth.setState( + AuthState.Authenticated( + ChefMateUser( + userId = "user-1", + userName = "Test", + userEmail = "test@test.com", + userProfileImageUrl = null, + ) + ) + ) + advanceUntilIdle() + repository.getGroceries().first().size shouldBe 0 + + fakeRemote.remoteItems[remoteListId] = + mutableListOf( + RemoteGroceryItem( + id = "item-remote-1", + listId = remoteListId, + name = "Eggs", + clientId = Uuid.random().toString(), + ) + ) + // A silent token refresh hands back the same user, so AuthState never changes and the + // StateFlow stays quiet — the session signal is the only thing that fires. + fakeAuth.emitSessionRefresh() + advanceUntilIdle() + + repository.getGroceries().test { awaitItem().single().name shouldBe "Eggs" } + } + @Test fun realtime_change_pulls_an_updated_checked_state_for_an_existing_item() = runTest(testDispatcher) { diff --git a/client/grocery/data/testing/src/commonMain/kotlin/com/plusmobileapps/chefmate/grocery/data/testing/FakeGroceryRepository.kt b/client/grocery/data/testing/src/commonMain/kotlin/com/plusmobileapps/chefmate/grocery/data/testing/FakeGroceryRepository.kt index 86c185889..62942c7b1 100644 --- a/client/grocery/data/testing/src/commonMain/kotlin/com/plusmobileapps/chefmate/grocery/data/testing/FakeGroceryRepository.kt +++ b/client/grocery/data/testing/src/commonMain/kotlin/com/plusmobileapps/chefmate/grocery/data/testing/FakeGroceryRepository.kt @@ -90,7 +90,12 @@ class FakeGroceryRepository : GroceryRepository { override suspend fun getGrocery(id: Long): GroceryItem? = _groceries.value.find { it.id == id } - override suspend fun syncAllUnsynced() {} + var syncAllUnsyncedCallCount: Int = 0 + private set + + override suspend fun syncAllUnsynced() { + syncAllUnsyncedCallCount += 1 + } override suspend fun updateGrocery(item: GroceryItem) { _groceries.update { items -> items.map { if (it.id == item.id) item else it } } diff --git a/client/meal/data/impl/src/commonMain/kotlin/com/plusmobileapps/chefmate/meal/data/impl/MealPlanRepositoryImpl.kt b/client/meal/data/impl/src/commonMain/kotlin/com/plusmobileapps/chefmate/meal/data/impl/MealPlanRepositoryImpl.kt index 9a3410631..35e050c15 100644 --- a/client/meal/data/impl/src/commonMain/kotlin/com/plusmobileapps/chefmate/meal/data/impl/MealPlanRepositoryImpl.kt +++ b/client/meal/data/impl/src/commonMain/kotlin/com/plusmobileapps/chefmate/meal/data/impl/MealPlanRepositoryImpl.kt @@ -49,12 +49,11 @@ class MealPlanRepositoryImpl( private val syncMutex = Mutex() init { + // Every session — the first sign-in and every silent token refresh after it. A refresh + // means anything stranded by the expired token can finally be pushed, and on desktop + // (a process that stays up for days) that is the only automatic retry there is. scope.launch { - authRepository.state.collect { state -> - if (state is AuthState.Authenticated) { - syncWithRemote(state.user.userId) - } - } + authRepository.authenticatedSessions.collect { user -> syncWithRemote(user.userId) } } } diff --git a/client/meal/data/testing/src/commonMain/kotlin/com/plusmobileapps/chefmate/meal/data/testing/FakeMealPlanRepository.kt b/client/meal/data/testing/src/commonMain/kotlin/com/plusmobileapps/chefmate/meal/data/testing/FakeMealPlanRepository.kt index 410ad8aea..233e70ec7 100644 --- a/client/meal/data/testing/src/commonMain/kotlin/com/plusmobileapps/chefmate/meal/data/testing/FakeMealPlanRepository.kt +++ b/client/meal/data/testing/src/commonMain/kotlin/com/plusmobileapps/chefmate/meal/data/testing/FakeMealPlanRepository.kt @@ -38,8 +38,11 @@ class FakeMealPlanRepository : MealPlanRepository { _meals.update { items -> items.filter { it.id != id } } } + var syncAllUnsyncedCallCount: Int = 0 + private set + override suspend fun syncAllUnsynced() { - // No-op in fake + syncAllUnsyncedCallCount += 1 } override suspend fun clearLocalData() { diff --git a/client/recipe/data/impl/src/commonMain/kotlin/com/plusmobileapps/chefmate/recipe/data/impl/RecipeRepositoryImpl.kt b/client/recipe/data/impl/src/commonMain/kotlin/com/plusmobileapps/chefmate/recipe/data/impl/RecipeRepositoryImpl.kt index 7f0327c17..1f3426dc2 100644 --- a/client/recipe/data/impl/src/commonMain/kotlin/com/plusmobileapps/chefmate/recipe/data/impl/RecipeRepositoryImpl.kt +++ b/client/recipe/data/impl/src/commonMain/kotlin/com/plusmobileapps/chefmate/recipe/data/impl/RecipeRepositoryImpl.kt @@ -64,12 +64,11 @@ class RecipeRepositoryImpl( private val syncingIds = MutableStateFlow>(emptySet()) init { + // Every session — the first sign-in and every silent token refresh after it. A refresh + // means anything stranded by the expired token can finally be pushed, and on desktop + // (a process that stays up for days) that is the only automatic retry there is. scope.launch { - authRepository.state.collect { state -> - if (state is AuthState.Authenticated) { - syncWithRemote(state.user.userId) - } - } + authRepository.authenticatedSessions.collect { user -> syncWithRemote(user.userId) } } } diff --git a/client/recipe/data/impl/src/commonTest/kotlin/com/plusmobileapps/chefmate/recipe/data/impl/RecipeRepositoryImplTest.kt b/client/recipe/data/impl/src/commonTest/kotlin/com/plusmobileapps/chefmate/recipe/data/impl/RecipeRepositoryImplTest.kt index 9dc0177fd..e58c71536 100644 --- a/client/recipe/data/impl/src/commonTest/kotlin/com/plusmobileapps/chefmate/recipe/data/impl/RecipeRepositoryImplTest.kt +++ b/client/recipe/data/impl/src/commonTest/kotlin/com/plusmobileapps/chefmate/recipe/data/impl/RecipeRepositoryImplTest.kt @@ -519,6 +519,22 @@ class RecipeRepositoryImplTest { recipeRepository.getRecipeByClientId("no-such-client-id") shouldBe null } + @Test + fun session_refresh_pushes_a_recipe_that_was_stranded_by_a_dead_token() = + runTest(testDispatcher) { + fakeAuth.setAuthenticated() + recipeRemote.upsertFailure = { RuntimeException("JWT expired") } + val created = recipeRepository.createRecipe(blankRecipe(title = "Stranded")) + db.recipeQueries.getById(created.id).executeAsOneOrNull()?.remoteId shouldBe null + + // A silent token refresh: same user, so AuthState is unchanged and only the session + // signal fires. This is the recovery the desktop app had no path to before. + recipeRemote.upsertFailure = null + fakeAuth.emitSessionRefresh() + + db.recipeQueries.getById(created.id).executeAsOneOrNull()?.remoteId shouldNotBe null + } + private fun blankRecipe(title: String, categories: Set = emptySet()) = Recipe( id = -1, @@ -549,11 +565,14 @@ class RecipeRepositoryImplTest { val attachmentCalls: MutableList>> = mutableListOf() val deleteCalls: MutableList = mutableListOf() var deleteFailure: (() -> Throwable)? = null + var upsertFailure: (() -> Throwable)? = null var fetchResult: List = emptyList() - override suspend fun upsertRecipe(recipe: RemoteRecipe): RemoteRecipe = + override suspend fun upsertRecipe(recipe: RemoteRecipe): RemoteRecipe { + upsertFailure?.invoke()?.let { throw it } // Stamp a stable remote id derived from the client id so tests can correlate. - recipe.copy(id = recipe.id ?: "remote-${recipe.clientId.orEmpty()}") + return recipe.copy(id = recipe.id ?: "remote-${recipe.clientId.orEmpty()}") + } override suspend fun deleteRecipe(remoteId: String) { deleteCalls += remoteId diff --git a/client/recipe/data/testing/src/commonMain/kotlin/com/plusmobileapps/chefmate/recipe/data/testing/FakeRecipeRepository.kt b/client/recipe/data/testing/src/commonMain/kotlin/com/plusmobileapps/chefmate/recipe/data/testing/FakeRecipeRepository.kt index 2cf6e62af..88aa56f86 100644 --- a/client/recipe/data/testing/src/commonMain/kotlin/com/plusmobileapps/chefmate/recipe/data/testing/FakeRecipeRepository.kt +++ b/client/recipe/data/testing/src/commonMain/kotlin/com/plusmobileapps/chefmate/recipe/data/testing/FakeRecipeRepository.kt @@ -99,7 +99,12 @@ class FakeRecipeRepository( recipes.value = emptyList() } - override suspend fun syncAllUnsynced() {} + var syncAllUnsyncedCallCount: Int = 0 + private set + + override suspend fun syncAllUnsynced() { + syncAllUnsyncedCallCount += 1 + } private fun Recipe.matchesFilter(presets: Set): Boolean { val recipeBuiltins = categories.mapNotNull { BuiltinCategory.fromId(it.builtinId) }.toSet() diff --git a/client/recipebook/data/impl/src/commonMain/kotlin/com/plusmobileapps/chefmate/recipebook/data/impl/RecipeBookRepositoryImpl.kt b/client/recipebook/data/impl/src/commonMain/kotlin/com/plusmobileapps/chefmate/recipebook/data/impl/RecipeBookRepositoryImpl.kt index 1df9bd4d9..841cd72a1 100644 --- a/client/recipebook/data/impl/src/commonMain/kotlin/com/plusmobileapps/chefmate/recipebook/data/impl/RecipeBookRepositoryImpl.kt +++ b/client/recipebook/data/impl/src/commonMain/kotlin/com/plusmobileapps/chefmate/recipebook/data/impl/RecipeBookRepositoryImpl.kt @@ -66,12 +66,11 @@ class RecipeBookRepositoryImpl( else -> defaultId } } + // Every session — the first sign-in and every silent token refresh after it. A refresh + // means anything stranded by the expired token can finally be pushed, and on desktop + // (a process that stays up for days) that is the only automatic retry there is. scope.launch { - authRepository.state.collect { state -> - if (state is AuthState.Authenticated) { - syncWithRemote(state.user.userId) - } - } + authRepository.authenticatedSessions.collect { user -> syncWithRemote(user.userId) } } } diff --git a/client/recipebook/data/testing/src/commonMain/kotlin/com/plusmobileapps/chefmate/recipebook/data/testing/FakeRecipeBookRepository.kt b/client/recipebook/data/testing/src/commonMain/kotlin/com/plusmobileapps/chefmate/recipebook/data/testing/FakeRecipeBookRepository.kt index 5915ae4b7..66d8e1318 100644 --- a/client/recipebook/data/testing/src/commonMain/kotlin/com/plusmobileapps/chefmate/recipebook/data/testing/FakeRecipeBookRepository.kt +++ b/client/recipebook/data/testing/src/commonMain/kotlin/com/plusmobileapps/chefmate/recipebook/data/testing/FakeRecipeBookRepository.kt @@ -81,7 +81,12 @@ class FakeRecipeBookRepository( } } - override suspend fun syncAllUnsynced() {} + var syncAllUnsyncedCallCount: Int = 0 + private set + + override suspend fun syncAllUnsynced() { + syncAllUnsyncedCallCount += 1 + } override suspend fun clearLocalData() { books.value = emptyList() diff --git a/client/ui/public/src/commonMain/composeResources/values/strings.xml b/client/ui/public/src/commonMain/composeResources/values/strings.xml index 008b2ebe5..748352648 100644 --- a/client/ui/public/src/commonMain/composeResources/values/strings.xml +++ b/client/ui/public/src/commonMain/composeResources/values/strings.xml @@ -20,4 +20,5 @@ Got it Sign In Sign Up + Can\'t sync right now — check your connection, or sign in again \ No newline at end of file