From 3ce3185118b74b2add36ec470a8ae63423e65e26 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sat, 4 Feb 2023 21:18:51 -0500 Subject: [PATCH] Don't retrieve Various Artists and Unknown Artist info from Last.fm --- consts/consts.go | 1 + core/agents/agents.go | 35 ++++++++++++++++++++++++++++------- core/agents/agents_test.go | 37 +++++++++++++++++++++++++++++++++++++ core/agents/lastfm/agent.go | 12 ++++++++++++ core/external_metadata.go | 6 +++--- scanner/mapping.go | 2 +- 6 files changed, 82 insertions(+), 11 deletions(-) diff --git a/consts/consts.go b/consts/consts.go index 0da96fe9..f2e19317 100644 --- a/consts/consts.go +++ b/consts/consts.go @@ -116,6 +116,7 @@ var ( var ( VariousArtists = "Various Artists" VariousArtistsID = fmt.Sprintf("%x", md5.Sum([]byte(strings.ToLower(VariousArtists)))) + UnknownAlbum = "[Unknown Album]" UnknownArtist = "[Unknown Artist]" UnknownArtistID = fmt.Sprintf("%x", md5.Sum([]byte(strings.ToLower(UnknownArtist)))) VariousArtistsMbzId = "89ad4ac3-39f7-470e-963a-56509c546377" diff --git a/core/agents/agents.go b/core/agents/agents.go index 23068844..1f9b8c0d 100644 --- a/core/agents/agents.go +++ b/core/agents/agents.go @@ -42,8 +42,11 @@ func (a *Agents) AgentName() string { } func (a *Agents) GetArtistMBID(ctx context.Context, id string, name string) (string, error) { - if id == consts.UnknownArtistID { + switch id { + case consts.UnknownArtistID: return "", ErrNotFound + case consts.VariousArtistsID: + return "", nil } start := time.Now() for _, ag := range a.agents { @@ -64,8 +67,11 @@ func (a *Agents) GetArtistMBID(ctx context.Context, id string, name string) (str } func (a *Agents) GetArtistURL(ctx context.Context, id, name, mbid string) (string, error) { - if id == consts.UnknownArtistID { + switch id { + case consts.UnknownArtistID: return "", ErrNotFound + case consts.VariousArtistsID: + return "", nil } start := time.Now() for _, ag := range a.agents { @@ -86,8 +92,11 @@ func (a *Agents) GetArtistURL(ctx context.Context, id, name, mbid string) (strin } func (a *Agents) GetArtistBiography(ctx context.Context, id, name, mbid string) (string, error) { - if id == consts.UnknownArtistID { + switch id { + case consts.UnknownArtistID: return "", ErrNotFound + case consts.VariousArtistsID: + return "", nil } start := time.Now() for _, ag := range a.agents { @@ -99,7 +108,7 @@ func (a *Agents) GetArtistBiography(ctx context.Context, id, name, mbid string) continue } bio, err := agent.GetArtistBiography(ctx, id, name, mbid) - if bio != "" && err == nil { + if err == nil { log.Debug(ctx, "Got Biography", "agent", ag.AgentName(), "artist", name, "len", len(bio), "elapsed", time.Since(start)) return bio, nil } @@ -108,8 +117,11 @@ func (a *Agents) GetArtistBiography(ctx context.Context, id, name, mbid string) } func (a *Agents) GetSimilarArtists(ctx context.Context, id, name, mbid string, limit int) ([]Artist, error) { - if id == consts.UnknownArtistID { + switch id { + case consts.UnknownArtistID: return nil, ErrNotFound + case consts.VariousArtistsID: + return nil, nil } start := time.Now() for _, ag := range a.agents { @@ -134,8 +146,11 @@ func (a *Agents) GetSimilarArtists(ctx context.Context, id, name, mbid string, l } func (a *Agents) GetArtistImages(ctx context.Context, id, name, mbid string) ([]ExternalImage, error) { - if id == consts.UnknownArtistID { + switch id { + case consts.UnknownArtistID: return nil, ErrNotFound + case consts.VariousArtistsID: + return nil, nil } start := time.Now() for _, ag := range a.agents { @@ -156,8 +171,11 @@ func (a *Agents) GetArtistImages(ctx context.Context, id, name, mbid string) ([] } func (a *Agents) GetArtistTopSongs(ctx context.Context, id, artistName, mbid string, count int) ([]Song, error) { - if id == consts.UnknownArtistID { + switch id { + case consts.UnknownArtistID: return nil, ErrNotFound + case consts.VariousArtistsID: + return nil, nil } start := time.Now() for _, ag := range a.agents { @@ -178,6 +196,9 @@ func (a *Agents) GetArtistTopSongs(ctx context.Context, id, artistName, mbid str } func (a *Agents) GetAlbumInfo(ctx context.Context, name, artist, mbid string) (*AlbumInfo, error) { + if name == consts.UnknownAlbum { + return nil, ErrNotFound + } start := time.Now() for _, ag := range a.agents { if utils.IsCtxDone(ctx) { diff --git a/core/agents/agents_test.go b/core/agents/agents_test.go index 485343b4..33687048 100644 --- a/core/agents/agents_test.go +++ b/core/agents/agents_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" + "github.com/navidrome/navidrome/consts" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/tests" @@ -61,6 +62,18 @@ var _ = Describe("Agents", func() { Expect(ag.GetArtistMBID(ctx, "123", "test")).To(Equal("mbid")) Expect(mock.Args).To(ConsistOf("123", "test")) }) + It("returns empty if artist is Various Artists", func() { + mbid, err := ag.GetArtistMBID(ctx, consts.VariousArtistsID, consts.VariousArtists) + Expect(err).ToNot(HaveOccurred()) + Expect(mbid).To(BeEmpty()) + Expect(mock.Args).To(BeEmpty()) + }) + It("returns not found if artist is Unknown Artist", func() { + mbid, err := ag.GetArtistMBID(ctx, consts.VariousArtistsID, consts.VariousArtists) + Expect(err).ToNot(HaveOccurred()) + Expect(mbid).To(BeEmpty()) + Expect(mock.Args).To(BeEmpty()) + }) It("skips the agent if it returns an error", func() { mock.Err = errors.New("error") _, err := ag.GetArtistMBID(ctx, "123", "test") @@ -80,6 +93,18 @@ var _ = Describe("Agents", func() { Expect(ag.GetArtistURL(ctx, "123", "test", "mb123")).To(Equal("url")) Expect(mock.Args).To(ConsistOf("123", "test", "mb123")) }) + It("returns empty if artist is Various Artists", func() { + url, err := ag.GetArtistURL(ctx, consts.VariousArtistsID, consts.VariousArtists, "") + Expect(err).ToNot(HaveOccurred()) + Expect(url).To(BeEmpty()) + Expect(mock.Args).To(BeEmpty()) + }) + It("returns not found if artist is Unknown Artist", func() { + url, err := ag.GetArtistURL(ctx, consts.VariousArtistsID, consts.VariousArtists, "") + Expect(err).ToNot(HaveOccurred()) + Expect(url).To(BeEmpty()) + Expect(mock.Args).To(BeEmpty()) + }) It("skips the agent if it returns an error", func() { mock.Err = errors.New("error") _, err := ag.GetArtistURL(ctx, "123", "test", "mb123") @@ -99,6 +124,18 @@ var _ = Describe("Agents", func() { Expect(ag.GetArtistBiography(ctx, "123", "test", "mb123")).To(Equal("bio")) Expect(mock.Args).To(ConsistOf("123", "test", "mb123")) }) + It("returns empty if artist is Various Artists", func() { + bio, err := ag.GetArtistBiography(ctx, consts.VariousArtistsID, consts.VariousArtists, "") + Expect(err).ToNot(HaveOccurred()) + Expect(bio).To(BeEmpty()) + Expect(mock.Args).To(BeEmpty()) + }) + It("returns not found if artist is Unknown Artist", func() { + bio, err := ag.GetArtistBiography(ctx, consts.VariousArtistsID, consts.VariousArtists, "") + Expect(err).ToNot(HaveOccurred()) + Expect(bio).To(BeEmpty()) + Expect(mock.Args).To(BeEmpty()) + }) It("skips the agent if it returns an error", func() { mock.Err = errors.New("error") _, err := ag.GetArtistBiography(ctx, "123", "test", "mb123") diff --git a/core/agents/lastfm/agent.go b/core/agents/lastfm/agent.go index 52b0a99a..f12cf2d0 100644 --- a/core/agents/lastfm/agent.go +++ b/core/agents/lastfm/agent.go @@ -6,6 +6,7 @@ import ( "net/http" "regexp" "strconv" + "strings" "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/consts" @@ -21,6 +22,11 @@ const ( sessionKeyProperty = "LastFMSessionKey" ) +var ignoredBiographies = []string{ + // Unknown Artist + ` images[j].Size }) diff --git a/scanner/mapping.go b/scanner/mapping.go index eab71100..812c9556 100644 --- a/scanner/mapping.go +++ b/scanner/mapping.go @@ -122,7 +122,7 @@ func (s mediaFileMapper) mapArtistName(md metadata.Tags) string { func (s mediaFileMapper) mapAlbumName(md metadata.Tags) string { name := md.Album() if name == "" { - return "[Unknown Album]" + return consts.UnknownAlbum } return name }