fix(jellyfin): honor the Filters, SortBy and MaxHeight params clients actually send (#5981)

* fix(jellyfin): honor Filters=IsFavorite on /Artists and /Artists/AlbumArtists

listArtistsByRole hand-built its itemsQuery and never set favOnly, so the
favorites filter was silently dropped on both artist routes while /Items
honored it. Finamp's home screen asks for favorite artists once per load and
was served the entire artist list instead: 10,298 artists, 6.15 MB, 2.7s on
a real library, and the wrong data on screen.

Extract the favOnly parsing that parseItemsQuery already did into
parseFavOnly and use it in both places. listArtists now adds the starred
predicate to notMissing rather than replacing it, matching listAlbums and
listSongs, so a favorite artist whose files are gone stays excluded.

* fix(jellyfin): map SortBy=Runtime to duration for albums and songs

sortColumnsByType had no runtime/runtimeticks key for any type, so Finamp's
"Duration" sort silently misbehaved in two different ways.

Albums: Finamp sends a bare SortBy=Runtime. Nothing matched, opts.Sort stayed
empty, and applyOptions skips OrderBy entirely when Sort is empty — so the
query ran with no ORDER BY at all and Ascending and Descending returned
identical lists.

Songs: Finamp sends SortBy=Runtime,AlbumArtist,Album,SortName. applySort takes
the first *recognized* key, so Runtime was skipped and the list came back
sorted by album artist while looking correct.

Both repos already accept a duration sort (mediafile_repository maps it
explicitly; album_repository falls through to the column name), so no
migration is needed. Sorting 97k songs by duration costs a temp B-tree
(~114ms on a prod-sized copy) — the same cost the Subsonic and UI duration
sorts already pay, and correct where the previous behaviour was merely fast.

* fix(jellyfin): apply the played/unplayed filters and MaxHeight image bound

Filters was matched with a substring test for IsFavorite, so every other token
Jellyfin defines was silently dropped and the response kept rows it should
have excluded. Finamp sends Filters=IsUnplayed in normal use.

Replace the bool with a parsed itemFilters carrying nullable favorite and
played flags, so isFavorite=false and isPlayed=false are real filters rather
than indistinguishable from an absent param. Standalone params are read first
and the Filters list overrides them, the precedence real Jellyfin has.
IsFavoriteOrLikes now maps to favorites deliberately instead of by substring
accident; Likes, Dislikes, IsFolder, IsNotFolder and IsResumable have no
Navidrome equivalent and are dropped rather than half-applied. The negative
cases match NULL as well, since annotations are LEFT JOINed and an untouched
item has no row.

getItemImage read only maxwidth, so a client sending just MaxHeight got the
full-size original: measured against a real cover, maxHeight=100 returned
82,570 bytes where maxWidth=100 returned 3,316. Use the tighter of the two
bounds.

* refactor(jellyfin): share the plain-param parser between /Items and /Artists

listArtistsByRole hand-listed the itemsQuery fields it happened to need, which
is exactly how the favorites filter went missing: the literal has been amended
in four of the five commits that touched it. Extract listParams for the fields
that come straight from query params so both paths read one parser, and the
next supported param reaches every list path instead of only /Items.

Also from the cleanup pass: collapse imageSize to a single clamped comparison
and read its bounds through req.Params like the rest of the package, which
drops the strconv import; build the artist and playlist filter lists with the
flat append shape the album and song paths already use, instead of re-wrapping
opts.Filters into a nested And per predicate; drop a nil guard in
listPlaylists that no caller can reach, since both paths into queryItemsOfType
build QueryOptions without Filters.

applySort now logs when no SortBy key resolves at all — a miss inside a
fallback list is normal, but none matching means a silently ignored sort, the
failure mode that hid the Runtime bug. Its doc comment records why the
remaining keys cannot simply be joined.

Folds three duplicated test bodies into the tables that already parameterize
them, and covers the artist-parent album branch, which reaches notMissing
through filter.AlbumsByArtistID rather than the default branch.

* docs(jellyfin): correct how applySort describes Jellyfin's SortBy semantics

The comment claimed SortBy is a comma-separated fallback list. It is not:
RequestHelpers.GetOrderBy (10.10) builds one (ItemSortBy, SortOrder) pair per
key, so Jellyfin orders by every key in turn. Navidrome applies only the first
recognized one, which is a real divergence — secondary keys never break ties —
not the intended reading of the parameter.

The assertion that the keys cannot be joined was also wrong. buildSortOrder
does split its input on commas; what it maps is the whole string, so joining
raw Jellyfin key names misses the mappings. Mapping each key first and joining
the results would work, which makes multi-key sorting a real option rather
than a blocked one. Documenting the current behaviour as a known divergence
until then.

* fix(jellyfin): order by every recognized SortBy key, not just the first

Jellyfin orders by each SortBy key in turn, so "DatePlayed,SortName" means
break ties by name. Navidrome applied only the first recognized key and dropped
the rest, which is 28% of the sort traffic on a real server (23 of 82 requests
in 12h carry 2-5 keys). Most were harmless because the primary key dominates,
but PremiereDate,Album,ParentIndexNumber,IndexNumber,SortName came back
unordered within a year.

The keys cannot simply be joined: sortMapping keyed on the whole Sort string,
so a joined value missed every mapping and fell through to raw column names.
Make it resolve a comma list per part, but only when every part is a known key
— the four existing callers that pass raw column lists (core/matcher,
core/lyrics, core/maintenance, subsonic/browsing) all carry a part that is not
a mapping key, several with their own direction, so they keep falling through
exactly as before. Verified each one.

applySort now collects every recognized key, skipping duplicates so
ParentIndexNumber,IndexNumber does not repeat a column. random stays alone: the
repo matches it by exact string equality, so joining it would both break that
path and emit a bare 'random' column into the ORDER BY.

Verified against a prod-sized copy: every multi-key combination seen in real
traffic returns 200, and a secondary key now changes the order within a tied
year for songs. Albums are unchanged there, because their max_year mapping
already ended in ", name".

* fix(persistence): resolve sort mappings exactly once

Making sortMapping resolve a comma list per part broke an invariant it had
been relying on: idempotence. sanitizeSort mapped the sort key up front and
applyOptions then ran buildSortOrder over the result, so sortMapping was
already being handed its own output. That was harmless only while a mapped
value could never look like a key list.

media_file's rated_at maps to "rating, rated_at", and both parts are keys, so
the second pass expanded it to "rating, rating, rated_at". Found by
round-tripping every mapping in all four repositories; it was the only
collision, and the duplicate sort key was benign in SQL, but any future mapping
of that shape would silently change meaning.

sanitizeSort now validates without resolving, leaving buildSortOrder as the
single mapping point. The generated SQL is unchanged — the whole suite passes
apart from the two specs that asserted the old return value, which are updated
and joined by a round-trip guard covering exactly the rated_at shape.

Also use the paren-aware splitFunc that buildSortOrder already uses, so an
expression carrying commas inside its parentheses cannot be split apart.

* refactor(jellyfin,persistence): flatten the sort resolution paths

Cleanup pass over the branch, no behavior change.

sortMapping loses the len(parts)>1 guard, which existed only to pick between
two identical toSnakeCase exits; the single-key case now falls through the same
loop. lookupSortMapping hands back the snake_case form it had to derive so the
fallback stops recomputing it — toSnakeCase is two regexps, and on a miss it was
running twice per call. sanitizeSort now asks lookupSortMapping instead of
probing the map itself, so "is this a known sort key" has one answer; the two
had already drifted, since sanitizeSort tried one casing where the resolver
tries three.

applySort folds the nested random branch into the skip condition and the two
trailing length tests into one switch. setSortMappings documents the invariant
the comma-list rule depends on, where someone adding a mapping will read it.

The README line describing SortBy still said only the first key applied, which
the commit before last made false.

Tests: the twelve near-identical sorting specs become one DescribeTable of
(itemType, SortBy, want) triples, 124 lines to 36, and the applyOptions
round-trip assertion collapses to the buildSortOrder call its sibling uses.

* fix(jellyfin): keep annotation filters out of search, resolve sorts per part

Two findings from the Codex review on #5981.

The played/unplayed filters turned working requests into 500s when combined
with SearchTerm. Search runs a two-phase FTS query whose first phase selects
rowids with no annotation join, so a starred or play_count predicate there is
"no such column", not a filter. Measured against master: MusicAlbum with
SearchTerm and Filters=IsUnplayed went 200 -> 500, likewise IsPlayed and the
Audio equivalents. listAlbums and listSongs now skip those predicates on the
search path, matching what listArtists already did. That also clears the same
500 master already had for Filters=IsFavorite with SearchTerm.

sortMapping resolved a comma list only while every part was a known key, so a
list mixing a plain column with a mapped key kept neither: MusicAlbum
SortBy=Runtime,SortName arrives as "duration, name", and duration is a plain
album column, so name stayed raw instead of expanding to order_album_name.
Albums whose name differs from its sort form — 1,366 of 6,987 on a real
library — then ordered by the wrong secondary key, and PreferSortTags was
ignored. Each part is now resolved on its own, which is what setSortMappings
already documents for a single field. Verified every in-tree caller that passes
a raw column list still produces its original ORDER BY.

Codex also asked for the artist search path to apply the same filters. It
would 500 for the reason above, and wrapping the library scope in a compound
filter makes requestedLibraryIDs stop recognizing it, silently widening the
search past the requested ParentId.

* fix(jellyfin): honor the first SortOrder value for a multi-key sort

applySort compared the whole SortOrder string with "Descending", so a per-key
list like SortOrder=Descending,Ascending failed the match and every key,
including the primary, sorted ascending — the exact opposite of the request.
Take the first comma-separated value, which Jellyfin also uses for any key past
the end of the SortOrder list. True per-key directions can't be expressed
through the single opts.Sort string and are left out; no observed client sends
a SortOrder list.
This commit is contained in:
Deluan Quintão 2026-08-19 08:36:44 -04:00 committed by GitHub
parent 2e03766a9d
commit 7a11ca69bb
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 394 additions and 148 deletions

View File

@ -97,6 +97,8 @@ func (r *sqlRepository) registerModel(instance any, filters map[string]filterFun
}
// setSortMappings sets the mappings for the sort fields. If the sort field is not in the map, it will be used as is.
// This applies per comma-separated part, so a key added here also defines that bare name wherever a
// caller uses it inside a sort list.
//
// If PreferSortTags is enabled, it will map the order fields to the corresponding sort expression,
// which gives precedence to sort tags.
@ -147,17 +149,37 @@ func (r sqlRepository) applyOptions(sq SelectBuilder, options ...model.QueryOpti
// TODO Change all sortMappings to have a consistent case
func (r sqlRepository) sortMapping(sort string) string {
if mapping, ok := r.sortMappings[sort]; ok {
if mapping, _, ok := r.lookupSortMapping(sort); ok {
return mapping
}
if mapping, ok := r.sortMappings[toCamelCase(sort)]; ok {
return mapping
// Each part of a comma list is resolved on its own, so a mix of mapped keys and plain columns
// keeps the mappings the recognized parts have.
parts := strings.FieldsFunc(sort, splitFunc(','))
mapped := make([]string, 0, len(parts))
for _, part := range parts {
part = strings.TrimSpace(part)
if partMapping, _, ok := r.lookupSortMapping(part); ok {
part = partMapping
} else {
part = toSnakeCase(part)
}
mapped = append(mapped, part)
}
sort = toSnakeCase(sort)
if mapping, ok := r.sortMappings[sort]; ok {
return mapping
return strings.Join(mapped, ", ")
}
// lookupSortMapping also returns the snake_case form when it had to derive one, so a caller's
// fallback doesn't recompute it: toSnakeCase runs two regexps.
func (r sqlRepository) lookupSortMapping(sort string) (mapping, snakeCased string, ok bool) {
if mapping, ok = r.sortMappings[sort]; ok {
return mapping, sort, true
}
return sort
if mapping, ok = r.sortMappings[toCamelCase(sort)]; ok {
return mapping, "", true
}
snakeCased = toSnakeCase(sort)
mapping, ok = r.sortMappings[snakeCased]
return mapping, snakeCased, ok
}
func (r sqlRepository) buildSortOrder(sort, order string) string {

View File

@ -92,14 +92,28 @@ var _ = Describe("sqlRepository", func() {
Expect(sort).To(BeEmpty())
})
It("returns the mapped value when sort key exists", func() {
// Validation only: buildSortOrder resolves the mapping, so mapping here too would hand
// sortMapping its own output and re-map values whose parts are themselves keys.
It("accepts a known sort key without resolving it", func() {
sort, _ := r.sanitizeSort("sort1", "")
Expect(sort).To(Equal("mappedSort1"))
Expect(sort).To(Equal("sort1"))
})
It("is case insensitive", func() {
sort, _ := r.sanitizeSort("Sort1", "")
Expect(sort).To(Equal("mappedSort1"))
Expect(sort).To(Equal("sort1"))
})
It("still resolves the mapping by the time the SQL is built", func() {
Expect(r.buildSortOrder("sort1", "asc")).To(Equal("mappedSort1 asc"))
})
// A mapping whose parts are themselves keys (media_file rated_at = "rating, rated_at")
// must survive the round trip through sanitizeSort and buildSortOrder unduplicated.
It("does not re-map a value whose parts are also keys", func() {
r.sortMappings = map[string]string{"rating": "rating", "rated_at": "rating, rated_at"}
sort, _ := r.sanitizeSort("rated_at", "")
Expect(r.buildSortOrder(sort, "asc")).To(Equal("rating asc, rated_at asc"))
})
It("returns the field if it is a valid field", func() {
@ -135,6 +149,45 @@ var _ = Describe("sqlRepository", func() {
})
})
Describe("sortMapping", func() {
BeforeEach(func() {
r.sortMappings = map[string]string{
"name": "order_album_name, order_album_artist_name",
"recently_added": "album.created_at, album.id",
}
})
It("maps a single key", func() {
Expect(r.sortMapping("recently_added")).To(Equal("album.created_at, album.id"))
})
It("maps every part of a comma list when all of them are known keys", func() {
Expect(r.sortMapping("recently_added, name")).
To(Equal("album.created_at, album.id, order_album_name, order_album_artist_name"))
})
It("resolves the known parts of a mixed list and leaves the rest as columns", func() {
Expect(r.sortMapping("recently_added, play_count")).
To(Equal("album.created_at, album.id, play_count"))
})
// Jellyfin's MusicAlbum SortBy=Runtime,SortName arrives as "duration, name"; duration is a
// plain album column while name is mapped, and the mapping must survive the mix.
It("keeps a mapping when an earlier part is a plain column", func() {
Expect(r.sortMapping("duration, name")).
To(Equal("duration, order_album_name, order_album_artist_name"))
})
It("leaves a raw column list with directions untouched", func() {
Expect(r.sortMapping("starred desc, rating desc")).To(Equal("starred desc, rating desc"))
})
It("does not split an expression on a comma inside its parentheses", func() {
Expect(r.sortMapping("coalesce(name, ''), title")).To(Equal("coalesce(name, ''), title"))
Expect(r.sortMapping("coalesce(nullif(a,''), b) desc, c")).To(Equal("coalesce(nullif(a,''), b) desc, c"))
})
It("keeps a mapping whose value nests commas inside parentheses", func() {
r.sortMappings["max_year"] = "coalesce(nullif(original_date,''), cast(max_year as text)), release_date"
Expect(r.sortMapping("max_year, name")).To(Equal(
"coalesce(nullif(original_date,''), cast(max_year as text)), release_date, " +
"order_album_name, order_album_artist_name"))
})
})
Describe("buildSortOrder", func() {
BeforeEach(func() {
r.sortMappings = map[string]string{}

View File

@ -69,13 +69,11 @@ func (r *sqlRepository) parseRestOptions(ctx context.Context, options ...rest.Qu
func (r sqlRepository) sanitizeSort(sort, order string) (string, string) {
if sort != "" {
sort = toSnakeCase(sort)
if mapped, ok := r.sortMappings[sort]; ok {
sort = mapped
} else {
if !r.isFieldWhiteListed(sort) {
log.Warn(r.ctx, "Ignoring sort not whitelisted", "sort", sort, "table", r.tableName)
sort = ""
}
// Validate only: buildSortOrder resolves the mapping later, and mapping here as well would
// feed sortMapping its own output.
if _, _, known := r.lookupSortMapping(sort); !known && !r.isFieldWhiteListed(sort) {
log.Warn(r.ctx, "Ignoring sort not whitelisted", "sort", sort, "table", r.tableName)
sort = ""
}
}
if order != "" {

View File

@ -104,7 +104,11 @@ album's tracks — Feishin fetches them this way instead of `ParentId`); `GenreI
genre's albums or tracks — Finamp's genre screen sends it the same way; `/Artists/AlbumArtists`
and `MusicArtist` queries accept it too, matching artists credited on an album of that genre);
`SearchTerm`;
favorites-only (`Filters=IsFavorite` or the standalone `isFavorite=true`); `SortBy`/`SortOrder`;
`Filters` (`IsFavorite`, `IsFavoriteOrLikes`, `IsPlayed`, `IsUnplayed`) and the standalone
`isFavorite`/`isPlayed` booleans it can also be expressed as — `Filters` wins when both are sent, as
in Jellyfin; `Likes`, `Dislikes`, `IsFolder`, `IsNotFolder` and `IsResumable` have no Navidrome
equivalent and are ignored; `SortBy`/`SortOrder` (every recognized key is applied in order, so secondary keys break ties;
unrecognized keys are skipped, and `Random` always sorts alone);
`StartIndex`/`Limit`; and `Ids` (batch fetch by id). `Recursive=false` with a library `ParentId`
returns direct children only (no tracks — no track is a library's direct child).

View File

@ -24,10 +24,6 @@ func (api *Router) getAlbumArtists(w http.ResponseWriter, r *http.Request) {
// when accessible (like queryItems) or all accessible libraries otherwise.
func (api *Router) listArtistsByRole(w http.ResponseWriter, r *http.Request, role model.Role) {
ctx := r.Context()
p := req.Params(r)
opts := model.QueryOptions{Offset: p.IntOr("startindex", 0), Max: p.IntOr("limit", 0)}
applySort(&opts, "MusicArtist", p.StringOr("sortby", ""), p.StringOr("sortorder", ""))
scopeIDs, _, ok := parentIDScope(ctx, r)
if !ok {
http.Error(w, "Not Found", http.StatusNotFound)
@ -38,14 +34,13 @@ func (api *Router) listArtistsByRole(w http.ResponseWriter, r *http.Request, rol
http.Error(w, "Not Found", http.StatusNotFound)
return
}
// Only the fields listArtists reads; /Artists has no favorites filter, so favOnly stays false.
// Finamp's artist tab sends GenreIds when a genre filter is active.
q := itemsQuery{
scopeIDs: scopeIDs,
genreIds: genreIds,
search: searchTerm(p),
fields: dto.ParseFields(p.Strings("fields")...),
}
// This route resolves its own scope, so it shares only the plain query params with /Items.
q := listParams(req.Params(r))
q.scopeIDs = scopeIDs
q.genreIds = genreIds
opts := model.QueryOptions{Offset: q.offset, Max: q.limit}
applySort(&opts, "MusicArtist", q.sortBy, q.sortOrder)
if q.search != "" {
opts.Max = clampLimit(opts.Max, defaultSearchLimit, maxSearchLimit)
}

View File

@ -156,6 +156,29 @@ var _ = Describe("Browsing", func() {
Expect(sql).NotTo(ContainSubstring("library_artist.library_id"))
})
DescribeTable("restricts to favorites",
func(url string, handler func(*Router) http.HandlerFunc) {
artistRepo := ds.Artist(context.Background()).(*tests.MockArtistRepo)
artistRepo.SetData(model.Artists{{ID: testID("ar1"), Name: "Artist"}})
w := httptest.NewRecorder()
r := httptest.NewRequest("GET", url, nil).WithContext(ctxUser(model.Libraries{{ID: 1}}))
invoke(handler(api), w, r)
Expect(w.Code).To(Equal(http.StatusOK))
sql, args, err := artistRepo.Options.Filters.ToSql()
Expect(err).NotTo(HaveOccurred())
Expect(sql).To(ContainSubstring("starred"))
// listArtists always ANDs notMissing, favorites filter or not.
Expect(sql).To(ContainSubstring("missing"))
Expect(args).To(ContainElement(true))
},
Entry("Filters=IsFavorite", "/Artists?Filters=IsFavorite",
func(a *Router) http.HandlerFunc { return a.getArtists }),
Entry("isFavorite=true", "/Artists?isFavorite=true",
func(a *Router) http.HandlerFunc { return a.getArtists }),
Entry("on /Artists/AlbumArtists", "/Artists/AlbumArtists?Filters=IsFavorite",
func(a *Router) http.HandlerFunc { return a.getAlbumArtists }),
)
It("404s a malformed ParentId instead of listing every library's artists", func() {
artistRepo := ds.Artist(context.Background()).(*tests.MockArtistRepo)
artistRepo.SetData(model.Artists{{ID: testID("ar1"), Name: "Artist"}})

View File

@ -11,7 +11,6 @@ import (
_ "image/png"
"io"
"net/http"
"strconv"
"github.com/dustin/go-humanize"
"github.com/navidrome/navidrome/conf"
@ -20,9 +19,20 @@ import (
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/request"
"github.com/navidrome/navidrome/server/imghttp"
"github.com/navidrome/navidrome/utils/req"
_ "golang.org/x/image/webp"
)
// imageSize picks the tighter of Jellyfin's two bounds, because Navidrome resizes on a single
// dimension: reading only MaxWidth serves the full-size original to a client that sent MaxHeight.
func imageSize(maxWidth, maxHeight int) int {
w, h := max(maxWidth, 0), max(maxHeight, 0)
if w == 0 || h == 0 {
return max(w, h)
}
return min(w, h)
}
func (api *Router) getItemImage(w http.ResponseWriter, r *http.Request) {
// Public endpoint, like real Jellyfin's image routes: clients fetch cover URLs without credentials
// and item ids are unguessable, so resolution runs elevated to bypass the visibility filter.
@ -31,7 +41,8 @@ func (api *Router) getItemImage(w http.ResponseWriter, r *http.Request) {
if !ok {
return
}
size, _ := strconv.Atoi(r.URL.Query().Get("maxwidth"))
p := req.Params(r)
size := imageSize(p.IntOr("maxwidth", 0), p.IntOr("maxheight", 0))
artID := api.resolveArtworkID(ctx, itemId)
img, err := api.artwork.GetOrPlaceholder(ctx, artID, size, false)

View File

@ -30,14 +30,16 @@ import (
type fakeArtwork struct {
artwork.Artwork
recvId string
recvCtx context.Context
data []byte
hash string
recvId string
recvSize int
recvCtx context.Context
data []byte
hash string
}
func (f *fakeArtwork) GetOrPlaceholder(ctx context.Context, id string, size int, square bool) (*artwork.Image, error) {
f.recvId = id
f.recvSize = size
f.recvCtx = ctx
data := f.data
if data == nil {
@ -61,6 +63,29 @@ func newImageRequest(itemId string) (*httptest.ResponseRecorder, *http.Request)
}
var _ = Describe("Images", func() {
// Real Jellyfin fits the image inside either bound, so a client that sends only MaxHeight must
// still get a resized image rather than the full-size original.
DescribeTable("derives the requested size from MaxWidth or MaxHeight",
func(query string, wantSize int) {
ds := &tests.MockDataStore{}
ds.Album(context.Background()).(*tests.MockAlbumRepo).SetData(model.Albums{{ID: testID("a1"), Name: "One"}})
fa := &fakeArtwork{}
api := &Router{ds: ds, artwork: fa}
w, r := newImageRequest(dto.EncodeID(testID("a1")))
r.URL.RawQuery = query
api.getItemImage(w, r)
Expect(w.Code).To(Equal(http.StatusOK))
Expect(fa.recvSize).To(Equal(wantSize))
},
Entry("MaxWidth only", "maxwidth=300", 300),
Entry("MaxHeight only", "maxheight=300", 300),
Entry("both, smaller bound wins", "maxwidth=200&maxheight=300", 200),
Entry("both, smaller bound wins regardless of order", "maxwidth=300&maxheight=200", 200),
Entry("neither", "", 0),
)
It("streams album artwork", func() {
ds := &tests.MockDataStore{}
ds.Album(context.Background()).(*tests.MockAlbumRepo).SetData(model.Albums{{ID: testID("a1"), Name: "One"}})

View File

@ -31,6 +31,51 @@ func searchTerm(p *req.Values) string {
return strings.TrimSpace(p.StringOr("searchterm", ""))
}
// itemFilters is the parsed Filters=... list together with the standalone isFavorite/isPlayed params
// clients may send instead. A nil field means the client asked for no filtering on that dimension.
type itemFilters struct {
favorite *bool
played *bool
}
// parseItemFilters reads the standalone params first and lets the Filters list win, matching real
// Jellyfin. Tokens with no Navidrome equivalent (Likes, IsFolder, IsResumable) are dropped.
func parseItemFilters(p *req.Values) itemFilters {
f := itemFilters{favorite: p.BoolPtr("isfavorite"), played: p.BoolPtr("isplayed")}
for token := range strings.SplitSeq(p.StringOr("filters", ""), ",") {
switch strings.TrimSpace(token) {
case "IsFavorite", "IsFavoriteOrLikes":
f.favorite = new(true)
case "IsPlayed":
f.played = new(true)
case "IsUnplayed":
f.played = new(false)
}
}
return f
}
// predicates renders the filters as annotation-column conditions. The negative cases have to match
// NULL as well: annotations are LEFT JOINed, so an item nobody has touched has no row at all.
func (f itemFilters) predicates() []squirrel.Sqlizer {
var out []squirrel.Sqlizer
if f.favorite != nil {
if *f.favorite {
out = append(out, squirrel.Eq{"starred": true})
} else {
out = append(out, squirrel.Or{squirrel.Eq{"starred": nil}, squirrel.Eq{"starred": false}})
}
}
if f.played != nil {
if *f.played {
out = append(out, squirrel.Gt{"play_count": 0})
} else {
out = append(out, squirrel.Or{squirrel.Eq{"play_count": nil}, squirrel.Eq{"play_count": 0}})
}
}
return out
}
func (api *Router) getItems(w http.ResponseWriter, r *http.Request) {
res, err := api.queryItems(r.Context(), r)
if err != nil {
@ -214,7 +259,7 @@ type itemsQuery struct {
sortOrder string
offset int
limit int
favOnly bool
filters itemFilters
// parentId scopes the query. entityParent is the same id only when it names an entity (an artist
// for MusicAlbum, an album for Audio) rather than a library.
parentId string
@ -231,6 +276,19 @@ type itemsQuery struct {
studioIds []string
}
// listParams reads the itemsQuery fields that come straight from query params.
func listParams(p *req.Values) itemsQuery {
return itemsQuery{
fields: dto.ParseFields(p.Strings("fields")...),
search: searchTerm(p),
sortBy: p.StringOr("sortby", ""),
sortOrder: p.StringOr("sortorder", ""),
offset: p.IntOr("startindex", 0),
limit: p.IntOr("limit", 0),
filters: parseItemFilters(p),
}
}
// parseItemsQuery also resolves the entity types (inferring them from the parent when
// IncludeItemTypes is absent) and the library scope. Query keys are read lowercase because
// normalizeQueryKeys folded them (Jellyfin binds case-insensitively). A non-empty id param that
@ -261,24 +319,14 @@ func (api *Router) parseItemsQuery(ctx context.Context, r *http.Request) (itemsQ
if !ok {
return itemsQuery{}, model.ErrNotFound
}
q := itemsQuery{
fields: dto.ParseFields(p.Strings("fields")...),
ids: ids,
rawTypes: p.StringOr("includeitemtypes", ""),
search: searchTerm(p),
sortBy: p.StringOr("sortby", ""),
sortOrder: p.StringOr("sortorder", ""),
offset: p.IntOr("startindex", 0),
limit: p.IntOr("limit", 0),
// Clients express "favorites only" two ways: Filters=IsFavorite and the standalone
// isFavorite=true param (Finamp's "Favourite tracks" widget uses the latter).
favOnly: strings.Contains(p.StringOr("filters", ""), "IsFavorite") || p.BoolOr("isfavorite", false),
parentId: parentId,
genreIds: genreIds,
albumIds: albumIds,
years: parseYears(r),
studioIds: studioIds,
}
q := listParams(p)
q.ids = ids
q.rawTypes = p.StringOr("includeitemtypes", "")
q.parentId = parentId
q.genreIds = genreIds
q.albumIds = albumIds
q.years = parseYears(r)
q.studioIds = studioIds
// An artist's page filters by artist, not ParentId: Finamp sends ParentId=<libraryId> for scoping
// plus AlbumArtistIds/ArtistIds/contributingArtistIds for the artist.
albumArtistScope := firstNonEmpty(p.StringOr("albumartistids", ""), p.StringOr("artistids", ""))
@ -621,8 +669,10 @@ func (api *Router) listAlbums(ctx context.Context, opts model.QueryOptions, q it
if len(q.studioIds) > 0 {
filters = append(filters, filter.ByStudioID(q.studioIds))
}
if q.favOnly {
filters = append(filters, filter.ByStarred().Filters)
// Not on the search path: its first FTS phase selects rowids with no annotation join, so a
// starred/play_count predicate there is "no such column" rather than a filter.
if q.search == "" {
filters = append(filters, q.filters.predicates()...)
}
opts.Filters = filters
opts = filter.ApplyLibraryFilter(opts, q.scopeIDs)
@ -668,8 +718,10 @@ func (api *Router) listSongs(ctx context.Context, opts model.QueryOptions, q ite
if len(q.studioIds) > 0 {
filters = append(filters, filter.ByStudioID(q.studioIds))
}
if q.favOnly {
filters = append(filters, filter.ByStarred().Filters)
// Not on the search path: its first FTS phase selects rowids with no annotation join, so a
// starred/play_count predicate there is "no such column" rather than a filter.
if q.search == "" {
filters = append(filters, q.filters.predicates()...)
}
opts.Filters = filters
opts = filter.ApplyLibraryFilter(opts, q.scopeIDs)
@ -720,14 +772,12 @@ func (api *Router) listArtists(ctx context.Context, opts model.QueryOptions, q i
return materialized(result(slice.Map(artists, toItem), total, opts.Offset)), nil
}
if q.favOnly {
opts.Filters = filter.ArtistsByStarred().Filters
} else {
opts.Filters = notMissing
}
filters := squirrel.And{notMissing}
filters = append(filters, q.filters.predicates()...)
if len(q.genreIds) > 0 {
opts.Filters = squirrel.And{opts.Filters, filter.ArtistsByGenreID(q.genreIds)}
filters = append(filters, filter.ArtistsByGenreID(q.genreIds))
}
opts.Filters = filters
opts = filter.ArtistsByRole(opts, role)
opts = filter.ApplyArtistLibraryFilter(opts, q.scopeIDs)
total, _ := repo.CountAll(model.QueryOptions{Filters: opts.Filters})
@ -752,13 +802,8 @@ func (api *Router) listGenres(ctx context.Context, opts model.QueryOptions) (ite
// listPlaylists lists playlists visible to the current user. Visibility (public or owned) is
// enforced by playlistRepository, not scopeIDs.
func (api *Router) listPlaylists(ctx context.Context, opts model.QueryOptions, q itemsQuery) (itemsResult, error) {
if q.favOnly {
starred := squirrel.Eq{"starred": true}
if opts.Filters == nil {
opts.Filters = starred
} else {
opts.Filters = squirrel.And{opts.Filters, starred}
}
if preds := q.filters.predicates(); len(preds) > 0 {
opts.Filters = squirrel.And(preds)
}
repo := api.ds.Playlist(ctx)
total, err := repo.CountAll(model.QueryOptions{Filters: opts.Filters})
@ -908,18 +953,32 @@ func result(items []dto.BaseItemDto, total, start int) dto.QueryResult {
return dto.QueryResult{Items: items, TotalRecordCount: total, StartIndex: start}
}
// applySort translates Jellyfin's SortBy/SortOrder into a valid model.QueryOptions sort key for the
// item type. Clients send SortBy as a comma-separated fallback list (e.g. "DateCreated,SortName");
// this uses the first recognized key. An unrecognized SortBy is left untouched (the repo's default),
// not passed through raw where it could produce an invalid ORDER BY.
// applySort keeps every recognized SortBy key, so secondary keys break ties as Jellyfin intends.
// Unrecognized keys are skipped, not passed through raw where they could make an invalid ORDER BY.
func applySort(opts *model.QueryOptions, itemType, sortBy, order string) {
var cols []string
for key := range strings.SplitSeq(sortBy, ",") {
if col, ok := sortColumn(itemType, strings.TrimSpace(key)); ok {
opts.Sort = col
col, ok := sortColumn(itemType, strings.TrimSpace(key))
// The repo matches random by exact string equality, so it can only ever sort alone.
if !ok || slices.Contains(cols, col) || (col == "random" && len(cols) > 0) {
continue
}
cols = append(cols, col)
if col == "random" {
break
}
}
if strings.EqualFold(order, "Descending") {
switch {
case len(cols) > 0:
opts.Sort = strings.Join(cols, ", ")
case sortBy != "":
log.Debug("Jellyfin API: no usable SortBy key, falling back to the default order",
"itemType", itemType, "sortBy", sortBy)
}
// Jellyfin allows a per-key SortOrder list, which one Order can't express; honor the first value
// for every key, as Jellyfin does for keys past the end of the list.
first, _, _ := strings.Cut(order, ",")
if strings.EqualFold(first, "Descending") {
opts.Order = "desc"
}
}
@ -941,6 +1000,8 @@ var sortColumnsByType = map[string]map[string]string{
"dateplayed": "play_date",
"communityrating": "rating",
"random": "random",
"runtime": "duration",
"runtimeticks": "duration",
// Finamp's "Latest Releases" sorts by PremiereDate; "year" matches songs' ProductionYear.
"premieredate": "year",
"productionyear": "year",
@ -964,6 +1025,8 @@ var sortColumnsByType = map[string]map[string]string{
"playcount": "play_count",
"dateplayed": "play_date",
"communityrating": "rating",
"runtime": "duration",
"runtimeticks": "duration",
"premieredate": "max_year", "productionyear": "max_year",
},
"MusicGenre": {

View File

@ -327,17 +327,75 @@ var _ = Describe("Items", func() {
Expect(albumRepo.Options.Max).To(Equal(3))
})
It("applies a starred filter when Filters=IsFavorite", func() {
albumRepo := ds.Album(context.Background()).(*tests.MockAlbumRepo)
albumRepo.SetData(model.Albums{{ID: testID("a1"), Name: "One"}})
w := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=MusicAlbum&Filters=IsFavorite", nil).WithContext(ctxUser())
invoke(api.getItems, w, r)
Expect(w.Code).To(Equal(http.StatusOK))
sql, _, err := albumRepo.Options.Filters.ToSql()
Expect(err).NotTo(HaveOccurred())
Expect(sql).To(ContainSubstring("starred"))
})
DescribeTable("translates the Filters list and its standalone equivalents",
func(query string, wantSQL, notWantSQL []string) {
albumRepo := ds.Album(context.Background()).(*tests.MockAlbumRepo)
albumRepo.SetData(model.Albums{{ID: testID("a1"), Name: "One"}})
w := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=MusicAlbum&"+query, nil).WithContext(ctxUser())
invoke(api.getItems, w, r)
Expect(w.Code).To(Equal(http.StatusOK))
sql, _, err := albumRepo.Options.Filters.ToSql()
Expect(err).NotTo(HaveOccurred())
for _, want := range wantSQL {
Expect(sql).To(ContainSubstring(want))
}
for _, not := range notWantSQL {
Expect(sql).NotTo(ContainSubstring(not))
}
},
Entry("IsFavorite", "Filters=IsFavorite", []string{"starred"}, nil),
Entry("IsFavorite,IsUnplayed combined", "Filters=IsFavorite,IsUnplayed",
[]string{"starred", "play_count"}, nil),
Entry("IsUnplayed", "Filters=IsUnplayed", []string{"play_count"}, []string{"starred"}),
Entry("IsPlayed", "Filters=IsPlayed", []string{"play_count"}, []string{"starred"}),
Entry("IsFavoriteOrLikes is treated as favorites", "Filters=IsFavoriteOrLikes", []string{"starred"}, nil),
Entry("isPlayed=false", "isPlayed=false", []string{"play_count"}, nil),
Entry("isFavorite=false still filters", "isFavorite=false", []string{"starred"}, nil),
// Jellyfin builds the query from the standalone params, then applies Filters over the top.
Entry("Filters wins over the standalone param", "isFavorite=false&Filters=IsFavorite",
[]string{"starred = "}, nil),
// No Navidrome equivalent: these must be dropped, not half-applied.
Entry("Likes is ignored", "Filters=Likes", nil, []string{"starred", "play_count"}),
Entry("IsResumable is ignored", "Filters=IsResumable", nil, []string{"starred", "play_count"}),
// The artist-parent branch gets notMissing from filter.AlbumsByArtistID, not the default
// branch, so favorites must not be the only predicate left on it.
Entry("keeps missing excluded under an artist parent",
"Filters=IsFavorite&ArtistIds="+dto.EncodeID(testID("ar1")),
[]string{"starred", "missing"}, nil),
)
// Search runs a two-phase FTS query whose first phase has no annotation join, so an
// annotation predicate there is "no such column: starred" -> 500.
DescribeTable("does not push annotation filters into a search",
func(itemType, filters string) {
ds.Album(context.Background()).(*tests.MockAlbumRepo).SetData(model.Albums{{ID: testID("a1"), Name: "One"}})
ds.MediaFile(context.Background()).(*tests.MockMediaFileRepo).SetData(model.MediaFiles{{ID: testID("s1"), Title: "Song"}})
w := httptest.NewRecorder()
r := httptest.NewRequest("GET",
"/Items?IncludeItemTypes="+itemType+"&SearchTerm=one&Filters="+filters, nil).WithContext(ctxUser())
invoke(api.getItems, w, r)
Expect(w.Code).To(Equal(http.StatusOK))
var opts model.QueryOptions
if itemType == "MusicAlbum" {
opts = ds.Album(context.Background()).(*tests.MockAlbumRepo).Options
} else {
opts = ds.MediaFile(context.Background()).(*tests.MockMediaFileRepo).Options
}
if opts.Filters == nil {
return
}
sql, _, err := opts.Filters.ToSql()
Expect(err).NotTo(HaveOccurred())
Expect(sql).NotTo(ContainSubstring("starred"))
Expect(sql).NotTo(ContainSubstring("play_count"))
},
Entry("albums, IsFavorite", "MusicAlbum", "IsFavorite"),
Entry("albums, IsUnplayed", "MusicAlbum", "IsUnplayed"),
Entry("albums, IsPlayed", "MusicAlbum", "IsPlayed"),
Entry("songs, IsFavorite", "Audio", "IsFavorite"),
Entry("songs, IsUnplayed", "Audio", "IsUnplayed"),
)
It("forwards SearchTerm to the repo's Search method", func() {
albumRepo := ds.Album(context.Background()).(*tests.MockAlbumRepo)
@ -601,65 +659,59 @@ var _ = Describe("Items", func() {
})
Describe("sorting", func() {
It("maps SortBy=PlayCount to the play_count column", func() {
albumRepo := ds.Album(context.Background()).(*tests.MockAlbumRepo)
albumRepo.SetData(model.Albums{{ID: testID("a1"), Name: "One"}})
w := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=MusicAlbum&SortBy=PlayCount", nil).WithContext(ctxUser())
invoke(api.getItems, w, r)
Expect(w.Code).To(Equal(http.StatusOK))
Expect(albumRepo.Options.Sort).To(Equal("play_count"))
})
DescribeTable("translates SortBy into the repo's sort keys",
func(itemType, sortBy, want string) {
albumRepo := ds.Album(context.Background()).(*tests.MockAlbumRepo)
albumRepo.SetData(model.Albums{{ID: testID("a1"), Name: "One"}})
mfRepo := ds.MediaFile(context.Background()).(*tests.MockMediaFileRepo)
mfRepo.SetData(model.MediaFiles{{ID: testID("s1"), Title: "Song"}})
w := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/Items?IncludeItemTypes="+itemType+"&SortBy="+sortBy, nil).WithContext(ctxUser())
invoke(api.getItems, w, r)
Expect(w.Code).To(Equal(http.StatusOK))
got := mfRepo.Options.Sort
if itemType == "MusicAlbum" {
got = albumRepo.Options.Sort
}
Expect(got).To(Equal(want))
},
Entry("PlayCount", "MusicAlbum", "PlayCount", "play_count"),
Entry("DatePlayed", "Audio", "DatePlayed", "play_date"),
Entry("Runtime on albums", "MusicAlbum", "Runtime", "duration"),
Entry("RunTimeTicks alias", "MusicAlbum", "RunTimeTicks", "duration"),
// Finamp leads its track sort with Runtime: unless that resolves, the first recognized
// key is AlbumArtist and the list looks sorted while being sorted by the wrong thing.
Entry("Finamp's Runtime-led track sort", "Audio", "Runtime,AlbumArtist,Album,SortName",
"duration, album_artist, album, title"),
Entry("every recognized key, in order", "MusicAlbum", "DateCreated,SortName", "recently_added, name"),
Entry("a key repeating a column is dropped", "Audio",
"PremiereDate,Album,ParentIndexNumber,IndexNumber,SortName", "year, album, title"),
// random is matched by exact string equality in the repo, so it can never share a sort.
Entry("Random stays alone", "MusicAlbum", "Random,SortName", "random"),
Entry("unrecognized keys are skipped", "Audio", "Runtime,Nonsense,SortName", "duration, title"),
Entry("only the last key recognized", "Audio", "Unknown1,Unknown2,SortName", "title"),
Entry("Finamp's album view is disc+track", "Audio", "ParentIndexNumber,IndexNumber,SortName", "album, title"),
Entry("nothing recognized leaves the repo default", "MusicAlbum", "SeriesSortName", ""),
)
It("maps SortBy=DatePlayed to the play_date column", func() {
mfRepo := ds.MediaFile(context.Background()).(*tests.MockMediaFileRepo)
mfRepo.SetData(model.MediaFiles{{ID: testID("s1"), Title: "Song"}})
w := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=Audio&SortBy=DatePlayed", nil).WithContext(ctxUser())
invoke(api.getItems, w, r)
Expect(w.Code).To(Equal(http.StatusOK))
Expect(mfRepo.Options.Sort).To(Equal("play_date"))
})
It("uses the first recognized key in a comma-separated SortBy list", func() {
albumRepo := ds.Album(context.Background()).(*tests.MockAlbumRepo)
albumRepo.SetData(model.Albums{{ID: testID("a1"), Name: "One"}})
w := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=MusicAlbum&SortBy=DateCreated,SortName", nil).WithContext(ctxUser())
invoke(api.getItems, w, r)
Expect(w.Code).To(Equal(http.StatusOK))
Expect(albumRepo.Options.Sort).To(Equal("recently_added"))
})
It("skips unrecognized keys in a comma-separated SortBy list to find one that is", func() {
mfRepo := ds.MediaFile(context.Background()).(*tests.MockMediaFileRepo)
mfRepo.SetData(model.MediaFiles{{ID: testID("s1"), Title: "Song"}})
w := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=Audio&SortBy=Unknown1,Unknown2,SortName", nil).WithContext(ctxUser())
invoke(api.getItems, w, r)
Expect(w.Code).To(Equal(http.StatusOK))
Expect(mfRepo.Options.Sort).To(Equal("title"))
})
It("maps Finamp's album view SortBy (ParentIndexNumber,IndexNumber) to disc+track order", func() {
mfRepo := ds.MediaFile(context.Background()).(*tests.MockMediaFileRepo)
mfRepo.SetData(model.MediaFiles{{ID: testID("s1"), Title: "Song"}})
w := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=Audio&SortBy=ParentIndexNumber,IndexNumber,SortName", nil).WithContext(ctxUser())
invoke(api.getItems, w, r)
Expect(w.Code).To(Equal(http.StatusOK))
Expect(mfRepo.Options.Sort).To(Equal("album"))
})
It("leaves Sort at the repo default when no SortBy key is recognized", func() {
albumRepo := ds.Album(context.Background()).(*tests.MockAlbumRepo)
albumRepo.SetData(model.Albums{{ID: testID("a1"), Name: "One"}})
w := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=MusicAlbum&SortBy=SeriesSortName", nil).WithContext(ctxUser())
invoke(api.getItems, w, r)
Expect(w.Code).To(Equal(http.StatusOK))
Expect(albumRepo.Options.Sort).To(Equal(""))
})
// Jellyfin allows a per-key SortOrder list; we cannot express that through one Order, so
// we honor the first value for all keys, matching Jellyfin's fallback for extra keys.
DescribeTable("reads the first SortOrder value for the whole sort",
func(sortOrder, want string) {
albumRepo := ds.Album(context.Background()).(*tests.MockAlbumRepo)
albumRepo.SetData(model.Albums{{ID: testID("a1"), Name: "One"}})
w := httptest.NewRecorder()
r := httptest.NewRequest("GET",
"/Items?IncludeItemTypes=MusicAlbum&SortBy=Runtime,SortName&SortOrder="+sortOrder, nil).WithContext(ctxUser())
invoke(api.getItems, w, r)
Expect(w.Code).To(Equal(http.StatusOK))
Expect(albumRepo.Options.Order).To(Equal(want))
},
Entry("ascending", "Ascending", ""),
Entry("descending", "Descending", "desc"),
Entry("descending leading a list", "Descending,Ascending", "desc"),
Entry("ascending leading a list", "Ascending,Descending", ""),
)
})
Describe("library scoping", func() {