feat(server/m7-353): admin endpoints for cover refetch (per-album + bulk)

Adds POST /api/admin/albums/{id}/cover/refetch (synchronous single-album retry) and POST /api/admin/covers/refetch-missing (async bulk drain). Wires coverart.Enricher through server.New → api.Mount → handlers struct. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-04 15:11:53 -04:00
parent 6c33acf062
commit d4a837b904
6 changed files with 245 additions and 31 deletions
@@ -134,7 +134,7 @@ func run() error {

 	srv := server.New(logger, pool, scanner, subsonic.Config{
 		AllowPlaintextPassword: cfg.Subsonic.AllowPlaintextPassword,
-	}, cfg.Events, cfg.Recommendation, cfg.Storage.DataDir, cfg.Branding)
+	}, cfg.Events, cfg.Recommendation, cfg.Storage.DataDir, cfg.Branding, coverEnricher, cfg.Library.CoverArtBackfillCap)
 	playlists.StartSystemPlaylistCron(ctx, pool, logger.With("component", "system_playlist_cron"))
 	httpServer := &http.Server{
 		Addr:              cfg.Server.Address,
@@ -0,0 +1,74 @@
+package api
+
+import (
+	"context"
+	"errors"
+	"net/http"
+
+	"github.com/go-chi/chi/v5"
+	"github.com/jackc/pgx/v5"
+
+	"git.fabledsword.com/bvandeusen/minstrel/internal/coverart"
+	"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
+)
+
+type adminCoverRefetchResp struct {
+	AlbumID        string  `json:"album_id"`
+	CoverArtPath   *string `json:"cover_art_path"`
+	CoverArtSource *string `json:"cover_art_source"`
+}
+
+// handleAdminAlbumRefetchCover implements POST /api/admin/albums/{id}/cover/refetch.
+// Synchronously clears + re-runs the enricher for one album.
+func (h *handlers) handleAdminAlbumRefetchCover(w http.ResponseWriter, r *http.Request) {
+	albumID, ok := parseUUID(chi.URLParam(r, "id"))
+	if !ok {
+		writeErr(w, http.StatusBadRequest, "bad_request", "invalid album id")
+		return
+	}
+	if err := h.coverart.RetryAlbum(r.Context(), albumID); err != nil {
+		if errors.Is(err, coverart.ErrNotFound) || errors.Is(err, pgx.ErrNoRows) {
+			writeErr(w, http.StatusNotFound, "not_found", "album not found")
+			return
+		}
+		h.logger.Error("admin: cover refetch failed", "err", err)
+		writeErr(w, http.StatusInternalServerError, "server_error", "refetch failed")
+		return
+	}
+	row, err := dbq.New(h.pool).GetAlbumWithFirstTrackPath(r.Context(), albumID)
+	if err != nil {
+		h.logger.Error("admin: post-refetch read failed", "err", err)
+		writeErr(w, http.StatusInternalServerError, "server_error", "read failed")
+		return
+	}
+	writeJSON(w, http.StatusOK, adminCoverRefetchResp{
+		AlbumID:        uuidToString(albumID),
+		CoverArtPath:   row.CoverArtPath,
+		CoverArtSource: row.CoverArtSource,
+	})
+}
+
+type adminBulkRefetchResp struct {
+	Queued int `json:"queued"`
+}
+
+// handleAdminBulkRefetchCovers implements POST /api/admin/covers/refetch-missing.
+// Returns immediately; actual drain runs in a background goroutine bounded by
+// the configured backfill cap.
+func (h *handlers) handleAdminBulkRefetchCovers(w http.ResponseWriter, r *http.Request) {
+	cap := h.coverArtBackfillCap
+	q := dbq.New(h.pool)
+	rows, err := q.ListAlbumsRetryMissing(r.Context(), int32(cap))
+	if err != nil {
+		h.logger.Error("admin: bulk count failed", "err", err)
+		writeErr(w, http.StatusInternalServerError, "server_error", "count failed")
+		return
+	}
+	go func() {
+		bgCtx := context.Background()
+		if _, err := h.coverart.EnrichRetryMissing(bgCtx, cap); err != nil {
+			h.logger.Warn("admin: bulk cover refetch failed", "err", err)
+		}
+	}()
+	writeJSON(w, http.StatusOK, adminBulkRefetchResp{Queued: len(rows)})
+}
@@ -0,0 +1,129 @@
+package api
+
+import (
+	"encoding/json"
+	"log/slog"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"testing"
+
+	"github.com/go-chi/chi/v5"
+
+	"git.fabledsword.com/bvandeusen/minstrel/internal/auth"
+	"git.fabledsword.com/bvandeusen/minstrel/internal/coverart"
+	"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
+)
+
+// newAdminCoversRouter builds a test chi router with the two cover admin
+// endpoints. RequireAdmin middleware is applied; tests inject the user into
+// context manually (RequireUser is bypassed).
+func newAdminCoversRouter(h *handlers) chi.Router {
+	r := chi.NewRouter()
+	r.Route("/api/admin", func(admin chi.Router) {
+		admin.Use(auth.RequireAdmin())
+		admin.Post("/albums/{id}/cover/refetch", h.handleAdminAlbumRefetchCover)
+		admin.Post("/covers/refetch-missing", h.handleAdminBulkRefetchCovers)
+	})
+	return r
+}
+
+// doAdminCoversReq fires an HTTP request against the admin covers router
+// with the given user in context.
+func doAdminCoversReq(t *testing.T, h *handlers, method, path string, user dbq.User) *httptest.ResponseRecorder {
+	t.Helper()
+	req := httptest.NewRequest(method, path, nil)
+	req = withUser(req, user)
+	w := httptest.NewRecorder()
+	newAdminCoversRouter(h).ServeHTTP(w, req)
+	return w
+}
+
+// testHandlersWithCovers returns a handlers instance with a stub coverart
+// Enricher wired in. The stub uses mbcaaOn=false + nil fetcher so the
+// sidecar-only path is exercised and cover_art_source lands as 'none' for
+// albums without real track files.
+func testHandlersWithCovers(t *testing.T) (*handlers, *coverart.Enricher) {
+	t.Helper()
+	h, pool := testHandlers(t)
+	enricher := coverart.NewEnricher(pool, slog.Default(), nil, false)
+	h.coverart = enricher
+	h.coverArtBackfillCap = 100
+	return h, enricher
+}
+
+func TestAdminAlbumRefetchCover_NonAdminReturns403(t *testing.T) {
+	if os.Getenv("MINSTREL_TEST_DATABASE_URL") == "" {
+		t.Skip("MINSTREL_TEST_DATABASE_URL not set")
+	}
+	h, _ := testHandlersWithCovers(t)
+	user := seedUser(t, h.pool, "alice", "pw", false)
+	artist := seedArtist(t, h.pool, "TestArtist")
+	album := seedAlbum(t, h.pool, artist.ID, "TestAlbum", 0)
+
+	w := doAdminCoversReq(t, h, http.MethodPost,
+		"/api/admin/albums/"+uuidToString(album.ID)+"/cover/refetch", user)
+	if w.Code != http.StatusForbidden {
+		t.Errorf("status = %d, want 403; body=%s", w.Code, w.Body.String())
+	}
+}
+
+func TestAdminAlbumRefetchCover_AdminOK(t *testing.T) {
+	if os.Getenv("MINSTREL_TEST_DATABASE_URL") == "" {
+		t.Skip("MINSTREL_TEST_DATABASE_URL not set")
+	}
+	h, _ := testHandlersWithCovers(t)
+	admin := seedUser(t, h.pool, "rootuser", "pw", true)
+	artist := seedArtist(t, h.pool, "TestArtist")
+	album := seedAlbum(t, h.pool, artist.ID, "TestAlbum", 0)
+
+	w := doAdminCoversReq(t, h, http.MethodPost,
+		"/api/admin/albums/"+uuidToString(album.ID)+"/cover/refetch", admin)
+	if w.Code != http.StatusOK {
+		t.Errorf("status = %d, want 200; body=%s", w.Code, w.Body.String())
+	}
+	var resp adminCoverRefetchResp
+	if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
+		t.Fatalf("decode: %v", err)
+	}
+	// Album has no MBID and no sidecar, so source should be 'none'.
+	if resp.CoverArtSource == nil || *resp.CoverArtSource != "none" {
+		t.Errorf("source = %v, want 'none'", resp.CoverArtSource)
+	}
+}
+
+func TestAdminBulkRefetchCovers_AdminReturnsQueuedCount(t *testing.T) {
+	if os.Getenv("MINSTREL_TEST_DATABASE_URL") == "" {
+		t.Skip("MINSTREL_TEST_DATABASE_URL not set")
+	}
+	h, _ := testHandlersWithCovers(t)
+	admin := seedUser(t, h.pool, "rootuser2", "pw", true)
+	artist := seedArtist(t, h.pool, "TestArtist")
+	_ = seedAlbum(t, h.pool, artist.ID, "Album1", 0)
+	_ = seedAlbum(t, h.pool, artist.ID, "Album2", 0)
+
+	w := doAdminCoversReq(t, h, http.MethodPost, "/api/admin/covers/refetch-missing", admin)
+	if w.Code != http.StatusOK {
+		t.Errorf("status = %d, want 200; body=%s", w.Code, w.Body.String())
+	}
+	var resp adminBulkRefetchResp
+	if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
+		t.Fatalf("decode: %v", err)
+	}
+	if resp.Queued < 2 {
+		t.Errorf("queued = %d, want >= 2", resp.Queued)
+	}
+}
+
+func TestAdminBulkRefetchCovers_NonAdminReturns403(t *testing.T) {
+	if os.Getenv("MINSTREL_TEST_DATABASE_URL") == "" {
+		t.Skip("MINSTREL_TEST_DATABASE_URL not set")
+	}
+	h, _ := testHandlersWithCovers(t)
+	user := seedUser(t, h.pool, "alicebulk", "pw", false)
+
+	w := doAdminCoversReq(t, h, http.MethodPost, "/api/admin/covers/refetch-missing", user)
+	if w.Code != http.StatusForbidden {
+		t.Errorf("status = %d, want 403; body=%s", w.Code, w.Body.String())
+	}
+}
@@ -13,6 +13,7 @@ import (

 	"git.fabledsword.com/bvandeusen/minstrel/internal/auth"
 	"git.fabledsword.com/bvandeusen/minstrel/internal/config"
+	"git.fabledsword.com/bvandeusen/minstrel/internal/coverart"
 	"git.fabledsword.com/bvandeusen/minstrel/internal/lidarrconfig"
 	"git.fabledsword.com/bvandeusen/minstrel/internal/lidarrquarantine"
 	"git.fabledsword.com/bvandeusen/minstrel/internal/lidarrrequests"
@@ -24,17 +25,19 @@ import (
 // Mount attaches /api/* handlers to r. Public endpoints (login) are outside
 // RequireUser; everything else is gated by the middleware. The events writer
 // is shared with the Subsonic mount so /rest/scrobble feeds the same store.
-func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playevents.Writer, recCfg config.RecommendationConfig, lidarrCfg *lidarrconfig.Service, lidarrReqs *lidarrrequests.Service, lidarrQuar *lidarrquarantine.Service, tracksSvc *tracks.Service, playlistsSvc *playlists.Service, dataDir string) {
+func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playevents.Writer, recCfg config.RecommendationConfig, lidarrCfg *lidarrconfig.Service, lidarrReqs *lidarrrequests.Service, lidarrQuar *lidarrquarantine.Service, tracksSvc *tracks.Service, playlistsSvc *playlists.Service, coverEnricher *coverart.Enricher, coverBackfillCap int, dataDir string) {
 	rng := rand.New(rand.NewSource(rand.Int63()))
 	h := &handlers{
 		pool: pool, logger: logger, events: events, recCfg: recCfg,
-		rng:              rng.Float64,
-		lidarrCfg:        lidarrCfg,
-		lidarrRequests:   lidarrReqs,
-		lidarrQuarantine: lidarrQuar,
-		tracks:           tracksSvc,
-		playlists:        playlistsSvc,
-		dataDir:          dataDir,
+		rng:                rng.Float64,
+		lidarrCfg:          lidarrCfg,
+		lidarrRequests:     lidarrReqs,
+		lidarrQuarantine:   lidarrQuar,
+		tracks:             tracksSvc,
+		playlists:          playlistsSvc,
+		coverart:           coverEnricher,
+		coverArtBackfillCap: coverBackfillCap,
+		dataDir:            dataDir,
 	}

 	r.Route("/api", func(api chi.Router) {
@@ -101,6 +104,9 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev
 				admin.Get("/quarantine/actions", h.handleListQuarantineActions)

 				admin.Delete("/tracks/{id}", h.handleRemoveTrack)
+
+				admin.Post("/albums/{id}/cover/refetch", h.handleAdminAlbumRefetchCover)
+				admin.Post("/covers/refetch-missing", h.handleAdminBulkRefetchCovers)
 			})

 			authed.Get("/playlists", h.handleListPlaylists)
@@ -117,15 +123,17 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev
 }

 type handlers struct {
-	pool             *pgxpool.Pool
-	logger           *slog.Logger
-	events           *playevents.Writer
-	recCfg           config.RecommendationConfig
-	rng              func() float64
-	lidarrCfg        *lidarrconfig.Service
-	lidarrRequests   *lidarrrequests.Service
-	lidarrQuarantine *lidarrquarantine.Service
-	tracks           *tracks.Service
-	playlists        *playlists.Service
-	dataDir          string
+	pool                *pgxpool.Pool
+	logger              *slog.Logger
+	events              *playevents.Writer
+	recCfg              config.RecommendationConfig
+	rng                 func() float64
+	lidarrCfg           *lidarrconfig.Service
+	lidarrRequests      *lidarrrequests.Service
+	lidarrQuarantine    *lidarrquarantine.Service
+	tracks              *tracks.Service
+	playlists           *playlists.Service
+	coverart            *coverart.Enricher
+	coverArtBackfillCap int
+	dataDir             string
 }
@@ -17,6 +17,7 @@ import (
 	"git.fabledsword.com/bvandeusen/minstrel/internal/api"
 	"git.fabledsword.com/bvandeusen/minstrel/internal/auth"
 	"git.fabledsword.com/bvandeusen/minstrel/internal/config"
+	"git.fabledsword.com/bvandeusen/minstrel/internal/coverart"
 	"git.fabledsword.com/bvandeusen/minstrel/internal/library"
 	"git.fabledsword.com/bvandeusen/minstrel/internal/lidarr"
 	"git.fabledsword.com/bvandeusen/minstrel/internal/lidarrconfig"
@@ -67,12 +68,14 @@ type Server struct {
 	// playlist cover collages under <DataDir>/playlist_covers/). Empty
 	// strings are tolerated by tests that don't exercise persisted-cover
 	// codepaths; production callers should pass a writable directory.
-	DataDir     string
-	BrandingCfg config.BrandingConfig
+	DataDir             string
+	BrandingCfg         config.BrandingConfig
+	CoverEnricher       *coverart.Enricher
+	CoverArtBackfillCap int
 }

-func New(logger *slog.Logger, pool *pgxpool.Pool, scanner ScanTrigger, subCfg subsonic.Config, eventsCfg config.EventsConfig, recCfg config.RecommendationConfig, dataDir string, brandingCfg config.BrandingConfig) *Server {
-	return &Server{Logger: logger, Pool: pool, Scanner: scanner, SubsonicCfg: subCfg, EventsCfg: eventsCfg, RecommendationCfg: recCfg, DataDir: dataDir, BrandingCfg: brandingCfg}
+func New(logger *slog.Logger, pool *pgxpool.Pool, scanner ScanTrigger, subCfg subsonic.Config, eventsCfg config.EventsConfig, recCfg config.RecommendationConfig, dataDir string, brandingCfg config.BrandingConfig, coverEnricher *coverart.Enricher, coverArtBackfillCap int) *Server {
+	return &Server{Logger: logger, Pool: pool, Scanner: scanner, SubsonicCfg: subCfg, EventsCfg: eventsCfg, RecommendationCfg: recCfg, DataDir: dataDir, BrandingCfg: brandingCfg, CoverEnricher: coverEnricher, CoverArtBackfillCap: coverArtBackfillCap}
 }

 func (s *Server) Router() http.Handler {
@@ -106,7 +109,7 @@ func (s *Server) Router() http.Handler {
 		lidarrQuar := lidarrquarantine.NewService(s.Pool, lidarrCfg, lidarrClientFn)
 		tracksSvc := tracks.NewService(s.Pool, s.Logger, lidarrUnmonitorAdapter{fn: lidarrClientFn})
 		playlistsSvc := playlists.NewService(s.Pool, s.Logger, s.DataDir)
-		api.Mount(r, s.Pool, s.Logger, writer, s.RecommendationCfg, lidarrCfg, lidarrReqs, lidarrQuar, tracksSvc, playlistsSvc, s.DataDir)
+		api.Mount(r, s.Pool, s.Logger, writer, s.RecommendationCfg, lidarrCfg, lidarrReqs, lidarrQuar, tracksSvc, playlistsSvc, s.CoverEnricher, s.CoverArtBackfillCap, s.DataDir)
 		// /api/admin/scan is the only admin route owned by the server package
 		// (it needs the Scanner). Register it as a single inline-middleware
 		// route — using r.Route("/api/admin", ...) here would create a second
@@ -24,7 +24,7 @@ import (
 )

 func TestHealthz(t *testing.T) {
-	s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), nil, nil, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{})
+	s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), nil, nil, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{}, nil, 0)
 	ts := httptest.NewServer(s.Router())
 	defer ts.Close()

@@ -73,7 +73,7 @@ func TestHealthz_IncludesMinClientVersion(t *testing.T) {
 }

 func TestRouter_ServesSPAAtRoot(t *testing.T) {
-	s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), nil, nil, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{})
+	s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), nil, nil, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{}, nil, 0)
 	ts := httptest.NewServer(s.Router())
 	defer ts.Close()

@@ -92,7 +92,7 @@ func TestRouter_ServesSPAAtRoot(t *testing.T) {
 }

 func TestRouter_DeepLinkFallbackReturnsSPA(t *testing.T) {
-	s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), nil, nil, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{})
+	s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), nil, nil, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{}, nil, 0)
 	ts := httptest.NewServer(s.Router())
 	defer ts.Close()

@@ -111,7 +111,7 @@ func TestRouter_DeepLinkFallbackReturnsSPA(t *testing.T) {
 }

 func TestRouter_APIPathNotSwallowedBySPA(t *testing.T) {
-	s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), nil, nil, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{})
+	s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), nil, nil, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{}, nil, 0)
 	ts := httptest.NewServer(s.Router())
 	defer ts.Close()

@@ -130,7 +130,7 @@ func TestRouter_APIPathNotSwallowedBySPA(t *testing.T) {
 }

 func TestRouter_RestPathNotSwallowedBySPA(t *testing.T) {
-	s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), nil, nil, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{})
+	s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), nil, nil, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{}, nil, 0)
 	ts := httptest.NewServer(s.Router())
 	defer ts.Close()

@@ -206,7 +206,7 @@ func TestRouter_AdminSubtreeNotShadowed(t *testing.T) {
 	}

 	s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), pool,
-		stubScanner{}, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{})
+		stubScanner{}, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{}, nil, 0)
 	ts := httptest.NewServer(s.Router())
 	defer ts.Close()