From 04d8070fd48a3a33e9a9d694f3aa10852aebe42f Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Sat, 25 Jul 2026 18:36:42 +0200 Subject: [PATCH] refactor: pin the look-alike code paths that must not be merged Several pairs of near-identical code paths differ in one load-bearing line. Each site now carries a comment naming the invariant that forces it apart, backed by a characterization test so a future deduplication fails loudly instead of silently changing behaviour. Pinned: focusable wrapper vs. chip D-pad activation policy, profile connection cleanup's raw-id vs. ServerId-typed server projections, live TV tab loaders, video player display matching and playback service wiring, track selection container ordering, tracker HTTP client status ladder, and the MediaServerHttpClient shutdown/cancellation contract versus ManagedHttpClient's closing guard. New tests: test/focus/dpad_activation_policy_test.dart test/services/track_selection_container_ordinal_test.dart test/services/trackers/tracker_status_ladder_test.dart test/utils/media_server_http_client_shutdown_test.dart --- lib/focus/focusable_chip_mixin.dart | 6 + lib/focus/focusable_wrapper.dart | 5 + lib/profiles/active_profile_binder.dart | 5 + lib/profiles/profile_connection_cleanup.dart | 7 ++ lib/screens/livetv/tabs/guide_tab.dart | 3 + lib/screens/livetv/tabs/recordings_tab.dart | 3 + lib/screens/livetv/tabs/whats_on_tab.dart | 3 + .../profile/profile_detail_screen.dart | 8 +- .../video_player/parts/display_matching.dart | 4 +- .../video_player/parts/playback_services.dart | 14 +++ lib/screens/video_player_screen.dart | 11 ++ lib/services/track_selection_service.dart | 8 ++ .../trackers/anilist/anilist_client.dart | 2 + lib/services/trackers/mal/mal_client.dart | 2 + lib/services/trackers/simkl/simkl_client.dart | 2 + .../trackers/tracker_http_client.dart | 8 ++ lib/services/trakt/trakt_client.dart | 2 + lib/utils/abortable_http_request.dart | 3 + lib/utils/managed_http_client.dart | 3 + lib/utils/media_server_http_client.dart | 9 ++ test/focus/dpad_activation_policy_test.dart | 88 +++++++++++++++ ...rack_selection_container_ordinal_test.dart | 36 ++++++ .../trackers/tracker_status_ladder_test.dart | 106 ++++++++++++++++++ ...edia_server_http_client_shutdown_test.dart | 83 ++++++++++++++ 24 files changed, 419 insertions(+), 2 deletions(-) create mode 100644 test/focus/dpad_activation_policy_test.dart create mode 100644 test/services/track_selection_container_ordinal_test.dart create mode 100644 test/services/trackers/tracker_status_ladder_test.dart create mode 100644 test/utils/media_server_http_client_shutdown_test.dart diff --git a/lib/focus/focusable_chip_mixin.dart b/lib/focus/focusable_chip_mixin.dart index e2fe987d..9757acec 100644 --- a/lib/focus/focusable_chip_mixin.dart +++ b/lib/focus/focusable_chip_mixin.dart @@ -102,6 +102,12 @@ mixin FocusableChipStateMixin on State { /// /// Returns [KeyEventResult.handled] if the event was consumed, /// [KeyEventResult.ignored] otherwise. + /// + /// Runs the same activation sequence as `_FocusableWrapperState._handleKeyEvent` + /// but is deliberately kept separate: a chip leaves the context-menu key + /// unconsumed when [ChipKeyCallbacks.onLongPress] is null and traps RIGHT/DOWN + /// so focus cannot escape the strip, where a wrapper does the opposite on both + /// counts. KeyEventResult handleChipKeyEvent(FocusNode _, KeyEvent event, ChipKeyCallbacks callbacks) { final key = event.logicalKey; diff --git a/lib/focus/focusable_wrapper.dart b/lib/focus/focusable_wrapper.dart index 88050c25..fe2b30e8 100644 --- a/lib/focus/focusable_wrapper.dart +++ b/lib/focus/focusable_wrapper.dart @@ -410,6 +410,11 @@ class _FocusableWrapperState extends State with SingleTickerPr }); } + // Runs the same activation sequence as FocusableChipStateMixin.handleChipKeyEvent + // but is deliberately kept separate: a wrapper always consumes the context-menu + // key (even with no onLongPress, so a card never leaks it upward) and passes + // every unmapped arrow through to framework traversal, where a chip does the + // opposite on both counts. KeyEventResult _handleKeyEvent(FocusNode node, KeyEvent event) { final key = event.logicalKey; final diagnosticsEnabled = TextInputDiagnostics.enabled; diff --git a/lib/profiles/active_profile_binder.dart b/lib/profiles/active_profile_binder.dart index 43c1edd8..07083332 100644 --- a/lib/profiles/active_profile_binder.dart +++ b/lib/profiles/active_profile_binder.dart @@ -372,6 +372,11 @@ class ActiveProfileBinder { return success; } + /// Server ids the profile should reach once bound: its join rows plus the + /// implicit Plex Home parent, which normally has no row. Not shared with + /// `_serverIdsForProfile` (profile_connection_cleanup.dart) — that one is + /// join-rows-only and [ServerId]-typed, while this set keeps growing with + /// bind results and is compared against the manager's raw string ids. Set _expectedServerIdsForProfile( Profile profile, { required List joinRows, diff --git a/lib/profiles/profile_connection_cleanup.dart b/lib/profiles/profile_connection_cleanup.dart index 53fa0875..b754b598 100644 --- a/lib/profiles/profile_connection_cleanup.dart +++ b/lib/profiles/profile_connection_cleanup.dart @@ -283,6 +283,10 @@ Future _clearProfileServerPrefsNoLongerReferenced({ } } +/// Server ids reachable through this profile's join rows. Narrower than +/// `ActiveProfileBinder._expectedServerIdsForProfile`: an implicit Plex Home +/// parent is not counted here, so folding the two together would change which +/// per-profile prefs survive an unlink. Future> _serverIdsForProfile( String profileId, { required ProfileConnectionRegistry profileConnections, @@ -316,6 +320,9 @@ Future _isServerReferenced( return false; } +// [ServerId]-typed for the preference APIs, which drops ids that fail to +// parse; the twin in profile_detail_screen.dart stays raw so it can be +// differenced against download keys. Set _serverIdsForConnection(Connection connection) { return switch (connection) { PlexAccountConnection(:final servers) => { diff --git a/lib/screens/livetv/tabs/guide_tab.dart b/lib/screens/livetv/tabs/guide_tab.dart index 66045c14..a26fab6c 100644 --- a/lib/screens/livetv/tabs/guide_tab.dart +++ b/lib/screens/livetv/tabs/guide_tab.dart @@ -192,6 +192,9 @@ class GuideTabState extends State with MountedSetStateMixin, WidgetsBi }); } + // Not the gated data-refresh timer the other tabs run: pause/resume drive the + // per-minute UI ticker, and pause has to stamp _hiddenSince on both a section + // hide and an app background so _catchUpIfStale can measure the absence. void pauseRefresh() { _hiddenSince ??= DateTime.now(); _timeIndicatorTimer?.cancel(); diff --git a/lib/screens/livetv/tabs/recordings_tab.dart b/lib/screens/livetv/tabs/recordings_tab.dart index 998515f9..a68e0746 100644 --- a/lib/screens/livetv/tabs/recordings_tab.dart +++ b/lib/screens/livetv/tabs/recordings_tab.dart @@ -125,6 +125,9 @@ class RecordingsTabState extends State with WidgetsBindingObserve } } + // Same three gates as WhatsOnTab (tab selected, subtree visible, app + // foregrounded), but resume also reloads: a recording scheduled from the + // guide has to show up on arrival, not on the next 30s tick. void pauseRefresh() { _refreshRequested = false; _syncRefreshTimer(); diff --git a/lib/screens/livetv/tabs/whats_on_tab.dart b/lib/screens/livetv/tabs/whats_on_tab.dart index 70c400b9..7d46f7c2 100644 --- a/lib/screens/livetv/tabs/whats_on_tab.dart +++ b/lib/screens/livetv/tabs/whats_on_tab.dart @@ -80,6 +80,9 @@ class WhatsOnTabState extends State } } + // Refreshes only while all three gates hold: tab selected, subtree visible, + // app foregrounded. Resume just re-arms the tick — unlike RecordingsTab there + // is no immediate reload, since nothing done on the other tabs changes hubs. void pauseRefresh() { _refreshRequested = false; _syncRefreshTimer(); diff --git a/lib/screens/profile/profile_detail_screen.dart b/lib/screens/profile/profile_detail_screen.dart index 39c04303..bf652cef 100644 --- a/lib/screens/profile/profile_detail_screen.dart +++ b/lib/screens/profile/profile_detail_screen.dart @@ -228,7 +228,10 @@ class _ProfileDetailScreenState extends State with Controll /// Server ids the profile keeps after removing [excludingConnectionId]: /// its other join rows plus, for Plex Home profiles, the implicit parent - /// account. + /// account. Raw ids, matching the download keys this is differenced + /// against; `_serverIdsForProfile` in profile_connection_cleanup.dart is + /// ServerId-typed and ignores the parent, so the two are not the same + /// projection. Future> _retainedServerIds({ required String excludingConnectionId, required ProfileConnectionRegistry profileConnections, @@ -260,6 +263,9 @@ class _ProfileDetailScreenState extends State with Controll unawaited(context.read().rebindIfActive(_profile.id)); } + // Raw machine ids rather than the ServerId-typed twin in + // profile_connection_cleanup.dart: these are differenced against retained + // ids and matched to download global keys, which carry the unparsed id. Set _serverIdsForConnection(Connection conn) { return switch (conn) { PlexAccountConnection(:final servers) => servers.map((s) => s.clientIdentifier).toSet(), diff --git a/lib/screens/video_player/parts/display_matching.dart b/lib/screens/video_player/parts/display_matching.dart index 35d45795..5e2255fd 100644 --- a/lib/screens/video_player/parts/display_matching.dart +++ b/lib/screens/video_player/parts/display_matching.dart @@ -143,7 +143,9 @@ extension _VideoPlayerDisplayMatchingMethods on VideoPlayerScreenState { } } - /// Restore Windows display mode to original state. + /// Restore Windows display mode to original state. Fullscreen-exit only: + /// `dispose()` runs its own fire-and-forget variant because it cannot await + /// the HDR settle below. Future _restoreWindowsDisplayMode() async { if (_displayModeService == null || !_displayModeService!.anyChangeApplied) return; diff --git a/lib/screens/video_player/parts/playback_services.dart b/lib/screens/video_player/parts/playback_services.dart index 4b327042..7ce9b50d 100644 --- a/lib/screens/video_player/parts/playback_services.dart +++ b/lib/screens/video_player/parts/playback_services.dart @@ -61,6 +61,9 @@ extension _VideoPlayerPlaybackServiceMethods on VideoPlayerScreenState { required SettingsService settingsService, required bool useExoPlayer, }) async { + // Re-wire scope: exactly the nine subscriptions re-created below. The + // media-controls listeners belong to _setupMediaControls and the + // sleep-timer/Apple TV ones to initState; both outlive a re-wire. await Future.wait([ if (_playingSubscription != null) _playingSubscription!.cancel(), if (_completedSubscription != null) _completedSubscription!.cancel(), @@ -190,10 +193,21 @@ extension _VideoPlayerPlaybackServiceMethods on VideoPlayerScreenState { }); } + /// Roll the screen back to a re-runnable state after a failed player + /// attempt. The player is gone but the screen stays mounted and + /// [_retryPlayerInitialization] may run again, so every collaborator is + /// released *and* nulled so it can be built once more. Kept separate from + /// `dispose()`, which instead destroys the notifiers, focus nodes and + /// player, and cannot await any of this. Future _tearDownFailedPlayerAttempt(Player attemptPlayer) async { final activePlayer = player; if (activePlayer != null && !identical(activePlayer, attemptPlayer)) return; + // Rollback scope: the nine player streams plus the five media-controls + // ones. _sleepTimerSubscription and _appleTvPlayPauseSubscription are + // initState-owned and never re-created — cancelling them here would kill + // the sleep-timer prompt and the Apple TV remote for the rest of the + // screen's life. final cancellationFutures = >[ if (_playingSubscription != null) _playingSubscription!.cancel(), if (_completedSubscription != null) _completedSubscription!.cancel(), diff --git a/lib/screens/video_player_screen.dart b/lib/screens/video_player_screen.dart index 8ad82a5d..3910c919 100644 --- a/lib/screens/video_player_screen.dart +++ b/lib/screens/video_player_screen.dart @@ -1509,6 +1509,11 @@ class VideoPlayerScreenState extends State with WidgetsBindin _chromeController.dispose(); _toastController.dispose(); + // The release sequence below mirrors _tearDownFailedPlayerAttempt but is + // deliberately separate: dispose() cannot await, and it destroys the + // notifiers, focus nodes and player that the rollback path keeps alive + // for a retry on a still-mounted screen. + // // Stop progress tracking and send final state. Normal back navigation // awaits this before popping; dispose keeps a fallback for externally // removed routes where dispose() cannot await. @@ -1531,6 +1536,9 @@ class VideoPlayerScreenState extends State with WidgetsBindin SleepTimerService().markNeedsRestart(); } + // Teardown scope: every subscription the screen ever owns, including the + // initState-owned sleep-timer and Apple TV ones that the rollback path + // must leave alive. _playingSubscription?.cancel(); _completedSubscription?.cancel(); _errorSubscription?.cancel(); @@ -1577,6 +1585,9 @@ class VideoPlayerScreenState extends State with WidgetsBindin FullscreenStateManager().removeListener(_onFullscreenChanged); _fullscreenListenerAttached = false; } + // Not _restoreWindowsDisplayMode(): that helper waits 200ms after clearing + // the HDR hint before restoring, which dispose() cannot do. Fire the hint + // clear at the still-live player and restore immediately. if (!isReplacingWithVideo && Platform.isWindows && _displayModeService != null && diff --git a/lib/services/track_selection_service.dart b/lib/services/track_selection_service.dart index 4086fef3..a8dadc2b 100644 --- a/lib/services/track_selection_service.dart +++ b/lib/services/track_selection_service.dart @@ -127,6 +127,8 @@ SubtitleTrack? findMpvTrackForPlexSubtitle( // A container track has no stable native ID. Its source-container ordinal // is authoritative; a metadata-identical earlier track is not a match. + // Narrower than the guard in [findPlexTrackForMpvSubtitle]: a Plex stream + // carrying no container ordinal still falls back to metadata scoring. if (mpvTrack.isContainer && plexOrdinal >= 0 && !ordinalMatches) continue; final score = _scoreSubtitleMatch(mpvTrack, plexTrack, ordinalMatches: ordinalMatches); @@ -191,6 +193,8 @@ MediaSubtitleTrack? findPlexTrackForMpvSubtitle( final ordinalMatches = containerPlexTracks != null && mpvOrdinal >= 0 && containerPlexTracks.indexOf(plexTrack) == mpvOrdinal; + // The probe fixes isContainer here, so once a container ordinal list exists + // a container track matches at its own ordinal or not at all. if (mpvTrack.isContainer && containerPlexTracks != null && !ordinalMatches) continue; final score = _scoreSubtitleMatch(mpvTrack, plexTrack, ordinalMatches: ordinalMatches); @@ -217,6 +221,8 @@ AudioTrack? findMpvTrackForPlexAudio( AudioTrack? bestMatch; int bestScore = 0; + // Ordinal identity is cross-side: the probe's index in the Plex list against + // the candidate's index in the MPV list. final plexOrdinal = allPlexTracks?.indexOf(plexTrack) ?? -1; for (final mpvTrack in mpvTracks) { @@ -244,6 +250,8 @@ MediaAudioTrack? findPlexTrackForMpvAudio( MediaAudioTrack? bestMatch; int bestScore = 0; + // Same cross-side ordinal rule as [findMpvTrackForPlexAudio] with the two + // lists swapped; the score arguments stay MPV-first either way. final mpvOrdinal = allMpvTracks?.indexOf(mpvTrack) ?? -1; for (final plexTrack in plexTracks) { diff --git a/lib/services/trackers/anilist/anilist_client.dart b/lib/services/trackers/anilist/anilist_client.dart index 91247283..214dcccb 100644 --- a/lib/services/trackers/anilist/anilist_client.dart +++ b/lib/services/trackers/anilist/anilist_client.dart @@ -408,6 +408,8 @@ class AnilistClient implements DisposableTrackerClient { final res = await send(); + // Rate limits are typed here and in Trakt only; MAL and Simkl surface a 429 + // as a plain TrackerApiException. if (res.statusCode == 429) { throw TrackerRateLimitException( service: TrackerService.anilist, diff --git a/lib/services/trackers/mal/mal_client.dart b/lib/services/trackers/mal/mal_client.dart index 9cd09e52..595f3024 100644 --- a/lib/services/trackers/mal/mal_client.dart +++ b/lib/services/trackers/mal/mal_client.dart @@ -193,6 +193,8 @@ class MalClient implements DisposableTrackerClient { try { await _refresh(); } catch (_) { + // Reported as an API 401, not as the TrackerAuthException Trakt + // propagates from the same path. throw const TrackerApiException(service: TrackerService.mal, statusCode: 401); } res = await _send(method, path, body: body, formBody: formBody); diff --git a/lib/services/trackers/simkl/simkl_client.dart b/lib/services/trackers/simkl/simkl_client.dart index dace4a59..99e2cfe7 100644 --- a/lib/services/trackers/simkl/simkl_client.dart +++ b/lib/services/trackers/simkl/simkl_client.dart @@ -154,6 +154,8 @@ class SimklClient implements DisposableTrackerClient { allowedMethods: const {'GET', 'POST'}, ); + // Only the authenticated host may invalidate: the data host is called + // without a token, so its 401s say nothing about the session. if (mainApiHost && response.statusCode == 401) { onSessionInvalidated(); throw const TrackerAuthException( diff --git a/lib/services/trackers/tracker_http_client.dart b/lib/services/trackers/tracker_http_client.dart index 8a96c167..3c8b541b 100644 --- a/lib/services/trackers/tracker_http_client.dart +++ b/lib/services/trackers/tracker_http_client.dart @@ -9,6 +9,14 @@ import '../../utils/platform_http_client_stub.dart' as platform; import 'tracker_constants.dart'; +/// Transport shared by the tracker clients: builds, times and logs a request, +/// then hands back the raw response. +/// +/// Status handling stays with each client because the rules genuinely differ: +/// MAL and Simkl accept any 2xx, Trakt a per-call set (200/201/204, plus 409 +/// for scrobble), AniList only 200 (GraphQL errors ride a 200 body); and a 401 +/// means refresh-and-retry for Trakt/MAL but a terminal session for AniList +/// and Simkl. class TrackerHttpClient { static const Set allMethods = {'GET', 'POST', 'PATCH', 'PUT', 'DELETE'}; diff --git a/lib/services/trakt/trakt_client.dart b/lib/services/trakt/trakt_client.dart index f686c1e9..4facd12f 100644 --- a/lib/services/trakt/trakt_client.dart +++ b/lib/services/trakt/trakt_client.dart @@ -307,6 +307,8 @@ class TraktClient implements DisposableTrackerClient { var res = await _send(method, path, body: body); if (res.statusCode == 401) { + // A failed refresh propagates its TrackerAuthException; MAL's equivalent + // path flattens the same failure into TrackerApiException(401). await refresh(); res = await _send(method, path, body: body); } diff --git a/lib/utils/abortable_http_request.dart b/lib/utils/abortable_http_request.dart index fce70294..2dedb64e 100644 --- a/lib/utils/abortable_http_request.dart +++ b/lib/utils/abortable_http_request.dart @@ -14,6 +14,9 @@ Future sendAbortableHttpRequest( Future? abortTrigger, String? operation, }) { + // Deliberately not `AbortController`: that type lives with the media-server + // client and throws `MediaServerHttpException`, which the tracker/Seerr + // callers of this helper must stay independent of. final abort = Completer(); void abortRequest() { if (!abort.isCompleted) abort.complete(); diff --git a/lib/utils/managed_http_client.dart b/lib/utils/managed_http_client.dart index bbdf8770..31644e1b 100644 --- a/lib/utils/managed_http_client.dart +++ b/lib/utils/managed_http_client.dart @@ -240,6 +240,9 @@ class _ManagedStreamedResponseWithUrl extends http.StreamedResponse implements h final Uri url; } +/// Deliberately not `AbortController`: this layer stays a plain [http.Client] +/// with no media-server dependency, and it needs two independent latches +/// (aborted vs. drained) plus the response canceller. class _TrackedRequest { _TrackedRequest(this.url); diff --git a/lib/utils/media_server_http_client.dart b/lib/utils/media_server_http_client.dart index c0a2f732..52e61524 100644 --- a/lib/utils/media_server_http_client.dart +++ b/lib/utils/media_server_http_client.dart @@ -78,7 +78,16 @@ class AbortController { /// timeouts, logging, and optional endpoint failover. class MediaServerHttpClient { final http.Client _client; + + /// Requests owned by this client, aborted at the transport on shutdown so an + /// in-flight body raises [http.RequestAbortedException] instead of truncating. final Set _activeAborts = {}; + + /// Not delegated to [ManagedHttpClient]'s own closing guard: that reports + /// shutdown as an [http.ClientException], which maps to + /// [MediaServerHttpErrorType.connectionError] and so reads as transient. + /// Failover, pagination and download retry all branch on + /// [MediaServerHttpException.isCancellation]. bool _closing = false; MediaServerHttpClient({ diff --git a/test/focus/dpad_activation_policy_test.dart b/test/focus/dpad_activation_policy_test.dart new file mode 100644 index 00000000..3b315c6d --- /dev/null +++ b/test/focus/dpad_activation_policy_test.dart @@ -0,0 +1,88 @@ +import 'package:flutter/material.dart'; +import 'package:flutter/services.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:plezy/focus/focusable_wrapper.dart'; +import 'package:plezy/widgets/focusable_tab_chip.dart'; + +void main() { + // FocusableWrapper and FocusableChipStateMixin run the same d-pad activation + // sequence under opposite consume policies. These pin the two differences that + // keep the handlers separate. + group('d-pad activation policies', () { + Future> escapedKeysFor( + WidgetTester tester, + FocusNode node, + Widget child, + LogicalKeyboardKey key, + ) async { + final escaped = []; + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: Focus( + onKeyEvent: (_, event) { + if (event is KeyDownEvent) escaped.add(event.logicalKey); + return KeyEventResult.handled; + }, + child: child, + ), + ), + ), + ); + node.requestFocus(); + await tester.pump(); + + await tester.sendKeyEvent(key); + await tester.pump(); + return escaped; + } + + testWidgets('wrapper consumes the context menu key with no onLongPress', (tester) async { + final node = FocusNode(debugLabel: 'card'); + addTearDown(node.dispose); + + final escaped = await escapedKeysFor( + tester, + node, + FocusableWrapper(focusNode: node, onSelect: () {}, child: const SizedBox(width: 10, height: 10)), + LogicalKeyboardKey.contextMenu, + ); + + expect(escaped, isEmpty); + }); + + testWidgets('chip leaves the context menu key to its ancestors with no onLongPress', (tester) async { + final node = FocusNode(debugLabel: 'chip'); + addTearDown(node.dispose); + + final escaped = await escapedKeysFor( + tester, + node, + FocusableTabChip(label: 'Tab', isSelected: true, focusNode: node, onSelect: () {}), + LogicalKeyboardKey.contextMenu, + ); + + expect(escaped, [LogicalKeyboardKey.contextMenu]); + }); + + testWidgets('wrapper passes unmapped RIGHT/DOWN through to the framework', (tester) async { + final node = FocusNode(debugLabel: 'card'); + addTearDown(node.dispose); + Widget card() => FocusableWrapper(focusNode: node, onSelect: () {}, child: const SizedBox(width: 10, height: 10)); + + expect(await escapedKeysFor(tester, node, card(), LogicalKeyboardKey.arrowRight), [ + LogicalKeyboardKey.arrowRight, + ]); + expect(await escapedKeysFor(tester, node, card(), LogicalKeyboardKey.arrowDown), [LogicalKeyboardKey.arrowDown]); + }); + + testWidgets('chip traps unmapped RIGHT/DOWN so focus cannot escape the strip', (tester) async { + final node = FocusNode(debugLabel: 'chip'); + addTearDown(node.dispose); + Widget chip() => FocusableTabChip(label: 'Tab', isSelected: true, focusNode: node, onSelect: () {}); + + expect(await escapedKeysFor(tester, node, chip(), LogicalKeyboardKey.arrowRight), isEmpty); + expect(await escapedKeysFor(tester, node, chip(), LogicalKeyboardKey.arrowDown), isEmpty); + }); + }); +} diff --git a/test/services/track_selection_container_ordinal_test.dart b/test/services/track_selection_container_ordinal_test.dart new file mode 100644 index 00000000..b639db2b --- /dev/null +++ b/test/services/track_selection_container_ordinal_test.dart @@ -0,0 +1,36 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:plezy/media/media_source_info.dart'; +import 'package:plezy/mpv/mpv.dart'; +import 'package:plezy/services/track_selection_service.dart'; + +// The container-ordinal guards in `findMpvTrackForPlexSubtitle` and +// `findPlexTrackForMpvSubtitle` look like mirrors but are not: when the probe +// has no ordinal in the container list, the Plex->MPV direction still scores by +// metadata while the MPV->Plex direction refuses to match at all. These tests +// pin that difference so the two guards are not "symmetrised". + +MediaSubtitleTrack _plexSub(int id, {int? index, String? languageCode}) => + MediaSubtitleTrack(id: id, index: index, languageCode: languageCode, selected: false, forced: false); + +SubtitleTrack _containerSub(String id, {String? lang}) => + SubtitleTrack(id: id, language: lang, isExternal: true, isContainer: true); + +void main() { + group('container-ordinal guard asymmetry', () { + test('Plex->MPV keeps metadata scoring when the Plex stream has no container ordinal', () { + final probe = _plexSub(40, index: 0, languageCode: 'eng'); + final otherPlexTracks = [_plexSub(41, index: 1, languageCode: 'eng')]; + final nativeTracks = [_containerSub('2_0', lang: 'eng')]; + + expect(findMpvTrackForPlexSubtitle(probe, nativeTracks, allPlexTracks: otherPlexTracks), nativeTracks.first); + }); + + test('MPV->Plex refuses to match when the container track has no ordinal', () { + final probe = _containerSub('2_0', lang: 'eng'); + final otherNativeTracks = [_containerSub('2_1', lang: 'eng')]; + final plexTracks = [_plexSub(40, index: 0, languageCode: 'eng')]; + + expect(findPlexTrackForMpvSubtitle(probe, plexTracks, allMpvTracks: otherNativeTracks), isNull); + }); + }); +} diff --git a/test/services/trackers/tracker_status_ladder_test.dart b/test/services/trackers/tracker_status_ladder_test.dart new file mode 100644 index 00000000..28b81a9c --- /dev/null +++ b/test/services/trackers/tracker_status_ladder_test.dart @@ -0,0 +1,106 @@ +import 'dart:convert'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:http/http.dart' as http; +import 'package:http/testing.dart'; +import 'package:plezy/models/trakt/trakt_ids.dart'; +import 'package:plezy/models/trakt/trakt_scrobble_request.dart'; +import 'package:plezy/services/trackers/simkl/simkl_client.dart'; +import 'package:plezy/services/trackers/simkl/simkl_constants.dart'; +import 'package:plezy/services/trackers/tracker_exceptions.dart'; +import 'package:plezy/services/trackers/tracker_session.dart'; +import 'package:plezy/services/trakt/trakt_client.dart'; + +TrackerSession _session({String refreshToken = 'refresh-old'}) { + final now = DateTime.now().millisecondsSinceEpoch ~/ 1000; + return TrackerSession( + accessToken: 'access-old', + refreshToken: refreshToken, + expiresAt: now + 86400, + createdAt: now, + username: 'alice', + ); +} + +const _scrobble = TraktScrobbleRequest.movie(ids: TraktIds(trakt: 1)); + +void main() { + group('Trakt status ladder', () { + test('accepts 409 on scrobble but not on other requests', () async { + final client = TraktClient( + _session(), + onSessionInvalidated: () => fail('409 should not invalidate the session'), + httpClient: MockClient((_) async => http.Response('conflict', 409)), + ); + addTearDown(client.dispose); + + await client.scrobbleStart(_scrobble); + + await expectLater( + client.getUserSettings(), + throwsA(isA().having((e) => e.statusCode, 'statusCode', 409)), + ); + }); + + test('propagates the refresh TrackerAuthException after a 401', () async { + var invalidated = 0; + final client = TraktClient( + _session(refreshToken: 'refresh-ladder'), + onSessionInvalidated: () => invalidated++, + httpClient: MockClient((request) async { + if (request.url.path == '/oauth/token') { + return http.Response(json.encode({'error': 'invalid_grant'}), 400); + } + return http.Response('unauthorized', 401); + }), + ); + addTearDown(client.dispose); + + await expectLater( + client.getUserSettings(), + throwsA(isA().having((e) => e.isPermanent, 'isPermanent', isTrue)), + ); + expect(invalidated, 1); + }); + }); + + group('Simkl status ladder', () { + test('only the authenticated host invalidates on 401', () async { + var invalidated = 0; + final client = SimklClient( + _session(), + onSessionInvalidated: () => invalidated++, + httpClient: MockClient((_) async => http.Response('unauthorized', 401)), + ); + addTearDown(client.dispose); + + await expectLater(client.getTrending(SimklCatalogType.tv), throwsA(isA())); + expect(invalidated, 0); + + await expectLater( + client.getUserSettings(), + throwsA(isA().having((e) => e.isPermanent, 'isPermanent', isTrue)), + ); + expect(invalidated, 1); + }); + + test('surfaces 429 as a plain API failure', () async { + final client = SimklClient( + _session(), + onSessionInvalidated: () => fail('429 should not invalidate the session'), + httpClient: MockClient((_) async => http.Response('slow down', 429, headers: {'retry-after': '23'})), + ); + addTearDown(client.dispose); + + await expectLater( + client.getUserSettings(), + throwsA( + allOf( + isA().having((e) => e.statusCode, 'statusCode', 429), + isNot(isA()), + ), + ), + ); + }); + }); +} diff --git a/test/utils/media_server_http_client_shutdown_test.dart b/test/utils/media_server_http_client_shutdown_test.dart new file mode 100644 index 00000000..77387f04 --- /dev/null +++ b/test/utils/media_server_http_client_shutdown_test.dart @@ -0,0 +1,83 @@ +import 'dart:async'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:http/http.dart' as http; +import 'package:plezy/exceptions/media_server_exceptions.dart'; +import 'package:plezy/utils/managed_http_client.dart'; +import 'package:plezy/utils/media_server_http_client.dart'; + +void main() { + group('MediaServerHttpClient shutdown', () { + test('rejects new requests as a cancellation, not a transient failure', () async { + final client = MediaServerHttpClient( + client: ManagedHttpClient(_AbortAwareClient(), debugLabel: 'test'), + baseUrl: 'https://example.test/', + ); + + client.close(); + + await expectLater( + client.get('library/sections'), + throwsA( + isA() + .having((e) => e.type, 'type', MediaServerHttpErrorType.cancelled) + .having((e) => e.isTransient, 'isTransient', isFalse), + ), + ); + }); + + test('the layer beneath reports the same shutdown as a transient connection error', () async { + final managed = ManagedHttpClient(_AbortAwareClient(), debugLabel: 'test'); + await managed.closeGracefully(drainTimeout: Duration.zero); + + await expectLater( + managed.send(http.Request('GET', Uri.parse('https://example.test/library/sections'))), + throwsA( + isA() + .having( + (e) => MediaServerHttpException.from(e).type, + 'mapped type', + MediaServerHttpErrorType.connectionError, + ) + .having((e) => MediaServerHttpException.from(e).isTransient, 'mapped isTransient', isTrue), + ), + ); + }); + + test('aborts requests already in flight at the transport', () async { + final transport = _AbortAwareClient(); + final client = MediaServerHttpClient(client: transport, baseUrl: 'https://example.test/'); + + final pending = client.get('library/sections'); + await Future.delayed(Duration.zero); + + client.close(); + + await expectLater(transport.abortTrigger, completes); + await expectLater( + pending, + throwsA(isA().having((e) => e.type, 'type', MediaServerHttpErrorType.cancelled)), + ); + }); + }); +} + +class _AbortAwareClient extends http.BaseClient { + final _response = Completer(); + late final Future abortTrigger; + + @override + Future send(http.BaseRequest request) { + final trigger = (request as http.Abortable).abortTrigger!; + abortTrigger = trigger; + unawaited( + trigger.whenComplete(() { + if (!_response.isCompleted) _response.completeError(http.RequestAbortedException(request.url)); + }), + ); + return _response.future; + } + + @override + void close() {} +}