goleak now ignores notify's nonrecursive-tree goroutines (linux uses
inotify, which spawns dispatch+internal instead of darwin's recursive
dispatch), and the read-only-dir prune spec skips under root, where
permission bits cannot make Remove fail.
The playlist ExternalImageURL step used sources.go's fromURL, which maps any
non-200 to a generic error, so a stale URL returning 404/410 was classified
transient: infinite backoff plus it counted toward the circuit breaker,
blocking valid external work. Add a local fetch in resolve.go that maps
404/410 to model.ErrNotFound (definitive) while keeping other non-200s
transient. sources.go is left untouched.
With CoverArtPriority="external,cover.jpg", a transient external failure
followed by a folder hit dropped the external error: the worker recorded
found and deleted the queue row, so the configured higher-priority external
art was never retried. Carry extError onto the fallback resolution and add
an outcomeFoundStale that persists+serves the art but reschedules via
MarkFailed, giving the external source another chance. When external later
answers definitively-not-found, the hit is not stale and the row is deleted.
A user-editable ExternalImageURL can point at an arbitrarily large endpoint;
a fast server could make the worker buffer hundreds of MB inside the 5s HTTP
timeout. Bound the read to a fixed 20MB cap (no config knob) via io.LimitReader
and fail the item if it is exceeded.
A playlist ExternalImageURL pointing at a local file that fails to open was
routed through extError, causing failed/48h-retry loops that burn a rate
limiter token forever instead of falling through to the generated grid.
Correct the inverted workerDeps.extGate comment, trim over-budget doc comments, and add conf.Server.ArtistImageFolder to the resolution fingerprint so an image-folder change re-resolves artist artwork.
Queue rows for deleted entities failed forever (Get -> ErrNotFound -> failed -> capped retries, unbounded). Add ArtworkQueueRepository.PurgeDangling, called from Prune next to the item_artwork purge. Separately, the found/absent path unconditionally deleted the dequeued row, erasing a concurrent scan re-enqueue; switch to DeleteIfUnchanged, which deletes only while retry_at still matches the dequeued value (verified retry_at is the column an Enqueue upsert resets).
resolvePlaylist only built the generated grid, dropping the uploaded-image, sidecar and ExternalImageURL sources the old reader_playlist.go chain serves. Port the full chain before the grid fallback: uploaded (upload), sidecar (folder), and ExternalImageURL routed through extGate with the same extError semantics as the other external steps. Also rewires the artist external step onto ArtistImageResult.
callGetImage swallowed all agent errors, so an agent outage surfaced as ErrNotFound and the worker settled artist artwork as a definitive absent (and reset the breaker). Add an additive ArtistImageResult path that returns the underlying agent error on transient failure while keeping ArtistImage byte-identical for existing callers; the worker's artist external step uses it via fromArtistExternalResult.
Resolves one queue item end to end: hash/dedup, decode + 128px thumbnail
blurhash, place bytes (store vs source file), and persist found/absent/
failed state for the worker (Task 4) to act on.
Known-absent states carry an empty hash and malformed persisted hashes could panic path sharding or inject separators; Write/Open/Remove now return an error for anything but 16 lowercase hex chars.
Write falls through to a real write when the liveness touch fails, and sweep retention now matches the recorded mime's extension so obsolete variants are reclaimed.
Duplicate ImageStore writes refresh the file mtime and Remove skips files newer than the cutoff, so overlapping acquisitions cannot lose their store files to a concurrent prune.
Sweep now honors an mtime grace window (in-flight acquisitions and temp files), reacquired orphans reset the prune grace window, and the queue mock implements real stale-absent semantics.
* fix(artwork): convert decoded images to a fast-path type before resizing
x/image/draw's CatmullRom scaler only has optimized paths for *image.RGBA,
*image.NRGBA, *image.Gray and *image.YCbCr. Other concrete types — notably
*image.NYCbCrA (from WebP) and *image.Paletted (indexed PNGs) — fall back to
a generic per-pixel At()/RGBA() loop that is several times slower.
Convert such images to *image.RGBA once before scaling; fast-path types are
returned unchanged. This makes resize performance independent of which decoder
wins the image.Decode("webp") registration, and also speeds up indexed PNGs.
Signed-off-by: Deluan <deluan@navidrome.org>
* chore(deps): update gen2brain/webp to v0.6.0
v0.6.0 replaces the wazero WASM runtime with a self-contained
wasm2go-transpiled WebP decoder/encoder. This drops the webp -> wazero
dependency edge (wazero is still used by the plugin system) and makes the
WASM-only build path (32-bit / nodynamic) faster and far lighter on
allocations.
Signed-off-by: Deluan <deluan@navidrome.org>
* perf(artwork): defer fast-path conversion until a resize is needed
Move toFastScaleType to just before the CatmullRom.Scale call, after the
no-upscale early return. Previously the conversion ran right after decode, so a
request for a size >= the source dimensions would allocate and walk a full RGBA
copy only to discard it when resizeStaticImage returns nil. The resize path is
unchanged; the no-op path drops ~30-40% time and up to ~79% memory for large
indexed/WebP artwork.
Signed-off-by: Deluan <deluan@navidrome.org>
---------
Signed-off-by: Deluan <deluan@navidrome.org>
* fix(artwork): avoid WebP segfault on 32-bit ARM
On 32-bit ARM, the gen2brain/webp native libwebp path uses ebitengine/purego
reverse callbacks, which purego does not support on that architecture. Selecting
it crashes the process with a SIGSEGV when encoding or decoding WebP cover art,
taking down the whole server on the first web UI artwork request (issue #5597).
Force the safe WASM path on armv7/v6 in two layers: build the Docker arm binary
with the gen2brain/webp "nodynamic" tag so purego is never linked, and add a
runtime GOARCH guard in the init hook so source builds on 32-bit ARM are also
protected. arm64 keeps the native libwebp path.
* fix(artwork): also disable native WebP on 32-bit x86
purego's callback implementation is built with the constraint !386 && !arm,
so 32-bit x86 (386) crashes with the same SIGSEGV as 32-bit ARM when the native
libwebp path is used. Navidrome ships linux/386 and windows/386 builds, so guard
386 alongside arm: extend the runtime GOARCH check and the Docker nodynamic build
tag to cover both. 64-bit arches keep the native libwebp path.
* fix(artwork): rely on nodynamic build tag, drop ineffective runtime guard
The previous runtime GOARCH guard did not actually prevent the crash:
gen2brain/webp selects the native (purego) vs WASM backend in its own package
init() and registers the purego write callback at import time, before any
Navidrome hook runs. webp.Dynamic() is only a status getter, and Decode/Encode
branch on the library's unexported flag, so the guard merely skipped a log line
while the native path stayed active.
The effective fix is the nodynamic build tag (applied for 32-bit ARM and x86 in
the Dockerfile), which compiles gen2brain/webp WASM-only so purego is never
linked. Drop the misleading guard and document that source builds on 32-bit
architectures must be built with -tags nodynamic.
* fix(artwork): don't enable WebP encoding by default in Docker
The Docker image set ND_ENABLEWEBPENCODING=true, which (a) forced cover-art
thumbnails through WebP for every install and (b) overrode any
EnableWebPEncoding=false set in the user's navidrome.toml, since env vars take
precedence over the config file in Viper.
On 32-bit platforms the only available WebP backend is the WASM encoder, which
is slow on the underpowered hardware those builds typically run on, so enabling
it by default is the wrong tradeoff there. Remove the env default and leave
EnableWebPEncoding off unless the user opts in. Combined with the nodynamic
build tag, 32-bit images neither crash nor pay the WASM cost out of the box.
A smarter automatic policy (use WebP only when native libwebp is available) can
be revisited separately.
* test(artwork): add failing e2e tests for artist image leaking as album art
Reproduces a v0.62.0 regression (#5451/#5457): the album cover-art
parent-folder fallback can include the artist folder, serving the artist
thumbnail (e.g. Artist/folder.jpg) as album art for any album without
image files in its own folder(s). Covers three scenarios: a plain
Artist/Album layout with no album images, a single-disc album spread
across sibling folders under the artist folder, and a spread album whose
own front.jpg is shadowed by the artist's cover.jpg via CoverArtPriority
order. Also adds an albumByName test helper for multi-album layouts.
The tests are expected to fail until the parent-folder inclusion is
gated by a structural check (skip the common parent when audio from
other albums lives under it).
* fix(artwork): never serve artist folder images as album art
The album cover-art parent-folder fallback (introduced in #5451/#5457)
could include the artist folder as a source of album images, serving the
artist thumbnail (e.g. Artist/folder.jpg) as cover art for any album
without image files in its own folder(s). This affected both plain
Artist/Album layouts and single-disc albums spread across sibling
folders under the artist folder.
Gate the common-parent inclusion with a structural check: the parent
only qualifies as an album root when no audio belonging to other albums
lives in it or anywhere beneath it. An artist folder contains other
albums' tracks, while an album root above disc subfolders contains only
this album's, so the check works for any disc folder naming scheme and
never affects the multi-disc fixes from #5376/#5456. A single-album
artist with no images anywhere remains structurally indistinguishable
from an album root and is a known residual case.
* refactor(artwork): move album-root audio check into folder repository
Replace the raw subtree SQL (LIKE/ESCAPE expression and wildcard
escaping) that lived in core/artwork with an explicit
FolderRepository.HasAudioOutsideFolders method, implemented in the
persistence layer next to the existing folder-subtree query pattern.
This also removes the test mock's brittle dispatch that sniffed the
generated SQL to recognize the query; the fake now overrides the new
method directly.
Extract the whole parent-folder resolution from loadAlbumFoldersPaths
into an albumRootParent helper, flattening four levels of nesting back
into a linear flow. Behavior is unchanged; the unit test for a parent
containing audio moved to the persistence suite, with added coverage
for subtree boundaries, missing folders, and LIKE-wildcard escaping in
folder paths.
* refactor(persistence): use exists helper in HasAudioOutsideFolders
Replace the hand-rolled count(*) query with the repository's canonical
exists helper, as suggested in PR review.
* feat(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>
* fix(artwork): include top-level album folders in parent cover art lookup
The Path != "." guard added in #5451 was too aggressive — it excluded
any folder with Path=".", which includes top-level album folders (not
just the library root). Changed to ParentID != "" which correctly
excludes only the actual library root folder.
Fixes#5456
* fix: correct comment in test — album is under library root, not artist root
* test: add ascii tree diagram to top-level album e2e test
* test: replace internal bug references with issue link in e2e comments
Signed-off-by: Deluan <deluan@navidrome.org>
* test: add e2e test matching reporter's exact library layout (#5456)
Adds a deeply nested test (Genre/Artist/Album/Disc) with 12 discs
using the reporter's actual folder names to verify artwork resolution
works for non-top-level album folders too.
* fix(scanner): use a syntectic admin user when no admin user is found
Signed-off-by: Deluan <deluan@navidrome.org>
* fix(scanner): bump album UpdatedAt on Phase 3 refresh to invalidate artwork cache
When Phase 3 corrects an album's FolderIDs (or any other field), bump
UpdatedAt to the current time. This ensures the artwork cache key changes,
invalidating any stale artwork that was resolved and cached during Phase 1
when the album had incomplete folder data.
* fix(artwork): include ImportedAt in artwork cache key to invalidate stale cache
Reverts the Phase 3 UpdatedAt bump (which would change album.UpdatedAt
semantics) and instead includes album.ImportedAt in the artwork cache key
computation. Since ImportedAt is bumped to time.Now() on every album Put,
any Phase 3 correction naturally invalidates cached artwork that was
resolved mid-scan with incomplete folder data.
* fix(artwork): simplify lastUpdate logic using TimeNewest utility
Signed-off-by: Deluan <deluan@navidrome.org>
---------
Signed-off-by: Deluan <deluan@navidrome.org>
Fixed two bugs in album cover art resolution for multi-disc layouts:
1. compareImageFiles now sorts by path depth (shallower first) when basenames
tie, so album-root images like Artist/Album/cover.jpg are preferred over
disc-subfolder images like Artist/Album/CD1/cover.jpg.
2. commonParentFolder now includes the parent folder for single-disc-subfolder
albums, with a Path != "." guard to avoid pulling artist-folder images.
Closes#5376
* 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
* 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>
* 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.
* refactor(artwork): rename DevJpegCoverArt to EnableWebPEncoding
Replaced the internal DevJpegCoverArt flag with a user-facing
EnableWebPEncoding config option (defaults to true). When disabled, the
fallback encoding now preserves the original image format — PNG sources
stay PNG for non-square resizes, matching v0.60.3 behavior. The previous
implementation incorrectly re-encoded PNG sources as JPEG in non-square
mode. Also added EnableWebPEncoding to the insights data.
* feat: add configurable UICoverArtSize option
Converted the hardcoded UICoverArtSize constant (600px) into a
configurable option, allowing users to reduce the cover art size
requested by the UI to mitigate slow image encoding. The value is
served to the frontend via the app config and used by all components
that request cover art. Also simplified the cache warmer by removing
a single-iteration loop in favor of direct code.
* style: fix prettier formatting in subsonic test
* feat: log WebP encoder/decoder selection
Signed-off-by: Deluan <deluan@navidrome.org>
* fix(artwork): address PR review feedback
- Add DevJpegCoverArt to logRemovedOptions so users with the old config
key get a clear warning instead of a silent ignore.
- Include EnableWebPEncoding in the resized artwork cache key to prevent
stale WebP responses after toggling the setting.
- Skip animated GIF to WebP conversion via ffmpeg when EnableWebPEncoding
is false, so the setting is consistent across all image types.
- Fix data race in cache warmer by reading UICoverArtSize at construction
time instead of per-image, avoiding concurrent access with config
cleanup in tests.
- Clarify cache warmer docstring to accurately describe caching behavior.
* Revert "fix(artwork): address PR review feedback"
This reverts commit 3a213ef03e.
* fix(artwork): avoid data race in cache warmer config access
Capture UICoverArtSize at construction time instead of reading from
conf.Server on each doCacheImage call. The background goroutine could
race with test config cleanup, causing intermittent race detector
failures in CI.
* fix(configuration): clamp UICoverArtSize to be within 200 and 1200
Signed-off-by: Deluan <deluan@navidrome.org>
* fix(artwork): preserve album cache key compatibility with v0.60.3
Restored the v0.60.3 hash input order for album artwork cache keys
(Agents + CoverArtPriority) so that existing caches remain valid on
upgrade when EnableExternalServices is true. Also ensures
CoverArtPriority is always part of the hash even when external services
are disabled, fixing a v0.60.3 bug where changing CoverArtPriority had
no effect on cache invalidation.
Signed-off-by: Deluan <deluan@navidrome.org>
* fix: default EnableWebPEncoding to false and reduce artwork parallelism
Changed EnableWebPEncoding default to false so that upgrading users get
the same JPEG/PNG encoding behavior as v0.60.3 out of the box, avoiding
the WebP WASM overhead until native libwebp is available. Users can
opt in to WebP by setting EnableWebPEncoding=true. Also reduced the
default DevArtworkMaxRequests to half the CPU count (min 2) to lower
resource pressure during artwork processing.
* fix(configuration): update DefaultUICoverArtSize to 300
Signed-off-by: Deluan <deluan@navidrome.org>
* fix(Makefile): append EXTRA_BUILD_TAGS to GO_BUILD_TAGS
Signed-off-by: Deluan <deluan@navidrome.org>
---------
Signed-off-by: Deluan <deluan@navidrome.org>
