diff --git a/app/src/gplay/java/eu/darken/capod/common/upgrade/core/data/BillingDataRepo.kt b/app/src/gplay/java/eu/darken/capod/common/upgrade/core/data/BillingDataRepo.kt index c7e15fe6..97f39305 100644 --- a/app/src/gplay/java/eu/darken/capod/common/upgrade/core/data/BillingDataRepo.kt +++ b/app/src/gplay/java/eu/darken/capod/common/upgrade/core/data/BillingDataRepo.kt @@ -35,21 +35,30 @@ class BillingDataRepo @Inject constructor( // Null until the first attempt, so devices with less than an hour of uptime still refresh. private var lastForegroundRefreshAt: Long? = null + // Explicit billing operations kick a waiting connection-retry backoff so a user action (restore + // tap, buy tap, opening the upgrade screen) reconnects immediately after Play was fixed instead + // of waiting out the timer. Zero replay: kicks only matter while a retry is actively waiting — + // a healthy connection must not accumulate stale wake-ups. (Ported from sdmaid-se#2562.) + private val connectionKicks = MutableSharedFlow(extraBufferCapacity = 1) + private val connectionProvider = billingClientConnectionProvider.connection .retryWhen { cause, attempt -> if (cause is CancellationException) return@retryWhen false log(TAG, ERROR) { "Unable to provide client connection (attempt=$attempt):\n${cause.asLog()}" } // Capped backoff: don't hammer a persistently broken Play from the always-hot process - // (upstream already did 5 quick retries). A foreground *entry* short-circuits the - // wait — e.g. the user just returned from signing into the missing Google account - // and shouldn't have to wait out the full backoff. + // (upstream already did 5 quick retries). An explicit billing operation or a foreground + // *entry* short-circuits the wait — e.g. the user just returned from signing into the + // missing Google account and shouldn't have to wait out the full backoff. val backoffMs = (RETRY_BACKOFF_BASE_MS * (attempt + 1)).coerceAtMost(RETRY_BACKOFF_MAX_MS) - // StateFlow dedupes, so after dropping the current value the next `true` is a real - // foreground *entry*, not the pre-existing foreground state. val kicked = withTimeoutOrNull(backoffMs) { - appForegroundState.isForeground.drop(1).first { it } + merge( + connectionKicks, + // StateFlow dedupes, so after dropping the current value the next `true` is a + // real foreground *entry*, not the pre-existing foreground state. + appForegroundState.isForeground.drop(1).filter { it }.map { }, + ).first() } - if (kicked != null) log(TAG) { "App came to foreground, retrying billing connection early" } + if (kicked != null) log(TAG) { "User action or foreground entry, retrying billing connection early" } true } .replayingShare(scope) @@ -160,6 +169,7 @@ class BillingDataRepo @Inject constructor( } suspend fun refresh(): BillingData = try { + connectionKicks.tryEmit(Unit) val clientConnection = connectionProvider.first() val purchases = clientConnection.refreshPurchases() @@ -169,6 +179,7 @@ class BillingDataRepo @Inject constructor( } suspend fun querySkus(vararg skus: Sku): Collection = try { + connectionKicks.tryEmit(Unit) val clientConnection = connectionProvider.first() clientConnection.querySkus(*skus) } catch (e: Exception) { @@ -181,6 +192,7 @@ class BillingDataRepo @Inject constructor( offer: Sku.Subscription.Offer? = null, ) { try { + connectionKicks.tryEmit(Unit) val clientConnection = connectionProvider.first() clientConnection.launchBillingFlow(activity, sku, offer) } catch (e: CancellationException) { diff --git a/app/src/testGplay/java/eu/darken/capod/common/upgrade/core/data/BillingDataRepoTest.kt b/app/src/testGplay/java/eu/darken/capod/common/upgrade/core/data/BillingDataRepoTest.kt index 55906203..e36e85a8 100644 --- a/app/src/testGplay/java/eu/darken/capod/common/upgrade/core/data/BillingDataRepoTest.kt +++ b/app/src/testGplay/java/eu/darken/capod/common/upgrade/core/data/BillingDataRepoTest.kt @@ -18,6 +18,7 @@ import io.mockk.coVerify import io.mockk.every import io.mockk.mockk import kotlinx.coroutines.cancel +import kotlinx.coroutines.launch import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.emptyFlow import kotlinx.coroutines.flow.flow @@ -107,26 +108,72 @@ class BillingDataRepoTest : BaseTest() { every { isForeground } returns foreground } - BillingDataRepo(provider, testScope, foregroundState, TestTimeSource()) - testScope.testScheduler.runCurrent() - attempts shouldBe 1 + try { + BillingDataRepo(provider, testScope, foregroundState, TestTimeSource()) + testScope.testScheduler.runCurrent() + attempts shouldBe 1 - // First backoff is 60s — not a second sooner. - testScope.testScheduler.advanceTimeBy(59_000) - testScope.testScheduler.runCurrent() - attempts shouldBe 1 - testScope.testScheduler.advanceTimeBy(2_000) - testScope.testScheduler.runCurrent() - attempts shouldBe 2 + // First backoff is 60s — not a second sooner. + testScope.testScheduler.advanceTimeBy(59_000) + testScope.testScheduler.runCurrent() + attempts shouldBe 1 + testScope.testScheduler.advanceTimeBy(2_000) + testScope.testScheduler.runCurrent() + attempts shouldBe 2 - // Second backoff would be 120s — a foreground entry short-circuits it, so a user - // returning from e.g. Google sign-in doesn't wait out the full backoff. - testScope.testScheduler.advanceTimeBy(5_000) - foreground.value = true - testScope.testScheduler.runCurrent() - attempts shouldBe 3 + // Second backoff would be 120s — a foreground entry short-circuits it, so a user + // returning from e.g. Google sign-in doesn't wait out the full backoff. A foreground + // entry drives two independent early-retry paths (the retry loop's own foreground + // branch, plus the init foreground-refresh calling refresh() which emits a kick), so + // the exact count is interleaving-dependent — either can produce attempt 3 or 4. + testScope.testScheduler.advanceTimeBy(5_000) + foreground.value = true + testScope.testScheduler.runCurrent() + (attempts in 3..4) shouldBe true - testScope.cancel() + // ...and then it settles: with no further action/lifecycle signal and time still well + // under the next backoff, the double-kick must not compound into a busy-loop. + val settled = attempts + testScope.testScheduler.advanceTimeBy(5_000) + testScope.testScheduler.runCurrent() + attempts shouldBe settled + } finally { + // The repo pipelines and retry loop are infinite — a leaked scope after a failed + // assertion would keep them alive for the rest of the JVM. + testScope.cancel() + } + } + + @Test + fun `explicit billing operations kick a waiting connection retry`() = runTest2 { + val testScope = TestScope(UnconfinedTestDispatcher(testScheduler)) + var attempts = 0 + val provider = mockk { + every { connection } returns flow { + attempts++ + throw BillingException("still broken") + } + } + val foregroundState = mockk { + every { isForeground } returns MutableStateFlow(false) + } + + try { + val repo = BillingDataRepo(provider, testScope, foregroundState, TestTimeSource()) + testScope.testScheduler.runCurrent() + attempts shouldBe 1 + + // A restore-style refresh() while the retry is waiting out its 60s backoff kicks it + // immediately — the user shouldn't wait out the timer after fixing Play themselves. + testScope.testScheduler.advanceTimeBy(5_000) + val refreshJob = testScope.launch { runCatching { repo.refresh() } } + testScope.testScheduler.runCurrent() + attempts shouldBe 2 + + refreshJob.cancel() + } finally { + testScope.cancel() + } } @Test