diff --git a/lib/media/media_source_info.dart b/lib/media/media_source_info.dart index ec4e89720..6bbd5c109 100644 --- a/lib/media/media_source_info.dart +++ b/lib/media/media_source_info.dart @@ -353,6 +353,15 @@ class PlaybackExtras { PlaybackExtras({required this.chapters, required this.markers}); + /// Whether any marker is a credits marker (detected or chapter-derived). + bool get hasCreditsMarkers => markers.any((marker) => marker.isCredits); + + /// A copy with every credits marker removed; chapters and the remaining + /// markers are kept. Used when the server admin explicitly disabled + /// credits detection for the item's show/movie. + PlaybackExtras withoutCreditsMarkers() => + PlaybackExtras(chapters: chapters, markers: markers.where((marker) => !marker.isCredits).toList()); + static String? _classifyChapterTitle(String title, RegExp introPattern, RegExp creditsPattern) { if (introPattern.hasMatch(title)) return 'intro'; if (creditsPattern.hasMatch(title)) return 'credits'; diff --git a/lib/services/cached_playback_metadata_service.dart b/lib/services/cached_playback_metadata_service.dart index d5497fa75..1bc3f532a 100644 --- a/lib/services/cached_playback_metadata_service.dart +++ b/lib/services/cached_playback_metadata_service.dart @@ -87,12 +87,19 @@ class CachedPlaybackMetadataService { }) async { final metadata = await _plexMetadata(ServerId(serverId), itemId); if (metadata == null) return null; - return plexPlaybackExtrasFromCacheJson( + final extras = plexPlaybackExtrasFromCacheJson( metadata, introPattern: introPattern, creditsPattern: creditsPattern, forceChapterFallback: forceChapterFallback, ); + // Offline twin of the suppression in `PlexClient.getPlaybackExtras`; + // cache-only, so an uncached show row fails open and keeps the markers. + return plexApplyCreditsDetectionPreference( + extras, + metadata, + loadMetadataJson: (ratingKey) => _plexMetadata(serverId, ratingKey), + ); } static Future?> _plexMetadata(ServerId serverId, String itemId) async { diff --git a/lib/services/plex_client.dart b/lib/services/plex_client.dart index 9744834de..aa2c07c78 100644 --- a/lib/services/plex_client.dart +++ b/lib/services/plex_client.dart @@ -1841,18 +1841,50 @@ class PlexClient parseResponse: parseResponse, ); final metadataJson = _getFirstMetadataJsonFromData(data); - return _parsePlaybackExtrasFromMetadataJson( + final extras = _parsePlaybackExtrasFromMetadataJson( metadataJson, introPattern: introPattern, creditsPattern: creditsPattern, forceChapterFallback: forceChapterFallback, ); + // Movies carry `enableCreditsMarkerGeneration` on the row already in + // hand; episodes resolve the grandparent show through the same shared + // `/library/metadata/{id}` cache row the detail screen populates, so the + // lookup normally costs no network round trip. + return await plexApplyCreditsDetectionPreference( + extras, + metadataJson, + loadMetadataJson: (ratingKey) => _fetchSharedMetadataRow(ratingKey, requestContext: requestContext), + ); } catch (e) { appLogger.w('Failed to get playback extras', error: e); return PlaybackExtras(chapters: [], markers: []); } } + /// The first Metadata JSON of the shared `/library/metadata/{id}` cache row, + /// cache-first. A miss fetches with the same query shape as + /// [_getMetadataWithImages] so the written row matches what the detail + /// screen would cache. + Future?> _fetchSharedMetadataRow( + String ratingKey, { + required ({ServerId cacheScope, Map headers}) requestContext, + }) async { + final cacheKey = '/library/metadata/$ratingKey'; + final data = await fetchWithCacheFirst>( + cacheScope: requestContext.cacheScope, + cacheKey: cacheKey, + networkCall: () => _http.get( + cacheKey, + queryParameters: {'includeChapters': 1, 'includeMarkers': 1, 'checkFiles': 1, 'includeStreams': 1}, + headers: requestContext.headers, + ), + parseCache: (cached) => cached as Map?, + parseResponse: (response) => response.data as Map?, + ); + return _getFirstMetadataJsonFromData(data); + } + /// Parse PlaybackExtras from metadata JSON PlaybackExtras _parsePlaybackExtrasFromMetadataJson( Map? metadataJson, { diff --git a/lib/services/plex_mappers.dart b/lib/services/plex_mappers.dart index abbf45712..479a92509 100644 --- a/lib/services/plex_mappers.dart +++ b/lib/services/plex_mappers.dart @@ -1338,6 +1338,57 @@ PlaybackExtras plexPlaybackExtrasFromCacheJson( ); } +/// Plex's per-item credits-detection state, read from a show/movie metadata +/// JSON. `enableCreditsMarkerGeneration` is `0` when the admin explicitly +/// disabled the setting; it is `-1` ("Library default") or absent otherwise, +/// including on servers without the credits-detection feature. Episodes never +/// carry the attribute — callers must read the grandparent show row. +bool plexCreditsDetectionDisabled(Map? metadataJson) => + flexibleInt(metadataJson?['enableCreditsMarkerGeneration']) == 0; + +/// Drops credits markers from [extras] when the owning show/movie explicitly +/// disables Plex credits detection (`enableCreditsMarkerGeneration == 0`). +/// +/// PMS already strips *detected* credits markers from its responses when the +/// setting is disabled, so this mostly guards the chapter-title fallback in +/// [PlaybackExtras.withChapterFallback], which would otherwise resurrect a +/// credits skip action the server admin turned off (#2137). Stale cached rows +/// that predate the setting change are suppressed by the same check. +/// +/// Movies carry the attribute on [metadataJson] itself; episodes resolve the +/// grandparent show row through [loadMetadataJson] — cache-first with a +/// network miss path online (`PlexClient.getPlaybackExtras`), cache-only +/// offline (`CachedPlaybackMetadataService`). The lookup runs only when a +/// credits marker is actually present. An absent attribute (servers without +/// the credits-detection feature), `-1` ("Library default"), and any loader +/// failure all keep the markers untouched. +Future plexApplyCreditsDetectionPreference( + PlaybackExtras extras, + Map? metadataJson, { + required Future?> Function(String ratingKey) loadMetadataJson, +}) async { + if (metadataJson == null || !extras.hasCreditsMarkers) return extras; + + final ownPreference = flexibleInt(metadataJson['enableCreditsMarkerGeneration']); + bool disabled; + if (ownPreference != null) { + disabled = ownPreference == 0; + } else { + final grandparentRatingKey = metadataJson['grandparentRatingKey']?.toString(); + if (grandparentRatingKey == null || grandparentRatingKey.isEmpty) return extras; + try { + disabled = plexCreditsDetectionDisabled(await loadMetadataJson(grandparentRatingKey)); + } catch (e) { + appLogger.w('Failed to resolve credits-detection preference for show $grandparentRatingKey', error: e); + return extras; + } + } + if (!disabled) return extras; + + appLogger.d('Suppressing credits markers: Plex credits detection disabled for this item'); + return extras.withoutCreditsMarkers(); +} + List plexChaptersFromCacheJson(Map? metadataJson) { final chapterList = metadataJson?['Chapter']; if (chapterList is! List) return const []; diff --git a/test/services/plex_credits_detection_suppression_test.dart b/test/services/plex_credits_detection_suppression_test.dart new file mode 100644 index 000000000..874f90aa2 --- /dev/null +++ b/test/services/plex_credits_detection_suppression_test.dart @@ -0,0 +1,305 @@ +import 'dart:convert'; + +import 'package:drift/native.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:http/http.dart' as http; +import 'package:plezy/database/app_database.dart'; +import 'package:plezy/media/ids.dart'; +import 'package:plezy/media/media_backend.dart'; +import 'package:plezy/services/cached_playback_metadata_service.dart'; +import 'package:plezy/services/plex_api_cache.dart'; +import 'package:plezy/services/plex_client.dart'; +import 'package:plezy/utils/active_client_scope.dart'; + +import '../test_helpers/backend_client_fixtures.dart'; + +/// Plex strips detected credits markers from its responses when the admin +/// disables credits detection for a show or movie +/// (`enableCreditsMarkerGeneration == 0`), but Plezy's chapter-title fallback +/// would resurrect a credits skip action from a "Credits"/"Ending" chapter, +/// showing "Next Episode" during credits the admin asked to leave alone +/// (#2137). These tests pin the suppression rule: explicit `0` on the owning +/// show/movie drops credits markers; absent (non-Plex-Pass servers) and `-1` +/// ("Library default") keep them. +void main() { + const episodeKey = 'ep-1'; + const showKey = 'show-1'; + const episodeEndpoint = '/library/metadata/$episodeKey'; + const showEndpoint = '/library/metadata/$showKey'; + final serverId = ServerId('server-1'); + final scope = buildPlexProfileScopeId(serverId: serverId, profileId: 'profile-a'); + + late AppDatabase database; + late PlexApiCache cache; + + setUp(() { + database = AppDatabase.forTesting(NativeDatabase.memory()); + PlexApiCache.initialize(database); + cache = PlexApiCache.instance; + }); + + tearDown(() => database.close()); + + PlexClient clientRecording(List requested, {required Map?> responsesByPath}) { + final client = testPlexClient( + serverId: serverId, + profileScopeId: scope, + handler: (request) async { + final probe = _failoverProbe(request, serverId); + if (probe != null) return probe; + requested.add(request.url); + final body = responsesByPath[request.url.path]; + if (body == null) return http.Response('', 404); + return _jsonResponse(body); + }, + ); + addTearDown(client.close); + return client; + } + + group('online (PlexClient.getPlaybackExtras)', () { + test('drops the chapter-derived credits marker when the show disables credits detection', () async { + final requested = []; + final client = clientRecording( + requested, + responsesByPath: { + episodeEndpoint: _episodeResponse(chapters: _reporterChapters), + showEndpoint: _showResponse(enableCreditsMarkerGeneration: 0), + }, + ); + + final extras = await client.getPlaybackExtras(episodeKey); + + expect(extras.markers.where((m) => m.isCredits), isEmpty); + // Only credits behavior is disabled; the intro chapter still maps. + expect(extras.markers.map((m) => m.type), ['intro']); + expect(extras.chapters, hasLength(3), reason: 'chapters themselves stay for the timeline'); + expect(requested.map((u) => u.path), [episodeEndpoint, showEndpoint]); + }); + + test('keeps the chapter-derived credits marker when the show has no preference attribute', () async { + // Non-Plex-Pass servers never emit the attribute; the fallback is the + // only credits source there and must keep working. + final requested = []; + final client = clientRecording( + requested, + responsesByPath: { + episodeEndpoint: _episodeResponse(chapters: _reporterChapters), + showEndpoint: _showResponse(enableCreditsMarkerGeneration: null), + }, + ); + + final extras = await client.getPlaybackExtras(episodeKey); + + expect(extras.markers.map((m) => m.type), containsAll(['intro', 'credits'])); + }); + + test('keeps credits markers when the show preference is -1 (library default)', () async { + final client = clientRecording( + [], + responsesByPath: { + episodeEndpoint: _episodeResponse(chapters: _reporterChapters), + showEndpoint: _showResponse(enableCreditsMarkerGeneration: -1), + }, + ); + + final extras = await client.getPlaybackExtras(episodeKey); + + expect(extras.markers.any((m) => m.isCredits), isTrue); + }); + + test('never requests the show row when the episode yields no credits marker', () async { + final requested = []; + final client = clientRecording( + requested, + responsesByPath: { + episodeEndpoint: _episodeResponse(chapters: [_chapter(id: 1, tag: 'Scene 1', start: 0, end: 351280)]), + }, + ); + + final extras = await client.getPlaybackExtras(episodeKey); + + expect(extras.markers, isEmpty); + expect(requested.map((u) => u.path), [episodeEndpoint]); + }); + + test('serves the show preference from the shared cache row without network', () async { + await cache.put(scope.cacheServerId, showEndpoint, _showResponse(enableCreditsMarkerGeneration: 0)); + final requested = []; + final client = clientRecording( + requested, + responsesByPath: {episodeEndpoint: _episodeResponse(chapters: _reporterChapters)}, + ); + + final extras = await client.getPlaybackExtras(episodeKey); + + expect(extras.markers.where((m) => m.isCredits), isEmpty); + expect(requested.map((u) => u.path), [episodeEndpoint], reason: 'show row must come from cache'); + }); + + test('a movie reads its own attribute and needs no second request', () async { + final requested = []; + final client = clientRecording( + requested, + responsesByPath: { + episodeEndpoint: _movieResponse( + enableCreditsMarkerGeneration: 0, + chapters: [_chapter(id: 9, tag: 'End Credits', start: 300000, end: 351280)], + ), + }, + ); + + final extras = await client.getPlaybackExtras(episodeKey); + + expect(extras.markers, isEmpty); + expect(requested.map((u) => u.path), [episodeEndpoint]); + }); + + test('also drops detected credits markers from a stale row when the show disables detection', () async { + // A cached episode row can predate the setting change and still contain + // server-detected markers; "any skip credits behavior" must stay off. + await cache.put( + scope.cacheServerId, + episodeEndpoint, + _episodeResponse( + chapters: const [], + markers: [ + {'id': 5, 'type': 'credits', 'startTimeOffset': 311080, 'endTimeOffset': 351280, 'final': true}, + ], + ), + ); + final client = clientRecording( + [], + responsesByPath: {showEndpoint: _showResponse(enableCreditsMarkerGeneration: 0)}, + ); + + final extras = await client.getPlaybackExtras(episodeKey); + + expect(extras.markers, isEmpty); + }); + + test('fails open when the show lookup errors', () async { + final client = clientRecording( + [], + // showEndpoint missing -> 404 on the grandparent fetch. + responsesByPath: {episodeEndpoint: _episodeResponse(chapters: _reporterChapters)}, + ); + + final extras = await client.getPlaybackExtras(episodeKey); + + expect(extras.markers.any((m) => m.isCredits), isTrue); + }); + }); + + group('offline (CachedPlaybackMetadataService)', () { + test('suppresses credits markers when the cached show row disables detection', () async { + await cache.put(scope.cacheServerId, episodeEndpoint, _episodeResponse(chapters: _reporterChapters)); + await cache.put(scope.cacheServerId, showEndpoint, _showResponse(enableCreditsMarkerGeneration: 0)); + + final extras = await CachedPlaybackMetadataService.fetchPlaybackExtras( + backend: MediaBackend.plex, + cacheServerId: scope.cacheServerId.value, + itemId: episodeKey, + ); + + expect(extras!.markers.where((m) => m.isCredits), isEmpty); + expect(extras.markers.map((m) => m.type), ['intro']); + }); + + test('keeps credits markers when the show row is not cached', () async { + await cache.put(scope.cacheServerId, episodeEndpoint, _episodeResponse(chapters: _reporterChapters)); + + final extras = await CachedPlaybackMetadataService.fetchPlaybackExtras( + backend: MediaBackend.plex, + cacheServerId: scope.cacheServerId.value, + itemId: episodeKey, + ); + + expect(extras!.markers.any((m) => m.isCredits), isTrue); + }); + }); +} + +/// The failover client probes server identity before the first real request. +http.Response? _failoverProbe(http.Request request, ServerId serverId) { + if (request.url.path == '/') { + return _jsonResponse({ + 'MediaContainer': {'machineIdentifier': serverId.value}, + }); + } + if (request.url.path == '/media/providers') { + return _jsonResponse({ + 'MediaContainer': {'MediaProvider': []}, + }); + } + return null; +} + +http.Response _jsonResponse(Map body) => + http.Response(jsonEncode(body), 200, headers: const {'content-type': 'application/json'}); + +Map _chapter({required int id, required String tag, required int start, required int end}) => { + 'id': id, + 'tag': tag, + 'index': id, + 'startTimeOffset': start, + 'endTimeOffset': end, +}; + +/// Chapter layout from the #2137 report: the "Credits" chapter ends at EOF, so +/// the synthesized marker is what relabeled the skip button "Next Episode". +final _reporterChapters = [ + _chapter(id: 1, tag: 'Intro', start: 0, end: 52520), + _chapter(id: 2, tag: 'Scene 1', start: 52520, end: 311080), + _chapter(id: 3, tag: 'Credits', start: 311080, end: 351280), +]; + +Map _episodeResponse({ + required List> chapters, + List> markers = const [], +}) => { + 'MediaContainer': { + 'Metadata': [ + { + 'ratingKey': 'ep-1', + 'grandparentRatingKey': 'show-1', + 'type': 'episode', + 'title': 'Episode', + 'duration': 351338, + if (chapters.isNotEmpty) 'Chapter': chapters, + if (markers.isNotEmpty) 'Marker': markers, + }, + ], + }, +}; + +Map _showResponse({required int? enableCreditsMarkerGeneration}) => { + 'MediaContainer': { + 'Metadata': [ + { + 'ratingKey': 'show-1', + 'type': 'show', + 'title': 'Show', + 'enableCreditsMarkerGeneration': ?enableCreditsMarkerGeneration, + }, + ], + }, +}; + +Map _movieResponse({ + required int? enableCreditsMarkerGeneration, + required List> chapters, +}) => { + 'MediaContainer': { + 'Metadata': [ + { + 'ratingKey': 'ep-1', + 'type': 'movie', + 'title': 'Movie', + 'duration': 351338, + 'enableCreditsMarkerGeneration': ?enableCreditsMarkerGeneration, + if (chapters.isNotEmpty) 'Chapter': chapters, + }, + ], + }, +};