From 621686d8e2785b0944f3dc730f84f8c0fa7d7356 Mon Sep 17 00:00:00 2001 From: Jake <45441845+4eh5xitv6787h645ebv@users.noreply.github.com> Date: Fri, 16 Oct 2026 23:28:01 +0800 Subject: [PATCH] Include image tags in item image URLs (#2031) ## Description Wholphin builds item image URLs without Jellyfin's `tag` query parameter. `ImageUrlService` already receives the item's `imageTags` and `backdropTags`, and the low-level `getItemImageUrl` supports `tag`, but no branch passed it. Today only chapter and now-playing images set it. Before: `/Items/{id}/Images/Primary?quality=96&fillHeight=229` After: `/Items/{id}/Images/Primary?quality=96&fillHeight=229&tag={ImageTags.Primary}` This is how the official Jellyfin clients request images. jellyfin-web and the official Jellyfin Android TV app both copy `ImageTags[...]` (and the parent/series tags) into the image URL. I also added `ImageUrlServiceTest`, which covers each branch. ### Related issues I tested each one on the emulator against a stock Jellyfin 12.1 server (no plugins), comparing `main` with this branch: - **#366 (Image cache expiration): fixed by this PR.** - **#1249 (blank tile for a newly added movie) and #1338 (episode cover never refreshes): already fixed on `main` since v1.0.3, not by this PR.** - **#360 (Option to hide spoilers): partly.** ## AI or LLM usage This change was made with AI assistance (Claude Code). It was used for: - the code changes in `ImageUrlService` and the call sites listed above - the new `ImageUrlServiceTest` - the emulator testing and benchmarks - drafting this description The testing is what's described above: unit tests, an Android TV emulator against Jellyfin 12.1 servers (with Jellyfin Enhanced and stock), request logs from a proxy, and a check of every tag against the server's metadata. I've reviewed the change and can explain it. I'm also a maintainer on Jellyfin Enhanced, which is how I ran into this. I'm happy to share examples of my own coding work if that helps with reviewing this. --- .../damontecres/wholphin/data/model/Person.kt | 4 +- .../wholphin/services/HomeSettingsService.kt | 1 + .../wholphin/services/ImageUrlService.kt | 24 ++ .../wholphin/services/MusicService.kt | 1 + .../wholphin/ui/components/StudioCardGrid.kt | 1 + .../wholphin/ui/components/TitleOrLogo.kt | 6 +- .../wholphin/ui/detail/PersonPage.kt | 1 + .../ui/detail/livetv/LiveTvViewModel.kt | 7 +- .../wholphin/services/ImageUrlServiceTest.kt | 207 ++++++++++++++++++ 9 files changed, 248 insertions(+), 4 deletions(-) create mode 100644 app/src/test/java/com/github/damontecres/wholphin/services/ImageUrlServiceTest.kt diff --git a/app/src/main/java/com/github/damontecres/wholphin/data/model/Person.kt b/app/src/main/java/com/github/damontecres/wholphin/data/model/Person.kt index 4e639009..a20a2dcd 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/data/model/Person.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/data/model/Person.kt @@ -35,7 +35,7 @@ data class Person( name = dto.name, role = personRole(context, dto.role, dto.type), type = dto.type, - imageUrl = api.imageApi.getItemImageUrl(dto.id, ImageType.PRIMARY), + imageUrl = api.imageApi.getItemImageUrl(dto.id, ImageType.PRIMARY, tag = dto.primaryImageTag), favorite = false, ) @@ -50,7 +50,7 @@ data class Person( name = dto.name, role = personRole(context, dto.role, dto.type), type = dto.type, - imageUrl = api.imageApi.getItemImageUrl(dto.id, ImageType.PRIMARY), + imageUrl = api.imageApi.getItemImageUrl(dto.id, ImageType.PRIMARY, tag = dto.primaryImageTag), favorite = favorite, ) } diff --git a/app/src/main/java/com/github/damontecres/wholphin/services/HomeSettingsService.kt b/app/src/main/java/com/github/damontecres/wholphin/services/HomeSettingsService.kt index b5dd3b7f..7145f690 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/services/HomeSettingsService.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/services/HomeSettingsService.kt @@ -753,6 +753,7 @@ class HomeSettingsService imageUrlService.getItemImageUrl( itemId = it.id, imageType = ImageType.THUMB, + tag = it.imageTags?.get(ImageType.THUMB), ) BaseItem( it, diff --git a/app/src/main/java/com/github/damontecres/wholphin/services/ImageUrlService.kt b/app/src/main/java/com/github/damontecres/wholphin/services/ImageUrlService.kt index 6b71d683..967629da 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/services/ImageUrlService.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/services/ImageUrlService.kt @@ -35,6 +35,11 @@ class ImageUrlService parentBackdropId: UUID? = null, fillWidth: Int? = null, fillHeight: Int? = null, + seriesPrimaryTag: String? = null, + parentLogoId: UUID? = null, + parentLogoTag: String? = null, + parentThumbTag: String? = null, + parentBackdropTags: List = emptyList(), ): String? = when (imageType) { ImageType.LOGO -> { @@ -44,6 +49,7 @@ class ImageUrlService imageType = imageType, fillWidth = fillWidth, fillHeight = fillHeight, + tag = parentLogoTag.takeIf { parentLogoId == seriesId }, ) } else { getItemImageUrl( @@ -51,6 +57,7 @@ class ImageUrlService imageType = imageType, fillWidth = fillWidth, fillHeight = fillHeight, + tag = imageTags[imageType], ) } } @@ -63,6 +70,7 @@ class ImageUrlService imageType = imageType, fillWidth = fillWidth, fillHeight = fillHeight, + tag = parentBackdropTags.firstOrNull().takeIf { parentBackdropId == seriesId }, ) } else if (backdropTags.isNotEmpty()) { getItemImageUrl( @@ -70,6 +78,7 @@ class ImageUrlService imageType = imageType, fillWidth = fillWidth, fillHeight = fillHeight, + tag = backdropTags.first(), ) } else { null @@ -86,6 +95,7 @@ class ImageUrlService imageType = imageType, fillWidth = fillWidth, fillHeight = fillHeight, + tag = parentThumbTag, ) } else if (useSeriesForPrimary && parentBackdropId != null && (itemType == BaseItemKind.EPISODE || itemType == BaseItemKind.SEASON) @@ -96,6 +106,7 @@ class ImageUrlService imageType = ImageType.BACKDROP, fillWidth = fillWidth, fillHeight = fillHeight, + tag = parentBackdropTags.firstOrNull(), ) } else if (parentThumbId != null && itemType == BaseItemKind.SEASON && imageType !in imageTags) { getItemImageUrl( @@ -103,6 +114,7 @@ class ImageUrlService imageType = imageType, fillWidth = fillWidth, fillHeight = fillHeight, + tag = parentThumbTag, ) } else if (useSeriesForPrimary && parentThumbId == null && @@ -115,6 +127,7 @@ class ImageUrlService imageType = ImageType.PRIMARY, fillWidth = fillWidth, fillHeight = fillHeight, + tag = imageTags[ImageType.PRIMARY], ) } else if (imageType !in imageTags && backdropTags.isNotEmpty()) { // If no thumb, use backdrop if available @@ -123,6 +136,7 @@ class ImageUrlService imageType = ImageType.BACKDROP, fillWidth = fillWidth, fillHeight = fillHeight, + tag = backdropTags.first(), ) } else { getItemImageUrl( @@ -130,6 +144,7 @@ class ImageUrlService imageType = imageType, fillWidth = fillWidth, fillHeight = fillHeight, + tag = imageTags[imageType], ) } } @@ -145,6 +160,7 @@ class ImageUrlService imageType = imageType, fillWidth = fillWidth, fillHeight = fillHeight, + tag = seriesPrimaryTag.takeIf { imageType == ImageType.PRIMARY }, ) } else if (seriesId != null && itemType == BaseItemKind.SEASON && imageType !in imageTags) { getItemImageUrl( @@ -152,6 +168,7 @@ class ImageUrlService imageType = imageType, fillWidth = fillWidth, fillHeight = fillHeight, + tag = seriesPrimaryTag.takeIf { imageType == ImageType.PRIMARY }, ) } else { getItemImageUrl( @@ -159,6 +176,7 @@ class ImageUrlService imageType = imageType, fillWidth = fillWidth, fillHeight = fillHeight, + tag = imageTags[imageType], ) } } @@ -169,6 +187,7 @@ class ImageUrlService imageType = imageType, fillWidth = fillWidth, fillHeight = fillHeight, + tag = imageTags[imageType], ) } } @@ -193,6 +212,11 @@ class ImageUrlService backdropTags = item.data.backdropImageTags.orEmpty(), fillWidth = fillWidth, fillHeight = fillHeight, + seriesPrimaryTag = item.data.seriesPrimaryImageTag, + parentLogoId = item.data.parentLogoItemId, + parentLogoTag = item.data.parentLogoImageTag, + parentThumbTag = item.data.parentThumbImageTag, + parentBackdropTags = item.data.parentBackdropImageTags.orEmpty(), ) } else { null diff --git a/app/src/main/java/com/github/damontecres/wholphin/services/MusicService.kt b/app/src/main/java/com/github/damontecres/wholphin/services/MusicService.kt index ec6e9d2c..4e38ffcc 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/services/MusicService.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/services/MusicService.kt @@ -327,6 +327,7 @@ class MusicService imageUrlService.getItemImageUrl( itemId = albumId, imageType = ImageType.PRIMARY, + tag = audio.data.albumPrimaryImageTag, ) } return MediaItem diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/components/StudioCardGrid.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/components/StudioCardGrid.kt index 9ff0e2c0..b69e1126 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/components/StudioCardGrid.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/components/StudioCardGrid.kt @@ -97,6 +97,7 @@ class StudioViewModel itemId = it.id, imageType = ImageType.THUMB, fillWidth = cardWidthPx, + tag = it.imageTags?.get(ImageType.THUMB), ) Studio( id = it.id, diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/components/TitleOrLogo.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/components/TitleOrLogo.kt index c5da7565..4268f2d9 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/components/TitleOrLogo.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/components/TitleOrLogo.kt @@ -95,7 +95,11 @@ fun rememberLogoUrl(item: BaseItem?): String? { item.data.seriesId != null && item.data.parentLogoImageTag != null ) { - imageUrlService.getItemImageUrl(item.data.seriesId!!, ImageType.LOGO) + imageUrlService.getItemImageUrl( + itemId = item.data.seriesId!!, + imageType = ImageType.LOGO, + tag = item.data.parentLogoImageTag.takeIf { item.data.parentLogoItemId == item.data.seriesId }, + ) } else if (ImageType.LOGO in item?.data?.imageTags.orEmpty()) { imageUrlService.getItemImageUrl(item, ImageType.LOGO) } else { diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/PersonPage.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/PersonPage.kt index 27212b5e..64c48f0e 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/PersonPage.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/PersonPage.kt @@ -257,6 +257,7 @@ fun PersonPage( imageUrlService.getItemImageUrl( itemId = person.id, imageType = ImageType.PRIMARY, + tag = person.data.imageTags?.get(ImageType.PRIMARY), ) } // A long filmography is hard to navigate as a row, so it is cut at the same diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/livetv/LiveTvViewModel.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/livetv/LiveTvViewModel.kt index 470f33c2..bcba145c 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/livetv/LiveTvViewModel.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/livetv/LiveTvViewModel.kt @@ -149,7 +149,11 @@ class LiveTvViewModel number = it.channelNumber, name = it.channelName ?: it.name, imageUrl = - imageUrlService.getItemImageUrl(it.id, ImageType.PRIMARY), + imageUrlService.getItemImageUrl( + itemId = it.id, + imageType = ImageType.PRIMARY, + tag = it.imageTags?.get(ImageType.PRIMARY), + ), favorite = it.userData?.isFavorite == true, ) } @@ -299,6 +303,7 @@ class LiveTvViewModel imageUrlService.getItemImageUrl( dto.id, ImageType.PRIMARY, + tag = dto.imageTags?.get(ImageType.PRIMARY), ), ) if (index == 0) { diff --git a/app/src/test/java/com/github/damontecres/wholphin/services/ImageUrlServiceTest.kt b/app/src/test/java/com/github/damontecres/wholphin/services/ImageUrlServiceTest.kt new file mode 100644 index 00000000..d179c7e1 --- /dev/null +++ b/app/src/test/java/com/github/damontecres/wholphin/services/ImageUrlServiceTest.kt @@ -0,0 +1,207 @@ +package com.github.damontecres.wholphin.services + +import com.github.damontecres.wholphin.data.model.BaseItem +import io.mockk.every +import io.mockk.mockk +import org.jellyfin.sdk.api.client.ApiClient +import org.jellyfin.sdk.api.client.extensions.imageApi +import org.jellyfin.sdk.api.operations.ImageApi +import org.jellyfin.sdk.model.UUID +import org.jellyfin.sdk.model.api.BaseItemDto +import org.jellyfin.sdk.model.api.BaseItemKind +import org.jellyfin.sdk.model.api.ImageType +import org.jellyfin.sdk.model.serializer.toUUID +import org.junit.Assert +import org.junit.Before +import org.junit.Test + +class ImageUrlServiceTest { + private val mockApiClient = mockk() + private val imageUrlService = ImageUrlService(mockApiClient) + + private val itemId = UUID.randomUUID() + private val seriesId = UUID.randomUUID() + private val seasonId = UUID.randomUUID() + + @Before + fun setup() { + every { mockApiClient.baseUrl } returns "http://localhost" + every { mockApiClient.createUrl(any(), any(), any(), any()) } answers { callOriginal() } + every { mockApiClient.imageApi } returns ImageApi(mockApiClient) + } + + private fun episode( + imageTags: Map = emptyMap(), + parentLogoItemId: UUID? = null, + parentThumbItemId: UUID? = null, + parentBackdropItemId: UUID? = null, + ) = BaseItemDto( + id = itemId, + type = BaseItemKind.EPISODE, + seriesId = seriesId, + seasonId = seasonId, + imageTags = imageTags, + seriesPrimaryImageTag = "seriesPrimary", + parentLogoItemId = parentLogoItemId, + parentLogoImageTag = parentLogoItemId?.let { "parentLogo" }, + parentThumbItemId = parentThumbItemId, + parentThumbImageTag = parentThumbItemId?.let { "parentThumb" }, + parentBackdropItemId = parentBackdropItemId, + parentBackdropImageTags = parentBackdropItemId?.let { listOf("parentBackdrop0", "parentBackdrop1") }, + ) + + private fun movie( + imageTags: Map = emptyMap(), + backdropImageTags: List? = null, + ) = BaseItemDto( + id = itemId, + type = BaseItemKind.MOVIE, + imageTags = imageTags, + backdropImageTags = backdropImageTags, + ) + + private fun assertImage( + url: String?, + expectedId: UUID, + expectedType: ImageType, + expectedTag: String?, + ) { + Assert.assertNotNull(url) + val path = url!!.substringBefore('?') + Assert.assertEquals(expectedId, path.substringAfter("/Items/").substringBefore('/').toUUID()) + Assert.assertEquals(expectedType.serialName, path.substringAfter("/Images/")) + val tag = + url + .substringAfter('?', "") + .split('&') + .firstOrNull { it.startsWith("tag=") } + ?.substringAfter("tag=") + Assert.assertEquals(expectedTag, tag) + } + + @Test + fun `Own images use own tags`() { + val item = + BaseItem( + movie( + imageTags = + mapOf( + ImageType.PRIMARY to "primary", + ImageType.LOGO to "logo", + ImageType.THUMB to "thumb", + ImageType.BANNER to "banner", + ImageType.ART to "art", + ), + backdropImageTags = listOf("backdrop0", "backdrop1"), + ), + ) + assertImage(imageUrlService.getItemImageUrl(item, ImageType.PRIMARY), itemId, ImageType.PRIMARY, "primary") + assertImage(imageUrlService.getItemImageUrl(item, ImageType.LOGO), itemId, ImageType.LOGO, "logo") + assertImage(imageUrlService.getItemImageUrl(item, ImageType.THUMB), itemId, ImageType.THUMB, "thumb") + assertImage(imageUrlService.getItemImageUrl(item, ImageType.BANNER), itemId, ImageType.BANNER, "banner") + assertImage(imageUrlService.getItemImageUrl(item, ImageType.ART), itemId, ImageType.ART, "art") + assertImage(imageUrlService.getItemImageUrl(item, ImageType.BACKDROP), itemId, ImageType.BACKDROP, "backdrop0") + } + + @Test + fun `Missing tags are omitted`() { + val item = BaseItem(movie()) + assertImage(imageUrlService.getItemImageUrl(item, ImageType.PRIMARY), itemId, ImageType.PRIMARY, null) + assertImage(imageUrlService.getItemImageUrl(item, ImageType.LOGO), itemId, ImageType.LOGO, null) + Assert.assertNull(imageUrlService.getItemImageUrl(item, ImageType.BACKDROP)) + } + + @Test + fun `Thumb falls back to own backdrop`() { + val item = BaseItem(movie(backdropImageTags = listOf("backdrop0"))) + assertImage(imageUrlService.getItemImageUrl(item, ImageType.THUMB), itemId, ImageType.BACKDROP, "backdrop0") + } + + @Test + fun `Episode primary uses series tag`() { + val item = BaseItem(episode(imageTags = mapOf(ImageType.PRIMARY to "primary")), useSeriesForPrimary = true) + assertImage(imageUrlService.getItemImageUrl(item, ImageType.PRIMARY), seriesId, ImageType.PRIMARY, "seriesPrimary") + assertImage(imageUrlService.getItemImageUrl(item, ImageType.BANNER), seriesId, ImageType.BANNER, null) + assertImage( + imageUrlService.getItemImageUrl(item, ImageType.PRIMARY, useSeriesForPrimary = false), + itemId, + ImageType.PRIMARY, + "primary", + ) + } + + @Test + fun `Season without primary uses series tag`() { + val item = BaseItem(episode().copy(id = seasonId, type = BaseItemKind.SEASON, seasonId = null)) + assertImage(imageUrlService.getItemImageUrl(item, ImageType.PRIMARY), seriesId, ImageType.PRIMARY, "seriesPrimary") + } + + @Test + fun `Episode logo uses parent logo tag only if from series`() { + assertImage( + imageUrlService.getItemImageUrl(BaseItem(episode(parentLogoItemId = seriesId)), ImageType.LOGO), + seriesId, + ImageType.LOGO, + "parentLogo", + ) + assertImage( + imageUrlService.getItemImageUrl(BaseItem(episode(parentLogoItemId = seasonId)), ImageType.LOGO), + seriesId, + ImageType.LOGO, + null, + ) + } + + @Test + fun `Episode backdrop uses parent backdrop tag only if from series`() { + assertImage( + imageUrlService.getItemImageUrl(BaseItem(episode(parentBackdropItemId = seriesId)), ImageType.BACKDROP), + seriesId, + ImageType.BACKDROP, + "parentBackdrop0", + ) + assertImage( + imageUrlService.getItemImageUrl(BaseItem(episode(parentBackdropItemId = seasonId)), ImageType.BACKDROP), + seriesId, + ImageType.BACKDROP, + null, + ) + } + + @Test + fun `Episode thumb uses parent tags`() { + assertImage( + imageUrlService.getItemImageUrl( + BaseItem(episode(parentThumbItemId = seriesId, parentBackdropItemId = seriesId), useSeriesForPrimary = true), + ImageType.THUMB, + ), + seriesId, + ImageType.THUMB, + "parentThumb", + ) + assertImage( + imageUrlService.getItemImageUrl( + BaseItem(episode(parentBackdropItemId = seriesId), useSeriesForPrimary = true), + ImageType.THUMB, + ), + seriesId, + ImageType.BACKDROP, + "parentBackdrop0", + ) + assertImage( + imageUrlService.getItemImageUrl( + BaseItem(episode(imageTags = mapOf(ImageType.PRIMARY to "primary")), useSeriesForPrimary = true), + ImageType.THUMB, + ), + itemId, + ImageType.PRIMARY, + "primary", + ) + } + + @Test + fun `Season without thumb uses parent thumb tag`() { + val item = BaseItem(episode(parentThumbItemId = seriesId).copy(id = seasonId, type = BaseItemKind.SEASON)) + assertImage(imageUrlService.getItemImageUrl(item, ImageType.THUMB), seriesId, ImageType.THUMB, "parentThumb") + } +}