diff --git a/pkg/appview/db/batch.go b/pkg/appview/db/batch.go index e6f77a5..758dd97 100644 --- a/pkg/appview/db/batch.go +++ b/pkg/appview/db/batch.go @@ -1,6 +1,8 @@ package db import ( + "crypto/sha256" + "encoding/hex" "encoding/json" "fmt" "strings" @@ -59,7 +61,7 @@ func BatchInsertManifests(db DBTX, manifests []Manifest) (map[string]int64, erro start, end := chunk(len(manifests), i) batch := manifests[start:end] - const cols = 11 + const cols = 12 args := make([]any, 0, len(batch)*cols) for _, m := range batch { args = append(args, @@ -68,13 +70,19 @@ func BatchInsertManifests(db DBTX, manifests []Manifest) (map[string]int64, erro m.ConfigSize, m.ArtifactType, nullString(m.SubjectDigest), m.CreatedAt, + ManifestKey(m.DID, m.Repository, m.Digest), ) } + // The trailing "manifest_key IS NULL" clause on the WHERE is what + // populates rows that predate the column. Without it this upsert skips + // unchanged manifests entirely, so re-running the backfill would never + // fill their key and the column would stay half-empty forever. query := ` INSERT INTO manifests (did, repository, digest, hold_endpoint, schema_version, media_type, - config_digest, config_size, artifact_type, subject_digest, created_at) + config_digest, config_size, artifact_type, subject_digest, created_at, + manifest_key) VALUES ` + buildPlaceholders(len(batch), cols) + ` ON CONFLICT(did, repository, digest) DO UPDATE SET hold_endpoint = excluded.hold_endpoint, @@ -83,7 +91,8 @@ func BatchInsertManifests(db DBTX, manifests []Manifest) (map[string]int64, erro config_digest = excluded.config_digest, config_size = excluded.config_size, artifact_type = excluded.artifact_type, - subject_digest = excluded.subject_digest + subject_digest = excluded.subject_digest, + manifest_key = excluded.manifest_key WHERE excluded.hold_endpoint != manifests.hold_endpoint OR excluded.schema_version != manifests.schema_version OR excluded.media_type != manifests.media_type @@ -91,6 +100,7 @@ func BatchInsertManifests(db DBTX, manifests []Manifest) (map[string]int64, erro OR excluded.config_size IS NOT manifests.config_size OR excluded.artifact_type != manifests.artifact_type OR excluded.subject_digest IS NOT manifests.subject_digest + OR manifests.manifest_key IS NULL ` if _, err := db.Exec(query, args...); err != nil { return nil, fmt.Errorf("batch insert manifests: %w", err) @@ -135,7 +145,7 @@ func BatchInsertManifests(db DBTX, manifests []Manifest) (map[string]int64, erro return nil, fmt.Errorf("scan manifest id: %w", err) } // Key format matches what callers use: "did|repo|digest". - out[manifestKey(did, repo, digest)] = id + out[ManifestKey(did, repo, digest)] = id } rows.Close() } @@ -143,15 +153,30 @@ func BatchInsertManifests(db DBTX, manifests []Manifest) (map[string]int64, erro return out, nil } -// ManifestKey builds the lookup key used by BatchInsertManifests' result map. -// Callers construct the same key from their in-memory Manifest structs to -// find the assigned id. +// ManifestKey derives a manifest's node-independent identity from its natural +// key, (did, repository, digest). +// +// This is both the value stored in manifests.manifest_key and the lookup key for +// BatchInsertManifests' result map, which are the same concept: "which manifest +// is this" answered without asking the database. +// +// Deterministic on purpose. A rowid is allocated by whichever node performs the +// insert, so it is only a stable identity while every write funnels through a +// single writer. Any node can compute this one, before the write, without a +// round trip. +// +// Truncated to 16 bytes: 128 bits puts the birthday bound at 2^64, which is far +// past the number of manifests that could ever exist here. The column carries a +// UNIQUE index anyway, so real data checks the assumption rather than trusting +// it. func ManifestKey(did, repository, digest string) string { - return manifestKey(did, repository, digest) -} - -func manifestKey(did, repository, digest string) string { - return did + "|" + repository + "|" + digest + h := sha256.New() + h.Write([]byte(did)) + h.Write([]byte{0}) + h.Write([]byte(repository)) + h.Write([]byte{0}) + h.Write([]byte(digest)) + return hex.EncodeToString(h.Sum(nil)[:16]) } // BatchInsertLayers inserts a batch of layers, skipping any that already exist. diff --git a/pkg/appview/db/manifest_key_test.go b/pkg/appview/db/manifest_key_test.go new file mode 100644 index 0000000..9ce94f4 --- /dev/null +++ b/pkg/appview/db/manifest_key_test.go @@ -0,0 +1,253 @@ +package db + +import ( + "database/sql" + "testing" + "time" +) + +func manifestKeyTestUser(t *testing.T, database *sql.DB) string { + t.Helper() + const did = "did:plc:manifestowner" + if err := UpsertUser(database, &User{ + DID: did, + Handle: "owner.example.com", + PDSEndpoint: "https://pds.example.com", + LastSeen: time.Now(), + }); err != nil { + t.Fatalf("UpsertUser: %v", err) + } + return did +} + +func sampleManifest(did, repository, digest string) *Manifest { + return &Manifest{ + DID: did, + Repository: repository, + Digest: digest, + HoldEndpoint: "did:web:hold.example.com", + SchemaVersion: 2, + MediaType: "application/vnd.oci.image.manifest.v1+json", + ArtifactType: "container-image", + CreatedAt: time.Now(), + } +} + +func storedManifestKey(t *testing.T, database *sql.DB, did, repository, digest string) (string, bool) { + t.Helper() + var key sql.NullString + err := database.QueryRow( + `SELECT manifest_key FROM manifests WHERE did = ? AND repository = ? AND digest = ?`, + did, repository, digest, + ).Scan(&key) + if err != nil { + t.Fatalf("read manifest_key: %v", err) + } + return key.String, key.Valid +} + +// TestManifestKeyIsDeterministicAndDistinct: the key stands in for a manifest's +// identity, so the same natural key must always produce the same value and +// different ones must not collide. +func TestManifestKeyIsDeterministicAndDistinct(t *testing.T) { + a := ManifestKey("did:plc:alice", "myapp", "sha256:abc") + if a != ManifestKey("did:plc:alice", "myapp", "sha256:abc") { + t.Error("ManifestKey is not deterministic") + } + if len(a) != 32 { + t.Errorf("expected a 32-character key, got %d", len(a)) + } + + distinct := map[string]string{ + "different did": ManifestKey("did:plc:bob", "myapp", "sha256:abc"), + "different repo": ManifestKey("did:plc:alice", "otherapp", "sha256:abc"), + "different digest": ManifestKey("did:plc:alice", "myapp", "sha256:def"), + } + for name, key := range distinct { + if key == a { + t.Errorf("%s produced the same key", name) + } + } + + // The separator matters: without it, ("ab", "c", d) and ("a", "bc", d) + // would hash identically and two different manifests would share an identity. + if ManifestKey("ab", "c", "d") == ManifestKey("a", "bc", "d") { + t.Error("field boundaries are not encoded; concatenation is ambiguous") + } +} + +// TestInsertManifestPopulatesKey: new rows must never be written without a key, +// or the backfill would have to keep chasing them. +func TestInsertManifestPopulatesKey(t *testing.T) { + database := revTestDB(t) + did := manifestKeyTestUser(t, database) + + if _, err := InsertManifest(database, sampleManifest(did, "myapp", "sha256:abc")); err != nil { + t.Fatalf("InsertManifest: %v", err) + } + + got, ok := storedManifestKey(t, database, did, "myapp", "sha256:abc") + if !ok { + t.Fatal("manifest_key was left NULL on insert") + } + if want := ManifestKey(did, "myapp", "sha256:abc"); got != want { + t.Errorf("manifest_key = %q, want %q", got, want) + } +} + +// TestBackfillFillsKeyForUnchangedManifest is the mechanism this whole approach +// rests on. +// +// The column cannot be filled by its migration, because the value is a truncated +// sha256 and SQLite has no hash builtin. Instead the ordinary Jetstream backfill +// fills it while re-upserting every manifest. That only works because the upsert +// carries an extra "OR manifests.manifest_key IS NULL" clause on its WHERE +// guard: without it the upsert skips manifests whose other fields are unchanged, +// which is nearly all of them on a re-run, and the column would stay half-empty +// forever. +func TestBackfillFillsKeyForUnchangedManifest(t *testing.T) { + database := revTestDB(t) + did := manifestKeyTestUser(t, database) + + m := sampleManifest(did, "myapp", "sha256:abc") + if _, err := InsertManifest(database, m); err != nil { + t.Fatalf("InsertManifest: %v", err) + } + + // Simulate a row that predates the column. + if _, err := database.Exec( + `UPDATE manifests SET manifest_key = NULL WHERE did = ? AND repository = ? AND digest = ?`, + did, "myapp", "sha256:abc", + ); err != nil { + t.Fatalf("clear manifest_key: %v", err) + } + if _, ok := storedManifestKey(t, database, did, "myapp", "sha256:abc"); ok { + t.Fatal("failed to simulate a pre-existing row") + } + + // Re-upsert exactly the same manifest, as the backfill does. Nothing else + // about it has changed. + if _, err := InsertManifest(database, sampleManifest(did, "myapp", "sha256:abc")); err != nil { + t.Fatalf("re-insert: %v", err) + } + + got, ok := storedManifestKey(t, database, did, "myapp", "sha256:abc") + if !ok { + t.Fatal("an unchanged manifest kept its NULL key; the upsert's WHERE guard skipped it") + } + if want := ManifestKey(did, "myapp", "sha256:abc"); got != want { + t.Errorf("manifest_key = %q, want %q", got, want) + } +} + +// TestBatchBackfillFillsKeyForUnchangedManifests is the same property for the +// batch path, which is the one the backfill worker actually uses. +func TestBatchBackfillFillsKeyForUnchangedManifests(t *testing.T) { + database := revTestDB(t) + did := manifestKeyTestUser(t, database) + + manifests := []Manifest{ + *sampleManifest(did, "myapp", "sha256:aaa"), + *sampleManifest(did, "myapp", "sha256:bbb"), + *sampleManifest(did, "otherapp", "sha256:ccc"), + } + if _, err := BatchInsertManifests(database, manifests); err != nil { + t.Fatalf("BatchInsertManifests: %v", err) + } + if _, err := database.Exec(`UPDATE manifests SET manifest_key = NULL`); err != nil { + t.Fatalf("clear keys: %v", err) + } + + if _, err := BatchInsertManifests(database, manifests); err != nil { + t.Fatalf("re-run BatchInsertManifests: %v", err) + } + + remaining, total, err := ManifestKeyBackfillProgress(database) + if err != nil { + t.Fatalf("ManifestKeyBackfillProgress: %v", err) + } + if total != int64(len(manifests)) { + t.Errorf("total = %d, want %d", total, len(manifests)) + } + if remaining != 0 { + t.Errorf("%d manifests still have no key after a full backfill pass", remaining) + } +} + +// TestManifestKeyBackfillProgressCountsUnfilled: this is how anyone decides the +// follow-up migration is safe to run, so it has to actually count. +func TestManifestKeyBackfillProgressCountsUnfilled(t *testing.T) { + database := revTestDB(t) + did := manifestKeyTestUser(t, database) + + for _, digest := range []string{"sha256:aaa", "sha256:bbb", "sha256:ccc"} { + if _, err := InsertManifest(database, sampleManifest(did, "myapp", digest)); err != nil { + t.Fatalf("InsertManifest: %v", err) + } + } + + remaining, total, err := ManifestKeyBackfillProgress(database) + if err != nil { + t.Fatalf("ManifestKeyBackfillProgress: %v", err) + } + if remaining != 0 || total != 3 { + t.Errorf("remaining=%d total=%d, want 0 and 3", remaining, total) + } + + if _, err := database.Exec( + `UPDATE manifests SET manifest_key = NULL WHERE digest IN ('sha256:aaa', 'sha256:bbb')`, + ); err != nil { + t.Fatalf("clear keys: %v", err) + } + + remaining, total, err = ManifestKeyBackfillProgress(database) + if err != nil { + t.Fatalf("ManifestKeyBackfillProgress: %v", err) + } + if remaining != 2 || total != 3 { + t.Errorf("remaining=%d total=%d, want 2 and 3", remaining, total) + } +} + +// TestManifestKeyUniqueIndexRejectsCollisions: the index is what turns the +// 16-byte truncation from an assumption into something the database checks. If +// it were ever dropped, a collision would silently attach one manifest's layers +// to another after the eventual swap. +func TestManifestKeyUniqueIndexRejectsCollisions(t *testing.T) { + database := revTestDB(t) + did := manifestKeyTestUser(t, database) + + if _, err := InsertManifest(database, sampleManifest(did, "myapp", "sha256:abc")); err != nil { + t.Fatalf("InsertManifest: %v", err) + } + key, _ := storedManifestKey(t, database, did, "myapp", "sha256:abc") + + // Force a second, genuinely different manifest to claim the same key. + if _, err := InsertManifest(database, sampleManifest(did, "myapp", "sha256:def")); err != nil { + t.Fatalf("InsertManifest: %v", err) + } + _, err := database.Exec( + `UPDATE manifests SET manifest_key = ? WHERE digest = ?`, key, "sha256:def") + if err == nil { + t.Error("two manifests were allowed to share a manifest_key; the unique index is missing") + } +} + +// TestManifestKeyNullsDoNotCollide: the unique index has to tolerate many +// unfilled rows, or the migration could not add it before the backfill runs. +func TestManifestKeyNullsDoNotCollide(t *testing.T) { + database := revTestDB(t) + did := manifestKeyTestUser(t, database) + + for _, digest := range []string{"sha256:aaa", "sha256:bbb", "sha256:ccc"} { + if _, err := InsertManifest(database, sampleManifest(did, "myapp", digest)); err != nil { + t.Fatalf("InsertManifest: %v", err) + } + } + + // SQLite treats NULLs as distinct in a unique index; several unfilled rows + // must coexist. + if _, err := database.Exec(`UPDATE manifests SET manifest_key = NULL`); err != nil { + t.Errorf("multiple NULL manifest_key rows were rejected: %v", err) + } +} diff --git a/pkg/appview/db/migrations/0033_add_manifest_key.yaml b/pkg/appview/db/migrations/0033_add_manifest_key.yaml new file mode 100644 index 0000000..2a12c07 --- /dev/null +++ b/pkg/appview/db/migrations/0033_add_manifest_key.yaml @@ -0,0 +1,33 @@ +description: | + Add manifests.manifest_key, a node-independent identity derived from the + natural key (did, repository, digest). + + Nothing depends on it yet. id remains the primary key and layers / + manifest_references still reference it. This is the first half of replacing the + rowid: get the column in place and populated, so the eventual swap operates on + data that is already complete and already proven unique, rather than doing the + fill and three table rebuilds in one step. + + Nullable, and there is no UPDATE here to fill it, because the value cannot be + computed in SQL: it is a truncated sha256 and SQLite has no hash builtin, nor + can go-libsql register one. The fill happens through the machinery that already + exists. The Jetstream backfill re-upserts every manifest across the protocol on + startup, and both upsert paths now write manifest_key and carry an extra + "OR manifests.manifest_key IS NULL" clause on their WHERE guard. Without that + clause the upserts would skip unchanged manifests and the column would never + fill; with it, the existing backfill populates the table as a side effect of a + run it was making anyway. + + The index is UNIQUE even though the column is nullable. SQLite treats NULLs as + distinct, so rows not yet filled do not collide with each other, while every + filled row is checked. That makes production data verify the 16-byte + truncation instead of us assuming it: if two manifests ever derived the same + key, the insert fails loudly here rather than silently attaching one manifest's + layers to another after the swap. + + AppView logs the count of unfilled rows at startup. When it reaches zero the + follow-up migration can make manifest_key NOT NULL, move layers and + manifest_references onto it, and drop id. +query: | + ALTER TABLE manifests ADD COLUMN manifest_key TEXT; + CREATE UNIQUE INDEX IF NOT EXISTS idx_manifests_manifest_key ON manifests(manifest_key); diff --git a/pkg/appview/db/queries.go b/pkg/appview/db/queries.go index 9027c0a..641095c 100644 --- a/pkg/appview/db/queries.go +++ b/pkg/appview/db/queries.go @@ -800,11 +800,16 @@ func DeleteTagsNotInList(db DBTX, did string, keepTags []struct{ Repository, Tag // Returns the manifest ID (works correctly for both insert and update) // Note: Annotations are stored separately in repository_annotations table func InsertManifest(db DBTX, manifest *Manifest) (int64, error) { + // The trailing "manifest_key IS NULL" clause on the WHERE is what populates + // rows that predate the column. Without it this upsert skips unchanged + // manifests entirely, so re-running the backfill would never fill their key + // and the column would stay half-empty forever. _, err := db.Exec(` INSERT INTO manifests (did, repository, digest, hold_endpoint, schema_version, media_type, - config_digest, config_size, artifact_type, subject_digest, created_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + config_digest, config_size, artifact_type, subject_digest, created_at, + manifest_key) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT(did, repository, digest) DO UPDATE SET hold_endpoint = excluded.hold_endpoint, schema_version = excluded.schema_version, @@ -812,7 +817,8 @@ func InsertManifest(db DBTX, manifest *Manifest) (int64, error) { config_digest = excluded.config_digest, config_size = excluded.config_size, artifact_type = excluded.artifact_type, - subject_digest = excluded.subject_digest + subject_digest = excluded.subject_digest, + manifest_key = excluded.manifest_key WHERE excluded.hold_endpoint != manifests.hold_endpoint OR excluded.schema_version != manifests.schema_version OR excluded.media_type != manifests.media_type @@ -820,11 +826,13 @@ func InsertManifest(db DBTX, manifest *Manifest) (int64, error) { OR excluded.config_size IS NOT manifests.config_size OR excluded.artifact_type != manifests.artifact_type OR excluded.subject_digest IS NOT manifests.subject_digest + OR manifests.manifest_key IS NULL `, manifest.DID, manifest.Repository, manifest.Digest, manifest.HoldEndpoint, manifest.SchemaVersion, manifest.MediaType, manifest.ConfigDigest, manifest.ConfigSize, manifest.ArtifactType, sql.NullString{String: manifest.SubjectDigest, Valid: manifest.SubjectDigest != ""}, - manifest.CreatedAt) + manifest.CreatedAt, + ManifestKey(manifest.DID, manifest.Repository, manifest.Digest)) if err != nil { return 0, err @@ -3188,3 +3196,24 @@ func GetDailyStats(db DBTX, did, repository, startDate, endDate string) ([]Daily } return stats, rows.Err() } + +// ManifestKeyBackfillProgress reports how far the manifest_key fill has got: +// the number of manifests still missing a key, and the total. +// +// manifest_key is populated by the ordinary Jetstream backfill rather than by a +// migration, because the value is a truncated sha256 and SQLite cannot compute +// it. That makes "is it done yet" a question worth being able to answer: the +// follow-up migration that makes the column NOT NULL, moves layers and +// manifest_references onto it, and drops id is only safe once remaining is zero. +func ManifestKeyBackfillProgress(db DBTX) (remaining, total int64, err error) { + err = db.QueryRow(` + SELECT + COUNT(*) FILTER (WHERE manifest_key IS NULL), + COUNT(*) + FROM manifests + `).Scan(&remaining, &total) + if err != nil { + return 0, 0, fmt.Errorf("manifest_key backfill progress: %w", err) + } + return remaining, total, nil +} diff --git a/pkg/appview/db/schema.sql b/pkg/appview/db/schema.sql index 78e2c54..72bba05 100644 --- a/pkg/appview/db/schema.sql +++ b/pkg/appview/db/schema.sql @@ -33,9 +33,23 @@ CREATE TABLE IF NOT EXISTS manifests ( artifact_type TEXT NOT NULL DEFAULT 'container-image', -- container-image, helm-chart, unknown subject_digest TEXT, -- digest of the parent manifest (for attestations/referrers) created_at TIMESTAMP NOT NULL, + -- Node-independent identity: hex(sha256(did || 0 || repository || 0 || digest)) + -- truncated to 16 bytes. See db.ManifestKey. + -- + -- Not yet load-bearing. id is still the primary key and layers / + -- manifest_references still reference it; this column is being populated + -- first so the swap can happen later against data that is already complete + -- and already proven unique. Nullable only because rows predating it are + -- filled in by the ordinary Jetstream backfill rather than all at once. + manifest_key TEXT, UNIQUE(did, repository, digest), FOREIGN KEY(did) REFERENCES users(did) ON DELETE CASCADE ); +-- UNIQUE despite the column being nullable: SQLite treats NULLs as distinct, so +-- unfilled rows do not collide, while every filled row is checked. That turns +-- the 16-byte truncation from an assumption into something production data +-- verifies before anything depends on it as a key. +CREATE UNIQUE INDEX IF NOT EXISTS idx_manifests_manifest_key ON manifests(manifest_key); 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); diff --git a/pkg/appview/jetstream/backfill_batch_test.go b/pkg/appview/jetstream/backfill_batch_test.go index 4234703..723b0b0 100644 --- a/pkg/appview/jetstream/backfill_batch_test.go +++ b/pkg/appview/jetstream/backfill_batch_test.go @@ -27,19 +27,6 @@ func TestBatchCaptains_VerifiesHoldService(t *testing.T) { db := setupTestDB(t) defer db.Close() - execStatements(t, db, ` - CREATE TABLE 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, - successor TEXT, - updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP - ); - `) - realHold := "did:web:realhold.example.com" notAHold := "did:plc:notahold" unresolvable := "did:plc:unresolvable" diff --git a/pkg/appview/jetstream/processor_test.go b/pkg/appview/jetstream/processor_test.go index 079d8f3..be1a337 100644 --- a/pkg/appview/jetstream/processor_test.go +++ b/pkg/appview/jetstream/processor_test.go @@ -5,10 +5,10 @@ import ( "database/sql" "encoding/json" "fmt" - "strings" "testing" "time" + "atcr.io/pkg/appview/db" "atcr.io/pkg/atproto" "github.com/bluesky-social/indigo/atproto/identity" "github.com/bluesky-social/indigo/atproto/syntax" @@ -55,112 +55,28 @@ func holdIdentity(did, url string) *identity.Identity { } } -// execStatements splits a multi-statement SQL string and executes each statement individually. -// go-libsql does not support multi-statement Exec like mattn/go-sqlite3. -func execStatements(t *testing.T, db *sql.DB, schema string) { - t.Helper() - for stmt := range strings.SplitSeq(schema, ";") { - stmt = strings.TrimSpace(stmt) - if stmt == "" { - continue - } - if _, err := db.Exec(stmt); err != nil { - t.Fatalf("Failed to execute statement: %v\nSQL: %s", err, stmt) - } - } -} - -// setupTestDB creates an in-memory SQLite database for testing +// setupTestDB returns a database built from the real schema. +// +// This used to hand-maintain its own CREATE TABLE statements, which is the same +// drift problem TestSchemaMatchesMigrations exists to prevent, just moved into a +// test: the copy silently fell behind (it still had tags.id after that column +// was dropped, and lacked manifests.manifest_key) and only failed once a query +// happened to touch the difference. Using db.InitDB means schema changes cannot +// rot this file. func setupTestDB(t *testing.T) *sql.DB { - database, err := sql.Open("libsql", ":memory:") + database, err := db.InitDB(":memory:", db.LibsqlConfig{}) if err != nil { t.Fatalf("Failed to open test database: %v", err) } - // Create schema - schema := ` - CREATE TABLE users ( - did TEXT PRIMARY KEY, - handle TEXT NOT NULL, - pds_endpoint TEXT NOT NULL, - avatar TEXT, - default_hold_did TEXT, - oci_client TEXT DEFAULT '', - registry_domain TEXT DEFAULT '', - last_seen TIMESTAMP NOT NULL - ); - - CREATE TABLE 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', - subject_digest TEXT, - created_at TIMESTAMP NOT NULL, - UNIQUE(did, repository, digest) - ); - - CREATE TABLE 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 TABLE layers ( - manifest_id INTEGER NOT NULL, - digest TEXT NOT NULL, - size INTEGER NOT NULL, - media_type TEXT NOT NULL, - layer_index INTEGER NOT NULL, - annotations TEXT, - PRIMARY KEY(manifest_id, layer_index) - ); - - CREATE TABLE 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) - ); - - CREATE TABLE 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) - ); - - CREATE TABLE 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) - ); - ` - - execStatements(t, database, schema) - + // Foreign keys off, matching the hand-rolled schema this replaced. These + // tests insert manifests and tags for DIDs with no users row, exercising the + // processor rather than referential integrity, and libSQL enables foreign + // keys by default (unlike mattn). Exec, not QueryRow: unlike most libSQL + // PRAGMAs this one returns no rows, the same way schema.go sets it. + if _, err := database.Exec("PRAGMA foreign_keys = OFF"); err != nil { + t.Fatalf("Failed to disable foreign keys: %v", err) + } return database } @@ -779,40 +695,8 @@ func TestProcessRecord_RoutesCorrectly(t *testing.T) { db := setupTestDB(t) defer db.Close() - // Add missing tables for this test - execStatements(t, db, ` - CREATE TABLE 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) - ); - CREATE TABLE 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, - successor TEXT, - updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP - ); - CREATE TABLE 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) - ); - `) + // repo_pages, hold_captain_records and hold_crew_members come from the real + // schema now; setupTestDB creates them. // Register the hold DID so captain verification resolves locally instead // of hitting the network. @@ -886,19 +770,6 @@ func TestProcessCaptain_VerifiesHoldService(t *testing.T) { db := setupTestDB(t) defer db.Close() - execStatements(t, db, ` - CREATE TABLE 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, - successor TEXT, - updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP - ); - `) - holdDID := "did:web:realhold.example.com" userDID := "did:plc:notahold" atproto.SetDirectory(&fakeDirectory{byDID: map[string]*identity.Identity{ diff --git a/pkg/appview/server.go b/pkg/appview/server.go index 782db6f..9dbc623 100644 --- a/pkg/appview/server.go +++ b/pkg/appview/server.go @@ -201,6 +201,8 @@ func NewAppViewServer(cfg *Config, branding *BrandingOverrides) (*AppViewServer, slog.Info("Lease manager initialized", "component", "leases", "enabled", cfg.Leases.Enabled, "holder", s.Leases.HolderID()) + reportManifestKeyBackfill(s.Database) + // The cleanup worker starts later, once the hold authorizer exists: it // clears the crew denial backoffs as its first act and needs a handle on it. @@ -975,6 +977,30 @@ func (s *AppViewServer) waitForLeasedWorkers() { s.Leases.Wait(leaseDrainTimeout) } +// reportManifestKeyBackfill logs how much of manifests.manifest_key is still +// unfilled. +// +// The column is populated by the ordinary Jetstream backfill rather than by its +// migration, because the value is a truncated sha256 and SQLite cannot compute +// one. So there is no single moment at which it becomes complete, and the +// follow-up work that moves layers and manifest_references onto the key is only +// safe once this reaches zero. Logging it is how anyone finds out. +func reportManifestKeyBackfill(database *sql.DB) { + remaining, total, err := db.ManifestKeyBackfillProgress(database) + if err != nil { + slog.Warn("Could not check manifest_key backfill progress", "error", err) + return + } + if remaining == 0 { + slog.Info("manifest_key backfill complete", "manifests", total) + return + } + slog.Info("manifest_key backfill in progress", + "remaining", remaining, + "total", total, + "hint", "the Jetstream backfill fills these as it re-upserts each manifest") +} + // startCleanupWorker runs the periodic expiry sweep under the cleanup lease, // after clearing the crew denial backoffs once on acquiring it. //