From 11fd780b311a2b32a4036a82ca9c91bb7c02bad8 Mon Sep 17 00:00:00 2001 From: Justin Caveda Date: Thu, 12 Feb 2026 17:27:59 -0600 Subject: [PATCH] Fix suggestions cache miss handling for missing files (#880) ## Description Fix a bug where movie suggestions would get stuck loading when the on-disk cache file doesn't exist. ### Related issues Fixes #876 ### Testing Ran unit tests locally. ## Screenshots N/A ## AI or LLM usage Used AI to add testing for this scenario to unit test --------- Co-authored-by: Ray <154766448+damontecres@users.noreply.github.com> --- .../wholphin/services/SuggestionService.kt | 35 ++++++++++------- .../wholphin/services/SuggestionsCache.kt | 17 +++++--- .../ui/components/RecommendedMovie.kt | 3 ++ .../ui/components/RecommendedTvShow.kt | 3 ++ .../services/SuggestionServiceTest.kt | 39 +++++++++++++++++++ 5 files changed, 76 insertions(+), 21 deletions(-) diff --git a/app/src/main/java/com/github/damontecres/wholphin/services/SuggestionService.kt b/app/src/main/java/com/github/damontecres/wholphin/services/SuggestionService.kt index db6cef2c..0fe32179 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/services/SuggestionService.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/services/SuggestionService.kt @@ -6,6 +6,7 @@ import androidx.work.WorkManager import com.github.damontecres.wholphin.data.ServerRepository import com.github.damontecres.wholphin.data.model.BaseItem import com.github.damontecres.wholphin.util.GetItemsRequestHandler +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.flatMapLatest @@ -49,21 +50,8 @@ class SuggestionService .asFlow() .flatMapLatest { user -> val userId = user?.id ?: return@flatMapLatest flowOf(SuggestionsResource.Empty) - val cachedIds = cache.get(userId, parentId, itemKind)?.ids.orEmpty() - if (cachedIds.isNotEmpty()) { - flow { - try { - emit( - SuggestionsResource.Success( - fetchItemsByIds(cachedIds, itemKind), - ), - ) - } catch (e: Exception) { - Timber.e(e, "Failed to fetch items") - emit(SuggestionsResource.Empty) - } - } - } else { + val cachedSuggestions = cache.get(userId, parentId, itemKind) + if (cachedSuggestions == null) { workManager .getWorkInfosForUniqueWorkFlow(SuggestionsWorker.WORK_NAME) .map { workInfos -> @@ -73,6 +61,23 @@ class SuggestionService } if (isActive) SuggestionsResource.Loading else SuggestionsResource.Empty } + } else if (cachedSuggestions.ids.isEmpty()) { + flowOf(SuggestionsResource.Empty) + } else { + flow { + try { + emit( + SuggestionsResource.Success( + fetchItemsByIds(cachedSuggestions.ids, itemKind), + ), + ) + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + Timber.e(e, "Failed to fetch items") + emit(SuggestionsResource.Empty) + } + } } } } diff --git a/app/src/main/java/com/github/damontecres/wholphin/services/SuggestionsCache.kt b/app/src/main/java/com/github/damontecres/wholphin/services/SuggestionsCache.kt index 5542ea51..0921a1be 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/services/SuggestionsCache.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/services/SuggestionsCache.kt @@ -60,15 +60,20 @@ class SuggestionsCache ): CachedSuggestions? { val key = cacheKey(userId, libraryId, itemKind) return memoryCache.getOrPut(key) { - try { - mutex.withLock { - File(cacheDir, "$key.json").inputStream().use { + mutex.withLock { + try { + val cacheFile = File(cacheDir, "$key.json") + if (!cacheFile.exists()) { + return@withLock null + } + + cacheFile.inputStream().use { json.decodeFromStream(it) } + } catch (ex: Exception) { + Timber.e(ex, "Exception reading from disk cache") + null } - } catch (ex: Exception) { - Timber.e(ex, "Exception reading from disk cache") - null } } } diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/components/RecommendedMovie.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/components/RecommendedMovie.kt index a1a5be80..403ff5fa 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/components/RecommendedMovie.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/components/RecommendedMovie.kt @@ -32,6 +32,7 @@ import dagger.assisted.AssistedFactory import dagger.assisted.AssistedInject import dagger.hilt.android.lifecycle.HiltViewModel import dagger.hilt.android.qualifiers.ApplicationContext +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.Deferred import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.MutableStateFlow @@ -207,6 +208,8 @@ class RecommendedMovieViewModel } update(R.string.suggestions, state) } + } catch (ex: CancellationException) { + throw ex } catch (ex: Exception) { Timber.e(ex, "Failed to fetch suggestions") update( diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/components/RecommendedTvShow.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/components/RecommendedTvShow.kt index 00430a2c..3258930c 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/components/RecommendedTvShow.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/components/RecommendedTvShow.kt @@ -33,6 +33,7 @@ import dagger.assisted.AssistedFactory import dagger.assisted.AssistedInject import dagger.hilt.android.lifecycle.HiltViewModel import dagger.hilt.android.qualifiers.ApplicationContext +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.Deferred import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.async @@ -254,6 +255,8 @@ class RecommendedTvShowViewModel } update(R.string.suggestions, state) } + } catch (ex: CancellationException) { + throw ex } catch (ex: Exception) { Timber.e(ex, "Failed to fetch suggestions") update( diff --git a/app/src/test/java/com/github/damontecres/wholphin/services/SuggestionServiceTest.kt b/app/src/test/java/com/github/damontecres/wholphin/services/SuggestionServiceTest.kt index e1663543..f8fc8445 100644 --- a/app/src/test/java/com/github/damontecres/wholphin/services/SuggestionServiceTest.kt +++ b/app/src/test/java/com/github/damontecres/wholphin/services/SuggestionServiceTest.kt @@ -10,6 +10,7 @@ import com.github.damontecres.wholphin.util.GetItemsRequestHandler import io.mockk.coEvery import io.mockk.every import io.mockk.mockk +import io.mockk.verify import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.first @@ -150,6 +151,44 @@ class SuggestionServiceTest { assertEquals(SuggestionsResource.Empty, result) } + @Test + fun getSuggestionsFlow_returnsEmpty_whenCachedIdsEmpty_evenIfWorkIsEnqueued() = + runTest { + val userId = UUID.randomUUID() + val parentId = UUID.randomUUID() + val currentUser = MutableLiveData(mockUser(userId)) + + every { mockServerRepository.currentUser } returns currentUser + coEvery { mockCache.get(userId, parentId, BaseItemKind.MOVIE) } returns CachedSuggestions(emptyList()) + every { mockWorkManager.getWorkInfosForUniqueWorkFlow(any()) } returns + flowOf(listOf(mockWorkInfo(WorkInfo.State.ENQUEUED))) + + val service = createService() + val result = service.getSuggestionsFlow(parentId, BaseItemKind.MOVIE).first() + + assertEquals(SuggestionsResource.Empty, result) + verify(exactly = 0) { mockWorkManager.getWorkInfosForUniqueWorkFlow(any()) } + } + + @Test + fun getSuggestionsFlow_returnsLoading_whenCacheMissing_andWorkIsEnqueued() = + runTest { + val userId = UUID.randomUUID() + val parentId = UUID.randomUUID() + val currentUser = MutableLiveData(mockUser(userId)) + + every { mockServerRepository.currentUser } returns currentUser + coEvery { mockCache.get(userId, parentId, BaseItemKind.MOVIE) } returns null + every { mockWorkManager.getWorkInfosForUniqueWorkFlow(any()) } returns + flowOf(listOf(mockWorkInfo(WorkInfo.State.ENQUEUED))) + + val service = createService() + val result = service.getSuggestionsFlow(parentId, BaseItemKind.MOVIE).first() + + assertEquals(SuggestionsResource.Loading, result) + verify(exactly = 1) { mockWorkManager.getWorkInfosForUniqueWorkFlow(any()) } + } + @Test fun passes_correct_arguments_to_cache() = runTest {