mirror of
https://github.com/navidrome/navidrome.git
synced 2026-07-30 16:56:22 -04:00
dependabot/github_actions/dot-github/workflows/actions/setup-go-7
195 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Deluan
|
b27d6f61ae |
fix(db): make album ReplayGain backfill scale to large libraries
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. |
||
|
Deluan Quintão
|
4efd92cf83 |
feat(server): expose album-level ReplayGain in albums (#5816)
* 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. |
||
|
Deluan Quintão
|
53d54baef0 |
fix(jellyfin): stream collection responses to prevent OOM on large libraries (#5783)
* 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.
|
||
|
Deluan Quintão
|
fe6ac2e577 |
feat(jellyfin): experimental Jellyfin Music API support (#5730)
* 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
|
||
|
Deluan Quintão
|
ca27335d06 |
feat(playlists): per-user starred/rating annotations (backend) (#5749)
* 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.
|
||
|
Deluan Quintão
|
cc315dcc8c |
perf(db): keep query planner statistics trustworthy with full ANALYZE (#5740)
* 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 |
||
|
Kendall Garner
|
4998ac2c59 |
feat(server): add scrobble history Native API (#5761)
* initial scrobble api * feat: add scrobble retrieval api * address feedback (1) * fix spelling * be explicit about get * add primary key field, update index, remove rowid references * use unix timestamp for input and output --------- Co-authored-by: Deluan Quintão <deluan@navidrome.org> |
||
|
Deluan Quintão
|
d4387c5502 |
perf(db): index media_file album/artist sort orders (#5706)
* 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>
|
||
|
Deluan Quintão
|
427d4b9bce |
fix(search): artists with atomic non-ASCII names unfindable after FTS5 migration (#5703)
* 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 |
||
|
Deluan Quintão
|
01b7c86f90 |
fix(scanner): stop logging expected lyrics sniff misses as warnings (#5702)
* 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. |
||
|
Deluan Quintão
|
0fab1861a0 |
fix(subsonic): make "recently added" order reproducible and consistent with RecentlyAddedByModTime (#5678)
* 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. |
||
|
Deluan Quintão
|
aa5aa731dc |
refactor(lyrics): single ParseLyrics entry point + all-format plugin lyrics (#5632)
* 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
|
||
|
Deluan
|
32ac53dc9f |
refactor(migrations): propagate context.Context through all DB calls
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> |
||
|
Deluan Quintão
|
6abc2ed517 |
fix(transcoding): preserve source metadata when transcoding downloads (#5628)
* 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 |
||
|
Deluan Quintão
|
da56df3160 |
feat(smartplaylist): extend isMissing/isPresent to bpm, bitDepth and many text fields (#5603)
* 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. |
||
|
Deluan Quintão
|
5ec6e6a8d4 |
fix(opensubsonic): make search3 empty-query pagination fast at large offsets (#5601)
* 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. |
||
|
Deluan
|
bd3192be0b |
fix(server): make DB PRAGMA optimize error non-fatal
Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
Deluan Quintão
|
74185dc6d1 |
fix(smartplaylists): optimize smart playlist performance for role and tag criteria (#5515)
* fix(server): optimize smart playlist role queries for large criteria (#5511) Role-based smart playlist criteria (artist, composer, etc.) now query the indexed media_file_artists join table instead of parsing JSON via json_tree() on every row. Multiple conditions for the same role within an OR group are merged into a single EXISTS subquery (batched at 200 to stay under SQLite's expression tree depth limit). A composite index (media_file_id, role) replaces the now-redundant single-column (media_file_id) index on media_file_artists. Benchmark (40k tracks, 500 patterns, 3 artists/track): - Merged join-table: 15ms (9.3x faster) - Merged json_tree: 30ms (4.6x faster) - Unmerged baseline: 137ms * refactor: simplify role condition SQL generation and benchmark Extract shared roleCondSQL/roleExistsSQL helpers to deduplicate the EXISTS template between roleCond and roleCondGroup. Use slices.Chunk for batching per project convention. Extract runBenchQuery helper to eliminate triplicated benchmark execution loop. * chore: raise roleCondBatchSize to 350 The empirical SQLite limit is 496 conditions per merged EXISTS subquery. Raising from 200 to 350 reduces the number of batches (e.g. 500 patterns now splits into 2 batches instead of 3). * fix(server): apply OR-merge optimization to tag conditions too Generalize mergeRoleConds into mergeJsonConds to also collapse multiple tag conditions for the same tag (e.g. genre) within OR groups. This gives the same ~5x speedup for tag-heavy smart playlists as the role optimization gives for artist-heavy ones. * refactor: benchmark uses real criteria pipeline instead of hand-built SQL The "Current" sub-benchmark now builds criteria.Criteria expressions and runs them through the actual newSmartPlaylistCriteria → Where() → ToSql() pipeline, validating the real production code path. The baseline still uses hand-built SQL representing the old json_tree approach. * fix: stabilize merged group ordering and close rows before error check Sort group keys in mergeJsonConds so the merged additions have deterministic order across runs, improving SQLite statement cache reuse. Move rows.Close() before rows.Err() in benchmark helper. |
||
|
Deluan Quintão
|
8f0b4930ff |
refactor(conf): replace eager dir creation with lazy Dir type (#5495)
* feat(conf): add Dir type with lazy directory creation Introduces the Dir type that wraps a directory path string and defers os.MkdirAll until the first call to Path() or MustPath(), using sync.Once to ensure the creation happens exactly once. Implements fmt.Stringer, encoding.TextMarshaler, and encoding.TextUnmarshaler for config integration. Includes Ginkgo/Gomega tests covering all methods and error paths. * refactor(conf): replace eager dir creation with lazy Dir type Change DataFolder, CacheFolder, Plugins.Folder, and Backup.Path from string to Dir. Remove all os.MkdirAll calls from Load() so directories are created lazily on first Path()/MustPath() call. Artwork folder creation was already handled at point-of-use in image_upload.go. Add SnapshotConfig() to conf package for safe test config save/restore that avoids copying sync.Once inside Dir fields. Fix copy-lock vet warning in nativeapi/config.go by marshalling pointer instead of value. * refactor(conf): migrate tests and db init to lazy Dir type Update all test files to use conf.NewDir() for Dir field assignments. Ensure DataFolder is created lazily when the database is first opened in db.Db(). Remove eager directory creation from conf.Load() tests. * fix(conf): address review findings for Dir type - Use os.ModePerm for DataFolder/CacheFolder (was 0700, should match original behavior). Add NewDirWithPerm for PluginsFolder (0700). - Use Path() instead of MustPath() in db.Prune() to avoid logFatal from background cron job. - Panic on marshal/unmarshal errors in SnapshotConfig (test helper). - Clean up redundant String()/MustPath() calls in plugin manager. - Remove dead code in dir_test.go. Signed-off-by: Deluan <deluan@navidrome.org> * fix(conf): add GoString to Dir for clean config dump output Implement fmt.GoStringer on Dir so pretty.Sprintf shows the path string instead of internal struct fields (sync.Once, perm, err). Also add TODO comment to configtest about removing the indirection. * fix(dir): improve error logging in MustPath method Signed-off-by: Deluan <deluan@navidrome.org> * refactor(tests): remove redundant tests for unwritable DataFolder and CacheFolder Signed-off-by: Deluan <deluan@navidrome.org> * fix(conf): address PR review feedback - Ensure Plugins.Folder always uses 0700, even when user-configured (previously only the derived default got restrictive permissions). - Create LogFile parent directory before opening, so LogFile paths inside a not-yet-created DataFolder work correctly. --------- Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
Deluan Quintão
|
24e526e09a |
fix(transcoding): place -ss before -i for fast input seeking (#5492)
Move the ffmpeg -ss (seek/offset) parameter before -i in all transcoding commands so ffmpeg uses input seeking instead of output seeking. Per the ffmpeg docs, placing -ss before -i seeks at the demuxer level by keyframe (very fast), and since FFmpeg 2.1 it is also frame-accurate when transcoding. The previous placement after -i caused ffmpeg to decode and discard all audio up to the seek point, which was unnecessarily slow — especially problematic for lengthy files (4+ hours). Both code paths are updated: buildDynamicArgs (for default formats) and createFFmpegCommand (for custom templates without %t). A database migration updates existing default commands in the transcoding table. |
||
|
Deluan
|
1f3a7efa75 |
fix(backup): surface real SQLite error when backup step fails
The error-check ordering after backupOp.Step(-1) checked !done before err, which masked the underlying SQLite error (e.g. SQLITE_BUSY, I/O errors) with a generic "backup not done with step -1" message. On failure, Step returns done=false together with a non-nil err, so the !done branch short-circuited before the real error was ever reported. Swap the checks so the SQLite error is returned first, making failing backups actually diagnosable. Refs https://github.com/navidrome/navidrome/issues/5305#issuecomment-4230470593 |
||
|
Deluan Quintão
|
9b0bfc606b |
fix(subsonic): always emit required created field on AlbumID3 (#5340)
* fix(subsonic): always emit required `created` field on AlbumID3 Strict OpenSubsonic clients (e.g. Navic via dev.zt64.subsonic) reject search3/getAlbum/getAlbumList2 responses that omit the `created` field, which the spec marks as required. Navidrome was dropping it whenever the album's CreatedAt was zero. Root cause was threefold: 1. buildAlbumID3/childFromAlbum conditionally emitted `created`, so a zero CreatedAt became a missing JSON key. 2. ToAlbum's `older()` helper treated a zero BirthTime as the minimum, so a single track with missing filesystem birth time could poison the album aggregation. 3. phase_1_folders' CopyAttributes copied `created_at` from the previous album row unconditionally, propagating an already-zero value forward on every metadata-driven album ID change. Since sql_base_repository drops `created_at` on UPDATE, a poisoned row could never self-heal. Fixes: - Always emit `created`, falling back to UpdatedAt/ImportedAt when CreatedAt is zero. Adds albumCreatedAt() helper used by both buildAlbumID3 and childFromAlbum. - Guard `older()` against a zero second argument. - Skip the CopyAttributes call in phase_1_folders when the previous album's created_at is zero, so the freshly-computed value survives. - New migration backfills existing broken rows from media_file.birth_time (falling back to updated_at). Tested against a real DB: repaired 605/6922 affected rows, no side effects on healthy rows. Signed-off-by: Deluan <deluan@navidrome.org> * refactor(subsonic): return albumCreatedAt by value to avoid heap escape Returning *time.Time from albumCreatedAt caused Go escape analysis to move the entire model.Album parameter to the heap, since the returned pointer aliased a field of the value receiver. For hot endpoints like getAlbumList2 and search3, this meant one full-struct heap allocation per album result. Return time.Time by value and let callers wrap it with gg.P() to take the address locally. Only the small time.Time value escapes; the model.Album struct stays on the stack. Also corrects the doc comment to reflect the actual guarantee ("best-effort" rather than "non-zero"), matching the test case that exercises the all-zero fallback. --------- Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
Barend
|
991bd3ed21 |
fix(db): resolve schema inconsistencies in library_artist and scrobble_buffer tables (#5047)
* fix(db): resolve schema inconsistencies in library_artist and scrobble_buffer tables * fix(db): address PR comments around speed of the migration * fix(db): simplify schema inconsistencies migration Remove ineffective PRAGMA foreign_keys and cache_size statements, which are no-ops inside goose's wrapping transaction. Drop the down migration body (Navidrome does not run down migrations) and document the intent. Rename the file to refresh the timestamp after rebase. --------- Co-authored-by: Deluan Quintão <deluan@navidrome.org> |
||
|
Deluan Quintão
|
ba8d427890 |
feat(ui): add cover art support for internet radio stations (#5229)
* feat(artwork): add KindRadioArtwork and EntityRadio constant * feat(model): add UploadedImage field and artwork methods to Radio * feat(model): add Radio to GetEntityByID lookup chain * feat(db): add uploaded_image column to radio table * feat(artwork): add radio artwork reader with uploaded image fallback * feat(api): add radio image upload/delete endpoints * feat(ui): add radio artwork ID prefix to getCoverArtUrl * feat(ui): add cover art display and upload to RadioEdit * feat(ui): add cover art thumbnails to radio list * feat(ui): prefer artwork URL in radio player helper * refactor: remove redundant code in radio artwork - Remove duplicate Avatar rendering in RadioList by reusing CoverArtField - Remove redundant UpdatedAt assignment in radio image handlers (already set by repository Put) * refactor(ui): extract shared useImageLoadingState hook Move image loading/error/lightbox state management into a shared useImageLoadingState hook in common/. Consolidates duplicated logic from AlbumDetails, PlaylistDetails, RadioEdit, and artist detail views. * feat(ui): use radio placeholder icon when no uploaded image Remove album placeholder fallback from radio artwork reader so radios without an uploaded image return ErrUnavailable. On the frontend, show the internet-radio-icon.svg placeholder instead of requesting server artwork when no image is uploaded, allowing favicon fallback in the player. * refactor(ui): update defaultOff fields in useSelectedFields for RadioList Signed-off-by: Deluan <deluan@navidrome.org> * fix: address code review feedback - Add missing alt attribute to CardMedia in RadioEdit for accessibility - Fix UpdateInternetRadio to preserve UploadedImage field by fetching existing radio before updating (prevents Subsonic API from clearing custom artwork) - Add Reader() level tests to verify ErrUnavailable is returned when radio has no uploaded image * refactor: add colsToUpdate to RadioRepository.Put Use the base sqlRepository.put with column filtering instead of hand-rolled SQL. UpdateInternetRadio now specifies only the Subsonic API fields, preventing UploadedImage from being cleared. Image upload/delete handlers specify only UploadedImage. * fix: ensure UpdatedAt is included in colsToUpdate for radio Put --------- Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
Deluan Quintão
|
e7c6e78dd0 |
fix(db): normalize timestamps and fix recently added album sorting (#5176)
* fix(db): normalize timestamps and fix recently added album sorting
SQLite stores timestamps as TEXT and uses string comparison for ORDER BY.
Timestamps in RFC3339 T-format ('2024-01-01T10:00:00Z') sort incorrectly
against space-format ('2024-01-01 10:00:00+00:00') because 'T' (ASCII 84)
> ' ' (ASCII 32), causing albums with T-format timestamps to appear as
newer than they are in the "Recently Added" list.
This adds a migration to normalize all T-format timestamps across all
tables to the space-format expected by go-sqlite3, wraps the
recently_added sort with datetime() to make it format-agnostic, and
replaces the plain album timestamp indexes with expression indexes to
maintain query performance.
* fix(test): improve recently_added sort test robustness
Use same-date timestamps (2024-01-15T08:00:00Z vs 2024-01-15 20:00:00)
so the T-vs-space character difference at position 10 actually triggers
the sorting bug. Initialize index variables to -1 and assert both test
albums are found before comparing positions.
* chore(db): update migration timestamp to 2026-03-16
|
||
|
Deluan Quintão
|
ab8a58157a |
feat: add artist image uploads and image-folder artwork source (#5198)
* feat: add shared ImageUploadService for entity image management * feat: add UploadedImage field and methods to Artist model * feat: add uploaded_image column to artist table * feat: add ArtistImageFolder config option * refactor: wire ImageUploadService and delegate playlist file ops to it Wire ImageUploadService into the DI container and refactor the playlist service to delegate image file operations (SetImage/RemoveImage) to the shared ImageUploadService, removing duplicated file I/O logic. A local ImageUploadService interface is defined in core/playlists to avoid an import cycle between core and core/playlists. * feat: artist artwork reader checks uploaded image first * feat: add image-folder priority source for artist artwork * feat: cache key invalidation for image-folder and uploaded images * refactor: extract shared image upload HTTP helpers * feat: add artist image upload/delete API endpoints * refactor: playlist handlers use shared image upload helpers * feat: add shared ImageUploadOverlay component * feat: add i18n keys for artist image upload * feat: add image upload overlay to artist detail pages * refactor: playlist details uses shared ImageUploadOverlay component * fix: add gosec nolint directive for ParseMultipartForm * refactor: deduplicate image upload code and optimize dir scanning - Remove dead ImageFilename methods from Artist and Playlist models (production code uses core.imageFilename exclusively) - Extract shared uploadedImagePath helper in model/image.go - Extract findImageInArtistFolder to deduplicate dir-scanning logic between fromArtistImageFolder and getArtistImageFolderModTime - Fix fileInputRef in useCallback dependency array * fix: include artist UpdatedAt in artwork cache key Without this, uploading or deleting an artist image would not invalidate the cached artwork because the cache key was only based on album folder timestamps, not the artist's own UpdatedAt field. * feat: add Portuguese translations for artist image upload * refactor: use shared i18n keys for cover art upload messages Move cover art upload/remove translations from per-entity sections (artist, playlist) to a shared top-level "message" section, avoiding duplication across entity types and translation files. * refactor: move cover art i18n keys to shared message section for all languages * refactor: simplify image upload code and eliminate redundancies Extracted duplicate image loading/lightbox state logic from DesktopArtistDetails and MobileArtistDetails into a shared useArtistImageState hook. Moved entity type constants to the consts package and replaced raw string literals throughout model, core, and nativeapi packages. Exported model.UploadedImagePath and reused it in core/image_upload.go to consolidate path construction. Cached the ArtistImageFolder lookup result in artistReader to eliminate a redundant os.ReadDir call on every artwork request. Signed-off-by: Deluan <deluan@navidrome.org> * style: fix prettier formatting in ImageUploadOverlay * fix: address code review feedback on image upload error handling - RemoveImage now returns errors instead of swallowing them - Artist handlers distinguish not-found from other DB errors - Defer multipart temp file cleanup after parsing * fix: enforce hard request size limit with MaxBytesReader for image uploads Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
Deluan Quintão
|
d0fbba14ff |
fix(db): check both name and target_format in default transcodings migration (#5175)
The ensure_default_transcodings migration only checked target_format before inserting, but the transcoding table has UNIQUE constraints on both name and target_format. Older installations may have entries where the name matches a default (e.g., 'opus audio') but the target_format differs (e.g., 'oga' instead of 'opus'), causing a UNIQUE constraint violation on name during the INSERT. Fixes #5174 |
||
|
Deluan Quintão
|
d8bc41fbb1 |
fix: use ADTS for AAC transcoding, temporarily exclude AAC from transcode decisions (#5167)
* fix: use ADTS format for AAC transcoding to avoid silent output on ffmpeg 8.0+ The fragmented MP4 muxer (`-f ipod -movflags frag_keyframe+empty_moov`) produces corrupt/silent audio when ffmpeg pipes to stdout, confirmed on ffmpeg 8.0+. The moof atom offset values are zeroed out in pipe mode, causing AAC decoder errors. Switch to `-f adts` (raw AAC framing) which works reliably via pipe and is widely supported by clients including UPnP/Sonos devices. * fix: exclude AAC from transcode decision, as it is not working for Sonos. Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
Deluan Quintão
|
d7c3a50f86 |
fix: player MaxBitRate cap, format-aware defaults, browser profile filtering (#5165)
* feat(transcode): apply player MaxBitRate cap and use format-aware default bitrates Add player MaxBitRate cap to the transcode decider so server-side player bitrate limits are respected when making OpenSubsonic transcode decisions. The player cap is applied only when it is more restrictive than the client's maxAudioBitrate (or when the client has no limit). Also replace the hardcoded 256 kbps default with a format-aware lookup that checks the DB first (for user-customized values), then built-in defaults, and finally falls back to 256 kbps. For lossless→lossy transcoding, prefer maxTranscodingAudioBitrate over maxAudioBitrate when available. * test(e2e): add tests for player MaxBitRate cap and format-aware default bitrates Add e2e tests covering: - Player MaxBitRate forcing transcode when source exceeds cap - Player MaxBitRate having no effect when source is under cap - Client limit winning when more restrictive than player MaxBitRate - Player MaxBitRate winning when more restrictive than client limit - Player MaxBitRate=0 having no effect - Format-aware defaults: mp3 (192kbps), opus (128kbps) instead of hardcoded 256 - maxAudioBitrate fallback for lossless→lossy when no maxTranscodingAudioBitrate - maxTranscodingAudioBitrate taking priority over maxAudioBitrate - Combined player + client limits flowing correctly through decision→stream * feat(transcode): update transcoding profiles to add flac, filter by supported codecs, and ensure mp3 fallback Signed-off-by: Deluan <deluan@navidrome.org> * fix(db): ensure all default transcodings exist on upgrade Older installations that were seeded before aac/flac were added to DefaultTranscodings may be missing these entries. The previous migration only added flac; this one ensures all default transcodings are present without touching user-customized entries. * test: remove duplication Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
Deluan
|
928741ef25 |
fix(db): recreate probe_data column as NOT NULL with empty string default
The probe_data column was added with DEFAULT NULL in migration 20260307175815, which causes sql.Scan errors when reading into Go string fields. This migration drops and recreates the column with DEFAULT '' NOT NULL to prevent NULL scan errors. |
||
|
Deluan Quintão
|
ae1e0ddb11 |
feat(subsonic): implement OpenSubsonic Transcoding extension (#4990)
* feat(subsonic): implement transcode decision logic and codec handling for media files Signed-off-by: Deluan <deluan@navidrome.org> * fix(subsonic): update codec limitation structure and decision logic for improved clarity Signed-off-by: Deluan <deluan@navidrome.org> * fix(transcoding): update bitrate handling to use kilobits per second (kbps) across transcode decision logic Signed-off-by: Deluan <deluan@navidrome.org> * refactor(transcoding): simplify container alias handling in matchesContainer function Signed-off-by: Deluan <deluan@navidrome.org> * fix(transcoding): enforce POST method for GetTranscodeDecision and handle non-POST requests Signed-off-by: Deluan <deluan@navidrome.org> * feat(transcoding): add enums for protocol, comparison operators, limitations, and codec profiles in transcode decision logic Signed-off-by: Deluan <deluan@navidrome.org> * refactor(transcoding): streamline limitation checks and applyLimitation logic for improved readability and maintainability Signed-off-by: Deluan <deluan@navidrome.org> * refactor(transcoding): replace strings.EqualFold with direct comparison for protocol and limitation checks Signed-off-by: Deluan <deluan@navidrome.org> * refactor(transcoding): rename token methods to CreateTranscodeParams and ParseTranscodeParams for clarity Signed-off-by: Deluan <deluan@navidrome.org> * refactor(transcoding): enhance logging for transcode decision process and client info conversion Signed-off-by: Deluan <deluan@navidrome.org> * refactor(transcoding): rename TranscodeDecision to Decider and update related methods for clarity Signed-off-by: Deluan <deluan@navidrome.org> * refactor(transcoding): enhance transcoding config lookup logic for audio codecs Signed-off-by: Deluan <deluan@navidrome.org> * refactor(transcoding): enhance transcoding options with sample rate support and improve command handling Signed-off-by: Deluan <deluan@navidrome.org> * refactor(transcoding): add bit depth support for audio transcoding and enhance related logic Signed-off-by: Deluan <deluan@navidrome.org> * refactor(transcoding): enhance AAC command handling and support for audio channels in streaming Signed-off-by: Deluan <deluan@navidrome.org> * refactor(transcoding): streamline transcoding logic by consolidating stream parameter handling and enhancing alias mapping Signed-off-by: Deluan <deluan@navidrome.org> * refactor(transcoding): update default command handling and add codec support for transcoding Signed-off-by: Deluan <deluan@navidrome.org> * fix: implement noopDecider for transcoding decision handling in tests Signed-off-by: Deluan <deluan@navidrome.org> * fix: address review findings for OpenSubsonic transcoding PR Fix multiple issues identified during code review of the transcoding extension: add missing return after error in shared stream handler preventing nil pointer panic, replace dead r.Body nil check with MaxBytesReader size limit, distinguish not-found from other DB errors, fix bpsToKbps integer truncation with rounding, add "pcm" to isLosslessFormat for consistency with model.IsLossless(), add sampleRate/bitDepth/channels to streaming log, fix outdated test comment, and add tests for conversion functions and GetTranscodeStream parameter passing. * feat(transcoding): add sourceUpdatedAt to decision and validate transcode parameters Signed-off-by: Deluan <deluan@navidrome.org> * fix: small issues Updated mock AAC transcoding command to use the new default (ipod with fragmented MP4) matching the migration, ensuring tests exercise the same buildDynamicArgs code path as production. Improved archiver test mock to match on the whole StreamRequest struct instead of decomposing fields, making it resilient to future field additions. Added named constants for JWT claim keys in the transcode token and wrapped ParseTranscodeParams errors with ErrTokenInvalid for consistency. Documented the IsLossless BitDepth fallback heuristic as temporary until Codec column is populated. Signed-off-by: Deluan <deluan@navidrome.org> * fix(transcoding): adapt transcode claims to struct-based auth.Claims Updated transcode token handling to use the struct-based auth.Claims introduced on master, replacing the previous map[string]any approach. Extended auth.Claims with transcoding-specific fields (MediaID, DirectPlay, UpdatedAt, Channels, SampleRate, BitDepth) and added float64 fallback in ClaimsFromToken for numeric claims that lose their Go type during JWT string serialization. Also added the missing lyrics parameter to all subsonic.New() calls in test files. * feat(model): add ProbeData field and UpdateProbeData repository method Add probe_data TEXT column to media_file for caching ffprobe results. Add UpdateProbeData to MediaFileRepository interface and implementations. Use hash:"ignore" tag so probe data doesn't affect MediaFile fingerprints. * feat(ffmpeg): add ProbeAudioStream for authoritative audio metadata Add ProbeAudioStream to FFmpeg interface, using ffprobe to extract codec, profile, bitrate, sample rate, bit depth, and channels. Parse bits_per_raw_sample as fallback for FLAC/ALAC bit depth. Normalize "unknown" profile to empty string. All parseProbeOutput tests use real ffprobe JSON from actual files. * feat(transcoding): integrate ffprobe into transcode decisions Add ensureProbed to probe media files on first transcode decision, caching results in probe_data. Build SourceStream from probe data with fallback to tag-based metadata. Refactor decision logic to pass StreamDetails instead of MediaFile, enabling codec profile limitations (e.g., audioProfile) to use probe data. Add normalizeProbeCodec to map ffprobe codec names (dsd_lsbf_planar, pcm_s16le) to internal names (dsd, pcm). NewDecider now accepts ffmpeg.FFmpeg; wire_gen.go regenerated. * feat(transcoding): add DevEnableMediaFileProbe config flag Add DevEnableMediaFileProbe (default true) to allow disabling ffprobe- based media file probing as a safety fallback. When disabled, the decider uses tag-based metadata from the scanner instead. * test(transcode): add ensureProbed unit tests Test probing when ProbeData is empty, skipping when already set, error propagation from ffprobe, and DevEnableMediaFileProbe flag. * refactor(ffmpeg): use command constant and select_streams for ProbeAudioStream Move ffprobe arguments to a probeAudioStreamCmd constant, following the same pattern as extractImageCmd and probeCmd. Add -select_streams a:0 to only probe the first audio stream, avoiding unnecessary parsing of video and artwork streams. Derive the ffprobe binary path safely using filepath.Dir/Base instead of replacing within the full path string. * refactor(transcode): decouple transcode token claims from auth.Claims Remove six transcode-specific fields (MediaID, DirectPlay, UpdatedAt, Channels, SampleRate, BitDepth) from auth.Claims, which is shared with session and share tokens. Transcode tokens are signed parameter-passing tokens, not authentication tokens, so coupling them to auth created misleading dependencies. The transcode package now owns its own JWT claim serialization via Decision.toClaimsMap() and paramsFromToken(), using generic auth.EncodeToken/DecodeAndVerifyToken wrappers that keep TokenAuth encapsulated. Wire format (JWT claim keys) is unchanged, so in-flight tokens remain compatible. Signed-off-by: Deluan <deluan@navidrome.org> * refactor(transcode): simplify code after review Extract getIntClaim helper to eliminate repeated int/int64/float64 JWT claim extraction pattern in paramsFromToken and ClaimsFromToken. Rewrite checkIntLimitation as a one-liner delegating to applyIntLimitation. Return probe result from ensureProbed to avoid redundant JSON round-trip. Extract toResponseStreamDetails helper and mediaTypeSong constant in the API layer, and use transcode.ProtocolHTTP constant instead of hardcoded string. Signed-off-by: Deluan <deluan@navidrome.org> * fix(ffmpeg): enhance bit_rate parsing logic for audio streams Signed-off-by: Deluan <deluan@navidrome.org> * fix(transcode): improve code review findings across transcode implementation - Fix parseProbeData to return nil on JSON unmarshal failure instead of a zero-valued struct, preventing silent degradation of source stream details - Use probe-resolved codec for lossless detection in buildSourceStream instead of the potentially stale scanner data - Remove MediaFile.IsLossless() (dead code) and consolidate lossless detection in isLosslessFormat(), using codec name only — bit depth is not reliable since lossy codecs like ADPCM report non-zero values - Add "wavpack" to lossless codec list (ffprobe codec_name for WavPack) - Guard bpsToKbps against negative input values - Fix misleading comment in buildTemplateArgs about conditional injection - Avoid leaking internal error details in Subsonic API responses - Add missing test for ErrNotFound branch in GetTranscodeDecision - Add TODO for hardcoded protocol in toResponseStreamDetails * refactor(transcode): streamline transcoding command lookup and format resolution Signed-off-by: Deluan <deluan@navidrome.org> * feat(transcode): implement server-side transcoding override for player formats Signed-off-by: Deluan <deluan@navidrome.org> * fix(transcode): honor bit depth and channel constraints in transcoding selection selectTranscodingOptions only checked sample rate when deciding whether same-format transcoding was needed, ignoring requested bit depth and channel reductions. This caused the streamer to return raw audio when the transcode decision requested downmix or bit-depth conversion. * refactor(transcode): unify streaming decision engine via MakeDecision Move transcoding decision-making out of mediaStreamer and into the subsonic Stream/Download handlers, using transcode.Decider.MakeDecision as the single decision engine. This eliminates selectTranscodingOptions and the mismatch between decision and streaming code paths (decision used LookupTranscodeCommand with built-in fallbacks, while streaming used FindByFormat which only checked the DB). - Add DecisionOptions with SkipProbe to MakeDecision so the legacy streaming path never calls ffprobe - Add buildLegacyClientInfo to translate legacy stream params (format, maxBitRate, DefaultDownsamplingFormat) into a synthetic ClientInfo - Add resolveStreamRequest on the subsonic Router to resolve legacy params into a fully specified StreamRequest via MakeDecision - Simplify DoStream to a dumb executor that receives pre-resolved params - Remove selectTranscodingOptions entirely Signed-off-by: Deluan <deluan@navidrome.org> * refactor(transcode): move MediaStreamer into core/transcode and unify StreamRequest Moved MediaStreamer, Stream, TranscodingCache and related types from core/media_streamer.go into core/transcode/, eliminating the duplicate StreamRequest type. The transcode.StreamRequest now carries all fields (ID, Format, BitRate, SampleRate, BitDepth, Channels, Offset) and ResolveStream returns a fully-populated value, removing manual field copying at every call site. Also moved buildLegacyClientInfo into the transcode package alongside ResolveStream, and unexported ParseTranscodeParams since it was only used internally by ValidateTranscodeParams. * refactor(transcode): rename Decider methods and unexport Params type Rename ResolveStream → ResolveRequest and ValidateTranscodeParams → ResolveRequestFromToken for clarity and consistency. The new ResolveRequestFromToken returns a StreamRequest directly (instead of the intermediate Params type), eliminating manual Params→StreamRequest conversion in callers. Unexport Params to params since it is now only used internally for JWT token parsing. * test(transcode): remove redundant tests and use constants Remove tests that duplicate coverage from integration-level tests (toClaimsMap, paramsFromToken round-trips, applyServerOverride direct call, duplicate 410 handler test). Replace raw "http" strings with ProtocolHTTP constant. Consolidate lossy -sample_fmt tests into DescribeTable. * refactor(transcode): split oversized files into focused modules Split transcode.go and transcode_test.go into focused files by concern: - decider.go: decision engine (MakeDecision, direct play/transcode evaluation, probe) - token.go: JWT token encode/decode (params, toClaimsMap, paramsFromToken, CreateTranscodeParams, ResolveRequestFromToken) - legacy_client.go: legacy Subsonic bridge (buildLegacyClientInfo, ResolveRequest) - codec_test.go: isLosslessFormat and normalizeProbeCodec tests - token_test.go: token round-trip and ResolveRequestFromToken tests Moved the Decider interface from types.go to decider.go to keep it near its implementation, and cleaned up types.go to contain only pure type definitions and constants. No public API changes. * refactor(transcode): reorder parameters in applyServerOverride function Signed-off-by: Deluan <deluan@navidrome.org> * test(e2e): add NewTestStream function and implement spyStreamer for testing Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
Deluan Quintão
|
acd69f6a4f |
feat(playlist): support #EXTALBUMARTURL directive and sidecar images (#5131)
* feat(playlist): add migration for playlist image field rename and external URL * refactor(playlist): rename ImageFile to UploadedImage and ArtworkPath to UploadedImagePath Rename playlist model fields and methods for clarity in preparation for adding external image URL and sidecar image support. Add the new ExternalImageURL field to the Playlist model. * feat(playlist): parse #EXTALBUMARTURL directive in M3U imports * feat(playlist): always sync ExternalImageURL on re-scan, preserve UploadedImage * feat(artwork): add sidecar image discovery and cache invalidation for playlists Add playlist sidecar image support to the artwork reader fallback chain. A sidecar image (e.g., MyPlaylist.jpg next to MyPlaylist.m3u) is discovered via case-insensitive base name matching using model.IsImageFile(). Cache invalidation uses max(playlist.UpdatedAt, imageFile.ModTime()) to bust stale artwork when sidecar or ExternalImageURL local files change. * feat(artwork): add external image URL source to playlist artwork reader Add fromPlaylistExternalImage source function that resolves playlist cover art from ExternalImageURL, supporting both HTTP(S) URLs (via the existing fromURL helper) and local file paths (via os.Open). Insert it in the Reader() fallback chain between sidecar and tiled cover. * refactor(artwork): simplify playlist artwork source functions Extract shared fromLocalFile helper, use url.Parse for scheme check, and collapse sidecar directory scan conditions. * test(artwork): remove redundant fromPlaylistSidecar tests These tests duplicated scenarios already covered by findPlaylistSidecarPath tests combined with fromLocalFile (tested via fromPlaylistExternalImage). After refactoring fromPlaylistSidecar to a one-liner composing those two functions, the wrapper tests add no value. * fix(playlist): address security review comments from PR #5131: - Use url.PathUnescape instead of url.QueryUnescape for file:// URLs so that '+' in filenames is preserved (not decoded as space). - Validate all local image paths (file://, absolute, relative) against known library boundaries via libraryMatcher, rejecting paths outside any configured library. - Harden #EXTALBUMARTURL against path traversal and SSRF by adding EnableM3UExternalAlbumArt config flag (default false, also disabled by EnableExternalServices=false) to gate HTTP(S) URL storage at parse time and fetching at read time (defense in depth). - Log a warning when os.ReadDir fails in findPlaylistSidecarPath for diagnosability. - Extract resolveLocalPath helper to simplify resolveImageURL. Signed-off-by: Deluan <deluan@navidrome.org> * feat(playlist): implement human-friendly filename generation for uploaded playlist cover images Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
adrbn
|
d004f99f8f |
feat(playlist): add custom playlist cover art upload (#5110)
* feat(playlist): add custom playlist cover art upload - #406 Allow users to upload, view, and remove custom cover images for playlists. Custom images take priority over the auto-generated tiled artwork. Backend: - Add `image_path` column to playlist table (migration with proper rollback) - Add `SetImage`/`RemoveImage` methods to playlist service - Add `POST/DELETE /api/playlist/{id}/image` endpoints - Prioritize custom image in artwork reader pipeline - Clean up image files on playlist deletion - Use glob-based cleanup to prevent orphaned files across format changes - Reject uploads with undetermined image type (400) Frontend: - Hover overlay on playlist cover with upload (camera) and remove (trash) buttons - Lightbox for full-size cover art viewing - Cover art thumbnails in the playlist list view - Loading/error states and i18n strings Closes #406 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: adrbn <128328324+adrbn@users.noreply.github.com> * refactor: rename playlist image path migration file Signed-off-by: Deluan <deluan@navidrome.org> * fix(playlist): address review feedback for cover art upload - #406 - Use httpClient instead of raw fetch for image upload/remove - Revert glob cleanup to simple imagePath check - Add log.Error before all error HTTP responses - Add backend tests for SetImage and RemoveImage Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: adrbn <128328324+adrbn@users.noreply.github.com> * refactor(playlist): use Playlist.ArtworkPath() for image storage Migrate all playlist image path handling to use the new Playlist.ArtworkPath() method as the single source of truth. The DB now stores only the filename (e.g. "pls-1.jpg") instead of a relative path, and images are stored under {DataFolder}/artwork/playlist/ instead of {DataFolder}/playlist_images/. The artwork root directory is created at startup alongside DataFolder and CacheFolder. This also removes the conf dependency from reader_playlist.go since path resolution is now fully encapsulated in the model. Signed-off-by: Deluan <deluan@navidrome.org> * refactor(playlist): streamline artwork image selection logic Signed-off-by: Deluan <deluan@navidrome.org> * refactor: move translation keys, add pt-BR translations Signed-off-by: Deluan <deluan@navidrome.org> * refactor(playlist): rename image_path to image_file Rename the playlist cover art column and field from image_path/ImagePath to image_file/ImageFile across the migration, model, service, tests, and UI. The new name more accurately describes what the field stores (a filename, not a path) and aligns with the existing ImageFiles/IsImageFile naming conventions in the codebase. --------- Signed-off-by: adrbn <128328324+adrbn@users.noreply.github.com> Signed-off-by: Deluan <deluan@navidrome.org> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Deluan Quintão <deluan@navidrome.org> |
||
|
Deluan Quintão
|
d9a215e1e3 |
feat(plugins): allow mounting library directories as read-write (#5122)
* feat(plugins): mount library directories as read-only by default Add an AllowWriteAccess boolean to the plugin model, defaulting to false. When off, library directories are mounted with the extism "ro:" prefix (read-only). Admins can explicitly grant write access via a new toggle in the Library Permission card. * test: add tests to buildAllowedPaths Signed-off-by: Deluan <deluan@navidrome.org> * chore: improve allowed paths logging for library access Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
Deluan Quintão
|
54de0dbc52 |
feat(server): implement FTS5-based full-text search (#5079)
* build: add sqlite_fts5 build tag to enable FTS5 support
* feat: add SearchBackend config option (default: fts)
* feat: add buildFTS5Query for safe FTS5 query preprocessing
* feat: add FTS5 search backend with config toggle, refactor legacy search
- Add searchExprFunc type and getSearchExpr() for backend selection
- Rename fullTextExpr to legacySearchExpr
- Add ftsSearchExpr using FTS5 MATCH subquery
- Update fullTextFilter in sql_restful.go to use configured backend
* feat: add FTS5 migration with virtual tables, triggers, and search_participants
Creates FTS5 virtual tables for media_file, album, and artist with
unicode61 tokenizer and diacritic folding. Adds search_participants
column, populates from JSON, and sets up INSERT/UPDATE/DELETE triggers.
* feat: populate search_participants in PostMapArgs for FTS5 indexing
* test: add FTS5 search integration tests
* fix: exclude FTS5 virtual tables from e2e DB restore
The restoreDB function iterates all tables in sqlite_master and
runs DELETE + INSERT to reset state. FTS5 contentless virtual tables
cannot be directly deleted from. Since triggers handle FTS5 sync
automatically, simply skip tables matching *_fts and *_fts_* patterns.
* build: add compile-time guard for sqlite_fts5 build tag
Same pattern as netgo: compilation fails with a clear error if
the sqlite_fts5 build tag is missing.
* build: add sqlite_fts5 tag to reflex dev server config
* build: extract GO_BUILD_TAGS variable in Makefile to avoid duplication
* fix: strip leading * from FTS5 queries to prevent "unknown special query" error
* feat: auto-append prefix wildcard to FTS5 search tokens for broader matching
Every plain search token now gets a trailing * appended (e.g., "love" becomes
"love*"), so searching for "love" also matches "lovelace", "lovely", etc.
Quoted phrases are preserved as exact matches without wildcards. Results are
ordered alphabetically by name/title, so shorter exact matches naturally
appear first.
* fix: clarify comments about FTS5 operator neutralization
The comments said "strip" but the code lowercases operators to
neutralize them (FTS5 operators are case-sensitive). Updated comments
to accurately describe the behavior.
* fix: use fmt.Sprintf for FTS5 phrase placeholders
The previous encoding used rune('0'+index) which silently breaks with
10+ quoted phrases. Use fmt.Sprintf for arbitrary index support.
* fix: validate and normalize SearchBackend config option
Normalize the value to lowercase and fall back to "fts" with a log
warning for unrecognized values. This prevents silent misconfiguration
from typos like "FTS", "Legacy", or "fts5".
* refactor: improve documentation for build tags and FTS5 requirements
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: convert FTS5 query and search backend normalization tests to DescribeTable format
Signed-off-by: Deluan <deluan@navidrome.org>
* fix: add sqlite_fts5 build tag to golangci configuration
Signed-off-by: Deluan <deluan@navidrome.org>
* feat: add UISearchDebounceMs configuration option and update related components
Signed-off-by: Deluan <deluan@navidrome.org>
* fix: fall back to legacy search when SearchFullString is enabled
FTS5 is token-based and cannot match substrings within words, so
getSearchExpr now returns legacySearchExpr when SearchFullString
is true, regardless of SearchBackend setting.
* fix: add sqlite_fts5 build tag to CI pipeline and Dockerfile
* fix: add WHEN clauses to FTS5 AFTER UPDATE triggers
Added WHEN clauses to the media_file_fts_au, album_fts_au, and
artist_fts_au triggers so they only fire when FTS-indexed columns
actually change. Previously, every row update (e.g., play count, rating,
starred status) triggered an unnecessary delete+insert cycle in the FTS
shadow tables. The WHEN clauses use IS NOT for NULL-safe comparison of
each indexed column, avoiding FTS index churn for non-indexed updates.
* feat: add SearchBackend configuration option to data and insights components
Signed-off-by: Deluan <deluan@navidrome.org>
* fix: enhance input sanitization for FTS5 by stripping additional punctuation and special characters
Signed-off-by: Deluan <deluan@navidrome.org>
* feat: add search_normalized column for punctuated name search (R.E.M., AC/DC)
Add index-time normalization and query-time single-letter collapsing to
fix FTS5 search for punctuated names. A new search_normalized column
stores concatenated forms of punctuated words (e.g., "R.E.M." → "REM",
"AC/DC" → "ACDC") and is indexed in FTS5 tables. At query time, runs of
consecutive single letters (from dot-stripping) are collapsed into OR
expressions like ("R E M" OR REM*) to match both the original tokens and
the normalized form. This enables searching by "R.E.M.", "REM", "AC/DC",
"ACDC", "A-ha", or "Aha" and finding the correct results.
* refactor: simplify isSingleUnicodeLetter to avoid []rune allocation
Use utf8.DecodeRuneInString to check for a single Unicode letter
instead of converting the entire string to a []rune slice.
* feat: define ftsSearchColumns for flexible FTS5 search column inclusion
Signed-off-by: Deluan <deluan@navidrome.org>
* feat: update collapseSingleLetterRuns to return quoted phrases for abbreviations
Signed-off-by: Deluan <deluan@navidrome.org>
* feat: implement extractPunctuatedWords to handle artist/album names with embedded punctuation
Signed-off-by: Deluan <deluan@navidrome.org>
* feat: implement extractPunctuatedWords to handle artist/album names with embedded punctuation
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: punctuated word handling to improve processing of artist/album names
Signed-off-by: Deluan <deluan@navidrome.org>
* feat: add CJK support for search queries with LIKE filters
Signed-off-by: Deluan <deluan@navidrome.org>
* feat: enhance FTS5 search by adding album version support and CJK handling
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: search configuration to use structured options
Signed-off-by: Deluan <deluan@navidrome.org>
* feat: enhance search functionality to support punctuation-only queries and update related tests
Signed-off-by: Deluan <deluan@navidrome.org>
---------
Signed-off-by: Deluan <deluan@navidrome.org>
|
||
|
Maximilian
|
a704e86ac1 | refactor: run Go modernize (#5002) | ||
|
Terry Raimondo
|
03120bac32 |
feat(subsonic): Add avgRating from subsonic spec (#4900)
* feat(subsonic): add averageRating to API responses Add averageRating attribute to Subsonic API responses for artists, albums, and songs. The average is calculated across all user ratings. * perf(db): add index for average rating queries Add composite index on (item_id, item_type, rating) to optimize the correlated subquery used for calculating average ratings. Signed-off-by: Terry Raimondo <terry.raimondo@gmail.com> * test: add tests for averageRating feature Add tests for: - Album.AverageRating calculation in persistence layer - MediaFile.AverageRating calculation in persistence layer - AverageRating mapping in subsonic response helpers Signed-off-by: Terry Raimondo <terry.raimondo@gmail.com> * test: improve averageRating rounding test with 3 users Add third test user to fixtures and update rounding test to use 3 ratings (5 + 4 + 4) / 3 = 4.33 for proper decimal rounding coverage. Signed-off-by: Terry Raimondo <terry.raimondo@gmail.com> * perf: store avg_rating on entity tables instead of using subquery - Add avg_rating column to album, media_file, and artist tables - Update SetRating() to recalculate and store average when ratings change - Read avg_rating directly from entity table in withAnnotation() - Remove old annotation index migration (no longer needed) This trades write-time computation for read-time performance by pre-computing the average rating instead of using a correlated subquery on every read. * feat: add Subsonic.EnableAverageRating config option (default true) Allow administrators to disable exposing averageRating in Subsonic API responses if they don't want to expose other users' rating data. The avg_rating column is still updated internally when users rate items, but the value is only included in API responses when this option is enabled. * address PR comments - Use structs:"avg_rating" with db:"avg_rating" tag instead of SQL alias - Remove avg_rating indexes (not needed) - Populate avg_rating columns from existing ratings in migration * Woops * rename avg_rating column to average_rating --------- Signed-off-by: Terry Raimondo <terry.raimondo@gmail.com> |
||
|
Deluan Quintão
|
03a45753e9 |
feat(plugins): New Plugin System with multi-language PDK support (#4833)
* chore(plugins): remove the old plugins system implementation Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins): implement new plugin system with using Extism Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins): add capability detection for plugins based on exported functions Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins): add auto-reload functionality for plugins with file watcher support Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins): add auto-reload functionality for plugins with file watcher support Signed-off-by: Deluan <deluan@navidrome.org> * refactor(plugins): standardize variable names and remove superfluous wrapper functions Signed-off-by: Deluan <deluan@navidrome.org> * fix(plugins): improve error handling and logging in plugin manager Signed-off-by: Deluan <deluan@navidrome.org> * refactor(plugins): implement plugin function call helper and refactor MetadataAgent methods Signed-off-by: Deluan <deluan@navidrome.org> * fix(plugins): race condition in plugin manager * tests(plugins): change BeforeEach to BeforeAll in MetadataAgent tests Signed-off-by: Deluan <deluan@navidrome.org> * tests(plugins): optimize tests Signed-off-by: Deluan <deluan@navidrome.org> * tests(plugins): more optimizations Signed-off-by: Deluan <deluan@navidrome.org> * test(plugins): ignore goroutine leaks from notify library in tests Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins): add Wikimedia plugin for Navidrome to fetch artist metadata Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins): enhance plugin logging and set User-Agent header Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins): implement scrobbler plugin with authorization and scrobbling capabilities Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins): integrate logs Signed-off-by: Deluan <deluan@navidrome.org> * refactor(plugins): clean up manifest struct and improve plugin loading logic Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins): add metadata agent and scrobbler schemas for bootstrapping plugins Signed-off-by: Deluan <deluan@navidrome.org> * feat(hostgen): add hostgen tool for generating Extism host function wrappers - Implemented hostgen tool to generate wrappers from annotated Go interfaces. - Added command-line flags for input/output directories and package name. - Introduced parsing and code generation logic for host services. - Created test data for various service interfaces and expected generated code. - Added documentation for host services and annotations for code generation. - Implemented SubsonicAPI service with corresponding generated code. * feat(subsonicapi): update Call method to return JSON string response Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins): implement SubsonicAPI host function integration with permissions Signed-off-by: Deluan <deluan@navidrome.org> * fix(generator): error-only methods in response handling Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins): generate client wrappers for host functions Signed-off-by: Deluan <deluan@navidrome.org> * refactor(generator): remove error handling for response.Error in client templates Signed-off-by: Deluan <deluan@navidrome.org> * feat(scheduler): add Scheduler service interface with host function wrappers for scheduling tasks * feat(plugins): add WASI build constraints to client wrapper templates, to avoid lint errors Signed-off-by: Deluan <deluan@navidrome.org> * feat(scheduler): implement Scheduler service with one-time and recurring scheduling capabilities Signed-off-by: Deluan <deluan@navidrome.org> * refactor(manifest): remove unused ConfigPermission from permissions schema Signed-off-by: Deluan <deluan@navidrome.org> * feat(scheduler): add scheduler callback schema and implementation for plugins Signed-off-by: Deluan <deluan@navidrome.org> * refactor(scheduler): streamline scheduling logic and remove unused callback tracking Signed-off-by: Deluan <deluan@navidrome.org> * refactor(scheduler): add Close method for resource cleanup on plugin unload Signed-off-by: Deluan <deluan@navidrome.org> * docs(scheduler): clarify SchedulerCallback requirement for scheduling functions Signed-off-by: Deluan <deluan@navidrome.org> * fix: update wasm build rule to include all Go files in the directory Signed-off-by: Deluan <deluan@navidrome.org> * feat: rewrite the wikimedia plugin using the XTP CLI Signed-off-by: Deluan <deluan@navidrome.org> * refactor(scheduler): replace uuid with id.NewRandom for schedule ID generation Signed-off-by: Deluan <deluan@navidrome.org> * refactor: capabilities registration Signed-off-by: Deluan <deluan@navidrome.org> * test: add scheduler service isolation test for plugin instances Signed-off-by: Deluan <deluan@navidrome.org> * refactor: update plugin manager initialization and encapsulate logic Signed-off-by: Deluan <deluan@navidrome.org> * feat: add WebSocket service definitions for plugin communication Signed-off-by: Deluan <deluan@navidrome.org> * feat: implement WebSocket service for plugin integration and connection management Signed-off-by: Deluan <deluan@navidrome.org> * feat: add Crypto Ticker example plugin for real-time cryptocurrency price updates via Coinbase WebSocket API Also add the lifecycle capability Signed-off-by: Deluan <deluan@navidrome.org> * fix: use context.Background() in invokeCallback for scheduled tasks Signed-off-by: Deluan <deluan@navidrome.org> * refactor: rename plugin.create() to plugin.instance() Signed-off-by: Deluan <deluan@navidrome.org> * refactor: rename pluginInstance to plugin for consistency across the codebase Signed-off-by: Deluan <deluan@navidrome.org> * refactor: simplify schedule cloning in Close method and enhance plugin cleanup error handling Signed-off-by: Deluan <deluan@navidrome.org> * feat: implement Artwork service for generating artwork URLs in Navidrome plugins - WIP Signed-off-by: Deluan <deluan@navidrome.org> * refactor: moved public URL builders to avoid import cycles Signed-off-by: Deluan <deluan@navidrome.org> * feat: add Cache service for in-memory TTL-based caching in plugins Signed-off-by: Deluan <deluan@navidrome.org> * feat: add Discord Rich Presence example plugin for Navidrome integration Signed-off-by: Deluan <deluan@navidrome.org> * refactor: host function wrappers to use structured request and response types - Updated the host function signatures in `nd_host_artwork.go`, `nd_host_scheduler.go`, `nd_host_subsonicapi.go`, and `nd_host_websocket.go` to accept a single parameter for JSON requests. - Introduced structured request and response types for various cache operations in `nd_host_cache.go`. - Modified cache functions to marshal requests to JSON and unmarshal responses, improving error handling and code clarity. - Removed redundant memory allocation for string parameters in favor of JSON marshaling. - Enhanced error handling in WebSocket and cache operations to return structured error responses. * refactor: error handling in various plugins to convert response.Error to Go errors - Updated error handling in `nd_host_scheduler.go`, `nd_host_websocket.go`, `nd_host_artwork.go`, `nd_host_cache.go`, and `nd_host_subsonicapi.go` to convert string errors from responses into Go errors. - Removed redundant error checks in test data plugins for cleaner code. - Ensured consistent error handling across all plugins to improve reliability and maintainability. * refactor: rename fake plugins to test plugins for clarity in integration tests Signed-off-by: Deluan <deluan@navidrome.org> * feat: add help target to Makefile for plugin usage instructions Signed-off-by: Deluan <deluan@navidrome.org> * feat: add Cover Art Archive plugin as an example of Python plugin Signed-off-by: Deluan <deluan@navidrome.org> * feat: update Makefile and README to clarify Go plugin usage Signed-off-by: Deluan <deluan@navidrome.org> * feat: include plugin capabilities in loading log message Signed-off-by: Deluan <deluan@navidrome.org> * feat: add trace logging for plugin availability and error handling in agents Signed-off-by: Deluan <deluan@navidrome.org> * feat: add Now Playing Logger plugin to showcase calling host functions from Python plugins Signed-off-by: Deluan <deluan@navidrome.org> * feat: generate Python client wrappers for various host services Signed-off-by: Deluan <deluan@navidrome.org> * feat: add generated host function wrappers for Scheduler and SubsonicAPI services Signed-off-by: Deluan <deluan@navidrome.org> * feat: update Python plugin documentation and usage instructions for host function wrappers Signed-off-by: Deluan <deluan@navidrome.org> * feat: add Webhook Scrobbler plugin in Rust to send HTTP notifications on scrobble events Signed-off-by: Deluan <deluan@navidrome.org> * feat: enable parallel loading of plugins during startup Signed-off-by: Deluan <deluan@navidrome.org> * docs: update README to include WebSocket callback schema in plugin documentation Signed-off-by: Deluan <deluan@navidrome.org> * feat: extend plugin watcher with improved logging and debounce duration adjustment Signed-off-by: Deluan <deluan@navidrome.org> * add trace message for plugin recompiles Signed-off-by: Deluan <deluan@navidrome.org> * feat: implement plugin cache purging functionality Signed-off-by: Deluan <deluan@navidrome.org> * test: move purgeCacheBySize unit tests Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins UI): add plugin repository and database support Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins UI): add plugin management routes and middleware Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins UI): implement plugin synchronization with database for add, update, and remove actions Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins UI): add PluginList and PluginShow components with plugin management functionality Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins): optimize plugin change detection Signed-off-by: Deluan <deluan@navidrome.org> * refactor(plugins UI): improve PluginList structure Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins UI): enhance PluginShow with author, website, and permissions display Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins UI): refactor to use MUI and RA components Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins UI): add error handling for plugin enable/disable actions Signed-off-by: Deluan <deluan@navidrome.org> * refactor(plugins): inject PluginManager into native API Signed-off-by: Deluan <deluan@navidrome.org> * refactor(plugins): update GetManager to accept DataStore parameter Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins): add subsonicRouter to Manager and refactor host service registration Signed-off-by: Deluan <deluan@navidrome.org> * refactor(plugins): enhance debug logging for plugin actions and recompile logic Signed-off-by: Deluan <deluan@navidrome.org> * refactor(plugins): break manager.go into smaller, focused files Signed-off-by: Deluan <deluan@navidrome.org> * refactor(plugins): streamline error handling and improve plugin retrieval logic Signed-off-by: Deluan <deluan@navidrome.org> * refactor(plugins): update newWebSocketService to use WebSocketPermission for allowed hosts Signed-off-by: Deluan <deluan@navidrome.org> * refactor(plugins): introduce ToggleEnabledSwitch for managing plugin enable/disable state Signed-off-by: Deluan <deluan@navidrome.org> * docs: update READMEs Signed-off-by: Deluan <deluan@navidrome.org> * feat(library): add Library service for metadata access and filesystem integration Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins): add Library Inspector plugin for periodic library inspection and file size logging Signed-off-by: Deluan <deluan@navidrome.org> * docs: update README to reflect JSON configuration format for plugins Signed-off-by: Deluan <deluan@navidrome.org> * fix(build): update target to wasm32-wasip1 for improved WASI support Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins): implement configuration management UI with key-value pairs support Signed-off-by: Deluan <deluan@navidrome.org> * feat(ui): adjust grid layout in InfoRow component for improved responsiveness Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins): rename ErrorIndicator to EnabledOrErrorField and enhance error handling logic Signed-off-by: Deluan <deluan@navidrome.org> * feat(i18n): add Portuguese translations for plugin management and notifications Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins): add support for .ndp plugin packages and update build process Signed-off-by: Deluan <deluan@navidrome.org> * docs: update README for .ndp plugin packaging and installation instructions Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins): implement KVStore service for persistent key-value storage Signed-off-by: Deluan <deluan@navidrome.org> * docs: enhance README with Extism plugin development resources and recommendations Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins): integrate event broker into plugin manager Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins): update config handling in PluginShow to track last record state Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins): add Rust host function library and example implementation of Discord Rich Presence plugin in Rust Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins): generate Rust lib.rs file to expose host function wrappers Signed-off-by: Deluan <deluan@navidrome.org> * refactor(plugins): update JSON field names to camelCase for consistency Signed-off-by: Deluan <deluan@navidrome.org> * refactor: reduce cyclomatic complexity by refactoring main function Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins): enhance Rust code generation with typed struct support and improved type handling Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins): add Go client library with host function wrappers and documentation Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins): generate Go client stubs for non-WASM platforms Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins): update client template file names for consistency Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins): add initial implementation of the Navidrome Plugin Development Kit code generator - Pahse 1 Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins): implementation of the Navidrome Plugin Development Kit with generated client wrappers and service interfaces - Phase 2 Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins): implementation of the Navidrome Plugin Development Kit with generated client wrappers and service interfaces - Phase 2 (2) Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins): implementation of the Navidrome Plugin Development Kit with generated client wrappers and service interfaces - Phase 3 Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins): implementation of the Navidrome Plugin Development Kit with generated client wrappers and service interfaces - Phase 4 Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins): implementation of the Navidrome Plugin Development Kit with generated client wrappers and service interfaces - Phase 5 Signed-off-by: Deluan <deluan@navidrome.org> * refactor(plugins): consistent naming/types across PDK Signed-off-by: Deluan <deluan@navidrome.org> * refactor(plugins): streamline plugin function signatures and error handling Signed-off-by: Deluan <deluan@navidrome.org> * refactor(plugins): update scrobbler interface to return errors directly instead of response structs Signed-off-by: Deluan <deluan@navidrome.org> * test: make all test plugins use the PDK Signed-off-by: Deluan <deluan@navidrome.org> * refactor(plugins): reorganize and sort type definitions for consistency Signed-off-by: Deluan <deluan@navidrome.org> * refactor(plugins): update error handling for methods to return errors directly Signed-off-by: Deluan <deluan@navidrome.org> * refactor(plugins): update function signatures to return values directly instead of response structs Signed-off-by: Deluan <deluan@navidrome.org> * refactor(plugins): update request/response types to use private naming conventions Signed-off-by: Deluan <deluan@navidrome.org> * build: mark .wasm files as intermediate for cleanup after building .ndp Signed-off-by: Deluan <deluan@navidrome.org> * refactor(plugins): consolidate PDK module path and update Go version to 1.25 Signed-off-by: Deluan <deluan@navidrome.org> * feat: implement Rust PDK Signed-off-by: Deluan <deluan@navidrome.org> * refactor(plugins): reorganize Rust output structure to follow standard conventions Signed-off-by: Deluan <deluan@navidrome.org> * refactor(plugins): update Discord Rich Presence and Library Inspector plugins to use nd-pdk for service calls and implement lifecycle management Signed-off-by: Deluan <deluan@navidrome.org> * refactor(plugins): update macro names for websocket and metadata registration to improve clarity and consistency Signed-off-by: Deluan <deluan@navidrome.org> * refactor(plugins): rename scheduler callback methods for consistency and clarity Signed-off-by: Deluan <deluan@navidrome.org> * refactor(plugins): update export wrappers to use `//go:wasmexport` for WebAssembly compatibility Signed-off-by: Deluan <deluan@navidrome.org> * docs: update plugin registration docs Signed-off-by: Deluan <deluan@navidrome.org> * fix(plugins): generate host wrappers Signed-off-by: Deluan <deluan@navidrome.org> * test(plugins): conditionally run goleak checks based on CI environment Signed-off-by: Deluan <deluan@navidrome.org> * docs: update README to reflect changes in plugin import paths Signed-off-by: Deluan <deluan@navidrome.org> * refactor(plugins): update plugin instance creation to accept context for cancellation support Signed-off-by: Deluan <deluan@navidrome.org> * fix(plugins): update return types in metadata interfaces to use pointers Signed-off-by: Deluan <deluan@navidrome.org> * fix(plugins): enhance type handling for Rust and XTP output in capability generation Signed-off-by: Deluan <deluan@navidrome.org> * fix(plugins): update IsAuthorized method to return boolean instead of response object Signed-off-by: Deluan <deluan@navidrome.org> * test(plugins): add unit tests for rustOutputType and isPrimitiveRustType functions Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins): implement XTP JSONSchema validation for generated schemas Signed-off-by: Deluan <deluan@navidrome.org> * fix(plugins): update response types in testMetadataAgent methods to use pointers Signed-off-by: Deluan <deluan@navidrome.org> * docs: update Go and Rust plugin developer sections for clarity Signed-off-by: Deluan <deluan@navidrome.org> * docs: correct example link for library inspector in README Signed-off-by: Deluan <deluan@navidrome.org> * docs: clarify artwork URL generation capabilities in service descriptions Signed-off-by: Deluan <deluan@navidrome.org> * docs: update README to include Rust PDK crate information for plugin developers Signed-off-by: Deluan <deluan@navidrome.org> * fix: handle URL parsing errors and use atomic upsert in plugin repository Added proper error handling for url.Parse calls in PublicURL and AbsoluteURL functions. When parsing fails, PublicURL now falls back to AbsoluteURL, and AbsoluteURL logs the error and returns an empty string, preventing malformed URLs from being generated. Replaced the non-atomic UPDATE-then-INSERT pattern in plugin repository Put method with a single atomic INSERT ... ON CONFLICT statement. This eliminates potential race conditions and improves consistency with the upsert pattern already used in host_kvstore.go. * feat: implement mock service instances for non-WASM builds using testify/mock Signed-off-by: Deluan <deluan@navidrome.org> * refactor: Discord RPC struct to encapsulate WebSocket logic Signed-off-by: Deluan <deluan@navidrome.org> * feat: add support for experimental WebAssembly threads Signed-off-by: Deluan <deluan@navidrome.org> * feat: add PDK abstraction layer with mock support for non-WASM builds Signed-off-by: Deluan <deluan@navidrome.org> * feat: add unit tests for Discord plugin and RPC functionality Signed-off-by: Deluan <deluan@navidrome.org> * fix: update return types in minimalPlugin and wikimediaPlugin methods to use pointers Signed-off-by: Deluan <deluan@navidrome.org> * fix: context cancellation and implement WebSocket callback timeout for improved error handling Signed-off-by: Deluan <deluan@navidrome.org> * feat: conditionally include error handling in generated client code templates Signed-off-by: Deluan <deluan@navidrome.org> * feat: implement ConfigService for plugin configuration management Signed-off-by: Deluan <deluan@navidrome.org> * feat: enhance plugin manager to support metrics recording Signed-off-by: Deluan <deluan@navidrome.org> * refactor: make MockPDK private Signed-off-by: Deluan <deluan@navidrome.org> * refactor: update interface types to use 'any' in plugin repository methods Signed-off-by: Deluan <deluan@navidrome.org> * refactor: rename List method to Keys for clarity in configuration management Signed-off-by: Deluan <deluan@navidrome.org> * test: add ndpgen plugin tests in the pipeline and update Makefile Signed-off-by: Deluan <deluan@navidrome.org> * feat: add users permission management to plugin system Signed-off-by: Deluan <deluan@navidrome.org> * refactor: streamline users integration tests and enhance plugin user management Signed-off-by: Deluan <deluan@navidrome.org> * refactor: remove UserID from scrobbler request structure Signed-off-by: Deluan <deluan@navidrome.org> * test: add integration tests for UsersService enable gate behavior Signed-off-by: Deluan <deluan@navidrome.org> * feat: implement user permissions for SubsonicAPI and scrobbler plugins Signed-off-by: Deluan <deluan@navidrome.org> * fix: show proper error in the UI when enabling a plugin fails Signed-off-by: Deluan <deluan@navidrome.org> * feat: add library permission management to plugin system Signed-off-by: Deluan <deluan@navidrome.org> * feat: add user permission for processing scrobbles in Discord Rich Presence plugin Signed-off-by: Deluan <deluan@navidrome.org> * fix: implement dynamic loading for buffered scrobbler plugins Signed-off-by: Deluan <deluan@navidrome.org> * feat: add GetAdmins method to retrieve admin users from the plugin Signed-off-by: Deluan <deluan@navidrome.org> * feat: update Portuguese translations for user and library permissions Signed-off-by: Deluan <deluan@navidrome.org> * reorder migrations Signed-off-by: Deluan <deluan@navidrome.org> * fix: remove unnecessary bulkActionButtons prop from PluginList component * feat: add manual plugin rescan functionality and corresponding UI action Signed-off-by: Deluan <deluan@navidrome.org> * feat: implement user/library and plugin management integration with cleanup on deletion Signed-off-by: Deluan <deluan@navidrome.org> * feat: replace core mock services with test-specific implementations to avoid import cycles * feat: add ID fields to Artist and Song structs and enhance track loading logic by prioritizing ID matches Signed-off-by: Deluan <deluan@navidrome.org> * feat: update plugin permissions from allowedHosts to requiredHosts for better clarity and consistency * feat: refactor plugin host permissions to use RequiredHosts directly for improved clarity * fix: don't record metrics for plugin calls that aren't implemented at all Signed-off-by: Deluan <deluan@navidrome.org> * fix: enhance connection management with improved error handling and cleanup logic Signed-off-by: Deluan <deluan@navidrome.org> * feat: introduce ArtistRef struct for better artist representation and update track metadata handling Signed-off-by: Deluan <deluan@navidrome.org> * feat: update user configuration handling to use user key prefix for improved clarity Signed-off-by: Deluan <deluan@navidrome.org> * feat: enhance ConfigCard input fields with multiline support and vertical resizing Signed-off-by: Deluan <deluan@navidrome.org> * fix: rust plugin compilation error Signed-off-by: Deluan <deluan@navidrome.org> * feat: implement IsOptionPattern method for better return type handling in Rust PDK generation Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
Deluan Quintão
|
5c3568f758 |
fix(ui): make playlist name sorting case-insensitive (#4845)
* fix: make playlist name sorting case-insensitive Add collation NOCASE to playlist.name column to ensure case-insensitive sorting, matching the behavior of other tables like radio and user. This fixes the issue where uppercase playlist names would appear before lowercase names regardless of alphabetical order. The migration recreates the playlist table with the proper collation and recreates all associated indexes. Corresponding collation tests are added to verify the fix persists through future migrations. * fix: add default sorting to playlist names Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
Deluan Quintão
|
a521c74a59 |
feat(server): track scrobble/linstens history (#4770)
* feat(scrobble): implement scrobble repository and record scrobble history Signed-off-by: Deluan <deluan@navidrome.org> * feat(scrobble): add configuration option to enable scrobble history Signed-off-by: Deluan <deluan@navidrome.org> * test(scrobble): enhance scrobble history tests for repository recording Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
Deluan
|
ca83ebbb53 |
feat: add DevOptimizeDB flag to control SQLite optimization
Added a new DevOptimizeDB configuration flag (default true) that controls whether SQLite PRAGMA OPTIMIZE and ANALYZE commands are executed. This allows disabling database optimization operations for debugging or testing purposes. The flag guards optimization commands in: - db/db.go: Initial connection, post-migration, and shutdown optimization - persistence/library_repository.go: Post-scan optimization - db/migrations/migration.go: ANALYZE during forced full rescans Set ND_DEVOPTIMIZEDB=false to disable all database optimization commands. |
||
|
zacaj
|
3294bcacfc |
feat: add Rated At field - #4653 (#4660)
* feat(model): add Rated At field - #4653 Signed-off-by: zacaj <zacaj@zacaj.com> * fix(ui): ignore empty dates in rating/love tooltips - #4653 * refactor(ui): add isDateSet util function Signed-off-by: zacaj <zacaj@zacaj.com> * feat: add tests for isDateSet and rated_at sort mappings Added comprehensive tests for isDateSet and urlValidate functions in ui/src/utils/validations.test.js covering falsy values, Go zero date handling, valid date strings, Date objects, and edge cases. Added rated_at sort mapping to album, artist, and mediafile repositories, following the same pattern as starred_at (sorting by rating first, then by timestamp). This enables proper sorting by rating date in the UI. --------- Signed-off-by: zacaj <zacaj@zacaj.com> Co-authored-by: zacaj <zacaj@zacaj.com> Co-authored-by: Deluan <deluan@navidrome.org> |
||
|
Kendall Garner
|
a128b3cf98 | fix(db): make playqueue position field an integer (#4481) | ||
|
Deluan Quintão
|
00c83af170 |
feat: Multi-library support (#4181)
* feat(database): add user_library table and library access methods Signed-off-by: Deluan <deluan@navidrome.org> # Conflicts: # tests/mock_library_repo.go * feat(database): enhance user retrieval with library associations Signed-off-by: Deluan <deluan@navidrome.org> * feat(api): implement library management and user-library association endpoints Signed-off-by: Deluan <deluan@navidrome.org> * feat(api): restrict access to library and config endpoints to admin users Signed-off-by: Deluan <deluan@navidrome.org> * refactor(library): implement library management service and update API routes Signed-off-by: Deluan <deluan@navidrome.org> * feat(database): add library filtering to album, folder, and media file queries Signed-off-by: Deluan <deluan@navidrome.org> * refactor library service to use REST repository pattern and remove CRUD operations Signed-off-by: Deluan <deluan@navidrome.org> * add total_duration column to library and update user_library table Signed-off-by: Deluan <deluan@navidrome.org> * fix migration file name Signed-off-by: Deluan <deluan@navidrome.org> * feat(library): add library management features including create, edit, delete, and list functionalities - WIP Signed-off-by: Deluan <deluan@navidrome.org> * feat(library): enhance library validation and management with path checks and normalization - WIP Signed-off-by: Deluan <deluan@navidrome.org> * feat(library): improve library path validation and error handling - WIP Signed-off-by: Deluan <deluan@navidrome.org> * use utils/formatBytes Signed-off-by: Deluan <deluan@navidrome.org> * simplify DeleteLibraryButton.jsx Signed-off-by: Deluan <deluan@navidrome.org> * feat(library): enhance validation messages and error handling for library paths Signed-off-by: Deluan <deluan@navidrome.org> * lint Signed-off-by: Deluan <deluan@navidrome.org> * test(scanner): add tests for multi-library scanning and validation Signed-off-by: Deluan <deluan@navidrome.org> * test(scanner): improve handling of filesystem errors and ensure warnings are returned Signed-off-by: Deluan <deluan@navidrome.org> * feat(controller): add function to retrieve the most recent scan time across all libraries Signed-off-by: Deluan <deluan@navidrome.org> * feat(library): add additional fields and restructure LibraryEdit component for enhanced statistics display Signed-off-by: Deluan <deluan@navidrome.org> * feat(library): enhance LibraryCreate and LibraryEdit components with additional props and styling Signed-off-by: Deluan <deluan@navidrome.org> * feat(mediafile): add LibraryName field and update queries to include library name Signed-off-by: Deluan <deluan@navidrome.org> * feat(missingfiles): add library filter and display in MissingFilesList component Signed-off-by: Deluan <deluan@navidrome.org> * feat(library): implement scanner interface for triggering library scans on create/update Signed-off-by: Deluan <deluan@navidrome.org> # Conflicts: # cmd/wire_gen.go # cmd/wire_injectors.go # Conflicts: # cmd/wire_gen.go # Conflicts: # cmd/wire_gen.go # cmd/wire_injectors.go * feat(library): trigger scan after successful library deletion to clean up orphaned data Signed-off-by: Deluan <deluan@navidrome.org> * rename migration file for user library table to maintain versioning order Signed-off-by: Deluan <deluan@navidrome.org> * refactor: move scan triggering logic into a helper method for clarity Signed-off-by: Deluan <deluan@navidrome.org> * feat(library): add library path and name fields to album and mediafile models Signed-off-by: Deluan <deluan@navidrome.org> * feat(library): add/remove watchers on demand, not only when server starts Signed-off-by: Deluan <deluan@navidrome.org> * refactor(scanner): streamline library handling by using state-libraries for consistency Signed-off-by: Deluan <deluan@navidrome.org> * fix: track processed libraries by updating state with scan timestamps Signed-off-by: Deluan <deluan@navidrome.org> * prepend libraryID for track and album PIDs Signed-off-by: Deluan <deluan@navidrome.org> * feat(repository): apply library filtering in CountAll methods for albums, folders, and media files Signed-off-by: Deluan <deluan@navidrome.org> * feat(user): add library selection for user creation and editing Signed-off-by: Deluan <deluan@navidrome.org> * feat(library): implement library selection functionality with reducer and UI component Signed-off-by: Deluan <deluan@navidrome.org> # Conflicts: # .github/copilot-instructions.md # Conflicts: # .gitignore * feat(library): add tests for LibrarySelector and library selection hooks Signed-off-by: Deluan <deluan@navidrome.org> * test: add unit tests for file utility functions Signed-off-by: Deluan <deluan@navidrome.org> * feat(library): add library ID filtering for album resources Signed-off-by: Deluan <deluan@navidrome.org> * feat(library): streamline library ID filtering in repositories and update resource filtering logic Signed-off-by: Deluan <deluan@navidrome.org> * fix(repository): add table name handling in filter functions for SQL queries Signed-off-by: Deluan <deluan@navidrome.org> * feat(library): add refresh functionality on LibrarySelector close Signed-off-by: Deluan <deluan@navidrome.org> * feat(artist): add library ID filtering for artists in repository and update resource filtering logic Signed-off-by: Deluan <deluan@navidrome.org> # Conflicts: # persistence/artist_repository.go * Add library_id field support for smart playlists - Add library_id field to smart playlist criteria system - Supports Is and IsNot operators for filtering by library ID - Includes comprehensive test coverage for single values and lists - Enables creation of library-specific smart playlists * feat(subsonic): implement user-specific library access in GetMusicFolders Signed-off-by: Deluan <deluan@navidrome.org> * feat(library): enhance LibrarySelectionField to extract library IDs from record Signed-off-by: Deluan <deluan@navidrome.org> * feat(subsonic): update GetIndexes and GetArtists method to support library ID filtering Signed-off-by: Deluan <deluan@navidrome.org> * fix: ensure LibrarySelector dropdown refreshes on button close Added refresh() call when closing the dropdown via button click to maintain consistency with the ClickAwayListener behavior. This ensures the UI updates properly regardless of how the dropdown is closed, fixing an inconsistent refresh behavior between different closing methods. The fix tracks the previous open state and calls refresh() only when the dropdown was open and is being closed by the button click. * refactor: simplify getUserAccessibleLibraries function and update related tests Signed-off-by: Deluan <deluan@navidrome.org> * feat: enhance selectedMusicFolderIds function to handle valid music folder IDs and improve fallback logic Signed-off-by: Deluan <deluan@navidrome.org> * refactor: change ArtistRepository.GetIndex to accept multiple library IDs Updated the GetIndex method signature to accept a slice of library IDs instead of a single ID, enabling support for filtering artists across multiple libraries simultaneously. Changes include: - Modified ArtistRepository interface in model/artist.go - Updated implementation in persistence/artist_repository.go with improved library filtering logic - Refactored Subsonic API browsing.go to use new selectedMusicFolderIds helper - Added comprehensive test coverage for multiple library scenarios - Updated mock repository implementation for testing This change improves flexibility for multi-library operations while maintaining backward compatibility through the selectedMusicFolderIds helper function. * feat: add library access validation to selectedMusicFolderIds Enhanced the selectedMusicFolderIds function to validate musicFolderId parameters against the user's accessible libraries. Invalid library IDs (those the user doesn't have access to) are now silently filtered out, improving security by preventing users from accessing libraries they don't have permission for. Changes include: - Added validation logic to check musicFolderId parameters against user's accessible libraries - Added slices package import for efficient validation - Enhanced function documentation to clarify validation behavior - Added comprehensive test cases covering validation scenarios - Maintains backward compatibility with existing behavior * feat: implement multi-library support for GetAlbumList and GetAlbumList2 endpoints - Enhanced selectedMusicFolderIds helper to validate and filter library IDs - Added ApplyLibraryFilter function in filter/filters.go for library filtering - Updated getAlbumList to support musicFolderId parameter filtering - Added comprehensive tests for multi-library functionality - Supports single and multiple musicFolderId values - Falls back to all accessible libraries when no musicFolderId provided - Validates library access permissions for user security * feat: implement multi-library support for GetRandomSongs, GetSongsByGenre, GetStarred, and GetStarred2 - Added multi-library filtering to GetRandomSongs endpoint using musicFolderId parameter - Added multi-library filtering to GetSongsByGenre endpoint using musicFolderId parameter - Enhanced GetStarred and GetStarred2 to filter artists, albums, and songs by library - Added Options field to MockMediaFileRepo and MockArtistRepo for test compatibility - Added comprehensive Ginkgo/Gomega tests for all new multi-library functionality - All tests verify proper SQL filter generation and library access validation - Supports single/multiple musicFolderId values with fallback to all accessible libraries * refactor: optimize starred items queries with parallel execution and fix test isolation Refactored starred items functionality by extracting common logic into getStarredItems() method that executes artist, album, and media file queries in parallel for better performance. This eliminates code duplication between GetStarred and GetStarred2 methods while improving response times through concurrent database queries using run.Parallel(). Also fixed test isolation issues by adding missing auth.Init(ds) call in album lists test setup. This resolves nil pointer dereference errors in GetStarred and GetStarred2 tests when run independently. * fix: add ApplyArtistLibraryFilter to filter artists by associated music folders Signed-off-by: Deluan <deluan@navidrome.org> * feat: add library access methods to User model Signed-off-by: Deluan <deluan@navidrome.org> * feat: implement library access filtering for artist queries based on user permissions Signed-off-by: Deluan <deluan@navidrome.org> * feat: enhance artist library filtering based on user permissions and optimize library ID retrieval Signed-off-by: Deluan <deluan@navidrome.org> * fix: return error when any musicFolderId is invalid or inaccessible Changed behavior from silently filtering invalid library IDs to returning ErrorDataNotFound (code 70) when any provided musicFolderId parameter is invalid or the user doesn't have access to it. The error message includes the specific library number for better debugging. This affects album/song list endpoints (getAlbumList, getRandomSongs, getSongsByGenre, getStarred) to provide consistent error handling across all Subsonic API endpoints. Updated corresponding tests to expect errors instead of silent filtering. * feat: add musicFolderId parameter support to Search2 and Search3 endpoints Implemented musicFolderId parameter support for Subsonic API Search2 and Search3 endpoints, completing multi-library functionality across all Subsonic endpoints. Key changes: - Added musicFolderId parameter handling to Search2 and Search3 endpoints - Updated search logic to filter results by specified library or all accessible libraries when parameter not provided - Added proper error handling for invalid/inaccessible musicFolderId values - Refactored SearchableRepository interface to support library filtering with variadic QueryOptions - Updated repository implementations (Album, Artist, MediaFile) to handle library filtering in search operations - Added comprehensive test coverage with robust assertions verifying library filtering works correctly - Enhanced mock repositories to capture QueryOptions for test validation Signed-off-by: Deluan <deluan@navidrome.org> * feat: refresh LibraryList on scan end Signed-off-by: Deluan <deluan@navidrome.org> * fix: allow editing name of main library Signed-off-by: Deluan <deluan@navidrome.org> * refactor: implement SendBroadcastMessage method for event broadcasting Signed-off-by: Deluan <deluan@navidrome.org> * feat: add event broadcasting for library creation, update, and deletion Signed-off-by: Deluan <deluan@navidrome.org> * feat: add useRefreshOnEvents hook for custom refresh logic on event changes Signed-off-by: Deluan <deluan@navidrome.org> * feat: enhance library management with refresh event broadcasting Signed-off-by: Deluan <deluan@navidrome.org> * feat: replace AddUserLibrary and RemoveUserLibrary with SetUserLibraries for better library management Signed-off-by: Deluan <deluan@navidrome.org> * chore: remove commented-out genre repository code from persistence tests * feat: enhance library selection with master checkbox functionality Added a master checkbox to the SelectLibraryInput component, allowing users to select or deselect all libraries at once. This improves user experience by simplifying the selection process when multiple libraries are available. Additionally, updated translations in the en.json file to include a new message for selecting all libraries, ensuring consistency in user interface messaging. Signed-off-by: Deluan <deluan@navidrome.org> * feat: add default library assignment for new users Introduced a new column `default_new_users` in the library table to facilitate automatic assignment of default libraries to new regular users. When a new user is created, they will now be assigned to libraries marked as default, enhancing user experience by ensuring they have immediate access to essential resources. Additionally, updated the user repository logic to handle this new functionality and modified the user creation validation to reflect that library selection is optional for non-admin users. Signed-off-by: Deluan <deluan@navidrome.org> * fix: correct updated_at assignment in library repository Signed-off-by: Deluan <deluan@navidrome.org> * fix: improve cache buffering logic Refactored the cache buffering logic to ensure thread safety when checking the buffer length Signed-off-by: Deluan <deluan@navidrome.org> * fix formating Signed-off-by: Deluan <deluan@navidrome.org> * feat: implement per-library artist statistics with automatic aggregation Implemented comprehensive multi-library support for artist statistics that automatically aggregates stats from user-accessible libraries. This fundamental change moves artist statistics from global scope to per-library granularity while maintaining backward compatibility and transparent operation. Key changes include: - Migrated artist statistics from global artist.stats to per-library library_artist.stats - Added automatic library filtering and aggregation in existing Get/GetAll methods - Updated role-based filtering to work with per-library statistics storage - Enhanced statistics calculation to process and store stats per library - Implemented user permission-aware aggregation that respects library access control - Added comprehensive test coverage for library filtering and restricted user access - Created helper functions to ensure proper library associations in tests This enables users to see statistics that accurately reflect only the content from libraries they have access to, providing proper multi-tenant behavior while maintaining the existing API surface and UI functionality. Signed-off-by: Deluan <deluan@navidrome.org> * feat: add multi-library support with per-library tag statistics - WIP Signed-off-by: Deluan <deluan@navidrome.org> * refactor: genre and tag repositories. add comprehensive tests Signed-off-by: Deluan <deluan@navidrome.org> * feat: add multi-library support to tag repository system Implemented comprehensive library filtering for tag repositories to support the multi-library feature. This change ensures that users only see tags from libraries they have access to, while admin users can see all tags. Key changes: - Enhanced TagRepository.Add() method to accept libraryID parameter for proper library association - Updated baseTagRepository to implement library-aware queries with proper joins - Added library_tag table integration for per-library tag statistics - Implemented user permission-based filtering through user_library associations - Added comprehensive test coverage for library filtering scenarios - Updated UI data provider to include tag filtering by selected libraries - Modified scanner to pass library ID when adding tags during folder processing The implementation maintains backward compatibility while providing proper isolation between libraries for tag-based operations like genres and other metadata tags. * refactor: simplify artist repository library filtering Removed conditional admin logic from applyLibraryFilterToArtistQuery method and unified the library filtering approach to match the tag repository pattern. The method now always uses the same SQL join structure regardless of user role, with admin access handled automatically through user_library associations. Added artistLibraryIdFilter function to properly qualify library_id column references and prevent SQL ambiguity errors when multiple tables contain library_id columns. This ensures the filter targets library_artist.library_id specifically rather than causing ambiguous column name conflicts. * fix: resolve LibrarySelectionField validation error for non-admin users Fixed validation error 'At least one library must be selected for non-admin users' that appeared even when libraries were selected. The issue was caused by a data format mismatch between backend and frontend. The backend sends user data with libraries as an array of objects, but the LibrarySelectionField component expects libraryIds as an array of IDs. Added data transformation in the data provider's getOne method to automatically convert libraries array to libraryIds format when fetching user records. Also extracted validation logic into a separate userValidation module for better code organization and added comprehensive test coverage to prevent similar issues. * refactor: remove unused library access functions and related tests Signed-off-by: Deluan <deluan@navidrome.org> * refactor: rename search_test.go to searching_test.go for consistency Signed-off-by: Deluan <deluan@navidrome.org> * fix: add user context to scrobble buffer getParticipants call Added user context handling to scrobbleBufferRepository.Next method to resolve SQL error 'no such column: library_artist.library_id' when processing scrobble entries in multi-library environments. The artist repository now requires user context for proper library filtering, so we fetch the user and temporarily inject it into the context before calling getParticipants. This ensures background scrobbling operations work correctly with multi-library support. * feat: add cross-library move detection for scanner Implemented cross-library move detection for the scanner phase 2 to properly handle files moved between libraries. This prevents users from losing play counts, ratings, and other metadata when moving files across library boundaries. Changes include: - Added MediaFileRepository methods for two-tier matching: FindRecentFilesByMBZTrackID (primary) and FindRecentFilesByProperties (fallback) - Extended scanner phase 2 pipeline with processCrossLibraryMoves stage that processes files unmatched within their library - Implemented findCrossLibraryMatch with MusicBrainz Release Track ID priority and intrinsic properties fallback - Updated producer logic to handle missing tracks without matches, ensuring cross-library processing - Updated tests to reflect new producer behavior and cross-library functionality The implementation uses existing moveMatched function for unified move operations, automatically preserving all user data through database foreign key relationships. Cross-library moves are detected using the same Equals() and IsEquivalent() matching logic as within-library moves for consistency. Signed-off-by: Deluan <deluan@navidrome.org> * feat: add album annotation reassignment for cross-library moves Implemented album annotation reassignment functionality for the scanner's missing tracks phase. When tracks move between libraries and change album IDs, the system now properly reassigns album annotations (starred status, ratings) from the old album to the new album. This prevents loss of user annotations when tracks are moved across library boundaries. The implementation includes: - Thread-safe annotation reassignment using mutex protection - Duplicate reassignment prevention through processed album tracking - Graceful error handling that doesn't fail the entire move operation - Comprehensive test coverage for various scenarios including error conditions This enhancement ensures data integrity and user experience continuity during cross-library media file movements. * fix: address PR review comments for multi-library support Fixed several issues identified in PR review: - Removed unnecessary artist stats initialization check since the map is already initialized in PostScan() - Improved code clarity in user repository by extracting isNewUser variable to avoid checking count == 0 twice - Fixed library selection logic to properly handle initial library state and prevent overriding user selections These changes address code quality and logic issues identified during the multi-library support PR review. * feat: add automatic playlist statistics refreshing Implemented automatic playlist statistics (duration, size, song count) refreshing when tracks are modified. Added new refreshStats() method to recalculate statistics from playlist tracks, and SetTracks() method to update tracks and refresh statistics atomically. Modified all track manipulation methods (RemoveTracks, AddTracks, AddMediaFiles) to automatically refresh statistics. Updated playlist repository to use the new SetTracks method for consistent statistics handling. * refactor: rename AddTracks to AddMediaFilesByID for clarity Renamed the AddTracks method to AddMediaFilesByID throughout the codebase to better reflect its purpose of adding media files to a playlist by their IDs. This change improves code readability and makes the method name more descriptive of its actual functionality. Updated all references in playlist model, tests, core playlist logic, and Subsonic API handlers to use the new method name. * refactor: consolidate user context access in persistence layer Removed duplicate helper functions userId() and isAdmin() from sql_base_repository.go and consolidated all user context access to use loggedUser(r.ctx).ID and loggedUser(r.ctx).IsAdmin consistently across the persistence layer. This change eliminates code duplication and provides a single, consistent pattern for accessing user context information in repository methods. All functionality remains unchanged - this is purely a code cleanup refactoring. * refactor: eliminate MockLibraryService duplication using embedded struct - Replace 235-line MockLibraryService with 40-line embedded struct pattern - Enhance MockLibraryRepo with service-layer methods (192→310 lines) - Maintain full compatibility with existing tests - All 72 nativeapi specs pass with proper error handling Signed-off-by: Deluan <deluan@navidrome.org> * refactor: cleanup Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
Deluan Quintão
|
bfa5b29913 |
feat: MBID search functionality for albums, artists and songs (#4286)
* feat(subsonic): search by MBID functionality Updated the search methods in the mediaFileRepository, albumRepository, and artistRepository to support searching by MBID in addition to the existing query methods. This change improves the efficiency of media file, album, and artist searches, allowing for faster retrieval of records based on MBID. Signed-off-by: Deluan <deluan@navidrome.org> * feat(subsonic): enhance MBID search functionality for albums and artists Updated the search functionality to support searching by MBID for both albums and artists. The fullTextFilter function was modified to accept additional MBID fields, allowing for more comprehensive searches. New tests were added to ensure that the search functionality correctly handles MBID queries, including cases for missing entries and the includeMissing parameter. This enhancement improves the overall search capabilities of the application, making it easier for users to find specific media items by their unique identifiers. Signed-off-by: Deluan <deluan@navidrome.org> * fix(subsonic): normalize MBID to lowercase for consistent querying Updated the MBID handling in the SQL search logic to convert the input to lowercase before executing the query. This change ensures that searches are case-insensitive, improving the accuracy and reliability of the search results when querying by MBID. Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
Kendall Garner
|
2741b1a5c5 |
feat(server): expose main credit stat to reflect only album artist | artist credit (#4268)
* attempt using artist | albumartist * add primary stats, expose to ND and Subsonic * response to feedback (1) * address feedback part 1 * fix docs and artist show * fix migration order --------- Co-authored-by: Deluan Quintão <deluan@navidrome.org> |
||
|
Deluan Quintão
|
d4f8419d83 |
fix(db): clear dangling music from BFR upgrade (#4262)
* fix(db): remove dangling items from BFR upgrade. Signed-off-by: Deluan <deluan@navidrome.org> * chore: .gitignore any navidrome binary Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
Deluan
|
cfa1d7fa81 |
fix(scanner): filter folders by num_audio_files to ensure accurate statistics
Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
Kendall Garner
|
7640c474cf |
fix: Allow nullable ReplayGain and support 0.0 (#4239)
* fix(ui,scanner,subsonic): Allow nullable replaygain and support 0.0 Resolves #4236. Makes the replaygain columns (track/album gain/peak) nullable. Converts the type to a pointer, allowing for 0.0 (a valid value) to be returned from Subsonic. Updates tests for this behavior. * small refactor Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org> Co-authored-by: Deluan <deluan@navidrome.org> |
||
|
Deluan
|
65029968ab |
refactor: rename chain package to run and update references
Signed-off-by: Deluan <deluan@navidrome.org> |