diff --git a/lib/media/media_item_merge.dart b/lib/media/media_item_merge.dart index f99499de..56b0bfe1 100644 --- a/lib/media/media_item_merge.dart +++ b/lib/media/media_item_merge.dart @@ -1,5 +1,6 @@ import 'ids.dart'; import 'media_item.dart'; +import 'media_version.dart'; /// Merge freshly fetched metadata with identity and library context already /// known by the caller. The fetched item owns descriptive fields, while @@ -12,3 +13,63 @@ MediaItem mergeFetchedMediaItem({required MediaItem fetched, required ServerId f libraryTitle: fetched.libraryTitle ?? existing?.libraryTitle, ); } + +/// Tallest version height an item exposes, or 0 when the backend reported +/// none. +int _bestResolutionHeight(MediaItem item) { + var best = 0; + for (final version in item.mediaVersions ?? const []) { + final height = version.resolutionHeight; + if (height != null && height > best) best = height; + } + return best; +} + +/// Order the library copies of one title best-first: highest resolution, then +/// library title, then server name, then global key. +/// +/// Total and derived purely from the items, so a chooser can re-sort after +/// merging a later resolution pass without its rows jumping around. +int compareLibraryCopies(MediaItem a, MediaItem b) { + final byResolution = _bestResolutionHeight(b).compareTo(_bestResolutionHeight(a)); + if (byResolution != 0) return byResolution; + final byLibrary = (a.libraryTitle ?? '').compareTo(b.libraryTitle ?? ''); + if (byLibrary != 0) return byLibrary; + final byServer = (a.serverName ?? '').compareTo(b.serverName ?? ''); + if (byServer != 0) return byServer; + return a.globalKey.compareTo(b.globalKey); +} + +/// Fold a re-resolved copy into the one already on screen. +/// +/// The addition is fresher, but a degraded pass must not erase context. The +/// Jellyfin library stamp is a best-effort `/Items/{id}/Ancestors` call that +/// hands back an unstamped item when it fails, and a copy that lost its +/// library title is indistinguishable from its sibling in the same server's +/// other library — exactly the ambiguity the chooser exists to resolve. The +/// version list behind the resolution hint is treated the same way. +MediaItem _mergeCopy(MediaItem existing, MediaItem addition) { + final versions = addition.mediaVersions; + return addition.copyWith( + libraryId: addition.libraryId ?? existing.libraryId, + libraryTitle: addition.libraryTitle ?? existing.libraryTitle, + serverName: addition.serverName ?? existing.serverName, + mediaVersions: versions == null || versions.isEmpty ? existing.mediaVersions : versions, + ); +} + +/// Union [additions] into [current] by [MediaItem.globalKey], then re-sort +/// with [compareLibraryCopies]. A key on both sides is folded by [_mergeCopy]. +/// +/// Never removes a copy, and never downgrades one. The cross-server fan-out +/// behind these lists logs and skips per-server failures, so a later pass can +/// legitimately come back short a server, or short the best-effort library +/// stamp of a copy it did return. +List mergeLibraryCopies(Iterable current, Iterable additions) { + final byKey = {for (final item in current) item.globalKey: item}; + for (final item in additions) { + final existing = byKey[item.globalKey]; + byKey[item.globalKey] = existing == null ? item : _mergeCopy(existing, item); + } + return byKey.values.toList()..sort(compareLibraryCopies); +} diff --git a/lib/media/media_server_client.dart b/lib/media/media_server_client.dart index e2596e7a..7c9e8a25 100644 --- a/lib/media/media_server_client.dart +++ b/lib/media/media_server_client.dart @@ -504,7 +504,7 @@ abstract class MediaServerClient { /// has no external mapping for the item. Future fetchExternalIds(String itemId); - /// Reverse lookup: find a library movie/show matching any of [ids]. + /// Reverse lookup: find every library movie/show matching any of [ids]. /// /// Neither backend can filter by external id — Plex's `guid=` matches only /// the primary `plex://` guid (verified on PMS 1.43) and Jellyfin dropped @@ -512,7 +512,16 @@ abstract class MediaServerClient { /// title and verify candidates against their exact external ids. Title /// alone never produces a match. /// - /// [titles] are tried in order until one yields an id-verified candidate; + /// One title can own several library items: a server with a 4K section and + /// an HD section holds two rating keys for the same movie, and a library + /// still on a legacy agent carries a different primary guid than its modern + /// sibling. Implementations MUST return every id-verified copy rather than + /// the first, and MUST NOT truncate — external-id verification is the only + /// bound, so a long list means the user genuinely owns that many copies and + /// the caller (the Explore "In these libraries" chooser) exists to show + /// them. Ordering is the implementation's, and callers re-sort. + /// + /// [titles] are tried in order until one yields id-verified candidates; /// pass the entry's own title first and broader forms after (see /// `titleMatchCandidates`). A sequel entry's own title never matches its /// parent show, which is why more than one is needed. [year] applies a ±1 @@ -520,22 +529,21 @@ abstract class MediaServerClient { /// season's, not the show's. /// /// [plexGuid] is a Plex-only escape hatch: a `plex://show/…` guid the caller - /// already holds, which the local server *can* filter on exactly, skipping - /// the title search. It is never resolved over the network — only Plex - /// Discover catalog items carry one, in their own rating key. Other backends - /// ignore it. + /// already holds, which the local server *can* filter on exactly. It is + /// never resolved over the network — only Plex Discover catalog items carry + /// one, in their own rating key. Other backends ignore it. /// - /// [season] gates the result: when the entry maps to season 2+ of a longer - /// series, a match is only returned if the server actually has that season. + /// [season] gates the results: when the entry maps to season 2+ of a longer + /// series, a candidate is only kept if the server actually has that season. /// Implementations MUST gate only on [ExternalSeasonRef.agreedSeason] — the /// provider a library numbers its seasons by is a server-side setting no /// dataset supplies, so a disagreeing ref is left ungated rather than gated /// on a guess. /// - /// Returns null when this server has no match or [kind] is not movie/show. - /// Used to match external catalog items (Explore tab) back to the user's - /// libraries. - Future findByExternalIds( + /// Returns an empty list when this server has no match or [kind] is not + /// movie/show. Used to match external catalog items (Explore tab) back to + /// the user's libraries. + Future> findByExternalIds( ExternalIds ids, { required MediaKind kind, List titles = const [], diff --git a/lib/media/media_version.dart b/lib/media/media_version.dart index fea88afc..c6503b5b 100644 --- a/lib/media/media_version.dart +++ b/lib/media/media_version.dart @@ -68,6 +68,25 @@ class MediaVersion { /// them when metadata is fetched with `checkFiles=1`. bool get isPlayable => parts.isEmpty || parts.any((part) => part.isPlayable); + /// Approximate vertical resolution, for ordering versions best-first. + /// + /// Plex reports either a numeric height (`"1080"`) or a named tier + /// (`"sd"`, `"4k"`) and usually both; Jellyfin reports [height] directly. + /// Null when the backend gave neither. + int? get resolutionHeight { + final reported = height; + if (reported != null && reported > 0) return reported; + final named = (videoResolution ?? '').trim().toLowerCase(); + return switch (named) { + '' => null, + 'sd' => 480, + 'hd' => 720, + '4k' => 2160, + '8k' => 4320, + _ => int.tryParse(named), + }; + } + /// Display label with detailed information: "1080p H.264 MKV (8.5 Mbps)". /// When [name] is set, it prefixes the technical label so a user can tell /// "Director's Cut · 1080p H.264 MKV" apart from "Theatrical Cut · 1080p diff --git a/lib/screens/catalog_item_detail_screen.dart b/lib/screens/catalog_item_detail_screen.dart index 1af289cd..ef2aecc0 100644 --- a/lib/screens/catalog_item_detail_screen.dart +++ b/lib/screens/catalog_item_detail_screen.dart @@ -19,7 +19,9 @@ import '../i18n/app_locale_utils.dart'; import '../i18n/strings.g.dart'; import '../media/media_hub.dart'; import '../media/media_item.dart'; +import '../media/media_item_merge.dart'; import '../media/media_rating.dart'; +import '../media/media_version.dart'; import '../models/catalog/catalog_cast_member.dart'; import '../models/catalog/catalog_item.dart'; import '../models/catalog/catalog_metadata.dart'; @@ -77,6 +79,12 @@ class _CatalogItemDetailScreenState extends State { List _streamingLinks = const []; List _otherLinks = const []; bool _showSpoilerTags = false; + + /// Focus node per library copy, keyed by [MediaItem.globalKey] so a later + /// resolution pass that adds a copy keeps the nodes — and therefore the + /// focus — of the rows already on screen. [_libraryMatchFocusNodes] is the + /// same nodes in display order, for index-based dpad traversal. + final Map _libraryMatchNodesByKey = {}; List _libraryMatchFocusNodes = const []; CatalogSource? _watchlistSource; SeerrCatalogSource? _requestSource; @@ -135,7 +143,7 @@ class _CatalogItemDetailScreenState extends State { node.dispose(); } _watchlistSource?.watchlistChanges.removeListener(_onWatchlistChanged); - for (final node in _libraryMatchFocusNodes) { + for (final node in _libraryMatchNodesByKey.values) { node.dispose(); } for (final node in _relationFocusNodes) { @@ -150,37 +158,60 @@ class _CatalogItemDetailScreenState extends State { setState(() {}); } - /// 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 { matches = await context.read().match(item); } catch (e) { appLogger.w('Catalog library match failed for ${item.identityKey}', error: e); - matches = const []; + // A failed pass is no evidence about copies an earlier pass already + // found; only claim "not in your library" when nothing has resolved. + if (_matches == null) _mergeMatches(const []); + return; } - if (generation == _matchGeneration) _setMatches(matches); + _mergeMatches(matches); } - void _setMatches(List matches) { + /// Fold a resolution pass into the visible copies. + /// + /// Union, never replace. The bare row item and its detail-enriched form + /// resolve concurrently, and `findByExternalIdsAcrossServers` logs and skips + /// per-server failures — so a later pass can legitimately come back short a + /// server that answered the first one. Replacing would drop rows for copies + /// that are still there. + void _mergeMatches(List matches) { if (!mounted) return; - for (final node in _libraryMatchFocusNodes) { - node.dispose(); + final focused = _libraryMatchNodesByKey.values.firstWhereOrNull((node) => node.hasPrimaryFocus); + final merged = mergeLibraryCopies(_matches ?? const [], matches); + final keys = {for (final match in merged) match.globalKey}; + for (final key in _libraryMatchNodesByKey.keys.toList()) { + if (!keys.contains(key)) _libraryMatchNodesByKey.remove(key)!.dispose(); } _libraryMatchFocusNodes = [ - for (var index = 0; index < matches.length; index++) - FocusNode( - debugLabel: 'catalog_library_match_$index', - onKeyEvent: (node, event) => _handleLibraryMatchKey(index, event), + for (final match in merged) + _libraryMatchNodesByKey.putIfAbsent( + match.globalKey, + () => FocusNode( + debugLabel: 'catalog_library_match_${match.globalKey}', + // Resolved live: merging a pass can reorder the rows, so a + // captured index would steer the wrong copy. + onKeyEvent: (node, event) { + final index = _libraryMatchFocusNodes.indexOf(node); + return index < 0 ? KeyEventResult.ignored : _handleLibraryMatchKey(index, event); + }, + ), ), ]; - setState(() => _matches = matches); + setState(() => _matches = merged); + // A merge re-sorts, and SettingsGroup shapes its cards by list index, so + // the tile holding the focused node can be rebuilt from scratch and drop + // it. Reclaim it rather than dumping a dpad user on another copy. + if (focused != null) { + WidgetsBinding.instance.addPostFrameCallback((_) { + if (!mounted || focused.hasPrimaryFocus || focused.context == null) return; + focused.requestFocus(); + }); + } } CatalogSource? get _ownSource => @@ -232,11 +263,14 @@ class _CatalogItemDetailScreenState extends State { _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. + // the detail body brings the external ids (#1715). Ask again with the + // full set whenever enrichment added id forms — not just when the bare + // lookup came back empty: the exact `plex://` guid finds only copies in + // libraries on the modern agent, while a legacy-agent sibling is + // reachable solely through the imdb/tmdb forms (#1754). The result + // merges, so a re-ask can only add copies. final gainedIds = !widget.item.ids.allKeys.toSet().containsAll(detail.item.ids.allKeys); - if (gainedIds && (_matches?.isEmpty ?? true)) { + if (gainedIds) { unawaited(_resolveMatches(detail.item)); } } catch (e) { @@ -500,14 +534,30 @@ class _CatalogItemDetailScreenState extends State { } } + /// The technical label of a copy's best version, e.g. "4K HEVC MKV + /// (45.0 Mbps)". Library names are user-chosen and need not mention + /// resolution, which is exactly the question two copies of one movie pose + /// (#1754), so the row states it outright. Omitted when the backend + /// reported no resolution at all, rather than showing "Unknown". + static String? _libraryMatchQuality(MediaItem match) { + MediaVersion? best; + for (final version in match.mediaVersions ?? const []) { + if (version.resolutionHeight == null) continue; + if (best == null || version.resolutionHeight! > best.resolutionHeight!) best = version; + } + return best?.displayLabel; + } + Widget _buildLibraryMatchTile(MediaItem match, int index) { + // Plex matches carry their library title; Jellyfin's search-based lookup + // only does when the ancestors call succeeded, so fall back to the server + // name alone. The subtitle carries whatever else tells two copies apart. + final details = [?_libraryMatchQuality(match), ?(match.libraryTitle == null ? null : match.serverName)]; return FocusableListTile( focusNode: _libraryMatchFocusNodes[index], leading: BackendBadge(backend: match.backend, size: 24), - // Plex matches carry their library title; Jellyfin's search-based - // lookup doesn't, so fall back to the server name alone. title: Text(match.libraryTitle ?? match.serverName ?? match.backend.name), - subtitle: match.libraryTitle != null && match.serverName != null ? Text(match.serverName!) : null, + subtitle: details.isEmpty ? null : Text(details.join(' • ')), trailing: const AppIcon(Symbols.chevron_right_rounded, fill: 1), onTap: () => unawaited(navigateToMediaItemDetails(context, match)), ); diff --git a/lib/services/data_aggregation_service.dart b/lib/services/data_aggregation_service.dart index 16f6aba1..09af55b7 100644 --- a/lib/services/data_aggregation_service.dart +++ b/lib/services/data_aggregation_service.dart @@ -3,6 +3,7 @@ import '../media/ids.dart'; import '../media/media_hub.dart'; import '../media/media_item.dart'; +import '../media/media_item_merge.dart'; import '../media/media_kind.dart'; import '../media/media_library.dart'; import '../media/media_server_client.dart'; @@ -594,6 +595,15 @@ class DataAggregationService { /// Reverse external-id lookup fanned out to every online server (see /// [MediaServerClient.findByExternalIds]). One request wave per tap on an /// Explore catalog item; per-server failures are logged and skipped. + /// + /// Every server contributes every copy it holds, not one apiece: the same + /// movie routinely sits in a 4K library and an HD library on one server + /// (#1754). Results are deduped by global key and ordered best-first with + /// [compareLibraryCopies] so the chooser is stable across repeated passes. + /// + /// Because per-server failures are dropped here, a caller holding earlier + /// results must merge rather than replace (see [mergeLibraryCopies]) — a + /// degraded wave is not evidence that a copy went away. Future> findByExternalIdsAcrossServers( ExternalIds ids, { required MediaKind kind, @@ -618,11 +628,11 @@ class DataAggregationService { ); } catch (e, st) { appLogger.w('External-id lookup failed on ${entry.key}', error: e, stackTrace: st); - return null; + return const []; } }); - return (await Future.wait(futures)).nonNulls.toList(); + return mergeLibraryCopies(const [], (await Future.wait(futures)).expand((items) => items)); } /// Group libraries by server (internal aggregation helper). diff --git a/lib/services/jellyfin_client/parts/browse.dart b/lib/services/jellyfin_client/parts/browse.dart index fd81318d..2fe24d65 100644 --- a/lib/services/jellyfin_client/parts/browse.dart +++ b/lib/services/jellyfin_client/parts/browse.dart @@ -1115,12 +1115,16 @@ mixin _JellyfinBrowseMethods on _JellyfinClientInternals { /// each candidate's inline `ProviderIds`. [plexGuid] is a Plex-only hint and /// has no meaning in Jellyfin's provider-id model, so it is ignored. /// + /// Every id-verified candidate of the first matching title is returned, not + /// just the first: one movie can sit in both a 4K library and an HD library + /// as two separate items, and the caller shows the user each copy (#1754). + /// /// Jellyfin cannot report season ordering to a non-admin: on 10.11.10, /// `/Library/VirtualFolders` returns 403 and Series items omit /// `DisplayOrder`. Resolve against an unknown provider rather than guessing /// from `ProviderIds`, so only seasons on which TVDB and TMDB agree are gated. @override - Future findByExternalIds( + Future> findByExternalIds( ExternalIds ids, { required MediaKind kind, List titles = const [], @@ -1133,7 +1137,7 @@ mixin _JellyfinBrowseMethods on _JellyfinClientInternals { MediaKind.show => 'Series', _ => null, }; - if (itemType == null || !ids.hasAny || titles.isEmpty) return null; + if (itemType == null || !ids.hasAny || titles.isEmpty) return const []; final seasonIndex = season?.agreedSeason; final shouldGateSeason = kind == MediaKind.show && seasonIndex != null && seasonIndex > 1; @@ -1155,20 +1159,34 @@ mixin _JellyfinBrowseMethods on _JellyfinClientInternals { 'years': ?years, ...jellyfinImageQueryParameters, }); - final match = ExternalIds.jellyfinCandidateMatching(candidates, ids); - if (match == null) continue; - final item = _mapItems([match]).firstOrNull; - if (item == null) continue; + final matches = _mapItems(ExternalIds.jellyfinCandidatesMatching(candidates, ids)); + if (matches.isEmpty) continue; - if (shouldGateSeason) { - final children = await fetchChildren(item.id); - if (!children.any((child) => child.kind == MediaKind.season && child.index == seasonIndex)) { - return null; - } - } - return _withLibraryFromAncestors(item); + final kept = shouldGateSeason ? await _keepMatchesWithSeason(matches, seasonIndex) : matches; + // A title that verified but has no season-gated survivor is a definitive + // "this server has the show, just not that season"; broader title forms + // would only reach other shows. + if (kept.isEmpty) return const []; + return Future.wait([for (final item in kept) _withLibraryFromAncestors(item)]); } - return null; + return const []; + } + + /// Keep only the series that actually have [seasonIndex]. One + /// `fetchChildren` per candidate, issued concurrently because the match list + /// is deliberately never truncated (see + /// [MediaServerClient.findByExternalIds]). + Future> _keepMatchesWithSeason(List items, int seasonIndex) async { + final kept = await Future.wait([ + for (final item in items) + fetchChildren( + item.id, + ).then((children) => children.any((child) => child.kind == MediaKind.season && child.index == seasonIndex)), + ]); + return [ + for (var index = 0; index < items.length; index++) + if (kept[index]) items[index], + ]; } /// Best-effort library stamp for items found outside a library context diff --git a/lib/services/plex_client.dart b/lib/services/plex_client.dart index c2375350..d8339778 100644 --- a/lib/services/plex_client.dart +++ b/lib/services/plex_client.dart @@ -844,6 +844,16 @@ class PlexClient return null; } + /// Every raw `Metadata` entry of a container, for callers that must inspect + /// each sibling rather than assume the first is the only one (the + /// external-id reverse lookup: `/library/all` is server-wide, so one movie + /// held by two libraries answers with two entries). + List> _getMetadataJsonList(MediaServerResponse response) { + final metadata = _getMediaContainer(response)?['Metadata']; + if (metadata is! List) return const []; + return metadata.whereType>().toList(); + } + List _extractDirectoryList(MediaServerResponse response, T Function(Map) fromJson) { final container = _getMediaContainer(response); if (container != null && container['Directory'] != null) { @@ -3832,38 +3842,67 @@ class PlexClient return ExternalIds.fromGuids(guids); } - /// Drop a sequel match when the server does not actually have that season. + /// Map id-verified candidates to items, dropping any sequel the server does + /// not actually have that season of. /// /// Only an [ExternalSeasonRef.agreedSeason] is gated on. Which provider a /// library numbers its seasons by is a server-side setting no dataset can /// supply, and reading it costs two extra requests per lookup, so a /// disagreeing ref is left ungated rather than gated on a guess. - Future _applyExternalIdSeasonGate( - Map metadata, { + /// + /// Gating costs one `fetchChildren` per show candidate. The candidate list + /// is deliberately never truncated (see + /// [MediaServerClient.findByExternalIds]), so the gates run concurrently + /// rather than letting latency grow with the number of library copies. + Future> _gateExternalIdMatches( + Iterable> candidates, { required MediaKind kind, required ExternalSeasonRef? season, }) async { - final item = PlexMappers.mediaItem(_createTaggedMetadataWithLibrary(metadata)); - if (kind != MediaKind.show) return item; - + final entries = [ + for (final metadata in candidates) + (metadata: metadata, item: PlexMappers.mediaItem(_createTaggedMetadataWithLibrary(metadata))), + ]; final seasonIndex = season?.agreedSeason; - if (seasonIndex == null || seasonIndex <= 1) return item; + if (kind != MediaKind.show || seasonIndex == null || seasonIndex <= 1) { + return [for (final entry in entries) entry.item]; + } - final ratingKey = metadata['ratingKey']?.toString(); - // Cannot ask the question => do not gate. - if (ratingKey == null || ratingKey.isEmpty) return item; - - final children = await fetchChildren(ratingKey); - return children.any((child) => child.kind == MediaKind.season && child.index == seasonIndex) ? item : null; + final kept = await Future.wait([for (final entry in entries) _hasSeason(entry.metadata, seasonIndex)]); + return [ + for (var index = 0; index < entries.length; index++) + if (kept[index]) entries[index].item, + ]; } - /// Server-wide title filter + client-side external-ID verification. Plex's - /// `guid=` field filter matches only the item's primary `plex://` guid - /// (verified against PMS 1.43), so external ids in modern `Guid` arrays are - /// verified after title search. A resolved primary [plexGuid] can use the - /// exact server-side filter directly. + Future _hasSeason(Map metadata, int seasonIndex) async { + final ratingKey = metadata['ratingKey']?.toString(); + // Cannot ask the question => do not gate. + if (ratingKey == null || ratingKey.isEmpty) return true; + final children = await fetchChildren(ratingKey); + return children.any((child) => child.kind == MediaKind.season && child.index == seasonIndex); + } + + /// Server-wide external-id reverse lookup. Plex's `guid=` field filter + /// matches only the item's primary `plex://` guid (verified against PMS + /// 1.43), so a resolved [plexGuid] uses that exact filter while ids in + /// modern `Guid` arrays are verified client-side after a title search. + /// + /// `/library/all` is server-wide — it is not scoped to a section — so a + /// movie held by both a 4K and an HD library answers as two sibling + /// `Metadata` entries, each carrying its own `librarySectionID`. Every + /// id-verified entry is kept (#1754). + /// + /// An exact-guid hit does not short-circuit the title ladder: a library + /// still on a legacy agent carries `com.plexapp.agents.*` as its primary + /// guid, so that copy is invisible to the `guid=` filter and only the + /// id-verified title search finds it. Likewise the year-filtered page can + /// surface a copy the unfiltered page cut off at the container size, so + /// both contribute. The extra requests are spent once per uncached lookup, + /// off the render path and memoized for the session by + /// `CatalogLibraryMatcher`. @override - Future findByExternalIds( + Future> findByExternalIds( ExternalIds ids, { required MediaKind kind, List titles = const [], @@ -3876,82 +3915,88 @@ class PlexClient MediaKind.show => 2, _ => null, }; - if (plexType == null) return null; - if (!ids.hasAny && plexGuid == null) return null; - if (titles.isEmpty && plexGuid == null) return null; + if (plexType == null) return const []; + if (!ids.hasAny && plexGuid == null) return const []; + if (titles.isEmpty && plexGuid == null) return const []; + + // Rating-key keyed so the guid filter and the title ladder can both + // contribute without doubling a copy they agree on. Kept in three buckets + // because the result order is exact-guid hits, then modern `Guid` + // matches, then legacy-agent ones. + final exact = >{}; + final modern = >{}; + final legacy = >{}; + + void collect(Map> into, Map item) { + final ratingKey = item['ratingKey']?.toString(); + if (ratingKey == null || ratingKey.isEmpty) return; + into.putIfAbsent(ratingKey, () => item); + } if (plexGuid != null) { final response = await _getWithFailover( '/library/all', queryParameters: {'guid': plexGuid, 'type': plexType, 'includeGuids': 1}, ); - final metadata = _getFirstMetadataJson(response); - if (metadata != null) { - return _applyExternalIdSeasonGate(metadata, kind: kind, season: season); + for (final item in _getMetadataJsonList(response)) { + collect(exact, item); } } // 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, - String? years, - }) async { - final response = await _getWithFailover( - '/library/all', - queryParameters: { - 'title': title, - 'type': plexType, - 'includeGuids': 1, - 'X-Plex-Container-Size': size, - 'year': ?years, - }, - ); - final container = _getMediaContainer(response); - final metadata = container?['Metadata']; - if (metadata is! List) return (modern: null, legacy: null); - - Map? legacy; - for (final item in metadata) { - if (item is! Map) continue; - final guids = item['Guid']; - if (guids is List && ids.intersects(ExternalIds.fromGuids(guids))) { - return (modern: item, legacy: legacy); - } - if (legacy == null && ids.intersects(ExternalIds.fromLegacyPlexGuid(item['guid']))) { - legacy = item; - } - } - return (modern: null, legacy: legacy); - } - - // Not `resolve(null)`: when the two providers disagree the season number is - // unresolvable but the entry is still a sequel, and the ±1 window around a - // sequel's own year excludes the parent show (its year is season one's). - final skipYearWindow = season?.isSequel ?? false; - for (var index = 0; index < titles.length; index++) { - final title = titles[index]; - final size = index == 0 ? 20 : 50; - ({Map? modern, Map? legacy})? filtered; - if (index == 0 && year != null && !skipYearWindow) { - filtered = await attempt(title, size: size, years: '${year - 1},$year,${year + 1}'); - final modern = filtered.modern; - if (modern != null) { - return _applyExternalIdSeasonGate(modern, kind: kind, season: season); + if (ids.hasAny) { + Future attempt(String title, {required int size, String? years}) async { + final response = await _getWithFailover( + '/library/all', + queryParameters: { + 'title': title, + 'type': plexType, + 'includeGuids': 1, + 'X-Plex-Container-Size': size, + 'year': ?years, + }, + ); + var matched = false; + for (final item in _getMetadataJsonList(response)) { + final guids = item['Guid']; + if (guids is List && ids.intersects(ExternalIds.fromGuids(guids))) { + collect(modern, item); + matched = true; + } else if (ids.intersects(ExternalIds.fromLegacyPlexGuid(item['guid']))) { + collect(legacy, item); + matched = true; + } } + return matched; } - final unfiltered = await attempt(title, size: size); - final match = unfiltered.modern ?? filtered?.legacy ?? unfiltered.legacy; - if (match != null) { - return _applyExternalIdSeasonGate(match, kind: kind, season: season); + // Not `resolve(null)`: when the two providers disagree the season number is + // unresolvable but the entry is still a sequel, and the ±1 window around a + // sequel's own year excludes the parent show (its year is season one's). + final skipYearWindow = season?.isSequel ?? false; + for (var index = 0; index < titles.length; index++) { + final title = titles[index]; + final size = index == 0 ? 20 : 50; + final filteredMatched = index == 0 && year != null && !skipYearWindow + ? await attempt(title, size: size, years: '${year - 1},$year,${year + 1}') + : false; + final unfilteredMatched = await attempt(title, size: size); + // The ladder exists to widen a title that matched nothing; once a + // title has produced copies, broader forms would only add other shows. + if (filteredMatched || unfilteredMatched) break; } } - return null; + + final ordered = >{...exact}; + for (final bucket in [modern, legacy]) { + for (final entry in bucket.entries) { + ordered.putIfAbsent(entry.key, () => entry.value); + } + } + if (ordered.isEmpty) return const []; + return _gateExternalIdMatches(ordered.values, kind: kind, season: season); } @override diff --git a/lib/utils/external_ids.dart b/lib/utils/external_ids.dart index 2a0e1556..003f4b8f 100644 --- a/lib/utils/external_ids.dart +++ b/lib/utils/external_ids.dart @@ -153,17 +153,19 @@ class ExternalIds { return allowBareDigits && _decimalId.hasMatch(normalized) ? 'tt$normalized' : null; } - /// Pick the first raw Jellyfin item whose inline `ProviderIds` intersect - /// [ids]. Pure helper so the reverse-lookup verification stays + /// Every raw Jellyfin item whose inline `ProviderIds` intersect [ids], in + /// response order. Pure helper so the reverse-lookup verification stays /// unit-testable (its call site lives in a part file). - static Map? jellyfinCandidateMatching(List> candidates, ExternalIds ids) { - for (final item in candidates) { - final providerIds = item['ProviderIds']; - if (providerIds is! Map) continue; - final candidate = ExternalIds.fromJellyfinProviderIds(providerIds.cast()); - if (ids.intersects(candidate)) return item; - } - return null; + /// + /// Plural because one title can own several library items — a 4K library + /// and an HD library hold separate items for the same movie (#1754) — and + /// the caller shows the user every copy. + static List> jellyfinCandidatesMatching(List> candidates, ExternalIds ids) { + return [ + for (final item in candidates) + if (item['ProviderIds'] case final Map providerIds) + if (ids.intersects(ExternalIds.fromJellyfinProviderIds(providerIds.cast()))) item, + ]; } /// Build from a Jellyfin `ProviderIds` map. Jellyfin stores external IDs diff --git a/test/media/media_item_merge_test.dart b/test/media/media_item_merge_test.dart index 843ca49d..3cab6da4 100644 --- a/test/media/media_item_merge_test.dart +++ b/test/media/media_item_merge_test.dart @@ -4,6 +4,7 @@ import 'package:plezy/media/media_backend.dart'; import 'package:plezy/media/media_item.dart'; import 'package:plezy/media/media_item_merge.dart'; import 'package:plezy/media/media_kind.dart'; +import 'package:plezy/media/media_version.dart'; import '../test_helpers/media_items.dart'; void main() { @@ -47,4 +48,69 @@ void main() { expect(merged.libraryId, 'old-lib'); expect(merged.libraryTitle, 'Old'); }); + + MediaItem copy({ + required String id, + String? libraryTitle, + String? videoResolution, + List? mediaVersions, + }) => testMediaItem( + id: id, + serverId: 'server-1', + serverName: 'Living Room', + libraryId: libraryTitle == null ? null : '$id-lib', + libraryTitle: libraryTitle, + mediaVersions: + mediaVersions ?? + (videoResolution == null ? null : [MediaVersion(id: '$id-v', videoResolution: videoResolution)]), + ); + + test('orders library copies by resolution, best first', () { + final merged = mergeLibraryCopies(const [], [ + copy(id: 'hd', libraryTitle: 'Movies', videoResolution: '1080'), + copy(id: 'uhd', libraryTitle: '4K Movies', videoResolution: '4k'), + ]); + + expect(merged.map((item) => item.id), ['uhd', 'hd']); + }); + + test('keeps copies a later pass did not return', () { + final merged = mergeLibraryCopies([copy(id: 'hd', libraryTitle: 'Movies')], const []); + + expect(merged.map((item) => item.id), ['hd']); + }); + + test('an unstamped re-resolve does not strip a copy of its library', () { + // Jellyfin's library stamp is a best-effort ancestors lookup that returns + // the item unstamped when it fails. Losing the title would make this copy + // indistinguishable from its sibling in the server's other library. + final merged = mergeLibraryCopies( + [copy(id: 'hd', libraryTitle: 'Movies', videoResolution: '1080')], + [copy(id: 'hd')], + ); + + expect(merged.single.libraryId, 'hd-lib'); + expect(merged.single.libraryTitle, 'Movies'); + expect(merged.single.serverName, 'Living Room'); + expect(merged.single.mediaVersions?.single.videoResolution, '1080'); + }); + + test('a re-resolve carrying fresher library context wins', () { + final merged = mergeLibraryCopies( + [copy(id: 'hd', libraryTitle: 'Movies', videoResolution: '1080')], + [copy(id: 'hd', libraryTitle: 'Renamed Movies', videoResolution: '4k')], + ); + + expect(merged.single.libraryTitle, 'Renamed Movies'); + expect(merged.single.mediaVersions?.single.videoResolution, '4k'); + }); + + test('an empty version list is treated as absent, not as a downgrade', () { + final merged = mergeLibraryCopies( + [copy(id: 'hd', libraryTitle: 'Movies', videoResolution: '1080')], + [copy(id: 'hd', libraryTitle: 'Movies', mediaVersions: const [])], + ); + + expect(merged.single.mediaVersions?.single.videoResolution, '1080'); + }); } diff --git a/test/screens/catalog_item_detail_screen_test.dart b/test/screens/catalog_item_detail_screen_test.dart index 7c28b2aa..7e43d062 100644 --- a/test/screens/catalog_item_detail_screen_test.dart +++ b/test/screens/catalog_item_detail_screen_test.dart @@ -10,6 +10,7 @@ import 'package:plezy/i18n/strings.g.dart'; import 'package:plezy/media/media_kind.dart'; import 'package:plezy/media/media_rating.dart'; import 'package:plezy/media/media_item.dart'; +import 'package:plezy/media/media_version.dart'; 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'; @@ -24,6 +25,7 @@ import 'package:plezy/theme/mono_theme.dart'; import 'package:plezy/utils/platform_detector.dart'; import 'package:plezy/widgets/overlay_sheet.dart'; import 'package:plezy/widgets/hub_section.dart'; +import 'package:plezy/widgets/focusable_list_tile.dart'; import 'package:plezy/widgets/media_card.dart'; import 'package:plezy/widgets/optimized_media_image.dart'; import 'package:provider/provider.dart'; @@ -146,6 +148,53 @@ class _ExternalIdGatedMatcher extends CatalogLibraryMatcher { } } +/// Serves one scripted result per `match` call, so a test can model the bare +/// row lookup and the detail-enriched lookup independently. +class _ScriptedMatcher extends CatalogLibraryMatcher { + _ScriptedMatcher(super.multiServer, this.passes); + + final List Function()> passes; + int calls = 0; + + @override + Future> match(CatalogItem item) async { + final pass = passes[calls < passes.length ? calls : passes.length - 1]; + calls++; + return pass(); + } +} + +/// A Plex Discover row whose bare form carries only its own id; the detail +/// body adds the external ids (#1715), which is what triggers a second pass. +const _bareRow = CatalogItem( + source: CatalogSourceId.trakt, + kind: MediaKind.movie, + title: 'Row-only Movie', + ids: CatalogItemIds(trakt: 5), +); +const _enrichedRow = CatalogItem( + source: CatalogSourceId.trakt, + kind: MediaKind.movie, + title: 'Row-only Movie', + ids: CatalogItemIds(trakt: 5, tmdb: 99), +); + +MediaItem _libraryCopy({ + required String id, + String? libraryTitle, + String? videoResolution, + String? serverName = 'Living Room', +}) => testMediaItem( + id: id, + serverId: 'server-1', + serverName: serverName, + libraryId: libraryTitle == null ? null : id, + libraryTitle: libraryTitle, + mediaVersions: videoResolution == null + ? null + : [MediaVersion(id: '$id-v', videoResolution: videoResolution, videoCodec: 'hevc', container: 'mkv')], +); + const _item = CatalogItem( source: CatalogSourceId.trakt, kind: MediaKind.movie, @@ -284,6 +333,159 @@ void main() { expect(find.text('Movies'), findsOneWidget); }); + testWidgets('lists every library copy of one title, best quality first', (tester) async { + // #1754: one movie held by both a 4K library and an HD library on the same + // server. Library names are user-chosen, so each row also states the + // resolution the user is actually choosing between. + final source = _FakeCatalogSource(); + + await _pumpDetail( + tester, + source, + matches: [ + _libraryCopy(id: 'hd-copy', libraryTitle: 'Movies', videoResolution: '1080'), + _libraryCopy(id: 'uhd-copy', libraryTitle: '4K Movies', videoResolution: '4k'), + ], + ); + + expect(find.text(t.explore.inTheseLibraries), findsOneWidget); + expect(find.text('Movies'), findsOneWidget); + expect(find.text('4K Movies'), findsOneWidget); + + final subtitles = tester.widgetList(find.textContaining('Living Room')).map((text) => text.data!).toList(); + expect(subtitles, hasLength(2)); + expect(subtitles.first, contains('4K'), reason: 'the 4K copy sorts above the HD one'); + expect(subtitles.last, contains('1080p')); + }); + + testWidgets('a re-resolve that comes back short keeps the copies already found', (tester) async { + // The cross-server fan-out logs and skips per-server failures, so a later + // pass can answer without a server that replied to the first one. Those + // rows are still valid and must not be wiped. + late _ScriptedMatcher matcher; + final source = _FakeCatalogSource(detail: const CatalogDetail(item: _enrichedRow)); + + await _pumpDetail( + tester, + source, + item: _bareRow, + matcherBuilder: (multiServer) => matcher = _ScriptedMatcher(multiServer, [ + () => [_libraryCopy(id: 'hd-copy', libraryTitle: 'Movies')], + () => const [], + ]), + ); + + expect(matcher.calls, 2); + expect(find.text(t.explore.inTheseLibraries), findsOneWidget); + expect(find.text('Movies'), findsOneWidget); + expect(find.text(t.explore.notInLibrary), findsNothing); + }); + + testWidgets('a failed re-resolve does not claim the title left the library', (tester) async { + late _ScriptedMatcher matcher; + final source = _FakeCatalogSource(detail: const CatalogDetail(item: _enrichedRow)); + + await _pumpDetail( + tester, + source, + item: _bareRow, + matcherBuilder: (multiServer) => matcher = _ScriptedMatcher(multiServer, [ + () => [_libraryCopy(id: 'hd-copy', libraryTitle: 'Movies')], + () => throw StateError('server unreachable'), + ]), + ); + + expect(matcher.calls, 2); + expect(find.text('Movies'), findsOneWidget); + expect(find.text(t.explore.notInLibrary), findsNothing); + }); + + testWidgets('a re-resolve that lost its library stamp keeps the one already shown', (tester) async { + // Jellyfin stamps a copy's library with a best-effort ancestors lookup + // that returns the item bare when it fails. A row that fell back to the + // server name would be indistinguishable from its sibling in the same + // server's other library. + late _ScriptedMatcher matcher; + final source = _FakeCatalogSource(detail: const CatalogDetail(item: _enrichedRow)); + + await _pumpDetail( + tester, + source, + item: _bareRow, + matcherBuilder: (multiServer) => matcher = _ScriptedMatcher(multiServer, [ + () => [_libraryCopy(id: 'hd-copy', libraryTitle: 'Movies', videoResolution: '1080')], + () => [_libraryCopy(id: 'hd-copy', serverName: null)], + ]), + ); + + expect(matcher.calls, 2); + expect(find.text('Movies'), findsOneWidget); + final subtitles = tester.widgetList(find.textContaining('Living Room')).map((text) => text.data!); + expect(subtitles.single, contains('1080p'), reason: 'the quality hint survives an unstamped re-resolve too'); + }); + + testWidgets('a re-resolve that finds another library adds it to the list', (tester) async { + // The exact `plex://` guid only sees libraries on the modern agent; a + // legacy-agent sibling arrives with the enriched imdb/tmdb pass (#1754). + // The first pass being non-empty must not suppress the second. + late _ScriptedMatcher matcher; + final source = _FakeCatalogSource(detail: const CatalogDetail(item: _enrichedRow)); + + await _pumpDetail( + tester, + source, + item: _bareRow, + matcherBuilder: (multiServer) => matcher = _ScriptedMatcher(multiServer, [ + () => [_libraryCopy(id: 'uhd-copy', libraryTitle: '4K Movies', videoResolution: '4k')], + () => [ + _libraryCopy(id: 'uhd-copy', libraryTitle: '4K Movies', videoResolution: '4k'), + _libraryCopy(id: 'hd-copy', libraryTitle: 'Movies', videoResolution: '1080'), + ], + ]), + ); + + expect(matcher.calls, 2); + expect(find.text('4K Movies'), findsOneWidget, reason: 'the copy both passes agree on is not doubled'); + expect(find.text('Movies'), findsOneWidget); + }); + + testWidgets('focus stays on a library copy when a later pass adds one above it', (tester) async { + // Merging re-sorts, so the rows can move. Focus nodes are keyed by the + // copy, not by row index, or a dpad user would be thrown to another copy. + final detailCompleter = Completer(); + final source = _FakeCatalogSource(detailCompleter: detailCompleter); + + await _pumpDetail( + tester, + source, + item: _bareRow, + matcherBuilder: (multiServer) => _ScriptedMatcher(multiServer, [ + () => [_libraryCopy(id: 'hd-copy', libraryTitle: 'Movies', videoResolution: '1080')], + () => [ + _libraryCopy(id: 'hd-copy', libraryTitle: 'Movies', videoResolution: '1080'), + _libraryCopy(id: 'uhd-copy', libraryTitle: '4K Movies', videoResolution: '4k'), + ], + ]), + ); + + final tile = tester.widget( + find.ancestor(of: find.text('Movies'), matching: find.byType(FocusableListTile)), + ); + tile.focusNode!.requestFocus(); + await tester.pump(); + expect(FocusManager.instance.primaryFocus?.debugLabel, 'catalog_library_match_server-1:hd-copy'); + + detailCompleter.complete(const CatalogDetail(item: _enrichedRow)); + await tester.pumpAndSettle(); + + expect(find.text('4K Movies'), findsOneWidget); + expect( + FocusManager.instance.primaryFocus?.debugLabel, + 'catalog_library_match_server-1:hd-copy', + reason: 'the 4K copy sorted above the focused HD copy without stealing focus', + ); + }); + testWidgets('fetchDetail failure leaves the opening item rendered', (tester) async { final source = _FakeCatalogSource(detailError: StateError('detail unavailable')); @@ -832,19 +1034,31 @@ void main() { addTearDown(tester.view.resetDevicePixelRatio); addTearDown(tester.view.resetPhysicalSize); + // Copies render best-first, so the 4K one leads whatever order the + // matcher returned them in. final matches = [ - testMediaItem(id: 'match_1', libraryTitle: 'Movies', serverName: 'Living Room'), - testMediaItem(id: 'match_2', libraryTitle: 'Favorites', serverName: 'Bedroom'), + testMediaItem( + id: 'match_1', + libraryTitle: 'Movies', + serverName: 'Living Room', + mediaVersions: const [MediaVersion(id: 'v1', videoResolution: '4k')], + ), + testMediaItem( + id: 'match_2', + libraryTitle: 'Favorites', + serverName: 'Bedroom', + mediaVersions: const [MediaVersion(id: 'v2', videoResolution: '1080')], + ), ]; await _pumpDetail(tester, _FakeCatalogSource(), matches: matches); await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown); await tester.pumpAndSettle(); - expect(FocusManager.instance.primaryFocus?.debugLabel, 'catalog_library_match_0'); + expect(FocusManager.instance.primaryFocus?.debugLabel, 'catalog_library_match_match_1'); await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown); await tester.pumpAndSettle(); - expect(FocusManager.instance.primaryFocus?.debugLabel, 'catalog_library_match_1'); + expect(FocusManager.instance.primaryFocus?.debugLabel, 'catalog_library_match_match_2'); await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown); await tester.pumpAndSettle(); @@ -852,7 +1066,7 @@ void main() { await tester.sendKeyEvent(LogicalKeyboardKey.arrowUp); await tester.pumpAndSettle(); - expect(FocusManager.instance.primaryFocus?.debugLabel, 'catalog_library_match_1'); + expect(FocusManager.instance.primaryFocus?.debugLabel, 'catalog_library_match_match_2'); }); testWidgets('pending watchlist action keeps initial focus and its press retries the snapshot', (tester) async { diff --git a/test/services/jellyfin_external_id_lookup_test.dart b/test/services/jellyfin_external_id_lookup_test.dart index b3f3ee1d..dc619234 100644 --- a/test/services/jellyfin_external_id_lookup_test.dart +++ b/test/services/jellyfin_external_id_lookup_test.dart @@ -53,16 +53,17 @@ void main() { ); addTearDown(client.close); - final match = await client.findByExternalIds( + final matches = await client.findByExternalIds( const ExternalIds(tmdb: 42), kind: MediaKind.show, titles: const ['Parent Series Season 2', 'Parent Series'], ); expect(searchTerms, ['Parent Series Season 2', 'Parent Series']); - expect(match?.id, 'series-1'); - expect(match?.libraryId, 'library-1'); - expect(match?.libraryTitle, 'Shows'); + expect(matches, hasLength(1)); + expect(matches.single.id, 'series-1'); + expect(matches.single.libraryId, 'library-1'); + expect(matches.single.libraryTitle, 'Shows'); }); test('rejects a title hit whose provider ids do not intersect', () async { @@ -76,13 +77,13 @@ void main() { ); addTearDown(client.close); - final match = await client.findByExternalIds( + final matches = await client.findByExternalIds( const ExternalIds(tmdb: 42), kind: MediaKind.show, titles: const ['Parent Series'], ); - expect(match, isNull); + expect(matches, isEmpty); }); test('uses the year window only for the first title and broadens the later limit', () async { @@ -102,14 +103,14 @@ void main() { ); addTearDown(client.close); - final match = await client.findByExternalIds( + final matches = await client.findByExternalIds( const ExternalIds(tmdb: 42), kind: MediaKind.show, titles: const ['Parent Series Season 2', 'Parent Series'], year: 2024, ); - expect(match?.id, 'series-1'); + expect(matches.map((item) => item.id), ['series-1']); expect(searches, hasLength(2)); expect(searches.first.queryParameters['years'], '2023,2024,2025'); expect(searches.first.queryParameters['Limit'], '20'); @@ -118,7 +119,7 @@ void main() { }); test('requires an agreed sequel season to exist in the matched series', () async { - Future lookupWithSeasons(List seasonNumbers) async { + Future> lookupWithSeasons(List seasonNumbers) async { final client = testJellyfinClient( httpClient: MockClient((request) async { if (request.url.path == '/Items') { @@ -140,18 +141,18 @@ void main() { ); addTearDown(client.close); - final match = await client.findByExternalIds( + final matches = await client.findByExternalIds( const ExternalIds(tmdb: 42), kind: MediaKind.show, titles: const ['Parent Series'], year: 2024, season: const ExternalSeasonRef(tvdb: 2, tmdb: 2), ); - return match?.id; + return [for (final item in matches) item.id]; } - expect(await lookupWithSeasons([1]), isNull); - expect(await lookupWithSeasons([1, 2]), 'series-1'); + expect(await lookupWithSeasons([1]), isEmpty); + expect(await lookupWithSeasons([1, 2]), ['series-1']); }); test('does not gate when TVDB and TMDB seasons disagree and Jellyfin order is unknown', () async { @@ -168,13 +169,61 @@ void main() { ); addTearDown(client.close); - final match = await client.findByExternalIds( + final matches = await client.findByExternalIds( const ExternalIds(tmdb: 42), kind: MediaKind.show, titles: const ['Parent Series'], season: const ExternalSeasonRef(tvdb: 2, tmdb: 1), ); - expect(match?.id, 'series-1'); + expect(matches.map((item) => item.id), ['series-1']); + }); + + test('returns every library copy of a title, each stamped with its own library', () async { + final client = testJellyfinClient( + httpClient: MockClient((request) async { + if (request.url.path == '/Items') { + return _json({ + 'Items': [ + { + 'Id': 'movie-4k', + 'Type': 'Movie', + 'Name': 'The Matrix', + 'ProviderIds': {'Tmdb': '603'}, + }, + { + 'Id': 'movie-hd', + 'Type': 'Movie', + 'Name': 'The Matrix', + 'ProviderIds': {'Tmdb': '603'}, + }, + ], + }); + } + if (request.url.path == '/Items/movie-4k/Ancestors') { + return _json([ + {'Id': 'library-4k', 'Name': 'Movies 4K', 'Type': 'CollectionFolder'}, + ]); + } + if (request.url.path == '/Items/movie-hd/Ancestors') { + return _json([ + {'Id': 'library-hd', 'Name': 'Movies', 'Type': 'CollectionFolder'}, + ]); + } + fail('Unexpected request: ${request.url}'); + }), + ); + addTearDown(client.close); + + final matches = await client.findByExternalIds( + const ExternalIds(tmdb: 603), + kind: MediaKind.movie, + titles: const ['The Matrix'], + ); + + expect(matches.map((item) => (item.id, item.libraryId, item.libraryTitle)), [ + ('movie-4k', 'library-4k', 'Movies 4K'), + ('movie-hd', 'library-hd', 'Movies'), + ]); }); } diff --git a/test/services/plex_external_id_lookup_test.dart b/test/services/plex_external_id_lookup_test.dart index 28a3975b..c208e8db 100644 --- a/test/services/plex_external_id_lookup_test.dart +++ b/test/services/plex_external_id_lookup_test.dart @@ -50,17 +50,17 @@ void main() { ); addTearDown(client.close); - final match = await client.findByExternalIds( + final matches = await client.findByExternalIds( const ExternalIds(imdb: 'tt29768334'), kind: MediaKind.movie, titles: const ['Legacy Movie'], ); - expect(match?.id, 'legacy-movie'); - expect(match?.libraryId, '5'); - expect(match?.libraryTitle, 'Legacy Movies'); - expect(match?.serverId, 'server-1'); - expect(match?.serverName, 'Server'); + expect(matches.map((match) => match.id), ['legacy-movie']); + expect(matches.single.libraryId, '5'); + expect(matches.single.libraryTitle, 'Legacy Movies'); + expect(matches.single.serverId, 'server-1'); + expect(matches.single.serverName, 'Server'); expect(requestUri.path, '/library/all'); expect(requestUri.queryParameters['title'], 'Legacy Movie'); expect(requestUri.queryParameters['type'], '1'); @@ -86,16 +86,16 @@ void main() { ); addTearDown(client.close); - final match = await client.findByExternalIds( + final matches = await client.findByExternalIds( const ExternalIds(tvdb: 315500), kind: MediaKind.show, titles: const ['Legacy Show'], ); - expect(match?.id, 'legacy-show'); + expect(matches.map((match) => match.id), ['legacy-show']); }); - test('prefers a modern Guid match over an earlier legacy candidate', () async { + test('returns both agent variants of one title, modern Guid match first', () async { final client = testPlexClient( handler: (request) async => _json({ 'MediaContainer': { @@ -121,16 +121,17 @@ void main() { ); addTearDown(client.close); - final match = await client.findByExternalIds( + final matches = await client.findByExternalIds( const ExternalIds(imdb: 'tt12345'), kind: MediaKind.movie, titles: const ['Duplicate'], ); - expect(match?.id, 'modern-match'); + // Two rating keys, so two library items — the modern-agent copy leads. + expect(matches.map((match) => match.id), ['modern-match', 'legacy-match']); }); - test('prefers an unfiltered modern match over a year-filtered legacy candidate', () async { + test('unions the year-filtered and unfiltered pages, modern match first', () async { final requests = []; final client = testPlexClient( handler: (request) async { @@ -163,14 +164,14 @@ void main() { ); addTearDown(client.close); - final match = await client.findByExternalIds( + final matches = await client.findByExternalIds( const ExternalIds(tmdb: 42), kind: MediaKind.movie, titles: const ['Missing Year'], year: 2024, ); - expect(match?.id, 'unfiltered-modern'); + expect(matches.map((match) => match.id), ['unfiltered-modern', 'filtered-legacy']); expect(requests, hasLength(2)); expect(requests.first.queryParameters['year'], '2023,2024,2025'); expect(requests.last.queryParameters.containsKey('year'), isFalse); @@ -194,13 +195,13 @@ void main() { ); addTearDown(client.close); - final match = await client.findByExternalIds( + final matches = await client.findByExternalIds( const ExternalIds(tvdb: 315500), kind: MediaKind.show, titles: const ['Unsupported'], ); - expect(match, isNull); + expect(matches, isEmpty); }); test('tries broader title candidates in order and still verifies external ids', () async { @@ -239,13 +240,13 @@ void main() { ); addTearDown(client.close); - final match = await client.findByExternalIds( + final matches = await client.findByExternalIds( const ExternalIds(tvdb: 123), kind: MediaKind.show, titles: const ['Parent Show Season 2', 'Parent Show'], ); - expect(match?.id, 'verified-parent'); + expect(matches.map((match) => match.id), ['verified-parent']); expect(requests.map((uri) => uri.queryParameters['title']), ['Parent Show Season 2', 'Parent Show']); expect(requests.first.queryParameters['X-Plex-Container-Size'], '20'); expect(requests.last.queryParameters['X-Plex-Container-Size'], '50'); @@ -272,14 +273,14 @@ void main() { ); addTearDown(client.close); - final match = await client.findByExternalIds( + final matches = await client.findByExternalIds( const ExternalIds(), kind: MediaKind.show, titles: const ['Never Searched'], plexGuid: 'plex://show/5e01fc33932ff9001db3b242', ); - expect(match?.id, 'exact-show'); + expect(matches.map((match) => match.id), ['exact-show']); expect(requests, hasLength(1)); expect(requests.single.path, '/library/all'); expect(requests.single.queryParameters['guid'], 'plex://show/5e01fc33932ff9001db3b242'); @@ -303,14 +304,14 @@ void main() { ); addTearDown(client.close); - final match = await client.findByExternalIds( + final matches = await client.findByExternalIds( const ExternalIds(), kind: MediaKind.movie, titles: const ['Night on the Galactic Railroad'], plexGuid: 'plex://movie/5d776b59ad5437001f79c6f8', ); - expect(match, isNull); + expect(matches, isEmpty); expect(requests, hasLength(1)); expect(requests.single.queryParameters['guid'], 'plex://movie/5d776b59ad5437001f79c6f8'); expect(requests.single.queryParameters.containsKey('title'), isFalse); @@ -377,8 +378,8 @@ void main() { season: const ExternalSeasonRef(tvdb: 2, tmdb: 2), ); - expect(missing, isNull); - expect(present?.id, 'complete-show'); + expect(missing, isEmpty); + expect(present.map((match) => match.id), ['complete-show']); expect(childRequests, ['/library/metadata/incomplete-show/children', '/library/metadata/complete-show/children']); expect(extraRequests, isEmpty, reason: 'season ordering is a server setting the gate deliberately never reads'); }); @@ -410,14 +411,172 @@ void main() { ); addTearDown(client.close); - final match = await client.findByExternalIds( + final matches = await client.findByExternalIds( const ExternalIds(tvdb: 300), kind: MediaKind.show, titles: const ['Ungated Show'], season: const ExternalSeasonRef(tvdb: 2, tmdb: 1), ); - expect(match?.id, 'ungated-show'); + expect(matches.map((match) => match.id), ['ungated-show']); expect(requests.map((uri) => uri.path), ['/library/all'], reason: 'no children, no preferences'); }); + + test('returns every library copy sharing one exact Plex guid', () async { + // #1754: `/library/all` is server-wide, so a movie held by both a 4K + // section and an HD section answers with two sibling entries. Taking + // Metadata[0] is what hid the second copy from the Explore chooser. + final client = testPlexClient( + handler: (request) async => _json({ + 'MediaContainer': { + 'Metadata': [ + { + 'ratingKey': 'hd-copy', + 'type': 'movie', + 'title': 'Dual Library', + 'guid': 'plex://movie/dual', + 'librarySectionID': 1, + 'librarySectionTitle': 'Movies', + 'Media': [ + {'id': 10, 'videoResolution': '1080', 'videoCodec': 'h264', 'container': 'mkv'}, + ], + }, + { + 'ratingKey': 'uhd-copy', + 'type': 'movie', + 'title': 'Dual Library', + 'guid': 'plex://movie/dual', + 'librarySectionID': 2, + 'librarySectionTitle': '4K Movies', + 'Media': [ + {'id': 20, 'videoResolution': '4k', 'videoCodec': 'hevc', 'container': 'mkv'}, + ], + }, + ], + }, + }), + ); + addTearDown(client.close); + + final matches = await client.findByExternalIds( + const ExternalIds(), + kind: MediaKind.movie, + titles: const [], + plexGuid: 'plex://movie/dual', + ); + + expect(matches.map((match) => match.id), ['hd-copy', 'uhd-copy']); + expect(matches.map((match) => match.libraryId), ['1', '2']); + expect(matches.map((match) => match.libraryTitle), ['Movies', '4K Movies']); + expect(matches.map((match) => match.mediaVersions?.single.videoResolution), ['1080', '4k']); + }); + + test('keeps searching by title after an exact guid hit so legacy-agent copies surface', () async { + // A library still on a legacy agent carries `com.plexapp.agents.*` as its + // primary guid, so the `guid=` filter cannot see it at all (#1754). The + // copy the two passes agree on must still appear only once. + final requests = []; + final modernCopy = { + 'ratingKey': 'modern-copy', + 'type': 'movie', + 'title': 'Mixed Agents', + 'guid': 'plex://movie/mixed', + 'librarySectionTitle': '4K Movies', + 'Guid': [ + {'id': 'imdb://tt777'}, + ], + }; + final client = testPlexClient( + handler: (request) async { + requests.add(request.url); + final byGuid = request.url.queryParameters.containsKey('guid'); + return _json({ + 'MediaContainer': { + 'Metadata': [ + modernCopy, + if (!byGuid) + { + 'ratingKey': 'legacy-copy', + 'type': 'movie', + 'title': 'Mixed Agents', + 'guid': 'com.plexapp.agents.imdb://tt777?lang=en', + 'librarySectionTitle': 'Movies', + }, + ], + }, + }); + }, + ); + addTearDown(client.close); + + final matches = await client.findByExternalIds( + const ExternalIds(imdb: 'tt777'), + kind: MediaKind.movie, + titles: const ['Mixed Agents'], + plexGuid: 'plex://movie/mixed', + ); + + expect(matches.map((match) => match.id), ['modern-copy', 'legacy-copy']); + expect(requests.map((uri) => uri.queryParameters.containsKey('guid')), [true, false]); + }); + + test('season-gates every candidate, not just the first', () async { + final client = testPlexClient( + handler: (request) async { + if (request.url.path.endsWith('/children')) { + final parentId = request.url.pathSegments[2]; + return _json({ + 'MediaContainer': { + 'totalSize': 1, + 'Metadata': [ + { + 'ratingKey': '$parentId-season', + 'type': 'season', + 'title': 'Season', + 'index': parentId == 'full-copy' ? 2 : 1, + }, + ], + }, + }); + } + if (request.url.path.startsWith('/library/metadata/')) { + return _json({'MediaContainer': {}}); + } + return _json({ + 'MediaContainer': { + 'Metadata': [ + { + 'ratingKey': 'partial-copy', + 'type': 'show', + 'title': 'Split Show', + 'librarySectionTitle': 'Shows', + 'Guid': [ + {'id': 'tvdb://555'}, + ], + }, + { + 'ratingKey': 'full-copy', + 'type': 'show', + 'title': 'Split Show', + 'librarySectionTitle': '4K Shows', + 'Guid': [ + {'id': 'tvdb://555'}, + ], + }, + ], + }, + }); + }, + ); + addTearDown(client.close); + + final matches = await client.findByExternalIds( + const ExternalIds(tvdb: 555), + kind: MediaKind.show, + titles: const ['Split Show'], + season: const ExternalSeasonRef(tvdb: 2, tmdb: 2), + ); + + expect(matches.map((match) => match.id), ['full-copy']); + }); } diff --git a/test/utils/external_ids_matching_test.dart b/test/utils/external_ids_matching_test.dart index e36a6087..5266a22f 100644 --- a/test/utils/external_ids_matching_test.dart +++ b/test/utils/external_ids_matching_test.dart @@ -17,7 +17,7 @@ void main() { }); }); - group('ExternalIds.jellyfinCandidateMatching', () { + group('ExternalIds.jellyfinCandidatesMatching', () { const target = ExternalIds(imdb: 'tt0133093', tmdb: 603); test('picks the candidate whose ProviderIds intersect, skipping others', () { @@ -32,17 +32,41 @@ void main() { 'ProviderIds': {'Tmdb': '603'}, }, ]; - expect(ExternalIds.jellyfinCandidateMatching(candidates, target)?['Name'], 'The Matrix'); + final matches = ExternalIds.jellyfinCandidatesMatching(candidates, target); + expect(matches, hasLength(1)); + expect(matches.single['Name'], 'The Matrix'); }); - test('returns null when nothing verifies', () { + test('returns every library copy of the same title, in response order', () { + final candidates = >[ + { + 'Id': 'movie-4k', + 'Name': 'The Matrix (4K)', + 'ProviderIds': {'Tmdb': '603'}, + }, + { + 'Id': 'movie-other', + 'Name': 'The Matrix Reloaded', + 'ProviderIds': {'Imdb': 'tt0234215'}, + }, + { + 'Id': 'movie-hd', + 'Name': 'The Matrix', + 'ProviderIds': {'Imdb': 'tt0133093'}, + }, + ]; + final matches = ExternalIds.jellyfinCandidatesMatching(candidates, target); + expect(matches.map((candidate) => candidate['Id']), ['movie-4k', 'movie-hd']); + }); + + test('returns no candidates when nothing verifies', () { final candidates = >[ { 'Name': 'Similar title, different film', 'ProviderIds': {'Imdb': 'tt0234215'}, }, ]; - expect(ExternalIds.jellyfinCandidateMatching(candidates, target), isNull); + expect(ExternalIds.jellyfinCandidatesMatching(candidates, target), isEmpty); }); }); }