mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-01 07:21:17 +00:00
f48416685f
* 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>
400 lines
12 KiB
Go
400 lines
12 KiB
Go
package artwork
|
|
|
|
import (
|
|
"context"
|
|
"errors"
|
|
"time"
|
|
|
|
"github.com/navidrome/navidrome/model"
|
|
. "github.com/onsi/ginkgo/v2"
|
|
. "github.com/onsi/gomega"
|
|
)
|
|
|
|
var _ = Describe("Album Artwork Reader", func() {
|
|
Describe("loadAlbumFoldersPaths", func() {
|
|
var (
|
|
ctx context.Context
|
|
ds *fakeDataStore
|
|
repo *fakeFolderRepo
|
|
album model.Album
|
|
now time.Time
|
|
expectedAt time.Time
|
|
)
|
|
|
|
BeforeEach(func() {
|
|
ctx = context.Background()
|
|
now = time.Now().Truncate(time.Second)
|
|
expectedAt = now.Add(5 * time.Minute)
|
|
|
|
// Set up the test folders with image files
|
|
repo = &fakeFolderRepo{}
|
|
ds = &fakeDataStore{
|
|
folderRepo: repo,
|
|
}
|
|
album = model.Album{
|
|
ID: "album1",
|
|
Name: "Album",
|
|
FolderIDs: []string{"folder1", "folder2", "folder3"},
|
|
}
|
|
})
|
|
|
|
It("returns sorted image files", func() {
|
|
repo.result = []model.Folder{
|
|
{
|
|
Path: "Artist/Album/Disc1",
|
|
ImagesUpdatedAt: expectedAt,
|
|
ImageFiles: []string{"cover.jpg", "back.jpg", "cover.1.jpg"},
|
|
},
|
|
{
|
|
Path: "Artist/Album/Disc2",
|
|
ImagesUpdatedAt: now,
|
|
ImageFiles: []string{"cover.jpg"},
|
|
},
|
|
{
|
|
Path: "Artist/Album/Disc10",
|
|
ImagesUpdatedAt: now,
|
|
ImageFiles: []string{"cover.jpg"},
|
|
},
|
|
}
|
|
|
|
_, imgFiles, imagesUpdatedAt, err := loadAlbumFoldersPaths(ctx, ds, album)
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(*imagesUpdatedAt).To(Equal(expectedAt))
|
|
|
|
// Check that image files are sorted by base name (without extension)
|
|
Expect(imgFiles).To(HaveLen(5))
|
|
|
|
// Files should be sorted by base filename without extension, then by full path
|
|
// "back" < "cover", so back.jpg comes first
|
|
// Then all cover.jpg files, sorted by path
|
|
Expect(imgFiles[0]).To(Equal("Artist/Album/Disc1/back.jpg"))
|
|
Expect(imgFiles[1]).To(Equal("Artist/Album/Disc1/cover.jpg"))
|
|
Expect(imgFiles[2]).To(Equal("Artist/Album/Disc2/cover.jpg"))
|
|
Expect(imgFiles[3]).To(Equal("Artist/Album/Disc10/cover.jpg"))
|
|
Expect(imgFiles[4]).To(Equal("Artist/Album/Disc1/cover.1.jpg"))
|
|
})
|
|
|
|
It("prioritizes files without numeric suffixes", func() {
|
|
// Test case for issue #4683: cover.jpg should come before cover.1.jpg
|
|
repo.result = []model.Folder{
|
|
{
|
|
Path: "Artist/Album",
|
|
ImagesUpdatedAt: now,
|
|
ImageFiles: []string{"cover.1.jpg", "cover.jpg", "cover.2.jpg"},
|
|
},
|
|
}
|
|
|
|
_, imgFiles, _, err := loadAlbumFoldersPaths(ctx, ds, album)
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(imgFiles).To(HaveLen(3))
|
|
|
|
// cover.jpg should come first because "cover" < "cover.1" < "cover.2"
|
|
Expect(imgFiles[0]).To(Equal("Artist/Album/cover.jpg"))
|
|
Expect(imgFiles[1]).To(Equal("Artist/Album/cover.1.jpg"))
|
|
Expect(imgFiles[2]).To(Equal("Artist/Album/cover.2.jpg"))
|
|
})
|
|
|
|
It("handles case-insensitive sorting", func() {
|
|
// Test that Cover.jpg and cover.jpg are treated as equivalent
|
|
repo.result = []model.Folder{
|
|
{
|
|
Path: "Artist/Album",
|
|
ImagesUpdatedAt: now,
|
|
ImageFiles: []string{"Folder.jpg", "cover.jpg", "BACK.jpg"},
|
|
},
|
|
}
|
|
|
|
_, imgFiles, _, err := loadAlbumFoldersPaths(ctx, ds, album)
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(imgFiles).To(HaveLen(3))
|
|
|
|
// Files should be sorted case-insensitively: BACK, cover, Folder
|
|
Expect(imgFiles[0]).To(Equal("Artist/Album/BACK.jpg"))
|
|
Expect(imgFiles[1]).To(Equal("Artist/Album/cover.jpg"))
|
|
Expect(imgFiles[2]).To(Equal("Artist/Album/Folder.jpg"))
|
|
})
|
|
|
|
It("includes images from parent folder for multi-disc albums", func() {
|
|
// Simulates: Artist/Album/cover.jpg with tracks in Artist/Album/CD1/ and Artist/Album/CD2/
|
|
repo.result = []model.Folder{
|
|
{
|
|
ID: "folder1",
|
|
Path: "Artist/Album",
|
|
Name: "CD1",
|
|
ParentID: "parentFolder",
|
|
ImagesUpdatedAt: now,
|
|
ImageFiles: []string{},
|
|
},
|
|
{
|
|
ID: "folder2",
|
|
Path: "Artist/Album",
|
|
Name: "CD2",
|
|
ParentID: "parentFolder",
|
|
ImagesUpdatedAt: now,
|
|
ImageFiles: []string{},
|
|
},
|
|
}
|
|
repo.parentResult = &model.Folder{
|
|
ID: "parentFolder",
|
|
Path: "Artist",
|
|
Name: "Album",
|
|
ParentID: "artistFolder",
|
|
ImagesUpdatedAt: expectedAt,
|
|
ImageFiles: []string{"cover.jpg", "back.jpg"},
|
|
}
|
|
|
|
_, imgFiles, imagesUpdatedAt, err := loadAlbumFoldersPaths(ctx, ds, album)
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(*imagesUpdatedAt).To(Equal(expectedAt))
|
|
Expect(imgFiles).To(HaveLen(2))
|
|
Expect(imgFiles[0]).To(Equal("Artist/Album/back.jpg"))
|
|
Expect(imgFiles[1]).To(Equal("Artist/Album/cover.jpg"))
|
|
})
|
|
|
|
It("does not query parent when parent ID is already in album folders", func() {
|
|
// When the parent folder is already one of the album's folders, skip it
|
|
repo.result = []model.Folder{
|
|
{
|
|
ID: "folder1",
|
|
Path: "Artist",
|
|
Name: "Album",
|
|
ParentID: "folder2",
|
|
ImagesUpdatedAt: now,
|
|
ImageFiles: []string{"cover.jpg"},
|
|
},
|
|
{
|
|
ID: "folder2",
|
|
Path: "",
|
|
Name: "Artist",
|
|
ImagesUpdatedAt: now,
|
|
ImageFiles: []string{},
|
|
},
|
|
}
|
|
|
|
_, imgFiles, _, err := loadAlbumFoldersPaths(ctx, ds, album)
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(imgFiles).To(HaveLen(1))
|
|
Expect(imgFiles[0]).To(Equal("Artist/Album/cover.jpg"))
|
|
// Get should not have been called (parent already in folder set)
|
|
Expect(repo.getCallCount).To(Equal(0))
|
|
})
|
|
|
|
It("does not query parent when folders have different parents", func() {
|
|
// When album folders span different parents, don't search any parent
|
|
repo.result = []model.Folder{
|
|
{
|
|
ID: "folder1",
|
|
Path: "Artist1/Album",
|
|
Name: "part1",
|
|
ParentID: "parentA",
|
|
ImagesUpdatedAt: now,
|
|
ImageFiles: []string{"cover.jpg"},
|
|
},
|
|
{
|
|
ID: "folder2",
|
|
Path: "Artist2/Album",
|
|
Name: "part2",
|
|
ParentID: "parentB",
|
|
ImagesUpdatedAt: now,
|
|
ImageFiles: []string{},
|
|
},
|
|
}
|
|
|
|
_, imgFiles, _, err := loadAlbumFoldersPaths(ctx, ds, album)
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(imgFiles).To(HaveLen(1))
|
|
Expect(imgFiles[0]).To(Equal("Artist1/Album/part1/cover.jpg"))
|
|
// Get should not have been called (different parents)
|
|
Expect(repo.getCallCount).To(Equal(0))
|
|
})
|
|
|
|
It("does not include library root parent for multi-folder albums", func() {
|
|
// Two album parts directly under the library root — parent is the root itself
|
|
repo.result = []model.Folder{
|
|
{
|
|
ID: "folder1",
|
|
Path: ".",
|
|
Name: "AlbumPart1",
|
|
ParentID: "rootFolder",
|
|
ImagesUpdatedAt: now,
|
|
ImageFiles: []string{"cover.jpg"},
|
|
},
|
|
{
|
|
ID: "folder2",
|
|
Path: ".",
|
|
Name: "AlbumPart2",
|
|
ParentID: "rootFolder",
|
|
ImagesUpdatedAt: now,
|
|
ImageFiles: []string{},
|
|
},
|
|
}
|
|
repo.parentResult = &model.Folder{
|
|
ID: "rootFolder",
|
|
Path: "",
|
|
Name: ".",
|
|
ParentID: "",
|
|
ImageFiles: []string{"unrelated.jpg"},
|
|
}
|
|
|
|
_, imgFiles, _, err := loadAlbumFoldersPaths(ctx, ds, album)
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(imgFiles).To(HaveLen(1))
|
|
Expect(imgFiles[0]).To(Equal("AlbumPart1/cover.jpg"))
|
|
Expect(repo.getCallCount).To(Equal(1))
|
|
})
|
|
|
|
It("includes top-level album folder for multi-disc albums", func() {
|
|
// Album folder directly under library root, with disc subfolders
|
|
repo.result = []model.Folder{
|
|
{
|
|
ID: "folder1",
|
|
Path: "Album",
|
|
Name: "Disc1",
|
|
ParentID: "albumFolder",
|
|
ImagesUpdatedAt: now,
|
|
ImageFiles: []string{"folder.jpg"},
|
|
},
|
|
{
|
|
ID: "folder2",
|
|
Path: "Album",
|
|
Name: "Disc2",
|
|
ParentID: "albumFolder",
|
|
ImagesUpdatedAt: now,
|
|
ImageFiles: []string{"folder.jpg"},
|
|
},
|
|
}
|
|
repo.parentResult = &model.Folder{
|
|
ID: "albumFolder",
|
|
Path: ".",
|
|
Name: "Album",
|
|
ParentID: "rootFolder",
|
|
ImagesUpdatedAt: expectedAt,
|
|
ImageFiles: []string{"cover.jpg"},
|
|
}
|
|
|
|
_, imgFiles, imagesUpdatedAt, err := loadAlbumFoldersPaths(ctx, ds, album)
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(*imagesUpdatedAt).To(Equal(expectedAt))
|
|
Expect(imgFiles).To(HaveLen(3))
|
|
Expect(imgFiles[0]).To(Equal("Album/cover.jpg"))
|
|
Expect(imgFiles[1]).To(Equal("Album/Disc1/folder.jpg"))
|
|
Expect(imgFiles[2]).To(Equal("Album/Disc2/folder.jpg"))
|
|
Expect(repo.getCallCount).To(Equal(1))
|
|
})
|
|
|
|
It("does not query parent for single-folder albums that already have images", func() {
|
|
repo.result = []model.Folder{
|
|
{
|
|
ID: "folder1",
|
|
Path: "Artist",
|
|
Name: "Album",
|
|
ParentID: "artistFolder",
|
|
ImagesUpdatedAt: now,
|
|
ImageFiles: []string{"cover.jpg"},
|
|
},
|
|
}
|
|
|
|
_, imgFiles, _, err := loadAlbumFoldersPaths(ctx, ds, album)
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(imgFiles).To(HaveLen(1))
|
|
Expect(imgFiles[0]).To(Equal("Artist/Album/cover.jpg"))
|
|
Expect(repo.getCallCount).To(Equal(0))
|
|
})
|
|
|
|
It("includes parent images for single-disc-subfolder albums", func() {
|
|
repo.result = []model.Folder{
|
|
{
|
|
ID: "folder1",
|
|
Path: "Artist/Album",
|
|
Name: "disc1",
|
|
ParentID: "albumFolder",
|
|
ImagesUpdatedAt: now,
|
|
ImageFiles: []string{},
|
|
},
|
|
}
|
|
repo.parentResult = &model.Folder{
|
|
ID: "albumFolder",
|
|
Path: "Artist",
|
|
Name: "Album",
|
|
ParentID: "artistFolder",
|
|
ImagesUpdatedAt: expectedAt,
|
|
ImageFiles: []string{"cover.jpg"},
|
|
}
|
|
|
|
_, imgFiles, imagesUpdatedAt, err := loadAlbumFoldersPaths(ctx, ds, album)
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(*imagesUpdatedAt).To(Equal(expectedAt))
|
|
Expect(imgFiles).To(HaveLen(1))
|
|
Expect(imgFiles[0]).To(Equal("Artist/Album/cover.jpg"))
|
|
Expect(repo.getCallCount).To(Equal(1))
|
|
})
|
|
|
|
It("propagates non-ErrNotFound errors from parent folder lookup", func() {
|
|
repo.result = []model.Folder{
|
|
{
|
|
ID: "folder1",
|
|
Path: "Artist/Album",
|
|
Name: "CD1",
|
|
ParentID: "parentFolder",
|
|
ImagesUpdatedAt: now,
|
|
ImageFiles: []string{"cover.jpg"},
|
|
},
|
|
{
|
|
ID: "folder2",
|
|
Path: "Artist/Album",
|
|
Name: "CD2",
|
|
ParentID: "parentFolder",
|
|
ImagesUpdatedAt: now,
|
|
ImageFiles: []string{},
|
|
},
|
|
}
|
|
repo.getErr = errors.New("db connection failed")
|
|
|
|
_, _, _, err := loadAlbumFoldersPaths(ctx, ds, album)
|
|
|
|
Expect(err).To(MatchError("db connection failed"))
|
|
Expect(repo.getCallCount).To(Equal(1))
|
|
})
|
|
|
|
It("continues gracefully when parent folder is not found", func() {
|
|
// Parent folder may have been deleted; should log a warning and continue
|
|
repo.result = []model.Folder{
|
|
{
|
|
ID: "folder1",
|
|
Path: "Artist/Album",
|
|
Name: "CD1",
|
|
ParentID: "missingParent",
|
|
ImagesUpdatedAt: now,
|
|
ImageFiles: []string{"cover.jpg"},
|
|
},
|
|
{
|
|
ID: "folder2",
|
|
Path: "Artist/Album",
|
|
Name: "CD2",
|
|
ParentID: "missingParent",
|
|
ImagesUpdatedAt: now,
|
|
ImageFiles: []string{},
|
|
},
|
|
}
|
|
// parentResult is nil, so Get will return ErrNotFound
|
|
|
|
_, imgFiles, _, err := loadAlbumFoldersPaths(ctx, ds, album)
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(imgFiles).To(HaveLen(1))
|
|
Expect(imgFiles[0]).To(Equal("Artist/Album/CD1/cover.jpg"))
|
|
Expect(repo.getCallCount).To(Equal(1))
|
|
})
|
|
})
|
|
})
|