fix: simplify artist image provider, remove unnecessary LookupArtist call
The previous implementation called LookupArtist (rate-limited MB API) before fetching url-rels, wasting a rate limiter slot. The fetchURL for rels also bypassed the MB rate limiter, risking 503 rejections. Rewrite: fetch MB url-rels once (direct HTTP, cached 30 days), parse both image and wikidata relations from the same response, resolve Wikimedia thumb URL. No dependency on MusicBrainzClient — just the Cache for storage and a plain http.Client. Also cleared 34 stale cached empty results from previous failed resolution attempts that were blocking image lookup.
This commit is contained in:
+101
-141
@@ -29,14 +29,17 @@ const (
|
|||||||
|
|
||||||
// artistImageTimeout is the HTTP timeout for image URL lookups.
|
// artistImageTimeout is the HTTP timeout for image URL lookups.
|
||||||
artistImageTimeout = 10 * time.Second
|
artistImageTimeout = 10 * time.Second
|
||||||
|
|
||||||
|
// artistImageCacheTTL is how long resolved image URLs are cached.
|
||||||
|
artistImageCacheTTL = 30 * 24 * time.Hour
|
||||||
)
|
)
|
||||||
|
|
||||||
// ArtistImageProvider resolves artist MBIDs to image URLs. It
|
// ArtistImageProvider resolves artist MBIDs to image URLs. It
|
||||||
// checks multiple sources in priority order and caches results in
|
// fetches the artist's MB url-rels (once, cached 30 days), extracts
|
||||||
// the explore_cache. Designed to be extended with additional
|
// image sources from them, and returns a Wikimedia Commons thumbnail
|
||||||
// sources (fanart.tv, etc.) by adding to the providers slice.
|
// URL. Designed to be extended with additional sources (fanart.tv,
|
||||||
|
// etc.) by adding to the resolve chain.
|
||||||
type ArtistImageProvider struct {
|
type ArtistImageProvider struct {
|
||||||
mb *MusicBrainzClient
|
|
||||||
cache *Cache
|
cache *Cache
|
||||||
client *http.Client
|
client *http.Client
|
||||||
logger *slog.Logger
|
logger *slog.Logger
|
||||||
@@ -45,12 +48,10 @@ type ArtistImageProvider struct {
|
|||||||
// NewArtistImageProvider creates a provider that resolves artist
|
// NewArtistImageProvider creates a provider that resolves artist
|
||||||
// images via MusicBrainz relationships and Wikidata.
|
// images via MusicBrainz relationships and Wikidata.
|
||||||
func NewArtistImageProvider(
|
func NewArtistImageProvider(
|
||||||
mb *MusicBrainzClient,
|
|
||||||
cache *Cache,
|
cache *Cache,
|
||||||
logger *slog.Logger,
|
logger *slog.Logger,
|
||||||
) *ArtistImageProvider {
|
) *ArtistImageProvider {
|
||||||
return &ArtistImageProvider{
|
return &ArtistImageProvider{
|
||||||
mb: mb,
|
|
||||||
cache: cache,
|
cache: cache,
|
||||||
client: &http.Client{Timeout: artistImageTimeout},
|
client: &http.Client{Timeout: artistImageTimeout},
|
||||||
logger: logger,
|
logger: logger,
|
||||||
@@ -59,111 +60,114 @@ func NewArtistImageProvider(
|
|||||||
|
|
||||||
// GetArtistImageURL returns a Wikimedia Commons thumbnail URL for
|
// GetArtistImageURL returns a Wikimedia Commons thumbnail URL for
|
||||||
// the given artist MBID, or "" if no image is available. Results
|
// the given artist MBID, or "" if no image is available. Results
|
||||||
// are cached in explore_cache with a 30-day TTL.
|
// are cached for 30 days.
|
||||||
func (p *ArtistImageProvider) GetArtistImageURL(artistMBID string) string {
|
func (p *ArtistImageProvider) GetArtistImageURL(artistMBID string) string {
|
||||||
if artistMBID == "" {
|
if artistMBID == "" {
|
||||||
return ""
|
return ""
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Check resolved URL cache first.
|
||||||
cacheKey := "artist-image:" + artistMBID
|
cacheKey := "artist-image:" + artistMBID
|
||||||
|
|
||||||
// Check cache.
|
|
||||||
if data, ok := p.cache.Get(cacheKey); ok {
|
if data, ok := p.cache.Get(cacheKey); ok {
|
||||||
return string(data)
|
return string(data)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Resolve image URL.
|
// Fetch MB url-rels (cached separately, shared with other uses).
|
||||||
imageURL := p.resolve(artistMBID)
|
rels := p.fetchMBRels(artistMBID)
|
||||||
|
if rels == nil {
|
||||||
|
p.cache.Set(cacheKey, []byte(""), artistImageCacheTTL, artistMBID, "artist")
|
||||||
|
|
||||||
// Cache the result (even empty string = no image found).
|
return ""
|
||||||
cacheTTL := 30 * 24 * time.Hour
|
}
|
||||||
p.cache.Set(cacheKey, []byte(imageURL), cacheTTL, artistMBID, "artist")
|
|
||||||
|
// Try each source in priority order.
|
||||||
|
imageURL := p.fromDirectImageRel(rels)
|
||||||
|
|
||||||
|
if imageURL == "" {
|
||||||
|
imageURL = p.fromWikidataRel(rels)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Cache the result (even "" = no image).
|
||||||
|
p.cache.Set(cacheKey, []byte(imageURL), artistImageCacheTTL, artistMBID, "artist")
|
||||||
|
|
||||||
|
if imageURL != "" {
|
||||||
|
p.logger.Debug("artist image resolved",
|
||||||
|
"mbid", artistMBID,
|
||||||
|
"url", imageURL,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
return imageURL
|
return imageURL
|
||||||
}
|
}
|
||||||
|
|
||||||
// resolve tries each source in order and returns the first image
|
// ---------------------------------------------------------------------------
|
||||||
// URL found.
|
// MB url-rels fetching (shared by all sources)
|
||||||
func (p *ArtistImageProvider) resolve(artistMBID string) string {
|
// ---------------------------------------------------------------------------
|
||||||
// Source 1: MB direct image relation (Commons wiki page link).
|
|
||||||
if url := p.fromMBImageRelation(artistMBID); url != "" {
|
|
||||||
return url
|
|
||||||
}
|
|
||||||
|
|
||||||
// Source 2: MB wikidata relation → Wikidata P18 → Commons thumb.
|
type mbRelation struct {
|
||||||
if url := p.fromWikidata(artistMBID); url != "" {
|
Type string `json:"type"`
|
||||||
return url
|
URL struct {
|
||||||
}
|
Resource string `json:"resource"`
|
||||||
|
} `json:"url"`
|
||||||
// No image found from any source.
|
|
||||||
return ""
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// ---------------------------------------------------------------------------
|
func (p *ArtistImageProvider) fetchMBRels(artistMBID string) []mbRelation {
|
||||||
// Source 1: MB direct image relation
|
|
||||||
// ---------------------------------------------------------------------------
|
|
||||||
|
|
||||||
// fromMBImageRelation checks the artist's MB url-rels for a direct
|
|
||||||
// "image" type pointing to Wikimedia Commons.
|
|
||||||
func (p *ArtistImageProvider) fromMBImageRelation(artistMBID string) string {
|
|
||||||
ctx := context.Background()
|
|
||||||
|
|
||||||
artist, err := p.mb.LookupArtist(ctx, artistMBID)
|
|
||||||
if err != nil || artist == nil {
|
|
||||||
return ""
|
|
||||||
}
|
|
||||||
|
|
||||||
// The LookupArtist doesn't include rels in our current wrapper.
|
|
||||||
// We need the raw MB data with url-rels. Check if there's a
|
|
||||||
// cached response that includes relations.
|
|
||||||
cacheKey := "mb:artist-rels:" + artistMBID
|
cacheKey := "mb:artist-rels:" + artistMBID
|
||||||
|
|
||||||
if data, ok := p.cache.Get(cacheKey); ok {
|
if data, ok := p.cache.Get(cacheKey); ok {
|
||||||
return p.parseImageFromRels(data)
|
var envelope struct {
|
||||||
|
Relations []mbRelation `json:"relations"`
|
||||||
|
}
|
||||||
|
|
||||||
|
if err := json.Unmarshal(data, &envelope); err == nil {
|
||||||
|
return envelope.Relations
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Fetch with url-rels included.
|
|
||||||
url := fmt.Sprintf(
|
url := fmt.Sprintf(
|
||||||
"https://musicbrainz.org/ws/2/artist/%s?fmt=json&inc=url-rels",
|
"https://musicbrainz.org/ws/2/artist/%s?fmt=json&inc=url-rels",
|
||||||
artistMBID,
|
artistMBID,
|
||||||
)
|
)
|
||||||
|
|
||||||
body, err := p.fetchURL(ctx, url)
|
body, err := p.fetchURL(url)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return ""
|
p.logger.Debug("artist image: MB rels fetch failed",
|
||||||
|
"mbid", artistMBID,
|
||||||
|
"error", err,
|
||||||
|
)
|
||||||
|
|
||||||
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// Cache the response.
|
p.cache.Set(cacheKey, body, artistImageCacheTTL, artistMBID, "artist")
|
||||||
cacheTTL := 30 * 24 * time.Hour
|
|
||||||
p.cache.Set(cacheKey, body, cacheTTL, artistMBID, "artist")
|
|
||||||
|
|
||||||
return p.parseImageFromRels(body)
|
var envelope struct {
|
||||||
|
Relations []mbRelation `json:"relations"`
|
||||||
|
}
|
||||||
|
|
||||||
|
if err := json.Unmarshal(body, &envelope); err != nil {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
return envelope.Relations
|
||||||
}
|
}
|
||||||
|
|
||||||
func (p *ArtistImageProvider) parseImageFromRels(data []byte) string {
|
// ---------------------------------------------------------------------------
|
||||||
var mb struct {
|
// Source 1: direct image relation (Commons wiki page link)
|
||||||
Relations []struct {
|
// ---------------------------------------------------------------------------
|
||||||
Type string `json:"type"`
|
|
||||||
URL struct {
|
|
||||||
Resource string `json:"resource"`
|
|
||||||
} `json:"url"`
|
|
||||||
} `json:"relations"`
|
|
||||||
}
|
|
||||||
|
|
||||||
if err := json.Unmarshal(data, &mb); err != nil {
|
func (p *ArtistImageProvider) fromDirectImageRel(rels []mbRelation) string {
|
||||||
return ""
|
for _, rel := range rels {
|
||||||
}
|
|
||||||
|
|
||||||
for _, rel := range mb.Relations {
|
|
||||||
if rel.Type != "image" {
|
if rel.Type != "image" {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
resource := rel.URL.Resource
|
resource := rel.URL.Resource
|
||||||
|
|
||||||
// Direct Commons file link: "https://commons.wikimedia.org/wiki/File:Name.jpg"
|
// "https://commons.wikimedia.org/wiki/File:Name.jpg"
|
||||||
if strings.Contains(resource, "commons.wikimedia.org/wiki/File:") {
|
if idx := strings.LastIndex(resource, "File:"); idx >= 0 {
|
||||||
filename := resource[strings.LastIndex(resource, "File:")+5:]
|
filename := resource[idx+5:]
|
||||||
|
|
||||||
return wikimediaThumbURL(filename)
|
return wikimediaThumbURL(filename)
|
||||||
}
|
}
|
||||||
@@ -173,33 +177,40 @@ func (p *ArtistImageProvider) parseImageFromRels(data []byte) string {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
// Source 2: Wikidata P18
|
// Source 2: Wikidata P18 property
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
// fromWikidata looks up the artist's Wikidata Q-ID from MB rels,
|
func (p *ArtistImageProvider) fromWikidataRel(rels []mbRelation) string {
|
||||||
// then fetches the P18 (image) property from Wikidata.
|
// Find the wikidata Q-ID.
|
||||||
func (p *ArtistImageProvider) fromWikidata(artistMBID string) string {
|
qid := ""
|
||||||
// Get the wikidata Q-ID from cached MB rels.
|
|
||||||
qid := p.getWikidataQID(artistMBID)
|
for _, rel := range rels {
|
||||||
|
if rel.Type == "wikidata" {
|
||||||
|
parts := strings.Split(rel.URL.Resource, "/")
|
||||||
|
qid = parts[len(parts)-1]
|
||||||
|
|
||||||
|
break
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
if qid == "" {
|
if qid == "" {
|
||||||
return ""
|
return ""
|
||||||
}
|
}
|
||||||
|
|
||||||
// Check cache for Wikidata image.
|
// Check cache for this Wikidata entity.
|
||||||
cacheKey := "wikidata-image:" + qid
|
cacheKey := "wikidata-p18:" + qid
|
||||||
|
|
||||||
if data, ok := p.cache.Get(cacheKey); ok {
|
if data, ok := p.cache.Get(cacheKey); ok {
|
||||||
return string(data)
|
return string(data)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Fetch P18 from Wikidata API.
|
// Fetch P18 from Wikidata.
|
||||||
ctx := context.Background()
|
|
||||||
url := fmt.Sprintf(
|
url := fmt.Sprintf(
|
||||||
"%s?action=wbgetclaims&entity=%s&property=P18&format=json",
|
"%s?action=wbgetclaims&entity=%s&property=P18&format=json",
|
||||||
wikidataAPIBase, qid,
|
wikidataAPIBase, qid,
|
||||||
)
|
)
|
||||||
|
|
||||||
body, err := p.fetchURL(ctx, url)
|
body, err := p.fetchURL(url)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return ""
|
return ""
|
||||||
}
|
}
|
||||||
@@ -216,78 +227,24 @@ func (p *ArtistImageProvider) fromWikidata(artistMBID string) string {
|
|||||||
} `json:"claims"`
|
} `json:"claims"`
|
||||||
}
|
}
|
||||||
|
|
||||||
if err := json.Unmarshal(body, &wd); err != nil || len(wd.Claims.P18) == 0 {
|
thumbURL := ""
|
||||||
// Cache empty result.
|
|
||||||
cacheTTL := 30 * 24 * time.Hour
|
|
||||||
p.cache.Set(cacheKey, []byte(""), cacheTTL, "", "")
|
|
||||||
|
|
||||||
return ""
|
if err := json.Unmarshal(body, &wd); err == nil && len(wd.Claims.P18) > 0 {
|
||||||
|
filename := strings.ReplaceAll(wd.Claims.P18[0].Mainsnak.Datavalue.Value, " ", "_")
|
||||||
|
thumbURL = wikimediaThumbURL(filename)
|
||||||
}
|
}
|
||||||
|
|
||||||
filename := strings.ReplaceAll(wd.Claims.P18[0].Mainsnak.Datavalue.Value, " ", "_")
|
p.cache.Set(cacheKey, []byte(thumbURL), artistImageCacheTTL, "", "")
|
||||||
thumbURL := wikimediaThumbURL(filename)
|
|
||||||
|
|
||||||
cacheTTL := 30 * 24 * time.Hour
|
|
||||||
p.cache.Set(cacheKey, []byte(thumbURL), cacheTTL, "", "")
|
|
||||||
|
|
||||||
return thumbURL
|
return thumbURL
|
||||||
}
|
}
|
||||||
|
|
||||||
func (p *ArtistImageProvider) getWikidataQID(artistMBID string) string {
|
|
||||||
cacheKey := "mb:artist-rels:" + artistMBID
|
|
||||||
|
|
||||||
data, ok := p.cache.Get(cacheKey)
|
|
||||||
if !ok {
|
|
||||||
// Need to fetch rels — fromMBImageRelation should have
|
|
||||||
// populated this, but if not, fetch now.
|
|
||||||
ctx := context.Background()
|
|
||||||
url := fmt.Sprintf(
|
|
||||||
"https://musicbrainz.org/ws/2/artist/%s?fmt=json&inc=url-rels",
|
|
||||||
artistMBID,
|
|
||||||
)
|
|
||||||
|
|
||||||
var err error
|
|
||||||
|
|
||||||
data, err = p.fetchURL(ctx, url)
|
|
||||||
if err != nil {
|
|
||||||
return ""
|
|
||||||
}
|
|
||||||
|
|
||||||
cacheTTL := 30 * 24 * time.Hour
|
|
||||||
p.cache.Set(cacheKey, data, cacheTTL, artistMBID, "artist")
|
|
||||||
}
|
|
||||||
|
|
||||||
var mb struct {
|
|
||||||
Relations []struct {
|
|
||||||
Type string `json:"type"`
|
|
||||||
URL struct {
|
|
||||||
Resource string `json:"resource"`
|
|
||||||
} `json:"url"`
|
|
||||||
} `json:"relations"`
|
|
||||||
}
|
|
||||||
|
|
||||||
if err := json.Unmarshal(data, &mb); err != nil {
|
|
||||||
return ""
|
|
||||||
}
|
|
||||||
|
|
||||||
for _, rel := range mb.Relations {
|
|
||||||
if rel.Type == "wikidata" {
|
|
||||||
parts := strings.Split(rel.URL.Resource, "/")
|
|
||||||
|
|
||||||
return parts[len(parts)-1]
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
return ""
|
|
||||||
}
|
|
||||||
|
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
// Helpers
|
// Helpers
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
// wikimediaThumbURL constructs a Wikimedia Commons thumbnail URL
|
// wikimediaThumbURL constructs a Wikimedia Commons thumbnail URL
|
||||||
// from a filename. The URL scheme uses MD5 hashing of the filename
|
// from a filename using the MD5 directory bucketing scheme.
|
||||||
// for directory bucketing.
|
|
||||||
func wikimediaThumbURL(filename string) string {
|
func wikimediaThumbURL(filename string) string {
|
||||||
if filename == "" {
|
if filename == "" {
|
||||||
return ""
|
return ""
|
||||||
@@ -304,7 +261,10 @@ func wikimediaThumbURL(filename string) string {
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
func (p *ArtistImageProvider) fetchURL(ctx context.Context, url string) ([]byte, error) {
|
func (p *ArtistImageProvider) fetchURL(url string) ([]byte, error) {
|
||||||
|
ctx, cancel := context.WithTimeout(context.Background(), artistImageTimeout)
|
||||||
|
defer cancel()
|
||||||
|
|
||||||
req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
|
req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
|
|||||||
@@ -38,7 +38,7 @@ func NewExploreService(logger *slog.Logger, db *database.DB) *Service {
|
|||||||
lb := NewListenBrainzClient(limiter, cache, logger.WithGroup("listenbrainz"))
|
lb := NewListenBrainzClient(limiter, cache, logger.WithGroup("listenbrainz"))
|
||||||
index := NewSearchIndex(db, lb, logger.WithGroup("search-index"))
|
index := NewSearchIndex(db, lb, logger.WithGroup("search-index"))
|
||||||
artProxy := NewCoverArtProxy(db, limiter)
|
artProxy := NewCoverArtProxy(db, limiter)
|
||||||
artistImg := NewArtistImageProvider(mb, cache, logger.WithGroup("artist-image"))
|
artistImg := NewArtistImageProvider(cache, logger.WithGroup("artist-image"))
|
||||||
|
|
||||||
logger.Info("explore service created")
|
logger.Info("explore service created")
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user