fix: A2 (relax art-source CHECK) + D + E integration residual

A2 — migration 0030: the albums/artists art `*_source` CHECK was a
fixed provider-ID allowlist fighting the extensible coverart.Register
registry (per-provider migration churn at 0016/0018/0020; rejected
test stub providers → ~11 enricher tests failed). Relax to "NULL or
non-empty"; the registry is the source of truth. Same brittleness
class as the #433 discovery-mix CHECK.

D — lidarr Approve resolves metadata/quality profiles (JSON arrays)
before the add; the test stubs returned {"id":1} for every path, so
ListMetadataProfiles failed to unmarshal → 500/Approve errors. Make
the lidarrrequests + admin_requests stubs path-aware (arrays for
/metadataprofile, /qualityprofile).

E:
- auth: requireUser (prelude.go) emitted code "auth_required"; the
  canonical code is "unauthenticated" (operator decision). Change the
  code; drop the now-dead "auth_required" web error-copy key
  ("unauthenticated" already has copy).
- playlists_system_test wrapped the real auth.RequireUser middleware
  but only injected withUser() context → 401. Like every other api
  handler test, drop the middleware and rely on requireUser().
- admin_users dup-username test seeded "test-existing" (seedUser
  prefixes) but POSTed "existing" → no collision; POST the prefixed
  name.
- me_timezone test decoded a top-level {"code"} but the envelope is
  {"error":{"code"}}; decode the nested shape.
- audit test asserted compact JSON; Postgres jsonb::text is spaced.
- put-lidarr-config tests predated the missing_defaults gate (correct
  handler behavior); supply the required defaults in the bodies.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-05-17 20:22:30 -04:00
parent 75163ea483
commit 53a02322fb
11 changed files with 89 additions and 18 deletions
+4 -3
View File
@@ -137,8 +137,9 @@ func TestHandlePutLidarrConfig_EmptyKeyPreservesSaved(t *testing.T) {
APIKey: "originalkey",
})
// PUT with empty api_key — should preserve "originalkey".
body := []byte(`{"enabled":true,"base_url":"http://lidarr.lan:8686","api_key":""}`)
// PUT with empty api_key — should preserve "originalkey". enabled=true
// requires the defaults gate (missing_defaults) to be satisfied.
body := []byte(`{"enabled":true,"base_url":"http://lidarr.lan:8686","api_key":"","default_quality_profile_id":1,"default_metadata_profile_id":1,"default_root_folder_path":"/music"}`)
w := doAdminReq(t, h, http.MethodPut, "/api/admin/lidarr/config", body, admin)
if w.Code != http.StatusOK {
t.Fatalf("status = %d, want 200; body = %s", w.Code, w.Body.String())
@@ -183,7 +184,7 @@ func TestHandlePutLidarrConfig_HappyPath(t *testing.T) {
resetLidarrState(t, h)
admin := seedAdminUser(t, h)
body := []byte(`{"enabled":true,"base_url":"http://lidarr.lan:8686","api_key":"newkey","default_quality_profile_id":2,"default_root_folder_path":"/music"}`)
body := []byte(`{"enabled":true,"base_url":"http://lidarr.lan:8686","api_key":"newkey","default_quality_profile_id":2,"default_metadata_profile_id":1,"default_root_folder_path":"/music"}`)
w := doAdminReq(t, h, http.MethodPut, "/api/admin/lidarr/config", body, admin)
if w.Code != http.StatusOK {
t.Fatalf("status = %d, want 200; body = %s", w.Code, w.Body.String())
+16 -1
View File
@@ -6,6 +6,7 @@ import (
"encoding/json"
"net/http"
"net/http/httptest"
"strings"
"testing"
"github.com/go-chi/chi/v5"
@@ -174,10 +175,17 @@ func TestHandleApproveRequest_HappyPath(t *testing.T) {
h, _ := testHandlersWithClientFn(t)
resetLidarrState(t, h)
stub := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
stub := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusOK)
switch {
case strings.Contains(r.URL.Path, "/metadataprofile"):
_, _ = w.Write([]byte(`[{"id":1,"name":"Standard"}]`))
case strings.Contains(r.URL.Path, "/qualityprofile"):
_, _ = w.Write([]byte(`[{"id":1,"name":"Lossless"}]`))
default:
_, _ = w.Write([]byte(`{"id":1}`))
}
}))
t.Cleanup(stub.Close)
@@ -221,7 +229,14 @@ func TestHandleApproveRequest_OverrideUsed(t *testing.T) {
}
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusOK)
switch {
case strings.Contains(r.URL.Path, "/metadataprofile"):
_, _ = w.Write([]byte(`[{"id":1,"name":"Standard"}]`))
case strings.Contains(r.URL.Path, "/qualityprofile"):
_, _ = w.Write([]byte(`[{"id":1,"name":"Lossless"}]`))
default:
_, _ = w.Write([]byte(`{"id":1}`))
}
}))
t.Cleanup(stub.Close)
+4 -1
View File
@@ -163,7 +163,10 @@ func TestAdminCreateUser_DuplicateUsername_409(t *testing.T) {
admin := seedUser(t, pool, "duper", "pw", true)
seedUser(t, pool, "existing", "pw", false)
body := `{"username":"existing","password":"abcd1234"}`
// seedUser prefixes usernames with dbtest.TestUserPrefix, so the
// row above is "test-existing"; POST that exact name to actually
// collide (stays test-prefixed for ResetDB).
body := `{"username":"test-existing","password":"abcd1234"}`
req := httptest.NewRequest(http.MethodPost, "/api/admin/users", bytes.NewReader([]byte(body)))
req = withUser(req, admin)
rec := httptest.NewRecorder()
+6 -2
View File
@@ -69,14 +69,18 @@ func TestPutTimezone_InvalidIANA(t *testing.T) {
t.Fatalf("status = %d, want 400; body=%s", rec.Code, rec.Body.String())
}
// Error envelope is nested: {"error":{"code":...}} (matches the
// rest of /api/*), not a top-level {"code":...}.
var errResp struct {
Error struct {
Code string `json:"code"`
} `json:"error"`
}
if err := json.NewDecoder(rec.Body).Decode(&errResp); err != nil {
t.Fatalf("decode err response: %v", err)
}
if errResp.Code != "invalid_timezone" {
t.Errorf("error code = %q, want invalid_timezone", errResp.Code)
if errResp.Error.Code != "invalid_timezone" {
t.Errorf("error code = %q, want invalid_timezone", errResp.Error.Code)
}
}
+5 -3
View File
@@ -8,8 +8,6 @@ import (
"testing"
"github.com/go-chi/chi/v5"
"git.fabledsword.com/bvandeusen/minstrel/internal/auth"
)
// Generic registry-driven system-playlist endpoints (#411 R2).
@@ -17,7 +15,11 @@ import (
func newSystemPlaylistRouter(h *handlers) chi.Router {
r := chi.NewRouter()
r.Route("/api", func(api chi.Router) {
api.Use(auth.RequireUser(h.pool))
// No real auth.RequireUser middleware: like every other api
// handler test, auth is supplied via withUser() context and the
// handlers self-guard with requireUser() (prelude.go). The real
// middleware needs a live session and would 401 the injected
// test user.
api.Post("/playlists/system/{kind}/refresh", h.handleSystemPlaylistRefresh)
api.Get("/playlists/system/{kind}/shuffle", h.handleSystemPlaylistShuffle)
})
+1 -1
View File
@@ -19,7 +19,7 @@ import (
func requireUser(w http.ResponseWriter, r *http.Request) (dbq.User, bool) {
user, ok := auth.UserFromContext(r.Context())
if !ok {
writeErr(w, apierror.Unauthorized("auth_required", ""))
writeErr(w, apierror.Unauthorized("unauthenticated", ""))
return dbq.User{}, false
}
return user, true
+2
View File
@@ -65,6 +65,8 @@ func TestWrite_WithMetadata(t *testing.T) {
).Scan(&meta); err != nil {
t.Fatalf("read metadata: %v", err)
}
// Postgres jsonb::text renders a space after ':' and ',', e.g.
// {"reason": "test", "first_admin": true}.
if !contains(meta, `"first_admin": true`) || !contains(meta, `"reason": "test"`) {
t.Errorf("metadata = %q, expected first_admin + reason fields", meta)
}
@@ -0,0 +1,15 @@
-- Restore the 0020 fixed-allowlist CHECKs. FAILS if any row holds a
-- source value outside the allowlist (the exact case 0030 enables) —
-- normalise such rows before rolling back.
ALTER TABLE albums DROP CONSTRAINT IF EXISTS albums_cover_art_source_check;
ALTER TABLE albums
ADD CONSTRAINT albums_cover_art_source_check
CHECK (cover_art_source IS NULL
OR cover_art_source IN ('embedded','sidecar','mbcaa','theaudiodb','deezer','lastfm','none'));
ALTER TABLE artists DROP CONSTRAINT IF EXISTS artists_artist_art_source_check;
ALTER TABLE artists
ADD CONSTRAINT artists_artist_art_source_check
CHECK (artist_art_source IS NULL
OR artist_art_source IN ('theaudiodb','deezer','lastfm','none'));
@@ -0,0 +1,19 @@
-- The cover/artist art `*_source` column stores the recording
-- provider's registry ID. coverart.Register makes the provider set
-- extensible, so a fixed IN-list CHECK (0016 → 0018 → 0020) required a
-- schema migration for every new provider and rejected any
-- out-of-list value (e.g. test stub providers). Same brittleness
-- class as the discovery-mix CHECK incident (#433): a fixed-value
-- CHECK fighting an extensible value set. Relax to "NULL or any
-- non-empty string" — the registry, not the schema, is the source of
-- truth for valid provider IDs.
ALTER TABLE albums DROP CONSTRAINT IF EXISTS albums_cover_art_source_check;
ALTER TABLE albums
ADD CONSTRAINT albums_cover_art_source_check
CHECK (cover_art_source IS NULL OR cover_art_source <> '');
ALTER TABLE artists DROP CONSTRAINT IF EXISTS artists_artist_art_source_check;
ALTER TABLE artists
ADD CONSTRAINT artists_artist_art_source_check
CHECK (artist_art_source IS NULL OR artist_art_source <> '');
+12 -1
View File
@@ -8,6 +8,7 @@ import (
"net/http"
"net/http/httptest"
"os"
"strings"
"testing"
"github.com/jackc/pgx/v5/pgtype"
@@ -236,10 +237,20 @@ func TestListForUser_OnlyOwnRows(t *testing.T) {
func approveTestSetup(t *testing.T) (*Service, pgtype.UUID, *pgxpool.Pool, *httptest.Server) {
t.Helper()
pool := newPool(t)
stub := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
stub := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusOK)
// Approve resolves metadata/quality profiles before the add;
// those list endpoints return JSON arrays, not the {"id":1}
// object the add endpoints return.
switch {
case strings.Contains(r.URL.Path, "/metadataprofile"):
_, _ = w.Write([]byte(`[{"id":1,"name":"Standard"}]`))
case strings.Contains(r.URL.Path, "/qualityprofile"):
_, _ = w.Write([]byte(`[{"id":7,"name":"Lossless"}]`))
default:
_, _ = w.Write([]byte(`{"id":1}`))
}
}))
t.Cleanup(stub.Close)
-1
View File
@@ -1,7 +1,6 @@
{
"unknown": "Something went wrong.",
"unauthenticated": "Your session has ended. Please sign in again.",
"auth_required": "You need to sign in to do that.",
"forbidden": "You don't have permission to do that.",
"not_authorized": "You don't have permission to do that.",
"invalid_credentials": "Wrong username or password.",