From d4a837b904c96e7d1014ad443c39852ec22b5e09 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 4 May 2026 15:11:53 -0400 Subject: [PATCH] feat(server/m7-353): admin endpoints for cover refetch (per-album + bulk) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- cmd/minstrel/main.go | 2 +- internal/api/admin_covers.go | 74 +++++++++++++++++ internal/api/admin_covers_test.go | 129 ++++++++++++++++++++++++++++++ internal/api/api.go | 46 ++++++----- internal/server/server.go | 13 +-- internal/server/server_test.go | 12 +-- 6 files changed, 245 insertions(+), 31 deletions(-) create mode 100644 internal/api/admin_covers.go create mode 100644 internal/api/admin_covers_test.go diff --git a/cmd/minstrel/main.go b/cmd/minstrel/main.go index bb2ae9bd..3905aa42 100644 --- a/cmd/minstrel/main.go +++ b/cmd/minstrel/main.go @@ -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, diff --git a/internal/api/admin_covers.go b/internal/api/admin_covers.go new file mode 100644 index 00000000..2090c177 --- /dev/null +++ b/internal/api/admin_covers.go @@ -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)}) +} diff --git a/internal/api/admin_covers_test.go b/internal/api/admin_covers_test.go new file mode 100644 index 00000000..ee2202f5 --- /dev/null +++ b/internal/api/admin_covers_test.go @@ -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()) + } +} diff --git a/internal/api/api.go b/internal/api/api.go index 0c3a274b..cd34139d 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -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 } diff --git a/internal/server/server.go b/internal/server/server.go index e901f1c9..2fe4ea62 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -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 /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 diff --git a/internal/server/server_test.go b/internal/server/server_test.go index ba87f22d..fa7a7420 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -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()