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.
This commit is contained in:
parent
3ef221bf91
commit
621686d8e2
9 changed files with 248 additions and 4 deletions
|
|
@ -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,
|
||||
)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -753,6 +753,7 @@ class HomeSettingsService
|
|||
imageUrlService.getItemImageUrl(
|
||||
itemId = it.id,
|
||||
imageType = ImageType.THUMB,
|
||||
tag = it.imageTags?.get(ImageType.THUMB),
|
||||
)
|
||||
BaseItem(
|
||||
it,
|
||||
|
|
|
|||
|
|
@ -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<String> = 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
|
||||
|
|
|
|||
|
|
@ -327,6 +327,7 @@ class MusicService
|
|||
imageUrlService.getItemImageUrl(
|
||||
itemId = albumId,
|
||||
imageType = ImageType.PRIMARY,
|
||||
tag = audio.data.albumPrimaryImageTag,
|
||||
)
|
||||
}
|
||||
return MediaItem
|
||||
|
|
|
|||
|
|
@ -97,6 +97,7 @@ class StudioViewModel
|
|||
itemId = it.id,
|
||||
imageType = ImageType.THUMB,
|
||||
fillWidth = cardWidthPx,
|
||||
tag = it.imageTags?.get(ImageType.THUMB),
|
||||
)
|
||||
Studio(
|
||||
id = it.id,
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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<ApiClient>()
|
||||
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<ImageType, String> = 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<ImageType, String> = emptyMap(),
|
||||
backdropImageTags: List<String>? = 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")
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue