mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-29 14:37:49 +01:00
fix(jellyfin): resolve playlist ids in getItem
This commit is contained in:
parent
0c4e6b2c70
commit
17c1b44e60
3 changed files with 53 additions and 4 deletions
|
|
@ -234,9 +234,9 @@ func (api *Router) listPlaylists(ctx context.Context, opts model.QueryOptions, f
|
|||
return result(slice.Map(playlists, dto.PlaylistToBaseItem), int(total), opts.Offset), nil
|
||||
}
|
||||
|
||||
// getItem fetches a single entity by id, trying library view, album, artist and song in turn.
|
||||
// For albums and songs (which each belong to exactly one library) it 404s if the current user
|
||||
// lacks access to that library, so an id can't be used to probe content outside the user's
|
||||
// getItem fetches a single entity by id, trying library view, album, artist, song and playlist in
|
||||
// turn. For albums and songs (which each belong to exactly one library) it 404s if the current
|
||||
// user lacks access to that library, so an id can't be used to probe content outside the user's
|
||||
// libraries.
|
||||
func (api *Router) getItem(w http.ResponseWriter, r *http.Request) {
|
||||
ctx := r.Context()
|
||||
|
|
@ -281,6 +281,12 @@ func (api *Router) getItem(w http.ResponseWriter, r *http.Request) {
|
|||
api.ok(w, r, dto.SongToBaseItem(*mf))
|
||||
return
|
||||
}
|
||||
// api.playlists.Get enforces ownership/visibility itself, so a non-owned or missing playlist
|
||||
// id falls through to the generic 404 below, same as every other probe here.
|
||||
if pl, err := api.playlists.Get(ctx, id); err == nil {
|
||||
api.ok(w, r, dto.PlaylistToBaseItem(*pl))
|
||||
return
|
||||
}
|
||||
http.Error(w, "Not Found", http.StatusNotFound)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -26,6 +26,7 @@ func withChiURLParam(r *http.Request, key, value string) *http.Request {
|
|||
var _ = Describe("Items", func() {
|
||||
var api *Router
|
||||
var ds *tests.MockDataStore
|
||||
var fp *fakePlaylists
|
||||
// alice has access to library 1 only; used by tests that don't care about scoping.
|
||||
ctxUser := func() context.Context {
|
||||
return request.WithUser(context.Background(), model.User{ID: "u1", UserName: "alice", Libraries: model.Libraries{{ID: 1, Name: "Music"}}})
|
||||
|
|
@ -39,7 +40,8 @@ var _ = Describe("Items", func() {
|
|||
}
|
||||
BeforeEach(func() {
|
||||
ds = &tests.MockDataStore{}
|
||||
api = &Router{ds: ds}
|
||||
fp = &fakePlaylists{}
|
||||
api = &Router{ds: ds, playlists: fp}
|
||||
})
|
||||
|
||||
Describe("getItems", func() {
|
||||
|
|
@ -428,6 +430,31 @@ var _ = Describe("Items", func() {
|
|||
Expect(w.Code).To(Equal(http.StatusNotFound))
|
||||
})
|
||||
|
||||
// Finamp's SyncBuffer fetches a playlist by id as a plain item; without this probe it
|
||||
// 404s with "Could not fetch BaseItemDto <playlist> from server."
|
||||
It("resolves a playlist id via the playlists service", func() {
|
||||
fp.getByIDPls = &model.Playlist{ID: "p1", Name: "My Mix", SongCount: 5}
|
||||
w := httptest.NewRecorder()
|
||||
r := httptest.NewRequest("GET", "/Items/p1", nil).WithContext(ctxUser())
|
||||
r = withChiURLParam(r, "itemId", "p1")
|
||||
api.getItem(w, r)
|
||||
Expect(w.Code).To(Equal(http.StatusOK))
|
||||
var item dto.BaseItemDto
|
||||
Expect(json.Unmarshal(w.Body.Bytes(), &item)).To(Succeed())
|
||||
Expect(item.Id).To(Equal("p1"))
|
||||
Expect(item.Name).To(Equal("My Mix"))
|
||||
Expect(item.Type).To(Equal("Playlist"))
|
||||
})
|
||||
|
||||
It("returns 404 for a non-owned or absent playlist id", func() {
|
||||
fp.getByIDErr = model.ErrNotFound
|
||||
w := httptest.NewRecorder()
|
||||
r := httptest.NewRequest("GET", "/Items/p1", nil).WithContext(ctxUser())
|
||||
r = withChiURLParam(r, "itemId", "p1")
|
||||
api.getItem(w, r)
|
||||
Expect(w.Code).To(Equal(http.StatusNotFound))
|
||||
})
|
||||
|
||||
It("resolves a library-view id for an admin even though their Libraries slice is empty", func() {
|
||||
ds.Library(context.Background()).(*tests.MockLibraryRepo).SetData(model.Libraries{{ID: 1, Name: "Music Library"}})
|
||||
w := httptest.NewRecorder()
|
||||
|
|
|
|||
|
|
@ -27,6 +27,9 @@ type fakePlaylists struct {
|
|||
getPls *model.Playlist
|
||||
getErr error
|
||||
|
||||
getByIDPls *model.Playlist
|
||||
getByIDErr error
|
||||
|
||||
addPlaylistID string
|
||||
addIds []string
|
||||
addErr error
|
||||
|
|
@ -45,6 +48,19 @@ func (f *fakePlaylists) Create(_ context.Context, _ string, name string, ids []s
|
|||
return "pl-new", nil
|
||||
}
|
||||
|
||||
// Get defaults to model.ErrNotFound when getByIDPls/getByIDErr aren't set, matching the real
|
||||
// service's behavior for a missing or inaccessible playlist and letting getItem tests that don't
|
||||
// care about playlists leave it unconfigured.
|
||||
func (f *fakePlaylists) Get(_ context.Context, _ string) (*model.Playlist, error) {
|
||||
if f.getByIDErr != nil {
|
||||
return nil, f.getByIDErr
|
||||
}
|
||||
if f.getByIDPls == nil {
|
||||
return nil, model.ErrNotFound
|
||||
}
|
||||
return f.getByIDPls, nil
|
||||
}
|
||||
|
||||
func (f *fakePlaylists) GetWithTracks(_ context.Context, _ string) (*model.Playlist, error) {
|
||||
if f.getErr != nil {
|
||||
return nil, f.getErr
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue