The "main"/"translation"/"pronunciation" kind values were redefined in
three places (ttmlLyricKind* in the TTML parser, lyricsfileKindMain in the
Lyricsfile parser, and inline literals in the subsonic response builder).
Define them once as model.LyricKind* (kept as wire-compatible string values)
plus a LyricKindOrMain helper, and use them everywhere a kind is produced,
defaulted, or filtered. No wire-facing types change.
buildLyricCues (subsonic response) reimplemented the same fill-from-next /
fallback / all-or-none end-time logic that normalizeCueLine (model) already
ran at parse time. They operate on different groupings -- the response
builder normalizes per-agent cue subsets, the model the whole line -- so they
cannot be collapsed, but the algorithm itself was duplicated.
Extract model.NormalizeCueEnds([]Cue, fallbackEnd) as the single
implementation (now also clamping ends within the model line normalization),
and have buildLyricCues delegate to it and only map to the response type.
mapLyrics() used a dedicated rawPairs()/rawTags path solely to bypass the
32KB lyrics truncation cap (the comment claiming sanitization "strips XML"
was inaccurate -- pair-type tags are only truncated, never stripped). That
duplicated every file s tags into a second lowercased map on the scan hot
path.
Instead, raise the lyrics maxLength so the normal Pairs() path keeps large
word-timed TTML/Enhanced-LRC intact, and remove rawTags/rawPairs. The
existing ">32KB embedded TTML" test continues to pass through Pairs().
Relocates the TTML, SRT, and embedded-dispatch parsers (pure text ->
model.LyricList transforms) into package model, alongside the existing
ToLyrics (LRC/ELRC) and ParseLyricsfile parsers.
This removes the model/metadata -> core/lyrics import edge (a lower layer
depending on business logic), which was the only reason model imported
core. core/lyrics now contains only the source-priority + plugin service,
which is consumed solely by server/subsonic.
parseTTML/parseSRT are exported as ParseTTML/ParseSRT so the remaining
core/lyrics service can still call them across the package boundary.
getRandomSongs used a single ORDER BY random() over the media_file table. At large
library sizes that forces SQLite to scan the wide table and sort every matching row
before applying the limit — roughly 4 seconds for a 1M-track library, regardless of how
many songs are requested.
Add MediaFileRepository.GetRandom, which does this in two passes: first select N random
rowids over a narrow index (filters + library scope only, no wide columns or joins), so
the random sort runs over compact, index-friendly data; then hydrate just those rows with
the full select. The wide media_file row is never part of the sort. End-to-end this drops
getRandomSongs from ~4s to ~0.3s on a 1M-track library, and the cost no longer grows with
the requested size.
The handler now calls GetRandom directly. The filter helper that builds the genre/year
filters is renamed SongsByRandom -> SongsByGenreAndYearRange to reflect what it does, since
the random ordering is now owned by GetRandom rather than a Sort option. Filters (genre,
year, library) compose into the first pass unchanged. The album random list path is left
as-is (far fewer rows, already fast).
* fix(scanner): import playlists skipped when no admin existed yet (#5499)
On a fresh install the first scan runs before any admin user exists, so the
scanner's playlist phase skips all playlists (playlists are owned by the
first admin). Nothing re-imported them afterwards because folder selection
is gated on updated_at > last_scan_at, which nothing bumps.
The playlist phase now:
- resolves the admin at phase time (FindFirstAdmin) instead of trusting the
context snapshot taken at scan start, so a long admin-less scan still
imports playlists in its own phase if an admin was created meanwhile;
- records a persisted PlaylistsImportPending flag when no admin exists yet;
- when that flag is set, imports ALL playlist folders via a new
GetAllWithPlaylists (bypassing the timestamp gate) and clears the flag.
Playlists are recovered by the next scan that runs with an admin, with no
dependency on scan duration and no changes to the auth/server layers.
* fix(scanner): surface datastore errors in playlist import deferral (#5499)
Address review feedback:
- distinguish model.ErrNotFound (no admin yet -> defer) from real datastore
errors when resolving the admin, so DB failures are propagated, not swallowed;
- propagate the error if the pending-import flag can't be persisted, so a scan
doesn't complete as successful without recording the recovery;
- surface read errors when checking the pending flag.
Also name the no-admin condition for readability.
* fix(scanner): simplify admin existence check in playlist import
Signed-off-by: Deluan <deluan@navidrome.org>
* fix(scanner): streamline folder access in playlist import logic
Signed-off-by: Deluan <deluan@navidrome.org>
---------
Signed-off-by: Deluan <deluan@navidrome.org>
* test(artwork): add failing e2e tests for artist image leaking as album art
Reproduces a v0.62.0 regression (#5451/#5457): the album cover-art
parent-folder fallback can include the artist folder, serving the artist
thumbnail (e.g. Artist/folder.jpg) as album art for any album without
image files in its own folder(s). Covers three scenarios: a plain
Artist/Album layout with no album images, a single-disc album spread
across sibling folders under the artist folder, and a spread album whose
own front.jpg is shadowed by the artist's cover.jpg via CoverArtPriority
order. Also adds an albumByName test helper for multi-album layouts.
The tests are expected to fail until the parent-folder inclusion is
gated by a structural check (skip the common parent when audio from
other albums lives under it).
* fix(artwork): never serve artist folder images as album art
The album cover-art parent-folder fallback (introduced in #5451/#5457)
could include the artist folder as a source of album images, serving the
artist thumbnail (e.g. Artist/folder.jpg) as cover art for any album
without image files in its own folder(s). This affected both plain
Artist/Album layouts and single-disc albums spread across sibling
folders under the artist folder.
Gate the common-parent inclusion with a structural check: the parent
only qualifies as an album root when no audio belonging to other albums
lives in it or anywhere beneath it. An artist folder contains other
albums' tracks, while an album root above disc subfolders contains only
this album's, so the check works for any disc folder naming scheme and
never affects the multi-disc fixes from #5376/#5456. A single-album
artist with no images anywhere remains structurally indistinguishable
from an album root and is a known residual case.
* refactor(artwork): move album-root audio check into folder repository
Replace the raw subtree SQL (LIKE/ESCAPE expression and wildcard
escaping) that lived in core/artwork with an explicit
FolderRepository.HasAudioOutsideFolders method, implemented in the
persistence layer next to the existing folder-subtree query pattern.
This also removes the test mock's brittle dispatch that sniffed the
generated SQL to recognize the query; the fake now overrides the new
method directly.
Extract the whole parent-folder resolution from loadAlbumFoldersPaths
into an albumRootParent helper, flattening four levels of nesting back
into a linear flow. Behavior is unchanged; the unit test for a parent
containing audio moved to the persistence suite, with added coverage
for subtree boundaries, missing folders, and LIKE-wildcard escaping in
folder paths.
* refactor(persistence): use exists helper in HasAudioOutsideFolders
Replace the hand-rolled count(*) query with the repository's canonical
exists helper, as suggested in PR review.
* feat(smartplaylist): support isMissing/isPresent on mbz_* and lyrics fields
Mark the six mbz_* MusicBrainz ID columns and the lyrics column as Nullable
in the criteria field map, then extend missingExpr to handle string columns
where absence is encoded as NULL or empty string (plus '[]' for lyrics). The
Numeric/Boolean path (ReplayGain) is preserved via an explicit type check.
* refactor(model): make MediaFile BPM and BitDepth nullable pointers
Convert BPM and BitDepth fields in model.MediaFile from int to *int so that
'tag absent' is distinguishable from zero. The metadata mapper now uses
NullableFloat for BPM (nil when absent or zero/unparseable) and only sets
BitDepth when the audio property is non-zero (lossy codecs report 0).
All read sites use gg.V() for zero-fallback deref so Subsonic API output and
transcoding behaviour are byte-identical to before. The persistence layer
bridges the existing NOT NULL DB columns by coercing nil to 0 on write and 0
back to nil on read in PostMapArgs/PostScan; a later migration task will drop
those constraints.
Hash upgrade safety is verified by a new MediaFile.Hash describe block: nil
*int hashes identically to the old int(0) default via ZeroNil+IgnoreZeroValue,
so no files will be spuriously re-imported after this change.
Extra files touched beyond the plan's list: core/stream/legacy_client_test.go
(BitDepth in model.MediaFile literals), persistence/mediafile_repository.go
(NOT NULL bridge).
* test(model): pin pre-conversion golden hashes for BPM/BitDepth
* feat(smartplaylist): support isMissing/isPresent on bpm and bitDepth
* feat(db): make bpm and bit_depth columns nullable, backfill 0 to NULL
Drop the NOT NULL constraint on media_file.bpm and bit_depth via a
lossless migration that converts legacy 0-means-absent values to real
NULL. Remove the temporary shim in PostScan/PostMapArgs that was bridging
the old NOT NULL columns to the *int model fields. Add round-trip
persistence tests asserting NULL storage for nil pointers and correct
value round-trip for non-nil pointers.
* test(e2e): verify isMissing/isPresent partition for nullable fields
Add DescribeTable covering bpm, bitdepth, lyrics, and mbz_recording_id:
for each field, isMissing + isPresent song counts must equal the total
library count, proving the nullable-column SQL is exhaustive and correct.
* test(e2e): seed bpm tag so isMissing/isPresent partition is non-trivial
* fix(model): omit bitDepth from JSON when absent instead of emitting null
* feat(smartplaylist): support isMissing/isPresent on more string fields
Enable isMissing/isPresent operators for album, comment, catalognumber,
discsubtitle, albumcomment, sorttitle, sortalbum, sortartist,
sortalbumartist, and explicitstatus by marking them Nullable in fieldMap.
* refactor(smartplaylist): unify missingExpr column logic into one flow
Collapse the numeric/string fork in missingExpr into a single
empties-driven loop (numeric/boolean fields simply have no empties),
and replace the duplicated IsTag/IsRole guard with a three-way switch
that expresses the dispatch model once. No SQL semantics change for
string fields; numeric/boolean fields now emit a single-element Or/And
which squirrel parenthesizes (e.g. `(col IS NULL)` instead of bare
`col IS NULL`) — update the affected test expectations accordingly.
* fix(smartplaylist): support isMissing/isPresent operators on ReplayGain fields
ReplayGain values are stored in dedicated nullable columns (rg_album_gain,
rg_album_peak, rg_track_gain, rg_track_peak) rather than in the media_file.tags
JSON blob. The isMissing/isPresent operators previously only supported tag and
role fields, causing two failure modes:
1. Using the documented alias names (replaygain_album_gain etc.) from PR #5256:
these got registered as JSON tags from mappings.yaml, so isMissing queried
json_tree(media_file.tags, '$.replaygain_album_gain') which is always empty
-> the playlist matched ALL songs.
2. Using the canonical field names (rgalbumgain etc.): not a tag/role, so SQL
generation returned an error. Because refreshSmartPlaylist deletes old tracks
before regenerating, the abort left the playlist empty.
Fix: add Nullable bool to FieldInfo and mark the four ReplayGain fields. Add
static alias entries (replaygain_album_gain -> rgalbumgain etc.) with
Numeric+Nullable set; because AddTagNames skips names already in the field map,
these static entries take precedence over the mappings.yaml tag registration.
missingExpr now emits IS NULL / IS NOT NULL for nullable column fields instead
of the json_tree lookup.
Fixes#5584
* chore(smartplaylist): address code review feedback
- Simplify the isMissing/isPresent unsupported-field error message,
removing the internal "nullable fields" jargon
- Standardize comments on mappings.yaml (the actual filename)
- Clarify the alias precedence comment in the LookupField test
* fix: split tag values from multiple sources individually
When a file has multiple tag frames mapping to the same logical tag
(e.g. both TXXX:MOOD and TMOO), TagLib merges them into one key with
multiple values. SplitTagValue had a len(values) != 1 guard that
skipped splitting entirely in this case, leaving comma-separated
values unsplit.
Change SplitTagValue to split each value individually regardless of
input count. Empty values are filtered during splitting.
Fixes#5065
* test: cover SplitTagValue with multi-frame regression cases
Add tests pinning the behavior fixed by SplitTagValue iterating over each
input value. The previous len(values) != 1 short-circuit silently skipped
splitting whenever TagLib merged multiple ID3v2 frames into the same
property (e.g. TMOO + TXXX:MOOD for mood, or duplicate TIPL entries for
composer), as reported in #5065.
Three layers of coverage:
- model/tag_mappings_test.go: direct unit tests on TagConf.SplitTagValue
covering single/multi-value input, case-insensitive separators, missing
SplitRx, empty input, and the empty-strings-passed-through contract that
the downstream metadata pipeline relies on.
- model/metadata/metadata_test.go: end-to-end check that a "mood" tag
surfaced as two raw values (the exact shape from the bug report) is
split, trimmed, and deduplicated to the expected three moods.
- model/metadata/map_participants_test.go: parallel multi-value case for
the COMPOSER tag, ensuring the same fix also corrects multi-frame role
parsing.
All three new specs fail on the pre-fix code and pass on the patched
SplitTagValue.
---------
Co-authored-by: Deluan Quintão <deluan@navidrome.org>
* 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>
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
* 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: 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: 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.
* 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.
- Rename token/tokenLine to cue/cueLine across Go backend and JS frontend
- Move role from individual cue to cueLine level (server pre-splits by role)
- Add enhanced query parameter to getLyricsBySongId for backward compat
- Add enhanced=true to UI API client so translations/pronunciations load
- Update all Go and JS tests to match new naming and structure
Add a full TTML (Timed Text Markup Language) sidecar lyrics parser that extracts
word/syllable-level timing from <span> elements, plus translation and pronunciation
(transliteration) tracks from Apple Music TTML metadata sections.
Backend changes:
- TTML parser (core/lyrics/ttml.go) with support for all TTML time formats,
nested timing contexts, and bare decimal second offsets
- Translation/pronunciation tracks resolved via key-based metadata linking
- Line timing hydration from token-level start/end values
- 'kind' field added to Lyrics model and StructuredLyric API response
(main/translation/pronunciation)
- 'tokenLine' array in API response for word-level timing data
- UTF-8 BOM and UTF-16 LE encoding support for TTML files
- Fix for ambiguous time resolution in pronunciation spans (pre-1-minute)
Frontend changes:
- KaraokeLyricsOverlay rewritten with scrollable multi-line layout,
word-level wipe highlighting with eased alpha transitions,
rAF-driven playback clock with drift correction
- Inline translation (above) and pronunciation (below) each main line,
with smart filtering to hide redundant lines (same normalized text)
- TR/PR toggle buttons and layer selection via selectLyricLayers()
- Click-to-seek: click any lyric line to jump to that position
- Customization popover with font-size sliders and color presets
for each line type (TR/Default/PR), persisted to localStorage
- Smooth font-size transition between active and inactive lines
- Resizable overlay height via drag handle
- lyrics.js: resolveKaraokeTokenWindow, buildSyntheticWordTokens,
findLayerLineIndexForMain, token sorting, collapsed timing detection
API extension (non-breaking, additive):
- tokenLine[].token[] provides per-word start/end timing (ms)
- tokenLine[].index maps back to the corresponding line[] entry
- kind field: 'main', 'translation', 'pronunciation'
- Clients ignoring tokenLine/kind continue to work unchanged
* 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>
The Vorbis/FLAC VERSION field is for track-level disambiguation (e.g.
remix, live, 30s edit), not album versioning. Including it in the
albumversion aliases caused albums to split incorrectly when tracks
had different VERSION values and no MusicBrainz Album ID was set.
Remove 'version' from the albumversion aliases in mappings.yaml.
Users who want the old behavior can re-add it via Tags config.
Update the PID test to use 'albumversion' directly instead of
'version' as the raw PID attribute, with a realistic value.
Fixes#5082
Co-authored-by: Deluan Quintão <deluan@navidrome.org>
* feat(artwork): add KindRadioArtwork and EntityRadio constant
* feat(model): add UploadedImage field and artwork methods to Radio
* feat(model): add Radio to GetEntityByID lookup chain
* feat(db): add uploaded_image column to radio table
* feat(artwork): add radio artwork reader with uploaded image fallback
* feat(api): add radio image upload/delete endpoints
* feat(ui): add radio artwork ID prefix to getCoverArtUrl
* feat(ui): add cover art display and upload to RadioEdit
* feat(ui): add cover art thumbnails to radio list
* feat(ui): prefer artwork URL in radio player helper
* refactor: remove redundant code in radio artwork
- Remove duplicate Avatar rendering in RadioList by reusing CoverArtField
- Remove redundant UpdatedAt assignment in radio image handlers (already set by repository Put)
* refactor(ui): extract shared useImageLoadingState hook
Move image loading/error/lightbox state management into a shared
useImageLoadingState hook in common/. Consolidates duplicated logic
from AlbumDetails, PlaylistDetails, RadioEdit, and artist detail views.
* feat(ui): use radio placeholder icon when no uploaded image
Remove album placeholder fallback from radio artwork reader so radios
without an uploaded image return ErrUnavailable. On the frontend, show
the internet-radio-icon.svg placeholder instead of requesting server
artwork when no image is uploaded, allowing favicon fallback in the
player.
* refactor(ui): update defaultOff fields in useSelectedFields for RadioList
Signed-off-by: Deluan <deluan@navidrome.org>
* fix: address code review feedback
- Add missing alt attribute to CardMedia in RadioEdit for accessibility
- Fix UpdateInternetRadio to preserve UploadedImage field by fetching
existing radio before updating (prevents Subsonic API from clearing
custom artwork)
- Add Reader() level tests to verify ErrUnavailable is returned when
radio has no uploaded image
* refactor: add colsToUpdate to RadioRepository.Put
Use the base sqlRepository.put with column filtering instead of
hand-rolled SQL. UpdateInternetRadio now specifies only the Subsonic API
fields, preventing UploadedImage from being cleared. Image upload/delete
handlers specify only UploadedImage.
* fix: ensure UpdatedAt is included in colsToUpdate for radio Put
---------
Signed-off-by: Deluan <deluan@navidrome.org>
* fix(artwork): fallback mediafile cover art to disc artwork before album
Changed the mediafile cover art fallback chain to go through disc artwork
before album artwork (mediafile → disc → album). Previously, mediafiles
without embedded art fell back directly to album cover, bypassing any
disc-specific artwork. Renamed AlbumCoverArtID() to DiscCoverArtID() to
encapsulate the disc-vs-album decision in a single method, used by both
CoverArtID() and the mediafile artwork reader.
Signed-off-by: Deluan <deluan@navidrome.org>
* fix(artwork): fix cache invalidation for mediafile and album cover art
Include imagesUpdatedAt from album folders in the mediafile artwork
reader's cache key, so that when a cover image file changes on disk
(without audio metadata changes) the mediafile cache properly
invalidates. Also include CoverArtPriority unconditionally in the album
artwork reader's cache key hash, so that changing the priority order
with external services disabled correctly invalidates the album cache.
* fix(artwork): skip disc artwork resolution for single-disc albums
Single-disc albums with DiscNumber=1 were unnecessarily routed through
discArtworkReader, which does extra DB queries only to fall through to
album art anyway. Now only multi-disc albums use the disc fallback path.
* refactor(artwork): restore AlbumCoverArtID as a separate method
Extract AlbumCoverArtID back out of DiscCoverArtID so the single-disc
fallback path in reader_mediafile can reference it by name instead of
inlining the artwork ID construction.
---------
Signed-off-by: Deluan <deluan@navidrome.org>
* feat: add shared ImageUploadService for entity image management
* feat: add UploadedImage field and methods to Artist model
* feat: add uploaded_image column to artist table
* feat: add ArtistImageFolder config option
* refactor: wire ImageUploadService and delegate playlist file ops to it
Wire ImageUploadService into the DI container and refactor the playlist
service to delegate image file operations (SetImage/RemoveImage) to the
shared ImageUploadService, removing duplicated file I/O logic. A local
ImageUploadService interface is defined in core/playlists to avoid an
import cycle between core and core/playlists.
* feat: artist artwork reader checks uploaded image first
* feat: add image-folder priority source for artist artwork
* feat: cache key invalidation for image-folder and uploaded images
* refactor: extract shared image upload HTTP helpers
* feat: add artist image upload/delete API endpoints
* refactor: playlist handlers use shared image upload helpers
* feat: add shared ImageUploadOverlay component
* feat: add i18n keys for artist image upload
* feat: add image upload overlay to artist detail pages
* refactor: playlist details uses shared ImageUploadOverlay component
* fix: add gosec nolint directive for ParseMultipartForm
* refactor: deduplicate image upload code and optimize dir scanning
- Remove dead ImageFilename methods from Artist and Playlist models
(production code uses core.imageFilename exclusively)
- Extract shared uploadedImagePath helper in model/image.go
- Extract findImageInArtistFolder to deduplicate dir-scanning logic
between fromArtistImageFolder and getArtistImageFolderModTime
- Fix fileInputRef in useCallback dependency array
* fix: include artist UpdatedAt in artwork cache key
Without this, uploading or deleting an artist image would not
invalidate the cached artwork because the cache key was only based
on album folder timestamps, not the artist's own UpdatedAt field.
* feat: add Portuguese translations for artist image upload
* refactor: use shared i18n keys for cover art upload messages
Move cover art upload/remove translations from per-entity sections
(artist, playlist) to a shared top-level "message" section, avoiding
duplication across entity types and translation files.
* refactor: move cover art i18n keys to shared message section for all languages
* refactor: simplify image upload code and eliminate redundancies
Extracted duplicate image loading/lightbox state logic from
DesktopArtistDetails and MobileArtistDetails into a shared
useArtistImageState hook. Moved entity type constants to the consts
package and replaced raw string literals throughout model, core, and
nativeapi packages. Exported model.UploadedImagePath and reused it in
core/image_upload.go to consolidate path construction. Cached the
ArtistImageFolder lookup result in artistReader to eliminate a redundant
os.ReadDir call on every artwork request.
Signed-off-by: Deluan <deluan@navidrome.org>
* style: fix prettier formatting in ImageUploadOverlay
* fix: address code review feedback on image upload error handling
- RemoveImage now returns errors instead of swallowing them
- Artist handlers distinguish not-found from other DB errors
- Defer multipart temp file cleanup after parsing
* fix: enforce hard request size limit with MaxBytesReader for image uploads
Signed-off-by: Deluan <deluan@navidrome.org>
---------
Signed-off-by: Deluan <deluan@navidrome.org>
