diff --git a/lib/services/jellyfin_client/parts/browse.dart b/lib/services/jellyfin_client/parts/browse.dart index 6302524f..2cff6ccf 100644 --- a/lib/services/jellyfin_client/parts/browse.dart +++ b/lib/services/jellyfin_client/parts/browse.dart @@ -612,6 +612,12 @@ mixin _JellyfinBrowseMethods on _JellyfinClientInternals { /// (`enableResumable=true`, `disableFirstEpisode=false`) match Plex /// OnDeck semantics: returns the resume episode when one exists, or S1E1 /// when the user hasn't started. Movies and other kinds short-circuit. + /// + /// Deliberately still chained rather than fired in parallel off a caller + /// kind hint: measured against a remote server that saved nothing, because + /// the two requests contend rather than overlap (`/Shows/NextUp` went from + /// 380ms alone to 1395ms beside the detail fetch), and it would cost a + /// wasted request on every movie whose hint was absent or wrong. @override Future<({MediaItem? item, MediaItem? onDeckEpisode})> fetchItemWithOnDeck(String id) async { final item = await fetchItem(id); @@ -629,8 +635,33 @@ mixin _JellyfinBrowseMethods on _JellyfinClientInternals { return (item: item, onDeckEpisode: onDeckEpisode); } + /// In-flight `fetchItem` requests, keyed by item id. + /// + /// Opening a detail screen issues two identical full-detail GETs for the + /// same id at the same time — `_loadFullMetadata` and, from + /// `_initWatchlistState`, `fetchExternalIds` — and playback start adds three + /// more. Each one makes the server rebuild the whole dto (People, Chapters + /// and MediaSources are a DB query apiece, Trickplay several), so the + /// duplicates are expensive on both ends (#1784). + /// + /// Single-flight only: once a request settles the next caller re-fetches, so + /// nothing here can serve a stale item. + final Map> _inFlightItems = {}; + @override - Future fetchItem(String id) async { + Future fetchItem(String id) { + final existing = _inFlightItems[id]; + if (existing != null) return existing; + + late final Future request; + request = _fetchItemOnce(id).whenComplete(() { + if (identical(_inFlightItems[id], request)) _inFlightItems.remove(id); + }); + _inFlightItems[id] = request; + return request; + } + + Future _fetchItemOnce(String id) async { final endpoint = '/Users/${_segment(connection.userId)}/Items/${_segment(id)}'; // Contract: // - 200 with parseable Map → MediaItem diff --git a/test/services/jellyfin_client_urls_test.dart b/test/services/jellyfin_client_urls_test.dart index 3d9f71d1..8cdc250d 100644 --- a/test/services/jellyfin_client_urls_test.dart +++ b/test/services/jellyfin_client_urls_test.dart @@ -134,6 +134,36 @@ void main() { await scoped.fetchLibraries(); expect(views, 2); }); + test('concurrent fetchItem calls for one id share a single request', () async { + // Opening a detail screen fires `_loadFullMetadata` and, via + // `_initWatchlistState`, `fetchExternalIds` — both a full-detail GET for + // the same id at the same time. Each makes the server rebuild the whole + // dto (People, Chapters and MediaSources are a DB query apiece). + var detailFetches = 0; + final scoped = JellyfinClient.forTesting( + connection: _conn(), + httpClient: MockClient((req) async { + if (req.url.path == '/Users/user-1/Items/item-1') detailFetches++; + return http.Response( + jsonEncode({'Id': 'item-1', 'Name': 'Item', 'Type': 'Movie'}), + 200, + headers: {'content-type': 'application/json'}, + ); + }), + ); + addTearDown(scoped.close); + + final results = await Future.wait([scoped.fetchItem('item-1'), scoped.fetchItem('item-1')]); + + expect(detailFetches, 1); + expect(results.map((item) => item?.id), ['item-1', 'item-1']); + + // Different ids never share, and a later pass re-fetches — single-flight, + // not a cache, so nothing here can serve a stale item. + await scoped.fetchItem('item-2'); + await scoped.fetchItem('item-1'); + expect(detailFetches, 2); + }); test('buildDirectStreamUrl includes static flag, api_key, and device id', () { final url = client.buildDirectStreamUrl('item-99');