perf(metadata): coalesce MusicBrainz ISRC lookups

go_backend/exports_musicbrainz.go

@@ -9,6 +9,7 @@ import (
 	"sync"
 	"time"
 
+	"golang.org/x/sync/singleflight"
 	"golang.org/x/text/cases"
 	"golang.org/x/text/language"
 )
@@ -34,6 +35,7 @@ type musicBrainzCacheEntry struct {
 var (
 	musicBrainzCacheMu sync.Mutex
 	musicBrainzCache   = make(map[string]musicBrainzCacheEntry)
+	musicBrainzFlight  singleflight.Group
 )
 
 func musicBrainzCached(key string, fetch func() (string, error)) (string, error) {
@@ -44,31 +46,45 @@ func musicBrainzCached(key string, fetch func() (string, error)) (string, error)
 	}
 	musicBrainzCacheMu.Unlock()
 
-	value, err := fetch()
+	result, err, _ := musicBrainzFlight.Do(key, func() (any, error) {
+		// A waiter can reach the flight after the leader populated the cache.
+		musicBrainzCacheMu.Lock()
+		if entry, ok := musicBrainzCache[key]; ok && time.Now().Before(entry.expiresAt) {
+			musicBrainzCacheMu.Unlock()
+			return entry.value, entry.err
+		}
+		musicBrainzCacheMu.Unlock()
 
-	ttl := musicBrainzCachePositiveTTL
+		value, fetchErr := fetch()
-	if err != nil || value == "" {
+		ttl := musicBrainzCachePositiveTTL
-		ttl = musicBrainzCacheNegativeTTL
+		if fetchErr != nil || value == "" {
-	}
+			ttl = musicBrainzCacheNegativeTTL
-	musicBrainzCacheMu.Lock()
+		}
-	if len(musicBrainzCache) >= musicBrainzCacheMaxEntries {
+		musicBrainzCacheMu.Lock()
-		now := time.Now()
+		if len(musicBrainzCache) >= musicBrainzCacheMaxEntries {
-		for k, e := range musicBrainzCache {
+			now := time.Now()
-			if now.After(e.expiresAt) {
+			for k, e := range musicBrainzCache {
-				delete(musicBrainzCache, k)
+				if now.After(e.expiresAt) {
+					delete(musicBrainzCache, k)
+				}
+			}
+			if len(musicBrainzCache) >= musicBrainzCacheMaxEntries {
+				musicBrainzCache = make(map[string]musicBrainzCacheEntry)
 			}
 		}
-		if len(musicBrainzCache) >= musicBrainzCacheMaxEntries {
+		musicBrainzCache[key] = musicBrainzCacheEntry{
-			musicBrainzCache = make(map[string]musicBrainzCacheEntry)
+			value:     value,
+			err:       fetchErr,
+			expiresAt: time.Now().Add(ttl),
 		}
+		musicBrainzCacheMu.Unlock()
+		return value, fetchErr
+	})
+	if err != nil {
+		return "", err
 	}
-	musicBrainzCache[key] = musicBrainzCacheEntry{
+	value, _ := result.(string)
-		value:     value,
+	return value, nil
-		err:       err,
-		expiresAt: time.Now().Add(ttl),
-	}
-	musicBrainzCacheMu.Unlock()
-	return value, err
 }
 
 type musicBrainzTag struct {
@@ -76,12 +92,6 @@ type musicBrainzTag struct {
 	Name  string `json:"name"`
 }
 
-type musicBrainzRecordingResponse struct {
-	Recordings []struct {
-		Tags []musicBrainzTag `json:"tags"`
-	} `json:"recordings"`
-}
-
 type musicBrainzArtistCredit struct {
 	Name       string `json:"name"`
 	JoinPhrase string `json:"joinphrase"`
@@ -92,12 +102,49 @@ type musicBrainzRelease struct {
 	ArtistCredit []musicBrainzArtistCredit `json:"artist-credit"`
 }
 
-type musicBrainzAlbumArtistResponse struct {
+type musicBrainzCombinedResponse struct {
 	Recordings []struct {
+		Tags     []musicBrainzTag     `json:"tags"`
 		Releases []musicBrainzRelease `json:"releases"`
 	} `json:"recordings"`
 }
 
+// fetchMusicBrainzCombinedByISRC is the shared recording snapshot used by
+// both genre and album-artist lookups. MusicBrainz accepts all includes in a
+// single request, so two native calls for the same ISRC still cost one HTTP
+// request and concurrent callers share that request through singleflight.
+func fetchMusicBrainzCombinedByISRC(isrc string) (*musicBrainzCombinedResponse, string, error) {
+	normalizedISRC := strings.ToUpper(strings.TrimSpace(isrc))
+	key := "recording\x00" + normalizedISRC
+	encoded, err := musicBrainzCached(key, func() (string, error) {
+		var payload musicBrainzCombinedResponse
+		normalized, fetchErr := fetchMusicBrainzRecordingByISRC(
+			isrc,
+			"tags+releases+artist-credits",
+			&payload,
+		)
+		if fetchErr != nil {
+			return "", fetchErr
+		}
+		data, marshalErr := json.Marshal(payload)
+		if marshalErr != nil {
+			return "", marshalErr
+		}
+		if normalizedISRC == "" {
+			normalizedISRC = normalized
+		}
+		return string(data), nil
+	})
+	if err != nil {
+		return nil, normalizedISRC, err
+	}
+	var payload musicBrainzCombinedResponse
+	if err := json.Unmarshal([]byte(encoded), &payload); err != nil {
+		return nil, normalizedISRC, err
+	}
+	return &payload, normalizedISRC, nil
+}
+
 func formatMusicBrainzGenre(tags []musicBrainzTag) string {
 	if len(tags) == 0 {
 		return ""
@@ -227,40 +274,31 @@ func fetchMusicBrainzRecordingByISRC(isrc string, inc string, payload any) (stri
 }
 
 func FetchMusicBrainzAlbumArtistByISRC(isrc string, albumName string) (string, error) {
-	key := "albumartist\x00" + strings.ToUpper(strings.TrimSpace(isrc)) +
+	payload, normalizedISRC, err := fetchMusicBrainzCombinedByISRC(isrc)
-		"\x00" + strings.ToLower(strings.TrimSpace(albumName))
+	if err != nil {
-	return musicBrainzCached(key, func() (string, error) {
+		return "", err
-		var payload musicBrainzAlbumArtistResponse
+	}
-		normalizedISRC, err := fetchMusicBrainzRecordingByISRC(isrc, "releases+artist-credits", &payload)
+	for _, recording := range payload.Recordings {
-		if err != nil {
+		if albumArtist := selectMusicBrainzAlbumArtist(recording.Releases, albumName); albumArtist != "" {
-			return "", err
+			return albumArtist, nil
-		}
-		for _, recording := range payload.Recordings {
-			if albumArtist := selectMusicBrainzAlbumArtist(recording.Releases, albumName); albumArtist != "" {
-				return albumArtist, nil
-			}
 		}
+	}
 
-		return "", fmt.Errorf("no MusicBrainz album artist found for ISRC: %s", normalizedISRC)
+	return "", fmt.Errorf("no MusicBrainz album artist found for ISRC: %s", normalizedISRC)
-	})
 }
 
 func FetchMusicBrainzGenreByISRC(isrc string) (string, error) {
-	key := "genre\x00" + strings.ToUpper(strings.TrimSpace(isrc))
+	payload, normalizedISRC, err := fetchMusicBrainzCombinedByISRC(isrc)
-	return musicBrainzCached(key, func() (string, error) {
+	if err != nil {
-		var payload musicBrainzRecordingResponse
+		return "", err
-		normalizedISRC, err := fetchMusicBrainzRecordingByISRC(isrc, "tags", &payload)
+	}
-		if err != nil {
+	if len(payload.Recordings) == 0 {
-			return "", err
+		return "", fmt.Errorf("no recordings found for ISRC: %s", normalizedISRC)
-		}
+	}
-		if len(payload.Recordings) == 0 {
-			return "", fmt.Errorf("no recordings found for ISRC: %s", normalizedISRC)
-		}
 
-		genre := formatMusicBrainzGenre(payload.Recordings[0].Tags)
+	genre := formatMusicBrainzGenre(payload.Recordings[0].Tags)
-		if genre == "" {
+	if genre == "" {
-			return "", fmt.Errorf("no MusicBrainz genre tags found for ISRC: %s", normalizedISRC)
+		return "", fmt.Errorf("no MusicBrainz genre tags found for ISRC: %s", normalizedISRC)
-		}
+	}
-		return genre, nil
+	return genre, nil
-	})
 }