fix(artwork): refresh stale artist image URLs on expiry (#5267)

* fix(external): refresh stale artist image URLs on expiry

ArtistImage() was serving cached image URLs from the database
indefinitely, ignoring ExternalInfoUpdatedAt. When users changed agent
configuration (e.g. disabling Deezer), old URLs persisted because only
the UpdateArtistInfo code path checked the TTL.

Now ArtistImage() checks the expiry and enqueues a background refresh
when the cached info is stale, matching the pattern used by
refreshArtistInfo(). The stale URL is still returned immediately to
avoid blocking clients.

Fixes #5266

* test: add expired artist image info test with log assertion

Verify that ArtistImage() enqueues a background refresh when cached
info is expired, by capturing log output and checking for the expected
debug message. Also asserts the stale URL is returned immediately
without calling the agent.

Signed-off-by: Deluan <deluan@navidrome.org>

* fix: only enqueue refresh when returning a stale cached URL

Move the expiry check to the else branch so we only enqueue a
background refresh when a cached image URL exists and is being
returned. This avoids doubling external API calls when the URL is
empty (synchronous fetch) but ExternalInfoUpdatedAt is old.

---------

Signed-off-by: Deluan <deluan@navidrome.org>

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 == "" {

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