Merge pull request 'Release v2026.05.18.0 — integration-tests-in-CI + recommendations/cover-art/discover batch' (#51) from dev into main

This commit was merged in pull request #51.
This commit is contained in:
2026-05-17 22:41:20 -04:00
32 changed files with 727 additions and 56 deletions
+58 -2
View File
@@ -4,8 +4,16 @@ name: test-go
# dev/main and PRs to main, scoped to Go-side files only — web-only or
# Flutter-only diffs don't trigger this workflow.
#
# Integration tests needing Postgres/ffmpeg run locally via docker-compose;
# they should guard with testing.Short() so this short-mode run skips them.
# Two jobs: `test` (fast — vet + lint + `go test -short -race`, no DB) and
# `integration` (full `go test -race` against an ephemeral Postgres).
#
# Integration-job DB wiring follows the act_runner shared-daemon pattern:
# the runner's Docker daemon also runs the operator's dev compose stack,
# so service containers get NO published ports (collision) and no
# service-name DNS. We discover the service container by the job-scoped
# name filter via the mounted docker socket and reach it by bridge IP.
# The exactly-one assertion is a hard guard — pointing tests at the dev
# Postgres would truncate it (the disaster Fable #339 exists to prevent).
#
# `web/build/` has a committed placeholder index.html so go:embed succeeds
# without needing the SPA to be freshly built. Real builds happen in
@@ -54,3 +62,51 @@ jobs:
- name: go test (short, race)
run: go test -short -race ./...
integration:
runs-on: go-ci
services:
postgres:
image: postgres:16-alpine
env:
POSTGRES_USER: minstrel
POSTGRES_PASSWORD: minstrel
POSTGRES_DB: minstrel_test
# No `ports:` — the runner shares the operator's dev compose
# Docker daemon; publishing a fixed host port collides.
steps:
- name: Checkout
uses: actions/checkout@v4
- name: Integration suite (discover service by bridge IP, migrate, test)
run: |
set -eux
# Discover THIS job's Postgres service container via the
# mounted docker socket. Scope by the job-name filter so the
# operator's dev compose `minstrel-postgres-*` (same image,
# same daemon) can never match. Require exactly one — abort
# loudly otherwise; a wrong target would truncate real data.
PG_LIST=$(docker ps --filter "name=integration" --filter "ancestor=postgres:16-alpine" --format '{{.ID}} {{.Names}}')
echo "candidates: ${PG_LIST:-<none>}"
PG_COUNT=$(printf '%s\n' "$PG_LIST" | grep -c . || true)
test "$PG_COUNT" = "1" || { echo "FATAL: expected exactly 1 postgres service container, got $PG_COUNT"; exit 1; }
PG_ID=$(printf '%s' "$PG_LIST" | awk '{print $1}')
PG_NAME=$(printf '%s' "$PG_LIST" | awk '{print $2}')
case "$PG_NAME" in
*minstrel-postgres*|*_postgres_*) echo "FATAL: matched the dev compose container ($PG_NAME), refusing"; exit 1 ;;
esac
PG_IP=$(docker inspect -f '{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}' "$PG_ID")
test -n "$PG_IP"
export MINSTREL_TEST_DATABASE_URL="postgres://minstrel:minstrel@${PG_IP}:5432/minstrel_test?sslmode=disable"
# Wait for Postgres to accept TCP (no health-check dependency).
for i in $(seq 1 60); do (echo > "/dev/tcp/${PG_IP}/5432") 2>/dev/null && break; sleep 2; done
# Apply embedded migrations to the fresh test DB, then run the
# full suite (no -short → integration tests execute). -p 1:
# every integration package TRUNCATEs the one shared test DB;
# concurrent package binaries → TRUNCATE deadlocks. Serialize
# package execution (the documented local invocation too).
MINSTREL_DATABASE_URL="$MINSTREL_TEST_DATABASE_URL" go run ./cmd/minstrel migrate
go test -p 1 -race ./...
+11 -1
View File
@@ -1,4 +1,4 @@
.PHONY: generate test test-short lint build
.PHONY: generate test test-short test-integration lint build
SQLC_VERSION := 1.31.1
@@ -11,6 +11,16 @@ test:
test-short:
go test -short -race ./...
# Full suite incl. integration tests, against the dedicated minstrel_test
# DB so a run never truncates the dev `minstrel` DB (Fable #339). Ensures
# the test DB exists (idempotent — createdb errors if present, ignored).
test-integration:
docker compose up -d postgres
-docker compose exec -T postgres createdb -U minstrel minstrel_test
# -p 1: integration packages share one test DB and each TRUNCATEs it;
# concurrent package binaries deadlock on TRUNCATE. Serialize packages.
MINSTREL_TEST_DATABASE_URL=postgres://minstrel:minstrel@localhost:5432/minstrel_test?sslmode=disable go test -p 1 -race ./...
lint:
golangci-lint run ./...
+10
View File
@@ -83,6 +83,16 @@ Two concurrent dev processes:
1. **Backend:** `docker compose up` — Postgres + Minstrel on `:4533`.
2. **Frontend:** `cd web && npm install && npm run dev` — Vite dev server on `:5173` with HMR. The Vite server proxies `/api/*` and `/rest/*` to `:4533` so session cookies work.
### Testing
- Unit + race (no DB): `make test-short`.
- Full suite incl. integration tests: `make test-integration`. This runs
against a dedicated `minstrel_test` database so a test run never
truncates your dev `minstrel` data (admin user, library, likes). It
brings up the compose Postgres and creates the test DB if missing.
- CI runs both: a fast `go test -short -race` gate plus an integration
job with its own ephemeral Postgres (`.forgejo/workflows/test-go.yml`).
### Production build
`docker build -t minstrel .` runs the SvelteKit build inside a `node` stage, copies the output into the `golang` stage, and `//go:embed`s it into the final binary. The container serves the SPA from `/` alongside the API surfaces; no separate static-file server is required.
+129
View File
@@ -0,0 +1,129 @@
package main
import (
"context"
"crypto/rand"
"encoding/base64"
"errors"
"flag"
"fmt"
"os"
"golang.org/x/crypto/bcrypt"
"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/logging"
)
// runMigrate applies the embedded migrations and exits. The server also
// auto-migrates on startup (main.go); this standalone path exists for CI
// (provision a fresh DB before the integration suite) and operators who
// want to migrate without starting the server.
func runMigrate(args []string) error {
fs := flag.NewFlagSet("migrate", flag.ContinueOnError)
configPath := fs.String("config", os.Getenv("MINSTREL_CONFIG"), "path to YAML config file")
if err := fs.Parse(args); err != nil {
return err
}
cfg, err := config.Load(*configPath)
if err != nil {
return fmt.Errorf("load config: %w", err)
}
logger, err := logging.New(os.Stdout, cfg.Log.Level, cfg.Log.Format)
if err != nil {
return fmt.Errorf("init logger: %w", err)
}
if err := db.Migrate(cfg.Database.URL, logger); err != nil {
return fmt.Errorf("migrate: %w", err)
}
fmt.Println("minstrel: migrations applied")
return nil
}
// runAdmin dispatches `minstrel admin <subcommand>`.
func runAdmin(args []string) error {
if len(args) == 0 {
return errors.New("usage: minstrel admin reset-password [-user NAME] [-password PW] [-config PATH]")
}
switch args[0] {
case "reset-password":
return adminResetPassword(args[1:])
default:
return fmt.Errorf("unknown admin subcommand %q", args[0])
}
}
// adminResetPassword resets a user's credentials. It updates BOTH
// password_hash (bcrypt, for /api/auth/login) and subsonic_password
// (plaintext, required for Subsonic t+s token verification) so neither
// auth path is left stale. Recovers a locked-out operator when the
// bootstrap password was missed or the DB volume was recreated (Fable
// #321) without DB surgery.
func adminResetPassword(args []string) error {
fs := flag.NewFlagSet("admin reset-password", flag.ContinueOnError)
configPath := fs.String("config", os.Getenv("MINSTREL_CONFIG"), "path to YAML config file")
username := fs.String("user", "admin", "username to reset")
password := fs.String("password", "", "new password; if empty a strong one is generated and printed")
if err := fs.Parse(args); err != nil {
return err
}
cfg, err := config.Load(*configPath)
if err != nil {
return fmt.Errorf("load config: %w", err)
}
if cfg.Database.URL == "" {
return errors.New("no database URL configured (set it in the config file or MINSTREL_DATABASE_URL)")
}
ctx := context.Background()
pool, err := db.Open(ctx, cfg.Database.URL)
if err != nil {
return fmt.Errorf("open db: %w", err)
}
defer pool.Close()
q := dbq.New(pool)
user, err := q.GetUserByUsername(ctx, *username)
if err != nil {
return fmt.Errorf("look up user %q: %w", *username, err)
}
pw := *password
generated := false
if pw == "" {
b := make([]byte, 18)
if _, err := rand.Read(b); err != nil {
return fmt.Errorf("generate password: %w", err)
}
pw = base64.RawURLEncoding.EncodeToString(b)
generated = true
}
hash, err := bcrypt.GenerateFromPassword([]byte(pw), bcrypt.DefaultCost)
if err != nil {
return fmt.Errorf("hash password: %w", err)
}
if err := q.ChangeUserPassword(ctx, dbq.ChangeUserPasswordParams{
ID: user.ID,
PasswordHash: string(hash),
}); err != nil {
return fmt.Errorf("update password_hash: %w", err)
}
sp := pw
if err := q.SetSubsonicPassword(ctx, dbq.SetSubsonicPasswordParams{
ID: user.ID,
SubsonicPassword: &sp,
}); err != nil {
return fmt.Errorf("update subsonic_password: %w", err)
}
if generated {
fmt.Printf("minstrel: password for %q reset.\nNew password: %s\n", *username, pw)
} else {
fmt.Printf("minstrel: password for %q reset.\n", *username)
}
return nil
}
+16
View File
@@ -29,6 +29,22 @@ import (
)
func main() {
if len(os.Args) > 1 {
switch os.Args[1] {
case "admin":
if err := runAdmin(os.Args[2:]); err != nil {
fmt.Fprintf(os.Stderr, "minstrel admin: %v\n", err)
os.Exit(1)
}
return
case "migrate":
if err := runMigrate(os.Args[2:]); err != nil {
fmt.Fprintf(os.Stderr, "minstrel migrate: %v\n", err)
os.Exit(1)
}
return
}
}
if err := run(); err != nil {
fmt.Fprintf(os.Stderr, "minstrel: %v\n", err)
os.Exit(1)
+6
View File
@@ -0,0 +1,6 @@
-- Runs once, only on a fresh Postgres data volume (Docker entrypoint
-- initdb hook). Creates the dedicated integration-test database so
-- `go test` never truncates the operator's dev `minstrel` DB (Fable
-- #339). For an already-initialised volume this script does NOT run;
-- `make test-integration` creates the DB idempotently instead.
CREATE DATABASE minstrel_test OWNER minstrel;
+11 -4
View File
@@ -1,10 +1,14 @@
version: "3.9"
# Local development environment for Minstrel.
# Not used by CI — integration tests run against this compose stack manually:
# docker compose up -d postgres
# MINSTREL_TEST_DATABASE_URL=postgres://minstrel:minstrel@localhost:5432/minstrel?sslmode=disable \
# go test ./...
#
# Integration tests run against a SEPARATE database (minstrel_test) so a
# test run never truncates the dev `minstrel` DB (Fable #339). Use the
# Makefile target, which ensures the test DB exists then points the
# tests at it:
# make test-integration
# (CI runs the same suite against its own ephemeral Postgres service —
# see .forgejo/workflows/test-go.yml.)
#
# Full stack (server + db):
# docker compose up --build
@@ -22,6 +26,9 @@ services:
# - "5432:5432"
volumes:
- minstrel-pgdata:/var/lib/postgresql/data
# Creates minstrel_test on a fresh volume (Fable #339). No-op on
# an existing volume — make test-integration ensures it instead.
- ./deploy/initdb:/docker-entrypoint-initdb.d:ro
healthcheck:
test: ["CMD-SHELL", "pg_isready -U minstrel -d minstrel"]
interval: 5s
@@ -1,11 +1,25 @@
import 'package:dio/dio.dart';
import '../../models/artist_suggestion.dart';
import '../../models/lidarr.dart';
class DiscoverApi {
DiscoverApi(this._dio);
final Dio _dio;
/// GET /api/discover/suggestions — out-of-library artist suggestions
/// (ListenBrainz-derived; image_url resolved on-demand from Lidarr,
/// may be empty). The server already filters in-library and
/// non-terminal-request candidates.
Future<List<ArtistSuggestion>> listSuggestions() async {
final r = await _dio.get<List<dynamic>>('/api/discover/suggestions');
final raw = r.data ?? const [];
return raw
.map((e) =>
ArtistSuggestion.fromJson((e as Map).cast<String, dynamic>()))
.toList(growable: false);
}
/// GET /api/lidarr/search?q=...&kind=artist|album|track. Server has a
/// 60s LRU cache for repeat queries so re-typing the same string in
/// quick succession is cheap.
@@ -8,6 +8,7 @@ import '../api/endpoints/discover.dart';
import '../api/errors.dart';
import '../cache/mutation_queue.dart';
import '../library/library_providers.dart' show dioProvider;
import '../models/artist_suggestion.dart';
import '../models/lidarr.dart';
import '../shared/widgets/main_app_bar_actions.dart';
import '../theme/theme_extension.dart';
@@ -27,6 +28,22 @@ class _DiscoverScreenState extends ConsumerState<DiscoverScreen> {
final _ctrl = TextEditingController();
LidarrRequestKind _kind = LidarrRequestKind.artist;
Future<List<LidarrSearchResult>>? _resultsFuture;
// Default (empty-search) surface: LB-derived out-of-library artist
// suggestions, mirroring web's SuggestionFeed.
Future<List<ArtistSuggestion>>? _suggestionsFuture;
final _requested = <String>{};
@override
void initState() {
super.initState();
_loadSuggestions();
// Clearing the box returns to suggestions (web swaps live too).
_ctrl.addListener(() {
if (_ctrl.text.trim().isEmpty && _resultsFuture != null) {
setState(() => _resultsFuture = null);
}
});
}
@override
void dispose() {
@@ -34,6 +51,55 @@ class _DiscoverScreenState extends ConsumerState<DiscoverScreen> {
super.dispose();
}
// Assigns the future only; callers trigger the rebuild (initState
// runs before first build, so setState here would be a no-op/warn).
void _loadSuggestions() {
_suggestionsFuture = ref
.read(_discoverApiProvider.future)
.then((api) => api.listSuggestions());
}
Future<void> _requestSuggestion(ArtistSuggestion s) async {
final fs = Theme.of(context).extension<FabledSwordTheme>()!;
final args = {
'kind': LidarrRequestKind.artist.wire,
'artistMbid': s.mbid,
'artistName': s.name,
'albumMbid': null,
'albumTitle': null,
};
try {
final api = await ref.read(_discoverApiProvider.future);
await api.createRequest(
kind: LidarrRequestKind.artist,
artistMbid: s.mbid,
artistName: s.name,
);
if (mounted) {
// Reassign the future first; the setState below rebuilds and
// the FutureBuilder picks up the fresh fetch (server now
// filters this candidate out). _requested hides it meanwhile.
_loadSuggestions();
setState(() => _requested.add(s.mbid));
ScaffoldMessenger.of(context).showSnackBar(SnackBar(
content: Text('Requested: ${s.name}'),
backgroundColor: fs.iron,
));
}
} on DioException catch (_) {
await ref
.read(mutationQueueProvider)
.enqueue(MutationKinds.requestCreate, args);
if (mounted) {
setState(() => _requested.add(s.mbid));
ScaffoldMessenger.of(context).showSnackBar(SnackBar(
content: Text('Request queued: ${s.name}'),
backgroundColor: fs.iron,
));
}
}
}
void _runSearch() {
final q = _ctrl.text.trim();
if (q.isEmpty) {
@@ -153,13 +219,7 @@ class _DiscoverScreenState extends ConsumerState<DiscoverScreen> {
),
Expanded(
child: _resultsFuture == null
? Center(
child: Text(
'Type to search, then tap Request to send to Lidarr.',
textAlign: TextAlign.center,
style: TextStyle(color: fs.ash),
),
)
? _buildSuggestions(fs)
: FutureBuilder<List<LidarrSearchResult>>(
future: _resultsFuture,
builder: (ctx, snap) {
@@ -199,6 +259,62 @@ class _DiscoverScreenState extends ConsumerState<DiscoverScreen> {
]),
);
}
Widget _buildSuggestions(FabledSwordTheme fs) {
return FutureBuilder<List<ArtistSuggestion>>(
future: _suggestionsFuture,
builder: (ctx, snap) {
if (snap.connectionState != ConnectionState.done) {
return const Center(child: CircularProgressIndicator());
}
final items = (snap.data ?? const <ArtistSuggestion>[])
.where((s) => !_requested.contains(s.mbid))
.toList(growable: false);
return ListView(
padding: const EdgeInsets.only(bottom: 16),
children: [
Padding(
padding: const EdgeInsets.fromLTRB(16, 12, 16, 4),
child: Column(
crossAxisAlignment: CrossAxisAlignment.start,
children: [
Text('Suggested for you',
style: TextStyle(
color: fs.parchment,
fontSize: 20,
fontWeight: FontWeight.w500)),
const SizedBox(height: 2),
Text(
"Out-of-library artists drawn from what you've liked and played.",
style: TextStyle(color: fs.ash, fontSize: 13),
),
],
),
),
if (snap.hasError)
Padding(
padding: const EdgeInsets.all(16),
child: Text("Couldn't load suggestions.",
style: TextStyle(color: fs.ash)),
)
else if (items.isEmpty)
Padding(
padding: const EdgeInsets.all(16),
child: Text(
'Listen to something or like an artist to start getting suggestions.',
style: TextStyle(color: fs.ash),
),
)
else
...items.map((s) => _SuggestionTile(
s: s,
onRequest: () => _requestSuggestion(s),
)),
],
);
},
);
}
}
class _ResultTile extends StatelessWidget {
@@ -284,3 +400,63 @@ class _Pill extends StatelessWidget {
);
}
}
class _SuggestionTile extends StatelessWidget {
const _SuggestionTile({required this.s, required this.onRequest});
final ArtistSuggestion s;
final VoidCallback onRequest;
@override
Widget build(BuildContext context) {
final fs = Theme.of(context).extension<FabledSwordTheme>()!;
return Padding(
padding: const EdgeInsets.symmetric(horizontal: 16, vertical: 10),
child: Row(children: [
ClipRRect(
borderRadius: BorderRadius.circular(6),
child: Container(
width: 56,
height: 56,
color: fs.slate,
child: s.imageUrl.isEmpty
? Icon(Icons.person, color: fs.ash)
: CachedNetworkImage(
imageUrl: s.imageUrl,
fit: BoxFit.cover,
fadeInDuration: const Duration(milliseconds: 120),
fadeOutDuration: Duration.zero,
errorWidget: (_, __, ___) =>
Icon(Icons.person, color: fs.ash),
),
),
),
const SizedBox(width: 12),
Expanded(
child: Column(
crossAxisAlignment: CrossAxisAlignment.start,
children: [
Text(s.name,
maxLines: 1,
overflow: TextOverflow.ellipsis,
style: TextStyle(color: fs.parchment, fontSize: 14)),
if (s.attributionText.isNotEmpty)
Text(s.attributionText,
maxLines: 2,
overflow: TextOverflow.ellipsis,
style: TextStyle(color: fs.ash, fontSize: 12)),
],
),
),
const SizedBox(width: 8),
FilledButton(
onPressed: onRequest,
style: FilledButton.styleFrom(
backgroundColor: fs.accent,
foregroundColor: fs.parchment,
),
child: const Text('Request'),
),
]),
);
}
}
@@ -0,0 +1,51 @@
/// Mirrors web/src/lib/api/types.ts ArtistSuggestion / SeedContribution —
/// one out-of-library artist from GET /api/discover/suggestions. image_url
/// is resolved on-demand from Lidarr server-side (may be empty).
class SeedContribution {
const SeedContribution({required this.name, required this.isLiked});
final String name;
final bool isLiked;
factory SeedContribution.fromJson(Map<String, dynamic> j) => SeedContribution(
name: j['name'] as String? ?? '',
isLiked: j['is_liked'] as bool? ?? false,
);
}
class ArtistSuggestion {
const ArtistSuggestion({
required this.mbid,
required this.name,
required this.imageUrl,
required this.attribution,
});
final String mbid;
final String name;
final String imageUrl;
final List<SeedContribution> attribution;
factory ArtistSuggestion.fromJson(Map<String, dynamic> j) => ArtistSuggestion(
mbid: j['mbid'] as String? ?? '',
name: j['name'] as String? ?? '',
imageUrl: j['image_url'] as String? ?? '',
attribution: ((j['attribution'] as List?) ?? const [])
.map((e) =>
SeedContribution.fromJson((e as Map).cast<String, dynamic>()))
.toList(growable: false),
);
/// Mirrors web SuggestionFeed.attributionText (Oxford comma, max 3).
String get attributionText {
if (attribution.isEmpty) return '';
final phrases = attribution
.map((s) => '${s.isLiked ? 'liked' : 'played'} ${s.name}')
.toList(growable: false);
if (phrases.length == 1) return 'Because you ${phrases[0]}.';
if (phrases.length == 2) {
return 'Because you ${phrases[0]} and ${phrases[1]}.';
}
return 'Because you ${phrases[0]}, ${phrases[1]}, and ${phrases[2]}.';
}
}
+1 -1
View File
@@ -1,7 +1,7 @@
name: minstrel
description: Minstrel mobile client
publish_to: 'none'
version: 2026.5.16+8
version: 2026.5.18+9
environment:
sdk: '>=3.5.0 <4.0.0'
+1 -1
View File
@@ -27,7 +27,7 @@ func (p *apiTestAlbumProvider) DisplayName() string { re
func (p *apiTestAlbumProvider) RequiresAPIKey() bool { return false }
func (p *apiTestAlbumProvider) DefaultEnabled() bool { return true }
func (p *apiTestAlbumProvider) Configure(_ coverart.ProviderSettings) error { return nil }
func (p *apiTestAlbumProvider) FetchAlbumCover(_ context.Context, _ string) ([]byte, error) {
func (p *apiTestAlbumProvider) FetchAlbumCover(_ context.Context, _ coverart.AlbumRef) ([]byte, error) {
return []byte("img"), nil
}
+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())
+18 -3
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)
_, _ = w.Write([]byte(`{"id":1}`))
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)
_, _ = w.Write([]byte(`{"id":1}`))
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()
+7 -3
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 {
Code string `json:"code"`
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
+55
View File
@@ -1,12 +1,15 @@
package api
import (
"context"
"net/http"
"strconv"
"sync"
"github.com/jackc/pgx/v5/pgtype"
"git.fabledsword.com/bvandeusen/minstrel/internal/apierror"
"git.fabledsword.com/bvandeusen/minstrel/internal/lidarr"
"git.fabledsword.com/bvandeusen/minstrel/internal/recommendation"
)
@@ -16,6 +19,11 @@ type suggestionView struct {
Name string `json:"name"`
Score float64 `json:"score"`
Attribution []seedContributionView `json:"attribution"`
// ImageURL is resolved on-demand from Lidarr (out-of-library
// artists have no local art row). Omitted when Lidarr is disabled
// or has no match — the client falls back to a placeholder. Not
// cached: a remote URL Lidarr surfaced, fetched by the browser.
ImageURL string `json:"image_url,omitempty"`
}
type seedContributionView struct {
@@ -80,5 +88,52 @@ func (h *handlers) handleListSuggestions(w http.ResponseWriter, r *http.Request)
MBID: s.MBID, Name: s.Name, Score: s.Score, Attribution: attr,
})
}
h.resolveSuggestionArt(r.Context(), out)
writeJSON(w, http.StatusOK, out)
}
// resolveSuggestionArt fills ImageURL on-demand from Lidarr's artist
// lookup, matched by MBID (foreignArtistId). Best-effort and cache-free:
// Lidarr is the only source — when it's disabled, unreachable, or has
// no match for a candidate, that entry keeps an empty ImageURL and the
// client renders its placeholder. Lookups run with bounded concurrency
// so a full Discover page doesn't serialize ~12 round-trips. Never
// fails the request; the suggestions list is the contract, art is a
// nicety.
func (h *handlers) resolveSuggestionArt(ctx context.Context, views []suggestionView) {
if len(views) == 0 {
return
}
cfg, err := h.lidarrCfg.Get(ctx)
if err != nil || !cfg.Enabled || cfg.BaseURL == "" || cfg.APIKey == "" {
return // Lidarr off / unconfigured → placeholders only
}
client := lidarr.NewClient(cfg.BaseURL, cfg.APIKey)
const maxConcurrent = 6
sem := make(chan struct{}, maxConcurrent)
var wg sync.WaitGroup
for i := range views {
if views[i].MBID == "" || views[i].Name == "" {
continue
}
wg.Add(1)
sem <- struct{}{}
go func(idx int) {
defer wg.Done()
defer func() { <-sem }()
results, lerr := client.LookupArtist(ctx, views[idx].Name)
if lerr != nil {
return // best-effort: no art on lookup failure
}
for _, res := range results {
if res.MBID == views[idx].MBID && res.ImageURL != "" {
// Distinct slice index per goroutine → race-free.
views[idx].ImageURL = res.ImageURL
return
}
}
}(i)
}
wg.Wait()
}
+3 -1
View File
@@ -65,7 +65,9 @@ func TestWrite_WithMetadata(t *testing.T) {
).Scan(&meta); err != nil {
t.Fatalf("read metadata: %v", err)
}
if !contains(meta, `"first_admin":true`) || !contains(meta, `"reason":"test"`) {
// 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)
}
}
+8 -4
View File
@@ -496,7 +496,7 @@ func TestCleanupArtistArt_IdempotentMissingDir(t *testing.T) {
// --- MBID guard ---
func TestEnrichArtist_NoMBID_LeavesNull(t *testing.T) {
func TestEnrichArtist_NoMBID_SettlesNone(t *testing.T) {
pool := newPool(t)
ctx := context.Background()
q := dbq.New(pool)
@@ -504,9 +504,13 @@ func TestEnrichArtist_NoMBID_LeavesNull(t *testing.T) {
resetRegistryForTests()
t.Cleanup(resetRegistryForTests)
// No MBID still runs the provider chain (name-based providers can
// resolve without one). An MBID-only provider returns ErrNotFound
// for an empty MBID; with the whole chain returning ErrNotFound the
// row settles 'none' (version-stamped), NOT NULL.
stub := &stubArtistProvider{
fakeProvider: fakeProvider{id: "stub-artist", defaultOn: true},
thumb: []byte("should_not_write"),
err: ErrNotFound,
}
Register(stub)
@@ -523,7 +527,7 @@ func TestEnrichArtist_NoMBID_LeavesNull(t *testing.T) {
if err != nil {
t.Fatalf("GetArtistByID: %v", err)
}
if row.ArtistArtSource != nil {
t.Errorf("source = %v, want nil (no MBID — skip)", row.ArtistArtSource)
if row.ArtistArtSource == nil || *row.ArtistArtSource != "none" {
t.Errorf("source = %v, want 'none' (settled)", row.ArtistArtSource)
}
}
+33 -10
View File
@@ -49,8 +49,11 @@ func discardLogger() *slog.Logger {
// pick them up.
func newTestEnricher(t *testing.T, pool *pgxpool.Pool) *Enricher {
t.Helper()
resetRegistryForTests()
t.Cleanup(resetRegistryForTests)
// Do NOT reset the registry here: callers register their fakes
// before calling this (see doc above) and resetting would wipe them
// so reconcile() sees zero providers — every art source then stays
// NULL. Registry lifecycle is the caller's (each test does
// resetRegistryForTests + t.Cleanup before Register()).
s, err := NewSettingsService(context.Background(), pool, discardLogger())
if err != nil {
t.Fatalf("NewSettingsService: %v", err)
@@ -100,6 +103,8 @@ func TestEnrichAlbum_SidecarFound(t *testing.T) {
t.Fatal(err)
}
resetRegistryForTests() // deterministic empty registry (sidecar-only path)
t.Cleanup(resetRegistryForTests)
e := newTestEnricher(t, pool)
if err := e.EnrichAlbum(context.Background(), id); err != nil {
t.Fatalf("enrich: %v", err)
@@ -116,9 +121,11 @@ func TestEnrichAlbum_SidecarFound(t *testing.T) {
}
}
func TestEnrichAlbum_NoSidecarNoMBID_LeavesNull(t *testing.T) {
func TestEnrichAlbum_NoSidecarNoMBID_SettlesNone(t *testing.T) {
pool := newPool(t)
id, _ := seedAlbumWithTrack(t, pool, "NoMbid", "Artist", "")
resetRegistryForTests() // deterministic empty registry (no provider can supply)
t.Cleanup(resetRegistryForTests)
e := newTestEnricher(t, pool)
if err := e.EnrichAlbum(context.Background(), id); err != nil {
t.Fatalf("enrich: %v", err)
@@ -127,8 +134,11 @@ func TestEnrichAlbum_NoSidecarNoMBID_LeavesNull(t *testing.T) {
if err != nil {
t.Fatalf("get: %v", err)
}
if row.CoverArtSource != nil {
t.Errorf("source = %v, want nil (NULL — eligible for retry on next scan)", row.CoverArtSource)
// No sidecar, no MBID, no provider yields → the chain settles
// 'none' (allWere404 stays true), version-stamped so it is only
// re-tried when the registered provider set changes.
if row.CoverArtSource == nil || *row.CoverArtSource != "none" {
t.Errorf("source = %v, want 'none' (settled)", row.CoverArtSource)
}
}
@@ -227,6 +237,8 @@ func TestEnrichAlbum_AlreadySidecar_NoOp(t *testing.T) {
t.Fatal(err)
}
resetRegistryForTests() // deterministic empty registry (sidecar-only path)
t.Cleanup(resetRegistryForTests)
e := newTestEnricher(t, pool)
if err := e.EnrichAlbum(context.Background(), id); err != nil {
t.Fatalf("first enrich: %v", err)
@@ -253,16 +265,27 @@ func TestEnrichBatch_DrainsNullSourceOnly(t *testing.T) {
}
id2, _ := seedAlbumWithTrack(t, pool, "Drain2", "B", "")
// Pre-mark id2 as 'none' with current version — should NOT be drained by EnrichBatch
// (ListAlbumsMissingCover only returns stale-version 'none').
resetRegistryForTests() // deterministic empty registry
t.Cleanup(resetRegistryForTests)
e := newTestEnricher(t, pool)
// Pre-mark id2 as 'none' AT the current sources version so
// ListAlbumsMissingCover excludes it (it only returns NULL or
// stale-version 'none'). SetAlbumCover does NOT stamp the version;
// SetAlbumCoverWithVersion does — use the live current version, the
// same value EnrichBatch compares against.
curVer, err := dbq.New(pool).GetCurrentSourcesVersion(context.Background())
if err != nil {
t.Fatalf("current version: %v", err)
}
src := "none"
if err := dbq.New(pool).SetAlbumCover(context.Background(), dbq.SetAlbumCoverParams{
ID: id2, Column2: "", CoverArtSource: &src,
if err := dbq.New(pool).SetAlbumCoverWithVersion(context.Background(), dbq.SetAlbumCoverWithVersionParams{
ID: id2, CoverArtPath: nil, CoverArtSource: &src, CoverArtSourcesVersion: curVer,
}); err != nil {
t.Fatal(err)
}
e := newTestEnricher(t, pool)
processed, _, _, err := e.EnrichBatch(context.Background(), 100, nil)
if err != nil {
t.Fatalf("batch: %v", err)
@@ -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 <> '');
+17
View File
@@ -55,6 +55,13 @@ var dataTables = []string{
"playlist_tracks",
"playlists",
"library_changes", // M7 #357 — must reset to keep cursor isolated per test
// SettingsService.reconcile() idempotently re-UpsertProviderSettings
// for every registered provider at boot, so truncating this is the
// correct per-test reset (clears test-modified enabled/api_key rows).
// cover_art_sources_meta is NOT truncated — boot only READS it
// (never recreates the singleton, seeded once by 0018); ResetDB
// resets its counter via UPDATE below instead.
"cover_art_provider_settings",
"tracks",
"albums",
"artists",
@@ -76,4 +83,14 @@ func ResetDB(t *testing.T, pool *pgxpool.Pool) {
); err != nil {
t.Fatalf("dbtest.ResetDB delete test users: %v", err)
}
// Reset the monotonic cover-art source-version counter to its
// post-migration seeded value. Truncating the row would break
// SettingsService boot, which reads (never recreates) this
// singleton; an UPDATE keeps the row while clearing cross-test
// version accumulation (CurrentVersion=4 want 1, key-only-bump).
if _, err := pool.Exec(ctx,
"UPDATE cover_art_sources_meta SET current_version = 1",
); err != nil {
t.Fatalf("dbtest.ResetDB reset cover-art version: %v", err)
}
}
+11
View File
@@ -103,6 +103,17 @@ func TestScanner_Integration(t *testing.T) {
t.Errorf("artist sort = [%q, %q], want [Artist X, Artist Y]", artists[0].SortName, artists[1].SortName)
}
// The synthetic MP3s carry only an ID3 tag — no decodable audio —
// so ffprobe yields duration 0. The scanner deliberately refuses to
// skip zero-duration rows (it re-runs them so a later scan can
// backfill duration once probing works), which is orthogonal to the
// mtime-based incremental-skip this test covers. Simulate a
// normally-probed library so the skip path is actually exercised;
// updated_at is left untouched (still ≥ file mtime).
if _, err := pool.Exec(ctx, "UPDATE tracks SET duration_ms = 1000 WHERE duration_ms = 0"); err != nil {
t.Fatalf("seed durations: %v", err)
}
stats2, err := scanner.Scan(ctx, nil)
if err != nil {
t.Fatalf("second scan: %v", err)
+13 -2
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)
_, _ = w.Write([]byte(`{"id":1}`))
// 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)
+16 -6
View File
@@ -24,9 +24,16 @@ func discardLogger() *slog.Logger {
// `InsertPlayEvent` doesn't accept was_skipped (that's set by an UPDATE).
func seedPlayEvent(t *testing.T, pool *pgxpool.Pool, userID, trackID pgtype.UUID, startedAt time.Time, wasSkipped bool) {
t.Helper()
// play_events.session_id has a FK to play_sessions; create a parent
// session in the same statement (a random UUID violates the FK).
_, err := pool.Exec(context.Background(), `
WITH s AS (
INSERT INTO play_sessions (user_id, started_at, last_event_at)
VALUES ($1, $3, $3)
RETURNING id
)
INSERT INTO play_events (user_id, track_id, session_id, started_at, was_skipped)
VALUES ($1, $2, gen_random_uuid(), $3, $4)
SELECT $1, $2, s.id, $3, $4 FROM s
`, userID, trackID, startedAt, wasSkipped)
if err != nil {
t.Fatalf("seed play_event: %v", err)
@@ -38,7 +45,7 @@ func seedQuarantine(t *testing.T, pool *pgxpool.Pool, userID, trackID pgtype.UUI
t.Helper()
_, err := pool.Exec(context.Background(), `
INSERT INTO lidarr_quarantine (user_id, track_id, reason)
VALUES ($1, $2, 'test-hide')
VALUES ($1, $2, 'other')
`, userID, trackID)
if err != nil {
t.Fatalf("seed quarantine: %v", err)
@@ -105,16 +112,19 @@ func TestBuildSystemPlaylists_SufficientActivity(t *testing.T) {
if !r.SeedArtistID.Valid {
t.Errorf("songs_like_artist row should have non-NULL seed_artist_id")
}
case "discover":
// Discover playlist is valid — no seed_artist_id required.
case "discover", "deep_cuts", "rediscover", "new_for_you", "on_this_day", "first_listens":
// Discover + the #411 discovery mixes are all seedless —
// no seed_artist_id required.
default:
t.Errorf("unknown system_variant=%q", *r.SystemVariant)
}
if r.TrackCount == 0 {
t.Errorf("row %s: empty track_count", uuidString(r.ID))
}
if r.TrackCount > 25 {
t.Errorf("row %s: track_count=%d exceeds 25", uuidString(r.ID), r.TrackCount)
// For-You / Discover / the discovery mixes are sized to ~100
// (#352/#411); songs_like_artist stays at systemMixLength (25).
if r.TrackCount > 100 {
t.Errorf("row %s: track_count=%d exceeds 100", uuidString(r.ID), r.TrackCount)
}
}
if !hasForYou {
@@ -114,8 +114,11 @@ func markPlayed(t *testing.T, f fixture, trackID pgtype.UUID) {
func newTestWorker(f fixture, lbBaseURL string) *Worker {
logger := slog.New(slog.NewTextHandler(io.Discard, nil))
return &Worker{
pool: f.pool,
client: &listenbrainz.Client{BaseURL: lbBaseURL, HTTP: http.DefaultClient},
pool: f.pool,
// LabsBaseURL must point at the stub too: similarity calls hit
// the Labs API (commit 4fca0e6), so an unset LabsBaseURL would
// fall through to the real labs.api and the stub never runs.
client: &listenbrainz.Client{BaseURL: lbBaseURL, LabsBaseURL: lbBaseURL, HTTP: http.DefaultClient},
logger: logger,
tick: 1 * time.Hour,
batch: 5,
+1
View File
@@ -298,6 +298,7 @@ export type ArtistSuggestion = {
name: string;
score: number;
attribution: SeedContribution[]; // up to 3 entries, ordered by contribution DESC
image_url?: string; // resolved on-demand from Lidarr; absent → card placeholder
};
// Mirrors internal/api/types.go HomePayload. All slices are non-null
@@ -65,6 +65,7 @@
<DiscoverResultCard
kind="artist"
title={s.name}
imageUrl={s.image_url}
state="requestable"
attribution={attributionText(s.attribution)}
onRequest={() => onRequest(s)}
-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.",