diff --git a/core/agents/agents.go b/core/agents/agents.go index 0a11297c3..335ebc57f 100644 --- a/core/agents/agents.go +++ b/core/agents/agents.go @@ -24,15 +24,23 @@ func New(ds model.DataStore) *Agents { } order = append(order, LocalAgentName) var res []Interface + var enabled []string for _, name := range order { init, ok := Map[name] if !ok { - log.Error("Agent not available. Check configuration", "name", name) + log.Error("Invalid agent. Check `Agents` configuration", "name", name, "conf", conf.Server.Agents) continue } + agent := init(ds) + if agent == nil { + log.Debug("Agent not available. Missing configuration?", "name", name) + continue + } + enabled = append(enabled, name) res = append(res, init(ds)) } + log.Debug("List of agents enabled", "names", enabled) return &Agents{ds: ds, agents: res} } diff --git a/core/agents/agents_test.go b/core/agents/agents_test.go index d61d63f79..53ca5099d 100644 --- a/core/agents/agents_test.go +++ b/core/agents/agents_test.go @@ -7,6 +7,7 @@ import ( "github.com/navidrome/navidrome/consts" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/tests" + "github.com/navidrome/navidrome/utils/slice" "github.com/navidrome/navidrome/conf" . "github.com/onsi/ginkgo/v2" @@ -44,19 +45,21 @@ var _ = Describe("Agents", func() { var mock *mockAgent BeforeEach(func() { mock = &mockAgent{} - Register("fake", func(ds model.DataStore) Interface { - return mock - }) - Register("empty", func(ds model.DataStore) Interface { - return struct { - Interface - }{} - }) - conf.Server.Agents = "empty,fake" + Register("fake", func(model.DataStore) Interface { return mock }) + Register("disabled", func(model.DataStore) Interface { return nil }) + Register("empty", func(model.DataStore) Interface { return &emptyAgent{} }) + conf.Server.Agents = "empty,fake,disabled" ag = New(ds) Expect(ag.AgentName()).To(Equal("agents")) }) + It("does not register disabled agents", func() { + ags := slice.Map(ag.agents, func(a Interface) string { return a.AgentName() }) + // local agent is always appended to the end of the agents list + Expect(ags).To(HaveExactElements("empty", "fake", "local")) + Expect(ags).ToNot(ContainElement("disabled")) + }) + Describe("GetArtistMBID", func() { It("returns on first match", func() { Expect(ag.GetArtistMBID(ctx, "123", "test")).To(Equal("mbid")) @@ -344,3 +347,11 @@ func (a *mockAgent) GetAlbumInfo(ctx context.Context, name, artist, mbid string) }, }, nil } + +type emptyAgent struct { + Interface +} + +func (e *emptyAgent) AgentName() string { + return "empty" +} diff --git a/core/agents/lastfm/agent.go b/core/agents/lastfm/agent.go index 1c46b20e4..01ffa677e 100644 --- a/core/agents/lastfm/agent.go +++ b/core/agents/lastfm/agent.go @@ -42,6 +42,9 @@ type lastfmAgent struct { } func lastFMConstructor(ds model.DataStore) *lastfmAgent { + if !conf.Server.LastFM.Enabled || conf.Server.LastFM.ApiKey == "" || conf.Server.LastFM.Secret == "" { + return nil + } l := &lastfmAgent{ ds: ds, lang: conf.Server.LastFM.Language, @@ -340,15 +343,19 @@ func (l *lastfmAgent) IsAuthorized(ctx context.Context, userId string) bool { func init() { conf.AddHook(func() { - if conf.Server.LastFM.Enabled { - if conf.Server.LastFM.ApiKey != "" && conf.Server.LastFM.Secret != "" { - agents.Register(lastFMAgentName, func(ds model.DataStore) agents.Interface { - return lastFMConstructor(ds) - }) - scrobbler.Register(lastFMAgentName, func(ds model.DataStore) scrobbler.Scrobbler { - return lastFMConstructor(ds) - }) + agents.Register(lastFMAgentName, func(ds model.DataStore) agents.Interface { + a := lastFMConstructor(ds) + if a != nil { + return a } - } + return nil + }) + scrobbler.Register(lastFMAgentName, func(ds model.DataStore) scrobbler.Scrobbler { + a := lastFMConstructor(ds) + if a != nil { + return a + } + return nil + }) }) } diff --git a/core/agents/lastfm/agent_test.go b/core/agents/lastfm/agent_test.go index 461387cd4..de4fac6d6 100644 --- a/core/agents/lastfm/agent_test.go +++ b/core/agents/lastfm/agent_test.go @@ -11,6 +11,7 @@ import ( "time" "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/conf/configtest" "github.com/navidrome/navidrome/core/agents" "github.com/navidrome/navidrome/core/scrobbler" "github.com/navidrome/navidrome/model" @@ -30,16 +31,38 @@ var _ = Describe("lastfmAgent", func() { BeforeEach(func() { ds = &tests.MockDataStore{} ctx = context.Background() + DeferCleanup(configtest.SetupConfig()) + conf.Server.LastFM.Enabled = true + conf.Server.LastFM.ApiKey = "123" + conf.Server.LastFM.Secret = "secret" }) Describe("lastFMConstructor", func() { - It("uses configured api key and language", func() { - conf.Server.LastFM.ApiKey = "123" - conf.Server.LastFM.Secret = "secret" - conf.Server.LastFM.Language = "pt" - agent := lastFMConstructor(ds) - Expect(agent.apiKey).To(Equal("123")) - Expect(agent.secret).To(Equal("secret")) - Expect(agent.lang).To(Equal("pt")) + When("Agent is properly configured", func() { + It("uses configured api key and language", func() { + conf.Server.LastFM.Language = "pt" + agent := lastFMConstructor(ds) + Expect(agent.apiKey).To(Equal("123")) + Expect(agent.secret).To(Equal("secret")) + Expect(agent.lang).To(Equal("pt")) + }) + }) + When("Agent is disabled", func() { + It("returns nil", func() { + conf.Server.LastFM.Enabled = false + Expect(lastFMConstructor(ds)).To(BeNil()) + }) + }) + When("ApiKey is empty", func() { + It("returns nil", func() { + conf.Server.LastFM.ApiKey = "" + Expect(lastFMConstructor(ds)).To(BeNil()) + }) + }) + When("Secret is empty", func() { + It("returns nil", func() { + conf.Server.LastFM.Secret = "" + Expect(lastFMConstructor(ds)).To(BeNil()) + }) }) }) diff --git a/core/agents/spotify/spotify.go b/core/agents/spotify/spotify.go index 869c0ecc8..633c32984 100644 --- a/core/agents/spotify/spotify.go +++ b/core/agents/spotify/spotify.go @@ -27,6 +27,9 @@ type spotifyAgent struct { } func spotifyConstructor(ds model.DataStore) agents.Interface { + if conf.Server.Spotify.ID == "" || conf.Server.Spotify.Secret == "" { + return nil + } l := &spotifyAgent{ ds: ds, id: conf.Server.Spotify.ID, @@ -88,8 +91,6 @@ func (s *spotifyAgent) searchArtist(ctx context.Context, name string) (*Artist, func init() { conf.AddHook(func() { - if conf.Server.Spotify.ID != "" && conf.Server.Spotify.Secret != "" { - agents.Register(spotifyAgentName, spotifyConstructor) - } + agents.Register(spotifyAgentName, spotifyConstructor) }) } diff --git a/core/scrobbler/play_tracker.go b/core/scrobbler/play_tracker.go index 64dea0697..7a8a87d7b 100644 --- a/core/scrobbler/play_tracker.go +++ b/core/scrobbler/play_tracker.go @@ -53,13 +53,20 @@ func newPlayTracker(ds model.DataStore, broker events.Broker) *playTracker { m := cache.NewSimpleCache[string, NowPlayingInfo]() p := &playTracker{ds: ds, playMap: m, broker: broker} p.scrobblers = make(map[string]Scrobbler) + var enabled []string for name, constructor := range constructors { s := constructor(ds) + if s == nil { + log.Debug("Scrobbler not available. Missing configuration?", "name", name) + continue + } + enabled = append(enabled, name) if conf.Server.DevEnableBufferedScrobble { s = newBufferedScrobbler(ds, s, name) } p.scrobblers[name] = s } + log.Debug("List of scrobblers enabled", "names", enabled) return p } diff --git a/core/scrobbler/play_tracker_test.go b/core/scrobbler/play_tracker_test.go index 55bca5615..a4bd7cec2 100644 --- a/core/scrobbler/play_tracker_test.go +++ b/core/scrobbler/play_tracker_test.go @@ -35,9 +35,12 @@ var _ = Describe("PlayTracker", func() { ctx = request.WithPlayer(ctx, model.Player{ScrobbleEnabled: true}) ds = &tests.MockDataStore{} fake = fakeScrobbler{Authorized: true} - Register("fake", func(ds model.DataStore) Scrobbler { + Register("fake", func(model.DataStore) Scrobbler { return &fake }) + Register("disabled", func(model.DataStore) Scrobbler { + return nil + }) tracker = newPlayTracker(ds, events.GetBroker()) track = model.MediaFile{ @@ -61,6 +64,11 @@ var _ = Describe("PlayTracker", func() { _ = ds.Album(ctx).(*tests.MockAlbumRepo).Put(&album) }) + It("does not register disabled scrobblers", func() { + Expect(tracker.(*playTracker).scrobblers).To(HaveKey("fake")) + Expect(tracker.(*playTracker).scrobblers).ToNot(HaveKey("disabled")) + }) + Describe("NowPlaying", func() { It("sends track to agent", func() { err := tracker.NowPlaying(ctx, "player-1", "player-one", "123") diff --git a/persistence/artist_repository_test.go b/persistence/artist_repository_test.go index 33a9ace8e..f9e58d216 100644 --- a/persistence/artist_repository_test.go +++ b/persistence/artist_repository_test.go @@ -18,6 +18,7 @@ var _ = Describe("ArtistRepository", func() { var repo model.ArtistRepository BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) ctx := log.NewContext(context.TODO()) ctx = request.WithUser(ctx, model.User{ID: "userid"}) repo = NewArtistRepository(ctx, GetDBXBuilder()) @@ -51,7 +52,6 @@ var _ = Describe("ArtistRepository", func() { r := artistRepository{indexGroups: utils.ParseIndexGroups(conf.Server.IndexGroups)} When("PreferSortTags is false", func() { BeforeEach(func() { - DeferCleanup(configtest.SetupConfig) conf.Server.PreferSortTags = false }) It("returns the OrderArtistName key is SortArtistName is empty", func() { @@ -68,7 +68,6 @@ var _ = Describe("ArtistRepository", func() { }) When("PreferSortTags is true", func() { BeforeEach(func() { - DeferCleanup(configtest.SetupConfig) conf.Server.PreferSortTags = true }) It("returns the SortArtistName key if it is not empty", func() { @@ -87,7 +86,6 @@ var _ = Describe("ArtistRepository", func() { Describe("GetIndex", func() { When("PreferSortTags is true", func() { BeforeEach(func() { - DeferCleanup(configtest.SetupConfig()) conf.Server.PreferSortTags = true }) It("returns the index when PreferSortTags is true and SortArtistName is not empty", func() { @@ -128,7 +126,6 @@ var _ = Describe("ArtistRepository", func() { When("PreferSortTags is false", func() { BeforeEach(func() { - DeferCleanup(configtest.SetupConfig()) conf.Server.PreferSortTags = false }) It("returns the index when SortArtistName is NOT empty", func() {