Phase 3 built the seam; nothing stopped it being undone one convenient import at
a time. Two rules, both proven able to fail.
TestDatabaseSQLImportersAreDeclared freezes the blast radius of database/sql.
Twenty packages import it today and that set is now explicit. A new package that
needs its own SQL must be added deliberately, with a reason. The rule also fails
in the other direction — if a listed package stops importing database/sql, the
list must shrink, so it cannot rot into a meaningless allowlist.
TestThreadedConsumersStayOffSQL is the concrete regression guard: internal/
playback was moved onto catalog.Store, and this fails if it reaches for *sql.DB
again.
Both checks read non-test files only, so a package that needs database/sql
purely to build a test database is not forced onto the list.
Proven by breaking each on purpose:
- dropping internal/media from the list -> "imports database/sql but is not
in sqlImporters"
- adding internal/natsort (no SQL) -> "is listed ... but no longer
imports database/sql"
- re-adding database/sql to playback -> "undoes the seam"
All restored afterwards and the files verified unchanged.
Seven arch rules now, each able to fail. gofmt clean, build 0, vet 0, 25
packages pass.
360 lines
11 KiB
Go
360 lines
11 KiB
Go
// Package arch holds no code. It exists so that the import rules this project
|
|
// depends on are enforced by the test suite instead of by good intentions.
|
|
//
|
|
// A layering rule that is only written down in a comment is a rule that will be
|
|
// broken within a month, and the breakage is invisible: an upward import does
|
|
// not fail to compile, it just quietly welds two layers together until nothing
|
|
// can be moved. These tests fail loudly instead.
|
|
package arch
|
|
|
|
import (
|
|
"go/parser"
|
|
"go/token"
|
|
"os"
|
|
"path/filepath"
|
|
"sort"
|
|
"strings"
|
|
"testing"
|
|
)
|
|
|
|
const modulePath = "github.com/mithral/vellum/"
|
|
|
|
// Layers, lowest first. A package may import from a LOWER layer or its own
|
|
// layer, never from a higher one. Peers are allowed because they genuinely
|
|
// collaborate (metadata enriches items that media serves); upward imports are
|
|
// not, because they invert the dependency and make the lower layer unusable on
|
|
// its own.
|
|
var layers = map[string]int{
|
|
// 0 — foundation. No knowledge of the application.
|
|
"internal/core": 0,
|
|
"internal/httpx": 0,
|
|
"internal/natsort": 0,
|
|
"internal/arch": 0,
|
|
"migrations": 0,
|
|
|
|
// 1 — primitives the application is built from.
|
|
"internal/auth": 1,
|
|
"internal/jobs": 1,
|
|
"internal/stream": 1,
|
|
"internal/progress": 1,
|
|
"internal/probe": 1,
|
|
"internal/engine": 1,
|
|
"internal/apitoken": 1,
|
|
|
|
// 2 — leaves that the application is built from: the data seam, and the
|
|
// external catalogues. providers sits here so it stays a leaf — a
|
|
// metadata client must be usable from a test with no database.
|
|
"internal/catalog": 2,
|
|
"internal/providers": 2,
|
|
|
|
// 3 — the application.
|
|
"internal/library": 3,
|
|
"internal/scanner": 3,
|
|
"internal/media": 3,
|
|
"internal/home": 3,
|
|
"internal/search": 3,
|
|
"internal/metadata": 3,
|
|
"internal/playback": 3,
|
|
"internal/modules/comics": 3,
|
|
"internal/modules/ebooks": 3,
|
|
"internal/modules/games": 3,
|
|
"internal/modules/photos": 3,
|
|
|
|
// 4 — adapters: HTTP surfaces and foreign protocols.
|
|
"internal/httpapi": 4,
|
|
"internal/jellyfin": 4,
|
|
"internal/jellyfin/dto": 3,
|
|
"internal/subsonic": 4,
|
|
"internal/subsonic/dto": 3,
|
|
"internal/cli": 4,
|
|
|
|
// 5 — the only place everything is wired together.
|
|
"cmd/vellum": 5,
|
|
}
|
|
|
|
// repoRoot walks up until it finds go.mod, so the test works regardless of
|
|
// where `go test` is invoked from.
|
|
func repoRoot(t *testing.T) string {
|
|
t.Helper()
|
|
dir, err := os.Getwd()
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
for {
|
|
if _, err := os.Stat(filepath.Join(dir, "go.mod")); err == nil {
|
|
return dir
|
|
}
|
|
parent := filepath.Dir(dir)
|
|
if parent == dir {
|
|
t.Fatal("could not find go.mod above the test directory")
|
|
}
|
|
dir = parent
|
|
}
|
|
}
|
|
|
|
// importsOf parses a directory and returns the internal packages it imports.
|
|
func importsOf(t *testing.T, dir string) []string {
|
|
t.Helper()
|
|
entries, err := os.ReadDir(dir)
|
|
if err != nil {
|
|
t.Fatalf("read %s: %v", dir, err)
|
|
}
|
|
seen := map[string]bool{}
|
|
for _, e := range entries {
|
|
if e.IsDir() || !strings.HasSuffix(e.Name(), ".go") || strings.HasSuffix(e.Name(), "_test.go") {
|
|
continue
|
|
}
|
|
fset := token.NewFileSet()
|
|
f, err := parser.ParseFile(fset, filepath.Join(dir, e.Name()), nil, parser.ImportsOnly)
|
|
if err != nil {
|
|
t.Fatalf("parse %s: %v", e.Name(), err)
|
|
}
|
|
for _, imp := range f.Imports {
|
|
p := strings.Trim(imp.Path.Value, `"`)
|
|
if strings.HasPrefix(p, modulePath) {
|
|
seen[strings.TrimPrefix(p, modulePath)] = true
|
|
}
|
|
}
|
|
}
|
|
out := make([]string, 0, len(seen))
|
|
for p := range seen {
|
|
out = append(out, p)
|
|
}
|
|
sort.Strings(out)
|
|
return out
|
|
}
|
|
|
|
// TestNoUpwardImports is the rule that keeps the layers separable.
|
|
func TestNoUpwardImports(t *testing.T) {
|
|
root := repoRoot(t)
|
|
for pkg, layer := range layers {
|
|
dir := filepath.Join(root, filepath.FromSlash(pkg))
|
|
if _, err := os.Stat(dir); err != nil {
|
|
continue // package not present (e.g. a module was removed)
|
|
}
|
|
for _, dep := range importsOf(t, dir) {
|
|
depLayer, known := layers[dep]
|
|
if !known {
|
|
continue // a package outside the map; TestEveryPackageIsLayered catches it
|
|
}
|
|
if depLayer > layer {
|
|
t.Errorf("%s (layer %d) imports %s (layer %d): upward dependency",
|
|
pkg, layer, dep, depLayer)
|
|
}
|
|
}
|
|
}
|
|
}
|
|
|
|
// TestCoreStaysPure is the module rule: core may not know about any feature, or
|
|
// it stops being the thing everything else can depend on.
|
|
func TestCoreStaysPure(t *testing.T) {
|
|
root := repoRoot(t)
|
|
for _, dep := range importsOf(t, filepath.Join(root, "internal", "core")) {
|
|
if dep != "migrations" {
|
|
t.Errorf("internal/core imports %s; core may depend only on migrations", dep)
|
|
}
|
|
}
|
|
}
|
|
|
|
// TestCatalogHasNoHTTP keeps the data seam a data seam. If it grows an
|
|
// http.ResponseWriter it can no longer be reused by a job, a CLI or a test
|
|
// without dragging a request in.
|
|
func TestCatalogHasNoHTTP(t *testing.T) {
|
|
root := repoRoot(t)
|
|
for _, dep := range importsOf(t, filepath.Join(root, "internal", "catalog")) {
|
|
if dep == "internal/httpx" || dep == "internal/httpapi" {
|
|
t.Errorf("internal/catalog imports %s; the data seam must not know about HTTP", dep)
|
|
}
|
|
}
|
|
}
|
|
|
|
// TestModulesDoNotImportEachOther keeps modules independently removable. A
|
|
// module that imports a sibling cannot be dropped without dropping both.
|
|
func TestModulesDoNotImportEachOther(t *testing.T) {
|
|
root := repoRoot(t)
|
|
modsDir := filepath.Join(root, "internal", "modules")
|
|
mods, err := os.ReadDir(modsDir)
|
|
if err != nil {
|
|
t.Skip("no modules directory")
|
|
}
|
|
for _, m := range mods {
|
|
if !m.IsDir() {
|
|
continue
|
|
}
|
|
self := "internal/modules/" + m.Name()
|
|
for _, dep := range importsOf(t, filepath.Join(modsDir, m.Name())) {
|
|
if strings.HasPrefix(dep, "internal/modules/") && dep != self {
|
|
t.Errorf("%s imports %s; modules must not depend on each other", self, dep)
|
|
}
|
|
}
|
|
}
|
|
}
|
|
|
|
// TestEveryPackageIsLayered stops the map above from silently going stale. A
|
|
// new package that nobody placed in a layer is a package with no rules, which
|
|
// is how the layering erodes.
|
|
func TestEveryPackageIsLayered(t *testing.T) {
|
|
root := repoRoot(t)
|
|
var found []string
|
|
|
|
var walk func(base string)
|
|
walk = func(base string) {
|
|
entries, err := os.ReadDir(base)
|
|
if err != nil {
|
|
return
|
|
}
|
|
for _, e := range entries {
|
|
if !e.IsDir() {
|
|
continue
|
|
}
|
|
full := filepath.Join(base, e.Name())
|
|
rel, _ := filepath.Rel(root, full)
|
|
rel = filepath.ToSlash(rel)
|
|
// Only directories that actually contain Go files are packages.
|
|
if hasGoFiles(full) {
|
|
found = append(found, rel)
|
|
}
|
|
walk(full)
|
|
}
|
|
}
|
|
walk(filepath.Join(root, "internal"))
|
|
found = append(found, "cmd/vellum")
|
|
|
|
for _, pkg := range found {
|
|
if _, ok := layers[pkg]; !ok {
|
|
t.Errorf("%s is not in the layers map; add it so the import rules apply to it", pkg)
|
|
}
|
|
}
|
|
}
|
|
|
|
func hasGoFiles(dir string) bool {
|
|
entries, err := os.ReadDir(dir)
|
|
if err != nil {
|
|
return false
|
|
}
|
|
for _, e := range entries {
|
|
if !e.IsDir() && strings.HasSuffix(e.Name(), ".go") {
|
|
return true
|
|
}
|
|
}
|
|
return false
|
|
}
|
|
|
|
// allImportsOf returns every import of a package's non-test files, including
|
|
// standard library ones. importsOf only reports internal packages, which is
|
|
// right for the layering rules and wrong for asking "does this package know
|
|
// about SQL".
|
|
func allImportsOf(t *testing.T, dir string) []string {
|
|
t.Helper()
|
|
entries, err := os.ReadDir(dir)
|
|
if err != nil {
|
|
t.Fatalf("read %s: %v", dir, err)
|
|
}
|
|
seen := map[string]bool{}
|
|
for _, e := range entries {
|
|
if e.IsDir() || !strings.HasSuffix(e.Name(), ".go") || strings.HasSuffix(e.Name(), "_test.go") {
|
|
continue
|
|
}
|
|
fset := token.NewFileSet()
|
|
f, err := parser.ParseFile(fset, filepath.Join(dir, e.Name()), nil, parser.ImportsOnly)
|
|
if err != nil {
|
|
t.Fatalf("parse %s: %v", e.Name(), err)
|
|
}
|
|
for _, imp := range f.Imports {
|
|
seen[strings.Trim(imp.Path.Value, `"`)] = true
|
|
}
|
|
}
|
|
out := make([]string, 0, len(seen))
|
|
for p := range seen {
|
|
out = append(out, p)
|
|
}
|
|
sort.Strings(out)
|
|
return out
|
|
}
|
|
|
|
// sqlImporters are the packages permitted to import database/sql in their
|
|
// non-test files.
|
|
//
|
|
// This list is the guard against the storage seam eroding by drift. The seam
|
|
// exists so the engine is swappable; it only holds if the set of packages that
|
|
// know about *sql.DB does not quietly grow. Adding a package here is a
|
|
// deliberate act with a reason, not a side effect of a feature.
|
|
//
|
|
// A package that only needs database/sql to build a TEST database does not
|
|
// belong here — the check ignores _test.go files on purpose.
|
|
var sqlImporters = map[string]bool{
|
|
"internal/apitoken": true,
|
|
"internal/auth": true,
|
|
"internal/catalog": true,
|
|
"internal/cli": true,
|
|
"internal/core": true,
|
|
"internal/home": true,
|
|
"internal/httpapi": true,
|
|
"internal/jellyfin": true,
|
|
"internal/jobs": true,
|
|
"internal/library": true,
|
|
"internal/media": true,
|
|
"internal/metadata": true,
|
|
"internal/modules/comics": true,
|
|
"internal/modules/ebooks": true,
|
|
"internal/modules/games": true,
|
|
"internal/modules/photos": true,
|
|
"internal/progress": true,
|
|
"internal/scanner": true,
|
|
"internal/search": true,
|
|
"internal/subsonic": true,
|
|
}
|
|
|
|
// threadedConsumers must NOT import database/sql. These are packages that have
|
|
// been moved onto the catalog.Store interface; if one of them reaches for
|
|
// *sql.DB again, the seam is being undone and this test says so.
|
|
var threadedConsumers = []string{
|
|
"internal/playback",
|
|
}
|
|
|
|
// TestDatabaseSQLImportersAreDeclared freezes the blast radius of database/sql.
|
|
// A new package that needs it must be added to the list deliberately, so the
|
|
// seam cannot erode one convenient import at a time.
|
|
func TestDatabaseSQLImportersAreDeclared(t *testing.T) {
|
|
root := repoRoot(t)
|
|
for pkg := range layers {
|
|
dir := filepath.Join(root, filepath.FromSlash(pkg))
|
|
if _, err := os.Stat(dir); err != nil {
|
|
continue
|
|
}
|
|
usesSQL := false
|
|
for _, imp := range allImportsOf(t, dir) {
|
|
if imp == "database/sql" {
|
|
usesSQL = true
|
|
break
|
|
}
|
|
}
|
|
switch {
|
|
case usesSQL && !sqlImporters[pkg]:
|
|
t.Errorf("%s imports database/sql but is not in sqlImporters.\n"+
|
|
"If it genuinely needs its own SQL, add it to the list with a reason.\n"+
|
|
"If it is reading items, take a catalog.Store instead.", pkg)
|
|
case !usesSQL && sqlImporters[pkg]:
|
|
t.Errorf("%s is listed in sqlImporters but no longer imports database/sql.\n"+
|
|
"Remove it from the list so the list keeps meaning something.", pkg)
|
|
}
|
|
}
|
|
}
|
|
|
|
// TestThreadedConsumersStayOffSQL is the concrete regression guard: a package
|
|
// already moved onto the seam must not drift back.
|
|
func TestThreadedConsumersStayOffSQL(t *testing.T) {
|
|
root := repoRoot(t)
|
|
for _, pkg := range threadedConsumers {
|
|
dir := filepath.Join(root, filepath.FromSlash(pkg))
|
|
if _, err := os.Stat(dir); err != nil {
|
|
continue
|
|
}
|
|
for _, imp := range allImportsOf(t, dir) {
|
|
if imp == "database/sql" {
|
|
t.Errorf("%s is on the catalog.Store seam but imports database/sql again.\n"+
|
|
"That undoes the seam: take a catalog.Store, do not reach for *sql.DB.", pkg)
|
|
}
|
|
}
|
|
}
|
|
}
|