diff --git a/core/external/provider.go b/core/external/provider.go index a30afed1..40ca3406 100644 --- a/core/external/provider.go +++ b/core/external/provider.go @@ -374,8 +374,6 @@ func (e *provider) ArtistImage(ctx context.Context, id string) (*url.URL, error) return nil, err } - // Use already-stored image URL if available, avoiding expensive external API calls. - // If the info is expired, the background refresh (via UpdateArtistInfo/artistQueue) will update it. imageUrl := artist.ArtistImageUrl() if imageUrl == "" { // No cached URL — must fetch from external source synchronously @@ -385,6 +383,14 @@ func (e *provider) ArtistImage(ctx context.Context, id string) (*url.URL, error) return nil, ctx.Err() } imageUrl = artist.ArtistImageUrl() + } else { + // If cached info is expired, enqueue a background refresh so that config changes + // (e.g. disabling an agent) take effect without waiting for a full artist info refresh. + updatedAt := V(artist.ExternalInfoUpdatedAt) + if !updatedAt.IsZero() && time.Since(updatedAt) > conf.Server.DevArtistInfoTimeToLive { + log.Debug(ctx, "Artist image info expired, enqueuing background refresh", "artist", artist.Name(), "updatedAt", updatedAt) + e.artistQueue.enqueue(&artist) + } } if imageUrl == "" { diff --git a/core/external/provider_artistimage_test.go b/core/external/provider_artistimage_test.go index 11290bb6..529289ed 100644 --- a/core/external/provider_artistimage_test.go +++ b/core/external/provider_artistimage_test.go @@ -1,14 +1,17 @@ package external_test import ( + "bytes" "context" "errors" "net/url" + "time" "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/conf/configtest" "github.com/navidrome/navidrome/core/agents" . "github.com/navidrome/navidrome/core/external" + "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" @@ -266,6 +269,68 @@ var _ = Describe("Provider - ArtistImage", func() { mockImageAgent.AssertCalled(GinkgoT(), "GetArtistImages", ctx, "artist-1", "Artist One", "") }) + It("returns cached URL and does not call agent when info is not expired", func() { + // Arrange: artist has a cached image URL with recent ExternalInfoUpdatedAt + recentTime := time.Now().Add(-1 * time.Minute) + cachedArtist := &model.Artist{ + ID: "artist-cached", + Name: "Cached Artist", + LargeImageUrl: "http://example.com/cached-large.jpg", + ExternalInfoUpdatedAt: &recentTime, + } + mockArtistRepo.On("Get", "artist-cached").Return(cachedArtist, nil).Maybe() + expectedURL, _ := url.Parse("http://example.com/cached-large.jpg") + + // Capture log output + var logBuf bytes.Buffer + log.SetOutput(&logBuf) + defer log.SetOutput(GinkgoWriter) + log.SetLevel(log.LevelDebug) + + // Act + imgURL, err := provider.ArtistImage(ctx, "artist-cached") + + // Assert + Expect(err).ToNot(HaveOccurred()) + Expect(imgURL).To(Equal(expectedURL)) + mockImageAgent.AssertNotCalled(GinkgoT(), "GetArtistImages", mock.Anything, "artist-cached", mock.Anything, mock.Anything) + + // Assert: background refresh was NOT enqueued + Expect(logBuf.String()).ToNot(ContainSubstring("Artist image info expired, enqueuing background refresh")) + + }) + + It("returns stale URL and enqueues refresh when info is expired", func() { + // Arrange + conf.Server.DevArtistInfoTimeToLive = 1 * time.Nanosecond + expiredTime := time.Now().Add(-1 * time.Hour) + staleArtist := &model.Artist{ + ID: "artist-expired", + Name: "Expired Artist", + LargeImageUrl: "http://example.com/expired-large.jpg", + ExternalInfoUpdatedAt: &expiredTime, + } + mockArtistRepo.On("Get", "artist-expired").Return(staleArtist, nil).Maybe() + expectedURL, _ := url.Parse("http://example.com/expired-large.jpg") + + // Capture log output + var logBuf bytes.Buffer + log.SetOutput(&logBuf) + defer log.SetOutput(GinkgoWriter) + log.SetLevel(log.LevelDebug) + + // Act + imgURL, err := provider.ArtistImage(ctx, "artist-expired") + + // Assert: returns stale URL immediately, no agent call + Expect(err).ToNot(HaveOccurred()) + Expect(imgURL).To(Equal(expectedURL)) + mockImageAgent.AssertNotCalled(GinkgoT(), "GetArtistImages", mock.Anything, "artist-expired", mock.Anything, mock.Anything) + + // Assert: background refresh was enqueued + Expect(logBuf.String()).To(ContainSubstring("Artist image info expired, enqueuing background refresh")) + }) + Context("Unicode handling in artist names", func() { var artistWithEnDash *model.Artist var expectedURL *url.URL