* 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. Fixes navidrome/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>
190 lines
5.8 KiB
Go
190 lines
5.8 KiB
Go
package artwork
|
|
|
|
import (
|
|
"cmp"
|
|
"context"
|
|
"crypto/md5"
|
|
"errors"
|
|
"fmt"
|
|
"io"
|
|
"path/filepath"
|
|
"slices"
|
|
"strings"
|
|
"time"
|
|
|
|
"github.com/Masterminds/squirrel"
|
|
"github.com/navidrome/navidrome/conf"
|
|
"github.com/navidrome/navidrome/core"
|
|
"github.com/navidrome/navidrome/core/external"
|
|
"github.com/navidrome/navidrome/core/ffmpeg"
|
|
"github.com/navidrome/navidrome/log"
|
|
"github.com/navidrome/navidrome/model"
|
|
"github.com/navidrome/navidrome/utils/natural"
|
|
)
|
|
|
|
type albumArtworkReader struct {
|
|
cacheKey
|
|
a *artwork
|
|
provider external.Provider
|
|
album model.Album
|
|
updatedAt *time.Time
|
|
imgFiles []string
|
|
rootFolder string
|
|
}
|
|
|
|
func newAlbumArtworkReader(ctx context.Context, artwork *artwork, artID model.ArtworkID, provider external.Provider) (*albumArtworkReader, error) {
|
|
al, err := artwork.ds.Album(ctx).Get(artID.ID)
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
_, imgFiles, imagesUpdateAt, err := loadAlbumFoldersPaths(ctx, artwork.ds, *al)
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
a := &albumArtworkReader{
|
|
a: artwork,
|
|
provider: provider,
|
|
album: *al,
|
|
updatedAt: imagesUpdateAt,
|
|
imgFiles: imgFiles,
|
|
rootFolder: core.AbsolutePath(ctx, artwork.ds, al.LibraryID, ""),
|
|
}
|
|
a.cacheKey.artID = artID
|
|
if a.updatedAt != nil && a.updatedAt.After(al.UpdatedAt) {
|
|
a.cacheKey.lastUpdate = *a.updatedAt
|
|
} else {
|
|
a.cacheKey.lastUpdate = al.UpdatedAt
|
|
}
|
|
return a, nil
|
|
}
|
|
|
|
func (a *albumArtworkReader) Key() string {
|
|
hashInput := conf.Server.CoverArtPriority
|
|
if conf.Server.EnableExternalServices {
|
|
hashInput = conf.Server.Agents + hashInput
|
|
}
|
|
hash := md5.Sum([]byte(hashInput))
|
|
return fmt.Sprintf(
|
|
"%s.%x.%t",
|
|
a.cacheKey.Key(),
|
|
hash,
|
|
conf.Server.EnableExternalServices,
|
|
)
|
|
}
|
|
func (a *albumArtworkReader) LastUpdated() time.Time {
|
|
return a.lastUpdate
|
|
}
|
|
|
|
func (a *albumArtworkReader) Reader(ctx context.Context) (io.ReadCloser, string, error) {
|
|
var ff = a.fromCoverArtPriority(ctx, a.a.ffmpeg, conf.Server.CoverArtPriority)
|
|
return selectImageReader(ctx, a.artID, ff...)
|
|
}
|
|
|
|
func (a *albumArtworkReader) fromCoverArtPriority(ctx context.Context, ffmpeg ffmpeg.FFmpeg, priority string) []sourceFunc {
|
|
var ff []sourceFunc
|
|
for pattern := range strings.SplitSeq(strings.ToLower(priority), ",") {
|
|
pattern = strings.TrimSpace(pattern)
|
|
switch {
|
|
case pattern == "embedded":
|
|
embedArtPath := filepath.Join(a.rootFolder, a.album.EmbedArtPath)
|
|
ff = append(ff, fromTag(ctx, embedArtPath), fromFFmpegTag(ctx, ffmpeg, embedArtPath))
|
|
case pattern == "external":
|
|
ff = append(ff, fromAlbumExternalSource(ctx, a.album, a.provider))
|
|
case len(a.imgFiles) > 0:
|
|
ff = append(ff, fromExternalFile(ctx, a.imgFiles, pattern))
|
|
}
|
|
}
|
|
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}})
|
|
if err != nil {
|
|
return nil, nil, nil, err
|
|
}
|
|
|
|
folderIDSet := make(map[string]bool, len(folderIDs))
|
|
for _, id := range folderIDs {
|
|
folderIDSet[id] = true
|
|
}
|
|
|
|
// For multi-disc albums (2+ folders), check if all folders share a common parent
|
|
// that is not already included. This finds cover art in the album root folder
|
|
// (e.g., "Artist/Album/cover.jpg" when tracks are in "Artist/Album/CD1/" and "Artist/Album/CD2/").
|
|
// We skip single-folder albums to avoid pulling images from the artist folder.
|
|
if commonParentID := commonParentFolder(folders, folderIDSet); commonParentID != "" {
|
|
parentFolder, err := ds.Folder(ctx).Get(commonParentID)
|
|
if errors.Is(err, model.ErrNotFound) {
|
|
log.Warn(ctx, "Parent folder not found for album cover art lookup", "parentID", commonParentID)
|
|
} else if err != nil {
|
|
return nil, nil, nil, err
|
|
}
|
|
if parentFolder != nil {
|
|
folders = append(folders, *parentFolder)
|
|
}
|
|
}
|
|
|
|
var paths []string
|
|
var imgFiles []string
|
|
var updatedAt time.Time
|
|
for _, f := range folders {
|
|
path := f.AbsolutePath()
|
|
paths = append(paths, path)
|
|
if f.ImagesUpdatedAt.After(updatedAt) {
|
|
updatedAt = f.ImagesUpdatedAt
|
|
}
|
|
for _, img := range f.ImageFiles {
|
|
imgFiles = append(imgFiles, filepath.Join(path, 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
|
|
}
|
|
|
|
// commonParentFolder returns the shared parent folder ID when all folders have the
|
|
// same parent and that parent is not already in folderIDSet. Returns "" otherwise.
|
|
func commonParentFolder(folders []model.Folder, folderIDSet map[string]bool) string {
|
|
if len(folders) < 2 {
|
|
return ""
|
|
}
|
|
parentID := folders[0].ParentID
|
|
if parentID == "" || folderIDSet[parentID] {
|
|
return ""
|
|
}
|
|
for _, f := range folders[1:] {
|
|
if f.ParentID != parentID {
|
|
return ""
|
|
}
|
|
}
|
|
return parentID
|
|
}
|
|
|
|
// compareImageFiles compares two image file paths for sorting.
|
|
// It extracts the base filename (without extension) and compares case-insensitively.
|
|
// This ensures that "cover.jpg" sorts before "cover.1.jpg" since "cover" < "cover.1".
|
|
// Note: This function is called O(n log n) times during sorting, but in practice albums
|
|
// typically have only 1-20 image files, making the repeated string operations negligible.
|
|
func compareImageFiles(a, b string) int {
|
|
// Case-insensitive comparison
|
|
a = strings.ToLower(a)
|
|
b = strings.ToLower(b)
|
|
|
|
// Extract base filenames without extensions
|
|
baseA := strings.TrimSuffix(filepath.Base(a), filepath.Ext(a))
|
|
baseB := strings.TrimSuffix(filepath.Base(b), filepath.Ext(b))
|
|
|
|
// Compare base names first, then full paths if equal
|
|
return cmp.Or(
|
|
natural.Compare(baseA, baseB),
|
|
natural.Compare(a, b),
|
|
)
|
|
}
|