Get better artist images results

core/external_info.go

@@ -2,6 +2,7 @@ package core
 
 import (
 	"context"
+	"fmt"
 	"sort"
 	"strings"
 	"sync"
@@ -12,6 +13,7 @@ import (
 	"github.com/deluan/navidrome/log"
 	"github.com/deluan/navidrome/model"
 	"github.com/microcosm-cc/bluemonday"
+	"github.com/xrash/smetrics"
 )
 
 const placeholderArtistImageSmallUrl = "https://lastfm.freetls.fastly.net/i/u/64s/2a96cbd8b46e442fc41c2b86b821562f.png"
@@ -30,7 +32,7 @@ type LastFMClient interface {
 }
 
 type SpotifyClient interface {
-	ArtistImages(ctx context.Context, name string) ([]spotify.Image, error)
+	SearchArtists(ctx context.Context, name string, limit int) ([]spotify.Artist, error)
 }
 
 func NewExternalInfo(ds model.DataStore, lfm LastFMClient, spf SpotifyClient) ExternalInfo {
@@ -77,6 +79,11 @@ func (e *externalInfo) SimilarSongs(ctx context.Context, id string, count int) (
 }
 
 func (e *externalInfo) SimilarArtists(ctx context.Context, id string, includeNotPresent bool, count int) (model.Artists, error) {
+	if e.lfm == nil {
+		log.Warn(ctx, "Last.FM client not configured")
+		return nil, model.ErrNotAvailable
+	}
+
 	artist, err := e.getArtist(ctx, id)
 	if err != nil {
 		return nil, err
@@ -85,6 +92,7 @@ func (e *externalInfo) SimilarArtists(ctx context.Context, id string, includeNot
 	var result model.Artists
 	var notPresent []string
 
+	log.Debug(ctx, "Calling Last.FM ArtistGetSimilar", "artist", artist.Name)
 	similar, err := e.lfm.ArtistGetSimilar(ctx, artist.Name, count)
 	if err != nil {
 		return nil, err
@@ -158,22 +166,44 @@ func (e *externalInfo) callArtistInfo(ctx context.Context, artist *model.Artist,
 	}
 }
 
+func (e *externalInfo) findArtist(ctx context.Context, name string) (*spotify.Artist, error) {
+	artists, err := e.spf.SearchArtists(ctx, name, 40)
+	if err != nil || len(artists) == 0 {
+		return nil, model.ErrNotFound
+	}
+	name = strings.ToLower(name)
+
+	// Sort results, prioritizing artists with images, with similar names and with high popularity, in this order
+	sort.Slice(artists, func(i, j int) bool {
+		ai := fmt.Sprintf("%-5t-%03d-%04d", len(artists[i].Images) == 0, smetrics.WagnerFischer(name, strings.ToLower(artists[i].Name), 1, 1, 2), 1000-artists[i].Popularity)
+		aj := fmt.Sprintf("%-5t-%03d-%04d", len(artists[j].Images) == 0, smetrics.WagnerFischer(name, strings.ToLower(artists[j].Name), 1, 1, 2), 1000-artists[j].Popularity)
+		return strings.Compare(ai, aj) < 0
+	})
+
+	// If the first one has the same name, that's the one
+	if strings.ToLower(artists[0].Name) != name {
+		return nil, model.ErrNotFound
+	}
+	return &artists[0], err
+}
+
 func (e *externalInfo) callArtistImages(ctx context.Context, artist *model.Artist, wg *sync.WaitGroup, info *model.ArtistInfo) {
 	if e.spf != nil {
-		log.Debug(ctx, "Calling Spotify ArtistImages", "artist", artist.Name)
+		log.Debug(ctx, "Calling Spotify SearchArtist", "artist", artist.Name)
 		wg.Add(1)
 		go func() {
 			start := time.Now()
 			defer wg.Done()
-			spfImages, err := e.spf.ArtistImages(ctx, artist.Name)
+
+			a, err := e.findArtist(ctx, artist.Name)
 			if err != nil {
 				log.Error(ctx, "Error calling Spotify", "artist", artist.Name, err)
-			} else {
+				return
-				log.Debug(ctx, "Got images from Spotify", "artist", artist.Name, "images", spfImages, "elapsed", time.Since(start))
 			}
+			spfImages := a.Images
+			log.Debug(ctx, "Got images from Spotify", "artist", artist.Name, "images", spfImages, "elapsed", time.Since(start))
 
 			sort.Slice(spfImages, func(i, j int) bool { return spfImages[i].Width > spfImages[j].Width })
-
 			if len(spfImages) >= 1 {
 				e.setLargeImageUrl(info, spfImages[0].URL)
 			}

core/spotify/client.go

@@ -35,7 +35,7 @@ type Client struct {
 	hc     HttpClient
 }
 
-func (c *Client) ArtistImages(ctx context.Context, name string) ([]Image, error) {
+func (c *Client) SearchArtists(ctx context.Context, name string, limit int) ([]Artist, error) {
 	token, err := c.authorize(ctx)
 	if err != nil {
 		return nil, err
@@ -45,7 +45,7 @@ func (c *Client) ArtistImages(ctx context.Context, name string) ([]Image, error)
 	params.Add("type", "artist")
 	params.Add("q", name)
 	params.Add("offset", "0")
-	params.Add("limit", "1")
+	params.Add("limit", strconv.Itoa(limit))
 	req, _ := http.NewRequest("GET", apiBaseUrl+"search", nil)
 	req.URL.RawQuery = params.Encode()
 	req.Header.Add("Authorization", "Bearer "+token)
@@ -60,7 +60,7 @@ func (c *Client) ArtistImages(ctx context.Context, name string) ([]Image, error)
 		return nil, ErrNotFound
 	}
 	log.Debug(ctx, "Found artist in Spotify", "artist", results.Artists.Items[0].Name)
-	return results.Artists.Items[0].Images, err
+	return results.Artists.Items, err
 }
 
 func (c *Client) authorize(ctx context.Context) (string, error) {

core/spotify/client_test.go

@@ -29,8 +29,12 @@ var _ = Describe("Client", func() {
 				Body:       ioutil.NopCloser(bytes.NewBufferString(`{"access_token": "NEW_ACCESS_TOKEN","token_type": "Bearer","expires_in": 3600}`)),
 			})
 
-			images, err := client.ArtistImages(context.TODO(), "U2")
+			artists, err := client.SearchArtists(context.TODO(), "U2", 10)
 			Expect(err).To(BeNil())
+			Expect(artists).To(HaveLen(20))
+			Expect(artists[0].Popularity).To(Equal(82))
+
+			images := artists[0].Images
 			Expect(images).To(HaveLen(3))
 			Expect(images[0].Width).To(Equal(640))
 			Expect(images[1].Width).To(Equal(320))
@@ -51,7 +55,7 @@ var _ = Describe("Client", func() {
 				Body:       ioutil.NopCloser(bytes.NewBufferString(`{"access_token": "NEW_ACCESS_TOKEN","token_type": "Bearer","expires_in": 3600}`)),
 			})
 
-			_, err := client.ArtistImages(context.TODO(), "U2")
+			_, err := client.SearchArtists(context.TODO(), "U2", 10)
 			Expect(err).To(MatchError(ErrNotFound))
 		})
 
@@ -63,7 +67,7 @@ var _ = Describe("Client", func() {
 				Body:       ioutil.NopCloser(bytes.NewBufferString(`{"error":"invalid_client","error_description":"Invalid client"}`)),
 			})
 
-			_, err := client.ArtistImages(context.TODO(), "U2")
+			_, err := client.SearchArtists(context.TODO(), "U2", 10)
 			Expect(err).To(MatchError("spotify error(invalid_client): Invalid client"))
 		})
 	})

core/spotify/responses.go

@@ -10,11 +10,12 @@ type ArtistsResult struct {
 }
 
 type Artist struct {
-	Genres []string `json:"genres"`
+	Genres     []string `json:"genres"`
-	HRef   string   `json:"href"`
+	HRef       string   `json:"href"`
-	ID     string   `json:"id"`
+	ID         string   `json:"id"`
-	Images []Image  `json:"images"`
+	Popularity int      `json:"popularity"`
-	Name   string   `json:"name"`
+	Images     []Image  `json:"images"`
+	Name       string   `json:"name"`
 }
 
 type Image struct {

go.mod

@@ -37,6 +37,7 @@ require (
 	github.com/spf13/jwalterweatherman v1.1.0 // indirect
 	github.com/spf13/viper v1.7.1
 	github.com/unrolled/secure v1.0.8
+	github.com/xrash/smetrics v0.0.0-20200730060457-89a2a8a1fb0b
 	github.com/ziutek/mymysql v1.5.4 // indirect
 	golang.org/x/image v0.0.0-20191009234506-e7c1f5e7dbb8
 	golang.org/x/tools v0.0.0-20200812195022-5ae4c3c160a0

go.sum

@@ -557,6 +557,8 @@ github.com/wader/tag v0.0.0-20200426234345-d072771f6a51/go.mod h1:f3YqVk9PEeVf7T
 github.com/wendal/errors v0.0.0-20130201093226-f66c77a7882b/go.mod h1:Q12BUT7DqIlHRmgv3RskH+UCM/4eqVMgI0EMmlSpAXc=
 github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU=
 github.com/xordataexchange/crypt v0.0.3-0.20170626215501-b2862e3d0a77/go.mod h1:aYKd//L2LvnjZzWKhF00oedf4jCCReLcmhLdhm1A27Q=
+github.com/xrash/smetrics v0.0.0-20200730060457-89a2a8a1fb0b h1:tnWgqoOBmInkt5pbLjagwNVjjT4RdJhFHzL1ebCSRh8=
+github.com/xrash/smetrics v0.0.0-20200730060457-89a2a8a1fb0b/go.mod h1:N3UwUGtsrSj3ccvlPHLoLsHnpR27oXr4ZE984MbSER8=
 github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=

model/errors.go

@@ -6,4 +6,5 @@ var (
 	ErrNotFound      = errors.New("data not found")
 	ErrInvalidAuth   = errors.New("invalid authentication")
 	ErrNotAuthorized = errors.New("not authorized")
+	ErrNotAvailable  = errors.New("functionality not available")
 )