Log warning when artist has a MBID of Various Artists

consts/consts.go

@@ -85,9 +85,10 @@ var (
 )
 
 var (
-	VariousArtists   = "Various Artists"
-	VariousArtistsID = fmt.Sprintf("%x", md5.Sum([]byte(strings.ToLower(VariousArtists))))
-	UnknownArtist    = "[Unknown Artist]"
+	VariousArtists      = "Various Artists"
+	VariousArtistsID    = fmt.Sprintf("%x", md5.Sum([]byte(strings.ToLower(VariousArtists))))
+	UnknownArtist       = "[Unknown Artist]"
+	VariousArtistsMbzId = "89ad4ac3-39f7-470e-963a-56509c546377"
 
 	ServerStart = time.Now()
 )

persistence/album_repository.go

@@ -236,7 +236,7 @@ func (r *albumRepository) refresh(ids ...string) error {
 
 		al.AlbumArtistID, al.AlbumArtist = getAlbumArtist(al)
 		al.MinYear = getMinYear(al.Years)
-		al.MbzAlbumID = getMbzId(r.ctx, al.MbzAlbumID, r.tableName, al.Name)
+		al.MbzAlbumID = getMostFrequentMbzID(r.ctx, al.MbzAlbumID, r.tableName, al.Name)
 		al.Comment = getComment(al.Comments, zwsp)
 		if al.CurrentId != "" {
 			toUpdate++

persistence/artist_repository.go

@@ -198,7 +198,7 @@ func (r *artistRepository) refresh(ids ...string) error {
 		} else {
 			toInsert++
 		}
-		ar.MbzArtistID = getMbzId(r.ctx, ar.MbzArtistID, r.tableName, ar.Name)
+		ar.MbzArtistID = getMostFrequentMbzID(r.ctx, ar.MbzArtistID, r.tableName, ar.Name)
 		err := r.Put(&ar.Artist)
 		if err != nil {
 			return err

persistence/helpers.go

@@ -7,8 +7,9 @@ import (
 	"regexp"
 	"strings"
 
-	"github.com/Masterminds/squirrel"
 	"github.com/navidrome/navidrome/consts"
+
+	"github.com/Masterminds/squirrel"
 	"github.com/navidrome/navidrome/log"
 	"github.com/navidrome/navidrome/model"
 	"github.com/navidrome/navidrome/utils"
@@ -59,31 +60,32 @@ func (e existsCond) ToSql() (string, []interface{}, error) {
 	return sql, args, err
 }
 
-func getMbzId(ctx context.Context, mbzIDS, entityName, name string) string {
-	ids := strings.Fields(mbzIDS)
+func getMostFrequentMbzID(ctx context.Context, mbzIDs, entityName, name string) string {
+	ids := strings.Fields(mbzIDs)
 	if len(ids) == 0 {
 		return ""
 	}
+	if len(ids) == 1 {
+		return ids[0]
+	}
 	idCounts := map[string]int{}
+	var topId string
+	var topCount int
 	for _, id := range ids {
-		if c, ok := idCounts[id]; ok {
-			idCounts[id] = c + 1
-		} else {
-			idCounts[id] = 1
+		c := idCounts[id] + 1
+		idCounts[id] = c
+		if c > topCount {
+			topId = id
+			topCount = c
 		}
 	}
 
-	var topKey string
-	var topCount int
-	for k, v := range idCounts {
-		if v > topCount {
-			topKey = k
-			topCount = v
+	if name != consts.VariousArtists {
+		if topId == consts.VariousArtistsMbzId {
+			log.Warn(ctx, "Artist with mbid of Various Artists", "name", name, "mbid", topId)
+		} else {
+			log.Warn(ctx, "Multiple MBIDs found for "+entityName, "name", name, "mbids", idCounts)
 		}
 	}
-
-	if len(idCounts) > 1 && name != consts.VariousArtists {
-		log.Warn(ctx, "Multiple MBIDs found for "+entityName, "name", name, "mbids", idCounts)
-	}
-	return topKey
+	return topId
 }

persistence/helpers_test.go

@@ -53,7 +53,7 @@ var _ = Describe("Helpers", func() {
 		})
 	})
 
-	Describe("Exists", func() {
+	Describe("exists", func() {
 		It("constructs the correct EXISTS query", func() {
 			e := exists("album", squirrel.Eq{"id": 1})
 			sql, args, err := e.ToSql()
@@ -63,15 +63,15 @@ var _ = Describe("Helpers", func() {
 		})
 	})
 
-	Describe("getMbzId", func() {
+	Describe("getMostFrequentMbzID", func() {
 		It(`returns "" when no ids are passed`, func() {
-			Expect(getMbzId(context.TODO(), " ", "", "")).To(Equal(""))
+			Expect(getMostFrequentMbzID(context.TODO(), " ", "", "")).To(Equal(""))
 		})
 		It(`returns the only id passed`, func() {
-			Expect(getMbzId(context.TODO(), "1234 ", "", "")).To(Equal("1234"))
+			Expect(getMostFrequentMbzID(context.TODO(), "111 ", "", "")).To(Equal("111"))
 		})
 		It(`returns the id with higher frequency`, func() {
-			Expect(getMbzId(context.TODO(), "1 2 3 4 1", "", "")).To(Equal("1"))
+			Expect(getMostFrequentMbzID(context.TODO(), "1 2 3 4 2", "", "")).To(Equal("2"))
 		})
 	})
 })