diff --git a/app/src/main/kotlin/com/arflix/tv/data/api/TraktApi.kt b/app/src/main/kotlin/com/arflix/tv/data/api/TraktApi.kt index da242fd8d..44e937228 100644 --- a/app/src/main/kotlin/com/arflix/tv/data/api/TraktApi.kt +++ b/app/src/main/kotlin/com/arflix/tv/data/api/TraktApi.kt @@ -463,7 +463,9 @@ interface TraktApi { @Path("type") type: String, @Query("extended") extended: String = "full", @Query("page") page: Int = 1, - @Query("limit") limit: Int = 100 + @Query("limit") limit: Int = 100, + /** See listFreshnessMarker: keeps a day-long cache lifetime from hiding list edits. */ + @Query("_") freshness: Long? = null ): List @GET("lists/{listId}") @@ -481,7 +483,9 @@ interface TraktApi { @Path("type") type: String, @Query("extended") extended: String = "full", @Query("page") page: Int = 1, - @Query("limit") limit: Int = 100 + @Query("limit") limit: Int = 100, + /** See listFreshnessMarker: keeps a day-long cache lifetime from hiding list edits. */ + @Query("_") freshness: Long? = null ): List // ========== User Profile ========== diff --git a/app/src/main/kotlin/com/arflix/tv/data/repository/ListFreshness.kt b/app/src/main/kotlin/com/arflix/tv/data/repository/ListFreshness.kt new file mode 100644 index 000000000..4e361b198 --- /dev/null +++ b/app/src/main/kotlin/com/arflix/tv/data/repository/ListFreshness.kt @@ -0,0 +1,19 @@ +package com.arflix.tv.data.repository + +/** + * Lists a user edits themselves (Trakt and MDBList lists) are served with long cache lifetimes: + * Trakt allows a client to keep a list's items for a day, and MDBList's servers keep handing out + * the same copy for about as long. Followed as-is, an edit took up to a day to reach a home row. + * + * Requests for those lists carry a marker that changes every [LIST_REFRESH_MS]. A new marker is a + * new address, so neither the app's HTTP cache nor the service's own cache can answer it with an + * older copy, while loads within one window still share a single cached response. + */ +internal const val LIST_REFRESH_MS = 30 * 60 * 1000L + +internal fun listFreshnessMarker(nowMs: Long = System.currentTimeMillis()): Long = nowMs / LIST_REFRESH_MS + +internal fun withListFreshness(url: String, nowMs: Long = System.currentTimeMillis()): String { + val separator = if ('?' in url) '&' else '?' + return "$url${separator}_=${listFreshnessMarker(nowMs)}" +} diff --git a/app/src/main/kotlin/com/arflix/tv/data/repository/MediaRepository.kt b/app/src/main/kotlin/com/arflix/tv/data/repository/MediaRepository.kt index dfecc7e71..c59a498c0 100644 --- a/app/src/main/kotlin/com/arflix/tv/data/repository/MediaRepository.kt +++ b/app/src/main/kotlin/com/arflix/tv/data/repository/MediaRepository.kt @@ -1427,7 +1427,7 @@ class MediaRepository @Inject constructor( val slug = source.mdblistSlug?.trim()?.trim('/').orEmpty() if (slug.isBlank()) return emptyList() val body = withContext(Dispatchers.IO) { - fetchUrl("https://mdblist.com/lists/$slug/json") + fetchUrl(withListFreshness("https://mdblist.com/lists/$slug/json")) } ?: throw java.io.IOException("Collection list could not be loaded") val array = org.json.JSONArray(body) val refs = mutableListOf>() @@ -3316,10 +3316,12 @@ class MediaRepository @Inject constructor( val rows = when (parsed) { is ParsedCatalogUrl.TraktUserList -> traktApi.getUserListItems( clientId = Constants.TRAKT_CLIENT_ID, username = parsed.username, - listId = parsed.listId, type = type, page = page, limit = 100) + listId = parsed.listId, type = type, page = page, limit = 100, + freshness = listFreshnessMarker()) is ParsedCatalogUrl.TraktList -> traktApi.getListItems( clientId = Constants.TRAKT_CLIENT_ID, listId = parsed.listId, - type = type, page = page, limit = 100) + type = type, page = page, limit = 100, + freshness = listFreshnessMarker()) else -> emptyList() } if (rows == previous) break @@ -3418,7 +3420,7 @@ class MediaRepository @Inject constructor( } val url = sourceUrl ?: return emptyList() - val jsonUrl = "${url.removeSuffix("/")}/json" + val jsonUrl = withListFreshness("${url.removeSuffix("/")}/json") val fromJson = fetchUrl(jsonUrl)?.let { payload -> parseMdblistJson(payload) } ?: emptyList() diff --git a/app/src/test/kotlin/com/arflix/tv/data/repository/ListFreshnessTest.kt b/app/src/test/kotlin/com/arflix/tv/data/repository/ListFreshnessTest.kt new file mode 100644 index 000000000..f2565617e --- /dev/null +++ b/app/src/test/kotlin/com/arflix/tv/data/repository/ListFreshnessTest.kt @@ -0,0 +1,90 @@ +package com.arflix.tv.data.repository + +import com.arflix.tv.data.api.TraktApi +import com.arflix.tv.data.api.TraktIds +import com.arflix.tv.data.api.TraktPublicListItem +import com.arflix.tv.data.api.TraktShowInfo +import com.arflix.tv.data.model.CatalogConfig +import com.arflix.tv.data.model.CatalogSourceType +import com.arflix.tv.data.model.MediaItem +import com.arflix.tv.data.model.MediaType +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.mockk +import io.mockk.spyk +import kotlinx.coroutines.test.runTest +import okhttp3.MediaType.Companion.toMediaType +import okhttp3.OkHttpClient +import okhttp3.Protocol +import okhttp3.Response +import okhttp3.ResponseBody.Companion.toResponseBody +import org.junit.Assert.* +import org.junit.Test +import java.util.concurrent.CopyOnWriteArrayList + +class ListFreshnessTest { + @Test fun `marker holds for one window and moves with the next`() { + val start = 7 * LIST_REFRESH_MS + assertEquals(listFreshnessMarker(start), listFreshnessMarker(start + LIST_REFRESH_MS - 1)) + assertEquals(listFreshnessMarker(start) + 1, listFreshnessMarker(start + LIST_REFRESH_MS)) + } + + @Test fun `marker is appended as its own query parameter`() { + val now = 3 * LIST_REFRESH_MS + assertEquals("https://example.test/list/json?_=3", withListFreshness("https://example.test/list/json", now)) + assertEquals("https://example.test/list/json?a=1&_=3", withListFreshness("https://example.test/list/json?a=1", now)) + } + + @Test fun `trakt list rows ask for the current window`() = runTest { + val trakt = mockk() + coEvery { trakt.getUserListItems(any(), any(), any(), any(), "movies", any(), any(), any(), any()) } returns emptyList() + coEvery { trakt.getUserListItems(any(), any(), any(), any(), "shows", any(), 1, any(), any()) } returns listOf( + TraktPublicListItem(type = "show", show = TraktShowInfo("Some Show", 2020, TraktIds(tmdb = 501))) + ) + coEvery { trakt.getUserListItems(any(), any(), any(), any(), "shows", any(), 2, any(), any()) } returns emptyList() + val catalog = CatalogConfig( + id = "custom_list", title = "List", sourceType = CatalogSourceType.TRAKT, + sourceUrl = "https://trakt.tv/users/someone/lists/some-list", + sourceRef = "trakt_user:someone:some-list" + ) + + val page = repository(trakt = trakt).loadCustomCatalogPage(catalog, 0, 20) + + assertEquals(listOf(501), page.items.map { it.id }) + coVerify { trakt.getUserListItems(any(), any(), "someone", "some-list", "shows", any(), 1, any(), match { it != null }) } + coVerify(exactly = 0) { trakt.getUserListItems(any(), any(), any(), any(), any(), any(), any(), any(), null) } + } + + @Test fun `mdblist rows ask for the current window`() = runTest { + val requested = CopyOnWriteArrayList() + val client = OkHttpClient.Builder().addInterceptor { chain -> + requested += chain.request().url.toString() + Response.Builder().request(chain.request()).protocol(Protocol.HTTP_1_1).code(200).message("OK") + .body("""[{"id":501,"mediatype":"show"}]""".toResponseBody("application/json".toMediaType())) + .build() + }.build() + val catalog = CatalogConfig( + id = "custom_md", title = "List", sourceType = CatalogSourceType.MDBLIST, + sourceUrl = "https://mdblist.com/lists/someone/some-list", + sourceRef = "mdblist:https://mdblist.com/lists/someone/some-list" + ) + + val page = repository(client = client).loadCustomCatalogPage(catalog, 0, 20) + + assertEquals(listOf(501), page.items.map { it.id }) + assertTrue(requested.toString(), requested.any { + it.matches(Regex("""https://mdblist\.com/lists/someone/some-list/json\?_=\d+""")) + }) + } + + private fun repository( + trakt: TraktApi = mockk(relaxed = true), + client: OkHttpClient = mockk(relaxed = true) + ) = spyk(MediaRepository( + mockk(relaxed = true), mockk(relaxed = true), mockk(relaxed = true), trakt, + client, mockk(relaxed = true), mockk(relaxed = true) + )).also { repository -> + coEvery { repository.getMovieDetails(any()) } answers { MediaItem(firstArg(), "Movie ${firstArg()}") } + coEvery { repository.getTvDetails(any()) } answers { MediaItem(firstArg(), "Series ${firstArg()}", mediaType = MediaType.TV) } + } +}