Files
minstrel/internal/server/server_test.go
T
bvandeusenandClaude Opus 5 516413f4ca
test-web / test (push) Successful in 1m9s
test-go / test (push) Successful in 1m28s
test-go / integration (push) Successful in 3m57s
release / Build signed APK (releases and dev) (push) Successful in 5m20s
release / Build + push container image (push) Successful in 1m23s
release / Verify release artifacts (tag releases only) (push) Skipped
fix(admin): re-acquisition settings take effect without a restart, and say why a save was refused (#3936, #3937)
#3936: Router() built a reacquisition.SettingsService of its own, so a save
from the admin card refreshed that instance's cache while the sweeper in
main.go kept serving what it loaded at boot. The card showed the new
policy, the feature ran the old one, and only a restart reconciled them.
main.go now hands its instance to the server (srv.ReacqSettings), as it
already did for RecSettings, TagSettings and FingerprintSettings, and
Router() constructs one only when that field is nil. The regression test
saves through the router and reads the sweeper's instance.

#3937: the card's catch tested `e instanceof Error`, but api.put throws a
plain {code, message, status} object, so every reason the server gave was
discarded in favour of "Couldn't save settings." It now uses errMessage,
which appends the server's message for invalid_setting. Its test rejected
with an Error no code path produces, so it passed throughout; it now
rejects with what the client actually throws.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH
2026-09-11 20:15:35 -04:00

359 lines
13 KiB
Go

package server
import (
"bytes"
"context"
"encoding/json"
"io"
"log/slog"
"net/http"
"net/http/httptest"
"os"
"strings"
"testing"
"time"
"github.com/go-chi/chi/v5"
"github.com/jackc/pgx/v5/pgxpool"
"git.fabledsword.com/bvandeusen/minstrel/internal/auth"
"git.fabledsword.com/bvandeusen/minstrel/internal/config"
"git.fabledsword.com/bvandeusen/minstrel/internal/db"
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
"git.fabledsword.com/bvandeusen/minstrel/internal/library"
"git.fabledsword.com/bvandeusen/minstrel/internal/reacquisition"
"git.fabledsword.com/bvandeusen/minstrel/internal/subsonic"
)
func TestHealthz(t *testing.T) {
s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), nil, nil, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{}, nil, nil, nil, library.RunScanConfig{})
ts := httptest.NewServer(s.Router())
defer ts.Close()
resp, err := http.Get(ts.URL + "/healthz")
if err != nil {
t.Fatalf("GET /healthz: %v", err)
}
defer func() { _ = resp.Body.Close() }()
if resp.StatusCode != http.StatusOK {
t.Errorf("status = %d, want 200", resp.StatusCode)
}
var body map[string]string
if err := json.NewDecoder(resp.Body).Decode(&body); err != nil {
t.Fatalf("decode: %v", err)
}
if body["status"] != "ok" {
t.Errorf("body = %v, want status=ok", body)
}
}
func TestHealthz_IncludesMinClientVersion(t *testing.T) {
t.Parallel()
s := &Server{}
r := chi.NewRouter()
r.Get("/healthz", s.handleHealthz)
ts := httptest.NewServer(r)
defer ts.Close()
resp, err := http.Get(ts.URL + "/healthz")
if err != nil {
t.Fatalf("GET /healthz: %v", err)
}
defer func() { _ = resp.Body.Close() }()
var body map[string]string
if err := json.NewDecoder(resp.Body).Decode(&body); err != nil {
t.Fatalf("decode: %v", err)
}
if body["status"] != "ok" {
t.Errorf("status: got %q want \"ok\"", body["status"])
}
if body["min_client_version"] == "" {
t.Error("min_client_version is empty; clients can't enforce pairing")
}
}
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{}, nil, nil, nil, library.RunScanConfig{})
ts := httptest.NewServer(s.Router())
defer ts.Close()
resp, err := http.Get(ts.URL + "/")
if err != nil {
t.Fatalf("GET /: %v", err)
}
defer func() { _ = resp.Body.Close() }()
if resp.StatusCode != http.StatusOK {
t.Errorf("status = %d, want 200", resp.StatusCode)
}
if ct := resp.Header.Get("Content-Type"); !strings.Contains(ct, "text/html") {
t.Errorf("Content-Type = %q, want text/html*", ct)
}
}
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{}, nil, nil, nil, library.RunScanConfig{})
ts := httptest.NewServer(s.Router())
defer ts.Close()
resp, err := http.Get(ts.URL + "/artists/00000000-0000-0000-0000-000000000001")
if err != nil {
t.Fatalf("GET deep link: %v", err)
}
defer func() { _ = resp.Body.Close() }()
if resp.StatusCode != http.StatusOK {
t.Errorf("status = %d, want 200", resp.StatusCode)
}
if ct := resp.Header.Get("Content-Type"); !strings.Contains(ct, "text/html") {
t.Errorf("Content-Type = %q, want text/html*", ct)
}
}
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{}, nil, nil, nil, library.RunScanConfig{})
ts := httptest.NewServer(s.Router())
defer ts.Close()
resp, err := http.Get(ts.URL + "/api/does-not-exist")
if err != nil {
t.Fatalf("GET /api/does-not-exist: %v", err)
}
defer func() { _ = resp.Body.Close() }()
if resp.StatusCode != http.StatusNotFound {
t.Errorf("status = %d, want 404", resp.StatusCode)
}
if ct := resp.Header.Get("Content-Type"); strings.Contains(ct, "text/html") {
t.Errorf("Content-Type = %q; /api/* must never return text/html", ct)
}
}
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{}, nil, nil, nil, library.RunScanConfig{})
ts := httptest.NewServer(s.Router())
defer ts.Close()
resp, err := http.Get(ts.URL + "/rest/does-not-exist")
if err != nil {
t.Fatalf("GET /rest/does-not-exist: %v", err)
}
defer func() { _ = resp.Body.Close() }()
if resp.StatusCode != http.StatusNotFound {
t.Errorf("status = %d, want 404", resp.StatusCode)
}
if ct := resp.Header.Get("Content-Type"); strings.Contains(ct, "text/html") {
t.Errorf("Content-Type = %q; /rest/* must never return text/html", ct)
}
}
// TestRouter_AdminSubtreeNotShadowed is a regression test for a real bug we
// shipped in 06a1fe1 and didn't catch until M6a: r.Route("/api/admin", ...)
// in server.go was creating a second chi subtree at the same prefix as the
// nested admin Route inside api.Mount. chi.Walk shows both subtrees as
// registered, but runtime routing dispatch only matches one — every admin
// endpoint registered by api.Mount (Lidarr config, quarantine, etc.)
// silently 404'd for authenticated callers in production.
//
// chi.Walk enumeration would NOT have caught this (both branches are in
// the tree). The only reliable check is to make a real authenticated
// request and confirm it reaches a handler — i.e. doesn't fall through
// to the JSON NotFound (404) that signals shadowing.
//
// The existing tests in this file pass nil for pool, which skips api.Mount
// entirely (gated behind `if s.Pool != nil`), so the conflict didn't manifest.
//
// Gated on MINSTREL_TEST_DATABASE_URL like other integration-leaning tests.
func TestRouter_AdminSubtreeNotShadowed(t *testing.T) {
dsn := os.Getenv("MINSTREL_TEST_DATABASE_URL")
if dsn == "" {
t.Skip("MINSTREL_TEST_DATABASE_URL not set")
}
if err := db.Migrate(dsn, slog.New(slog.NewTextHandler(io.Discard, nil))); err != nil {
t.Fatalf("migrate: %v", err)
}
pool, err := pgxpool.New(context.Background(), dsn)
if err != nil {
t.Fatalf("pool: %v", err)
}
t.Cleanup(pool.Close)
// Seed a test admin user + session so the request actually reaches the
// inner route lookup (auth-middleware short-circuits don't catch shadowing).
ctx := context.Background()
q := dbq.New(pool)
_, _ = pool.Exec(ctx, "DELETE FROM sessions WHERE user_agent = 'shadowing-test'")
_, _ = pool.Exec(ctx, "DELETE FROM users WHERE username = 'test-shadowing-admin'")
user, err := q.CreateUser(ctx, dbq.CreateUserParams{
Username: "test-shadowing-admin",
PasswordHash: "x",
ApiToken: "test-shadowing-token",
IsAdmin: true,
})
if err != nil {
t.Fatalf("CreateUser: %v", err)
}
t.Cleanup(func() { _, _ = pool.Exec(ctx, "DELETE FROM users WHERE id = $1", user.ID) })
token := "shadow-test-" + time.Now().Format("20060102150405")
tokenHash := auth.HashSessionToken(token)
_, err = pool.Exec(ctx,
"INSERT INTO sessions (user_id, token_hash, user_agent) VALUES ($1, $2, 'shadowing-test')",
user.ID, tokenHash[:],
)
if err != nil {
t.Fatalf("insert session: %v", err)
}
s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), pool,
stubScanner{}, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{}, nil, nil, nil, library.RunScanConfig{})
ts := httptest.NewServer(s.Router())
defer ts.Close()
// Each path is registered by a different package: lidarr/config from
// api.Mount and admin/scan from server.Router. If chi runtime dispatch
// only routes one /api/admin subtree, one of these returns 404 — that
// IS the bug we shipped in 06a1fe1.
cases := []struct {
method string
path string
owner string
}{
{http.MethodGet, "/api/admin/lidarr/config", "api.Mount"},
// /api/admin/scan would actually trigger a real library scan via
// stubScanner; we don't include it. The lidarr/config path is the
// canonical regression check.
}
for _, tc := range cases {
req, err := http.NewRequest(tc.method, ts.URL+tc.path, nil)
if err != nil {
t.Fatalf("build %s %s: %v", tc.method, tc.path, err)
}
req.Header.Set("Authorization", "Bearer "+token)
resp, err := http.DefaultClient.Do(req)
if err != nil {
t.Fatalf("%s %s: %v", tc.method, tc.path, err)
}
_ = resp.Body.Close()
if resp.StatusCode == http.StatusNotFound {
t.Errorf("%s %s (owner=%s) returned 404 with valid admin auth — route is shadowed",
tc.method, tc.path, tc.owner)
}
}
}
// TestRouter_ReacquisitionSettingsSavedThroughTheAPIReachTheSweeper is a
// regression test for #3936. Router() used to construct a
// reacquisition.SettingsService of its own, so a save from the admin card
// refreshed THAT instance's cache while the sweeper in cmd/minstrel/main.go
// kept serving what it had loaded at boot. The card showed the new policy, the
// feature kept running the old one, and only a restart reconciled them — the
// exact thing rule 25 says a setting must not need.
//
// The assertion is made against the instance main.go hands the sweeper: save
// through the router, then read that instance. A second service leaves it stale.
func TestRouter_ReacquisitionSettingsSavedThroughTheAPIReachTheSweeper(t *testing.T) {
dsn := os.Getenv("MINSTREL_TEST_DATABASE_URL")
if dsn == "" {
t.Skip("MINSTREL_TEST_DATABASE_URL not set")
}
logger := slog.New(slog.NewTextHandler(io.Discard, nil))
if err := db.Migrate(dsn, logger); err != nil {
t.Fatalf("migrate: %v", err)
}
pool, err := pgxpool.New(context.Background(), dsn)
if err != nil {
t.Fatalf("pool: %v", err)
}
t.Cleanup(pool.Close)
ctx := context.Background()
q := dbq.New(pool)
_, _ = pool.Exec(ctx, "DELETE FROM sessions WHERE user_agent = 'reacq-settings-test'")
_, _ = pool.Exec(ctx, "DELETE FROM users WHERE username = 'test-reacq-settings-admin'")
user, err := q.CreateUser(ctx, dbq.CreateUserParams{
Username: "test-reacq-settings-admin",
PasswordHash: "x",
ApiToken: "test-reacq-settings-token",
IsAdmin: true,
})
if err != nil {
t.Fatalf("CreateUser: %v", err)
}
t.Cleanup(func() { _, _ = pool.Exec(ctx, "DELETE FROM users WHERE id = $1", user.ID) })
token := "reacq-settings-test-" + time.Now().Format("20060102150405.000000")
tokenHash := auth.HashSessionToken(token)
if _, err := pool.Exec(ctx,
"INSERT INTO sessions (user_id, token_hash, user_agent) VALUES ($1, $2, 'reacq-settings-test')",
user.ID, tokenHash[:],
); err != nil {
t.Fatalf("insert session: %v", err)
}
// The sweeper's service. Nothing else in the process may write to the
// settings for the assertion below to mean what it says.
sweeperSettings, err := reacquisition.NewSettingsService(ctx, pool, logger)
if err != nil {
t.Fatalf("reacquisition settings: %v", err)
}
before := sweeperSettings.Get()
t.Cleanup(func() {
if _, err := sweeperSettings.Set(context.Background(), before); err != nil {
t.Errorf("restore reacquisition settings: %v", err)
}
})
wantGrace := before.GraceHours + 1
if wantGrace > 720 {
wantGrace = before.GraceHours - 1
}
s := New(logger, pool, stubScanner{}, subsonic.Config{}, config.EventsConfig{},
config.RecommendationConfig{}, "", config.BrandingConfig{}, nil, nil, nil, library.RunScanConfig{})
s.ReacqSettings = sweeperSettings
ts := httptest.NewServer(s.Router())
defer ts.Close()
body, err := json.Marshal(map[string]any{
"enabled": before.Enabled,
"grace_hours": wantGrace,
"backoff_base_hours": before.BackoffBaseHours,
"backoff_max_hours": before.BackoffMaxHours,
"max_attempts": before.MaxAttempts,
"max_per_pass": before.MaxPerPass,
"auto_approve": before.AutoApprove,
})
if err != nil {
t.Fatalf("marshal: %v", err)
}
req, err := http.NewRequest(http.MethodPut, ts.URL+"/api/admin/library/reacquisition", bytes.NewReader(body))
if err != nil {
t.Fatalf("build request: %v", err)
}
req.Header.Set("Authorization", "Bearer "+token)
req.Header.Set("Content-Type", "application/json")
resp, err := http.DefaultClient.Do(req)
if err != nil {
t.Fatalf("PUT reacquisition settings: %v", err)
}
defer func() { _ = resp.Body.Close() }()
if resp.StatusCode != http.StatusOK {
t.Fatalf("PUT reacquisition settings: status = %d, want 200", resp.StatusCode)
}
if got := sweeperSettings.Get().GraceHours; got != wantGrace {
t.Fatalf("the sweeper's settings hold grace_hours = %d after the save, want %d — "+
"the API wrote through a different service instance", got, wantGrace)
}
}
// stubScanner is a no-op ScanTrigger used only to make Server.Router()
// register /api/admin/scan. Its Scan method must never be called by the
// route-presence assertions in this file.
type stubScanner struct{}
func (stubScanner) Scan(_ context.Context, _ func(library.Stats)) (library.Stats, error) {
panic("stubScanner.Scan called from server_test")
}