Cancel dropped page states and restore the failure catch (#336)

Evicted week and day states kept their sharing coroutine, and their last loaded events, alive until the view was closed; each now shares on its own job, cancelled when it is let go.
2026-09-25 14:35:50 +02:00
parent 7c17bf2ff4
commit eda616b42e
4 changed files with 144 additions and 46 deletions
@@ -0,0 +1,53 @@
+package de.jeanlucmakiola.calendula.ui.common
+
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Job
+import kotlinx.coroutines.SupervisorJob
+import kotlinx.coroutines.flow.Flow
+import kotlinx.coroutines.flow.SharingStarted
+import kotlinx.coroutines.flow.StateFlow
+import kotlinx.coroutines.flow.stateIn
+import kotlinx.coroutines.plus
+
+/**
+ * One shared state per pager page — a week, a day, a month — so a page and the
+ * screen's anchor read the same query, and the pages either side stay loaded
+ * across a swipe.
+ *
+ * Every entry shares on its own job, cancelled when [keep] lets it go: a
+ * `stateIn` on the ViewModel's scope would stay running, holding its last state,
+ * until the ViewModel itself was cleared. Main thread only.
+ *
+ * @param keep whether a cached key is still worth holding once [get] asks for another.
+ */
+class PageStateCache<K, S>(
+    private val scope: CoroutineScope,
+    private val initial: S,
+    private val keep: (cached: K, requested: K) -> Boolean,
+    private val load: (K) -> Flow<S>,
+) {
+    private class Entry<S>(val state: StateFlow<S>, val job: Job)
+
+    private val entries = HashMap<K, Entry<S>>()
+
+    /** The number of entries held, for tests. */
+    internal val size: Int get() = entries.size
+
+    fun get(key: K): StateFlow<S> {
+        val iterator = entries.iterator()
+        while (iterator.hasNext()) {
+            val (cached, entry) = iterator.next()
+            if (cached != key && !keep(cached, key)) {
+                entry.job.cancel()
+                iterator.remove()
+            }
+        }
+        return entries.getOrPut(key) {
+            val job = SupervisorJob(scope.coroutineContext[Job])
+            Entry(
+                state = load(key).stateIn(scope + job, SharingStarted.WhileSubscribed(5_000L), initial),
+                job = job,
+            )
+        }.state
+    }
+}
@@ -9,6 +9,7 @@ import de.jeanlucmakiola.calendula.domain.CalendarSource
 import de.jeanlucmakiola.calendula.domain.EventInstance
 import de.jeanlucmakiola.calendula.domain.FailureReason
 import de.jeanlucmakiola.calendula.domain.calendarListFailure
+import de.jeanlucmakiola.calendula.ui.common.PageStateCache
 import de.jeanlucmakiola.calendula.ui.week.layoutAllDay
 import de.jeanlucmakiola.calendula.ui.week.layoutDay
 import kotlinx.coroutines.CoroutineDispatcher
@@ -68,38 +69,33 @@ class DayViewModel @Inject constructor(
     */
    val state: StateFlow<DayUiState> = _date
        .flatMapLatest { date -> day(date).filterNot { it is DayUiState.Loading } }
+        .catch { emit(DayUiState.Failure(FailureReason.ProviderUnavailable)) }
        .stateIn(
            scope = viewModelScope,
            started = SharingStarted.WhileSubscribed(5_000L),
            initialValue = DayUiState.Loading,
        )

-    private val days = HashMap<LocalDate, StateFlow<DayUiState>>()
-
-    /**
-     * The day [date], shared between its pager page and [state]. Days far from
-     * the one asked for are dropped, so paging through a month doesn't keep a
-     * month of queries around. Main thread only.
-     */
-    fun day(date: LocalDate): StateFlow<DayUiState> {
-        days.keys.removeAll { abs(it.daysUntil(date)) > DAY_CACHE_DAYS }
-        return days.getOrPut(date) {
-            combine(
-                repository.calendars(),
-                repository.instances(dayRange(date, zone)),
-            ) { calendars, instances ->
-                buildState(date, calendars, instances)
-            }
-                .catch { emit(DayUiState.Failure(FailureReason.ProviderUnavailable)) }
-                .flowOn(io)
-                .stateIn(
-                    scope = viewModelScope,
-                    started = SharingStarted.WhileSubscribed(5_000L),
-                    initialValue = DayUiState.Loading,
-                )
+    private val days = PageStateCache<LocalDate, DayUiState>(
+        scope = viewModelScope,
+        initial = DayUiState.Loading,
+        // Far enough to cover the pages either side and a swipe back, so paging
+        // through a month doesn't keep a month of queries around.
+        keep = { cached, requested -> abs(cached.daysUntil(requested)) <= DAY_CACHE_DAYS },
+    ) { date ->
+        combine(
+            repository.calendars(),
+            repository.instances(dayRange(date, zone)),
+        ) { calendars, instances ->
+            buildState(date, calendars, instances)
        }
+            .catch { emit(DayUiState.Failure(FailureReason.ProviderUnavailable)) }
+            .flowOn(io)
    }

+    /** The day [date], shared between its pager page and [state]. Main thread only. */
+    fun day(date: LocalDate): StateFlow<DayUiState> = days.get(date)
+
    /** The pager came to rest on [page]; follow it unless it is already the anchor's. */
    fun onPageSettled(page: Int) {
        if (dayPageFor(_date.value) != page) _date.value = dayForPage(page)
@@ -12,6 +12,7 @@ import de.jeanlucmakiola.calendula.domain.EventInstance
 import de.jeanlucmakiola.calendula.domain.FailureReason
 import de.jeanlucmakiola.calendula.domain.calendarListFailure
 import de.jeanlucmakiola.calendula.domain.isDeclined
+import de.jeanlucmakiola.calendula.ui.common.PageStateCache
 import kotlinx.coroutines.CoroutineDispatcher
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.flow.MutableStateFlow
@@ -101,38 +102,33 @@ class WeekViewModel @Inject constructor(
        combine(_anchor, firstDayOfWeek) { anchor, ws -> anchor.startOfWeek(ws) }
            .distinctUntilChanged()
            .flatMapLatest { start -> week(start).filterNot { it is WeekUiState.Loading } }
+            .catch { emit(WeekUiState.Failure(FailureReason.ProviderUnavailable)) }
            .stateIn(
                scope = viewModelScope,
                started = SharingStarted.WhileSubscribed(5_000L),
                initialValue = WeekUiState.Loading,
            )

-    private val weeks = HashMap<LocalDate, StateFlow<WeekUiState>>()
-
-    /**
-     * The week starting at [start], shared between its pager page and [state].
-     * Weeks far from the one asked for are dropped, so paging through a year
-     * doesn't keep a year of queries around. Main thread only.
-     */
-    fun week(start: LocalDate): StateFlow<WeekUiState> {
-        weeks.keys.removeAll { abs(it.daysUntil(start)) > WEEK_CACHE_DAYS }
-        return weeks.getOrPut(start) {
-            combine(
-                repository.calendars(),
-                repository.instances(weekRange(start, zone)),
-            ) { calendars, instances ->
-                buildState(start, calendars, instances)
-            }
-                .catch { emit(WeekUiState.Failure(FailureReason.ProviderUnavailable)) }
-                .flowOn(io)
-                .stateIn(
-                    scope = viewModelScope,
-                    started = SharingStarted.WhileSubscribed(5_000L),
-                    initialValue = WeekUiState.Loading,
-                )
+    private val weeks = PageStateCache<LocalDate, WeekUiState>(
+        scope = viewModelScope,
+        initial = WeekUiState.Loading,
+        // Far enough to cover the pages either side and a swipe back, so paging
+        // through a year doesn't keep a year of queries around.
+        keep = { cached, requested -> abs(cached.daysUntil(requested)) <= WEEK_CACHE_DAYS },
+    ) { start ->
+        combine(
+            repository.calendars(),
+            repository.instances(weekRange(start, zone)),
+        ) { calendars, instances ->
+            buildState(start, calendars, instances)
        }
+            .catch { emit(WeekUiState.Failure(FailureReason.ProviderUnavailable)) }
+            .flowOn(io)
    }

+    /** The week starting at [start], shared between its pager page and [state]. Main thread only. */
+    fun week(start: LocalDate): StateFlow<WeekUiState> = weeks.get(start)
+
    /** The pager came to rest on [page]; follow it unless it is already the anchor's. */
    fun onPageSettled(page: Int) {
        val ws = firstDayOfWeek.value
@@ -0,0 +1,53 @@
+package de.jeanlucmakiola.calendula.ui.common
+
+import com.google.common.truth.Truth.assertThat
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.awaitCancellation
+import kotlinx.coroutines.flow.flow
+import kotlinx.coroutines.launch
+import kotlinx.coroutines.test.UnconfinedTestDispatcher
+import kotlinx.coroutines.test.runTest
+import org.junit.jupiter.api.Test
+import kotlin.math.abs
+
+@OptIn(ExperimentalCoroutinesApi::class)
+class PageStateCacheTest {
+
+    @Test
+    fun `the same key shares one state`() = runTest {
+        val cache = PageStateCache<Int, Int>(backgroundScope, 0, { a, b -> abs(a - b) <= 1 }) { flow { emit(it) } }
+        assertThat(cache.get(5)).isSameInstanceAs(cache.get(5))
+    }
+
+    @Test
+    fun `a dropped entry stops loading`() = runTest(UnconfinedTestDispatcher()) {
+        var running = 0
+        val cache = PageStateCache<Int, Int>(backgroundScope, 0, { a, b -> abs(a - b) <= 1 }) { key ->
+            flow {
+                running++
+                try {
+                    emit(key)
+                    awaitCancellation()
+                } finally {
+                    running--
+                }
+            }
+        }
+        backgroundScope.launch { cache.get(0).collect {} }
+        assertThat(running).isEqualTo(1)
+
+        // Far enough away that 0 is let go, even though it is still collected.
+        cache.get(10)
+        assertThat(running).isEqualTo(0)
+        assertThat(cache.size).isEqualTo(1)
+    }
+
+    @Test
+    fun `nearby entries are kept`() = runTest {
+        val cache = PageStateCache<Int, Int>(backgroundScope, 0, { a, b -> abs(a - b) <= 2 }) { flow { emit(it) } }
+        cache.get(4)
+        cache.get(5)
+        cache.get(6)
+        assertThat(cache.size).isEqualTo(3)
+    }
+}