* 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>
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.
The `held` channel in `runTwoRequests` was unbuffered, creating a race
condition with the `select/default` send in the handler. Under CI load
(slow runner, -race, -shuffle=on), the handler goroutine could reach
the select before the test goroutine blocked on `<-held`, causing the
send to silently fall through to `default` and deadlocking both
goroutines permanently.
Buffer the channel (capacity 1) so the send always succeeds regardless
of goroutine scheduling order.
* fix(transcoding): don't apply server-side transcoding override on getTranscodeDecision
The getTranscodeDecision endpoint was incorrectly applying server-side
player transcoding overrides (forced format and MaxBitRate cap), which
replaced the client's declared capabilities with synthetic profiles.
This caused the endpoint to ignore what the client can actually play and
return decisions for formats the client never requested (e.g. AAC when
the client only supports FLAC/opus/mp3). The override is now gated
behind an ApplyServerOverride flag in TranscodeOptions, which is only
set by the legacy stream endpoint where this behavior is expected.
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: move server-side transcoding override to ResolveRequest
Moved the server-side player transcoding override logic (forced format
and MaxBitRate cap) from MakeDecision into ResolveRequest, where the
legacy stream context is handled. This makes MakeDecision a pure
function that only operates on the ClientInfo it receives, removing the
ApplyServerOverride flag and all context-sniffing from the decision
engine. Tests moved accordingly to legacy_client_test.go.
* test(e2e): update transcode decision tests for server override removal
Updated e2e tests to reflect that getTranscodeDecision no longer applies
server-side player overrides (MaxBitRate cap and forced transcoding
profile). The player MaxBitRate tests now verify the endpoint ignores
the player cap and relies solely on client-declared capabilities.
* test(e2e): assert opus default bitrate when player cap is ignored
Added bitrate assertion to verify the player MaxBitRate cap is truly
ignored: the target bitrate should be the opus format default (128kbps),
not the player cap (320kbps).
---------
Signed-off-by: Deluan <deluan@navidrome.org>
* fix(server): prevent artwork throttle token starvation on slow clients
Replace Chi's ThrottleBacklog middleware for artwork endpoints with a
custom RequestThrottle that releases processing tokens before writing
the HTTP response. Previously, a slow or stalled client could hold a
throttle token indefinitely during io.Copy, exhausting all 2-4 slots
and blocking artwork requests for all users (reported after 15+ days
uptime).
The new approach buffers artwork into memory while holding the token,
releases it immediately, then writes the buffered response. A 30-second
per-request write deadline (SetWriteTimeout) prevents stalled writes
from blocking indefinitely. Throttle exhaustion is now logged with
context for operator visibility.
* refactor(server): simplify throttle to middleware with same API as Chi
Restructure RequestThrottle from a DI-injected type into a drop-in
middleware function with the same signature as Chi's ThrottleBacklog.
Handlers are reverted to their original simple form (no throttle
awareness), and the middleware is applied at route definition time
just like before. This eliminates the DI dependency, removes the
artworkThrottle field from both Router structs, and consolidates
SetWriteTimeout into the throttle file. When limit <= 0, the
middleware returns a passthrough so callers don't need a guard.
Signed-off-by: Deluan <deluan@navidrome.org>
* feat(server): add opt-out flag for buffered artwork throttle
Add DevArtworkThrottleBuffered config (default true) that controls
whether the new buffered ThrottleBacklog middleware is used. When set
to false, it falls back to Chi's original middleware, giving users a
safety valve in case the buffered implementation causes issues.
Signed-off-by: Deluan <deluan@navidrome.org>
* test(server): clean up throttle tests for clarity and speed
Consolidate duplicate router setup into runTwoRequests() and
slowClientTest() helpers. Replace time.Sleep-based token holding with
channel synchronization, reducing suite time from ~7s to ~1.5s.
Remove redundant test, fix duplicate comment block, and add comment
explaining why slowTestWriter can't embed httptest.ResponseRecorder.
* fix: release artwork throttle tokens on panic
Defer the buffered artwork throttle release inside the handler closure so tokens are returned even when a downstream handler panics before response flushing. Document that the middleware buffers full responses in memory and add a regression test covering recovery after a panic.
* fix: align buffered throttle response behavior
Keep only the first status code written to the buffered artwork throttle response writer so it matches net/http semantics. Strengthen the opt-out test to verify DevArtworkThrottleBuffered=false uses Chi's original slow-client behavior instead of only checking shared 429 handling.
* refactor(server): remove setWriteTimeout from throttle middleware
SetWriteDeadline only constrains the server's Write syscall, not how
fast the client reads from the TCP buffer. For artwork-sized responses
(up to ~500KB), the kernel accepts the entire write immediately even
over real network interfaces due to TCP buffer auto-tuning. Verified
by testing with a stalled client over both loopback and en0 — the
deadline never triggers. The actual protection comes from buffering +
early token release, which is already in place.
---------
Signed-off-by: Deluan <deluan@navidrome.org>
Log skipped entries, total entries vs plugins found, and DB state
to help diagnose why sync may not find new plugins.
Signed-off-by: Deluan <deluan@navidrome.org>
* feat(ui): add Rescan button to plugin list empty state
When no plugins are installed and the folder watcher fails to detect new
plugins, users had no way to trigger a rescan. Extract RescanButton into
a shared component and render it in a custom empty state for the plugin
list.
* refactor(ui): address review feedback for plugin empty state
- Pass label translation key directly to RA Button (auto-translates)
- Use within() from Testing Library instead of querySelector for
scoped queries with better error messages
* fix(artwork): include top-level album folders in parent cover art lookup
The Path != "." guard added in #5451 was too aggressive — it excluded
any folder with Path=".", which includes top-level album folders (not
just the library root). Changed to ParentID != "" which correctly
excludes only the actual library root folder.
Fixes#5456
* fix: correct comment in test — album is under library root, not artist root
* test: add ascii tree diagram to top-level album e2e test
* test: replace internal bug references with issue link in e2e comments
Signed-off-by: Deluan <deluan@navidrome.org>
* test: add e2e test matching reporter's exact library layout (#5456)
Adds a deeply nested test (Genre/Artist/Album/Disc) with 12 discs
using the reporter's actual folder names to verify artwork resolution
works for non-top-level album folders too.
* fix(scanner): use a syntectic admin user when no admin user is found
Signed-off-by: Deluan <deluan@navidrome.org>
* fix(scanner): bump album UpdatedAt on Phase 3 refresh to invalidate artwork cache
When Phase 3 corrects an album's FolderIDs (or any other field), bump
UpdatedAt to the current time. This ensures the artwork cache key changes,
invalidating any stale artwork that was resolved and cached during Phase 1
when the album had incomplete folder data.
* fix(artwork): include ImportedAt in artwork cache key to invalidate stale cache
Reverts the Phase 3 UpdatedAt bump (which would change album.UpdatedAt
semantics) and instead includes album.ImportedAt in the artwork cache key
computation. Since ImportedAt is bumped to time.Now() on every album Put,
any Phase 3 correction naturally invalidates cached artwork that was
resolved mid-scan with incomplete folder data.
* fix(artwork): simplify lastUpdate logic using TimeNewest utility
Signed-off-by: Deluan <deluan@navidrome.org>
---------
Signed-off-by: Deluan <deluan@navidrome.org>
* chore(ci): update GitHub Actions to latest major versions
Update actions/cache v4→v5, actions/github-script v3→v7,
actions/stale v9→v10, docker/login-action v3→v4,
docker/setup-buildx-action v3→v4, and docker/metadata-action v5→v6.
The github-script upgrade also migrates Octokit API calls from
github.* to github.rest.* namespace (required since v5).
* fix(ci): address review feedback on GitHub Actions update
Pass github_token to docker/metadata-action@v6 to avoid API rate
limiting. Fix github-script pagination to use the correct Octokit
paginate.iterator pattern (pass endpoint method, not awaited response).
Replace timing-sensitive time.Sleep synchronization with proper
Eventually/Consistently assertions in watcher tests, and increase
Eventually timeouts from 200ms to 500ms. Add FlakeAttempts(3) to the
inherently timing-dependent tests. For the scheduler test, increase
the Eventually timeout from 1s to 5s for the cron job execution check.
Fixed two bugs in album cover art resolution for multi-disc layouts:
1. compareImageFiles now sorts by path depth (shallower first) when basenames
tie, so album-root images like Artist/Album/cover.jpg are preferred over
disc-subfolder images like Artist/Album/CD1/cover.jpg.
2. commonParentFolder now includes the parent folder for single-disc-subfolder
albums, with a Path != "." guard to avoid pulling artist-folder images.
Closes#5376
* feat(plugins): add PlaybackReport to Scrobbler interface and all implementations
* feat(plugins): add PlaybackReport worker and dispatch in PlayTracker
* feat(plugins): add PlaybackReportRequest to plugin scrobbler capability
* chore(plugins): regenerate PDK files with PlaybackReport
* feat(plugins): add PlaybackReport to test scrobbler plugin
* feat(plugins): add PlaybackReport to plugin scrobbler adapter
* refactor(plugins): fix double DB fetch in StateStopped and batch getActiveScrobblers
- Hoist mf from scrobble branch so PlaybackReport reuses it instead of
fetching again from DB
- Call getActiveScrobblers once per drain batch instead of per-entry
* chore(plugins): include generated scrobbler schema with PlaybackReport
* fix(plugins): skip PlaybackReport for plugins that don't export it
Plugins detected as scrobblers only need to export one scrobbler
function. Older plugins that don't export nd_scrobbler_playback_report
would cause noisy error logs on every reportPlayback call. Now
errFunctionNotFound and errNotImplemented are treated as no-ops.
* refactor: rename NowPlayingInfo to PlaybackReport
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: rename stopNowPlayingWorker to stopBackgroundWorkers
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: move NowPlaying and PlaybackReport logic to separate worker files
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor(scrobbler): rename NowPlayingInfo to PlaybackSession and add expired state
Rename NowPlayingInfo struct to PlaybackSession to better reflect its role
as a complete playback session representation. Add UserId field to make
sessions self-contained, removing redundant userId parameters from
PlaybackReport interface method and internal dispatch functions. Introduce
StateExpired internal state that fires when a session cache entry expires
without an explicit stop, ensuring plugins always receive a terminal event
regardless of client behavior.
* fix(scrobbler): update playback state description to include 'expired'
Signed-off-by: Deluan <deluan@navidrome.org>
* fix(scrobbler): resolve data race in OnExpiration callback
Capture conf.Server.EnableNowPlaying at construction time instead of
reading it from the background ttlcache eviction goroutine. The previous
code raced with test config cleanup that writes to the same field
concurrently.
* fix(scrobbler): return error when media file lookup fails in StateStopped
Simplify the MediaFile population logic in the stopped case to return an
error if the track cannot be found. A stop report with an empty MediaFile
is useless to plugins, and returning the error allows clients to retry
or alert the user when auto-scrobble is enabled.
* refactor(scrobbler): use session data directly in PlaybackReport adapter
Use info.Username from PlaybackSession instead of extracting it from
context in the plugin adapter, since the session is now self-contained.
Add debug/trace logging for session expiration and enqueue the expired
report with a user-enriched context so downstream handlers can identify
the user.
---------
Signed-off-by: Deluan <deluan@navidrome.org>
* feat(config): add UIPlaybackReportInterval setting
* feat(server): expose playbackReportIntervalMs to UI config
* feat(ui): add playbackReportIntervalMs config default
* feat(ui): replace scrobble/nowPlaying with reportPlayback in subsonic API layer
* feat(ui): replace scrobble logic with reportPlayback state machine in Player
* refactor(ui): simplify Player heartbeat using useInterval hook
- Replace manual setInterval/clearInterval with existing useInterval hook
- Extract shared reportPlaybackUrl helper to deduplicate URL construction
- Use ref for currentTrackId in beforeunload to stabilize effect deps
- Have heartbeat read lastPositionMsRef instead of audioInstance.currentTime
* feat(ui): redesign NowPlaying panel with Discord-style layout
Show album art with play/pause overlay icon, track title, artist,
album name, progress bar with position/duration, and username.
* fix(ui): adjust NowPlaying panel height to fit 3 entries
* fix(ui): send stopped on player close and tab close while paused
- onBeforeDestroy now sends reportPlayback stopped before clearing queue
- beforeunload sends stopped beacon regardless of pause state
* feat(ui): animate NowPlaying progress bar with 1s client-side tick
* fix(ui): account for playbackRate in NowPlaying progress interpolation
* fix(ui): use timestamp-based interpolation for smooth NowPlaying progress
Replace tick counter with fetchedAt timestamp so the progress bar
advances smoothly without resetting on each server poll.
* fix(ui): fix NowPlaying progress bar not animating
Pass `now` (Date.now()) as a prop that changes every tick, so
React.memo'd components actually re-render each second.
* fix(ui): prevent progress bar reset on NowPlaying poll
Set fetchedAt and now atomically on fetch so the elapsed offset
starts at zero and the server's already-estimated positionMs
is used as the base without a visible jump.
* fix(ui): stamp entries with fetch time to prevent progress bar reset
Embed _fetchedAt timestamp directly into each entry object so the
position and its reference timestamp are always in the same state
update, eliminating the React 17 multi-setState batching race.
* fix(server): estimate position for starting state in GetNowPlaying
GetNowPlaying was only estimating elapsed position for the "playing"
state, returning raw positionMs=0 for "starting". Since the UI
player sends "starting" once and then doesn't update until the
60s heartbeat, NowPlaying polls returned 0 for up to a minute,
causing the progress bar to reset on every poll.
* fix(ui): send playing immediately after starting to enable position estimation
The server only estimates elapsed position for "playing" state in
GetNowPlaying. The Player was sending "starting" once and then not
updating until the 60s heartbeat, leaving the server state as
"starting" with positionMs=0 for up to a minute.
Now the Player follows up "starting" with an immediate "playing"
call, transitioning the server state so position estimation works
from the first poll.
* fix(subsonic): fix getNowPlaying returning same playerId for all entries
PlayerId was never incremented in the map callback, so every entry
got playerId=1. This caused the UI to use duplicate React keys,
mixing up rendered entries between players. Also use a stable
composite key in the UI instead of the sequential playerId.
* fix(ui): only send stopped beacon when tab is actually closing
Move the reportPlaybackBeacon call from beforeunload to pagehide.
beforeunload fires before the confirmation dialog, so cancelling
the close would still send stopped. pagehide only fires when the
page is actually being unloaded.
* fix(ui): revert to beforeunload for stopped beacon
pagehide does not fire reliably in Chrome when closing tabs.
Use beforeunload instead — if the user cancels the close dialog,
the heartbeat will re-register the NowPlaying entry on its next tick.
* fix(ui): use synchronous XHR for stopped report on tab close
Replace sendBeacon with synchronous XMLHttpRequest in beforeunload.
This blocks the page from closing until the server acknowledges
the stopped state, ensuring the NowPlaying entry is always removed.
* fix(ui): fix confirmation dialog and use fetch keepalive for tab close
- Move e.preventDefault() before the stopped report so the dialog
always shows regardless of XHR errors
- Use fetch with keepalive:true instead of sync XHR (more reliable,
non-blocking, survives page teardown)
- Fall back to sendBeacon if fetch throws
* fix(ui): prevent heartbeat from re-adding entry after stopped on tab close
Set a stoppedRef flag in beforeunload so the heartbeat interval
skips sending playing reports after stopped has been sent.
Without this, the heartbeat could re-register the NowPlaying
entry after the stopped event removed it.
* fix(ui): include client unique ID header in stopped report on tab close
Root cause: reportPlaybackSync (fetch keepalive) did not include the
X-ND-Client-Unique-Id header. Regular reportPlayback calls via
httpClient include this header, and the server uses it as the playMap
key. Without the header, the stopped call fell back to player.ID
as the key, which didn't match the entry added with the UUID key.
The playMap.Remove targeted the wrong key, so the entry persisted.
Fix: export clientUniqueId from httpClient and include it as a header
in the fetch keepalive request.
* fix(ui): use pagehide for stopped report to avoid premature send
beforeunload fires before the confirmation dialog, so the stopped
event was sent even when the user cancelled closing. Move the
stopped report to pagehide, which only fires when the page is
actually being unloaded (after confirmation).
* feat(server): broadcast NowPlaying SSE on every state change
Previously, the SSE broadcast only fired when the NowPlaying count
changed. Now it fires on every reportPlayback call (starting,
playing, paused, stopped), so the NowPlaying panel gets instant
updates for state transitions and position changes.
The UI reducer stores a nowPlayingLastUpdate timestamp alongside
the count, ensuring every SSE event triggers a re-fetch even when
the count is unchanged (e.g., pause/resume).
* fix(ui): clamp NowPlaying position to prevent negative time display
* fix(ui): debounce NowPlaying fetches to prevent progress bar trembling
During track changes, rapid SSE events (stopped, starting, playing)
triggered multiple refetches within milliseconds, each resetting the
interpolation base and causing the progress bar to oscillate. Skip
fetches within 1 second of the previous fetch.
* feat(ui): report playback position on seek
Send a reportPlayback(playing) call when the user seeks/scrubs in
the player, so the NowPlaying panel and server position stay in
sync immediately instead of waiting for the next 60s heartbeat.
* refactor: code review cleanup
- Export clientUniqueIdHeader from httpClient, use in subsonic layer
- Fix variable shadowing (now → fetchNow) in NowPlayingPanel fetchList
- Fix onBeforeDestroy nested dep (read isRadio from ref instead)
- Only broadcast SSE on state transitions, not heartbeat position updates
- Only enqueue NowPlaying to external scrobblers on state transitions
Signed-off-by: Deluan <deluan@navidrome.org>
* fix(ui): use trailing-edge debounce for NowPlaying fetch
Replace the leading-edge throttle (which fetched on the first event
and blocked subsequent ones) with a trailing-edge debounce (300ms).
During track transitions, the burst of events (stopped → starting →
playing) now collapses into a single fetch after the burst settles,
showing the new track immediately instead of briefly showing empty.
* fix(ui): only show overlay on NowPlaying artwork when paused
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor(ui): remove unnecessary sendBeacon fallback from reportPlaybackSync
* refactor(ui): rename reportPlaybackSync to reportPlaybackKeepalive
The function was never synchronous — it uses fetch with keepalive:true,
which is fire-and-forget. The name now reflects the actual behavior.
* style: format code with prettier
* test: add tests for reportPlayback SSE broadcast and UI changes
- play_tracker: verify SSE broadcast on every state transition and
that broadcasts are skipped when EnableNowPlaying is false
- activityReducer: verify nowPlayingLastUpdate timestamp is set
- subsonic/index: verify reportPlayback URL construction
Signed-off-by: Deluan <deluan@navidrome.org>
* fix(ui): prevent NowPlaying from fetching every second when panel is open
fetchList had unstable identity because it depended on doFetch
(which depended on notify/dispatch). Each 1s setNow re-render
recreated the callback chain, re-triggering the useEffect that
calls fetchList. Use a ref for the fetch logic so fetchList has
a stable identity with empty deps.
* fix(ui): break fetch→dispatch→effect→fetch loop in NowPlaying panel
The fetch dispatched nowPlayingCountUpdate on every result, which
updated nowPlayingLastUpdate in Redux, which triggered the SSE
effect to call fetchList again — creating a fetch loop.
Fix: remove dispatch from fetch results. The badge count uses
entries.length (from local state) when entries are loaded, falling
back to Redux count (from SSE) when they aren't. SSE events remain
the only trigger for nowPlayingLastUpdate, breaking the loop.
* fix(ui): clear NowPlaying entries on panel close so badge uses SSE count
* style: format code with prettier
* fix: address code review feedback
- Fix currentTime truthiness check to handle position 0 correctly
- Report actual player state (playing/paused) on seek instead of
always sending 'playing'
- Remove idx from React key to avoid reorder issues
- Add debounce timer cleanup on unmount
- Keep entries on panel close so badge stays accurate from polling
- Fix test description to match actual assertion
* fix(ui): keep NowPlaying badge count accurate from polling
Add a separate nowPlayingCountSync action that updates the Redux
count without setting nowPlayingLastUpdate (which would trigger
the SSE effect and cause a fetch loop). Polling results now sync
the badge count via this action, so the badge stays accurate even
when SSE is unavailable.
* style: format code with prettier
---------
Signed-off-by: Deluan <deluan@navidrome.org>
* feat(req): add Float64Or helper for parsing float query params
* feat(scrobbler): extend NowPlayingInfo with state/position/rate fields
* feat(scrobbler): implement ReportPlayback with state machine and auto-scrobble
* feat(responses): add state/positionMs/playbackRate to NowPlayingEntry
* feat(subsonic): add reportPlayback endpoint handler
* feat(subsonic): include state/positionMs/playbackRate in getNowPlaying response
* feat(subsonic): register playbackReport OpenSubsonic extension
* test(e2e): add reportPlayback endpoint e2e tests
* refactor(scrobbler): simplify ReportPlayback — extract helpers, remove duplication
- Add state constants and exported ValidStates map
- Extract remainingTTL() helper (was duplicated 3x)
- Merge playing/paused switch cases into single branch
- Use Get instead of GetWithParticipants for non-stopped states
- Guard NowPlayingCount broadcast with count-change detection
- Use cache entry for NowPlaying dispatch instead of extra DB query
- Remove redundant Position field from NowPlayingInfo
* refactor(scrobbler): skip DB query in playing/paused when playMap has entry
* fix(play_tracker): handle errors when adding/updating NowPlayingInfo in cache
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor(play_tracker): replace sort with slices.SortFunc for NowPlayingInfo
Signed-off-by: Deluan <deluan@navidrome.org>
* fix(play_tracker): check all ReportPlayback errors in tests
Replace _ = with explicit error assertions to avoid masking
failures in intermediate calls.
Signed-off-by: Deluan <deluan@navidrome.org>
* test(e2e): use real PlayTracker and assert getNowPlaying after reportPlayback
Replace noopPlayTracker with a real PlayTracker backed by the E2E
database. E2E tests now verify the full round-trip: reportPlayback
creates/updates/removes entries visible via getNowPlaying, including
state, positionMs, and playbackRate fields.
Export NewPlayTracker constructor for use outside the scrobbler package.
* fix(play_tracker): account for playback rate in TTL and detect track switches
The remainingTTL function now divides remaining time by the playback rate,
so cache entries expire correctly at non-1x speeds (e.g., 2x playback halves
the TTL). Zero/negative rates default to 1.0. The playing/paused case now
checks if the cached MediaFile ID matches the reported mediaId, falling back
to a DB fetch when the client switches tracks without sending stopped/starting.
Adds parameterized tests for remainingTTL covering rate variations and edge cases.
* fix(subsonic): validate positionMs and playbackRate in reportPlayback
Reject negative positionMs values and invalid playbackRate values (NaN,
Inf, zero, negative) at the API boundary before they reach TTL and
position estimation math. Returns clear error messages for each case.
* feat(play_tracker): add ClientId and ClientName to ReportPlayback parameters
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor(play_tracker): replace NowPlaying method with ReportPlayback calls
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor(play_tracker_test): remove redundant TTL behavior tests and clean up mockPluginLoader
Signed-off-by: Deluan <deluan@navidrome.org>
---------
Signed-off-by: Deluan <deluan@navidrome.org>
When a user played an album, advanced a few tracks, closed the player,
then played a different album, playback started mid-album at the
previous track index instead of track 1.
The root cause was in reduceSyncQueue: the hasPendingSwitch check
compared playIndex against savedPlayIndex, but after clearQueue both
reset to 0, making the check falsely conclude no switch was pending.
This caused PLAYER_SYNC_QUEUE to prematurely clear the playIndex and
clear flags before the music player library could act on them.
Fix: also treat clear=true as a signal that a track switch is pending,
since it means a new queue was just loaded.
* fix(ui): show album tile actions on keyboard focus - #4836
The album grid tile bar (containing play/heart/context-menu buttons) had
opacity:0 by default and only became visible on mouse :hover. Keyboard
users tabbing through the album grid never saw which tile was focused
and could not discover the available actions.
Mirrors the existing :hover rule with :focus-within, which matches when
the link itself or any descendant (e.g. the play button) has focus.
Signed-off-by: Daniel Banariba <banaribad@gmail.com>
* fix(ui): also disable pointer events on hidden album tile bar - #4836
Per review feedback (@gemini-code-assist): the tileBar's buttons remained
clickable even at opacity:0, causing accidental Play/Menu triggers when a
user clicked what looked like the album cover.
Set 'pointerEvents: none' on the base tileBar and restore 'auto' on the
same selector that turns it visible.
Signed-off-by: Daniel Banariba <banaribad@gmail.com>
---------
Signed-off-by: Daniel Banariba <banaribad@gmail.com>
Rename FieldInfo.Name to Alias (only meaningful for backward-compat
entries like albumtype→releasetype) and FieldInfo.alias to tagAlias
(tag names from mappings.yml that resolve to existing fields). Add a
Name() method that derives the canonical name from the map key,
eliminating 60+ redundant Name declarations where the value matched
the key. LookupField now populates the private name field automatically.
Signed-off-by: Deluan <deluan@navidrome.org>
Expose the four ReplayGain database columns (rg_album_gain, rg_album_peak,
rg_track_gain, rg_track_peak) as first-class numeric criteria fields for
smart playlists. This allows users to filter tracks by ReplayGain values,
e.g. finding tracks with album gain below a threshold.
* feat(smartplaylist): add IsMissing and IsPresent operator types
Add two new Expression types for detecting absent/present tags and
roles in smart playlist criteria. Includes JSON marshal/unmarshal
support and Walk visitor registration.
* test(smartplaylist): add JSON marshal/unmarshal tests for isMissing/isPresent
* feat(smartplaylist): add SQL generation for isMissing/isPresent operators
Tags check json_tree(media_file.tags) for key existence.
Roles check json_tree(media_file.participants) for key existence.
Regular DB column fields are rejected with an error.
* test(smartplaylist): add e2e tests for isMissing/isPresent operators
Tests cover tag presence/absence with selective matching (grouping),
universal absence (lyricist role), universal presence (composer role),
and combined operator usage.
* refactor(smartplaylist): use strconv.ParseBool in IsTruthy
Replace hand-rolled string truthiness check with strconv.ParseBool,
which correctly handles standard boolean strings and rejects
unrecognized values as false.
* refactor(smartplaylist): clarify missingExpr parameter naming
Rename defaultNegate to checkAbsence and extract truthy local for
readability. The XNOR logic (checkAbsence == truthy) is now easier
to follow: isMissing passes true, isPresent passes false.
* refactor(smartplaylist): reuse jsonExpr in missingExpr, improve errors
- tagCond/roleCond now handle nil cond (existence-only check)
- missingExpr delegates to jsonExpr(info, nil, negate) instead of
building SQL manually
- Better error messages: unknown fields now report the field name
When clearing the queue, the reducer resets to initialState which lacks
an autoPlay field (undefined). The autoPlay option was computed as true
because undefined !== false, causing the music player library to attempt
playback of the first song before internal state was fully cleared.
Added a queue.length > 0 guard to the autoPlay calculation so it is
never true when the queue is empty, regardless of other state flags.
Fixes#5331
* fix(test): enable Subsonic response snapshot tests on Windows
Replaced cupaloy with a simple custom snapshot matcher that normalizes
CRLF line endings before comparison. The tests were skipped on Windows
via a //go:build unix tag because Git for Windows checks out snapshot
files with CRLF, while Go's xml/json.MarshalIndent always produces LF,
causing direct string comparison to fail. The new matcher reads snapshot
files with os.ReadFile and normalizes \r\n to \n before comparing.
Also added a .gitattributes in the .snapshots directory to enforce LF
checkout, and removed the now-unused cupaloy dependency.
* fix(test): add UPDATE_SNAPSHOTS support to custom snapshot matcher
Restore the ability to update snapshots via `make snapshots`
(UPDATE_SNAPSHOTS=true), which was lost when replacing cupaloy
with the custom matcher.
* fix(sharing): validate JWT expiration and share existence on stream endpoint
The public stream endpoint (/public/s/{token}) was using
TokenAuth.Decode() which only verifies the JWT signature but skips
exp claim validation. This allowed expired share stream URLs to remain
functional indefinitely. Additionally, deleting a share did not revoke
previously issued stream tokens since the handler never performed a
server-side share lookup.
Fixed by switching decodeStreamInfo() to use auth.Validate() which
properly checks the exp claim, and by embedding the share ID ("sid")
in stream tokens so the handler can verify the share still exists.
Old tokens without the sid claim remain backward compatible but still
benefit from expiration validation.
* fix(sharing): check share expiration on stream requests
Replace the lightweight Exists() check with Get() + expiration
validation, so that shares whose ExpiresAt was updated to an earlier
time after token issuance are also rejected (410 Gone). Reuses the
existing checkShareError handler for consistent error responses.
* feat(ui): add Not Starred filter option - #5108
Signed-off-by: Daniel Banariba <banaribad@gmail.com>
* fix(ui): apply notStarred translation to playlistTrack resource - #5108
Signed-off-by: Daniel Banariba <banaribad@gmail.com>
* refactor(ui): use NullableBooleanInput for starred filter - #5108
Replace QuickFilter approach with NullableBooleanInput per maintainer
review feedback. Single tri-state filter (Yes/No/Any) instead of two
separate buttons + dataProvider translation. Matches the existing pattern
used by the 'missing' filter.
Signed-off-by: Daniel Banariba <banaribad@gmail.com>
---------
Signed-off-by: Daniel Banariba <banaribad@gmail.com>
Co-authored-by: Deluan Quintão <deluan@navidrome.org>
* feat(plugins): add sonicSimilarity capability types
Defines the SonicSimilarity plugin capability interface with
GetSonicSimilarTracks and FindSonicPath methods, along with
their request/response types.
* feat(sonic): add core sonic similarity service
Implements the Sonic service with HasProvider, GetSonicSimilarTracks,
and FindSonicPath, delegating to the PluginLoader and using the
Matcher for index-preserving library resolution.
* test(sonic): add sonic service unit tests
Covers HasProvider, GetSonicSimilarTracks, and FindSonicPath with
mock plugin loader and provider, verifying error propagation and
successful match resolution via the library matcher.
* feat(matcher): add MatchSongsToLibraryMap for index-preserving matching
Adds a new method alongside MatchSongsToLibrary that returns a
map[int]MediaFile keyed by input song index rather than a flat slice,
enabling callers to correlate similarity scores back to the original
position in the results.
* fix(sonic): check provider availability before MediaFile lookup
Avoids unnecessary DB call when no plugin is available, and ensures
the correct error path is tested.
* feat(plugins): add sonic similarity adapter
Adds SonicSimilarityAdapter implementing sonic.Provider, bridging the
plugin system to the core sonic service via Extism plugin functions.
Reuses existing songRefsToAgentSongs helper for SongRef conversion.
* feat(plugins): add LoadSonicSimilarity to plugin manager
Adds Manager.LoadSonicSimilarity method following the pattern of
LoadLyricsProvider, enabling the core sonic service to load a
SonicSimilarityAdapter from a named plugin.
* feat(subsonic): add sonicMatch response type
Add SonicMatch struct with Entry and Similarity fields, and a SonicMatches slice to the Subsonic response struct. These types support the OpenSubsonic sonicSimilarity extension for returning similarity-scored track results.
* feat(subsonic): add getSonicSimilarTracks and findSonicPath handlers
Add two new Subsonic API handlers for the sonicSimilarity OpenSubsonic extension: GetSonicSimilarTracks returns similarity-scored tracks similar to a given song, and FindSonicPath returns a path of tracks connecting two songs. Both handlers delegate to the sonic core service and map results to SonicMatch response types.
* feat(subsonic): advertise sonicSimilarity extension when plugin available
Update GetOpenSubsonicExtensions to conditionally include the sonicSimilarity extension only when a sonic similarity plugin provider is available. The nil guard ensures backward compatibility with tests that pass nil for the sonic field. Also update the existing test to pass the new nil parameter.
* feat(subsonic): wire sonic similarity service into router
Add the sonic.Sonic service to the Router struct and New() constructor, register the getSonicSimilarTracks and findSonicPath routes, and wire sonic.New and its PluginLoader binding into the Wire dependency injection graph. Update all existing test call sites to pass the new nil parameter. Regenerate wire_gen.go.
* fix(e2e): add sonic parameter to subsonic.New call in e2e tests
* test(subsonic): add sonicSimilarity extension advertisement tests
Restructures the GetOpenSubsonicExtensions test into two contexts: one verifying the baseline 5 extensions are returned when no sonic similarity plugin is configured, and one verifying that the sonicSimilarity extension is advertised (making 6 total) when a plugin loader reports an available provider. Adds a mockSonicPluginLoader to satisfy the sonic.PluginLoader interface without requiring a real plugin.
* feat(subsonic): add nil guard and e2e tests for sonic similarity endpoints
Handlers return ErrorDataNotFound when no sonic service is configured,
preventing nil panics. E2e tests verify both endpoints return proper
error responses when no plugin is available.
* fix(subsonic): return HTTP 404 when no sonic similarity plugin available
Endpoints are always registered but return 404 when no provider is
available, rather than a subsonic error code 70.
* refactor: clean up sonic similarity code after review
Extract shared helpers to reduce duplication across the sonic similarity
implementation: loadAllMatches in matcher consolidates the 4-phase
matching pipeline, songRefToAgentSong eliminates per-iteration slice
allocation in the adapter, sonicMatchResponse deduplicates response
building in handlers, and a package-level constant replaces raw
capability name strings in core/sonic.
* fix empty response shapes
Signed-off-by: Deluan <deluan@navidrome.org>
* test(plugins): add testdata plugin and e2e tests for sonic similarity
Add a test-sonic-similarity WASM plugin that implements both
GetSonicSimilarTracks and FindSonicPath via the generated sonicsimilarity
PDK. The plugin returns deterministic test data with decreasing similarity
scores and supports error injection via config. Adapter tests verify the
full round-trip through the WASM plugin including error handling. Also
includes regenerated PDK code from make gen.
* docs: update README to include new capabilities and usage examples for plugins
Signed-off-by: Deluan <deluan@navidrome.org>
* test(e2e): enhance sonic similarity tests with additional scenarios and mock provider
Signed-off-by: Deluan <deluan@navidrome.org>
* fix: address PR review feedback for sonic similarity
Fix incorrect field names in README documentation ({from, to} →
{startSong, endSong}) and remove unnecessary XML serialization test
from e2e suite since OpenSubsonic endpoints only use JSON.
* refactor: rename Matcher methods for conciseness
Rename MatchSongsToLibrary to MatchSongs and MatchSongsToLibraryMap to
MatchSongsIndexed. The Matcher receiver already establishes the "to
library" context, making that suffix redundant, and "Indexed" better
describes the intent (preserving input ordering) than "Map" which
describes the data structure.
* refactor: standardize variable naming for media files in sonic path methods
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: simplify plugin loading by introducing adapter constructors
Signed-off-by: Deluan <deluan@navidrome.org>
---------
Signed-off-by: Deluan <deluan@navidrome.org>
* fix(playlists): allow toggling auto-import (sync) via REST API
The updatePlaylistEntity handler was not applying the sync field from
incoming requests, causing the auto-import toggle in the UI to have no
effect. Apply the sync value for file-backed playlists only.
* fix(playlists): enhance update logic for playlist metadata and sync toggle
Signed-off-by: Deluan <deluan@navidrome.org>
* fix(playlists): address code review feedback
- Add pointer equality short-circuit in rulesEqual before reflect.DeepEqual
- Guard against empty ID in Put's partial-update path
- Only apply Sync when it actually differs from current value, preventing
zero-value overwrites from partial payloads
* fix(playlists): remove unused parameters from Update method
Signed-off-by: Deluan <deluan@navidrome.org>
---------
Signed-off-by: Deluan <deluan@navidrome.org>
* test(artwork): add e2e suite documenting album/disc resolution
Adds core/artwork/e2e/ with a real-tempdir + scanner harness that exercises
artwork resolution end-to-end. Covers album and disc kinds; pending (PIt)
cases document two known bugs in reader_album.go for regression-guard
flipping once they are fixed.
* refactor(artwork): add libraryFS helper to resolve MusicFS for a library
* test(artwork): tighten libraryFS test isolation and add scheme-error case
* test(artwork): update libraryFS test description to match implementation
* refactor(artwork): convert fromExternalFile to use fs.FS
Add a temporary fromExternalFileAbs shim so existing absolute-path callers
still compile; the shim is removed once all readers are migrated.
* refactor(artwork): make fromExternalFileAbs a thin delegator
Introduce a minimal osDirectFS adapter so the shim no longer duplicates
the matching loop. Both will be removed in Task 9.
* refactor(artwork): convert fromTag to taglib.OpenStream over fs.FS
Add a temporary fromTagAbs shim so existing absolute-path callers still
compile; removed in Task 9. Reuses the osDirectFS adapter from Task 2.
* refactor(artwork): defer fs.File close until after taglib reads finish
Mirror the lifetime pattern used by adapters/gotaglib/gotaglib.go:
keep the underlying fs.File open until taglib.File is closed, and
pass WithFilename so format detection doesn't rely on content sniffing.
* docs(artwork): note ffmpeg's path-based API limitation
* refactor(artwork): migrate album reader to MusicFS
- Add libFS (storage.MusicFS) field to albumArtworkReader; resolved
once at construction time via libraryFS()
- Switch fromCoverArtPriority from abs-path shims to FS-based
fromTag/fromExternalFile; only fromFFmpegTag retains absolute path
- Build imgFiles as library-relative forward-slash paths in
loadAlbumFoldersPaths using path.Join(f.Path, f.Name, img)
- Guard embedAbs so that an empty EmbedArtPath never produces a
non-empty absolute path (prevents accidental ffmpeg invocation)
- Register testfile:// storage scheme in artwork test suite to provide
an os.DirFS-backed MusicFS without requiring the taglib extractor
- Update test assertions from filepath.FromSlash(abs) to bare
forward-slash relative strings
* fix(artwork): use path package in compareImageFiles for forward-slash relative paths
* refactor(artwork): migrate disc reader to MusicFS
Replace os.Open absolute-path access with libFS.Open on library-relative
forward-slash paths. Rename discFolders→discFoldersRel, split
firstTrackPath into firstTrackRelPath (for fromTag) and firstTrackAbsPath
(for fromFFmpegTag), and switch path.Dir/Base/Ext for forward-slash safety.
* refactor(artwork): build discFoldersRel directly and guard empty first track
* refactor(artwork): migrate mediafile reader to MusicFS
* refactor(artwork): migrate artist album-art lookup to MusicFS
* refactor(artwork): remove temporary path-based shims
All readers now use the FS-based fromTag and fromExternalFile directly,
so the absolute-path adapters and the osDirectFS helper that backed
them can go away.
* test(artwork): rewrite e2e suite to use storagetest.FakeFS
Switches from real-tempdir + local storage to FakeFS via the storage
registry. Adds a proper multi-disc scenario using the disc tag, which
previously required curated MP3 fixtures we did not have.
* test(artwork): use maps.Copy in trackFile tag merge
Lint cleanup: replace the manual map-copy loop flagged by mapsloop.
* test(artwork): reuse tests.MockFFmpeg in e2e harness
Replace the hand-rolled noopFFmpeg stub with tests.NewMockFFmpeg, which
already satisfies the full ffmpeg.FFmpeg interface and won't drift when
new methods are added. Also tie imageBytes to imageFile so they cannot
silently disagree on the on-disk encoding.
* test(artwork): add e2e scenarios from artwork documentation
Covers the behaviors documented at
https://www.navidrome.org/docs/usage/library/artwork/:
- Album: folder.*/front.* fallbacks and priority order with cover.*.
- Disc: cd*.* match, cover.* inside disc folder, DiscArtPriority="" skip
path, the documented multi-disc layout, and the discsubtitle keyword.
- MediaFile: disc-level fallback for multi-disc tracks and album-level
fallback for single-disc tracks (doc section "MediaFiles" items 2-3).
- Artist: album/artist.* lookup via libFS (passes). The artist-folder
branch is XIt-marked because fromArtistFolder still calls os.DirFS
directly on an absolute path and can't read from a FakeFS-backed
library — migrating that to storage.MusicFS is a follow-up.
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor(artwork): scope artist folder traversal to library root
Route fromArtistFolder reads through storage.MusicFS and bound the
parent-directory walk at the library root. This keeps artwork
resolution scoped to the configured library and unblocks FakeFS-backed
e2e scenarios that depend on the artist folder.
Also consolidate the libraryFS + core.AbsolutePath pairing (used by
three readers) into a single libraryFSAndRoot helper.
* test(artwork): add ASCII file-tree diagrams to e2e scenarios
Each It/PIt block now shows the on-disk layout it exercises, with
arrows indicating which file wins (or should win, for the known-bug
PIt cases). Makes scenarios readable at a glance without having to
parse the MapFS map.
* test(artwork): add e2e tests for playlist and radio artwork resolution
Signed-off-by: Deluan <deluan@navidrome.org>
* test(artwork): enhance e2e tests with real MP3 fixtures for embedded artwork
Signed-off-by: Deluan <deluan@navidrome.org>
* test(ffmpeg): add support for animated WebP encoder detection and fallback handling
Signed-off-by: Deluan <deluan@navidrome.org>
* test(artwork): cover additional edge cases in e2e suite
Add high-value scenarios uncovered by the existing specs:
- Album: three-way basename tie (unsuffixed wins), unknown pattern in
CoverArtPriority is skipped, embedded-first with no embedded art
falls through.
- Disc: discsubtitle with no matching image falls through.
- Artist: ArtistArtPriority can reach images via album/<pattern>.
- Playlist: generates a 2x2 tiled cover from album art when the playlist
has no uploaded/sidecar/external image.
New helper realPNG() produces real taglib/image-decodable bytes so the
tiled-cover test can exercise the generator's decode + compose path.
* test(artwork): refactor image upload logic in e2e tests for consistency
Signed-off-by: Deluan <deluan@navidrome.org>
* test(ffmpeg): simplify animated WebP encoder check by removing context parameter
Signed-off-by: Deluan <deluan@navidrome.org>
* fix(artwork): normalize rel path for fs.Glob on Windows
filepath.Rel returns backslash-separated paths on Windows, but fs.Glob
and path.Join require forward slashes. Convert with filepath.ToSlash
after computing the relative path and use path.Dir for the parent walk
so the artist-folder lookup works cross-platform.
* fix(ffmpeg): retry animated WebP probe on transient failure
The probe previously used the caller's request context inside sync.Once,
so a single cancelled first request would permanently disable animated
WebP for the rest of the process. Switch to a mutex + probed flag, use
a fresh background context with its own timeout, and only cache the
result when the probe actually succeeds.
* test(ffmpeg): reset ffOnce so ConvertAnimatedImage test is order-independent
The ConvertAnimatedImage stand-in test sets ffmpegPath directly but
does not reset ffOnce. If ffmpegCmd() has not been called earlier in
the test process, the next call inside hasAnimatedWebPEncoder runs
ffOnce.Do and re-resolves the real ffmpeg binary, overwriting the
stand-in and breaking the test. Reset ffOnce and conf.Server.FFmpegPath
alongside the other globals to pin resolution to the stand-in.
* test(artwork): unblock Windows CI — forward-slash fs paths and suite-level DB lifetime
The internal artwork test planted a Windows absolute path (backslashes) into
Folder.Path and then fed it through libFS.Open, which fs.ValidPath rejects.
Rooting the testfile library at the temp dir directly and using
filepath.ToSlash keeps the path model library-relative and forward-slash,
matching production.
The e2e suite opened a per-spec DB in a per-spec TempDir, but the go-sqlite3
singleton kept the file open across specs. Ginkgo's per-spec TempDir cleanup
then tried to unlink a file still held by that handle — fine on POSIX, fails
on Windows. Moving the DB to a suite-level tempdir and closing it in
AfterSuite avoids the race.
* test(artwork): keep Windows drive letters intact in testfile library URLs
url.Parse on `testfile://C:/path` reads `C` as the host and the path loses
the drive letter, so Windows libFS lookups go to `/path` and fail.
testFileLibPath now prepends a `/` when the OS path has no leading slash,
and the testfile constructor strips that extra slash back off before
handing the path to os.Stat / os.DirFS.
* refactor(artwork): consolidate libFS + root into libraryView helper
Collapses the per-reader libFS/libPath/rootFolder/firstTrackAbsPath fields
into a single libraryView{FS, absRoot} with an Abs(rel) method. Also folds
the two library lookups (ds.Library.Get + core.AbsolutePath) into one, and
uses mf.Path directly instead of stripping libRoot off an absolute path.
* refactor(ffmpeg): replace hasAnimatedWebPEncoder with encoderProbe for state management
Signed-off-by: Deluan <deluan@navidrome.org>
* fix: escape artist folder names in artwork glob
Escape glob metacharacters in the library-relative artist folder path before composing the fs.Glob pattern for artist image lookup. This preserves literal folder names such as Artist [Live] while keeping the configured filename pattern behavior unchanged, and adds a regression test for bracketed artist folders.
Signed-off-by: Deluan <deluan@navidrome.org>
* fix(artwork): correct test path assertions after MusicFS migration
Source functions (fromTag, fromExternalFile) now return forward-slash
fs.FS-relative paths, so test assertions should compare against plain
forward-slash strings, not filepath.FromSlash(). The artistArtPriority
test needs filepath.FromSlash() on the suffix because findImageInFolder
returns OS-native absolute paths via filepath.Join.
* fix(artwork): normalize path separators in artistArtPriority assertion
The two table entries exercise different code paths: entry 1 goes through
fromArtistFolder (returns OS-native paths via filepath.Join), while entry 2
goes through fromExternalFile (returns forward-slash fs.FS paths). Using
filepath.FromSlash on the expected value only works for entry 1.
Normalize the actual path to forward slashes with filepath.ToSlash so a
single HaveSuffix assertion works for both code paths on all platforms.
---------
Signed-off-by: Deluan <deluan@navidrome.org>
* fix(test): enable artwork tests on Windows by using OS-aware path assertions
Replace hardcoded forward-slash path expectations with filepath.FromSlash()
so assertions match OS-native separators on Windows. Removes all 8
SkipOnWindows("#TBD-path-sep-artwork") guards from artwork unit tests.
* test: add comment explaining forward-slash paths in test fixtures
Add LibraryID field to TrackInfo so plugins with library filesystem access
can determine which library a track belongs to. This lets plugins resolve
the full filesystem path by combining the library's root path with the
track's relative path. LibraryID is gated behind the same filesystem
access permission check as Path.
* test: add tests for recordingdate alias resolution in smart playlists
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: update FieldInfo structure and simplify fieldMap initialization
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: move sort parsing logic from persistence to criteria package
Extracted sort field parsing, validation, and direction handling from
persistence/criteria_sql.go into model/criteria/sort.go. The new
OrderByFields method on Criteria parses the Sort/Order strings into
validated SortField structs (field name + direction), resolving aliases
and handling +/- prefixes and order inversion. The persistence layer now
consumes these parsed fields and only handles SQL expression mapping.
This centralizes sort parsing to enforce consistent implementations.
* refactor: standardize field access in smartPlaylistCriteria structure
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: add ResolveLimit method to Criteria
Moved the percentage-limit resolution logic from playlist_repository
into Criteria.ResolveLimit, replacing the 3-line mutate-after-query
pattern with a single method call. The method preserves LimitPercent
rather than zeroing it, since IsPercentageLimit already returns false
once Limit is set, making the clear redundant and lossy.
* refactor: improve child playlist loading and error handling in refresh logic
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: extract smart playlist logic to dedicated files
Moved refreshSmartPlaylist, addSmartPlaylistAnnotationJoins, and
addCriteria methods from playlist_repository.go to a new
smart_playlist_repository.go file. Extracted all smart playlist tests
to smart_playlist_repository_test.go. Added DeferCleanup to the
"valid rules" test to fix ordering flakiness when Ginkgo randomizes
test execution across files.
* refactor: break refreshSmartPlaylist into smaller focused methods
Split the monolithic refreshSmartPlaylist method into discrete helpers
for readability: shouldRefreshSmartPlaylist for guard checks,
refreshChildPlaylists for recursive dependency refresh,
resolvePercentageLimit for count-based limit resolution,
buildSmartPlaylistQuery for assembling the SELECT with joins, and
addMediaFileAnnotationJoin to DRY up the repeated annotation join clause.
* refactor: deduplicate child playlist IDs in Criteria
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: simplify withSmartPlaylistOwner to accept model.User
Replaced separate ownerID string and ownerIsAdmin bool parameters with a
single model.User struct, reducing the field count in smartPlaylistCriteria
and making the option function signature clearer. Updated all call sites
and tests accordingly.
* fix: handle empty sort fields and propagate child playlist load errors
OrderByFields now falls back to [{title, asc}] when all user-supplied
sort fields are invalid, preventing empty ORDER BY clauses that would
produce invalid SQL in row_number() window functions. Also restored the
original behavior where a DB error loading child playlists aborts the
parent smart playlist refresh, by making refreshChildPlaylists return a
bool.
* refactor: log warning when no valid sort fields are found
Signed-off-by: Deluan <deluan@navidrome.org>
---------
Signed-off-by: Deluan <deluan@navidrome.org>
Replaced the Fields() type switch with a fields() method on the
Expression interface, eliminating the need to update a central switch
when adding new expression types. Removed the now-redundant
criteriaExpression() marker method since fields() alone suffices to
restrict the interface. Extracted a conjunction interface for the
ChildPlaylistIds() lookup used by All and Any.
* refactor: rename ImportFile to ImportFromFolder in playlists service
* feat: add ImportFile method with library/folder resolution
* feat: allow sync flag upgrade on re-import of non-synced playlists
* feat: add pls export subcommand with bulk and single export
Add `navidrome pls export` command that supports:
- Single playlist export to stdout (-p flag only)
- Single playlist export to directory (-p and -o flags)
- Bulk export of all playlists to a directory (-o flag only)
- Filtering by user (-u flag)
- Automatic filename sanitization and collision detection
Also extracts findPlaylist helper from runExporter for reuse.
* feat: add pls import subcommand with sync flag support
* fix: improve error message for export without output directory
* test: add tests for ImportFile sync flag and sync upgrade behavior
* refactor: streamline export and import logic by removing redundant comments and improving library path matching
Signed-off-by: Deluan <deluan@navidrome.org>
* feat: update ImportFile method to include sync flag for playlist imports
Signed-off-by: Deluan <deluan@navidrome.org>
* feat: implement fetchPlaylists function to streamline playlist retrieval
Signed-off-by: Deluan <deluan@navidrome.org>
* feat: replace inline filename sanitization with centralized utility function
Signed-off-by: Deluan <deluan@navidrome.org>
* feat: refactor playlist import logic to consolidate sync handling and improve method signatures
Signed-off-by: Deluan <deluan@navidrome.org>
* fix: address code review feedback on playlist import/export
- Fix duplicate playlist creation on non-sync re-import: only reconcile
sync flag when the playlist was actually persisted (has an ID)
- Distinguish "not in any library" from real errors in resolveFolder
using a sentinel error, so DB/folder errors propagate instead of
falling back to ImportM3U
- Use bufio.Scanner in countM3UTrackLines instead of reading entire file
* feat: replace bufio.Scanner with UTF8Reader and LinesFrom utility for improved file reading
Signed-off-by: Deluan <deluan@navidrome.org>
* fix: record path for outside-library imports to prevent duplicates
Files outside all libraries now go through updatePlaylist with the
absolute path recorded, so re-importing the same file updates the
existing playlist instead of creating a duplicate.
* refactor: name guard condition in updatePlaylist for readability
Extracted the compound boolean expression into a named local variable
`alreadyImportedAndNotSynced` to make the intent of the early-return
guard clearer at a glance.
* add godocs
Signed-off-by: Deluan <deluan@navidrome.org>
---------
Signed-off-by: Deluan <deluan@navidrome.org>
* fix(search): transliterate non-ASCII letters symmetrically in FTS5 path
Songs and artists with letters like ø, æ, œ, ß were unsearchable. The
query path in server/subsonic/searching.go transliterates with
sanitize.Accents (Øystein → Oystein), but the FTS5 tokenizer's
remove_diacritics 2 only strips NFKD-decomposable marks — atomic
letters with built-in strokes/ligatures survive tokenization, so the
query side and index side disagreed.
Apply sanitize.Accents on both sides:
- normalizeForFTS now also emits an ASCII-transliterated form for each
word, so search_normalized contains the variant the query produces.
- buildFTS5Query transliterates the unquoted portion of the input so
every caller (Subsonic, REST fullTextFilter) gets the same handling.
Quoted phrases stay as typed, preserving phrase matches against the
original title/artist columns.
Existing libraries pick up the fix as records are re-scanned; users
can trigger a manual full rescan to refresh older entries.
* fix(search): cache transliteration and add ß/quoted-phrase test coverage
Address review feedback: call sanitize.Accents once per word and reuse
the result for both the punct-stripped and accent-only paths. Add missing
test entries for ß→ss transliteration and quoted Unicode phrase preservation.
---------
Co-authored-by: Claude <noreply@anthropic.com>
* test(e2e): add end-to-end tests for smart playlists functionality
Signed-off-by: Deluan <deluan@navidrome.org>
* fix: enforce playlist visibility in smart playlist InPlaylist/NotInPlaylist rules
Previously, the InPlaylist/NotInPlaylist smart playlist criteria only
allowed referencing public playlists, regardless of who owned the smart
playlist. This was too restrictive for owners referencing their own
private playlists and for admins who should have unrestricted access.
The fix passes the smart playlist owner's identity and admin status into
the criteria SQL builder, so that: admins can reference any playlist,
regular users can reference public playlists plus their own private ones,
and inaccessible referenced playlists produce a warning instead of a hard
error. Also prevents recursive refresh of child playlists the owner
cannot access.
* test(e2e): clarify user roles and fix playlist visibility tests
Renamed testUser/otherUser to adminUser/regularUser to make the admin
vs regular user distinction explicit in test code. Fixed three playlist
visibility tests that were evaluating as admin (bypassing all access
checks) instead of as a regular user, so the public playlist path is
now actually exercised. All playlist operator tests now use explicit
evaluateRuleAs calls with the appropriate user role.
* fix: sync rulesSQL criteria after limitPercent resolution
The rulesSQL struct captures a copy of rules at creation time. When
limitPercent is resolved later, rules.Limit is updated but rulesSQL
still holds the stale value. This caused percentage-based smart playlist
limits to be silently ignored. Fix by updating rulesSQL.criteria after
the resolution.
* refactor: convert inList to a method on smartPlaylistCriteria
The inList function already receives ownerID and ownerIsAdmin from the
smartPlaylistCriteria caller. Making it a method lets it access those
fields directly from the receiver, simplifying the signature and staying
consistent with exprSQL which was already converted to a method.
* refactor: simplify function signatures by removing type parameters in criteria_sql.go
Signed-off-by: Deluan <deluan@navidrome.org>
---------
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: move criteria SQL generation to persistence
Keep model/criteria as a domain DSL with JSON parsing, field metadata, expression traversal, and child playlist extraction only. Move smart playlist SQL translation, sort SQL, and join planning into persistence behind smartPlaylistCriteria so repository code uses a small query-building API.
* refactor: simplify criteria translator metadata
Use generic helper functions for criteria operator maps so the SQL translator can pass named criteria map types directly. Remove unused pseudo-field metadata from the criteria field API while preserving special field name lookup.
* test: add coverage check for criteria-to-SQL field mappings
Add a test that iterates all fields registered in the criteria package and
verifies that every non-tag/non-role field has a corresponding entry in
the persistence layer's smartPlaylistFields map. This prevents silent
drift between the domain field registry and the SQL translation layer.
Also adds an AllFieldNames() function to the criteria package to support
field enumeration from outside the package.
* test: add smart playlist e2e suite infrastructure
* test: add string field smart playlist e2e tests
* test: add numeric, boolean, tag, participant, annotation, logic, sorting smart playlist e2e tests
* test: add playlist operator smart playlist e2e tests
* test: add isNot and endsWith string field e2e tests
* test: add date/time field smart playlist e2e tests
* fix: add gosec nolint directives for safe SQL concatenation in e2e restore
* refactor: address code review feedback for smart playlist e2e tests
- Deduplicate evaluateRule by delegating to evaluateRuleOrdered
- Cache table list in BeforeSuite instead of querying sqlite_master per test
- Wrap restoreDB in a transaction with defer cleanup for DETACH/foreign_keys
- Use JSON numbers for numeric criteria values to match canonical JSON shape
* refactor: simplify e2e test infrastructure
- Remove unused return value from buildTestFS
- Add deferred ROLLBACK as safety net in restoreDB transaction
- Cache Come Together ID to avoid repeated lookups in BeforeSuite
- Use range-over-int for play count loop
* test: add missing operator coverage to smart playlist e2e tests
Add 4 tests for operators/paths that had no e2e coverage:
- notContains on string fields (LIKE negation path)
- before on date fields (Lt for dates, only after was tested)
- startsWith on tag fields (json_tree + LIKE subquery)
- endsWith on participant/role fields (json_tree + LIKE subquery)
