diff --git a/lib/services/jellyfin_client/parts/browse.dart b/lib/services/jellyfin_client/parts/browse.dart index cbcc10d7..7320ba30 100644 --- a/lib/services/jellyfin_client/parts/browse.dart +++ b/lib/services/jellyfin_client/parts/browse.dart @@ -121,11 +121,31 @@ const _queueFields = 'UserData,PremiereDate'; /// bounded while still returning the full series queue. const _episodeQueuePageSize = 200; -/// How many recently played episodes to scan when stamping `/Shows/NextUp` -/// rows with their series' last-watched date (see [_attachSeriesLastPlayed]). -/// Mirrors [_episodeQueuePageSize]; covers far more distinct series than the -/// Next Up list ever returns, while keeping the response bounded. -const _continueWatchingSeriesLookback = 200; +/// How many pending series [_attachSeriesLastPlayed] resolves at once. Each +/// lookup returns a single row, so the batch exists only to keep a long Next Up +/// shelf from opening one request per series at the same instant; measured +/// against a 12.0-rc3 server, 4 is where the wall time for 21 series stops +/// improving (0.65s at 3, 0.56s at both 4 and 6). +const _seriesLastPlayedConcurrency = 4; + +/// Ceiling on how many series [_attachSeriesLastPlayed] dates in one call. Sits +/// just above `DiscoverProvider`'s 21-row continue-watching probe so the home +/// shelf is always fully dated, and caps the uncapped `count: null` shelf, whose +/// Next Up half is limited only by how many series the user has started. +const _seriesLastPlayedLookupLimit = 24; + +/// Per-lookup budget for [_fetchSeriesLastPlayed]. A `ParentId`-scoped +/// `Limit=1` row answered in tens of milliseconds even on the pathological +/// 12.0-rc3 sort, so anything near this means the endpoint is in trouble and +/// the shelf is better off unstamped than waiting on the shared default. +const _seriesLastPlayedRequestTimeout = Duration(seconds: 3); + +/// Wall-clock ceiling on [_attachSeriesLastPlayed]'s sequential batches, checked +/// before each one. Bounds the whole pass at this plus one +/// [_seriesLastPlayedRequestTimeout] — still under the single default-budget +/// request this enrichment replaced, so a stalled endpoint cannot make the +/// scoped form slower than the unscoped one it fixes. +const _seriesLastPlayedBudget = Duration(seconds: 4); const _childrenPageSize = 500; const _pagedListPageSize = 200; @@ -1658,7 +1678,36 @@ mixin _JellyfinBrowseMethods on _JellyfinClientInternals { /// interleave Next Up with resume items by recency, stamp each Next Up episode /// with its series' last-watched date, read from the most recently played /// episode of that series. + /// + /// One `ParentId`-scoped lookup per pending series, not a single server-wide + /// DatePlayed scan. Jellyfin 12.0-rc3 builds that sort key by OR-ing an item's + /// own progress with its alternate versions' (`ItemId == e.Id || + /// Item.PrimaryVersionId == e.Id`, jellyfin/jellyfin#17044), which no index can + /// serve, so the user's entire UserData table is scanned per sorted row: an + /// unscoped episode sort measured 5.8s on a 6k-episode rc3 library against + /// 25ms on 10.10.7, pegging a core for its whole duration. That blew this + /// call's request budget and starved every other client of the server (#1699). + /// `ParentId` bounds the sort input to one series' episodes — 21 series resolve + /// in ~0.6s against the same rc3 server. Upstream fixed the order mapper after + /// rc3 (jellyfin/jellyfin#17422); scoping keeps the cost flat on servers that + /// still carry the regression. + /// + /// At most [_seriesLastPlayedLookupLimit] series are enriched. `/Shows/NextUp` + /// already returns series in last-played-descending order, so the cap keeps the + /// rows whose dates decide the top of the shelf while bounding total work for + /// an uncapped `count: null` shelf, which can carry far more series than the + /// home preview. Rows past the cap keep a null date and degrade to their + /// `addedAt` in the sort — the same degradation the previous 200-row lookback + /// window applied to a series whose last play fell outside it. + /// + /// The batches are sequential, so they also share a wall-clock budget: a stalled + /// endpoint must not let a best-effort enrichment serialise + /// [_seriesLastPlayedRequestTimeout] six times over. Checking + /// [_seriesLastPlayedBudget] before each batch caps the whole pass at budget + + /// one request timeout, below the single default-budget request it replaced. Future> _attachSeriesLastPlayed(List nextUp) async { + // Set literal over `nextUp` order: insertion-ordered, so `take` below keeps + // the most recently played series. final pendingSeriesIds = { for (final item in nextUp) if (item.kind == MediaKind.episode && item.lastViewedAt == null && item.grandparentId != null) @@ -1666,35 +1715,22 @@ mixin _JellyfinBrowseMethods on _JellyfinClientInternals { }; if (pendingSeriesIds.isEmpty) return nextUp; - // One lightweight pass over the most recently played episodes server-wide, - // ordered DatePlayed-descending so the first time we see a series is its - // newest play. We deliberately do NOT filter on the Played flag: Jellyfin's - // own NextUp ranks series by MAX(LastPlayedDate) across every episode, and an - // episode can carry a LastPlayedDate while Played==false (started but not - // finished, or later marked unwatched). Filtering to IsPlayed would miss - // those and leave such series un-dated. Null dates sort last, so the limit - // still captures the genuinely-recent episodes; a series whose last play - // falls beyond the window keeps a null date and degrades to its addedAt in - // the sort — it would rank near the bottom anyway, being least-recent. - final rawPlayed = await _safeFetchItemsArray('/Items', { - 'userId': connection.userId, - 'IncludeItemTypes': 'Episode', - 'Recursive': 'true', - 'SortBy': 'DatePlayed', - 'SortOrder': 'Descending', - 'Fields': _queueFields, - 'Limit': _continueWatchingSeriesLookback.toString(), - 'EnableImages': 'false', - 'EnableTotalRecordCount': 'false', - }); - + final seriesIds = pendingSeriesIds.take(_seriesLastPlayedLookupLimit).toList(growable: false); final lastPlayedBySeries = {}; - for (final episode in _mapItems(rawPlayed)) { - final seriesId = episode.grandparentId; - final playedAt = episode.lastViewedAt; - if (seriesId == null || playedAt == null) continue; - if (!pendingSeriesIds.contains(seriesId)) continue; - lastPlayedBySeries.putIfAbsent(seriesId, () => playedAt); + // A Timer, not a Stopwatch: the batches are the only thing that has to stop, + // and a timer is the deadline primitive the test harness can virtualise. + var withinBudget = true; + final deadline = Timer(_seriesLastPlayedBudget, () => withinBudget = false); + try { + for (var start = 0; start < seriesIds.length; start += _seriesLastPlayedConcurrency) { + if (!withinBudget) break; + final batch = seriesIds.skip(start).take(_seriesLastPlayedConcurrency); + for (final (seriesId, playedAt) in await Future.wait(batch.map(_fetchSeriesLastPlayed))) { + if (playedAt != null) lastPlayedBySeries[seriesId] = playedAt; + } + } + } finally { + deadline.cancel(); } if (lastPlayedBySeries.isEmpty) return nextUp; @@ -1707,6 +1743,39 @@ mixin _JellyfinBrowseMethods on _JellyfinClientInternals { ]; } + /// Newest `LastPlayedDate` across [seriesId]'s episodes, or null when the + /// series has never been played — or when the lookup failed, in which case the + /// row keeps a null date and degrades to its `addedAt` in the shelf sort. + /// + /// Deliberately no `Filters=IsPlayed`: Jellyfin's own NextUp ranks series by + /// MAX(LastPlayedDate) across every episode, and an episode can carry a + /// LastPlayedDate while Played==false (started but not finished, or later + /// marked unwatched). Filtering to IsPlayed would leave those series un-dated. + /// Null dates sort last under `Descending`, so the single row returned is the + /// series' newest play whenever it has one. Endpoint failover stays off: a slow + /// enrichment row must not move the whole client off a working endpoint. + Future<(String, int?)> _fetchSeriesLastPlayed(String seriesId) async { + final raw = await _safeFetchItemsArray( + '/Items', + { + 'userId': connection.userId, + 'ParentId': seriesId, + 'IncludeItemTypes': 'Episode', + 'Recursive': 'true', + 'SortBy': 'DatePlayed', + 'SortOrder': 'Descending', + // Only `UserData.LastPlayedDate` is read off the row. + 'Fields': 'UserData', + 'Limit': '1', + 'EnableImages': 'false', + 'EnableTotalRecordCount': 'false', + }, + timeout: _seriesLastPlayedRequestTimeout, + allowEndpointFailover: false, + ); + return (seriesId, _mapItems(raw).firstOrNull?.lastViewedAt); + } + /// Merge Jellyfin's two continue-watching sources into one recency-ordered /// shelf. Resume items are deduped first so an in-progress episode wins over /// the same series' Next Up entry, then the combined list is ordered by @@ -1751,14 +1820,26 @@ mixin _JellyfinBrowseMethods on _JellyfinClientInternals { /// failover** — a slow hub row must not move the whole client off an /// otherwise working endpoint (same policy as Plex's three hub fetches; /// see [retryTransientMediaServerCall] / [FailoverHttpClient]). + /// + /// [timeout] and [allowEndpointFailover] configure the un-retried path only; a + /// [retry] policy carries its own per-attempt timeouts and always disables + /// failover. Future _getItemsResponse( String path, Map queryParameters, _HubRetryPolicy? retry, { AbortController? abort, + Duration? timeout, + bool allowEndpointFailover = true, }) { if (retry == null) { - return _http.get(path, queryParameters: queryParameters, abort: abort); + return _http.get( + path, + queryParameters: queryParameters, + abort: abort, + timeout: timeout, + allowEndpointFailover: allowEndpointFailover, + ); } abort?.throwIfAborted(); return retryTransientMediaServerCall( @@ -1791,9 +1872,18 @@ mixin _JellyfinBrowseMethods on _JellyfinClientInternals { Map queryParameters, { _HubRetryPolicy? retry, AbortController? abort, + Duration? timeout, + bool allowEndpointFailover = true, }) async { try { - final response = await _getItemsResponse(path, queryParameters, retry, abort: abort); + final response = await _getItemsResponse( + path, + queryParameters, + retry, + abort: abort, + timeout: timeout, + allowEndpointFailover: allowEndpointFailover, + ); abort?.throwIfAborted(); throwIfHttpError(response); final data = response.data; diff --git a/lib/services/jellyfin_client/parts/live_tv.dart b/lib/services/jellyfin_client/parts/live_tv.dart index fa31ce41..593aacbf 100644 --- a/lib/services/jellyfin_client/parts/live_tv.dart +++ b/lib/services/jellyfin_client/parts/live_tv.dart @@ -8,6 +8,10 @@ mixin _JellyfinLiveTvMethods on _JellyfinClientInternals { _HubRetryPolicy? retry, // ignore: unused_element_parameter AbortController? abort, + // ignore: unused_element_parameter + Duration? timeout, + // ignore: unused_element_parameter + bool allowEndpointFailover, }); /// Returns `true` when this server has Live TV configured (channels diff --git a/test/services/jellyfin_client_urls_test.dart b/test/services/jellyfin_client_urls_test.dart index f499adae..dc854143 100644 --- a/test/services/jellyfin_client_urls_test.dart +++ b/test/services/jellyfin_client_urls_test.dart @@ -1,5 +1,7 @@ +import 'dart:async'; import 'dart:convert'; +import 'package:fake_async/fake_async.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:http/http.dart' as http; import 'package:http/testing.dart'; @@ -2661,16 +2663,222 @@ void main() { final lookup = requests.singleWhere((uri) => uri.path == '/Items'); expect(lookup.queryParameters['userId'], 'user-1'); + // Scoped to the one series that needs a date, not a server-wide episode + // sort: the unscoped form pegs Jellyfin 12.0-rc3 for seconds (#1699). + expect(lookup.queryParameters['ParentId'], 'show-recent'); expect(lookup.queryParameters['IncludeItemTypes'], 'Episode'); expect(lookup.queryParameters['Recursive'], 'true'); expect(lookup.queryParameters['SortBy'], 'DatePlayed'); expect(lookup.queryParameters['SortOrder'], 'Descending'); - expect(lookup.queryParameters['Limit'], '200'); + expect(lookup.queryParameters['Limit'], '1'); + expect(lookup.queryParameters['Fields'], 'UserData'); // No Filters=IsPlayed: a series' newest engagement can sit on an episode // with a LastPlayedDate but Played==false (see _attachSeriesLastPlayed). expect(lookup.queryParameters.containsKey('Filters'), isFalse); }); + test('fetchContinueWatching issues one last-played lookup per pending series', () async { + final requests = []; + final scoped = JellyfinClient.forTesting( + connection: _conn(), + httpClient: MockClient((req) async { + requests.add(req.url); + if (req.url.path == '/UserItems/Resume') return jsonResponse({'Items': []}); + if (req.url.path == '/Shows/NextUp') { + return jsonResponse({ + 'Items': [ + for (var i = 1; i <= 6; i++) + {'Id': 'next-$i', 'Type': 'Episode', 'Name': 'Next $i', 'SeriesId': 'show-$i'}, + // Second row for an already-pending series: still one lookup. + {'Id': 'next-1b', 'Type': 'Episode', 'Name': 'Next 1b', 'SeriesId': 'show-1'}, + ], + }); + } + if (req.url.path == '/Items') { + final seriesId = req.url.queryParameters['ParentId']!; + return jsonResponse({ + 'Items': [ + { + 'Id': 'played-$seriesId', + 'Type': 'Episode', + 'SeriesId': seriesId, + 'UserData': {'LastPlayedDate': '2026-06-0${seriesId.split('-').last}T00:00:00.0000000Z'}, + }, + ], + }); + } + return http.Response('not found', 404); + }), + ); + addTearDown(scoped.close); + + final items = await scoped.fetchContinueWatching(count: 10); + + final lookups = requests.where((uri) => uri.path == '/Items').toList(); + expect( + lookups.map((uri) => uri.queryParameters['ParentId']).toList(), + unorderedEquals(['show-1', 'show-2', 'show-3', 'show-4', 'show-5', 'show-6']), + reason: 'a series pending on two Next Up rows must not be looked up twice', + ); + // Series 6 played most recently, series 1 least; the shelf follows the + // stamped dates rather than Next Up's own row order. + expect(items.map((item) => item.id).take(2), ['next-6', 'next-5']); + }); + + test('fetchContinueWatching caps last-played lookups on an uncapped shelf', () async { + final requests = []; + final scoped = JellyfinClient.forTesting( + connection: _conn(), + httpClient: MockClient((req) async { + requests.add(req.url); + if (req.url.path == '/UserItems/Resume') return jsonResponse({'Items': []}); + if (req.url.path == '/Shows/NextUp') { + return jsonResponse({ + 'Items': [ + for (var i = 0; i < 60; i++) + {'Id': 'next-$i', 'Type': 'Episode', 'Name': 'Next $i', 'SeriesId': 'show-$i'}, + ], + }); + } + if (req.url.path == '/Items') { + return jsonResponse({ + 'Items': [ + { + 'Id': 'played-${req.url.queryParameters['ParentId']}', + 'Type': 'Episode', + 'SeriesId': req.url.queryParameters['ParentId'], + 'UserData': {'LastPlayedDate': '2026-06-01T00:00:00.0000000Z'}, + }, + ], + }); + } + return http.Response('not found', 404); + }), + ); + addTearDown(scoped.close); + + final items = await scoped.fetchContinueWatching(count: null); + + final lookups = requests.where((uri) => uri.path == '/Items').toList(); + // 60 series on the shelf, but the enrichment stays bounded — an unbounded + // fan-out here is what the #1699 fix exists to prevent. + expect(lookups, hasLength(24)); + // Next Up order decides which series get dated: the newest ones. + expect( + lookups.map((uri) => uri.queryParameters['ParentId']).toList(), + unorderedEquals([for (var i = 0; i < 24; i++) 'show-$i']), + ); + // Every row is still returned; the undated tail just sorts by addedAt. + expect(items, hasLength(60)); + }); + + test('fetchContinueWatching abandons last-played lookups against a stalled endpoint', () { + fakeAsync((async) { + final requests = []; + final stalled = Completer(); + final scoped = JellyfinClient.forTesting( + connection: _conn(), + httpClient: MockClient((req) { + requests.add(req.url); + if (req.url.path == '/UserItems/Resume') return Future.value(jsonResponse({'Items': []})); + if (req.url.path == '/Shows/NextUp') { + return Future.value( + jsonResponse({ + 'Items': [ + for (var i = 0; i < 24; i++) + {'Id': 'next-$i', 'Type': 'Episode', 'Name': 'Next $i', 'SeriesId': 'show-$i'}, + ], + }), + ); + } + // Never answers: the endpoint accepted the request and went quiet, + // which is exactly how the #1699 server behaved. + if (req.url.path == '/Items') return stalled.future; + return Future.value(http.Response('not found', 404)); + }), + ); + + List? items; + unawaited(scoped.fetchContinueWatching(count: null).then((result) => items = result)); + async.flushMicrotasks(); + + // Only one batch is ever open at a time, and it is exactly + // _seriesLastPlayedConcurrency wide — a wider burst is the failure mode + // this whole change exists to remove. + expect( + requests.where((uri) => uri.path == '/Items').length, + 4, + reason: 'the first batch must be four lookups, not the whole shelf', + ); + + // Six batches at the shared 10s default would serialise into a minute of + // blocking. The documented bound is the shared budget plus the one + // in-flight request timeout. + async.elapse(const Duration(seconds: 7)); + async.flushMicrotasks(); + + expect(items, isNotNull, reason: 'the enrichment must give up, not hang on a silent endpoint'); + expect(items!.map((item) => item.id), [for (var i = 0; i < 24; i++) 'next-$i']); + expect( + requests.where((uri) => uri.path == '/Items').length, + 8, + reason: 'the 4s budget expires during the second batch, so the last four series go unstamped', + ); + scoped.close(); + }); + }); + + test('fetchContinueWatching keeps other series dated when one lookup fails', () async { + final scoped = JellyfinClient.forTesting( + connection: _conn(), + httpClient: MockClient((req) async { + if (req.url.path == '/UserItems/Resume') { + return jsonResponse({ + 'Items': [ + { + 'Id': 'resume-mid', + 'Type': 'Movie', + 'Name': 'Mid Movie', + 'UserData': {'LastPlayedDate': '2026-05-01T00:00:00.0000000Z'}, + }, + ], + }); + } + if (req.url.path == '/Shows/NextUp') { + return jsonResponse({ + 'Items': [ + {'Id': 'next-ok', 'Type': 'Episode', 'Name': 'Next Ok', 'SeriesId': 'show-ok'}, + {'Id': 'next-broken', 'Type': 'Episode', 'Name': 'Next Broken', 'SeriesId': 'show-broken'}, + ], + }); + } + if (req.url.path == '/Items') { + if (req.url.queryParameters['ParentId'] == 'show-broken') { + return http.Response('Internal error', 500); + } + return jsonResponse({ + 'Items': [ + { + 'Id': 'played-ok', + 'Type': 'Episode', + 'SeriesId': 'show-ok', + 'UserData': {'LastPlayedDate': '2026-06-01T00:00:00.0000000Z'}, + }, + ], + }); + } + return http.Response('not found', 404); + }), + ); + addTearDown(scoped.close); + + final items = await scoped.fetchContinueWatching(count: 10); + + // The dated series still outranks the resume item; the failed one keeps a + // null date and falls to the bottom instead of sinking the whole shelf. + expect(items.map((item) => item.id), ['next-ok', 'resume-mid', 'next-broken']); + }); + test('fetchContinueWatching does not let resume items starve Next Up under the limit', () async { final scoped = JellyfinClient.forTesting( connection: _conn(),