mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-28 06:07:17 +01:00
fix(jellyfin): read lowercase ids/entryIds playlist params and add playlist-users endpoints
This commit is contained in:
parent
17c1b44e60
commit
da451cf927
4 changed files with 93 additions and 11 deletions
|
|
@ -83,6 +83,8 @@ func (api *Router) routes() http.Handler {
|
|||
r.Get("/Playlists/{playlistId}/Items", api.getPlaylistItems)
|
||||
r.Post("/Playlists/{playlistId}/Items", api.addToPlaylist)
|
||||
r.Delete("/Playlists/{playlistId}/Items", api.removeFromPlaylist)
|
||||
r.Get("/Playlists/{playlistId}/Users", api.getPlaylistUsers)
|
||||
r.Get("/Playlists/{playlistId}/Users/{userId}", api.getPlaylistUser)
|
||||
|
||||
r.Get("/Audio/{itemId}/stream", api.streamAudio)
|
||||
r.Get("/Audio/{itemId}/stream.{container}", api.streamAudio)
|
||||
|
|
|
|||
|
|
@ -72,6 +72,13 @@ type BaseItemDto struct {
|
|||
CanDownload bool `json:"CanDownload"`
|
||||
}
|
||||
|
||||
// PlaylistUserPermissions is the response shape for GET /Playlists/{id}/Users(/{userId}), which
|
||||
// Finamp probes before allowing playlist edits.
|
||||
type PlaylistUserPermissions struct {
|
||||
UserId string `json:"UserId"`
|
||||
CanEdit bool `json:"CanEdit"`
|
||||
}
|
||||
|
||||
type QueryResult struct {
|
||||
Items []BaseItemDto `json:"Items"`
|
||||
TotalRecordCount int `json:"TotalRecordCount"`
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ import (
|
|||
|
||||
"github.com/go-chi/chi/v5"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/model/request"
|
||||
"github.com/navidrome/navidrome/server/jellyfin/dto"
|
||||
"github.com/navidrome/navidrome/utils/slice"
|
||||
)
|
||||
|
|
@ -69,12 +70,21 @@ func splitIds(s string) []string {
|
|||
return strings.Split(s, ",")
|
||||
}
|
||||
|
||||
// queryParam reads lower first, falling back to pascal. Real Jellyfin clients (e.g. Finamp) send
|
||||
// lowercase query params (ids, entryIds); PascalCase is also accepted for robustness.
|
||||
func queryParam(r *http.Request, lower, pascal string) string {
|
||||
if v := r.URL.Query().Get(lower); v != "" {
|
||||
return v
|
||||
}
|
||||
return r.URL.Query().Get(pascal)
|
||||
}
|
||||
|
||||
// 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"))
|
||||
ids := splitIds(queryParam(r, "ids", "Ids"))
|
||||
if _, err := api.playlists.AddTracks(ctx, id, ids); err != nil {
|
||||
http.Error(w, "Not Found", http.StatusNotFound)
|
||||
return
|
||||
|
|
@ -82,7 +92,7 @@ func (api *Router) addToPlaylist(w http.ResponseWriter, r *http.Request) {
|
|||
w.WriteHeader(http.StatusNoContent)
|
||||
}
|
||||
|
||||
// removeFromPlaylist removes entries by EntryIds. Unlike addToPlaylist's Ids, these must be
|
||||
// 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;
|
||||
|
|
@ -90,10 +100,23 @@ func (api *Router) addToPlaylist(w http.ResponseWriter, r *http.Request) {
|
|||
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"))
|
||||
ids := splitIds(queryParam(r, "entryIds", "EntryIds"))
|
||||
if err := api.playlists.RemoveTracks(ctx, id, ids); err != nil {
|
||||
http.Error(w, "Not Found", http.StatusNotFound)
|
||||
return
|
||||
}
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
}
|
||||
|
||||
// getPlaylistUsers and getPlaylistUser are best-effort probes some clients (e.g. Finamp) make
|
||||
// before allowing playlist edits. Navidrome has no per-playlist ACL model, so every user is
|
||||
// reported as able to edit; ownership is still enforced by AddTracks/RemoveTracks themselves.
|
||||
func (api *Router) getPlaylistUsers(w http.ResponseWriter, r *http.Request) {
|
||||
u, _ := request.UserFrom(r.Context())
|
||||
api.ok(w, r, []dto.PlaylistUserPermissions{{UserId: u.ID, CanEdit: true}})
|
||||
}
|
||||
|
||||
func (api *Router) getPlaylistUser(w http.ResponseWriter, r *http.Request) {
|
||||
userId := chi.URLParam(r, "userId")
|
||||
api.ok(w, r, dto.PlaylistUserPermissions{UserId: userId, CanEdit: true})
|
||||
}
|
||||
|
|
|
|||
|
|
@ -8,8 +8,10 @@ import (
|
|||
"net/http/httptest"
|
||||
"strings"
|
||||
|
||||
"github.com/go-chi/chi/v5"
|
||||
"github.com/navidrome/navidrome/core/playlists"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/model/request"
|
||||
"github.com/navidrome/navidrome/server/jellyfin/dto"
|
||||
. "github.com/onsi/ginkgo/v2"
|
||||
. "github.com/onsi/gomega"
|
||||
|
|
@ -157,9 +159,9 @@ var _ = Describe("Playlists", func() {
|
|||
})
|
||||
|
||||
Describe("addToPlaylist", func() {
|
||||
It("adds tracks by song id and returns 204", func() {
|
||||
It("adds tracks by song id from the lowercase ids param real Jellyfin clients send", func() {
|
||||
w := httptest.NewRecorder()
|
||||
r := httptest.NewRequest("POST", "/Playlists/pl1/Items?Ids=s1,s2", nil).WithContext(context.Background())
|
||||
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))
|
||||
|
|
@ -167,16 +169,25 @@ var _ = Describe("Playlists", func() {
|
|||
Expect(fp.addIds).To(Equal([]string{"s1", "s2"}))
|
||||
})
|
||||
|
||||
It("falls back to the PascalCase Ids param", 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.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 := 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))
|
||||
})
|
||||
|
||||
It("passes no ids (not a spurious empty string) when the Ids param is absent", func() {
|
||||
It("passes no ids (not a spurious empty string) when the ids param is absent", func() {
|
||||
w := httptest.NewRecorder()
|
||||
r := httptest.NewRequest("POST", "/Playlists/pl1/Items", nil).WithContext(context.Background())
|
||||
r = withChiURLParam(r, "playlistId", "pl1")
|
||||
|
|
@ -188,9 +199,9 @@ var _ = Describe("Playlists", func() {
|
|||
})
|
||||
|
||||
Describe("removeFromPlaylist", func() {
|
||||
It("removes entries by EntryIds (playlist-track position ids, not song ids) and returns 204", func() {
|
||||
It("removes entries by the lowercase entryIds param real Jellyfin clients send (playlist-track position ids, not song ids)", func() {
|
||||
w := httptest.NewRecorder()
|
||||
r := httptest.NewRequest("DELETE", "/Playlists/pl1/Items?EntryIds=1,2", nil).WithContext(context.Background())
|
||||
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))
|
||||
|
|
@ -198,16 +209,25 @@ var _ = Describe("Playlists", func() {
|
|||
Expect(fp.removeIds).To(Equal([]string{"1", "2"}))
|
||||
})
|
||||
|
||||
It("falls back to the PascalCase EntryIds param", 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.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 := 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))
|
||||
})
|
||||
|
||||
It("passes no ids (not a spurious empty string) when the EntryIds param is absent", func() {
|
||||
It("passes no ids (not a spurious empty string) when the entryIds param is absent", func() {
|
||||
w := httptest.NewRecorder()
|
||||
r := httptest.NewRequest("DELETE", "/Playlists/pl1/Items", nil).WithContext(context.Background())
|
||||
r = withChiURLParam(r, "playlistId", "pl1")
|
||||
|
|
@ -217,4 +237,34 @@ var _ = Describe("Playlists", func() {
|
|||
Expect(fp.removeIds).To(BeEmpty())
|
||||
})
|
||||
})
|
||||
|
||||
Describe("getPlaylistUsers", func() {
|
||||
It("returns the current user with CanEdit true", func() {
|
||||
w := httptest.NewRecorder()
|
||||
ctx := request.WithUser(context.Background(), model.User{ID: "u1", UserName: "alice"})
|
||||
r := httptest.NewRequest("GET", "/Playlists/pl1/Users", nil).WithContext(ctx)
|
||||
r = withChiURLParam(r, "playlistId", "pl1")
|
||||
api.getPlaylistUsers(w, r)
|
||||
Expect(w.Code).To(Equal(http.StatusOK))
|
||||
var res []dto.PlaylistUserPermissions
|
||||
Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed())
|
||||
Expect(res).To(Equal([]dto.PlaylistUserPermissions{{UserId: "u1", CanEdit: true}}))
|
||||
})
|
||||
})
|
||||
|
||||
Describe("getPlaylistUser", func() {
|
||||
It("returns CanEdit true for the requested user", func() {
|
||||
w := httptest.NewRecorder()
|
||||
r := httptest.NewRequest("GET", "/Playlists/pl1/Users/u1", nil).WithContext(context.Background())
|
||||
rctx := chi.NewRouteContext()
|
||||
rctx.URLParams.Add("playlistId", "pl1")
|
||||
rctx.URLParams.Add("userId", "u1")
|
||||
r = r.WithContext(context.WithValue(r.Context(), chi.RouteCtxKey, rctx))
|
||||
api.getPlaylistUser(w, r)
|
||||
Expect(w.Code).To(Equal(http.StatusOK))
|
||||
var res dto.PlaylistUserPermissions
|
||||
Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed())
|
||||
Expect(res).To(Equal(dto.PlaylistUserPermissions{UserId: "u1", CanEdit: true}))
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue