refactor: share future coalescing, Plex client access, and event helpers

Deduplicates the hand-rolled coalescing/caching maps, the Plex client cast,
the missing-serverId event guard and the progress-failure backoff, and drops
the MusicPlaybackService availability gate, which could never fail in
production.
This commit is contained in:
edde746
2026-07-26 06:09:50 +02:00
parent 9429a76acc
commit eb3ed45af1
21 changed files with 118 additions and 154 deletions
@@ -41,7 +41,6 @@ import '../../../widgets/media_card_list_layout.dart';
import '../../../widgets/bottom_sheet_page_scaffold.dart';
import '../../../widgets/overlay_sheet.dart';
import '../../../mixins/library_tab_focus_mixin.dart';
import '../../../services/plex_client.dart';
import '../folder_tree_view.dart';
import '../filters_bottom_sheet.dart';
import '../sort_bottom_sheet.dart';
@@ -1030,8 +1029,8 @@ class _LibraryBrowseTabState extends BaseLibraryTabState<MediaItem, LibraryBrows
Future<List<MediaFilterValue>> _loadFilterValues(MediaFilter filter) async {
if (!mounted) return const [];
final client = context.tryGetMediaClientForServer(serverIdOrNull(widget.library.serverId));
if (client is PlexClient) return client.getFilterValues(filter.key);
final client = context.tryGetPlexClientForServer(serverIdOrNull(widget.library.serverId));
if (client != null) return client.getFilterValues(filter.key);
// Jellyfin's canonical filter values come from the cached `/Items/Filters`
// payload. If that payload missed a category, there is no neutral endpoint
+1 -2
View File
@@ -74,8 +74,7 @@ class _ArtistDetailScreenState extends BaseMediaListDetailScreen<ArtistDetailScr
}
/// Plays the artist's full track list. The tracks aren't part of the album
/// listing this screen loads, so this costs one extra server round-trip
/// gated on playback availability first so the stub never fetches.
/// listing this screen loads, so this costs one extra server round-trip.
Future<void> _playAll({bool shuffle = false}) async {
await playFetchedTracks(
context,
@@ -38,10 +38,6 @@ class MusicPlayContext {
/// discrete changes (track, status, queue shape, modes) — progress bars
/// subscribe to [positionStream] instead.
abstract class MusicPlaybackService extends ChangeNotifier {
/// False on the stub — playback affordances should render disabled or
/// fall back to a "not supported yet" notice.
bool get isAvailable;
MediaItem? get currentTrack;
MusicPlaybackStatus get status;
bool get isPlaying => status == MusicPlaybackStatus.playing;
@@ -158,8 +154,6 @@ class StubMusicPlaybackService extends MusicPlaybackService {
final ValueNotifier<double> _volumeNotifier = ValueNotifier<double>(100);
int _playIntentGeneration = 0;
int _queueSessionRevision = 0;
@override
bool get isAvailable => false;
@override
MediaItem? get currentTrack => null;
@@ -169,9 +169,6 @@ class MusicPlaybackServiceImpl extends MusicPlaybackService with WidgetsBindingO
// Getters
// ---------------------------------------------------------------------
@override
bool get isAvailable => true;
@override
MediaItem? get currentTrack => _currentTrack;
+13 -15
View File
@@ -255,27 +255,14 @@ class PlaybackProgressTracker {
}
})
.catchError((Object e) {
_consecutiveFailures++;
// Exponential backoff: skip 1, 2, 4, 8... ticks (capped at 6 ≈ 60s)
_ticksToSkip = (1 << (_consecutiveFailures - 1)).clamp(1, 6);
appLogger.d(
'Progress update failed ($_consecutiveFailures consecutive), '
'skipping next $_ticksToSkip tick(s)',
error: e,
);
_recordProgressFailure(e);
unawaited(_queueOnlineFailureProgress(position, duration));
}),
);
}
} catch (e) {
if (!isOffline) {
_consecutiveFailures++;
_ticksToSkip = (1 << (_consecutiveFailures - 1)).clamp(1, 6);
appLogger.d(
'Progress update failed ($_consecutiveFailures consecutive), '
'skipping next $_ticksToSkip tick(s)',
error: e,
);
_recordProgressFailure(e);
await _queueOnlineFailureProgress(
attemptedPosition ?? player.state.position,
attemptedDuration ?? player.state.duration,
@@ -303,6 +290,17 @@ class PlaybackProgressTracker {
}
}
void _recordProgressFailure(Object e) {
_consecutiveFailures++;
// Exponential backoff: skip 1, 2, 4, 8... ticks (capped at 6 ≈ 60s)
_ticksToSkip = (1 << (_consecutiveFailures - 1)).clamp(1, 6);
appLogger.d(
'Progress update failed ($_consecutiveFailures consecutive), '
'skipping next $_ticksToSkip tick(s)',
error: e,
);
}
void _resetBackoff() {
if (_consecutiveFailures > 0) {
_consecutiveFailures = 0;
@@ -3,6 +3,7 @@ import '../../media/media_kind.dart';
import '../../media/media_server_client.dart';
import '../../models/trackers/anime_lists_mapping.dart';
import '../../utils/app_logger.dart';
import 'future_coalescer.dart';
enum AnimeProgressScope { show, season, mapped }
@@ -30,7 +31,7 @@ abstract interface class AnimeEpisodeProgressLookup {
class AnimeEpisodeProgressResolver implements AnimeEpisodeProgressLookup {
final MediaServerClient _client;
final Map<String, Future<Map<int, _SeasonProgress>?>> _seasonProgressLoads = {};
final KeyedFutureCoalescer<String, Map<int, _SeasonProgress>?> _seasonProgressLoads = KeyedFutureCoalescer();
AnimeEpisodeProgressResolver(this._client);
@@ -61,7 +62,7 @@ class AnimeEpisodeProgressResolver implements AnimeEpisodeProgressLookup {
return includeCurrentEpisode ? ResolvedAnimeProgress(progress: animeMatch.anidbEpisode) : null;
}
final progressBySeason = await _seasonProgressFor(showId);
final progressBySeason = await _seasonProgressLoads.run(showId, () => _loadSeasonProgress(showId));
if (progressBySeason == null) return null;
final currentAlreadyWatched = (episode.viewCount ?? 0) > 0 || !includeCurrentEpisode;
@@ -97,20 +98,6 @@ class AnimeEpisodeProgressResolver implements AnimeEpisodeProgressLookup {
}
}
Future<Map<int, _SeasonProgress>?> _seasonProgressFor(String showId) async {
final existing = _seasonProgressLoads[showId];
if (existing != null) return existing;
late final Future<Map<int, _SeasonProgress>?> loading;
loading = _loadSeasonProgress(showId).whenComplete(() {
if (identical(_seasonProgressLoads[showId], loading)) {
final _ = _seasonProgressLoads.remove(showId);
}
});
_seasonProgressLoads[showId] = loading;
return loading;
}
ResolvedAnimeProgress? _showProgress(Map<int, _SeasonProgress> seasons, bool currentAlreadyWatched) {
if (seasons.isEmpty) return null;
var watched = 0;
@@ -1,12 +1,13 @@
import '../../models/trackers/anime_ids.dart';
import '../../models/trackers/tracker_context.dart';
import '../../utils/app_logger.dart';
import 'future_coalescer.dart';
import 'tracker.dart';
import 'tracker_id_resolver.dart';
mixin AnimeListTrackerBase<TClient extends DisposableTrackerClient> on TrackerBase, ClientBackedTracker<TClient>
implements TrackerRatingSource {
final Map<int, Future<int?>> _episodeCountLoads = {};
final KeyedFutureCache<int, int?> _episodeCountLoads = KeyedFutureCache();
@override
bool get needsFribb => true;
@@ -83,19 +84,11 @@ mixin AnimeListTrackerBase<TClient extends DisposableTrackerClient> on TrackerBa
return (activeClient, id);
}
Future<int?> _episodeCount(TClient activeClient, int id) {
final existing = _episodeCountLoads[id];
if (existing != null) return existing;
late final Future<int?> loading;
loading = loadAnimeEpisodeCount(activeClient, id).catchError((Object e) {
if (identical(_episodeCountLoads[id], loading)) {
final _ = _episodeCountLoads.remove(id);
}
appLogger.d('$logLabel: failed to fetch anime episode count ($name=$id)', error: e);
return null;
});
_episodeCountLoads[id] = loading;
return loading;
}
Future<int?> _episodeCount(TClient activeClient, int id) => _episodeCountLoads
.run(
id,
() => loadAnimeEpisodeCount(activeClient, id),
onError: (e) => appLogger.d('$logLabel: failed to fetch anime episode count ($name=$id)', error: e),
)
.catchError((Object _) => null);
}
@@ -38,4 +38,35 @@ class KeyedFutureCoalescer<K, T> {
_inFlight[key] = future;
return future;
}
/// Detach every in-flight future — the keyed form of [FutureCoalescer.reset].
void clear() {
_inFlight.clear();
}
}
/// Keyed cache of loads: like [KeyedFutureCoalescer], but a successful future
/// stays memoized instead of being dropped on completion, and only a failure
/// evicts the key so the next call retries. [onError] fires once per failed
/// load, before the error is rethrown to every caller.
class KeyedFutureCache<K, T> {
final Map<K, Future<T>> _entries = {};
Future<T> run(K key, Future<T> Function() create, {void Function(Object error)? onError}) {
final existing = _entries[key];
if (existing != null) return existing;
late final Future<T> future;
future = create().catchError((Object e) {
if (identical(_entries[key], future)) _entries.remove(key);
onError?.call(e);
throw e;
});
_entries[key] = future;
return future;
}
void clear() {
_entries.clear();
}
}
+4 -14
View File
@@ -8,6 +8,7 @@ import '../../utils/external_ids.dart';
import 'anime_episode_progress_resolver.dart';
import 'anime_lists_mapping_store.dart';
import 'fribb_mapping_store.dart';
import 'future_coalescer.dart';
/// Paired ID output: always-present Plex external IDs (tvdb/imdb/tmdb) plus
/// optional Fribb-sourced anime IDs (mal/anilist/simkl). Simkl uses [external]
@@ -77,7 +78,7 @@ class TrackerIdResolver {
/// Null entries mean "the server had no IDs" — cached so scrubbing on an
/// un-matched item doesn't re-hit the server every position update.
final Map<String, TrackerIds?> _cache = {};
final Map<String, Future<ExternalIds>> _externalIdLoads = {};
final KeyedFutureCache<String, ExternalIds> _externalIdLoads = KeyedFutureCache();
TrackerIdResolver(
MediaServerClient client, {
@@ -97,19 +98,8 @@ class TrackerIdResolver {
/// [MediaServerClient.fetchExternalIds] surface — Plex hits
/// `/library/metadata/{id}?includeGuids=1`, Jellyfin reads the inline
/// `ProviderIds` map.
Future<ExternalIds> _fetchExternalIds(String itemId) {
final existing = _externalIdLoads[itemId];
if (existing != null) return existing;
late final Future<ExternalIds> loading;
loading = _client.fetchExternalIds(itemId).catchError((Object e) {
if (identical(_externalIdLoads[itemId], loading)) {
final _ = _externalIdLoads.remove(itemId);
}
throw e;
});
_externalIdLoads[itemId] = loading;
return loading;
}
Future<ExternalIds> _fetchExternalIds(String itemId) =>
_externalIdLoads.run(itemId, () => _client.fetchExternalIds(itemId));
/// Resolve IDs for a movie.
Future<TrackerIds?> resolveForMovie(String itemId) async {
+3 -5
View File
@@ -4,6 +4,7 @@ import 'app_logger.dart';
import 'base_notifier.dart';
import 'global_key_utils.dart';
import 'hierarchical_event_mixin.dart';
import 'media_event_keys.dart';
/// Event representing a media item deletion with parent chain for hierarchical invalidation
class DeletionEvent with HierarchicalEventMixin {
@@ -73,11 +74,8 @@ class DeletionNotifier extends BaseNotifier<DeletionEvent> {
}
void notifyDeletedItem({required MediaItem item, bool isDownloadOnly = false}) {
final serverId = serverIdOrNull(item.serverId);
if (serverId == null) {
appLogger.w('DeletionNotifier: missing serverId for ${item.id}, skipping deletion event');
return;
}
final serverId = serverIdForEvent(item, notifier: 'DeletionNotifier', event: 'deletion');
if (serverId == null) return;
notify(
DeletionEvent(
itemId: item.id,
+12
View File
@@ -1,5 +1,6 @@
import '../media/ids.dart';
import '../media/media_item.dart';
import 'app_logger.dart';
import 'global_key_utils.dart';
/// Builds the id filter for a screen showing [items].
@@ -34,3 +35,14 @@ Set<String>? hierarchicalEventGlobalKeys(Iterable<MediaItem> items, {String? fal
}
return keys;
}
/// The [ServerId] an event emitted for [item] should carry, or `null` — after
/// warning as `<notifier>: … skipping <event> event` — when the item carries
/// none, since an event without a server id cannot be routed to subscribers.
ServerId? serverIdForEvent(MediaItem item, {required String notifier, required String event}) {
final serverId = serverIdOrNull(item.serverId);
if (serverId == null) {
appLogger.w('$notifier: missing serverId for ${item.id}, skipping $event event');
}
return serverId;
}
+4 -20
View File
@@ -1,7 +1,6 @@
import 'package:flutter/material.dart';
import 'package:provider/provider.dart';
import '../i18n/strings.g.dart';
import '../media/media_item.dart';
import '../navigation/profile_navigation_scope.dart';
import '../screens/music/album_detail_screen.dart';
@@ -14,7 +13,6 @@ import '../theme/mono_tokens.dart';
import 'app_logger.dart';
import 'platform_detector.dart';
import 'provider_extensions.dart';
import 'snackbar_helper.dart';
/// Route name of the now-playing screen — the mini-player's route observer
/// suppresses itself while this (or the video player) is in the stack.
@@ -90,17 +88,7 @@ void _autoOpenNowPlayingOnTv(BuildContext context) {
});
}
/// True when a real music playback engine is bound. On the stub this shows
/// the standard "not supported yet" notice and returns false — check it
/// BEFORE fetching tracks so the stub never costs a server round-trip.
bool ensureMusicPlaybackAvailable(BuildContext context) {
if (context.read<MusicPlaybackService>().isAvailable) return true;
showAppSnackBar(context, t.messages.musicNotSupported);
return false;
}
/// Start playback of [tracks] via the session [MusicPlaybackService],
/// surfacing the "not supported yet" notice while the stub is bound.
/// Start playback of [tracks] via the session [MusicPlaybackService].
Future<void> playTracks(
BuildContext context, {
required List<MediaItem> tracks,
@@ -108,7 +96,6 @@ Future<void> playTracks(
required MusicPlayContext playContext,
bool shuffle = false,
}) async {
if (!ensureMusicPlaybackAvailable(context)) return;
await context.read<MusicPlaybackService>().playFromList(
tracks: tracks,
startTrack: startTrack,
@@ -120,9 +107,9 @@ Future<void> playTracks(
/// Fetch a track list with [fetch], then play it — the shape every music
/// entry point that needs a server round-trip before playback repeats:
/// availability gate → [MusicPlaybackService.beginPlayIntent] → fetch →
/// mounted/intent re-check → [playTracks]. Guarding the round-trip with the
/// intent keeps a slow fetch from replacing a queue the user started later.
/// [MusicPlaybackService.beginPlayIntent] → fetch → mounted/intent re-check →
/// [playTracks]. Guarding the round-trip with the intent keeps a slow fetch
/// from replacing a queue the user started later.
///
/// [onError] reports a failed fetch and runs only while the intent is still
/// current and [context] mounted; passing null instead lets the failure
@@ -138,7 +125,6 @@ Future<void> playFetchedTracks(
MediaItem? startTrack,
bool shuffle = false,
}) async {
if (!ensureMusicPlaybackAvailable(context)) return;
final service = context.read<MusicPlaybackService>();
final intent = service.beginPlayIntent();
final List<MediaItem> tracks;
@@ -167,7 +153,6 @@ Future<void> playFetchedTracks(
/// must play under the *same* intent as the album fetch, so a stale fallback
/// can never supersede a newer request.
Future<void> playTrackWithAlbumContext(BuildContext context, MediaItem track) async {
if (!ensureMusicPlaybackAvailable(context)) return;
final service = context.read<MusicPlaybackService>();
final intent = service.beginPlayIntent();
@@ -206,7 +191,6 @@ Future<void> playTrackWithAlbumContext(BuildContext context, MediaItem track) as
/// Only call when the seed's server advertises
/// `ServerCapabilities.instantMix`.
Future<void> playInstantMix(BuildContext context, MediaItem seed) async {
if (!ensureMusicPlaybackAvailable(context)) return;
await context.read<MusicPlaybackService>().playInstantMix(seed);
if (context.mounted) _autoOpenNowPlayingOnTv(context);
}
+7 -15
View File
@@ -5,6 +5,7 @@ import 'app_logger.dart';
import 'base_notifier.dart';
import 'global_key_utils.dart';
import 'hierarchical_event_mixin.dart';
import 'media_event_keys.dart';
enum WatchStateChangeType { watched, unwatched, progressUpdate, removedFromContinueWatching }
@@ -98,11 +99,8 @@ class WatchStateNotifier extends BaseNotifier<WatchStateEvent> {
/// Helper to emit a watched/unwatched event from a [MediaItem].
void notifyWatched({required MediaItem item, bool isNowWatched = true, String? cacheServerId}) {
final serverId = serverIdOrNull(item.serverId);
if (serverId == null) {
appLogger.w('WatchStateNotifier: missing serverId for ${item.id}, skipping watched event');
return;
}
final serverId = serverIdForEvent(item, notifier: 'WatchStateNotifier', event: 'watched');
if (serverId == null) return;
notify(
WatchStateEvent(
itemId: item.id,
@@ -127,11 +125,8 @@ class WatchStateNotifier extends BaseNotifier<WatchStateEvent> {
String? cacheServerId,
double watchedThreshold = 0.9,
}) {
final serverId = serverIdOrNull(item.serverId);
if (serverId == null) {
appLogger.w('WatchStateNotifier: missing serverId for ${item.id}, skipping progress event');
return;
}
final serverId = serverIdForEvent(item, notifier: 'WatchStateNotifier', event: 'progress');
if (serverId == null) return;
final isNowWatched = isWatchedProgress(positionMs: viewOffset, durationMs: duration, threshold: watchedThreshold);
notify(
@@ -151,11 +146,8 @@ class WatchStateNotifier extends BaseNotifier<WatchStateEvent> {
/// Helper to emit a Continue Watching removal event.
void notifyRemovedFromContinueWatching({required MediaItem item}) {
final serverId = serverIdOrNull(item.serverId);
if (serverId == null) {
appLogger.w('WatchStateNotifier: missing serverId for ${item.id}, skipping continue-watching removal event');
return;
}
final serverId = serverIdForEvent(item, notifier: 'WatchStateNotifier', event: 'continue-watching removal');
if (serverId == null) return;
notify(
WatchStateEvent(
itemId: item.id,
+5 -7
View File
@@ -296,15 +296,13 @@ class MediaContextMenuState extends State<MediaContextMenu> {
_MenuAction(value: 'delete', icon: Symbols.delete_rounded, label: t.common.delete, destructive: true),
);
} else {
// Music (artist/album/track) playback + navigation actions. Play is
// always offered — the shared music_navigation helpers surface the
// "not supported yet" notice while the stub service is bound. Queue
// insertion only exists once a real playback engine is available.
// Music (artist/album/track) playback + navigation actions. Queue
// insertion only exists where a playback session is bound.
final isMusicKind = mediaKind != null && mediaKind.isMusic;
if (isMusicKind) {
menuActions.add(_MenuAction(value: 'music_play', icon: Symbols.play_arrow_rounded, label: t.common.play));
final musicAvailable = context.read<MusicPlaybackService?>()?.isAvailable ?? false;
final musicAvailable = context.read<MusicPlaybackService?>() != null;
if (musicAvailable) {
menuActions.add(
_MenuAction(value: 'music_play_next', icon: Symbols.playlist_play_rounded, label: t.music.playNext),
@@ -1076,8 +1074,8 @@ class MediaContextMenuState extends State<MediaContextMenu> {
Future<void> _handleMusicEnqueue(BuildContext context, {required bool playNext}) async {
final service = context.read<MusicPlaybackService?>();
// Menu entries are hidden on the stub; defensive re-check.
if (service == null || !service.isAvailable) return;
// Menu entries are hidden without a session; defensive re-check.
if (service == null) return;
final queueSessionRevision = service.queueSessionRevision;
List<MediaItem> tracks;
try {
@@ -10,7 +10,6 @@ import '../../../focus/input_mode_tracker.dart';
import '../../../i18n/strings.g.dart';
import '../../../mixins/controller_disposer_mixin.dart';
import '../../../models/plex/plex_subtitle_search_result.dart';
import '../../../services/plex_client.dart';
import '../../../services/settings_service.dart';
import '../../../utils/language_codes.dart';
import '../../../utils/provider_extensions.dart';
@@ -103,8 +102,7 @@ class _SubtitleSearchSheetState extends State<SubtitleSearchSheet> with Controll
});
try {
final neutral = context.tryGetMediaClientForServer(ServerId(widget.serverId));
final client = neutral is PlexClient ? neutral : null;
final client = context.tryGetPlexClientForServer(ServerId(widget.serverId));
if (client == null) {
if (!mounted || generation != _searchGeneration) return;
setState(() => _isSearching = false);
@@ -185,10 +183,7 @@ class _SubtitleSearchSheetState extends State<SubtitleSearchSheet> with Controll
setState(() => _downloadingKey = result.key);
try {
// Same Plex-only guard as in [_search]. Don't throw if a Jellyfin
// server somehow reaches the download path.
final neutral = context.tryGetMediaClientForServer(ServerId(widget.serverId));
final client = neutral is PlexClient ? neutral : null;
final client = context.tryGetPlexClientForServer(ServerId(widget.serverId));
if (client == null) {
if (!mounted) return;
setState(() => _downloadingKey = null);
@@ -52,19 +52,6 @@ void main() {
// Track numbers restart per disc.
expect(find.text('1'), findsNWidgets(2));
});
testWidgets('tapping a track on the stub service shows the not-supported notice', (tester) async {
final harness = await _createHarness(_multiDiscTracks());
await tester.pumpWidget(harness.wrap(const AlbumDetailScreen(album: _album)));
await tester.pumpAndSettle();
await tester.tap(find.text('Track One'));
await tester.pump();
await tester.pump(const Duration(milliseconds: 300));
expect(find.text(t.messages.musicNotSupported), findsOneWidget);
});
}
const _album = MediaItem.plex(
@@ -54,9 +54,6 @@ class _FakeMusicService extends StubMusicPlaybackService {
_positionController.add(position);
}
@override
bool get isAvailable => true;
@override
MediaItem get currentTrack => track;
-3
View File
@@ -40,9 +40,6 @@ class _FakeQueueService extends StubMusicPlaybackService {
_FakeQueueService(this.tracks);
@override
bool get isAvailable => true;
@override
MediaItem? get currentTrack => tracks[1];
@@ -34,4 +34,26 @@ void main() {
expect(first, 1);
expect(second, 2);
});
test('KeyedFutureCache memoizes successes and evicts failures', () async {
final cache = KeyedFutureCache<String, int>();
final errors = <Object>[];
var calls = 0;
Future<int> create({required bool fail}) async {
calls++;
if (fail) throw StateError('boom');
return calls;
}
await expectLater(cache.run('a', () => create(fail: true), onError: errors.add), throwsStateError);
expect(errors, hasLength(1));
expect(await cache.run('a', () => create(fail: false)), 2);
expect(await cache.run('a', () => create(fail: false)), 2);
expect(calls, 2);
cache.clear();
expect(await cache.run('a', () => create(fail: false)), 3);
});
}
@@ -670,9 +670,6 @@ class _RecordingMusicPlaybackService extends StubMusicPlaybackService {
bool? shuffle;
int callCount = 0;
@override
bool get isAvailable => true;
@override
Future<void> playFromList({
required List<MediaItem> tracks,
-3
View File
@@ -74,9 +74,6 @@ class _FakeMusicService extends StubMusicPlaybackService {
_FakeMusicService({this.track});
@override
bool get isAvailable => true;
@override
MediaItem? get currentTrack => track;