EXPAND ONLY, per the migration order (expand, migrate, verify, contract). No
Postgres implementation, no behaviour change.
internal/catalog now exposes:
type Store interface {
Item / ItemFor / Files / FilesFor / FileByID / FirstFile / FileCount
}
type SQL struct{ db *sql.DB }; func New(db *sql.DB) *SQL
The seven bodies are the old functions moved verbatim. ItemFor still returns the
same ErrNotFound for absent and forbidden, so the security property is
unchanged. A compile-time `var _ Store = (*SQL)(nil)` keeps the implementation
honest.
Deliberately NOT moved: browse lists, home shelves and subsonic album queries.
Those are each package's own presentation concern, and pulling all of them into
the seam is exactly the 186-statement rewrite the seam exists to avoid. The
package doc says so.
Proven substitutable, not just declared: store_test.go implements a fake Store
in memory with no database at all and drives a caller through the interface. A
seam nobody can substitute proves nothing.
The agent's own risk note was right and I acted on it: the seam only pays off
once something actually threads it, and initially nothing did. So internal/
playback — the smallest consumer — now takes a catalog.Store instead of *sql.DB
and is wired with catalog.New(db) at the composition point. It has no database
reference left outside a comment. The remaining consumers (media, jellyfin,
subsonic, metadata) still call the package-level wrappers; threading them is
mechanical, and I would rather do it deliberately than in one sweep.
Verified: gofmt clean, build exit 0, vet exit 0, 25 packages pass, arch rules
pass. End to end against a running server: /playback/resolve still returns
vcodec:hevc for html5 and engine mpv when mpv is offered, an unknown item 404s,
/home and Jellyfin /Items still answer.
Known gap, unchanged by this commit: internal/arch does not enforce "new code
must not take *sql.DB", so the seam can erode by drift. That rule would be the
natural next step if this is to hold.
208 lines
6.3 KiB
Go
208 lines
6.3 KiB
Go
package catalog
|
|
|
|
import (
|
|
"context"
|
|
"errors"
|
|
"testing"
|
|
"time"
|
|
|
|
"github.com/mithral/vellum/internal/auth"
|
|
)
|
|
|
|
// This file exists to keep the Store seam honest. An interface that no one can
|
|
// substitute is decoration, so fakeStore implements Store entirely in memory —
|
|
// no database, no SQL — and the test drives a caller through it. If a method is
|
|
// added to Store later, this file stops compiling, which is the intended alarm.
|
|
//
|
|
// What is NOT tested here: the query text. That is the SQL implementation's job
|
|
// and catalog_test.go covers it against a real database.
|
|
|
|
// fakeStore is an in-memory Store. Its authorization model mirrors auth's: an
|
|
// admin sees everything, everyone else sees only granted libraries.
|
|
type fakeStore struct {
|
|
items map[int64]Item
|
|
files map[int64][]File
|
|
granted map[int64][]int64 // user id -> library ids
|
|
admins map[int64]bool
|
|
|
|
// calls records which methods ran, so the test can prove the caller went
|
|
// through the interface rather than reaching for a database.
|
|
calls []string
|
|
}
|
|
|
|
var _ Store = (*fakeStore)(nil)
|
|
|
|
func newFakeStore() *fakeStore {
|
|
return &fakeStore{
|
|
items: map[int64]Item{},
|
|
files: map[int64][]File{},
|
|
granted: map[int64][]int64{},
|
|
admins: map[int64]bool{},
|
|
}
|
|
}
|
|
|
|
func (f *fakeStore) allowed(u auth.User, libID int64) bool {
|
|
if f.admins[u.ID] {
|
|
return true
|
|
}
|
|
for _, id := range f.granted[u.ID] {
|
|
if id == libID {
|
|
return true
|
|
}
|
|
}
|
|
return false
|
|
}
|
|
|
|
func (f *fakeStore) Item(_ context.Context, id int64) (Item, error) {
|
|
f.calls = append(f.calls, "Item")
|
|
it, ok := f.items[id]
|
|
if !ok {
|
|
return Item{}, ErrNotFound
|
|
}
|
|
return it, nil
|
|
}
|
|
|
|
func (f *fakeStore) ItemFor(_ context.Context, u auth.User, id int64) (Item, error) {
|
|
f.calls = append(f.calls, "ItemFor")
|
|
it, ok := f.items[id]
|
|
if !ok || !f.allowed(u, it.LibraryID) {
|
|
// The whole point: absent and forbidden are the same error.
|
|
return Item{}, ErrNotFound
|
|
}
|
|
return it, nil
|
|
}
|
|
|
|
func (f *fakeStore) Files(_ context.Context, itemID int64) ([]File, error) {
|
|
f.calls = append(f.calls, "Files")
|
|
return f.files[itemID], nil
|
|
}
|
|
|
|
func (f *fakeStore) FilesFor(ctx context.Context, u auth.User, itemID int64) ([]File, error) {
|
|
f.calls = append(f.calls, "FilesFor")
|
|
if _, err := f.ItemFor(ctx, u, itemID); err != nil {
|
|
return nil, err
|
|
}
|
|
return f.Files(ctx, itemID)
|
|
}
|
|
|
|
func (f *fakeStore) FileByID(_ context.Context, itemID, fileID int64) (File, error) {
|
|
f.calls = append(f.calls, "FileByID")
|
|
for _, fl := range f.files[itemID] { // scoped to the item, like the SQL
|
|
if fl.ID == fileID {
|
|
return fl, nil
|
|
}
|
|
}
|
|
return File{}, ErrNotFound
|
|
}
|
|
|
|
func (f *fakeStore) FirstFile(_ context.Context, itemID int64) (File, error) {
|
|
f.calls = append(f.calls, "FirstFile")
|
|
fs := f.files[itemID]
|
|
if len(fs) == 0 {
|
|
return File{}, ErrNotFound
|
|
}
|
|
return fs[0], nil
|
|
}
|
|
|
|
func (f *fakeStore) FileCount(_ context.Context, itemID int64) (int, error) {
|
|
f.calls = append(f.calls, "FileCount")
|
|
return len(f.files[itemID]), nil
|
|
}
|
|
|
|
// serveItem is a stand-in for a real caller (media.playback, jellyfin.playback)
|
|
// written against the Store interface instead of *sql.DB. It is here because
|
|
// that is what the seam is FOR: this function cannot be told it needs SQLite.
|
|
func serveItem(ctx context.Context, s Store, u auth.User, id int64) (Item, File, error) {
|
|
it, err := s.ItemFor(ctx, u, id)
|
|
if err != nil {
|
|
return Item{}, File{}, err
|
|
}
|
|
f, err := s.FirstFile(ctx, id)
|
|
if err != nil {
|
|
return Item{}, File{}, err
|
|
}
|
|
return it, f, nil
|
|
}
|
|
|
|
func fixture() (*fakeStore, auth.User, auth.User, int64) {
|
|
s := newFakeStore()
|
|
const libID = 42
|
|
s.items[7] = Item{ID: 7, LibraryID: libID, Kind: "ebooks", Path: "Thing.epub",
|
|
Title: "Thing", Year: 1999, RootPath: "/media/A",
|
|
ModTime: time.Unix(1700000000, 0).UTC()}
|
|
s.files[7] = []File{{ID: 700, Path: "Thing.epub", Size: 100, Container: "mp4"}}
|
|
s.files[8] = []File{{ID: 800, Path: "Other.epub", Size: 100}}
|
|
|
|
granted := auth.User{ID: 1, Username: "granted"}
|
|
outsider := auth.User{ID: 2, Username: "outsider"}
|
|
s.granted[granted.ID] = []int64{libID}
|
|
return s, granted, outsider, 7
|
|
}
|
|
|
|
func TestStoreIsSubstitutableWithoutADatabase(t *testing.T) {
|
|
s, granted, _, itemID := fixture()
|
|
it, f, err := serveItem(context.Background(), s, granted, itemID)
|
|
if err != nil {
|
|
t.Fatalf("in-memory Store rejected a granted user: %v", err)
|
|
}
|
|
if it.Title != "Thing" || it.AbsPath() != "/media/A/Thing.epub" {
|
|
t.Errorf("item lost in translation: %+v", it)
|
|
}
|
|
if f.ID != 700 {
|
|
t.Errorf("FirstFile = %d, want 700", f.ID)
|
|
}
|
|
if want := []string{"ItemFor", "FirstFile"}; len(s.calls) != 2 ||
|
|
s.calls[0] != want[0] || s.calls[1] != want[1] {
|
|
t.Errorf("calls = %v, want %v", s.calls, want)
|
|
}
|
|
}
|
|
|
|
// The security property has to survive the seam: a caller that could tell a
|
|
// forbidden item from an absent one would leak another user's library.
|
|
func TestStoreKeepsForbiddenIndistinguishableFromAbsent(t *testing.T) {
|
|
s, _, outsider, itemID := fixture()
|
|
ctx := context.Background()
|
|
|
|
_, _, ferr := serveItem(ctx, s, outsider, itemID)
|
|
_, _, aerr := serveItem(ctx, s, outsider, itemID+9999)
|
|
if !errors.Is(ferr, ErrNotFound) || !errors.Is(aerr, ErrNotFound) {
|
|
t.Fatalf("errors = %v / %v, want ErrNotFound for both", ferr, aerr)
|
|
}
|
|
if ferr.Error() != aerr.Error() {
|
|
t.Errorf("errors differ and would leak existence: %q vs %q", ferr, aerr)
|
|
}
|
|
}
|
|
|
|
func TestStoreAdminSeesEverythingWithoutAGrant(t *testing.T) {
|
|
s, _, _, itemID := fixture()
|
|
admin := auth.User{ID: 3, Username: "root", IsAdmin: true}
|
|
s.admins[admin.ID] = true
|
|
|
|
if _, err := s.ItemFor(context.Background(), admin, itemID); err != nil {
|
|
t.Errorf("admin denied: %v", err)
|
|
}
|
|
if _, err := s.FilesFor(context.Background(), admin, itemID); err != nil {
|
|
t.Errorf("admin FilesFor denied: %v", err)
|
|
}
|
|
if n, err := s.FileCount(context.Background(), itemID); err != nil || n != 1 {
|
|
t.Errorf("FileCount = %d, %v; want 1, nil", n, err)
|
|
}
|
|
}
|
|
|
|
// FileByID must stay scoped to its item through the interface too, or the
|
|
// in-memory Store would be laxer than the SQL one it stands in for.
|
|
func TestStoreFileByIDIsScopedToItsItem(t *testing.T) {
|
|
s, _, _, itemID := fixture()
|
|
ctx := context.Background()
|
|
|
|
other, err := s.FirstFile(ctx, itemID+1)
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if _, err := s.FileByID(ctx, itemID, other.ID); !errors.Is(err, ErrNotFound) {
|
|
t.Errorf("file id leaked across items: %v", err)
|
|
}
|
|
if _, err := s.FileByID(ctx, itemID+1, other.ID); err != nil {
|
|
t.Errorf("legitimate lookup failed: %v", err)
|
|
}
|
|
}
|