From 5a98f3d2e72c49b5d16a0b4bde049c42f1ee69ac Mon Sep 17 00:00:00 2001 From: Barnabas Balogh Date: Sat, 20 Jun 2026 09:27:25 +0000 Subject: [PATCH] refactor(core): separate data loading from repository reads Move implicit loading side effects out of getX methods into explicit loadX methods, making data flow predictable. Remove seriesTitle StateFlow in favor of direct seriesName access. Add proper error handling to repository load operations. --- .../ui/screen/episode/TvEpisodeScreen.kt | 3 +- .../data/CompositeLocalMediaRepository.kt | 21 +++++++ .../purefin/core/data/LocalMediaRepository.kt | 3 + .../content/episode/EpisodeScreenViewModel.kt | 15 +---- .../content/movie/MovieScreenViewModel.kt | 3 + .../feature/content/series/SeriesViewModel.kt | 4 +- .../core/player/viewmodel/PlayerViewModel.kt | 6 +- .../catalog/InMemoryLocalMediaRepository.kt | 61 +++++++++++-------- .../catalog/OfflineLocalMediaRepository.kt | 12 ++++ 9 files changed, 86 insertions(+), 42 deletions(-) diff --git a/app-tv/src/main/java/hu/bbara/purefin/ui/screen/episode/TvEpisodeScreen.kt b/app-tv/src/main/java/hu/bbara/purefin/ui/screen/episode/TvEpisodeScreen.kt index a69c8ada..09867520 100644 --- a/app-tv/src/main/java/hu/bbara/purefin/ui/screen/episode/TvEpisodeScreen.kt +++ b/app-tv/src/main/java/hu/bbara/purefin/ui/screen/episode/TvEpisodeScreen.kt @@ -41,7 +41,6 @@ fun TvEpisodeScreen( } val episode = viewModel.episode.collectAsStateWithLifecycle() - val seriesTitle = viewModel.seriesTitle.collectAsStateWithLifecycle() val selectedEpisode = episode.value if (selectedEpisode == null) { @@ -51,7 +50,7 @@ fun TvEpisodeScreen( TvEpisodeScreenContent( episode = selectedEpisode, - seriesTitle = seriesTitle.value, + seriesTitle = selectedEpisode.seriesName, onPlay = remember(selectedEpisode.id, navigationManager) { { navigationManager.navigate( diff --git a/core/src/main/java/hu/bbara/purefin/core/data/CompositeLocalMediaRepository.kt b/core/src/main/java/hu/bbara/purefin/core/data/CompositeLocalMediaRepository.kt index 1fe7eb17..b30dd901 100644 --- a/core/src/main/java/hu/bbara/purefin/core/data/CompositeLocalMediaRepository.kt +++ b/core/src/main/java/hu/bbara/purefin/core/data/CompositeLocalMediaRepository.kt @@ -67,6 +67,27 @@ class CompositeLocalMediaRepository @Inject constructor( ) } + override suspend fun loadMovie(id: UUID) { + runOnlineOrOfflineNoOp( + onlineAction = { onlineRepository.loadMovie(id) }, + offlineAction = { offlineRepository.loadMovie(id) }, + ) + } + + override suspend fun loadSeries(id: UUID) { + runOnlineOrOfflineNoOp( + onlineAction = { onlineRepository.loadSeries(id) }, + offlineAction = { offlineRepository.loadSeries(id) }, + ) + } + + override suspend fun loadEpisode(id: UUID) { + runOnlineOrOfflineNoOp( + onlineAction = { onlineRepository.loadEpisode(id) }, + offlineAction = { offlineRepository.loadEpisode(id) }, + ) + } + override suspend fun loadSeasons(seriesId: UUID) { runOnlineOrOfflineNoOp( onlineAction = { onlineRepository.loadSeasons(seriesId) }, diff --git a/core/src/main/java/hu/bbara/purefin/core/data/LocalMediaRepository.kt b/core/src/main/java/hu/bbara/purefin/core/data/LocalMediaRepository.kt index d2e39997..87cc6797 100644 --- a/core/src/main/java/hu/bbara/purefin/core/data/LocalMediaRepository.kt +++ b/core/src/main/java/hu/bbara/purefin/core/data/LocalMediaRepository.kt @@ -14,6 +14,9 @@ interface LocalMediaRepository : MediaMetadataUpdater { suspend fun getMovie(id: UUID): Flow suspend fun getSeries(id: UUID): Flow suspend fun getEpisode(id: UUID): Flow + suspend fun loadMovie(id: UUID) + suspend fun loadSeries(id: UUID) + suspend fun loadEpisode(id: UUID) suspend fun loadSeasons(seriesId: UUID) suspend fun loadSeasonEpisodes(seriesId: UUID, seasonId: UUID) } diff --git a/core/src/main/java/hu/bbara/purefin/core/feature/content/episode/EpisodeScreenViewModel.kt b/core/src/main/java/hu/bbara/purefin/core/feature/content/episode/EpisodeScreenViewModel.kt index 1cada4ca..20a04309 100644 --- a/core/src/main/java/hu/bbara/purefin/core/feature/content/episode/EpisodeScreenViewModel.kt +++ b/core/src/main/java/hu/bbara/purefin/core/feature/content/episode/EpisodeScreenViewModel.kt @@ -20,7 +20,6 @@ import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.flowOf -import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch import javax.inject.Inject @@ -47,17 +46,6 @@ class EpisodeScreenViewModel @Inject constructor( } .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), null) - @OptIn(ExperimentalCoroutinesApi::class) - val seriesTitle: StateFlow = _episode - .flatMapLatest { episode -> - if (episode == null) { - flowOf(null) - } else { - mediaCatalogReader(episode.offline).getSeries(episode.seriesId).map { it?.name } - } - } - .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), null) - private val _downloadState = MutableStateFlow(DownloadState.NotDownloaded) val downloadState: StateFlow = _downloadState.asStateFlow() @@ -76,6 +64,9 @@ class EpisodeScreenViewModel @Inject constructor( fun selectEpisode(episode: EpisodeDto) { _episode.value = episode + viewModelScope.launch { + mediaCatalogReader(episode.offline).loadEpisode(episode.id) + } viewModelScope.launch { mediaDownloadManager.observeDownloadState(episode.id.toString()).collect { _downloadState.value = it diff --git a/core/src/main/java/hu/bbara/purefin/core/feature/content/movie/MovieScreenViewModel.kt b/core/src/main/java/hu/bbara/purefin/core/feature/content/movie/MovieScreenViewModel.kt index ad5f9b3b..f7a2c688 100644 --- a/core/src/main/java/hu/bbara/purefin/core/feature/content/movie/MovieScreenViewModel.kt +++ b/core/src/main/java/hu/bbara/purefin/core/feature/content/movie/MovieScreenViewModel.kt @@ -63,6 +63,9 @@ class MovieScreenViewModel @Inject constructor( fun selectMovie(movie: MovieDto) { _movie.value = movie + viewModelScope.launch { + mediaCatalogReader(movie.offline).loadMovie(movie.id) + } viewModelScope.launch { mediaDownloadManager.observeDownloadState(movie.id.toString()).collect { _downloadState.value = it diff --git a/core/src/main/java/hu/bbara/purefin/core/feature/content/series/SeriesViewModel.kt b/core/src/main/java/hu/bbara/purefin/core/feature/content/series/SeriesViewModel.kt index 8d38059e..ee0c05fe 100644 --- a/core/src/main/java/hu/bbara/purefin/core/feature/content/series/SeriesViewModel.kt +++ b/core/src/main/java/hu/bbara/purefin/core/feature/content/series/SeriesViewModel.kt @@ -195,7 +195,9 @@ class SeriesViewModel @Inject constructor( fun selectSeries(series: SeriesDto) { _series.value = series viewModelScope.launch { - mediaCatalogReader(series.offline).loadSeasons(series.id) + val mediaCatalogReader = mediaCatalogReader(series.offline) + mediaCatalogReader.loadSeries(series.id) + mediaCatalogReader.loadSeasons(series.id) } } diff --git a/core/src/main/java/hu/bbara/purefin/core/player/viewmodel/PlayerViewModel.kt b/core/src/main/java/hu/bbara/purefin/core/player/viewmodel/PlayerViewModel.kt index 618b990c..d040fadc 100644 --- a/core/src/main/java/hu/bbara/purefin/core/player/viewmodel/PlayerViewModel.kt +++ b/core/src/main/java/hu/bbara/purefin/core/player/viewmodel/PlayerViewModel.kt @@ -169,8 +169,7 @@ class PlayerViewModel @Inject constructor( _uiState.update { it.copy(error = dataErrorMessage) } return } - //TODO hack to preload the series media - mediaCatalogReader.getEpisode(UUID.fromString(id)) + mediaCatalogReader.loadEpisode(UUID.fromString(id)) viewModelScope.launch { runCatching { playerManager.play(uuid) @@ -243,6 +242,7 @@ class PlayerViewModel @Inject constructor( private suspend fun PlayableMedia.toPlaylistElementUiModel(currentMediaId: UUID?): PlaylistElementUiModel? { return when (this) { is PlayableMedia.Movie -> { + mediaCatalogReader.loadMovie(id) val movie = mediaCatalogReader.getMovie(id).first() if (movie == null) { Timber.tag(TAG).e("Movie not found for playlist item: $id") @@ -261,6 +261,7 @@ class PlayerViewModel @Inject constructor( } is PlayableMedia.Series -> { + mediaCatalogReader.loadSeries(id) val series = mediaCatalogReader.getSeries(id).first() if (series == null) { Timber.tag(TAG).e("Series not found for playlist item: $id") @@ -279,6 +280,7 @@ class PlayerViewModel @Inject constructor( } is PlayableMedia.Episode -> { + mediaCatalogReader.loadEpisode(id) val episode = mediaCatalogReader.getEpisode(id).first() if (episode == null) { Timber.tag(TAG).e("Episode not found for playlist item: $id") diff --git a/data/src/main/java/hu/bbara/purefin/data/catalog/InMemoryLocalMediaRepository.kt b/data/src/main/java/hu/bbara/purefin/data/catalog/InMemoryLocalMediaRepository.kt index 1f538ecb..d7d66d0d 100644 --- a/data/src/main/java/hu/bbara/purefin/data/catalog/InMemoryLocalMediaRepository.kt +++ b/data/src/main/java/hu/bbara/purefin/data/catalog/InMemoryLocalMediaRepository.kt @@ -20,7 +20,6 @@ import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.first -import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.update import org.jellyfin.sdk.model.api.BaseItemKind @@ -48,48 +47,70 @@ class InMemoryLocalMediaRepository @Inject constructor( private val episodesState = MutableStateFlow>(emptyMap()) override val episodes: StateFlow> = episodesState.asStateFlow() - override suspend fun getMovie(id: UUID): Flow { - if (!moviesState.value.containsKey(id)) { + override suspend fun getMovie(id: UUID): Flow = + moviesState.map { it[id] }.distinctUntilChanged() + + override suspend fun getSeries(id: UUID): Flow = + seriesState.map { it[id] }.distinctUntilChanged() + + override suspend fun getEpisode(id: UUID): Flow = + episodesState.map { it[id] }.distinctUntilChanged() + + override suspend fun loadMovie(id: UUID) { + if (moviesState.value.containsKey(id)) return + try { jellyfinApiClient.getItemInfo(id)?.let { item -> if (item.type != BaseItemKind.MOVIE) { Timber.tag(TAG).d("Item is not an movie: ${item.type}") - return flowOf(null) + return } val movie = item.toMovie(serverUrl.first()) moviesState.update { current -> current + (movie.id to movie) } } + } catch (error: CancellationException) { + throw error + } catch (error: Exception) { + Timber.tag(TAG).e(error, "Failed to load movie $id") + throw error } - return moviesState.map { it[id] }.distinctUntilChanged() } - override suspend fun getSeries(id: UUID): Flow { - if (!seriesState.value.containsKey(id)) { + override suspend fun loadSeries(id: UUID) { + if (seriesState.value.containsKey(id)) return + try { jellyfinApiClient.getItemInfo(id)?.let { item -> if (item.type != BaseItemKind.SERIES) { Timber.tag(TAG).d("Item is not an series: ${item.type}") - return flowOf(null) + return } val series = item.toSeries(serverUrl.first()) seriesState.update { current -> current + (series.id to series) } } + } catch (error: CancellationException) { + throw error + } catch (error: Exception) { + Timber.tag(TAG).e(error, "Failed to load series $id") + throw error } - return seriesState.map { it[id] }.distinctUntilChanged() } - override suspend fun getEpisode(id: UUID): Flow { - if (!episodesState.value.containsKey(id)) { + override suspend fun loadEpisode(id: UUID) { + if (episodesState.value.containsKey(id)) return + try { jellyfinApiClient.getItemInfo(id)?.let { item -> if (item.type != BaseItemKind.EPISODE) { Timber.tag(TAG).d("Item is not an episode: ${item.type}") - return flowOf(null) + return } val episode = item.toEpisode(serverUrl.first()) episodesState.update { current -> current + (episode.id to episode) } } + } catch (error: CancellationException) { + throw error + } catch (error: Exception) { + Timber.tag(TAG).e(error, "Failed to load episode $id") + throw error } - val episode = episodesState.value[id] ?: return flowOf(null) - preloadSeasonsForEpisode(episode.seriesId) - return episodesState.map { it[id] }.distinctUntilChanged() } fun upsertMovies(movies: List) { @@ -115,16 +136,6 @@ class InMemoryLocalMediaRepository @Inject constructor( } } - private suspend fun preloadSeasonsForEpisode(seriesId: UUID) { - try { - loadSeasonsInternal(seriesId) - } catch (error: CancellationException) { - throw error - } catch (error: Exception) { - Timber.tag(TAG).w(error, "Unable to preload seasons for episode series $seriesId") - } - } - private suspend fun loadSeasonsInternal(seriesId: UUID) { seriesState.value[seriesId]?.takeIf { it.seasons.isNotEmpty() }?.let { return } diff --git a/data/src/main/java/hu/bbara/purefin/data/catalog/OfflineLocalMediaRepository.kt b/data/src/main/java/hu/bbara/purefin/data/catalog/OfflineLocalMediaRepository.kt index 88081b6c..0f462664 100644 --- a/data/src/main/java/hu/bbara/purefin/data/catalog/OfflineLocalMediaRepository.kt +++ b/data/src/main/java/hu/bbara/purefin/data/catalog/OfflineLocalMediaRepository.kt @@ -46,6 +46,18 @@ class OfflineLocalMediaRepository @Inject constructor( return episodes.map { it[id] } } + override suspend fun loadMovie(id: UUID) { + // Offline movie content is already persisted. + } + + override suspend fun loadSeries(id: UUID) { + // Offline series content is already persisted. + } + + override suspend fun loadEpisode(id: UUID) { + // Offline episode content is already persisted. + } + override suspend fun loadSeasons(seriesId: UUID) { // Offline series content is already emitted with its saved seasons. }