* 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)
* fix: split html sanitization from plaintext handling
Add a dedicated SanitizeHTML helper for HTML-rendered values so entity-encoded markup is decoded before bluemonday sanitization. Use the new helper for the login welcome message and artist biographies while preserving SanitizeText semantics for lyrics and other plaintext callers. Add regression coverage for both helpers and the serveIndex welcomeMessage path.
* docs: add SanitizeText and SanitizeHTML godoc
Signed-off-by: Deluan <deluan@navidrome.org>
* fix: preserve plain text in artist biographies
Revert artist biography storage to SanitizeText so entity-encoded plain text remains decoded for Subsonic consumers. This avoids double-escaping values like R&B in XML responses while keeping the new welcomeMessage HTML sanitization in place, and adds a regression test covering the biography storage behavior.
---------
Signed-off-by: Deluan <deluan@navidrome.org>
* feat(config): Add EnforceNonRootUser config option to exit early if started as root
Signed-off-by: Aengus Walton <ventolin@gmail.com>
* Move validateEnforceNonRootUser check to directly after parsing the config
* Ensure the data directory hasn't been created in test
---------
Signed-off-by: Aengus Walton <ventolin@gmail.com>
Co-authored-by: Deluan Quintão <deluan@navidrome.org>
* ci(windows): add skeleton go-windows job (compile-only smoke test)
* ci(windows): fix comment to reference Task 7 not Task 6
* ci(windows): harden PATH visibility and set explicit bash shell
* ci(windows): enable full go test suite and ndpgen check
* test(gotaglib): skip Unix-only permission tests on Windows
* test(lyrics): skip Windows-incompatible tests
* test(utils): skip Windows-incompatible tests
* test(mpv): skip Windows-incompatible playback tests
Skip 3 subprocess-execution tests that rely on Unix-style mpv
invocation; .bat output includes \r-terminated lines that break
argument parsing (#TBD-mpv-windows).
* test(storage): skip Windows-incompatible tests
Skip relative-path test where filepath.Join uses backslash but the
storage implementation returns a forward-slash URL path
(#TBD-path-sep-storage).
* test(storage/local): skip Windows-incompatible tests
Skip 13 tests that fail because url.Parse("file://" + windowsPath)
treats the drive letter colon as an invalid port; also skip the
Windows drive-letter path test that exposes a backslash vs
forward-slash normalisation bug (#TBD-path-sep-storage-local).
* test(playlists): skip Windows-incompatible tests
* test(model): skip Windows-incompatible tests
* test(model/metadata): skip Windows-incompatible tests
* test(core): skip Windows-incompatible tests
AbsolutePath uses filepath.Join which produces OS-native path separators;
skip the assertion test on Windows until the production code is fixed
(#TBD-path-sep-core).
* test(artwork): skip Windows-incompatible tests
Artwork readers produce OS-native path separators on Windows while tests
assert forward-slash paths; skip 11 affected tests pending a fix in
production code (#TBD-path-sep-artwork).
* test(persistence): skip Windows-incompatible tests
Skip flaky timestamp comparison (#TBD-flake-persistence) and path-separator
real-bugs (#TBD-path-sep-persistence) in FolderRepository.GetFolderUpdateInfo
which uses filepath.Clean/os.PathSeparator converting stored forward-slash paths
to backslashes on Windows.
* test(scanner): skip Windows-incompatible tests
Skip symlink tests (Unix-assumption), ndignore path-separator bugs
(#TBD-path-sep-scanner) in processLibraryEvents/resolveFolderPath where
filepath.Rel/filepath.Split return backslash paths incompatible with fs.FS
forward-slash expectations, error message mismatch on Windows, and file
format upgrade detection (#TBD-path-sep-scanner).
* test(plugins): skip Windows-incompatible tests
Add //go:build !windows tags to test files that reference the suite
bootstrap (testManager, testdataDir, createTestManager) which is only
compiled on non-Windows. Add a Windows-only suite stub that skips all
specs via BeforeEach to prevent [build failed] on Windows CI.
* test(server): skip Windows-incompatible tests
Skip createUnixSocketFile tests that rely on Unix file permission bits
(chmod/fchmod) which are not supported on Windows.
* test(nativeapi): skip Windows-incompatible tests
Skip the i18n JSON validation test that uses filepath.Join to build
embedded-FS paths; filepath.Join produces backslashes on Windows which
breaks fs.Open (embedded FS always uses forward slashes).
* test(e2e): skip Windows-incompatible tests
On Windows, SQLite holds file locks that prevent the Ginkgo TempDir
DeferCleanup from deleting the DB file. Register an explicit db.Close
DeferCleanup (LIFO before TempDir cleanup) on Windows so the file lock
is released before the temp directory is removed.
* test(windows): fix e2e AfterSuite and skip remaining scanner path test
* test(scanner): skip another Windows path-sep test (#TBD-path-sep-scanner)
* test(subsonic): skip timing-flaky test on Windows (#TBD-flake-time-resolution-subsonic)
* test(scanner): skip 'detects file moved to different folder' on Windows
* test(scanner): consolidate 'Library changes' Windows skips into BeforeEach
* test(scanner): close DB before TempDir cleanup to fix Windows file lock
* test(scanner): skip ScanFolders suite on Windows instead of closing shared DB
* ci: retrigger for Windows soak run 2/3
* ci: retrigger for Windows soak run 3/3
* ci: retrigger for Windows soak run 3/3 (take 2)
* test(scanner): skip Multi-Library suite on Windows (SQLite file lock)
* ci(windows): promote go-windows to blocking status check
* test(plugins): run platform-neutral specs on Windows, drop blanket Skip
* test(windows): make tests cross-platform instead of skipping
- subsonic: back-date submissionTime baseline by 1s so
BeTemporally(">") passes under millisecond clock resolution
- persistence: sleep briefly between Put calls so UpdatedAt is
strictly after CreatedAt on low-resolution clocks
- utils/files: close tempFile before os.Remove so the test works on
Windows (where an open handle holds a file lock)
- tests.TempFile: close the handle before returning; metadata tests
no longer leak the open file into Ginkgo's TempDir cleanup
Resolves Copilot review comments on #5380.
* test(tests): add SkipOnWindows helper to reduce boilerplate
Introduces tests.SkipOnWindows(reason) that wraps the 3-line
runtime.GOOS guard pattern used in every Windows-skipped spec.
* test(adapters): use tests.SkipOnWindows helper
* test(core): use tests.SkipOnWindows helper
* test(model): use tests.SkipOnWindows helper
* test(persistence): use tests.SkipOnWindows helper
* test(scanner): use tests.SkipOnWindows helper
* test(server): use tests.SkipOnWindows helper
* test(plugins): run pure-Go unit tests on Windows
config_validation_test, manager_loader_test, and migrate_test have no
WASM/exec dependencies and don't rely on the make-built test plugins
from plugins_suite_test.go. Let them run on Windows too.
* fix(artwork): return imagesUpdatedAt in LastUpdated when cover art changes
When cover art (cover.jpg) is updated in an album folder, the HTTP
Last-Modified header was incorrectly returning album.UpdatedAt (which
only tracks media file changes) instead of imagesUpdatedAt (which
tracks cover art changes).
This caused browsers to use their cached cover art because the
Last-Modified header didn't change, even though the actual cover art
image data was new (due to cache key changing based on imagesUpdatedAt).
The fix ensures LastUpdated() returns a.lastUpdate (which is the max of
album.UpdatedAt and imagesUpdatedAt) instead of always returning
album.UpdatedAt.
Fixesnavidrome/navidrome#5377
* refactor tests
Signed-off-by: Deluan <deluan@navidrome.org>
* fix(artwork): return imagesUpdatedAt in disc LastUpdated
The discArtworkReader had the same bug as albumArtworkReader (fixed in
9a741859f): LastUpdated() returned album.UpdatedAt while Key() used the
max of album.UpdatedAt and ImagesUpdatedAt. This mismatch caused browsers
to keep stale disc cover art in cache when only the image file changed.
Also strengthen the album LastUpdated tests and add matching tests for
the disc reader. The tests use DescribeTable and were verified to fail
when the fix is reverted.
---------
Signed-off-by: Deluan <deluan@navidrome.org>
Co-authored-by: Deluan <deluan@navidrome.org>
Bump the main module, Dockerfile build stages, and devcontainer to Go
1.26.0. Plugin sub-modules under plugins/ remain on go 1.25 intentionally
(independent modules, untouched in this change).
Also add an explicit actions/setup-go@v6 step (with go-version-file:
go.mod) to the go-lint and go jobs in the CI pipeline. This matches the
golangci-lint-action v4+ requirement that setup-go run before the linter,
and pins the runner Go version to go.mod so CI does not depend on the
ubuntu-latest tools cache picking up Go 1.26.
* feat(docker): add musl build stage for native libwebp support
Add a new build-alpine stage using Alpine/musl with xx cross-compilation,
producing a dynamically-linked musl binary for the Docker image. The
runtime image now installs libwebp, libwebpdemux, and libwebpmux and
creates .so symlinks so gen2brain/webp can detect native libwebp via
purego/dlopen at startup and use it automatically.
The existing Debian/glibc 'build' stage is kept for standalone binary
distribution (darwin, windows, and glibc linux binaries); the Docker
image now ships the musl build from build-alpine instead.
* fix(docker): use dynamic symlinks for libwebp libraries
Avoid hardcoding SONAME versions (.so.7, .so.2, .so.3) which break
on Alpine version bumps. Also fix misleading comment: the musl build
is dynamic (required for purego dlopen), not static.
* feat(docker): enable WebP encoding in Docker environment
Signed-off-by: Deluan <deluan@navidrome.org>
* fix(docker): pin build-alpine stage to Go 1.25 to match base stage
Align the new build-alpine stage with the existing glibc 'base' stage,
both pinned to Go 1.25. Bumping build-alpine independently would create
a version skew between the Docker image binary and the standalone
binaries, which should be avoided unless there is a specific reason.
* fix(docker): harden build-alpine stage (musl pin, -latomic, dynamic-link check)
Address review feedback on the build-alpine stage:
- Pin Go builder to golang:1.25-alpine3.20 so the musl version used at
build time matches the alpine:3.20 runtime image, eliminating any
potential musl ABI skew between builder and runtime.
- Add -extldflags '-latomic' so SQLite's 64-bit atomics resolve when
cross-compiling for 32-bit arm targets (arm/v6, arm/v7).
- Add a build-time check that the produced binary is dynamically
linked (using 'file' from Alpine), failing the build if it is not.
A fully-static binary cannot dlopen libwebp and would silently fall
back to the WASM encoder, defeating the whole point of this stage.
* fix(docker): revert to unpinned golang:1.25-alpine builder
The golang:1.25-alpine3.20 tag suggested during review does not exist on
public.ecr.aws (only 3.21, 3.22, 3.23, and unpinned 'alpine' are
published). Revert to the unpinned 'golang:1.25-alpine' tag so the
Docker build can resolve the base image.
This means the builder's Alpine version can drift relative to the
alpine:3.20 runtime, but in practice musl's backward compatibility
covers this for Navidrome's small dlopen surface (a few libwebp
symbols, no direct libc calls from the dlopen path). If a skew ever
manifests, we can pin both builder and runtime to the same specific
Alpine release in a follow-up.
---------
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor(build): remove CPP taglib adapter
Remove the CGO-based TagLib adapter (adapters/taglib/) and all
cross-taglib build infrastructure. The WASM-based go-taglib adapter
(adapters/gotaglib/) is now the sole metadata extractor.
- Delete adapters/taglib/ (CPP/CGO wrapper)
- Delete .github/actions/download-taglib/
- Remove CROSS_TAGLIB_VERSION, CGO_CFLAGS_ALLOW, and all taglib-related
references from Dockerfile, Makefile, CI pipeline, and devcontainer
* fix(scanner): gracefully fallback to default extractor instead of crashing
Replace log.Fatal with a graceful fallback when the configured scanner
extractor is not found. Instead of terminating the process, the code now
warns and falls back to the default taglib extractor using the existing
consts.DefaultScannerExtractor constant. A fatal log is retained only for
the case where the default extractor itself is not registered, which
indicates a broken build.
* test(scanner): cover default extractor fallback and suppress redundant warn
Address review feedback on the extractor fallback in newLocalStorage:
- Only log the "using default" warning when the configured extractor
differs from the default, so a broken build (default extractor itself
missing) logs only the fatal — not a misleading "falling back" warn
followed immediately by the fatal.
- Add a unit test that registers a mock under consts.DefaultScannerExtractor,
sets the configured extractor to an unknown name, and asserts the local
storage is constructed using the default extractor's constructor.
* refactor: extract matchSongsToLibrary to core/matcher package
Move the song-to-library matching algorithm from core/external into its own
core/matcher package. The Matcher struct exposes a single public method
MatchSongsToLibrary that implements a multi-phase matching algorithm
(ID > MBID > ISRC > fuzzy title+artist). Includes pre-sanitization
optimization for the fuzzy matching loop.
No behavioral changes — the algorithm is identical to the version in
core/external/provider_matching.go.
* refactor: inject matcher.Matcher via Wire instead of creating it inline
Add *matcher.Matcher as a dependency of external.NewProvider, wired via
Google Wire. Update all provider test files to pass matcher.New(ds).
This eliminates tight coupling so future consumers can reuse the matcher
without depending on the external package.
* refactor: remove old provider_matching files
Delete core/external/provider_matching.go and its tests. All matching
logic now lives in core/matcher/.
* test(matcher): restore test coverage lost in extraction
Port back 23 specs that existed in the old provider_matching_test.go
but were dropped during the extraction. Covers specificity levels,
fuzzy matching thresholds, fuzzy album matching, duration matching,
and deduplication edge cases.
* test(matcher): extract matchFieldInAnd/matchFieldInEq helpers
The four inline mock.MatchedBy closures in setupAllPhaseExpectations
all followed the same squirrel.And -> squirrel.Eq -> field-name-check
pattern. Extract into two small helpers to reduce duplication and
make the setup functions read as a concise list of phase expectations.
* refactor(matcher): address PR #5348 review feedback
- sanitizedTrack now holds *model.MediaFile instead of a value copy. Since
MediaFile is a large struct (~74 fields), this avoids the per-track copy
into sanitized[] and a second copy when findBestMatch assigns the winner.
loadTracksByTitleAndArtist updated to iterate by index and pass &tracks[i].
- loadTracksByISRC now sorts results (starred desc, rating desc, year asc,
compilation asc) so that when multiple library tracks share an ISRC the
most relevant one is picked deterministically, matching the sort order
already used by loadTracksByTitleAndArtist.
- Restored the four worked examples (MBID Priority, ISRC Priority,
Specificity Ranking, Fuzzy Title Matching) in the MatchSongsToLibrary
godoc that were dropped during the extraction.
- matcher_test.go: tests now enforce expectations via AssertExpectations in
a DeferCleanup. The old setupAllPhaseExpectations helper was replaced
with per-phase helpers (expectIDPhase/expectMBIDPhase/expectISRCPhase +
allowOtherPhases) so each test deterministically verifies which matching
phases fire. This surfaced (and fixes) a latent issue copilot flagged:
the old .Once() expectations were not actually asserted, so tests would
silently pass even when phases short-circuited unexpectedly.
* feat(config): make max image upload size configurable
Let max image upload size be set from config or environment instead of a fixed 10 MB cap. The upload handler still falls back to 10 MB when MaxImageUploadSize is not set.
Signed-off-by: M8te <38794725+m8tec@users.noreply.github.com>
* feat(config): support human-readable MaxImageUploadSize values
Max image upload size can now be configured as a readable string like 10MB or 1GB instead of raw bytes. The config load validates it at startup, and the upload handler parses it before applying request limits (10MB fallback if it fails).
+ MaxImageUploadSize as human-readable string
+ removed redundant max(1, ...) to address code review
+ cap memory usage of ParseMultipartForm to 10MB (address code review)
Signed-off-by: M8te <38794725+m8tec@users.noreply.github.com>
* refactor(config): consolidate MaxImageUploadSize default and add tests
Move the "10MB" default constant to consts.DefaultMaxImageUploadSize so
both the viper default and the runtime fallback share a single source of
truth. Improve the validator error message with fmt.Errorf wrapping to
match the project convention (e.g. validatePurgeMissingOption). Add unit
tests for validateMaxImageUploadSize (valid/invalid inputs) and
maxImageUploadSize (configured, empty, invalid, raw bytes). Compute
maxImageSize once at handler creation rather than per request.
---------
Signed-off-by: M8te <38794725+m8tec@users.noreply.github.com>
Co-authored-by: Deluan Quintão <deluan@navidrome.org>
* fix(transcoding): clamp target channels to codec limit (#5336)
When transcoding a multi-channel source (e.g. 6-channel FLAC) to MP3, the
decider passed the source channel count through to ffmpeg unchanged. The
default MP3 command path then emitted `-ac 6`, and the template path injected
`-ac 6` after the template's own `-ac 2`, causing ffmpeg to honor the last
occurrence and fail with exit code 234 since libmp3lame only supports up to
2 channels.
Introduce `codecMaxChannels()` in core/stream/codec.go (mp3→2, opus→8),
mirroring the existing `codecMaxSampleRate` pattern, and apply the clamp in
`computeTranscodedStream` right after the sample-rate clamps. Also fix a
pre-existing ordering bug where the profile's MaxAudioChannels check compared
against src.Channels rather than ts.Channels, which would have let a looser
profile setting raise the codec-clamped value back up. Comparing against the
already-clamped ts.Channels makes profile limits strictly narrowing, which
matches how the sample-rate block already behaves.
The ffmpeg buildTemplateArgs comment is refreshed to point at the new upstream
clamp, since the flags it injects are now always codec-safe.
Adds unit tests for codecMaxChannels and four decider scenarios covering the
literal issue repro (6-ch FLAC→MP3 clamps to 2), a stricter profile limit
winning over the codec clamp, a looser profile limit leaving the codec clamp
intact, and a codec with no hard limit (AAC) passing 6 channels through.
* test(e2e): pin codec channel clamp at the Subsonic API surface (#5336)
Add a 6-channel FLAC fixture to the e2e test suite and use it to assert the
codec channel clamp end-to-end on both Subsonic streaming endpoints:
- getTranscodeDecision (mp3OnlyClient, no MaxAudioChannels in profile):
expects TranscodeStream.AudioChannels == 2 for the 6-channel source. This
exercises the new codecMaxChannels() helper through the OpenSubsonic
decision endpoint, with no profile-level channel limit masking the bug.
- /rest/stream (legacy): requests format=mp3 against the multichannel
fixture and asserts streamerSpy.LastRequest.Channels == 2, confirming
the clamp propagates through ResolveRequest into the stream.Request that
the streamer receives.
The fixture is metadata-only (channels: 6 plumbed via the existing
storagetest.File helper) — no real audio bytes required, since the e2e
suite uses a spy streamer rather than invoking ffmpeg. Bumps the empty-query
search3 song count expectation from 13 to 14 to account for the new fixture.
* test(decider): clarify codec-clamp comment terminology
Distinguish "transcoding profile MaxAudioChannels" (Profile.MaxAudioChannels
field) from "LimitationAudioChannels" (CodecProfile rule constant). The
regression test bypasses the former, not the latter.
* test(artwork): expect shared disc art for unnumbered filenames in single-folder albums
* fix(artwork): match unnumbered disc art for every disc in single-folder albums
* test(artwork): verify shared disc art resolves for every disc number
* test(artwork): regression guard for numbered disc filter with mixed filenames
* test(artwork): verify DiscArtPriority order decides numbered vs shared disc art
* test(artwork): strengthen regression guard to exercise both disc art branches
* refactor(artwork): simplify disc art matching and drop redundant comments
- Lowercase the pattern and filename once in fromExternalFile and pass
lowered values into extractDiscNumber, eliminating the duplicate
strings.ToLower calls inside that helper.
- Drop narrating comments in reader_disc.go and reader_disc_test.go that
duplicated information already conveyed by nearby code or doc comments.
* fix(artwork): prefer numbered disc art over shared fallback within a pattern
Review feedback: with files [disc.jpg, disc1.jpg, disc2.jpg] in a single
folder, the previous single-folder fall-through returned the first match
in imgFiles order. Because compareImageFiles sorts 'disc' before 'disc1'
and 'disc2', disc.jpg would mask the per-disc numbered files for every
disc, regressing the behavior from before the shared-disc-art change.
Within a single pattern the loop now records the first viable unnumbered
candidate as a fallback and keeps scanning for a numbered match equal to
the target disc. Numbered matches still win immediately; the shared file
is only returned when no numbered match for the target disc exists.
Also drops the redundant strings.ToLower(pattern) at the top of
fromExternalFile; fromDiscArtPriority already lowercases the whole
priority string before splitting, so the function contract is now
'pattern must be lowercase' (documented on the function).
* refactor(artwork): trim disc art matching comments and table-drive tests
Doc comment on fromExternalFile is trimmed to the one non-obvious
contract (caller must pre-lowercase the pattern) plus the headline
behavior; the bulleted restatement of the branch logic went away.
Two inline comments that narrated what the code already shows are
also gone.
Hoisting a `hasWildcard := strings.ContainsRune(pattern, '*')` check
out of the loop avoids per-iteration extractDiscNumber calls for
literal patterns (e.g. `shellac.png`) and lets the loop break as
soon as a viable fallback is found, since literal patterns can never
be beaten by a numbered match. Wildcard patterns keep the original
scan-to-end-for-numbered-match behavior.
The two regression tests added in the previous commit were
structurally identical apart from discNumber/expected, so they are
collapsed into a DescribeTable with two entries — matching the
existing table style used for extractDiscNumber tests in the same
file.
* fix(artwork): support '?' and '[...]' wildcards in disc art patterns
filepath.Match understands three glob metacharacters ('*', '?', '[')
but extractDiscNumber only looked for '*'. A pattern like 'disc?.jpg'
or 'cd[12].jpg' would therefore be treated as unnumbered, and every
disc of a multi-disc album would resolve to the same (first-sorted)
file instead of the per-disc numbered art.
extractDiscNumber now finds the literal prefix of the pattern by
scanning for the first '*', '?', or '[' (via strings.IndexAny),
strips it from the filename, and parses the leading digits that
follow. The standalone filepath.Match check is dropped; HasPrefix
plus the leading-digits requirement is enough to reject non-matches,
and the caller already verifies the glob match before calling.
fromExternalFile's literal-pattern optimization is widened
correspondingly: a pattern is treated as literal only when it
contains none of '*', '?', '['. Any wildcard form now keeps the
scan-to-end behavior so a numbered match can beat a fallback.
Adds table entries for both the extractDiscNumber parser and the
fromExternalFile higher-level behavior, covering '?' and '[...]'
patterns as well as a literal-pattern baseline.
* refactor(artwork): tidy extractDiscNumber after glob-wildcard support
- Name the '*?[' charset as globMetaChars, used by both extractDiscNumber
and fromExternalFile so the two call sites can't drift.
- Trim the extractDiscNumber doc comment: keep the non-obvious caller
contract, drop the algorithm narration.
- Replace the byte-slice digit accumulator with a direct filename slice
fed to strconv.Atoi.
- Rename the four new non-'*' wildcard Entry descriptions so they read
like the existing extractDiscNumber table ('pattern, target → expected')
instead of the ambiguous 'disc 1' shorthand.
* fix(artwork): retry remaining fallbacks when the first one fails to open
Review feedback: the previous shape remembered only the first unnumbered
candidate and fell through to a generic error if os.Open failed on it,
even though other matching unnumbered files in imgFiles could have
succeeded. The pre-PR code was more resilient because it looped and
continued on open failure.
fromExternalFile now collects every viable unnumbered candidate into a
slice during the scan, then tries them in order after the loop, mirroring
the pre-PR retry-on-open-failure behavior. Numbered matches still return
immediately on first success and skip the candidate list entirely — an
open failure on a numbered match means no other file has that number
anyway.
Also:
- globMetaChars doc comment now notes that '\' escape is intentionally
excluded (filepath.Match supports it but treating it as a metachar here
would misalign extractDiscNumber's literal-prefix extraction with no
benefit for realistic config patterns).
- The 'cover.jpg doesn't match disc*.*' Entry in the extractDiscNumber
table is renamed to 'cover.jpg with disc*.* (no prefix match)' to
reflect that the test now exercises the HasPrefix defensive guard,
not the removed internal filepath.Match check.
Regression test added: a single-folder album with a deleted first
candidate file resolves to the second candidate.
* fix(artwork): scan all literal-pattern matches so fallback retry works
Review feedback: the 'break on first literal match' optimization
assumed only one file in imgFiles could match a literal basename,
but filepath.Match compares basenames only — multiple folders can
contribute files with the same basename, and the fallback-list retry
in 5d79f751c is defeated if the loop breaks after recording just
the first one.
Removing the break makes literal and wildcard patterns follow the
same scan-to-end path, preserving the retry-on-open-failure
resilience regained in 5d79f751c. The efficiency cost is negligible
— imgFiles is 5-20 entries per album and this is a cache-miss path.
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
* perf(subsonic): keep album/mediafile params on stack in response helpers
Two helpers were forcing their entire value parameter onto the heap via
pointer-to-field aliasing, adding one full-struct heap allocation per
response item on hot Subsonic endpoints (search3, getAlbumList2, etc.).
- childFromMediaFile assigned &mf.BirthTime to the returned Child,
pulling the whole ~1KB model.MediaFile to the heap on every call.
- buildDiscSubtitles passed &a.UpdatedAt to NewArtworkID inside a loop,
pulling the whole model.Album to the heap on every album with discs.
Both now copy the time.Time to a stack-local and use gg.P / &local so
only the small time.Time escapes. Verified via go build -gcflags=-m=2:
moved to heap: mf and moved to heap: a are gone at these sites.
* perf(metadata): avoid per-track closure allocations in PID computation
createGetPID was a factory that returned nested closures capturing
mf model.MediaFile (~992 bytes) by reference. Since it is called three
times per track during scans (trackPID, albumID, artistID), every track
triggered the allocation of three closures plus a heap copy of the full
MediaFile.
Refactor the body into package-level functions (computePID, getPIDAttr)
that take hash as an explicit parameter and the inner slice.Map callback
to an indexed for loop, removing the closure-capture of mf entirely.
trackPID/albumID/artistID now call computePID directly.
The tiny createGetPID wrapper was kept only for tests; move the
closure-building into the test file so production has no dead API.
Verified via go build -gcflags=-m=2 on model/metadata: no
"moved to heap: mf" anywhere in persistent_ids.go, and the callers in
map_mediafile.go / map_participants.go no longer heap-promote their
MediaFile argument.
* 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>
* fix(msi): include ffprobe executable in MSI build
Signed-off-by: Deluan <deluan@navidrome.org>
* feat(ffmpeg): add IsProbeAvailable() to FFmpeg interface
Add runtime check for ffprobe binary availability with cached result
and startup logging. When ffprobe is missing, logs a warning at startup.
* feat(stream): guard MakeDecision behind ffprobe availability
When ffprobe is not available, MakeDecision returns a decision with
ErrorReason set and both CanDirectPlay and CanTranscode false, instead
of failing with an opaque exec error.
* feat(subsonic): only advertise transcoding extension when ffprobe is available
The OpenSubsonic transcoding extension is now conditionally included
based on ffprobe availability, so clients know not to call
getTranscodeDecision when ffprobe is missing.
* refactor(ffmpeg): move ffprobe startup warning to initial_setup
Move the ffprobe availability warning from the lazy IsProbeAvailable()
check to checkFFmpegInstallation() in server/initial_setup.go, alongside
the existing ffmpeg warning. This ensures the warning appears at startup
rather than on first endpoint call.
* fix(e2e): set noopFFmpeg.IsProbeAvailable to true
The e2e tests use pre-populated probe data and don't need a real ffprobe
binary. Setting IsProbeAvailable to true allows the transcode decision
logic to proceed normally in e2e tests.
* fix(stream): only guard on ffprobe when probing is needed
Move the IsProbeAvailable() guard inside the SkipProbe check so that
legacy stream requests (which pass SkipProbe: true) are not blocked
when ffprobe is missing. The guard only applies when probing is
actually required (i.e., getTranscodeDecision endpoint).
* refactor(stream): fall back to tag metadata when ffprobe is unavailable
Instead of blocking getTranscodeDecision when ffprobe is missing,
fall back to tag-based metadata (same behavior as /rest/stream).
The transcoding extension is always advertised. A startup warning
still alerts admins when ffprobe is not found.
* fix(stream): downgrade ffprobe-unavailable log to Debug
Avoids log spam when clients call getTranscodeDecision repeatedly
without ffprobe installed. The startup warning in initial_setup.go
already alerts admins at Warn level.
---------
Signed-off-by: Deluan <deluan@navidrome.org>
The cleanupLoop goroutine could execute cleanupExpired against a closed
database because Close() did not wait for the goroutine to exit before
calling db.Close(). This caused 'sql: database is closed' errors during
plugin unload or shutdown.
Close() now cancels the cleanup goroutine's context and waits for it to
finish via a sync.WaitGroup before running the final cleanup and closing
the database.
Signed-off-by: Deluan <deluan@navidrome.org>
The Squiddies Glass theme applies a CSS color filter to all images inside
table cells (MuiTableCell '& img'), which was intended for small playback
indicator icons. This inadvertently also applied to disc cover art
thumbnails in multi-disc album views, turning them into solid color
blocks. Adding 'filter: none !important' to the discCoverArt style
ensures cover art images are always displayed correctly regardless of
the active theme.
Signed-off-by: Deluan <deluan@navidrome.org>
