* feat(ui): add perPageStore helper for items-per-page persistence
* feat(ui): persist items-per-page selection in localStorage
* feat(ui): enable per-page persistence on album grid, missing files and playlist tracks
* feat(ui): restore saved album grid page size on fresh load
* feat(ui): restore saved items-per-page on list load
* fix(ui): move defaultRowsPerPageOptions to perPageStore to satisfy react-refresh lint
* fix(ui): correct defaultRowsPerPageOptions import in List and add render test
* refactor(ui): default getStoredPerPage fallback to the first option
The fallback equalled options[0] at three of four call sites; default it so
those sites stop restating it. SongList and useAlbumsPerPage still pass an
explicit fallback where it legitimately differs from the first option.
* fix(ui): persist only user-selected page sizes, validate album size against width
Persisting on every pagination context value let a URL-injected perPage (e.g.
the perPage=15 album link in NowPlayingPanel) or a forced single-option mobile
grid overwrite the saved preference. Persist only a value that is an actual
option in a multi-option selector. Also validate the album grid's redux session
value against the current width so a size chosen at a wider breakpoint can't
leave an out-of-range selector.
* fix(ui): restore saved items-per-page on the radio list
RadioList passed a hard-coded perPage that overrode List's stored seed via the
props spread, so a radio-list page size was persisted but never restored. Seed
it from storage like the other list views.
* fix(ui): persist page size only on an actual selector change
Watching the pagination context value meant any valid value persisted itself:
loading a list at a breakpoint where the saved size is invalid stored the
responsive fallback, and opening a URL with a valid ?perPage= stored that too,
either way discarding the user's real preference. Inject a wrapped setPerPage
instead, so only the rows-per-page selector writes. This also drops the
option-validation heuristics, which the new trigger makes unnecessary.
* fix(subsonic): surface the reason a transcode decision failed
getTranscodeDecision returned a bare "failed to make transcode decision"
with no clue why, and the probe command ran ffprobe with -v quiet, so even
the server log bottomed out at "exit status 1". A user whose files had been
moved by an external tool only saw the opaque error.
ProbeAudioStream now returns a typed ProbeError that separates the file path
from the reason: ffprobe runs with -v error so its stderr diagnostic is
captured, and a missing or unreadable file is reported as "file not found"
rather than ffprobe's misleading "Invalid data found". The handler logs the
full detail (including the path) and returns the reason to the client with
the server path stripped out.
Reported-by: Tolriq (Symfonium)
* refactor(ffmpeg): use errors.AsType for ExitError match
probeErrorReason used the older var+errors.As form while the rest of the
codebase (and its sibling transcodeFailureReason) uses the generic
errors.AsType. Switch to it for consistency; behavior is unchanged.
* fix(subsonic): return error 70 when the source file is missing
A getTranscodeDecision probe failure was always reported as generic error 0.
When the source file is gone (moved or deleted out from under the DB), that is
a not-found condition, so return the standard Subsonic error 70 ("data not
found") instead — matching what the endpoint already returns for an unknown
mediaId. Files that exist but are corrupt or unreadable stay error 0.
ProbeError now wraps the underlying cause and implements Unwrap, so the handler
detects the case with errors.Is(err, fs.ErrNotExist).
* fix(ffmpeg): keep probe error paths out of client-facing reasons
Addresses review feedback on the ProbeError type: the Reason field doubled as
both the log detail and the client message, so an ffprobe launch failure (a
*os.PathError from fork/exec) could leak the ffprobe binary path to clients,
and an unexpected stat error was reduced to "file not accessible" in the log.
Split the two concerns: Reason now holds only a path-free, client-safe string
(built at construction), while Error() logs the full underlying cause. Launch
failures return a generic "could not read file" instead of the raw exec error.
SafeReason no longer does substring path-stripping (removing the empty-Path
edge case); the stripping happens once, against ffprobe's stderr.
* fix(subsonic): don't report a broken ffprobe as a missing media file
Two issues from review of the previous commit:
Code 70 was selected with errors.Is(err, fs.ErrNotExist), but a launch failure
of a deleted ffprobe binary is an *os.PathError that also wraps fs.ErrNotExist.
A server-side ffprobe problem was therefore reported to clients as a missing
media file. ProbeError now carries an explicit NotFound flag, set only on the
file-access branch, and the handler keys the code off that instead of the chain.
ffprobe can also exit 0 while yielding no audio stream (an audio-suffixed
container holding only video). That parse failure was returned unwrapped, so
clients got "internal error"; it is now wrapped in a ProbeError too.
* feat(scrobbler): add exponential backoff delay helper
* feat(scrobbler): back off retries up to 4m during outages
* test(scrobbler): verify backoff schedule with synctest; clarify backoffDelay doc
Adds a testing/synctest-based test that drives the real run loop against a
failing service and asserts the exact 5s/10s/20s/40s retry schedule and the
drain-on-recovery reset, addressing the review note that the run loop's
behavior was untested. Also clarifies the backoffDelay doc comment: the
argument is a zero-based retry index.
* feat(persistence): add AlbumRepository.GetYears for distinct album years
* test(persistence): verify GetYears de-duplicates repeated years
Regression test that adds two albums with the same non-zero max_year
(2005) and verifies that GetYears() returns that year exactly once,
ensuring the SQL DISTINCT clause is applied correctly. Catches any
future removal of DISTINCT from the GetYears query.
* feat(jellyfin): add legacy /Items/Filters endpoint (genres + years)
* feat(jellyfin): add /Studios endpoint from record label tags
* refactor(persistence): drop duplicate columns in tagRepository.GetAll
* fix(jellyfin): exclude missing albums from filter years
GetYears only filtered max_year > 0, so albums whose files were all removed
(missing=true, kept when Scanner.PurgeMissing=never) contributed stale years
to /Items/Filters. Filter them out like the normal album listings do. Also
return an empty slice from the MockAlbumRepo to match the real repository.
* feat(jellyfin): filter /Items by Years=
* feat(jellyfin): filter /Items by StudioIds= (record labels)
* feat(jellyfin): scope filter and studio lists to ParentId library
* refactor(jellyfin): extract parentIDScope and libraryScopeFilter helpers
Collapse the three inline resolveLibraryScope(dto.DecodeID(parentid)) call
sites and the duplicated empty-scope guard into two small helpers, so the
empty-scope=unrestricted contract lives in one place. Reuse the existing
names() helper in the Years= e2e test.
* feat(jellyfin): expose record labels as album Studios
Add a Studios field to the album BaseItemDto, populated from the record-label
tags and gated behind Fields=Studios (matching Jellyfin's ItemFields
convention). Studio ids reuse the record-label tag identity, so they round-trip
with the /Studios list and the StudioIds= filter. Real Jellyfin leaves Studios
empty for music; Feishin reads it as the album's record label.
The windowed-CTE backfill was compiled as a correlated scalar subquery
re-evaluated once per album (a full media_file group-by + window sort each
time), which never finished on a large library. Stage the ReplayGain-bearing
rows into an indexed temp table once and update only the affected albums:
~14s on a 727MB / 6945-album library, vs. effectively never.
* feat(model): aggregate album ReplayGain in ToAlbum
* feat(db): add nullable album ReplayGain columns with backfill
* feat(persistence): persist album ReplayGain fields
* feat(jellyfin): expose album NormalizationGain from ReplayGain
* refactor(model): lazy-init mostFrequentPtr map; clean up RG test rows
* fix(db): backfill album ReplayGain with most-frequent value, not max
A plain max() picked a minority outlier that diverged from MediaFiles.ToAlbum
(which uses the most-frequent value), and GetTouchedAlbums never re-derives an
unchanged album, so the wrong value would persist. Reproduce the modal
aggregation via grouped CTEs, which also groups media_file once instead of a
per-album correlated scan.
* refactor(model): return an owned pointer from mostFrequentPtr
Avoid aliasing a MediaFile field so the resulting Album is independent of the
source slice.
* feat(jellyfin): expose NormalizationGain from ReplayGain tags
Adds NormalizationGain and AlbumNormalizationGain to Audio BaseItemDtos,
sourced from the scanner's ReplayGain values (REPLAYGAIN_* tags, with R128_*
already converted to the same -18 LUFS reference). Same wire contract as real
Jellyfin: PascalCase keys, omitted when absent, no Fields gating. Album items
intentionally omit the field: model.Album has no gain column and Feishin reads
gain from song DTOs.
* test(jellyfin): e2e coverage for NormalizationGain fields
* test(jellyfin): drop redundant NormalizationGain passthrough spec
The JSON-casing spec already proves the mapped values (the substring
"NormalizationGain":-3.5 can only appear if the passthrough worked), so the
direct-struct spec added no coverage.
* style(jellyfin): use ASCII punctuation in gain comment
The image endpoint only served a private playlist's cover when the request
carried a token identifying its owner or an admin. But clients fetch cover
URLs without credentials — real Jellyfin's image routes are anonymous — so
every private playlist rendered the generic placeholder in Jellyfin clients
(observed in production), while the same covers displayed fine through the
always-authenticated Subsonic/native APIs.
Drop the gate and serve playlist covers like album/artist/track artwork:
playlist ids are unguessable without credentials, so anonymous access does
not meaningfully expose private playlist contents.
* refactor(req): drop redundant error return from Strings
The error from Strings carried no information beyond emptiness — it fired
exactly when the param was absent — and nearly every caller discarded it with
a blank identifier. Strings now just returns the values (empty when absent),
making the common optional-list reads one clean expression.
The few required-param callers (scrobble, createShare) check for emptiness and
return the same Subsonic error code 10 as before; their e2e tests now pin that
code. Ints and Times keep their contracts by synthesizing ErrMissingParam
themselves, so selectedMusicFolderIds is untouched. The jellyfin parseFields
helper is inlined away, since ParseFields(p.Strings("fields")...) now
compiles directly.
* docs(req): clarify Strings returns nil when param is absent
The Fields param was read with StringOr, which keeps only one value, so a
request sending it as repeated params (Fields=Genres&Fields=MediaSources) — as
Finamp and Feishin do — lost all but the first. Field-gated data like
MediaSources was then omitted for Audio items even though the client asked for
it; the comma-separated form happened to work because ParseFields splits on
commas. Real Jellyfin accepts both forms.
ParseFields is now variadic and a parseFields helper reads every repeated value
via req.Values.Strings, applied to the item list, single-item, and playlist
endpoints.
* fix(jellyfin): split Artists/ArtistItems per track artist
The Jellyfin API returned a single ArtistItems entry built from the flattened
display artist, so a multi-artist track (e.g. "De La Soul feat. Redman") lost
its individual artists and dropped every artist id but the first — while the
same track's OpenSubsonic response correctly split them. Real Jellyfin splits
Artists/ArtistItems one entry per track artist.
Build both from Participants[RoleArtist], which is already loaded on the search
and browse paths, falling back to the flattened display fields when absent.
AlbumArtists stays a single credit, matching real Jellyfin.
* fix(jellyfin): omit empty Artists in the fallback path
When a track has no participants and no display artist, the fallback set
Artists to a single-element slice holding an empty string. Only populate it
when the display artist is non-empty, so untagged tracks omit the field
instead of sending [""].
Like Subsonic's setStar/setRating, the Jellyfin favorite and rating
endpoints now broadcast a refreshResource event, so the web UI updates
immediately when a Jellyfin client changes an annotation.
Also fixes model.GetEntityByID to propagate unexpected repository errors
instead of reporting them as not-found, preserving the 500-vs-404
distinction for all its callers.
* fix(deezer): pick most-popular artist among same-name matches
The Deezer agent searched with order=RANKING and always took the
top-ranked result (artists[0]) as long as its name matched. Deezer's
RANKING order isn't reliable for homonyms, so for names shared by
several artists (e.g. "Queen") it locked onto a low-popularity artist
whose Top Tracks are empty, leaving getTopSongs empty.
Among the exact-name matches, select the one with the highest fan
count instead. This resolves "Queen" to the real band (Deezer ID 412)
and preserves the existing ErrNotFound behavior when nothing matches
the name exactly.
Fixes#5802
* fix(deezer): improve artist disambiguation by ranking exact-case names
Signed-off-by: Deluan <deluan@navidrome.org>
---------
Signed-off-by: Deluan <deluan@navidrome.org>
* feat(playlists): register starred REST filter on playlist repository
* feat(ui): add persisted sidebarPlaylistsOnlyFavourites setting
* feat(ui): playlist favourites heart column and list filter
* feat(ui): favourite heart on playlist details header
* feat(ui): sidebar favourites-only playlist toggle with live refresh
* fix(playlists): qualify id in REST filter to avoid ambiguous column
The annotation join in selectPlaylist made a bare id filter ambiguous, so
GET /api/playlist?id=X (react-admin getMany, used by the sidebar refetch and
useResourceRefresh) failed with 'ambiguous column name: id'. Register
idFilter("playlist") like the album/artist/mediafile repos.
* fix(ui): refresh favourites sidebar on local star toggle
The SSE broker skips the client that originated a star, so the acting client
never got the refreshResource echo and its favourites-only sidebar went stale.
Key the sidebar query on a fingerprint of locally-known starred playlists so a
star/unstar on this client refetches; SSE still covers other clients.
* style(ui): prettier-format PlaylistsSubMenu test
* feat(ui): refine playlist favourites layout
- Move the favourite heart column to just before the edit button
- Space the Playlists sidebar header text from its action icons
- Use a list icon instead of a cog for the playlist-management action
* feat(ui): make the playlist Favourite column toggleable
Move the heart into the toggleable fields map so users can show/hide it
from the column selector like the other optional columns, keeping it last
so it stays just before the edit button.
* refactor(ui): memoize sidebar star fingerprint and gate it on favourites-only
- Derive starFingerprint via useMemo on the playlist data reference instead
of recomputing sort/join inside useSelector on every app-wide dispatch.
- Only include the fingerprint in the query payload when favourites-only is
on, so a star toggle no longer refetches the sidebar when it shows all
playlists.
* fix(ui): don't refetch favourites sidebar on SSE events when showing all
When favourites-only is off the sidebar shows every playlist, so a star
event from another client changes nothing visible. Gate the SSE-driven
refresh counter (and its payload key) on onlyFavourites so the sidebar no
longer redraws on unrelated playlist star events.
* fix(ui): address automated review feedback on playlist favourites
- Ignore a persisted favourites-only preference when EnableFavourites is off,
so disabling the feature later can't strand a filtered sidebar (Codex P2).
- Make PlaylistLove's datagrid header props explicit (source/sortable via
defaultProps, className forwarded) instead of relying on prop pass-through.
- Add aria-label to the SubMenu secondary action button.
- Cover the PlaylistLove list column with tests (Codex P1).
* feat(utils): add ParseDuration/FormatDuration with day and week units
* feat(criteria): add per-playlist refreshDelay to smart playlist rules
* feat(smartplaylist): honor per-playlist refreshDelay in refresh gate
* feat(subsonic): compute smart playlist validUntil from effective refresh delay
* fix(playlists): reset smart playlist evaluation window when rules change via API
* refactor(utils): flatten FormatDuration recursion, single-pass duration regex
* fix(playlists): address PR review feedback
- Reset EvaluatedAt to nil instead of zero-time on rules change and NSP
re-import, so getPlaylist(s) never reports year-1 Changed/validUntil in
the window between an edit and the next owner read
- Parse negative d/w durations so they are rejected with the consistent
"negative duration" error instead of "unknown unit"
- Quote input in the negative-duration error, matching the parse error
- Gate per-playlist RefreshDelay behind IsSmartPlaylist, matching its doc
Update the Jellyfin API README to cover changes that landed after it was
written: add the Lyrics and AudioMuse-AI endpoints to the implemented-endpoints
table, document the AlbumIds filter (Feishin), Recursive=false handling, and
the Jellyfin.MaxConcurrentStreams config option. Mention Symfonium as a client
of the AudioMuse-AI endpoints. Update the lyrics limitation for the
singleflighted cache loader shipped in #5792. Drop two stale known-limitation
entries: sonic similarity (plugin metadata agents already feed
InstantMix/Similar) and MD5-hash ids (the codec round-trip is symmetric, and
with the API unreleased no client can hold a raw un-encoded id).
* fix(scrobbler): tolerate out-of-order playback reports
Clients may fire reportPlayback requests concurrently (Feishin sends
'starting' and 'playing' in parallel, and the previous track's 'stopped'
races the next track's start), so reports can be processed out of order.
Two cases corrupted the now-playing session: a late 'starting' for the
track already playing downgraded the session state, freezing position
estimation at 0:00 until the next report; and a late 'stopped' for the
previous track removed the new track's session and dispatched a playback
report mislabeled with the new track's metadata, causing presence-style
plugins (e.g. Discord Rich Presence) to clear or show stale state.
ReportPlayback now ignores a 'starting' report when the session already
has the same track in playing state, and ignores a 'stopped' report for
a track other than the current session's - skipping both the session
removal and the plugin dispatch, while still counting the play and
dispatching external scrobbles for the stopped track.
Reported in https://github.com/jeffvli/feishin/issues/2131
* fix(scrobbler): serialize session writes to close starting/playing race
The out-of-order 'starting' guard checked the session cache before the
media-file load, leaving a window where a concurrent 'playing' report on a
fresh session could write between the check and the write, and still be
overwritten back to 'starting'. Session check-then-write sections are now
serialized by a mutex, with the guard re-checked after the load. Also
tightens the guard comment to say 'playing session', matching the condition.
Found by Codex review on #5793.
* fix(scrobbler): fully exit ReportPlayback when ignoring out-of-order reports
The out-of-order guards used 'break', which only exits the switch, so the
post-switch NowPlaying block still ran for an ignored 'starting' report and
enqueued a NowPlaying dispatch with the stale report's position - potentially
overwriting a pending correct-position entry, since the queue is keyed by
client. Return nil instead, so ignored reports have no side effects.
Found by Gemini review on #5793.
* fix: dedupe and cap concurrent lyrics plugin fetches
Clients like Finamp prefetch lyrics for several queue tracks at once. The
resulting burst of concurrent plugin calls can rate-limit the primary
lyrics provider into a timeout, making the plugin fall back to a lower
quality source and cache the bad result.
SimpleCache.GetWithLoader now deduplicates concurrent loads of the same
key via singleflight, with every waiter receiving the winner's result or
error. The Jellyfin lyrics loader is detached from the request context so
one cancelled request cannot fail the load for all waiters, and the
lyrics adapter caps in-flight plugin calls at 2 per plugin, queueing the
rest. As a side effect, the cached HTTP client used by the Last.fm,
Deezer and ListenBrainz agents also collapses identical concurrent
requests into a single upstream call.
* fix: harden lyrics concurrency fixes per review
Replace the stringified singleflight keys with a per-cache flight map
keyed by the cache key type itself, eliminating potential key collisions
for non-string keys, the nil-interface assertion panic, and the
stringification overhead. Release the lyrics semaphore slot via defer so
a panicking plugin call cannot leak it, and bound the detached lyrics
load with a one-minute timeout so a hung plugin cannot pin its
singleflight and semaphore slot indefinitely.
* feat(jellyfin): add LyricDto and lyrics mapper
* feat(jellyfin): advertise Lyric media stream for embedded lyrics
* feat(jellyfin): implement GET /Audio/{itemId}/Lyrics
* feat(jellyfin): advertise pipeline-resolved lyrics in PlaybackInfo
* feat(jellyfin): advertise server version 10.9.11 for client lyrics gates
* test(jellyfin): e2e coverage for lyrics endpoint and advertising
Seeds "Stairway To Heaven" with an embedded LRC lyric tag (lyrics:eng)
and covers PlaybackInfo's Lyric MediaStream, GET /Audio/{id}/Lyrics,
and the HasLyrics badge end to end.
Fixes a bug the new seed exposed: HasLyrics and the Lyric MediaStream
gate compared mf.Lyrics against "", but the persistence layer never
stores an empty string post-scan (it normalizes to the JSON sentinel
"[]"), so every track was reporting HasLyrics=true. Both call sites
now parse the column via StructuredLyrics()/LyricList.Main() instead.
* fix(jellyfin): cheap sentinel check for embedded lyrics advertising
* chore(jellyfin): trim over-budget comments in lyrics code
* test(jellyfin): cover lyrics pipeline error and nil-start cue skip
* docs(jellyfin): document lyrics support and follow-ups in README
* refactor(jellyfin): promote embedded-lyrics sentinel check to MediaFile
The "[]" no-lyrics sentinel is persistence-layer knowledge; expose it as
model.MediaFile.HasEmbeddedLyrics() instead of a dto-local helper. Also
dedupe the test lyrics-cache construction and pre-size the media stream
slice.
* refactor(jellyfin): consolidate tick conversions around one constant
ticksPerMillis is now the single source of the 100ns-tick unit; the
scrobble handlers' three inline /10_000 divisions become
dto.MillisFromTicks.
* fix(jellyfin): align lyric advertising with the serving predicate
PlaybackInfo advertised on any non-empty LyricList while the endpoint
404s when the main lyric has no lines; both now share servableLyric.
Handler tests also send hex-encoded ids to match real traffic.
* chore(jellyfin): drop unneeded lyrics package alias in e2e suite
Wire up GET /System/Info returning the previously unused dto.SystemInfo,
available to any authenticated user, matching real Jellyfin's authorization
(FirstTimeSetupOrIgnoreParentalControl, not admin-only). Feishin calls this
endpoint on connect and reads Version to feature-gate; it previously got the
unhandled-route 404.
The advertised version stays 10.8.13: Feishin unlocks structured lyrics and
public-playlist share permissions at >=10.9.0, and this API serves neither
(no lyrics endpoint; playlist user permissions are stubs), so a higher
version would falsely advertise capabilities.
Feishin fetches an album's tracks with AlbumIds=<id>&IncludeItemTypes=Audio&Recursive=true, but the /Items handler never read the AlbumIds parameter, so the request degenerated into the entire library sorted by album: every track (with MediaSources, when requested) was counted and streamed for what should be one album's worth of songs — very slow on large libraries, and wrong results. Parse AlbumIds like GenreIds (comma-separated and repeated spellings, hex-decoded) and filter songs through a new filter.ByAlbumID helper, keeping the album_id column knowledge in the filter package.
Finamp's Radio Mix requests /Items/{id}/InstantMix?limit=250, but getInstantMix
clamped the limit to maxSimilarLimit (100), so the queue was cut to 100 tracks.
A mix is a playback queue, not a "related items" list, so it gets its own
ceiling instead of sharing the Similar one. The Similar handlers keep 100.
Verified against a live library: the sonic provider supplies all 250 tracks for
a seed that previously returned 100.
* refactor(jellyfin): inject core/sonic into the Jellyfin Router
* feat(jellyfin): add AudioMuse /info endpoint
* feat(jellyfin): add AudioMuse /similar_tracks endpoint
* feat(jellyfin): gate AudioMuse endpoints on sonic provider
* feat(jellyfin): add AudioMuse /find_path endpoint
* fix(jellyfin): fix case-insensitive route collision across positions
canonicalRouteSegments keyed canonical case by lower-cased segment name
alone, globally. Two unrelated routes sharing a segment name with
different casing at different tree depths (e.g. "Info" in
/System/Info/Public vs "info" in /AudioMuseAI/info) silently overwrote
each other, 404-ing the loser even for exact-case requests. Replace the
flat map with a position-aware trie mirroring the routing tree.
* test(jellyfin): e2e tests for AudioMuse endpoints
* docs(jellyfin): document AudioMuse compatibility endpoints
* test(jellyfin): harden AudioMuse tests and doc note (final-review follow-ups)
- Comment-lock the []string{} (not nil) contract for /AudioMuseAI/info's
AvailableEndpoints so it keeps serializing as [] rather than null, and
add a raw-body assertion to the existing empty-list test to catch a
regression a struct-only unmarshal can't detect.
- Cover the previously-untested engine-error branch in similar_tracks and
find_path, both of which degrade to an empty result.
- Document that find_path's path/total_distance only reflect hops through
libraries the caller can access in multi-library setups.
* refactor(jellyfin): dedup AudioMuse test request helper, presize dedup map
* refactor(sonic): expose sonic.Engine interface; drop typed-nil guard in jellyfin.New
The Jellyfin Router's sonic field was an interface but New() took the concrete
*sonic.Sonic, so a nil arg became a non-nil typed-nil and needed a guard — the
only injected dependency that did. Move the interface (sonic.Engine) beside its
implementation, take it in New() like every other service, and bind it in wire.
* refactor(jellyfin): case-insensitive routing via lowercased paths
Replace the position-aware route trie with a trivial middleware that lowercases
the request path, and register every route in lowercase. Simpler, and no segment
name can collide across positions. caseInsensitivePaths moves into middlewares.go
alongside normalizeQueryKeys. Relies on the invariant that no Jellyfin path segment
carries case-sensitive data (all ids are lowercase hex via dto.EncodeID).
* feat(jellyfin): add AudioMuse /health endpoint
A liveness probe matching the reference plugin: 200 with an empty body when a
SonicSimilarity provider is loaded, 404 otherwise. /AudioMuseAI/info now
advertises it (list alphabetized like the plugin's OrderBy).
Also trims the AudioMuse and case-insensitive-routing comments to their essential
rationale.
* fix(jellyfin): hex-encode user IDs so lowercased paths stay valid
Address PR review: user IDs were the one id the Jellyfin API emitted raw (base62,
uppercase-capable), so lowercasing request paths could alter a userId segment. Encode
them via dto.EncodeID like every other id, making the 'all boundary ids are lowercase
hex' invariant true — no routing special-casing needed. Also caps user-controlled n /
max_steps, fixes the songAgent test comment, and adds leading slashes to the README
endpoint list.
* fix(jellyfin): honor Recursive=false for library parents
/Items ignored the Recursive parameter entirely — it appeared only in tests,
never in production code. Finamp's download sync sends exactly one
Recursive=false request per library (ParentId=<library>&IncludeItemTypes=Audio)
to pick up tracks outside any album, supplementing its recursive per-album
fetch. We answered with every song in the library: 208MB and 21s per sync
measured on a real library, against 20KB for the album queries. Finamp then
required every track twice, once via its album and once under the library node.
In Jellyfin 10.10 Recursive never reaches SQL. It selects between an in-memory
walk of a folder's direct Children (false) and a DB ancestor query (true), with
IncludeItemTypes applied as a post-filter over that direct-child list
(ItemsController.cs:308, Folder.cs:949-994). The default is false, and neither
IncludeItemTypes nor SearchTerm forces recursion, so Recursive=false with
IncludeItemTypes=Audio on a music library returns an empty list — which is what
Finamp expects and codes for.
Filter the requested types to those nested directly under a library when the
parent is a library and Recursive is not true; the hierarchy this API exposes is
library -> album -> track, so no track is ever a library's direct child. Album
and playlist parents are untouched, as their tracks are real direct children in
Jellyfin (MusicAlbum.cs:86-89, Playlist.cs:140-167) and Jellify opens playlists
with Recursive=false. A library parent is the only case handled: /Items with no
ParentId still returns every song where Jellyfin returns the root's children,
but no observed client sends that, and honoring it would surprise any client
that simply omits Recursive.
* refactor(jellyfin): narrow the Recursive=false filter to Audio
Replace the libraryChildTypes allowlist with a direct Audio check. The two are
behaviorally identical — parseTypes only ever yields Audio, MusicArtist,
MusicAlbum, MusicGenre or Playlist, and the allowlist held the latter four, so
it excluded exactly Audio and nothing else.
The allowlist claimed those four are a library's direct children. That isn't
true of MusicGenre or Playlist: listGenres is deliberately unscoped because
genres are global tags, and listPlaylists ignores scopeIDs entirely, so neither
is nested under a library at all. They were on the list only to leave their
behavior untouched. The one verified invariant is that no track is a library's
direct child, so state just that — it is also more conservative, as a type added
later keeps its current behavior instead of being filtered by a stale list.
Add a test pinning the omitted-Recursive default: ItemsController binds
`bool? recursive` and reads it as `recursive ?? false`, so omitting the param is
a non-recursive request and filters Audio for a library parent.
* fix(jellyfin): stream playlist tracks instead of loading every one
PR #5783 left the playlist paths materializing: all three loaded every track
of a playlist, whatever the client asked for. A playlist can be the whole
library — a smart playlist matching everything — so this is the same OOM class
that PR fixed for the other collections. Measured on a 96k-track smart playlist,
/Items?ParentId=<playlist>&Limit=10 peaked at 1.3GB to return ten tracks.
Playlist tracks now stream from a cursor, like every other collection:
- PlaylistTrackRepository gains GetCursor and CountAll, sharing the select
builder with loadTracks so cursor rows hydrate identically, plus
GetMediaFileIDs for callers that need every id but no track data.
- playlists.Tracks(ctx, id) exposes that repo to the HTTP layer with visibility
enforced, returning ErrNotFound rather than the repo's nil-and-log-a-warning
(which /Items would hit on every album browse, since ParentId is usually not
a playlist).
- /Playlists/{id}/Items now honors StartIndex/Limit, which it silently ignored
before — it always returned the whole playlist. Real Jellyfin pages it.
- getPlaylist runs an id-only query: PlaylistInfo carries every track id so it
can't be paged, but it no longer hydrates rows it discards.
- The route joins the throttled group, as it's now cursor-backed.
Measured against a copy of a 96k-track production DB, peak RSS over idle, with
byte-for-byte identical responses on every endpoint:
/Items?ParentId=<pl>&Limit=10 1275MB -> 1MB 8.8s -> 3.6s (0.27s warm)
/Items?ParentId=<pl> unbounded 1353MB -> 8MB 8.7s -> 3.4s
/Playlists/<pl>/Items 1217MB -> 7MB 8.8s -> 3.4s
/Playlists/<pl> 1178MB -> 33MB 9.1s -> 3.8s
TotalRecordCount now costs a count query where the old path got it from
len(tracks): 6ms on the largest real playlist in that library (2638 tracks),
288ms on the synthetic all-96k one. The old path paid 1.3GB and 4s+ instead.
* fix(jellyfin): bound unbounded /Items searches
The other collections stream, so an unbounded one costs about one item of
memory. Search can't: the repositories' Search returns a slice, so it
materializes every match. PR #5783 left two ways to reach that.
A whitespace-only SearchTerm was the first. " " != "", so it took the search
path, where doSearch trims it back to empty and hits its "empty query, return
everything in natural order" branch — with no LIMIT, since executeTwoPhase only
applies one when Max > 0. The whole library, materialized. Trimming at the two
parse sites makes the `search != ""` checks mean what they look like they mean:
a blank term is not a search, so it takes the unfiltered streaming path, which
still returns everything, exactly as real Jellyfin does for an empty term.
A real search with no Limit was the second, and needs an actual bound. Search
gets a default of 100 when the client sends no Limit — matching the
DefaultSearchLimit in Jellyfin's unreleased SqlSearchProvider — plus a ceiling,
without which Limit=999999 would still materialize the library. The default
alone wouldn't have closed the hole. An explicit Limit under the ceiling is
honored unclamped, as upstream does; truncating a search is safe in a way
truncating /Items is not, since nothing syncs a library through searchTerm.
Note this is upstream's own bug: v10.11's /Search/Hints returns the entire
library for a whitespace-only term, which master fixed by switching to
ThrowIfNullOrWhiteSpace.
* fix(jellyfin): cap the search Limit the client asked for, not the merge window
searchPage sees two different things in opts.Max: the client's Limit for a
single-type query, and mergeTypes' internal offset+limit window for a multi-type
one. Clamping there hit both, so a multi-type search paging past the ceiling
fetched only `ceiling` rows of the first type and the merged page skipped into
the next one — IncludeItemTypes=Audio,MusicAlbum&SearchTerm=song&StartIndex=2000
&Limit=1 returned an album instead of the 2001st song.
The ceiling now applies where the client's Limit is read: queryItems (after the
playlist branch, so a playlist parent's page isn't capped by a stray SearchTerm)
and getArtists, which reads its own. A limit of 0 stays 0, keeping searchPage's
default. What a deep page materializes is then bounded by the client's
StartIndex, as it already was for any multi-type query, search or not.
Also from review: the playlist-track mock reused the previously stored Options
when called without any, so a later no-args call inherited stale paging.
* fix(jellyfin): apply the search default to the client's Limit, not per type
The default lived in searchPage, which runs per type and after mergeTypes has
already picked its branch. So an unbounded multi-type search left q.limit at 0,
mergeTypes took its chained branch, and StartIndex was applied to a list each
type had already truncated to the default — dropping matches rather than paging
them. With 200 matching songs, IncludeItemTypes=Audio,MusicAlbum&SearchTerm=song
&StartIndex=150 returned nothing at all, and without StartIndex it returned the
default per type instead of in total.
clampSearchLimit now applies the default and the ceiling together, to the
client's Limit, at the two places it's read (queryItems and getArtists). A search
therefore always gives mergeTypes a real window, so it pages the merged result
and cuts it once, at the end.
* fix(jellyfin): bound the multi-type search window against StartIndex
mergeTypes asks each type for offset+limit rows before paginating the merged
list, so a search still materialized whatever StartIndex asked for:
IncludeItemTypes=Audio,MusicAlbum&SearchTerm=x&StartIndex=500000&Limit=1 pulled
~500001 matches per type. Bounding the window alone isn't enough — below it the
merged rows are the client's page, but at it a truncated type is followed by the
next one's rows, which is what made a clamped window serve an album where the
2001st song belonged.
So the window is capped at maxSearchLimit and the page is clipped to it: pages
below the ceiling are served in full and unchanged, a page straddling it is cut
at it, and past it the result is empty rather than another type's rows. The
total reports what can actually be paged to, so a client stops instead of asking
for pages that no longer exist.
This is the merge's own limit, not the client's: a single-type search still pages
as deep as it likes, since its offset goes to SQL. Non-search multi-type queries
keep the unbounded offset+limit window, which predates this and wants the same
treatment via CountAll (exact per-type totals let whole types be skipped) rather
than a cap.
* fix(jellyfin): advertise the pageable search total, not the page window
Clipping the multi-type search total to `window` clipped it to StartIndex+Limit
on an ordinary page, so a first page of Limit=10 reported TotalRecordCount 10
however many matches there were, and a client paging on the total stopped after
one page. The cap belongs at the ceiling — what can be paged to overall — not at
the current page.
The tests missed it because the only one asserting a total used
StartIndex=maxSearchLimit, where the window happens to equal the ceiling.
* refactor(jellyfin): fold the search clamp into clampLimit and simplify mergeTypes
Cleanup pass over the branch, no behaviour change:
- clampSearchLimit was clampLimit (similar.go) with different constants, so the
latter takes the default and ceiling as arguments and both call it. Similar's
default moves out of three IntOr calls into defaultSimilarLimit.
- mergeTypes derived a second `limit` and needed an early return for the empty
page, only because paginate reads 0 as "unbounded". Clipping the merged slice
to the window instead lets q.limit be passed straight through: window is
min(offset+limit, ceiling), so clipping there is the same cut.
- playlistTracks returned (result, handled, error) where handled=false always
meant error=nil. Splitting the lookup out gives playlistTracksRepo returning
(repo, ok), and the nilerr suppression goes with it.
- The comment on playlists.Tracks blamed the log warning for its extra Get; the
reason is that PlaylistRepository.Tracks discards the error behind a nil. Also
drops a stale reference to a renamed variable.
* fix(jellyfin): stream /Items responses to prevent OOM on large libraries
Finamp's library sync issues an unbounded GET /Items?IncludeItemTypes=Audio
with Fields=MediaSources and no Limit. On a large library this built the whole
result set — every MediaFile and every BaseItemDto, each fat with MediaSources —
in memory, and json.Encoder then buffered the entire ~200MB response before
writing a byte. Measured on a 96k-track library, one such request peaked near
1.6GB RSS; in a memory-limited container the resulting slowness made clients
retry, stacking concurrent full-library builds until the process was OOM-killed.
Stream the song listing straight from a DB cursor (MediaFileRepository.GetCursor),
mapping and encoding one row at a time, so peak memory is bounded to about one
item regardless of library size. The cursor is opened lazily, when streaming
begins, so it doesn't hold a DB connection across the CountAll and ServerId
lookups that run first (ServerId can write on first use and would deadlock
against an open reader). Pagination still works — the cursor query carries
LIMIT/OFFSET/ORDER BY from StartIndex/Limit/SortBy — and TotalRecordCount stays
the full count. Other materialized responses stream their JSON too, avoiding the
encoder's buffer-the-whole-output cost.
Also bound artwork image concurrency with the same server.ThrottleBacklog that
Subsonic's getCoverArt uses, so a burst of image requests during sync can't
exhaust memory through concurrent decode/resize.
Verified against a copy of a 96k-track production database: byte-for-byte
identical response, peak RSS 1593MB -> 53MB, time-to-first-byte 8.1s -> 0.2s.
* fix(jellyfin): fail loudly on /Items streaming errors instead of a truncated 200
Addresses code review: a streamed /Items response commits HTTP 200 before an
error can occur, so failures were surfacing as misleadingly successful bodies.
- A cursor-open failure (e.g. a busy DB under connection pressure) is now
surfaced before the first byte is written: the cursor open is deferred into
the item source and run in writeItems after the ServerId lookup but before the
envelope, returning a clean 500 the client can retry — not a 200 with an empty
item list.
- A mid-stream (row-scan) error now aborts without closing the JSON envelope,
leaving the body malformed. A truncated-but-valid response would let a sync
client (Finamp) treat the short list as the whole library and prune local
tracks; malformed JSON forces the client's parser to fail and retry.
* refactor(jellyfin): stream every collection endpoint from a DB cursor
Streaming was applied only to the song listing, which left two write paths, two
ServerId stamping mechanisms and a listXxx return-type split ("songs is
special") that made the code hard to follow.
Add GetCursor to the album, artist, genre and playlist repositories, mirroring
MediaFileRepository.GetCursor: same select builder as GetAll, so each cursor
yields identical, fully-hydrated rows (hydration happens per-row in PostScan,
and toModels is only a deref loop). The playlist cursor keeps GetAll's
owner/public visibility filter. Each repository test asserts the cursor yields
exactly what GetAll returns, including Max/Offset.
With cursors everywhere, every listXxx returns itemsResult and every collection
streams through one writer:
- listAlbums, listArtists and listPlaylists now stream from their cursors;
search paths stay materialized (Search returns a slice).
- listGenres stays materialized: its total is the length of the full list and it
paginates in memory, so there is nothing for a cursor to page over.
- api.ok's QueryResult case delegates to writeItems, so the ~9 inline callers
funnel into the same writer; streamQueryResult and wrapResult are gone.
- /Items/Latest streams as a bare JSON array (its Jellyfin wire shape) via
writeItemsArray, which shares the item loop and stamping. That also closes the
same unbounded hole /Items had: limit=0 disabled its LIMIT.
- ServerId is now stamped in exactly one place, so api.ok only handles single,
non-collection payloads.
Verified against a copy of a 96k-track production database: every endpoint
byte-for-byte identical to master. Unbounded /Items peak RSS 1691MB -> 54MB;
time-to-first-byte 6.3s -> 0.19s, /Artists 1.14s -> 0.13s.
* refactor(jellyfin): route every response through api.ok
Handlers were split between api.ok and api.writeItems with no clear rule for
which to call. api.ok now accepts itemsResult too, so it is the single entry
point: callers hand it whatever they have and it routes collections (cursor
-backed or materialized) to the streaming writer. writeItems is reached only
through api.ok now. The one exception is /Items/Latest, which returns a bare
JSON array rather than a QueryResult envelope and so writes directly — noted in
both doc comments.
Also drop GenreRepository.GetCursor: listGenres derives its total from the
length of the full list and paginates in memory, so there is nothing for a
cursor to page over, leaving the method unused.
* fix(jellyfin): stream the unbounded multi-type /Items merge
The multi-type merge capped each per-type query at offset+limit only when a
Limit was given. Without one (Finamp's favorites screen sends multi-type), every
type ran an unbounded query and collect() drained each cursor into a slice — so
/Items?IncludeItemTypes=MusicAlbum,Audio with no Limit materialized every album
and every song, the same OOM class this branch set out to fix. The comment
claiming the set was "capped at offset+limit per type" was wrong for limit=0.
Without a limit the merged page is just each type's rows in order minus the
first offset, which is exactly what chaining the per-type cursors yields, so
stream that instead of merging in memory. The bounded path is unchanged: with a
Limit each type holds at most offset+limit rows, so merging and paginating
across the combined list is safe. Cursors are opened one at a time (each pins a
DB connection until drained), with the first opened eagerly so the usual failure
is still a clean error before any byte is written.
Measured on a 96k-track library, /Items?IncludeItemTypes=MusicAlbum,Audio with
no Limit (103,393 items): peak RSS 612MB -> 53MB, time-to-first-byte 4.9s ->
0.6s, response byte-for-byte identical.
* refactor(jellyfin): parse /Items params into a struct
The /Items dispatcher was a single 90-line block that parsed a dozen params and
then threaded them positionally through three layers — queryItemsOfType took 11
arguments, listSongs and listAlbums 9 each — so reading any one of them meant
decoding a long argument list.
Parse once into an itemsQuery and pass that instead. queryItems now reads as the
four things it actually does: parse, the id/playlists-folder/playlist-parent
special cases, single-type dispatch, multi-type merge — with the playlist-parent
and merge bodies moved to playlistTracks and mergeTypes. Every listXxx takes
(ctx, opts, q).
No behavior change: parsing, ordering and the entityParent rule are unchanged,
and /Artists still passes favOnly=false explicitly by building the subset of the
query it uses.
* docs(jellyfin): trim comments on the streaming path
Cut the comments back to the non-obvious why: the deferred cursor open (the
ServerId write would deadlock against an open reader), the deliberate malformed
JSON on a mid-stream error, why chained opens one cursor at a time, why
listGenres stays materialized, and why the cursor helpers take the underlying
func type. Dropped the rest — restatements of the code, doc comments that
repeated a test's own name, and the GetCursor interface comments that the
existing MediaFileRepository.GetCursor does without.
* refactor(persistence): fold the cursor wrappers into a generic wrapCursor
wrapAlbumCursor, wrapArtistCursor, wrapMediaFileCursor and wrapFolderCursor were
the same twelve lines four times over, differing only in the type name, and the
playlist cursor had its own inline copy of the loop.
Add one generic wrapCursor. It takes an extractor func rather than a method on
an interface: a type parameter can't reach an embedded field, and methods that
only satisfy a generic constraint are reported by the unused linter. The
extractor also lets both type parameters be inferred, so call sites need no
explicit instantiation. Each entity keeps its named wrapper as a one-liner,
since the model cursor types are defined types and need the conversion — and the
existing wrapper tests call them directly.
The nil-row error now names the model type via %T ("unexpected nil model.Album")
instead of a hand-written per-entity string; the three tests asserting that
message are updated.
* fix(jellyfin): bound concurrent collection streams
A streamed collection holds a DB cursor — and its pooled connection — for the whole client-paced
response, where the old materialize-then-write path released it as soon as the query finished. The
pool is shared with the scanner, Subsonic, the native API and the UI, so enough slow clients take
every connection and everything else blocks waiting for one. Measured against a copy of a 96k-track
production DB, 20 concurrent unbounded streams against a pool of 16 stalled a write for 16.2s (the
other 14 writes in the run took ~2ms — the signature of connection starvation, not lock contention).
Cap concurrent streams at half the pool. Excess requests queue rather than fail, so no client is
rejected: the same 20 streams now all complete and the worst write is 2.5ms.
- conf.MaxOpenConns() now owns the pool sizing (db calls it). It belongs in conf: the pool is a
tunable, db already imports conf, and putting it in db would force server/jellyfin to import db
just to size the cap against it. Expressing the cap as MaxOpenConns()/2 also keeps the two from
drifting apart.
- Uses chi's ThrottleBacklog, not server.ThrottleBacklog: the latter buffers the whole response to
release its token early, which is right for artwork but would undo the streaming. chi's panics on a
non-positive limit, so throttleStreams guards it — setting MaxConcurrentStreams=0 disables the cap
instead of crashing the server at startup.
* refactor(jellyfin): move throttleStreams to middlewares.go
It's a middleware, so it belongs beside normalizeQueryKeys, authenticate and
withPlayer rather than in api.go. Its tests move to middlewares_test.go with it,
keeping one test file per production file.
* fix(jellyfin): abandon the scan when the client stops reading
encodeItems discarded its write errors and relied on the final Flush to report
them, so once bufio's buffer filled and the flush failed, the loop still pulled
every remaining row through the cursor and serialized it for a client that was
gone. A test with a failing writer confirms it: all 20k items were drained.
That wastes CPU, and holds the cursor's pooled DB connection and a stream slot
(now a capped resource) for the length of a full scan nobody is reading. Check
the per-item writes so the first failure ends the scan; the fixed envelope
writes stay unchecked, since bufio latches for them anyway.
* feat(sharing): enable sharing by default
Flip the EnableSharing default from false to true so new installations have the sharing feature available out of the box. Users can still disable it via the EnableSharing config option.
The native API only registers the /share route when sharing is enabled, so the nativeapi tests that build the router without wiring a share service now explicitly disable sharing in their setup to avoid registering a route backed by a nil service.
* feat(jellyfin): add config flag and URL path constant
Adds the disabled-by-default Server.Jellyfin config option (Enabled,
ServerName) and consts.URLPathJellyfinAPI, following the existing
LastFM/ListenBrainz patterns. Later tasks will use these to mount the
Jellyfin-compatible API router.
* fix(jellyfin): default Jellyfin ServerName to "Navidrome"
* feat(jellyfin): package skeleton with System handshake endpoints
Adds server/jellyfin: the Router (mirrors server/subsonic and
server/public), its Wire-friendly New(...) constructor, a chi routes()
table, and the ok() JSON response helper. Implements the unauthenticated
handshake surface Jellyfin clients probe first: GET /System/Info/Public,
GET+POST /System/Ping, and GET /QuickConnect/Enabled (quick connect is
unsupported, so it always reports disabled).
The public info payload's Id must be stable across restarts (Jellyfin
clients cache ServerId), so it reuses the get-or-create Property pattern
already used for InsightsID in core/metrics/insights.go: fetch
consts.JellyfinServerIDKey from the Property repository, generating and
persisting a new UUID on first read. A dedicated key (rather than the
existing InsightsID) keeps the anonymous telemetry identifier from being
exposed on this unauthenticated endpoint.
* test(jellyfin): cover ping and quickConnectEnabled handlers
Both were added alongside the System/Info/Public handshake endpoint but
had no direct test coverage.
* fix(jellyfin): memoize stable server Id and cover get-or-create path
* feat(jellyfin): wire and mount the router behind Jellyfin.Enabled
Add jellyfin.New to the shared Wire provider set and a
CreateJellyfinAPIRouter injector, then mount the router at /jellyfin
in startServer(), gated by conf.Server.Jellyfin.Enabled, mirroring the
existing LastFM/ListenBrainz router mounts.
* feat(jellyfin): add Jellyfin DTOs and model mappers
Adds BaseItemDto, QueryResult, UserItemDataDto, UserDto,
AuthenticationResult, MediaSourceInfo, PlaybackInfoResponse,
NameGuidPair and SessionInfo DTOs to server/jellyfin/dto, plus
model-to-DTO mappers for songs, albums, artists and genres that
later browse/stream/write endpoints will consume.
* test(jellyfin): cover GenreToBaseItem mapper
* feat(jellyfin): advertise Jellyfin-compatible version, brand ServerName with Navidrome version
* fix(jellyfin): map LastPlayedDate and omit zero track/disc numbers
* feat(jellyfin): authentication middleware and AuthenticateByName login
* fix(jellyfin): reject empty login password, wire ServerName, add negative auth tests
* refactor(jellyfin): use new(x) builtin instead of intPtr helper
* feat(jellyfin): user views and current-user endpoints
* feat(jellyfin): Items query engine, item detail, and latest
Adds the /Items universal query endpoint, dispatching by IncludeItemTypes
over albums/artists/songs/genres with ParentId, SearchTerm, Filters=IsFavorite,
SortBy/SortOrder and StartIndex/Limit support, plus GET /Items/{itemId} and
/Users/{userId}/Items/Latest. Reuses server/subsonic/filter builders (by
artist/album/starred) instead of hand-rolled squirrel filters, and resolves
sort keys per item type since each repository maps sort names to different
real/aliased columns.
Also adds the missing CountAll to tests.MockArtistRepo, exposed by this task
(the mock embedded a nil ArtistRepository for it and would panic on use).
* refactor(server): extract shared query filter builders to server/filter
Moves server/subsonic/filter to server/filter so server/jellyfin can use
the shared query-option builders without importing server/subsonic,
enforcing the rule that no API package imports another API's package.
* feat(jellyfin): full multi-library support with per-user access scoping
Replace the single hardcoded "music" UserView with one CollectionFolder
view per library the user can access, and scope every /Items browse
query (albums, songs, artists, /Latest) to those libraries via
server/filter's ApplyLibraryFilter/ApplyArtistLibraryFilter.
ParentId is now disambiguated: a numeric value the user has access to is
treated as a library scope (browsing a UserView), otherwise it falls
through as an entity id (artist/album), which safely matches nothing
rather than leaking another library's content. getItem now 404s when
fetching an album or song outside the user's accessible libraries;
artists are skipped (they can span multiple libraries) with a TODO.
Genres remain unscoped since they're global tags, not per-library
entities.
* test(jellyfin): cover admin library-access path and drop nil test contexts
* feat(jellyfin): Artists and Genres endpoints
Add /Artists, /Artists/AlbumArtists, /Genres and /MusicGenres. Artists
listing is library-scoped (defaulting to the user's accessible
libraries, narrowed by an accessible ParentId), delegating access
control to listArtists/ApplyArtistLibraryFilter. Genres are global and
unscoped, matching the ML decision already made for listGenres.
* refactor(jellyfin): share resolveLibraryScope between items and artists
* feat(jellyfin): item image endpoint via artwork service
* fix(jellyfin): let net/http sniff image Content-Type instead of forcing jpeg
* feat(jellyfin): audio streaming and PlaybackInfo with library access control
* feat(jellyfin): favorites and rating write-back with library access control
Adds POST/DELETE handlers for /Users/{userId}/FavoriteItems/{itemId} and
/Users/{userId}/Items/{itemId}/Rating. A shared resolveAnnotated helper
probes album/artist/media file (mirroring getItem's order) and 404s before
writing if the user lacks access to the album/song's library; artists are
exempt since they span multiple libraries. Ratings are halved coming in
(Jellyfin 0-10 -> Navidrome 0-5) to match the doubling in dto.UserData.
Also teaches MockMediaFileRepo and MockArtistRepo's SetStar/SetRating (and
MockAlbumRepo's, previously a no-op) to actually mutate the backing data so
write-back can be asserted in tests.
* feat(jellyfin): playback reporting and scrobbling
Add /Sessions/Playing[/Progress|/Stopped] and /Sessions/Capabilities[/Full]
handlers, backed by core/scrobbler.PlayTracker. A new withPlayer middleware
resolves/registers a model.Player from the Emby DeviceId header (used
directly as the stable player id, unlike Subsonic's cookie fallback) and
injects it into the request context for scrobbling.
The Stopped report calls ReportPlayback with IgnoreScrobble to end the
now-playing session without double-counting, then calls Submit as the
single source of the play-count increment and external scrobble.
* feat(jellyfin): playlist read and write-back
Adds POST /Playlists, GET/POST/DELETE /Playlists/{id}/Items. Tags each
playlist item with PlaylistItemId (the entry's position within the
playlist) so DELETE .../Items?EntryIds=... can remove a specific
occurrence by the id core/playlists.RemoveTracks actually expects,
rather than the song id used everywhere else.
* test(jellyfin): cover empty Ids/EntryIds in playlist add/remove
* feat(jellyfin): unknown-route logging, generic 500s, rating clamp, docs
Hardening pass ahead of real client testing: unmatched routes and
unsupported methods now return a logged, JSON 404 instead of chi's
default plain-text response, so a missing endpoint a client needs is
easy to spot in the logs. Internal errors (ffmpeg output, file paths,
etc.) no longer leak into 500 response bodies -- a shared
internalError helper logs the real error server-side and always
returns a generic message. Inbound Jellyfin ratings are clamped to
0-10 before being halved into Navidrome's 0-5 scale, and /System/Ping
now replies with a bare plain-text body as real Jellyfin servers do.
Adds a README with an enable/curl walkthrough and known limitations.
* docs(jellyfin): clarify public image endpoint and accessibleLibraryIDs comments
* fix(jellyfin): case-insensitive path routing for Jellyfin client compatibility
* refactor(server): extract case-insensitive path routing to a shared helper
* fix(jellyfin): return User Policy and Configuration so Finamp completes login
* fix(jellyfin): support Playlist type, multi-type Items queries, and PlayCount/DatePlayed sort
Real Finamp requests break against three /Items query engine bugs: Playlist
requests fell through to albums, multi-type IncludeItemTypes (e.g. Finamp's
favorites screen) only returned the first requested type, and comma-separated
SortBy lists (e.g. "DateCreated,SortName") were matched as one opaque string
so they always fell through to the repo default.
Also extends tests/mock_playlist_repo.go with SetData/GetAll so playlist
listing is testable like the other mock repos.
* feat(jellyfin): implement /socket WebSocket for real-time client sessions
* fix(jellyfin): resolve library-view ids in getItem so clients can load the library
* fix(jellyfin): serve direct file at /Items/{id}/File and accept ApiKey query param
* fix(jellyfin): resolve playlist ids in getItem
* fix(jellyfin): read lowercase ids/entryIds playlist params and add playlist-users endpoints
* fix(jellyfin): include MediaSources with Size/Bitrate on tracks so clients show download size
* fix(jellyfin): populate all required MediaSourceInfo bool/array fields to match Jellyfin
* fix(jellyfin): hex-encode item ids at the API boundary for Jellyfin client compatibility
Finamp (and presumably other clients) parses ids as radix-16, but Navidrome's
base62 nanoids aren't valid hex and crash its queue packing. Hex-encode every
id emitted by the Jellyfin API and decode every id received, keeping the
transform reversible and stateless at the boundary rather than touching any
model id.
* fix(jellyfin): populate MediaStreams with the audio stream so clients can size/transcode
* fix(jellyfin): support Ids batch-fetch in /Items so clients can fetch items by id
* fix(jellyfin): do not disable Jellyfin server when disabling external services
* fix(jellyfin): emit per-image ImageBlurHashes so clients de-dupe images and stop warning
* fix(jellyfin): support playlist cover upload/delete and display via item image endpoints
* feat(jellyfin): implement GET /Playlists/{id} for playlist visibility
Finamp's playlist edit screen calls GET /Playlists/{id} to read the
OpenAccess (public visibility) flag; we only had the sub-routes, so it
404'd and the edit screen failed to load. Return the Jellyfin PlaylistDto
shape (OpenAccess from Public, empty Shares, media item ids).
* fix(jellyfin): display playlist cover art
Uploaded playlist covers never showed in clients for two reasons: the
playlist BaseItemDto advertised no Primary ImageTag (so clients didn't
know to fetch a cover), and the public image endpoint resolved artwork
under the request's anonymous context, so a private playlist failed its
visibility filter and fell back to the placeholder. Advertise the Primary
image tag/blurhash on playlists, and resolve artwork under an elevated
context (as core/artwork's cache warmer does).
* 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.
* fix(jellyfin): support playlist deletion via DELETE /Items/{id}
Finamp deletes a playlist with DELETE /Items/{id}, which we didn't route,
so deletion silently failed. Implement it via core/playlists.Delete (which
enforces ownership and removes the cover file). Only playlists are
deletable through this API; non-playlist ids return 404, non-owners 403.
* test(jellyfin): add e2e suite harness + smoke tests
* test(jellyfin): e2e for system, auth, routing, browsing, annotations
* test(jellyfin): e2e for playlists (CRUD, expansion, cover) and item images
* test(jellyfin): e2e for streaming, sessions, and multi-user access control
* fix(jellyfin): artist search 500 (library filter leaked into FTS query)
getArtists/listArtists reused the browse-path filters (notMissing +
ApplyArtistLibraryFilter) for the search path, but artist Search expects a
sole Eq{library_id} filter it can consume as a scope — artists have no
library_id column, so the compound/join filter leaked into the FTS query
and 500'd. Every Finamp artist search failed (the filter is applied even
unscoped, since admins resolve to all library ids). Build search filters
separately. Adds e2e search coverage.
* feat(jellyfin): implement POST /Playlists/{id} to update name, visibility, tracks
Finamp edits a playlist (make public, rename, reorder) via POST
/Playlists/{id}, which we didn't route, so every edit 404'd. Implement it:
Ids present -> replace track list (Create with existing id, preserving
name); otherwise update Name/IsPublic via core/playlists.Update. Adds a
shared playlistError helper (403/404/500) reused by deleteItem, plus e2e
coverage.
* docs(jellyfin): update README for playlists, images, id encoding, and e2e
* fix(jellyfin): default album track listing to track order
Browsing an album's tracks (Items?ParentId=<albumId>&IncludeItemTypes=Audio)
took only SongsByAlbum's filters and dropped its Sort, so tracks came back
in arbitrary order. Default opts.Sort to the album (disc+track) order when
browsing an album without an explicit SortBy — matching Subsonic's GetAlbum
and real Jellyfin. An explicit SortBy still wins.
* 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.
* fix(jellyfin): only count a play past the scrobble threshold
reportPlaybackStopped set IgnoreScrobble and force-submitted a play on
every Stopped report, so a briefly-played track (e.g. an immediate skip)
was marked played. Finamp sends Stopped on every track switch, so the
threshold must be applied server-side. Let ReportPlayback's StateStopped
logic decide (play + scrobble only past 50% of the track / 4-minute cap)
instead. Subsonic differs because there the client gates submission.
* fix(jellyfin): sort album tracks by track number when client sends IndexNumber SortBy
Finamp's album view requests SortBy=ParentIndexNumber,IndexNumber,SortName
(disc, track, name), but applySort didn't recognize ParentIndexNumber or
IndexNumber and fell through to SortName, sorting tracks alphabetically by
title. Map both keys to the album (disc+track) sort. The reversed-title
fixture lets the e2e tell track order from title order.
* fix(jellyfin): honor the isFavorite query param for favorites filtering
Finamp's artist 'Favourite tracks' widget requests favorites via the
standalone isFavorite=true query param, not Filters=IsFavorite, so the
filter was ignored and non-favorited tracks were returned. Detect both
forms. (The widget's reshuffling is Finamp's explicit SortBy=Random.)
* feat(jellyfin): emit DateCreated (Date Added) on items
BaseItemDto had no DateCreated, so clients showed 'No Date Added' and had
nothing to sort 'Recently Added' by. Emit it as ISO 8601 from each entity's
CreatedAt (the same field the recently_added sort uses) for songs, albums
and artists.
* fix(jellyfin): set ArtistItems/AlbumArtists on songs (Now Playing artist)
SongToBaseItem only set Artists (names) and AlbumArtist, not the structured
ArtistItems/AlbumArtists. Finamp's Now Playing screen reads ArtistItems and
shows 'Unknown Artist' when it's absent. Populate both (track artist and
album artist) as name+id pairs, mirroring AlbumToBaseItem.
* docs(jellyfin): note synthetic blurhash as a follow-up
Document that ImageBlurHashes are derived from the item id (a solid-color
placeholder), not computed from the cover art like real Jellyfin, and
outline what a proper implementation would take.
* docs(jellyfin): note WebSocket events and favourited playlists as follow-ups
* fix(jellyfin): filter the Artists page by role (album artist vs performer)
/Artists and /Artists/AlbumArtists both called the same role-agnostic
handler, so Navidrome's per-role artist entries (composers, arrangers,
performers) all showed as Album Artists, and the two tabs were identical.
Filter /Artists/AlbumArtists to RoleAlbumArtist and /Artists to RoleArtist
(and the MusicArtist browse to album artists), via filter.ArtistsByRole.
Verified live: Beatles Singles' album-artists dropped 138->5, composers
excluded, performers (Billy Preston) show only under /Artists.
* fix(jellyfin): sort playlists by name (missing Playlist sort mapping)
applySort had no sortColumnsByType entry for the Playlist type, so
SortBy=SortName was ignored and the Playlists screen showed them in the
repo's default order. Map SortName/Name -> name (as Subsonic does) and
DateCreated -> created_at. Verified live: case-insensitive alphabetical.
* fix(jellyfin): read query params case-insensitively (Jellify support)
Jellify (and the official Jellyfin TypeScript SDK) send query params in
camelCase (parentId, albumArtistIds, artistIds, includeItemTypes), where
Finamp sends PascalCase. Our handlers read fixed-case keys, so every
Jellify filter/sort/paging param was silently dropped:
- an artist's page listed albums and tracks from all artists
- opening an album listed every album instead of its tracks
Real Jellyfin binds query params case-insensitively (ASP.NET model
binding), so add a normalizeQueryKeys middleware that folds every query
key to lowercase once, and read params by their lowercase name. This
also removes the scattered dual-case hacks (Ids/ids, IsFavorite,
ContributingArtistIds, api_key/ApiKey, queryParam) that let this bug
class through.
Also infer the child type from an album parent: Jellify browses an album
with only parentId (no IncludeItemTypes), and Jellyfin infers Audio from
the parent; without it we fell back to listing all albums.
Verified live against Finamp+Jellify and with 4 new e2e specs replaying
Jellify's exact request shapes.
* fix(jellyfin): advertise LocalAddress in the public system info handshake
Jellify (and other @jellyfin/sdk clients) that connect by raw address fall
back to HTTP when TLS isn't available, then adopt the handshake's
LocalAddress as their server base URL. We never populated it, so Jellify's
SDK `api` object was undefined and sign-in crashed with "Cannot read
property 'configuration' of undefined" (getUserApi(api!) with an undefined
api). Over an HTTPS connection Jellify uses connectionType=hostname and
never reads LocalAddress, which is why it worked while an HTTPS proxy was
in front.
Populate LocalAddress from the request — scheme + host (honoring
X-Forwarded-*), plus the /jellyfin mount path — matching real Jellyfin,
which always sends it. Export server.ServerAddress for the resolution.
* fix(jellyfin): separate "Featured On" from an artist's own discography
Jellify's artist page fetches the discography via albumArtistIds and the
"Featured On" section via contributingArtistIds, relying on the server to
return disjoint sets. We collapsed albumArtistIds/artistIds/
contributingArtistIds into one AlbumsByArtistID filter, so an artist's own
albums appeared in both sections.
Add AlbumsByContributingArtistID — albums where the artist is a track
artist but NOT the album artist — matching Jellyfin's ContributingArtistIds
(in Artists, not in AlbumArtists), and route contributingArtistIds to it.
* fix(jellyfin): accept a bare Authorization token for audio streaming
Jellify's native player (react-native-nitro-player) authenticates the audio
stream by setting a bare Authorization header carrying the raw access token
({ AUTHORIZATION: api.accessToken }), not the "MediaBrowser ... Token=" scheme
parseEmbyAuth understands. tokenFromRequest didn't recognize it, so every
/Audio/{id}/stream request from the native player 401'd and no audio played
(the JS client still optimistically posted playback progress, masking it).
Accept a bare (or "Bearer <token>") Authorization header, as real Jellyfin
does. Verified live: the native player's exact request now streams 200.
* fix(jellyfin): embed a self-authenticating stream URL in PlaybackInfo
Jellify's native audio player (react-native-nitro-player / ExoPlayer) fetches
the stream without forwarding any auth — captured request headers were only
Connection/Icy-Metadata/Accept-Encoding/User-Agent, no Authorization and no
api_key — so every /Audio/{id}/stream request 401'd and nothing played (the
JS client still optimistically posted progress, masking it).
Real Jellyfin returns stream URLs with the token embedded; we returned none,
so Jellify fell back to a token-less DirectPlay URL. Populate
MediaSources[0].TranscodingUrl with /Audio/{id}/universal?api_key=<caller
token>, which Jellify's player uses verbatim. Direct-play clients (Finamp
builds its own /Items/{id}/File?ApiKey URL) ignore the field, so they're
unaffected.
* feat(jellyfin): serve per-item UserData (GET /UserItems/{id}/UserData)
Jellify fetches this per item to render played/favourite indicators; we 404'd
it. Resolve the item via the existing getItem resolver (which loads the
caller's annotations and enforces the same library-access gate) and return its
UserData, falling back to an empty-but-valid object for items without
annotations (e.g. playlists). Also registers the legacy
/Users/{userId}/Items/{itemId}/UserData spelling.
* refactor(jellyfin): drop unused bare-Authorization token support
Added mid-troubleshooting on the theory that Jellify's native player sends a
bare Authorization header, but the debug capture showed it sends no auth header
at all — the real fix was embedding api_key in PlaybackInfo's TranscodingUrl.
No client reaches this path (Jellify JS uses the MediaBrowser scheme, Finamp
uses X-Emby-Token, the native player uses the api_key URL), so remove it and
its tests per YAGNI. Effectively reverts 4f8ac1e0.
* feat(jellyfin): implement Similar endpoints via the external provider
Add GET /Artists/{id}/Similar (related artists) and GET /Items/{id}/Similar
(similar songs for a track, similar albums for an album, related artists for an
artist), sourced from the same external.Provider (Last.fm etc.) that powers
Subsonic's getArtistInfo2/getSimilarSongs. Only library-present artists are
returned so each is navigable; provider errors and unknown ids degrade to an
empty 200 result, so clients (Jellify) stop hammering these with 404 retries.
Injects external.Provider into the Router (regenerated via make wire).
* perf(jellyfin): make Similar endpoints non-blocking (bounded quick wait)
Each /Similar request fetched from Last.fm synchronously (500ms-1.4s), and
Jellify requests Similar for many items at once on the home/artist screens, so
the whole screen stalled — a home pull-to-refresh dragged for seconds.
Run the external lookup on a background context and return within a 500ms quick
wait: a cached artist resolves instantly, a cold one returns empty now while the
lookup finishes caching in the background, so a later load is fast and
populated. Verified live: cold calls bounded at ~500ms (was up to 1.4s), warm
calls 3-8ms.
* fix(jellyfin): answer ManualPlaylistsFolder so the home stops stalling
Jellify resolves its "playlists library" via IncludeItemTypes=
ManualPlaylistsFolder, then lists playlists with ParentId set to that folder's
id. We didn't recognize the type, so parseTypes fell back to MusicAlbum and
returned the album list. Jellify's query then found no item with
CollectionType=playlists and resolved undefined — which React Query rejects,
retrying it in a backoff loop that stalled the home pull-to-refresh for ~5s
(every response was fast server-side; the delay was the client's retries).
Return a synthetic "playlists" folder (CollectionType=playlists) for the
ManualPlaylistsFolder query, resolve ParentId=<that folder> to the user's
playlists, and give playlists a Path under "data" (Jellify drops playlists
whose Path lacks it). Adds a Path field to BaseItemDto.
* docs(jellyfin): note lyrics and InstantMix/sonic-similarity as follow-ups
* fix(jellyfin): genre paging params and same-key casing collisions
getGenres read StartIndex/Limit in PascalCase, which normalizeQueryKeys had
already folded to lowercase, so genre paging was silently ignored; totals now
come from the full (small) genre list instead of the page length.
normalizeQueryKeys now merges values when two casings of a key collide,
instead of nondeterministically keeping one.
* fix(jellyfin): round ratings to the nearest star instead of truncating
Rating is a nullable double 0-10 in Jellyfin's contract. Truncating integer
division stored 9 as 4 stars, and both Rating=1 and fractional values (which
failed integer parsing) became 0 — silently deleting the rating. Parse as
float, round, and floor nonzero input at one star.
* fix(jellyfin): apply Name/IsPublic sent together with a track replacement
Jellyfin's UpdatePlaylist applies every provided field, but the Ids branch
returned early, silently discarding a rename or visibility change sent in the
same body (core/playlists.Create with an existing id ignores the name).
* fix(jellyfin): don't rotate the stored server id on transient DB errors
serverID treated any Property.Get error as "no id yet" and persisted a fresh
UUID over JellyfinServerID, with sync.Once pinning it for the process lifetime
— a busy DB or canceled request context on the first request would break every
client's cached ServerId. Only ErrNotFound mints a new id now, and failures
yield an uncached temporary value so the next request retries.
* fix(jellyfin): report real search totals instead of page length or unfiltered counts
Artist search returned TotalRecordCount = len(page), so clients stopped after
the first page; album/song search counted via CountAll, which can't see the
search term, so clients paged through phantom results. The repos' Search API
has no match count, so fetch one row beyond the page: offset+len is exact on
the last page and a strictly growing lower bound before it — paging clients
terminate exactly at the last match.
* fix(jellyfin): browse playlists via the generic /Items path and resolve the playlists folder by id
A typeless /Items?ParentId=<playlistId> (legal in real Jellyfin, used by
generic clients) fell through to the MusicAlbum default and returned an empty
list; it now returns the playlist's tracks, paginated, with visibility
enforced by GetWithTracks. resolveItemByID also answers the synthetic
playlists-folder id the server itself advertises instead of 404ing it.
* perf(jellyfin): cap per-type queries in multi-type /Items requests
The multi-type merge path queried each type with a zero-value QueryOptions —
no LIMIT in SQL — materializing every matching row (each with embedded
MediaSources) just to slice out one page in memory. Each type now fetches at
most StartIndex+Limit rows, the worst case one type can contribute to the
merged window; totals still come from CountAll.
* fix(jellyfin): dedupe and bound the background Similar fetches
awaitSimilar spawned a detached, deadline-free goroutine per request; clients
re-polling after the empty quick-wait response piled up duplicate provider
chains (no singleflight anywhere below) racing writes on the same artist row.
Identical in-flight requests now share one fetch — keyed per user, since the
mapped items embed the user's annotations — and the background context gets a
one-minute deadline so a hung provider can't hold goroutines forever.
* fix(jellyfin): gate private playlist covers on the public image route
The unauthenticated image endpoint elevated every request to an admin context,
so anyone who knew or guessed a playlist id could fetch another user's private
uploaded cover. Library artwork still resolves elevated (Jellyfin clients fetch
images without auth headers), but playlist covers are now served only when the
playlist is public or the request's optional token identifies its owner or an
admin; everyone else gets the placeholder.
* fix(log): redact full api_key values, including JWTs
The api_key pattern matched only word characters, stopping at a JWT's first
'.' — the Jellyfin API embeds the session JWT as api_key in TranscodingUrl, so
request logs kept its payload and signature, enough to reconstruct a replayable
token by prepending the constant header. Match to the next query separator
instead, like the sibling s=/p=/jwt= patterns.
* fix(jellyfin): record LastLoginAt on Jellyfin logins
authenticateByName re-implements credential validation and skipped the
UpdateLastLoginAt call the web UI's validateLogin makes, so users who only log
in via Jellyfin clients showed a never/stale Last Login in the admin UI.
* perf(jellyfin): batch song resolution in /Items?ids= and playlist expansion
Both paths probed up to four repositories per client-supplied id. Songs — the
common case — now resolve via chunked media_file.id IN queries (same pattern
as playqueue's loadTracks); only the residue pays the container probes.
* fix(jellyfin): wait for the real Similar result instead of answering a cacheable empty list
The 500ms quick wait returned an empty 200 for any cold lookup —
indistinguishable from "no similar items exist", so clients cached the wrong
answer until an app restart. With fetches deduplicated, wait up to the agents'
HTTP timeout for the actual result; only a hung provider now yields the empty
fallback, and its fetch still warms the cache in the background. Also makes
the dedup test deterministic (the old one raced its release channel).
* refactor(tests): extract the shared e2e harness into tests/harness
The Subsonic and Jellyfin e2e suites each carried their own copy of the
golden-DB lifecycle (boot, seed users/library, scan, WAL snapshot), the
ATTACH-DATABASE restore, fixture-FS registration, and the SpyStreamer /
NoopFFmpeg doubles. Those now live in one importable package (same pattern as
core/storage/storagetest); fixture libraries and request helpers stay
per-suite since they encode each API's test expectations.
* docs(jellyfin): make comments concise
Compress the narrative comments accumulated during live client testing into
short why-only notes; keep the client quirks, security rationales and gotchas,
drop the exposition. Comments-only change (net -141 lines).
* fix(tests): silence gosec taint false-positive in harness snapshot write
The write moved from a _test.go file (which gosec skips) into the importable
harness package; the path derives from GinkgoT().TempDir().
* fix(jellyfin): route the current /UserFavoriteItems favorite endpoint
@jellyfin/sdk 0.13.0 (used by Jellify) posts favorites to
POST/DELETE /UserFavoriteItems/{itemId}, while we only routed the legacy
/Users/{userId}/FavoriteItems/{itemId} (Finamp). Jellify favorites 404'd
("Failed to add favourite"). Route both spellings to the same handlers.
* feat(jellyfin): honor Fields on /Items and add missing conformance fields
Match real Jellyfin's response shape: gate MediaSources (and MediaStreams)
behind Fields=MediaSources instead of always embedding them — clients that
need Size request it, as Finamp does — which cuts a 46-track artist response
from ~87KB to a fraction. Also emit the always-present fields Jellyfin sets:
ServerId (stamped centrally in ok), LocationType, HasLyrics, and SortName
(when Fields=SortName). PlaybackInfo still carries MediaSources (its purpose).
* fix(jellyfin): address code review security and correctness findings
Applies the reviewed findings from PR #5730:
- Gate similar songs/albums on the caller's library access, so the
external provider can't surface metadata from libraries the user
cannot see. Also clamp the client-supplied limit before it sizes any
allocation or provider fetch (CodeQL user-controlled allocation).
- Prepend the /jellyfin mount prefix to the PlaybackInfo TranscodingUrl,
so a client resolving it as an absolute host path still reaches the
mounted router.
- Recognize WebP and GIF magic numbers on raw cover uploads, matching
the formats Navidrome already supports.
- Bound the playlist cover upload: honor EnableArtworkUpload for
non-admins and cap the body with MaxBytesReader/MaxImageUploadSize,
mirroring the native image endpoint.
- Propagate non-not-found repository errors from resolveAnnotated as 500
instead of silently returning 404.
- Normalize the literal prefix of mixed literal.param path segments
(e.g. STREAM.mp3) so case-insensitive routing reaches stream.{container}.
- Rate-limit POST /Users/AuthenticateByName with the same per-IP limiter
as /auth/login when AuthRequestLimit is set.
- Guard against a nil user in authenticateByName.
* fix(jellyfin): correct playlist track browsing and multi-id edits
Fixes three playlist issues, two found testing against Jellify and one
from the PR review:
- Resolve a playlist ParentId to its tracks even when the client sends
IncludeItemTypes=Audio. Jellify opens a playlist with
ParentId=<playlist>&IncludeItemTypes=Audio; the id was routed through
listSongs as an album id, returning an empty list.
- Read repeated id query params (ids=X&ids=Y), not just the first value.
Jellify's @jellyfin/sdk serializes id arrays as repeated params, so
adding an album (which it expands client-side into many ids) only
added the first track. Applies to both add and remove; the
comma-separated form other clients use still works.
- Let an explicit empty Ids array clear a playlist. Ids is now a pointer
so an omitted field still means 'leave unchanged', while an empty list
clears the tracks (via RemoveTracks, since the repository skips track
writes for an empty list).
* fix(jellyfin): register clients as players on any authenticated request
Jellyfin clients did not appear in the players list. Unlike Subsonic,
whose getPlayer middleware runs on every authenticated endpoint, the
Jellyfin router only registered a player on the /Sessions/Playing
reports, so browsing or streaming never created one.
Apply withPlayer to the whole authenticated group, mirroring Subsonic,
so the calling device registers (and scrobbling has a player) as soon as
it makes any authenticated request. Two follow-ups found while testing:
- Skip registration when the request carries no client/device info (no
X-Emby-Authorization, e.g. the /socket handshake that auths via
?api_key= only), which otherwise created a junk player named ' []'.
- URL-decode the X-Emby-Authorization field values. Jellify's
@jellyfin/sdk percent-encodes them (Device='Pixel%208%20Pro') while
Finamp sends them raw, so the player name showed as
'Jellify [Pixel%208%20Pro]'.
* refactor(jellyfin): move case-insensitive routing into the jellyfin package
The case-insensitive path normalization lived in the server package but
was only ever used by the Jellyfin router (its whole purpose is that
Jellyfin clients route case-insensitively while chi does not). Move it
into server/jellyfin and unexport it, so it sits with its only caller
and no longer needs to be exported across a package boundary.
* docs(jellyfin): document player registration, playlist and image behavior
Updates the package README for the behavior added/fixed this round:
- New 'Players and sessions' section: any authenticated request now
registers the device as a player (like Subsonic), with the
Client [Device] naming, URL-decoding of the Emby auth fields, and the
/socket skip that avoids a nameless player.
- Authentication: note AuthenticateByName is rate-limited per IP.
- Playlists: repeated vs comma-separated id params, and that an explicit
empty Ids clears the playlist while an omitted Ids leaves it untouched.
- Cover art: WebP/GIF magic-number detection plus the MaxImageUploadSize
and EnableArtworkUpload gates.
- Endpoints table: add the /Similar and /UserFavoriteItems /
/UserItems/.../UserData routes that were already served but unlisted.
* feat(jellyfin): expose configured users on the login user-picker
Adds Jellyfin.ExposedPublicUsers, a comma-separated allowlist of
usernames that GET /Users/Public advertises so Jellyfin clients (Finamp,
Jellify) can show a login user-picker instead of a blank username field.
The endpoint is unauthenticated, so it defaults to exposing no users and
never lists the full user table: only the admin-configured names are
returned, resolved live per request (a name that doesn't exist is skipped
and logged). Each entry is a minimal DTO (Name, Id) with no
Policy/Configuration, so admin status isn't leaked pre-login, and no
avatar since Navidrome has no per-user profile images.
* style(jellyfin): trim redundant comments
Remove comments that restated the code or duplicated an explanation
already given nearby, keeping the ones that capture non-obvious rationale
(client-specific quirks, gotchas). Comment-only change; no behavior
difference.
* perf(db): keep query planner statistics trustworthy with full ANALYZE
PRAGMA optimize's internal ANALYZE runs with a limited analysis budget
(~2000 rows) that writes wrong sqlite_stat1 entries for low-cardinality
indexes: on a 96K-track library it claimed (missing, library_id) narrows
to ~2000 rows when it matches the whole table. The planner then prefers
that index over the sort index and falls back to a full-table temp
B-tree sort per request, turning paginated song listings into
multi-second queries (reproduced at 5.5s on real hardware; ~90x slower
than with correct stats). Every index-creating migration re-triggered
the poisoning via the post-migration optimize, and the daily optimizer
could re-trigger it on large library changes. Setting analysis_limit on
the connection does not help: optimize ignores it.
Run a plain full ANALYZE instead: after migrations with schema changes,
and in db.Optimize (daily schedule and scan-end). Stats are stored in
the database file, so one connection suffices and the per-connection
pool loop is gone. The Optimize call at shutdown is removed: stats are
maintained at migration/scan/daily points, and an ANALYZE during
shutdown only delays it and races container stop timeouts.
* perf(db): add covering index for title-sorted song listings
Deep pagination over songs sorted by title (WHERE missing/library_id,
ORDER BY order_title LIMIT/OFFSET - the shape Jellyfin clients use to
enumerate the library, and non-admin native/Subsonic song lists share)
walked media_file_order_title and fetched the table row for every
skipped entry just to evaluate the filter and the annotation/bookmark
join keys: offset+limit random reads, seconds per page on cold spinning
disks.
The new (missing, library_id, order_title, id) index makes the offset
skip fully index-resident: filter columns and the join key come from the
index, and only the emitted page touches table rows. Measured on a
96K-track library: offset 50000 drops from ~6.5s (poisoned stats) /
~100ms (good stats, warm) to 24ms, and cold deep pages on NAS hardware
from ~7s to ~0.5s.
* feat(jellyfin): support transcoding via HLS playlist and server-forced player format
- withPlayer now propagates the player's configured transcoding into the
request context (like Subsonic's getPlayer), so a format forced in
Settings > Players applies to the Jellyfin stream endpoints
- new GET /Audio/{itemId}/main.m3u8, the endpoint Finamp plays through
when its transcoding setting is enabled: a single-segment HLS VOD
playlist pointing at the existing progressive transcode endpoint
- streamAudio: treat audioBitRate as bits/sec (Jellyfin convention) and
fall back to audioCodec as target format when no container is given
* fix(jellyfin): honor GenreIds when browsing genre albums and tracks
Finamp's genre screen sends ParentId=<libraryId> plus GenreIds=<genreId>,
but /Items ignored the param, so every genre returned the whole library.
- filter.ByGenreID delegates to persistence.TagIDFilter (exported, was
tagIDFilter), the same mechanism behind the native API's genre_id filter
- id lists are read via queryIDs, covering both spellings clients use
(comma-separated and repeated params); /Items?ids= gains the repeated
form too
* feat(jellyfin): filter album artists by GenreIds
Finamp's artist tab sends GenreIds to /Artists/AlbumArtists when a genre
filter is active; the param was ignored, returning all artists.
filter.ArtistsByGenreID matches artists credited as album artist on an
album with the genre, via a non-correlated semi-join over album
participants (86ms on a 29K-artist library; the correlated EXISTS form
takes 11 minutes). Applied on the browse path of /Artists* and
/Items?IncludeItemTypes=MusicArtist.
The performers variant (/Artists?GenreIds=) still ignores the filter: it
needs the same semi-join against media_file.participants, unmeasured on
large libraries.
* fix(jellyfin): version playlist image tag so clients refresh uploaded covers
Uploads were stored and served correctly, but Finamp kept showing the old
cover: it caches covers keyed by blurHash (its imageId is the item id,
which never changes), and both our image tag and synthetic blurhash were
derived from the playlist id alone.
The tag is now <id>-<UpdatedAt millis hex> (SetImage/RemoveImage go
through a full Put, which bumps UpdatedAt), and the blurhash derives from
the tag, so every cover change rotates both. UpdatedAt over-invalidates
(any playlist edit busts the cover cache), which only costs a refetch;
hashing the actual image remains the proper long-term tag.
An e2e test guards the whole chain, since a partial Put(pls, cols...)
would silently stop bumping UpdatedAt.
* fix(jellyfin): align cover upload limit and validation with the native endpoint
Cover uploads of large photos failed with a silent 400: the
MaxImageUploadSize cap was applied to the wire body, which Jellyfin
clients base64-encode (4/3 inflation), so the effective raw-image limit
was only ~7.5MB — and Finamp uploads picked photos uncompressed.
Align with the native endpoint:
- the limit caps the decoded image; the read cap allows for base64
inflation
- validate by decoding (image.DecodeConfig) and take the storage
extension from the real format instead of the Content-Type header,
which clients get wrong (Finamp falls back to image/jpeg); a HEIC or
corrupt file is now rejected instead of stored as a broken cover
- rejected uploads log the reason (size/limit/decode error); diagnosing
this from production logs previously required guesswork
* fix(jellyfin): optimize SongsByArtistID filter to improve performance at library scale
Signed-off-by: Deluan <deluan@navidrome.org>
* fix(jellyfin): sort tracks by release year for SortBy=PremiereDate
Finamp's "Latest Releases" artist section sends
SortBy=PremiereDate,Album,... descending; PremiereDate wasn't in the
Audio sort map, so applySort fell through to Album and the view came
back in reverse album-name order (a 2002 remix album first, the 2013
release last).
Map premieredate/productionyear to the year sort key, matching the
ProductionYear the DTO exposes for songs and the existing MusicAlbum
mapping (premieredate -> max_year).
* feat(jellyfin): expose PremiereDate on tracks and albums
Finamp's "Latest Releases" artist/genre sections re-sort the merged
server responses client-side by PremiereDate and keep the top 5. Without
the field every comparison returns equal and Dart's unstable sort leaves
the picks in arbitrary order — a 2007 remix could lead the list even
with the server sorting by year correctly.
Serialize PremiereDate as ISO 8601 from the date tag (padding partial
"2007"/"2007-02" values so DateTime.tryParse accepts them), falling
back to the year; omitted when neither exists.
* feat(jellyfin): resolve Finamp-truncated item ids (saved queue restore)
Finamp persists its play queue by packing every item id into exactly 16
bytes (packIds assumes Jellyfin's 32-hex GUID ids), so our longer ids
come back truncated after an app restart and "Failed to restore queue"
loops forever: the /Items?ids= batch resolves nothing.
Navidrome ids can't be made GUID-shaped (nanoid ids can exceed 128 bits),
so compensate server-side: a 16-char id — a length no Navidrome id family
uses — is resolved by unique-prefix range scan, with ambiguity failing
safe. The ids= batch echoes the id as requested (Finamp matches restored
items by its stored ids), and stream, image, item, user-data, favorite,
rating and playback-report endpoints accept truncated ids transparently.
The proper fix belongs upstream in Finamp's packIds; documented in the
README so this layer can be removed once that ships.
* feat(jellyfin): implement Items/{id}/InstantMix
Finamp requests an instant mix on every track tap when its "start
instant mix for individual tracks" setting is on (plus the long-press
menus); the 404 made those taps fail with an error and play nothing.
A track seed returns itself first — Finamp plays exactly what comes
back — followed by the external provider's similar songs, capped at the
requested limit and filtered to the caller's libraries. Container seeds
(artist/album) return the provider's similar-songs blend. Provider
errors and unknown seeds degrade to seed-only/empty results instead of
404s, reusing the Similar endpoints' bounded-wait singleflight (with a
distinct cache key, since mixes and similar lists answer different
shapes). Sonic-similarity backing stays a follow-up (see README).
* style(jellyfin): trim wordy comments
* refactor(jellyfin): apply cleanup review findings
- batch truncated-id resolution in /Items?ids=: one chunked range query
for all media-file prefixes instead of a query per id (a restored
queue sends hundreds of truncated ids)
- resolve truncated ids on the /Similar endpoints too, matching the
neighboring InstantMix; document which entry points don't resolve
- hoist maxImageUploadSize to core, deleting the byte-identical copies
in nativeapi and jellyfin (tests moved to core)
- drop the unused type parameter on premiereDate
* fix(jellyfin): never drop the instant mix seed on a slow provider
The seed track was built inside the awaited provider fetch, so when the
external agent was slow or unreachable the request hit the 10s wait and
answered a fully empty mix — Finamp then played nothing on tap, even
though the seed needs no provider at all (seen live: Last.fm unreachable
from the server, responseSize=49).
Build the seed outside the await: only the similar-songs tail is fetched
and bounded, and a timeout now degrades to a seed-only mix. Also folds
instantMixForSong into getInstantMix, since the tail is exactly
similarSongs.
* fix(jellyfin): prefer the recommended Authorization scheme when picking a token
Jellyfin's authorization guidance deprecates X-Emby-Token,
X-MediaBrowser-Token, X-Emby-Authorization and api_key; the Authorization
MediaBrowser scheme is the recommended form. All spellings stay accepted,
but when a client sends several, the recommended one now wins. Adds
coverage for the canonical Authorization header, which no test exercised
directly.
* refactor(jellyfin): rename parseEmbyAuth to parseMediaBrowserAuth
The scheme is named MediaBrowser; the old name evoked the deprecated
X-Emby-* spellings even though the function also parses the recommended
Authorization header.
* fix(jellyfin): prefer the Authorization header over X-Emby-Authorization
The recommended header now wins when both carry MediaBrowser data — but
only when it actually parses as MediaBrowser: a reverse proxy may inject
Basic/Digest credentials into Authorization while the client sends the
deprecated header, and those must not swallow the client's auth.
* fix(jellyfin): require the MediaBrowser scheme when parsing auth headers
The parser extracted key="value" pairs from any Authorization value; a
foreign scheme whose parameters happened to use our field names would
have been misread as client auth. Validate the scheme word instead
(case-insensitively, per HTTP), accepting the legacy "Emby" spelling
like real Jellyfin. Replaces the any-recognized-field heuristic for
detecting proxy-injected Basic/Digest credentials.
* Revert "perf(db): keep query planner statistics trustworthy with full ANALYZE"
This reverts commit 118563e1053196f0e2e42dd4bee54e39a04ab145.
The planner-statistics work now lives in its own PR (#5740); this branch
keeps only the covering-index migration.
* fix(db): renumber jellyfin covering-index migration after master's latest
* fix(db): renumber Jellyfin covering-index migration
* docs(jellyfin): document playlist annotations
* refactor(log): clarify comments on external services query params
* refactor(e2e): move Subsonic e2e suite to server/subsonic/e2e
All files in server/e2e were Subsonic tests, so relocate the package
under server/subsonic/e2e to sit alongside the API it exercises. The
package name stays 'e2e'; only doc/comment references are updated.
* refactor(filter): consolidate genre filters and unexport tagIDFilter
Route filterByGenre, ByGenreID, and ArtistsByGenreID through a single
genreTagFilter helper so the EXISTS json_tree(tags,"$.genre") predicate
lives in one place. With server/filter no longer using it, persistence's
TagIDFilter is only referenced within the package, so unexport it.
---------
Signed-off-by: Deluan <deluan@navidrome.org>
* feat(playlists): add average_rating column to playlist table
* feat(playlists): store and read per-user starred/rating annotations
* feat(playlists): clean up annotations when a playlist is deleted
* feat(subsonic): route star/unstar of a playlist to the playlist repository
* feat(subsonic): route setRating of a playlist to the playlist repository
* test(subsonic): guard that playlist responses never expose annotations
* fix(playlists): clean stale mis-typed annotations on upgrade; cover GetAll read-back
* fix(playlists): scope annotation join by item_type and harden delete
Address code-review findings on the playlist-annotations branch:
- withAnnotation: add an item_type predicate to the LEFT JOIN so a
mis-typed annotation row sharing an id can no longer leak into (or
duplicate) another entity's read. Correct for every caller since each
repo writes annotations with item_type = tableName. Regression test added.
- migration: reclassify legacy media_file-typed rows for playlist ids to
item_type='playlist' (instead of deleting them), preserving users' prior
playlist star/rating; run before the average_rating backfill so those
ratings are included.
- playlist Delete: replace the per-request full-table cleanAnnotations()
anti-join with a targeted, permission-safe (rows-affected gated),
best-effort delete so a cleanup failure no longer misreports an
already-committed delete as an error.
- MockPlaylistRepo: implement GetAll/IncPlayCount/ReassignAnnotation to
remove the dead All field and the nil-interface panic traps.
- test: use slices.IndexFunc instead of a hand-rolled find loop.
* feat(playlists): streamline playlist deletion by relying on annotation sweep
* docs(playlists): trim comments in annotation migration and test
Condense the verbose comments added in this branch per the project's
comment-minimalism guideline, keeping only the non-obvious rationale.
The migration's reclassify block is shortened while preserving the safety
invariant (playlist and media_file ids never collide, so the item_type
rewrite touches only mis-typed rows and cannot violate the unique key) and
the ordering note. The redundant 'Populate average_rating' comment is
dropped since the UPDATE is self-evident. The repository test's leakage
comment is condensed to two lines. No code behavior changes.
* refactor(subsonic): resolve setStar targets via GetEntityByID
Replace setStar's Album/Artist/Playlist Exists probe chain with a single
model.GetEntityByID lookup and a type switch, mirroring setRating. This
removes three per-id existence queries and keeps the two annotation paths
consistent.
An id that resolves to no known entity is logged and skipped rather than
filed as a spurious media_file annotation, and a lookup failure on one id no
longer aborts the whole batch. Also drop a duplicate empty-ids guard.
* refactor(playlists): drop no-op reclassify/backfill from migration
The average_rating migration carried two data-fix UPDATEs that are no-ops on
any real database:
- The media_file->playlist reclassification only matches rows no released
build ever created: playlists were never annotatable, so star/setRating of
a playlist id was never written as item_type='playlist'. Any stray
media_file-typed row for a playlist id is already removed by the media_file
annotation GC sweep (item_id not in media_file).
- The average_rating backfill runs before any item_type='playlist' row can
exist, so it can only ever write the default 0. Going forward SetRating
keeps average_rating current via updateAvgRating.
Reduce the migration to the column add/drop.
* refactor(persistence): bind annotation join params, derive idField from tableName
Address PR review: use Squirrel parameter binding for item_type/user_id in
the shared withAnnotation join instead of string concatenation, and pass
r.tableName+".id" from selectPlaylist so the join field stays consistent
with the surrounding r.tableName usage.
* fix(subsonic): surface datastore errors in setStar instead of skipping
Address PR review: setStar swallowed every GetEntityByID error and continued,
so a real datastore failure would still commit the transaction and emit a
refresh event as if the star succeeded. Skip only on model.ErrNotFound (an
unknown id); return any other error so the request fails and rolls back.
* test(subsonic): assert absent JSON keys instead of substring matches
Address PR review: substring checks are brittle ("starred" matches "starredAt",
"rating" matches "userRating"). Unmarshal the response and assert the
annotation keys are absent.
* fix(subsonic): skip refresh broadcast when a star request changes nothing
Address PR review (Codex): once setStar began skipping unknown ids, a request
containing only unresolvable ids left the RefreshResource empty, which
SendMessage serializes as a {*:*} wildcard that forces every client to
refresh. Only broadcast when at least one id was actually starred.
* fix(db): rebase playlist average_rating migration timestamp past master
The 20260708011823 migration predated the newest migration merged to
master (20260712211040_add_primary_key...), which Goose would silently
skip on already-upgraded databases. Rename it to a current timestamp so
it applies in order.
* Add resource lists to default view options
* refactor(ui): reuse getStoredDefaultView in AlbumList default-view redirect
Avoid duplicating the localStorage fallback logic and skip the unused
albumLists lookup in the resource-redirect branch, per PR review feedback.
Update the GitHub Actions that had newer major versions available; all
other actions in the workflows were already pinned to their latest major
tag.
- actions/github-script: v7 -> v9
- actions/cache: v5 -> v6
- docker/setup-qemu-action: v3 -> v4
* perf(db): keep query planner statistics trustworthy with full ANALYZE
PRAGMA optimize's internal ANALYZE runs with a limited analysis budget
(~2000 rows) that writes wrong sqlite_stat1 entries for low-cardinality
indexes: on a 96K-track library it claimed (missing, library_id) narrows
to ~2000 rows when it matches the whole table. The planner then prefers
that index over the sort index and falls back to a full-table temp
B-tree sort per request, turning paginated song listings into
multi-second queries (reproduced at 5.5s on real hardware; ~90x slower
than with correct stats). Every index-creating migration re-triggered
the poisoning via the post-migration optimize, and the daily optimizer
could re-trigger it on large library changes. Setting analysis_limit on
the connection does not help: optimize ignores it.
Run a plain full ANALYZE instead: after migrations with schema changes,
and in db.Optimize (daily schedule and scan-end). Stats are stored in
the database file, so one connection suffices and the per-connection
pool loop is gone. The Optimize call at shutdown is removed: stats are
maintained at migration/scan/daily points, and an ANALYZE during
shutdown only delays it and races container stop timeouts.
* perf(db): drop startup PRAGMA optimize that re-poisons planner stats
The startup PRAGMA optimize=0x10002 runs SQLite's budget-limited internal
ANALYZE (bit 0x02), which writes truncated sqlite_stat1 rows for
low-cardinality indexes -- the exact statistics-poisoning this PR set out to
eliminate. Because DevOptimizeDB defaults to true, a restart with no pending
migrations would re-poison the planner until the next scan or daily Optimize.
Remove it: statistics are already refreshed with a full ANALYZE after
schema-changing migrations (Init) and via Optimize at scan-end and on the daily
schedule, so nothing on the startup path needs to touch them. Also clarify that
Optimize is a no-op unless DevOptimizeDB is enabled.
* chore(db): remove the DevOptimizeDB flag and skip Optimize on quick scans
The flag only gated the optimize/ANALYZE maintenance calls and there is
no reason to leave planner statistics unmaintained; the guards are gone
along with the flag. The scan-end Optimize now runs only after full
scans — quick scans barely move the statistics, and the daily schedule
covers drift.
* style(scanner): drop redundant comment in runOptimize
* chore(persistence): drop the no-op PRAGMA optimize from ScanEnd
Mask 0x10000 only selects candidate tables by size change; without the
0x02 action bit optimize does nothing (verified: sqlite_stat1 stays
stale after a 100x table growth). The scan-end statistics refresh is
db.Optimize's full ANALYZE, and the expression-collation-index concern
the old comment guarded against no longer applies.
* fix(scanner): run the post-scan ANALYZE in the server process
With the external scanner (the default), the scan pipeline runs in a
subprocess, so its ANALYZE was invisible to the server: SQLite loads
sqlite_stat1 into the process's shared schema cache, and an ANALYZE from
another process does not refresh it — verified with the production DSN
that even brand-new pool connections keep planning with the old
statistics until the server restarts. An in-process ANALYZE, by
contrast, is immediately visible to every pooled connection through the
same shared cache.
Move the full-scan Optimize from the scanner pipeline to the scan
controller, which always runs in the server process.
* fix(scanner): honor promoted full scans in the optimize gate
A quick scan resuming an interrupted full scan is promoted inside the
scanner (possibly in a subprocess); mirror the promotion in the
controller so the post-scan ANALYZE isn't skipped.
* refactor: apply cleanup review findings
- drop forceFullRescan's inline ANALYZE: Init already runs a full
ANALYZE after any migration batch with schema changes, so upgrades
including a full-rescan migration analyzed the whole DB twice
- resumingFullScan uses a filtered CountAll instead of fetching and
scanning all libraries
- document why CallScan (CLI) deliberately skips the post-scan Optimize
* perf(db): make planner analysis maintenance resilient
Check analysis freshness every 30 minutes and refresh statistics when the last successful run is over 24 hours old or a scan marked them pending. Persist successful analysis state, retry skipped or failed maintenance, coordinate checks with scans, and cover standalone CLI full scans.
* perf(db): avoid analyzing routine quick-scan changes
Reserve pending analysis for full scans, unscanned libraries, and retry state. Incremental quick scans now rely on the 24-hour freshness window instead of triggering a full ANALYZE at the next maintenance check.
* fix(scan): analyze resumed full scans in CLI
* fix(db): back off failed analysis retries
* feat(db): allow disabling scheduled analysis
* test(db): remove redundant analysis coverage
* refactor(db): split ANALYZE maintenance into optimize.go and dedupe call sites
- move query-planner statistics code from db.go to its own optimize.go
(and matching optimize_test.go)
- log ANALYZE elapsed time inside Optimize/OptimizeIfNeeded instead of
repeating the timing block at every call site
- drop the LastDBAnalyzeAttemptAt write on success: it is only read
while failures >= 1, and every failure rewrites it first
- extract runPostScanAnalysis (cmd) and anyIncludedLibrary (scanner)
helpers
* fix(share): enforce track membership on public share streams
The public share stream endpoint (GET /share/s/{jwt}) validated that the
share existed, was unexpired, and that the share owner had library access
to the requested track, but it never verified that the track was actually
a member of the share. It also accepted stream tokens with no share id
(sid) claim, skipping share checks entirely.
Enforce that the requested media file belongs to share.Tracks, and make
the sid claim mandatory on the stream path. The only producer of stream
tokens (encodeMediafileShare) always sets sid, so no legitimate flow is
affected; the image endpoint decodes independently and is unchanged.
Also document why a JWT is used to represent a shared track: it is a
signed, scoped capability for a single public share, not part of
authentication.
* docs(share): clarify JWT usage comment wording
getTranscodeDecision was copying the source file's bit depth into the
transcodeStream details, so a 24-bit FLAC negotiated to Opus reported
audioBitdepth=24. Lossy codecs (Opus, MP3, AAC) have no PCM bit depth,
and ffmpeg only honors a bit depth constraint (-sample_fmt) for lossless
outputs, so the value was both meaningless and misleading to clients
that use it as a quality indicator.
Only set the transcoded stream's bit depth when the target format is
lossless; a zero value omits audioBitdepth from the response. This also
makes audioBitdepth codec limitations a no-op for lossy targets instead
of rejecting the profile. Lossless targets (e.g. FLAC->FLAC downconvert)
keep reporting and clamping bit depth as before.
* ci: don't skip release jobs after the DB migration check on tag pushes
The validate-migrations job added in #5750 was gated at job level with
if: github.event_name == 'pull_request', so it concluded "skipped" on tag
pushes. GitHub Actions propagates a skipped job transitively through the
needs chain (actions/runner#491): even though Build overrode its own gate
with !cancelled() && !failure() and ran successfully, every job downstream
of Build (msi, release, push-manifest-*, PKG uploads) still failed the
implicit success() check and was skipped, which broke the v0.63.2 release.
Move the pull_request gate from the job to its steps. On non-PR events all
steps are skipped and the job concludes "success", so downstream jobs run
normally. This also restores the default success() gate on Build, keeping
the fail-fast behavior on PRs with a bad migration.
* ci: trim workflow comment
Condense the explanation of the step-level pull_request gate on the
validate-migrations job to the essential rationale.
* test(scanner): fix flaky Windows search_normalized rescan test
The 'repopulates a stale search_normalized on a full rescan' spec runs
two full scans back-to-back. Whether the second scan refreshes the
unchanged artist depends on folderEntry.isOutdated(), which compares
folder.updated_at (written during the first scan) against the second
scan's library.last_scan_started_at using a strict time.Before(). Both
are time.Now() values captured milliseconds apart.
On Linux's fine-grained clock they are always distinct, so the test
passes. On Windows the coarse wall-clock granularity frequently makes
the two timestamps land in the same tick and compare equal, so
Before() returns false, the folder is treated as up-to-date and
skipped, the artist is never re-persisted, and search_normalized stays
empty -- failing the assertion intermittently across unrelated PRs.
Backdate the folder's updated_at an hour before the second scan so the
comparison is unambiguous on every platform. This is a test-only
timing artifact (real rescans never run milliseconds apart on an
unchanged library), so no production code changes are needed.
* fix(smartplaylist): reject NSP mixing top-level 'any' and 'all'
A smart playlist (.nsp) that specified both a top-level "any" and a
top-level "all" group was imported by silently keeping only "any" and
discarding "all", regardless of key order. The Criteria model holds a
single top-level Expression, so it cannot represent both groups, and the
parser picked "any" without reporting the dropped rules.
Make Criteria.UnmarshalJSON return an error when both keys are present at
the top level, so the scanner fails loudly (logging the playlist as
invalid) instead of silently losing rules. Users should nest one group
inside the other, as shown in the documented examples.
Fixes#5757
* fix(smartplaylist): reject top-level any+all by key presence
Address code review feedback: the previous guard checked decoded slice
lengths, so it only rejected the mixed top-level any/all form when both
groups were non-empty. An input like {"any":[],"all":[...]} (or a
null group) slipped past and silently used just one group — the same
class of silent drop this change set out to prevent.
Decode the two keys as json.RawMessage and detect presence by key rather
than length, so any file that provides both top-level keys is rejected
regardless of whether one group is empty or null.
* refactor(smartplaylist): detect top-level any+all via presence type
Replace the json.RawMessage + manual double-unmarshal in
Criteria.UnmarshalJSON with a small optionalConjunction wrapper whose
UnmarshalJSON records that its key was present. Because encoding/json
invokes UnmarshalJSON even for a JSON null, this keeps the exact
behavior (a present-but-empty or null group still counts, so mixing
both top-level keys is rejected) while decoding in a single pass — no
raw-message capture, no re-decode, no shadow variables.
No behavior change; existing tests pass unchanged.
* fix(plugins): surface host service failures when loading plugins
When a host service failed to initialize during plugin load (e.g. the
taskqueue database could not be created because the data folder is not
writable), the error was logged and swallowed, and its host functions were
silently omitted. Instantiation then failed with a misleading error such as
'"task_createqueue" is not exported in module "extism:host/user"', which
reads as a plugin/host API mismatch and gets wrongly blamed on plugin
authors (see kgarner7/navidrome-listenbrainz-daily-playlist#26).
Host service factories now return an error, and loadPluginWithConfig fails
fast with the actual cause (e.g. 'creating Task service: creating plugin
data directory: ...'), which is also stored in the plugin's last_error.
Closers accumulated before a load failure are now closed (via a deferred
guard covering all failure paths), so partially-created services no longer
leak goroutines or database handles.
Also fix a panic in 'navidrome plugin enable': CLI commands use the plugin
Manager without calling Start, so manager.ctx was nil and
newTaskQueueService panicked in context.WithCancel. Long-lived host
services (taskqueue, kvstore, websocket) now receive their lifecycle
context explicitly in the constructor, sourced from serviceContext.baseCtx(),
which falls back to context.Background() for the unstarted-manager case.
* docs(plugins): correct websocket readLoop lifecycle comment
The comment claimed the read loop's context is always cancelled during
application shutdown, which is not true when the manager was never started
(one-shot CLI runs, where baseCtx falls back to context.Background()).
Clarify that connection closure via Close() on plugin unload is what ends
the read loop, with context cancellation as a server-shutdown backstop.
Addresses review feedback on #5756.
* test(plugins): exclude plugin-loading specs from Windows builds
The new loadPluginWithConfig specs reference test suite helpers
(testdataDir, noopMetricsRecorder) defined in plugins_suite_test.go, which
is excluded on Windows, breaking test compilation there. Move the specs to
their own file with the same build constraint, keeping the pure-function
specs in manager_loader_test.go running on Windows as before.
The 'repopulates a stale search_normalized on a full rescan' spec runs
two full scans back-to-back. Whether the second scan refreshes the
unchanged artist depends on folderEntry.isOutdated(), which compares
folder.updated_at (written during the first scan) against the second
scan's library.last_scan_started_at using a strict time.Before(). Both
are time.Now() values captured milliseconds apart.
On Linux's fine-grained clock they are always distinct, so the test
passes. On Windows the coarse wall-clock granularity frequently makes
the two timestamps land in the same tick and compare equal, so
Before() returns false, the folder is treated as up-to-date and
skipped, the artist is never re-persisted, and search_normalized stays
empty -- failing the assertion intermittently across unrelated PRs.
Backdate the folder's updated_at an hour before the second scan so the
comparison is unambiguous on every platform. This is a test-only
timing artifact (real rescans never run milliseconds apart on an
unchanged library), so no production code changes are needed.
* fix(scanner): resolve file symlinks with the production local storage FS
The symlink classification added for GHSA-r5qr-m328-qcf4 relied on
fs.ReadLink, but the local storage FS wraps os.DirFS behind the fs.FS
interface, hiding its ReadLinkFS implementation. Every resolution failed
at the first hop, so the scanner silently skipped ALL file symlinks,
regardless of target or the FollowSymlinks setting. Libraries made of
symlinks (e.g. shared-pool setups) lost all their tracks after
upgrading to 0.63.
The local storage now exposes full OS-level resolution (EvalSymlinks)
through a new optional storage.SymlinkResolverFS interface, which the
scanner prefers over the fs.ReadLink hop loop. This also classifies a
chain by its FINAL target even when it passes through an audio-named
intermediate outside the library, closing a bypass the hop loop had.
Regular (non-symlink) entries keep the same early-return path, so scan
performance is unaffected for normal libraries.
Fixes#5752
* fix(test): keep watcher specs off the real local storage
The watcher specs spawn watchLibrary goroutines that are not joined on
spec teardown. Now that the scanner test binary registers the file://
storage, those leaked goroutines reached newLocalStorage, which reads
conf.Server on construction, racing with the configtest cleanup that
restores the config snapshot (caught by CI's race detector). Point the
mock libraries at a fake storage scheme, which never touches the config
and does not support watching, so the goroutine exits immediately.
* fix(storage): reject invalid fs paths in ResolveSymlink
Defense-in-depth for the SymlinkResolverFS contract: names must be valid
fs.FS paths. A lexical ".." in the name would otherwise escape the
library root via filepath.Join cleaning. No current caller can produce
such a name (they come from ReadDir walks), but the guard enforces the
documented contract at the boundary.
* ci: add DB migration ordering/naming validation script
* ci: run DB migration validation on pull requests
* ci: don't reject non-migration .go files
* ci: fetch latest master before validating migration order
* ci: annotate the offending migration file on validation failure
* ci: gate Build on the migration check so a bad migration fails fast
* ci: reject migrations placed in a subdirectory of db/migrations
* ci: reword fetch-step comment to not hard-code the base branch name
* fix(build): force nodynamic webp tag on 32-bit standalone binaries
gen2brain/webp's native libwebp backend links ebitengine/purego, whose
reverse callbacks are unsupported on 32-bit ARM and x86. purego registers
its callback in package init(), so the binary crashes at startup (SIGSEGV
or SIGILL) before any Navidrome code runs.
The nodynamic build tag from #5606 forces the safe WASM path, but it was
only applied to the Docker-image build stage. The standalone build stage,
which produces the downloads-page tarballs and the deb/rpm packages, still
linked purego, so the armv7/v6/v5 and 386 downloads crashed on launch
(#5738, #5735).
Move the tag decision into release/build-tags.sh, shared by both build
stages so they can no longer drift, and add release/verify-binary.sh as a
build-time guard that fails if a 32-bit binary links purego.
* fix(build): harden webp build-tag scripts per review
- verify-binary.sh: fail loudly when the target binary is missing (e.g. an
unmatched glob) instead of letting `go version -m` fail inside a pipeline
and silently pass, which would bypass the guard.
- build-tags.sh / verify-binary.sh: fall back to `go env GOARCH` when xx-info
is unavailable, so the scripts stay correct outside the xx build image.
(Not `uname -m`, which reports the build host, not the cross target.)
- Dockerfile: use `set -e` in the standalone build block and drop the
redundant `|| exit 1` suffixes; keep the debug GOENV dump non-fatal.
* chore(build): quote -tags argument in both build stages
Defensive quoting per review; the value comes from release/build-tags.sh and
contains no whitespace today, but quoting prevents word-splitting if it ever does.
* fix(build): link 32-bit arm binaries with LLD to fix startup crash
The standalone armv7/v6/v5 binaries of 0.63.0 crash before main() with
SIGSEGV/SIGILL (issues #5738, #5735). Root cause, established from a core
dump of the crashing binary under qemu: GNU ld emits corrupt
R_ARM_IRELATIVE addends for libatomic's ifunc resolvers (wrong address and
missing Thumb bit) once .text outgrows the 16MB Thumb branch range. glibc's
static-init ifunc resolution then does `blx` into ARM-mode garbage and the
process dies before any log output. v0.62.0 was unaffected only because its
.text was still under 16MB (15.1MB); v0.63.0 crossed the line (17.5MB), so
every 0.63.0 32-bit arm build crashes regardless of Go or dependency
versions.
Link 32-bit arm with LLD (already installed in the build stage), which
emits correct IRELATIVE addends. Verified under qemu: the armv7 artifact
built by the unchanged pipeline now boots to "Navidrome server is ready"
with SQLite migrations working, where the previous binary segfaulted at
startup.
Also add a CI smoke test that runs each cross-compiled linux binary under
binfmt/qemu right after building it, so any future
crashes-at-startup-on-some-arch regression fails the pipeline instead of
shipping in a release.
kardianos/service v1.3.0 (shipped in Navidrome 0.63.0) replaced Go's
text/template with a small custom engine that uses bare keys ({{Description}})
instead of dotted fields ({{.Description}}). Our custom SystemdScript was
still written in text/template syntax, so 'navidrome service install' failed
with 'FATA service: unknown template key ".Description"', breaking the .deb
postinst and leaving an empty/masked systemd unit.
Rewrite the template in the new engine's syntax and add a test that validates
every key and pipeline function used in the template against the set the
library provides to systemd templates, so future engine/key drift is caught
at test time. Verified end-to-end on Linux: the previous binary reproduces
the reported fatal error and the fixed one generates a complete unit file.
Fixes#5742
Plugin scrobblers read the username from the request context via
getUsernameFromContext, but buffered scrobbles are persisted to the DB
scrobble buffer (which stores only the userId) and later drained by a
background worker running on context.Background(). That context carries
no authenticated user, so ScrobbleRequest.Username was always empty for
WASM scrobbler plugins. NowPlaying is unaffected because it is dispatched
synchronously on context.WithoutCancel(requestCtx), which retains the user.
Restore the user in the drain path: processUserQueue now looks up the
user by the buffered userId and injects it into the context via
request.WithUser before dispatching, mirroring the pattern already used
in play_tracker. Builtin scrobblers are unaffected as they resolve the
account from the userId argument rather than the context.
* fix(plugins): discard buffered scrobbles when a plugin is removed
Scrobbles are buffered in the DB per service, keyed by the plugin name.
When a plugin was removed (deleted from the plugins folder and detected by
the sync), its pending buffer entries were left behind forever: the drain
goroutine is stopped on the next scrobbler refresh, so the rows were never
retried nor discarded. Worse, if a plugin with the same name was installed
later, the stale entries would be drained into it - potentially a completely
unrelated plugin that just reuses the name.
Add a Discard(service) method to ScrobbleBufferRepository and call it from
removePluginFromDB, right after the plugin record is deleted. Disabling a
plugin intentionally keeps its buffered scrobbles, consistent with the
buffer's purpose of surviving temporary outages, and transient unload/reload
cycles during config updates are unaffected since they never delete the
plugin record.
* fix(plugins): don't wipe builtin scrobbler queues on plugin removal
Buffer entries are keyed by service name only, and removePluginFromDB runs
for any removed plugin file, so removing a plugin named e.g. lastfm.ndp -
regardless of its capability - would discard the builtin Last.fm retry
queue. Skip the discard when the plugin name is owned by a registered
builtin scrobbler, exposed via a new scrobbler.IsBuiltinScrobbler helper.
Reported by Codex review on the PR.
Also drop the testBroker usage from the new removePluginFromDB spec: it is
defined in manager_test.go which is excluded on Windows, breaking the
Windows test build. sendPluginRefreshEvent is nil-safe, so no broker is
needed.
* fix(build): derive version from reachable git tag
* fix(build): fall back gracefully when no tag is reachable
git describe --tags --abbrev=0 exits with an error when the checkout has
no reachable tag (e.g. tagless forks or pre-first-tag commits). In the
Makefile this printed a fatal message and produced a bare -SNAPSHOT
version, and in the CI git-version step the non-zero exit would abort
the job under bash -e before the empty-tag guard could run. Silence
stderr and fall back to v0.0.0 in the Makefile, and to an empty string
in the workflow so the existing guard keeps skipping the output as it
did before.
Flip the EnableSharing default from false to true so new installations have the sharing feature available out of the box. Users can still disable it via the EnableSharing config option.
The native API only registers the /share route when sharing is enabled, so the nativeapi tests that build the router without wiring a share service now explicitly disable sharing in their setup to avoid registering a route backed by a nil service.
* perf(db): add composite indexes for song list album/artist sorts
The media_file sort mappings for album, artist and albumArtist expand to
multi-column ORDER BY clauses that no existing index could satisfy, so SQLite
fell back to a full table scan plus a temp B-tree sort of every row (including
the large lyrics/tags/full_text columns) even for a single 15-item page. On a
96K-track library this made /api/song?_sort=album take 3.6s on a cold cache.
Add composite indexes matching the three sort mappings, allowing the query to
walk the index and stop at the page size, in both directions. Drop the now
redundant single-column order_album_name/order_artist_name indexes (strict
prefixes of the new composites) and three indexes with no query path:
birth_time is only read in Go code, and artist/album_artist text column
lookups go through the media_file_artists table instead.
* fix(ui): make composer and track number columns non-sortable in song list
Clicking the Composer header was a silent no-op: composer is not a media_file
column, so the native API's sanitizeSort drops the sort and returns rows in
table order. Track number sorting across the whole library is not meaningful
and cannot use an index (the existing index leads with disc_number). Mark both
columns sortable={false}, like quality and mood.
* test(persistence): add sort index coverage test for large tables
Guard against sort options silently losing index support: every sort mapping
on media_file, album and artist is now verified with EXPLAIN QUERY PLAN to be
satisfiable by an index (both directions), so adding a mapping or dropping an
index that reintroduces a full-table temp B-tree sort fails the test. Sorts
that genuinely cannot use an index (random, annotation-join columns, JSON
expressions) must be declared in an exceptions list with the reason, keeping
the trade-off visible in review.
To make the sort mappings the complete declared sort surface, add identity
mappings for the media_file columns the UI sorts by without a mapping (year,
genre, duration, channels, bpm, path, comment, play_count, play_date, rating).
These are behaviorally no-ops: the same ORDER BY was previously produced by
the field whitelist fallback.
* perf(db): drop PreferSortTags expression indexes from media_file
The media_file sort_title/sort_artist_name/sort_album_name expression indexes
are only usable when PreferSortTags is enabled - a config reported by ~0.1% of
installations (insights, week of 2026-06-22) - yet every install pays their
storage (~8.6MB on a 96K-track library) and scanner write overhead. Drop them:
PreferSortTags installs fall back to a full sort for title/artist/album orders,
everyone else gets smaller DBs and cheaper writes. The order_album_name and
order_artist_name collation checks remain valid, now satisfied by the composite
sort indexes.
Signed-off-by: Deluan <deluan@navidrome.org>
---------
Signed-off-by: Deluan <deluan@navidrome.org>
* feat(search): boost exact token matches over prefix matches
buildFTS5Query now emits (word OR word*) instead of word* for plain tokens. The match set is unchanged (exact is a subset of prefix), but bm25 gives the rare exact token a high-IDF contribution, so rows containing the literal query word rank above prefix-only matches. The degraded-query check keeps evaluating the plain prefix form, preserving the LIKE fallback for queries like "1+" and "C++".
* feat(search): weight artist search_normalized equal to name in bm25
For the artist table, search_normalized holds only the artist's name in alternate spelling (transliterated/punctuation-stripped), so a hit there is as meaningful as a name hit. Combined with exact-token boosting, artists like MØ now rank in the top results for the query "MO" instead of dead last. media_file and album keep weight 1.0 because their search_normalized mixes title, album, and artist variants.
* test(persistence): add exact-match ranking regression test
Seeds MØ, Morrissey, and Modest Mouse and asserts MØ ranks first for the queries "MO" and "MØ": the exact transliterated hit in search_normalized must outrank name-prefix matches. The corpus deliberately has no competing exact-word names, since exact-vs-exact ordering depends on corpus statistics rather than the guaranteed exact-over-prefix property. Rows are inserted per-test (with their library_artist associations) and cleaned up to avoid disturbing the shared seed fixtures and their count assertions.
* docs(search): document exact-token OR emission in buildFTS5Query
* test(persistence): harden exact-match ranking test fixtures
Register the corpus cleanup before the insert loop so a mid-loop assertion failure cannot leak fts-rank-% rows into the shared integration DB, and reuse the existing createArtistWithLibrary helper instead of hand-rolling Put+AddArtist (which also replaces the ad-hoc context.TODO with the helper's GinkgoT().Context).
* fix(search): flag multi-word degraded queries for the LIKE fallback
The degradation probe was joined with explicit " AND " like the real query, so ftsQueryDegraded counted the literal AND as a long token and never flagged queries where every term degrades to a short token (e.g. "1+ 2+"). This predates this branch (the old code passed the same AND-joined string), but the probe now exists separately, so join it with spaces — it only feeds ftsQueryDegraded, which needs no explicit operators.
* fix(scanner): update artist search_normalized when rescanning
The FTS5 migration back-fills artist.search_normalized with a SQL
punctuation-strip approximation, relying on the next scan to compute the
precise value in Go (normalizeForFTS transliterates atomic letters like
Ø/æ/ß that FTS5's remove_diacritics cannot fold). But the scanner
persisted artists with an explicit column list that omitted
search_normalized, so not even a full scan ever repaired it: an artist
migrated from a pre-FTS database (e.g. "GØGGS") stayed unfindable by
any ASCII search, while their albums and songs, which are saved with all
columns, were fixed by a full scan. Add search_normalized to the column
list so a full scan re-indexes the artist via the artist_fts trigger.
* refactor(persistence): move normalizeForFTS to utils/str
Export it as str.NormalizeForFTS so the upcoming migration can reuse the
exact index-time normalization. Migrations cannot import the persistence
package (persistence -> db -> db/migrations would be an import cycle).
* fix(persistence): backfill artist search_normalized via migration
Recompute artist.search_normalized with the precise Go normalization for
databases migrated from pre-FTS5 versions, where the SQL back-fill could
not transliterate atomic letters (Ø/æ/ß) and the scanner never rewrote
the column. Only changed rows are updated, so the artist_fts update
trigger re-indexes exactly the affected artists, making artists like
GØGGS or MØ findable again without requiring a full scan.
* refactor(persistence): share FTS punctuation-strip regex via utils/str
Index-time normalization (NormalizeForFTS) and query-time processing
(buildFTS5Query/ftsQueryDegraded) must produce matching tokens, so keep
the punctuation-strip pattern in a single exported symbol instead of two
identical private copies that could drift. Also document that derived
columns computed in dbArtist.PostMapArgs must be listed in the scanner's
artist Put, which is how search_normalized went stale in the first place.
* chore(migrations): announce artist search backfill in the log
Match the FTS5 migration's notice() pattern so startup isn't silent
while the backfill runs on large libraries.
* docs: tighten comments added in this branch
* docs: describe FTSPunctStrip by what it matches, not one replacement
* fix(scanner): stop logging expected lyrics sniff misses as warnings
During a scan, embedded lyrics are parsed with an empty suffix, which puts
ParseLyrics into content-sniffing mode: it tries the TTML, SRT and Lyricsfile
YAML parsers in turn before falling back to plain text. Every plain-text or LRC
lyric therefore fails the structured probes on its way to the fallback, and each
failure was logged at warning level with no indication of which file triggered
it, flooding the scan log with benign "Error parsing lyrics, falling back to
plain text" messages.
A probe rejecting content it does not own during sniffing is expected control
flow, so it is now logged at trace instead. A parse failure under an explicitly
requested suffix (e.g. a malformed .yaml/.srt/.ttml sidecar) still warns, since
the user declared that format. ParseLyrics gains ctx and path parameters so any
warning names the offending file and carries request context where available;
all call sites are updated accordingly.
Also fixes a test-isolation bug in the new logging spec: the BeforeEach swapped
the process-global default logger via SetDefaultLogger but only restored the log
level on cleanup, leaking the null logger and its hook into later specs in the
shared model suite.
* test: use spec-scoped contexts instead of context.Background in lyrics tests
Replace context.Background() with GinkgoT().Context() (and b.Context() in the
parse benchmarks) across the lyrics-related tests, so contexts are cancelled
when each spec ends. The embeddedLyrics fixture in core/lyrics is now a
hand-written literal like its sibling fixtures, removing the construction-time
ParseLyrics call that could not use a spec-scoped context.
* refactor(model): attach lyrics parse log attribution via context
Narrow ParseLyrics back to (ctx, suffix, lang, contents), dropping the path
parameter added by the previous commit. Attribution now uses the codebase's
existing idiom: callers that know the source attach it with log.NewContext
(e.g. "file" for the media file or sidecar), and the plugin adapter tags both
the plugin name and the track, fixing probe-miss logs that misattributed
plugin-returned content to the file's own tags. This removes three adjacent
string parameters that were easy to swap silently, and the "" placeholder most
call sites had to pass.
Also hardens the logging spec from the previous commit: the null test logger is
now swapped in before raising the level (SetLevel forces the current default
logger to trace, so the old order left the null logger at info and trace
entries never reached the hook), the sniff test now asserts probe misses are
observable at trace with file attribution instead of only asserting the absence
of warnings, and cleanup restores the actual previous logger — via a new return
value on log.SetDefaultLogger — instead of a bare logrus.New() that would
discard hooks configured on the process-wide logger.
* refactor(lyrics): hoist attributed log contexts out of loops
Address review feedback on #5702: build the log-attributed context once per
operation instead of per iteration, and reuse it on the surrounding log calls
so the error/trace lines around ParseLyrics carry the same attribution fields.
In fromExternalFile the sidecar path now rides the context for all log lines
in the function, replacing the repeated explicit "path" field.
* style(model): pass lyrics parse errors as final log arguments
Per the project logging convention, errors go as the last argument (the log
package normalizes them via its error case) instead of a keyed "error" pair,
which stores the raw error value and bypasses that handling. Flagged by review
on #5702; the keyed form was inherited from the original warning line.
* refactor(scanner): make tag value splitting position-based
Replaces the ZWSP substitution trick with index-based cutting, in
preparation for artist split exceptions, which need match positions.
* feat(scanner): protect whitelisted names in tag value splitting
Separator matches inside word-bounded exception matches no longer split.
Matching is case-insensitive and longest-first; boundaries are rune-aware.
* feat(scanner): add Scanner.ArtistSplitExceptions config option
* feat(scanner): honor artist split exceptions for participant tags
Applies Scanner.ArtistSplitExceptions to artist, albumartist and role tag
splitting. Generic tags (genre, mood, ...) are unaffected.
* fix(scanner): apply split exceptions when per-tag Split overrides participant tags
Per-tag Tags.<name>.Split makes the generic ingestion path split the tag
before participant mapping runs, bypassing the whitelist. Attach the
exceptions to participant tag mappings (including sort variants) in clean().
* feat(scanner): split performer names and honor split exceptions
Performer pair values were never split; multiple names in one PERFORMER
value stayed a single artist. Split them with the roles separators, using
the same whitelist protection as other participant tags.
* test(scanner): lock MBID ordering for split performer values
* refactor(scanner): consolidate split-exception wiring and drop hot-path lock
ArtistSplitExceptionsRx is called per tag mapping per scanned file across
concurrent goroutines; replace the mutex+joined-key cache with an atomic
pointer compared via slices.Equal. Route all participant call sites through
WithParticipantExceptions and a shared splitParticipantValues helper.
* refactor(scanner): unexport artistSplitExceptionsRx
All external callers go through WithParticipantExceptions, so the accessor
does not need to be part of the model package API.
* fix(ui): make self-service profile edits report their outcome
When a non-admin user saved their own profile (e.g. changing their
password via EnableUserEditing), the data provider followed the user
update with a call to the admin-only PUT /api/user/{id}/library
endpoint, which always failed with 403. The save error handler then
crashed reading error.body.errors on the plain-text response, so the
user got no notification at all - while the profile change had in fact
already been applied. This made password changes look like they were
silently ignored, and follow-up attempts failed with 'password does not
match' since the current password had already changed. Present since
the multi-library support introduced in v0.58.0 (#4181).
Only call the user-library association endpoint when the logged-in user
is an admin (the server manages assignments for self-edits), and make
the save error handler tolerate error bodies without field errors,
notifying a generic error instead of crashing.
* fix(ui): tolerate nullish rejection values in user save handler
Address review feedback: use optional chaining on the error itself in
the UserEdit save handler, so a nullish rejection value also results in
the generic error notification instead of a TypeError.
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>
* feat(plugins): add public Track and Artist DTOs for host services
* feat(plugins): add Matcher host-service interface and MatchSong DTO
* feat(plugins): generate Matcher host wrappers, PDK clients, and matcher permission
* feat(plugins): implement Matcher host service and MediaFile-to-Track converter
Also fixes an ndpgen bug where ParseDirectory parsed each host-service file
in isolation, so a service method referencing a struct defined in another
file of the same package (host.Track in track.go) could not be resolved.
ParseDirectory now collects package-wide structs in a first pass, mirroring
ParseCapabilities; PDK clients regenerated cleanly via make gen.
* feat(plugins): register Matcher host service in the manager
* test(plugins): add Matcher host service integration test plugin
* refactor(plugins): simplify matcher converter and parser file collection
- toTrack: use gg.V for nil-able field derefs and slice.Map for genres/
participants, removing the repeated nil-guard blocks and inner loop
- manager_loader: drop the redundant ds==nil guard (loadEnabledPlugins
already gates a nil DataStore), matching the other service entries
- ndpgen parser: extract collectGoFiles, shared by ParseDirectory and
ParseCapabilities instead of duplicating the file-filter loop
* fix(plugins): keep nullable Track numerics as pointers
ReplayGain values, BitDepth, and BPM are nullable in model.MediaFile, and 0
is a valid measured ReplayGain value. Flattening them to value types with
omitempty made a real 0 indistinguishable from absent. Model them as *float64
/*int32 so plugins can tell 'no data' from a measured 0. Regenerated PDK
clients; converter passes the model pointers through (RG) or maps *int->*int32
(BitDepth/BPM).
* refactor(plugins): trim redundant pass labels in ndpgen ParseDirectory
The function doc already explains the two-pass approach; the inline labels
restated it. Reduce to bare waypoints.
* fix(plugins): gate Track.Path on library filesystem permission
MatchSongs copied mf.Path into every result unconditionally, letting a plugin
with only the matcher permission enumerate on-disk file paths by matching known
songs. Gate Path behind library.filesystem, matching the Library host service.
toTrack is now a method carrying the permission flag.
* refactor(plugins): align MatchSong JSON casing and parse Go files once
- MatchSong: artistMBID/albumMBID JSON tags -> artistMbid/albumMbid so the Go
wire format matches the Rust SDK's camelCase serialization (cross-SDK fix)
- MatchSongs doc reworded to language-neutral 'empty (absent)' so generated
Rust/Python client docs no longer say Go-specific 'nil'
- ndpgen: parse each package file once (parseGoFiles) and reuse the ASTs across
both passes in ParseDirectory and ParseCapabilities, instead of re-parsing
* refactor(plugins): use shared types for Matcher host service
Move the Matcher host service onto the shared plugins/types package instead
of the host-local MatchSong and Track structs. MatchSongs now takes
[]types.SongRef and returns []*types.Track, dropping host.MatchSong and moving
host.Track (with its host.Artist dependency collapsed onto types.ArtistRef) into
plugins/types. ArtistRef gains SortName and SubRole so it can back a track's
Participants.
SongRef gains a millisecond-precision DurationMs field that supersedes the now
deprecated seconds-based Duration, with DurationInMs() resolving the effective
value and SetDurationMs() keeping both fields in sync when populating a SongRef
to send to a plugin.
The ndpgen host-wrapper template only ever imported context, json and extism, so
a host service referencing the shared types package produced uncompilable code.
Emit the plugins/types import when the service references shared types directly
(gated on the existing Service.ImportsSharedTypes), matching the client template,
and cover it with GenerateHost tests. This removes the need for host-local
re-export aliases. Regenerated the Go/Rust/Python PDK and capability schemas
accordingly.
* test(plugins): cover SongRef duration and artist conversion
Add unit coverage for the new SongRef behavior: SetDurationMs populating both
DurationMs and the deprecated seconds field, and the SongRef-to-agents.Song
conversion preferring DurationMs over Duration and the Artists list over the
scalar Artist/ArtistMBID.
Extract the inline SongRef-to-agents.Song closure in MatchSongs into a named
toAgentSong function so the conversion can be asserted directly rather than only
through the opaque matcher. The end-to-end wire shape of the moved types is
already validated by the existing MatcherService integration test, so no new
WASM-boundary test is needed.
* fix(plugins): harden and unify SongRef-to-agents.Song duration conversion
Address findings from a code review of the matcher host service:
- DurationInMs now clamps a negative deprecated-seconds value to 0 instead of
converting it through uint32, which previously wrapped a value like -1s into a
~49-day duration that corrupted the matcher's duration-proximity tiebreaker.
- Replace the unused SetDurationMs(uint32) with SetDuration(seconds float32),
which takes the unit callers actually hold (model.MediaFile.Duration is
float32 seconds) and centralizes the seconds-to-ms conversion. Wire it into
mediaFileToSongRef so outbound SongRefs carry both duration fields in sync.
- Make the metadata-agent path use DurationInMs() so every consumer of the
shared SongRef honors the DurationMs-over-Duration precedence contract; a
plugin sending only DurationMs no longer loses its duration on that path.
- Collapse the matcher's duplicate toAgentSong/agentArtists helpers into the
existing songRefToAgentSong converter, so there is a single SongRef-to-Song
mapping. Tests narrowed to the duration cases, with artist precedence still
covered in metadata_agent_test.go.
* feat(plugins): allow Matcher host service to scope a match to a user
Add an options struct to the Matcher host service so a plugin can run a match as
a specific user. When MatchOptions.Username is set, the match is run in that
user's context: their favourites and ratings inform the matcher's tiebreaker, and
the returned tracks carry that user's per-user annotations (Starred, StarredAt,
Rating, PlayCount, PlayDate, added to types.Track). An empty username preserves
the previous unscoped behaviour.
Cross-user access is gated by the same allowedUsers/allUsers permission the Users
and SubsonicAPI host services use: an unknown username, or one the plugin is not
permitted to act as, returns an error. User-library access applies automatically
once the user is in context (applyLibraryFilter). Independently, results are now
restricted to the libraries the plugin itself may access via the precomputed
libraryAccess set, dropping any matched track outside that set (the input index
stays unmatched) — this applies even without a username and even for an
admin-scoped user.
core/matcher is unchanged: it already loads and uses annotations and applies
user-library filtering from context, so the feature works by deriving the request
context and post-filtering by plugin library access in the host adapter. The new
opts parameter and the Track annotation fields are propagated to all PDK clients
(Go/Rust/Python) by make gen.
* fix(plugins): correct Matcher library scope and unify user-access checks
Address findings from a code review of the user-scoped Matcher host service:
- The plugin-library post-filter previously dropped every match for a plugin that
holds only the matcher permission, because library config is tied to the Library
permission and a matcher-only plugin has none (empty allowedLibraries,
AllLibraries=false). Gate the filter on whether the plugin actually declared the
Library permission: matcher-only plugins are no longer library-restricted, while
plugins that opt into a library scope are enforced as before. The per-user
library filter (applyLibraryFilter) still applies whenever a non-admin user is
scoped.
- resolveUser collapsed every FindByUsername error (including transient DB
failures) into a misleading "not found". Extract a shared userAccess type
(alongside libraryAccess) whose resolve() distinguishes model.ErrNotFound from a
real backend error and authorizes the user against the allowed set. The Matcher
service now uses it, and host_subsonicapi shares the same userAccess type for its
permission check (preserving its existing error messages), removing a third
divergent copy of the resolve-and-authorize logic.
- Document in the matcher tests that the mock MediaFileRepo returns annotations
unconditionally, so the unit tests cover the adapter's scoped-flag gating and
access checks but not the SQL per-user join. Add tests for the library-permission
gating and for surfacing a backend error instead of masking it as not-found.
* fix(plugins): require a library scope for Matcher, fail closed
Reverse the permissive default introduced when fixing the library post-filter: a
Matcher plugin now must be granted a library scope (all libraries, or at least one
specific library) and MatchSongs rejects the request with "no libraries
configured" when it has none, instead of either silently matching nothing or
defaulting to every library.
This mirrors how the SubsonicAPI host service requires a user scope
(checkPermissions errors with "no users configured" when none is set): the check
is a runtime guard via libraryAccess.configured(), needs no manifest changes, and
keeps the failure loud rather than silent. The per-match library post-filter then
always applies, and the restrictLibraries flag added in the previous commit is
removed.
* fix(plugins): require library permission for matcher; guard nil user
Close the gap where a plugin declaring only the matcher permission loaded
successfully but failed every MatchSongs call with "no libraries configured",
with no way for an admin to grant a library scope (the library-config UI is gated
on the library permission). Add a cross-field manifest rule, mirroring the
existing "subsonicapi requires users" rule, so the matcher permission requires
the library permission to be declared. A matcher plugin therefore also surfaces
the library-config panel and is subject to the existing load/enable-time library
configuration gate, making the fail-closed library check reachable and fixable
rather than a silent dead end. The test plugin manifest now declares the library
permission accordingly.
Also restore a defensive nil-user guard in userAccess.resolve: if a DataStore's
FindByUsername ever returns (nil, nil) instead of model.ErrNotFound, return a
clean "not found" error rather than dereferencing a nil *model.User.
* feat(plugins): expose track AverageRating in Matcher results
Add AverageRating to the Matcher's Track DTO. Unlike the per-user annotations
(Starred, Rating, PlayCount, ...), AverageRating is an aggregate stored on the
track itself and is loaded regardless of the request user, so it is populated
unconditionally rather than gated on a scoped username. Propagated to the PDK
types by make gen.
Signed-off-by: Deluan <deluan@navidrome.org>
* style(plugins): trim verbose comments in matcher host service
Condense the over-long explanatory comments added across the matcher host
service to one-liners that state the why, and simplify the ptrInt32/unixPtr
helpers to Go 1.26's new(value). No behavior change.
* refactor(plugins): pass userAccess into newSubsonicAPIService
Move newUserAccess construction to the loader call site so the SubsonicAPI service
constructor takes a userAccess value directly, matching newMatcherService. Pure
refactor: the service already stored a userAccess internally, so behavior and error
messages are unchanged.
* fix(plugins): regenerate PDK and drop omitempty from AverageRating
Re-run make gen so the generated PDK doc comments match the source comment
trimmed in an earlier commit (the source was simplified but the PDK was not
regenerated, leaving the committed files stale — a 'generated files up to date'
hazard).
Also drop omitempty from Track.AverageRating: it is always set (0 when unrated),
so it should be present in the payload like the other always-set fields
(BirthTime/CreatedAt/UpdatedAt), not dropped at zero. Tag change propagated to the
PDK by the same regeneration.
* fix(plugins): reject user-scoped match before lookup when plugin has no user scope
A matcher plugin requires the library permission but not the users permission, so
a matcher-only plugin always has an empty user scope (allUsers=false, no allowed
users). MatchSongs still ran FindByUsername for any opts.Username before checking
authorization and returned distinguishable errors ('user X not found' vs 'not
allowed to act as user X'), letting such a plugin enumerate account names from the
error text.
Guard userAccess.resolve to reject with a single fixed error before the lookup when
the plugin has no user scope, mirroring how the SubsonicAPI service short-circuits
with 'no users configured'. The unscoped match path (no username) is unaffected, so
matcher-only plugins still match normally.
* fix(plugins): run unscoped matcher as admin, not the inherited request user
A matcher host call can arrive on a context that already carries a request user
(e.g. a plugin capability invoked while serving that user's request — extism
propagates the call context into host functions). With no opts.Username, MatchSongs
passed that context straight through, so the media-file repository applied the
caller's library filter and per-user annotation ranking to an explicitly unscoped
match.
Set the user context explicitly: a username scopes to that user (overriding any
inherited one), and an unscoped match runs under adminContext so only the plugin's
own library scope constrains results. Adds tests using a context-capturing
DataStore to assert the user the matcher resolves in both cases.
* chore(plugins): drop the generated Python matcher PDK
The Python plugin PDK is no longer supported (ndpgen generates only Go and Rust
clients), so remove the stale generated nd_host_matcher.py rather than leave a
client that drifts from the host interface.
* docs(plugins): deprecate SongRef.Artist/ArtistMBID in favor of Artists
Mark the scalar single-artist fields deprecated; Artists (the ArtistRef list) is
the preferred way to supply artist data and already takes precedence for matching.
Propagated to the PDK and capability schemas by make gen.
* refactor(plugins): flatten Track.Participants and add Role to ArtistRef
Change Track.Participants from map[role][]ArtistRef to a flat []ArtistRef, and give
ArtistRef a Role field (the participation category: artist/composer/performer/...)
alongside SubRole (a specialization within a role, e.g. the instrument for a
performer). In the flat list each entry now self-describes its role rather than
relying on a map key, matching how SongRef.Artists is already a flat list; the
converter tags each entry with its role and emits them in a stable role order.
Propagated to the PDK and capability schemas by make gen.
---------
Signed-off-by: Deluan <deluan@navidrome.org>
* perf(persistence): skip annotation join in CountAll when unused
The Native API list endpoints (/api/song, /api/album, /api/artist) issue a
pagination count on every request via rest.GetAll. CountAll unconditionally
added a LEFT JOIN on the annotation table to a count(distinct id) query. The
join's columns are stripped by count(), but the distinct-over-join forced
SQLite to stream and dedup every row, making the count dominate the request
time on large libraries (e.g. ~190ms cold for 95k songs, seconds for
non-admin users behind the library subquery).
Gate the annotation join: only add it when a filter actually references an
annotation column. The need is detected by rendering the query to SQL and
matching annotation column names as whole words, which covers both named
filters (starred, has_rating) and raw squirrel filters. The column set is
derived from model.Annotations so it tracks schema changes; average_rating is
excluded because it lives on the base table, and word-boundary matching keeps
it from matching the annotation column rating.
Counts are unchanged; only the query plan changes. Unfiltered song counts drop
from ~37ms to ~4ms (warm) on a 95k-song library.
* test(persistence): make annotation-join detection case-insensitive
Address review feedback: SQLite column names are case-insensitive, so a raw
filter using e.g. "RATING" would previously evade the case-sensitive column
regex and wrongly drop the annotation join. Add the (?i) flag (average_rating
stays excluded — the underscore still prevents a word boundary before rating)
and cover it with mixed-case tests. Also make the starred-count assertion an
exact value instead of a range.
* refactor(plugins): remove Python PDK generation from ndpgen
* feat(plugins): parse Go type aliases distinctly in ndpgen
* feat(plugins): resolve shared-type aliases against a registry in ndpgen
* fix(plugins): resolve host-service shared aliases package-wide
Mirror the capability approach in ParseDirectoryWithShared: do a first
pass over all package files to build a package-wide alias map, then pass
it into parseServiceFile so that a shared-type alias declared in a sibling
file is visible when resolving types in the service interface file.
Add a focused test that writes the alias in one file and the hostservice
in another, confirming RED before the fix and GREEN after. Also
strengthens the existing Task 3 test with an ArtistRef.Target assertion.
* feat(plugins): add ndpgen -shared-types mode for the Go types package
* feat(plugins): generate the nd-pdk-types Rust crate from -shared-types
* feat(plugins): inject types import and emit deprecated aliases in Go output
* feat(plugins): emit deprecated Rust aliases to the shared types crate
* feat(plugins): inline shared-type shapes into XTP schemas
* feat(plugins): add nd-pdk-types crate and wire dependents
* feat(plugins): move shared capability types to plugins/types with deprecated aliases
* fix(plugins): point Rust deprecated-alias note at the replacement type
* fix(plugins): include shared aliases in KnownStructs so Rust fields keep their type
Capability.KnownStructs() and Service.KnownStructs() previously only
registered names from .Structs. After the shared-types migration, types
like ArtistRef/TrackInfo/SongRef live in .SharedAliases instead, so
ToRustTypeWithStructs could not find them and fell back to serde_json::Value
for every struct field referencing a shared type.
Add the shared-alias names to the knownStructs map in both methods.
Regenerate the Rust capability files; track/song/artist fields now render
as their named types (TrackInfo, SongRef, ArtistRef, etc.).
Add a regression test that verifies a struct field whose type is only in
SharedAliases renders as the named type and not serde_json::Value.
* docs(plugins): remove stale Python references from ndpgen and plugins READMEs
ndpgen no longer has a -python flag; remove it from the usage synopsis,
flags table, and defaults note in ndpgen/README.md. Delete the "Python
Client Library" section that described its output.
plugins/README.md referenced plugins/pdk/python/host/ (deleted) as the
source for Python host-service stubs. Remove that paragraph; Python plugins
still work via the XTP-schema / extism-py path (see examples/*-py).
* refactor(plugins): dedupe ndpgen helpers and tidy shared-type codegen
* docs(plugins): restore Python as a supported XTP schema target
The ndpgen-generated Python PDK was removed, but the XTP YAML schemas are
language-neutral and the XTP CLI still generates Python bindings from them
(as the extism-py examples demonstrate). Only the ndpgen Python output was
dropped, not Python support itself.
* test(plugins): use the shared types package in test plugins
The test fixtures referenced the now-deprecated capability aliases
(sonicsimilarity.SongRef, metadata.ArtistRef/SongRef). Point them at the
canonical types package so our own fixtures don't depend on symbols slated
for removal.
* refactor(plugins): use the shared types package in host adapters
Replace deprecated capabilities.TrackInfo, capabilities.ArtistRef, and
capabilities.SongRef aliases with the canonical types.TrackInfo,
types.ArtistRef, and types.SongRef from plugins/types.
* fix(plugins): reference shared types by canonical path in generated Rust
Previously the generator emitted `pub field: SongRef` (the local deprecated
alias) for struct fields whose type came from SharedAliases. Refactored
ToRustTypeWithStructs into a private toRustType that accepts a shared map,
and added ToRustTypeWithShared which resolves shared-alias names to their
canonical nd_pdk_types::X path before falling through to the knownStructs
check. Both rustCapabilityFuncMap and rustFuncMap now build the shared map
from SharedAliases and use it for fieldRustType, so the generated capability
files reference nd_pdk_types::SongRef / nd_pdk_types::TrackInfo directly.
The deprecated pub type aliases remain in place as the external back-compat
surface. Deprecation warning count from cargo build drops to 0.
* fix(examples): implement missing Scrobbler.playback_report in Rust examples
The Scrobbler trait gained a playback_report method but the two Rust example
plugins (webhook-rs and discord-rich-presence-rs) were not updated, causing
E0046 compile errors. Added the missing fn playback_report to both: webhook-rs
logs and returns Ok(()) mirroring its now_playing handler; discord-rich-presence-rs
is a no-op since Discord presence does not need playback reports. make all-rust
now exits 0.
* refactor(plugins): point Rust deprecation notes at the nd_pdk::types umbrella path
Plugin authors depend on the nd-pdk umbrella crate, which re-exports
nd_pdk_types as 'types', so the migration target they should type is
nd_pdk::types::X. The alias target stays nd_pdk_types::X (the real path
inside nd-pdk-capabilities).
* fix(plugins): error when a shared-type alias can't be resolved against the registry
* refactor(plugins): parse each Go source file once in ndpgen
* fix(plugins): correct ndpgen review nits (flag name, unused dep, docs)
* refactor(plugins): drop the now-unused path param from parseServiceFile
* refactor(plugins): use shared types directly, rename TrackInfo to Track
Capability interfaces now reference the shared `types` package by qualified
name (types.Track, types.SongRef, types.ArtistRef) instead of the package-local
deprecated aliases, and the shared TrackInfo type is renamed to Track to match
its role as the plugin-facing projection of a library media file.
The deprecated bare aliases (scrobbler.TrackInfo, metadata.ArtistRef,
sonicsimilarity.SongRef, etc.) are kept as re-exports so existing plugins keep
compiling, with a deprecation warning steering them to the canonical types.
To support this, ndpgen now resolves qualified types.X references: it collects
them during type discovery, maps each used canonical type back to its declared
deprecated alias for re-export, emits nd_pdk_types::X paths in Rust, and names
the XTP schema components by their canonical type. Regenerated the Go and Rust
PDK and the XTP schemas, and added generator tests covering the qualified-ref
path. Also adds clarifying doc comments to the shared types.
* refactor(plugins): extract shared types selector into a named const
Replace the "types." string literal that detects and strips the shared types
package selector with a single sharedTypesPrefix constant across the ndpgen
generator (parser, types, generator, xtp_schema), giving the package one source
of truth for the selector.
Also restore the single reused scratch map (cleared each iteration) in the
resolveSharedAliases BFS instead of allocating a fresh map per shared-struct
field, matching the prior implementation.
Pure cleanup from a /simplify pass: regeneration produces byte-for-byte
identical Go, Rust, and XTP output.
* refactor(plugins): keep TrackInfo in the capability package for now
Move the track type back out of the shared plugins/types package: it is again
defined inline as TrackInfo in plugins/capabilities/scrobbler.go and referenced
directly by the scrobbler and lyrics capabilities, reverting the rename to
types.Track. The host helper is renamed back to mediaFileToTrackInfo and now
returns capabilities.TrackInfo. SongRef and ArtistRef stay in the shared types
package; TrackInfo keeps using types.ArtistRef for its artist lists.
This type is expected to be reshaped in upcoming work, so leaving it in the
capability package avoids churning the shared types twice. Regenerated the Go
and Rust PDK and the XTP schemas accordingly.
* fix(plugins): emit the Go types import for direct shared-type refs
ndpgen's Capability/Service.ImportsSharedTypes only reported a shared-types
dependency when a deprecated re-export alias (type X = types.X) was declared. A
struct field referencing the canonical form directly (e.g. types.SongRef) with
no such alias produced an empty SharedAliases slice, so the Go templates skipped
the import while still emitting fields/signatures using types.SongRef — leaving
generated PDK code for new shared DTOs uncompilable unless an otherwise
unnecessary alias was added.
ImportsSharedTypes now also returns true when any struct field references the
types. package by qualified name, via a new structsReferenceSharedTypes helper
that reuses collectReferencedTypes (so []types.X and map[...]types.X are covered
too).
* fix(plugins): preserve base64 encoding for shared byte fields in Rust
The Rust shared-types crate template rendered a []byte field as a plain Vec<u8>
without the base64_bytes serde override used by the capability/client templates.
Go's encoding/json serializes []byte as a base64 string, so a Rust plugin using
nd_pdk::types would have serialized an array of numbers instead of the wire
format the Go/server side expects.
GenerateSharedTypesRust now registers the base64_bytes partial and passes a
HasByteFields flag (new anyFieldIsByteSlice helper); types.rs.tmpl emits the
base64_bytes module and a #[serde(with = "base64_bytes")] attribute on []byte
fields, mirroring the capability template.
* fix(plugins): include directly-referenced shared types in XTP schemas
buildSchemas registered shared types into the schema components only by iterating
cap.SharedAliases, which records deprecated re-export aliases. A capability that
referenced a shared DTO solely as types.Foo (no declared alias) therefore never
got Foo into the component set, so the self-contained XTP schema rendered the
field as a generic object (or emitted a dangling $ref), breaking the direct
shared-type use case enabled by -shared.
resolveSharedAliases now also returns the resolved shapes of every used shared
type (alias or not); these are carried on the new Capability.SharedTypes field
and registered by buildSchemas alongside SharedAliases. Validated end-to-end with
the xtp CLI: a direct types.Foo reference now produces a proper component plus a
$ref, so xtp generates a typed struct instead of an untyped serde_json::Map.
* fix(plugins): resolve renamed shared aliases to canonical schema refs
When a deprecated alias renames its canonical type (e.g. type TrackInfo =
types.Track) and a capability field is typed with the alias name (TrackInfo),
buildProperty emitted a $ref to #/components/schemas/TrackInfo. Components are
keyed by the canonical name (Track), so no TrackInfo component was emitted,
leaving a dangling reference that crashes the xtp code generator.
buildSchemas now builds an alias->canonical map; buildProperty (and the slice
item path) resolves $ref targets through it, and a used alias name marks its
canonical component used so it is emitted. Validated with the xtp CLI: the
renamed-alias schema previously crashed xtp and now generates cleanly.
* fix(plugins): detect shared types used directly in method signatures
ImportsSharedTypes only inspected struct fields, so a capability method using a
shared type directly in its signature (e.g. types.SongRef as input/output rather
than inside a local struct) was not detected. The generated Go templates still
rendered the provider/export signatures with types.SongRef, so the capability
package omitted the types import and failed to compile; the same gap applied to
service params/returns.
ImportsSharedTypes now also scans capability method input/output types and
service method params/returns, via a typeReferencesSharedTypes helper that reuses
collectReferencedTypes (covering pointer/slice/map wrappers).
* fix(plugins): add base64 dependency to the shared Rust types crate
When a shared DTO has a []byte field, ndpgen emits the base64_bytes serde helper
and use base64::... imports into nd-pdk-types/src/lib.rs, but the crate manifest
declared only serde. In that case make gen produced a crate that failed to
compile with 'unresolved module base64'.
Add base64 = "0.22" (matching nd-pdk-capabilities) so the generated shared types
crate compiles whenever a []byte field is present. Verified by generating a
shared crate with a []byte field and confirming cargo check fails before and
passes after.
* fix(plugins): translate shared method types in generated Rust
A capability method using a shared DTO directly as input/output (e.g.
types.SongRef) was passed through rustOutputType unchanged, so the Rust template
emitted invalid trait and extism_pdk::Json<$crate::pkg::types.SongRef> signatures
that do not compile.
Method input/output types now resolve through the shared registry: trait
signatures use rustTraitType (shared -> nd_pdk_types::X, locals stay bare) and the
export macros use rustMethodType (fully qualified: shared -> nd_pdk_types::X,
primitives -> Rust, locals -> $crate::<pkg>::X). Verified end-to-end by compiling
a generated capability that takes types.SongRef directly against the real
nd-pdk-types crate.
* fix(plugins): canonicalize XTP export refs for renamed shared aliases
buildSchemas canonicalized alias-to-canonical references for struct-field $ref
targets, but buildExport built export input/output $refs straight from
fieldBaseType. A capability whose export used a renamed deprecated alias
directly (e.g. type TrackInfo = types.Track with NowPlaying(TrackInfo)) emitted
$ref: #/components/schemas/TrackInfo, while the component is emitted under the
canonical name Track — a dangling export reference.
Lift the alias-to-canonical map into GenerateSchema (buildAliasToCanonical) and
apply it to export refs via canonicalRefName, the same resolution already used
for field properties.
* fix(plugins): route shared macro types through $crate for plugin builds
When a capability method used a shared type directly, the generated export macro
named the type as nd_pdk_types::SongRef. The macro expands in the downstream
plugin crate, which depends on the umbrella nd-pdk crate and not on nd-pdk-types
directly, so that path is unresolvable there and the plugin fails to build.
rustMethodType (macro-facing) now emits $crate::types::X, and the generated
nd-pdk-capabilities lib.rs re-exports nd_pdk_types as types so $crate resolves
it. Trait signatures keep nd_pdk_types::X since they live in nd-pdk-capabilities,
which has the direct dependency. Verified end-to-end: a plugin crate depending
only on the umbrella that uses a capability with a direct types.X method now
compiles via the macro.
* fix(plugins): add nd-pdk-types dependency to the Rust host crate
When a host service uses a shared type, ndpgen emits nd_pdk_types::X into the
generated nd-pdk-host client wrappers, but the host crate's manifest did not
depend on nd-pdk-types, so the crate failed to compile with 'unresolved module
nd_pdk_types'. Host client wrappers are plain functions resolved in the host
crate's own context (not macros expanded downstream), so a direct dependency is
the right fix.
Add nd-pdk-types = { path = "../nd-pdk-types" } to nd-pdk-host, mirroring
nd-pdk-capabilities. Found while auditing all Rust paths against the realistic
crate topology after the capability-side $crate fix; verified by generating a
host service with a shared-type return and confirming cargo check fails before
and passes after.
* fix(plugins): resolve shared aliases in Rust host signatures
The Rust host client rendered method params and returns through
RustTypeWithStructs, which only consults KnownStructs. A host service using a
shared alias in a signature (e.g. type Track = types.Track plus
MatchSongs(...) ([]Track, error)) therefore emitted a bare Vec<Track>, but the
client template emits no Track alias or import, so the generated nd-pdk-host
crate did not compile. Only struct fields went through the shared map.
rustType/rustParamType now use the shared map too (RustTypeWithShared /
RustParamTypeWithShared), so an aliased param/return resolves to its canonical
nd_pdk_types::X path, matching field handling. Verified by generating a host
service returning a shared alias and confirming cargo check fails before and
passes after.
* fix(subsonic): align album `created` with RecentlyAddedByModTime sort
The album `created` attribute returned by search3, getAlbumList2 and the
other album endpoints was always sourced from the album's CreatedAt (oldest
song birth time), while the "recently added" sort is governed by
RecentlyAddedByModTime: it orders by album.updated_at when that option is
enabled and album.created_at otherwise.
As a result, when RecentlyAddedByModTime was enabled, clients that cache
album results and sort locally by `created` (e.g. for a "Date added" view)
could not reproduce the order returned by getAlbumList2?type=newest, since
the exposed value did not match the column driving the sort.
Make albumCreatedAt config-aware so the primary timestamp it returns mirrors
recentlyAddedSort: UpdatedAt when RecentlyAddedByModTime is set, CreatedAt
otherwise. The existing zero-value fallback chain is preserved so this
required OpenSubsonic field is never emitted as zero on legacy rows.
Note: this is a behavior change for the contractual `created` attribute. With
RecentlyAddedByModTime enabled, an album's reported `created` now reflects the
newest song modification time and can change when files are modified.
Scope is limited to albums; the song-level `created` (BirthTime) is unchanged.
* fix(subsonic): order Recently Added by full-precision timestamp with tiebreak
The recently_added sort wrapped the timestamp in datetime(), truncating it
to whole seconds, and had no secondary sort key. Album timestamps carry
sub-second precision (aggregated from song file birth-times), so on a fresh
scan many albums tie at the second; SQLite then returns ties in query-plan
order, which changes when a library filter is applied. This made the web UI
"Recently Added" order invert between library selections and diverge from
getAlbumList2?type=newest, and clients receiving the full-precision created
value could never reproduce the server order.
Sort on the raw, full-precision column with an album.id / media_file.id
tiebreak instead. A new migration swaps the album datetime() expression
indexes (and the plain media_file indexes) for composite (col, id) indexes
that cover the new sort. Timestamps were already normalized to space-format
by 20260316000000_normalize_timestamps, so raw-string comparison is safe.
* fix(subsonic): make song created follow RecentlyAddedByModTime
The song Created field returned BirthTime (file ctime), but the recently_added
sort (shared by the Subsonic and native APIs, and the web UI) orders by
created_at, or updated_at when RecentlyAddedByModTime is set. A Subsonic client,
which can only sort by the created value it receives, could therefore never
reproduce the server's "recently added" order in either mode.
Add mediaFileCreatedAt mirroring albumCreatedAt and use it for child.Created:
CreatedAt by default, UpdatedAt under RecentlyAddedByModTime, with BirthTime as
a legacy fallback. This aligns song created with the sort column, matching how
album created already works and what the native API/web UI present.
Pulls in the lyric timing fixes: lyrics no longer stack from the previous
song on rapid track changes (#5661), stay in sync after seeking/scrubbing
(including while paused), and show a music-note placeholder during intros
and gaps instead of the 'no lyrics' message.
* feat(plugins): export ReadPackageManifest and ValidatePackage for off-disk inspection
* test(plugins): cover ValidatePackage cross-field validation branch
* feat(cmd): add 'plugin list' command
* refactor(cmd): drop premature pluginManager interface until first use
* feat(cmd): add 'plugin enable' and 'plugin disable' commands
* feat(cmd): add 'plugin edit' for config and permission updates
* test(cmd): cover plugin edit aborting on config validation failure
* feat(cmd): add 'plugin info' and 'plugin validate' (installed id or .ndp file)
* test(cmd): unit-test formatManifestInfo nil-guard and json branches
* feat(cmd): add 'plugin rescan' command
* feat(cmd): enrich plugin info text output and re-validate config on validate
* fix(cmd): omit zero-value timestamps in plugin info text output
* fix(cmd): give plugin list and info separate format flag vars
A shared package-global bound to both commands' --format flag caused the
later init() registration (info, default "text") to clobber the earlier
one (list, default "table"), so 'plugin list' with no -f failed with
'invalid output format "text"'. Split into pluginListFormat /
pluginInfoFormat and add a regression test on the registered defaults.
* fix(plugins): address code-review findings on plugin CLI
- edit: read-modify-write merge so flipping one permission flag no longer
wipes unspecified fields (matches the native API); reject non-JSON
--users/--libraries values.
- info: return an error on unknown --format (was silently text); include
sha256 in off-disk JSON output; align the text columns.
- move declaredPermissions onto plugins.Permissions.DeclaredNames() as the
single source of truth; drop ValidatePackage's redundant Validate call.
- readConfigFile: pass ctx to log.Fatal; copy flag globals before taking
their address; trim comments that restated the code.
* refactor(plugins): export ReadManifest/ComputeFileSHA256, drop thin CLI wrappers
Remove the ReadPackageManifest/ComputeFileSHA256/ValidatePackage wrappers in
favor of exporting the underlying readManifest->ReadManifest and
computeFileSHA256->ComputeFileSHA256 directly. ValidatePackage was identical to
ReadPackageManifest (readManifest already validates via ParseManifest), so the
CLI's validate path now calls ReadManifest too.
* test(plugins): consolidate ReadManifest specs and move ComputeFileSHA256 tests
Merge the two ReadManifest Describe blocks into one, and move the
ComputeFileSHA256 tests out of package_test.go into a new manager_sync_test.go
(where the function now lives), deduping the overlapping hash specs.
* test(plugins): use createTestPackage everywhere, drop writeNDP helper
The schema-invalid and cross-field ReadManifest specs now build packages with
createTestPackage (typed Manifest) instead of the raw-JSON writeNDP helper, so
there is a single package-building helper. Also trims the gratuitous 1MB wasm
buffer in the read-only spec to a few bytes.
* test(plugins): drop duplicate missing-manifest spec from ReadManifest
The openPackage block already covers the missing-manifest.json error path
(shared zip-walk code); ReadManifest's identical copy added no distinct
coverage.
* test(plugins): fix misleading ReadManifest spec name and drop unused wasm bytes
The spec was named 'should read only the manifest without loading wasm' but
ReadManifest returns only (*Manifest, error) — it cannot assert whether wasm was
loaded. Rename to what it actually verifies (manifest parses from a package that
also contains a wasm entry) and pass nil wasm bytes, since the contents are
never observed.
* fix(cmd): address PR review feedback on plugin CLI
- edit --users/--libraries now accept both comma-separated and JSON-array input
(CSV is converted to the JSON the manager stores); help text documents both.
- Permissions.DeclaredNames() derives names by reflecting over the generated
struct's json tags instead of a hand-maintained list, so new permission types
are picked up automatically.
- isPackagePath checks only the .ndp suffix, so a mistyped package path yields a
precise 'no such file' error instead of falling back to a misleading
'Plugin not found'.
- Restore a ReadManifest test for the missing-manifest.json error path (it has
its own branch, separate from openPackage).
* docs(cmd): trim verbose comments to the why
* fix(cmd): setting an explicit users/libraries list clears the all-* flag
Per Codex review: with allUsers/allLibraries true, 'plugin edit --users <list>'
preserved the all-* flag so the allow-list was silently ignored, and the
mutually-exclusive flags meant it couldn't be corrected in one command. An
explicit list now implies all-*=false.
Fixes a transient jump to the wrong song when switching the play queue.
When a new queue was loaded at a non-zero index (e.g. playing a different
album/playlist from a track other than the first, or playing a new album
after closing the player), the web player briefly loaded and played the
track that sat at the *previous* internal index in the new queue before
correcting to the chosen one — an audible "skip to a random song, then
back to the song I chose".
The root cause was in the player library: when loading a new audio list,
the initial track was picked using the stale internal play index instead
of the requested playIndex. Fixed in navidrome-music-player 4.25.3
(navidrome/react-music-player), which derives the initial track from the
requested playIndex.
* fix(lyrics): correct TTML background-vocal cue timing and whitespace
Two parsing defects surfaced by Apple Music TTML files that mix a main
vocal with an x-bg (background) span group within the same line:
- Cue end-time normalization ran over the whole line's cue list in
document order. Background cues are stored after the main cues but
interleave earlier on the timeline, so the next-cue clamp collapsed the
last main cue's end down to its own start (start == end). End times are
now normalized per agent group, matching how the Subsonic serializer
already groups cues, so parallel layers no longer corrupt each other.
- Whitespace between elements was treated as significant: pretty-printed
(indented) TTML injected spurious newlines into the line text, turning
one line into many. Per TTML2 default xml:space handling (linefeeds
treat-as-space, whitespace-collapse), formatting whitespace now collapses
to a single space and hard line breaks come only from <br/>.
The line-level value and per-agent cueLine.value remain the full line text,
as required by the OpenSubsonic songLyrics v2 contract; the per-agent text
is carried in each cueLine's cue[] array.
Two existing tests that encoded the buggy newline-as-break behavior are
corrected; new tests cover whitespace collapse, <br/> preservation, and
interleaved background cue timing.
* fix(lyrics): only collapse XML whitespace, preserve other Unicode spaces
Whitespace collapsing used unicode.IsSpace, which matches more than the XML
S production (space, tab, CR, LF): it also folds characters like NBSP and
U+3000 into a regular space, silently altering content. Restrict collapsing
to the four XML whitespace characters so other Unicode spaces pass through
unchanged, and add a regression test. Also clarify the doc comment that
collapsing is applied unconditionally (xml:space="preserve" is not supported).
* refactor(agents): drop single Artist fields from Song, keep only Artists
Song now represents credited artists solely via the Artists slice; the
single Artist/ArtistMBID fields and the ArtistList passthrough are removed.
Equals continues to hash the whole value.
* refactor(lastfm,listenbrainz): build Song.Artists in built-in agents
Last.fm and ListenBrainz now populate the Artists slice directly. For
ListenBrainz top songs, all credited artist MBIDs are mapped (the combined
display name on the first credit plus MBID-only collaborators) instead of
keeping only the first MBID, feeding the matcher's per-MBID specificity.
* refactor(external): set Song.Artists in top-songs enrichment
getMatchingTopSongs now seeds an Artists entry from the known artist when a
song carries none, replacing the single Artist/ArtistMBID writes.
* refactor(plugins): fold single-artist SongRef into Song.Artists
SongRef keeps its single Artist/ArtistMBID fields as part of the plugin wire
contract; songRefToAgentSong now folds them into a one-element Artists list
when a plugin sends no artists array.
* refactor(matcher): read Song.Artists directly
groupQueries consumes s.Artists now that ArtistList is gone; test inputs
build the Artists slice.
* fix(matcher): keep MBID-only artists as identity signals
Review follow-up: an artist credited only by MBID (empty ID and name) was
dropped before resolution in four places, defeating the multi-MBID matching
path this PR adds.
- matcher.groupQueries: treat a non-empty MBID as a usable artist signal
- external.getMatchingTopSongs: backfill the primary credit's name/MBID when
the agent left them empty
- plugins.songRefToAgentSong: fold a single-artist SongRef when only ArtistMBID
is set (not just when Artist name is set)
- listenbrainz.topSongArtists: return nil instead of an empty-name placeholder
when neither name nor MBIDs are present
* fix(external): only backfill top-song artist onto an unnamed credit
Review follow-up (codex P2): the previous backfill stamped the queried
artist's MBID onto Artists[0] whenever it was empty, even when that credit
already named a different (e.g. featured) artist — producing a mismatched
name+MBID pair that could mis-rank matches. Now only an unnamed first credit
is filled (it is, by construction, the queried artist); an already-named
credit is left untouched.
Signed-off-by: Deluan <deluan@navidrome.org>
---------
Signed-off-by: Deluan <deluan@navidrome.org>
* feat(matcher): add Song.Artists (agents.Artist) and field-wise song dedup
* refactor(matcher): make song equality an agents.Song.Equals method via hashstructure
Move the sameSong free function from core/matcher into an Equals method on
agents.Song, following the model.MediaFile/Album.Equals convention. Uses strict
hashstructure hashing (nil opts, no IgnoreZeroValue) to preserve the original
whole-value equality contract. Tests moved to core/agents.
* feat(matcher): match by multiple artists with overlap ranking and artist-ID fast-path
* refactor(matcher): rank artist overlap and specificity above the preferred-track flag
Identity signals (specificityLevel, artistOverlap) now outrank the taste
signal (preferredMatch) in betterThan. A starred/4-star track that is a worse
identity match no longer beats a more specific or higher-overlap track.
PreferStarred still breaks ties when specificity and overlap are equal.
* fix(matcher): score artist-MBID specificity against all credited artists, not just the last
sanitizedTrack.artistMBID (string) replaced with artistMBIDs (map[string]struct{}) so
bucketTracks collects all credited owned MBIDs per query instead of last-write-wins.
computeSpecificityLevel tests set membership, letting each of a collaboration's
MBID-bearing artists reach the proper specificity level (4/5) independently.
* feat(plugins): carry multiple artists (with IDs) through SongRef conversions
* feat(plugins): regenerate schemas and PDK wrappers for multi-artist SongRef
* refactor(matcher): tidy bucketTracks accumulator and artist resolution
Replace bucketTracks' two parallel per-track maps (overlapByQuery/mbidsByQuery)
with a named queryAccum struct (F2). Collapse resolveArtists' four hand-mutated
parallel maps into a pendingArtist slice with derived nameToQueries/mbidToQueries
maps (F1). Replace the own() method on resolvedArtists with a package-level
addToSet helper that drops the method/receiver indirection (F3).
* docs(matcher): trim comments that restate the code
* fix(plugins): use Vec::is_empty for slice fields in generated Rust PDK
* fix(matcher): treat a resolved artist ID as an identity match for specificity
* docs(matcher): reflect artist-ID identity in the specificity ladder
* fix(smartplaylist): use annotation index for playcount/rating/loved filters
Annotation-field criteria wrapped the column in COALESCE(col, default) so
missing annotation rows behave as 0/false. COALESCE prevents SQLite from
using the column index, forcing a full media_file scan during smart playlist
materialization - multi-second loads on large libraries, independent of rule
complexity.
Store the raw column plus its default and drop COALESCE when the compared
value cannot match the default; fall back to 'col <op> ? OR col IS NULL' when
the default would match, so never-annotated tracks are still preserved.
Sorting keeps COALESCE to retain deterministic NULL ordering. Result set is
unchanged; the materialize query now seeks the annotation index.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): keep COALESCE for list-valued annotation comparisons
Hardening from final review: a list value (IN (...)) can't drive the index
and a default-inclusive list has per-element NULL semantics, so route slice
values through COALESCE(col, default) to stay exactly equivalent to the prior
form. Also make the bool-default branch explicit (loved only supports
equality operators) and share the COALESCE rendering via coalesceExpr.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): make coalesced() a field method
Thermo-nuclear review follow-up: promote the free coalesceExpr(f) to a
smartPlaylistField.coalesced() method that returns the bare expression when
there is no default. This lets sortExpr call field.coalesced() unconditionally
and drop its 'if coalesceDefault != nil' branch, removing the 'only annotation
fields get coalesced' special case from the sort path. Behavior unchanged.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): keep COALESCE for LIKE, bool-ordering, and tag ranges
Code review (xhigh) found the index-friendly rewrite did not cover every
operator, breaking result-set equivalence on a few reachable raw-JSON paths:
- LIKE family (contains/startsWith/endsWith/notContains) on annotation fields
used the bare column, so a NULL column never matched and missing-annotation
rows were dropped.
- Ordering comparators (gt/lt/...) on bool fields (loved) were decided as
equality, wrongly including never-annotated rows.
- InTheRange on a numeric tag split into two independent json_tree EXISTS,
letting different tag values satisfy each bound.
Centralize the decision in annotationCond via bareNullInclusion: emit the
index-friendly bare form only for scalar values under an exactly-orderable
comparator, otherwise fall back to the COALESCE form (always equivalent to the
original). Route LIKE through coalesced(); reject tag/role ranges. Replace the
local toFloat/toBool with spf13/cast (fixes unhandled numeric types and string
bool forms), and drop the redundant LookupField + double reflect.TypeOf.
A 17-case brute-force check confirms row-set equivalence to the prior
COALESCE form across all operators including the fixed LIKE/bool cases.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): keep COALESCE for list values on bool annotation fields
Second review found the bareNullInclusion bool branch missed the non-scalar
guard the numeric branch has: a list value on loved/albumloved/artistloved
(e.g. {"is":{"loved":[true]}}) coerced through toBool (which swallowed the
cast error) to false, emitting the bare/OR-IS-NULL form and wrongly including
never-annotated rows. Make toBool return (value, ok) like toFloat and bail to
COALESCE when the value isn't a scalar bool. Also add the missing test for the
tag/role range rejection. A 21-case brute-force confirms row-set equivalence to
the original COALESCE form across every operator, including the bool/numeric
list paths.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): drop spf13/cast for stdlib value coercion
The value coercion only sees the handful of types criteria produces (int,
float64, string from JSON; bool already normalized at unmarshal), so cast's
broad conversion isn't needed. Use small explicit type switches over strconv
instead, keeping the string fallback (ParseFloat/ParseBool) that closes the
'1'/'t' gap. No dependency change — cast returns to indirect.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): share bool coercion via criteria.ToBool
normalizeBoolValue (unmarshal-time) and the persistence bool guard both parsed
bool-ish values independently. Extract the shared logic into an exported
criteria.ToBool(any) (bool, ok): normalizeBoolValue delegates to it (behavior
unchanged), and the persistence layer reuses it via its existing model/criteria
import instead of a local helper. No behavior change.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): trim sqlLiteral and dedup rationale comments
/simplify cleanup: fmt %v already renders bool defaults as false/true, so drop
sqlLiteral's redundant bool branch. Consolidate the COALESCE-vs-index rationale
to the smartPlaylistField comment instead of repeating it across annotationCond
and the struct. No behavior change.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): address review feedback on multi-field maps and *any
From the PR bot reviews:
- sqlFields now uses the field's coalesced() form, so annotation fields in a
multi-field operator map (Is/Gt/Contains with >1 key) keep COALESCE and don't
silently drop never-annotated rows. Covers both the comparison and LIKE
fallback paths. (Gemini high, Copilot)
- Replace coalesceDefault *any with a plain any (0/false are non-nil
interfaces, so nil still means 'no default'); drop the coalesce() boxing
helper and the pointer indirection. (Gemini)
- Give rangeExpr clear, range-specific errors for the multi-field and malformed
-pair cases instead of an empty-field / 'in operator' message. (Copilot)
Adds tests for the multi-field COALESCE behavior and the new range errors.
Signed-off-by: Deluan <deluan@deluan.com>
* Revert multi-field COALESCE handling (YAGNI)
The multi-field operator map case the bots flagged is unreachable: marshalExpression
rejects any operator map with more than one field, so a multi-field map can never be
persisted or loaded. Revert the sqlFields change and its tests rather than harden a
code path no supported input can reach. Keep the two reachable improvements from the
review: coalesceDefault any (not *any), and the clearer malformed-range error.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(persistence): model comparator as a behavior-carrying struct
The smart-playlist comparator was a bare string alias, forcing two parallel
switches over the same six operators: squirrelCmp mapped each to its squirrel
constructor, and bareNullInclusion restated each as a float predicate. Adding
or changing an operator meant editing both in sync.
Make comparator a struct that bundles those facts per operator (the squirrel
builder, the operator as a float predicate, and whether it's an ordering op).
Both switches collapse: squirrelCmp is deleted in favor of cmp.build, and
bareNullInclusion's numeric switch becomes a single cmp.satisfy call. Generated
SQL is unchanged, as the existing table-driven tests confirm.
* docs(smartplaylist): trim comments that restate the code
Remove or tighten comments that describe what the code already says (likeCond and
comparisonExpr doc lines, redundant clauses in annotationField/coalesced/ToBool/
normalizeBoolValue). Keep the comments that explain non-obvious rationale: the
COALESCE-vs-index tradeoff, the bareNullInclusion/annotationCond contracts, and the
why-we-fall-back notes.
* docs(smartplaylist): collapse coalesceDefault comment to one line
The field's six-line block duplicated the COALESCE-vs-index rationale that already
lives on annotationCond. Reduce it to a one-line description plus a pointer there.
---------
Signed-off-by: Deluan <deluan@deluan.com>
* feat(subsonic): add Work/Movement response types and tag constants
* feat(subsonic): surface works and movements in Child response
* test(subsonic): verify works/movements JSON serialization
Fix G109 lint: use strconv.ParseInt with bitSize=32 to avoid potential
integer overflow; add JSON serialization test confirming omitempty on
optional sub-fields.
* refactor(subsonic): use number.ParseInt idiom in buildMovements
* refactor(subsonic): move work/movement builders to MediaFile methods
Introduces model.Work and model.Movement types with Works()/Movements()
methods on MediaFile. The Subsonic layer maps them to response types inline
via slice.Map, replacing the deleted buildWorks/buildMovements helpers.
* test(subsonic): cover populated works/movements in response snapshots
* test(subsonic): clarify empty-case name and assert JSON structurally
* feat(cache): add completion marker helpers to spreadFS
* feat(cache): write completion marker after successful cache write
* fix(cache): adopt only complete files on reload, grandfather existing caches
* test(cache): regression tests for partial-transcode crash leftover (#5636)
* test(cache): guard concurrent in-progress streaming with completion marker
* test(cache): make concurrent-streaming guard actually attach a second reader mid-write
The previous test obtained s2 only after pw.Close(), so no reader ever
attached to the in-progress entry. Now pw.Write("hello ") is called
synchronously before the second Get — io.Pipe's blocking write gives a
deterministic happens-before — then both s1 and s2 are drained in parallel
goroutines while the producer writes the rest and closes the pipe.
* style(cache): clarify best-effort intent of cleanup os.Remove calls
* refactor(cache): lift one-time grandfather pass out of Reload's steady-state loop
* refactor(cache): have MarkComplete take the key, owning path mapping in spreadFS
* test(cache): assert no completion marker is written when the write fails
* refactor(cache): rename migration sentinel to generic .nd-migrated
* refactor(cache): rename grandfather migration to migrateExistingFiles
* refactor(cache): single-pass Reload with safer marker-error handling
Address PR review feedback:
- Merge the one-time migration into Reload's single directory walk,
avoiding a second full walk on first boot.
- Only delete a data file when its marker is definitively absent
(os.IsNotExist); skip on other stat errors to avoid destroying valid
entries under transient I/O failures.
- Write the migration sentinel only after a clean walk, so a partial
walk can't strand valid-but-unmarked files for later deletion.
- Return early from walkDataFiles on a WalkDir error.
- Assert fs.Create error in the marker-removal test.
* fix(playlist): respect the user's library access when resolving M3U paths
FindByPaths looked up paths across all libraries, so importing an M3U
could add tracks from libraries the importing user has no access to.
Apply the user's library filter to the lookup, matching every other
media_file read. Admins and the (admin-context) scanner are unaffected.
* fix(share): scope shared playlist tracks to the owner's libraries
loadMedia loaded playlist tracks with a fake-admin context, so a shared
playlist could include tracks from libraries the owner has no access to.
Load as the share owner instead, so the library filter applies.
Admin-owned shares are unchanged.
* fix(share): only serve shared tracks the owner can access
A shared stream fetched the media file by id without checking the share
owner's library access, so it could serve tracks from libraries the
owner has no access to. Gate share-scoped streams on the owner's library
access. Non-share streams are unaffected.
* test(share): tidy library-access test setup
Consolidate the repeated share-owner test fixture in handleStream into a
helper, assert on track fields with HaveField instead of building an id
slice, and delete the scratch media file through the public repository
method.
* style: trim verbose comments in library-access checks
* fix(share): guard against nil owner and clean up test users
Add a nil check after loading the share owner so a missing user yields a
clear error instead of a possible nil dereference, and delete the users
created by the new tests in their AfterEach blocks.
* fix(share): avoid panic when a shared playlist is no longer visible to its owner
Tracks() returns nil when the playlist can't be loaded under the owner's
context (e.g. a public playlist shared by a non-owner that was later made
private). Capture the result and return early instead of chaining GetAll
on a nil repository, leaving the share with no tracks.
* refactor(storage): extract LocalPathToURL from storage.For
* test(storage): re-enable local storage tests on Windows (#5381)
* test(storage): fix Windows drive-letter path expectation
The 'should handle Windows drive letters correctly' test was gated behind
the now-removed SkipOnWindows BeforeEach, so its expectation had never run.
On Windows, newLocalStorage re-joins u.Host+u.Path via filepath.Join, which
yields a backslash path (C:\music), not C:/music. Assert against
filepath.Join so the expectation matches the OS-native result.
* test(storage): probe Windows drive-letter path in LocalPathToURL
Three review bots flagged that LocalPathToURL escapes the drive-letter
colon (C: -> C%3A), which url.Parse rejects. There are no Windows bug
reports, so add a Windows-gated test that exercises the real conversion
on a drive-letter path and let CI decide whether the bug is real before
changing production code.
* fix(artwork): convert decoded images to a fast-path type before resizing
x/image/draw's CatmullRom scaler only has optimized paths for *image.RGBA,
*image.NRGBA, *image.Gray and *image.YCbCr. Other concrete types — notably
*image.NYCbCrA (from WebP) and *image.Paletted (indexed PNGs) — fall back to
a generic per-pixel At()/RGBA() loop that is several times slower.
Convert such images to *image.RGBA once before scaling; fast-path types are
returned unchanged. This makes resize performance independent of which decoder
wins the image.Decode("webp") registration, and also speeds up indexed PNGs.
Signed-off-by: Deluan <deluan@navidrome.org>
* chore(deps): update gen2brain/webp to v0.6.0
v0.6.0 replaces the wazero WASM runtime with a self-contained
wasm2go-transpiled WebP decoder/encoder. This drops the webp -> wazero
dependency edge (wazero is still used by the plugin system) and makes the
WASM-only build path (32-bit / nodynamic) faster and far lighter on
allocations.
Signed-off-by: Deluan <deluan@navidrome.org>
* perf(artwork): defer fast-path conversion until a resize is needed
Move toFastScaleType to just before the CatmullRom.Scale call, after the
no-upscale early return. Previously the conversion ran right after decode, so a
request for a size >= the source dimensions would allocate and walk a full RGBA
copy only to discard it when resizeStaticImage returns nil. The resize path is
unchanged; the no-op path drops ~30-40% time and up to ~79% memory for large
indexed/WebP artwork.
Signed-off-by: Deluan <deluan@navidrome.org>
---------
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor(matcher): carry resolved artist MBID on sanitizedTrack
* feat(matcher): two-phase artist resolution for title matching
* fix(matcher): wire phase-1 artist mock in consumer tests; guard back-map dupes
- External tests for title-fallback paths now register a second `.Maybe()`
artistRepo.GetAll expectation for the matcher's phase-1 artist-resolution
call, and supply RoleArtist Participants on phase-2 track mocks so the
back-map routes tracks to their query buckets correctly.
- Back-map loop extracted into buildTracksByQuery helper; guard ensures each
track is appended at most once per query bucket even when it credits multiple
resolved artists in that bucket, preventing duplicate scoring.
- matchTitlePhase2 helper: removed redundant squirrel.Sqlizer type assertion
(ranging over squirrel.And already yields Sqlizer).
* perf(matcher): use non-correlated IN subquery for phase-2 lookup
* refactor(matcher): decompose two-phase title matching into named steps
matchByTitle had grown to ~98 lines holding four jobs and eleven locals. Split
it into a thin orchestrator over named phases:
- groupQueriesByArtist: build the per-artist query buckets.
- resolveArtists / resolvedArtists: phase 1, with the three parallel maps
(byQuery / mbid / allIDs) folded into one type that owns artist-ID routing and
track bucketing. Artist ownership is now two direct map lookups (by order name,
by MBID) instead of the prior O(artists x queries) nested scan.
- fetchTracksCreditedTo: phase 2, using squirrel.Placeholders instead of a
hand-rolled placeholder string.
- bucketTracks / scoring loop: unchanged behavior.
Pure restructuring; the suite, race, consumers, and the real-DB benchmark are all
unchanged. Also dedupes the phase-1 MBID filter as a side effect.
* refactor(matcher): apply simplify cleanups to two-phase resolution
Behavior-preserving cleanups from a /simplify pass:
- struct{} sets instead of bool-valued sets (codebase convention)
- move newSanitizedTrack after the dedup guard in bucketTracks, so a track
credited to multiple resolved artists is sanitized only when actually bucketed
- pre-size the phase-1/phase-2 maps; drop the dead nil-guard in own()
- slice.Map for the []string->[]any arg conversion
- document why the raw media_file_artists SQL stays in this layer, and that
bucketTracks relies on the bulk participants JSON carrying artist IDs
- extract an artistParticipants() test helper, collapsing 57 four-level
Participants literals (test file -211 lines net)
* docs(matcher): rename inner title-matching steps to avoid 'phase' clash
The matcher's top-level strategies (ID/MBID/ISRC/Title) are already called phases.
Reusing 'phase 1/2' for the two steps inside title matching (resolve artists,
fetch their tracks) was confusing. Drop the ordinals and let the function names
(resolveArtists, fetchTracksCreditedTo) and prose describe the steps. Renamed the
matchTitlePhase2 test helper to matchTracksByArtistQuery. Comment-only; no behavior
change.
* fix(matcher): own MBID-resolved artists for every aliased query
Address bot review on PR #5637. When several agent queries share one
ArtistMBID under different sanitized names (agent aliases), the resolver
kept only the last query name per MBID, so the others never owned the
resolved artist and their songs fell through. Track all query names per
MBID instead; add a RED-proven test for the two-alias case.
Also invert byQuery into a reverse artist-ID -> query-name index in
bucketTracks, dropping the per-participant scan over all queries, and
guard fetchTracksCreditedTo against an empty artist-ID slice. Fix the
artist mock to forward variadic options with Called(options...) so
QueryOptions-shaped matchers receive the same argument shape as real
calls.
* docs(matcher): trim comments that restated the code
Cut three doc comments down to their why: groupQueriesByArtist, own, and
bucketTracks no longer restate what the signature and body already show.
Condense fetchTracksCreditedTo's rationale from two paragraphs to one,
keeping the role='artist', non-correlated-IN, and layer-boundary notes.
* refactor(matcher): resolve matches in song-index space
* test(matcher): pin per-index duration matching for duplicate title+artist songs
* refactor(matcher): drop unreachable specificity sentinel
* refactor(matcher): hoist PreferStarred read out of scoring loop
* docs(matcher): correct config field references in MatchSongs doc
* fix(matcher): log swallowed per-artist DB error in title matching
* fix(matcher): fail title matching when all artist lookups error
* refactor(matcher): simplify loaders and test helpers
* docs(matcher): move algorithm docs to package-level doc.go
* docs(matcher): focus examples on fuzzy matching behavior
* refactor(matcher): store index in dedup map and harden test helper
* fix(matcher): keep exact-phase matches when all title lookups fail
* perf(matcher): batch title-phase artist lookups into one query
matchByTitle issued one GetAll per distinct artist, run serially. On a large
library a batch of similar-songs spans dozens of artists, and profiling against
a 95k-track library showed the matcher was ~90% bound in that serial query loop
(a 100-song batch fired ~89 separate multi-join queries, taking ~6s).
Replace the loop with a single 'order_artist_name IN (...)' query, then group the
returned tracks by artist in memory and score each song against its bucket. This
cuts a 100-song batch from ~6s to ~0.4s (roughly 14x) with less memory.
Grouping keys on order_artist_name (the field the query filters on, matching how
the per-artist queries are keyed), falling back to the sanitized Artist when it
is unset. Because there is now a single query, matchByTitle is all-or-nothing
like the ID/MBID/ISRC loaders: the per-artist best-effort skip is gone, while
resolveMatches still preserves exact-phase matches when the title query fails.
* refactor(matcher): group batched title matches by order_artist_name
After batching the title-phase lookups into one query, the returned tracks must
be grouped back to their artist. Key on MediaFile.OrderArtistName — the exact
field the query filters on — so collaboration/"feat." tracks (whose display
Artist differs from the sort artist) bucket correctly, with a sanitized-Artist
fallback when it is unset.
OrderArtistName is deprecated in favor of Participants, but the bulk GetAll path
does not hydrate participant detail (the rich artist fields come only from the
per-record GetWithParticipants JOIN), so the participant order name is empty here
and the column is the only populated source.
Also adds a TODO in computeSpecificityLevel: its artist-MBID levels read the
deprecated, unpopulated MediaFile.MbzArtistID column, so they never fire today.
* feat(scanner): add Scanner.IgnoreDotFolders to allow scanning dot folders
Adds a new Scanner.IgnoreDotFolders option (default true, preserving current
behavior) that, when disabled, lets the scanner traverse folders whose names
start with a dot, such as albums like ".Hack Sign Original Soundtrack".
Previously every dot-prefixed entry was skipped unconditionally before the
directory check, so such album folders were never indexed. The walk loop now
determines whether an entry is a directory first, then skips dot-prefixed files
always and dot-prefixed folders only when IgnoreDotFolders is enabled. Special
system directories are still ignored in all cases via the ignoredDirs blocklist,
which now also lists .git explicitly (it was previously caught only by the
generic dot-prefix rule). isDirIgnored is reduced to a pure blocklist check and
the name-only predicate is renamed from isEntryIgnored to isDotEntry.
* refactor(scanner): centralize entry ignore policy in isIgnoredEntry
Consolidates the directory-entry ignore decision into a single isIgnoredEntry
helper so the walk loop reads as pure dispatch (recurse into directories,
classify files) instead of interleaving ignore policy with traversal.
The dot-prefix rule and the ignoredDirs blocklist were previously checked in two
separate places inside loadDir's loop. They are now combined behind one helper
that takes the entry name and whether it is a directory. isDirIgnored remains a
standalone blocklist predicate because the file watcher (isIgnoredPath) calls it
directly. Adds focused unit tests for isIgnoredEntry covering both states of
Scanner.IgnoreDotFolders. No behavior change.
* fix(scanner): stop watcher from scanning ignored dot folders
A filesystem change inside a dot-prefixed folder (e.g. ".Hidden Album/track.mp3")
previously triggered a targeted scan of that folder, because isIgnoredPath let
all media files through and only checked the changed path's parent against the
ignore list (which never matched for nested paths due to the trailing separator).
With Scanner.IgnoreDotFolders enabled this caused the folder to be indexed even
though a full scan would skip it.
The watcher now ignores any change located inside an ignored directory via a new
isUnderIgnoredDir helper that reuses the same isIgnoredEntry policy as the scan
walk, and checks the entry itself with isIgnoredEntry instead of the parent dir.
This keeps the watcher and the scanner in agreement for both dot-folders (gated
by the flag) and the ignoredDirs blocklist. Adds direct table tests for
isIgnoredPath covering both states of the option.
* fix(scanner): exclude '.' from isDotEntry and ignore dot media files in watcher
Addresses code review feedback:
- isDotEntry now excludes the literal "." reference, matching its documentation.
Previously isDotEntry(".") returned true, which could mark a path component as
a dot-entry in the watcher.
- isIgnoredPath now ignores dot-prefixed media files (e.g. ".hidden.mp3") so the
watcher matches the scanner, which always skips dot files. Non-media leaves
still fall through to the directory-assumption check, so dot-folders continue
to follow Scanner.IgnoreDotFolders.
Adds unit tests for isDotEntry and watcher coverage for dot-prefixed media files.
* docs(scanner): clarify isDotEntry multi-dot exclusion and add test
Expand the isDotEntry comment to explain why names with two or more
leading dots (".."/"..foo"/"...Album") are not treated as hidden, which
surprised a reviewer testing dot-folder scanning. Add a "..foo" test case
to make the two-leading-dots behavior explicit.
Claude-Session: https://claude.ai/code/session_012STiDTyhZAdH8JNtdNe8L1
* refactor(lyrics): read sidecar files via library storage FS
Routes fromExternalFile reads through storage.For(mf.LibraryPath) instead
of os.Open on AbsolutePath, fixing sidecar reads for non-local backends.
UTF-16 LE/BE and BOM handling preserved via ioutils.UTF8Reader.
* refactor(lyrics): address review feedback on sidecar FS read
- Move blank local-storage import from sources.go into lyrics_suite_test.go
(the test suite already imports the local package for RegisterExtractor,
so local's init() runs; production binaries get the scheme via normal wiring)
- Fix misleading comment: model.ParseLyrics → model.ParseLyricsFile
- Replace what-comment with why-comment in BeforeSuite explaining the
log.Fatal guard that requires the no-op extractor registration
* test(lyrics): add subsonic e2e baseline for getLyrics endpoints
Establishes a behavioral baseline for getLyricsBySongId (v2 structured)
and getLyrics (legacy) before the lyrics parser refactor. Covers embedded
formats (LRC synced, plain text, TTML) and sidecar formats (LRC, SRT,
YAML), all isolated under a Lyrics/ fixture folder so the new fixtures
do not perturb existing test behavior beyond fixture counts.
Sidecar files are injected as raw &fstest.MapFile{Data: []byte(...)}
entries; the scanner skips non-audio extensions (.lrc, .srt, .yaml) so
they are invisible to scanning but reachable via the fake FS at request
time through fromExternalFile/storage.For.
Update album/artist/song counts in the album-list, multi-library, and
search3 empty-query tests to reflect the six new tracks (1 new artist,
1 new album, 6 new songs).
* test(lyrics): strengthen e2e lyrics baseline (lang assertions, rename helper)
Rename the local helper `main` to `firstLyric` to avoid collision with the
reserved-feeling built-in name. Add `Lang` assertions to both embedded and
sidecar DescribeTable entries, locking the current observed values: "xxx"
(ISO 639-2 "no language specified") for all embedded and LRC/SRT sidecars,
and "eng" for the YAML sidecar (which explicitly sets `language: eng`).
* feat(lyrics): detect Lyricsfile YAML in content-sniffing
* feat(plugins): content-sniff plugin lyrics for all formats
Replace model.ToLyrics (LRC/plain only) with model.ParseEmbedded so plugin
responses are content-sniffed for TTML, SRT, YAML, LRC, and plain text.
ParseEmbedded returns a LyricList, so the loop now flattens multiple tracks
per response entry.
The test-lyrics WASM plugin gains a "ttml" format mode (configured via
pdk.GetConfig) that returns a minimal TTML document; rebuilt with the
standard Go wasip1 toolchain (GOOS=wasip1 GOARCH=wasm). A new Ginkgo test
asserts Synced==true and the exact cue value, which the old plain-text path
could not produce.
GetLyrics doc comment updated to reflect content-sniffing; a later task will
retarget it to ParseLyrics once that function is introduced.
* test(plugins): validate plugin lyrics auto-detect across all formats
The test-lyrics WASM plugin now supports per-format modes via the
"format" config key: ttml, srt, yaml, lrc, and plain, in addition to
the existing default plain-text response. The plugin is rebuilt with the
standard Go wasip1 compiler.
lyrics_adapter_test.go gains a DescribeTable covering all five formats,
asserting both Synced (the discriminator that proves correct format
detection) and the exact line value. This validates the full
auto-detect chain (TTML → SRT → YAML/Lyricsfile → LRC → plain) end-to-end
through the real plugin → adapter → parser flow.
* refactor(lyrics): consolidate parsers into model.ParseLyrics
* refactor(lyrics): retarget legacy callers to model.ParseLyrics
Pin suffix to ".lrc" to preserve byte-identical output for stored
plain/LRC text that was previously handled by the now-removed ToLyrics.
* test(lyrics): fix lyrics tests after parser consolidation
- Rewrite the YAML-fallback test to assert the correct design: a
non-Lyricsfile .yaml sidecar returns as plain text and shadows
lower-priority sources (rather than falling through to .lrc).
- Add LibraryPath + relative Path split to the three subsonic tests
that read sidecar files via storage.For(), so they resolve against
the correct fixtures directory.
- Register a no-op extractor in api_suite_test.go BeforeSuite so
newLocalStorage does not fatal when storage.For is called during
sidecar-lyrics tests.
* test(lyrics): add per-format ParseLyrics benchmarks
Baseline measurements (count=2 runs) on M2:
BenchmarkParseLyrics_LRC-8 5725 178796 ns/op 49.78 MB/s 427877 B/op 523 allocs/op
BenchmarkParseLyrics_Plain-8 5425 230854 ns/op 32.44 MB/s 102508 B/op 16 allocs/op
BenchmarkParseLyrics_EnhancedLRC-8 1942 605893 ns/op 17.66 MB/s 860678 B/op 4256 allocs/op
BenchmarkParseLyrics_SRT-8 3249 373991 ns/op 25.91 MB/s 1113575 B/op 4407 allocs/op
BenchmarkParseLyrics_TTML-8 1483 813027 ns/op 13.86 MB/s 2198052 B/op 8665 allocs/op
BenchmarkParseLyrics_YAML-8 1700 678250 ns/op 13.01 MB/s 1235096 B/op 8288 allocs/op
BenchmarkParseLyrics_SniffTTML-8 1525 776482 ns/op 14.51 MB/s 2225448 B/op 8681 allocs/op
BenchmarkParseLyrics_SniffSRT-8 2528 451210 ns/op 21.48 MB/s 1157000 B/op 4422 allocs/op
BenchmarkParseLyrics_SniffYAML-8 1333 827152 ns/op 10.67 MB/s 1337195 B/op 8718 allocs/op
BenchmarkParseLyrics_SniffLRC-8 2820 413038 ns/op 21.55 MB/s 588934 B/op 1812 allocs/op
BenchmarkParseLyrics_SniffPlain-8 2968 409091 ns/op 18.31 MB/s 254470 B/op 1491 allocs/op
Content-sniff path overhead: 1.5–15% depending on format.
* test(lyrics): use real public-domain fixtures for parser benchmarks
Replace synthetic benchmark payloads with 'Auld Lang Syne' (Robert Burns,
1788, public domain) rendered into every supported format (LRC, plain,
enhanced LRC, SRT, TTML, Lyricsfile YAML) so the numbers reflect realistic
content. Same song across formats makes per-format cost comparable.
Baseline (Apple M-series, -benchmem, real fixtures):
LRC ~28 us/op 42 KB 147 allocs
Plain ~23 us/op 18 KB 22 allocs
EnhancedLRC ~37 us/op 51 KB 374 allocs
SRT ~52 us/op 139 KB 581 allocs
TTML ~119 us/op 276 KB 1227 allocs
YAML ~142 us/op 193 KB 1732 allocs
Sniff(LRC) ~47 us/op 57 KB 237 allocs
Sniff(TTML) ~122 us/op 282 KB 1250 allocs
Sniff(YAML) ~186 us/op 218 KB 1847 allocs
Fixtures in tests/fixtures/lyrics/.
* fix(lyrics): preserve [] (not null) for empty lyrics in backfill migration
ParseLyrics returns nil for zero-line input (whitespace-only stored
lyrics). json.Marshal(nil LyricList) produces null, violating the DB
invariant that media_file.lyrics uses [] for empty lyrics, never null.
Initialize to model.LyricList{} when ParseLyrics returns nil so the
marshalled result is always [].
* refactor(lyrics): unify parser dispatch and centralize empty-list invariant
Apply thermo-nuclear review findings (behavior-preserving):
- Replace the suffix switch + three single-use closure adapters
(parseTTMLKnown/parseSRTKnown + inline YAML closure) with a
bySuffix map of a single lyricParser(lang, contents) signature.
Normalize parseTTMLWithDefaultLang/parseSRTWithLanguage to that
(lang, contents) order so no adapter glue is needed.
- Collapse the parallel sniffLyrics engine into one parseFirstMatch
primitive shared by both the suffix and content-sniff paths
(sniffOrder candidate list). TTML stays gated via parseTTMLIfDocument
in sniff mode to avoid running the XML decoder on plain/LRC text.
- Add LyricList.MarshalJSON so empty/nil always serializes to [] (the
lyrics column invariant), in one canonical place. Delete the
migration's nil-guard, which the marshaler now subsumes.
Behavior verified unchanged: full suite + race + e2e green.
* refactor(lyrics): single registry drives both suffix dispatch and sniff order
Collapse the bySuffix map and sniffOrder slice into one ordered registry:
slice order is the content-sniff probe order, each row's suffixes drive
sidecar dispatch, and per-row bySuffix/byContent parsers preserve the
gated-TTML-when-sniffing distinction. One source of truth, no duplicated
parser references.
* refactor(lyrics): self-skipping parsers collapse the format table to one column
Move the TTML <tt>-document gate into parseTTMLWithDefaultLang itself (after
the encoding fixup, so UTF-16-declared docs are still recognized): non-TTML
content returns (nil, nil) to skip; a malformed <tt> document still errors.
SRT and Lyricsfile YAML already self-skip. With every structured parser
self-skipping, the format table drops to one {suffixes, parse} column named
lyricFormats — no bySuffix/byContent split, no separate sniff-only TTML gate.
Both the suffix and content-sniff paths share the same parser per format.
* refactor(lyrics): strip BOM once at ParseLyrics entry for all paths
Previously only the content-sniff path stripped the BOM; the suffix path
relied on its callers (fromExternalFile via UTF8Reader) having already
stripped it. That implicit contract was fragile — a caller passing raw
BOM-prefixed bytes with a suffix would reach the parsers with the BOM intact
(SanitizeText does not strip it). Strip once at entry so every path and
parser sees clean bytes regardless of caller. No-op for already-stripped
input.
* refactor(lyrics): trim verbose comments to essential why
* refactor(lyrics): move LRC parser to its own lyrics_lrc.go
Extract parseLRC, the enhanced-LRC helpers (parseEnhancedLine, adjustGroup,
stripEnhancedMarkers, shiftELRCCues), parseTime, and the LRC regexes from
lyrics.go into lyrics_lrc.go, with the parseLRC tests in lyrics_lrc_test.go.
This makes the layout symmetric — one file per format (lrc/srt/ttml/yaml) —
and leaves lyrics.go holding only shared types and cue normalization. All
moved symbols were already LRC-private; no behavior change.
* refactor(lyrics): collapse ParseLyrics suffix/sniff branches into one loop
Both modes differ only in which formats to try, so select candidates in a
single loop (all formats when sniffing, the suffix's own otherwise) and run
them through parseFirstMatch once. Drops the projected-slice make+index and
the ContainsFunc closure; unmatched suffixes yield no candidates and fall to
the plain-text floor, as before.
* refactor(lyrics): apply simplify-review cleanups
- stripBOM: bytes.TrimPrefix instead of []byte<->string round-trip (no alloc)
- ParseLyrics: pre-size the candidates slice
- move isTTMLDocument to lyrics_ttml.go beside its only caller (the dispatch
layer should hold no per-format knowledge)
* refactor(lyrics): simplify test descriptions for structured lyrics
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor(lyrics): fold parseLyricsfile into lyricParser signature and rename file
- parseLyricsfile now matches the lyricParser signature directly (reads via
bytes.NewReader), removing the parseLyricsfileBytes adapter and the
string(contents) copy; the lyricFormats table references it directly.
- StructuredLyrics drops the vestigial LyricList{} init (json.Unmarshal
overwrites; MarshalJSON owns the empty->[] invariant).
- Rename lyricsfile.go -> lyrics_lyricsfile.go (and its test) to match the
lyrics_<format>.go convention used by lrc/srt/ttml.
* refactor(lyrics): move test-only parseTTML/parseSRT wrappers to test files
These zero-arg wrappers (defaulting lang to "xxx") had no production callers
after the consolidation — only the format tests used them. Move each beside
its tests so the production files carry no test-only code.
* build: exclude generated *_gen.go files from linting
The plugin host *_gen.go files (ndpgen output) were tripping the whitespace
linter despite carrying a generated marker. Exclude them by path so make lint
and the pre-push hook pass on untouched generated code.
* perf(lyrics): drop []byte/string round-trips in parsers
Apply code-review feedback to remove avoidable allocations in the lyrics
parsers. isTTMLDocument now takes []byte directly, so parseTTMLWithDefaultLang
no longer copies its buffer into a string before the TTML probe. parseSRTBlock
splits its block with strings.Split instead of converting to []byte and back
per line. ParseLyrics hoists strings.ToLower(suffix) out of the format loop.
No behavior change; the dropped len(scanner)==0 SRT guard was dead (strings.Split
never returns an empty slice, and the existing len(lines)==0 check still covers
empty input).
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor(lyrics): colocate and unexport cue-normalization helpers
Move the cue-normalization machinery out of lyrics.go into a dedicated
lyrics_normalize.go (with lyrics_normalize_test.go), leaving lyrics.go to hold
just the shared lyric types and their methods. lyrics.go was mixing the domain
type/contract definitions with format-agnostic post-processing.
Unexport normalizeLyrics, normalizeCueLines, and normalizeLineTiming: they have
no callers outside the model package, so they should not be part of its public
API. NormalizeCueEnds stays exported because the Subsonic enhanced-lyrics
serializer (server/subsonic/lyrics.go) resolves cue ends per agent group while
building the response; that is the only legitimate cross-package caller.
Also includes a small no-op robustness tweak in parseLRC: len(times) == 0
instead of times == nil (equivalent here, more idiomatic).
No behavior change.
* test(lyrics): add direct coverage for NormalizeCueEnds
NormalizeCueEnds is exported and carries the most intricate logic in the
normalization cluster (fill-from-next, fill-from-fallback, both clamps, and the
all-or-none clear), but was only exercised transitively. Add a focused spec
covering each branch plus the empty-input and no-mutation guarantees, bringing
the function to 100% coverage.
* test(lyrics): cover legacy getLyrics across formats and sources
Expand the legacy getLyrics e2e coverage from a single embedded-plain case to a
table over all six fixtures: embedded LRC/plain/TTML and sidecar LRC/SRT/YAML.
Each case asserts the v1 plain-text fallback contract — the structured lyric is
flattened to LRC-style plain text with no timing markup leaking through (no LRC
brackets, SRT arrows, or XML tags), regardless of the source format or whether
it is embedded or a sidecar file. This pins the behavior that synced TTML/SRT/
YAML formats degrade gracefully to plain text on the legacy endpoint.
* test(lyrics): cover songLyrics v1 vs v2 with word-level fixtures
Correct and expand the e2e lyrics coverage to match the OpenSubsonic songLyrics
extension contract:
- v1 (getLyricsBySongId, no enhanced): line-level lyrics with no cueLine, kind,
or agents — even for word-level formats (ELRC, Lyricsfile YAML).
- v2 (getLyricsBySongId?enhanced=true): word-level cueLine surfaces for ELRC and
YAML sources; kind="main" is set; a line-level source (SRT) still yields no
cueLine even when enhanced.
- legacy getLyrics (artist/title): the original Subsonic endpoint, flattening any
format to plain text. A prior commit mislabeled this as the "v1 contract";
getLyrics predates OpenSubsonic and is unrelated to the extension versions.
Drive these with the public-domain tests/fixtures/lyrics files (the same set the
parser benchmarks use) so the e2e content stays in sync and actually carries the
word-level timing needed to distinguish v1 from v2. The embedded "synced LRC"
fixture is upgraded to ELRC (word-level); track counts are unchanged, so the
rest of the suite is unaffected.
* test(lyrics): parameterize v2 enhanced coverage across all formats
Convert the v2 (enhanced) e2e block from three ad-hoc cases into a DescribeTable
covering all six formats, matching the v1 and legacy tables. Each entry declares
whether the source carries word-level timing: ELRC, TTML, and Lyricsfile YAML
surface a cueLine; LRC, SRT, and plain text do not. All six get kind="main".
Add word-level <span> timing to the first line of the auld-lang-syne.ttml
fixture so TTML exercises the word-level cueLine path (the parser already
supports <span begin/end>, but the fixture was line-level only). The first line
now yields the same five word cues as the ELRC and YAML fixtures, keeping the
table assertions uniform across formats.
* fix(lyrics): honor caller language when Lyricsfile YAML omits it
parseLyricsfile discarded the caller's language argument, so a Lyricsfile YAML
parsed from an embedded tag or plugin response with no metadata.language was
labeled "xxx" even when ParseLyrics was given a language. The SRT and TTML
parsers already use the caller language as their default; fall back to it here
too, preferring the document's own metadata.language when present.
Also reword a misleading TTML comment: isTTMLDocument still runs an XML decode
(it stops at the first element), so the skip avoids the full TTML parse, not the
XML decoder entirely.
* refactor(lyrics): consolidate lyrics parsing functions names
Signed-off-by: Deluan <deluan@navidrome.org>
* test(lyrics): drop test-only parse wrappers after parser rename
Commit 48c0173e8 renamed the production parsers to parseTTML/parseSRT, which
collided with the same-named test-only wrappers and broke the model test build
(parseTTML/parseSRT redeclared). Remove the wrappers and call the production
parsers directly with the placeholder language at each test site.
* test(lyrics): complete the truncated enhanced-LRC fixture
The auld-lang-syne.elrc fixture stopped after the first two stanzas (8 lyric
lines) while every other format fixture carries the full 24-line song. Extend it
to all 24 lines with per-word timing so it is a faithful enhanced-LRC sample and
the EnhancedLRC parser benchmark runs on a workload comparable to the others.
The first line's word timings are unchanged, so the e2e cueLine assertions still
hold.
---------
Signed-off-by: Deluan <deluan@navidrome.org>
Expand backend lyrics support with richer sidecar formats and upgrade the
OpenSubsonic songLyrics implementation to the version 2 structured karaoke
contract, while preserving version 1 behavior by default.
Sidecar formats and parsing:
- Add a TTML parser (core/lyrics/ttml.go): clock time, offset time, bare
decimal seconds, nested timing contexts, and token-level <span> timing for
word/syllable karaoke. Parses Apple Music-style metadata tracks (translation
and pronunciation/transliteration) and agent metadata into per-track agents[]
plus per-cue-line agentId. Hydrates missing line timing from cue timing.
- Add an SRT parser (core/lyrics/srt.go).
- Add a LRCLIB Lyricsfile (.yaml/.yml) parser (model/lyricsfile.go): maps
per-word lines[].words[] to cues with inclusive UTF-8 byte offsets and
attributes overlapping lines to synthetic voice agents so parallel vocals
split correctly in the enhanced response.
- Extend LRC parsing for Enhanced LRC inline <mm:ss.xx> word-timing markers.
- Add UTF-8 BOM and UTF-16 LE support for TTML/LRC sidecars.
- Parse the above formats from embedded tags as well as sidecar files.
Source resolution:
- Default lyricspriority is now
".ttml,.yaml,.yml,.elrc,.lrc,.srt,.txt,embedded" so the new formats are
discoverable without manual configuration.
- Preserve configured source priority across duplicate media-file candidates
instead of only checking the first DB match, so higher-priority sidecar
lyrics on older duplicates can still win.
- Raise the embedded-lyrics tag maxLength to 1 MB to fit word-timed
TTML/Enhanced-LRC karaoke for a full song.
OpenSubsonic songLyrics v2:
- Advertise songLyrics versions [1, 2].
- With enhanced=true, getLyricsBySongId may return structuredLyrics.kind
(main/translation/pronunciation), cueLine[] line-level karaoke groupings,
cueLine.cue[] timed words/syllables with required UTF-8 byteStart/byteEnd,
reusable structuredLyrics.agents[], and cueLine.agentId references.
- Without enhanced=true, the response stays v1-compatible: no kind, no cueLine,
no agents, no non-main tracks; the existing line[] payload is always
populated so legacy clients keep working.
Contract details:
- cueLine is emitted only for synced lyrics with cue data.
- Within a cueLine, cue.end is normalized all-or-none and overlaps are removed;
overlaps across separate cueLines remain valid for parallel vocal layers.
- Missing cue end-times are filled from the next cue or the parent line.
- When cueLines share an index, the one whose agent has role "main" is first.
- LyricCue.Value is serialized as XML chardata; cues with nil start are skipped
rather than serialized as 0.
Refactoring:
- Move pure format parsers into model/ (lyrics.go, lyrics_ttml.go,
lyrics_srt.go, lyrics_embedded.go, lyricsfile.go) and extract Subsonic
response building into server/subsonic/lyrics.go.
- Centralize lyric-kind constants and add Lyrics.EffectiveKind/IsMainKind.
- Add gg.Clone helper.
Spec references:
https://github.com/opensubsonic/open-subsonic-api/discussions/213https://github.com/opensubsonic/open-subsonic-api/pull/218 (songLyrics v2)
https://github.com/opensubsonic/open-subsonic-api/pull/228 (cue byte offsets)
The scanner classified a file by the name of the directory entry, so a
symlink was treated as audio/image/playlist based on the link name rather
than what it actually points to. Now symlinks are fully resolved (following
the whole chain) and classified by the resolved target's extension, so a
symlink to a non-audio file is no longer imported as a track.
This also makes Scanner.FollowSymlinks apply to file symlinks, not just
directory symlinks as before. The default stays true, so following symlinks
to real audio files (second drives, shared folders, etc.) keeps working.
Adds trace logging for symlink resolution decisions and real-fs regression
tests covering multi-level symlink chains.
Thread the context.Context that goose.UpContext already passes into every
migration through to all DB calls: tx.Exec/Query/QueryRow become
tx.ExecContext/QueryContext/QueryRowContext with ctx. The shared helpers in
migration.go (notice, forceFullRescan, isDBInitialized) gain a ctx parameter
and all call sites are updated. No-op migration functions use blank params
(_ context.Context, _ *sql.Tx).
This is a behavior-preserving change: the SQL, arguments, and ordering of every
migration are unchanged; only cancellation/deadline propagation is added.
Add a forbidigo lint rule scoped to db/migrations/ that forbids the
non-context tx.Exec/Query/QueryRow forms, preventing regression.
Signed-off-by: Deluan <deluan@navidrome.org>
* fix(transcoding): preserve source metadata when transcoding downloads
Default transcoding commands used `-map 0:a:0` with no metadata mapping, so
transcoded files lost all source tags (title, artist, album, etc.). Downloads
in the original format were unaffected because the file is copied byte-for-byte.
Add `-map_metadata 0 -map_metadata 0:s:0` to the default commands. Both flags
are required: `-map_metadata 0` copies format-level tags (MP3/FLAC sources)
and `-map_metadata 0:s:0` copies stream-level tags (OPUS/OGG sources), which
store tags at different levels.
The flags are added in three coordinated places, since for users on the default
command the args are built programmatically (buildDynamicArgs) rather than from
the stored command string:
- consts.go default commands, for new installations
- buildDynamicArgs, the active path for default-command users
- a migration updating only rows that still hold the exact old default, so
customized commands are left untouched
AAC is included for consistency but remains a no-op: its `-f adts` container
cannot hold metadata, and the MP4 alternative breaks pipe streaming.
Fixes#5623
* fix(transcoding): target audio stream for metadata and propagate ctx
Address review feedback on the metadata-preservation change:
- Use `-map_metadata 0:s:a:0` instead of `0:s:0` to copy tags from the first
audio stream specifically. When a source has embedded cover art exposed as a
video stream at index 0 (common in music files), `0:s:0` pulls the image
stream's metadata and the audio tags are lost. Verified empirically with
ffmpeg 7.1.3: a source with video at stream 0 and a tagged audio stream loses
its title under `0:s:0` but keeps it under `0:s:a:0`; audio-only OPUS/MP3/FLAC
sources are unaffected by the change.
- Propagate the migration context via `tx.ExecContext(ctx, ...)` instead of
discarding it, so the migration honors cancellation/timeouts.
Claude-Session: https://claude.ai/code/session_015iFHDzX53wCKt11qFHMeZk
* perf(subsonic): speed up artist search3 deep-offset pagination
Empty-query and FTS artist search (search3/search2) paginated via a
CROSS JOIN library_artist + DISTINCT in Phase 1 purely for library access
control. The DISTINCT forced a temp b-tree over the whole junction table on
every page, making deep offsets O(offset): ~200ms at offset 299k on 300k
artists.
Replace it with a join-free EXISTS predicate keyed on artist.id, backed by a
new covering index on library_artist(artist_id, library_id). EXISTS keeps
artist as the ordered driver and never fans out rowids, so Phase 1 stays a
plain ordered scan that LIMIT/OFFSET can short-circuit. Admin, headless, and
all-libraries users skip the filter entirely (the dominant case) for a flat
ordered walk over the primary key.
Measured on a 300k-artist / 1M-song library: admin/all-libs pagination is
~4.5-5.4x faster at depth (~180ms to ~33ms at offset 400k); restricted
subset users keep correct, gap-free pages while also getting faster.
The narrowing artist filter is applied at the subsonic layer only when the
request targets a strict subset of the user's libraries, so the common case
(and the admin fast-path) is never burdened with a redundant predicate.
* fix(subsonic): narrow artist search by library set, not count
narrowsArtistLibraries decided whether to add the subsonic-layer artist
narrowing filter by comparing len(requested) < len(accessible). musicFolderId
is not deduplicated, so duplicate IDs inflated the requested count: a user
requesting ?musicFolderId=1&musicFolderId=1&musicFolderId=2 against three
accessible libraries produced len([1,1,2])==3, which is not < 3, so the filter
was skipped and the user saw artists from the third library too.
Compare as set membership instead: the request narrows iff some accessible
library is absent from it (requested is always a subset of accessible, validated
upstream by selectedMusicFolderIds). This is immune to duplicate IDs. Add a
regression test that fails against the old length-based check.
Also consolidate the repeated EXISTS/no-DISTINCT/O(page) rationale that the
prior commit spread across five sites down to a single authoritative comment on
ArtistLibraryFilter, with the call sites referencing it.
* perf(subsonic): drop redundant library_artist covering index
The migration added an index on library_artist(artist_id, library_id) on the
theory that the restricted-subset artist-search EXISTS needed it to seek by
artist_id. Benchmarking on a 405k-artist / 5-library dataset showed no benefit:
the EXISTS subquery constrains both columns (artist_id = and library_id IN), so
SQLite already resolves it as a covering-index seek on the existing
(library_id, artist_id) UNIQUE autoindex. With the new index present the planner
still picks the autoindex and ignores it.
Drop the migration and correct the comment. Removing ~11MB of dead index plus
its write-amplification on every library_artist insert/delete, for zero query
gain.
* fix(scanner): mark artists missing when they lose their last library
Artist search Phase 1 filters on artist.missing and Phase 2 inner-joins
library_artist, so a non-missing artist with no library_artist row (an orphan)
takes a pagination slot in Phase 1 and then vanishes in Phase 2, shortening the
page and shifting deep offsets. The admin/headless search fast-path walks artist
unfiltered, so it is fully exposed to this.
Two paths created such orphans without updating artist.missing:
- RefreshStats deletes library_artist rows whose stats are '{}' (artist lost all
content in a library) after every scan. This is the common source.
- Library deletion cascades away the library's library_artist rows.
Mark newly-orphaned artists missing at both sources, so the shared
'missing = false' search filter excludes them immediately instead of waiting for
a later scan. In RefreshStats the update only runs when the cleanup actually
removed rows (the only way a new orphan can appear), so steady-state scans pay
nothing; measured ~160ms on 300k artists only when orphans can exist.
* refactor(subsonic): address review feedback on artist search filter
Code-review follow-ups to the artist search pagination change:
- ArtistLibraryFilter: short-circuit to a constant-false predicate when no
library IDs are given, avoiding a degenerate empty IN () subquery.
- ArtistLibraryFilter: add an inner LIMIT 1 to the correlated EXISTS so SQLite
cannot flatten it into a fan-out join (an artist in multiple of the user's
libraries would otherwise yield duplicate rowids and corrupt pagination).
- narrowsArtistLibraries: compare accessible-vs-requested as a set lookup
instead of slices.Contains in a loop.
- searchConfig.LibraryFilter: document that a join-free filter is now a
correctness requirement (DISTINCT was removed), not just a performance one.
* docs: trim verbose comments in artist search/orphan code
Condense the over-explained comments added in this PR to the essential 'why',
removing repeated cross-references and restatements of the adjacent code.
* fix(scanner): heal pre-existing orphan artists on full refresh
The orphan-marking added to RefreshStats only ran when its empty-stats cleanup
deleted rows, so it reconciled newly-created orphans but not ones already left
in the database by older versions (whose library_artist row was deleted before
this fix existed). Such legacy orphans would surface in the admin/headless search
fast-path as short/gappy pages.
Also run the orphan-marking on a full refresh (allArtists), so a full scan — which
upgrades commonly trigger and users can run manually — reconciles the backlog. No
migration needed; the runtime fixes prevent recurrence.
* perf(subsonic): extend artist search fast-path to all-library users
applyLibraryFilterToSearchQuery only skipped the library filter for admin and
headless processes. A regular (non-admin) user who can access every library has
the same result set as an admin, but was still given the EXISTS filter — an
O(offset) cost for a predicate that matches every non-missing artist anyway.
Skip the filter for them too, using a cheap library CountAll() (a count over the
tiny library table) compared against the user's library count. On any error it
falls back to the filtered path, which is correct, just slower.
* fix(scanner): log error as trailing arg, not explicit error key
Signed-off-by: Deluan <deluan@navidrome.org>
* test(scanner): e2e guard for orphan artists under PurgeMissing
Adds an end-to-end scanner test for the orphan-artist invariant fixed in
RefreshStats: with Scanner.PurgeMissing enabled, removing all of an artist's
files hard-deletes them, cascades away their media_file_artists rows, and
RefreshStats then drops the artist's emptied library_artist row. The test
asserts no non-missing artist is left without a library_artist row. Verified it
fails without the RefreshStats orphan-marking and passes with it.
* test(scanner): assert the orphaned artist is marked missing
The orphan e2e test only checked the aggregate no-orphan invariant
(orphanCount == 0), which a fully-deleted artist or an un-cleaned row would also
satisfy — so it could pass without exercising the fix. Assert Pink Floyd's row
specifically: missing=false before, missing=true after, and absent from the
non-missing results. Verified it fails without the RefreshStats orphan-marking.
* test(scanner): drop misleading non-missing-list assertion for orphan
GetAll has no default missing filter, but selectArtist inner-joins library_artist,
so an orphaned artist (no junction row) is excluded from the results whether or
not it is marked missing. The Not(ContainElement) check therefore passed for the
wrong reason. The direct floydMissing() == 1 query is the assertion that actually
validates the missing flag; keep that plus the orphan-count invariant and an
over-marking guard on The Beatles.
* test(scanner): document why orphan check reads the artist row directly
Clarify that GetAll cannot observe the orphan: selectArtist inner-joins
library_artist, so an artist with no junction row is excluded from results
whether or not it is marked missing. Asserting on GetAll would pass even without
the fix, so the test reads the artist row directly to check the missing flag.
* test(scanner): return descriptive artist state for clearer failures
floydState returns PRESENT/MISSING/NOT_FOUND instead of 0/1/-1, so a failure
reads '<string>: PRESENT to equal MISSING' rather than '0 to equal 1'.
* refactor(subsonic): keep artist library scoping in the repository
The search endpoint built a persistence-layer EXISTS predicate
(persistence.ArtistLibraryFilter) and injected it into artistOpts.Filters — the
only place the subsonic package reached into persistence, leaking a storage
detail up two layers.
Pass the same Eq{"library_id": ids} filter used for albums and songs, and let
the artist repository translate it to the join-free library_artist predicate
(scopeSearchToLibraries), where the junction knowledge belongs. The subset-vs-
fast-path decision moves there too, so narrowsArtistLibraries and the persistence
import are gone from the subsonic layer. Behavior is unchanged; coverage for the
translation moves to artist_repository_test.
* refactor(persistence): extract canonical markOrphansMissing helper
The 'mark non-missing artists with no library_artist row as missing' invariant
was hand-written as SQL in two places (RefreshStats and libraryRepository.Delete),
in two slightly different dialects (not exists vs id not in). Extract a single
artistRepository.markOrphansMissing method next to markMissing and call it from
both sites, so the invariant has one definition.
* fix(persistence): apply scoped library filter in both search phases
Two bugs from moving the artist library-scoping into the repository:
- Search() scoped opts.Filters for Phase 1 but still passed the original
(unscoped) options to selectArtist, so Phase 2 re-applied the raw
Eq{library_id} against the wrong columns and a restricted user's search
returned nothing. Pass the scoped opts to both phases.
- scopeSearchToLibraries dropped the filter unconditionally for admins, so an
admin explicitly narrowing via musicFolderId (e.g. search3?musicFolderId=2)
leaked content from other libraries. Compare the request against the user's
visible library set (all libraries for admin/headless), narrowing whenever it
is a strict subset.
Both regressions were caught by the server/e2e multi-library suite.
* fix(core): delete library and reconcile orphans in one transaction
libraryRepository.Delete runs the FK-cascade delete and the orphaned-artist
reconciliation (markOrphansMissing) as two writes on r.db. Called directly they
autocommit separately, so an interruption between them could leave non-missing
artists with no library_artist row — the orphan state the artist search
fast-path forbids. Wrap the deletion in ds.WithTx at the core wrapper so both
writes commit atomically; the watcher/scanner/broker side-effects stay
post-commit.
* refactor(persistence): unify artist search library scoping into one filter
Phase 1 previously applied two overlapping library predicates: cfg.LibraryFilter
(scoped to the user's libraries) AND options.Filters (the requested subset),
producing two correlated EXISTS subqueries per rowid even though the request is
always a subset of the user's libraries. And the 'does this user see everything'
decision was implemented twice (userHasAllLibraries via CountAll vs
scopeSearchToLibraries via set-membership), with applyLibraryFilterToSearchQuery
as a third scoping path.
Resolve the effective library scope once in Search() via searchScope (intersect
the requested set with the user's visible libraries; nil = fast-path), clear
opts.Filters, and realize that single scope as the only Phase-1 LibraryFilter.
The visibility logic is now one pipeline: requestedLibraryIDs + visibleLibraryIDs
+ userSeesAllLibraries. Behavior unchanged; one EXISTS instead of two on the hot
path, one source of truth for library visibility.
* fix(persistence): harden artist search against malformed library_id filter
Search consumed only an Eq{"library_id": []int} filter; an Eq whose library_id
value wasn't []int slipped through unconsumed and would reach Phase 1's bare
artist table (no library_id column) → SQL error. Recognize any Eq carrying a
library_id key (isLibraryIDFilter) and always consume it, falling back to the
user's visible scope for a malformed value. Non-library filters are still left
in place for doSearch.
* refactor(persistence): trim redundant comments and unexport artist library filter
The artist-search-pagination work left dense explanatory comments, with the
join-free / LIMIT-1 anti-flatten rationale and the orphan-artist mechanics each
restated in several places. Consolidate each rationale into one canonical home
(artistLibraryFilter for the EXISTS/LIMIT-1 trick, markOrphansMissing for the
orphan lifecycle) and have the other sites reference it instead of repeating it.
Also unexport ArtistLibraryFilter to artistLibraryFilter: its only caller is
searchCfg in the same package and no test references it, so it never needed to
be part of the package's exported surface.
Comments only plus the rename; no behavior change.
* refactor: add slice.ToSet and use it for the artist search subset check
searchScope's subset test compared the requested libraries against the visible
set with a nested slices.Contains, which is O(visible * requested). On an instance
with many libraries (e.g. 100 libraries, a user granted 99) and an explicit
musicFolderId request, that is ~9.8k comparisons; with a set it is ~200.
Add a small reusable slice.ToSet helper (a slice -> map[T]struct{} set, collapsing
duplicates) and use it to make the membership lookups O(1), restoring O(n+m) without
the throwaway struct{}{} literal that an inline ToMap would need. No behavior change.
* refactor(artist): move artistLibraryFilter to artist_repository
Signed-off-by: Deluan <deluan@navidrome.org>
---------
Signed-off-by: Deluan <deluan@navidrome.org>
getRandomSongs used a single ORDER BY random() over the media_file table. At large
library sizes that forces SQLite to scan the wide table and sort every matching row
before applying the limit — roughly 4 seconds for a 1M-track library, regardless of how
many songs are requested.
Add MediaFileRepository.GetRandom, which does this in two passes: first select N random
rowids over a narrow index (filters + library scope only, no wide columns or joins), so
the random sort runs over compact, index-friendly data; then hydrate just those rows with
the full select. The wide media_file row is never part of the sort. End-to-end this drops
getRandomSongs from ~4s to ~0.3s on a 1M-track library, and the cost no longer grows with
the requested size.
The handler now calls GetRandom directly. The filter helper that builds the genre/year
filters is renamed SongsByRandom -> SongsByGenreAndYearRange to reflect what it does, since
the random ordering is now owned by GetRandom rather than a Sort option. Filters (genre,
year, library) compose into the first pass unchanged. The album random list path is left
as-is (far fewer rows, already fast).
* feat(stream): add ClientInfo.ForceFormat for browser-aware forced format
Restricts the client to a forced transcoding format and suppresses direct
play, but only when the client declares it supports that format. Part of #5583.
* fix(transcoding): honor player forced format on getTranscodeDecision
When the WebUI player has a forced transcoding format configured and the
browser declares it can play that format, transcode to it (suppressing
direct play). Fall back to normal negotiation with a warning when the
format is unsupported. The MaxBitRate cap still applies on top. Fixes#5583.
* test(e2e): cover player forced format on getTranscodeDecision
Forced format honored when the client supports it, falls back to negotiation
otherwise, and the MaxBitRate cap still applies on top. Part of #5583.
* feat(ui): remove obsolete 'format ignored' helper text on player form
The web player now honors the forced transcoding format, so the caveat added
in #5611 no longer applies. Reverts the Transcoding field to a plain selector.
Part of #5583.
* fix(transcoding): enforce player MaxBitRate on getTranscodeDecision
The Web UI streams via getTranscodeDecision, which (since #5473) ignored
the server-side player config. Apply the player's MaxBitRate as a bitrate
ceiling on the client's declared limits before MakeDecision, restoring
per-player bitrate enforcement without reintroducing the forced-format
override. Fixes#5583.
* test(e2e): assert player MaxBitRate is enforced on getTranscodeDecision
Invert the assertions added in #5473 that expected the player cap to be
ignored; getTranscodeDecision now enforces it (issue #5583).
* feat(ui): clarify web player ignores forced transcoding format
Add helper text to the Transcoding field on the player edit form when the
player is the NavidromeUI web client, since it enforces only the Max. Bit
Rate, not the forced format. Part of issue #5583.
* refactor(stream): extract ClientInfo.CapBitrate, share across transcode paths
Move the player MaxBitRate ceiling logic into a canonical ClientInfo.CapBitrate
method in core/stream, used by both getTranscodeDecision and the legacy
ResolveRequest path. Removes handler-layer duplication and corrects a
misleading comment that wrongly implied the legacy single-field cap was buggy.
* fix(transcoding): downsample on legacy /stream when only player MaxBitRate is set
A bare /stream or /download request from a player configured with a
server-side MaxBitRate (but no forced format) was served raw, ignoring the
cap. buildLegacyClientInfo now triggers DefaultDownsamplingFormat when the
player MaxBitRate alone is below the source bitrate, matching the
already-correct forced-format and request-bitrate paths. Part of #5583.
* fix(ui): add Brazilian Portuguese translation for player transcoding helper text
Translates the new resources.player.helperTexts.transcodingId key added for
the web player transcoding-format clarification. Part of #5583.
* fix(ui): restore Transcoding field styling and render helper text
The TranscodingInput wrapper swallowed the variant SimpleForm injects into
its direct children (field lost its outlined box) and put helperText on the
ReferenceInput, which does not forward it to the input. Spread the form props
onto ReferenceInput and move helperText to the SelectInput child so both the
outlined styling and the helper text render. Part of #5583.
* fix(i18n): update Brazilian Portuguese translation for album artist field
Signed-off-by: Deluan <deluan@navidrome.org>
* fix(ui): clean up comments in PlayerEdit component
Signed-off-by: Deluan <deluan@navidrome.org>
* test(ui): mock useTranslate in PlayerEdit test for determinism
Avoid depending on ra-core's out-of-provider translation behavior, which can
vary by version. Part of #5583.
---------
Signed-off-by: Deluan <deluan@navidrome.org>
* fix(scanner): import playlists skipped when no admin existed yet (#5499)
On a fresh install the first scan runs before any admin user exists, so the
scanner's playlist phase skips all playlists (playlists are owned by the
first admin). Nothing re-imported them afterwards because folder selection
is gated on updated_at > last_scan_at, which nothing bumps.
The playlist phase now:
- resolves the admin at phase time (FindFirstAdmin) instead of trusting the
context snapshot taken at scan start, so a long admin-less scan still
imports playlists in its own phase if an admin was created meanwhile;
- records a persisted PlaylistsImportPending flag when no admin exists yet;
- when that flag is set, imports ALL playlist folders via a new
GetAllWithPlaylists (bypassing the timestamp gate) and clears the flag.
Playlists are recovered by the next scan that runs with an admin, with no
dependency on scan duration and no changes to the auth/server layers.
* fix(scanner): surface datastore errors in playlist import deferral (#5499)
Address review feedback:
- distinguish model.ErrNotFound (no admin yet -> defer) from real datastore
errors when resolving the admin, so DB failures are propagated, not swallowed;
- propagate the error if the pending-import flag can't be persisted, so a scan
doesn't complete as successful without recording the recovery;
- surface read errors when checking the pending flag.
Also name the no-admin condition for readability.
* fix(scanner): simplify admin existence check in playlist import
Signed-off-by: Deluan <deluan@navidrome.org>
* fix(scanner): streamline folder access in playlist import logic
Signed-off-by: Deluan <deluan@navidrome.org>
---------
Signed-off-by: Deluan <deluan@navidrome.org>
* fix(smartplaylists): merge negated artist/tag rules in AND groups
Smart playlists with many negated role/tag conditions ANDed together (e.g.
100+ "isNot artist" rules, issue #5511) generated one correlated NOT EXISTS
subquery per rule, scanning media_file_artists for every candidate row. On
large libraries this took minutes and triggered API timeouts and SQLite lock
contention.
By De Morgan, "NOT EXISTS(role=X) AND NOT EXISTS(role=Y)" is equivalent to
"NOT EXISTS(role=X OR role=Y)", so multiple negated conditions for the same
field can be collapsed into a single batched NOT EXISTS. This mirrors the
existing OR-group merge that #5515 added for positive conditions.
The shared grouping/batching logic is extracted into mergeSameFieldConds,
parameterized by polarity, so the OR/positive and AND/negated paths reuse one
algorithm instead of duplicating it. roleCondGroup/tagCondGroup gain a 'not'
flag to emit the negated subquery.
Benchmark (323k tracks, 120 isNot artist rules, reporter's exact shape):
merged ~54ms vs unmerged ~8.7s steady-state (~160x faster).
* docs: trim redundant comments on merge helpers
The De Morgan explanation was repeated across three doc comments. Keep it in
one place (mergeNegatedJsonConds, where negation is introduced) and reduce the
shared core and group-type comments to concise one-liners.
* fix(artwork): avoid WebP segfault on 32-bit ARM
On 32-bit ARM, the gen2brain/webp native libwebp path uses ebitengine/purego
reverse callbacks, which purego does not support on that architecture. Selecting
it crashes the process with a SIGSEGV when encoding or decoding WebP cover art,
taking down the whole server on the first web UI artwork request (issue #5597).
Force the safe WASM path on armv7/v6 in two layers: build the Docker arm binary
with the gen2brain/webp "nodynamic" tag so purego is never linked, and add a
runtime GOARCH guard in the init hook so source builds on 32-bit ARM are also
protected. arm64 keeps the native libwebp path.
* fix(artwork): also disable native WebP on 32-bit x86
purego's callback implementation is built with the constraint !386 && !arm,
so 32-bit x86 (386) crashes with the same SIGSEGV as 32-bit ARM when the native
libwebp path is used. Navidrome ships linux/386 and windows/386 builds, so guard
386 alongside arm: extend the runtime GOARCH check and the Docker nodynamic build
tag to cover both. 64-bit arches keep the native libwebp path.
* fix(artwork): rely on nodynamic build tag, drop ineffective runtime guard
The previous runtime GOARCH guard did not actually prevent the crash:
gen2brain/webp selects the native (purego) vs WASM backend in its own package
init() and registers the purego write callback at import time, before any
Navidrome hook runs. webp.Dynamic() is only a status getter, and Decode/Encode
branch on the library's unexported flag, so the guard merely skipped a log line
while the native path stayed active.
The effective fix is the nodynamic build tag (applied for 32-bit ARM and x86 in
the Dockerfile), which compiles gen2brain/webp WASM-only so purego is never
linked. Drop the misleading guard and document that source builds on 32-bit
architectures must be built with -tags nodynamic.
* fix(artwork): don't enable WebP encoding by default in Docker
The Docker image set ND_ENABLEWEBPENCODING=true, which (a) forced cover-art
thumbnails through WebP for every install and (b) overrode any
EnableWebPEncoding=false set in the user's navidrome.toml, since env vars take
precedence over the config file in Viper.
On 32-bit platforms the only available WebP backend is the WASM encoder, which
is slow on the underpowered hardware those builds typically run on, so enabling
it by default is the wrong tradeoff there. Remove the env default and leave
EnableWebPEncoding off unless the user opts in. Combined with the nodynamic
build tag, 32-bit images neither crash nor pay the WASM cost out of the box.
A smarter automatic policy (use WebP only when native libwebp is available) can
be revisited separately.
* test(artwork): add failing e2e tests for artist image leaking as album art
Reproduces a v0.62.0 regression (#5451/#5457): the album cover-art
parent-folder fallback can include the artist folder, serving the artist
thumbnail (e.g. Artist/folder.jpg) as album art for any album without
image files in its own folder(s). Covers three scenarios: a plain
Artist/Album layout with no album images, a single-disc album spread
across sibling folders under the artist folder, and a spread album whose
own front.jpg is shadowed by the artist's cover.jpg via CoverArtPriority
order. Also adds an albumByName test helper for multi-album layouts.
The tests are expected to fail until the parent-folder inclusion is
gated by a structural check (skip the common parent when audio from
other albums lives under it).
* fix(artwork): never serve artist folder images as album art
The album cover-art parent-folder fallback (introduced in #5451/#5457)
could include the artist folder as a source of album images, serving the
artist thumbnail (e.g. Artist/folder.jpg) as cover art for any album
without image files in its own folder(s). This affected both plain
Artist/Album layouts and single-disc albums spread across sibling
folders under the artist folder.
Gate the common-parent inclusion with a structural check: the parent
only qualifies as an album root when no audio belonging to other albums
lives in it or anywhere beneath it. An artist folder contains other
albums' tracks, while an album root above disc subfolders contains only
this album's, so the check works for any disc folder naming scheme and
never affects the multi-disc fixes from #5376/#5456. A single-album
artist with no images anywhere remains structurally indistinguishable
from an album root and is a known residual case.
* refactor(artwork): move album-root audio check into folder repository
Replace the raw subtree SQL (LIKE/ESCAPE expression and wildcard
escaping) that lived in core/artwork with an explicit
FolderRepository.HasAudioOutsideFolders method, implemented in the
persistence layer next to the existing folder-subtree query pattern.
This also removes the test mock's brittle dispatch that sniffed the
generated SQL to recognize the query; the fake now overrides the new
method directly.
Extract the whole parent-folder resolution from loadAlbumFoldersPaths
into an albumRootParent helper, flattening four levels of nesting back
into a linear flow. Behavior is unchanged; the unit test for a parent
containing audio moved to the persistence suite, with added coverage
for subtree boundaries, missing folders, and LIKE-wildcard escaping in
folder paths.
* refactor(persistence): use exists helper in HasAudioOutsideFolders
Replace the hand-rolled count(*) query with the repository's canonical
exists helper, as suggested in PR review.
* feat(smartplaylist): support isMissing/isPresent on mbz_* and lyrics fields
Mark the six mbz_* MusicBrainz ID columns and the lyrics column as Nullable
in the criteria field map, then extend missingExpr to handle string columns
where absence is encoded as NULL or empty string (plus '[]' for lyrics). The
Numeric/Boolean path (ReplayGain) is preserved via an explicit type check.
* refactor(model): make MediaFile BPM and BitDepth nullable pointers
Convert BPM and BitDepth fields in model.MediaFile from int to *int so that
'tag absent' is distinguishable from zero. The metadata mapper now uses
NullableFloat for BPM (nil when absent or zero/unparseable) and only sets
BitDepth when the audio property is non-zero (lossy codecs report 0).
All read sites use gg.V() for zero-fallback deref so Subsonic API output and
transcoding behaviour are byte-identical to before. The persistence layer
bridges the existing NOT NULL DB columns by coercing nil to 0 on write and 0
back to nil on read in PostMapArgs/PostScan; a later migration task will drop
those constraints.
Hash upgrade safety is verified by a new MediaFile.Hash describe block: nil
*int hashes identically to the old int(0) default via ZeroNil+IgnoreZeroValue,
so no files will be spuriously re-imported after this change.
Extra files touched beyond the plan's list: core/stream/legacy_client_test.go
(BitDepth in model.MediaFile literals), persistence/mediafile_repository.go
(NOT NULL bridge).
* test(model): pin pre-conversion golden hashes for BPM/BitDepth
* feat(smartplaylist): support isMissing/isPresent on bpm and bitDepth
* feat(db): make bpm and bit_depth columns nullable, backfill 0 to NULL
Drop the NOT NULL constraint on media_file.bpm and bit_depth via a
lossless migration that converts legacy 0-means-absent values to real
NULL. Remove the temporary shim in PostScan/PostMapArgs that was bridging
the old NOT NULL columns to the *int model fields. Add round-trip
persistence tests asserting NULL storage for nil pointers and correct
value round-trip for non-nil pointers.
* test(e2e): verify isMissing/isPresent partition for nullable fields
Add DescribeTable covering bpm, bitdepth, lyrics, and mbz_recording_id:
for each field, isMissing + isPresent song counts must equal the total
library count, proving the nullable-column SQL is exhaustive and correct.
* test(e2e): seed bpm tag so isMissing/isPresent partition is non-trivial
* fix(model): omit bitDepth from JSON when absent instead of emitting null
* feat(smartplaylist): support isMissing/isPresent on more string fields
Enable isMissing/isPresent operators for album, comment, catalognumber,
discsubtitle, albumcomment, sorttitle, sortalbum, sortartist,
sortalbumartist, and explicitstatus by marking them Nullable in fieldMap.
* refactor(smartplaylist): unify missingExpr column logic into one flow
Collapse the numeric/string fork in missingExpr into a single
empties-driven loop (numeric/boolean fields simply have no empties),
and replace the duplicated IsTag/IsRole guard with a three-way switch
that expresses the dispatch model once. No SQL semantics change for
string fields; numeric/boolean fields now emit a single-element Or/And
which squirrel parenthesizes (e.g. `(col IS NULL)` instead of bare
`col IS NULL`) — update the affected test expectations accordingly.
* fix(subsonic): make search3 empty-query pagination fast at large offsets
Empty-query search3 (used by clients like Symfonium to sync the whole
library) degraded linearly with songOffset: the offset optimization in
optimizePagination keeps the original query's LEFT JOINs (annotation,
bookmark, library) inside its rowid NOT IN subquery, making it as slow as
plain OFFSET (~5s per page at offset 900K on a 920K-track library).
Rewrite the empty-query branch of doSearch to use the same two-phase
approach as the FTS search: Phase 1 paginates rowids on the bare main
table, which SQLite satisfies with a covering index at any offset; Phase 2
hydrates only the page's rows with all JOINs. The Phase 2 hydration logic
is extracted into hydrateRowidPage, now shared with ftsSearch.execute.
Also replace the media_file_missing index with a composite covering index
on (missing, library_id), so Phase 1 stays covering for non-admin users,
whose queries include a library_id filter. The composite serves all
missing-only lookups via its prefix.
With a 920K-track / 85K-album test library, search3 empty-query responses
are now flat (~0.1s) at every offset, for both admin and non-admin users
(previously 3-5s at offsets above 600K).
* refactor(persistence): share search Phase 1 contract and dedup junction fan-out
Extract the Phase 1 query assembly that was duplicated between the FTS
search and the empty-query search into executeTwoPhase: both paths now
supply only their strategy-specific FROM/JOINs and ORDER BY, while the
shared contract (missing filter, library access, options.Filters, and
Max/Offset semantics) lives in one place.
Also fix a pagination integrity bug: the artist library filter joins the
library_artist junction table, so an artist present in multiple libraries
produced duplicate rowids in Phase 1, corrupting offset-based pagination
(short pages and repeated artists during full-library syncs). Phase 1 now
applies DISTINCT whenever a junction-based LibraryFilter is set. DISTINCT
is used instead of GROUP BY because bm25() cannot be evaluated in a
grouped query; plain-filter tables (media_file, album) skip the dedup so
their Phase 1 keeps the streaming covering-index plan. This also fixes the
same duplication in the pre-existing FTS search path.
* fix(persistence): pin artist search Phase 1 join order with CROSS JOIN
search3 always filters artists by library (library_artist.library_id IN
...), and with the junction JOIN in the search Phase 1 rowid query SQLite
chose to drive from library_artist, sorting every junction row with a temp
b-tree on each page — a flat ~200ms penalty per request at 405K artists,
even at offset 0 (the previous code avoided this by accident: its GROUP BY
artist.id pinned an artist-driven plan).
Use CROSS JOIN (SQLite's explicit join-order override) in a search-only
variant of the artist library filter, keeping artist as the outer table so
Phase 1 streams rowids in artist.id order from the primary key index and
LIMIT/OFFSET short-circuits. The DISTINCT dedup stays and costs nothing
under the streaming plan. Other artist queries keep the planner's freedom.
With 405K artists, empty-query artist search is now 0.07s at offset 0 and
0.25s at offset 399K end-to-end (was 0.31s/0.34s before this fix, and up
to 1.2s on master at deep offsets). Artist FTS text search is unaffected.
The jwx library always deserializes numeric claims from a parsed token as
float64, so the int and int64 branches in getIntClaim could never succeed
and were dead code. Keep only the float64 path, which is the one actually
exercised by the token round-trip, and update the comment to document the
library behavior.
* fix(smartplaylist): support isMissing/isPresent operators on ReplayGain fields
ReplayGain values are stored in dedicated nullable columns (rg_album_gain,
rg_album_peak, rg_track_gain, rg_track_peak) rather than in the media_file.tags
JSON blob. The isMissing/isPresent operators previously only supported tag and
role fields, causing two failure modes:
1. Using the documented alias names (replaygain_album_gain etc.) from PR #5256:
these got registered as JSON tags from mappings.yaml, so isMissing queried
json_tree(media_file.tags, '$.replaygain_album_gain') which is always empty
-> the playlist matched ALL songs.
2. Using the canonical field names (rgalbumgain etc.): not a tag/role, so SQL
generation returned an error. Because refreshSmartPlaylist deletes old tracks
before regenerating, the abort left the playlist empty.
Fix: add Nullable bool to FieldInfo and mark the four ReplayGain fields. Add
static alias entries (replaygain_album_gain -> rgalbumgain etc.) with
Numeric+Nullable set; because AddTagNames skips names already in the field map,
these static entries take precedence over the mappings.yaml tag registration.
missingExpr now emits IS NULL / IS NOT NULL for nullable column fields instead
of the json_tree lookup.
Fixes#5584
* chore(smartplaylist): address code review feedback
- Simplify the isMissing/isPresent unsupported-field error message,
removing the internal "nullable fields" jargon
- Standardize comments on mappings.yaml (the actual filename)
- Clarify the alias precedence comment in the LookupField test
* fix: load ND_DEFAULTLANGUAGE on app startup
Added in to apply on initial mount, ensuring the locale is set even when the login page is skipped by reverse-proxy authentication. Removed the redundant language-init effect from . Fixes#3605.
* style(ui): format App.jsx with Prettier
Ran Prettier on ui/src/App.jsx to satisfy code style checks after adding default-language useEffect.
* fix(ui): move default language initialization to Admin component
Signed-off-by: Deluan <deluan@navidrome.org>
* fix(ui): streamline locale setting in App component
Signed-off-by: Deluan <deluan@navidrome.org>
---------
Signed-off-by: Deluan <deluan@navidrome.org>
2026-06-09 19:27:15 -04:00
643 changed files with 38280 additions and 7872 deletions
{Name:"Dreaming of Me",MBID:"027b553e-7c74-3ed4-a95e-1d4fea51f174",Artists:[]agents.Artist{{Name:"Depeche Mode",MBID:"8538e728-ca0b-4321-b7e5-cff6565dd4c0"}}},
pluginEditCmd.Flags().StringVar(&editUsers,"users","",`usernames the plugin may access: comma-separated (alice,bob) or a JSON array (["alice","bob"])`)
pluginEditCmd.Flags().BoolVar(&editAllUsers,"all-users",false,"grant the plugin access to all users")
log.Trace(ctx,"Updating PlaybackSession in cache","clientId",clientId,"mediaId",params.MediaId,"state",params.State,"positionMs",params.PositionMs,"playbackRate",params.PlaybackRate,"ttl",ttl)
p.sessionsMu.Lock()
err:=p.playMap.AddWithTTL(clientId,info,ttl)
p.sessionsMu.Unlock()
iferr!=nil{
log.Warn(ctx,"Error updating PlaybackSession in cache","clientId",clientId,"mediaId",params.MediaId,"state",params.State,err)
// A late stop for a previous track must not end the current session nor reach
// playback reporters, or presence-style plugins would clear the active track.
ifgetErr==nil&&info.MediaFile.ID!=params.MediaId{
p.sessionsMu.Unlock()
log.Trace(ctx,"Ignoring out-of-order stopped report for different track","clientId",clientId,"stoppedMediaId",params.MediaId,"currentMediaId",info.MediaFile.ID)
Loaded 100 of 643 files, more files were not shown because too many files have changed in this diff.
Show more
Reference in new issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
dependabot[bot]