diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt index c525fff..e26c246 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt @@ -18,6 +18,7 @@ import de.jeanlucmakiola.clockula.ui.common.RingtonePicker import de.jeanlucmakiola.clockula.ui.common.RingtonePickerUiState import de.jeanlucmakiola.floret.di.ApplicationScope import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.CoroutineStart import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow @@ -26,6 +27,8 @@ import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.map import kotlinx.coroutines.launch +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import java.time.DayOfWeek import javax.inject.Inject import kotlin.time.Duration @@ -91,6 +94,9 @@ class AlarmEditorViewModel @Inject internal constructor( fun onAlarmSetNoticeShown(notice: AlarmSetNotice) = announcer.consumed(notice) + /** Orders [scheduleWrite]s. */ + private val scheduleWrites = Mutex() + /** Memoised catalog answers, keyed on URI — see [metaOf]. */ private val ringtoneMeta = mutableMapOf() @@ -146,9 +152,9 @@ class AlarmEditorViewModel @Inject internal constructor( } fun onSkipNextChanged(skip: Boolean) { - // On the application scope, like every write that moves a fire time: - // see [edit]. - appScope.launch { + // Absolute, so it must land in tap order against a day toggle that + // disarms the skip: see [scheduleWrite]. + scheduleWrite { alarms.setSkipNextOccurrence(alarmId, skip) engine.reschedule() // Skipping moves the next ring a day or more out, which is exactly @@ -238,7 +244,7 @@ class AlarmEditorViewModel @Inject internal constructor( /** Dismisses a ring first (M5 D11), deletes, and the state becomes [AlarmEditorUiState.Gone]. */ fun onDelete() { - viewModelScope.launch { + scheduleWrite { if (engine.ringSession()?.alarm?.id == alarmId) engine.dismiss(alarmId) alarms.delete(alarmId) engine.reschedule() @@ -281,13 +287,30 @@ class AlarmEditorViewModel @Inject internal constructor( settle: Duration = Duration.ZERO, transform: (Alarm) -> Alarm, ) { - (if (reschedule) appScope else viewModelScope).launch { + if (!reschedule) { + viewModelScope.launch { alarms.edit(alarmId, transform) } + return + } + scheduleWrite { alarms.edit(alarmId, transform) - if (reschedule) engine.reschedule() + engine.reschedule() if (announces) announcer.announce(alarmId, settle) } } + /** + * A write that moves a fire time, its reschedule and whatever follows, on + * the application scope (see [edit]) and **in tap order**: started + * undispatched, each takes its place in [scheduleWrites]' fair queue on the + * caller's thread, before the first suspension. The application scope is + * multi-threaded, so without it "skip next" followed at once by untapping + * the only repeat day could commit out of order, leaving a one-shot with a + * skip armed — which the resolver answers by disabling it (D14). + */ + private fun scheduleWrite(block: suspend () -> Unit) { + appScope.launch(start = CoroutineStart.UNDISPATCHED) { scheduleWrites.withLock { block() } } + } + private suspend fun render(inputs: Inputs): AlarmEditorUiState { val alarm = inputs.alarm ?: return AlarmEditorUiState.Gone return AlarmEditorUiState.Editing( diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt index bfb3df4..fbdf471 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt @@ -23,7 +23,8 @@ import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.collectLatest -import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch import kotlinx.coroutines.sync.Mutex @@ -84,14 +85,19 @@ internal class AlarmSetAnnouncer @Inject constructor( /** * The notice to show, or null. Past [FRESH_FOR] it is dropped unshown: a * countdown that surfaced when the user came back to the app a while later - * would be wrong, and about something they did long ago. It is checked - * against the engine again on the way out, so an alarm switched off or - * deleted by any path — the ring screen, a notification, an intent, a - * restore — is never announced as set. + * would be wrong, and about something they did long ago. + * + * Live against the engine: every alarm write re-reads it, so an alarm + * switched off or deleted by any path — the ring screen, a notification, + * an intent, a restore — takes a snackbar down even while it is showing, + * and one whose ring moved (a skip armed from the notification) says the + * new countdown rather than the one it was announced with. */ - val notice: Flow = current.map { notice -> - notice?.takeIf { isFresh(it) && stillRings(it) } - } + val notice: Flow = combine(current, engine.upcoming()) { notice, upcoming -> + if (notice == null || !isFresh(notice)) return@combine null + val entry = upcoming.firstOrNull { it.alarm.id == notice.alarmId } + AlarmSetCountdown.of(entry, wall.now())?.let { notice.copy(countdown = it) } + }.distinctUntilChanged() /** * Call once the write is stored. Announces [alarmId] after [settle], unless @@ -124,9 +130,14 @@ internal class AlarmSetAnnouncer @Inject constructor( } } - /** [notice] was shown until it timed out or was swiped away. */ + /** + * [notice] was shown until it timed out or was swiped away. Matched on the + * announcement, not the words: the countdown shown may be a re-read one. + */ fun consumed(notice: AlarmSetNotice) { - current.compareAndSet(notice, null) + current.update { standing -> + standing?.takeUnless { it.alarmId == notice.alarmId && it.announcedAt == notice.announcedAt } + } } private suspend fun publish(alarmId: Long) = lock.withLock { @@ -143,8 +154,6 @@ internal class AlarmSetAnnouncer @Inject constructor( private fun isFresh(notice: AlarmSetNotice): Boolean = elapsed.elapsedRealtime() - notice.announcedAt < FRESH_FOR - private suspend fun stillRings(notice: AlarmSetNotice): Boolean = countdownOf(notice.alarmId) != null - private suspend fun countdownOf(alarmId: Long): NextFireLabel? { // One `now` for both ends of the countdown, or a resolve that lands a // hair past the occurrence would read as a day and an hour. diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt index f245c7a..0ab4d95 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt @@ -473,6 +473,38 @@ class AlarmEditorViewModelTest { assertThat(announcer.notice.first()).isNull() } + @Test + fun `a ring moved by another path is announced with its new countdown`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir, now = fiveToday) + harness.givenAlarmWith(hour = 7, repeatDays = RepeatDays.EVERY_DAY) + val viewModel = editor(harness) + advanceUntilIdle() + viewModel.onTimePicked(7, 0) + advanceUntilIdle() + + // A skip armed from the upcoming-alarm notification: no retract. + harness.alarms.setSkipNextOccurrence(1L, true) + + assertThat(viewModel.countdown()).isEqualTo(NextFireLabel.InDaysHours(1, 2)) + } + + @Test + fun `skip then untapping the only day lands in tap order`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir, now = fiveToday) + harness.givenAlarmWith(hour = 7, repeatDays = RepeatDays.of(DayOfWeek.MONDAY)) + val viewModel = editor(harness) + advanceUntilIdle() + + viewModel.onSkipNextChanged(true) + viewModel.onDayToggled(DayOfWeek.MONDAY) + advanceUntilIdle() + + val stored = harness.alarmDao.stored.single() + assertThat(stored.enabled to stored.skipNextOccurrence).isEqualTo(true to false) + } + @Test fun `an alarm switched off by another path is not announced as set`(@TempDir tempDir: Path) = runTest(main.dispatcher) {