From 34c1ebc4be808bd04b9624178704c94e3fc41a6b Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 5 Jul 2026 17:21:42 -0400 Subject: [PATCH] feat(jellyfin): playlist read and write-back Adds POST /Playlists, GET/POST/DELETE /Playlists/{id}/Items. Tags each playlist item with PlaylistItemId (the entry's position within the playlist) so DELETE .../Items?EntryIds=... can remove a specific occurrence by the id core/playlists.RemoveTracks actually expects, rather than the song id used everywhere else. --- server/jellyfin/api.go | 5 + server/jellyfin/dto/dto.go | 10 +- server/jellyfin/playlists.go | 99 ++++++++++++++++ server/jellyfin/playlists_test.go | 184 ++++++++++++++++++++++++++++++ 4 files changed, 295 insertions(+), 3 deletions(-) create mode 100644 server/jellyfin/playlists.go create mode 100644 server/jellyfin/playlists_test.go diff --git a/server/jellyfin/api.go b/server/jellyfin/api.go index ed92a61e6..25ec5586a 100644 --- a/server/jellyfin/api.go +++ b/server/jellyfin/api.go @@ -78,6 +78,11 @@ func (api *Router) routes() http.Handler { r.Get("/Genres", api.getGenres) r.Get("/MusicGenres", api.getGenres) + r.Post("/Playlists", api.createPlaylist) + r.Get("/Playlists/{playlistId}/Items", api.getPlaylistItems) + r.Post("/Playlists/{playlistId}/Items", api.addToPlaylist) + r.Delete("/Playlists/{playlistId}/Items", api.removeFromPlaylist) + r.Get("/Audio/{itemId}/stream", api.streamAudio) r.Get("/Audio/{itemId}/stream.{container}", api.streamAudio) r.Get("/Audio/{itemId}/universal", api.streamAudio) diff --git a/server/jellyfin/dto/dto.go b/server/jellyfin/dto/dto.go index 54743b73c..bfa5ba426 100644 --- a/server/jellyfin/dto/dto.go +++ b/server/jellyfin/dto/dto.go @@ -37,9 +37,13 @@ type UserItemDataDto struct { } type BaseItemDto struct { - Name string `json:"Name"` - ServerId string `json:"ServerId,omitempty"` - Id string `json:"Id"` + Name string `json:"Name"` + ServerId string `json:"ServerId,omitempty"` + Id string `json:"Id"` + // PlaylistItemId identifies this entry within a playlist listing (GET /Playlists/{id}/Items). + // It's only set there, and is distinct from Id so a song appearing more than once in the same + // playlist can be removed by occurrence (DELETE .../Items?EntryIds=...) rather than by song id. + PlaylistItemId string `json:"PlaylistItemId,omitempty"` Type string `json:"Type"` IsFolder bool `json:"IsFolder"` MediaType string `json:"MediaType,omitempty"` diff --git a/server/jellyfin/playlists.go b/server/jellyfin/playlists.go new file mode 100644 index 000000000..fa1566ee5 --- /dev/null +++ b/server/jellyfin/playlists.go @@ -0,0 +1,99 @@ +package jellyfin + +import ( + "encoding/json" + "net/http" + "strings" + + "github.com/go-chi/chi/v5" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/server/jellyfin/dto" + "github.com/navidrome/navidrome/utils/slice" +) + +type createPlaylistRequest struct { + Name string `json:"Name"` + Ids []string `json:"Ids"` + MediaType string `json:"MediaType"` +} + +// createPlaylist always creates a brand-new playlist (playlistId "" tells core/playlists.Create +// not to replace an existing one), owned by the authenticated user. +func (api *Router) createPlaylist(w http.ResponseWriter, r *http.Request) { + var body createPlaylistRequest + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + http.Error(w, "Bad Request", http.StatusBadRequest) + return + } + id, err := api.playlists.Create(r.Context(), "", body.Name, body.Ids) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + api.ok(w, r, map[string]string{"Id": id}) +} + +// trackToBaseItem maps a playlist entry to a BaseItemDto, tagging it with PlaylistItemId — the +// entry's position within the playlist (model.PlaylistTrack.ID) — rather than only the underlying +// song id. Jellyfin clients read this list, then echo PlaylistItemId back via +// DELETE .../Items?EntryIds=... to remove a specific occurrence; that's also the id +// core/playlists.RemoveTracks expects (see removeFromPlaylist), so the round trip works even when +// the same song appears more than once in the playlist. +func trackToBaseItem(t model.PlaylistTrack) dto.BaseItemDto { + item := dto.SongToBaseItem(t.MediaFile) + item.PlaylistItemId = t.ID + return item +} + +// getPlaylistItems relies on core/playlists.GetWithTracks to enforce playlist visibility +// (public or owned by the current user); any error — not found or not visible — is reported as +// a generic 404 so a playlist id can't be used to probe for the existence of private playlists. +func (api *Router) getPlaylistItems(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + id := chi.URLParam(r, "playlistId") + pls, err := api.playlists.GetWithTracks(ctx, id) + if err != nil { + http.Error(w, "Not Found", http.StatusNotFound) + return + } + items := slice.Map(pls.Tracks, trackToBaseItem) + api.ok(w, r, dto.QueryResult{Items: items, TotalRecordCount: len(items)}) +} + +// splitIds parses a comma-separated query parameter into ids, treating an absent or empty +// parameter as no ids rather than a single empty-string id. +func splitIds(s string) []string { + if s == "" { + return nil + } + return strings.Split(s, ",") +} + +// addToPlaylist appends songs by their own id (core/playlists.AddTracks treats ids as media file +// ids). Ownership/editability is enforced by AddTracks itself; any error maps to 404. +func (api *Router) addToPlaylist(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + id := chi.URLParam(r, "playlistId") + ids := splitIds(r.URL.Query().Get("Ids")) + if _, err := api.playlists.AddTracks(ctx, id, ids); err != nil { + http.Error(w, "Not Found", http.StatusNotFound) + return + } + w.WriteHeader(http.StatusNoContent) +} + +// removeFromPlaylist removes entries by EntryIds. Unlike addToPlaylist's Ids, these must be +// playlist-entry ids (model.PlaylistTrack.ID / trackToBaseItem's PlaylistItemId) — the position +// of the entry within the playlist — because core/playlists.RemoveTracks deletes playlist_tracks +// rows by that id, not by media file id. Ownership/editability is enforced by RemoveTracks itself; +// any error maps to 404. +func (api *Router) removeFromPlaylist(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + id := chi.URLParam(r, "playlistId") + ids := splitIds(r.URL.Query().Get("EntryIds")) + if err := api.playlists.RemoveTracks(ctx, id, ids); err != nil { + http.Error(w, "Not Found", http.StatusNotFound) + return + } + w.WriteHeader(http.StatusNoContent) +} diff --git a/server/jellyfin/playlists_test.go b/server/jellyfin/playlists_test.go new file mode 100644 index 000000000..76184c867 --- /dev/null +++ b/server/jellyfin/playlists_test.go @@ -0,0 +1,184 @@ +package jellyfin + +import ( + "context" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "strings" + + "github.com/navidrome/navidrome/core/playlists" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/server/jellyfin/dto" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +// fakePlaylists is a local fake for core/playlists.Playlists. It embeds the interface so +// unimplemented methods aren't needed here; only the ones this test exercises are overridden. +type fakePlaylists struct { + playlists.Playlists + + createdName string + createdIds []string + createErr error + + getPls *model.Playlist + getErr error + + addPlaylistID string + addIds []string + addErr error + + removePlaylistID string + removeIds []string + removeErr error +} + +func (f *fakePlaylists) Create(_ context.Context, _ string, name string, ids []string) (string, error) { + f.createdName = name + f.createdIds = ids + if f.createErr != nil { + return "", f.createErr + } + return "pl-new", nil +} + +func (f *fakePlaylists) GetWithTracks(_ context.Context, _ string) (*model.Playlist, error) { + if f.getErr != nil { + return nil, f.getErr + } + return f.getPls, nil +} + +func (f *fakePlaylists) AddTracks(_ context.Context, playlistID string, ids []string) (int, error) { + f.addPlaylistID = playlistID + f.addIds = ids + return len(ids), f.addErr +} + +func (f *fakePlaylists) RemoveTracks(_ context.Context, playlistID string, trackIds []string) error { + f.removePlaylistID = playlistID + f.removeIds = trackIds + return f.removeErr +} + +var _ = Describe("Playlists", func() { + var api *Router + var fp *fakePlaylists + + BeforeEach(func() { + fp = &fakePlaylists{} + api = &Router{playlists: fp} + }) + + Describe("createPlaylist", func() { + It("creates a playlist and returns its id", func() { + w := httptest.NewRecorder() + r := httptest.NewRequest("POST", "/Playlists", strings.NewReader(`{"Name":"Mix","Ids":["s1","s2"]}`)). + WithContext(context.Background()) + api.createPlaylist(w, r) + Expect(w.Code).To(Equal(http.StatusOK)) + var res map[string]string + Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed()) + Expect(res["Id"]).To(Equal("pl-new")) + Expect(fp.createdName).To(Equal("Mix")) + Expect(fp.createdIds).To(Equal([]string{"s1", "s2"})) + }) + + It("returns 400 on an invalid JSON body", func() { + w := httptest.NewRecorder() + r := httptest.NewRequest("POST", "/Playlists", strings.NewReader(`not json`)). + WithContext(context.Background()) + api.createPlaylist(w, r) + Expect(w.Code).To(Equal(http.StatusBadRequest)) + }) + + It("returns 500 when the service fails", func() { + fp.createErr = errors.New("boom") + w := httptest.NewRecorder() + r := httptest.NewRequest("POST", "/Playlists", strings.NewReader(`{"Name":"Mix"}`)). + WithContext(context.Background()) + api.createPlaylist(w, r) + Expect(w.Code).To(Equal(http.StatusInternalServerError)) + }) + }) + + Describe("getPlaylistItems", func() { + It("maps playlist tracks to Audio BaseItemDtos, tagging each with its PlaylistItemId", func() { + fp.getPls = &model.Playlist{ + ID: "pl1", + Tracks: model.PlaylistTracks{ + {ID: "1", MediaFileID: "s1", PlaylistID: "pl1", MediaFile: model.MediaFile{ID: "s1", Title: "Song One"}}, + {ID: "2", MediaFileID: "s2", PlaylistID: "pl1", MediaFile: model.MediaFile{ID: "s2", Title: "Song Two"}}, + }, + } + w := httptest.NewRecorder() + r := httptest.NewRequest("GET", "/Playlists/pl1/Items", nil).WithContext(context.Background()) + r = withChiURLParam(r, "playlistId", "pl1") + api.getPlaylistItems(w, r) + Expect(w.Code).To(Equal(http.StatusOK)) + var res dto.QueryResult + Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed()) + Expect(res.TotalRecordCount).To(Equal(2)) + Expect(res.Items).To(HaveLen(2)) + Expect(res.Items[0].Id).To(Equal("s1")) + Expect(res.Items[0].Type).To(Equal("Audio")) + Expect(res.Items[0].PlaylistItemId).To(Equal("1")) + Expect(res.Items[1].Id).To(Equal("s2")) + Expect(res.Items[1].PlaylistItemId).To(Equal("2")) + }) + + It("returns 404 for a non-owned or absent playlist", func() { + fp.getErr = model.ErrNotFound + w := httptest.NewRecorder() + r := httptest.NewRequest("GET", "/Playlists/missing/Items", nil).WithContext(context.Background()) + r = withChiURLParam(r, "playlistId", "missing") + api.getPlaylistItems(w, r) + Expect(w.Code).To(Equal(http.StatusNotFound)) + }) + }) + + Describe("addToPlaylist", func() { + It("adds tracks by song id and returns 204", func() { + w := httptest.NewRecorder() + r := httptest.NewRequest("POST", "/Playlists/pl1/Items?Ids=s1,s2", nil).WithContext(context.Background()) + r = withChiURLParam(r, "playlistId", "pl1") + api.addToPlaylist(w, r) + Expect(w.Code).To(Equal(http.StatusNoContent)) + Expect(fp.addPlaylistID).To(Equal("pl1")) + Expect(fp.addIds).To(Equal([]string{"s1", "s2"})) + }) + + It("returns 404 when the service rejects the request (not found/not owned)", func() { + fp.addErr = model.ErrNotAuthorized + w := httptest.NewRecorder() + r := httptest.NewRequest("POST", "/Playlists/pl1/Items?Ids=s1", nil).WithContext(context.Background()) + r = withChiURLParam(r, "playlistId", "pl1") + api.addToPlaylist(w, r) + Expect(w.Code).To(Equal(http.StatusNotFound)) + }) + }) + + Describe("removeFromPlaylist", func() { + It("removes entries by EntryIds (playlist-track position ids, not song ids) and returns 204", func() { + w := httptest.NewRecorder() + r := httptest.NewRequest("DELETE", "/Playlists/pl1/Items?EntryIds=1,2", nil).WithContext(context.Background()) + r = withChiURLParam(r, "playlistId", "pl1") + api.removeFromPlaylist(w, r) + Expect(w.Code).To(Equal(http.StatusNoContent)) + Expect(fp.removePlaylistID).To(Equal("pl1")) + Expect(fp.removeIds).To(Equal([]string{"1", "2"})) + }) + + It("returns 404 when the service rejects the request (not found/not owned)", func() { + fp.removeErr = model.ErrNotFound + w := httptest.NewRecorder() + r := httptest.NewRequest("DELETE", "/Playlists/pl1/Items?EntryIds=1", nil).WithContext(context.Background()) + r = withChiURLParam(r, "playlistId", "pl1") + api.removeFromPlaylist(w, r) + Expect(w.Code).To(Equal(http.StatusNotFound)) + }) + }) +})