Tolerate unknown RT labels, expire cached ratings, avoid duplicate collectors and Jellyfin flash

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

@@ -15,6 +15,7 @@ import kotlinx.coroutines.flow.flowOf
 import kotlinx.coroutines.flow.mapLatest
 import org.jellyfin.sdk.model.api.BaseItemKind
 import timber.log.Timber
+import java.util.concurrent.TimeUnit
 import javax.inject.Inject
 import javax.inject.Singleton
 
@@ -50,6 +51,7 @@ class SeerrRatingsService
             CacheBuilder
                 .newBuilder()
                 .maximumSize(CACHE_SIZE)
+                .expireAfterWrite(CACHE_HOURS, TimeUnit.HOURS)
                 .build<Key, SourceRatings>()
 
         fun cached(
@@ -57,6 +59,26 @@ class SeerrRatingsService
             tmdbId: Int,
         ): SourceRatings? = cache.getIfPresent(Key(type, tmdbId))
 
+        private fun target(item: BaseItem): Pair<SeerrItemType, Int>? {
+            val type =
+                when (item.type) {
+                    BaseItemKind.MOVIE -> SeerrItemType.MOVIE
+                    BaseItemKind.SERIES -> SeerrItemType.TV
+                    else -> return null
+                }
+            val tmdbId =
+                item.data.providerIds
+                    ?.get("Tmdb")
+                    ?.toIntOrNull() ?: return null
+            return type to tmdbId
+        }
+
+        /**
+         * The already cached ratings for a Jellyfin movie or series, used to seed a screen's initial state so it
+         * doesn't show the Jellyfin values first
+         */
+        fun cachedFor(item: BaseItem): SourceRatings? = target(item)?.let { (type, id) -> cached(type, id) }
+
         /**
          * Ratings for a Jellyfin movie or series using its TMDB id. Follows whether Seerr is active, since the
          * connection is established asynchronously after login.
@@ -65,24 +87,11 @@ class SeerrRatingsService
          * Callers should keep showing the Jellyfin values for null.
          */
         fun forItem(item: BaseItem): Flow<SourceRatings?> {
-            val type =
-                when (item.type) {
-                    BaseItemKind.MOVIE -> SeerrItemType.MOVIE
-                    BaseItemKind.SERIES -> SeerrItemType.TV
-                    else -> null
-                }
-            val tmdbId =
-                item.data.providerIds
-                    ?.get("Tmdb")
-                    ?.toIntOrNull()
-            return if (type == null || tmdbId == null) {
-                flowOf(null)
-            } else {
-                seerrService.active
+            val (type, tmdbId) = target(item) ?: return flowOf(null)
+            return seerrService.active
                 .distinctUntilChanged()
                 .mapLatest { active -> if (active) get(type, tmdbId) else null }
         }
-        }
 
         /**
          * @param knownTmdb if the caller already has the TMDB vote data (ie from the details response), avoids another request
@@ -176,9 +185,9 @@ class SeerrRatingsService
                     val rt = combined.response
                     SourceRatings.of(
                         rtCriticsScore = rt.criticsScore,
-                        rtCriticsLabel = rt.criticsRating?.value,
+                        rtCriticsLabel = rt.criticsRating,
                         rtAudienceScore = rt.audienceScore,
-                        rtAudienceLabel = rt.audienceRating?.value,
+                        rtAudienceLabel = rt.audienceRating,
                         tmdbScore = tmdb?.average,
                         tmdbVotes = tmdb?.count,
                     )
@@ -195,9 +204,9 @@ class SeerrRatingsService
         ): SourceRatings =
             SourceRatings.of(
                 rtCriticsScore = response.rt?.criticsScore,
-                rtCriticsLabel = response.rt?.criticsRating?.value,
+                rtCriticsLabel = response.rt?.criticsRating,
                 rtAudienceScore = response.rt?.audienceScore,
-                rtAudienceLabel = response.rt?.audienceRating?.value,
+                rtAudienceLabel = response.rt?.audienceRating,
                 imdbScore = response.imdb?.criticsScore,
                 imdbVotes = response.imdb?.criticsScoreCount,
                 tmdbScore = tmdb?.average,
@@ -218,5 +227,6 @@ class SeerrRatingsService
 
         companion object {
             private const val CACHE_SIZE = 300L
+            private const val CACHE_HOURS = 6L
         }
     }

app/src/main/java/com/github/damontecres/wholphin/ui/detail/movie/MovieViewModel.kt

@@ -103,6 +103,8 @@ class MovieViewModel
             return item
         }
 
+        private var ratingsJob: Job? = null
+
         fun init(): Job =
             viewModelScope.launchDefault {
                 val movie =
@@ -127,6 +129,7 @@ class MovieViewModel
                         chosenStreams = chosenStreams,
                         trailers = remoteTrailers,
                         chapters = chapters,
+                        sourceRatings = it.sourceRatings ?: ratingsService.cachedFor(movie),
                     )
                 }
                 backdropService.submit(movie)
@@ -155,9 +158,12 @@ class MovieViewModel
                         )
                     }
                 }
+                // init() runs again on resume and the flow never completes, so don't stack collectors
+                ratingsJob?.cancel()
+                ratingsJob =
                     viewModelScope.launchIO {
                         ratingsService.forItem(movie).collect { ratings ->
-                        _state.update { it.copy(sourceRatings = ratings) }
+                            _state.update { it.copy(sourceRatings = ratings ?: it.sourceRatings) }
                         }
                     }
                 viewModelScope.launchIO {

app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesViewModel.kt

@@ -221,6 +221,7 @@ class SeriesViewModel
                         seasons = seasons,
                         episodes = episodes,
                         trailers = remoteTrailers,
+                        sourceRatings = ratingsService.cachedFor(series),
                     )
                 }
 
@@ -256,7 +257,7 @@ class SeriesViewModel
                     }
                     viewModelScope.launchIO {
                         ratingsService.forItem(series).collect { ratings ->
-                            _state.update { it.copy(sourceRatings = ratings) }
+                            _state.update { it.copy(sourceRatings = ratings ?: it.sourceRatings) }
                         }
                     }
                     viewModelScope.launchIO {

app/src/main/seerr/seerr-api.yml

@@ -6528,13 +6528,13 @@ paths:
                     example: 85
                   criticsRating:
                     type: string
-                    enum: ['Rotten', 'Fresh', 'Certified Fresh']
+                    example: 'Certified Fresh'
                   audienceScore:
                     type: integer
                     example: 65
                   audienceRating:
                     type: string
-                    enum: ['Spilled', 'Upright']
+                    example: 'Upright'
   /movie/{movieId}/ratingscombined:
     get:
       summary: Get RT and IMDB movie ratings combined
@@ -6573,13 +6573,13 @@ paths:
                         example: 85
                       criticsRating:
                         type: string
-                        enum: ['Rotten', 'Fresh', 'Certified Fresh']
+                        example: 'Certified Fresh'
                       audienceScore:
                         type: integer
                         example: 65
                       audienceRating:
                         type: string
-                        enum: ['Spilled', 'Upright']
+                        example: 'Upright'
                   imdb:
                     type: object
                     properties:
@@ -6777,13 +6777,13 @@ paths:
                     example: 85
                   criticsRating:
                     type: string
-                    enum: ['Rotten', 'Fresh', 'Certified Fresh']
+                    example: 'Certified Fresh'
                   audienceScore:
                     type: integer
                     example: 65
                   audienceRating:
                     type: string
-                    enum: ['Spilled', 'Upright']
+                    example: 'Upright'
   /person/{personId}:
     get:
       summary: Get person details

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

@@ -4,6 +4,7 @@ import com.github.damontecres.wholphin.api.seerr.MoviesApi
 import com.github.damontecres.wholphin.api.seerr.SeerrApiClient
 import com.github.damontecres.wholphin.api.seerr.TvApi
 import com.github.damontecres.wholphin.api.seerr.infrastructure.ClientException
+import com.github.damontecres.wholphin.api.seerr.infrastructure.Serializer
 import com.github.damontecres.wholphin.api.seerr.model.MovieDetails
 import com.github.damontecres.wholphin.api.seerr.model.MovieMovieIdRatingsGet200Response
 import com.github.damontecres.wholphin.api.seerr.model.MovieMovieIdRatingscombinedGet200Response
@@ -67,9 +68,9 @@ class SeerrRatingsServiceTest {
             rt =
                 MovieMovieIdRatingsGet200Response(
                     criticsScore = 96,
-                    criticsRating = MovieMovieIdRatingsGet200Response.CriticsRating.CERTIFIED_FRESH,
+                    criticsRating = "Certified Fresh",
                     audienceScore = 90,
-                    audienceRating = MovieMovieIdRatingsGet200Response.AudienceRating.UPRIGHT,
+                    audienceRating = "Upright",
                 ),
             imdb = MovieMovieIdRatingscombinedGet200ResponseImdb(criticsScore = 8.8.toBigDecimal(), criticsScoreCount = 2_500_000),
         )
@@ -89,15 +90,42 @@ class SeerrRatingsServiceTest {
             coVerify(exactly = 0) { moviesApi.movieMovieIdGet(any()) }
         }
 
+    @Test
+    fun unknownRtLabelsDoNotDropTheResponse() =
+        runTest {
+            val json = Serializer.kotlinxSerializationJson
+            // Labels Seerr might add later must not fail parsing; the scores still show
+            coEvery { moviesApi.movieMovieIdRatingscombinedGet(movieId = 30) } returns
+                json.decodeFromString<MovieMovieIdRatingscombinedGet200Response>(
+                    """{"rt":{"criticsScore":92,"criticsRating":"Mystery","audienceScore":80,"audienceRating":"Whatever"},""" +
+                        """"imdb":{"criticsScore":8.8,"criticsScoreCount":2500000}}""",
+                )
+            coEvery { tvApi.tvTvIdRatingsGet(tvId = 31) } returns
+                json.decodeFromString<TvTvIdRatingsGet200Response>(
+                    """{"criticsScore":70,"criticsRating":"Brand New","audienceScore":60,"audienceRating":"Other"}""",
+                )
+
+            val movie = service.get(SeerrItemType.MOVIE, 30)!!
+            assertEquals(92, movie.rtCritics?.percent)
+            assertEquals(RtMark.UNKNOWN, movie.rtCritics?.mark)
+            assertEquals(RtMark.UPRIGHT, movie.rtAudience?.mark)
+            assertEquals(8.8, movie.imdb!!, 0.0)
+
+            val tv = service.get(SeerrItemType.TV, 31)!!
+            assertEquals(70, tv.rtCritics?.percent)
+            assertEquals(RtMark.UNKNOWN, tv.rtCritics?.mark)
+            assertEquals(60, tv.rtAudience?.percent)
+        }
+
     @Test
     fun seriesUsesTvRatingsIncludingAudienceAndHasNoImdb() =
         runTest {
             coEvery { tvApi.tvTvIdRatingsGet(tvId = 20) } returns
                 TvTvIdRatingsGet200Response(
                     criticsScore = 70,
-                    criticsRating = TvTvIdRatingsGet200Response.CriticsRating.FRESH,
+                    criticsRating = "Fresh",
                     audienceScore = 30,
-                    audienceRating = TvTvIdRatingsGet200Response.AudienceRating.SPILLED,
+                    audienceRating = "Spilled",
                 )
 
             val ratings = service.get(SeerrItemType.TV, 20)!!
@@ -137,7 +165,7 @@ class SeerrRatingsServiceTest {
         runTest {
             coEvery { moviesApi.movieMovieIdRatingscombinedGet(movieId = 603) } returns fullMovieRatings
             coEvery { tvApi.tvTvIdRatingsGet(tvId = 603) } returns
-                TvTvIdRatingsGet200Response(criticsScore = 12, criticsRating = TvTvIdRatingsGet200Response.CriticsRating.ROTTEN)
+                TvTvIdRatingsGet200Response(criticsScore = 12, criticsRating = "Rotten")
 
             val movie = service.get(SeerrItemType.MOVIE, 603)!!
             val tv = service.get(SeerrItemType.TV, 603)!!
@@ -205,9 +233,15 @@ class SeerrRatingsServiceTest {
         runTest {
             coEvery { moviesApi.movieMovieIdRatingscombinedGet(movieId = 42) } returns fullMovieRatings
 
-            val ratings = service.forItem(jellyfinItem(BaseItemKind.MOVIE, mapOf("Tmdb" to "42"))).first()
+            val item = jellyfinItem(BaseItemKind.MOVIE, mapOf("Tmdb" to "42"))
+            assertNull(service.cachedFor(item))
+
+            val ratings = service.forItem(item).first()
 
             assertEquals(96, ratings?.rtCritics?.percent)
+            // a later screen can start from the cached value instead of the Jellyfin values
+            assertEquals(ratings, service.cachedFor(item))
+            assertNull(service.cachedFor(jellyfinItem(BaseItemKind.SERIES, mapOf("Tmdb" to "42"))))
         }
 
     @Test