From 32a56e83875fa9474ad9d57da0248ee144c887c5 Mon Sep 17 00:00:00 2001 From: Damontecres Date: Thu, 17 Sep 2026 14:41:36 -0400 Subject: [PATCH] Wait for switched user's home settings before fetching data (#1981) ## Description There is a race condition when switching users between `HomeViewModel.init` and `HomeSetttingsService.loadCurrentSettings`. This happens more often if the new user doesn't have a customized home page. The result is that the previous user's home page settings are used for the initial load of the home page. This PR fixes this by collecting the flows for current user & home settings, waiting until both are valid and refer to the same user, then fetch the data. ### Related issues Fixes #1976 Maybe related (and fixes) #1980, but need to investigate more ### Testing Emulator. It's easy to reproduce by adding a manual `delay(2.seconds)` at the start of `HomeSetttingsService.loadCurrentSettings`. ## Screenshots N/A ## AI or LLM usage None --- .../wholphin/services/HomeSettingsService.kt | 16 +- .../services/SetupNavigationManager.kt | 3 - .../wholphin/ui/main/HomeViewModel.kt | 323 +++++++++++------- .../ui/main/settings/HomeSettingsViewModel.kt | 8 +- .../wholphin/ui/setup/SwitchUserViewModel.kt | 1 + 5 files changed, 208 insertions(+), 143 deletions(-) diff --git a/app/src/main/java/com/github/damontecres/wholphin/services/HomeSettingsService.kt b/app/src/main/java/com/github/damontecres/wholphin/services/HomeSettingsService.kt index e7b5db00..3371f45e 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/services/HomeSettingsService.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/services/HomeSettingsService.kt @@ -245,7 +245,7 @@ class HomeSettingsService settings.rows.mapIndexed { index, config -> resolve(index, config) } - HomePageResolvedSettings(resolvedRows) + HomePageResolvedSettings(userId, resolvedRows) } else { createDefault(userId) } @@ -256,12 +256,15 @@ class HomeSettingsService /** * Resolve the settings and set them to be the current settings */ - suspend fun updateCurrent(settings: HomePageSettings) { + suspend fun updateCurrent( + userId: UUID, + settings: HomePageSettings, + ) { val resolvedRows = settings.rows.mapIndexed { index, config -> resolve(index, config) } - val resolvedSettings = HomePageResolvedSettings(resolvedRows) + val resolvedSettings = HomePageResolvedSettings(userId, resolvedRows) currentSettings.update { resolvedSettings } } @@ -312,7 +315,7 @@ class HomeSettingsService ), ) val rowConfig = continueWatchingRow + includedIds - return HomePageResolvedSettings(rowConfig) + return HomePageResolvedSettings(userId, rowConfig) } /** @@ -428,7 +431,7 @@ class HomeSettingsService null } }.flatten() - HomePageResolvedSettings(rowConfigs) + HomePageResolvedSettings(userId, rowConfigs) } else { null } @@ -1224,10 +1227,11 @@ data class HomeRowConfigDisplay( * @see HomePageSettings */ data class HomePageResolvedSettings( + val userId: UUID, val rows: List, ) { companion object { - val EMPTY = HomePageResolvedSettings(listOf()) + val EMPTY = HomePageResolvedSettings(UUID.randomUUID(), emptyList()) } } diff --git a/app/src/main/java/com/github/damontecres/wholphin/services/SetupNavigationManager.kt b/app/src/main/java/com/github/damontecres/wholphin/services/SetupNavigationManager.kt index fa42469e..abe39959 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/services/SetupNavigationManager.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/services/SetupNavigationManager.kt @@ -27,9 +27,6 @@ class SetupNavigationManager fun navigateTo(destination: SetupDestination) { backStack[0] = destination log() - if (destination !is SetupDestination.AppContent) { - navigationManager.reloadHome() - } } private fun log() { diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/main/HomeViewModel.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/main/HomeViewModel.kt index bceccc5e..9e405196 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/main/HomeViewModel.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/main/HomeViewModel.kt @@ -20,6 +20,8 @@ import com.github.damontecres.wholphin.services.ServerReportService import com.github.damontecres.wholphin.services.UserPreferencesService import com.github.damontecres.wholphin.services.deleteItem import com.github.damontecres.wholphin.services.tvAccess +import com.github.damontecres.wholphin.ui.collectLatestIn +import com.github.damontecres.wholphin.ui.combinePair import com.github.damontecres.wholphin.ui.data.RowColumn import com.github.damontecres.wholphin.ui.launchDefault import com.github.damontecres.wholphin.ui.launchIO @@ -31,6 +33,8 @@ import com.github.damontecres.wholphin.util.LoadingState import com.github.damontecres.wholphin.util.WholphinDispatchers import dagger.hilt.android.lifecycle.HiltViewModel import dagger.hilt.android.qualifiers.ApplicationContext +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.Job import kotlinx.coroutines.async import kotlinx.coroutines.awaitAll import kotlinx.coroutines.flow.MutableStateFlow @@ -44,6 +48,7 @@ import kotlinx.coroutines.sync.withPermit import kotlinx.coroutines.withContext import org.jellyfin.sdk.api.client.exception.InvalidStatusException import org.jellyfin.sdk.model.api.BaseItemKind +import org.jellyfin.sdk.model.api.UserDto import timber.log.Timber import java.util.UUID import javax.inject.Inject @@ -68,150 +73,204 @@ class HomeViewModel private val _state = MutableStateFlow(HomeState.EMPTY) val state: StateFlow = _state + private var dataLoadingJob: Job? = null + init { datePlayedService.invalidateAll() -// init() + serverRepository.currentUserDtoFlow + .combinePair(homeSettingsService.currentSettings) + .collectLatestIn(viewModelScope) { (userDto, settings) -> + Timber.v( + "Got new userDto & settings, userId=%s, settings?=%s", + userDto?.id, + settings != HomePageResolvedSettings.EMPTY, + ) + _state.update { HomeState.EMPTY } + dataLoadingJob?.cancel() + if (userDto == null) { + Timber.d("UserDto is null") + return@collectLatestIn + } + if (settings == HomePageResolvedSettings.EMPTY) { + Timber.d("Home settings are empty") + return@collectLatestIn + } + if (userDto.id != settings.userId) { + Timber.d("User IDs don't match: %s vs %s", userDto?.id, settings.userId) + return@collectLatestIn + } + dataLoadingJob = + viewModelScope.launchIO { + loadHomeRows(userDto, settings) + } + } } fun init() { viewModelScope.launchIO { Timber.d("init HomeViewModel") - try { - val preferences = userPreferencesService.getCurrent() - val prefs = preferences.appPreferences.homePagePreferences - - serverRepository.currentUserDto?.let { userDto -> - val libraries = - navDrawerService.getAllUserLibraries(userDto.id, userDto.tvAccess) + serverRepository.currentUserDto?.let { userDto -> + if (dataLoadingJob?.isActive == false) { val settings = homeSettingsService.currentSettings.first { it != HomePageResolvedSettings.EMPTY } - val state = state.value - - // Refreshing if a load has already occurred and the rows haven't significantly changed - val refresh = - state.loadingState == LoadingState.Success && state.settings == settings - Timber.v( - "refresh=%s, state.loadingState=%s, %s rows", - refresh, - state.loadingState, - settings.rows.size, - ) - _state.update { - it.copy( - loadingState = if (refresh) LoadingState.Success else LoadingState.Loading, - refreshState = LoadingState.Loading, - settings = settings, - homeRows = - if (refresh) { - it.homeRows - } else { - List(settings.rows.size) { - HomeRowLoadingState.Pending(EmptyStringProvider) - } - }, + if (userDto.id == settings.userId) { + dataLoadingJob = + viewModelScope.launchIO { + loadHomeRows(userDto, settings) + } + } else { + Timber.d( + "Init user IDs don't match: %s vs %s", + userDto.id, + settings.userId, ) } - - val semaphore = Semaphore(4) - - val deferred = - settings.rows - .map { row -> - viewModelScope.async(WholphinDispatchers.IO) { - semaphore.withPermit { - Timber.v("Fetching row: %s", row) - try { - homeSettingsService.fetchDataForRow( - row = row.config, - scope = viewModelScope, - prefs = prefs, - userDto = userDto, - libraries = libraries, - limit = prefs.maxItemsPerRow, - isRefresh = refresh, - ) - } catch (ex: InvalidStatusException) { - if (ex.status == 404) { - Timber.w(ex, "404 on row %s", row) - HomeRowLoadingState.Success( - row.title, - emptyList(), - ) - } else { - Timber.e( - ex, - "Error %s on row %s", - ex.status, - row, - ) - HomeRowLoadingState.Error( - row.title, - exception = ex, - ) - } - } catch (ex: Exception) { - Timber.e(ex, "Error on row %s", row) - HomeRowLoadingState.Error( - row.title, - exception = ex, - ) - } - } - } - } - - if (refresh) { - // Replace rows as they complete - val remaining = deferred.withIndex().toMutableList() - while (remaining.isNotEmpty()) { - val (rowIndex, rowData) = - select { - // "Return" the first remaining that is completed - remaining - .forEach { (rowIndex, deferred) -> - deferred.onAwait { rowIndex to it } - } - } - Timber.v("Got row data index=%s", rowIndex) - remaining.removeIf { it.index == rowIndex } - _state.update { state -> - val newRows = - state.homeRows.toMutableList().apply { - set(rowIndex, rowData) - } - state.copy( - homeRows = newRows, - ) - } - } - _state.update { - it.copy( - loadingState = LoadingState.Success, - refreshState = LoadingState.Success, - ) - } - } else { - val rows = deferred.awaitAll() - Timber.v("Got all rows") - _state.update { - it.copy( - loadingState = LoadingState.Success, - refreshState = LoadingState.Success, - homeRows = rows, - ) - } - } - Timber.d("Home page load complete") - } - } catch (ex: Exception) { - Timber.e(ex, "Exception during home page loading") - if (state.value.loadingState == LoadingState.Success) { - showToast(context, "Error refreshing home: ${ex.localizedMessage}") - _state.update { it.copy(refreshState = LoadingState.Error(ex)) } } else { - _state.update { - it.copy(loadingState = LoadingState.Error(ex)) + Timber.v("Data loading job is active") + } + } + } + } + + suspend fun loadHomeRows( + userDto: UserDto, + settings: HomePageResolvedSettings, + ) { + Timber.i("Starting loadHomeRows") + try { + val preferences = userPreferencesService.getCurrent() + val prefs = preferences.appPreferences.homePagePreferences + + val libraries = + navDrawerService.getAllUserLibraries(userDto.id, userDto.tvAccess) + + val state = state.value + + // Refreshing if a load has already occurred and the rows haven't significantly changed + val refresh = + state.loadingState == LoadingState.Success && state.settings == settings + Timber.v( + "refresh=%s, state.loadingState=%s, %s rows", + refresh, + state.loadingState, + settings.rows.size, + ) + _state.update { + it.copy( + loadingState = if (refresh) LoadingState.Success else LoadingState.Loading, + refreshState = LoadingState.Loading, + settings = settings, + homeRows = + if (refresh) { + it.homeRows + } else { + List(settings.rows.size) { + HomeRowLoadingState.Pending(EmptyStringProvider) + } + }, + ) + } + + val semaphore = Semaphore(4) + + val deferred = + settings.rows + .map { row -> + viewModelScope.async(WholphinDispatchers.IO) { + semaphore.withPermit { + Timber.v("Fetching row: %s", row) + try { + homeSettingsService.fetchDataForRow( + row = row.config, + scope = viewModelScope, + prefs = prefs, + userDto = userDto, + libraries = libraries, + limit = prefs.maxItemsPerRow, + isRefresh = refresh, + ) + } catch (ex: InvalidStatusException) { + if (ex.status == 404) { + Timber.w(ex, "404 on row %s", row) + HomeRowLoadingState.Success( + row.title, + emptyList(), + ) + } else { + Timber.e( + ex, + "Error %s on row %s", + ex.status, + row, + ) + HomeRowLoadingState.Error( + row.title, + exception = ex, + ) + } + } catch (ex: Exception) { + Timber.e(ex, "Error on row %s", row) + HomeRowLoadingState.Error( + row.title, + exception = ex, + ) + } + } + } } + + if (refresh) { + // Replace rows as they complete + val remaining = deferred.withIndex().toMutableList() + while (remaining.isNotEmpty()) { + val (rowIndex, rowData) = + select { + // "Return" the first remaining that is completed + remaining + .forEach { (rowIndex, deferred) -> + deferred.onAwait { rowIndex to it } + } + } + Timber.v("Got row data index=%s", rowIndex) + remaining.removeIf { it.index == rowIndex } + _state.update { state -> + val newRows = + state.homeRows.toMutableList().apply { + set(rowIndex, rowData) + } + state.copy( + homeRows = newRows, + ) + } + } + _state.update { + it.copy( + loadingState = LoadingState.Success, + refreshState = LoadingState.Success, + ) + } + } else { + val rows = deferred.awaitAll() + Timber.v("Got all rows") + _state.update { + it.copy( + loadingState = LoadingState.Success, + refreshState = LoadingState.Success, + homeRows = rows, + ) + } + } + Timber.d("Home page load complete") + } catch (ex: CancellationException) { + throw ex + } catch (ex: Exception) { + Timber.e(ex, "Exception during home page loading") + if (state.value.loadingState == LoadingState.Success) { + showToast(context, "Error refreshing home: ${ex.localizedMessage}") + _state.update { it.copy(refreshState = LoadingState.Error(ex)) } + } else { + _state.update { + it.copy(loadingState = LoadingState.Error(ex)) } } } @@ -298,7 +357,7 @@ data class HomeState( HomeState( LoadingState.Pending, LoadingState.Pending, - listOf(), + emptyList(), HomePageResolvedSettings.EMPTY, ) } diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/main/settings/HomeSettingsViewModel.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/main/settings/HomeSettingsViewModel.kt index 64c0308e..e3eafcbb 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/main/settings/HomeSettingsViewModel.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/main/settings/HomeSettingsViewModel.kt @@ -595,7 +595,7 @@ class HomeSettingsViewModel } if (shouldSave) { homeSettingsService.saveToLocal(user.id, settings) - homeSettingsService.updateCurrent(settings) + homeSettingsService.updateCurrent(user.id, settings) showSaveToast() } else { Timber.d("No changes") @@ -616,7 +616,11 @@ class HomeSettingsViewModel _state.update { update.invoke(it) } - homeSettingsService.currentSettings.update { HomePageResolvedSettings(state.value.rows) } + serverRepository.currentUser?.id?.let { userId -> + homeSettingsService.currentSettings.update { + HomePageResolvedSettings(userId, state.value.rows) + } + } } fun resizeCards(relative: Int) { diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/setup/SwitchUserViewModel.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/setup/SwitchUserViewModel.kt index c47b86c5..e1a6dd96 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/setup/SwitchUserViewModel.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/setup/SwitchUserViewModel.kt @@ -132,6 +132,7 @@ class SwitchUserViewModel viewModelScope.async(WholphinDispatchers.IO) { try { val current = serverRepository.changeUser(server, user) + navigationManager.reloadHome() setupNavigationManager.navigateTo(SetupDestination.AppContent(current)) SwitchUserResult.Success } catch (ex: InvalidStatusException) {