From ed1109ddb232184d69e655b72a2a44f9c1033e5b Mon Sep 17 00:00:00 2001 From: Kendall Garner <17521368+kgarner7@users.noreply.github.com> Date: Sat, 15 Mar 2025 01:21:03 +0000 Subject: [PATCH] fix(subsonic): fix albumCount in artists (#3827) * only do subsonic instead * make sure to actually populate response first * navidrome artist filtering * address discord feedback * perPage min 36 * various artist artist_id -> albumartist_id * artist_id, role_id separate * remove all ui changes I guess * Revert role filters Signed-off-by: Deluan --------- Signed-off-by: Deluan Co-authored-by: Deluan --- server/subsonic/browsing.go | 50 +++++++++++-------- server/subsonic/helpers.go | 19 ++++++- server/subsonic/helpers_test.go | 29 +++++++++++ ...onses Indexes with data should match .JSON | 1 - ...ponses Indexes with data should match .XML | 2 +- server/subsonic/responses/responses.go | 3 +- server/subsonic/responses/responses_test.go | 1 - server/subsonic/searching.go | 1 - 8 files changed, 78 insertions(+), 28 deletions(-) diff --git a/server/subsonic/browsing.go b/server/subsonic/browsing.go index df4083ae..82bf50dc 100644 --- a/server/subsonic/browsing.go +++ b/server/subsonic/browsing.go @@ -270,30 +270,43 @@ func (api *Router) GetGenres(r *http.Request) (*responses.Subsonic, error) { return response, nil } -func (api *Router) GetArtistInfo(r *http.Request) (*responses.Subsonic, error) { +func (api *Router) getArtistInfo(r *http.Request) (*responses.ArtistInfoBase, *model.Artists, error) { ctx := r.Context() p := req.Params(r) id, err := p.String("id") if err != nil { - return nil, err + return nil, nil, err } count := p.IntOr("count", 20) includeNotPresent := p.BoolOr("includeNotPresent", false) artist, err := api.externalMetadata.UpdateArtistInfo(ctx, id, count, includeNotPresent) + if err != nil { + return nil, nil, err + } + + base := responses.ArtistInfoBase{} + base.Biography = artist.Biography + base.SmallImageUrl = public.ImageURL(r, artist.CoverArtID(), 300) + base.MediumImageUrl = public.ImageURL(r, artist.CoverArtID(), 600) + base.LargeImageUrl = public.ImageURL(r, artist.CoverArtID(), 1200) + base.LastFmUrl = artist.ExternalUrl + base.MusicBrainzID = artist.MbzArtistID + + return &base, &artist.SimilarArtists, nil +} + +func (api *Router) GetArtistInfo(r *http.Request) (*responses.Subsonic, error) { + base, similarArtists, err := api.getArtistInfo(r) if err != nil { return nil, err } response := newResponse() response.ArtistInfo = &responses.ArtistInfo{} - response.ArtistInfo.Biography = artist.Biography - response.ArtistInfo.SmallImageUrl = public.ImageURL(r, artist.CoverArtID(), 300) - response.ArtistInfo.MediumImageUrl = public.ImageURL(r, artist.CoverArtID(), 600) - response.ArtistInfo.LargeImageUrl = public.ImageURL(r, artist.CoverArtID(), 1200) - response.ArtistInfo.LastFmUrl = artist.ExternalUrl - response.ArtistInfo.MusicBrainzID = artist.MbzArtistID - for _, s := range artist.SimilarArtists { + response.ArtistInfo.ArtistInfoBase = *base + + for _, s := range *similarArtists { similar := toArtist(r, s) if s.ID == "" { similar.Id = "-1" @@ -304,23 +317,20 @@ func (api *Router) GetArtistInfo(r *http.Request) (*responses.Subsonic, error) { } func (api *Router) GetArtistInfo2(r *http.Request) (*responses.Subsonic, error) { - info, err := api.GetArtistInfo(r) + base, similarArtists, err := api.getArtistInfo(r) if err != nil { return nil, err } response := newResponse() response.ArtistInfo2 = &responses.ArtistInfo2{} - response.ArtistInfo2.ArtistInfoBase = info.ArtistInfo.ArtistInfoBase - for _, s := range info.ArtistInfo.SimilarArtist { - similar := responses.ArtistID3{} - similar.Id = s.Id - similar.Name = s.Name - similar.AlbumCount = s.AlbumCount - similar.Starred = s.Starred - similar.UserRating = s.UserRating - similar.CoverArt = s.CoverArt - similar.ArtistImageUrl = s.ArtistImageUrl + response.ArtistInfo2.ArtistInfoBase = *base + + for _, s := range *similarArtists { + similar := toArtistID3(r, s) + if s.ID == "" { + similar.Id = "-1" + } response.ArtistInfo2.SimilarArtist = append(response.ArtistInfo2.SimilarArtist, similar) } return response, nil diff --git a/server/subsonic/helpers.go b/server/subsonic/helpers.go index 880bb6dd..0fb35a7b 100644 --- a/server/subsonic/helpers.go +++ b/server/subsonic/helpers.go @@ -77,11 +77,26 @@ func sortName(sortName, orderName string) string { return orderName } +func getArtistAlbumCount(a model.Artist) int32 { + albumStats := a.Stats[model.RoleAlbumArtist] + + // If ArtistParticipations are set, then `getArtist` will return albums + // where the artist is an album artist OR artist. While it may be an underestimate, + // guess the count by taking a max of the album artist and artist count. This is + // guaranteed to be <= the actual count. + // Otherwise, return just the roles as album artist (precise) + if conf.Server.Subsonic.ArtistParticipations { + artistStats := a.Stats[model.RoleArtist] + return int32(max(artistStats.AlbumCount, albumStats.AlbumCount)) + } else { + return int32(albumStats.AlbumCount) + } +} + func toArtist(r *http.Request, a model.Artist) responses.Artist { artist := responses.Artist{ Id: a.ID, Name: a.Name, - AlbumCount: int32(a.AlbumCount), UserRating: int32(a.Rating), CoverArt: a.CoverArtID().String(), ArtistImageUrl: public.ImageURL(r, a.CoverArtID(), 600), @@ -96,7 +111,7 @@ func toArtistID3(r *http.Request, a model.Artist) responses.ArtistID3 { artist := responses.ArtistID3{ Id: a.ID, Name: a.Name, - AlbumCount: int32(a.AlbumCount), + AlbumCount: getArtistAlbumCount(a), CoverArt: a.CoverArtID().String(), ArtistImageUrl: public.ImageURL(r, a.CoverArtID(), 600), UserRating: int32(a.Rating), diff --git a/server/subsonic/helpers_test.go b/server/subsonic/helpers_test.go index dd8bd88b..d703607b 100644 --- a/server/subsonic/helpers_test.go +++ b/server/subsonic/helpers_test.go @@ -10,6 +10,10 @@ import ( ) var _ = Describe("helpers", func() { + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + }) + Describe("fakePath", func() { var mf model.MediaFile BeforeEach(func() { @@ -134,4 +138,29 @@ var _ = Describe("helpers", func() { Entry("returns \"explicit\" when the db value is \"e\"", "e", "explicit"), Entry("returns an empty string when the db value is \"\"", "", ""), Entry("returns an empty string when there are unexpected values on the db", "abc", "")) + + Describe("getArtistAlbumCount", func() { + artist := model.Artist{ + Stats: map[model.Role]model.ArtistStats{ + model.RoleAlbumArtist: { + AlbumCount: 3, + }, + model.RoleArtist: { + AlbumCount: 4, + }, + }, + } + + It("Handles album count without artist participations", func() { + conf.Server.Subsonic.ArtistParticipations = false + result := getArtistAlbumCount(artist) + Expect(result).To(Equal(int32(3))) + }) + + It("Handles album count without with participations", func() { + conf.Server.Subsonic.ArtistParticipations = true + result := getArtistAlbumCount(artist) + Expect(result).To(Equal(int32(4))) + }) + }) }) diff --git a/server/subsonic/responses/.snapshots/Responses Indexes with data should match .JSON b/server/subsonic/responses/.snapshots/Responses Indexes with data should match .JSON index 585815fb..9f835da1 100644 --- a/server/subsonic/responses/.snapshots/Responses Indexes with data should match .JSON +++ b/server/subsonic/responses/.snapshots/Responses Indexes with data should match .JSON @@ -12,7 +12,6 @@ { "id": "111", "name": "aaa", - "albumCount": 2, "starred": "2016-03-02T20:30:00Z", "userRating": 3, "artistImageUrl": "https://lastfm.freetls.fastly.net/i/u/300x300/2a96cbd8b46e442fc41c2b86b821562f.png" diff --git a/server/subsonic/responses/.snapshots/Responses Indexes with data should match .XML b/server/subsonic/responses/.snapshots/Responses Indexes with data should match .XML index 86495a75..595f2ff0 100644 --- a/server/subsonic/responses/.snapshots/Responses Indexes with data should match .XML +++ b/server/subsonic/responses/.snapshots/Responses Indexes with data should match .XML @@ -1,7 +1,7 @@ - + diff --git a/server/subsonic/responses/responses.go b/server/subsonic/responses/responses.go index f329fae4..c0f499ef 100644 --- a/server/subsonic/responses/responses.go +++ b/server/subsonic/responses/responses.go @@ -92,7 +92,6 @@ type MusicFolders struct { type Artist struct { Id string `xml:"id,attr" json:"id"` Name string `xml:"name,attr" json:"name"` - AlbumCount int32 `xml:"albumCount,attr,omitempty" json:"albumCount,omitempty"` Starred *time.Time `xml:"starred,attr,omitempty" json:"starred,omitempty"` UserRating int32 `xml:"userRating,attr,omitempty" json:"userRating,omitempty"` CoverArt string `xml:"coverArt,attr,omitempty" json:"coverArt,omitempty"` @@ -233,7 +232,7 @@ type ArtistID3 struct { Id string `xml:"id,attr" json:"id"` Name string `xml:"name,attr" json:"name"` CoverArt string `xml:"coverArt,attr,omitempty" json:"coverArt,omitempty"` - AlbumCount int32 `xml:"albumCount,attr,omitempty" json:"albumCount,omitempty"` + AlbumCount int32 `xml:"albumCount,attr" json:"albumCount"` Starred *time.Time `xml:"starred,attr,omitempty" json:"starred,omitempty"` UserRating int32 `xml:"userRating,attr,omitempty" json:"userRating,omitempty"` ArtistImageUrl string `xml:"artistImageUrl,attr,omitempty" json:"artistImageUrl,omitempty"` diff --git a/server/subsonic/responses/responses_test.go b/server/subsonic/responses/responses_test.go index fed45419..f3796f10 100644 --- a/server/subsonic/responses/responses_test.go +++ b/server/subsonic/responses/responses_test.go @@ -103,7 +103,6 @@ var _ = Describe("Responses", func() { Name: "aaa", Starred: &t, UserRating: 3, - AlbumCount: 2, ArtistImageUrl: "https://lastfm.freetls.fastly.net/i/u/300x300/2a96cbd8b46e442fc41c2b86b821562f.png", } index := make([]Index, 1) diff --git a/server/subsonic/searching.go b/server/subsonic/searching.go index 235ebc13..f66846f3 100644 --- a/server/subsonic/searching.go +++ b/server/subsonic/searching.go @@ -94,7 +94,6 @@ func (api *Router) Search2(r *http.Request) (*responses.Subsonic, error) { a := responses.Artist{ Id: artist.ID, Name: artist.Name, - AlbumCount: int32(artist.AlbumCount), UserRating: int32(artist.Rating), CoverArt: artist.CoverArtID().String(), ArtistImageUrl: public.ImageURL(r, artist.CoverArtID(), 600),