feat(subsonic): content-hash coverArt ids, omit artwork on known-absent

This commit is contained in:
Deluan 2026-07-22 21:53:24 -04:00
parent 25b32f9706
commit 8fea7efa50
10 changed files with 228 additions and 29 deletions

View File

@ -230,9 +230,11 @@ func (api *Router) GetAlbumInfo(r *http.Request) (*responses.Subsonic, error) {
response := newResponse()
response.AlbumInfo = &responses.AlbumInfo{}
response.AlbumInfo.Notes = album.Description
response.AlbumInfo.SmallImageUrl = publicurl.ImageURL(r, album.CoverArtID(), 300)
response.AlbumInfo.MediumImageUrl = publicurl.ImageURL(r, album.CoverArtID(), 600)
response.AlbumInfo.LargeImageUrl = publicurl.ImageURL(r, album.CoverArtID(), 1200)
if !album.ImageAbsent {
response.AlbumInfo.SmallImageUrl = publicurl.ImageURL(r, album.CoverArtID(), 300)
response.AlbumInfo.MediumImageUrl = publicurl.ImageURL(r, album.CoverArtID(), 600)
response.AlbumInfo.LargeImageUrl = publicurl.ImageURL(r, album.CoverArtID(), 1200)
}
response.AlbumInfo.LastFmUrl = album.ExternalUrl
response.AlbumInfo.MusicBrainzID = album.MbzAlbumID
@ -295,9 +297,11 @@ func (api *Router) getArtistInfo(r *http.Request) (*responses.ArtistInfoBase, *m
base := responses.ArtistInfoBase{}
base.Biography = artist.Biography
base.SmallImageUrl = publicurl.ImageURL(r, artist.CoverArtID(), 300)
base.MediumImageUrl = publicurl.ImageURL(r, artist.CoverArtID(), 600)
base.LargeImageUrl = publicurl.ImageURL(r, artist.CoverArtID(), 1200)
if !artist.ImageAbsent {
base.SmallImageUrl = publicurl.ImageURL(r, artist.CoverArtID(), 300)
base.MediumImageUrl = publicurl.ImageURL(r, artist.CoverArtID(), 600)
base.LargeImageUrl = publicurl.ImageURL(r, artist.CoverArtID(), 1200)
}
base.LastFmUrl = artist.ExternalUrl
base.MusicBrainzID = artist.MbzArtistID
@ -453,7 +457,7 @@ func (api *Router) buildAlbumDirectory(ctx context.Context, album *model.Album)
dir.AverageRating = album.AverageRating
}
dir.SongCount = int32(album.SongCount)
dir.CoverArt = album.CoverArtID().String()
dir.CoverArt = coverArtOrEmpty(album.CoverArtID(), album.ImageAbsent)
if album.Starred {
dir.Starred = album.StarredAt
}

View File

@ -6,6 +6,7 @@ import (
"net/http/httptest"
"github.com/navidrome/navidrome/core/auth"
"github.com/navidrome/navidrome/core/external"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/request"
"github.com/navidrome/navidrome/tests"
@ -157,4 +158,57 @@ var _ = Describe("Browsing", func() {
Expect(response.Artist).ToNot(BeNil())
})
})
Describe("GetAlbumInfo", func() {
It("emits image URLs when the album artwork is unresolved", func() {
api.provider = &fakeInfoProvider{album: &model.Album{ID: "al-1"}}
r := httptest.NewRequest("GET", "/rest/getAlbumInfo?id=al-1", nil)
resp, err := api.GetAlbumInfo(r)
Expect(err).ToNot(HaveOccurred())
Expect(resp.AlbumInfo.SmallImageUrl).ToNot(BeEmpty())
Expect(resp.AlbumInfo.LargeImageUrl).ToNot(BeEmpty())
})
It("omits image URLs when the album artwork is known absent", func() {
api.provider = &fakeInfoProvider{album: &model.Album{ID: "al-1", ItemImage: model.ItemImage{ImageAbsent: true}}}
r := httptest.NewRequest("GET", "/rest/getAlbumInfo?id=al-1", nil)
resp, err := api.GetAlbumInfo(r)
Expect(err).ToNot(HaveOccurred())
Expect(resp.AlbumInfo.SmallImageUrl).To(BeEmpty())
Expect(resp.AlbumInfo.MediumImageUrl).To(BeEmpty())
Expect(resp.AlbumInfo.LargeImageUrl).To(BeEmpty())
})
})
Describe("GetArtistInfo", func() {
It("emits image URLs when the artist artwork is unresolved", func() {
api.provider = &fakeInfoProvider{artist: &model.Artist{ID: "ar-1"}}
r := httptest.NewRequest("GET", "/rest/getArtistInfo?id=ar-1", nil)
resp, err := api.GetArtistInfo(r)
Expect(err).ToNot(HaveOccurred())
Expect(resp.ArtistInfo.SmallImageUrl).ToNot(BeEmpty())
})
It("omits image URLs when the artist artwork is known absent", func() {
api.provider = &fakeInfoProvider{artist: &model.Artist{ID: "ar-1", ItemImage: model.ItemImage{ImageAbsent: true}}}
r := httptest.NewRequest("GET", "/rest/getArtistInfo?id=ar-1", nil)
resp, err := api.GetArtistInfo(r)
Expect(err).ToNot(HaveOccurred())
Expect(resp.ArtistInfo.SmallImageUrl).To(BeEmpty())
Expect(resp.ArtistInfo.MediumImageUrl).To(BeEmpty())
Expect(resp.ArtistInfo.LargeImageUrl).To(BeEmpty())
})
})
})
type fakeInfoProvider struct {
external.Provider
album *model.Album
artist *model.Artist
}
func (f *fakeInfoProvider) UpdateAlbumInfo(context.Context, string) (*model.Album, error) {
return f.album, nil
}
func (f *fakeInfoProvider) UpdateArtistInfo(context.Context, string, int, bool) (*model.Artist, error) {
return f.artist, nil
}

View File

@ -95,13 +95,24 @@ func getArtistAlbumCount(a *model.Artist) int32 {
}
}
// coverArtOrEmpty stamps the (hash-suffixed) coverArt id, or omits it (empty
// string + the field's omitempty tag) when artwork is known absent.
func coverArtOrEmpty(id model.ArtworkID, absent bool) string {
if absent {
return ""
}
return id.String()
}
func toArtist(r *http.Request, a model.Artist) responses.Artist {
artist := responses.Artist{
Id: a.ID,
Name: a.Name,
UserRating: int32(a.Rating),
CoverArt: a.CoverArtID().String(),
ArtistImageUrl: publicurl.ImageURL(r, a.CoverArtID(), 600),
Id: a.ID,
Name: a.Name,
UserRating: int32(a.Rating),
CoverArt: coverArtOrEmpty(a.CoverArtID(), a.ImageAbsent),
}
if !a.ImageAbsent {
artist.ArtistImageUrl = publicurl.ImageURL(r, a.CoverArtID(), 600)
}
if conf.Server.Subsonic.EnableAverageRating {
artist.AverageRating = a.AverageRating
@ -114,12 +125,14 @@ func toArtist(r *http.Request, a model.Artist) responses.Artist {
func toArtistID3(r *http.Request, a model.Artist) responses.ArtistID3 {
artist := responses.ArtistID3{
Id: a.ID,
Name: a.Name,
AlbumCount: getArtistAlbumCount(&a),
CoverArt: a.CoverArtID().String(),
ArtistImageUrl: publicurl.ImageURL(r, a.CoverArtID(), 600),
UserRating: int32(a.Rating),
Id: a.ID,
Name: a.Name,
AlbumCount: getArtistAlbumCount(&a),
CoverArt: coverArtOrEmpty(a.CoverArtID(), a.ImageAbsent),
UserRating: int32(a.Rating),
}
if !a.ImageAbsent {
artist.ArtistImageUrl = publicurl.ImageURL(r, a.CoverArtID(), 600)
}
if conf.Server.Subsonic.EnableAverageRating {
artist.AverageRating = a.AverageRating
@ -208,7 +221,7 @@ func childFromMediaFile(ctx context.Context, mf model.MediaFile) responses.Child
child.Size = mf.Size
child.Suffix = mf.Suffix
child.BitRate = int32(mf.BitRate)
child.CoverArt = mf.CoverArtID().String()
child.CoverArt = coverArtOrEmpty(mf.CoverArtID(), mf.ImageAbsent)
child.ContentType = mf.ContentType()
if ok && player.ReportRealPath {
@ -369,7 +382,7 @@ func childFromAlbum(ctx context.Context, al model.Album) responses.Child {
child.Artist = al.AlbumArtist
child.Year = int32(cmp.Or(al.MaxOriginalYear, al.MaxYear))
child.Genre = al.Genre
child.CoverArt = al.CoverArtID().String()
child.CoverArt = coverArtOrEmpty(al.CoverArtID(), al.ImageAbsent)
child.Created = new(albumCreatedAt(al))
child.Parent = al.AlbumArtistID
child.ArtistId = al.AlbumArtistID
@ -460,7 +473,7 @@ func buildAlbumID3(ctx context.Context, album model.Album) responses.AlbumID3 {
dir.Name = album.FullName()
dir.Artist = album.AlbumArtist
dir.ArtistId = album.AlbumArtistID
dir.CoverArt = album.CoverArtID().String()
dir.CoverArt = coverArtOrEmpty(album.CoverArtID(), album.ImageAbsent)
dir.SongCount = int32(album.SongCount)
dir.Duration = int32(album.Duration)
dir.PlayCount = album.PlayCount

View File

@ -320,6 +320,74 @@ var _ = Describe("helpers", func() {
})
})
Describe("artwork coverArt emission", func() {
const hash = "0123456789abcdef"
ctx := context.Background()
DescribeTable("coverArtOrEmpty",
func(id model.ArtworkID, absent bool, expected string) {
Expect(coverArtOrEmpty(id, absent)).To(Equal(expected))
},
Entry("emits the bare id when unresolved", model.ArtworkID{Kind: model.KindAlbumArtwork, ID: "1"}, false, "al-1"),
Entry("emits the hash-suffixed id when resolved", model.ArtworkID{Kind: model.KindAlbumArtwork, ID: "1", Hash: hash}, false, "al-1_"+hash),
Entry("omits (empty) when known absent", model.ArtworkID{Kind: model.KindAlbumArtwork, ID: "1", Hash: hash}, true, ""),
)
Describe("childFromAlbum", func() {
It("suffixes coverArt with the content hash when resolved", func() {
al := model.Album{ID: "al-1", ItemImage: model.ItemImage{ImageHash: hash}}
Expect(childFromAlbum(ctx, al).CoverArt).To(Equal("al-al-1_" + hash))
})
It("omits coverArt when known absent", func() {
al := model.Album{ID: "al-1", ItemImage: model.ItemImage{ImageAbsent: true}}
Expect(childFromAlbum(ctx, al).CoverArt).To(BeEmpty())
})
It("emits the bare id when unresolved", func() {
al := model.Album{ID: "al-1"}
Expect(childFromAlbum(ctx, al).CoverArt).To(Equal("al-al-1"))
})
})
Describe("childFromMediaFile", func() {
It("suffixes coverArt with the content hash when resolved", func() {
mf := model.MediaFile{ID: "mf-1", AlbumID: "al-1", ItemImage: model.ItemImage{ImageHash: hash}}
Expect(childFromMediaFile(ctx, mf).CoverArt).To(Equal("al-al-1_" + hash))
})
It("omits coverArt when known absent", func() {
mf := model.MediaFile{ID: "mf-1", AlbumID: "al-1", ItemImage: model.ItemImage{ImageAbsent: true}}
Expect(childFromMediaFile(ctx, mf).CoverArt).To(BeEmpty())
})
It("emits the bare id when unresolved", func() {
mf := model.MediaFile{ID: "mf-1", AlbumID: "al-1"}
Expect(childFromMediaFile(ctx, mf).CoverArt).To(Equal("al-al-1"))
})
})
Describe("toArtist / toArtistID3", func() {
r := httptest.NewRequest("GET", "/test", nil)
It("emits both coverArt and imageUrl when resolved", func() {
a := model.Artist{ID: "ar-1", ItemImage: model.ItemImage{ImageHash: hash}}
artist := toArtist(r, a)
Expect(artist.CoverArt).To(Equal("ar-ar-1_" + hash))
Expect(artist.ArtistImageUrl).ToNot(BeEmpty())
})
It("omits coverArt and imageUrl when known absent", func() {
a := model.Artist{ID: "ar-1", ItemImage: model.ItemImage{ImageAbsent: true}}
artist := toArtist(r, a)
Expect(artist.CoverArt).To(BeEmpty())
Expect(artist.ArtistImageUrl).To(BeEmpty())
id3 := toArtistID3(r, a)
Expect(id3.CoverArt).To(BeEmpty())
Expect(id3.ArtistImageUrl).To(BeEmpty())
})
It("emits an imageUrl when unresolved", func() {
a := model.Artist{ID: "ar-1"}
Expect(toArtist(r, a).ArtistImageUrl).ToNot(BeEmpty())
})
})
})
Describe("osChildFromMediaFile", func() {
var mf model.MediaFile
var ctx context.Context

View File

@ -151,7 +151,7 @@ func (api *Router) buildPlaylist(ctx context.Context, p model.Playlist) response
pls.Comment = p.Comment
pls.Owner = p.OwnerName
pls.Public = p.Public
pls.CoverArt = p.CoverArtID().String()
pls.CoverArt = coverArtOrEmpty(p.CoverArtID(), p.ImageAbsent)
pls.OpenSubsonicPlaylist = buildOSPlaylist(ctx, p)
return pls

View File

@ -129,6 +129,19 @@ var _ = Describe("buildPlaylist", func() {
})
})
Context("artwork emission", func() {
It("suffixes coverArt with the content hash when resolved", func() {
playlist.ImageHash = "0123456789abcdef"
result := router.buildPlaylist(ctx, playlist)
Expect(result.CoverArt).To(Equal("pl-pls-1_0123456789abcdef"))
})
It("omits coverArt when known absent", func() {
playlist.ImageAbsent = true
result := router.buildPlaylist(ctx, playlist)
Expect(result.CoverArt).To(BeEmpty())
})
})
Context("with legacy client", func() {
BeforeEach(func() {
conf.Server.Subsonic.LegacyClients = "legacy-client"

View File

@ -74,10 +74,10 @@ func (api *Router) GetInternetRadios(r *http.Request) (*responses.Subsonic, erro
if strings.Contains(conf.Server.Subsonic.LegacyClients, player.Client) {
continue
}
// Add coverArt if not legacy client
// Add coverArt if not legacy client; only radios have an uploaded image as their sole art source
var coverArt string
if g.UploadedImage != "" {
coverArt = g.CoverArtID().String()
coverArt = coverArtOrEmpty(g.CoverArtID(), g.ImageAbsent)
}
res[i].OpenSubsonicRadio = &responses.OpenSubsonicRadio{
CoverArt: coverArt,

View File

@ -75,6 +75,21 @@ var _ = Describe("Radio", func() {
Expect(response.InternetRadioStations.Radios[1].OpenSubsonicRadio).ToNot(BeNil())
Expect(response.InternetRadioStations.Radios[1].CoverArt).To(BeEmpty())
})
It("suffixes coverArt with the content hash when resolved and omits it when known absent", func() {
radioRepo.All = model.Radios{
{ID: "rd-1", Name: "Radio 1", UploadedImage: "rd-1_cover.jpg", ItemImage: model.ItemImage{ImageHash: "0123456789abcdef"}},
{ID: "rd-2", Name: "Radio 2", UploadedImage: "rd-2_cover.jpg", ItemImage: model.ItemImage{ImageAbsent: true}},
}
r := httptest.NewRequest("GET", "/rest/getInternetRadios", nil)
r = r.WithContext(ctx)
response, err := api.GetInternetRadios(r)
Expect(err).ToNot(HaveOccurred())
Expect(response.InternetRadioStations.Radios[0].CoverArt).To(Equal("ra-rd-1_0123456789abcdef"))
Expect(response.InternetRadioStations.Radios[1].CoverArt).To(BeEmpty())
})
})
Context("with a legacy client", func() {

View File

@ -110,11 +110,13 @@ func (api *Router) Search2(r *http.Request) (*responses.Subsonic, error) {
searchResult2 := &responses.SearchResult2{}
searchResult2.Artist = slice.Map(as, func(artist model.Artist) responses.Artist {
a := responses.Artist{
Id: artist.ID,
Name: artist.Name,
UserRating: int32(artist.Rating),
CoverArt: artist.CoverArtID().String(),
ArtistImageUrl: publicurl.ImageURL(r, artist.CoverArtID(), 600),
Id: artist.ID,
Name: artist.Name,
UserRating: int32(artist.Rating),
CoverArt: coverArtOrEmpty(artist.CoverArtID(), artist.ImageAbsent),
}
if !artist.ImageAbsent {
a.ArtistImageUrl = publicurl.ImageURL(r, artist.CoverArtID(), 600)
}
if artist.Starred {
a.Starred = artist.StarredAt

View File

@ -1,6 +1,8 @@
package subsonic
import (
"net/http"
"github.com/Masterminds/squirrel"
"github.com/navidrome/navidrome/core/auth"
"github.com/navidrome/navidrome/model"
@ -209,4 +211,32 @@ var _ = Describe("Search", func() {
})
})
})
Describe("artwork emission", func() {
newSearchRequest := func() *http.Request {
r := newGetRequest("query=test")
ctx := request.WithUser(r.Context(), model.User{ID: "user1", Libraries: []model.Library{{ID: 1}}})
return r.WithContext(ctx)
}
It("omits coverArt and imageUrl for a known-absent artist", func() {
mockArtistRepo.SetData(model.Artists{
{ID: "ar-1", Name: "Absent Artist", ItemImage: model.ItemImage{ImageAbsent: true}},
})
resp, err := router.Search2(newSearchRequest())
Expect(err).ToNot(HaveOccurred())
Expect(resp.SearchResult2.Artist).To(HaveLen(1))
Expect(resp.SearchResult2.Artist[0].CoverArt).To(BeEmpty())
Expect(resp.SearchResult2.Artist[0].ArtistImageUrl).To(BeEmpty())
})
It("emits coverArt and imageUrl for an unresolved artist", func() {
mockArtistRepo.SetData(model.Artists{{ID: "ar-1", Name: "Unresolved Artist"}})
resp, err := router.Search2(newSearchRequest())
Expect(err).ToNot(HaveOccurred())
Expect(resp.SearchResult2.Artist).To(HaveLen(1))
Expect(resp.SearchResult2.Artist[0].CoverArt).To(Equal("ar-ar-1"))
Expect(resp.SearchResult2.Artist[0].ArtistImageUrl).ToNot(BeEmpty())
})
})
})