fix(playqueue): keep the current track when earlier tracks are missing

RetrieveWithMediaFiles drops queue entries whose tracks are no longer in
the library (purged, or in a library the user lost access to), but it kept
the stored current index. That index then pointed one track further for
each dropped track before it, so getPlayQueue, getPlayQueueByIndex and
GET /api/queue reported the wrong song with the old position.

The current index is now remapped to the same track in the filtered list.
If the current track itself is gone, the queue moves to the next remaining
track (or the last one) and restarts it. The cleaned queue is then saved
back in the background, so partial PUT /api/queue updates use the same
list the client sees. The save is a conditional UPDATE that only applies
if the row is unchanged since it was read, so it never undoes a newer
client save, and it is skipped if loading the tracks failed.

Fixes #6297

persistence/playqueue_repository.go

@@ -80,10 +80,47 @@ func (r *playQueueRepository) RetrieveWithMediaFiles(ctx context.Context, userId
 	var res playQueue
 	err := r.queryOne(ctx, sel, &res)
 	q := r.toModel(&res)
-	q.Items = r.loadTracks(ctx, q.Items)
+	stored := q.Items
+	var loadErr error
+	q.Items, loadErr = r.loadTracks(ctx, stored)
+	if loadErr == nil && len(q.Items) < len(stored) {
+		q.Current, q.Position = remapCurrent(stored, q.Items, q.Current, q.Position)
+		go r.saveCleanedQueue(context.WithoutCancel(ctx), res, r.fromModel(&q))
+	}
 	return &q, err
 }
 
+// remapCurrent relies on loaded being stored in the same order, minus the missing tracks. If the current track
+// is gone, it moves to the next remaining one (or the last) and restarts it.
+func remapCurrent(stored, loaded model.MediaFiles, current int, position int64) (int, int64) {
+	kept := 0
+	for i, t := range stored {
+		if kept >= len(loaded) || loaded[kept].ID != t.ID {
+			continue
+		}
+		if i >= current {
+			if i > current {
+				position = 0
+			}
+			return kept, position
+		}
+		kept++
+	}
+	return max(len(loaded)-1, 0), 0
+}
+
+// saveCleanedQueue only writes if the queue was not changed since it was read, so it never undoes a newer save.
+func (r *playQueueRepository) saveCleanedQueue(ctx context.Context, read, cleaned playQueue) {
+	ctx, cancel := context.WithTimeout(ctx, 10*time.Second)
+	defer cancel()
+	upd := Update(r.tableName).
+		Set("items", cleaned.Items).Set("current", cleaned.Current).Set("position", cleaned.Position).
+		Where(Eq{"id": read.ID, "items": read.Items, "current": read.Current, "position": read.Position})
+	if _, err := r.executeSQL(ctx, upd); err != nil {
+		log.Warn(ctx, "Could not remove missing tracks from playqueue", "user", read.UserID, err)
+	}
+}
+
 func (r *playQueueRepository) Retrieve(ctx context.Context, userId string) (*model.PlayQueue, error) {
 	sel := r.newSelect(ctx).Columns("*").Where(Eq{"user_id": userId})
 	var res playQueue
@@ -130,13 +167,14 @@ func (r *playQueueRepository) toModel(pq *playQueue) model.PlayQueue {
 
 // loadTracks loads the tracks from the database. It receives a list of track IDs and returns a list of MediaFiles
 // in the same order as the input list.
-func (r *playQueueRepository) loadTracks(ctx context.Context, tracks model.MediaFiles) model.MediaFiles {
+func (r *playQueueRepository) loadTracks(ctx context.Context, tracks model.MediaFiles) (model.MediaFiles, error) {
 	if len(tracks) == 0 {
-		return nil
+		return nil, nil
 	}
 
 	mfRepo := NewMediaFileRepository(r.db)
 	trackMap := map[string]model.MediaFile{}
+	var loadErr error
 
 	// Create an iterator to collect all track IDs
 	ids := slice.SeqFunc(tracks, func(t model.MediaFile) string { return t.ID })
@@ -148,6 +186,7 @@ func (r *playQueueRepository) loadTracks(ctx context.Context, tracks model.Media
 		if err != nil {
 			u := loggedUser(ctx)
 			log.Error(ctx, "Could not load playqueue/bookmark's tracks", "user", u.UserName, err)
+			loadErr = err
 		}
 		for _, t := range tracks {
 			trackMap[t.ID] = t
@@ -162,7 +201,7 @@ func (r *playQueueRepository) loadTracks(ctx context.Context, tracks model.Media
 			newTracks = append(newTracks, track)
 		}
 	}
-	return newTracks
+	return newTracks, loadErr
 }
 
 func (r *playQueueRepository) clearPlayQueue(ctx context.Context, userId string) error {

persistence/playqueue_repository_test.go

@@ -20,7 +20,7 @@ var _ = Describe("PlayQueueRepository", func() {
 
 	BeforeEach(func() {
 		DeferCleanup(configtest.SetupConfig())
-		ctx = log.NewContext(context.TODO())
+		ctx = log.NewContext(GinkgoT().Context())
 		ctx = request.WithUser(ctx, model.User{ID: "userid", UserName: "userid", IsAdmin: true})
 		repo = NewPlayQueueRepository(GetDBXBuilder())
 	})
@@ -311,36 +311,68 @@ var _ = Describe("PlayQueueRepository", func() {
 			AssertPlayQueue(expected, actual)
 		})
 
-		It("does not return tracks if they don't exist in the DB", func() {
-			// Add a new song to the DB
-			newSong := songRadioactivity
-			newSong.ID = "temp-track"
-			newSong.Path = "/new-path"
-			mfRepo := NewMediaFileRepository(GetDBXBuilder())
+		Context("when tracks are no longer in the library", func() {
+			var mfRepo model.MediaFileRepository
+			gone := songRadioactivity
+			gone.ID = "gone-track"
+			gone.Path = "/gone-track"
 
-			Expect(mfRepo.Put(ctx, &newSong)).To(Succeed())
+			BeforeEach(func() {
+				mfRepo = NewMediaFileRepository(GetDBXBuilder())
+				Expect(mfRepo.Put(ctx, &gone)).To(Succeed())
+				DeferCleanup(func() { _ = mfRepo.Delete(ctx, gone.ID) })
+			})
 
-			// Create a playqueue with the new song
-			pq := aPlayQueue("userid", 0, 0, newSong, songAntenna)
-			Expect(repo.Store(ctx, pq)).To(Succeed())
+			retrieveAfterRemoval := func(current int, position int64, items ...model.MediaFile) *model.PlayQueue {
+				Expect(repo.Store(ctx, aPlayQueue("userid", current, position, items...))).To(Succeed())
+				Expect(mfRepo.Delete(ctx, gone.ID)).To(Succeed())
+				actual, err := repo.RetrieveWithMediaFiles(ctx, "userid")
+				Expect(err).ToNot(HaveOccurred())
+				return actual
+			}
 
-			// Retrieve the playqueue
-			actual, err := repo.RetrieveWithMediaFiles(ctx, "userid")
-			Expect(err).ToNot(HaveOccurred())
+			DescribeTable("points to the right current track",
+				func(current int, items []model.MediaFile, expectedID string, expectedPosition int64) {
+					actual := retrieveAfterRemoval(current, 45, items...)
 
-			// The playqueue should contain both tracks
-			AssertPlayQueue(pq, actual)
+					Expect(actual.Items).To(HaveLen(len(items) - 1))
+					Expect(actual.Items[actual.Current].ID).To(Equal(expectedID))
+					Expect(actual.Position).To(Equal(expectedPosition))
+				},
+				Entry("when an earlier track is removed", 1, []model.MediaFile{gone, songComeTogether, songDayInALife}, songComeTogether.ID, int64(45)),
+				Entry("when a later track is removed", 0, []model.MediaFile{songComeTogether, gone}, songComeTogether.ID, int64(45)),
+				Entry("moving to the next track from the start when the current one is removed", 1, []model.MediaFile{songComeTogether, gone, songDayInALife}, songDayInALife.ID, int64(0)),
+				Entry("moving to the last track when the removed current one was the last", 1, []model.MediaFile{songComeTogether, gone}, songComeTogether.ID, int64(0)),
+			)
 
-			// Delete the new song
-			Expect(mfRepo.Delete(ctx, "temp-track")).To(Succeed())
+			It("saves the cleaned queue", func() {
+				retrieveAfterRemoval(1, 45, gone, songComeTogether, songDayInALife)
 
-			// Retrieve the playqueue
-			actual, err = repo.RetrieveWithMediaFiles(ctx, "userid")
-			Expect(err).ToNot(HaveOccurred())
+				Eventually(func(g Gomega) {
+					stored, err := repo.Retrieve(ctx, "userid")
+					g.Expect(err).ToNot(HaveOccurred())
+					g.Expect(stored.Items).To(HaveLen(2))
+					g.Expect(stored.Items[0].ID).To(Equal(songComeTogether.ID))
+					g.Expect(stored.Current).To(BeZero())
+					g.Expect(stored.Position).To(Equal(int64(45)))
+				}).Should(Succeed())
+			})
 
-			// The playqueue should not contain the deleted track
-			Expect(actual.Items).To(HaveLen(1))
-			Expect(actual.Items[0].ID).To(Equal(songAntenna.ID))
+			It("does not overwrite a queue that changed after it was read", func() {
+				r := repo.(*playQueueRepository)
+				read := aPlayQueue("userid", 1, 45, gone, songComeTogether)
+				Expect(repo.Store(ctx, read)).To(Succeed())
+
+				By("Saving a new queue from a client before the cleanup runs")
+				Expect(repo.Store(ctx, aPlayQueue("userid", 0, 10, songAntenna))).To(Succeed())
+				r.saveCleanedQueue(ctx, r.fromModel(read), playQueue{Items: songComeTogether.ID, Position: 45})
+
+				stored, err := repo.Retrieve(ctx, "userid")
+				Expect(err).ToNot(HaveOccurred())
+				Expect(stored.Items).To(HaveLen(1))
+				Expect(stored.Items[0].ID).To(Equal(songAntenna.ID))
+				Expect(stored.Position).To(Equal(int64(10)))
+			})
 		})
 	})