From 70cd7e8c67cd6179caf8616e2b3b9516c917e6d1 Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Fri, 17 Jul 2026 16:30:10 +0200 Subject: [PATCH] fix(music): keep now playing state current close #1600 --- lib/media/media_item.dart | 4 + lib/screens/music/artist_detail_screen.dart | 7 +- lib/screens/music/now_playing_screen.dart | 76 ++++-- .../playlist/playlist_detail_screen.dart | 7 +- .../music/music_playback_service.dart | 41 +++- .../music/music_playback_service_impl.dart | 127 ++++++++-- lib/services/plex_mappers.dart | 13 +- lib/services/plex_mappers.g.dart | 2 + lib/utils/music_navigation.dart | 6 +- lib/widgets/media_context_menu.dart | 32 ++- lib/widgets/music/mini_player.dart | 4 +- test/media/media_item_test.dart | 14 ++ .../music/now_playing_screen_test.dart | 226 ++++++++++++++++++ test/services/jellyfin_music_mapper_test.dart | 1 + .../music/music_playback_service_test.dart | 94 +++++++- test/services/plex_mappers_test.dart | 5 + test/widgets/media_context_menu_test.dart | 91 ++++++- test/widgets/music/mini_player_test.dart | 58 +++++ 18 files changed, 741 insertions(+), 67 deletions(-) create mode 100644 test/screens/music/now_playing_screen_test.dart diff --git a/lib/media/media_item.dart b/lib/media/media_item.dart index fba642cf..42b05a5c 100644 --- a/lib/media/media_item.dart +++ b/lib/media/media_item.dart @@ -526,6 +526,10 @@ sealed class MediaItem with _$MediaItem { _ => null, }; + /// Release year for music items. Track mappers normalize the containing + /// album's year into [year] when the backend exposes it as parent metadata. + int? get albumYear => kind == MediaKind.track || kind == MediaKind.album ? year : null; + /// Album-artist name for music items: a track's grandparent, an album's /// parent. String? get albumArtistTitle => switch (kind) { diff --git a/lib/screens/music/artist_detail_screen.dart b/lib/screens/music/artist_detail_screen.dart index c07d3844..60b31c68 100644 --- a/lib/screens/music/artist_detail_screen.dart +++ b/lib/screens/music/artist_detail_screen.dart @@ -82,15 +82,18 @@ class _ArtistDetailScreenState extends BaseMediaListDetailScreen _playAll({bool shuffle = false}) async { if (!ensureMusicPlaybackAvailable(context)) return; + final service = context.read(); + final intent = service.beginPlayIntent(); List tracks; try { tracks = await mediaClient.fetchPlayableDescendants(widget.artist.id); } catch (e, stackTrace) { + if (!mounted || !service.isPlayIntentCurrent(intent)) return; final message = localizedLoadErrorMessage(e, stackTrace, context: widget.artist.displayTitle); - if (mounted) showErrorSnackBar(context, message); + showErrorSnackBar(context, message); return; } - if (!mounted) return; + if (!mounted || !service.isPlayIntentCurrent(intent)) return; if (tracks.isEmpty) { showAppSnackBar(context, emptyMessage); return; diff --git a/lib/screens/music/now_playing_screen.dart b/lib/screens/music/now_playing_screen.dart index 3cf2e608..bc3450f6 100644 --- a/lib/screens/music/now_playing_screen.dart +++ b/lib/screens/music/now_playing_screen.dart @@ -81,10 +81,7 @@ class _NowPlayingScreenState extends State /// ValueListenableBuilder-wrapped Transform so drag frames never rebuild /// the screen. final ValueNotifier _dismissDrag = ValueNotifier(0); - late final AnimationController _dismissSettle = AnimationController( - vsync: this, - duration: const Duration(milliseconds: 200), - )..addListener(_onDismissSettleTick); + late final AnimationController _dismissSettle; double _dismissSettleFrom = 0; final FocusNode _seekFocusNode = FocusNode(debugLabel: 'now_playing_seek'); @@ -95,6 +92,13 @@ class _NowPlayingScreenState extends State bool _poppedForIdle = false; + @override + void initState() { + super.initState(); + _dismissSettle = AnimationController(vsync: this, duration: const Duration(milliseconds: 200)) + ..addListener(_onDismissSettleTick); + } + @override void didChangeDependencies() { super.didChangeDependencies(); @@ -306,7 +310,7 @@ class _NowPlayingScreenState extends State Widget _buildPortraitLayout(MusicPlaybackService service, MediaItem track, MediaServerClient? client) { Widget upper = Column( children: [ - _buildTopBar(track, service.playContext?.title), + _buildTopBar(track, service.playContext), Expanded( child: Padding( padding: const EdgeInsets.symmetric(horizontal: 24, vertical: 8), @@ -332,7 +336,7 @@ class _NowPlayingScreenState extends State return Column( children: [ Expanded(child: upper), - Padding(padding: const EdgeInsets.fromLTRB(16, 8, 16, 0), child: _buildSeekBar()), + Padding(padding: const EdgeInsets.fromLTRB(16, 8, 16, 0), child: _buildSeekBar(track)), _buildTransportRow(service), _buildUtilityRow(showQueueButton: true), const SizedBox(height: 8), @@ -344,7 +348,7 @@ class _NowPlayingScreenState extends State final tk = tokens(context); return Column( children: [ - _buildTopBar(track, service.playContext?.title), + _buildTopBar(track, service.playContext), Expanded( child: Padding( padding: const EdgeInsets.fromLTRB(24, 0, 24, 20), @@ -360,7 +364,7 @@ class _NowPlayingScreenState extends State children: [ _buildTrackInfo(track, centered: false), const SizedBox(height: 8), - _buildSeekBar(), + _buildSeekBar(track), _buildWideControlBand(service), const SizedBox(height: 12), // Inline queue panel — same widget the queue sheet uses. @@ -386,7 +390,7 @@ class _NowPlayingScreenState extends State Widget _buildTvLayout(MusicPlaybackService service, MediaItem track, MediaServerClient? client) { final tk = tokens(context); final textTheme = Theme.of(context).textTheme; - final playContextTitle = service.playContext?.title; + final sourceTitle = _playingFromTitle(track, service.playContext); final artist = track.trackArtistTitle; return Padding( @@ -404,10 +408,10 @@ class _NowPlayingScreenState extends State Row( children: [ Expanded( - child: playContextTitle == null || playContextTitle.isEmpty + child: sourceTitle == null ? const SizedBox.shrink() : Text( - t.music.playingFrom(title: playContextTitle), + t.music.playingFrom(title: sourceTitle), maxLines: 1, overflow: .ellipsis, style: TextStyle(fontSize: 14, color: tk.textMuted), @@ -433,7 +437,7 @@ class _NowPlayingScreenState extends State ), ], const SizedBox(height: 28), - _buildSeekBar(), + _buildSeekBar(track), const SizedBox(height: 8), _buildTransportRow(service), _buildUtilityRow(showQueueButton: true), @@ -449,11 +453,12 @@ class _NowPlayingScreenState extends State // Pieces // ------------------------------------------------------------------- - /// [playContextTitle] is passed in (not selected) because this builds + /// [playContext] is passed in (not selected) because this builds /// inside the layout-phase LayoutBuilder, where `context.select` on the /// screen's element asserts; the screen already watches the service. - Widget _buildTopBar(MediaItem track, String? playContextTitle) { + Widget _buildTopBar(MediaItem track, MusicPlayContext? playContext) { final tk = tokens(context); + final sourceTitle = _playingFromTitle(track, playContext); // macOS pins the traffic lights at y=21 (16pt buttons → center 29, see // WindowUtilsPlugin.customButtonPositions); a fixed 58px row centers the // close button on that line regardless of the platform visual density @@ -479,10 +484,10 @@ class _NowPlayingScreenState extends State context: context, )!, Expanded( - child: playContextTitle == null || playContextTitle.isEmpty + child: sourceTitle == null ? const SizedBox.shrink() : Text( - t.music.playingFrom(title: playContextTitle), + t.music.playingFrom(title: sourceTitle), textAlign: TextAlign.center, maxLines: 1, overflow: .ellipsis, @@ -496,6 +501,19 @@ class _NowPlayingScreenState extends State ); } + /// Album and ad-hoc queues follow the active track's album as the queue + /// crosses album boundaries. Artist/playlist/mix contexts remain stable + /// provenance labels for the session they created. + String? _playingFromTitle(MediaItem track, MusicPlayContext? playContext) { + final followsCurrentAlbum = + playContext == null || + playContext.kind == MusicPlayContextKind.album || + playContext.kind == MusicPlayContextKind.tracks; + final title = (followsCurrentAlbum ? track.albumTitle : playContext.title)?.trim(); + if (title == null || title.isEmpty) return null; + return toBulletedString([title, if (followsCurrentAlbum && track.albumYear != null) '${track.albumYear}']); + } + /// Wide-layout control band: the transport row stays exactly centered /// (equal-width flanks), with lyrics + a desktop volume slider as a /// right-aligned cluster that scales down instead of overflowing when the @@ -670,8 +688,9 @@ class _NowPlayingScreenState extends State ); } - Widget _buildSeekBar() { + Widget _buildSeekBar(MediaItem track) { return _NowPlayingSeekBar( + trackKey: track.globalKey, focusNode: _seekFocusNode, onNavigateUp: PlatformDetector.isTV() ? _overflowFocusNode.requestFocus : null, onNavigateDown: _focusTransport, @@ -980,12 +999,14 @@ class _PlayPauseButton extends StatelessWidget { /// timeline's stepped key-repeat acceleration; focus renders as a /// text-based background pill behind the bar. class _NowPlayingSeekBar extends StatefulWidget { + final String trackKey; final FocusNode focusNode; final VoidCallback? onNavigateUp; final VoidCallback? onNavigateDown; final VoidCallback onBack; const _NowPlayingSeekBar({ + required this.trackKey, required this.focusNode, required this.onNavigateUp, required this.onNavigateDown, @@ -1012,13 +1033,27 @@ class _NowPlayingSeekBarState extends State<_NowPlayingSeekBar> { _keySeek = DebouncedSeekAccumulator( currentPosition: () => context.read().position, duration: () => context.read().duration ?? Duration.zero, - seek: (target) => unawaited(context.read().seek(target)), + seek: (target) { + final service = context.read(); + if (service.currentTrack?.globalKey == widget.trackKey) { + unawaited(service.seek(target)); + } + }, onChanged: () { if (mounted) setState(() {}); }, ); } + @override + void didUpdateWidget(covariant _NowPlayingSeekBar oldWidget) { + super.didUpdateWidget(oldWidget); + if (oldWidget.trackKey == widget.trackKey) return; + _keySeek.cancel(); + _dragValueMs = null; + _resetSeekState(); + } + @override void dispose() { _keySeek.dispose(); @@ -1082,6 +1117,7 @@ class _NowPlayingSeekBarState extends State<_NowPlayingSeekBar> { final showFocus = _focused && InputModeTracker.isKeyboardMode(context); final bar = StreamBuilder( + key: ValueKey(widget.trackKey), stream: service.positionStream, builder: (context, snapshot) { final duration = service.duration ?? Duration.zero; @@ -1120,7 +1156,9 @@ class _NowPlayingSeekBarState extends State<_NowPlayingSeekBar> { onChanged: hasDuration ? (value) => setState(() => _dragValueMs = value) : null, onChangeEnd: hasDuration ? (value) { - unawaited(service.seek(Duration(milliseconds: value.round()))); + if (service.currentTrack?.globalKey == widget.trackKey) { + unawaited(service.seek(Duration(milliseconds: value.round()))); + } setState(() => _dragValueMs = null); } : null, diff --git a/lib/screens/playlist/playlist_detail_screen.dart b/lib/screens/playlist/playlist_detail_screen.dart index 21e7f6a4..0a8562ea 100644 --- a/lib/screens/playlist/playlist_detail_screen.dart +++ b/lib/screens/playlist/playlist_detail_screen.dart @@ -96,6 +96,8 @@ class _PlaylistDetailScreenState extends BaseMediaListDetailScreen(); + final intent = service.beginPlayIntent(); List tracks; if (_isPlaylistFullyLoaded) { tracks = items; @@ -103,12 +105,13 @@ class _PlaylistDetailScreenState extends BaseMediaListDetailScreen get errors; + /// Claims the latest user intent to replace playback after asynchronous + /// queue construction. Callers must check [isPlayIntentCurrent] before + /// committing fetched tracks. + int beginPlayIntent(); + + /// Whether [intent] is still the latest playback-replacement request. + bool isPlayIntentCurrent(int intent); + + /// Changes whenever a queue session starts or stops. Asynchronous enqueue + /// actions use this to avoid appending fetched tracks to a newer session. + int get queueSessionRevision; + /// Start a new queue from [tracks], optionally at [startTrack] (defaults /// to the first track). [shuffle] shuffles with the start track anchored /// first. @@ -146,6 +160,8 @@ abstract class MusicPlaybackService extends ChangeNotifier { /// null-safe without per-call-site feature checks. class StubMusicPlaybackService extends MusicPlaybackService { final ValueNotifier _volumeNotifier = ValueNotifier(100); + int _playIntentGeneration = 0; + int _queueSessionRevision = 0; @override bool get isAvailable => false; @@ -182,13 +198,25 @@ class StubMusicPlaybackService extends MusicPlaybackService { @override Stream get errors => const Stream.empty(); + @override + int beginPlayIntent() => ++_playIntentGeneration; + + @override + bool isPlayIntentCurrent(int intent) => intent == _playIntentGeneration; + + @override + int get queueSessionRevision => _queueSessionRevision; + @override Future playFromList({ required List tracks, MediaItem? startTrack, required MusicPlayContext playContext, bool shuffle = false, - }) async {} + }) async { + beginPlayIntent(); + _queueSessionRevision++; + } @override Future playInstantMix(MediaItem seed) async {} @@ -244,7 +272,10 @@ class StubMusicPlaybackService extends MusicPlaybackService { void clearUpcoming() {} @override - Future stop() async {} + Future stop() async { + beginPlayIntent(); + _queueSessionRevision++; + } @override bool get sleepTimerActive => false; diff --git a/lib/services/music/music_playback_service_impl.dart b/lib/services/music/music_playback_service_impl.dart index 65f1fd61..52348f62 100644 --- a/lib/services/music/music_playback_service_impl.dart +++ b/lib/services/music/music_playback_service_impl.dart @@ -137,6 +137,20 @@ class MusicPlaybackServiceImpl extends MusicPlaybackService with WidgetsBindingO /// (resolves, opens, arms) drop out instead of acting on the new state. int _generation = 0; + /// Queue construction can involve a server round-trip before playback is + /// replaced. Only the latest explicit play intent may commit its result. + int _playIntentGeneration = 0; + + /// Identifies the queue session that asynchronous enqueue work belongs to. + int _queueSessionRevision = 0; + + /// Gapless arm work is serialized, and each requested recomputation gets + /// a revision. This prevents an older resolve/setNext continuation from + /// landing after a queue edit, repeat change, or sleep-timer change. + int _armRequestGeneration = 0; + int _processedArmRequestGeneration = 0; + Future? _armDrain; + int _consecutiveFailures = 0; bool _resumeAfterInterruption = false; bool _disposed = false; @@ -208,6 +222,15 @@ class MusicPlaybackServiceImpl extends MusicPlaybackService with WidgetsBindingO @override bool get sleepTimerEndOfTrack => _sleepTimerEndOfTrack; + @override + int beginPlayIntent() => ++_playIntentGeneration; + + @override + bool isPlayIntentCurrent(int intent) => !_disposed && intent == _playIntentGeneration; + + @override + int get queueSessionRevision => _queueSessionRevision; + // --------------------------------------------------------------------- // Session start // --------------------------------------------------------------------- @@ -219,25 +242,30 @@ class MusicPlaybackServiceImpl extends MusicPlaybackService with WidgetsBindingO required MusicPlayContext playContext, bool shuffle = false, }) { + beginPlayIntent(); return _startQueue(tracks: tracks, startTrack: startTrack, playContext: playContext, shuffle: shuffle); } @override Future playInstantMix(MediaItem seed) async { + final intent = beginPlayIntent(); final client = _clientFor(seed); if (client == null) { - _errorsController.add(StateError('No server available for instant mix')); + if (isPlayIntentCurrent(intent)) { + _errorsController.add(StateError('No server available for instant mix')); + } return; } List tracks; try { tracks = await client.fetchInstantMix(seed.id); } catch (e, st) { + if (!isPlayIntentCurrent(intent)) return; appLogger.w('Instant mix fetch failed for ${seed.id}', error: e, stackTrace: st); _errorsController.add(e); return; } - if (_disposed || tracks.isEmpty) return; + if (!isPlayIntentCurrent(intent) || tracks.isEmpty) return; await _startQueue( tracks: tracks, playContext: MusicPlayContext(title: seed.displayTitle, kind: MusicPlayContextKind.mix), @@ -252,11 +280,14 @@ class MusicPlaybackServiceImpl extends MusicPlaybackService with WidgetsBindingO bool autoplay = true, }) async { if (tracks.isEmpty || _disposed) return; + beginPlayIntent(); + _queueSessionRevision++; // Android 13+: the background playback notification needs // POST_NOTIFICATIONS. Fire-and-forget — playback and the foreground // service run regardless; a denial only hides the notification. unawaited(ensureNotificationPermission()); final generation = ++_generation; + _invalidateArmRequests(); _finalizeCurrentTrack(); var startIndex = 0; if (startTrack != null) { @@ -333,13 +364,14 @@ class MusicPlaybackServiceImpl extends MusicPlaybackService with WidgetsBindingO _setStatus(play ? MusicPlaybackStatus.playing : MusicPlaybackStatus.paused); _bindTrackServices(track, source); - unawaited(_armNext(generation)); + _requestArmNext(); } /// Manual advance: finalize the current tracker at its current position and /// open the queue entry at [cursor]. Future _advanceTo(int cursor, {bool play = true}) async { final generation = ++_generation; + _invalidateArmRequests(); _finalizeCurrentTrack(); _queue.jumpTo(cursor); await _openCurrent(generation, play: play); @@ -348,9 +380,9 @@ class MusicPlaybackServiceImpl extends MusicPlaybackService with WidgetsBindingO /// Arm (or clear) what the backend should auto-advance into. Skips the /// resolve round-trip when the desired target is already armed; repeat-one /// reuses the current track's resolved source. - Future _armNext(int generation) async { + Future _applyArmNext(int generation, int armRequest) async { final player = _player; - if (player == null || generation != _generation) return; + if (player == null || !_isCurrentArmRequest(player, generation, armRequest)) return; final targetCursor = _sleepTimerEndOfTrack ? null : _queue.nextIndex(); final target = targetCursor == null ? null : _queue.trackAt(targetCursor); @@ -366,7 +398,7 @@ class MusicPlaybackServiceImpl extends MusicPlaybackService with WidgetsBindingO _rememberStaleArm(); await _trySetNext(player, null); - if (generation != _generation || _player != player) return; + if (!_isCurrentArmRequest(player, generation, armRequest)) return; MusicSource source; if (targetCursor == _queue.cursor && _currentSource != null) { @@ -382,11 +414,12 @@ class MusicPlaybackServiceImpl extends MusicPlaybackService with WidgetsBindingO return; } } - if (generation != _generation || _player != player) return; - _armed = _ArmedTrack(track: target, source: source); + if (!_isCurrentArmRequest(player, generation, armRequest)) return; + final armed = _ArmedTrack(track: target, source: source); + _armed = armed; appLogger.d('Music: arming cursor $targetCursor "${target.title}"'); final ok = await _trySetNext(player, Media(source.url, headers: source.headers)); - if (!ok && generation == _generation && _player == player) { + if (!ok && identical(_armed, armed)) { // Nothing is armed natively; clear the record so the confirmed // completed fallback can advance explicitly instead of waiting for a // transition that can never come. @@ -394,6 +427,45 @@ class MusicPlaybackServiceImpl extends MusicPlaybackService with WidgetsBindingO } } + bool _isCurrentArmRequest(Player player, int generation, int armRequest) { + return !_disposed && generation == _generation && armRequest == _armRequestGeneration && _player == player; + } + + void _requestArmNext() { + if (_disposed) return; + _armRequestGeneration++; + _ensureArmDrain(); + } + + void _invalidateArmRequests() { + _armRequestGeneration++; + } + + void _ensureArmDrain() { + if (_disposed || _armDrain != null) return; + final drain = _drainArmRequests(); + _armDrain = drain; + unawaited( + drain.whenComplete(() { + if (_armDrain != drain) return; + _armDrain = null; + if (!_disposed && _processedArmRequestGeneration != _armRequestGeneration) { + _ensureArmDrain(); + } + }), + ); + } + + Future _drainArmRequests() async { + while (!_disposed) { + final armRequest = _armRequestGeneration; + final generation = _generation; + await _applyArmNext(generation, armRequest); + _processedArmRequestGeneration = armRequest; + if (armRequest == _armRequestGeneration) return; + } + } + /// Un-arm bookkeeping: [_armed] is cleared but remembered so /// [_onTrackTransition] can adopt a transition that raced the clear. void _rememberStaleArm() { @@ -418,11 +490,7 @@ class MusicPlaybackServiceImpl extends MusicPlaybackService with WidgetsBindingO /// edits that keep the same next track cost no server round-trip. void _rearmIfNeeded() { if (_player == null || _currentTrack == null) return; - final targetCursor = _sleepTimerEndOfTrack ? null : _queue.nextIndex(); - final target = targetCursor == null ? null : _queue.trackAt(targetCursor); - if (target == null && _armed == null) return; - if (target != null && _armed?.track.globalKey == target.globalKey) return; - unawaited(_armNext(_generation)); + _requestArmNext(); } // --------------------------------------------------------------------- @@ -501,7 +569,8 @@ class MusicPlaybackServiceImpl extends MusicPlaybackService with WidgetsBindingO } final adopted = armed; _armed = null; - final generation = ++_generation; + _generation++; + _invalidateArmRequests(); // The finished track played out fully — report stopped at its duration. final finishedMs = _currentTrack?.durationMs; @@ -537,7 +606,7 @@ class MusicPlaybackServiceImpl extends MusicPlaybackService with WidgetsBindingO appLogger.d('Music: transition received "${adopted.track.title}" → cursor ${_queue.cursor}'); _setStatus(MusicPlaybackStatus.playing, forceNotify: true); _bindTrackServices(_currentTrack!, adopted.source); - unawaited(_armNext(generation)); + _requestArmNext(); } /// Completed (eof-reached) is NOT a last-entry-only signal: mpv pulses it @@ -591,6 +660,7 @@ class MusicPlaybackServiceImpl extends MusicPlaybackService with WidgetsBindingO /// remains; pressing play restarts the current track from the top. void _parkAtEnd() { _generation++; + _invalidateArmRequests(); final finishedMs = _currentTrack?.durationMs; _finalizeCurrentTrack(positionOverride: finishedMs != null ? Duration(milliseconds: finishedMs) : null); _setStatus(MusicPlaybackStatus.paused, forceNotify: true); @@ -778,28 +848,37 @@ class MusicPlaybackServiceImpl extends MusicPlaybackService with WidgetsBindingO Future play() async { final player = _player; if (player == null || _currentTrack == null) return; + final generation = _generation; if (player.state.completed) { // Parked at queue end: restart the current track. await player.seek(Duration.zero); + if (!_isCurrentTransport(player, generation)) return; final currentTrack = _currentTrack; final currentSource = _currentSource; if (currentTrack != null && currentSource != null) { _bindTrackServices(currentTrack, currentSource); } - unawaited(_armNext(_generation)); + _requestArmNext(); } await player.play(); + if (!_isCurrentTransport(player, generation)) return; _setStatus(MusicPlaybackStatus.playing); } @override Future pause() async { final player = _player; - if (player == null) return; + if (player == null || _currentTrack == null) return; + final generation = _generation; await player.pause(); + if (!_isCurrentTransport(player, generation)) return; _setStatus(MusicPlaybackStatus.paused); } + bool _isCurrentTransport(Player player, int generation) { + return !_disposed && _currentTrack != null && generation == _generation && _player == player; + } + @override Future togglePlayPause() { final player = _player; @@ -1031,7 +1110,7 @@ class MusicPlaybackServiceImpl extends MusicPlaybackService with WidgetsBindingO // End-of-track mode suppresses gapless arming (and leaving it restores // the arm), so the track genuinely completes instead of transitioning. if (hadEndOfTrack != _sleepTimerEndOfTrack) { - unawaited(_armNext(_generation)); + _requestArmNext(); } notifyListeners(); } @@ -1064,7 +1143,10 @@ class MusicPlaybackServiceImpl extends MusicPlaybackService with WidgetsBindingO Future _stopForVideoClaim() => _stopSession(endStatus: MusicPlaybackStatus.idle); Future _stopSession({required MusicPlaybackStatus endStatus}) async { + beginPlayIntent(); + _queueSessionRevision++; _generation++; + _invalidateArmRequests(); _completedConfirmTimer?.cancel(); _completedConfirmTimer = null; _cancelSleepTimer(); @@ -1076,6 +1158,7 @@ class MusicPlaybackServiceImpl extends MusicPlaybackService with WidgetsBindingO _staleArm = null; _playContext = null; _resumeAfterInterruption = false; + _setStatus(endStatus, forceNotify: true); final player = _player; _player = null; @@ -1110,8 +1193,6 @@ class MusicPlaybackServiceImpl extends MusicPlaybackService with WidgetsBindingO unawaited(controls.clear()); controls.dispose(); } - - _setStatus(endStatus, forceNotify: true); } @override @@ -1137,6 +1218,10 @@ class MusicPlaybackServiceImpl extends MusicPlaybackService with WidgetsBindingO @override void dispose() { if (_disposed) return; + _playIntentGeneration++; + _queueSessionRevision++; + _generation++; + _invalidateArmRequests(); _disposed = true; if (_observesLifecycle) { WidgetsBinding.instance.removeObserver(this); diff --git a/lib/services/plex_mappers.dart b/lib/services/plex_mappers.dart index 2cff7ad2..c0381e17 100644 --- a/lib/services/plex_mappers.dart +++ b/lib/services/plex_mappers.dart @@ -564,6 +564,8 @@ class PlexMetadataDto { final double? userRating; @JsonKey(fromJson: flexibleInt) final int? year; + @JsonKey(fromJson: flexibleInt) + final int? parentYear; final String? originallyAvailableAt; final String? thumb; final String? art; @@ -666,6 +668,7 @@ class PlexMetadataDto { this.audienceRating, this.userRating, this.year, + this.parentYear, this.originallyAvailableAt, this.thumb, this.art, @@ -793,6 +796,7 @@ class PlexMetadataDto { double? audienceRating, double? userRating, int? year, + int? parentYear, String? originallyAvailableAt, String? thumb, String? art, @@ -862,6 +866,7 @@ class PlexMetadataDto { audienceRating: audienceRating ?? this.audienceRating, userRating: userRating ?? this.userRating, year: year ?? this.year, + parentYear: parentYear ?? this.parentYear, originallyAvailableAt: originallyAvailableAt ?? this.originallyAvailableAt, thumb: thumb ?? this.thumb, art: art ?? this.art, @@ -961,9 +966,10 @@ class PlexMappers { /// Map a parsed [PlexMetadataDto] into a [PlexMediaItem]. static PlexMediaItem mediaItem(PlexMetadataDto dto) { + final kind = MediaKind.fromString(dto.type); return PlexMediaItem( id: dto.ratingKey, - kind: MediaKind.fromString(dto.type), + kind: kind, guid: dto.guid, title: dto.title, titleSort: dto.titleSort, @@ -972,7 +978,10 @@ class PlexMappers { originalTitle: dto.originalTitle, editionTitle: dto.editionTitle, studio: dto.studio, - year: dto.year, + // Plex stores an ordinary track's release year on the parent album. + // Normalize it into the neutral item's year, matching Jellyfin Audio + // rows, while leaving episode/season hierarchy semantics unchanged. + year: kind == MediaKind.track ? dto.year ?? dto.parentYear : dto.year, originallyAvailableAt: dto.originallyAvailableAt, contentRating: dto.contentRating, parentId: dto.parentRatingKey, diff --git a/lib/services/plex_mappers.g.dart b/lib/services/plex_mappers.g.dart index ca421f38..03f8d784 100644 --- a/lib/services/plex_mappers.g.dart +++ b/lib/services/plex_mappers.g.dart @@ -93,6 +93,7 @@ PlexMetadataDto _$PlexMetadataDtoFromJson(Map json) => audienceRating: flexibleDouble(json['audienceRating']), userRating: flexibleDouble(json['userRating']), year: flexibleInt(json['year']), + parentYear: flexibleInt(json['parentYear']), originallyAvailableAt: json['originallyAvailableAt'] as String?, thumb: json['thumb'] as String?, art: json['art'] as String?, @@ -167,6 +168,7 @@ Map _$PlexMetadataDtoToJson(PlexMetadataDto instance) => 'audienceRating': ?instance.audienceRating, 'userRating': ?instance.userRating, 'year': ?instance.year, + 'parentYear': ?instance.parentYear, 'originallyAvailableAt': ?instance.originallyAvailableAt, 'thumb': ?instance.thumb, 'art': ?instance.art, diff --git a/lib/utils/music_navigation.dart b/lib/utils/music_navigation.dart index f6eae3e8..a5904911 100644 --- a/lib/utils/music_navigation.dart +++ b/lib/utils/music_navigation.dart @@ -123,6 +123,8 @@ Future playTracks( /// album, isn't found in it, or the album fetch fails. Future playTrackWithAlbumContext(BuildContext context, MediaItem track) async { if (!ensureMusicPlaybackAvailable(context)) return; + final service = context.read(); + final intent = service.beginPlayIntent(); final albumId = track.parentId; final client = context.getMediaClientForItemOrNull(track); @@ -130,7 +132,7 @@ Future playTrackWithAlbumContext(BuildContext context, MediaItem track) as try { final tracks = await client.fetchAlbumTracks(albumId); final startIndex = tracks.indexWhere((item) => item.id == track.id); - if (!context.mounted) return; + if (!context.mounted || !service.isPlayIntentCurrent(intent)) return; if (startIndex != -1) { await playTracks( context, @@ -141,11 +143,13 @@ Future playTrackWithAlbumContext(BuildContext context, MediaItem track) as return; } } catch (e) { + if (!service.isPlayIntentCurrent(intent)) return; appLogger.w('Failed to fetch album context for track ${track.id}; playing single track', error: e); if (!context.mounted) return; } } + if (!context.mounted || !service.isPlayIntentCurrent(intent)) return; await playTracks( context, tracks: [track], diff --git a/lib/widgets/media_context_menu.dart b/lib/widgets/media_context_menu.dart index ee106c0e..d7466263 100644 --- a/lib/widgets/media_context_menu.dart +++ b/lib/widgets/media_context_menu.dart @@ -1062,8 +1062,16 @@ class MediaContextMenuState extends State { // Availability gate before the container fetch so the stub costs no // server round-trip. if (!ensureMusicPlaybackAvailable(context)) return; - final tracks = await _musicTracksForItem(item); - if (!context.mounted) return; + final service = context.read(); + final intent = service.beginPlayIntent(); + List tracks; + try { + tracks = await _musicTracksForItem(item); + } catch (_) { + if (!service.isPlayIntentCurrent(intent)) return; + rethrow; + } + if (!context.mounted || !service.isPlayIntentCurrent(intent)) return; await playTracks( context, tracks: tracks, @@ -1079,8 +1087,15 @@ class MediaContextMenuState extends State { final service = context.read(); // Menu entries are hidden on the stub; defensive re-check. if (service == null || !service.isAvailable) return; - final tracks = await _musicTracksForItem(_mediaItem!); - if (tracks.isEmpty) return; + final queueSessionRevision = service.queueSessionRevision; + List tracks; + try { + tracks = await _musicTracksForItem(_mediaItem!); + } catch (_) { + if (!context.mounted || service.queueSessionRevision != queueSessionRevision) return; + rethrow; + } + if (!context.mounted || service.queueSessionRevision != queueSessionRevision || tracks.isEmpty) return; if (playNext) { service.addNext(tracks); } else { @@ -1383,18 +1398,19 @@ class MediaContextMenuState extends State { // Match PlaylistDetailScreen: fail the availability gate before paying // for a full playlist fetch, then hand the tracks to the music session. if (!ensureMusicPlaybackAvailable(context)) return; + final service = context.read(); + final intent = service.beginPlayIntent(); List tracks; try { tracks = await fetchAllPlaylistItems(_getMediaClientForItem(), playlist.id); } catch (e, st) { + if (!context.mounted || !service.isPlayIntentCurrent(intent)) return; appLogger.w('Failed to fetch audio playlist ${playlist.id}', error: e, stackTrace: st); - if (context.mounted) { - showErrorSnackBar(context, t.messages.errorLoading(error: e.toString())); - } + showErrorSnackBar(context, t.messages.errorLoading(error: e.toString())); return; } - if (!context.mounted) return; + if (!context.mounted || !service.isPlayIntentCurrent(intent)) return; if (tracks.isEmpty) { showErrorSnackBar(context, t.messages.failedToCreatePlayQueueNoItems); return; diff --git a/lib/widgets/music/mini_player.dart b/lib/widgets/music/mini_player.dart index de3cecd3..c8d91e2f 100644 --- a/lib/widgets/music/mini_player.dart +++ b/lib/widgets/music/mini_player.dart @@ -286,7 +286,7 @@ class _MiniPlayerCardState extends State<_MiniPlayerCard> with ContextMenuTapMix height: _MusicMiniPlayerOverlayState._cardHeight, child: Stack( children: [ - const Positioned.fill(child: _MiniPlayerProgress()), + Positioned.fill(child: _MiniPlayerProgress(key: ValueKey(widget.track.globalKey))), Padding( padding: const EdgeInsets.symmetric(horizontal: 8), child: Row( @@ -409,7 +409,7 @@ class _MiniPlayerCardState extends State<_MiniPlayerCard> with ContextMenuTapMix /// background layer, never the card content above it. The played fraction /// renders as a subtle full-height tint that fills the card left-to-right. class _MiniPlayerProgress extends StatelessWidget { - const _MiniPlayerProgress(); + const _MiniPlayerProgress({super.key}); @override Widget build(BuildContext context) { diff --git a/test/media/media_item_test.dart b/test/media/media_item_test.dart index 2c0817e1..af167a82 100644 --- a/test/media/media_item_test.dart +++ b/test/media/media_item_test.dart @@ -15,6 +15,7 @@ import '../test_helpers/media_items.dart'; MediaItem _movie({ String id = 'm1', String? title = 'Movie', + int? year, int? viewCount, int? leafCount, int? viewedLeafCount, @@ -29,6 +30,7 @@ MediaItem _movie({ backend: backend, kind: MediaKind.movie, title: title, + year: year, viewCount: viewCount, leafCount: leafCount, viewedLeafCount: viewedLeafCount, @@ -446,4 +448,16 @@ void main() { expect(movie.displayTitle, ''); }); }); + + group('MediaItem music metadata', () { + test('album year is exposed only for tracks and albums', () { + final track = testMediaItem(kind: MediaKind.track, parentTitle: 'Album', year: 2001); + final album = testMediaItem(kind: MediaKind.album, title: 'Album', year: 2001); + + expect(track.albumTitle, 'Album'); + expect(track.albumYear, 2001); + expect(album.albumYear, 2001); + expect(_movie(year: 2001).albumYear, isNull); + }); + }); } diff --git a/test/screens/music/now_playing_screen_test.dart b/test/screens/music/now_playing_screen_test.dart new file mode 100644 index 00000000..c3fd012a --- /dev/null +++ b/test/screens/music/now_playing_screen_test.dart @@ -0,0 +1,226 @@ +import 'dart:async'; + +import 'package:flutter/material.dart'; +import 'package:flutter/services.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:plezy/focus/input_mode_tracker.dart'; +import 'package:plezy/i18n/strings.g.dart'; +import 'package:plezy/media/media_backend.dart'; +import 'package:plezy/media/media_item.dart'; +import 'package:plezy/media/media_kind.dart'; +import 'package:plezy/providers/multi_server_provider.dart'; +import 'package:plezy/screens/music/now_playing_screen.dart'; +import 'package:plezy/services/data_aggregation_service.dart'; +import 'package:plezy/services/multi_server_manager.dart'; +import 'package:plezy/services/music/music_playback_service.dart'; +import 'package:plezy/theme/mono_theme.dart'; +import 'package:plezy/utils/platform_detector.dart'; +import 'package:provider/provider.dart'; + +import '../../test_helpers/media_items.dart'; + +MediaItem _track({required String id, required String title, required String album, required int year}) { + return testMediaItem( + id: id, + backend: MediaBackend.plex, + kind: MediaKind.track, + title: title, + parentId: 'album_$id', + parentTitle: album, + grandparentId: 'artist_$id', + grandparentTitle: 'Artist $id', + year: year, + durationMs: const Duration(minutes: 3).inMilliseconds, + serverId: 'server_1', + ); +} + +class _FakeMusicService extends StubMusicPlaybackService { + MediaItem track; + final MusicPlayContext context; + final StreamController _positionController = StreamController.broadcast(sync: true); + final List seeks = []; + Duration _position = Duration.zero; + + _FakeMusicService({required this.track, required this.context}); + + void advanceTo(MediaItem next) { + track = next; + _position = Duration.zero; + notifyListeners(); + } + + void emitPosition(Duration position) { + _position = position; + _positionController.add(position); + } + + @override + bool get isAvailable => true; + + @override + MediaItem get currentTrack => track; + + @override + MusicPlaybackStatus get status => MusicPlaybackStatus.playing; + + @override + Duration get position => _position; + + @override + Stream get positionStream => _positionController.stream; + + @override + Duration get duration => const Duration(minutes: 3); + + @override + List get queue => [track]; + + @override + int get currentIndex => 0; + + @override + MusicPlayContext get playContext => context; + + @override + Future seek(Duration position) async { + seeks.add(position); + _position = position; + } + + @override + void dispose() { + _positionController.close(); + super.dispose(); + } +} + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + setUp(() { + LocaleSettings.setLocaleSync(AppLocale.en); + TvDetectionService.debugSetAppleTVOverride(false); + PlatformDetector.debugSetIsDesktopOSOverride(null); + }); + + tearDown(() { + TvDetectionService.debugSetAppleTVOverride(null); + PlatformDetector.debugSetIsDesktopOSOverride(null); + }); + + Future pumpNowPlaying(WidgetTester tester, _FakeMusicService service, {required bool isTv}) async { + tester.view.devicePixelRatio = 1; + tester.view.physicalSize = const Size(1200, 700); + addTearDown(tester.view.resetDevicePixelRatio); + addTearDown(tester.view.resetPhysicalSize); + + TvDetectionService.debugSetAppleTVOverride(isTv); + PlatformDetector.debugSetIsDesktopOSOverride(!isTv); + + final manager = MultiServerManager(); + final multiServerProvider = MultiServerProvider(manager, DataAggregationService(manager)); + addTearDown(service.dispose); + addTearDown(() { + multiServerProvider.dispose(); + manager.dispose(); + }); + + await tester.pumpWidget( + InputModeTracker( + child: TranslationProvider( + child: MultiProvider( + providers: [ + ChangeNotifierProvider.value(value: multiServerProvider), + ChangeNotifierProvider.value(value: service), + ], + child: MaterialApp( + theme: monoTheme(dark: true).copyWith(platform: isTv ? TargetPlatform.android : TargetPlatform.windows), + home: const NowPlayingScreen(), + ), + ), + ), + ), + ); + await tester.pump(); + } + + for (final isTv in [false, true]) { + testWidgets('${isTv ? 'TV' : 'desktop'} follows the current album when an album queue crosses albums', ( + tester, + ) async { + final first = _track(id: 'one', title: 'First Track', album: 'First Album', year: 1973); + final second = _track(id: 'two', title: 'Second Track', album: 'Second Album', year: 1999); + final service = _FakeMusicService( + track: first, + context: const MusicPlayContext(id: 'album_one', title: 'First Album', kind: MusicPlayContextKind.album), + ); + + await pumpNowPlaying(tester, service, isTv: isTv); + + expect(find.text(t.music.playingFrom(title: 'First Album · 1973')), findsOneWidget); + + service.advanceTo(second); + await tester.pump(); + + expect(find.text(t.music.playingFrom(title: 'First Album · 1973')), findsNothing); + expect(find.text(t.music.playingFrom(title: 'Second Album · 1999')), findsOneWidget); + }); + } + + testWidgets('playlist playback retains its queue provenance label', (tester) async { + final service = _FakeMusicService( + track: _track(id: 'one', title: 'First Track', album: 'First Album', year: 1973), + context: const MusicPlayContext(id: 'playlist_1', title: 'Road Trip', kind: MusicPlayContextKind.playlist), + ); + + await pumpNowPlaying(tester, service, isTv: false); + + expect(find.text(t.music.playingFrom(title: 'Road Trip')), findsOneWidget); + expect(find.text(t.music.playingFrom(title: 'First Album · 1973')), findsNothing); + }); + + testWidgets('pending d-pad seek is discarded when the track changes', (tester) async { + final first = _track(id: 'one', title: 'First Track', album: 'First Album', year: 1973); + final second = _track(id: 'two', title: 'Second Track', album: 'Second Album', year: 1999); + final service = _FakeMusicService( + track: first, + context: const MusicPlayContext(title: 'Queue', kind: MusicPlayContextKind.tracks), + ); + + await pumpNowPlaying(tester, service, isTv: true); + await tester.sendKeyEvent(LogicalKeyboardKey.arrowUp); + await tester.pump(); + await tester.sendKeyDownEvent(LogicalKeyboardKey.arrowRight); + + service.advanceTo(second); + await tester.pump(); + await tester.sendKeyUpEvent(LogicalKeyboardKey.arrowRight); + await tester.pump(); + + expect(service.seeks, isEmpty); + }); + + testWidgets('seek progress resets immediately when the track changes', (tester) async { + final first = _track(id: 'one', title: 'First Track', album: 'First Album', year: 1973); + final second = _track(id: 'two', title: 'Second Track', album: 'Second Album', year: 1999); + final service = _FakeMusicService( + track: first, + context: const MusicPlayContext(title: 'Queue', kind: MusicPlayContextKind.tracks), + ); + + await pumpNowPlaying(tester, service, isTv: false); + service.emitPosition(const Duration(minutes: 2)); + await tester.pump(); + Slider seekSlider() => tester + .widgetList(find.byType(Slider)) + .singleWhere((slider) => slider.max == const Duration(minutes: 3).inMilliseconds); + + expect(seekSlider().value, 120000); + + service.advanceTo(second); + await tester.pump(); + + expect(seekSlider().value, 0); + }); +} diff --git a/test/services/jellyfin_music_mapper_test.dart b/test/services/jellyfin_music_mapper_test.dart index 8e063397..82fc1bee 100644 --- a/test/services/jellyfin_music_mapper_test.dart +++ b/test/services/jellyfin_music_mapper_test.dart @@ -90,6 +90,7 @@ void main() { expect(item.trackNumber, 1); expect(item.discNumber, 1); expect(item.albumTitle, 'Live at Testhalle'); + expect(item.albumYear, 2022); expect(item.albumArtistTitle, 'The Synth Pops'); // Artists == [AlbumArtist] → no per-track performer override. expect(item.originalTitle, isNull); diff --git a/test/services/music/music_playback_service_test.dart b/test/services/music/music_playback_service_test.dart index 35e427bc..86138df1 100644 --- a/test/services/music/music_playback_service_test.dart +++ b/test/services/music/music_playback_service_test.dart @@ -89,6 +89,8 @@ class FakePlayer implements Player { final List setNextCalls = []; final List seeks = []; final List volumes = []; + Completer? playGate; + Completer? pauseGate; /// Arming these URIs throws, simulating a native setNext failure. final Set failingSetNextUris = {}; @@ -178,6 +180,7 @@ class FakePlayer implements Player { @override Future play() async { playCalls++; + await playGate?.future; _state = _state.copyWith(playing: true, completed: false); playingCtrl.add(true); } @@ -185,6 +188,7 @@ class FakePlayer implements Player { @override Future pause() async { pauseCalls++; + await pauseGate?.future; _state = _state.copyWith(playing: false); playingCtrl.add(false); } @@ -351,6 +355,7 @@ class RecordedReport { class FakeMediaServerClient extends Fake implements MediaServerClient { final List reports = []; final List markedWatched = []; + Completer>? instantMixGate; Iterable reportsFor(String state) => reports.where((r) => r.state == state); @@ -363,6 +368,14 @@ class FakeMediaServerClient extends Fake implements MediaServerClient { @override bool get marksWatchedOnPlaybackStopped => false; + @override + void close() {} + + @override + Future> fetchInstantMix(String itemId, {int limit = 100}) { + return instantMixGate?.future ?? Future.value(const []); + } + @override Future markWatched(MediaItem item) async { markedWatched.add(item.id); @@ -419,6 +432,7 @@ class FakeMusicSourceResolver implements MusicSourceResolver { final MediaServerClient? client; final Set failingIds = {}; final Map resolveCounts = {}; + final Map> resolveGates = {}; /// Per-track URL overrides (e.g. content:// shapes for offline tracks). final Map urlOverrides = {}; @@ -426,6 +440,7 @@ class FakeMusicSourceResolver implements MusicSourceResolver { @override Future resolve(MediaItem track) async { resolveCounts[track.id] = (resolveCounts[track.id] ?? 0) + 1; + await resolveGates[track.id]?.future; if (failingIds.contains(track.id)) { throw StateError('resolve failed for ${track.id}'); } @@ -510,13 +525,14 @@ class _GatedVolumeWriter { } class _Harness { - _Harness._(this.service, this.resolver, this.client, this.controls, this.players); + _Harness._(this.service, this.resolver, this.client, this.controls, this.players, this.serverManager); final MusicPlaybackServiceImpl service; final FakeMusicSourceResolver resolver; final FakeMediaServerClient client; final FakeMediaControlsManager controls; final List players; + final MultiServerManager serverManager; /// Seeded into every created FakePlayer — lets a test configure arm /// failures before the first player exists. @@ -529,9 +545,10 @@ class _Harness { final resolver = FakeMusicSourceResolver(client: client); final controls = FakeMediaControlsManager(); final players = []; + final serverManager = MultiServerManager()..debugRegisterClientForTesting(client); late final _Harness harness; final service = MusicPlaybackServiceImpl( - serverManager: MultiServerManager(), + serverManager: serverManager, resolver: resolver, audioPlayerFactory: () { final player = FakePlayer(); @@ -545,7 +562,7 @@ class _Harness { completedConfirmDelay: Duration.zero, volumePersistenceWriter: volumePersistenceWriter, ); - harness = _Harness._(service, resolver, client, controls, players); + harness = _Harness._(service, resolver, client, controls, players, serverManager); return harness; } @@ -579,6 +596,7 @@ void main() { player.closeControllers(); } h.controls.closeControllers(); + h.serverManager.dispose(); }); test('volume updates notify only the dedicated volume listenable', () async { @@ -687,6 +705,47 @@ void main() { expect(h.controls.metadataTitles, ['Track t1']); }); + test('a superseded slow gapless resolve cannot overwrite the newly requested arm', () async { + final oldArmGate = Completer(); + h.resolver.resolveGates[t2.id] = oldArmGate; + + await h.playTracks([t1, t2]); + expect(h.player.armed, isNull); + + h.service.addNext([t3]); + oldArmGate.complete(); + await pumpEventQueue(); + + expect(h.player.armed?.uri, _urlFor(t3)); + }); + + test('end-of-track sleep invalidates a slow gapless resolve', () async { + final armGate = Completer(); + h.resolver.resolveGates[t2.id] = armGate; + + await h.playTracks([t1, t2]); + h.service.setSleepTimer(null, endOfTrack: true); + armGate.complete(); + await pumpEventQueue(); + + expect(h.service.sleepTimerEndOfTrack, isTrue); + expect(h.player.armed, isNull); + }); + + test('a slow instant mix cannot replace a newer explicit queue', () async { + final mixGate = Completer>(); + h.client.instantMixGate = mixGate; + + final mix = h.service.playInstantMix(t1); + await h.playTracks([t3]); + mixGate.complete([t1, t2]); + await mix; + await pumpEventQueue(); + + expect(h.service.currentTrack?.id, t3.id); + expect(h.service.queue.map((track) => track.id), [t3.id]); + }); + test('trackTransition advances the cursor, re-arms, and reports the previous track stopped at duration', () async { await h.playTracks([t1, t2, t3]); @@ -876,6 +935,35 @@ void main() { expect(h.client.reportsFor('stopped').map((r) => r.itemId), ['t1']); }); + test('a late play completion cannot revive a stopped session', () async { + await h.playTracks([t1]); + await h.service.pause(); + final gate = Completer(); + h.player.playGate = gate; + + final play = h.service.play(); + await h.service.stop(); + gate.complete(); + await play; + + expect(h.service.status, MusicPlaybackStatus.idle); + expect(h.service.currentTrack, isNull); + }); + + test('a late pause completion cannot change a stopped session', () async { + await h.playTracks([t1]); + final gate = Completer(); + h.player.pauseGate = gate; + + final pause = h.service.pause(); + await h.service.stop(); + gate.complete(); + await pause; + + expect(h.service.status, MusicPlaybackStatus.idle); + expect(h.service.currentTrack, isNull); + }); + test('interruption pauses and resumes when the system says shouldResume', () async { await h.playTracks([t1, t2]); diff --git a/test/services/plex_mappers_test.dart b/test/services/plex_mappers_test.dart index f4fb161f..0868fd70 100644 --- a/test/services/plex_mappers_test.dart +++ b/test/services/plex_mappers_test.dart @@ -243,6 +243,7 @@ void main() { 'parentIndex': 1, 'parentRatingKey': '510', 'parentTitle': 'Season 1', + 'parentYear': 2008, 'parentThumb': '/library/metadata/510/thumb/1', 'grandparentRatingKey': '500', 'grandparentTitle': 'Breaking Bad', @@ -258,6 +259,7 @@ void main() { expect(item.parentIndex, 1); expect(item.parentId, '510'); expect(item.parentTitle, 'Season 1'); + expect(item.year, isNull); expect(item.parentThumbPath, '/library/metadata/510/thumb/1'); expect(item.grandparentId, '500'); expect(item.grandparentTitle, 'Breaking Bad'); @@ -299,6 +301,7 @@ void main() { 'index': 8, 'parentRatingKey': '700', 'parentTitle': 'Random Access Memories', + 'parentYear': 2013, 'grandparentRatingKey': '699', 'grandparentTitle': 'Daft Punk', 'duration': 369000, @@ -311,6 +314,8 @@ void main() { expect(item.durationMs, 369000); expect(item.parentId, '700'); expect(item.parentTitle, 'Random Access Memories'); + expect(item.year, 2013); + expect(item.albumYear, 2013); expect(item.grandparentId, '699'); expect(item.grandparentTitle, 'Daft Punk'); }); diff --git a/test/widgets/media_context_menu_test.dart b/test/widgets/media_context_menu_test.dart index 7fed472d..ad2c82d6 100644 --- a/test/widgets/media_context_menu_test.dart +++ b/test/widgets/media_context_menu_test.dart @@ -1,4 +1,5 @@ import '../test_helpers/paged_fakes.dart'; +import 'dart:async'; import 'dart:convert'; import 'package:drift/native.dart'; @@ -200,6 +201,30 @@ void main() { expect(music.callCount, 2); expect(music.playedTracks, tracks); expect(music.shuffle, isTrue); + + final staleFetchGate = Completer(); + client.fetchGate = staleFetchGate; + menuKey.currentState!.showContextMenu(tester.element(find.text('audio target'))); + await tester.pumpAndSettle(); + await tester.tap(find.text(t.common.play)); + await tester.pump(); + + final newerTrack = testMediaItem( + id: 'newer-track', + backend: MediaBackend.jellyfin, + kind: MediaKind.track, + title: 'Newer Track', + serverId: 'srv-1', + ); + await music.playFromList( + tracks: [newerTrack], + playContext: const MusicPlayContext(title: 'Newer Queue', kind: MusicPlayContextKind.tracks), + ); + staleFetchGate.complete(); + await tester.pumpAndSettle(); + + expect(music.callCount, 3, reason: 'the stale playlist fetch must not start a fourth queue'); + expect(music.playedTracks, [newerTrack]); expect(tester.takeException(), isNull); }); @@ -408,6 +433,49 @@ void main() { same(harness.music), ); }); + + testWidgets('a slow music enqueue does not append to a newer queue session', (tester) async { + final album = testMediaItem( + id: 'album-1', + backend: MediaBackend.jellyfin, + kind: MediaKind.album, + title: 'Album', + serverId: 'srv-1', + ); + final albumTrack = testMediaItem( + id: 'album-track', + backend: MediaBackend.jellyfin, + kind: MediaKind.track, + title: 'Album Track', + serverId: 'srv-1', + ); + final harness = await _pumpSiblingMusicMenu(tester, item: album, relatedItems: [albumTrack]); + final fetchGate = Completer(); + harness.client.albumTracksGate = fetchGate; + + harness.menuKey.currentState!.showContextMenu(tester.element(find.text('mini-player menu target'))); + await tester.pumpAndSettle(); + await tester.tap(find.text(t.music.playNext)); + await tester.pump(); + + final newerTrack = testMediaItem( + id: 'newer-track', + backend: MediaBackend.jellyfin, + kind: MediaKind.track, + title: 'Newer Track', + serverId: 'srv-1', + ); + await harness.music.playFromList( + tracks: [newerTrack], + playContext: const MusicPlayContext(title: 'Newer Queue', kind: MusicPlayContextKind.tracks), + ); + fetchGate.complete(); + await tester.pumpAndSettle(); + + expect(harness.music.addedNext, isEmpty); + expect(harness.music.playedTracks, [newerTrack]); + expect(tester.takeException(), isNull); + }); }); } @@ -522,6 +590,7 @@ Future _openPlaylistPicker(WidgetTester tester, GlobalKey tracks; + Completer? fetchGate; _AudioPlaylistClient(this.tracks); @@ -539,6 +608,7 @@ class _AudioPlaylistClient implements MediaServerClient { @override Future> fetchPlaylistPage(String id, {int? start, int? size, AbortController? abort}) async { + await fetchGate?.future; return fakeLibraryPage(tracks, start: start, size: size); } @@ -551,6 +621,7 @@ class _AudioPlaylistClient implements MediaServerClient { class _RecordingMusicPlaybackService extends StubMusicPlaybackService { List? playedTracks; + final List addedNext = []; MusicPlayContext? playedContext; bool? shuffle; int callCount = 0; @@ -565,17 +636,27 @@ class _RecordingMusicPlaybackService extends StubMusicPlaybackService { required MusicPlayContext playContext, bool shuffle = false, }) async { + await super.playFromList(tracks: tracks, startTrack: startTrack, playContext: playContext, shuffle: shuffle); callCount++; playedTracks = tracks; playedContext = playContext; this.shuffle = shuffle; } + + @override + void addNext(List tracks) { + addedNext.addAll(tracks); + } } class _RelatedMusicClient implements MediaServerClient { - _RelatedMusicClient(Iterable items) : _items = {for (final item in items) item.id: item}; + _RelatedMusicClient(Iterable items) + : _items = {for (final item in items) item.id: item}, + albumTracks = items.where((item) => item.kind == MediaKind.track).toList(); final Map _items; + final List albumTracks; + Completer? albumTracksGate; @override ServerId get serverId => ServerId('srv-1'); @@ -593,7 +674,10 @@ class _RelatedMusicClient implements MediaServerClient { Future fetchItem(String id) async => _items[id]; @override - Future> fetchAlbumTracks(String albumId) async => const []; + Future> fetchAlbumTracks(String albumId) async { + await albumTracksGate?.future; + return albumTracks; + } @override Future> fetchArtistAlbums(MediaItem artist) async => const []; @@ -611,12 +695,14 @@ class _SiblingMusicMenuHarness { required this.profileNavigatorKey, required this.menuKey, required this.music, + required this.client, }); final GlobalKey rootNavigatorKey; final GlobalKey profileNavigatorKey; final GlobalKey menuKey; final _RecordingMusicPlaybackService music; + final _RelatedMusicClient client; } Future<_SiblingMusicMenuHarness> _pumpSiblingMusicMenu( @@ -722,6 +808,7 @@ Future<_SiblingMusicMenuHarness> _pumpSiblingMusicMenu( profileNavigatorKey: profileNavigatorKey, menuKey: menuKey, music: music, + client: client, ); } diff --git a/test/widgets/music/mini_player_test.dart b/test/widgets/music/mini_player_test.dart index fd100313..2b8e069f 100644 --- a/test/widgets/music/mini_player_test.dart +++ b/test/widgets/music/mini_player_test.dart @@ -1,3 +1,5 @@ +import 'dart:async'; + import 'package:drift/native.dart'; import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; @@ -69,6 +71,8 @@ class _FakeDownloadProvider extends ChangeNotifier implements DownloadProvider { /// Fixed-state fake: reports a playing session with a single-track queue. class _FakeMusicService extends StubMusicPlaybackService { MediaItem? track; + final StreamController _positionController = StreamController.broadcast(sync: true); + Duration _position = Duration.zero; int previousCalls = 0; int toggleCalls = 0; int nextCalls = 0; @@ -85,6 +89,12 @@ class _FakeMusicService extends StubMusicPlaybackService { @override MusicPlaybackStatus get status => track == null ? MusicPlaybackStatus.idle : MusicPlaybackStatus.playing; + @override + Duration get position => _position; + + @override + Stream get positionStream => _positionController.stream; + @override Duration? get duration => track == null ? null : const Duration(minutes: 3); @@ -94,6 +104,17 @@ class _FakeMusicService extends StubMusicPlaybackService { @override int get currentIndex => track == null ? -1 : 0; + void emitPosition(Duration position) { + _position = position; + _positionController.add(position); + } + + void advanceTo(MediaItem next) { + track = next; + _position = Duration.zero; + notifyListeners(); + } + @override Future previous() async { previousCalls++; @@ -113,6 +134,12 @@ class _FakeMusicService extends StubMusicPlaybackService { Future stop() async { stopCalls++; } + + @override + void dispose() { + _positionController.close(); + super.dispose(); + } } void main() { @@ -183,6 +210,37 @@ void main() { expect(find.byType(IconButton), findsNWidgets(2)); // play/pause + next (mobile layout) }); + testWidgets('progress resets immediately when the current track changes', (tester) async { + final nextTrack = testMediaItem( + id: 'track_2', + backend: MediaBackend.plex, + kind: MediaKind.track, + title: 'Noon', + durationMs: 180000, + serverId: 'server_1', + ); + final service = _FakeMusicService(track: _track); + final observer = MusicUiRouteObserver(); + + await tester.pumpWidget(wrap(service: service, observer: observer)); + await tester.pumpAndSettle(); + service.emitPosition(const Duration(minutes: 2)); + await tester.pump(); + + Iterable progressWidths() => tester + .widgetList(find.byType(FractionallySizedBox)) + .where((box) => box.heightFactor == 1) + .map((box) => box.widthFactor); + + expect(progressWidths(), contains(closeTo(2 / 3, 0.001))); + + service.advanceTo(nextTrack); + await tester.pump(); + + expect(progressWidths(), contains(0.0)); + expect(progressWidths(), isNot(contains(closeTo(2 / 3, 0.001)))); + }); + testWidgets('tapping the card edge opens the named Now Playing route', (tester) async { final service = _FakeMusicService(track: _track); final observer = MusicUiRouteObserver();