Make UpdatesViewModel collect updates only while subscribed (#3761)

Assisted-by: Claude:claude-opus-5

app/src/main/java/eu/kanade/tachiyomi/ui/updates/UpdatesTab.kt

@@ -6,9 +6,9 @@ import androidx.compose.animation.graphics.vector.AnimatedImageVector
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.DisposableEffect
 import androidx.compose.runtime.LaunchedEffect
-import androidx.compose.runtime.collectAsState
 import androidx.compose.runtime.getValue
 import androidx.compose.ui.platform.LocalContext
+import androidx.lifecycle.compose.collectAsStateWithLifecycle
 import androidx.lifecycle.viewmodel.compose.viewModel
 import cafe.adriel.voyager.navigator.LocalNavigator
 import cafe.adriel.voyager.navigator.Navigator
@@ -56,7 +56,7 @@ data object UpdatesTab : Tab {
         val navigator = LocalNavigator.currentOrThrow
         val viewModel = viewModel<UpdatesViewModel>()
         val settingsViewModel = viewModel<UpdatesSettingsViewModel>()
-        val state by viewModel.state.collectAsState()
+        val state by viewModel.state.collectAsStateWithLifecycle()
 
         UpdateScreen(
             state = state,

app/src/main/java/eu/kanade/tachiyomi/ui/updates/UpdatesViewModel.kt

@@ -5,6 +5,7 @@ import androidx.compose.material3.SnackbarHostState
 import androidx.compose.runtime.Immutable
 import androidx.compose.runtime.getValue
 import androidx.compose.ui.util.fastFilter
+import androidx.lifecycle.ViewModel
 import androidx.lifecycle.viewModelScope
 import eu.kanade.core.preference.asState
 import eu.kanade.core.util.addOrRemove
@@ -17,25 +18,28 @@ import eu.kanade.tachiyomi.data.download.DownloadManager
 import eu.kanade.tachiyomi.data.download.model.Download
 import eu.kanade.tachiyomi.data.library.LibraryUpdateJob
 import eu.kanade.tachiyomi.util.lang.toLocalDate
+import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.channels.Channel
 import kotlinx.coroutines.flow.Flow
+import kotlinx.coroutines.flow.MutableStateFlow
+import kotlinx.coroutines.flow.SharingStarted
+import kotlinx.coroutines.flow.StateFlow
+import kotlinx.coroutines.flow.WhileSubscribed
 import kotlinx.coroutines.flow.catch
-import kotlinx.coroutines.flow.collectLatest
 import kotlinx.coroutines.flow.combine
 import kotlinx.coroutines.flow.distinctUntilChanged
 import kotlinx.coroutines.flow.flatMapLatest
-import kotlinx.coroutines.flow.launchIn
+import kotlinx.coroutines.flow.flowOn
 import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.merge
-import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.flow.receiveAsFlow
+import kotlinx.coroutines.flow.stateIn
 import kotlinx.coroutines.flow.update
 import kotlinx.coroutines.launch
 import kotlinx.datetime.DateTimeUnit
 import kotlinx.datetime.TimeZone
 import kotlinx.datetime.minus
 import logcat.LogPriority
-import mihon.core.viewmodel.StateViewModel
 import tachiyomi.core.common.preference.TriState
 import tachiyomi.core.common.util.lang.launchIO
 import tachiyomi.core.common.util.lang.launchNonCancellable
@@ -53,6 +57,7 @@ import tachiyomi.domain.updates.service.UpdatesPreferences
 import uy.kohesive.injekt.Injekt
 import uy.kohesive.injekt.api.get
 import kotlin.time.Clock
+import kotlin.time.Duration.Companion.seconds
 
 class UpdatesViewModel(
     private val sourceManager: SourceManager = Injekt.get(),
@@ -66,7 +71,7 @@ class UpdatesViewModel(
     private val libraryPreferences: LibraryPreferences = Injekt.get(),
     private val updatesPreferences: UpdatesPreferences = Injekt.get(),
     val snackbarHostState: SnackbarHostState = SnackbarHostState(),
-) : StateViewModel<UpdatesViewModel.State>(State()) {
+) : ViewModel() {
 
     private val _events: Channel<Event> = Channel(Int.MAX_VALUE)
     val events: Flow<Event> = _events.receiveAsFlow()
@@ -75,74 +80,105 @@ class UpdatesViewModel(
 
     // First and last selected index in list
     private val selectedPositions: Array<Int> = arrayOf(-1, -1)
-    private val selectedChapterIds: HashSet<Long> = HashSet()
+    private val selectedChapterIds = MutableStateFlow(emptySet<Long>())
+
+    private val dialog = MutableStateFlow<Dialog?>(null)
+
+    private val downloadStates = MutableStateFlow(emptyMap<Long, DownloadProgress>())
 
     init {
-        viewModelScope.launchIO {
-            // Set date limit for recent chapters
-            val limit = Clock.System.now().minus(3, DateTimeUnit.MONTH, TimeZone.currentSystemDefault())
-
-            combine(
-                // needed for SQL filters (unread, started, bookmarked, etc)
-                getUpdatesItemPreferenceFlow()
-                    .distinctUntilChanged()
-                    .flatMapLatest {
-                        getUpdates.subscribe(
-                            limit,
-                            unread = it.filterUnread.toBooleanOrNull(),
-                            started = it.filterStarted.toBooleanOrNull(),
-                            bookmarked = it.filterBookmarked.toBooleanOrNull(),
-                            hideExcludedScanlators = it.filterExcludedScanlators,
-                            includedCategories = it.filterIncludedCategories,
-                            excludedCategories = it.filterExcludedCategories,
-                        ).distinctUntilChanged()
-                    },
-                downloadCache.changes,
-                downloadManager.queueState,
-                // needed for Kotlin filters (downloaded)
-                getUpdatesItemPreferenceFlow().distinctUntilChanged { old, new ->
-                    old.filterDownloaded == new.filterDownloaded
-                },
-            ) { updates, _, _, itemPreferences ->
-                updates
-                    .toUpdateItems()
-                    .applyFilters(itemPreferences)
-            }
-                .collectLatest { updateItems ->
-                    mutableState.update {
-                        it.copy(
-                            isLoading = false,
-                            items = updateItems,
-                        )
-                    }
-                }
-        }
-
         viewModelScope.launchIO {
             merge(downloadManager.statusFlow(), downloadManager.progressFlow())
                 .catch { logcat(LogPriority.ERROR, it) }
                 .collect(this@UpdatesViewModel::updateDownloadState)
         }
-
-        getUpdatesItemPreferenceFlow()
-            .map { prefs ->
-                listOf(
-                    prefs.filterUnread,
-                    prefs.filterDownloaded,
-                    prefs.filterStarted,
-                    prefs.filterBookmarked,
-                )
-                    .any { it != TriState.DISABLED }
-            }
-            .distinctUntilChanged()
-            .onEach {
-                mutableState.update { state ->
-                    state.copy(hasActiveFilters = it)
-                }
-            }
-            .launchIn(viewModelScope)
     }
 
+    private fun updateDownloadState(download: Download) {
+        val chapterId = download.chapter.id
+        downloadStates.update {
+            // Terminal states are derived by the queried item itself, so drop the override instead
+            // of letting it outlive reality, e.g. showing a since deleted chapter as downloaded.
+            if (download.status == Download.State.NOT_DOWNLOADED || download.status == Download.State.DOWNLOADED) {
+                it - chapterId
+            } else {
+                it + (chapterId to DownloadProgress(download.status, download.progress))
+            }
+        }
+    }
+
+    private val hasActiveFilters = getUpdatesItemPreferenceFlow()
+        .map { prefs ->
+            listOf(
+                prefs.filterUnread,
+                prefs.filterDownloaded,
+                prefs.filterStarted,
+                prefs.filterBookmarked,
+            )
+                .any { it != TriState.DISABLED }
+        }
+        .distinctUntilChanged()
+
+    private val updateItems = combine(
+        // needed for SQL filters (unread, started, bookmarked, etc)
+        getUpdatesItemPreferenceFlow()
+            .distinctUntilChanged()
+            .flatMapLatest {
+                getUpdates.subscribe(
+                    Clock.System.now().minus(3, DateTimeUnit.MONTH, TimeZone.currentSystemDefault()),
+                    unread = it.filterUnread.toBooleanOrNull(),
+                    started = it.filterStarted.toBooleanOrNull(),
+                    bookmarked = it.filterBookmarked.toBooleanOrNull(),
+                    hideExcludedScanlators = it.filterExcludedScanlators,
+                    includedCategories = it.filterIncludedCategories,
+                    excludedCategories = it.filterExcludedCategories,
+                ).distinctUntilChanged()
+            },
+        downloadCache.changes,
+        downloadManager.queueState,
+        // needed for Kotlin filters (downloaded)
+        getUpdatesItemPreferenceFlow().distinctUntilChanged { old, new ->
+            old.filterDownloaded == new.filterDownloaded
+        },
+    ) { updates, _, _, itemPreferences ->
+        updates
+            .toUpdateItems()
+            .applyFilters(itemPreferences)
+    }
+        .flowOn(Dispatchers.IO)
+        .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5.seconds), null)
+
+    val state: StateFlow<State> = combine(
+        updateItems,
+        selectedChapterIds,
+        downloadStates,
+        dialog,
+        hasActiveFilters,
+    ) { items, selectedIds, downloads, dialog, hasActiveFilters ->
+        State(
+            isLoading = items == null,
+            hasActiveFilters = hasActiveFilters,
+            items = items.orEmpty().map { item ->
+                val download = downloads[item.update.chapterId]
+                item.copy(
+                    selected = item.update.chapterId in selectedIds,
+                    downloadStateProvider = if (download != null) {
+                        { download.status }
+                    } else {
+                        item.downloadStateProvider
+                    },
+                    downloadProgressProvider = if (download != null) {
+                        { download.progress }
+                    } else {
+                        item.downloadProgressProvider
+                    },
+                )
+            },
+            dialog = dialog,
+        )
+    }
+        .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5.seconds), State())
+
     private fun List<UpdatesItem>.applyFilters(
         preferences: ItemPreferences,
     ): List<UpdatesItem> {
@@ -179,7 +215,6 @@ class UpdatesViewModel(
                     update = update,
                     downloadStateProvider = { downloadState },
                     downloadProgressProvider = { activeDownload?.progress ?: 0 },
-                    selected = update.chapterId in selectedChapterIds,
                 )
             }
     }
@@ -192,27 +227,6 @@ class UpdatesViewModel(
         return started
     }
 
-    /**
-     * Update status of chapters.
-     *
-     * @param download download object containing progress.
-     */
-    private fun updateDownloadState(download: Download) {
-        mutableState.update { state ->
-            val newItems = state.items.toMutableList().also { list ->
-                val modifiedIndex = list.indexOfFirst { it.update.chapterId == download.chapter.id }
-                if (modifiedIndex < 0) return@also
-
-                val item = list[modifiedIndex]
-                list[modifiedIndex] = item.copy(
-                    downloadStateProvider = { download.status },
-                    downloadProgressProvider = { download.progress },
-                )
-            }
-            state.copy(items = newItems)
-        }
-    }
-
     fun downloadChapters(items: List<UpdatesItem>, action: ChapterDownloadAction) {
         if (items.isEmpty()) return
         viewModelScope.launch {
@@ -327,91 +341,80 @@ class UpdatesViewModel(
         selected: Boolean,
         fromLongPress: Boolean = false,
     ) {
-        mutableState.update { state ->
-            val newItems = state.items.toMutableList().apply {
-                val selectedIndex = indexOfFirst { it.update.chapterId == item.update.chapterId }
-                if (selectedIndex < 0) return@apply
+        val items = state.value.items
+        val selectedIndex = items.indexOfFirst { it.update.chapterId == item.update.chapterId }
+        if (selectedIndex < 0) return
 
-                val selectedItem = get(selectedIndex)
-                if (selectedItem.selected == selected) return@apply
+        // Read selection from its own flow, not the derived items, which lag behind it.
+        val currentSelection = selectedChapterIds.value
+        if ((item.update.chapterId in currentSelection) == selected) return
 
-                val firstSelection = none { it.selected }
-                set(selectedIndex, selectedItem.copy(selected = selected))
-                selectedChapterIds.addOrRemove(item.update.chapterId, selected)
+        // Off the visible items, not the id set, which can retain ids filtered out of the list
+        val firstSelection = items.none { it.selected }
+        val newSelection = currentSelection.toHashSet()
+        newSelection.addOrRemove(item.update.chapterId, selected)
 
-                if (selected && fromLongPress) {
-                    if (firstSelection) {
-                        selectedPositions[0] = selectedIndex
-                        selectedPositions[1] = selectedIndex
-                    } else {
-                        // Try to select the items in-between when possible
-                        val range: IntRange
-                        if (selectedIndex < selectedPositions[0]) {
-                            range = selectedIndex + 1..<selectedPositions[0]
-                            selectedPositions[0] = selectedIndex
-                        } else if (selectedIndex > selectedPositions[1]) {
-                            range = (selectedPositions[1] + 1)..<selectedIndex
-                            selectedPositions[1] = selectedIndex
-                        } else {
-                            // Just select itself
-                            range = IntRange.EMPTY
-                        }
+        if (selected && fromLongPress) {
+            if (firstSelection) {
+                selectedPositions[0] = selectedIndex
+                selectedPositions[1] = selectedIndex
+            } else {
+                // Try to select the items in-between when possible
+                val range: IntRange
+                if (selectedIndex < selectedPositions[0]) {
+                    range = selectedIndex + 1..<selectedPositions[0]
+                    selectedPositions[0] = selectedIndex
+                } else if (selectedIndex > selectedPositions[1]) {
+                    range = (selectedPositions[1] + 1)..<selectedIndex
+                    selectedPositions[1] = selectedIndex
+                } else {
+                    // Just select itself
+                    range = IntRange.EMPTY
+                }
 
-                        range.forEach {
-                            val inbetweenItem = get(it)
-                            if (!inbetweenItem.selected) {
-                                selectedChapterIds.add(inbetweenItem.update.chapterId)
-                                set(it, inbetweenItem.copy(selected = true))
-                            }
-                        }
-                    }
-                } else if (!fromLongPress) {
-                    if (!selected) {
-                        if (selectedIndex == selectedPositions[0]) {
-                            selectedPositions[0] = indexOfFirst { it.selected }
-                        } else if (selectedIndex == selectedPositions[1]) {
-                            selectedPositions[1] = indexOfLast { it.selected }
-                        }
-                    } else {
-                        if (selectedIndex < selectedPositions[0]) {
-                            selectedPositions[0] = selectedIndex
-                        } else if (selectedIndex > selectedPositions[1]) {
-                            selectedPositions[1] = selectedIndex
-                        }
-                    }
+                range.forEach { newSelection.add(items[it].update.chapterId) }
+            }
+        } else if (!fromLongPress) {
+            if (!selected) {
+                if (selectedIndex == selectedPositions[0]) {
+                    selectedPositions[0] = items.indexOfFirst { it.update.chapterId in newSelection }
+                } else if (selectedIndex == selectedPositions[1]) {
+                    selectedPositions[1] = items.indexOfLast { it.update.chapterId in newSelection }
+                }
+            } else {
+                if (selectedIndex < selectedPositions[0]) {
+                    selectedPositions[0] = selectedIndex
+                } else if (selectedIndex > selectedPositions[1]) {
+                    selectedPositions[1] = selectedIndex
                 }
             }
-            state.copy(items = newItems)
         }
+
+        selectedChapterIds.update { newSelection }
     }
 
     fun toggleAllSelection(selected: Boolean) {
-        mutableState.update { state ->
-            val newItems = state.items.map {
-                selectedChapterIds.addOrRemove(it.update.chapterId, selected)
-                it.copy(selected = selected)
-            }
-            state.copy(items = newItems)
-        }
+        val ids = if (selected) state.value.items.map { it.update.chapterId }.toSet() else emptySet()
+        selectedChapterIds.update { ids }
 
         selectedPositions[0] = -1
         selectedPositions[1] = -1
     }
 
     fun invertSelection() {
-        mutableState.update { state ->
-            val newItems = state.items.map {
-                selectedChapterIds.addOrRemove(it.update.chapterId, !it.selected)
-                it.copy(selected = !it.selected)
-            }
-            state.copy(items = newItems)
-        }
+        val current = selectedChapterIds.value
+        val ids = state.value.items
+            .map { it.update.chapterId }
+            .filterNot { it in current }
+            .toSet()
+        selectedChapterIds.update { ids }
+
         selectedPositions[0] = -1
         selectedPositions[1] = -1
     }
 
     fun setDialog(dialog: Dialog?) {
-        mutableState.update { it.copy(dialog = dialog) }
+        this.dialog.update { dialog }
     }
 
     fun resetNewUpdatesCount() {
@@ -442,7 +445,7 @@ class UpdatesViewModel(
     }
 
     fun showFilterDialog() {
-        mutableState.update { it.copy(dialog = Dialog.FilterSheet) }
+        dialog.update { Dialog.FilterSheet }
     }
 
     @Immutable
@@ -456,6 +459,8 @@ class UpdatesViewModel(
         val filterExcludedCategories: List<Long>,
     )
 
+    private data class DownloadProgress(val status: Download.State, val progress: Int)
+
     @Immutable
     data class State(
         val isLoading: Boolean = true,