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.
This commit is contained in:
@@ -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,
|
// 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
|
// 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.
|
// 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()
|
private void OnTrackEnded()
|
||||||
{
|
{
|
||||||
if (!HasNext) return;
|
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
|
// 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
|
// 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
|
// stream, whose own failures surface through the player's ErrorMessage/state — the queue does
|
||||||
|
|||||||
@@ -335,6 +335,37 @@ public class QueueServiceTests
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// 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.
|
||||||
|
/// </summary>
|
||||||
|
[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]
|
[Test]
|
||||||
public async Task TrackEnded_PlaysWholeAlbumThroughToTheEnd()
|
public async Task TrackEnded_PlaysWholeAlbumThroughToTheEnd()
|
||||||
{
|
{
|
||||||
@@ -396,6 +427,13 @@ public class QueueServiceTests
|
|||||||
|
|
||||||
public void RaiseTrackEnded() => TrackEnded?.Invoke();
|
public void RaiseTrackEnded() => TrackEnded?.Invoke();
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Sets <see cref="CurrentTrack"/> to <paramref name="track"/> without recording a
|
||||||
|
/// <see cref="SelectedTracks"/> entry. Models a direct single-track play (SessionDetail,
|
||||||
|
/// StreamNowButton, resume) that overwrites the player state without going through the queue.
|
||||||
|
/// </summary>
|
||||||
|
public void SimulateDirectPlay(TrackDto track) => CurrentTrack = track;
|
||||||
|
|
||||||
public Task SelectTrackStreaming(TrackDto track)
|
public Task SelectTrackStreaming(TrackDto track)
|
||||||
{
|
{
|
||||||
SelectedTracks.Add(track);
|
SelectedTracks.Add(track);
|
||||||
|
|||||||
Reference in New Issue
Block a user