Fix: suggestions logic/re-implement logic into worker and address performance issues (#834)

## Description
Addresses suggestion logic oversight in recently merged suggestions fix,
and improve performance (especially app homepage startup time)

- Re-implements the logic that sorts/categorizes tailored suggestions
based on the seed items that are queried. This was implemented in the
original PR at first, but I overlooked moving that logic
- Removes `OneTimeWorkRequestBuilder` and replaces with trigger in
`PeriodicWorkRequest` to fire with a 30 seconds delay if cache is empty.

 
### Related issues
Fixes #833

### Testing
Will be testing in:
Android TV emulator in Android Studio
NVIDIA Shield TV Pro 2019

## AI or LLM usage
I will use AI to help with writing the tests/testing services.
This commit is contained in:
Justin Caveda 2026-02-07 19:38:26 -06:00 committed by GitHub
parent abf5ec8004
commit 8e9a6799f5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 186 additions and 26 deletions

View file

@ -7,7 +7,6 @@ import androidx.work.BackoffPolicy
import androidx.work.Constraints
import androidx.work.ExistingPeriodicWorkPolicy
import androidx.work.NetworkType
import androidx.work.OneTimeWorkRequestBuilder
import androidx.work.PeriodicWorkRequestBuilder
import androidx.work.WorkManager
import androidx.work.workDataOf
@ -23,6 +22,7 @@ import java.util.UUID
import javax.inject.Inject
import kotlin.time.Duration.Companion.hours
import kotlin.time.Duration.Companion.minutes
import kotlin.time.Duration.Companion.seconds
import kotlin.time.toJavaDuration
@ActivityScoped
@ -72,21 +72,7 @@ class SuggestionsSchedulerService
SuggestionsWorker.PARAM_SERVER_ID to serverId.toString(),
)
if (cache.isEmpty()) {
Timber.i("Suggestions cache empty, scheduling immediate fetch")
workManager.enqueue(
OneTimeWorkRequestBuilder<SuggestionsWorker>()
.setConstraints(constraints)
.setInputData(inputData)
.build(),
)
}
Timber.i("Scheduling periodic SuggestionsWorker")
workManager.enqueueUniquePeriodicWork(
uniqueWorkName = SuggestionsWorker.WORK_NAME,
existingPeriodicWorkPolicy = ExistingPeriodicWorkPolicy.UPDATE,
request =
val periodicWorkRequestBuilder =
PeriodicWorkRequestBuilder<SuggestionsWorker>(
repeatInterval = 12.hours.toJavaDuration(),
).setConstraints(constraints)
@ -94,7 +80,18 @@ class SuggestionsSchedulerService
BackoffPolicy.EXPONENTIAL,
15.minutes.toJavaDuration(),
).setInputData(inputData)
.build(),
if (cache.isEmpty()) {
Timber.i("Suggestions cache empty, scheduling periodic fetch with 30s delay")
periodicWorkRequestBuilder.setInitialDelay(30.seconds.toJavaDuration())
} else {
Timber.i("Scheduling periodic SuggestionsWorker")
}
workManager.enqueueUniquePeriodicWork(
uniqueWorkName = SuggestionsWorker.WORK_NAME,
existingPeriodicWorkPolicy = ExistingPeriodicWorkPolicy.UPDATE,
request = periodicWorkRequestBuilder.build(),
)
}
}

View file

@ -136,6 +136,10 @@ class SuggestionsWorker
val isSeries = itemKind == BaseItemKind.SERIES
val historyItemType = if (isSeries) BaseItemKind.EPISODE else itemKind
val contextualLimit = (itemsPerRow * 0.4).toInt().coerceAtLeast(1)
val randomLimit = (itemsPerRow * 0.3).toInt().coerceAtLeast(1)
val freshLimit = (itemsPerRow * 0.3).toInt().coerceAtLeast(1)
val historyDeferred =
async(Dispatchers.IO) {
fetchItems(
@ -144,11 +148,38 @@ class SuggestionsWorker
itemKind = historyItemType,
sortBy = ItemSortBy.DATE_PLAYED,
isPlayed = true,
limit = 10,
limit = 20,
extraFields = listOf(ItemFields.GENRES),
).distinctBy { it.relevantId }.take(3)
}
val seedItems = historyDeferred.await()
val allGenreIds =
seedItems
.flatMap { it.genreItems?.mapNotNull { g -> g.id } ?: emptyList() }
.distinct()
val excludeIds = seedItems.mapTo(HashSet()) { it.relevantId }
val contextualDeferred =
async(Dispatchers.IO) {
if (allGenreIds.isEmpty()) {
emptyList()
} else {
fetchItems(
parentId = parentId,
userId = userId,
itemKind = itemKind,
sortBy = ItemSortBy.RANDOM,
isPlayed = false,
limit = contextualLimit,
genreIds = allGenreIds,
excludeItemIds = excludeIds.toList(),
)
}
}
val randomDeferred =
async(Dispatchers.IO) {
fetchItems(
@ -157,7 +188,7 @@ class SuggestionsWorker
itemKind = itemKind,
sortBy = ItemSortBy.RANDOM,
isPlayed = false,
limit = itemsPerRow,
limit = randomLimit,
)
}
@ -170,17 +201,15 @@ class SuggestionsWorker
sortBy = ItemSortBy.DATE_CREATED,
sortOrder = SortOrder.DESCENDING,
isPlayed = false,
limit = (itemsPerRow * FRESH_CONTENT_RATIO).toInt().coerceAtLeast(1),
limit = freshLimit,
)
}
val seedItems = historyDeferred.await()
val contextual = contextualDeferred.await()
val random = randomDeferred.await()
val fresh = freshDeferred.await()
val excludeIds = seedItems.mapTo(HashSet()) { it.relevantId }
(fresh + random)
(contextual + fresh + random)
.asSequence()
.distinctBy { it.id }
.filterNot { excludeIds.contains(it.relevantId) }
@ -198,6 +227,7 @@ class SuggestionsWorker
limit: Int,
sortOrder: SortOrder? = null,
genreIds: List<UUID>? = null,
excludeItemIds: List<UUID>? = null,
extraFields: List<ItemFields> = emptyList(),
): List<BaseItemDto> {
val request =
@ -209,6 +239,7 @@ class SuggestionsWorker
genreIds = genreIds,
recursive = true,
isPlayed = isPlayed,
excludeItemIds = excludeItemIds,
sortBy = listOf(sortBy),
sortOrder = sortOrder?.let { listOf(it) },
limit = limit,
@ -225,6 +256,5 @@ class SuggestionsWorker
const val WORK_NAME = "com.github.damontecres.wholphin.services.SuggestionsWorker"
const val PARAM_USER_ID = "userId"
const val PARAM_SERVER_ID = "serverId"
private const val FRESH_CONTENT_RATIO = 0.4
}
}

View file

@ -6,6 +6,7 @@ import androidx.lifecycle.Lifecycle
import androidx.lifecycle.LifecycleOwner
import androidx.lifecycle.LifecycleRegistry
import androidx.lifecycle.MutableLiveData
import androidx.work.PeriodicWorkRequest
import androidx.work.WorkManager
import com.github.damontecres.wholphin.data.CurrentUser
import com.github.damontecres.wholphin.data.ServerRepository
@ -14,6 +15,7 @@ import com.github.damontecres.wholphin.data.model.JellyfinUser
import io.mockk.coEvery
import io.mockk.every
import io.mockk.mockk
import io.mockk.slot
import io.mockk.verify
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi
@ -23,10 +25,12 @@ import kotlinx.coroutines.test.resetMain
import kotlinx.coroutines.test.runTest
import kotlinx.coroutines.test.setMain
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Before
import org.junit.Rule
import org.junit.Test
import java.util.UUID
import java.util.concurrent.TimeUnit
@OptIn(ExperimentalCoroutinesApi::class)
class SuggestionsSchedulerServiceTest {
@ -87,4 +91,54 @@ class SuggestionsSchedulerServiceTest {
advanceUntilIdle()
verify { mockWorkManager.cancelUniqueWork(SuggestionsWorker.WORK_NAME) }
}
@Test
fun schedules_periodic_work_with_delay_when_cache_empty() =
runTest {
coEvery { mockCache.isEmpty() } returns true
val workRequestSlot = slot<PeriodicWorkRequest>()
every {
mockWorkManager.enqueueUniquePeriodicWork(
SuggestionsWorker.WORK_NAME,
any(),
capture(workRequestSlot),
)
} returns mockk()
createService()
currentLiveData.value =
CurrentUser(
user = JellyfinUser(id = UUID.randomUUID(), name = "User", serverId = UUID.randomUUID(), accessToken = "token"),
server = JellyfinServer(id = UUID.randomUUID(), name = "Server", url = "http://localhost", version = null),
)
advanceUntilIdle()
verify { mockWorkManager.enqueueUniquePeriodicWork(SuggestionsWorker.WORK_NAME, any(), any()) }
assertEquals(30000L, workRequestSlot.captured.workSpec.initialDelay)
}
@Test
fun schedules_periodic_work_without_delay_when_cache_not_empty() =
runTest {
coEvery { mockCache.isEmpty() } returns false
val workRequestSlot = slot<PeriodicWorkRequest>()
every {
mockWorkManager.enqueueUniquePeriodicWork(
SuggestionsWorker.WORK_NAME,
any(),
capture(workRequestSlot),
)
} returns mockk()
createService()
currentLiveData.value =
CurrentUser(
user = JellyfinUser(id = UUID.randomUUID(), name = "User", serverId = UUID.randomUUID(), accessToken = "token"),
server = JellyfinServer(id = UUID.randomUUID(), name = "Server", url = "http://localhost", version = null),
)
advanceUntilIdle()
verify { mockWorkManager.enqueueUniquePeriodicWork(SuggestionsWorker.WORK_NAME, any(), any()) }
assertEquals(0L, workRequestSlot.captured.workSpec.initialDelay)
}
}

View file

@ -141,6 +141,85 @@ class SuggestionsWorkerTest {
}
}
@Test
fun fetches_contextual_suggestions_when_genres_available() =
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)
val genreId = UUID.randomUUID()
val historyItem =
mockk<BaseItemDto>(relaxed = true) {
every { id } returns UUID.randomUUID()
every { genreItems } returns listOf(mockk { every { id } returns genreId })
}
val contextualItem = mockk<BaseItemDto>(relaxed = true) { every { id } returns UUID.randomUUID() }
val randomItem = mockk<BaseItemDto>(relaxed = true) { every { id } returns UUID.randomUUID() }
val freshItem = mockk<BaseItemDto>(relaxed = true) { every { id } returns UUID.randomUUID() }
var callCount = 0
coEvery { GetItemsRequestHandler.execute(mockApi, any()) } answers {
callCount++
when (callCount) {
1 -> mockQueryResult(listOf(historyItem))
2 -> mockQueryResult(listOf(contextualItem))
3 -> mockQueryResult(listOf(randomItem))
4 -> mockQueryResult(listOf(freshItem))
else -> mockQueryResult(emptyList())
}
}
val result = createWorker().doWork()
assertEquals(ListenableWorker.Result.success(), result)
coVerify { mockCache.put(testUserId, viewId, BaseItemKind.MOVIE, any()) }
}
@Test
fun skips_contextual_suggestions_when_no_genres_available() =
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)
val historyItem =
mockk<BaseItemDto>(relaxed = true) {
every { id } returns UUID.randomUUID()
every { genreItems } returns emptyList()
}
val randomItem = mockk<BaseItemDto>(relaxed = true) { every { id } returns UUID.randomUUID() }
val freshItem = mockk<BaseItemDto>(relaxed = true) { every { id } returns UUID.randomUUID() }
var callCount = 0
coEvery { GetItemsRequestHandler.execute(mockApi, any()) } answers {
callCount++
when (callCount) {
1 -> mockQueryResult(listOf(historyItem))
2 -> mockQueryResult(listOf(randomItem))
3 -> mockQueryResult(listOf(freshItem))
else -> mockQueryResult(emptyList())
}
}
val result = createWorker().doWork()
assertEquals(ListenableWorker.Result.success(), result)
coVerify { mockCache.put(testUserId, viewId, BaseItemKind.MOVIE, any()) }
}
@Test
fun returns_retry_on_network_error() =
runTest {