* 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.
The 'sees all libraries' specs hard-coded the user's libraries as {1, 2}
and assumed the shared test DB held exactly two libraries. applyLibraryFilter
only skips the filter when granted count == total library count, so when
another spec left an extra library behind (Ginkgo randomizes spec order),
the count was 3, the filter was applied, and the SQL assertion failed. This
surfaced on the Windows CI run but reproduces on any platform.
Grant the user exactly the library IDs that actually exist in the DB at
runtime instead of hard-coding them.
* perf(db): skip library filter when a non-admin sees all libraries
applyLibraryFilter already short-circuits for admins and headless
contexts, but a non-admin who has been granted every library still paid
for the correlated user_library subquery, which filters out nothing yet
is the slow non-admin list/count path.
Reuse the visibility check search3 already had: skip the subquery when
the user's granted libraries cover the whole library table. The two
helpers (userSeesAllLibraries/visibleLibraryIDs) are promoted from
artist_repository to the base sqlRepository so all ~13 call sites
benefit and search3 shares the single implementation.
The skip is strictly gated on granted count >= total library count,
never on an empty/unknown library set, so access control is unchanged
for restricted users.
* refactor(db): make all-libraries skip fail closed; test cleanups
- userSeesAllLibraries: require len(visible) == total (not >=) so the
filter skip can never over-grant if the visible set is ever inflated.
- tests: assert ToSql() returns no error; restore r.db in AfterEach to
avoid leaking mutated state to other specs.
Signed-off-by: Deluan <deluan@navidrome.org>
---------
Signed-off-by: Deluan <deluan@navidrome.org>
* fix: enable library access for headless processes
Fixed multi-library filtering to allow headless processes (shares, external providers) to access data by skipping library restrictions when no user context is present.
Previously, the library filtering system returned empty results (WHERE 1=0) for processes without user authentication, breaking functionality like public shares and external service integrations.
Key changes:
- Modified applyLibraryFilter methods to skip filtering when user.ID == invalidUserId
- Refactored tag repository to use helper method for library filtering logic
- Fixed SQL aggregation bug in tag statistics calculation across multiple libraries
- Added comprehensive test coverage for headless process scenarios
- Updated genre repository to support proper column mappings for aggregated data
This preserves the secure "safe by default" approach for authenticated users while restoring backward compatibility for background processes that need unrestricted data access.
Signed-off-by: Deluan <deluan@navidrome.org>
* fix: resolve SQL ambiguity errors in share queries
Fixed SQL ambiguity errors that were breaking share links after the Multi-library PR.
The Multi-library changes introduced JOINs between album and library tables,
both of which have 'id' columns, causing 'ambiguous column name: id' errors
when unqualified column references were used in WHERE clauses.
Changes made:
- Updated core/share.go to use 'album.id' instead of 'id' in contentsLabelFromAlbums
- Updated persistence/share_repository.go to use 'album.id' in album share loading
- Updated persistence/sql_participations.go to use 'artist.id' for consistency
- Added regression tests to prevent future SQL ambiguity issues
This resolves HTTP 500 errors that users experienced when accessing existing
share URLs after the Multi-library feature was merged.
Signed-off-by: Deluan <deluan@navidrome.org>
* fix: improve headless library access handling
Added proper user context validation and reordered joins in applyLibraryFilterToArtistQuery to ensure library filtering works correctly for both authenticated and headless operations. The user_library join is now only applied when a valid user context exists, while the library_artist join is always applied to maintain proper data relationships. (+1 squashed commit)
Squashed commits:
[a28c6965b] fix: remove headless library access guard
Removed the invalidUserId guard condition in applyLibraryFilterToArtistQuery that was preventing proper library filtering for headless operations. This fix ensures that library filtering joins are always applied consistently, allowing headless library access to work correctly with the library_artist junction table filtering.
The previous guard was skipping all library filtering when no user context was present, which could cause issues with headless operations that still need to respect library boundaries through the library_artist relationship.
* fix: simplify genre selection query in genre repository
Signed-off-by: Deluan <deluan@navidrome.org>
* fix: enhance tag library filtering tests for headless access
Signed-off-by: Deluan <deluan@navidrome.org>
* test: add comprehensive test coverage for headless library access
Added extensive test coverage for headless library access improvements including:
- Added 17 new tests across 4 test files covering headless access scenarios
- artist_repository_test.go: Added headless process tests for GetAll, Count,
Get operations and explicit library_id filtering functionality
- genre_repository_test.go: Added library filtering tests for headless processes
including GetAll, Count, ReadAll, and Read operations
- sql_base_repository_test.go: Added applyLibraryFilter method tests covering
admin users, regular users, and headless processes with/without custom table names
- share_repository_test.go: Added headless access tests and SQL ambiguity
verification for the album.id vs id fix in loadMedia function
- Cleaned up test setup by replacing log.NewContext usage with GinkgoT().Context()
and removing unnecessary configtest.SetupConfig() calls for better test isolation
These tests ensure that headless processes (background operations without user context)
can access all libraries while respecting explicit filters, and verify that the SQL
ambiguity fixes work correctly without breaking existing functionality.
* revert: remove user context handling from scrobble buffer getParticipants
Reverts commit 5b8ef74f05109ecf30ddfc936361b84314522869.
The artist repository no longer requires user context for proper library
filtering, so the workaround of temporarily injecting user context into
the scrobbleBufferRepository.Next method is no longer needed.
This simplifies the code and removes the dependency on fetching user
information during background scrobbling operations.
* fix: improve library access filtering for artists
Enhanced artist repository filtering to properly handle library access restrictions
and prevent artists with no accessible content from appearing in results.
Backend changes:
- Modified roleFilter to use direct JSON_EXTRACT instead of EXISTS subquery for better performance
- Enhanced applyLibraryFilterToArtistQuery to filter out artists with empty stats (no content)
- Changed from LEFT JOIN to INNER JOIN with library_artist table for stricter filtering
- Added condition to exclude artists where library_artist.stats = '{}' (empty content)
Frontend changes:
- Added null-checking in getCounter function to prevent TypeError when accessing undefined records
- Improved optional chaining for safer property access in role-based statistics display
These changes ensure that users only see artists that have actual accessible content
in their permitted libraries, fixing issues where artists appeared in the list
despite having no albums or songs available to the user.
* fix: update library access logic for non-admin users and enhance test coverage
Signed-off-by: Deluan <deluan@navidrome.org>
* fix: refine library artist query and implement cleanup for empty entries
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: consolidate artist repository tests to eliminate duplication
Significantly refactored artist_repository_test.go to reduce code duplication and
improve maintainability by ~27% (930 to 680 lines). Key improvements include:
- Added test helper functions createTestArtistWithMBID() and createUserWithLibraries()
to eliminate repetitive test data creation
- Consolidated duplicate MBID search tests using DescribeTable for parameterized testing
- Removed entire 'Permission-Based Behavior Comparison' section (~150 lines) that
duplicated functionality already covered in other test contexts
- Reorganized search tests into cohesive 'MBID and Text Search' section with proper
setup/teardown and shared test infrastructure
- Streamlined missing artist tests and moved them to dedicated section
- Maintained 100% test coverage while eliminating redundant test patterns
All tests continue to pass with identical functionality and coverage.
---------
Signed-off-by: Deluan <deluan@navidrome.org>
* fix(subsonic): Sort songs by presence of lyrics for `getLyrics`
The current implementation of `getLyrics` fetches any songs matching the artist and title.
However, this misses a case where there may be multiple matches for the same artist/song, and one has lyrics while the other doesn't.
Resolve this by adding a custom SQL dynamic column that checks for the presence of lyrics.
* add options to selectMediaFile, update test
* more robust testing of GetAllByLyrics
* fix(subsonic): refactor GetAllByLyrics to GetAll with lyrics sorting
Signed-off-by: Deluan <deluan@navidrome.org>
* use has_lyrics, and properly support multiple sort parts
* better handle complicated internal sorts
* just use a simpler filter
* add note to setSortMappings
* remove custom sort mapping, improve test with different updatedat
* refactor tests and mock
Signed-off-by: Deluan <deluan@navidrome.org>
* default order when not specified is `asc`
Signed-off-by: Deluan <deluan@navidrome.org>
---------
Signed-off-by: Deluan <deluan@navidrome.org>
Co-authored-by: Deluan <deluan@navidrome.org>
