mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-01 07:21:17 +00:00
fix(artwork): keep not-found distinct from artwork-absent
GetOrPlaceholder folded model.ErrNotFound in with ErrUnavailable, so an id matching no entity returned 200 and the placeholder PNG. That made the ErrorDataNotFound branch in getCoverArt unreachable: Subsonic went from error 70 to a successful placeholder, and Jellyfin's Primary image endpoint from 404 to 200. Neither is ours to change. An entity with no art and an id with no entity are different answers; only the first is a placeholder.
This commit is contained in:
parent
2ee2b2d66f
commit
6553a75b0e
@ -66,7 +66,9 @@ func (s *service) GetOrPlaceholder(ctx context.Context, id string, size int, squ
|
||||
if err == nil {
|
||||
img, err = s.Get(ctx, artID, size, square)
|
||||
}
|
||||
if errors.Is(err, ErrUnavailable) || errors.Is(err, model.ErrNotFound) {
|
||||
// Only a resolvable entity with no art gets the placeholder. An id that matches no entity
|
||||
// stays ErrNotFound, so getCoverArt keeps answering error 70 and Jellyfin keeps 404ing.
|
||||
if errors.Is(err, ErrUnavailable) {
|
||||
return s.placeholder(artID.Kind), nil
|
||||
}
|
||||
return img, err
|
||||
|
||||
@ -378,6 +378,16 @@ var _ = Describe("Service", func() {
|
||||
phBytes, _ := io.ReadAll(ph)
|
||||
Expect(readAll(img)).To(Equal(phBytes))
|
||||
})
|
||||
|
||||
// An entity that has no art and an id that names no entity are different answers:
|
||||
// Subsonic reports error 70 for the latter, and Jellyfin 404s.
|
||||
It("reports not-found rather than a placeholder for an id with no entity", func() {
|
||||
_, err := svc.GetOrPlaceholder(ctx, "al-nosuchalbum", 0, false)
|
||||
Expect(err).To(MatchError(model.ErrNotFound))
|
||||
|
||||
_, err = svc.GetOrPlaceholder(ctx, "nosuchrawid", 0, false)
|
||||
Expect(err).To(MatchError(model.ErrNotFound))
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@ -201,6 +201,15 @@ var _ = Describe("Artwork Serving", Ordered, func() {
|
||||
Expect(w.Header().Get("Cache-Control")).To(Equal("no-store"))
|
||||
})
|
||||
|
||||
// An album with no art and an id naming no album are different answers; only the former
|
||||
// is a placeholder.
|
||||
It("answers error 70 for an id that matches no entity", func() {
|
||||
w := getCover("id", "al-nosuchalbum")
|
||||
Expect(w.Code).To(Equal(http.StatusOK))
|
||||
Expect(w.Body.String()).To(ContainSubstring(`"code":70`))
|
||||
Expect(w.Body.Bytes()).ToNot(Equal(placeholder))
|
||||
})
|
||||
|
||||
It("serves /share/img immutably for a JWT whose payload carries the hash", func() {
|
||||
token, err := auth.CreateExpiringPublicToken(time.Now().Add(time.Hour),
|
||||
auth.Claims{ID: "al-" + artfulID + "_" + artfulHash})
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user