Fix: optimize suggestions performance (#849)

## Description
- Aims to further optimize performance for the suggestions fetching

### Related issues
Fixes #833 

### Testing
Android Emulator
NVIDIA Shield Pro 2019

## Screenshots
N/A

## AI or LLM usage
AI being used to assist with updating/creating unit tests

app/src/main/java/com/github/damontecres/wholphin/services/SuggestionsSchedulerService.kt

@@ -8,6 +8,7 @@ import androidx.work.Constraints
 import androidx.work.ExistingPeriodicWorkPolicy
 import androidx.work.NetworkType
 import androidx.work.PeriodicWorkRequestBuilder
+import androidx.work.WorkInfo
 import androidx.work.WorkManager
 import androidx.work.workDataOf
 import com.github.damontecres.wholphin.data.ServerRepository
@@ -17,9 +18,11 @@ import dagger.hilt.android.scopes.ActivityScoped
 import kotlinx.coroutines.CoroutineDispatcher
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.launch
+import kotlinx.coroutines.withContext
 import timber.log.Timber
 import java.util.UUID
 import javax.inject.Inject
+import kotlin.random.Random
 import kotlin.time.Duration.Companion.hours
 import kotlin.time.Duration.Companion.minutes
 import kotlin.time.Duration.Companion.seconds
@@ -31,7 +34,6 @@ class SuggestionsSchedulerService
     constructor(
         @param:ActivityContext private val context: Context,
         private val serverRepository: ServerRepository,
-        private val cache: SuggestionsCache,
         private val workManager: WorkManager,
     ) {
         private val activity =
@@ -42,6 +44,7 @@ class SuggestionsSchedulerService
 
         // Exposed for testing
         internal var dispatcher: CoroutineDispatcher = Dispatchers.IO
+        internal var initialDelaySecondsProvider: () -> Long = { 60L + Random.nextLong(0L, 121L) }
 
         init {
             serverRepository.current.observe(activity) { user ->
@@ -60,6 +63,28 @@ class SuggestionsSchedulerService
             userId: UUID,
             serverId: UUID,
         ) {
+            val workInfos =
+                withContext(dispatcher) {
+                    workManager
+                        .getWorkInfosForUniqueWork(SuggestionsWorker.WORK_NAME)
+                        .get()
+                }
+            val activeWork =
+                workInfos.firstOrNull {
+                    !it.state.isFinished
+                }
+            val scheduledUserId =
+                activeWork
+                    ?.tags
+                    ?.firstOrNull { it.startsWith("user:") }
+                    ?.removePrefix("user:")
+
+            val isAlreadyScheduledForUser = scheduledUserId == userId.toString()
+            if (isAlreadyScheduledForUser) {
+                Timber.d("SuggestionsWorker already scheduled for user %s", userId)
+                return
+            }
+
             val constraints =
                 Constraints
                     .Builder()
@@ -75,12 +100,18 @@ class SuggestionsSchedulerService
             val periodicWorkRequestBuilder =
                 PeriodicWorkRequestBuilder<SuggestionsWorker>(
                     repeatInterval = 12.hours.toJavaDuration(),
+                    flexTimeInterval = 1.hours.toJavaDuration(),
                 ).setConstraints(constraints)
                     .setBackoffCriteria(
                         BackoffPolicy.EXPONENTIAL,
                         15.minutes.toJavaDuration(),
                     ).setInputData(inputData)
-                    .setInitialDelay(60.seconds.toJavaDuration())
+                    .addTag("user:$userId")
+
+            val initialDelaySeconds = initialDelaySecondsProvider().coerceIn(60L, 180L)
+            periodicWorkRequestBuilder.setInitialDelay(initialDelaySeconds.seconds.toJavaDuration())
+
+            Timber.i("Scheduling periodic SuggestionsWorker with ${initialDelaySeconds}s delay")
 
             workManager.enqueueUniquePeriodicWork(
                 uniqueWorkName = SuggestionsWorker.WORK_NAME,

app/src/main/java/com/github/damontecres/wholphin/services/SuggestionsWorker.kt

@@ -99,11 +99,17 @@ class SuggestionsWorker
                                                 itemsPerRow,
                                             )
                                         ensureActive()
+                                        val newIds = suggestions.map { it.id }
+                                        val cachedIds = cache.get(userId, view.id, itemKind)?.ids
+                                        if (cachedIds == newIds) {
+                                            Timber.v("Suggestions unchanged for view %s, skipping cache write", view.id)
+                                            return@runCatching
+                                        }
                                         cache.put(
                                             userId,
                                             view.id,
                                             itemKind,
-                                            suggestions.map { it.id },
+                                            newIds,
                                         )
                                     }.onFailure { e ->
                                         Timber.e(
@@ -242,7 +248,7 @@ class SuggestionsWorker
                 GetItemsRequest(
                     parentId = parentId,
                     userId = userId,
-                    fields = listOf(ItemFields.PRIMARY_IMAGE_ASPECT_RATIO, ItemFields.OVERVIEW) + extraFields,
+                    fields = extraFields,
                     includeItemTypes = listOf(itemKind),
                     genreIds = genreIds,
                     recursive = true,
@@ -252,7 +258,7 @@ class SuggestionsWorker
                     sortOrder = sortOrder?.let { listOf(it) },
                     limit = limit,
                     enableTotalRecordCount = false,
-                    imageTypeLimit = 1,
+                    imageTypeLimit = 0,
                 )
             return GetItemsRequestHandler
                 .execute(api, request)

app/src/test/java/com/github/damontecres/wholphin/services/SuggestionsSchedulerServiceTest.kt

@@ -12,6 +12,7 @@ import com.github.damontecres.wholphin.data.CurrentUser
 import com.github.damontecres.wholphin.data.ServerRepository
 import com.github.damontecres.wholphin.data.model.JellyfinServer
 import com.github.damontecres.wholphin.data.model.JellyfinUser
+import com.google.common.util.concurrent.ListenableFuture
 import io.mockk.every
 import io.mockk.mockk
 import io.mockk.slot
@@ -37,7 +38,6 @@ class SuggestionsSchedulerServiceTest {
     private val currentLiveData = MutableLiveData<CurrentUser?>()
     private val mockActivity = mockk<AppCompatActivity>(relaxed = true)
     private val mockServerRepository = mockk<ServerRepository>(relaxed = true)
-    private val mockCache = mockk<SuggestionsCache>(relaxed = true)
     private val mockWorkManager = mockk<WorkManager>(relaxed = true)
     private val lifecycleRegistry = LifecycleRegistry(mockk<LifecycleOwner>(relaxed = true))
 
@@ -56,13 +56,23 @@ class SuggestionsSchedulerServiceTest {
         SuggestionsSchedulerService(
             context = mockActivity,
             serverRepository = mockServerRepository,
-            cache = mockCache,
             workManager = mockWorkManager,
-        ).also { it.dispatcher = testDispatcher }
+        ).also {
+            it.dispatcher = testDispatcher
+            it.initialDelaySecondsProvider = { 60L }
+        }
+
+    private fun mockWorkInfos(infos: List<androidx.work.WorkInfo>) {
+        @Suppress("UNCHECKED_CAST")
+        val future = mockk<ListenableFuture<List<androidx.work.WorkInfo>>>()
+        every { future.get() } returns infos
+        every { mockWorkManager.getWorkInfosForUniqueWork(SuggestionsWorker.WORK_NAME) } returns future
+    }
 
     @Test
     fun schedules_periodic_work_when_user_present() =
         runTest {
+            mockWorkInfos(emptyList())
             createService()
             currentLiveData.value =
                 CurrentUser(
@@ -76,6 +86,7 @@ class SuggestionsSchedulerServiceTest {
     @Test
     fun cancels_work_when_user_null() =
         runTest {
+            mockWorkInfos(emptyList())
             createService()
             currentLiveData.value =
                 CurrentUser(
@@ -91,6 +102,7 @@ class SuggestionsSchedulerServiceTest {
     @Test
     fun schedules_periodic_work_with_delay_when_cache_empty() =
         runTest {
+            mockWorkInfos(emptyList())
             val workRequestSlot = slot<PeriodicWorkRequest>()
             every {
                 mockWorkManager.enqueueUniquePeriodicWork(
@@ -115,6 +127,7 @@ class SuggestionsSchedulerServiceTest {
     @Test
     fun schedules_periodic_work_with_delay_when_cache_not_empty() =
         runTest {
+            mockWorkInfos(emptyList())
             val workRequestSlot = slot<PeriodicWorkRequest>()
             every {
                 mockWorkManager.enqueueUniquePeriodicWork(
@@ -135,4 +148,24 @@ class SuggestionsSchedulerServiceTest {
             verify { mockWorkManager.enqueueUniquePeriodicWork(SuggestionsWorker.WORK_NAME, any(), any()) }
             assertEquals(60000L, workRequestSlot.captured.workSpec.initialDelay)
         }
+
+    @Test
+    fun does_not_schedule_if_already_scheduled_for_same_user() =
+        runTest {
+            val userId = UUID.randomUUID()
+            val workInfo = mockk<androidx.work.WorkInfo>()
+            every { workInfo.state } returns androidx.work.WorkInfo.State.ENQUEUED
+            every { workInfo.tags } returns setOf("user:$userId")
+            mockWorkInfos(listOf(workInfo))
+
+            createService()
+            currentLiveData.value =
+                CurrentUser(
+                    user = JellyfinUser(id = userId, name = "User", serverId = UUID.randomUUID(), accessToken = "token"),
+                    server = JellyfinServer(id = UUID.randomUUID(), name = "Server", url = "http://localhost", version = null),
+                )
+            advanceUntilIdle()
+
+            verify(exactly = 0) { mockWorkManager.enqueueUniquePeriodicWork(any(), any(), any()) }
+        }
 }

app/src/test/java/com/github/damontecres/wholphin/services/SuggestionsWorkerTest.kt

@@ -46,6 +46,7 @@ class SuggestionsWorkerTest {
         every { mockApi.userViewsApi } returns mockUserViewsApi
         every { mockApi.baseUrl } returns "http://localhost"
         every { mockApi.accessToken } returns "test-token"
+        coEvery { mockCache.get(any(), any(), any()) } returns null
     }
 
     @After