feat(server): serve artwork from persisted state with content-hash caching

This commit is contained in:
Deluan 2026-07-22 21:37:04 -04:00
parent 313998fd65
commit 25b32f9706
14 changed files with 456 additions and 62 deletions

View File

@ -91,7 +91,14 @@ func CreateSubsonicAPIRouter(ctx context.Context) *subsonic.Router {
sqlDB := db.Db()
dataStore := persistence.New(sqlDB)
fileCache := artwork.GetImageCache()
imageStore := artwork.ProvideImageStore()
fFmpeg := ffmpeg.New()
service := artwork.NewService(dataStore, fileCache, imageStore, fFmpeg)
transcodingCache := stream.GetTranscodingCache()
mediaStreamer := stream.NewMediaStreamer(dataStore, fFmpeg, transcodingCache)
share := core.NewShare(dataStore)
archiver := core.NewArchiver(mediaStreamer, dataStore, share)
players := core.NewPlayers(dataStore)
broker := events.GetBroker()
metricsMetrics := metrics.GetPrometheusInstance(dataStore)
manager := plugins.GetManager(dataStore, broker, metricsMetrics)
@ -99,11 +106,6 @@ func CreateSubsonicAPIRouter(ctx context.Context) *subsonic.Router {
matcherMatcher := matcher.New(dataStore)
provider := external.NewProvider(dataStore, agentsAgents, matcherMatcher)
artworkArtwork := artwork.NewArtwork(dataStore, fileCache, fFmpeg, provider)
transcodingCache := stream.GetTranscodingCache()
mediaStreamer := stream.NewMediaStreamer(dataStore, fFmpeg, transcodingCache)
share := core.NewShare(dataStore)
archiver := core.NewArchiver(mediaStreamer, dataStore, share)
players := core.NewPlayers(dataStore)
cacheWarmer := artwork.NewCacheWarmer(artworkArtwork, fileCache)
imageUploadService := core.NewImageUploadService()
playlistsPlaylists := playlists.NewPlaylists(dataStore, imageUploadService)
@ -113,7 +115,7 @@ func CreateSubsonicAPIRouter(ctx context.Context) *subsonic.Router {
lyricsLyrics := lyrics.NewLyrics(dataStore, manager)
transcodeDecider := stream.NewTranscodeDecider(dataStore, fFmpeg)
sonicSonic := sonic.New(dataStore, manager, matcherMatcher)
router := subsonic.New(dataStore, artworkArtwork, mediaStreamer, archiver, players, provider, modelScanner, broker, playlistsPlaylists, playTracker, share, playbackServer, metricsMetrics, lyricsLyrics, transcodeDecider, sonicSonic)
router := subsonic.New(dataStore, service, mediaStreamer, archiver, players, provider, modelScanner, broker, playlistsPlaylists, playTracker, share, playbackServer, metricsMetrics, lyricsLyrics, transcodeDecider, sonicSonic)
return router
}
@ -121,24 +123,25 @@ func CreateJellyfinAPIRouter(ctx context.Context) *jellyfin.Router {
sqlDB := db.Db()
dataStore := persistence.New(sqlDB)
fileCache := artwork.GetImageCache()
imageStore := artwork.ProvideImageStore()
fFmpeg := ffmpeg.New()
broker := events.GetBroker()
metricsMetrics := metrics.GetPrometheusInstance(dataStore)
manager := plugins.GetManager(dataStore, broker, metricsMetrics)
agentsAgents := agents.GetAgents(dataStore, manager)
matcherMatcher := matcher.New(dataStore)
provider := external.NewProvider(dataStore, agentsAgents, matcherMatcher)
artworkArtwork := artwork.NewArtwork(dataStore, fileCache, fFmpeg, provider)
service := artwork.NewService(dataStore, fileCache, imageStore, fFmpeg)
transcodingCache := stream.GetTranscodingCache()
mediaStreamer := stream.NewMediaStreamer(dataStore, fFmpeg, transcodingCache)
transcodeDecider := stream.NewTranscodeDecider(dataStore, fFmpeg)
players := core.NewPlayers(dataStore)
broker := events.GetBroker()
metricsMetrics := metrics.GetPrometheusInstance(dataStore)
manager := plugins.GetManager(dataStore, broker, metricsMetrics)
playTracker := scrobbler.GetPlayTracker(dataStore, broker, manager)
imageUploadService := core.NewImageUploadService()
playlistsPlaylists := playlists.NewPlaylists(dataStore, imageUploadService)
agentsAgents := agents.GetAgents(dataStore, manager)
matcherMatcher := matcher.New(dataStore)
provider := external.NewProvider(dataStore, agentsAgents, matcherMatcher)
sonicSonic := sonic.New(dataStore, manager, matcherMatcher)
lyricsLyrics := lyrics.NewLyrics(dataStore, manager)
router := jellyfin.New(dataStore, artworkArtwork, mediaStreamer, transcodeDecider, players, playTracker, playlistsPlaylists, provider, sonicSonic, lyricsLyrics, broker)
router := jellyfin.New(dataStore, service, mediaStreamer, transcodeDecider, players, playTracker, playlistsPlaylists, provider, sonicSonic, lyricsLyrics, broker)
return router
}
@ -146,19 +149,14 @@ func CreatePublicRouter() *public.Router {
sqlDB := db.Db()
dataStore := persistence.New(sqlDB)
fileCache := artwork.GetImageCache()
imageStore := artwork.ProvideImageStore()
fFmpeg := ffmpeg.New()
broker := events.GetBroker()
metricsMetrics := metrics.GetPrometheusInstance(dataStore)
manager := plugins.GetManager(dataStore, broker, metricsMetrics)
agentsAgents := agents.GetAgents(dataStore, manager)
matcherMatcher := matcher.New(dataStore)
provider := external.NewProvider(dataStore, agentsAgents, matcherMatcher)
artworkArtwork := artwork.NewArtwork(dataStore, fileCache, fFmpeg, provider)
service := artwork.NewService(dataStore, fileCache, imageStore, fFmpeg)
transcodingCache := stream.GetTranscodingCache()
mediaStreamer := stream.NewMediaStreamer(dataStore, fFmpeg, transcodingCache)
share := core.NewShare(dataStore)
archiver := core.NewArchiver(mediaStreamer, dataStore, share)
router := public.New(dataStore, artworkArtwork, mediaStreamer, share, archiver)
router := public.New(dataStore, service, mediaStreamer, share, archiver)
return router
}

61
server/imghttp/headers.go Normal file
View File

@ -0,0 +1,61 @@
// Package imghttp holds the shared HTTP caching contract for artwork responses, so the
// subsonic, public, and jellyfin image handlers apply identical headers without importing
// each other.
package imghttp
import (
"net/http"
"strings"
"github.com/navidrome/navidrome/core/artwork"
)
// WriteImageHeaders applies the artwork caching contract and reports whether a 304 was written
// (in which case the caller must not write a body). requestedHash is the hash the client asserted
// (id suffix / JWT payload / jellyfin tag param), or "" when the request carried no hash.
func WriteImageHeaders(w http.ResponseWriter, r *http.Request, img *artwork.Image, requestedHash string) (wrote304 bool) {
h := w.Header()
// Placeholders are transient stand-ins for not-yet-resolved art: never cached, no validators.
if img.Placeholder {
h.Set("Cache-Control", "no-store")
return false
}
h.Set("ETag", `"`+img.Hash+`"`)
if !img.LastUpdated.IsZero() {
h.Set("Last-Modified", img.LastUpdated.UTC().Format(http.TimeFormat))
}
// Immutable only when the client asked for the exact current hash; bare/legacy/mismatched
// requests get cheap ETag revalidation instead, which fixes stale art after re-resolution.
if requestedHash != "" && requestedHash == img.Hash {
h.Set("Cache-Control", "public, max-age=31536000, immutable")
} else {
h.Set("Cache-Control", "public, no-cache")
}
if ifNoneMatch(r.Header.Get("If-None-Match"), img.Hash) {
w.WriteHeader(http.StatusNotModified)
return true
}
return false
}
// ifNoneMatch reports whether the If-None-Match header asserts the given hash, using weak
// comparison (RFC 9110): "*" matches any current representation and W/ prefixes are ignored.
func ifNoneMatch(header, hash string) bool {
header = strings.TrimSpace(header)
if header == "" {
return false
}
if header == "*" {
return true
}
for _, tag := range strings.Split(header, ",") {
tag = strings.TrimSpace(tag)
tag = strings.TrimPrefix(tag, "W/")
if strings.Trim(tag, `"`) == hash {
return true
}
}
return false
}

View File

@ -0,0 +1,166 @@
package imghttp_test
import (
"io"
"net/http"
"net/http/httptest"
"strings"
"testing"
"time"
"github.com/navidrome/navidrome/core/artwork"
"github.com/navidrome/navidrome/server/imghttp"
)
const testHash = "0123456789abcdef"
var lastMod = time.Date(2024, 1, 2, 3, 4, 5, 0, time.UTC)
func found() *artwork.Image {
return &artwork.Image{
ReadCloser: io.NopCloser(strings.NewReader("IMG")),
Hash: testHash,
LastUpdated: lastMod,
}
}
func placeholder() *artwork.Image {
return &artwork.Image{ReadCloser: io.NopCloser(strings.NewReader("PH")), Placeholder: true}
}
func TestWriteImageHeaders(t *testing.T) {
tests := []struct {
name string
img *artwork.Image
requestedHash string
ifNoneMatch string
want304 bool
wantCache string
wantETag string
wantLastMod bool
}{
{
name: "placeholder is never cached and carries no validators",
img: placeholder(),
wantCache: "no-store",
},
{
name: "found with matching requested hash is immutable",
img: found(),
requestedHash: testHash,
wantCache: "public, max-age=31536000, immutable",
wantETag: `"` + testHash + `"`,
wantLastMod: true,
},
{
name: "found with bare id revalidates via no-cache",
img: found(),
wantCache: "public, no-cache",
wantETag: `"` + testHash + `"`,
wantLastMod: true,
},
{
name: "found with mismatched requested hash revalidates",
img: found(),
requestedHash: "ffffffffffffffff",
wantCache: "public, no-cache",
wantETag: `"` + testHash + `"`,
wantLastMod: true,
},
{
name: "If-None-Match matching the hash yields 304",
img: found(),
requestedHash: testHash,
ifNoneMatch: `"` + testHash + `"`,
want304: true,
wantCache: "public, max-age=31536000, immutable",
wantETag: `"` + testHash + `"`,
wantLastMod: true,
},
{
name: "weak If-None-Match matches (weak comparison)",
img: found(),
ifNoneMatch: `W/"` + testHash + `"`,
want304: true,
wantCache: "public, no-cache",
wantETag: `"` + testHash + `"`,
wantLastMod: true,
},
{
name: "If-None-Match with multiple values matches one",
img: found(),
ifNoneMatch: `"deadbeefdeadbeef", W/"` + testHash + `", "cafecafecafecafe"`,
want304: true,
wantCache: "public, no-cache",
wantETag: `"` + testHash + `"`,
wantLastMod: true,
},
{
name: "If-None-Match star matches any current representation",
img: found(),
ifNoneMatch: "*",
want304: true,
wantCache: "public, no-cache",
wantETag: `"` + testHash + `"`,
wantLastMod: true,
},
{
name: "non-matching If-None-Match serves body",
img: found(),
ifNoneMatch: `"deadbeefdeadbeef"`,
want304: false,
wantCache: "public, no-cache",
wantETag: `"` + testHash + `"`,
wantLastMod: true,
},
{
name: "placeholder ignores If-None-Match and never 304s",
img: placeholder(),
ifNoneMatch: "*",
want304: false,
wantCache: "no-store",
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
w := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/img", nil)
if tc.ifNoneMatch != "" {
r.Header.Set("If-None-Match", tc.ifNoneMatch)
}
got := imghttp.WriteImageHeaders(w, r, tc.img, tc.requestedHash)
if got != tc.want304 {
t.Fatalf("wrote304 = %v, want %v", got, tc.want304)
}
h := w.Header()
if got := h.Get("Cache-Control"); got != tc.wantCache {
t.Errorf("Cache-Control = %q, want %q", got, tc.wantCache)
}
if got := h.Get("ETag"); got != tc.wantETag {
t.Errorf("ETag = %q, want %q", got, tc.wantETag)
}
if tc.img.Placeholder {
if h.Get("ETag") != "" {
t.Errorf("placeholder must not set ETag, got %q", h.Get("ETag"))
}
if h.Get("Last-Modified") != "" {
t.Errorf("placeholder must not set Last-Modified, got %q", h.Get("Last-Modified"))
}
}
if hasLM := h.Get("Last-Modified") != ""; hasLM != tc.wantLastMod {
t.Errorf("has Last-Modified = %v, want %v", hasLM, tc.wantLastMod)
}
if tc.want304 {
if w.Code != http.StatusNotModified {
t.Errorf("status = %d, want 304", w.Code)
}
if w.Body.Len() != 0 {
t.Errorf("304 must have empty body, got %q", w.Body.String())
}
}
})
}
}

View File

@ -30,7 +30,7 @@ import (
type Router struct {
http.Handler
ds model.DataStore
artwork artwork.Artwork
artwork artwork.Service
streamer stream.MediaStreamer
transcodeDecider stream.TranscodeDecider
players core.Players
@ -46,7 +46,7 @@ type Router struct {
serverIDVal string
}
func New(ds model.DataStore, artwork artwork.Artwork, streamer stream.MediaStreamer,
func New(ds model.DataStore, artwork artwork.Service, streamer stream.MediaStreamer,
transcodeDecider stream.TranscodeDecider, players core.Players,
scrobbler scrobbler.PlayTracker, playlists playlists.Playlists, provider external.Provider,
sonicSvc sonic.Engine, lyricsSvc lyrics.Lyrics, broker events.Broker) *Router {

View File

@ -33,7 +33,6 @@ import (
"strings"
"testing"
"testing/fstest"
"time"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/conf/configtest"
@ -406,18 +405,18 @@ type spyArtwork struct {
data []byte
}
func (s *spyArtwork) Get(context.Context, model.ArtworkID, int, bool) (io.ReadCloser, time.Time, error) {
return nil, time.Time{}, model.ErrNotFound
func (s *spyArtwork) Get(context.Context, model.ArtworkID, int, bool) (*artwork.Image, error) {
return nil, model.ErrNotFound
}
func (s *spyArtwork) GetOrPlaceholder(c context.Context, id string, _ int, _ bool) (io.ReadCloser, time.Time, error) {
func (s *spyArtwork) GetOrPlaceholder(c context.Context, id string, _ int, _ bool) (*artwork.Image, error) {
s.lastID = id
s.lastCtx = c
d := s.data
if d == nil {
d = []byte("IMG")
}
return io.NopCloser(bytes.NewReader(d)), time.Time{}, nil
return &artwork.Image{ReadCloser: io.NopCloser(bytes.NewReader(d))}, nil
}
var _ artwork.Artwork = &spyArtwork{}
var _ artwork.Service = &spyArtwork{}

View File

@ -20,6 +20,7 @@ import (
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/request"
"github.com/navidrome/navidrome/server/imghttp"
"github.com/navidrome/navidrome/server/jellyfin/dto"
_ "golang.org/x/image/webp"
)
@ -32,7 +33,7 @@ func (api *Router) getItemImage(w http.ResponseWriter, r *http.Request) {
size, _ := strconv.Atoi(r.URL.Query().Get("maxwidth"))
artID := api.resolveArtworkID(ctx, itemId)
reader, _, err := api.artwork.GetOrPlaceholder(ctx, artID, size, false)
img, err := api.artwork.GetOrPlaceholder(ctx, artID, size, false)
switch {
case errors.Is(err, context.Canceled):
return
@ -41,9 +42,27 @@ func (api *Router) getItemImage(w http.ResponseWriter, r *http.Request) {
http.Error(w, "Not Found", http.StatusNotFound)
return
}
defer reader.Close()
defer img.Close()
if imghttp.WriteImageHeaders(w, r, img, hashFromTag(r)) {
return
}
// Leave Content-Type unset so net/http sniffs it (covers may be PNG/WebP/JPEG).
_, _ = io.Copy(w, reader)
_, _ = io.Copy(w, img)
}
// hashFromTag returns the ?tag query param when it is exactly a 16-char lowercase-hex content
// hash (what Finamp/Jellyfin clients append), so a matching request can be served immutable.
func hashFromTag(r *http.Request) string {
tag := r.URL.Query().Get("tag")
if len(tag) != 16 {
return ""
}
for _, c := range tag {
if !(c >= '0' && c <= '9' || c >= 'a' && c <= 'f') {
return ""
}
}
return tag
}
// resolveArtworkID maps a Jellyfin item id to a Navidrome ArtworkID, probing

View File

@ -29,20 +29,25 @@ import (
)
type fakeArtwork struct {
artwork.Artwork
artwork.Service
recvId string
recvCtx context.Context
data []byte
hash string
}
func (f *fakeArtwork) GetOrPlaceholder(ctx context.Context, id string, size int, square bool) (io.ReadCloser, time.Time, error) {
func (f *fakeArtwork) GetOrPlaceholder(ctx context.Context, id string, size int, square bool) (*artwork.Image, error) {
f.recvId = id
f.recvCtx = ctx
data := f.data
if data == nil {
data = []byte("IMG")
}
return io.NopCloser(bytes.NewReader(data)), time.Now(), nil
return &artwork.Image{
ReadCloser: io.NopCloser(bytes.NewReader(data)),
Hash: f.hash,
LastUpdated: time.Now(),
}, nil
}
func newImageRequest(itemId string) (*httptest.ResponseRecorder, *http.Request) {
@ -115,6 +120,38 @@ var _ = Describe("Images", func() {
Expect(ok).To(BeTrue())
Expect(u.IsAdmin).To(BeTrue())
})
It("serves immutable when the tag param asserts the current hash", func() {
const hash = "0123456789abcdef"
ds := &tests.MockDataStore{}
ds.Album(context.Background()).(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "a1", Name: "One"}})
fa := &fakeArtwork{hash: hash}
api := &Router{ds: ds, artwork: fa}
w, r := newImageRequest(dto.EncodeID("a1"))
q := r.URL.Query()
q.Set("tag", hash)
r.URL.RawQuery = q.Encode()
api.getItemImage(w, r)
Expect(w.Code).To(Equal(http.StatusOK))
Expect(w.Header().Get("Cache-Control")).To(Equal("public, max-age=31536000, immutable"))
Expect(w.Header().Get("ETag")).To(Equal(`"` + hash + `"`))
})
It("revalidates via no-cache when no tag is provided", func() {
const hash = "0123456789abcdef"
ds := &tests.MockDataStore{}
ds.Album(context.Background()).(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "a1", Name: "One"}})
fa := &fakeArtwork{hash: hash}
api := &Router{ds: ds, artwork: fa}
w, r := newImageRequest(dto.EncodeID("a1"))
api.getItemImage(w, r)
Expect(w.Code).To(Equal(http.StatusOK))
Expect(w.Header().Get("Cache-Control")).To(Equal("public, no-cache"))
})
})
// Real image fixtures: postItemImage validates uploads by decoding them.

View File

@ -11,6 +11,7 @@ import (
"github.com/navidrome/navidrome/core/auth"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/server/imghttp"
"github.com/navidrome/navidrome/utils/req"
)
@ -40,7 +41,7 @@ func (pub *Router) handleImages(w http.ResponseWriter, r *http.Request) {
size := p.IntOr("size", 0)
square := p.BoolOr("square", false)
imgReader, lastUpdate, err := pub.artwork.Get(ctx, artId, size, square)
img, err := pub.artwork.Get(ctx, artId, size, square)
switch {
case errors.Is(err, context.Canceled):
return
@ -58,10 +59,11 @@ func (pub *Router) handleImages(w http.ResponseWriter, r *http.Request) {
return
}
defer imgReader.Close()
w.Header().Set("Cache-Control", "public, max-age=315360000")
w.Header().Set("Last-Modified", lastUpdate.Format(http.TimeFormat))
cnt, err := io.Copy(w, imgReader)
defer img.Close()
if imghttp.WriteImageHeaders(w, r, img, artId.Hash) {
return
}
cnt, err := io.Copy(w, img)
if err != nil {
log.Warn(ctx, "Error sending image", "count", cnt, err)
}

View File

@ -1,8 +1,17 @@
package public
import (
"bytes"
"context"
"io"
"net/http"
"net/http/httptest"
"net/url"
"github.com/go-chi/jwtauth/v5"
"github.com/navidrome/navidrome/core/artwork"
"github.com/navidrome/navidrome/core/auth"
"github.com/navidrome/navidrome/model"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
@ -23,3 +32,49 @@ var _ = Describe("decodeArtworkID", func() {
Expect(err).To(HaveOccurred())
})
})
type fakeArtwork struct {
artwork.Service
img *artwork.Image
err error
}
func (f *fakeArtwork) Get(context.Context, model.ArtworkID, int, bool) (*artwork.Image, error) {
return f.img, f.err
}
var _ = Describe("handleImages", func() {
var w *httptest.ResponseRecorder
newImageRequest := func(claimID string) *http.Request {
auth.TokenAuth = jwtauth.New("HS256", []byte("super secret"), nil)
token, _ := auth.CreatePublicToken(auth.Claims{ID: claimID})
return httptest.NewRequest("GET", "/img?:id="+url.QueryEscape(token), nil)
}
BeforeEach(func() {
w = httptest.NewRecorder()
})
It("returns 404 when the artwork is unavailable", func() {
pub := &Router{artwork: &fakeArtwork{err: artwork.ErrUnavailable}}
pub.handleImages(w, newImageRequest("al-1"))
Expect(w.Code).To(Equal(http.StatusNotFound))
})
It("returns 404 when the artwork is not found", func() {
pub := &Router{artwork: &fakeArtwork{err: model.ErrNotFound}}
pub.handleImages(w, newImageRequest("al-1"))
Expect(w.Code).To(Equal(http.StatusNotFound))
})
It("serves the image immutable when the token asserts the current hash", func() {
const hash = "0123456789abcdef"
img := &artwork.Image{ReadCloser: io.NopCloser(bytes.NewReader([]byte("IMG"))), Hash: hash}
pub := &Router{artwork: &fakeArtwork{img: img}}
pub.handleImages(w, newImageRequest("al-1_"+hash))
Expect(w.Code).To(Equal(http.StatusOK))
Expect(w.Header().Get("Cache-Control")).To(Equal("public, max-age=31536000, immutable"))
Expect(w.Header().Get("ETag")).To(Equal(`"` + hash + `"`))
})
})

View File

@ -18,7 +18,7 @@ import (
type Router struct {
http.Handler
artwork artwork.Artwork
artwork artwork.Service
streamer stream.MediaStreamer
archiver core.Archiver
share core.Share
@ -26,7 +26,7 @@ type Router struct {
ds model.DataStore
}
func New(ds model.DataStore, artwork artwork.Artwork, streamer stream.MediaStreamer, share core.Share, archiver core.Archiver) *Router {
func New(ds model.DataStore, artwork artwork.Service, streamer stream.MediaStreamer, share core.Share, archiver core.Archiver) *Router {
p := &Router{ds: ds, artwork: artwork, streamer: streamer, share: share, archiver: archiver}
shareRoot := path.Join(conf.Server.BasePath, consts.URLPathPublic)
p.assetsHandler = http.StripPrefix(shareRoot, http.FileServer(http.FS(ui.BuildAssets())))

View File

@ -40,7 +40,7 @@ type handlerRaw = func(http.ResponseWriter, *http.Request) (*responses.Subsonic,
type Router struct {
http.Handler
ds model.DataStore
artwork artwork.Artwork
artwork artwork.Service
streamer stream.MediaStreamer
archiver core.Archiver
players core.Players
@ -57,7 +57,7 @@ type Router struct {
sonic *sonicsvc.Sonic
}
func New(ds model.DataStore, artwork artwork.Artwork, streamer stream.MediaStreamer, archiver core.Archiver,
func New(ds model.DataStore, artwork artwork.Service, streamer stream.MediaStreamer, archiver core.Archiver,
players core.Players, provider external.Provider, scanner model.Scanner, broker events.Broker,
playlists playlistsvc.Playlists, scrobbler scrobbler.PlayTracker, share core.Share, playback playback.PlaybackServer,
metrics metrics.Metrics, lyrics lyricssvc.Lyrics, transcodeDecision stream.TranscodeDecider,

View File

@ -308,15 +308,15 @@ func parseJSONResponse(w *httptest.ResponseRecorder) *responses.Subsonic {
// --- Noop stub implementations for Router dependencies ---
// noopArtwork implements artwork.Artwork
// noopArtwork implements artwork.Service
type noopArtwork struct{}
func (n noopArtwork) Get(context.Context, model.ArtworkID, int, bool) (io.ReadCloser, time.Time, error) {
return nil, time.Time{}, model.ErrNotFound
func (n noopArtwork) Get(context.Context, model.ArtworkID, int, bool) (*artwork.Image, error) {
return nil, model.ErrNotFound
}
func (n noopArtwork) GetOrPlaceholder(_ context.Context, _ string, _ int, _ bool) (io.ReadCloser, time.Time, error) {
return io.NopCloser(io.LimitReader(nil, 0)), time.Time{}, nil
func (n noopArtwork) GetOrPlaceholder(_ context.Context, _ string, _ int, _ bool) (*artwork.Image, error) {
return &artwork.Image{ReadCloser: io.NopCloser(io.LimitReader(nil, 0))}, nil
}
// noopArchiver implements core.Archiver
@ -371,7 +371,7 @@ func (n noopProvider) AlbumImage(context.Context, string) (*url.URL, error) {
// Compile-time interface checks
var (
_ artwork.Artwork = noopArtwork{}
_ artwork.Service = noopArtwork{}
_ core.Archiver = noopArchiver{}
_ external.Provider = noopProvider{}
)

View File

@ -13,6 +13,7 @@ import (
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/resources"
"github.com/navidrome/navidrome/server/imghttp"
"github.com/navidrome/navidrome/server/subsonic/responses"
"github.com/navidrome/navidrome/utils/gravatar"
"github.com/navidrome/navidrome/utils/req"
@ -66,7 +67,7 @@ func (api *Router) GetCoverArt(w http.ResponseWriter, r *http.Request) (*respons
size := p.IntOr("size", 0)
square := p.BoolOr("square", false)
imgReader, lastUpdate, err := api.artwork.GetOrPlaceholder(ctx, id, size, square)
img, err := api.artwork.GetOrPlaceholder(ctx, id, size, square)
switch {
case errors.Is(err, context.Canceled):
return nil, nil
@ -77,12 +78,13 @@ func (api *Router) GetCoverArt(w http.ResponseWriter, r *http.Request) (*respons
log.Error(r, "Error retrieving coverArt", "id", id, err)
return nil, err
}
defer img.Close()
defer imgReader.Close()
w.Header().Set("cache-control", "public, max-age=315360000")
w.Header().Set("last-modified", lastUpdate.Format(http.TimeFormat))
cnt, err := io.Copy(w, imgReader)
artID, _ := model.ParseArtworkID(id)
if imghttp.WriteImageHeaders(w, r, img, artID.Hash) {
return nil, nil
}
cnt, err := io.Copy(w, img)
if err != nil {
log.Warn(ctx, "Error sending image", "count", cnt, err)
}

View File

@ -108,6 +108,53 @@ var _ = Describe("MediaRetrievalController", func() {
Expect(w.Body.String()).To(BeEmpty())
})
})
Describe("caching headers", func() {
const hash = "0123456789abcdef"
It("sets an ETag and no-cache for a bare id", func() {
artwork.hash = hash
r := newGetRequest("id=al-34")
_, err := router.GetCoverArt(w, r)
Expect(err).ToNot(HaveOccurred())
Expect(w.Header().Get("ETag")).To(Equal(`"` + hash + `"`))
Expect(w.Header().Get("Cache-Control")).To(Equal("public, no-cache"))
Expect(w.Body.String()).To(Equal(artwork.data))
})
It("marks the response immutable when the id asserts the current hash", func() {
artwork.hash = hash
r := newGetRequest("id=al-34_" + hash)
_, err := router.GetCoverArt(w, r)
Expect(err).ToNot(HaveOccurred())
Expect(w.Header().Get("Cache-Control")).To(Equal("public, max-age=31536000, immutable"))
})
It("returns 304 with no body when If-None-Match matches", func() {
artwork.hash = hash
r := newGetRequest("id=al-34")
r.Header.Set("If-None-Match", `"`+hash+`"`)
_, err := router.GetCoverArt(w, r)
Expect(err).ToNot(HaveOccurred())
Expect(w.Code).To(Equal(304))
Expect(w.Body.Len()).To(BeZero())
})
It("never caches a placeholder", func() {
artwork.placeholder = true
r := newGetRequest("id=al-missing")
_, err := router.GetCoverArt(w, r)
Expect(err).ToNot(HaveOccurred())
Expect(w.Code).To(Equal(200))
Expect(w.Header().Get("Cache-Control")).To(Equal("no-store"))
Expect(w.Header().Get("ETag")).To(BeEmpty())
Expect(w.Body.String()).To(Equal(artwork.data))
})
})
})
Describe("GetLyrics", func() {
@ -186,8 +233,11 @@ var _ = Describe("MediaRetrievalController", func() {
})
type fakeArtwork struct {
artwork.Artwork
artwork.Service
data string
hash string
lastUpdated time.Time
placeholder bool
err error
ctxCancelFunc func()
recvId string
@ -195,18 +245,23 @@ type fakeArtwork struct {
recvSquare bool
}
func (c *fakeArtwork) GetOrPlaceholder(_ context.Context, id string, size int, square bool) (io.ReadCloser, time.Time, error) {
func (c *fakeArtwork) GetOrPlaceholder(_ context.Context, id string, size int, square bool) (*artwork.Image, error) {
if c.err != nil {
return nil, time.Time{}, c.err
return nil, c.err
}
c.recvId = id
c.recvSize = size
c.recvSquare = square
if c.ctxCancelFunc != nil {
c.ctxCancelFunc()
return nil, time.Time{}, context.Canceled
return nil, context.Canceled
}
return io.NopCloser(bytes.NewReader([]byte(c.data))), time.Time{}, nil
return &artwork.Image{
ReadCloser: io.NopCloser(bytes.NewReader([]byte(c.data))),
Hash: c.hash,
LastUpdated: c.lastUpdated,
Placeholder: c.placeholder,
}, nil
}
type mockedMediaFile struct {