From 2b42e01cd0d7d89cc1b8415d529191a0c46740b3 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Tue, 16 Jun 2026 00:04:44 -0400 Subject: [PATCH 1/2] feat(player): add IQueueService orchestrating album playback above the single-slot player (P11 11.F) Queue owns ordered tracks, current index, skip-fwd/back, and auto-advance via the player's TrackEnded hook; binds through Attach (no ctor growth, no service-locator). Player-bar skip controls; empty-queue play unchanged. Adds QueueService unit tests. --- .../AudioPlayerBar/AudioPlayerBar.razor | 4 + .../AudioPlayerBar/AudioPlayerBar.razor.cs | 32 ++ .../AudioPlayerBar/PlayerControls.razor | 13 + .../AudioPlayerBar/PlayerControls.razor.cs | 9 + .../AudioPlayerBar/PlayerTransportZone.razor | 6 +- .../PlayerTransportZone.razor.cs | 4 + .../Controls/AudioPlayerProvider.razor | 4 +- .../Controls/AudioPlayerProvider.razor.cs | 13 + .../Services/AudioPlayerService.cs | 9 + .../Services/IPlayerService.cs | 9 + .../Services/IQueueService.cs | 80 ++++ .../Services/QueueService.cs | 136 ++++++ DeepDrftTests/DeepDrftTests.csproj | 4 + DeepDrftTests/QueueServiceTests.cs | 450 ++++++++++++++++++ 14 files changed, 771 insertions(+), 2 deletions(-) create mode 100644 DeepDrftPublic.Client/Services/IQueueService.cs create mode 100644 DeepDrftPublic.Client/Services/QueueService.cs create mode 100644 DeepDrftTests/QueueServiceTests.cs diff --git a/DeepDrftPublic.Client/Controls/AudioPlayerBar/AudioPlayerBar.razor b/DeepDrftPublic.Client/Controls/AudioPlayerBar/AudioPlayerBar.razor index e653de1..089f478 100644 --- a/DeepDrftPublic.Client/Controls/AudioPlayerBar/AudioPlayerBar.razor +++ b/DeepDrftPublic.Client/Controls/AudioPlayerBar/AudioPlayerBar.razor @@ -21,6 +21,10 @@ else Fixed="Fixed" TogglePlayPause="@TogglePlayPause" Stop="@Stop" + HasNext="HasNext" + HasPrevious="HasPrevious" + SkipNext="@SkipNext" + SkipPrevious="@SkipPrevious" Class="transport-zone"/> diff --git a/DeepDrftPublic.Client/Controls/AudioPlayerBar/AudioPlayerBar.razor.cs b/DeepDrftPublic.Client/Controls/AudioPlayerBar/AudioPlayerBar.razor.cs index 62cf5a9..7f9a75e 100644 --- a/DeepDrftPublic.Client/Controls/AudioPlayerBar/AudioPlayerBar.razor.cs +++ b/DeepDrftPublic.Client/Controls/AudioPlayerBar/AudioPlayerBar.razor.cs @@ -9,6 +9,7 @@ namespace DeepDrftPublic.Client.Controls.AudioPlayerBar; public partial class AudioPlayerBar : ComponentBase, IAsyncDisposable { [CascadingParameter] public IStreamingPlayerService? PlayerService { get; set; } + [CascadingParameter] public IQueueService? QueueService { get; set; } [Parameter] public bool Fixed { get; set; } = false; [Parameter] public EventCallback OnMinimized { get; set; } @@ -19,6 +20,7 @@ public partial class AudioPlayerBar : ComponentBase, IAsyncDisposable private bool _isSeeking = false; private double _seekPosition = 0; private IStreamingPlayerService? _subscribedService; + private IQueueService? _subscribedQueue; // Spacer-height bridge: the expanded dock is position:fixed, so MainLayout's // spacer reserves its space. We mirror this element's live height into a CSS @@ -48,6 +50,11 @@ public partial class AudioPlayerBar : ComponentBase, IAsyncDisposable private double LoadProgress => PlayerService?.LoadProgress ?? 0; private string? ErrorMessage => PlayerService?.ErrorMessage; + // Skip affordances reflect live queue state. With no queue (null) or an empty queue both are + // false, so the buttons sit disabled and the bar behaves exactly as it did before the queue. + private bool HasNext => QueueService?.HasNext ?? false; + private bool HasPrevious => QueueService?.HasPrevious ?? false; + /// /// Display time - shows seek position while dragging, otherwise current playback time. /// @@ -76,10 +83,35 @@ public partial class AudioPlayerBar : ComponentBase, IAsyncDisposable PlayerService.StateChanged += OnPlayerStateChanged; _subscribedService = PlayerService; } + + // The queue cascade is also IsFixed, so re-render the skip affordances off its own + // change signal — same posture as the player StateChanged subscription above. + if (QueueService != null && !ReferenceEquals(QueueService, _subscribedQueue)) + { + if (_subscribedQueue != null) + _subscribedQueue.QueueChanged -= OnQueueChanged; + + QueueService.QueueChanged += OnQueueChanged; + _subscribedQueue = QueueService; + } } private void OnPlayerStateChanged() => InvokeAsync(StateHasChanged); + private void OnQueueChanged() => InvokeAsync(StateHasChanged); + + private async Task SkipNext() + { + if (QueueService == null) return; + await QueueService.Next(); + } + + private async Task SkipPrevious() + { + if (QueueService == null) return; + await QueueService.Previous(); + } + protected override async Task OnAfterRenderAsync(bool firstRender) { // Only the docked, expanded shape needs a spacer: the Fixed embed is diff --git a/DeepDrftPublic.Client/Controls/AudioPlayerBar/PlayerControls.razor b/DeepDrftPublic.Client/Controls/AudioPlayerBar/PlayerControls.razor index d252d47..7f691d7 100644 --- a/DeepDrftPublic.Client/Controls/AudioPlayerBar/PlayerControls.razor +++ b/DeepDrftPublic.Client/Controls/AudioPlayerBar/PlayerControls.razor @@ -2,12 +2,25 @@ @using DeepDrftPublic.Client.Controls + @if (!Fixed) + { + + } @if (!Fixed) { + Whether the queue has a track to skip forward to. Drives the skip-next affordance. + [Parameter] public bool HasNext { get; set; } + + /// Whether the queue has a track to step back to. Drives the skip-previous affordance. + [Parameter] public bool HasPrevious { get; set; } + + [Parameter] public EventCallback SkipNext { get; set; } + [Parameter] public EventCallback SkipPrevious { get; set; } } diff --git a/DeepDrftPublic.Client/Controls/AudioPlayerBar/PlayerTransportZone.razor b/DeepDrftPublic.Client/Controls/AudioPlayerBar/PlayerTransportZone.razor index e11206f..3510f38 100644 --- a/DeepDrftPublic.Client/Controls/AudioPlayerBar/PlayerTransportZone.razor +++ b/DeepDrftPublic.Client/Controls/AudioPlayerBar/PlayerTransportZone.razor @@ -6,7 +6,11 @@ CanPlay="CanPlay" Fixed="Fixed" TogglePlayPause="TogglePlayPause" - Stop="Stop"/> + Stop="Stop" + HasNext="HasNext" + HasPrevious="HasPrevious" + SkipNext="SkipNext" + SkipPrevious="SkipPrevious"/> @if (IsLoading && !IsStreaming) { - @ChildContent + + @ChildContent + \ No newline at end of file diff --git a/DeepDrftPublic.Client/Controls/AudioPlayerProvider.razor.cs b/DeepDrftPublic.Client/Controls/AudioPlayerProvider.razor.cs index 2e56bbf..e8c01fc 100644 --- a/DeepDrftPublic.Client/Controls/AudioPlayerProvider.razor.cs +++ b/DeepDrftPublic.Client/Controls/AudioPlayerProvider.razor.cs @@ -12,6 +12,7 @@ public partial class AudioPlayerProvider : ComponentBase, IAsyncDisposable [Inject] public required ILogger Logger { get; set; } private IStreamingPlayerService? _audioPlayerService; + private QueueService? _queueService; [Parameter] public RenderFragment? ChildContent { get; set; } @@ -29,6 +30,13 @@ public partial class AudioPlayerProvider : ComponentBase, IAsyncDisposable // Children must not wrap or replace this callback. _audioPlayerService.OnStateChanged = new EventCallback(this, () => InvokeAsync(StateHasChanged)); // OnTrackSelected will be set by individual child components that need it + + // The queue orchestrates above the single-slot player. The player is not DI-registered + // (constructed here), so the queue binds to it via Attach rather than constructor injection — + // no construction cycle, no IServiceProvider. Cascaded alongside the player so the bar and a + // future up-next panel both read it. + _queueService = new QueueService(); + _queueService.Attach(_audioPlayerService); } /// @@ -38,6 +46,11 @@ public partial class AudioPlayerProvider : ComponentBase, IAsyncDisposable /// public async ValueTask DisposeAsync() { + // Dispose the queue first so it unsubscribes from the player's TrackEnded before the + // player tears down. + _queueService?.Dispose(); + _queueService = null; + if (_audioPlayerService is IAsyncDisposable disposable) { await disposable.DisposeAsync(); diff --git a/DeepDrftPublic.Client/Services/AudioPlayerService.cs b/DeepDrftPublic.Client/Services/AudioPlayerService.cs index df43100..e2f8a14 100644 --- a/DeepDrftPublic.Client/Services/AudioPlayerService.cs +++ b/DeepDrftPublic.Client/Services/AudioPlayerService.cs @@ -43,6 +43,9 @@ public abstract class AudioPlayerService : IPlayerService, IAsyncDisposable /// public event Action? StateChanged; + /// + public event Action? TrackEnded; + protected AudioPlayerService(AudioInteropService audioInterop, TrackMediaClient trackMediaClient) { _audioInterop = audioInterop; @@ -268,6 +271,12 @@ public abstract class AudioPlayerService : IPlayerService, IAsyncDisposable CurrentTime = 0; Duration = null; await NotifyStateChanged(); + + // Fire AFTER the state notification so any queue orchestrator that advances on this + // signal selects the next track against a fully-settled idle state. Raised only on + // organic end-of-stream — stop/unload/track-switch go through ResetToIdle, which does + // not raise this — so a subscriber can treat it unambiguously as "advance the queue." + TrackEnded?.Invoke(); } diff --git a/DeepDrftPublic.Client/Services/IPlayerService.cs b/DeepDrftPublic.Client/Services/IPlayerService.cs index ff74384..c0ff051 100644 --- a/DeepDrftPublic.Client/Services/IPlayerService.cs +++ b/DeepDrftPublic.Client/Services/IPlayerService.cs @@ -40,6 +40,15 @@ public interface IPlayerService /// (throttled to ~10/s during streaming). /// event Action? StateChanged; + + /// + /// Raised once when the current track reaches its natural end of playback (the JS + /// end-of-stream callback), distinct from a stop/unload/track-switch. This is the single + /// hook the play-queue subscribes to in order to auto-advance to the next track. It does + /// NOT fire when playback is stopped, the track is switched, or the player is unloaded — + /// only on organic completion — so an orchestrator can treat it as "advance the queue." + /// + event Action? TrackEnded; // Control methods Task InitializeAsync(); diff --git a/DeepDrftPublic.Client/Services/IQueueService.cs b/DeepDrftPublic.Client/Services/IQueueService.cs new file mode 100644 index 0000000..b3cea03 --- /dev/null +++ b/DeepDrftPublic.Client/Services/IQueueService.cs @@ -0,0 +1,80 @@ +using DeepDrftModels.DTOs; + +namespace DeepDrftPublic.Client.Services; + +/// +/// Orchestrates ordered playback ("what plays next") above the single-slot +/// . The player stays a single-track device; the queue owns the +/// track list, the current position, skip-forward/back, and auto-advance on natural track end. It +/// drives playback solely through the player's existing +/// — it adds no new playback semantics. +/// +/// +/// Extension posture (open/closed): future shuffle, repeat modes, reordering, and persistence are +/// expected. They are additive — a shuffle/repeat strategy slots in behind / +/// as the "which index is next" decision; reordering mutates +/// and re-emits ; persistence snapshots/restores + +/// . None of those require changing this interface's existing members, only +/// adding new ones — so consumers written against today's surface keep working. +/// +/// +/// +/// With an empty queue ( == -1) the queue is dormant: it drives nothing and +/// auto-advances nothing, so direct single-track play through the player behaves exactly as it did +/// before the queue existed. +/// +/// +public interface IQueueService +{ + /// The ordered tracks currently queued. Empty when nothing is enqueued. + IReadOnlyList Items { get; } + + /// + /// Index into of the track the queue considers current, or -1 when the + /// queue is empty. Always a valid index into when non-negative. + /// + int CurrentIndex { get; } + + /// The current track, or null when the queue is empty. + TrackDto? Current { get; } + + /// True when there is a track after to advance to. + bool HasNext { get; } + + /// True when there is a track before to step back to. + bool HasPrevious { get; } + + /// + /// Raised whenever the queue's contents or current position change. The player bar subscribes + /// to re-render its skip-forward/back affordances. Fires on enqueue, advance, step-back, and clear. + /// + event Action? QueueChanged; + + /// + /// Replaces the queue with (in the order given) and begins streaming + /// the track at . This is the "play album" entry point the Cuts + /// detail page consumes: pass the release's tracks in ordinal order. A header Play uses + /// startIndex: 0; a mid-album row play passes that row's index so the queue continues to + /// the end from there. No-op when is empty. + /// + Task PlayRelease(IEnumerable tracks, int startIndex = 0); + + /// Appends a track to the end of the queue without changing what is currently playing. + void Enqueue(TrackDto track); + + /// Appends tracks to the end of the queue without changing what is currently playing. + void EnqueueRange(IEnumerable tracks); + + /// + /// Advances to the next track and streams it. No-op when is false. + /// + Task Next(); + + /// + /// Steps back to the previous track and streams it. No-op when is false. + /// + Task Previous(); + + /// Empties the queue and resets the position. Does not stop the player. + void Clear(); +} diff --git a/DeepDrftPublic.Client/Services/QueueService.cs b/DeepDrftPublic.Client/Services/QueueService.cs new file mode 100644 index 0000000..03a14c9 --- /dev/null +++ b/DeepDrftPublic.Client/Services/QueueService.cs @@ -0,0 +1,136 @@ +using DeepDrftModels.DTOs; + +namespace DeepDrftPublic.Client.Services; + +/// +/// Default : a single-slot orchestrator over an +/// . Holds the ordered list and current index as pure state, +/// drives playback through the player's existing , +/// and auto-advances on the player's signal. +/// +/// +/// The player instance is not DI-registered — AudioPlayerProvider constructs and cascades it. +/// So the queue is bound to the player via (called once by the provider after it +/// creates the player) rather than constructor injection. This keeps the player single-slot, avoids a +/// construction cycle between provider/player/queue, and needs no IServiceProvider. The queue's +/// own constructor stays parameterless, so the queue logic is unit-testable against a fake player with +/// no container. +/// +/// +public sealed class QueueService : IQueueService, IDisposable +{ + private readonly List _items = new(); + private IStreamingPlayerService? _player; + + public IReadOnlyList Items => _items; + + public int CurrentIndex { get; private set; } = -1; + + public TrackDto? Current => + CurrentIndex >= 0 && CurrentIndex < _items.Count ? _items[CurrentIndex] : null; + + public bool HasNext => CurrentIndex >= 0 && CurrentIndex < _items.Count - 1; + + public bool HasPrevious => CurrentIndex > 0; + + public event Action? QueueChanged; + + /// + /// Binds the queue to the player instance the provider owns, and subscribes to its track-ended + /// signal so the queue auto-advances. Idempotent and re-bindable: re-attaching detaches the prior + /// player first, so the queue never holds a stale subscription after a player swap. Owned by the + /// provider's lifecycle; unsubscribes. + /// + public void Attach(IStreamingPlayerService player) + { + if (ReferenceEquals(_player, player)) return; + + if (_player != null) + _player.TrackEnded -= OnTrackEnded; + + _player = player; + _player.TrackEnded += OnTrackEnded; + } + + public async Task PlayRelease(IEnumerable tracks, int startIndex = 0) + { + var list = tracks as IReadOnlyList ?? tracks.ToList(); + if (list.Count == 0) return; + + var start = Math.Clamp(startIndex, 0, list.Count - 1); + + _items.Clear(); + _items.AddRange(list); + CurrentIndex = start; + QueueChanged?.Invoke(); + + await PlayCurrent(); + } + + public void Enqueue(TrackDto track) + { + _items.Add(track); + QueueChanged?.Invoke(); + } + + public void EnqueueRange(IEnumerable tracks) + { + var before = _items.Count; + _items.AddRange(tracks); + if (_items.Count != before) + QueueChanged?.Invoke(); + } + + public async Task Next() + { + if (!HasNext) return; + CurrentIndex++; + QueueChanged?.Invoke(); + await PlayCurrent(); + } + + public async Task Previous() + { + if (!HasPrevious) return; + CurrentIndex--; + QueueChanged?.Invoke(); + await PlayCurrent(); + } + + public void Clear() + { + if (_items.Count == 0 && CurrentIndex == -1) return; + _items.Clear(); + CurrentIndex = -1; + QueueChanged?.Invoke(); + } + + // Advance on organic end-of-stream only. TrackEnded is not raised by stop/unload/track-switch, + // so a manual stop or a fresh single-track selection elsewhere never spuriously advances the + // queue. When the queue is past its last track, end-of-stream simply stops — nothing to advance. + private void OnTrackEnded() + { + if (!HasNext) return; + // Fire-and-forget is deliberate: TrackEnded is a synchronous event invoked from the player's + // end-of-playback callback continuation; we must not block it. Advancing kicks off the next + // stream, whose own failures surface through the player's ErrorMessage/state — the queue does + // not own playback error handling. + _ = Next(); + } + + private async Task PlayCurrent() + { + var track = Current; + if (track is null || _player is null) return; + await _player.SelectTrackStreaming(track); + } + + public void Dispose() + { + if (_player != null) + { + _player.TrackEnded -= OnTrackEnded; + _player = null; + } + } +} diff --git a/DeepDrftTests/DeepDrftTests.csproj b/DeepDrftTests/DeepDrftTests.csproj index a3b1407..cab1aed 100644 --- a/DeepDrftTests/DeepDrftTests.csproj +++ b/DeepDrftTests/DeepDrftTests.csproj @@ -30,6 +30,10 @@ + + diff --git a/DeepDrftTests/QueueServiceTests.cs b/DeepDrftTests/QueueServiceTests.cs new file mode 100644 index 0000000..c44264c --- /dev/null +++ b/DeepDrftTests/QueueServiceTests.cs @@ -0,0 +1,450 @@ +using DeepDrftModels.DTOs; +using DeepDrftPublic.Client.Services; +using Microsoft.AspNetCore.Components; + +namespace DeepDrftTests; + +/// +/// Unit tests for the play-queue orchestrator (). The queue is pure +/// domain logic over the single-slot player, so it is exercised here against a recording fake +/// () — no browser, no JS interop, no DI container. Coverage: +/// enqueue, ordered advance, next/previous bounds, clear, current-index integrity, and +/// auto-advance on the player's signal. +/// +[TestFixture] +public class QueueServiceTests +{ + private FakeStreamingPlayer _player = null!; + private QueueService _queue = null!; + + [SetUp] + public void SetUp() + { + _player = new FakeStreamingPlayer(); + _queue = new QueueService(); + _queue.Attach(_player); + } + + [TearDown] + public void TearDown() => _queue.Dispose(); + + private static List Tracks(int count) => + Enumerable.Range(1, count) + .Select(i => new TrackDto { EntryKey = $"track-{i}", TrackName = $"Track {i}", TrackNumber = i }) + .ToList(); + + // --- Empty-queue invariants (no regression to single-track play) --- + + [Test] + public void NewQueue_IsEmptyWithCurrentIndexNegativeOne() + { + Assert.Multiple(() => + { + Assert.That(_queue.Items, Is.Empty); + Assert.That(_queue.CurrentIndex, Is.EqualTo(-1)); + Assert.That(_queue.Current, Is.Null); + Assert.That(_queue.HasNext, Is.False); + Assert.That(_queue.HasPrevious, Is.False); + }); + } + + [Test] + public async Task NextAndPrevious_OnEmptyQueue_AreNoOpsAndDriveNoPlayback() + { + await _queue.Next(); + await _queue.Previous(); + + Assert.Multiple(() => + { + Assert.That(_queue.CurrentIndex, Is.EqualTo(-1)); + Assert.That(_player.SelectedTracks, Is.Empty); + }); + } + + // --- PlayRelease: enqueue + ordered start --- + + [Test] + public async Task PlayRelease_LoadsTracksInOrderAndStreamsFirst() + { + var tracks = Tracks(3); + + await _queue.PlayRelease(tracks); + + Assert.Multiple(() => + { + Assert.That(_queue.Items.Select(t => t.EntryKey), + Is.EqualTo(new[] { "track-1", "track-2", "track-3" })); + Assert.That(_queue.CurrentIndex, Is.EqualTo(0)); + Assert.That(_queue.Current!.EntryKey, Is.EqualTo("track-1")); + Assert.That(_player.SelectedTracks.Single().EntryKey, Is.EqualTo("track-1")); + }); + } + + [Test] + public async Task PlayRelease_WithStartIndex_StartsMidAlbumAndKeepsRemainderQueued() + { + var tracks = Tracks(4); + + await _queue.PlayRelease(tracks, startIndex: 2); + + Assert.Multiple(() => + { + Assert.That(_queue.CurrentIndex, Is.EqualTo(2)); + Assert.That(_queue.Current!.EntryKey, Is.EqualTo("track-3")); + Assert.That(_player.SelectedTracks.Single().EntryKey, Is.EqualTo("track-3")); + Assert.That(_queue.HasNext, Is.True); + Assert.That(_queue.HasPrevious, Is.True); + }); + } + + [Test] + public async Task PlayRelease_ClampsOutOfRangeStartIndex() + { + var tracks = Tracks(3); + + await _queue.PlayRelease(tracks, startIndex: 99); + + Assert.Multiple(() => + { + Assert.That(_queue.CurrentIndex, Is.EqualTo(2)); + Assert.That(_player.SelectedTracks.Single().EntryKey, Is.EqualTo("track-3")); + }); + } + + [Test] + public async Task PlayRelease_WithEmptyTracks_IsNoOp() + { + await _queue.PlayRelease(Enumerable.Empty()); + + Assert.Multiple(() => + { + Assert.That(_queue.Items, Is.Empty); + Assert.That(_queue.CurrentIndex, Is.EqualTo(-1)); + Assert.That(_player.SelectedTracks, Is.Empty); + }); + } + + [Test] + public async Task PlayRelease_ReplacesAnExistingQueue() + { + await _queue.PlayRelease(Tracks(3)); + var second = new List + { + new() { EntryKey = "x-1", TrackName = "X1" }, + new() { EntryKey = "x-2", TrackName = "X2" }, + }; + + await _queue.PlayRelease(second); + + Assert.Multiple(() => + { + Assert.That(_queue.Items, Has.Count.EqualTo(2)); + Assert.That(_queue.Items.Select(t => t.EntryKey), Is.EqualTo(new[] { "x-1", "x-2" })); + Assert.That(_queue.CurrentIndex, Is.EqualTo(0)); + }); + } + + // --- Next / Previous mechanics and bounds --- + + [Test] + public async Task Next_AdvancesThroughTracksInOrder() + { + await _queue.PlayRelease(Tracks(3)); + + await _queue.Next(); + Assert.That(_queue.Current!.EntryKey, Is.EqualTo("track-2")); + + await _queue.Next(); + Assert.That(_queue.Current!.EntryKey, Is.EqualTo("track-3")); + + Assert.That(_player.SelectedTracks.Select(t => t.EntryKey), + Is.EqualTo(new[] { "track-1", "track-2", "track-3" })); + } + + [Test] + public async Task Next_AtLastTrack_IsNoOp() + { + await _queue.PlayRelease(Tracks(2), startIndex: 1); + + await _queue.Next(); + + Assert.Multiple(() => + { + Assert.That(_queue.CurrentIndex, Is.EqualTo(1)); + Assert.That(_queue.HasNext, Is.False); + // Only the initial PlayRelease selection — Next at the end drove no further playback. + Assert.That(_player.SelectedTracks, Has.Count.EqualTo(1)); + }); + } + + [Test] + public async Task Previous_StepsBackThroughTracks() + { + await _queue.PlayRelease(Tracks(3), startIndex: 2); + + await _queue.Previous(); + + Assert.Multiple(() => + { + Assert.That(_queue.CurrentIndex, Is.EqualTo(1)); + Assert.That(_queue.Current!.EntryKey, Is.EqualTo("track-2")); + Assert.That(_player.SelectedTracks.Last().EntryKey, Is.EqualTo("track-2")); + }); + } + + [Test] + public async Task Previous_AtFirstTrack_IsNoOp() + { + await _queue.PlayRelease(Tracks(3)); + + await _queue.Previous(); + + Assert.Multiple(() => + { + Assert.That(_queue.CurrentIndex, Is.EqualTo(0)); + Assert.That(_queue.HasPrevious, Is.False); + Assert.That(_player.SelectedTracks, Has.Count.EqualTo(1)); + }); + } + + // --- Enqueue / EnqueueRange --- + + [Test] + public void Enqueue_AppendsWithoutChangingCurrentOrStartingPlayback() + { + _queue.Enqueue(new TrackDto { EntryKey = "a", TrackName = "A" }); + + Assert.Multiple(() => + { + Assert.That(_queue.Items, Has.Count.EqualTo(1)); + Assert.That(_queue.CurrentIndex, Is.EqualTo(-1)); + Assert.That(_player.SelectedTracks, Is.Empty); + }); + } + + [Test] + public async Task Enqueue_AfterPlayRelease_ExtendsTheQueueAndEnablesHasNext() + { + await _queue.PlayRelease(Tracks(1)); + Assert.That(_queue.HasNext, Is.False); + + _queue.Enqueue(new TrackDto { EntryKey = "appended", TrackName = "Appended" }); + + Assert.Multiple(() => + { + Assert.That(_queue.HasNext, Is.True); + Assert.That(_queue.CurrentIndex, Is.EqualTo(0)); + }); + } + + [Test] + public void EnqueueRange_AppendsAllTracks() + { + _queue.EnqueueRange(Tracks(3)); + + Assert.That(_queue.Items, Has.Count.EqualTo(3)); + } + + // --- Clear --- + + [Test] + public async Task Clear_EmptiesQueueAndResetsIndexWithoutStoppingPlayer() + { + await _queue.PlayRelease(Tracks(3)); + + _queue.Clear(); + + Assert.Multiple(() => + { + Assert.That(_queue.Items, Is.Empty); + Assert.That(_queue.CurrentIndex, Is.EqualTo(-1)); + Assert.That(_queue.Current, Is.Null); + // Clear is a queue-state reset; it must not tear the player down. + Assert.That(_player.StopCount, Is.EqualTo(0)); + }); + } + + // --- QueueChanged notifications --- + + [Test] + public async Task MutatingOperations_RaiseQueueChanged() + { + var count = 0; + _queue.QueueChanged += () => count++; + + await _queue.PlayRelease(Tracks(3)); // 1 + await _queue.Next(); // 2 + await _queue.Previous(); // 3 + _queue.Enqueue(new TrackDto { EntryKey = "z", TrackName = "Z" }); // 4 + _queue.Clear(); // 5 + + Assert.That(count, Is.EqualTo(5)); + } + + [Test] + public void Clear_OnAlreadyEmptyQueue_DoesNotRaiseQueueChanged() + { + var raised = false; + _queue.QueueChanged += () => raised = true; + + _queue.Clear(); + + Assert.That(raised, Is.False); + } + + // --- Auto-advance on TrackEnded --- + + [Test] + public async Task TrackEnded_AutoAdvancesToNextTrack() + { + await _queue.PlayRelease(Tracks(3)); + + _player.RaiseTrackEnded(); + + Assert.Multiple(() => + { + Assert.That(_queue.CurrentIndex, Is.EqualTo(1)); + Assert.That(_queue.Current!.EntryKey, Is.EqualTo("track-2")); + Assert.That(_player.SelectedTracks.Last().EntryKey, Is.EqualTo("track-2")); + }); + } + + [Test] + public async Task TrackEnded_OnLastTrack_DoesNotAdvanceOrReplay() + { + await _queue.PlayRelease(Tracks(2), startIndex: 1); + + _player.RaiseTrackEnded(); + + Assert.Multiple(() => + { + Assert.That(_queue.CurrentIndex, Is.EqualTo(1)); + Assert.That(_player.SelectedTracks, Has.Count.EqualTo(1)); + }); + } + + [Test] + public void TrackEnded_OnEmptyQueue_IsIgnored() + { + _player.RaiseTrackEnded(); + + Assert.Multiple(() => + { + Assert.That(_queue.CurrentIndex, Is.EqualTo(-1)); + Assert.That(_player.SelectedTracks, Is.Empty); + }); + } + + [Test] + public async Task TrackEnded_PlaysWholeAlbumThroughToTheEnd() + { + await _queue.PlayRelease(Tracks(3)); + + _player.RaiseTrackEnded(); // → track-2 + _player.RaiseTrackEnded(); // → track-3 + _player.RaiseTrackEnded(); // last track: no advance + + Assert.Multiple(() => + { + Assert.That(_queue.CurrentIndex, Is.EqualTo(2)); + Assert.That(_player.SelectedTracks.Select(t => t.EntryKey), + Is.EqualTo(new[] { "track-1", "track-2", "track-3" })); + }); + } + + // --- Attach lifecycle --- + + [Test] + public async Task Dispose_UnsubscribesFromTrackEnded_SoNoAutoAdvanceAfterDispose() + { + await _queue.PlayRelease(Tracks(3)); + _queue.Dispose(); + + _player.RaiseTrackEnded(); + + Assert.That(_queue.CurrentIndex, Is.EqualTo(0)); + } + + [Test] + public async Task Attach_ToNewPlayer_RedirectsPlaybackAndAutoAdvance() + { + var second = new FakeStreamingPlayer(); + _queue.Attach(second); + + await _queue.PlayRelease(Tracks(3)); + Assert.That(second.SelectedTracks.Single().EntryKey, Is.EqualTo("track-1")); + + // The old player's TrackEnded must no longer drive this queue. + _player.RaiseTrackEnded(); + Assert.That(_queue.CurrentIndex, Is.EqualTo(0)); + + // The newly attached player does. + second.RaiseTrackEnded(); + Assert.That(_queue.CurrentIndex, Is.EqualTo(1)); + } + + /// + /// Records the tracks the queue asks the player to stream and lets a test raise the + /// player's organic end-of-stream signal. Implements the full + /// surface but only the members the queue actually drives carry behavior; the rest are inert + /// — the queue touches nothing else, which is exactly the seam this fake pins down. + /// + private sealed class FakeStreamingPlayer : IStreamingPlayerService + { + public List SelectedTracks { get; } = new(); + public int StopCount { get; private set; } + + public void RaiseTrackEnded() => TrackEnded?.Invoke(); + + public Task SelectTrackStreaming(TrackDto track) + { + SelectedTracks.Add(track); + CurrentTrack = track; + return Task.CompletedTask; + } + + public Task Stop() + { + StopCount++; + return Task.CompletedTask; + } + + public event Action? TrackEnded; + + // Part of the implemented contract but the queue never subscribes to it, so it is + // intentionally never raised here. +#pragma warning disable CS0067 + public event Action? StateChanged; +#pragma warning restore CS0067 + + // Inert remainder of the contract — the queue never invokes these. + public bool IsInitialized => false; + public bool IsLoaded => false; + public bool IsLoading => false; + public bool IsPlaying => false; + public bool IsPaused => false; + public double CurrentTime => 0; + public double? Duration => null; + public double Volume => 1.0; + public double LoadProgress => 0; + public string? ErrorMessage => null; + public TrackDto? CurrentTrack { get; private set; } + public double[]? WaveformProfile => null; + public EventCallback? OnStateChanged { get; set; } + public EventCallback? OnTrackSelected { get; set; } + public bool IsStreamingMode => false; + public bool CanStartStreaming => false; + public bool HeaderParsed => false; + public int BufferedChunks => 0; + + public Task InitializeAsync() => Task.CompletedTask; + public Task SelectTrack(TrackDto track) => SelectTrackStreaming(track); + public Task Unload() => Task.CompletedTask; + public Task TogglePlayPause() => Task.CompletedTask; + public Task Seek(double position) => Task.CompletedTask; + public Task SetVolume(double volume) => Task.CompletedTask; + public Task ClearError() => Task.CompletedTask; + public Task WarmAudioContext() => Task.CompletedTask; + public Task StageTrack(TrackDto track) => Task.CompletedTask; + } +} From 294414d00a3f9ae56789aff9068ee11095b1e911 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Tue, 16 Jun 2026 00:13:51 -0400 Subject: [PATCH 2/2] fix(queue): guard OnTrackEnded against direct-play cross-context advance Only advance when player's CurrentTrack.Id matches queue's Current.Id; direct-play call sites (SessionDetail, StreamNowButton, resume) that supersede the queue no longer spuriously advance the album. Adds regression test covering the scenario. --- .../Services/QueueService.cs | 8 ++++ DeepDrftTests/QueueServiceTests.cs | 38 +++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/DeepDrftPublic.Client/Services/QueueService.cs b/DeepDrftPublic.Client/Services/QueueService.cs index 03a14c9..2138c37 100644 --- a/DeepDrftPublic.Client/Services/QueueService.cs +++ b/DeepDrftPublic.Client/Services/QueueService.cs @@ -108,9 +108,17 @@ public sealed class QueueService : IQueueService, IDisposable // Advance on organic end-of-stream only. TrackEnded is not raised by stop/unload/track-switch, // so a manual stop or a fresh single-track selection elsewhere never spuriously advances the // queue. When the queue is past its last track, end-of-stream simply stops — nothing to advance. + // + // Guard: only advance when the track that just ended is the queue's own current item. Call sites + // that stream a single track directly (SessionDetail, StreamNowButton, resume from AudioPlayerBar) + // overwrite the player's CurrentTrack without touching the queue. If their track reaches natural + // end, the player fires TrackEnded — but the queue's Current no longer matches the player's + // CurrentTrack, so we must not advance. Id-based equality is used rather than ReferenceEquals + // because DTO copies through serialisation are not reference-equal. private void OnTrackEnded() { if (!HasNext) return; + if (_player?.CurrentTrack?.Id != Current?.Id) return; // Fire-and-forget is deliberate: TrackEnded is a synchronous event invoked from the player's // end-of-playback callback continuation; we must not block it. Advancing kicks off the next // stream, whose own failures surface through the player's ErrorMessage/state — the queue does diff --git a/DeepDrftTests/QueueServiceTests.cs b/DeepDrftTests/QueueServiceTests.cs index c44264c..0321ccd 100644 --- a/DeepDrftTests/QueueServiceTests.cs +++ b/DeepDrftTests/QueueServiceTests.cs @@ -335,6 +335,37 @@ public class QueueServiceTests }); } + /// + /// Regression guard for the cross-context spurious-advance bug: a direct single-track play + /// (Session, StreamNowButton, resume) overwrites the player's CurrentTrack without touching the + /// queue. When that external track reaches its natural end, TrackEnded fires — but the queue's + /// Current no longer matches the player's CurrentTrack, so the queue must NOT advance. + /// + [Test] + public async Task TrackEnded_WhenPlayerCurrentTrackIsNotQueueCurrent_DoesNotAdvance() + { + // Load a 3-track album into the queue (queue.Current → track-1, player.CurrentTrack → track-1). + await _queue.PlayRelease(Tracks(3)); + Assert.That(_queue.CurrentIndex, Is.EqualTo(0)); + + // Simulate a direct play (e.g. SessionDetail streams an unrelated track by Id = 99). + // The player's CurrentTrack is now the session track, but the queue is still on track-1. + var sessionTrack = new TrackDto { Id = 99, EntryKey = "session-track", TrackName = "Session Mix" }; + _player.SimulateDirectPlay(sessionTrack); + + // The session track finishes naturally — player raises TrackEnded. + _player.RaiseTrackEnded(); + + // The queue must not have advanced: index still 0, and no additional SelectTrackStreaming + // calls beyond the initial PlayRelease selection. + Assert.Multiple(() => + { + Assert.That(_queue.CurrentIndex, Is.EqualTo(0), "Queue must not advance when a direct-play track ends"); + Assert.That(_player.SelectedTracks, Has.Count.EqualTo(1), "No further streaming must be triggered by the queue"); + Assert.That(_player.SelectedTracks.Single().EntryKey, Is.EqualTo("track-1"), "Only the original PlayRelease selection must have been streamed"); + }); + } + [Test] public async Task TrackEnded_PlaysWholeAlbumThroughToTheEnd() { @@ -396,6 +427,13 @@ public class QueueServiceTests public void RaiseTrackEnded() => TrackEnded?.Invoke(); + /// + /// Sets to without recording a + /// entry. Models a direct single-track play (SessionDetail, + /// StreamNowButton, resume) that overwrites the player state without going through the queue. + /// + public void SimulateDirectPlay(TrackDto track) => CurrentTrack = track; + public Task SelectTrackStreaming(TrackDto track) { SelectedTracks.Add(track);