diff --git a/app/src/main/java/com/github/damontecres/wholphin/services/SuggestionsSchedulerService.kt b/app/src/main/java/com/github/damontecres/wholphin/services/SuggestionsSchedulerService.kt index edca582b..86bab2f3 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/services/SuggestionsSchedulerService.kt +++ b/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( 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, diff --git a/app/src/main/java/com/github/damontecres/wholphin/services/SuggestionsWorker.kt b/app/src/main/java/com/github/damontecres/wholphin/services/SuggestionsWorker.kt index eab87f20..4560f2d9 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/services/SuggestionsWorker.kt +++ b/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) diff --git a/app/src/test/java/com/github/damontecres/wholphin/services/SuggestionsSchedulerServiceTest.kt b/app/src/test/java/com/github/damontecres/wholphin/services/SuggestionsSchedulerServiceTest.kt index 7da1d63c..0b48a011 100644 --- a/app/src/test/java/com/github/damontecres/wholphin/services/SuggestionsSchedulerServiceTest.kt +++ b/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() private val mockActivity = mockk(relaxed = true) private val mockServerRepository = mockk(relaxed = true) - private val mockCache = mockk(relaxed = true) private val mockWorkManager = mockk(relaxed = true) private val lifecycleRegistry = LifecycleRegistry(mockk(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) { + @Suppress("UNCHECKED_CAST") + val future = mockk>>() + 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() 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() 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() + 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()) } + } } diff --git a/app/src/test/java/com/github/damontecres/wholphin/services/SuggestionsWorkerTest.kt b/app/src/test/java/com/github/damontecres/wholphin/services/SuggestionsWorkerTest.kt index 7d633d87..5679777e 100644 --- a/app/src/test/java/com/github/damontecres/wholphin/services/SuggestionsWorkerTest.kt +++ b/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