Four extractions, each chosen because the two halves have different callers,
different failure modes or different testing needs. Not a line-count exercise.
internal/home out of internal/media
- media serves item RESOURCES (detail, file, cover, progress); home computes
AGGREGATES (the five shelves). The 2328-line package was doing both.
- media keeps POST /items/{id}/view because it is item-scoped and must stay in
the package that authorizes the item; it imports home.RecordView for that one
thing. home does not import media.
- Verified live: all five shelves non-empty, and two consecutive /home calls
return different explore ids, which is the non-determinism the Explore rewrite
exists for.
internal/scanner out of internal/library
- The scanner walks the filesystem and reconciles; library CRUD is an admin HTTP
surface. Different callers (a job vs a request), different failure modes.
- The registry deliberately STAYED in internal/library: three of its four callers
are not the scanner (modules register types, handlers serve the list, metadata
reads the group mode, subsonic calls CleanTitle). It is a property of the kind,
not of the scan. The one genuinely scanner-owned helper became
library.ExtensionsFor.
- Two tests moved with the code rather than being rewritten; they now exercise
the boundary by calling library.ListMissing across it.
- Verified live: first scan updates, rescan reports unchanged=N, so change
detection still short-circuits.
internal/providers out of internal/metadata
- Providers talk to third-party APIs; the enrich job owns the matching policy and
the database writes. The split's whole point is that a provider is testable
WITHOUT a database, so a DB-free test was added and OpenLibrary gained a
BaseURL override to make it possible.
- providers imports zero internal packages — a true leaf.
- The matching policy (searchTitle, artistHint, pickBest) is untouched. It exists
because real false matches were found.
dto packages out of internal/jellyfin and internal/subsonic
- The wire types and their mapping are now separate from the handler logic, so
the contract with clients we do not control is reviewable on its own.
- Both dto packages are PURE: zero references to sql.DB or net/http, verified by
grep. That is what makes them reviewable.
- No field name, tag or error code changed.
internal/arch did its job: it failed the build twice during this phase, once for
each new package that was not yet placed in a layer. That is the rule working,
not an obstacle.
Verified: gofmt clean, build exit 0, vet exit 0, 23 packages pass, svelte-check
0/0, and end to end against a running server — home shelves, a queued scan
reporting unchanged, Jellyfin /Items, and Subsonic ping.
Process note: one agent (dto) stopped mid-task and left the tree red — the dto
packages unlayered and its test still referencing constants that had moved. The
arch test caught it immediately. I finished it by hand: exported constant
references in the test, and both dto packages placed at layer 3.
Still open from Phase 1: internal/jellyfin has no test files, catalog is
item-centric while Subsonic is file-first, and subsonic loadAlbum now scopes to
the item's own library.
328 lines
9.8 KiB
Go
328 lines
9.8 KiB
Go
package metadata
|
|
|
|
import (
|
|
"context"
|
|
"database/sql"
|
|
"errors"
|
|
"io"
|
|
"log/slog"
|
|
"path/filepath"
|
|
"testing"
|
|
|
|
"github.com/mithral/vellum/internal/core"
|
|
"github.com/mithral/vellum/internal/media"
|
|
"github.com/mithral/vellum/internal/providers"
|
|
"github.com/mithral/vellum/migrations"
|
|
)
|
|
|
|
func testDB(t *testing.T) *sql.DB {
|
|
t.Helper()
|
|
db, err := core.OpenDB(filepath.Join(t.TempDir(), "meta.db"))
|
|
if err != nil {
|
|
t.Fatalf("OpenDB: %v", err)
|
|
}
|
|
t.Cleanup(func() { _ = db.Close() })
|
|
if err := core.Migrate(context.Background(), db, migrations.FS, discard()); err != nil {
|
|
t.Fatalf("Migrate: %v", err)
|
|
}
|
|
return db
|
|
}
|
|
|
|
func discard() *slog.Logger { return slog.New(slog.NewTextHandler(io.Discard, nil)) }
|
|
|
|
func seedItem(t *testing.T, db *sql.DB, title string) int64 {
|
|
t.Helper()
|
|
ctx := context.Background()
|
|
var libID int64
|
|
if err := db.QueryRowContext(ctx,
|
|
`INSERT INTO libraries (name, kind, root_path) VALUES ('Ebooks','ebooks','/tmp/e') RETURNING id`).Scan(&libID); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
var itemID int64
|
|
if err := db.QueryRowContext(ctx, `
|
|
INSERT INTO media_items (library_id, kind, path, title, sort_title)
|
|
VALUES (?, 'ebooks', 'book.epub', ?, ?) RETURNING id`,
|
|
libID, title, title).Scan(&itemID); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
// A file row so catalog.Load can resolve a modtime.
|
|
if _, err := db.ExecContext(ctx,
|
|
`INSERT INTO media_files (item_id, path, size, mtime) VALUES (?, 'book.epub', 1, '1')`, itemID); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
return itemID
|
|
}
|
|
|
|
// fakeProvider is a Provider with scripted results.
|
|
type fakeProvider struct {
|
|
name string
|
|
kinds []string
|
|
results []providers.Candidate
|
|
err error
|
|
calls int
|
|
}
|
|
|
|
func (f *fakeProvider) Name() string { return f.name }
|
|
func (f *fakeProvider) Kinds() []string { return f.kinds }
|
|
func (f *fakeProvider) Search(context.Context, providers.Query) ([]providers.Candidate, error) {
|
|
f.calls++
|
|
return f.results, f.err
|
|
}
|
|
|
|
func enricherWith(t *testing.T, db *sql.DB, provs ...providers.Provider) *Enricher {
|
|
t.Helper()
|
|
e := NewEnricher(db, discard(), t.TempDir())
|
|
e.providers = func(string) []providers.Provider { return provs }
|
|
return e
|
|
}
|
|
|
|
func TestEnrichAppliesCandidate(t *testing.T) {
|
|
db := testDB(t)
|
|
itemID := seedItem(t, db, "the hobbit")
|
|
|
|
fake := &fakeProvider{
|
|
name: "fake",
|
|
kinds: []string{"ebooks"},
|
|
results: []providers.Candidate{{
|
|
Provider: "fake",
|
|
ExternalID: "OL123W",
|
|
Title: "The Hobbit",
|
|
Authors: []string{"J.R.R. Tolkien"},
|
|
Year: 1937,
|
|
Publisher: "Allen & Unwin",
|
|
}},
|
|
}
|
|
e := enricherWith(t, db, fake)
|
|
|
|
if err := e.EnrichItem(context.Background(), itemID); err != nil {
|
|
t.Fatalf("EnrichItem: %v", err)
|
|
}
|
|
|
|
ctx := context.Background()
|
|
var title string
|
|
var year sql.NullInt64
|
|
if err := db.QueryRowContext(ctx,
|
|
`SELECT title, year FROM media_items WHERE id = ?`, itemID).Scan(&title, &year); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if title != "The Hobbit" {
|
|
t.Errorf("title = %q, want The Hobbit", title)
|
|
}
|
|
if !year.Valid || year.Int64 != 1937 {
|
|
t.Errorf("year = %v, want 1937", year)
|
|
}
|
|
|
|
var provider, external string
|
|
if err := db.QueryRowContext(ctx,
|
|
`SELECT provider, external_id FROM provider_ids WHERE item_id = ?`, itemID).Scan(&provider, &external); err != nil {
|
|
t.Fatalf("provider_ids not written: %v", err)
|
|
}
|
|
if provider != "fake" || external != "OL123W" {
|
|
t.Errorf("provider_ids = %s/%s", provider, external)
|
|
}
|
|
|
|
var author string
|
|
if err := db.QueryRowContext(ctx, `
|
|
SELECT p.name FROM item_people ip JOIN people p ON p.id = ip.person_id
|
|
WHERE ip.item_id = ? AND ip.role = 'author'`, itemID).Scan(&author); err != nil {
|
|
t.Fatalf("author not linked: %v", err)
|
|
}
|
|
if author != "J.R.R. Tolkien" {
|
|
t.Errorf("author = %q", author)
|
|
}
|
|
|
|
var status string
|
|
if err := db.QueryRowContext(ctx,
|
|
`SELECT status FROM metadata_fetch WHERE item_id = ? AND provider = 'fake'`, itemID).Scan(&status); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if status != "ok" {
|
|
t.Errorf("fetch status = %q, want ok", status)
|
|
}
|
|
}
|
|
|
|
func TestEnrichFallsBackToSecondProvider(t *testing.T) {
|
|
db := testDB(t)
|
|
itemID := seedItem(t, db, "some book")
|
|
|
|
empty := &fakeProvider{name: "empty", kinds: []string{"ebooks"}}
|
|
good := &fakeProvider{
|
|
name: "good",
|
|
kinds: []string{"ebooks"},
|
|
results: []providers.Candidate{{Provider: "good", ExternalID: "g1", Title: "Some Book"}},
|
|
}
|
|
e := enricherWith(t, db, empty, good)
|
|
|
|
if err := e.EnrichItem(context.Background(), itemID); err != nil {
|
|
t.Fatalf("EnrichItem: %v", err)
|
|
}
|
|
if empty.calls != 1 || good.calls != 1 {
|
|
t.Errorf("calls: empty=%d good=%d, want 1/1", empty.calls, good.calls)
|
|
}
|
|
|
|
var provider string
|
|
if err := db.QueryRow(
|
|
`SELECT provider FROM provider_ids WHERE item_id = ?`, itemID).Scan(&provider); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if provider != "good" {
|
|
t.Errorf("matched provider = %q, want good", provider)
|
|
}
|
|
}
|
|
|
|
func TestEnrichContinuesAfterProviderError(t *testing.T) {
|
|
db := testDB(t)
|
|
itemID := seedItem(t, db, "some book")
|
|
|
|
broken := &fakeProvider{name: "broken", kinds: []string{"ebooks"}, err: errors.New("network down")}
|
|
good := &fakeProvider{
|
|
name: "good",
|
|
kinds: []string{"ebooks"},
|
|
results: []providers.Candidate{{Provider: "good", ExternalID: "g1", Title: "Some Book"}},
|
|
}
|
|
e := enricherWith(t, db, broken, good)
|
|
|
|
if err := e.EnrichItem(context.Background(), itemID); err != nil {
|
|
t.Fatalf("a provider error must not fail the item: %v", err)
|
|
}
|
|
if good.calls != 1 {
|
|
t.Error("fallback was not consulted after an error")
|
|
}
|
|
|
|
var status string
|
|
if err := db.QueryRow(
|
|
`SELECT status FROM metadata_fetch WHERE item_id = ? AND provider = 'broken'`, itemID).Scan(&status); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if status != "error" {
|
|
t.Errorf("error status = %q, want error", status)
|
|
}
|
|
}
|
|
|
|
func TestEnrichSkipsAlreadyMatched(t *testing.T) {
|
|
db := testDB(t)
|
|
itemID := seedItem(t, db, "book")
|
|
|
|
fake := &fakeProvider{name: "fake", kinds: []string{"ebooks"},
|
|
results: []providers.Candidate{{Provider: "fake", ExternalID: "x", Title: "Book"}}}
|
|
e := enricherWith(t, db, fake)
|
|
|
|
if err := e.EnrichItem(context.Background(), itemID); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := e.EnrichItem(context.Background(), itemID); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if fake.calls != 1 {
|
|
t.Errorf("provider called %d times, want 1 (second run must skip)", fake.calls)
|
|
}
|
|
}
|
|
|
|
func TestEnrichDoesNotRetryRecentNotFound(t *testing.T) {
|
|
db := testDB(t)
|
|
itemID := seedItem(t, db, "unmatchable")
|
|
|
|
empty := &fakeProvider{name: "empty", kinds: []string{"ebooks"}}
|
|
e := enricherWith(t, db, empty)
|
|
|
|
if err := e.EnrichItem(context.Background(), itemID); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := e.EnrichItem(context.Background(), itemID); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if empty.calls != 1 {
|
|
t.Errorf("provider called %d times, want 1 (notfound is cached)", empty.calls)
|
|
}
|
|
}
|
|
|
|
func TestEnrichIgnoresUnknownKind(t *testing.T) {
|
|
db := testDB(t)
|
|
itemID := seedItem(t, db, "book")
|
|
e := enricherWith(t, db) // no providers at all
|
|
|
|
if err := e.EnrichItem(context.Background(), itemID); err != nil {
|
|
t.Errorf("EnrichItem with no providers errored: %v", err)
|
|
}
|
|
}
|
|
|
|
func TestNormalizeTitle(t *testing.T) {
|
|
cases := map[string]string{
|
|
"Random Access Memories": "random access memories",
|
|
"The Hobbit, or There and Back Again": "the hobbit or there and back again",
|
|
"AC/DC — Back in Black!": "ac dc back in black",
|
|
" spaced out ": "spaced out",
|
|
}
|
|
for in, want := range cases {
|
|
if got := normalizeTitle(in); got != want {
|
|
t.Errorf("normalizeTitle(%q) = %q, want %q", in, got, want)
|
|
}
|
|
}
|
|
}
|
|
|
|
func TestSearchTitleStripsReleaseTags(t *testing.T) {
|
|
cases := map[string]string{
|
|
"Sonic the Hedgehog (USA)": "Sonic the Hedgehog",
|
|
"Super Mario Bros. (World)": "Super Mario Bros.",
|
|
"Game [!]": "Game",
|
|
"Pokemon (USA) (Rev 1)": "Pokemon",
|
|
"The Legend of Zelda": "The Legend of Zelda",
|
|
"Chrono Trigger (USA) [En,Fr]": "Chrono Trigger",
|
|
"(Just Brackets)": "(Just Brackets)",
|
|
}
|
|
for in, want := range cases {
|
|
if got := searchTitle(in); got != want {
|
|
t.Errorf("searchTitle(%q) = %q, want %q", in, got, want)
|
|
}
|
|
}
|
|
}
|
|
|
|
func TestPickBestRejectsUnrelated(t *testing.T) {
|
|
candidates := []providers.Candidate{
|
|
{Title: "Random Access Memories", ExternalID: "right"},
|
|
}
|
|
|
|
// The exact title is accepted.
|
|
if c, ok := pickBest(candidates, "Random Access Memories"); !ok || c.ExternalID != "right" {
|
|
t.Errorf("exact match rejected: ok=%v c=%+v", ok, c)
|
|
}
|
|
|
|
// Containment in either direction is accepted.
|
|
long := []providers.Candidate{{Title: "The Hobbit, or There and Back Again"}}
|
|
if _, ok := pickBest(long, "The Hobbit"); !ok {
|
|
t.Error("containment should be accepted")
|
|
}
|
|
|
|
// A same-named release by a different artist is still accepted by title —
|
|
// the artist hint, not the title check, is what disambiguates that.
|
|
// But a genuinely unrelated title must be rejected.
|
|
if _, ok := pickBest(candidates, "Franz Kirmann"); ok {
|
|
t.Error("unrelated title should be rejected")
|
|
}
|
|
|
|
// Too-short titles are never matched, to avoid substring false positives.
|
|
if _, ok := pickBest(candidates, "a"); ok {
|
|
t.Error("a one-character query should be rejected")
|
|
}
|
|
}
|
|
|
|
func TestArtistHint(t *testing.T) {
|
|
// Directory-grouped kinds take the artist from the parent directory.
|
|
album := media.Item{Kind: "music", Path: "Daft Punk/Random Access Memories"}
|
|
if got := artistHint(album); got != "Daft Punk" {
|
|
t.Errorf("artistHint(album) = %q, want Daft Punk", got)
|
|
}
|
|
|
|
// A loose root-level file has no artist to infer.
|
|
loose := media.Item{Kind: "music", Path: "."}
|
|
if got := artistHint(loose); got != "" {
|
|
t.Errorf("artistHint(loose) = %q, want empty", got)
|
|
}
|
|
|
|
// File-grouped kinds (ebooks) never take a hint: their parent directory is
|
|
// usually a series, not an author.
|
|
book := media.Item{Kind: "ebooks", Path: "Series/Book.epub"}
|
|
if got := artistHint(book); got != "" {
|
|
t.Errorf("artistHint(ebook) = %q, want empty", got)
|
|
}
|
|
}
|