mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-31 15:37:51 +01:00
fix(jellyfin): expand album/artist/playlist ids when building playlists
Jellyfin clients (Finamp) send container ids — an album, artist or playlist — in a playlist's Ids list and expect the server to expand each into its child tracks. core/playlists only understands media file ids, so creating or adding with an album id silently produced an empty playlist. Expand container ids to their tracks (in order) before create/add; bare song ids still pass through. Adds filter.SongsByArtistID.
This commit is contained in:
parent
d24ba9b5ea
commit
00b9e26d36
3 changed files with 109 additions and 5 deletions
|
|
@ -90,6 +90,18 @@ func SongsByAlbum(albumId string) Options {
|
|||
})
|
||||
}
|
||||
|
||||
// SongsByArtistID matches media files where the artist participates as either album artist or track
|
||||
// artist, in album order. Mirrors AlbumsByArtistID but at the media_file level.
|
||||
func SongsByArtistID(artistId string) Options {
|
||||
return addDefaultFilters(Options{
|
||||
Sort: "album",
|
||||
Filters: Or{
|
||||
persistence.Exists("json_tree(participants, '$.albumartist')", Eq{"value": artistId}),
|
||||
persistence.Exists("json_tree(participants, '$.artist')", Eq{"value": artistId}),
|
||||
},
|
||||
})
|
||||
}
|
||||
|
||||
func SongsByGenreAndYearRange(genre string, fromYear, toYear int) Options {
|
||||
options := Options{}
|
||||
ff := And{}
|
||||
|
|
|
|||
|
|
@ -1,13 +1,16 @@
|
|||
package jellyfin
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"strings"
|
||||
|
||||
"github.com/go-chi/chi/v5"
|
||||
"github.com/navidrome/navidrome/log"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/model/request"
|
||||
"github.com/navidrome/navidrome/server/filter"
|
||||
"github.com/navidrome/navidrome/server/jellyfin/dto"
|
||||
"github.com/navidrome/navidrome/utils/slice"
|
||||
)
|
||||
|
|
@ -26,7 +29,7 @@ func (api *Router) createPlaylist(w http.ResponseWriter, r *http.Request) {
|
|||
http.Error(w, "Bad Request", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
ids := slice.Map(body.Ids, dto.DecodeID)
|
||||
ids := api.expandContainerIDs(r.Context(), slice.Map(body.Ids, dto.DecodeID))
|
||||
id, err := api.playlists.Create(r.Context(), "", body.Name, ids)
|
||||
if err != nil {
|
||||
api.internalError(w, r, err)
|
||||
|
|
@ -99,12 +102,45 @@ func queryParam(r *http.Request, lower, pascal string) string {
|
|||
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.
|
||||
// expandContainerIDs turns the ids a Jellyfin client sends when building a playlist into the
|
||||
// underlying media file ids. Clients populate the id list with containers — albums, artists,
|
||||
// playlists — not just songs, and expect the server to expand each into its tracks, in order.
|
||||
// core/playlists only understands media file ids, so an unexpanded album id would silently add
|
||||
// nothing. A bare song id (or any id matching no container) passes through unchanged.
|
||||
func (api *Router) expandContainerIDs(ctx context.Context, ids []string) []string {
|
||||
out := make([]string, 0, len(ids))
|
||||
for _, id := range ids {
|
||||
if _, err := api.ds.MediaFile(ctx).Get(id); err == nil {
|
||||
out = append(out, id) // already a song
|
||||
} else if _, err := api.ds.Album(ctx).Get(id); err == nil {
|
||||
out = append(out, api.songIDs(ctx, filter.SongsByAlbum(id))...)
|
||||
} else if _, err := api.ds.Artist(ctx).Get(id); err == nil {
|
||||
out = append(out, api.songIDs(ctx, filter.SongsByArtistID(id))...)
|
||||
} else if pl, err := api.playlists.GetWithTracks(ctx, id); err == nil {
|
||||
out = append(out, slice.Map(pl.Tracks, func(t model.PlaylistTrack) string { return t.MediaFileID })...)
|
||||
} else {
|
||||
out = append(out, id) // unknown id — pass through unchanged
|
||||
}
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
func (api *Router) songIDs(ctx context.Context, opts model.QueryOptions) []string {
|
||||
mfs, err := api.ds.MediaFile(ctx).GetAll(opts)
|
||||
if err != nil {
|
||||
log.Error(ctx, "Jellyfin: error expanding container to tracks", err)
|
||||
return nil
|
||||
}
|
||||
return slice.Map(mfs, func(mf model.MediaFile) string { return mf.ID })
|
||||
}
|
||||
|
||||
// addToPlaylist appends items by id, expanding album/artist/playlist containers into their tracks
|
||||
// (see expandContainerIDs). 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 := dto.DecodeID(chi.URLParam(r, "playlistId"))
|
||||
ids := slice.Map(splitIds(queryParam(r, "ids", "Ids")), dto.DecodeID)
|
||||
ids := api.expandContainerIDs(ctx, slice.Map(splitIds(queryParam(r, "ids", "Ids")), dto.DecodeID))
|
||||
if _, err := api.playlists.AddTracks(ctx, id, ids); err != nil {
|
||||
http.Error(w, "Not Found", http.StatusNotFound)
|
||||
return
|
||||
|
|
|
|||
|
|
@ -13,7 +13,9 @@ import (
|
|||
"github.com/navidrome/navidrome/core/playlists"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/model/request"
|
||||
"github.com/navidrome/navidrome/server/filter"
|
||||
"github.com/navidrome/navidrome/server/jellyfin/dto"
|
||||
"github.com/navidrome/navidrome/tests"
|
||||
. "github.com/onsi/ginkgo/v2"
|
||||
. "github.com/onsi/gomega"
|
||||
)
|
||||
|
|
@ -76,6 +78,9 @@ func (f *fakePlaylists) GetWithTracks(_ context.Context, _ string) (*model.Playl
|
|||
if f.getErr != nil {
|
||||
return nil, f.getErr
|
||||
}
|
||||
if f.getPls == nil {
|
||||
return nil, model.ErrNotFound // mirror the real repo: never (nil, nil)
|
||||
}
|
||||
return f.getPls, nil
|
||||
}
|
||||
|
||||
|
|
@ -111,7 +116,7 @@ var _ = Describe("Playlists", func() {
|
|||
|
||||
BeforeEach(func() {
|
||||
fp = &fakePlaylists{}
|
||||
api = &Router{playlists: fp}
|
||||
api = &Router{ds: &tests.MockDataStore{}, playlists: fp}
|
||||
})
|
||||
|
||||
Describe("createPlaylist", func() {
|
||||
|
|
@ -181,6 +186,57 @@ var _ = Describe("Playlists", func() {
|
|||
})
|
||||
})
|
||||
|
||||
Describe("container id expansion", func() {
|
||||
var ds *tests.MockDataStore
|
||||
var ctx context.Context
|
||||
|
||||
BeforeEach(func() {
|
||||
ctx = context.Background()
|
||||
ds = &tests.MockDataStore{}
|
||||
api = &Router{ds: ds, playlists: fp}
|
||||
})
|
||||
|
||||
createWith := func(id string) {
|
||||
w := httptest.NewRecorder()
|
||||
r := httptest.NewRequest("POST", "/Playlists", strings.NewReader(`{"Name":"Mix","Ids":["`+id+`"]}`)).
|
||||
WithContext(ctx)
|
||||
api.createPlaylist(w, r)
|
||||
Expect(w.Code).To(Equal(http.StatusOK))
|
||||
}
|
||||
|
||||
It("passes a bare song id through unchanged", func() {
|
||||
ds.MediaFile(ctx).(*tests.MockMediaFileRepo).SetData(model.MediaFiles{{ID: "s1"}})
|
||||
createWith("s1")
|
||||
Expect(fp.createdIds).To(Equal([]string{"s1"}))
|
||||
})
|
||||
|
||||
It("expands an album id into its songs, filtered by album", func() {
|
||||
ds.Album(ctx).(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "al1"}})
|
||||
ds.MediaFile(ctx).(*tests.MockMediaFileRepo).SetData(model.MediaFiles{
|
||||
{ID: "s1", AlbumID: "al1"}, {ID: "s2", AlbumID: "al1"},
|
||||
})
|
||||
createWith("al1")
|
||||
Expect(fp.createdIds).To(Equal([]string{"s1", "s2"}))
|
||||
Expect(ds.MediaFile(ctx).(*tests.MockMediaFileRepo).Options.Filters).To(Equal(filter.SongsByAlbum("al1").Filters))
|
||||
})
|
||||
|
||||
It("expands an artist id into its songs", func() {
|
||||
ds.Artist(ctx).(*tests.MockArtistRepo).SetData(model.Artists{{ID: "ar1"}})
|
||||
ds.MediaFile(ctx).(*tests.MockMediaFileRepo).SetData(model.MediaFiles{{ID: "s1"}, {ID: "s2"}})
|
||||
createWith("ar1")
|
||||
Expect(fp.createdIds).To(Equal([]string{"s1", "s2"}))
|
||||
Expect(ds.MediaFile(ctx).(*tests.MockMediaFileRepo).Options.Filters).To(Equal(filter.SongsByArtistID("ar1").Filters))
|
||||
})
|
||||
|
||||
It("expands a playlist id into its tracks' media file ids", func() {
|
||||
fp.getPls = &model.Playlist{ID: "pl9", Tracks: model.PlaylistTracks{
|
||||
{ID: "1", MediaFileID: "s3"}, {ID: "2", MediaFileID: "s4"},
|
||||
}}
|
||||
createWith("pl9")
|
||||
Expect(fp.createdIds).To(Equal([]string{"s3", "s4"}))
|
||||
})
|
||||
})
|
||||
|
||||
Describe("getPlaylist", func() {
|
||||
It("returns OpenAccess from Public and item ids (encoded media file ids, not entry ids)", func() {
|
||||
fp.getPls = &model.Playlist{
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue