diff --git a/docs/superpowers/plans/2026-04-21-web-ui-media-endpoints.md b/docs/superpowers/plans/2026-04-21-web-ui-media-endpoints.md new file mode 100644 index 00000000..14a069f2 --- /dev/null +++ b/docs/superpowers/plans/2026-04-21-web-ui-media-endpoints.md @@ -0,0 +1,1066 @@ +# Web UI Media Endpoints Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Ship `/api/albums/{id}/cover` and `/api/tracks/{id}/stream` so Plan 2's forward-reference `cover_url` and `stream_url` fields resolve in the SPA. + +**Architecture:** One new file `internal/api/media.go` holds both handlers plus locally-duplicated filesystem/MIME helpers — no shared package with `internal/subsonic`, per the subsonic-is-long-term-legacy direction. Byte-serving uses `http.ServeContent`, which handles Range / If-Modified-Since / ETag automatically. Auth is the existing `RequireUser` middleware (cookie or bearer token). + +**Tech Stack:** Go 1.23, chi/v5, pgx/v5, pgxpool, stdlib `net/http`, `os`, `path/filepath`. + +**Spec:** `docs/superpowers/specs/2026-04-21-web-ui-media-endpoints-design.md` + +--- + +## File Structure + +New files: + +| File | Responsibility | +|---|---| +| `internal/api/media.go` | Both handlers + helpers (`resolveAlbumCoverPath`, `findSidecarCover`, `audioContentType`, `imageContentType`) | +| `internal/api/media_test.go` | Unit tests for MIME helpers + integration tests for both endpoints | + +Modified files: + +| File | Change | +|---|---| +| `internal/api/library_fixtures_test.go` | Add `seedTrackWithFile` helper that materializes bytes on disk | +| `internal/api/library_test.go` | Extend `newLibraryRouter` (register `/cover` + `/stream`) and `TestRoutesRegisteredInMount` (assert production Mount wires both) | +| `internal/api/api.go` | Register both routes inside the existing `authed` chi group | + +No migrations. No new sqlc queries — `GetAlbumByID`, `GetTrackByID`, and `ListTracksByAlbum` already exist. + +--- + +## Working Conventions + +- **Commit after each green step.** One task = at least one commit. Multiple commits per task if convenient. +- **Integration tests require a live Postgres** at `MINSTREL_TEST_DATABASE_URL`. `docker-compose.yml` spins one up; tests skip cleanly when the env var is unset. Run the full suite with `MINSTREL_TEST_DATABASE_URL=postgres://… go test ./...` before committing. +- **Do NOT touch `internal/subsonic/*`**. If you notice the helpers here duplicate code in `internal/subsonic/stream.go`, that's intentional — see the memory note `project_subsonic_legacy.md`. +- **Go unused-import rule:** only add an import once the code actually uses it; the compiler rejects unused imports. +- **File-format and extension tables** in this plan are authoritative — they are the ones the spec approved, which differ slightly from subsonic's table. Do not "fix" them to match subsonic. + +--- + +## Task 1: Scaffold `media.go` with helpers + unit tests + +**Files:** +- Create: `internal/api/media.go` +- Create: `internal/api/media_test.go` +- Modify: `internal/api/library_fixtures_test.go` + +This task produces the four package-private helpers — no handlers yet. The handlers land in Tasks 2 and 3. Separating them keeps each commit's diff tight. + +- [ ] **Step 1: Create `internal/api/media.go` with helpers only** + +```go +// Package api — media endpoints serve raw bytes (cover art, audio streams). +// The helpers below duplicate shape with internal/subsonic/stream.go by +// design; the two packages are kept independent so subsonic can freeze as a +// compatibility surface while /api evolves. See project memory +// `project_subsonic_legacy.md` for the rationale. + +package api + +import ( + "context" + "os" + "path/filepath" + "strings" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +// sidecarNames is the lookup order for cover art living next to audio files. +// Matches common library conventions: cover.* wins over folder.*; JPEG wins +// over PNG when both exist. +var sidecarNames = []string{ + "cover.jpg", "cover.jpeg", "cover.png", + "folder.jpg", "folder.jpeg", "folder.png", +} + +// findSidecarCover looks for a conventional cover image in the directory that +// contains trackPath. Returns "" if no sidecar exists. +func findSidecarCover(trackPath string) string { + dir := filepath.Dir(trackPath) + for _, name := range sidecarNames { + candidate := filepath.Join(dir, name) + if info, err := os.Stat(candidate); err == nil && !info.IsDir() { + return candidate + } + } + return "" +} + +// resolveAlbumCoverPath returns the filesystem path to the album's cover art. +// It prefers an explicit cover_art_path (set by the scanner in a future +// milestone) and falls back to a sidecar next to the first track in the +// album's directory. "" means no art was found. +func resolveAlbumCoverPath(ctx context.Context, q *dbq.Queries, album dbq.Album) string { + if album.CoverArtPath != nil && *album.CoverArtPath != "" { + if _, err := os.Stat(*album.CoverArtPath); err == nil { + return *album.CoverArtPath + } + } + tracks, err := q.ListTracksByAlbum(ctx, album.ID) + if err != nil || len(tracks) == 0 { + return "" + } + return findSidecarCover(tracks[0].FilePath) +} + +// audioContentType maps the short file_format recorded on tracks (mp3, flac, +// ogg, opus, m4a, aac, wav) to a MIME type for the Content-Type header. +// Unknown formats fall back to octet-stream so the browser downloads them +// rather than attempting to decode. +func audioContentType(format string) string { + switch strings.ToLower(format) { + case "mp3": + return "audio/mpeg" + case "flac": + return "audio/flac" + case "ogg", "opus": + return "audio/ogg" + case "m4a": + return "audio/mp4" + case "aac": + return "audio/aac" + case "wav": + return "audio/wav" + } + return "application/octet-stream" +} + +// imageContentType maps a file extension to a MIME type for cover art. +// Unknown extensions fall back to octet-stream. +func imageContentType(path string) string { + switch strings.ToLower(filepath.Ext(path)) { + case ".jpg", ".jpeg": + return "image/jpeg" + case ".png": + return "image/png" + case ".webp": + return "image/webp" + case ".gif": + return "image/gif" + } + return "application/octet-stream" +} +``` + +- [ ] **Step 2: Append `seedTrackWithFile` to `internal/api/library_fixtures_test.go`** + +Inside the same `package api` file, add (alongside the existing helpers): + +```go +// seedTrackWithFile creates a fresh temp directory, writes fileBody to +// /.<ext>, and inserts a Track row whose file_path points at it. +// Returns the inserted Track and the directory (callers drop sidecar covers +// into this dir when a test needs them). FileFormat is ext lowercased. +func seedTrackWithFile(t *testing.T, pool *pgxpool.Pool, albumID, artistID pgtype.UUID, title string, fileBody []byte, ext string) (dbq.Track, string) { + t.Helper() + dir := t.TempDir() + path := filepath.Join(dir, title+"."+ext) + if err := os.WriteFile(path, fileBody, 0o644); err != nil { + t.Fatalf("WriteFile(%s): %v", path, err) + } + one := int32(1) + tr, err := dbq.New(pool).UpsertTrack(context.Background(), dbq.UpsertTrackParams{ + Title: title, + AlbumID: albumID, + ArtistID: artistID, + TrackNumber: &one, + DiscNumber: nil, + DurationMs: int32(len(fileBody)), + FilePath: path, + FileSize: int64(len(fileBody)), + FileFormat: strings.ToLower(ext), + Bitrate: nil, + Mbid: nil, + Genre: nil, + }) + if err != nil { + t.Fatalf("UpsertTrack(%s): %v", title, err) + } + return tr, dir +} +``` + +Add these imports to `library_fixtures_test.go` (keep existing imports; add only the new ones): + +```go +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/jackc/pgx/v5/pgtype" + "github.com/jackc/pgx/v5/pgxpool" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) +``` + +(`os`, `path/filepath`, `strings` are the additions.) + +- [ ] **Step 3: Create `internal/api/media_test.go` with unit tests for the MIME helpers** + +```go +package api + +import ( + "context" + "os" + "path/filepath" + "testing" +) + +func TestAudioContentType(t *testing.T) { + cases := map[string]string{ + "mp3": "audio/mpeg", + "MP3": "audio/mpeg", + "flac": "audio/flac", + "ogg": "audio/ogg", + "opus": "audio/ogg", + "m4a": "audio/mp4", + "aac": "audio/aac", + "wav": "audio/wav", + "unknown": "application/octet-stream", + "": "application/octet-stream", + } + for in, want := range cases { + if got := audioContentType(in); got != want { + t.Errorf("audioContentType(%q) = %q, want %q", in, got, want) + } + } +} + +func TestImageContentType(t *testing.T) { + cases := map[string]string{ + "/a/cover.jpg": "image/jpeg", + "/a/cover.JPG": "image/jpeg", + "/a/cover.jpeg": "image/jpeg", + "/a/cover.png": "image/png", + "/a/cover.webp": "image/webp", + "/a/cover.gif": "image/gif", + "/a/cover.bmp": "application/octet-stream", + "/a/cover": "application/octet-stream", + } + for in, want := range cases { + if got := imageContentType(in); got != want { + t.Errorf("imageContentType(%q) = %q, want %q", in, got, want) + } + } +} + +func TestFindSidecarCover_PrefersCoverOverFolder(t *testing.T) { + dir := t.TempDir() + // Both present — cover.jpg wins. + if err := os.WriteFile(filepath.Join(dir, "cover.jpg"), []byte("c"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "folder.jpg"), []byte("f"), 0o644); err != nil { + t.Fatal(err) + } + got := findSidecarCover(filepath.Join(dir, "any-track.flac")) + if got != filepath.Join(dir, "cover.jpg") { + t.Errorf("got %q, want cover.jpg path", got) + } +} + +func TestFindSidecarCover_FallsBackToFolder(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "folder.png"), []byte("f"), 0o644); err != nil { + t.Fatal(err) + } + got := findSidecarCover(filepath.Join(dir, "t.flac")) + if got != filepath.Join(dir, "folder.png") { + t.Errorf("got %q, want folder.png path", got) + } +} + +func TestFindSidecarCover_NoneFound(t *testing.T) { + dir := t.TempDir() + if got := findSidecarCover(filepath.Join(dir, "t.flac")); got != "" { + t.Errorf("got %q, want empty string", got) + } +} + +func TestResolveAlbumCoverPath_ExplicitPresent(t *testing.T) { + _, pool := testHandlers(t) + truncateLibrary(t, pool) + artist := seedArtist(t, pool, "A") + album := seedAlbum(t, pool, artist.ID, "X", 0) + + // Put an explicit cover file on disk, then write its path to album.cover_art_path. + dir := t.TempDir() + explicit := filepath.Join(dir, "explicit.jpg") + if err := os.WriteFile(explicit, []byte("e"), 0o644); err != nil { + t.Fatal(err) + } + if _, err := pool.Exec(context.Background(), + `UPDATE albums SET cover_art_path = $1 WHERE id = $2`, explicit, album.ID); err != nil { + t.Fatalf("UPDATE albums: %v", err) + } + // Refetch so album.CoverArtPath reflects the update. + got := resolveAlbumCoverPath(context.Background(), dbq.New(pool), fetchAlbum(t, pool, album.ID)) + if got != explicit { + t.Errorf("got %q, want %q", got, explicit) + } +} + +func TestResolveAlbumCoverPath_ExplicitMissingFallsThroughToSidecar(t *testing.T) { + _, pool := testHandlers(t) + truncateLibrary(t, pool) + artist := seedArtist(t, pool, "A") + album := seedAlbum(t, pool, artist.ID, "X", 0) + + // Seed track first (helper owns the temp dir), then drop the sidecar in + // that same dir. Explicit cover_art_path points at a file we never create. + _, dir := seedTrackWithFile(t, pool, album.ID, artist.ID, "t", []byte("audio"), "mp3") + sidecar := filepath.Join(dir, "cover.jpg") + if err := os.WriteFile(sidecar, []byte("s"), 0o644); err != nil { + t.Fatal(err) + } + if _, err := pool.Exec(context.Background(), + `UPDATE albums SET cover_art_path = $1 WHERE id = $2`, + filepath.Join(dir, "does-not-exist.jpg"), album.ID); err != nil { + t.Fatalf("UPDATE albums: %v", err) + } + + got := resolveAlbumCoverPath(context.Background(), dbq.New(pool), fetchAlbum(t, pool, album.ID)) + if got != sidecar { + t.Errorf("got %q, want %q", got, sidecar) + } +} + +func TestResolveAlbumCoverPath_NoArt(t *testing.T) { + _, pool := testHandlers(t) + truncateLibrary(t, pool) + artist := seedArtist(t, pool, "A") + album := seedAlbum(t, pool, artist.ID, "X", 0) + seedTrack(t, pool, album.ID, artist.ID, "t", 1, 1000) // synthetic /seed path, no real file + got := resolveAlbumCoverPath(context.Background(), dbq.New(pool), album) + if got != "" { + t.Errorf("got %q, want empty string", got) + } +} + +// fetchAlbum reloads the album row so tests see updates made via raw SQL. +func fetchAlbum(t *testing.T, pool *pgxpool.Pool, id pgtype.UUID) dbq.Album { + t.Helper() + a, err := dbq.New(pool).GetAlbumByID(context.Background(), id) + if err != nil { + t.Fatalf("GetAlbumByID: %v", err) + } + return a +} +``` + +Add to the imports at top of `media_test.go` as they're used: + +```go +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/jackc/pgx/v5/pgtype" + "github.com/jackc/pgx/v5/pgxpool" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) +``` + +- [ ] **Step 4: Run the new tests; all must pass** + +``` +MINSTREL_TEST_DATABASE_URL=postgres://minstrel:minstrel@localhost:5432/minstrel_test?sslmode=disable \ + go test ./internal/api/ -run 'TestAudioContentType|TestImageContentType|TestFindSidecarCover|TestResolveAlbumCoverPath' -v +``` + +Expected: all 7 tests PASS. + +- [ ] **Step 5: Run the full api test suite to catch regressions in shared fixtures** + +``` +MINSTREL_TEST_DATABASE_URL=postgres://minstrel:minstrel@localhost:5432/minstrel_test?sslmode=disable \ + go test ./internal/api/... -count=1 +``` + +Expected: all tests PASS (existing + new). + +- [ ] **Step 6: Commit** + +``` +git add internal/api/media.go internal/api/media_test.go internal/api/library_fixtures_test.go +git commit -m "feat(api): scaffold media helpers for cover + stream endpoints" +``` + +--- + +## Task 2: Implement `GET /api/albums/{id}/cover` + +**Files:** +- Modify: `internal/api/media.go` (add handler) +- Modify: `internal/api/media_test.go` (add integration tests) +- Modify: `internal/api/library_test.go` (register route in `newLibraryRouter`) + +TDD order — tests first, then handler. Tests fail first because the handler doesn't exist yet; they pass after it's added. + +- [ ] **Step 1: Register `/cover` in `newLibraryRouter`** + +Edit `internal/api/library_test.go:16-24` and add the cover route: + +```go +func newLibraryRouter(h *handlers) chi.Router { + r := chi.NewRouter() + r.Get("/api/artists", h.handleListArtists) + r.Get("/api/artists/{id}", h.handleGetArtist) + r.Get("/api/albums/{id}", h.handleGetAlbum) + r.Get("/api/albums/{id}/cover", h.handleGetCover) + r.Get("/api/tracks/{id}", h.handleGetTrack) + r.Get("/api/search", h.handleSearch) + return r +} +``` + +At this point the package won't compile — `handleGetCover` is undefined. That's expected; the integration tests need the route registered so they can fail with "handler missing" in the same compile unit as the test router. (Implementer hint: if the compile error bothers you, add the method body as `func (h *handlers) handleGetCover(w http.ResponseWriter, r *http.Request) {}` as a temporary stub for Step 2. Remove the stub before Step 4.) + +- [ ] **Step 2: Append integration tests to `internal/api/media_test.go`** + +```go +func TestHandleGetCover_SidecarHappyPath(t *testing.T) { + h, pool := testHandlers(t) + truncateLibrary(t, pool) + artist := seedArtist(t, pool, "A") + album := seedAlbum(t, pool, artist.ID, "X", 0) + + _, dir := seedTrackWithFile(t, pool, album.ID, artist.ID, "t", []byte("audio"), "mp3") + coverBytes := []byte{0xFF, 0xD8, 0xFF, 0xE0, 0x01, 0x02, 0x03} // small JPEG-ish payload + if err := os.WriteFile(filepath.Join(dir, "cover.jpg"), coverBytes, 0o644); err != nil { + t.Fatal(err) + } + + req := httptest.NewRequest(http.MethodGet, "/api/albums/"+uuidToString(album.ID)+"/cover", nil) + w := httptest.NewRecorder() + newLibraryRouter(h).ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d body = %s", w.Code, w.Body.String()) + } + if ct := w.Header().Get("Content-Type"); ct != "image/jpeg" { + t.Errorf("Content-Type = %q", ct) + } + if !bytes.Equal(w.Body.Bytes(), coverBytes) { + t.Errorf("body = %x, want %x", w.Body.Bytes(), coverBytes) + } +} + +func TestHandleGetCover_ExplicitPathHappyPath(t *testing.T) { + h, pool := testHandlers(t) + truncateLibrary(t, pool) + artist := seedArtist(t, pool, "A") + album := seedAlbum(t, pool, artist.ID, "X", 0) + + dir := t.TempDir() + explicit := filepath.Join(dir, "art.png") + pngBytes := []byte{0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A} // PNG magic + if err := os.WriteFile(explicit, pngBytes, 0o644); err != nil { + t.Fatal(err) + } + if _, err := pool.Exec(context.Background(), + `UPDATE albums SET cover_art_path = $1 WHERE id = $2`, explicit, album.ID); err != nil { + t.Fatal(err) + } + // No track needed — explicit path wins before we ever check the track dir. + + req := httptest.NewRequest(http.MethodGet, "/api/albums/"+uuidToString(album.ID)+"/cover", nil) + w := httptest.NewRecorder() + newLibraryRouter(h).ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d body = %s", w.Code, w.Body.String()) + } + if ct := w.Header().Get("Content-Type"); ct != "image/png" { + t.Errorf("Content-Type = %q", ct) + } + if !bytes.Equal(w.Body.Bytes(), pngBytes) { + t.Errorf("body mismatch") + } +} + +func TestHandleGetCover_ExplicitMissingFallsThroughToSidecar(t *testing.T) { + h, pool := testHandlers(t) + truncateLibrary(t, pool) + artist := seedArtist(t, pool, "A") + album := seedAlbum(t, pool, artist.ID, "X", 0) + + _, dir := seedTrackWithFile(t, pool, album.ID, artist.ID, "t", []byte("audio"), "mp3") + sidecarBytes := []byte("sidecar") + if err := os.WriteFile(filepath.Join(dir, "cover.jpg"), sidecarBytes, 0o644); err != nil { + t.Fatal(err) + } + if _, err := pool.Exec(context.Background(), + `UPDATE albums SET cover_art_path = $1 WHERE id = $2`, + filepath.Join(dir, "does-not-exist.jpg"), album.ID); err != nil { + t.Fatal(err) + } + + req := httptest.NewRequest(http.MethodGet, "/api/albums/"+uuidToString(album.ID)+"/cover", nil) + w := httptest.NewRecorder() + newLibraryRouter(h).ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d", w.Code) + } + if !bytes.Equal(w.Body.Bytes(), sidecarBytes) { + t.Errorf("body = %q, want %q", w.Body.Bytes(), sidecarBytes) + } +} + +func TestHandleGetCover_NoArt(t *testing.T) { + h, pool := testHandlers(t) + truncateLibrary(t, pool) + artist := seedArtist(t, pool, "A") + album := seedAlbum(t, pool, artist.ID, "X", 0) + seedTrack(t, pool, album.ID, artist.ID, "t", 1, 1000) // synthetic path, no sidecar + + req := httptest.NewRequest(http.MethodGet, "/api/albums/"+uuidToString(album.ID)+"/cover", nil) + w := httptest.NewRecorder() + newLibraryRouter(h).ServeHTTP(w, req) + + if w.Code != http.StatusNotFound { + t.Fatalf("status = %d", w.Code) + } + var body errorBody + _ = json.Unmarshal(w.Body.Bytes(), &body) + if body.Error.Code != "not_found" || body.Error.Message != "cover not found" { + t.Errorf("error body = %+v", body) + } +} + +func TestHandleGetCover_BadUUID(t *testing.T) { + h, _ := testHandlers(t) + req := httptest.NewRequest(http.MethodGet, "/api/albums/not-a-uuid/cover", nil) + w := httptest.NewRecorder() + newLibraryRouter(h).ServeHTTP(w, req) + if w.Code != http.StatusBadRequest { + t.Errorf("status = %d, want 400", w.Code) + } +} + +func TestHandleGetCover_UnknownAlbum(t *testing.T) { + h, pool := testHandlers(t) + truncateLibrary(t, pool) + req := httptest.NewRequest(http.MethodGet, + "/api/albums/00000000-0000-0000-0000-000000000001/cover", nil) + w := httptest.NewRecorder() + newLibraryRouter(h).ServeHTTP(w, req) + if w.Code != http.StatusNotFound { + t.Fatalf("status = %d", w.Code) + } + var body errorBody + _ = json.Unmarshal(w.Body.Bytes(), &body) + if body.Error.Message != "album not found" { + t.Errorf("message = %q, want \"album not found\"", body.Error.Message) + } +} +``` + +Update the imports at the top of `media_test.go` to add `"bytes"`, `"encoding/json"`, `"net/http"`, `"net/http/httptest"`: + +```go +import ( + "bytes" + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + + "github.com/jackc/pgx/v5/pgtype" + "github.com/jackc/pgx/v5/pgxpool" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) +``` + +- [ ] **Step 3: Run the cover tests — expect FAIL (handler not implemented)** + +``` +MINSTREL_TEST_DATABASE_URL=postgres://minstrel:minstrel@localhost:5432/minstrel_test?sslmode=disable \ + go test ./internal/api/ -run TestHandleGetCover -v +``` + +Expected: compile error OR 6 FAILS. + +- [ ] **Step 4: Append `handleGetCover` to `internal/api/media.go`** + +Add these imports to media.go (alongside existing): + +```go +"errors" +"net/http" + +"github.com/go-chi/chi/v5" +"github.com/jackc/pgx/v5" +``` + +Append the handler: + +```go +// handleGetCover implements GET /api/albums/{id}/cover. Resolves the cover +// path (explicit column, else sidecar next to first track), then delegates +// byte-serving to http.ServeContent so clients get Range / If-Modified-Since +// / ETag for free. +func (h *handlers) handleGetCover(w http.ResponseWriter, r *http.Request) { + id, ok := parseUUID(chi.URLParam(r, "id")) + if !ok { + writeErr(w, http.StatusBadRequest, "bad_request", "invalid album id") + return + } + q := dbq.New(h.pool) + album, err := q.GetAlbumByID(r.Context(), id) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + writeErr(w, http.StatusNotFound, "not_found", "album not found") + return + } + h.logger.Error("api: get album for cover failed", "err", err) + writeErr(w, http.StatusInternalServerError, "server_error", "server error") + return + } + path := resolveAlbumCoverPath(r.Context(), q, album) + if path == "" { + writeErr(w, http.StatusNotFound, "not_found", "cover not found") + return + } + f, err := os.Open(path) + if err != nil { + // resolveAlbumCoverPath saw the file a moment ago; if it vanished + // between stat and open, treat it the same as no-art — the user + // experience (404) matches, and 500 would misleadingly imply a server + // bug rather than a filesystem race. + writeErr(w, http.StatusNotFound, "not_found", "cover not found") + return + } + defer func() { _ = f.Close() }() + info, err := f.Stat() + if err != nil { + h.logger.Error("api: stat cover failed", "err", err) + writeErr(w, http.StatusInternalServerError, "server_error", "server error") + return + } + w.Header().Set("Content-Type", imageContentType(path)) + http.ServeContent(w, r, filepath.Base(path), info.ModTime(), f) +} +``` + +- [ ] **Step 5: Run the cover tests — expect all PASS** + +``` +MINSTREL_TEST_DATABASE_URL=postgres://minstrel:minstrel@localhost:5432/minstrel_test?sslmode=disable \ + go test ./internal/api/ -run TestHandleGetCover -v +``` + +Expected: 6 PASS. + +- [ ] **Step 6: Run the full api suite** + +``` +MINSTREL_TEST_DATABASE_URL=postgres://minstrel:minstrel@localhost:5432/minstrel_test?sslmode=disable \ + go test ./internal/api/... -count=1 +``` + +Expected: all PASS. + +- [ ] **Step 7: Commit** + +``` +git add internal/api/media.go internal/api/media_test.go internal/api/library_test.go +git commit -m "feat(api): GET /api/albums/{id}/cover with sidecar fallback" +``` + +--- + +## Task 3: Implement `GET /api/tracks/{id}/stream` + +**Files:** +- Modify: `internal/api/media.go` (add handler) +- Modify: `internal/api/media_test.go` (add tests) +- Modify: `internal/api/library_test.go` (register route in `newLibraryRouter`) + +Same TDD shape as Task 2. + +- [ ] **Step 1: Register `/stream` in `newLibraryRouter`** + +Edit `internal/api/library_test.go` so the helper also knows the stream route: + +```go +func newLibraryRouter(h *handlers) chi.Router { + r := chi.NewRouter() + r.Get("/api/artists", h.handleListArtists) + r.Get("/api/artists/{id}", h.handleGetArtist) + r.Get("/api/albums/{id}", h.handleGetAlbum) + r.Get("/api/albums/{id}/cover", h.handleGetCover) + r.Get("/api/tracks/{id}", h.handleGetTrack) + r.Get("/api/tracks/{id}/stream", h.handleGetStream) + r.Get("/api/search", h.handleSearch) + return r +} +``` + +(Same compile-time hint as Task 2 — add a stub if that compile error is in your way while writing the tests.) + +- [ ] **Step 2: Append integration tests to `internal/api/media_test.go`** + +```go +// deterministicBytes returns n bytes whose values cycle 0..255. Using a +// fixed pattern (not crypto/rand) makes assertions easy and the test +// reproducible. +func deterministicBytes(n int) []byte { + b := make([]byte, n) + for i := range b { + b[i] = byte(i % 256) + } + return b +} + +func TestHandleGetStream_HappyPath(t *testing.T) { + h, pool := testHandlers(t) + truncateLibrary(t, pool) + artist := seedArtist(t, pool, "A") + album := seedAlbum(t, pool, artist.ID, "X", 0) + body := deterministicBytes(32 * 1024) + track, _ := seedTrackWithFile(t, pool, album.ID, artist.ID, "t", body, "mp3") + + req := httptest.NewRequest(http.MethodGet, "/api/tracks/"+uuidToString(track.ID)+"/stream", nil) + w := httptest.NewRecorder() + newLibraryRouter(h).ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d body = %s", w.Code, w.Body.String()) + } + if ct := w.Header().Get("Content-Type"); ct != "audio/mpeg" { + t.Errorf("Content-Type = %q", ct) + } + if ar := w.Header().Get("Accept-Ranges"); ar != "bytes" { + t.Errorf("Accept-Ranges = %q", ar) + } + if cl := w.Header().Get("Content-Length"); cl != "32768" { + t.Errorf("Content-Length = %q, want 32768", cl) + } + if !bytes.Equal(w.Body.Bytes(), body) { + t.Errorf("body len = %d, want %d", len(w.Body.Bytes()), len(body)) + } +} + +func TestHandleGetStream_RangeFromStart(t *testing.T) { + h, pool := testHandlers(t) + truncateLibrary(t, pool) + artist := seedArtist(t, pool, "A") + album := seedAlbum(t, pool, artist.ID, "X", 0) + body := deterministicBytes(32 * 1024) + track, _ := seedTrackWithFile(t, pool, album.ID, artist.ID, "t", body, "mp3") + + req := httptest.NewRequest(http.MethodGet, "/api/tracks/"+uuidToString(track.ID)+"/stream", nil) + req.Header.Set("Range", "bytes=0-99") + w := httptest.NewRecorder() + newLibraryRouter(h).ServeHTTP(w, req) + + if w.Code != http.StatusPartialContent { + t.Fatalf("status = %d, want 206", w.Code) + } + if cr := w.Header().Get("Content-Range"); cr != "bytes 0-99/32768" { + t.Errorf("Content-Range = %q", cr) + } + if got := w.Body.Bytes(); !bytes.Equal(got, body[:100]) { + t.Errorf("body mismatch: len=%d, want 100", len(got)) + } +} + +func TestHandleGetStream_RangeFromMiddleToEnd(t *testing.T) { + h, pool := testHandlers(t) + truncateLibrary(t, pool) + artist := seedArtist(t, pool, "A") + album := seedAlbum(t, pool, artist.ID, "X", 0) + body := deterministicBytes(32 * 1024) + track, _ := seedTrackWithFile(t, pool, album.ID, artist.ID, "t", body, "mp3") + + req := httptest.NewRequest(http.MethodGet, "/api/tracks/"+uuidToString(track.ID)+"/stream", nil) + req.Header.Set("Range", "bytes=32000-") + w := httptest.NewRecorder() + newLibraryRouter(h).ServeHTTP(w, req) + + if w.Code != http.StatusPartialContent { + t.Fatalf("status = %d", w.Code) + } + if cr := w.Header().Get("Content-Range"); cr != "bytes 32000-32767/32768" { + t.Errorf("Content-Range = %q", cr) + } + if got := w.Body.Bytes(); !bytes.Equal(got, body[32000:]) { + t.Errorf("body mismatch: len=%d, want %d", len(got), len(body)-32000) + } +} + +func TestHandleGetStream_IfModifiedSinceReturns304(t *testing.T) { + h, pool := testHandlers(t) + truncateLibrary(t, pool) + artist := seedArtist(t, pool, "A") + album := seedAlbum(t, pool, artist.ID, "X", 0) + body := deterministicBytes(1024) + track, _ := seedTrackWithFile(t, pool, album.ID, artist.ID, "t", body, "mp3") + + info, err := os.Stat(track.FilePath) + if err != nil { + t.Fatal(err) + } + req := httptest.NewRequest(http.MethodGet, "/api/tracks/"+uuidToString(track.ID)+"/stream", nil) + req.Header.Set("If-Modified-Since", info.ModTime().UTC().Format(http.TimeFormat)) + w := httptest.NewRecorder() + newLibraryRouter(h).ServeHTTP(w, req) + + if w.Code != http.StatusNotModified { + t.Fatalf("status = %d, want 304", w.Code) + } + if got := w.Body.Len(); got != 0 { + t.Errorf("body len = %d, want 0 for 304", got) + } +} + +func TestHandleGetStream_BadUUID(t *testing.T) { + h, _ := testHandlers(t) + req := httptest.NewRequest(http.MethodGet, "/api/tracks/not-a-uuid/stream", nil) + w := httptest.NewRecorder() + newLibraryRouter(h).ServeHTTP(w, req) + if w.Code != http.StatusBadRequest { + t.Errorf("status = %d, want 400", w.Code) + } +} + +func TestHandleGetStream_UnknownTrack(t *testing.T) { + h, pool := testHandlers(t) + truncateLibrary(t, pool) + req := httptest.NewRequest(http.MethodGet, + "/api/tracks/00000000-0000-0000-0000-000000000001/stream", nil) + w := httptest.NewRecorder() + newLibraryRouter(h).ServeHTTP(w, req) + if w.Code != http.StatusNotFound { + t.Fatalf("status = %d", w.Code) + } + var body errorBody + _ = json.Unmarshal(w.Body.Bytes(), &body) + if body.Error.Message != "track not found" { + t.Errorf("message = %q, want \"track not found\"", body.Error.Message) + } +} + +func TestHandleGetStream_VanishedFile(t *testing.T) { + h, pool := testHandlers(t) + truncateLibrary(t, pool) + artist := seedArtist(t, pool, "A") + album := seedAlbum(t, pool, artist.ID, "X", 0) + dir := t.TempDir() + // Insert a track whose file_path points somewhere we never create. + gone := filepath.Join(dir, "does-not-exist.mp3") + if _, err := dbq.New(pool).UpsertTrack(context.Background(), dbq.UpsertTrackParams{ + Title: "gone", AlbumID: album.ID, ArtistID: artist.ID, + DurationMs: 1, FilePath: gone, FileSize: 1, FileFormat: "mp3", + }); err != nil { + t.Fatal(err) + } + // Look up the just-inserted track so we can address it by id. + tr, err := dbq.New(pool).GetTrackByPath(context.Background(), gone) + if err != nil { + t.Fatal(err) + } + + req := httptest.NewRequest(http.MethodGet, "/api/tracks/"+uuidToString(tr.ID)+"/stream", nil) + w := httptest.NewRecorder() + newLibraryRouter(h).ServeHTTP(w, req) + + if w.Code != http.StatusNotFound { + t.Fatalf("status = %d", w.Code) + } + var body errorBody + _ = json.Unmarshal(w.Body.Bytes(), &body) + if body.Error.Message != "track file not found" { + t.Errorf("message = %q, want \"track file not found\"", body.Error.Message) + } +} +``` + +- [ ] **Step 3: Run the stream tests — expect FAIL (handler not implemented)** + +``` +MINSTREL_TEST_DATABASE_URL=postgres://minstrel:minstrel@localhost:5432/minstrel_test?sslmode=disable \ + go test ./internal/api/ -run TestHandleGetStream -v +``` + +Expected: compile error OR 7 FAILS. + +- [ ] **Step 4: Append `handleGetStream` to `internal/api/media.go`** + +```go +// handleGetStream implements GET /api/tracks/{id}/stream. Opens the file on +// disk and delegates byte-serving to http.ServeContent, which handles Range, +// If-Modified-Since, and ETag based on the file's mod time. +func (h *handlers) handleGetStream(w http.ResponseWriter, r *http.Request) { + id, ok := parseUUID(chi.URLParam(r, "id")) + if !ok { + writeErr(w, http.StatusBadRequest, "bad_request", "invalid track id") + return + } + q := dbq.New(h.pool) + track, err := q.GetTrackByID(r.Context(), id) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + writeErr(w, http.StatusNotFound, "not_found", "track not found") + return + } + h.logger.Error("api: get track for stream failed", "err", err) + writeErr(w, http.StatusInternalServerError, "server_error", "server error") + return + } + f, err := os.Open(track.FilePath) + if err != nil { + // File vanished (scanner indexed it, filesystem lost it). Treat as + // 404 so clients can fall back to the next item in a queue. + writeErr(w, http.StatusNotFound, "not_found", "track file not found") + return + } + defer func() { _ = f.Close() }() + info, err := f.Stat() + if err != nil { + h.logger.Error("api: stat track failed", "err", err) + writeErr(w, http.StatusInternalServerError, "server_error", "server error") + return + } + w.Header().Set("Content-Type", audioContentType(track.FileFormat)) + w.Header().Set("Accept-Ranges", "bytes") + http.ServeContent(w, r, filepath.Base(track.FilePath), info.ModTime(), f) +} +``` + +- [ ] **Step 5: Run the stream tests — expect all PASS** + +``` +MINSTREL_TEST_DATABASE_URL=postgres://minstrel:minstrel@localhost:5432/minstrel_test?sslmode=disable \ + go test ./internal/api/ -run TestHandleGetStream -v +``` + +Expected: 7 PASS. + +- [ ] **Step 6: Run the full suite** + +``` +MINSTREL_TEST_DATABASE_URL=postgres://minstrel:minstrel@localhost:5432/minstrel_test?sslmode=disable \ + go test ./... -count=1 +``` + +Expected: all PASS. + +- [ ] **Step 7: Commit** + +``` +git add internal/api/media.go internal/api/media_test.go internal/api/library_test.go +git commit -m "feat(api): GET /api/tracks/{id}/stream with Range support" +``` + +--- + +## Task 4: Wire routes into production `Mount` + route-registration regression test + +**Files:** +- Modify: `internal/api/api.go` +- Modify: `internal/api/library_test.go` + +- [ ] **Step 1: Extend the existing `TestRoutesRegisteredInMount` paths list** + +Edit `internal/api/library_test.go:436-441` (the `paths := []string{...}` slice inside `TestRoutesRegisteredInMount`) so it becomes: + +```go +paths := []string{ + "/api/artists", + "/api/artists/00000000-0000-0000-0000-000000000001", + "/api/albums/00000000-0000-0000-0000-000000000001", + "/api/albums/00000000-0000-0000-0000-000000000001/cover", + "/api/tracks/00000000-0000-0000-0000-000000000001", + "/api/tracks/00000000-0000-0000-0000-000000000001/stream", + "/api/search?q=x", +} +``` + +- [ ] **Step 2: Run the regression test — expect FAIL (routes not in Mount)** + +``` +MINSTREL_TEST_DATABASE_URL=postgres://minstrel:minstrel@localhost:5432/minstrel_test?sslmode=disable \ + go test ./internal/api/ -run TestRoutesRegisteredInMount -v +``` + +Expected: FAIL — the two new paths return 404 because `Mount` doesn't wire them yet. + +- [ ] **Step 3: Register both routes in production `Mount`** + +Edit `internal/api/api.go:23-34` (the authed chi.Group body) to add the two new routes. The full authed block becomes: + +```go +api.Group(func(authed chi.Router) { + authed.Use(auth.RequireUser(pool)) + authed.Post("/auth/logout", h.handleLogout) + authed.Get("/me", h.handleGetMe) + + authed.Get("/artists", h.handleListArtists) + authed.Get("/artists/{id}", h.handleGetArtist) + authed.Get("/albums/{id}", h.handleGetAlbum) + authed.Get("/albums/{id}/cover", h.handleGetCover) + authed.Get("/tracks/{id}", h.handleGetTrack) + authed.Get("/tracks/{id}/stream", h.handleGetStream) + authed.Get("/search", h.handleSearch) +}) +``` + +- [ ] **Step 4: Run the regression test — expect PASS** + +``` +MINSTREL_TEST_DATABASE_URL=postgres://minstrel:minstrel@localhost:5432/minstrel_test?sslmode=disable \ + go test ./internal/api/ -run TestRoutesRegisteredInMount -v +``` + +Expected: PASS — both new paths now return 401 (auth required), not 404. + +- [ ] **Step 5: Run the full suite** + +``` +MINSTREL_TEST_DATABASE_URL=postgres://minstrel:minstrel@localhost:5432/minstrel_test?sslmode=disable \ + go test ./... -count=1 +``` + +Expected: all PASS. + +- [ ] **Step 6: Commit** + +``` +git add internal/api/api.go internal/api/library_test.go +git commit -m "feat(api): register cover + stream routes under RequireUser" +``` + +--- + +## Post-merge smoke checklist + +After the PR merges, run this sanity pass against a live build to confirm end-to-end behavior — unit and integration tests cover correctness, but browsers reveal surprises. + +- Log in through the SPA and hit `GET /api/albums/<id>/cover` for an album with a sidecar — expect the image bytes and `Content-Type: image/jpeg`. +- `GET /api/albums/<id>/cover` for an album with no art — expect a JSON 404 body. +- `GET /api/tracks/<id>/stream` in a browser `<audio>` tag — expect playback and seekable timeline (Range support). +- `curl -I -H 'Range: bytes=0-99'` against a stream — expect `206 Partial Content` and `Content-Range: bytes 0-99/<size>`. +- `curl` a second time with `If-Modified-Since` set to the first response's `Last-Modified` — expect `304 Not Modified`.