Add optional MusicBrainz/Cover Art Archive metadata enrichment

getArtistInfo/2 and getAlbumInfo/2 now return a real MusicBrainz disambiguation/cover-art lookup for entities that carry a locally tagged MusicBrainz ID, gated behind the new VIBESONIC_ENABLE_EXTERNAL_METADATA flag (default off). Responses are cached in the existing external_metadata_cache table for 30 days to avoid hammering the free MusicBrainz/CAA services. VibeSonic still works completely offline: with the flag off, or when an entity has no tagged MusicBrainz ID, or on any network/lookup failure, the handlers fall back to local-only fields exactly as before — nothing is ever fabricated, and a failed external lookup never fails the request. internal/metadataenrich is a small, independently-testable HTTP client for the two real APIs (base URLs overridable for tests, exercised against httptest servers rather than the live network). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014r98AtcBbevpscWLsfwVvv
2026-08-23 08:35:40 -05:00
parent 2a69f6dc33
commit a921f9e2bc
7 changed files with 649 additions and 12 deletions
@@ -88,44 +88,50 @@ func handleGetVideoInfo(rc *Context) *apiError {
 	return nil
 }

-// --- Artist/album info: VibeSonic has no external metadata provider
-// (last.fm etc.) in Phase 2, so these honestly return only what's known
-// locally — which for biography/images/similar-artists is nothing —
-// rather than fabricating placeholder content. ---
+// --- Artist/album info: local data only (title/name, and a
+// MusicBrainz ID if one was tagged), optionally enriched from
+// MusicBrainz/Cover Art Archive when VIBESONIC_ENABLE_EXTERNAL_METADATA
+// is set and the entity has a tagged MusicBrainz ID — see enrich.go.
+// VibeSonic must work completely offline; enrichment is always
+// best-effort and never blocks or fails the response. ---

 func handleGetArtistInfo(rc *Context) *apiError {
-	if _, aerr := rc.Params.Required("id"); aerr != nil {
+	id, aerr := rc.Params.Required("id")
+	if aerr != nil {
 		return aerr
 	}
 	rc.Response = newResponse()
-	rc.Response.ArtistInfo = &ArtistInfo{}
+	rc.Response.ArtistInfo = &ArtistInfo{ArtistInfoBase: artistInfoBase(rc, id)}
 	return nil
 }

 func handleGetArtistInfo2(rc *Context) *apiError {
-	if _, aerr := rc.Params.Required("id"); aerr != nil {
+	id, aerr := rc.Params.Required("id")
+	if aerr != nil {
 		return aerr
 	}
 	rc.Response = newResponse()
-	rc.Response.ArtistInfo2 = &ArtistInfo2{}
+	rc.Response.ArtistInfo2 = &ArtistInfo2{ArtistInfoBase: artistInfoBase(rc, id)}
 	return nil
 }

 func handleGetAlbumInfo(rc *Context) *apiError {
-	if _, aerr := rc.Params.Required("id"); aerr != nil {
+	id, aerr := rc.Params.Required("id")
+	if aerr != nil {
 		return aerr
 	}
 	rc.Response = newResponse()
-	rc.Response.AlbumInfo = &AlbumInfo{}
+	rc.Response.AlbumInfo = albumInfo(rc, id)
 	return nil
 }

 func handleGetAlbumInfo2(rc *Context) *apiError {
-	if _, aerr := rc.Params.Required("id"); aerr != nil {
+	id, aerr := rc.Params.Required("id")
+	if aerr != nil {
 		return aerr
 	}
 	rc.Response = newResponse()
-	rc.Response.AlbumInfo = &AlbumInfo{}
+	rc.Response.AlbumInfo = albumInfo(rc, id)
 	return nil
 }

@@ -0,0 +1,120 @@
+package subsonic
+
+import (
+	"context"
+	"database/sql"
+	"encoding/json"
+	"time"
+
+	"git.noroutes.net/NoRoutes/vibetunes/internal/database"
+	"git.noroutes.net/NoRoutes/vibetunes/internal/metadataenrich"
+)
+
+// enrichmentCacheTTL is how long a cached MusicBrainz/Cover Art Archive
+// response is trusted before VibeSonic tries fetching again. This data
+// changes rarely, so a long TTL avoids hammering the (rate-limited,
+// shared, free) MusicBrainz service on every request.
+const enrichmentCacheTTL = 30 * 24 * time.Hour
+
+var enrichClient = &metadataenrich.Client{}
+
+// artistInfoBase builds the local ArtistInfoBase for an artist ID,
+// filling in the MusicBrainz-derived fields only when external
+// metadata is enabled, the artist has a tagged MusicBrainz ID, and a
+// (possibly cached) lookup actually succeeds. Any failure along the
+// way — unknown id, no mbid, network error, disabled feature — leaves
+// those fields blank rather than erroring the whole request or
+// inventing a value.
+func artistInfoBase(rc *Context, id string) ArtistInfoBase {
+	db := rc.Server.DB
+	artist, err := database.GetArtist(db, id)
+	if err != nil {
+		return ArtistInfoBase{}
+	}
+	base := ArtistInfoBase{MusicBrainzID: artist.MusicBrainzID}
+	if !rc.Server.Config.EnableExternalMetadata || artist.MusicBrainzID == "" {
+		return base
+	}
+
+	data, err := fetchArtistCached(rc.R.Context(), db, artist.MusicBrainzID)
+	if err != nil || data == nil {
+		return base
+	}
+	base.Biography = data.Disambiguation
+	return base
+}
+
+// albumInfo builds the local AlbumInfo for an album ID, enriched with
+// Cover Art Archive image URLs under the same conditions as
+// artistInfoBase.
+func albumInfo(rc *Context, id string) *AlbumInfo {
+	db := rc.Server.DB
+	album, err := database.GetAlbum(db, id)
+	if err != nil {
+		return &AlbumInfo{}
+	}
+	info := &AlbumInfo{MusicBrainzID: album.MusicBrainzID}
+	if !rc.Server.Config.EnableExternalMetadata || album.MusicBrainzID == "" {
+		return info
+	}
+
+	art, err := fetchCoverArtCached(rc.R.Context(), db, album.MusicBrainzID)
+	if err != nil || art == nil {
+		return info
+	}
+	info.SmallImageURL = art.SmallURL
+	info.MediumImageURL = art.MediumURL
+	info.LargeImageURL = art.LargeURL
+	return info
+}
+
+func fetchArtistCached(ctx context.Context, db *sql.DB, mbid string) (*metadataenrich.ArtistData, error) {
+	if cached, ok := readEnrichmentCache(db, "artist", mbid, "musicbrainz"); ok {
+		var data metadataenrich.ArtistData
+		if err := json.Unmarshal([]byte(cached), &data); err == nil {
+			return &data, nil
+		}
+	}
+
+	data, err := enrichClient.FetchArtist(ctx, mbid)
+	if err != nil {
+		return nil, err
+	}
+	if raw, err := json.Marshal(data); err == nil {
+		_ = database.PutCachedExternalMetadata(db, "artist", mbid, "musicbrainz", string(raw))
+	}
+	return data, nil
+}
+
+func fetchCoverArtCached(ctx context.Context, db *sql.DB, mbid string) (*metadataenrich.CoverArt, error) {
+	if cached, ok := readEnrichmentCache(db, "release", mbid, "coverartarchive"); ok {
+		var art metadataenrich.CoverArt
+		if err := json.Unmarshal([]byte(cached), &art); err == nil {
+			return &art, nil
+		}
+	}
+
+	art, err := enrichClient.FetchReleaseCoverArt(ctx, mbid)
+	if err != nil {
+		return nil, err
+	}
+	if art != nil {
+		if raw, err := json.Marshal(art); err == nil {
+			_ = database.PutCachedExternalMetadata(db, "release", mbid, "coverartarchive", string(raw))
+		}
+	}
+	return art, nil
+}
+
+// readEnrichmentCache returns a cached response's JSON body if one
+// exists and is still within enrichmentCacheTTL.
+func readEnrichmentCache(db *sql.DB, entityType, mbid, source string) (string, bool) {
+	dataJSON, fetchedAt, found, err := database.GetCachedExternalMetadata(db, entityType, mbid, source)
+	if err != nil || !found {
+		return "", false
+	}
+	if time.Since(fetchedAt) > enrichmentCacheTTL {
+		return "", false
+	}
+	return dataJSON, true
+}
@@ -0,0 +1,184 @@
+package subsonic
+
+import (
+	"database/sql"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"sync/atomic"
+	"testing"
+
+	"git.noroutes.net/NoRoutes/vibetunes/internal/database"
+	"git.noroutes.net/NoRoutes/vibetunes/internal/metadataenrich"
+)
+
+// withEnrichClient temporarily swaps the package-level enrichClient for
+// a test double and restores it afterward, since tests run
+// sequentially in this package.
+func withEnrichClient(t *testing.T, c *metadataenrich.Client) {
+	t.Helper()
+	orig := enrichClient
+	enrichClient = c
+	t.Cleanup(func() { enrichClient = orig })
+}
+
+func TestArtistInfo2EnrichedWhenEnabled(t *testing.T) {
+	s, db := newTestServer(t)
+	createTestUser(t, s, "alice", "password123", false)
+	artist, err := database.FindOrCreateArtist(db, "Test Artist", "", "mbid-artist-1")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	var calls int32
+	mb := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		atomic.AddInt32(&calls, 1)
+		json.NewEncoder(w).Encode(map[string]any{"disambiguation": "a real band"})
+	}))
+	defer mb.Close()
+	withEnrichClient(t, &metadataenrich.Client{MusicBrainzBaseURL: mb.URL})
+
+	s.Config.EnableExternalMetadata = true
+	srv := newSubsonicTestServer(t, s)
+	q := baseParams("alice", "password123")
+	q.Set("id", artist.ID)
+	q.Set("f", "json")
+
+	resp := getPath(t, srv, "/rest/getArtistInfo2.view", q)
+	j := decodeJSON(t, resp)
+	info, ok := j["artistInfo2"].(map[string]any)
+	if !ok || info["biography"] != "a real band" {
+		t.Fatalf("artistInfo2 = %v, want biography from the mocked MusicBrainz response", j)
+	}
+	if info["musicBrainzId"] != "mbid-artist-1" {
+		t.Errorf("musicBrainzId = %v", info["musicBrainzId"])
+	}
+
+	// A second lookup must hit the cache, not MusicBrainz again.
+	getPath(t, srv, "/rest/getArtistInfo2.view", q)
+	if got := atomic.LoadInt32(&calls); got != 1 {
+		t.Errorf("MusicBrainz was called %d times, want 1 (second lookup should be served from cache)", got)
+	}
+}
+
+func TestArtistInfo2NotEnrichedWhenDisabled(t *testing.T) {
+	s, db := newTestServer(t)
+	createTestUser(t, s, "alice", "password123", false)
+	artist, err := database.FindOrCreateArtist(db, "Test Artist", "", "mbid-artist-2")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	called := false
+	mb := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		called = true
+		json.NewEncoder(w).Encode(map[string]any{"disambiguation": "should never be fetched"})
+	}))
+	defer mb.Close()
+	withEnrichClient(t, &metadataenrich.Client{MusicBrainzBaseURL: mb.URL})
+
+	// EnableExternalMetadata left false (the default): must never
+	// call out to the network, matching "must work completely offline".
+	srv := newSubsonicTestServer(t, s)
+	q := baseParams("alice", "password123")
+	q.Set("id", artist.ID)
+	q.Set("f", "json")
+	resp := getPath(t, srv, "/rest/getArtistInfo2.view", q)
+	j := decodeJSON(t, resp)
+	info := j["artistInfo2"].(map[string]any)
+	if _, has := info["biography"]; has {
+		t.Errorf("biography should be absent when external metadata is disabled, got %v", info)
+	}
+	if called {
+		t.Error("MusicBrainz must never be called when external metadata is disabled")
+	}
+}
+
+func TestArtistInfo2NoMusicBrainzIDNeverCallsNetwork(t *testing.T) {
+	s, db := newTestServer(t)
+	createTestUser(t, s, "alice", "password123", false)
+	artist, err := database.FindOrCreateArtist(db, "Untagged Artist", "", "")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	called := false
+	mb := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		called = true
+	}))
+	defer mb.Close()
+	withEnrichClient(t, &metadataenrich.Client{MusicBrainzBaseURL: mb.URL})
+	s.Config.EnableExternalMetadata = true
+
+	srv := newSubsonicTestServer(t, s)
+	q := baseParams("alice", "password123")
+	q.Set("id", artist.ID)
+	q.Set("f", "json")
+	getPath(t, srv, "/rest/getArtistInfo2.view", q)
+	if called {
+		t.Error("must never look up an artist with no locally-tagged MusicBrainz ID")
+	}
+}
+
+func TestArtistInfo2NetworkFailureLeavesFieldsBlank(t *testing.T) {
+	s, db := newTestServer(t)
+	createTestUser(t, s, "alice", "password123", false)
+	artist, err := database.FindOrCreateArtist(db, "Test Artist", "", "mbid-artist-3")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	mb := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusInternalServerError)
+	}))
+	defer mb.Close()
+	withEnrichClient(t, &metadataenrich.Client{MusicBrainzBaseURL: mb.URL})
+	s.Config.EnableExternalMetadata = true
+
+	srv := newSubsonicTestServer(t, s)
+	q := baseParams("alice", "password123")
+	q.Set("id", artist.ID)
+	q.Set("f", "json")
+	resp := getPath(t, srv, "/rest/getArtistInfo2.view", q)
+	j := decodeJSON(t, resp)
+	if j["status"] != nil && j["status"] != "ok" {
+		t.Fatalf("a MusicBrainz failure must not fail the whole response: %v", j)
+	}
+	info := j["artistInfo2"].(map[string]any)
+	if _, has := info["biography"]; has {
+		t.Errorf("biography should be absent after a fetch failure, got %v", info)
+	}
+}
+
+func TestAlbumInfo2EnrichedCoverArt(t *testing.T) {
+	s, db := newTestServer(t)
+	createTestUser(t, s, "alice", "password123", false)
+	album, err := database.FindOrCreateAlbum(db, "Test Album", sql.NullString{}, "", sql.NullInt64{}, sql.NullInt64{}, "", "mbid-release-1")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	caa := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		json.NewEncoder(w).Encode(map[string]any{
+			"images": []map[string]any{
+				{"front": true, "image": "http://example.com/full.jpg", "thumbnails": map[string]string{
+					"small": "http://example.com/small.jpg", "large": "http://example.com/large.jpg",
+				}},
+			},
+		})
+	}))
+	defer caa.Close()
+	withEnrichClient(t, &metadataenrich.Client{CoverArtArchiveBaseURL: caa.URL})
+	s.Config.EnableExternalMetadata = true
+
+	srv := newSubsonicTestServer(t, s)
+	q := baseParams("alice", "password123")
+	q.Set("id", album.ID)
+	q.Set("f", "json")
+	resp := getPath(t, srv, "/rest/getAlbumInfo2.view", q)
+	j := decodeJSON(t, resp)
+	info, ok := j["albumInfo"].(map[string]any)
+	if !ok || info["largeImageUrl"] != "http://example.com/full.jpg" {
+		t.Fatalf("albumInfo = %v, want cover art from the mocked Cover Art Archive response", j)
+	}
+}