db: cover the orphan drop in 0034, the case production actually presents

TestMigration0034PreservesLayersAndReferences seeds no orphan and says why: the live foreign key refuses to create one, so the migration's join through manifest_id is "insurance for a database whose foreign keys were off at some point, not for anything reachable now". Production is that database. It carries 160 layers and 22 manifest_references pointing at manifests.id values that no longer exist, left by deletes performed under mattn/go-sqlite3, where the constraint the DDL declared was not enforced. libSQL turns foreign keys on by default and mattn did not, so the rows predate the driver swap. The insurance is load-bearing on the only database that matters, and nothing tested it. The new case rebuilds the pre-0034 shape with the child foreign keys absent, which is what that era's schema behaved like, and seeds three orphaned layers and two orphaned references beside live ones. It asserts the orphans are gone, the live rows survive attached to the right key, nothing lands keyless, and foreign_key_check is clean afterwards. Verified against the defect: with the child manifest_key made nullable and the joins turned into LEFT JOINs, all five orphans survive and the test fails on both counts. Recorded honestly, the two guards are redundant with each other — LEFT JOIN alone still drops them, because INSERT OR IGNORE swallows the NOT NULL violation. Only removing both carries an orphan forward, and such a row counts, selects, and joins to no manifest ever again. Confirmed on a copy of the production database: layers 18803 -> 18643 and manifest_references 2175 -> 2153, exactly the rows the join excludes, with manifests unchanged at 3864. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011rmjvU2gSRL9wFnmqVsWaF
2026-09-02 00:06:58 +00:00 · 2026-08-25 16:34:26 -05:00
parent 8c9a85826d
commit ff0942196e
1 changed files with 181 additions and 0 deletions
@@ -375,3 +375,184 @@ func seedPublicHold(t *testing.T, database *sql.DB) {
 		t.Fatalf("seed hold: %v", err)
 	}
 }
+
+// reshapeToPre0034Unenforced builds the same pre-0034 shape as
+// reshapeToPre0034WithChildren, but without the foreign keys on the two child
+// tables. That is the state a real database can be in: libSQL enables foreign
+// keys by default, mattn/go-sqlite3 did not, and rows written under the old
+// driver were never checked against the constraint the DDL declared.
+func reshapeToPre0034Unenforced(t *testing.T, database *sql.DB) {
+	t.Helper()
+	stmts := []string{
+		`DROP TABLE layers`,
+		`DROP TABLE manifest_references`,
+		`DROP TABLE manifests`,
+		`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,
+			manifest_key TEXT,
+			UNIQUE(did, repository, digest),
+			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)
+		)`,
+	}
+	for _, stmt := range stmts {
+		if _, err := database.Exec(stmt); err != nil {
+			t.Fatalf("reshape: %v\n%s", err, stmt)
+		}
+	}
+}
+
+// TestMigration0034DropsOrphanedChildren covers the case the sibling test
+// cannot reach, and the one the production database actually presents.
+//
+// The sibling seeds no orphan because the live foreign key refuses to create
+// one, and reasons that the migration's join is "insurance for a database whose
+// foreign keys were off at some point". Production is exactly that database:
+// 160 layers and 22 manifest_references there point at manifests.id values that
+// no longer exist, left behind by deletes performed under mattn/go-sqlite3
+// where the constraint was not enforced. So the insurance is load-bearing on
+// the only database that matters, and nothing tested it.
+//
+// What must hold is narrow and total: orphans are discarded, live rows survive
+// attached to the right parent, and nothing lands with an empty key. That last
+// assertion is the one with teeth — carrying an orphan forward under a nullable
+// key produces a row that satisfies every count and joins to nothing.
+func TestMigration0034DropsOrphanedChildren(t *testing.T) {
+	database := revTestDB(t)
+	reshapeToPre0034Unenforced(t, database)
+
+	const did = "did:plc:orphans"
+	if _, err := database.Exec(
+		`INSERT INTO users (did, handle, pds_endpoint, last_seen) VALUES (?, ?, ?, ?)`,
+		did, "orphans.example.com", "https://pds.example.com", time.Now(),
+	); err != nil {
+		t.Fatalf("seed user: %v", err)
+	}
+
+	res, err := database.Exec(`
+		INSERT INTO manifests (did, repository, digest, hold_endpoint, schema_version, media_type, artifact_type, created_at, manifest_key)
+		VALUES (?, 'myapp', 'sha256:live', 'did:web:hold', 2, 'application/vnd.oci.image.manifest.v1+json', 'container-image', ?, ?)
+	`, did, time.Now(), ManifestKey(did, "myapp", "sha256:live"))
+	if err != nil {
+		t.Fatalf("seed manifest: %v", err)
+	}
+	liveID, err := res.LastInsertId()
+	if err != nil {
+		t.Fatalf("last insert id: %v", err)
+	}
+
+	for i := range 2 {
+		if _, err := database.Exec(`
+			INSERT INTO layers (manifest_id, digest, size, media_type, layer_index)
+			VALUES (?, 'sha256:livelayer', 100, 'application/vnd.oci.image.layer.v1.tar+gzip', ?)
+		`, liveID, i); err != nil {
+			t.Fatalf("seed live layer: %v", err)
+		}
+	}
+	if _, err := database.Exec(`
+		INSERT INTO manifest_references (manifest_id, digest, media_type, size, platform_architecture, platform_os, reference_index)
+		VALUES (?, 'sha256:liveref', 'application/vnd.oci.image.manifest.v1+json', 500, 'amd64', 'linux', 0)
+	`, liveID); err != nil {
+		t.Fatalf("seed live reference: %v", err)
+	}
+
+	// The orphans: children of a manifest id that does not exist.
+	const goneID = 999999
+	for i := range 3 {
+		if _, err := database.Exec(`
+			INSERT INTO layers (manifest_id, digest, size, media_type, layer_index)
+			VALUES (?, 'sha256:orphanlayer', 100, 'application/vnd.oci.image.layer.v1.tar+gzip', ?)
+		`, goneID, i); err != nil {
+			t.Fatalf("seed orphan layer: %v", err)
+		}
+	}
+	for i := range 2 {
+		if _, err := database.Exec(`
+			INSERT INTO manifest_references (manifest_id, digest, media_type, size, platform_architecture, platform_os, reference_index)
+			VALUES (?, 'sha256:orphanref', 'application/vnd.oci.image.manifest.v1+json', 500, 'amd64', 'linux', ?)
+		`, goneID, i); err != nil {
+			t.Fatalf("seed orphan reference: %v", err)
+		}
+	}
+
+	if got := countRows(t, database, `SELECT COUNT(*) FROM layers`); got != 5 {
+		t.Fatalf("fixture: layer count = %d, want 5 (2 live + 3 orphaned)", got)
+	}
+
+	applyMigration(t, database, 34)
+
+	liveKey := ManifestKey(did, "myapp", "sha256:live")
+
+	if got := countRows(t, database, `SELECT COUNT(*) FROM layers`); got != 2 {
+		t.Errorf("layer count = %d, want 2: the three orphans must not survive", got)
+	}
+	if got := countRows(t, database, `SELECT COUNT(*) FROM layers WHERE manifest_key = ?`, liveKey); got != 2 {
+		t.Errorf("layers attached to the live manifest = %d, want 2", got)
+	}
+	if got := countRows(t, database, `SELECT COUNT(*) FROM manifest_references`); got != 1 {
+		t.Errorf("manifest_references count = %d, want 1: both orphans must not survive", got)
+	}
+	if got := countRows(t, database, `SELECT COUNT(*) FROM manifest_references WHERE manifest_key = ?`, liveKey); got != 1 {
+		t.Errorf("references attached to the live manifest = %d, want 1", got)
+	}
+
+	// Nothing may land keyless. A carried-forward orphan under a nullable key
+	// still counts, still selects, and joins to no manifest ever again.
+	if got := countRows(t, database,
+		`SELECT COUNT(*) FROM layers WHERE manifest_key IS NULL OR manifest_key = ''`); got != 0 {
+		t.Errorf("%d layers have no manifest_key", got)
+	}
+	if got := countRows(t, database,
+		`SELECT COUNT(*) FROM manifest_references WHERE manifest_key IS NULL OR manifest_key = ''`); got != 0 {
+		t.Errorf("%d manifest_references have no manifest_key", got)
+	}
+
+	// And the rebuild must leave the database referentially clean.
+	rows, err := database.Query(`PRAGMA foreign_key_check`)
+	if err != nil {
+		t.Fatalf("foreign_key_check: %v", err)
+	}
+	defer rows.Close()
+	var violations int
+	for rows.Next() {
+		violations++
+	}
+	if err := rows.Err(); err != nil {
+		t.Fatalf("foreign_key_check rows: %v", err)
+	}
+	if violations != 0 {
+		t.Errorf("foreign_key_check reports %d violations after the swap", violations)
+	}
+}