diff --git a/app/src/gplay/java/eu/darken/capod/common/review/GplayReviewTool.kt b/app/src/gplay/java/eu/darken/capod/common/review/GplayReviewTool.kt index dac10bd1..a0ee0e5e 100644 --- a/app/src/gplay/java/eu/darken/capod/common/review/GplayReviewTool.kt +++ b/app/src/gplay/java/eu/darken/capod/common/review/GplayReviewTool.kt @@ -20,6 +20,7 @@ import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.catch import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.flatMapLatest @@ -65,6 +66,13 @@ class GplayReviewTool @Inject constructor( hasPaidForPro && !isSnoozed && !hasReviewed } .distinctUntilChanged() + // Upstream of the shares below: an exception there would kill the sharing coroutine on + // AppScope (crashing the process) instead of reaching any downstream `catch`. + .catch { e -> + if (e is CancellationException) throw e + log(TAG, ERROR) { "Eligibility failed: ${e.asLog()}" } + emit(false) + } // Only probed once the user is eligible: Play counts requests against the app's quota, and an // `isNoOp` answer is Play's deliberate verdict, i.e. an answer and not a failure to retry. @@ -107,6 +115,11 @@ class GplayReviewTool @Inject constructor( } .throttleLatest(500) .onStart { emit(ReviewTool.State()) } + .catch { e -> + if (e is CancellationException) throw e + log(TAG, ERROR) { "State failed: ${e.asLog()}" } + emit(ReviewTool.State()) + } .replayingShare(appScope) // Single-flight: a second tap must not queue up behind the first, or Play's flow would be diff --git a/app/src/main/java/eu/darken/capod/main/ui/overview/OverviewViewModel.kt b/app/src/main/java/eu/darken/capod/main/ui/overview/OverviewViewModel.kt index f01fd9aa..ce92d1c7 100644 --- a/app/src/main/java/eu/darken/capod/main/ui/overview/OverviewViewModel.kt +++ b/app/src/main/java/eu/darken/capod/main/ui/overview/OverviewViewModel.kt @@ -332,14 +332,15 @@ class OverviewViewModel @Inject constructor( /** * Whether the overview is currently showing a card that outranks the review prompt: a - * missing permission, the troubleshooter hint, the background-monitoring-off notice or the - * no-profiles setup card. All of those ask the user to do something, so the review prompt - * stays hidden while any of them is on screen. + * missing permission, the troubleshooter hint, the background-monitoring-off notice, the + * no-profiles setup card or the enable-Bluetooth prompt. All of those ask the user to do + * something, so the review prompt stays hidden while any of them is on screen. */ val hasHigherPriorityCard: Boolean get() = permissions.isNotEmpty() || showTroubleshootSuggestion || monitoringStatus == MonitoringStatus.BACKGROUND_OFF || + !isBluetoothEnabled || (profiles.isEmpty() && !isScanBlocked && isBluetoothEnabled) val soleProfileId: ProfileId? get() = profiles.singleOrNull()?.id diff --git a/app/src/test/java/eu/darken/capod/main/ui/overview/OverviewViewModelTest.kt b/app/src/test/java/eu/darken/capod/main/ui/overview/OverviewViewModelTest.kt index b4bd9c77..cbcdc56d 100644 --- a/app/src/test/java/eu/darken/capod/main/ui/overview/OverviewViewModelTest.kt +++ b/app/src/test/java/eu/darken/capod/main/ui/overview/OverviewViewModelTest.kt @@ -927,6 +927,16 @@ class OverviewViewModelTest : BaseTest() { vm.state.first().showReviewCard shouldBe false } + @Test + fun `suppressed by the enable-Bluetooth prompt`() = runTest(testDispatcher) { + quietOverview() + isBluetoothEnabledFlow.value = false + + val vm = createViewModel() + + vm.state.first().showReviewCard shouldBe false + } + @Test fun `suppressed by the troubleshooter suggestion`() = runTest(testDispatcher) { quietOverview() diff --git a/app/src/testGplay/java/eu/darken/capod/common/review/GplayReviewToolTest.kt b/app/src/testGplay/java/eu/darken/capod/common/review/GplayReviewToolTest.kt index 02ba6dc3..82d589a6 100644 --- a/app/src/testGplay/java/eu/darken/capod/common/review/GplayReviewToolTest.kt +++ b/app/src/testGplay/java/eu/darken/capod/common/review/GplayReviewToolTest.kt @@ -19,11 +19,13 @@ import io.mockk.verify import kotlinx.coroutines.CancellationException import kotlinx.coroutines.flow.drop import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.launch import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.withTimeoutOrNull +import kotlinx.serialization.SerializationException import org.junit.jupiter.api.Test import testhelpers.BaseTest import testhelpers.coroutine.runTest2 @@ -234,6 +236,32 @@ class GplayReviewToolTest : BaseTest() { verify(exactly = 3) { manager.requestReviewFlow() } } + @Test fun `corrupt review settings fall back to the default state`() = runTest2 { + every { manager.requestReviewFlow() } returns Tasks.forResult(reviewInfo()) + lastDismissedMock = rwSetting(null) + // A malformed stored timestamp throws when the DataStore flow is read. + reviewedAtMock = mockk>(relaxed = true).apply { + every { flow } returns flow { throw SerializationException("corrupt") } + } + every { settings.lastDismissed } returns lastDismissedMock + every { settings.reviewedAt } returns reviewedAtMock + + val upgradeInfo = mockk() + every { upgradeInfo.upgradedAt } returns Instant.now().minus(Duration.ofDays(30)) + every { upgradeRepo.upgradeInfo } returns flowOf(upgradeInfo) + + val tool = GplayReviewTool( + appScope = backgroundScope, + settings = settings, + manager = manager, + upgradeRepo = upgradeRepo, + ) + + // The failure has to be absorbed upstream of the share: on the shared coroutine it would go + // to AppScope (no handler, i.e. a process crash) and never reach a downstream collector. + tool.computedState() shouldBe ReviewTool.State() + } + @Test fun `cancellation during reviewNow is not swallowed`() = runTest2 { every { manager.requestReviewFlow() } throws CancellationException("scope died") val tool = tool()