mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-01 07:21:17 +00:00
Merge branch 'master' into i18n/zh-hans
This commit is contained in:
commit
9bc147c7d7
@ -202,6 +202,7 @@ var _ = Describe("Artwork", func() {
|
||||
repoRoot, err := os.Getwd()
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
folderRepo.result = []model.Folder{{
|
||||
ID: "f1",
|
||||
LibraryPath: testFileLibPath(repoRoot),
|
||||
Path: "tests/fixtures/artist/an-album",
|
||||
ImageFiles: []string{"artist.png"},
|
||||
|
||||
@ -80,6 +80,101 @@ 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": imageFile("artist-folder"),
|
||||
})
|
||||
scan()
|
||||
|
||||
ar := soleArtist()
|
||||
artID := model.NewArtworkID(model.KindArtistArtwork, ar.ID, nil)
|
||||
Expect(readArtwork(artID)).To(Equal(imageBytes("artist-folder")))
|
||||
})
|
||||
})
|
||||
|
||||
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": imageFile("artist-folder"),
|
||||
"Artist/Album/artist.jpg": imageFile("album-artist"),
|
||||
})
|
||||
scan()
|
||||
|
||||
ar := soleArtist()
|
||||
artID := model.NewArtworkID(model.KindArtistArtwork, ar.ID, nil)
|
||||
Expect(readArtwork(artID)).To(Equal(imageBytes("artist-folder")))
|
||||
})
|
||||
})
|
||||
|
||||
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": imageFile("artist-folder"),
|
||||
"Artist/Album1/artist.jpg": imageFile("album-artist"),
|
||||
})
|
||||
scan()
|
||||
|
||||
ar := soleArtist()
|
||||
artID := model.NewArtworkID(model.KindArtistArtwork, ar.ID, nil)
|
||||
Expect(readArtwork(artID)).To(Equal(imageBytes("artist-folder")))
|
||||
})
|
||||
})
|
||||
|
||||
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": imageFile("artist-folder"),
|
||||
"Artist/Album1/artist.jpg": imageFile("album-artist"),
|
||||
})
|
||||
scan()
|
||||
|
||||
ar := soleArtist()
|
||||
artID := model.NewArtworkID(model.KindArtistArtwork, ar.ID, nil)
|
||||
Expect(readArtwork(artID)).To(Equal(imageBytes("artist-folder")))
|
||||
})
|
||||
})
|
||||
|
||||
When("an artist has an uploaded image and a matching artist.* file", func() {
|
||||
// <DataFolder>/
|
||||
// └── artwork/
|
||||
|
||||
@ -20,6 +20,7 @@ import (
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/utils"
|
||||
"github.com/navidrome/navidrome/utils/natural"
|
||||
"github.com/navidrome/navidrome/utils/slice"
|
||||
)
|
||||
|
||||
type albumArtworkReader struct {
|
||||
@ -103,17 +104,13 @@ func (a *albumArtworkReader) fromCoverArtPriority(ctx context.Context, ffmpeg ff
|
||||
return ff
|
||||
}
|
||||
|
||||
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
|
||||
}
|
||||
@ -121,11 +118,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
|
||||
}
|
||||
@ -134,13 +141,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
|
||||
|
||||
@ -3,6 +3,7 @@ package artwork
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"path/filepath"
|
||||
"time"
|
||||
|
||||
"github.com/navidrome/navidrome/model"
|
||||
@ -339,6 +340,32 @@ var _ = Describe("Album Artwork Reader", func() {
|
||||
Expect(repo.getCallCount).To(Equal(1))
|
||||
})
|
||||
|
||||
It("promotes the album root parent into the returned paths", func() {
|
||||
repo.result = []model.Folder{
|
||||
{
|
||||
ID: "folder1",
|
||||
Path: "Artist",
|
||||
Name: "Album",
|
||||
ParentID: "artistFolder",
|
||||
ImagesUpdatedAt: now,
|
||||
ImageFiles: []string{},
|
||||
},
|
||||
}
|
||||
repo.parentResult = &model.Folder{
|
||||
ID: "artistFolder",
|
||||
Path: ".",
|
||||
Name: "Artist",
|
||||
ParentID: "libraryRoot",
|
||||
ImagesUpdatedAt: expectedAt,
|
||||
ImageFiles: []string{"folder.jpg"},
|
||||
}
|
||||
|
||||
paths, _, _, err := loadAlbumFoldersPaths(ctx, ds, album)
|
||||
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(paths).To(Equal([]string{filepath.Join("Artist", "Album"), "Artist"}))
|
||||
})
|
||||
|
||||
It("does not include parent images when other albums' audio lives under the parent", func() {
|
||||
// Simulates: Artist/folder.jpg with Artist/Album (no images) and
|
||||
// another album's tracks elsewhere under the artist folder
|
||||
|
||||
@ -19,6 +19,7 @@ import (
|
||||
"github.com/navidrome/navidrome/core/external"
|
||||
"github.com/navidrome/navidrome/log"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/utils/slice"
|
||||
"github.com/navidrome/navidrome/utils/str"
|
||||
)
|
||||
|
||||
@ -54,7 +55,7 @@ func newArtistArtworkReader(ctx context.Context, artwork *artwork, artID model.A
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
albumPaths, imgFiles, imagesUpdatedAt, err := loadAlbumFoldersPaths(ctx, artwork.ds, als...)
|
||||
albumPaths, imgFiles, imagesUpdatedAt, err := loadArtistAlbumRoots(ctx, artwork.ds, als)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
@ -232,17 +233,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 // Just need one of the albums, as they should all be in the same Library - for now! TODO: Support 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)
|
||||
|
||||
// Manipulate the path to get the folder ID
|
||||
// TODO: This is a bit hacky, but it's the easiest way to get the folder ID, ATM
|
||||
|
||||
@ -85,6 +85,19 @@ var _ = Describe("artistArtworkReader", func() {
|
||||
})
|
||||
})
|
||||
|
||||
When("two albums share the same folder", func() {
|
||||
It("climbs above the shared album folder", func() {
|
||||
paths = []string{
|
||||
filepath.FromSlash("/music/artist/split"),
|
||||
filepath.FromSlash("/music/artist/split"),
|
||||
}
|
||||
folder, upd, err := loadArtistFolder(ctx, fds, albums, paths)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(folder).To(Equal(filepath.FromSlash("/music/artist")))
|
||||
Expect(upd).To(Equal(expectedUpdTime))
|
||||
})
|
||||
})
|
||||
|
||||
When("the album paths contain same prefix", func() {
|
||||
It("returns the common prefix", func() {
|
||||
paths = []string{
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user