fix(server): ensure single record per user by reusing existing playqueue ID

Signed-off-by: Deluan <deluan@navidrome.org>
This commit is contained in:
Deluan
2025-06-11 17:26:13 -04:00
parent 410e457e5a
commit f7e005a991
2 changed files with 67 additions and 1 deletions

View File

@@ -2,6 +2,7 @@ package persistence
import (
"context"
"errors"
"strings"
"time"
@@ -38,6 +39,18 @@ type playQueue struct {
func (r *playQueueRepository) Store(q *model.PlayQueue, colNames ...string) error {
u := loggedUser(r.ctx)
// Always find existing playqueue for this user
existingQueue, err := r.Retrieve(q.UserID)
if err != nil && !errors.Is(err, model.ErrNotFound) {
log.Error(r.ctx, "Error retrieving existing playqueue", "user", u.UserName, err)
return err
}
// Use existing ID if found, otherwise keep the provided ID (which may be empty for new records)
if !errors.Is(err, model.ErrNotFound) && existingQueue.ID != "" {
q.ID = existingQueue.ID
}
// When no specific columns are provided, we replace the whole queue
if len(colNames) == 0 {
err := r.clearPlayQueue(q.UserID)
@@ -55,7 +68,7 @@ func (r *playQueueRepository) Store(q *model.PlayQueue, colNames ...string) erro
pq.CreatedAt = time.Now()
}
pq.UpdatedAt = time.Now()
_, err := r.put(pq.ID, pq, colNames...)
_, err = r.put(pq.ID, pq, colNames...)
if err != nil {
log.Error(r.ctx, "Error saving playqueue", "user", u.UserName, err)
return err

View File

@@ -169,6 +169,59 @@ var _ = Describe("PlayQueueRepository", func() {
Expect(actual.Position).To(Equal(int64(200)))
Expect(actual.Items).To(HaveLen(2)) // Should remain unchanged
})
It("ensures only one record per user by reusing existing record ID", func() {
By("Storing initial playqueue")
initial := aPlayQueue("userid", 0, 100, songComeTogether)
Expect(repo.Store(initial)).To(Succeed())
initialCount := countPlayQueues(repo, "userid")
Expect(initialCount).To(Equal(1))
By("Storing another playqueue with different ID but same user")
different := aPlayQueue("userid", 1, 200, songDayInALife)
different.ID = "different-id" // Force a different ID
Expect(repo.Store(different)).To(Succeed())
By("Verifying only one record exists for the user")
finalCount := countPlayQueues(repo, "userid")
Expect(finalCount).To(Equal(1))
By("Verifying the record was updated, not duplicated")
actual, err := repo.Retrieve("userid")
Expect(err).ToNot(HaveOccurred())
Expect(actual.Current).To(Equal(1)) // Should be updated value
Expect(actual.Position).To(Equal(int64(200))) // Should be updated value
Expect(actual.Items).To(HaveLen(1)) // Should be new items
Expect(actual.Items[0].ID).To(Equal(songDayInALife.ID))
})
It("ensures only one record per user even with partial updates", func() {
By("Storing initial playqueue")
initial := aPlayQueue("userid", 0, 100, songComeTogether, songDayInALife)
Expect(repo.Store(initial)).To(Succeed())
initialCount := countPlayQueues(repo, "userid")
Expect(initialCount).To(Equal(1))
By("Storing partial update with different ID but same user")
partialUpdate := &model.PlayQueue{
ID: "completely-different-id", // Use a completely different ID
UserID: "userid",
Current: 1,
ChangedBy: "test-partial",
}
Expect(repo.Store(partialUpdate, "current")).To(Succeed())
By("Verifying only one record still exists for the user")
finalCount := countPlayQueues(repo, "userid")
Expect(finalCount).To(Equal(1))
By("Verifying the existing record was updated with new current value")
actual, err := repo.Retrieve("userid")
Expect(err).ToNot(HaveOccurred())
Expect(actual.Current).To(Equal(1)) // Should be updated value
Expect(actual.Position).To(Equal(int64(100))) // Should remain unchanged
Expect(actual.Items).To(HaveLen(2)) // Should remain unchanged
})
})
Describe("Retrieve", func() {