bvandeusen/minstrel
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 38 Wiki Activity

refactor(server/api): resolveByID helper for parseUUID->fetch->404 (T4)

Browse Source
This commit is contained in:
Bryan Van Deusen
2026-05-07 21:28:20 -04:00
parent 754a7955cc
commit 49218e5231
6 changed files with 168 additions and 102 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/api/admin_users.go
+5 -14
View File
@@ -212,20 +212,11 @@ func (h *handlers) handleAdminDeleteUser(w http.ResponseWriter, r *http.Request)
writeErr(w, apierror.Unauthorized("auth_required", ""))
return
}
targetID, ok := parseUUID(chi.URLParam(r, "id"))
if !ok {
writeErr(w, apierror.BadRequest("invalid_id", ""))
return
}
q := dbq.New(h.pool)
target, err := q.GetUserByID(r.Context(), targetID)
if err != nil {
if errors.Is(err, pgx.ErrNoRows) {
writeErr(w, &apierror.Error{Status: http.StatusNotFound, Code: "not_found", Message: ""})
return
}
writeErrWithLog(w, h.logger, "admin delete user: lookup failed", apierror.Internal(err))
target, apiErr := resolveByID(r, "id", q.GetUserByID, "user")
if apiErr != nil {
writeErr(w, apiErr)
return
}
@@ -243,12 +234,12 @@ func (h *handlers) handleAdminDeleteUser(w http.ResponseWriter, r *http.Request)
}
}
if err := q.DeleteUser(r.Context(), targetID); err != nil {
if err := q.DeleteUser(r.Context(), target.ID); err != nil {
writeErrWithLog(w, h.logger, "admin delete user: delete failed", apierror.Internal(err))
return
}
if err := audit.Write(r.Context(), h.pool, caller.ID, targetID, audit.ActionDeleteUser,
if err := audit.Write(r.Context(), h.pool, caller.ID, target.ID, audit.ActionDeleteUser,
map[string]any{"username": target.Username, "was_admin": target.IsAdmin}); err != nil {
h.logger.Warn("admin delete user: audit failed", "err", err)
}
internal/api/library.go
+12 -42
View File
@@ -16,20 +16,10 @@ import (
// artist names so the response is self-contained — clients should not need
// a follow-up request to render a track outside an album view.
func (h *handlers) handleGetTrack(w http.ResponseWriter, r *http.Request) {
id, ok := parseUUID(chi.URLParam(r, "id"))
if !ok {
writeErr(w, apierror.BadRequest("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, apierror.NotFound("track"))
return
}
h.logger.Error("api: get track failed", "err", err)
writeErr(w, apierror.InternalMsg("lookup failed", err))
track, apiErr := resolveByID(r, "id", q.GetTrackByID, "track")
if apiErr != nil {
writeErr(w, apiErr)
return
}
album, err := q.GetAlbumByID(r.Context(), track.AlbumID)
@@ -51,20 +41,10 @@ func (h *handlers) handleGetTrack(w http.ResponseWriter, r *http.Request) {
// tracks (ordered by disc/track number via the underlying query) with
// duration summed from the track list — keeps one source of truth.
func (h *handlers) handleGetAlbum(w http.ResponseWriter, r *http.Request) {
id, ok := parseUUID(chi.URLParam(r, "id"))
if !ok {
writeErr(w, apierror.BadRequest("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, apierror.NotFound("album"))
return
}
h.logger.Error("api: get album failed", "err", err)
writeErr(w, apierror.InternalMsg("lookup failed", err))
album, apiErr := resolveByID(r, "id", q.GetAlbumByID, "album")
if apiErr != nil {
writeErr(w, apiErr)
return
}
artist, err := q.GetArtistByID(r.Context(), album.ArtistID)
@@ -76,10 +56,10 @@ func (h *handlers) handleGetAlbum(w http.ResponseWriter, r *http.Request) {
var tracks []dbq.Track
if user, ok := auth.UserFromContext(r.Context()); ok {
tracks, err = q.ListTracksByAlbumForUser(r.Context(), dbq.ListTracksByAlbumForUserParams{
AlbumID: id, UserID: user.ID,
AlbumID: album.ID, UserID: user.ID,
})
} else {
tracks, err = q.ListTracksByAlbum(r.Context(), id)
tracks, err = q.ListTracksByAlbum(r.Context(), album.ID)
}
if err != nil {
h.logger.Error("api: list tracks failed", "err", err)
@@ -104,23 +84,13 @@ func (h *handlers) handleGetAlbum(w http.ResponseWriter, r *http.Request) {
// each album carries its own track_count (one count query per album, same
// pattern as subsonic). At M1 sizes (albums-per-artist << 100) this is fine.
func (h *handlers) handleGetArtist(w http.ResponseWriter, r *http.Request) {
id, ok := parseUUID(chi.URLParam(r, "id"))
if !ok {
writeErr(w, apierror.BadRequest("bad_request", "invalid artist id"))
return
}
q := dbq.New(h.pool)
artist, err := q.GetArtistByID(r.Context(), id)
if err != nil {
if errors.Is(err, pgx.ErrNoRows) {
writeErr(w, apierror.NotFound("artist"))
return
}
h.logger.Error("api: get artist failed", "err", err)
writeErr(w, apierror.InternalMsg("lookup failed", err))
artist, apiErr := resolveByID(r, "id", q.GetArtistByID, "artist")
if apiErr != nil {
writeErr(w, apiErr)
return
}
albums, err := q.ListAlbumsByArtist(r.Context(), id)
albums, err := q.ListAlbumsByArtist(r.Context(), artist.ID)
if err != nil {
h.logger.Error("api: list albums by artist failed", "err", err)
writeErr(w, apierror.InternalMsg("lookup failed", err))
internal/api/media.go
+6 -31
View File
@@ -8,15 +8,11 @@ package api
import (
"context"
"errors"
"net/http"
"os"
"path/filepath"
"strings"
"github.com/go-chi/chi/v5"
"github.com/jackc/pgx/v5"
"git.fabledsword.com/bvandeusen/minstrel/internal/apierror"
"git.fabledsword.com/bvandeusen/minstrel/internal/coverart"
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
@@ -88,20 +84,10 @@ func imageContentType(path string) string {
// 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, apierror.BadRequest("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, &apierror.Error{Status: 404, Code: "not_found", Message: "album not found"})
return
}
h.logger.Error("api: get album for cover failed", "err", err)
writeErr(w, apierror.InternalMsg("server error", err))
album, apiErr := resolveByID(r, "id", q.GetAlbumByID, "album")
if apiErr != nil {
writeErr(w, apiErr)
return
}
path := resolveAlbumCoverPath(r.Context(), q, album)
@@ -133,20 +119,9 @@ func (h *handlers) handleGetCover(w http.ResponseWriter, r *http.Request) {
// 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, apierror.BadRequest("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, &apierror.Error{Status: 404, Code: "not_found", Message: "track not found"})
return
}
h.logger.Error("api: get track for stream failed", "err", err)
writeErr(w, apierror.InternalMsg("server error", err))
track, apiErr := resolveByID(r, "id", dbq.New(h.pool).GetTrackByID, "track")
if apiErr != nil {
writeErr(w, apiErr)
return
}
f, err := os.Open(track.FilePath)
internal/api/requests.go
+3 -15
View File
@@ -7,7 +7,6 @@ import (
"net/http"
"github.com/go-chi/chi/v5"
"github.com/jackc/pgx/v5"
"github.com/jackc/pgx/v5/pgtype"
"git.fabledsword.com/bvandeusen/minstrel/internal/apierror"
@@ -218,20 +217,9 @@ func (h *handlers) handleGetRequest(w http.ResponseWriter, r *http.Request) {
return
}
id, ok := parseUUID(chi.URLParam(r, "id"))
if !ok {
writeErr(w, apierror.BadRequest("bad_request", "invalid request id"))
return
}
row, err := dbq.New(h.pool).GetLidarrRequestByID(r.Context(), id)
if err != nil {
if errors.Is(err, pgx.ErrNoRows) {
writeErr(w, apierror.NotFound("request"))
return
}
h.logger.Error("api: get request", "err", err)
writeErr(w, apierror.InternalMsg("get failed", err))
row, apiErr := resolveByID(r, "id", dbq.New(h.pool).GetLidarrRequestByID, "request")
if apiErr != nil {
writeErr(w, apiErr)
return
}
internal/api/resolve.go
+41
View File
@@ -0,0 +1,41 @@
package api
import (
"context"
"errors"
"net/http"
"github.com/go-chi/chi/v5"
"github.com/jackc/pgx/v5"
"github.com/jackc/pgx/v5/pgtype"
"git.fabledsword.com/bvandeusen/minstrel/internal/apierror"
)
// resolveByID parses paramKey as a UUID, fetches the row via fetch, and
// returns the row. On invalid UUID, returns BadRequest. On pgx.ErrNoRows,
// returns NotFound with code "<notFoundCode>_not_found". On other fetch
// errors, returns Internal (caller is responsible for logging if needed).
//
// Generic over T because dbq getters return concrete dbq row types.
func resolveByID[T any](
r *http.Request,
paramKey string,
fetch func(context.Context, pgtype.UUID) (T, error),
notFoundCode string,
) (T, *apierror.Error) {
var zero T
raw := chi.URLParam(r, paramKey)
id, ok := parseUUID(raw)
if !ok {
return zero, apierror.BadRequest("bad_request", "invalid "+paramKey+" id")
}
row, err := fetch(r.Context(), id)
if err != nil {
if errors.Is(err, pgx.ErrNoRows) {
return zero, apierror.NotFound(notFoundCode)
}
return zero, apierror.Internal(err)
}
return row, nil
}
internal/api/resolve_test.go
+101
View File
@@ -0,0 +1,101 @@
package api
import (
"context"
"errors"
"net/http"
"net/http/httptest"
"testing"
"github.com/go-chi/chi/v5"
"github.com/jackc/pgx/v5"
"github.com/jackc/pgx/v5/pgtype"
)
type fakeRow struct {
ID pgtype.UUID
Name string
}
func reqWithParam(_ *testing.T, key, value string) *http.Request {
r := httptest.NewRequest("GET", "/", nil)
rctx := chi.NewRouteContext()
rctx.URLParams.Add(key, value)
return r.WithContext(context.WithValue(r.Context(), chi.RouteCtxKey, rctx))
}
func TestResolveByID_InvalidUUID(t *testing.T) {
called := 0
fetch := func(ctx context.Context, id pgtype.UUID) (fakeRow, error) {
called++
return fakeRow{}, nil
}
r := reqWithParam(t, "id", "not-a-uuid")
row, apiErr := resolveByID(r, "id", fetch, "track")
if apiErr == nil {
t.Fatalf("expected apiErr, got nil (row=%+v)", row)
}
if apiErr.Status != 400 {
t.Errorf("Status: got %d, want 400", apiErr.Status)
}
if apiErr.Code != "bad_request" {
t.Errorf("Code: got %q, want %q", apiErr.Code, "bad_request")
}
if called != 0 {
t.Errorf("fetch was called %d times; expected 0", called)
}
}
func TestResolveByID_NotFound(t *testing.T) {
fetch := func(ctx context.Context, id pgtype.UUID) (fakeRow, error) {
return fakeRow{}, pgx.ErrNoRows
}
r := reqWithParam(t, "id", "11111111-1111-1111-1111-111111111111")
_, apiErr := resolveByID(r, "id", fetch, "track")
if apiErr == nil {
t.Fatal("expected apiErr, got nil")
}
if apiErr.Status != 404 {
t.Errorf("Status: got %d, want 404", apiErr.Status)
}
if apiErr.Code != "track_not_found" {
t.Errorf("Code: got %q, want %q", apiErr.Code, "track_not_found")
}
}
func TestResolveByID_Internal(t *testing.T) {
cause := errors.New("boom")
fetch := func(ctx context.Context, id pgtype.UUID) (fakeRow, error) {
return fakeRow{}, cause
}
r := reqWithParam(t, "id", "11111111-1111-1111-1111-111111111111")
_, apiErr := resolveByID(r, "id", fetch, "track")
if apiErr == nil {
t.Fatal("expected apiErr, got nil")
}
if apiErr.Status != 500 {
t.Errorf("Status: got %d, want 500", apiErr.Status)
}
if !errors.Is(apiErr, cause) {
t.Errorf("expected cause %v wrapped in apiErr; got %v", cause, apiErr)
}
}
func TestResolveByID_Success(t *testing.T) {
want := fakeRow{Name: "alpha"}
fetch := func(ctx context.Context, id pgtype.UUID) (fakeRow, error) {
want.ID = id
return want, nil
}
r := reqWithParam(t, "id", "11111111-1111-1111-1111-111111111111")
got, apiErr := resolveByID(r, "id", fetch, "track")
if apiErr != nil {
t.Fatalf("unexpected apiErr: %v", apiErr)
}
if got.Name != want.Name {
t.Errorf("Name: got %q, want %q", got.Name, want.Name)
}
if got.ID != want.ID {
t.Errorf("ID: got %+v, want %+v", got.ID, want.ID)
}
}