mirror of
https://github.com/navidrome/navidrome.git
synced 2026-11-06 18:28:39 +01:00
fix(jellyfin): filter items by AlbumArtistIds/ArtistIds
An artist's page in Finamp sends ParentId=<libraryId> (scoping) plus AlbumArtistIds/ArtistIds/contributingArtistIds for the artist itself, but queryItems only honored ParentId, so an artist's albums and tracks came back unfiltered (every artist's content). Parse the artist-id params and apply AlbumsByArtistID (albums) / SongsByArtistID (tracks). Adds e2e coverage.
This commit is contained in:
parent
90e28148db
commit
c3f1e29e71
3 changed files with 80 additions and 12 deletions
|
|
@ -64,6 +64,14 @@ Browsing (`/Items`), artists, and the "Latest" list are all scoped to the librar
|
|||
authenticated user has access to; a library (or item within it) the user cannot access returns
|
||||
`404`, never `403`, so ids can't be used as an existence oracle.
|
||||
|
||||
### Browsing filters
|
||||
|
||||
`GET /Items` accepts the filter params clients use to build screens: `ParentId` (a library view id
|
||||
for scoping, an artist id when browsing into an artist's albums, or an album id when browsing into
|
||||
an album's tracks); `AlbumArtistIds`/`ArtistIds`/`contributingArtistIds` (an artist's albums or
|
||||
tracks — Finamp's artist screen sends these *alongside* `ParentId=<libraryId>`); `SearchTerm`;
|
||||
`Filters=IsFavorite`; `SortBy`/`SortOrder`; `StartIndex`/`Limit`; and `Ids` (batch fetch by id).
|
||||
|
||||
## Implemented endpoints
|
||||
|
||||
| Area | Endpoints |
|
||||
|
|
|
|||
|
|
@ -85,6 +85,32 @@ var _ = Describe("Browsing", func() {
|
|||
})
|
||||
})
|
||||
|
||||
// Finamp's artist screen sends ParentId=<libraryId> (scoping) plus AlbumArtistIds/ArtistIds
|
||||
// for the actual artist filter, not ParentId=<artistId>.
|
||||
Describe("artist filtering (AlbumArtistIds / ArtistIds)", func() {
|
||||
lib1 := enc("1")
|
||||
|
||||
It("filters albums by AlbumArtistIds", func() {
|
||||
q := queryResult(get("/Items?IncludeItemTypes=MusicAlbum&Recursive=true&ParentId=" + lib1 + "&AlbumArtistIds=" + enc(artistID("The Beatles"))))
|
||||
Expect(names(q.Items)).To(ConsistOf("Abbey Road", "Help!"))
|
||||
})
|
||||
|
||||
It("filters songs by ArtistIds", func() {
|
||||
q := queryResult(get("/Items?IncludeItemTypes=Audio&Recursive=true&ParentId=" + lib1 + "&ArtistIds=" + enc(artistID("The Beatles"))))
|
||||
Expect(names(q.Items)).To(ConsistOf("Come Together", "Something", "Help!"))
|
||||
})
|
||||
|
||||
It("filters albums by a single-album artist", func() {
|
||||
q := queryResult(get("/Items?IncludeItemTypes=MusicAlbum&Recursive=true&AlbumArtistIds=" + enc(artistID("Led Zeppelin"))))
|
||||
Expect(names(q.Items)).To(ConsistOf("IV"))
|
||||
})
|
||||
|
||||
It("filters songs by a single-track artist", func() {
|
||||
q := queryResult(get("/Items?IncludeItemTypes=Audio&Recursive=true&ArtistIds=" + enc(artistID("Miles Davis"))))
|
||||
Expect(names(q.Items)).To(ConsistOf("So What"))
|
||||
})
|
||||
})
|
||||
|
||||
Describe("search, batch and pagination", func() {
|
||||
It("searches albums by term", func() {
|
||||
q := queryResult(get("/Items?IncludeItemTypes=MusicAlbum&Recursive=true&SearchTerm=Abbey"))
|
||||
|
|
|
|||
|
|
@ -57,6 +57,14 @@ func (api *Router) queryItems(ctx context.Context, r *http.Request) (dto.QueryRe
|
|||
offset := p.IntOr("StartIndex", 0)
|
||||
limit := p.IntOr("Limit", 0)
|
||||
types := parseTypes(p.StringOr("IncludeItemTypes", ""))
|
||||
// An artist's page filters by artist, not by ParentId: Finamp sends ParentId=<libraryId> for
|
||||
// scoping plus AlbumArtistIds/ArtistIds/contributingArtistIds for the artist itself. Without
|
||||
// this, an artist's albums/tracks come back unfiltered (all artists).
|
||||
artistId := firstDecodedID(firstNonEmpty(
|
||||
p.StringOr("AlbumArtistIds", ""),
|
||||
p.StringOr("ArtistIds", ""),
|
||||
p.StringOr("ContributingArtistIds", p.StringOr("contributingArtistIds", "")),
|
||||
))
|
||||
|
||||
scopeIDs, isLibraryParent := resolveLibraryScope(ctx, parentId)
|
||||
entityParent := parentId
|
||||
|
|
@ -70,7 +78,7 @@ func (api *Router) queryItems(ctx context.Context, r *http.Request) (dto.QueryRe
|
|||
if len(types) == 1 {
|
||||
opts := model.QueryOptions{Offset: offset, Max: limit}
|
||||
applySort(&opts, types[0], sortBy, sortOrder)
|
||||
return api.queryItemsOfType(ctx, types[0], opts, entityParent, scopeIDs, search, favOnly)
|
||||
return api.queryItemsOfType(ctx, types[0], opts, entityParent, artistId, scopeIDs, search, favOnly)
|
||||
}
|
||||
|
||||
var items []dto.BaseItemDto
|
||||
|
|
@ -78,7 +86,7 @@ func (api *Router) queryItems(ctx context.Context, r *http.Request) (dto.QueryRe
|
|||
for _, itemType := range types {
|
||||
var opts model.QueryOptions
|
||||
applySort(&opts, itemType, sortBy, sortOrder)
|
||||
res, err := api.queryItemsOfType(ctx, itemType, opts, entityParent, scopeIDs, search, favOnly)
|
||||
res, err := api.queryItemsOfType(ctx, itemType, opts, entityParent, artistId, scopeIDs, search, favOnly)
|
||||
if err != nil {
|
||||
return dto.QueryResult{}, err
|
||||
}
|
||||
|
|
@ -88,10 +96,10 @@ func (api *Router) queryItems(ctx context.Context, r *http.Request) (dto.QueryRe
|
|||
return result(paginate(items, offset, limit), total, offset), nil
|
||||
}
|
||||
|
||||
func (api *Router) queryItemsOfType(ctx context.Context, itemType string, opts model.QueryOptions, entityParent string, scopeIDs []int, search string, favOnly bool) (dto.QueryResult, error) {
|
||||
func (api *Router) queryItemsOfType(ctx context.Context, itemType string, opts model.QueryOptions, entityParent, artistId string, scopeIDs []int, search string, favOnly bool) (dto.QueryResult, error) {
|
||||
switch itemType {
|
||||
case "Audio":
|
||||
return api.listSongs(ctx, opts, entityParent, scopeIDs, search, favOnly)
|
||||
return api.listSongs(ctx, opts, entityParent, artistId, scopeIDs, search, favOnly)
|
||||
case "MusicArtist":
|
||||
return api.listArtists(ctx, opts, scopeIDs, search, favOnly)
|
||||
case "MusicGenre":
|
||||
|
|
@ -99,10 +107,29 @@ func (api *Router) queryItemsOfType(ctx context.Context, itemType string, opts m
|
|||
case "Playlist":
|
||||
return api.listPlaylists(ctx, opts, favOnly)
|
||||
default: // MusicAlbum
|
||||
return api.listAlbums(ctx, opts, entityParent, scopeIDs, search, favOnly)
|
||||
return api.listAlbums(ctx, opts, entityParent, artistId, scopeIDs, search, favOnly)
|
||||
}
|
||||
}
|
||||
|
||||
// firstNonEmpty returns the first non-empty string, or "".
|
||||
func firstNonEmpty(vals ...string) string {
|
||||
for _, v := range vals {
|
||||
if v != "" {
|
||||
return v
|
||||
}
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
// firstDecodedID decodes the first id from a (possibly comma-separated) Jellyfin id list.
|
||||
func firstDecodedID(s string) string {
|
||||
if s == "" {
|
||||
return ""
|
||||
}
|
||||
first, _, _ := strings.Cut(s, ",")
|
||||
return dto.DecodeID(strings.TrimSpace(first))
|
||||
}
|
||||
|
||||
// parseTypes returns every recognized entry in the (possibly comma-separated) IncludeItemTypes,
|
||||
// in the order they appear, defaulting to []string{"MusicAlbum"} when nothing is recognized.
|
||||
// The MusicAlbum default makes ParentId=<artistId> (no explicit type) browse into that artist's
|
||||
|
|
@ -138,11 +165,13 @@ func paginate(items []dto.BaseItemDto, offset, limit int) []dto.BaseItemDto {
|
|||
return items
|
||||
}
|
||||
|
||||
func (api *Router) listAlbums(ctx context.Context, opts model.QueryOptions, parentId string, scopeIDs []int, search string, fav bool) (dto.QueryResult, error) {
|
||||
func (api *Router) listAlbums(ctx context.Context, opts model.QueryOptions, parentId, artistId string, scopeIDs []int, search string, fav bool) (dto.QueryResult, error) {
|
||||
repo := api.ds.Album(ctx)
|
||||
filters := squirrel.And{}
|
||||
if parentId != "" {
|
||||
filters = append(filters, filter.AlbumsByArtistID(parentId).Filters)
|
||||
// For albums, both ParentId (browse into an artist) and AlbumArtistIds/ArtistIds mean "this
|
||||
// artist's albums".
|
||||
if scope := firstNonEmpty(artistId, parentId); scope != "" {
|
||||
filters = append(filters, filter.AlbumsByArtistID(scope).Filters)
|
||||
} else {
|
||||
filters = append(filters, notMissing)
|
||||
}
|
||||
|
|
@ -166,12 +195,17 @@ func (api *Router) listAlbums(ctx context.Context, opts model.QueryOptions, pare
|
|||
return result(slice.Map(albums, dto.AlbumToBaseItem), int(total), opts.Offset), nil
|
||||
}
|
||||
|
||||
func (api *Router) listSongs(ctx context.Context, opts model.QueryOptions, parentId string, scopeIDs []int, search string, fav bool) (dto.QueryResult, error) {
|
||||
func (api *Router) listSongs(ctx context.Context, opts model.QueryOptions, parentId, artistId string, scopeIDs []int, search string, fav bool) (dto.QueryResult, error) {
|
||||
repo := api.ds.MediaFile(ctx)
|
||||
filters := squirrel.And{}
|
||||
if parentId != "" {
|
||||
// For songs, ArtistIds/AlbumArtistIds selects an artist's tracks, while ParentId selects an
|
||||
// album's tracks — different filters.
|
||||
switch {
|
||||
case artistId != "":
|
||||
filters = append(filters, filter.SongsByArtistID(artistId).Filters)
|
||||
case parentId != "":
|
||||
filters = append(filters, filter.SongsByAlbum(parentId).Filters)
|
||||
} else {
|
||||
default:
|
||||
filters = append(filters, notMissing)
|
||||
}
|
||||
if fav {
|
||||
|
|
@ -188,7 +222,7 @@ func (api *Router) listSongs(ctx context.Context, opts model.QueryOptions, paren
|
|||
// When browsing an album's tracks, default to track order (disc + track number) like
|
||||
// Subsonic's GetAlbum and real Jellyfin do; an explicit SortBy from the client still wins,
|
||||
// since applySort would already have set opts.Sort.
|
||||
if parentId != "" && opts.Sort == "" {
|
||||
if artistId == "" && parentId != "" && opts.Sort == "" {
|
||||
opts.Sort = filter.SongsByAlbum(parentId).Sort
|
||||
}
|
||||
mfs, err = repo.GetAll(opts)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue