diff --git a/pkg/appview/db/schema_drift_test.go b/pkg/appview/db/schema_drift_test.go new file mode 100644 index 0000000..3c7a0c8 --- /dev/null +++ b/pkg/appview/db/schema_drift_test.go @@ -0,0 +1,250 @@ +package db + +import ( + "database/sql" + "fmt" + "os" + "regexp" + "slices" + "sort" + "strings" + "testing" +) + +// TestSchemaMatchesMigrations enforces the invariant that schema.sql and the +// migrations describe the same database. +// +// The two are supposed to be updated in lockstep, and nothing checked that they +// were. The cost of them drifting is already on the record: migration 0009 +// exists only because "Migration 0004 was supposed to drop these columns but +// either failed or was only recorded (not executed) on production", which left +// production carrying eleven columns that fresh installs did not have. +// +// The check builds the schema both ways and compares them: +// +// A: apply schema.sql (what a fresh install gets) +// B: apply testdata/base_schema.sql, then +// run every migration (what an upgrade gets) +// +// Columns are compared as a SET, ignoring ordinal position. That is deliberate. +// Migrations append with ALTER TABLE ADD COLUMN while schema.sql places the same +// column mid-table, so the two orders legitimately differ on manifests, users, +// devices and repo_pages. Reordering those tables would mean four rebuild +// migrations for no functional gain, and nothing in pkg/appview depends on +// ordinal position (no SELECT *, no column-less INSERT ... VALUES). If you add +// code that does depend on column order, this test will not save you. +func TestSchemaMatchesMigrations(t *testing.T) { + fresh := freshSchemaDB(t) + defer fresh.Close() + + migrated := migratedSchemaDB(t) + defer migrated.Close() + + freshTables := tableNames(t, fresh) + migratedTables := tableNames(t, migrated) + + if diff := diffStringSets(freshTables, migratedTables); diff != "" { + t.Errorf("table sets differ between schema.sql and the migrations:\n%s\n\n"+ + "A table only in schema.sql will never exist on an upgraded database, because InitDB\n"+ + "skips schema.sql entirely once schema_migrations has rows. New tables need BOTH a\n"+ + "schema.sql entry and a migration.", diff) + } + + for _, table := range freshTables { + if !contains(migratedTables, table) { + continue // already reported above + } + freshCols := columnSet(t, fresh, table) + migratedCols := columnSet(t, migrated, table) + if diff := diffStringSets(freshCols, migratedCols); diff != "" { + t.Errorf("table %q differs between schema.sql and the migrations:\n%s", table, diff) + } + } + + freshIdx := indexSet(t, fresh) + migratedIdx := indexSet(t, migrated) + if diff := diffStringSets(freshIdx, migratedIdx); diff != "" { + t.Errorf("index sets differ between schema.sql and the migrations:\n%s", diff) + } +} + +// freshSchemaDB returns a database built the way a fresh install builds one: +// schema.sql applied directly, migrations recorded but not executed. +func freshSchemaDB(t *testing.T) *sql.DB { + t.Helper() + db, err := InitDB(":memory:", LibsqlConfig{}) + if err != nil { + t.Fatalf("InitDB (fresh install path): %v", err) + } + return db +} + +// migratedSchemaDB returns a database built the way an upgrade builds one: the +// pre-0009 base snapshot, then every migration executed in order. +func migratedSchemaDB(t *testing.T) *sql.DB { + t.Helper() + + base, err := os.ReadFile("testdata/base_schema.sql") + if err != nil { + t.Fatalf("read testdata/base_schema.sql: %v", err) + } + + connector, err := openLibsqlLocalConnector(":memory:") + if err != nil { + t.Fatalf("open libsql connector: %v", err) + } + db := sql.OpenDB(connector) + // One connection so every statement lands in the same in-memory database. + db.SetMaxOpenConns(1) + + if _, err := db.Exec("PRAGMA foreign_keys = ON"); err != nil { + db.Close() + t.Fatalf("enable foreign keys: %v", err) + } + + for i, stmt := range splitSQLStatements(string(base)) { + if _, err := db.Exec(stmt); err != nil { + db.Close() + t.Fatalf("apply base_schema.sql statement %d: %v\n%s", i+1, err, stmt) + } + } + + // freshDB=false so the DDL actually executes rather than merely being + // recorded. base_schema.sql creates schema_migrations empty, so every + // migration is pending. + if err := runMigrations(db, false); err != nil { + db.Close() + t.Fatalf("runMigrations against the base snapshot: %v", err) + } + return db +} + +// tableNames returns the user tables in db, sorted. SQLite internal tables +// (sqlite_sequence and friends, created implicitly by AUTOINCREMENT) are +// excluded since they are an artifact of the engine, not of our schema. +func tableNames(t *testing.T, db *sql.DB) []string { + t.Helper() + rows, err := db.Query(` + SELECT name FROM sqlite_master + WHERE type = 'table' AND name NOT LIKE 'sqlite_%' + ORDER BY name + `) + if err != nil { + t.Fatalf("list tables: %v", err) + } + defer rows.Close() + + var names []string + for rows.Next() { + var n string + if err := rows.Scan(&n); err != nil { + t.Fatalf("scan table name: %v", err) + } + names = append(names, n) + } + if err := rows.Err(); err != nil { + t.Fatalf("iterate tables: %v", err) + } + return names +} + +// columnSet returns one descriptor per column: name, type, not-null, default and +// primary-key position. Ordinal position (cid) is deliberately excluded. +func columnSet(t *testing.T, db *sql.DB, table string) []string { + t.Helper() + rows, err := db.Query(fmt.Sprintf("PRAGMA table_info(%q)", table)) + if err != nil { + t.Fatalf("table_info(%s): %v", table, err) + } + defer rows.Close() + + var cols []string + for rows.Next() { + var ( + cid int + name string + colType string + notNull int + dfltValue sql.NullString + pk int + ) + if err := rows.Scan(&cid, &name, &colType, ¬Null, &dfltValue, &pk); err != nil { + t.Fatalf("scan table_info(%s): %v", table, err) + } + dflt := "" + if dfltValue.Valid { + dflt = dfltValue.String + } + cols = append(cols, fmt.Sprintf("%s type=%s notnull=%d default=%s pk=%d", + name, strings.ToUpper(colType), notNull, dflt, pk)) + } + if err := rows.Err(); err != nil { + t.Fatalf("iterate table_info(%s): %v", table, err) + } + sort.Strings(cols) + return cols +} + +var whitespaceRun = regexp.MustCompile(`\s+`) + +// indexSet returns the explicitly-created indexes as normalized DDL. Indexes +// SQLite creates implicitly for UNIQUE and PRIMARY KEY constraints have a NULL +// sql column and are skipped: they are already implied by the column set. +func indexSet(t *testing.T, db *sql.DB) []string { + t.Helper() + rows, err := db.Query(` + SELECT sql FROM sqlite_master + WHERE type = 'index' AND sql IS NOT NULL + `) + if err != nil { + t.Fatalf("list indexes: %v", err) + } + defer rows.Close() + + var out []string + for rows.Next() { + var ddl string + if err := rows.Scan(&ddl); err != nil { + t.Fatalf("scan index sql: %v", err) + } + norm := whitespaceRun.ReplaceAllString(strings.TrimSpace(ddl), " ") + norm = strings.ReplaceAll(norm, "IF NOT EXISTS ", "") + out = append(out, norm) + } + if err := rows.Err(); err != nil { + t.Fatalf("iterate indexes: %v", err) + } + sort.Strings(out) + return out +} + +// diffStringSets returns a human-readable diff, or "" when the sets match. +// "schema.sql" is the a side, "migrations" the b side. +func diffStringSets(a, b []string) string { + var onlyA, onlyB []string + for _, s := range a { + if !contains(b, s) { + onlyA = append(onlyA, s) + } + } + for _, s := range b { + if !contains(a, s) { + onlyB = append(onlyB, s) + } + } + if len(onlyA) == 0 && len(onlyB) == 0 { + return "" + } + var sb strings.Builder + for _, s := range onlyA { + fmt.Fprintf(&sb, " only in schema.sql: %s\n", s) + } + for _, s := range onlyB { + fmt.Fprintf(&sb, " only in migrations: %s\n", s) + } + return strings.TrimRight(sb.String(), "\n") +} + +func contains(haystack []string, needle string) bool { + return slices.Contains(haystack, needle) +} diff --git a/pkg/appview/db/testdata/base_schema.sql b/pkg/appview/db/testdata/base_schema.sql new file mode 100644 index 0000000..ec0b59b --- /dev/null +++ b/pkg/appview/db/testdata/base_schema.sql @@ -0,0 +1,312 @@ +-- Base schema snapshot: the database shape immediately BEFORE migration 0009. +-- +-- This file exists solely to feed TestSchemaMatchesMigrations. Applying this +-- file and then running every migration in migrations/ must produce the same +-- schema as applying schema.sql directly. That is the invariant the test +-- enforces, and it is what keeps schema.sql and the migrations in lockstep. +-- +-- HOW THIS WAS DERIVED +-- +-- Not from history. It was reconstructed by taking the current schema.sql and +-- reversing migrations 0029 down to 0009: +-- +-- 0029 drop stripe_processed_events +-- 0028 drop devices.secret_lookup and its index +-- 0027 drop users.registry_domain +-- 0026 drop webhooks.last_fired_at +-- 0025 drop taken_down_subjects and labeler_cursor +-- 0023 (labels table is created by 0023 and dropped by 0025: absent here) +-- 0022 drop jetstream_cursor +-- 0021 drop repository_stats_daily +-- 0020 drop manifests.subject_digest and its index +-- 0019 drop advisor_suggestions +-- 0018 drop users.oci_client +-- 0017 drop repo_pages.user_edited +-- 0015 drop webhooks and scans +-- 0014 drop users.default_hold_did +-- (hold_captain_records.supporter_badge_tiers is added by 0014 and +-- dropped by 0016, so it nets out and is absent here) +-- 0013 drop crypto_keys +-- 0012 drop layers.annotations +-- 0011 hold_captain_records is rebuilt by 0011; 0010 added successor, so +-- the pre-0010 table has neither successor nor supporter_badge_tiers +-- 0009 restore the 11 dead columns on manifests that 0009 removes +-- +-- WHAT THIS DOES AND DOES NOT PROVE +-- +-- It proves that migrations 0009-0029 transform this shape into schema.sql, and +-- it will prove the same for every migration added from now on. That is the +-- point: new work is covered automatically. +-- +-- It does NOT independently verify tables that appear in no migration (tags, +-- oauth_sessions, stars, and the rest predate the migration system). Those are +-- copied from schema.sql here, so the test compares them against themselves and +-- can only ever agree. Drift in those tables on a real database is caught by the +-- startup drift check in schema.go, not by this test. +-- +-- The 11 dead columns on manifests are typed TEXT because migration 0009 never +-- reads them (its INSERT ... SELECT names only the columns it keeps), so their +-- original types cannot be recovered and do not affect the outcome. +-- +-- WHEN YOU ADD A MIGRATION: do not touch this file. It is a fixed starting +-- point. Update schema.sql instead, which is what the migration must converge +-- on. Only edit this file if you are correcting the reconstruction itself. + +CREATE TABLE IF NOT EXISTS schema_migrations ( + version INTEGER PRIMARY KEY, + applied_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP +); + +-- Versions 1-8 are recorded as already applied, because "the state before 0009" +-- means exactly that. Only 0001 still exists as a file; 0002-0008 were deleted +-- from the tree after they shipped, and migration 0009's own comment confirms +-- 0004 was recorded on production. +-- +-- This also keeps the test off a live rake: migration 0001 is the example +-- migration, whose query is `SELECT COUNT(*) FROM schema_migrations`. go-libsql +-- rejects a row-returning statement passed to Exec ("Execute returned rows"), so +-- 0001 would fail hard on any database that has not recorded it. Every real +-- database recorded it years ago, so this is invisible in production, but a +-- future migration that opens with a SELECT would fail the same way. Migrations +-- must not return rows. +INSERT INTO schema_migrations (version) VALUES (1), (2), (3), (4), (5), (6), (7), (8); + +CREATE TABLE IF NOT EXISTS users ( + did TEXT PRIMARY KEY, + handle TEXT NOT NULL, + pds_endpoint TEXT NOT NULL, + avatar TEXT, + last_seen TIMESTAMP NOT NULL, + UNIQUE(handle) +); +CREATE INDEX IF NOT EXISTS idx_users_handle ON users(handle); + +-- Pre-0009: still carries the 11 columns that migration 0009 removes. +CREATE TABLE IF NOT EXISTS manifests ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + did TEXT NOT NULL, + repository TEXT NOT NULL, + digest TEXT NOT NULL, + hold_endpoint TEXT NOT NULL, + schema_version INTEGER NOT NULL, + media_type TEXT NOT NULL, + config_digest TEXT, + config_size INTEGER, + artifact_type TEXT NOT NULL DEFAULT 'container-image', + title TEXT, + description TEXT, + source_url TEXT, + documentation_url TEXT, + licenses TEXT, + icon_url TEXT, + readme_url TEXT, + platform_os TEXT, + platform_architecture TEXT, + platform_variant TEXT, + platform_os_version TEXT, + created_at TIMESTAMP NOT NULL, + UNIQUE(did, repository, digest), + FOREIGN KEY(did) REFERENCES users(did) ON DELETE CASCADE +); +CREATE INDEX IF NOT EXISTS idx_manifests_did_repo ON manifests(did, repository); +CREATE INDEX IF NOT EXISTS idx_manifests_created_at ON manifests(created_at DESC); +CREATE INDEX IF NOT EXISTS idx_manifests_digest ON manifests(digest); +CREATE INDEX IF NOT EXISTS idx_manifests_artifact_type ON manifests(artifact_type); + +CREATE TABLE IF NOT EXISTS repository_annotations ( + did TEXT NOT NULL, + repository TEXT NOT NULL, + key TEXT NOT NULL, + value TEXT NOT NULL, + updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, + PRIMARY KEY(did, repository, key), + FOREIGN KEY(did) REFERENCES users(did) ON DELETE CASCADE +); +CREATE INDEX IF NOT EXISTS idx_repository_annotations_did_repo ON repository_annotations(did, repository); +CREATE INDEX IF NOT EXISTS idx_repository_annotations_key ON repository_annotations(key); + +CREATE TABLE IF NOT EXISTS layers ( + manifest_id INTEGER NOT NULL, + digest TEXT NOT NULL, + size INTEGER NOT NULL, + media_type TEXT NOT NULL, + layer_index INTEGER NOT NULL, + PRIMARY KEY(manifest_id, layer_index), + FOREIGN KEY(manifest_id) REFERENCES manifests(id) ON DELETE CASCADE +); +CREATE INDEX IF NOT EXISTS idx_layers_digest ON layers(digest); + +CREATE TABLE IF NOT EXISTS manifest_references ( + manifest_id INTEGER NOT NULL, + digest TEXT NOT NULL, + media_type TEXT NOT NULL, + size INTEGER NOT NULL, + platform_architecture TEXT, + platform_os TEXT, + platform_variant TEXT, + platform_os_version TEXT, + is_attestation BOOLEAN DEFAULT FALSE, + reference_index INTEGER NOT NULL, + PRIMARY KEY(manifest_id, reference_index), + FOREIGN KEY(manifest_id) REFERENCES manifests(id) ON DELETE CASCADE +); +CREATE INDEX IF NOT EXISTS idx_manifest_references_digest ON manifest_references(digest); + +CREATE TABLE IF NOT EXISTS tags ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + did TEXT NOT NULL, + repository TEXT NOT NULL, + tag TEXT NOT NULL, + digest TEXT NOT NULL, + created_at TIMESTAMP NOT NULL, + UNIQUE(did, repository, tag), + FOREIGN KEY(did) REFERENCES users(did) ON DELETE CASCADE +); +CREATE INDEX IF NOT EXISTS idx_tags_did_repo ON tags(did, repository); + +CREATE TABLE IF NOT EXISTS oauth_sessions ( + session_key TEXT PRIMARY KEY, + account_did TEXT NOT NULL, + session_id TEXT NOT NULL, + session_data TEXT NOT NULL, + created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, + updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, + UNIQUE(account_did, session_id) +); +CREATE INDEX IF NOT EXISTS idx_oauth_sessions_did ON oauth_sessions(account_did); +CREATE INDEX IF NOT EXISTS idx_oauth_sessions_updated ON oauth_sessions(updated_at DESC); + +CREATE TABLE IF NOT EXISTS oauth_auth_requests ( + state TEXT PRIMARY KEY, + request_data TEXT NOT NULL, + created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP +); +CREATE INDEX IF NOT EXISTS idx_oauth_auth_requests_created ON oauth_auth_requests(created_at); + +CREATE TABLE IF NOT EXISTS ui_sessions ( + id TEXT PRIMARY KEY, + did TEXT NOT NULL, + handle TEXT NOT NULL, + pds_endpoint TEXT NOT NULL, + oauth_session_id TEXT, + expires_at TIMESTAMP NOT NULL, + created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, + FOREIGN KEY(did) REFERENCES users(did) ON DELETE CASCADE +); +CREATE INDEX IF NOT EXISTS idx_ui_sessions_did ON ui_sessions(did); +CREATE INDEX IF NOT EXISTS idx_ui_sessions_expires ON ui_sessions(expires_at); + +CREATE TABLE IF NOT EXISTS devices ( + id TEXT PRIMARY KEY, + did TEXT NOT NULL, + handle TEXT NOT NULL, + name TEXT NOT NULL, + secret_hash TEXT NOT NULL UNIQUE, + ip_address TEXT, + location TEXT, + user_agent TEXT, + created_at TIMESTAMP NOT NULL, + last_used TIMESTAMP, + FOREIGN KEY(did) REFERENCES users(did) ON DELETE CASCADE +); +CREATE INDEX IF NOT EXISTS idx_devices_did ON devices(did); +CREATE INDEX IF NOT EXISTS idx_devices_hash ON devices(secret_hash); + +CREATE TABLE IF NOT EXISTS pending_device_auth ( + device_code TEXT PRIMARY KEY, + user_code TEXT NOT NULL UNIQUE, + device_name TEXT NOT NULL, + ip_address TEXT, + user_agent TEXT, + expires_at TIMESTAMP NOT NULL, + approved_did TEXT, + approved_at TIMESTAMP, + device_secret TEXT, + created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP +); +CREATE INDEX IF NOT EXISTS idx_pending_device_auth_user_code ON pending_device_auth(user_code); +CREATE INDEX IF NOT EXISTS idx_pending_device_auth_expires ON pending_device_auth(expires_at); + +CREATE TABLE IF NOT EXISTS repository_stats ( + did TEXT NOT NULL, + repository TEXT NOT NULL, + pull_count INTEGER NOT NULL DEFAULT 0, + last_pull TIMESTAMP, + push_count INTEGER NOT NULL DEFAULT 0, + last_push TIMESTAMP, + PRIMARY KEY(did, repository), + FOREIGN KEY(did) REFERENCES users(did) ON DELETE CASCADE +); +CREATE INDEX IF NOT EXISTS idx_repository_stats_did ON repository_stats(did); +CREATE INDEX IF NOT EXISTS idx_repository_stats_pull_count ON repository_stats(pull_count DESC); + +CREATE TABLE IF NOT EXISTS stars ( + starrer_did TEXT NOT NULL, + owner_did TEXT NOT NULL, + repository TEXT NOT NULL, + created_at TIMESTAMP NOT NULL, + PRIMARY KEY(starrer_did, owner_did, repository), + FOREIGN KEY(starrer_did) REFERENCES users(did) ON DELETE CASCADE, + FOREIGN KEY(owner_did) REFERENCES users(did) ON DELETE CASCADE +); +CREATE INDEX IF NOT EXISTS idx_stars_owner_repo ON stars(owner_did, repository); +CREATE INDEX IF NOT EXISTS idx_stars_starrer ON stars(starrer_did); + +-- Pre-0010: no successor column yet (0010 adds it, 0011 rebuilds the table). +CREATE TABLE IF NOT EXISTS hold_captain_records ( + hold_did TEXT PRIMARY KEY, + owner_did TEXT NOT NULL, + public BOOLEAN NOT NULL, + allow_all_crew BOOLEAN NOT NULL, + deployed_at TEXT, + region TEXT, + updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP +); +CREATE INDEX IF NOT EXISTS idx_hold_captain_updated ON hold_captain_records(updated_at); + +CREATE TABLE IF NOT EXISTS hold_crew_approvals ( + hold_did TEXT NOT NULL, + user_did TEXT NOT NULL, + approved_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, + expires_at TIMESTAMP NOT NULL, + PRIMARY KEY(hold_did, user_did) +); +CREATE INDEX IF NOT EXISTS idx_crew_approvals_expires ON hold_crew_approvals(expires_at); + +CREATE TABLE IF NOT EXISTS hold_crew_denials ( + hold_did TEXT NOT NULL, + user_did TEXT NOT NULL, + denial_count INTEGER NOT NULL DEFAULT 1, + next_retry_at TIMESTAMP NOT NULL, + last_denied_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, + PRIMARY KEY(hold_did, user_did) +); +CREATE INDEX IF NOT EXISTS idx_crew_denials_retry ON hold_crew_denials(next_retry_at); + +CREATE TABLE IF NOT EXISTS hold_crew_members ( + hold_did TEXT NOT NULL, + member_did TEXT NOT NULL, + rkey TEXT NOT NULL, + role TEXT, + permissions TEXT, + tier TEXT, + added_at TEXT, + created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP, + updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP, + PRIMARY KEY (hold_did, member_did) +); +CREATE INDEX IF NOT EXISTS idx_hold_crew_member ON hold_crew_members(member_did); +CREATE INDEX IF NOT EXISTS idx_hold_crew_hold ON hold_crew_members(hold_did); +CREATE INDEX IF NOT EXISTS idx_hold_crew_rkey ON hold_crew_members(hold_did, rkey); + +CREATE TABLE IF NOT EXISTS repo_pages ( + did TEXT NOT NULL, + repository TEXT NOT NULL, + description TEXT, + avatar_cid TEXT, + created_at TIMESTAMP NOT NULL, + updated_at TIMESTAMP NOT NULL, + PRIMARY KEY(did, repository), + FOREIGN KEY(did) REFERENCES users(did) ON DELETE CASCADE +); +CREATE INDEX IF NOT EXISTS idx_repo_pages_did ON repo_pages(did);