Merge branch 'master' into artwork-blurhash

Adapt #5856's artist-folder fix to the new artwork pipeline. The readers
it patched (reader_album.go, reader_artist.go) no longer exist here, so
the fix moves into their replacements:

- folders_album.go: loadAlbumFoldersPaths narrows to a single album and
  gains master's loadFolders/folderImages split.
- folders_artist.go: adds loadArtistAlbumRoots and commonDir, and
  loadArtistFolder only climbs when the artist has a single album root.
- resolve.go: the artist chain reads album roots instead of the
  parent-promoted album folder list.

The album and disc chains keep using loadAlbumFoldersPaths, whose
promoted parent is what finds cover art above disc subfolders, so the
artist chain needed its own loader rather than a changed shared one.

The e2e specs came in from master; they are rewritten against this
branch's harness, which asserts the resolved source path instead of the
served bytes and so pins the artist folder itself.
This commit is contained in:
Deluan 2026-07-29 21:58:43 -04:00
commit b876bdf28e
4 changed files with 152 additions and 20 deletions

View File

@ -74,6 +74,89 @@ var _ = Describe("Artist artwork resolution", func() {
})
})
When("ArtistArtPriority has no album/ fallback", func() {
// Artist/
// ├── artist.jpg ← must resolve via the artist folder itself
// └── Album/
// └── 01 - Track.mp3
It("still resolves the artist folder and returns artist.*", func() {
conf.Server.ArtistArtPriority = "artist.*"
setLayout(fstest.MapFS{
"Artist/Album/01 - Track.mp3": trackFile(1, "Track", map[string]any{"albumartist": "Artist"}),
"Artist/artist.jpg": smallPNG("artist-folder"),
})
scan()
expectArtistFolder(soleArtist(), "Artist/artist.jpg")
})
})
When("the artist's only album has its tracks in disc subfolders", func() {
// Artist/
// ├── artist.jpg ← wins (artist.* before album/artist.*)
// └── Album/
// ├── artist.jpg
// ├── CD1/01 - Track.mp3
// └── CD2/02 - Track.mp3
It("prefers the artist-folder image over the album-folder one", func() {
conf.Server.ArtistArtPriority = "artist.*, album/artist.*, external"
setLayout(fstest.MapFS{
"Artist/Album/CD1/01 - Track.mp3": trackFile(1, "Track 1", map[string]any{"albumartist": "Artist", "album": "Album"}),
"Artist/Album/CD2/02 - Track.mp3": trackFile(2, "Track 2", map[string]any{"albumartist": "Artist", "album": "Album"}),
"Artist/artist.jpg": smallPNG("artist-folder"),
"Artist/Album/artist.jpg": smallPNG("album-artist"),
})
scan()
expectArtistFolder(soleArtist(), "Artist/artist.jpg")
})
})
When("one album has disc subfolders and another sits at artist level", func() {
// Artist/
// ├── artist.jpg ← wins
// ├── Album1/
// │ ├── artist.jpg
// │ ├── CD1/01 - Track.mp3
// │ └── CD2/02 - Track.mp3
// └── Album2/03 - Track.mp3
It("prefers the artist-folder image over the album-folder one", func() {
conf.Server.ArtistArtPriority = "artist.*, album/artist.*, external"
setLayout(fstest.MapFS{
"Artist/Album1/CD1/01 - Track.mp3": trackFile(1, "Track 1", map[string]any{"albumartist": "Artist", "album": "Album1"}),
"Artist/Album1/CD2/02 - Track.mp3": trackFile(2, "Track 2", map[string]any{"albumartist": "Artist", "album": "Album1"}),
"Artist/Album2/03 - Track.mp3": trackFile(3, "Track 3", map[string]any{"albumartist": "Artist", "album": "Album2"}),
"Artist/artist.jpg": smallPNG("artist-folder"),
"Artist/Album1/artist.jpg": smallPNG("album-artist"),
})
scan()
expectArtistFolder(soleArtist(), "Artist/artist.jpg")
})
})
When("every album of the artist has its tracks in disc subfolders", func() {
// Artist/
// ├── artist.jpg ← wins
// ├── Album1/
// │ ├── artist.jpg
// │ ├── CD1/01 - Track.mp3
// │ └── CD2/02 - Track.mp3
// └── Album2/
// ├── CD1/03 - Track.mp3
// └── CD2/04 - Track.mp3
It("prefers the artist-folder image over the album-folder one", func() {
conf.Server.ArtistArtPriority = "artist.*, album/artist.*, external"
setLayout(fstest.MapFS{
"Artist/Album1/CD1/01 - Track.mp3": trackFile(1, "Track 1", map[string]any{"albumartist": "Artist", "album": "Album1"}),
"Artist/Album1/CD2/02 - Track.mp3": trackFile(2, "Track 2", map[string]any{"albumartist": "Artist", "album": "Album1"}),
"Artist/Album2/CD1/03 - Track.mp3": trackFile(3, "Track 3", map[string]any{"albumartist": "Artist", "album": "Album2"}),
"Artist/Album2/CD2/04 - Track.mp3": trackFile(4, "Track 4", map[string]any{"albumartist": "Artist", "album": "Album2"}),
"Artist/artist.jpg": smallPNG("artist-folder"),
"Artist/Album1/artist.jpg": smallPNG("album-artist"),
})
scan()
expectArtistFolder(soleArtist(), "Artist/artist.jpg")
})
})
When("an artist has an uploaded image and a matching artist.* file", func() {
// <DataFolder>/
// └── artwork/

View File

@ -13,19 +13,16 @@ import (
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/utils/natural"
"github.com/navidrome/navidrome/utils/slice"
)
func loadAlbumFoldersPaths(ctx context.Context, ds model.DataStore, albums ...model.Album) ([]string, []string, *time.Time, error) {
var folderIDs []string
for _, album := range albums {
folderIDs = append(folderIDs, album.FolderIDs...)
}
folders, err := ds.Folder(ctx).GetAll(model.QueryOptions{Filters: squirrel.Eq{"folder.id": folderIDs, "missing": false}})
func loadAlbumFoldersPaths(ctx context.Context, ds model.DataStore, album model.Album) ([]string, []string, *time.Time, error) {
folders, err := loadFolders(ctx, ds, album.FolderIDs)
if err != nil {
return nil, nil, nil, err
}
parent, err := albumRootParent(ctx, ds, folders, folderIDs)
parent, err := albumRootParent(ctx, ds, folders, album.FolderIDs)
if err != nil {
return nil, nil, nil, err
}
@ -33,11 +30,21 @@ func loadAlbumFoldersPaths(ctx context.Context, ds model.DataStore, albums ...mo
folders = append(folders, *parent)
}
var paths []string
paths := slice.Map(folders, func(f model.Folder) string { return f.AbsolutePath() })
imgFiles, updatedAt := folderImages(folders)
return paths, imgFiles, &updatedAt, nil
}
func loadFolders(ctx context.Context, ds model.DataStore, folderIDs []string) ([]model.Folder, error) {
return ds.Folder(ctx).GetAll(model.QueryOptions{Filters: squirrel.Eq{"folder.id": folderIDs, "missing": false}})
}
// folderImages collects the folders' image files, sorted so files without
// numeric suffixes win (e.g. cover.jpg over cover.1.jpg).
func folderImages(folders []model.Folder) ([]string, time.Time) {
var imgFiles []string
var updatedAt time.Time
for _, f := range folders {
paths = append(paths, f.AbsolutePath())
if f.ImagesUpdatedAt.After(updatedAt) {
updatedAt = f.ImagesUpdatedAt
}
@ -46,13 +53,8 @@ func loadAlbumFoldersPaths(ctx context.Context, ds model.DataStore, albums ...mo
imgFiles = append(imgFiles, path.Join(rel, img))
}
}
// Sort image files to ensure consistent selection of cover art
// This prioritizes files without numeric suffixes (e.g., cover.jpg over cover.1.jpg)
// by comparing base filenames without extensions
slices.SortFunc(imgFiles, compareImageFiles)
return paths, imgFiles, &updatedAt, nil
return imgFiles, updatedAt
}
// albumRootParent returns the common parent of the album's folders when it

View File

@ -110,17 +110,64 @@ func escapeGlobLiteral(s string) string {
return b.String()
}
// loadArtistAlbumRoots returns one path per album — the deepest folder holding
// all of that album's tracks — so an album split into disc subfolders can't
// pull the artist folder's common prefix below the artist level.
func loadArtistAlbumRoots(ctx context.Context, ds model.DataStore, albums model.Albums) ([]string, []string, *time.Time, error) {
var folderIDs []string
for _, album := range albums {
folderIDs = append(folderIDs, album.FolderIDs...)
}
folders, err := loadFolders(ctx, ds, folderIDs)
if err != nil {
return nil, nil, nil, err
}
pathByID := slice.ToMap(folders, func(f model.Folder) (string, string) {
return f.ID, f.AbsolutePath()
})
var roots []string
for _, album := range albums {
var albumPaths []string
for _, fid := range album.FolderIDs {
if p, ok := pathByID[fid]; ok {
albumPaths = append(albumPaths, p)
}
}
if len(albumPaths) > 0 {
roots = append(roots, commonDir(albumPaths))
}
}
imgFiles, updatedAt := folderImages(folders)
return roots, imgFiles, &updatedAt, nil
}
// commonDir returns the deepest directory containing all paths. Trailing
// separators keep the comparison on segment boundaries, so a shared name
// fragment (".../Album" and ".../Album2") is never read as a shared directory.
func commonDir(paths []string) string {
sep := string(filepath.Separator)
common := str.LongestCommonPrefix(slice.Map(paths, func(p string) string { return p + sep }))
if !strings.HasSuffix(common, sep) {
common, _ = filepath.Split(common)
}
return filepath.Clean(common)
}
func loadArtistFolder(ctx context.Context, ds model.DataStore, albums model.Albums, paths []string) (string, time.Time, error) {
if len(albums) == 0 {
return "", time.Time{}, nil
}
libID := albums[0].LibraryID // TODO: Support albums spanning multiple libraries
folderPath := str.LongestCommonPrefix(paths)
if !strings.HasSuffix(folderPath, string(filepath.Separator)) {
folderPath, _ = filepath.Split(folderPath)
// paths holds one root per album: two or more distinct roots already meet at
// the artist folder, while a single root is an album folder needing a climb.
roots := slices.Compact(slices.Sorted(slices.Values(paths)))
folderPath := commonDir(roots)
if len(roots) < 2 {
folderPath = filepath.Dir(folderPath)
}
folderPath = filepath.Dir(folderPath)
// TODO: Hacky, but the easiest way to get the folder ID ATM
libPath := core.AbsolutePath(ctx, ds, libID, "")

View File

@ -175,7 +175,7 @@ func (r *resolver) resolveArtist(ctx context.Context, artistID string) (resoluti
if err != nil {
return resolution{}, err
}
albumPaths, imgFiles, _, err := loadAlbumFoldersPaths(ctx, r.ds, als...)
albumPaths, imgFiles, _, err := loadArtistAlbumRoots(ctx, r.ds, als)
if err != nil {
return resolution{}, err
}