diff --git a/lib/screens/catalog_item_detail_screen.dart b/lib/screens/catalog_item_detail_screen.dart index 03b3c11f..d0f736cb 100644 --- a/lib/screens/catalog_item_detail_screen.dart +++ b/lib/screens/catalog_item_detail_screen.dart @@ -103,7 +103,7 @@ class _CatalogItemDetailScreenState extends State { void initState() { super.initState(); _syncDetailCollections(widget.item); - unawaited(_resolveMatches()); + unawaited(_resolveMatches(widget.item)); unawaited(_loadDetail()); final sources = context.read(); _watchlistSource = sources.watchlistSourceFor(widget.item); @@ -149,13 +149,22 @@ class _CatalogItemDetailScreenState extends State { setState(() {}); } - Future _resolveMatches() async { + /// Monotonic guard for match resolution: the bare row item and its + /// detail-enriched form resolve concurrently, and only the latest-issued + /// resolution may publish (a slow bare lookup must not overwrite the + /// enriched verdict with its own). + int _matchGeneration = 0; + + Future _resolveMatches(CatalogItem item) async { + final generation = ++_matchGeneration; + List matches; try { - _setMatches(await context.read().match(widget.item)); + matches = await context.read().match(item); } catch (e) { - appLogger.w('Catalog library match failed for ${widget.item.identityKey}', error: e); - _setMatches(const []); + appLogger.w('Catalog library match failed for ${item.identityKey}', error: e); + matches = const []; } + if (generation == _matchGeneration) _setMatches(matches); } void _setMatches(List matches) { @@ -221,6 +230,14 @@ class _CatalogItemDetailScreenState extends State { _related = detail.related; _relationEntries = relationEntries; }); + // The row form of a Plex Discover item carries only its rating key; + // the detail body brings the external ids (#1715). When enrichment + // added id forms and the bare lookup found nothing, ask again with + // the full set. + final gainedIds = !widget.item.ids.allKeys.toSet().containsAll(detail.item.ids.allKeys); + if (gainedIds && (_matches?.isEmpty ?? true)) { + unawaited(_resolveMatches(detail.item)); + } } catch (e) { appLogger.d('Catalog detail load failed for ${widget.item.identityKey}', error: e); } diff --git a/lib/services/catalog/catalog_library_matcher.dart b/lib/services/catalog/catalog_library_matcher.dart index 87402a04..3dc42029 100644 --- a/lib/services/catalog/catalog_library_matcher.dart +++ b/lib/services/catalog/catalog_library_matcher.dart @@ -33,8 +33,12 @@ class CatalogLibraryMatcher { // `mal45576 s1`, `mal51179 s2`, `mal55888 s2`, `mal59193 s3`) collapse to // `imdb:tt13293588`, so the first season-gated result would poison the rest. // Namespace by source too: MAL and AniList can share a MAL id while - // contributing different localized title candidates. - final key = '${item.source.name}/${item.entryIdentityKey}'; + // contributing different localized title candidates. The id forms join + // the key because a detail load can enrich an item with external ids its + // row form lacked (#1715: Plex rows carry only a rating key); the richer + // lookup must not be short-circuited by the poorer form's cached + // negative. + final key = '${item.source.name}/${item.entryIdentityKey}/${item.ids.allKeys.join(',')}'; final cached = _cache[key]; if (cached != null && (cached.items.isNotEmpty || _now().difference(cached.at) < negativeTtl)) { return cached.items; diff --git a/lib/services/catalog/plex_catalog_source.dart b/lib/services/catalog/plex_catalog_source.dart index 7c89ebf4..95782497 100644 --- a/lib/services/catalog/plex_catalog_source.dart +++ b/lib/services/catalog/plex_catalog_source.dart @@ -35,11 +35,16 @@ class PlexCatalogSource with CatalogWatchlistMachinery implements CatalogSource, @override String get watchlistLogLabel => 'Plex: watchlist'; + // Discover validates X-Plex-Container-Size against a cap it drifts + // without notice (#1715: 500 became invalid). 100 keeps the snapshot at + // few requests while staying well under the observed cap, and the client + // degrades to 25-item chunks if the cap ever drops below it; more pages + // preserve the 5000-entry coverage. @override - int get watchlistPageLimit => 500; + int get watchlistPageLimit => 100; @override - int get watchlistMaxPages => 10; + int get watchlistMaxPages => 50; @override Future fetchRow(CatalogRowId row, {int page = 1, int limit = 25}) async { diff --git a/lib/services/plex_client.dart b/lib/services/plex_client.dart index 3bf7b1e3..a83ef337 100644 --- a/lib/services/plex_client.dart +++ b/lib/services/plex_client.dart @@ -3876,6 +3876,11 @@ class PlexClient } } + // Title attempts confirm candidates by external-id intersection, so + // without external ids they cannot match anything — stop at the exact + // guid lookup instead of burning requests that always come back empty. + if (!ids.hasAny) return null; + Future<({Map? modern, Map? legacy})> attempt( String title, { required int size, diff --git a/lib/services/plex_discover_client.dart b/lib/services/plex_discover_client.dart index 7c6c8f0c..a64825ff 100644 --- a/lib/services/plex_discover_client.dart +++ b/lib/services/plex_discover_client.dart @@ -85,14 +85,41 @@ class PlexDiscoverClient { PlexDiscoverClient(this.session, {http.Client? httpClient, this.requestTimeout = const Duration(seconds: 20)}) : _http = httpClient ?? http.Client(); + /// Largest container size field-proven against Discover's request + /// validation: the Explore row fetch uses it on every load. Oversized + /// pages refetch as chunks of it, so the cap can drift below a caller's + /// page size without breaking that caller's offset math (#1715: 500 + /// became "Invalid value provided for x-plex-container-size!"). + static const int _watchlistChunkSize = 25; + Future getWatchlist({int page = 1, int limit = 25}) async { final safePage = page < 1 ? 1 : page; final safeLimit = limit.clamp(1, 500); final offset = (safePage - 1) * safeLimit; + try { + return await _watchlistRange(offset, safeLimit); + } on PlexDiscoverException catch (error) { + if (safeLimit <= _watchlistChunkSize || !_isContainerSizeRejection(error)) rethrow; + appLogger.w('Plex Discover: container size $safeLimit rejected, refetching in chunks of $_watchlistChunkSize'); + final items = >[]; + PlexDiscoverPage chunk; + do { + final remaining = safeLimit - items.length; + chunk = await _watchlistRange( + offset + items.length, + remaining > _watchlistChunkSize ? _watchlistChunkSize : remaining, + ); + items.addAll(chunk.items); + } while (items.length < safeLimit && chunk.hasMore && chunk.items.isNotEmpty); + return PlexDiscoverPage(items: items, hasMore: chunk.hasMore, totalResults: chunk.totalResults); + } + } + + Future _watchlistRange(int offset, int size) async { final data = await _request( 'GET', '/library/sections/watchlist/all', - query: {'X-Plex-Container-Start': offset, 'X-Plex-Container-Size': safeLimit, 'includeMeta': 1}, + query: {'X-Plex-Container-Start': offset, 'X-Plex-Container-Size': size, 'includeGuids': 1, 'includeMeta': 1}, ); final container = _mediaContainer(data!); final items = flexibleMapList(container['Metadata']); @@ -101,6 +128,9 @@ class PlexDiscoverClient { return PlexDiscoverPage(items: items, hasMore: offset + items.length < total, totalResults: reportedTotal); } + static bool _isContainerSizeRejection(PlexDiscoverException error) => + error.statusCode == 400 && error.message.toLowerCase().contains('container-size'); + /// Discover's Home shelves — the rows Plex's own web client renders on its /// Home ▸ Trending tab, which reads /// `provider://tv.plex.provider.discover/hubs/sections/home`. @@ -188,6 +218,7 @@ class PlexDiscoverClient { key, query: { 'limit': safeLimit, + 'includeGuids': 1, 'includeMeta': 1, 'includeUserState': 1, // Allowing Image on the measured 26-item hub grew 27,287 -> 55,925 @@ -216,6 +247,7 @@ class PlexDiscoverClient { 'limit': limit.clamp(1, 100), 'searchTypes': 'movies,tv', 'searchProviders': 'discover', + 'includeGuids': 1, 'includeMetadata': 1, 'filterPeople': 1, }, @@ -237,7 +269,12 @@ class PlexDiscoverClient { _ => null, }; if (guid == null) return null; - final data = await _request('GET', '/library/metadata/matches', query: {'guid': guid}, allowNotFound: true); + final data = await _request( + 'GET', + '/library/metadata/matches', + query: {'guid': guid, 'includeGuids': 1}, + allowNotFound: true, + ); if (data == null) return null; return firstFlexibleMap(_mediaContainer(data)['Metadata']); } @@ -306,7 +343,11 @@ class PlexDiscoverClient { 'PUT' => _http.put(uri, headers: headers), _ => throw ArgumentError.value(method, 'method', 'Unsupported Plex Discover method'), }; + final stopwatch = Stopwatch()..start(); final response = await request.timeout(requestTimeout); + // The other API surfaces log every request; without this line Discover + // drift is invisible in reporter logs (#1715 shipped blind). + appLogger.d('Discover $method ${relative.path} → ${response.statusCode} (${stopwatch.elapsedMilliseconds}ms)'); if (allowNotFound && response.statusCode == 404) return null; if (response.statusCode < 200 || response.statusCode >= 300) { throw PlexDiscoverException(response.statusCode, _errorMessage(response.body)); diff --git a/test/screens/catalog_item_detail_screen_test.dart b/test/screens/catalog_item_detail_screen_test.dart index b35bdd36..1d48236b 100644 --- a/test/screens/catalog_item_detail_screen_test.dart +++ b/test/screens/catalog_item_detail_screen_test.dart @@ -13,6 +13,7 @@ import 'package:plezy/models/catalog/catalog_cast_member.dart'; import 'package:plezy/models/catalog/catalog_item.dart'; import 'package:plezy/models/catalog/catalog_metadata.dart'; import 'package:plezy/providers/catalog_sources_provider.dart'; +import 'package:plezy/providers/multi_server_provider.dart'; import 'package:plezy/screens/catalog_item_detail_screen.dart'; import 'package:plezy/services/catalog/catalog_source.dart'; import 'package:plezy/services/catalog/catalog_library_matcher.dart'; @@ -128,6 +129,22 @@ class _FakeCatalogLibraryMatcher extends CatalogLibraryMatcher { Future> match(CatalogItem item) async => matches; } +/// Matches only items that carry an external id, the way a real lookup for a +/// Plex Discover row does (#1715): the bare rating-key form misses, the +/// detail-enriched form hits. +class _ExternalIdGatedMatcher extends CatalogLibraryMatcher { + _ExternalIdGatedMatcher(super.multiServer, this.hit); + + final MediaItem hit; + final List calls = []; + + @override + Future> match(CatalogItem item) async { + calls.add(item); + return item.ids.toExternalIds().hasAny ? [hit] : const []; + } +} + const _item = CatalogItem( source: CatalogSourceId.trakt, kind: MediaKind.movie, @@ -142,11 +159,12 @@ Future _pumpDetail( List matches = const [], bool pushedRoute = false, CatalogItem item = _item, + CatalogLibraryMatcher Function(MultiServerProvider multiServer)? matcherBuilder, }) async { final sources = _FakeCatalogSourcesProvider(source); final serverManager = MultiServerManager(); final multiServer = testMultiServerProvider(serverManager); - final matcher = _FakeCatalogLibraryMatcher(multiServer, matches); + final matcher = matcherBuilder?.call(multiServer) ?? _FakeCatalogLibraryMatcher(multiServer, matches); addTearDown(sources.dispose); addTearDown(source.dispose); addTearDown(serverManager.dispose); @@ -231,6 +249,40 @@ void main() { expect(find.text('Catalog Movie'), findsNothing); }); + testWidgets('detail enrichment that adds external ids re-resolves library matches', (tester) async { + // #1715: the row form of a Plex Discover item carries only its rating + // key and the first lookup misses; the detail body brings the external + // ids, which must trigger a second lookup instead of leaving the screen + // on "Not in your library". + const bare = CatalogItem( + source: CatalogSourceId.trakt, + kind: MediaKind.movie, + title: 'Row-only Movie', + ids: CatalogItemIds(trakt: 5), + ); + const enriched = CatalogItem( + source: CatalogSourceId.trakt, + kind: MediaKind.movie, + title: 'Row-only Movie', + ids: CatalogItemIds(trakt: 5, tmdb: 99), + ); + final hit = testMediaItem(id: 'server-match', libraryTitle: 'Movies', serverName: 'Living Room'); + late _ExternalIdGatedMatcher matcher; + final source = _FakeCatalogSource(detail: const CatalogDetail(item: enriched)); + + await _pumpDetail( + tester, + source, + item: bare, + matcherBuilder: (multiServer) => matcher = _ExternalIdGatedMatcher(multiServer, hit), + ); + + expect(matcher.calls.map((call) => call.ids.tmdb), [null, 99]); + expect(find.text(t.explore.notInLibrary), findsNothing); + expect(find.text(t.explore.inTheseLibraries), findsOneWidget); + expect(find.text('Movies'), findsOneWidget); + }); + testWidgets('fetchDetail failure leaves the opening item rendered', (tester) async { final source = _FakeCatalogSource(detailError: StateError('detail unavailable')); diff --git a/test/services/catalog/catalog_library_matcher_test.dart b/test/services/catalog/catalog_library_matcher_test.dart index 658bcf5f..96e9f21a 100644 --- a/test/services/catalog/catalog_library_matcher_test.dart +++ b/test/services/catalog/catalog_library_matcher_test.dart @@ -243,6 +243,43 @@ void main() { expect(call.plexGuid, 'plex://movie/5d776828880197001ec90e13'); }); + test('an id-poor negative does not suppress the detail-enriched retry', () async { + // #1715: a Plex Discover row item carries only its rating key, and the + // exact-guid lookup can miss even for owned titles (Discover dupes). + // The detail body brings the external ids moments later; that richer + // lookup must reach the servers instead of the bare form's cached + // negative. + final harness = _Harness(); + addTearDown(harness.dispose); + final hit = testMediaItem(id: 'server-movie', kind: MediaKind.movie); + harness.aggregation.responses.addAll([ + const [], + [hit], + ]); + const bare = CatalogItem( + source: CatalogSourceId.plex, + kind: MediaKind.movie, + title: 'Night on the Galactic Railroad', + ids: CatalogItemIds(plex: '5d776b59ad5437001f79c6f8'), + ); + const enriched = CatalogItem( + source: CatalogSourceId.plex, + kind: MediaKind.movie, + title: 'Night on the Galactic Railroad', + ids: CatalogItemIds(plex: '5d776b59ad5437001f79c6f8', imdb: 'tt0089445', tmdb: 34523), + ); + + expect(bare.entryIdentityKey, enriched.entryIdentityKey); + expect(await harness.matcher.match(bare), isEmpty); + expect((await harness.matcher.match(enriched)).single, same(hit)); + expect(harness.aggregation.calls, hasLength(2)); + expect(harness.aggregation.calls.last.ids.imdb, 'tt0089445'); + + // Both forms stay memoized independently. + expect((await harness.matcher.match(enriched)).single, same(hit)); + expect(harness.aggregation.calls, hasLength(2)); + }); + test('only a Plex Discover item contributes a guid, and it costs no request', () async { final harness = _Harness(); addTearDown(harness.dispose); diff --git a/test/services/catalog/plex_catalog_source_test.dart b/test/services/catalog/plex_catalog_source_test.dart index 33a869d4..53833832 100644 --- a/test/services/catalog/plex_catalog_source_test.dart +++ b/test/services/catalog/plex_catalog_source_test.dart @@ -83,6 +83,7 @@ void main() { expect(captured.url.path, '/library/sections/watchlist/all'); expect(captured.url.queryParameters['X-Plex-Container-Start'], '25'); expect(captured.url.queryParameters['X-Plex-Container-Size'], '25'); + expect(captured.url.queryParameters['includeGuids'], '1'); expect(captured.url.queryParameters['includeMeta'], '1'); expect(captured.headers['X-Plex-Token'], 'profile-token'); expect(captured.headers['X-Plex-Client-Identifier'], 'client-id'); @@ -485,6 +486,72 @@ void main() { await source.removeFromWatchlist(MediaKind.movie, ids); expect(source.isOnWatchlist(MediaKind.movie, ids), isFalse); expect(requests, hasLength(2)); + // The snapshot page must stay under Discover's container-size cap + // (#1715: 500 was rejected outright). + expect(requests.first.url.queryParameters['X-Plex-Container-Size'], '100'); + }); + + test('an oversized watchlist page refetches as chunks when Discover rejects it', () async { + final requests = []; + final client = PlexDiscoverClient( + _session, + httpClient: MockClient((request) async { + requests.add(request); + final size = int.parse(request.url.queryParameters['X-Plex-Container-Size']!); + if (size > 25) { + return jsonResponse({ + 'Error': {'message': 'Invalid value provided for x-plex-container-size!'}, + }, status: 400); + } + final start = int.parse(request.url.queryParameters['X-Plex-Container-Start']!); + const total = 180; + final count = (total - start).clamp(0, size); + return jsonResponse({ + 'MediaContainer': { + 'totalSize': total, + 'Metadata': [ + for (var i = 0; i < count; i++) + {'ratingKey': 'rk-${start + i}', 'type': 'movie', 'title': 'Movie ${start + i}'}, + ], + }, + }); + }), + ); + addTearDown(client.dispose); + + final page = await client.getWatchlist(page: 2, limit: 100); + + // The rejected request, then the same range in proven-size chunks: + // the caller's offset math survives the cap drift. + expect( + requests.map( + (request) => + (request.url.queryParameters['X-Plex-Container-Start'], request.url.queryParameters['X-Plex-Container-Size']), + ), + [('100', '100'), ('100', '25'), ('125', '25'), ('150', '25'), ('175', '25')], + ); + expect(page.items, hasLength(80)); + expect(page.items.first['ratingKey'], 'rk-100'); + expect(page.items.last['ratingKey'], 'rk-179'); + expect(page.hasMore, isFalse); + expect(page.totalResults, 180); + }); + + test('other Discover rejections surface instead of chunking', () async { + var requestCount = 0; + final client = PlexDiscoverClient( + _session, + httpClient: MockClient((request) async { + requestCount++; + return jsonResponse({ + 'Error': {'message': 'Maintenance'}, + }, status: 400); + }), + ); + addTearDown(client.dispose); + + await expectLater(client.getWatchlist(limit: 100), throwsA(isA())); + expect(requestCount, 1); }); test('watchlist mutation resolves a missing Plex rating key from external ids', () async { diff --git a/test/services/plex_external_id_lookup_test.dart b/test/services/plex_external_id_lookup_test.dart index d6c410da..28a3975b 100644 --- a/test/services/plex_external_id_lookup_test.dart +++ b/test/services/plex_external_id_lookup_test.dart @@ -288,6 +288,34 @@ void main() { expect(requests.single.queryParameters.containsKey('title'), isFalse); }); + test('a guid-only lookup stops after the exact guid miss', () async { + // Title attempts verify candidates by external-id intersection, so with + // no external ids they can never confirm anything — the guid miss must + // be the lookup's only request (#1715). + final requests = []; + final client = testPlexClient( + handler: (request) async { + requests.add(request.url); + return _json({ + 'MediaContainer': {'Metadata': []}, + }); + }, + ); + addTearDown(client.close); + + final match = await client.findByExternalIds( + const ExternalIds(), + kind: MediaKind.movie, + titles: const ['Night on the Galactic Railroad'], + plexGuid: 'plex://movie/5d776b59ad5437001f79c6f8', + ); + + expect(match, isNull); + expect(requests, hasLength(1)); + expect(requests.single.queryParameters['guid'], 'plex://movie/5d776b59ad5437001f79c6f8'); + expect(requests.single.queryParameters.containsKey('title'), isFalse); + }); + test('an agreed season ref gates on the season hierarchy and nothing else', () async { final childRequests = []; final extraRequests = [];