From d3074ab6fe361a6a33db25f5389f98324bd3db80 Mon Sep 17 00:00:00 2001 From: Owen Rummage Date: Sun, 23 Aug 2026 08:07:16 -0500 Subject: [PATCH] OpenSubsonic: indexBasedQueue (getPlayQueueByIndex, savePlayQueueByIndex) with real duplicate-song support --- internal/subsonic/opensubsonic_core_test.go | 1 + internal/subsonic/playqueue_by_index.go | 90 +++++++++++ internal/subsonic/playqueue_by_index_test.go | 153 +++++++++++++++++++ 3 files changed, 244 insertions(+) create mode 100644 internal/subsonic/playqueue_by_index.go create mode 100644 internal/subsonic/playqueue_by_index_test.go diff --git a/internal/subsonic/opensubsonic_core_test.go b/internal/subsonic/opensubsonic_core_test.go index 6020c34..037444b 100644 --- a/internal/subsonic/opensubsonic_core_test.go +++ b/internal/subsonic/opensubsonic_core_test.go @@ -55,6 +55,7 @@ func TestGetOpenSubsonicExtensions(t *testing.T) { // set once Phase 3 is done. for _, want := range []string{ "apiKeyAuthentication", "formPost", "getPodcastEpisode", "topSongsByArtistId", "transcodeOffset", + "indexBasedQueue", } { if !names[want] { t.Errorf("extension %q not advertised: %v", want, names) diff --git a/internal/subsonic/playqueue_by_index.go b/internal/subsonic/playqueue_by_index.go new file mode 100644 index 0000000..2544ee8 --- /dev/null +++ b/internal/subsonic/playqueue_by_index.go @@ -0,0 +1,90 @@ +package subsonic + +import ( + "encoding/json" + + "git.noroutes.net/NoRoutes/vibetunes/internal/database" +) + +func init() { + registerExtension("indexBasedQueue", 1) + register("getPlayQueueByIndex", handleGetPlayQueueByIndex) + register("savePlayQueueByIndex", handleSavePlayQueueByIndex) +} + +// The indexBasedQueue extension shares the same play_queues storage as +// the classic getPlayQueue/savePlayQueue (internal/subsonic/playqueue.go) +// — the table has always tracked the current *index*, not a song ID; the +// classic endpoints are the ones that lossily resolve index -> song ID +// for their "current" attribute. Duplicate song IDs in the queue are +// therefore already handled correctly by construction: saving stores a +// plain ordered ID list, and both current-track values (index here, +// looked-up ID for the classic endpoint) are derived from the same +// unambiguous index. +func handleGetPlayQueueByIndex(rc *Context) *apiError { + db := rc.Server.DB + q, err := database.GetPlayQueue(db, rc.User.ID) + if err == database.ErrNotFound { + rc.Response = newResponse() + return nil + } + if err != nil { + return newError(ErrGeneric, err.Error()) + } + + var ids []string + if uerr := json.Unmarshal([]byte(q.SongIDsJSON), &ids); uerr != nil { + return newError(ErrGeneric, "corrupt saved play queue") + } + + out := &PlayQueueByIndex{ + CurrentIndex: q.CurrentIndex, + Position: int64(q.PositionMs), + Username: rc.User.Username, + Changed: isoTime(q.UpdatedAt), + ChangedBy: q.Client, + } + for _, id := range ids { + song, serr := database.GetSong(db, id) + if serr != nil { + continue + } + mf, merr := database.GetMediaFile(db, song.MediaFileID) + if merr != nil { + continue + } + out.Entry = append(out.Entry, childFromSong(db, song, mf, albumOfSong(db, song), rc.User.ID)) + } + + rc.Response = newResponse() + rc.Response.PlayQueueByIndex = out + return nil +} + +func handleSavePlayQueueByIndex(rc *Context) *apiError { + ids := rc.Params.GetAll("id") + position := rc.Params.Int("position", 0) + client := rc.Params.Get("c") + + currentIndex := 0 + if len(ids) > 0 { + var aerr *apiError + currentIndex, aerr = rc.Params.RequiredInt("currentIndex") + if aerr != nil { + return aerr + } + if currentIndex < 0 || currentIndex >= len(ids) { + return newError(ErrGeneric, "currentIndex out of range") + } + } + + encoded, err := json.Marshal(ids) + if err != nil { + return newError(ErrGeneric, err.Error()) + } + if err := database.SavePlayQueue(rc.Server.DB, rc.User.ID, string(encoded), currentIndex, position, client); err != nil { + return newError(ErrGeneric, err.Error()) + } + rc.Response = newResponse() + return nil +} diff --git a/internal/subsonic/playqueue_by_index_test.go b/internal/subsonic/playqueue_by_index_test.go new file mode 100644 index 0000000..db4931f --- /dev/null +++ b/internal/subsonic/playqueue_by_index_test.go @@ -0,0 +1,153 @@ +package subsonic + +import ( + "testing" + + "git.noroutes.net/NoRoutes/vibetunes/internal/database" +) + +func TestPlayQueueByIndexRoundTrip(t *testing.T) { + s, srv := newTestRecorderServer(t) + createTestUser(t, s, "alice", "password123", false) + seedLibrary(t, s) + songs, err := allSongIDs(t, s) + if err != nil || len(songs) < 2 { + t.Fatalf("need >=2 songs, got %v %v", songs, err) + } + + q := baseParams("alice", "password123") + q.Add("id", songs[0]) + q.Add("id", songs[1]) + q.Set("currentIndex", "1") + q.Set("position", "5000") + resp := getPath(t, srv.URL, "/rest/savePlayQueueByIndex.view", q) + x := decodeXML(t, resp) + if x.Status != "ok" { + t.Fatalf("savePlayQueueByIndex failed: %+v", x) + } + + q2 := baseParams("alice", "password123") + q2.Set("f", "json") + resp = getPath(t, srv.URL, "/rest/getPlayQueueByIndex.view", q2) + j := decodeJSON(t, resp) + pq, ok := j["playQueueByIndex"].(map[string]any) + if !ok { + t.Fatalf("playQueueByIndex missing: %v", j) + } + if int(pq["currentIndex"].(float64)) != 1 { + t.Fatalf("currentIndex = %v, want 1", pq["currentIndex"]) + } + entries, ok := pq["entry"].([]any) + if !ok || len(entries) != 2 { + t.Fatalf("entries = %v, want 2", pq["entry"]) + } +} + +// TestPlayQueueByIndexHandlesDuplicateSongs is the whole point of this +// extension: the same song ID can appear twice, and currentIndex +// unambiguously identifies which occurrence is playing — unlike the +// classic getPlayQueue's "current" song-ID field, which can't +// distinguish between two occurrences of the same song. +func TestPlayQueueByIndexHandlesDuplicateSongs(t *testing.T) { + s, srv := newTestRecorderServer(t) + createTestUser(t, s, "alice", "password123", false) + seedLibrary(t, s) + songs, err := allSongIDs(t, s) + if err != nil || len(songs) == 0 { + t.Fatalf("need >=1 song, got %v %v", songs, err) + } + same := songs[0] + + q := baseParams("alice", "password123") + q.Add("id", same) + q.Add("id", same) + q.Add("id", same) + q.Set("currentIndex", "2") // the third occurrence, not the first + resp := getPath(t, srv.URL, "/rest/savePlayQueueByIndex.view", q) + x := decodeXML(t, resp) + if x.Status != "ok" { + t.Fatalf("save failed: %+v", x) + } + + q2 := baseParams("alice", "password123") + q2.Set("f", "json") + resp = getPath(t, srv.URL, "/rest/getPlayQueueByIndex.view", q2) + j := decodeJSON(t, resp) + pq := j["playQueueByIndex"].(map[string]any) + if int(pq["currentIndex"].(float64)) != 2 { + t.Fatalf("currentIndex = %v, want 2 (third occurrence)", pq["currentIndex"]) + } + entries := pq["entry"].([]any) + if len(entries) != 3 { + t.Fatalf("entries = %v, want 3 (duplicates preserved)", entries) + } +} + +func TestSavePlayQueueByIndexClears(t *testing.T) { + s, srv := newTestRecorderServer(t) + createTestUser(t, s, "alice", "password123", false) + seedLibrary(t, s) + songs, _ := allSongIDs(t, s) + + q := baseParams("alice", "password123") + q.Add("id", songs[0]) + q.Set("currentIndex", "0") + getPath(t, srv.URL, "/rest/savePlayQueueByIndex.view", q) + + clear := baseParams("alice", "password123") + resp := getPath(t, srv.URL, "/rest/savePlayQueueByIndex.view", clear) + x := decodeXML(t, resp) + if x.Status != "ok" { + t.Fatalf("clear failed: %+v", x) + } + + q2 := baseParams("alice", "password123") + q2.Set("f", "json") + resp = getPath(t, srv.URL, "/rest/getPlayQueueByIndex.view", q2) + j := decodeJSON(t, resp) + pq := j["playQueueByIndex"].(map[string]any) + if _, ok := pq["entry"]; ok { + t.Fatalf("expected empty queue after clearing, got %v", pq) + } +} + +func TestSavePlayQueueByIndexMissingCurrentIndex(t *testing.T) { + s, srv := newTestRecorderServer(t) + createTestUser(t, s, "alice", "password123", false) + seedLibrary(t, s) + songs, _ := allSongIDs(t, s) + + q := baseParams("alice", "password123") + q.Add("id", songs[0]) + resp := getPath(t, srv.URL, "/rest/savePlayQueueByIndex.view", q) + x := decodeXML(t, resp) + if x.Status != "failed" || x.Error == nil || x.Error.Code != ErrMissingParameter { + t.Fatalf("expected error 10 when ids given without currentIndex, got %+v", x) + } +} + +func TestGetPlayQueueByIndexEmpty(t *testing.T) { + s, srv := newTestRecorderServer(t) + createTestUser(t, s, "alice", "password123", false) + + q := baseParams("alice", "password123") + q.Set("f", "json") + resp := getPath(t, srv.URL, "/rest/getPlayQueueByIndex.view", q) + j := decodeJSON(t, resp) + if j["status"] != "ok" { + t.Fatalf("status = %v", j["status"]) + } +} + +func allSongIDs(t *testing.T, s *Server) ([]string, error) { + t.Helper() + songs, err := database.ListSongs(s.DB, 10, 0) + if err != nil { + return nil, err + } + ids := make([]string, len(songs)) + for i, song := range songs { + ids[i] = song.ID + } + return ids, nil +}