feat: a release catalogue from MusicBrainz; request search asks it directly (M502 #5519)
release / govulncheck (push) Successful in 22s
release / web (push) Failing after 24s
release / go (push) Successful in 1m57s
release / integration (push) Successful in 4m1s
release / Attach APK to the Release (tag releases only) (push) Canceled after 0s
release / Build + push container image (push) Canceled after 0s
release / Verify release artifacts (tag releases only) (push) Canceled after 0s
release / android (push) Canceled after 4m38s
release / Build signed APK (releases and dev) (push) Canceled after 4m37s

Minstrel keeps its own record of what artists have released, read from
MusicBrainz, so the wanted list, search and import no longer need Lidarr.

- internal/musicbrainz: searches (artists, release groups, recordings) and
  browses (an artist's release groups; a group's releases with track lists).
  One process-wide pacer at 1.1s, shared with tag enrichment, because
  MusicBrainz's limit is per address. A 503 throttle is retried.
- Migration 0080: mb_artists, mb_release_groups, mb_artist_release_groups
  (a collaboration belongs to each credited artist), mb_releases,
  mb_release_tracks.
- internal/catalogue: Discography and Releases serve the held copy and
  re-read it when older than 7 or 30 days. An unreachable MusicBrainz
  serves the stale copy, marked. An hourly pass re-reads due artists.
- Art for what isn't held comes through /api/catalogue/art from the
  enabled cover-art sources (Cover Art Archive by release group, then the
  name-based sources) and is kept on disk, so the browser never fetches a
  third-party host. A miss is a 404 and the card shows its icon.
- Request search moves from /api/lidarr/search to /api/catalogue/search,
  with the same response shape. Albums are release groups, and "in
  library" matches albums.release_group_mbid. A 10-minute cache absorbs
  search-as-you-type. Discover suggestions point at the art endpoint
  instead of looking up Lidarr.
- Android calls the new path and loads Discover art through ServerImage.
  Installed builds older than this one lose Discover search until they
  update (no compatibility path, rule 22).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-10-09 21:08:22 -04:00
co-authored by Claude Opus 5.5
parent a2e1890d4c
commit 77389f88bc
40 changed files with 3223 additions and 577 deletions
+26 -1
View File
@@ -15,6 +15,7 @@ import (
"git.fabledsword.com/bvandeusen/minstrel/internal/acquire"
"git.fabledsword.com/bvandeusen/minstrel/internal/auth"
"git.fabledsword.com/bvandeusen/minstrel/internal/catalogue"
"git.fabledsword.com/bvandeusen/minstrel/internal/config"
"git.fabledsword.com/bvandeusen/minstrel/internal/coverart"
"git.fabledsword.com/bvandeusen/minstrel/internal/eventbus"
@@ -72,6 +73,9 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev
loudnessSettings: loudSettings,
acoustIDLookup: acoustIDLookup,
acquisition: acquire.NewStore(pool),
catalogue: catalogue.New(pool, nil, logger),
catalogueSearches: newSearchCache(),
catalogueArt: catalogue.NewArt(dataDir, artSources(coverSettings)),
leveled: newLeveledRenderer(dataDir, loudSettings, logger),
librarySize: recommendation.NewLibrarySize(nil),
loginGuard: auth.NewLoginGuard(),
@@ -184,7 +188,11 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev
authed.Get("/likes/artists", h.handleListLikedArtists)
authed.Get("/likes/ids", h.handleGetLikedIDs)
authed.Get("/lidarr/search", h.handleLidarrSearch)
// The release catalogue (M502 #5519): request search asks
// MusicBrainz, and art for what isn't held comes through
// Minstrel from the enabled cover-art sources.
authed.Get("/catalogue/search", h.handleCatalogueSearch)
authed.Get("/catalogue/art/{kind}/{mbid}", h.handleCatalogueArt)
authed.Post("/requests", h.handleCreateRequest)
authed.Get("/requests", h.handleListRequests)
@@ -385,6 +393,13 @@ type handlers struct {
acoustIDLookup *library.AcoustIDLookupWorker
// acquisition is the operator's indexers and download clients (M502).
acquisition *acquire.Store
// catalogue is the release catalogue read from MusicBrainz, and
// catalogueArt the images for what isn't held (M502 #5519).
catalogue *catalogue.Service
catalogueArt *catalogue.Art
// catalogueSearches reuses recent MusicBrainz answers; nil asks every
// time, which is what handler tests get.
catalogueSearches *searchCache
// leveled renders the leveled streams handed to Sonos/UPnP speakers
// (M464 #5001). Nil when its cache directory cannot be made: a level
// request then gets the plain stream.
@@ -427,3 +442,13 @@ func newLeveledRenderer(dataDir string, settings *library.LoudnessSettingsServic
}
return r
}
// artSources hands the art cache the cover-art settings, or nothing when
// there are none: a nil *SettingsService inside the interface would not
// compare equal to nil and would be called.
func artSources(s *coverart.SettingsService) catalogue.ArtSources {
if s == nil {
return nil
}
return s
}
+284
View File
@@ -0,0 +1,284 @@
package api
import (
"context"
"errors"
"net/http"
"net/url"
"slices"
"strings"
"sync"
"time"
"github.com/go-chi/chi/v5"
"git.fabledsword.com/bvandeusen/minstrel/internal/apierror"
"git.fabledsword.com/bvandeusen/minstrel/internal/catalogue"
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
"git.fabledsword.com/bvandeusen/minstrel/internal/musicbrainz"
)
// catalogueSearchLimit is how many results one search shows.
const catalogueSearchLimit = 25
// catalogueSearchResult is one row of GET /api/catalogue/search. The shape is
// the one the Lidarr search answered with, so the Discover page and request
// rows read it unchanged. artist_mbid and album_mbid are the parents a
// request needs: empty on an artist, the artist on an album, both on a track.
// An album is a MusicBrainz release group, as Lidarr's albums were.
type catalogueSearchResult struct {
MBID string `json:"mbid"`
Name string `json:"name"`
SecondaryText string `json:"secondary_text"`
ImageURL string `json:"image_url"`
ArtistMBID string `json:"artist_mbid"`
AlbumMBID string `json:"album_mbid"`
InLibrary bool `json:"in_library"`
Requested bool `json:"requested"`
}
// handleCatalogueSearch implements GET /api/catalogue/search?q=&kind=artist|album|track
// (M502 #5519), asking MusicBrainz directly. An unreachable MusicBrainz is a
// 503 the page shows as such (rule 164).
func (h *handlers) handleCatalogueSearch(w http.ResponseWriter, r *http.Request) {
kind := strings.TrimSpace(r.URL.Query().Get("kind"))
switch kind {
case "artist", "album", "track":
default:
writeErr(w, apierror.BadRequest("bad_kind", "kind must be artist, album, or track"))
return
}
q := strings.TrimSpace(r.URL.Query().Get("q"))
if q == "" {
writeErr(w, apierror.BadRequest("missing_query", "q is required"))
return
}
out, err := h.catalogueSearches.get(r.Context(), kind, q, h.searchCatalogue)
if err != nil {
switch {
case errors.Is(err, musicbrainz.ErrUnreachable), errors.Is(err, musicbrainz.ErrBadResponse):
writeErr(w, &apierror.Error{Status: 503, Code: "musicbrainz_unreachable", Message: "MusicBrainz can't be reached", Cause: err})
case r.Context().Err() != nil:
return
default:
h.logger.Error("api: catalogue search", "kind", kind, "err", err)
writeErr(w, apierror.InternalMsg("search failed", err))
}
return
}
dbQ := dbq.New(h.pool)
for i := range out {
inLib, err := h.catalogueInLibrary(r.Context(), kind, out[i].MBID)
if err != nil {
writeErr(w, apierror.InternalMsg("library check failed", err))
return
}
requested, err := dbQ.HasNonTerminalRequestForMBID(r.Context(), out[i].MBID)
if err != nil {
writeErr(w, apierror.InternalMsg("request check failed", err))
return
}
out[i].InLibrary, out[i].Requested = inLib, requested
}
writeJSON(w, http.StatusOK, out)
}
func (h *handlers) searchCatalogue(ctx context.Context, kind, q string) ([]catalogueSearchResult, error) {
mb := h.catalogue.MusicBrainz()
out := []catalogueSearchResult{}
switch kind {
case "artist":
artists, err := mb.SearchArtists(ctx, q, catalogueSearchLimit)
if err != nil {
return nil, err
}
for _, a := range artists {
out = append(out, catalogueSearchResult{
MBID: a.MBID, Name: a.Name, SecondaryText: artistLine(a),
ImageURL: artistArtURL(a.MBID, a.Name),
})
}
case "album":
groups, err := mb.SearchReleaseGroups(ctx, q, catalogueSearchLimit)
if err != nil {
return nil, err
}
for _, g := range groups {
out = append(out, catalogueSearchResult{
MBID: g.MBID, Name: g.Title, SecondaryText: joinNonEmpty(g.ArtistCredit, g.Year(), groupType(g)),
ImageURL: groupArtURL(g), ArtistMBID: first(g.ArtistMBIDs),
})
}
case "track":
recs, err := mb.SearchRecordings(ctx, q, catalogueSearchLimit)
if err != nil {
return nil, err
}
for _, rec := range recs {
res := catalogueSearchResult{
MBID: rec.MBID, Name: rec.Title,
SecondaryText: joinNonEmpty(rec.ReleaseGroup.Title, rec.ArtistCredit),
ArtistMBID: first(rec.ArtistMBIDs), AlbumMBID: rec.ReleaseGroup.MBID,
}
if rec.ReleaseGroup.MBID != "" {
res.ImageURL = groupArtURL(rec.ReleaseGroup)
}
out = append(out, res)
}
}
return out, nil
}
// catalogueSearchTTL is how long a search's MusicBrainz answer is reused.
// MusicBrainz allows a request a second, and Discover searches as the user
// types, so the same words are often asked again within moments.
const catalogueSearchTTL = 10 * time.Minute
// catalogueSearchCacheMax bounds the cache; past it, the cache starts over.
const catalogueSearchCacheMax = 500
// searchCache keeps recent MusicBrainz answers by kind and query. A nil
// cache asks every time. Whether a result is held or requested is not
// cached: the handler works that out on every request.
type searchCache struct {
mu sync.Mutex
entries map[string]searchEntry
now func() time.Time
}
type searchEntry struct {
at time.Time
results []catalogueSearchResult
}
func newSearchCache() *searchCache {
return &searchCache{entries: map[string]searchEntry{}, now: time.Now}
}
func (c *searchCache) get(ctx context.Context, kind, q string,
ask func(context.Context, string, string) ([]catalogueSearchResult, error),
) ([]catalogueSearchResult, error) {
if c == nil {
return ask(ctx, kind, q)
}
key := kind + "\x00" + strings.ToLower(q)
c.mu.Lock()
e, ok := c.entries[key]
c.mu.Unlock()
if ok && c.now().Sub(e.at) < catalogueSearchTTL {
return slices.Clone(e.results), nil
}
results, err := ask(ctx, kind, q)
if err != nil {
return nil, err
}
c.mu.Lock()
if len(c.entries) >= catalogueSearchCacheMax {
c.entries = map[string]searchEntry{}
}
c.entries[key] = searchEntry{at: c.now(), results: slices.Clone(results)}
c.mu.Unlock()
return results, nil
}
// artistLine is what tells two artists of one name apart: MusicBrainz's own
// disambiguation when it has one, else the kind of artist and the country.
func artistLine(a musicbrainz.Artist) string {
if a.Disambiguation != "" {
return a.Disambiguation
}
return joinNonEmpty(a.Type, a.Country)
}
// groupType names an album's kind the way people do: "Album", "EP",
// "Album · Live", "Single · Remix".
func groupType(g musicbrainz.ReleaseGroup) string {
return joinNonEmpty(append([]string{g.PrimaryType}, g.SecondaryTypes...)...)
}
func joinNonEmpty(parts ...string) string {
kept := parts[:0:0]
for _, p := range parts {
if p != "" {
kept = append(kept, p)
}
}
return strings.Join(kept, " · ")
}
func first(s []string) string {
if len(s) == 0 {
return ""
}
return s[0]
}
// artistArtURL and groupArtURL point at Minstrel's own art endpoints. The
// names ride along for the sources that look art up by name.
func artistArtURL(mbid, name string) string {
if mbid == "" {
return ""
}
return "/api/catalogue/art/artist/" + mbid + "?" + url.Values{"name": {name}}.Encode()
}
func groupArtURL(g musicbrainz.ReleaseGroup) string {
return "/api/catalogue/art/release-group/" + g.MBID + "?" +
url.Values{"artist": {g.ArtistCredit}, "title": {g.Title}}.Encode()
}
// catalogueInLibrary reports whether a result is already held: an artist by
// its MBID, an album by its release group (or a release id that happens to be
// the same), a track by its recording.
func (h *handlers) catalogueInLibrary(ctx context.Context, kind, mbid string) (bool, error) {
var query string
switch kind {
case "artist":
query = `SELECT EXISTS(SELECT 1 FROM artists WHERE mbid = $1)`
case "album":
query = `SELECT EXISTS(SELECT 1 FROM albums WHERE release_group_mbid = $1 OR mbid = $1)`
case "track":
query = `SELECT EXISTS(SELECT 1 FROM tracks WHERE mbid = $1)`
default:
return false, nil
}
var exists bool
err := h.pool.QueryRow(ctx, query, mbid).Scan(&exists)
return exists, err
}
// handleCatalogueArt implements GET /api/catalogue/art/{kind}/{mbid} for an
// artist or a release group Minstrel doesn't hold. The image comes from the
// operator's enabled cover-art sources and is kept on disk, so the browser
// never fetches a third-party host. A 404 shows the placeholder.
func (h *handlers) handleCatalogueArt(w http.ResponseWriter, r *http.Request) {
kind, mbid := chi.URLParam(r, "kind"), chi.URLParam(r, "mbid")
if !catalogue.ValidMBID(mbid) {
writeErr(w, apierror.NotFound("art"))
return
}
var (
body []byte
err error
)
switch kind {
case "artist":
body, err = h.catalogueArt.Artist(r.Context(), mbid, r.URL.Query().Get("name"))
case "release-group":
body, err = h.catalogueArt.ReleaseGroup(r.Context(), mbid, r.URL.Query().Get("artist"), r.URL.Query().Get("title"))
default:
writeErr(w, apierror.NotFound("art"))
return
}
if err != nil {
if r.Context().Err() == nil {
writeErr(w, apierror.NotFound("art"))
}
return
}
w.Header().Set("Content-Type", http.DetectContentType(body))
w.Header().Set("Cache-Control", "private, max-age=86400")
_, _ = w.Write(body)
}
+253
View File
@@ -0,0 +1,253 @@
package api
import (
"context"
"encoding/json"
"net/http"
"net/http/httptest"
"net/url"
"testing"
"time"
"github.com/go-chi/chi/v5"
"git.fabledsword.com/bvandeusen/minstrel/internal/catalogue"
"git.fabledsword.com/bvandeusen/minstrel/internal/coverart"
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
"git.fabledsword.com/bvandeusen/minstrel/internal/dbtest"
"git.fabledsword.com/bvandeusen/minstrel/internal/musicbrainz"
)
const (
searchArtistMBID = "a74b1b7f-71a5-4011-9441-d0b5e4122711"
searchGroupMBID = "b1392450-e666-3926-a536-22c65f834433"
searchRecMBID = "c1a0e1b2-0000-4000-8000-000000000001"
)
// fakeMusicBrainz answers the three searches with one Radiohead result each,
// or 503 when down is set.
func fakeMusicBrainz(t *testing.T, down bool) *httptest.Server {
t.Helper()
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if down {
w.WriteHeader(http.StatusServiceUnavailable)
return
}
credit := `[{"name": "Radiohead", "artist": {"id": "` + searchArtistMBID + `"}}]`
group := `{"id": "` + searchGroupMBID + `", "title": "OK Computer", "primary-type": "Album",
"first-release-date": "1997-05-21", "artist-credit": ` + credit + `}`
switch r.URL.Path {
case "/artist":
_, _ = w.Write([]byte(`{"artists": [{"id": "` + searchArtistMBID + `", "name": "Radiohead", "type": "Group", "country": "GB"}]}`))
case "/release-group":
_, _ = w.Write([]byte(`{"release-groups": [` + group + `]}`))
case "/recording":
_, _ = w.Write([]byte(`{"recordings": [{"id": "` + searchRecMBID + `", "title": "Karma Police",
"artist-credit": ` + credit + `,
"releases": [{"id": "r1", "status": "Official", "release-group": ` + group + `}]}]}`))
default:
w.WriteHeader(http.StatusNotFound)
}
}))
t.Cleanup(srv.Close)
return srv
}
func catalogueHandlers(t *testing.T, down bool) *handlers {
t.Helper()
h, pool := testHandlers(t)
resetLidarrState(t, h)
mb := &musicbrainz.Client{BaseURL: fakeMusicBrainz(t, down).URL, Pace: &musicbrainz.Pacer{}, Retries: -1}
h.catalogue = catalogue.New(pool, mb, h.logger)
return h
}
func doCatalogueSearch(t *testing.T, h *handlers, q, kind string) *httptest.ResponseRecorder {
t.Helper()
req := httptest.NewRequest(http.MethodGet, "/api/catalogue/search?"+url.Values{"q": {q}, "kind": {kind}}.Encode(), nil)
w := httptest.NewRecorder()
h.handleCatalogueSearch(w, req)
return w
}
func decodeResults(t *testing.T, w *httptest.ResponseRecorder) []catalogueSearchResult {
t.Helper()
if w.Code != http.StatusOK {
t.Fatalf("status = %d, body = %s", w.Code, w.Body.String())
}
var out []catalogueSearchResult
if err := json.Unmarshal(w.Body.Bytes(), &out); err != nil {
t.Fatalf("decode: %v; body = %s", err, w.Body.String())
}
return out
}
func errorCode(t *testing.T, w *httptest.ResponseRecorder) string {
t.Helper()
var resp struct {
Error struct {
Code string `json:"code"`
} `json:"error"`
}
if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
t.Fatalf("decode: %v; body = %s", err, w.Body.String())
}
return resp.Error.Code
}
func TestCatalogueSearch_RefusesABadKindOrNoQuery(t *testing.T) {
h := catalogueHandlers(t, false)
if w := doCatalogueSearch(t, h, "x", "playlist"); w.Code != http.StatusBadRequest || errorCode(t, w) != "bad_kind" {
t.Errorf("bad kind: %d %s", w.Code, w.Body.String())
}
if w := doCatalogueSearch(t, h, " ", "artist"); w.Code != http.StatusBadRequest || errorCode(t, w) != "missing_query" {
t.Errorf("no query: %d %s", w.Code, w.Body.String())
}
}
func TestCatalogueSearch_EachKindAsksMusicBrainz(t *testing.T) {
h := catalogueHandlers(t, false)
artists := decodeResults(t, doCatalogueSearch(t, h, "radiohead", "artist"))
if len(artists) != 1 || artists[0].MBID != searchArtistMBID || artists[0].SecondaryText != "Group · GB" {
t.Fatalf("artists = %+v", artists)
}
if artists[0].ImageURL != "/api/catalogue/art/artist/"+searchArtistMBID+"?name=Radiohead" {
t.Errorf("artist art = %q: it must be Minstrel's own endpoint", artists[0].ImageURL)
}
albums := decodeResults(t, doCatalogueSearch(t, h, "ok computer", "album"))
if len(albums) != 1 || albums[0].MBID != searchGroupMBID || albums[0].ArtistMBID != searchArtistMBID ||
albums[0].SecondaryText != "Radiohead · 1997 · Album" {
t.Fatalf("albums = %+v", albums)
}
tracks := decodeResults(t, doCatalogueSearch(t, h, "karma police", "track"))
if len(tracks) != 1 || tracks[0].AlbumMBID != searchGroupMBID || tracks[0].ArtistMBID != searchArtistMBID ||
tracks[0].SecondaryText != "OK Computer · Radiohead" {
t.Fatalf("tracks = %+v", tracks)
}
}
func TestCatalogueSearch_MarksWhatIsHeldAndWhatIsRequested(t *testing.T) {
h := catalogueHandlers(t, false)
ctx := context.Background()
q := dbq.New(h.pool)
artistMBID := searchArtistMBID
artist, err := q.UpsertArtist(ctx, dbq.UpsertArtistParams{Name: dbtest.TestUserPrefix + "Radiohead", SortName: "Radiohead", Mbid: &artistMBID})
if err != nil {
t.Fatal(err)
}
// The album is held under a release id; its release group is what matches.
if _, err := h.pool.Exec(ctx,
`INSERT INTO albums (title, sort_title, artist_id, mbid, release_group_mbid) VALUES ('OK Computer', 'OK Computer', $1, 'some-release-id', $2)`,
artist.ID, searchGroupMBID); err != nil {
t.Fatal(err)
}
if got := decodeResults(t, doCatalogueSearch(t, h, "radiohead", "artist")); !got[0].InLibrary {
t.Error("the artist is held")
}
if got := decodeResults(t, doCatalogueSearch(t, h, "ok computer", "album")); !got[0].InLibrary {
t.Error("the album is held, matched by release group")
}
user := seedUser(t, h.pool, "requester", "pw", false)
track := searchRecMBID
if _, err := q.CreateLidarrRequest(ctx, dbq.CreateLidarrRequestParams{
UserID: user.ID, Kind: dbq.LidarrRequestKindTrack, LidarrArtistMbid: searchArtistMBID,
LidarrTrackMbid: &track, ArtistName: "Radiohead",
}); err != nil {
t.Fatal(err)
}
got := decodeResults(t, doCatalogueSearch(t, h, "karma police", "track"))
if got[0].InLibrary || !got[0].Requested {
t.Errorf("track = %+v, want requested and not held", got[0])
}
}
func TestCatalogueSearch_UnreachableMusicBrainzIs503(t *testing.T) {
h := catalogueHandlers(t, true)
w := doCatalogueSearch(t, h, "radiohead", "artist")
if w.Code != http.StatusServiceUnavailable || errorCode(t, w) != "musicbrainz_unreachable" {
t.Fatalf("status %d, body %s", w.Code, w.Body.String())
}
}
// artSource is an artist-art source with one image.
type artSource struct{ apiTestAlbumProvider }
func (artSource) FetchArtistArt(_ context.Context, _ coverart.ArtistRef) ([]byte, []byte, error) {
return []byte("\x89PNG\r\n\x1a\nrest"), nil, nil
}
type fakeArtSources struct{ artists []coverart.ArtistArtProvider }
func (s fakeArtSources) EnabledArtistProviders() []coverart.ArtistArtProvider { return s.artists }
func (fakeArtSources) EnabledAlbumProviders() []coverart.AlbumCoverProvider { return nil }
func TestCatalogueArt_ServesTheImageOrAPlaceholder404(t *testing.T) {
h := &handlers{catalogueArt: catalogue.NewArt(t.TempDir(), fakeArtSources{artists: []coverart.ArtistArtProvider{&artSource{}}})}
r := chi.NewRouter()
r.Get("/api/catalogue/art/{kind}/{mbid}", h.handleCatalogueArt)
get := func(path string) *httptest.ResponseRecorder {
w := httptest.NewRecorder()
r.ServeHTTP(w, httptest.NewRequest(http.MethodGet, path, nil))
return w
}
if w := get("/api/catalogue/art/artist/" + searchArtistMBID + "?name=Radiohead"); w.Code != http.StatusOK || w.Header().Get("Content-Type") != "image/png" {
t.Errorf("artist: %d %q", w.Code, w.Header().Get("Content-Type"))
}
// No album source is enabled: a 404, which the page shows as its placeholder.
if w := get("/api/catalogue/art/release-group/" + searchGroupMBID); w.Code != http.StatusNotFound {
t.Errorf("release group with no source: %d", w.Code)
}
for _, bad := range []string{"/api/catalogue/art/artist/not-an-id", "/api/catalogue/art/label/" + searchArtistMBID} {
if w := get(bad); w.Code != http.StatusNotFound {
t.Errorf("%s: %d", bad, w.Code)
}
}
}
func TestSuggestionArt_PointsAtMinstrelsOwnEndpoint(t *testing.T) {
views := []suggestionView{{MBID: searchArtistMBID, Name: "Sigur Rós"}, {Name: "No MBID"}}
resolveSuggestionArt(views)
if views[0].ImageURL != "/api/catalogue/art/artist/"+searchArtistMBID+"?name=Sigur+R%C3%B3s" {
t.Errorf("image_url = %q", views[0].ImageURL)
}
if views[1].ImageURL != "" {
t.Errorf("no MBID, no image: %q", views[1].ImageURL)
}
}
func TestSearchCache_ReusesAnAnswerButNotTheFlags(t *testing.T) {
c := newSearchCache()
now := time.Now()
c.now = func() time.Time { return now }
asks := 0
ask := func(context.Context, string, string) ([]catalogueSearchResult, error) {
asks++
return []catalogueSearchResult{{MBID: "m"}}, nil
}
first, _ := c.get(context.Background(), "artist", "Radiohead", ask)
first[0].InLibrary = true // the handler sets flags on what it is given
again, _ := c.get(context.Background(), "artist", "radiohead", ask)
if asks != 1 {
t.Errorf("asks = %d: the same words in another case are the same search", asks)
}
if again[0].InLibrary {
t.Error("a flag set on one answer leaked into the cache")
}
if _, _ = c.get(context.Background(), "album", "radiohead", ask); asks != 2 {
t.Error("another kind is another search")
}
now = now.Add(catalogueSearchTTL + time.Second)
if _, _ = c.get(context.Background(), "artist", "radiohead", ask); asks != 3 {
t.Error("an old answer is asked again")
}
var none *searchCache
if _, _ = none.get(context.Background(), "artist", "x", ask); asks != 4 {
t.Error("a nil cache asks every time")
}
}
-138
View File
@@ -1,138 +0,0 @@
package api
import (
"context"
"errors"
"net/http"
"strings"
"git.fabledsword.com/bvandeusen/minstrel/internal/apierror"
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
"git.fabledsword.com/bvandeusen/minstrel/internal/lidarr"
)
// lidarrSearchResult is the JSON shape returned by GET /api/lidarr/search.
// Field names follow the lowercase snake_case convention used by all /api/*
// responses. artist_mbid and album_mbid are the parent identifiers surfaced
// by LookupAlbum / LookupTrack; they are empty strings on artist results.
type lidarrSearchResult struct {
MBID string `json:"mbid"`
Name string `json:"name"`
SecondaryText string `json:"secondary_text"`
ImageURL string `json:"image_url"`
ArtistMBID string `json:"artist_mbid"`
AlbumMBID string `json:"album_mbid"`
InLibrary bool `json:"in_library"`
Requested bool `json:"requested"`
}
// handleLidarrSearch implements GET /api/lidarr/search?q=&kind=artist|album|track.
//
// Steps:
// 1. Validate kind and q params.
// 2. Load lidarr config; 503 when disabled.
// 3. Call the matching Lidarr lookup.
// 4. For each result, compute in_library (EXISTS by MBID) and requested
// (HasNonTerminalRequestForMBID).
// 5. Return normalized JSON array.
func (h *handlers) handleLidarrSearch(w http.ResponseWriter, r *http.Request) {
kind := strings.TrimSpace(r.URL.Query().Get("kind"))
switch kind {
case "artist", "album", "track":
// valid
default:
writeErr(w, apierror.BadRequest("bad_kind", "kind must be artist, album, or track"))
return
}
q := strings.TrimSpace(r.URL.Query().Get("q"))
if q == "" {
writeErr(w, apierror.BadRequest("missing_query", "q is required"))
return
}
cfg, err := h.lidarrCfg.Get(r.Context())
if err != nil {
h.logger.Error("api: lidarr search: load config", "err", err)
writeErr(w, &apierror.Error{Status: 500, Code: "lidarr_lookup_failed", Message: "failed to load lidarr config", Cause: err})
return
}
if !cfg.Enabled {
writeErr(w, &apierror.Error{Status: 503, Code: "lidarr_disabled", Message: "Lidarr integration is not enabled"})
return
}
client := lidarr.NewClient(cfg.BaseURL, cfg.APIKey)
var results []lidarr.LookupResult
switch kind {
case "artist":
results, err = client.LookupArtist(r.Context(), q)
case "album":
results, err = client.LookupAlbum(r.Context(), q)
case "track":
results, err = client.LookupTrack(r.Context(), q)
}
if err != nil {
switch {
case errors.Is(err, lidarr.ErrUnreachable):
writeErr(w, &apierror.Error{Status: 503, Code: "lidarr_unreachable", Message: "Lidarr is unreachable", Cause: err})
case errors.Is(err, lidarr.ErrAuthFailed):
writeErr(w, &apierror.Error{Status: 503, Code: "lidarr_auth_failed", Message: "Lidarr authentication failed", Cause: err})
default:
h.logger.Error("api: lidarr search: lookup failed", "kind", kind, "err", err)
writeErr(w, &apierror.Error{Status: 500, Code: "lidarr_lookup_failed", Message: "Lidarr lookup failed", Cause: err})
}
return
}
dbQ := dbq.New(h.pool)
out := make([]lidarrSearchResult, 0, len(results))
for _, res := range results {
inLib, libErr := h.lidarrInLibrary(r.Context(), kind, res.MBID)
if libErr != nil {
h.logger.Error("api: lidarr search: in_library check failed", "err", libErr)
writeErr(w, &apierror.Error{Status: 500, Code: "lidarr_lookup_failed", Message: "library check failed", Cause: libErr})
return
}
requested, reqErr := dbQ.HasNonTerminalRequestForMBID(r.Context(), res.MBID)
if reqErr != nil {
h.logger.Error("api: lidarr search: requested check failed", "err", reqErr)
writeErr(w, &apierror.Error{Status: 500, Code: "lidarr_lookup_failed", Message: "request check failed", Cause: reqErr})
return
}
out = append(out, lidarrSearchResult{
MBID: res.MBID,
Name: res.Name,
SecondaryText: res.Secondary,
ImageURL: res.ImageURL,
ArtistMBID: res.ArtistMBID,
AlbumMBID: res.AlbumMBID,
InLibrary: inLib,
Requested: requested,
})
}
writeJSON(w, http.StatusOK, out)
}
// lidarrInLibrary checks whether the given MBID exists in the corresponding
// local library table based on kind. Runs a direct EXISTS query since
// these per-kind MBID lookups are not in the sqlc-generated query set.
func (h *handlers) lidarrInLibrary(ctx context.Context, kind, mbid string) (bool, error) {
var query string
switch kind {
case "artist":
query = `SELECT EXISTS(SELECT 1 FROM artists WHERE mbid = $1)`
case "album":
query = `SELECT EXISTS(SELECT 1 FROM albums WHERE mbid = $1)`
case "track":
query = `SELECT EXISTS(SELECT 1 FROM tracks WHERE mbid = $1)`
default:
return false, nil
}
var exists bool
if err := h.pool.QueryRow(ctx, query, mbid).Scan(&exists); err != nil {
return false, err
}
return exists, nil
}
+4 -276
View File
@@ -2,20 +2,17 @@ package api
import (
"context"
"encoding/json"
"fmt"
"net/http"
"net/http/httptest"
"testing"
"time"
"github.com/jackc/pgx/v5/pgtype"
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
"git.fabledsword.com/bvandeusen/minstrel/internal/dbtest"
"git.fabledsword.com/bvandeusen/minstrel/internal/lidarrconfig"
)
// Helpers shared by the request, quarantine and notification tests. The
// Lidarr search they once sat beside is now the catalogue search
// (catalogue_test.go, M502 #5519).
// resetLidarrState clears lidarr_requests rows and resets lidarr_config
// to the factory-default state (enabled=false, nulls) between tests.
func resetLidarrState(t *testing.T, h *handlers) {
@@ -44,11 +41,6 @@ func saveLidarrConfig(t *testing.T, h *handlers, baseURL string, enabled bool) {
}
}
// lidarrArtistStubBody returns a minimal Lidarr artist lookup JSON payload.
func lidarrArtistStubBody(mbid, name string) string {
return fmt.Sprintf(`[{"foreignArtistId":%q,"artistName":%q,"genres":["Rock"],"albumCount":5,"images":[]}]`, mbid, name)
}
// newLidarrStub creates an httptest.Server that always returns the given body
// with 200 OK for any /api/v1/* path.
func newLidarrStub(t *testing.T, body string) *httptest.Server {
@@ -61,267 +53,3 @@ func newLidarrStub(t *testing.T, body string) *httptest.Server {
t.Cleanup(srv.Close)
return srv
}
// seedArtistWithMBID inserts an artist row with a specific MBID for in_library testing.
func seedArtistWithMBID(t *testing.T, h *handlers, name, mbid string) dbq.Artist {
t.Helper()
a, err := dbq.New(h.pool).UpsertArtist(context.Background(), dbq.UpsertArtistParams{
Name: name,
SortName: name,
Mbid: &mbid,
})
if err != nil {
t.Fatalf("seedArtistWithMBID: %v", err)
}
return a
}
// seedRequestForMBID inserts a pending lidarr_requests row for the given MBID.
func seedRequestForMBID(t *testing.T, h *handlers, userID pgtype.UUID, kind dbq.LidarrRequestKind, artistMBID, mbid string) {
t.Helper()
var albumMBID, trackMBID *string
switch kind {
case dbq.LidarrRequestKindAlbum:
albumMBID = &mbid
case dbq.LidarrRequestKindTrack:
trackMBID = &mbid
}
_, err := dbq.New(h.pool).CreateLidarrRequest(context.Background(), dbq.CreateLidarrRequestParams{
UserID: userID,
Kind: kind,
LidarrArtistMbid: artistMBID,
LidarrAlbumMbid: albumMBID,
LidarrTrackMbid: trackMBID,
ArtistName: "Test Artist",
AlbumTitle: nil,
TrackTitle: nil,
})
if err != nil {
t.Fatalf("seedRequestForMBID: %v", err)
}
}
// doLidarrSearch fires a GET /api/lidarr/search request directly at the handler.
// It simulates the RequireUser middleware by injecting a dummy user context.
func doLidarrSearch(t *testing.T, h *handlers, q, kind string) *httptest.ResponseRecorder {
t.Helper()
url := fmt.Sprintf("/api/lidarr/search?q=%s&kind=%s", q, kind)
req := httptest.NewRequest(http.MethodGet, url, nil)
// Inject a minimal user context the same way other handler tests do it.
// The lidarr search handler doesn't use the user from context, so any
// non-nil user value keeps the handler from panicking.
w := httptest.NewRecorder()
h.handleLidarrSearch(w, req)
return w
}
// TestHandleLidarrSearch_ValidationErrors covers bad kind and missing query
// in a table-driven test.
func TestHandleLidarrSearch_ValidationErrors(t *testing.T) {
h, _ := testHandlers(t)
cases := []struct {
name string
q string
kind string
wantStatus int
wantCode string
}{
{
name: "bad kind playlist",
q: "Beatles",
kind: "playlist",
wantStatus: http.StatusBadRequest,
wantCode: "bad_kind",
},
{
name: "missing query",
q: "",
kind: "artist",
wantStatus: http.StatusBadRequest,
wantCode: "missing_query",
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
url := fmt.Sprintf("/api/lidarr/search?q=%s&kind=%s", tc.q, tc.kind)
req := httptest.NewRequest(http.MethodGet, url, nil)
w := httptest.NewRecorder()
h.handleLidarrSearch(w, req)
if w.Code != tc.wantStatus {
t.Fatalf("status = %d, want %d; body = %s", w.Code, tc.wantStatus, w.Body.String())
}
var resp struct {
Error struct {
Code string `json:"code"`
} `json:"error"`
}
if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
t.Fatalf("decode body: %v; body=%s", err, w.Body.String())
}
if resp.Error.Code != tc.wantCode {
t.Errorf("error.code = %q, want %q", resp.Error.Code, tc.wantCode)
}
})
}
}
// TestHandleLidarrSearch_DisabledReturns503 verifies that a disabled Lidarr
// config causes the handler to return 503 lidarr_disabled.
func TestHandleLidarrSearch_DisabledReturns503(t *testing.T) {
h, _ := testHandlers(t)
resetLidarrState(t, h)
// Config is already disabled after resetLidarrState; ensure it's explicit.
saveLidarrConfig(t, h, "http://127.0.0.1:1", false)
req := httptest.NewRequest(http.MethodGet, "/api/lidarr/search?q=Beatles&kind=artist", nil)
w := httptest.NewRecorder()
h.handleLidarrSearch(w, req)
if w.Code != http.StatusServiceUnavailable {
t.Fatalf("status = %d, want 503; body = %s", w.Code, w.Body.String())
}
var resp struct {
Error struct {
Code string `json:"code"`
} `json:"error"`
}
if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
t.Fatalf("decode body: %v; body=%s", err, w.Body.String())
}
if resp.Error.Code != "lidarr_disabled" {
t.Errorf("error.code = %q, want lidarr_disabled", resp.Error.Code)
}
}
// TestHandleLidarrSearch_HappyPath_RequestableResult checks a result that is
// neither in the library nor already requested.
func TestHandleLidarrSearch_HappyPath_RequestableResult(t *testing.T) {
h, _ := testHandlers(t)
resetLidarrState(t, h)
const mbid = "artist-mbid-abc123"
stub := newLidarrStub(t, lidarrArtistStubBody(mbid, "The Beatles"))
saveLidarrConfig(t, h, stub.URL, true)
w := doLidarrSearch(t, h, "Beatles", "artist")
if w.Code != http.StatusOK {
t.Fatalf("status = %d, want 200; body = %s", w.Code, w.Body.String())
}
var results []lidarrSearchResult
if err := json.Unmarshal(w.Body.Bytes(), &results); err != nil {
t.Fatalf("decode: %v; body=%s", err, w.Body.String())
}
if len(results) != 1 {
t.Fatalf("len(results) = %d, want 1", len(results))
}
r := results[0]
if r.MBID != mbid {
t.Errorf("mbid = %q, want %q", r.MBID, mbid)
}
if r.InLibrary {
t.Errorf("in_library = true, want false")
}
if r.Requested {
t.Errorf("requested = true, want false")
}
}
// TestHandleLidarrSearch_HappyPath_InLibraryResult checks that a result whose
// MBID matches an artist in the local DB has in_library=true.
func TestHandleLidarrSearch_HappyPath_InLibraryResult(t *testing.T) {
h, _ := testHandlers(t)
resetLidarrState(t, h)
const mbid = "artist-mbid-inlib"
stub := newLidarrStub(t, lidarrArtistStubBody(mbid, "Library Artist"))
saveLidarrConfig(t, h, stub.URL, true)
// Seed the artist with the matching MBID so in_library becomes true.
seedArtistWithMBID(t, h, dbtest.TestUserPrefix+"LibraryArtist", mbid)
w := doLidarrSearch(t, h, "Library+Artist", "artist")
if w.Code != http.StatusOK {
t.Fatalf("status = %d, want 200; body = %s", w.Code, w.Body.String())
}
var results []lidarrSearchResult
if err := json.Unmarshal(w.Body.Bytes(), &results); err != nil {
t.Fatalf("decode: %v; body=%s", err, w.Body.String())
}
if len(results) != 1 {
t.Fatalf("len(results) = %d, want 1", len(results))
}
if !results[0].InLibrary {
t.Errorf("in_library = false, want true")
}
}
// TestHandleLidarrSearch_HappyPath_AlreadyRequested verifies that a result
// whose MBID matches a pending lidarr_requests row has requested=true.
func TestHandleLidarrSearch_HappyPath_AlreadyRequested(t *testing.T) {
h, _ := testHandlers(t)
resetLidarrState(t, h)
const mbid = "artist-mbid-requested"
stub := newLidarrStub(t, lidarrArtistStubBody(mbid, "Requested Artist"))
saveLidarrConfig(t, h, stub.URL, true)
// Seed a user and a pending request for the artist MBID.
user := seedUser(t, h.pool, "requester", "pw", false)
seedRequestForMBID(t, h, user.ID, dbq.LidarrRequestKindArtist, mbid, mbid)
w := doLidarrSearch(t, h, "Requested+Artist", "artist")
if w.Code != http.StatusOK {
t.Fatalf("status = %d, want 200; body = %s", w.Code, w.Body.String())
}
var results []lidarrSearchResult
if err := json.Unmarshal(w.Body.Bytes(), &results); err != nil {
t.Fatalf("decode: %v; body=%s", err, w.Body.String())
}
if len(results) != 1 {
t.Fatalf("len(results) = %d, want 1", len(results))
}
if !results[0].Requested {
t.Errorf("requested = false, want true")
}
}
// TestHandleLidarrSearch_LidarrUnreachableReturns503 verifies that a
// connection-refused Lidarr base URL returns 503 lidarr_unreachable.
func TestHandleLidarrSearch_LidarrUnreachableReturns503(t *testing.T) {
h, _ := testHandlers(t)
resetLidarrState(t, h)
// Port 1 is reserved and will refuse connections immediately.
saveLidarrConfig(t, h, "http://127.0.0.1:1", true)
req := httptest.NewRequest(http.MethodGet, "/api/lidarr/search?q=Beatles&kind=artist", nil)
// Use a short-deadline context so the test doesn't hang waiting for TCP.
ctx, cancel := context.WithTimeout(req.Context(), 3*time.Second)
defer cancel()
req = req.WithContext(ctx)
w := httptest.NewRecorder()
h.handleLidarrSearch(w, req)
if w.Code != http.StatusServiceUnavailable {
t.Fatalf("status = %d, want 503; body = %s", w.Code, w.Body.String())
}
var resp struct {
Error struct {
Code string `json:"code"`
} `json:"error"`
}
if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
t.Fatalf("decode: %v; body=%s", err, w.Body.String())
}
if resp.Error.Code != "lidarr_unreachable" {
t.Errorf("error.code = %q, want lidarr_unreachable", resp.Error.Code)
}
}
+13 -49
View File
@@ -1,21 +1,18 @@
package api
import (
"context"
"encoding/json"
"errors"
"io"
"net/http"
"strconv"
"strings"
"sync"
"github.com/go-chi/chi/v5"
"github.com/jackc/pgx/v5/pgtype"
"git.fabledsword.com/bvandeusen/minstrel/internal/apierror"
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
"git.fabledsword.com/bvandeusen/minstrel/internal/lidarr"
"git.fabledsword.com/bvandeusen/minstrel/internal/recommendation"
)
@@ -31,10 +28,10 @@ type suggestionView struct {
// out-of-library artists is permanently partial (#2376), and the card
// falls back to the seed attribution it has always shown.
MatchedTags []string `json:"matched_tags,omitempty"`
// 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 is Minstrel's catalogue art endpoint for the candidate
// (out-of-library artists have no local art row, M502 #5519). It
// answers 404 when no enabled source has an image, and the client
// falls back to a placeholder.
ImageURL string `json:"image_url,omitempty"`
}
@@ -105,7 +102,7 @@ func (h *handlers) handleListSuggestions(w http.ResponseWriter, r *http.Request)
MatchedTags: s.MatchedTags,
})
}
h.resolveSuggestionArt(r.Context(), out)
resolveSuggestionArt(out)
writeJSON(w, http.StatusOK, out)
}
@@ -250,48 +247,15 @@ func (h *handlers) handleListSuggestionSnoozes(w http.ResponseWriter, r *http.Re
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
// resolveSuggestionArt points each suggestion's ImageURL at Minstrel's
// catalogue art endpoint (M502 #5519). Nothing is fetched here: the image is
// looked up from the enabled cover-art sources when the browser asks for it,
// and kept, so listing suggestions makes no outbound call. A candidate with
// no MBID keeps an empty ImageURL and the client renders its placeholder.
func resolveSuggestionArt(views []suggestionView) {
for i := range views {
if views[i].MBID == "" || views[i].Name == "" {
continue
if views[i].MBID != "" {
views[i].ImageURL = artistArtURL(views[i].MBID, views[i].Name)
}
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()
}