Fix: suggestions get stuck on loading for hours/days (#1364)
## Description This should fix the issue where suggestions would sometimes get stuck loading. My best guess is that they were getting stuck because the app thought it was still loading, even though the background job was just waiting. This should fix that by making each library kick off its own suggestions job when needed. ### Related issues Fixes #1065 ### Testing Emulator and nvidia shield ## Screenshots N/A no ui changes ## AI or LLM usage Used codex to help diagnosis the bug. Also added unit tests and more debug logging for the suggestions system using ai. Wrote the fix myself. --------- Co-authored-by: Damontecres <damontecres@gmail.com>
This commit is contained in:
Justin Caveda
GitHub
parent
c3ba14557c
commit
df2dd3e7ca
6 changed files with 452 additions and 70 deletions
|
|
@ -0,0 +1,68 @@
|
|||
package com.github.damontecres.wholphin.services
|
||||
|
||||
import androidx.appcompat.app.AppCompatActivity
|
||||
import androidx.arch.core.executor.testing.InstantTaskExecutorRule
|
||||
import androidx.lifecycle.Lifecycle
|
||||
import androidx.lifecycle.LifecycleOwner
|
||||
import androidx.lifecycle.LifecycleRegistry
|
||||
import androidx.lifecycle.MutableLiveData
|
||||
import androidx.work.WorkManager
|
||||
import com.github.damontecres.wholphin.data.CurrentUser
|
||||
import com.github.damontecres.wholphin.data.ServerRepository
|
||||
import io.mockk.every
|
||||
import io.mockk.mockk
|
||||
import io.mockk.verify
|
||||
import kotlinx.coroutines.Dispatchers
|
||||
import kotlinx.coroutines.ExperimentalCoroutinesApi
|
||||
import kotlinx.coroutines.test.StandardTestDispatcher
|
||||
import kotlinx.coroutines.test.advanceUntilIdle
|
||||
import kotlinx.coroutines.test.resetMain
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import kotlinx.coroutines.test.setMain
|
||||
import org.junit.After
|
||||
import org.junit.Before
|
||||
import org.junit.Rule
|
||||
import org.junit.Test
|
||||
|
||||
@OptIn(ExperimentalCoroutinesApi::class)
|
||||
class LatestNextUpSchedulerServiceTest {
|
||||
@get:Rule val instantTaskExecutorRule = InstantTaskExecutorRule()
|
||||
private val testDispatcher = StandardTestDispatcher()
|
||||
private val currentLiveData = MutableLiveData<CurrentUser?>()
|
||||
private val mockActivity = mockk<AppCompatActivity>(relaxed = true)
|
||||
private val mockServerRepository = mockk<ServerRepository>(relaxed = true)
|
||||
private val mockWorkManager = mockk<WorkManager>(relaxed = true)
|
||||
private val lifecycleRegistry = LifecycleRegistry(mockk<LifecycleOwner>(relaxed = true))
|
||||
|
||||
@Before
|
||||
fun setUp() {
|
||||
Dispatchers.setMain(testDispatcher)
|
||||
every { mockActivity.lifecycle } returns lifecycleRegistry
|
||||
every { mockServerRepository.current } returns currentLiveData
|
||||
lifecycleRegistry.handleLifecycleEvent(Lifecycle.Event.ON_RESUME)
|
||||
}
|
||||
|
||||
@After
|
||||
fun tearDown() = Dispatchers.resetMain()
|
||||
|
||||
private fun createService() =
|
||||
LatestNextUpSchedulerService(
|
||||
context = mockActivity,
|
||||
serverRepository = mockServerRepository,
|
||||
workManager = mockWorkManager,
|
||||
).also {
|
||||
it.dispatcher = testDispatcher
|
||||
}
|
||||
|
||||
@Test
|
||||
fun cancels_latestNextUp_work_when_user_null() =
|
||||
runTest {
|
||||
createService()
|
||||
|
||||
currentLiveData.value = null
|
||||
advanceUntilIdle()
|
||||
|
||||
verify { mockWorkManager.cancelUniqueWork(LatestNextUpWorker.WORK_NAME) }
|
||||
verify(exactly = 0) { mockWorkManager.cancelUniqueWork(SuggestionsWorker.WORK_NAME) }
|
||||
}
|
||||
}
|
||||
|
|
@ -2,6 +2,8 @@ package com.github.damontecres.wholphin.services
|
|||
|
||||
import androidx.arch.core.executor.testing.InstantTaskExecutorRule
|
||||
import androidx.lifecycle.MutableLiveData
|
||||
import androidx.work.ExistingWorkPolicy
|
||||
import androidx.work.OneTimeWorkRequest
|
||||
import androidx.work.WorkInfo
|
||||
import androidx.work.WorkManager
|
||||
import com.github.damontecres.wholphin.data.ServerRepository
|
||||
|
|
@ -63,13 +65,15 @@ class SuggestionServiceTest {
|
|||
workManager = mockWorkManager,
|
||||
)
|
||||
|
||||
private fun mockQueryResult(items: List<BaseItemDto>): Response<BaseItemDtoQueryResult> =
|
||||
mockk {
|
||||
every { content } returns
|
||||
mockk {
|
||||
every { this@mockk.items } returns items
|
||||
}
|
||||
private fun mockQueryResult(items: List<BaseItemDto>): Response<BaseItemDtoQueryResult> {
|
||||
val queryResult =
|
||||
mockk<BaseItemDtoQueryResult> {
|
||||
every { this@mockk.items } returns items
|
||||
}
|
||||
return mockk {
|
||||
every { content } returns queryResult
|
||||
}
|
||||
}
|
||||
|
||||
private fun mockUser(id: UUID = UUID.randomUUID()): JellyfinUser =
|
||||
JellyfinUser(
|
||||
|
|
@ -81,12 +85,21 @@ class SuggestionServiceTest {
|
|||
|
||||
private fun mockWorkInfo(state: WorkInfo.State): WorkInfo = mockk<WorkInfo> { every { this@mockk.state } returns state }
|
||||
|
||||
private fun mockEnqueueOnDemandWork() {
|
||||
every {
|
||||
mockWorkManager.enqueueUniqueWork(
|
||||
any<String>(),
|
||||
any<ExistingWorkPolicy>(),
|
||||
any<OneTimeWorkRequest>(),
|
||||
)
|
||||
} returns mockk(relaxed = true)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun getSuggestionsFlow_returnsEmpty_whenNoUserLoggedIn() =
|
||||
runTest {
|
||||
val currentUser = MutableLiveData<JellyfinUser?>(null)
|
||||
every { mockServerRepository.currentUser } returns currentUser
|
||||
every { mockWorkManager.getWorkInfosForUniqueWorkFlow(any()) } returns flowOf(emptyList())
|
||||
|
||||
val service = createService()
|
||||
val result = service.getSuggestionsFlow(UUID.randomUUID(), BaseItemKind.MOVIE).first()
|
||||
|
|
@ -95,22 +108,31 @@ class SuggestionServiceTest {
|
|||
}
|
||||
|
||||
@Test
|
||||
fun maps_active_work_states_to_Loading() =
|
||||
fun maps_active_onDemand_work_states_to_Loading() =
|
||||
runTest {
|
||||
listOf(WorkInfo.State.RUNNING, WorkInfo.State.ENQUEUED).forEach { state ->
|
||||
val userId = UUID.randomUUID()
|
||||
val parentId = UUID.randomUUID()
|
||||
val currentUser = MutableLiveData<JellyfinUser?>(mockUser(userId))
|
||||
val workName = SuggestionsWorker.getOnDemandWorkName(userId, parentId, BaseItemKind.MOVIE)
|
||||
|
||||
every { mockServerRepository.currentUser } returns currentUser
|
||||
coEvery { mockCache.get(userId, parentId, BaseItemKind.MOVIE) } returns null
|
||||
every { mockWorkManager.getWorkInfosForUniqueWorkFlow(any()) } returns
|
||||
mockEnqueueOnDemandWork()
|
||||
every { mockWorkManager.getWorkInfosForUniqueWorkFlow(workName) } returns
|
||||
flowOf(listOf(mockWorkInfo(state)))
|
||||
|
||||
val service = createService()
|
||||
val result = service.getSuggestionsFlow(parentId, BaseItemKind.MOVIE).first()
|
||||
|
||||
assertEquals(SuggestionsResource.Loading, result)
|
||||
verify {
|
||||
mockWorkManager.enqueueUniqueWork(
|
||||
workName,
|
||||
ExistingWorkPolicy.REPLACE,
|
||||
any<OneTimeWorkRequest>(),
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -121,10 +143,12 @@ class SuggestionServiceTest {
|
|||
val userId = UUID.randomUUID()
|
||||
val parentId = UUID.randomUUID()
|
||||
val currentUser = MutableLiveData<JellyfinUser?>(mockUser(userId))
|
||||
val workName = SuggestionsWorker.getOnDemandWorkName(userId, parentId, BaseItemKind.MOVIE)
|
||||
|
||||
every { mockServerRepository.currentUser } returns currentUser
|
||||
coEvery { mockCache.get(userId, parentId, BaseItemKind.MOVIE) } returns null
|
||||
every { mockWorkManager.getWorkInfosForUniqueWorkFlow(any()) } returns
|
||||
mockEnqueueOnDemandWork()
|
||||
every { mockWorkManager.getWorkInfosForUniqueWorkFlow(workName) } returns
|
||||
flowOf(listOf(mockWorkInfo(state)))
|
||||
|
||||
val service = createService()
|
||||
|
|
@ -140,15 +164,55 @@ class SuggestionServiceTest {
|
|||
val userId = UUID.randomUUID()
|
||||
val parentId = UUID.randomUUID()
|
||||
val currentUser = MutableLiveData<JellyfinUser?>(mockUser(userId))
|
||||
val workName = SuggestionsWorker.getOnDemandWorkName(userId, parentId, BaseItemKind.MOVIE)
|
||||
|
||||
every { mockServerRepository.currentUser } returns currentUser
|
||||
coEvery { mockCache.get(userId, parentId, BaseItemKind.MOVIE) } returns null
|
||||
every { mockWorkManager.getWorkInfosForUniqueWorkFlow(any()) } returns flowOf(emptyList())
|
||||
mockEnqueueOnDemandWork()
|
||||
every { mockWorkManager.getWorkInfosForUniqueWorkFlow(workName) } returns flowOf(emptyList())
|
||||
|
||||
val service = createService()
|
||||
val result = service.getSuggestionsFlow(parentId, BaseItemKind.MOVIE).first()
|
||||
|
||||
assertEquals(SuggestionsResource.Empty, result)
|
||||
verify {
|
||||
mockWorkManager.enqueueUniqueWork(
|
||||
workName,
|
||||
ExistingWorkPolicy.REPLACE,
|
||||
any<OneTimeWorkRequest>(),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun getSuggestionsFlow_returnsSuccess_whenOnDemandWorkCachesItems() =
|
||||
runTest {
|
||||
val userId = UUID.randomUUID()
|
||||
val parentId = UUID.randomUUID()
|
||||
val currentUser = MutableLiveData<JellyfinUser?>(mockUser(userId))
|
||||
val workName = SuggestionsWorker.getOnDemandWorkName(userId, parentId, BaseItemKind.MOVIE)
|
||||
val cachedId = UUID.randomUUID()
|
||||
|
||||
every { mockServerRepository.currentUser } returns currentUser
|
||||
coEvery { mockCache.get(userId, parentId, BaseItemKind.MOVIE) } returnsMany
|
||||
listOf(null, CachedSuggestions(listOf(cachedId)))
|
||||
mockEnqueueOnDemandWork()
|
||||
every { mockWorkManager.getWorkInfosForUniqueWorkFlow(workName) } returns
|
||||
flowOf(listOf(mockWorkInfo(WorkInfo.State.SUCCEEDED)))
|
||||
|
||||
val dto =
|
||||
mockk<BaseItemDto>(relaxed = true) {
|
||||
every { id } returns cachedId
|
||||
every { type } returns BaseItemKind.MOVIE
|
||||
}
|
||||
io.mockk.mockkObject(GetItemsRequestHandler)
|
||||
coEvery { GetItemsRequestHandler.execute(mockApi, any()) } returns mockQueryResult(listOf(dto))
|
||||
|
||||
val service = createService()
|
||||
val result = service.getSuggestionsFlow(parentId, BaseItemKind.MOVIE).first()
|
||||
|
||||
assertTrue(result is SuggestionsResource.Success)
|
||||
assertEquals(cachedId, (result as SuggestionsResource.Success).items.single().id)
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -160,33 +224,42 @@ class SuggestionServiceTest {
|
|||
|
||||
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()) }
|
||||
verify(exactly = 0) {
|
||||
mockWorkManager.enqueueUniqueWork(
|
||||
any<String>(),
|
||||
any<ExistingWorkPolicy>(),
|
||||
any<OneTimeWorkRequest>(),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun getSuggestionsFlow_returnsLoading_whenCacheMissing_andWorkIsEnqueued() =
|
||||
fun getSuggestionsFlow_ignores_periodic_work_whenCacheMissing() =
|
||||
runTest {
|
||||
val userId = UUID.randomUUID()
|
||||
val parentId = UUID.randomUUID()
|
||||
val currentUser = MutableLiveData<JellyfinUser?>(mockUser(userId))
|
||||
val workName = SuggestionsWorker.getOnDemandWorkName(userId, parentId, BaseItemKind.MOVIE)
|
||||
|
||||
every { mockServerRepository.currentUser } returns currentUser
|
||||
coEvery { mockCache.get(userId, parentId, BaseItemKind.MOVIE) } returns null
|
||||
every { mockWorkManager.getWorkInfosForUniqueWorkFlow(any()) } returns
|
||||
mockEnqueueOnDemandWork()
|
||||
every { mockWorkManager.getWorkInfosForUniqueWorkFlow(SuggestionsWorker.WORK_NAME) } returns
|
||||
flowOf(listOf(mockWorkInfo(WorkInfo.State.ENQUEUED)))
|
||||
every { mockWorkManager.getWorkInfosForUniqueWorkFlow(workName) } returns flowOf(emptyList())
|
||||
|
||||
val service = createService()
|
||||
val result = service.getSuggestionsFlow(parentId, BaseItemKind.MOVIE).first()
|
||||
|
||||
assertEquals(SuggestionsResource.Loading, result)
|
||||
verify(exactly = 1) { mockWorkManager.getWorkInfosForUniqueWorkFlow(any()) }
|
||||
assertEquals(SuggestionsResource.Empty, result)
|
||||
verify(exactly = 0) { mockWorkManager.getWorkInfosForUniqueWorkFlow(SuggestionsWorker.WORK_NAME) }
|
||||
verify(exactly = 1) { mockWorkManager.getWorkInfosForUniqueWorkFlow(workName) }
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -196,9 +269,11 @@ class SuggestionServiceTest {
|
|||
val libraryId = UUID.randomUUID()
|
||||
val otherLibraryId = UUID.randomUUID()
|
||||
val currentUser = MutableLiveData<JellyfinUser?>(mockUser(userId))
|
||||
val workName = SuggestionsWorker.getOnDemandWorkName(userId, libraryId, BaseItemKind.MOVIE)
|
||||
|
||||
every { mockServerRepository.currentUser } returns currentUser
|
||||
every { mockWorkManager.getWorkInfosForUniqueWorkFlow(any()) } returns flowOf(emptyList())
|
||||
mockEnqueueOnDemandWork()
|
||||
every { mockWorkManager.getWorkInfosForUniqueWorkFlow(workName) } returns flowOf(emptyList())
|
||||
|
||||
coEvery { mockCache.get(userId, libraryId, BaseItemKind.MOVIE) } returns null
|
||||
coEvery {
|
||||
|
|
@ -250,8 +325,6 @@ class SuggestionServiceTest {
|
|||
io.mockk.mockkObject(GetItemsRequestHandler)
|
||||
coEvery { GetItemsRequestHandler.execute(mockApi, any()) } returns mockQueryResult(listOf(dto))
|
||||
|
||||
every { mockWorkManager.getWorkInfosForUniqueWorkFlow(any()) } returns flowOf(emptyList())
|
||||
|
||||
val service = createService()
|
||||
val result = service.getSuggestionsFlow(parentId, BaseItemKind.MOVIE).first()
|
||||
|
||||
|
|
@ -282,8 +355,6 @@ class SuggestionServiceTest {
|
|||
io.mockk.mockkObject(GetItemsRequestHandler)
|
||||
coEvery { GetItemsRequestHandler.execute(mockApi, any()) } throws RuntimeException("Network error")
|
||||
|
||||
every { mockWorkManager.getWorkInfosForUniqueWorkFlow(any()) } returns flowOf(emptyList())
|
||||
|
||||
val service = createService()
|
||||
val result = service.getSuggestionsFlow(parentId, BaseItemKind.MOVIE).first()
|
||||
|
||||
|
|
|
|||
|
|
@ -55,6 +55,8 @@ class SuggestionsWorkerTest {
|
|||
private fun createWorker(
|
||||
userId: UUID? = testUserId,
|
||||
serverId: UUID? = testServerId,
|
||||
parentId: UUID? = null,
|
||||
itemKind: BaseItemKind? = null,
|
||||
): SuggestionsWorker {
|
||||
val inputData =
|
||||
Data
|
||||
|
|
@ -62,6 +64,8 @@ class SuggestionsWorkerTest {
|
|||
.apply {
|
||||
userId?.let { putString(SuggestionsWorker.PARAM_USER_ID, it.toString()) }
|
||||
serverId?.let { putString(SuggestionsWorker.PARAM_SERVER_ID, it.toString()) }
|
||||
parentId?.let { putString(SuggestionsWorker.PARAM_PARENT_ID, it.toString()) }
|
||||
itemKind?.let { putString(SuggestionsWorker.PARAM_ITEM_KIND, it.serialName) }
|
||||
}.build()
|
||||
every { mockWorkerParams.inputData } returns inputData
|
||||
return SuggestionsWorker(
|
||||
|
|
@ -90,6 +94,17 @@ class SuggestionsWorkerTest {
|
|||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun returns_failure_on_partial_onDemand_input() =
|
||||
runTest {
|
||||
listOf(
|
||||
createWorker(parentId = UUID.randomUUID()),
|
||||
createWorker(itemKind = BaseItemKind.MOVIE),
|
||||
).forEach { worker ->
|
||||
assertEquals(ListenableWorker.Result.failure(), worker.doWork())
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun restores_session_when_api_not_configured() =
|
||||
runTest {
|
||||
|
|
@ -111,6 +126,21 @@ class SuggestionsWorkerTest {
|
|||
assertEquals(ListenableWorker.Result.success(), result)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun onDemand_work_caches_only_requested_library() =
|
||||
runTest {
|
||||
val parentId = UUID.randomUUID()
|
||||
every { mockPreferences.data } returns flowOf(mockPrefs())
|
||||
mockkObject(GetItemsRequestHandler)
|
||||
coEvery { GetItemsRequestHandler.execute(mockApi, any()) } returns mockQueryResult(emptyList())
|
||||
|
||||
val result = createWorker(parentId = parentId, itemKind = BaseItemKind.MOVIE).doWork()
|
||||
|
||||
assertEquals(ListenableWorker.Result.success(), result)
|
||||
coVerify(exactly = 0) { mockUserViewsApi.getUserViews(userId = any()) }
|
||||
coVerify { mockCache.put(testUserId, parentId, BaseItemKind.MOVIE, emptyList()) }
|
||||
}
|
||||
|
||||
@Test
|
||||
fun caches_suggestions_for_supported_types() =
|
||||
runTest {
|
||||
|
|
@ -136,6 +166,43 @@ class SuggestionsWorkerTest {
|
|||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun caches_empty_suggestions_for_supported_types() =
|
||||
runTest {
|
||||
val viewId = UUID.randomUUID()
|
||||
val view =
|
||||
mockk<BaseItemDto>(relaxed = true) {
|
||||
every { id } returns viewId
|
||||
every { this@mockk.collectionType } returns CollectionType.MOVIES
|
||||
}
|
||||
every { mockPreferences.data } returns flowOf(mockPrefs())
|
||||
coEvery { mockUserViewsApi.getUserViews(userId = testUserId) } returns mockQueryResult(listOf(view))
|
||||
mockkObject(GetItemsRequestHandler)
|
||||
coEvery { GetItemsRequestHandler.execute(mockApi, any()) } returns mockQueryResult(emptyList())
|
||||
|
||||
val result = createWorker().doWork()
|
||||
|
||||
assertEquals(ListenableWorker.Result.success(), result)
|
||||
coVerify { mockCache.put(testUserId, viewId, BaseItemKind.MOVIE, emptyList()) }
|
||||
}
|
||||
|
||||
@Test
|
||||
fun skips_unsupported_collection_types() =
|
||||
runTest {
|
||||
val view =
|
||||
mockk<BaseItemDto>(relaxed = true) {
|
||||
every { id } returns UUID.randomUUID()
|
||||
every { this@mockk.collectionType } returns CollectionType.MUSIC
|
||||
}
|
||||
every { mockPreferences.data } returns flowOf(mockPrefs())
|
||||
coEvery { mockUserViewsApi.getUserViews(userId = testUserId) } returns mockQueryResult(listOf(view))
|
||||
|
||||
val result = createWorker().doWork()
|
||||
|
||||
assertEquals(ListenableWorker.Result.success(), result)
|
||||
coVerify(exactly = 0) { mockCache.put(any(), any(), any(), any()) }
|
||||
}
|
||||
|
||||
@Test
|
||||
fun fetches_contextual_suggestions_when_genres_available() =
|
||||
runTest {
|
||||
|
|
@ -227,7 +294,12 @@ class SuggestionsWorkerTest {
|
|||
}
|
||||
}
|
||||
|
||||
fun mockQueryResult(items: List<BaseItemDto> = emptyList()) =
|
||||
mockk<org.jellyfin.sdk.api.client.Response<BaseItemDtoQueryResult>>(relaxed = true) {
|
||||
every { content } returns mockk { every { this@mockk.items } returns items }
|
||||
fun mockQueryResult(items: List<BaseItemDto> = emptyList()): org.jellyfin.sdk.api.client.Response<BaseItemDtoQueryResult> {
|
||||
val queryResult =
|
||||
mockk<BaseItemDtoQueryResult> {
|
||||
every { this@mockk.items } returns items
|
||||
}
|
||||
return mockk(relaxed = true) {
|
||||
every { content } returns queryResult
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue