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.
251 lines
6.5 KiB
Go
251 lines
6.5 KiB
Go
package home
|
|
|
|
import (
|
|
"context"
|
|
"database/sql"
|
|
"testing"
|
|
)
|
|
|
|
type fixture struct {
|
|
db *sql.DB
|
|
libA int64
|
|
libB int64
|
|
userA int64
|
|
userB int64
|
|
itemA int64 // in libA
|
|
itemA2 int64 // in libA, unstarted
|
|
itemB int64 // in libB
|
|
}
|
|
|
|
func setup(t *testing.T) fixture {
|
|
t.Helper()
|
|
db := testDB(t)
|
|
ctx := context.Background()
|
|
f := fixture{db: db}
|
|
|
|
mk := func(q string, args ...any) int64 {
|
|
var id int64
|
|
if err := db.QueryRowContext(ctx, q, args...).Scan(&id); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
return id
|
|
}
|
|
|
|
f.libA = mk(`INSERT INTO libraries (name, kind, root_path) VALUES ('A','ebooks','/a') RETURNING id`)
|
|
f.libB = mk(`INSERT INTO libraries (name, kind, root_path) VALUES ('B','ebooks','/b') RETURNING id`)
|
|
f.userA = mk(`INSERT INTO users (username, password_hash) VALUES ('a','x') RETURNING id`)
|
|
f.userB = mk(`INSERT INTO users (username, password_hash) VALUES ('b','x') RETURNING id`)
|
|
|
|
f.itemA = mk(`INSERT INTO media_items (library_id, kind, path, title) VALUES (?, 'ebooks','a1.epub','A One') RETURNING id`, f.libA)
|
|
f.itemA2 = mk(`INSERT INTO media_items (library_id, kind, path, title) VALUES (?, 'ebooks','a2.epub','A Two') RETURNING id`, f.libA)
|
|
f.itemB = mk(`INSERT INTO media_items (library_id, kind, path, title) VALUES (?, 'ebooks','b1.epub','B One') RETURNING id`, f.libB)
|
|
|
|
// A file for each item, so progress rows can reference one. media_files.path
|
|
// is globally unique, so each gets its own.
|
|
for i, it := range []int64{f.itemA, f.itemA2, f.itemB} {
|
|
if _, err := db.ExecContext(ctx,
|
|
`INSERT INTO media_files (item_id, path, size) VALUES (?, ?, 1)`, it, "file-"+itoa(int64(i))); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
}
|
|
return f
|
|
}
|
|
|
|
func itoa(n int64) string {
|
|
if n == 0 {
|
|
return "0"
|
|
}
|
|
var b [20]byte
|
|
i := len(b)
|
|
for n > 0 {
|
|
i--
|
|
b[i] = byte('0' + n%10)
|
|
n /= 10
|
|
}
|
|
return string(b[i:])
|
|
}
|
|
|
|
func startItem(t *testing.T, f fixture, userID, itemID int64) {
|
|
t.Helper()
|
|
var fileID int64
|
|
if err := f.db.QueryRow(`SELECT id FROM media_files WHERE item_id = ? LIMIT 1`, itemID).Scan(&fileID); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if _, err := f.db.Exec(`
|
|
INSERT INTO progress (user_id, item_id, file_id, kind, position, percent, finished)
|
|
VALUES (?, ?, ?, 'ebooks', 1, 0.5, 0)`, userID, itemID, fileID); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
}
|
|
|
|
func ids(items []shelfItem) []int64 {
|
|
out := make([]int64, len(items))
|
|
for i, it := range items {
|
|
out[i] = it.ID
|
|
}
|
|
return out
|
|
}
|
|
|
|
func contains64(hay []int64, needle int64) bool {
|
|
for _, h := range hay {
|
|
if h == needle {
|
|
return true
|
|
}
|
|
}
|
|
return false
|
|
}
|
|
|
|
func TestMostViewedIsPerUser(t *testing.T) {
|
|
f := setup(t)
|
|
ctx := context.Background()
|
|
|
|
for i := 0; i < 3; i++ {
|
|
if _, err := RecordView(ctx, f.db, f.userA, f.itemA); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
}
|
|
if _, err := RecordView(ctx, f.db, f.userA, f.itemA2); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
|
|
got, err := MostViewed(ctx, f.db, f.userA, []int64{f.libA}, 10)
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if len(got) != 2 {
|
|
t.Fatalf("got %d, want 2: %v", len(got), ids(got))
|
|
}
|
|
if got[0].ID != f.itemA || got[0].Views != 3 {
|
|
t.Errorf("first = id %d with %d views, want id %d with 3", got[0].ID, got[0].Views, f.itemA)
|
|
}
|
|
|
|
// User B has viewed nothing.
|
|
other, err := MostViewed(ctx, f.db, f.userB, []int64{f.libA}, 10)
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if len(other) != 0 {
|
|
t.Errorf("user B sees %v, want nothing", ids(other))
|
|
}
|
|
}
|
|
|
|
// TestScopingRespectsLibraryAndUser guards the parameter order in the
|
|
// suggestions and explore queries, where the IN list precedes the user id.
|
|
// Getting it wrong silently swaps a user id for a library id.
|
|
func TestScopingRespectsLibraryAndUser(t *testing.T) {
|
|
f := setup(t)
|
|
ctx := context.Background()
|
|
|
|
// User A started itemA, which lives in libA.
|
|
startItem(t, f, f.userA, f.itemA)
|
|
|
|
// Suggestions: only libA, only unstarted.
|
|
sugg, err := Suggestions(ctx, f.db, f.userA, []int64{f.libA}, 10)
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
for _, s := range sugg {
|
|
if s.LibraryID != f.libA {
|
|
t.Errorf("suggestion %d is from library %d, want only %d", s.ID, s.LibraryID, f.libA)
|
|
}
|
|
if s.ID == f.itemA {
|
|
t.Error("a started item was suggested")
|
|
}
|
|
}
|
|
if len(sugg) == 0 {
|
|
t.Fatal("no suggestions at all")
|
|
}
|
|
|
|
// Explore: same scoping, and it must not include the in-progress item.
|
|
explore, err := Explore(ctx, f.db, f.userA, []int64{f.libA}, 10)
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if len(explore) == 0 {
|
|
t.Fatal("explore returned nothing")
|
|
}
|
|
for _, e := range explore {
|
|
if e.LibraryID != f.libA {
|
|
t.Errorf("explore item %d is from library %d, want only %d", e.ID, e.LibraryID, f.libA)
|
|
}
|
|
if e.ID == f.itemA {
|
|
t.Error("explore included an in-progress item")
|
|
}
|
|
}
|
|
|
|
// User B, restricted to libB, must not see libA content at all.
|
|
suggB, err := Suggestions(ctx, f.db, f.userB, []int64{f.libB}, 10)
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
for _, s := range suggB {
|
|
if s.LibraryID != f.libB {
|
|
t.Errorf("user B suggestion %d leaked from library %d", s.ID, s.LibraryID)
|
|
}
|
|
}
|
|
if contains64(func() []int64 {
|
|
out := make([]int64, len(suggB))
|
|
for i, s := range suggB {
|
|
out[i] = s.ID
|
|
}
|
|
return out
|
|
}(), f.itemA) {
|
|
t.Error("user B was suggested an item from a library they cannot see")
|
|
}
|
|
}
|
|
|
|
func TestSuggestionsExplainThemselves(t *testing.T) {
|
|
f := setup(t)
|
|
ctx := context.Background()
|
|
|
|
// Give user A an author relationship on the started item.
|
|
if _, err := f.db.Exec(`
|
|
INSERT INTO people (name) VALUES ('Ursula Le Guin')`); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
var personID int64
|
|
if err := f.db.QueryRow(`SELECT id FROM people WHERE name = 'Ursula Le Guin'`).Scan(&personID); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if _, err := f.db.Exec(
|
|
`INSERT INTO item_people (item_id, person_id, role, ord) VALUES (?, ?, 'author', 0)`,
|
|
f.itemA, personID); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
// And on the candidate.
|
|
if _, err := f.db.Exec(
|
|
`INSERT INTO item_people (item_id, person_id, role, ord) VALUES (?, ?, 'author', 0)`,
|
|
f.itemA2, personID); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
startItem(t, f, f.userA, f.itemA)
|
|
|
|
sugg, err := Suggestions(ctx, f.db, f.userA, []int64{f.libA}, 10)
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if len(sugg) == 0 {
|
|
t.Fatal("no suggestions")
|
|
}
|
|
if sugg[0].ID != f.itemA2 {
|
|
t.Errorf("first suggestion = %d, want the same-author item %d", sugg[0].ID, f.itemA2)
|
|
}
|
|
if sugg[0].Reason != "More by Ursula Le Guin" {
|
|
t.Errorf("reason = %q", sugg[0].Reason)
|
|
}
|
|
}
|
|
|
|
func TestRecordViewCounts(t *testing.T) {
|
|
f := setup(t)
|
|
ctx := context.Background()
|
|
|
|
for i := 1; i <= 4; i++ {
|
|
n, err := RecordView(ctx, f.db, f.userA, f.itemA)
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if n != i {
|
|
t.Errorf("view %d returned count %d", i, n)
|
|
}
|
|
}
|
|
}
|