diff --git a/app/src/foss/java/eu/darken/capod/common/upgrade/ui/UpgradeViewModel.kt b/app/src/foss/java/eu/darken/capod/common/upgrade/ui/UpgradeViewModel.kt index c6d5f418..482dfbbe 100644 --- a/app/src/foss/java/eu/darken/capod/common/upgrade/ui/UpgradeViewModel.kt +++ b/app/src/foss/java/eu/darken/capod/common/upgrade/ui/UpgradeViewModel.kt @@ -173,10 +173,16 @@ class UpgradeViewModel @Inject constructor( } catch (e: Exception) { // The marker was consumed above; neither a failed entitlement read nor a failed write may // eat the user's valid sponsor visit — restore it so the next return/resume can retry the - // unlock. Rethrow unconditionally: cancellation is not swallowed, other errors surface via - // the normal error path. A restored marker after a successful persist is harmless — the - // next evaluation hits the quiet isPro path. - handle[KEY_SPONSOR_PRESSED_AT] = pressedAt + // unlock. Conditional: the user may have armed a NEWER launch while this attempt was + // suspended, and that one must survive. The contains-check has a small check-then-act + // window against a concurrent new arm; accepted — the create-only transaction owns data + // integrity, a wrong winner only changes which REAL visit's timestamp gates the unlock. + // Rethrow unconditionally: cancellation is not swallowed, other errors surface via the + // normal error path. A restored marker after a successful persist is harmless — the next + // evaluation hits the quiet isPro path. + if (!handle.contains(KEY_SPONSOR_PRESSED_AT)) { + handle[KEY_SPONSOR_PRESSED_AT] = pressedAt + } throw e } } diff --git a/app/src/main/java/eu/darken/capod/common/debug/recording/core/RecorderModule.kt b/app/src/main/java/eu/darken/capod/common/debug/recording/core/RecorderModule.kt index 6d37d879..07fe4996 100644 --- a/app/src/main/java/eu/darken/capod/common/debug/recording/core/RecorderModule.kt +++ b/app/src/main/java/eu/darken/capod/common/debug/recording/core/RecorderModule.kt @@ -121,7 +121,7 @@ class RecorderModule @Inject constructor( recorder = newRecorder, currentLogDir = sessionDir, recordingStartedAt = startTime, - recordingStartedAtMonotonic = if (isResume) 0L else timeSource.elapsedRealtime(), + recordingStartedAtMonotonic = if (isResume) null else timeSource.elapsedRealtime(), persistedLogDir = null, ) } else if (!shouldRecord && isRecording) { @@ -137,6 +137,7 @@ class RecorderModule @Inject constructor( recorder = null, currentLogDir = null, recordingStartedAt = 0L, + recordingStartedAtMonotonic = null, ) } else { this @@ -238,9 +239,10 @@ class RecorderModule @Inject constructor( if (!currentState.isRecording) return StopResult.NotRecording val logDir = currentState.currentLogDir ?: return StopResult.NotRecording - val elapsed = if (currentState.recordingStartedAtMonotonic > 0L) { + val startedAtMono = currentState.recordingStartedAtMonotonic + val elapsed = if (startedAtMono != null) { // Live session: monotonic, immune to wall-clock adjustments mid-recording. - timeSource.elapsedRealtime() - currentState.recordingStartedAtMonotonic + timeSource.elapsedRealtime() - startedAtMono } else { // Resumed session: the trigger file persists wall time only — it has to survive reboots, // which monotonic time does not. @@ -266,10 +268,10 @@ class RecorderModule @Inject constructor( internal val recorder: Recorder? = null, val currentLogDir: File? = null, val recordingStartedAt: Long = 0L, - // Monotonic base for the duration heuristic, 0L when there is none: a resumed session's + // Monotonic base for the duration heuristic, null when there is none: a resumed session's // only start time is the persisted wall clock, and a monotonic value from a previous // process or boot is meaningless. - val recordingStartedAtMonotonic: Long = 0L, + internal val recordingStartedAtMonotonic: Long? = null, internal val persistedLogDir: File? = null, ) { val isRecording: Boolean diff --git a/app/src/test/java/eu/darken/capod/common/debug/recording/core/RecorderModuleDurationTest.kt b/app/src/test/java/eu/darken/capod/common/debug/recording/core/RecorderModuleDurationTest.kt index e9de2304..e8db6484 100644 --- a/app/src/test/java/eu/darken/capod/common/debug/recording/core/RecorderModuleDurationTest.kt +++ b/app/src/test/java/eu/darken/capod/common/debug/recording/core/RecorderModuleDurationTest.kt @@ -172,6 +172,53 @@ class RecorderModuleDurationTest : BaseTest() { } } + @Test + fun `a recording started at monotonic zero still uses the monotonic path`() { + // Boot-adjacent start: elapsedRealtime() is legitimately 0 right after boot. A 0L sentinel + // reads as "resumed" and diverts to the wall clock, so a clock correction would turn three + // seconds of recording into an hour and skip the prompt. + val timeSource = TestTimeSource(elapsedRealtimeMs = 0L) + withModule(timeSource) { module -> + module.startRecorder() + + timeSource.elapsedRealtimeMs += 3_000L + timeSource.wallNow = timeSource.wallNow.plus(Duration.ofHours(1)) + + module.requestStopRecorder() shouldBe RecorderModule.StopResult.TooShort + module.state.first().isRecording shouldBe true + } + } + + @Test + fun `a live session carries a monotonic base and a stop clears it`() { + val timeSource = TestTimeSource(elapsedRealtimeMs = 100_000L) + withModule(timeSource) { module -> + module.startRecorder() + module.state.first { it.isRecording }.recordingStartedAtMonotonic shouldBe 100_000L + + timeSource.advanceBy(Duration.ofSeconds(10)) + module.requestStopRecorder().shouldBeInstanceOf() + + // A stale base left on the stopped state would be a lie about a session that is over. + module.state.first { !it.isRecording }.recordingStartedAtMonotonic shouldBe null + } + } + + @Test + fun `a resumed session carries no monotonic base`() { + // Nothing monotonic survives a process death or reboot, so the resumed state has no base + // at all — the wall-clock fallback is the only measurement it can make. + val timeSource = TestTimeSource(elapsedRealtimeMs = 100_000L) + seedTriggerFile(timeSource.currentTimeMillis() - 20_000L) + + withModule(timeSource) { module -> + module.state.first { it.isRecording }.recordingStartedAtMonotonic shouldBe null + + module.requestStopRecorder().shouldBeInstanceOf() + module.state.first { !it.isRecording }.recordingStartedAtMonotonic shouldBe null + } + } + @Test fun `a resumed session measures from the persisted start time`() { // Resumed after a process death: there is no monotonic base to measure against, so the diff --git a/app/src/test/java/eu/darken/capod/common/flow/DynamicStateFlowTest.kt b/app/src/test/java/eu/darken/capod/common/flow/DynamicStateFlowTest.kt index cbb53c2f..144cf163 100644 --- a/app/src/test/java/eu/darken/capod/common/flow/DynamicStateFlowTest.kt +++ b/app/src/test/java/eu/darken/capod/common/flow/DynamicStateFlowTest.kt @@ -2,11 +2,13 @@ package eu.darken.capod.common.flow import eu.darken.capod.common.collections.mutate import io.kotest.assertions.throwables.shouldThrow +import io.kotest.matchers.ints.shouldBeGreaterThan import io.kotest.matchers.shouldBe import io.kotest.matchers.types.shouldBeInstanceOf import io.mockk.coEvery import io.mockk.coVerify import io.mockk.mockk +import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.SupervisorJob @@ -304,8 +306,10 @@ class DynamicStateFlowTest : BaseTest() { // own, keeping the producer busy between the other callers' updates. The echo updates // are value-neutral so the final count stays exact, and capped so the echo terminates. val echoes = AtomicInteger(0) + val subscribed = CompletableDeferred() scope.launch { hotData.flow.collect { value -> + subscribed.complete(Unit) if (value % 2 == 1 && echoes.getAndIncrement() < 100) { hotData.updateAsync { this + 0 } } @@ -314,6 +318,10 @@ class DynamicStateFlowTest : BaseTest() { runBlocking { withTimeout(20_000) { + // The contention only means anything with the reactive collector actually + // attached: its first received value proves the subscription exists. + subscribed.await() + val workers = (1..2).map { launch(Dispatchers.IO) { repeat(100) { hotData.updateBlocking { this + 1 } } @@ -323,6 +331,9 @@ class DynamicStateFlowTest : BaseTest() { workers.all { it.isCompleted } shouldBe true hotData.flow.first() shouldBe 200 + // Non-vacuity: without a single echo there was no successor update to displace + // an awaited State, and the test would pass for the wrong reason. + echoes.get() shouldBeGreaterThan 0 } } } finally { diff --git a/app/src/testFoss/java/eu/darken/capod/common/upgrade/ui/FossUpgradeViewModelTest.kt b/app/src/testFoss/java/eu/darken/capod/common/upgrade/ui/FossUpgradeViewModelTest.kt index 4d5ab6bb..6bfd37c3 100644 --- a/app/src/testFoss/java/eu/darken/capod/common/upgrade/ui/FossUpgradeViewModelTest.kt +++ b/app/src/testFoss/java/eu/darken/capod/common/upgrade/ui/FossUpgradeViewModelTest.kt @@ -1,5 +1,6 @@ package eu.darken.capod.common.upgrade.ui +import android.os.SystemClock import androidx.lifecycle.SavedStateHandle import eu.darken.capod.R import eu.darken.capod.common.navigation.NavEvent @@ -14,6 +15,7 @@ import io.mockk.coVerify import io.mockk.every import io.mockk.mockk import io.mockk.verify +import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineStart import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.async @@ -407,6 +409,47 @@ class FossUpgradeViewModelTest : BaseTest() { errorCollector.cancel() } + @Test + fun `a newer sponsor launch survives a failed older attempt`() = runTest2(context = testDispatcher) { + // The restore must not clobber a launch armed while the old attempt was still suspended: + // the newer visit is the one the user is actually waiting on. + val repo = mockRepo() + val gate = CompletableDeferred() + coEvery { repo.persistUpgrade() } coAnswers { + gate.await() + throw IOException("write failed") + } + val handle = SavedStateHandle() + val vm = buildVm(repo = repo, handle = handle) + + val errors = mutableListOf() + val errorCollector = launch(start = CoroutineStart.UNDISPATCHED) { vm.errorEvents.collect { errors.add(it) } } + + vm.goGithubSponsors() + ShadowSystemClock.advanceBy(Duration.ofSeconds(6)) + vm.checkSponsorReturn() + advanceUntilIdle() + // Consumed and parked in the write. + vm.hasPendingSponsorLaunch() shouldBe false + + // A second sponsor visit while the first attempt is still hanging. + ShadowSystemClock.advanceBy(Duration.ofSeconds(30)) + val newerPressedAt = SystemClock.elapsedRealtime() + vm.goGithubSponsors() + advanceUntilIdle() + + gate.complete(Unit) + advanceUntilIdle() + + vm.hasPendingSponsorLaunch() shouldBe true + // Mirrors the ViewModel's private KEY_SPONSOR_PRESSED_AT: the newer timestamp must still be + // the one stored, the failed older attempt must not have written its own back over it. + handle.get("sponsor_pressed_at") shouldBe newerPressedAt + errors.single().shouldBeInstanceOf() + + errorCollector.cancel() + } + @Test fun `a sponsor page that never opened arms nothing and a later retry still works`() = runTest2( context = testDispatcher,