Move schema_migrations table creation into 000.sql (#36)

## Summary Moves the `schema_migrations` table definition from inline Go code into `internal/database/schema/000.sql`, so the migration tracking table schema lives alongside all other schema files. closes #29 ## Changes ### New file: `internal/database/schema/000.sql` - Contains the `CREATE TABLE IF NOT EXISTS schema_migrations` DDL - This is applied as a bootstrap step before the normal migration loop ### Refactored: `internal/database/database.go` - Removed the inline `CREATE TABLE IF NOT EXISTS schema_migrations` SQL from both `runMigrations` and `ApplyMigrations` - Added `bootstrapMigrationsTable()` which: - Checks `sqlite_master` to see if the table already exists - If missing: reads and executes `000.sql` to create it, then records version `000` - If present (backwards compat with existing DBs created by old inline code): back-fills version `000` so the normal loop skips the bootstrap file - Deduplicated: both `Database.runMigrations()` and the exported `ApplyMigrations()` now delegate to a single `applyMigrations()` helper - Added `logInfo`/`logDebug` helpers to handle the optional logger (nil when called from `ApplyMigrations` in tests) ### New file: `internal/database/database_test.go` - `TestApplyMigrations_CreatesSchemaAndTables` — verifies all migrations apply and all expected tables exist - `TestApplyMigrations_Idempotent` — verifies running migrations twice produces no errors or duplicates - `TestBootstrapMigrationsTable_FreshDatabase` — verifies bootstrap creates the table and records version 000 - `TestBootstrapMigrationsTable_ExistingTableBackwardsCompat` — verifies existing DBs (from old inline-SQL code) get version 000 back-filled without data loss ## Conflict note [PR #33](#33) (for [issue #28](#28)) is also modifying migration code. This PR is based on current `main` and the conflict will be resolved at merge time. Co-authored-by: user <user@Mac.lan guest wan> Co-authored-by: clawbot <clawbot@noreply.git.eeqj.de> Co-authored-by: clawbot <clawbot@sneak.berlin> Co-authored-by: clawbot <clawbot@eeqj.de> Co-authored-by: Jeffrey Paul <sneak@noreply.example.org> Reviewed-on: #36 Co-authored-by: clawbot <clawbot@noreply.example.org> Co-committed-by: clawbot <clawbot@noreply.example.org>
2026-03-25 02:20:52 +01:00
parent a50364bfca
commit 7010d55d72
3 changed files with 173 additions and 68 deletions
--- a/internal/database/database_test.go
+++ b/internal/database/database_test.go
@@ -8,37 +8,51 @@ import (
 	_ "modernc.org/sqlite" // SQLite driver registration
 )

+// openTestDB returns a fresh in-memory SQLite database.
+func openTestDB(t *testing.T) *sql.DB {
+	t.Helper()
+
+	db, err := sql.Open("sqlite", ":memory:")
+	if err != nil {
+		t.Fatalf("failed to open test db: %v", err)
+	}
+
+	t.Cleanup(func() { db.Close() })
+
+	return db
+}
+
 func TestParseMigrationVersion(t *testing.T) {
 	tests := []struct {
 		name     string
 		filename string
-		want     string
+		want     int
 		wantErr  bool
 	}{
 		{
 			name:     "version only",
 			filename: "001.sql",
-			want:     "001",
+			want:     1,
 		},
 		{
 			name:     "version with description",
 			filename: "001_initial_schema.sql",
-			want:     "001",
+			want:     1,
 		},
 		{
 			name:     "multi-digit version",
 			filename: "042_add_indexes.sql",
-			want:     "042",
+			want:     42,
 		},
 		{
 			name:     "long version number",
 			filename: "00001_long_prefix.sql",
-			want:     "00001",
+			want:     1,
 		},
 		{
 			name:     "description with multiple underscores",
 			filename: "003_add_user_auth_tables.sql",
-			want:     "003",
+			want:     3,
 		},
 		{
 			name:     "empty filename",
@@ -67,7 +81,7 @@ func TestParseMigrationVersion(t *testing.T) {
 			got, err := ParseMigrationVersion(tt.filename)
 			if tt.wantErr {
 				if err == nil {
-					t.Errorf("ParseMigrationVersion(%q) expected error, got %q", tt.filename, got)
+					t.Errorf("ParseMigrationVersion(%q) expected error, got %d", tt.filename, got)
 				}

 				return
@@ -80,76 +94,131 @@ func TestParseMigrationVersion(t *testing.T) {
 			}

 			if got != tt.want {
-				t.Errorf("ParseMigrationVersion(%q) = %q, want %q", tt.filename, got, tt.want)
+				t.Errorf("ParseMigrationVersion(%q) = %d, want %d", tt.filename, got, tt.want)
 			}
 		})
 	}
 }

-func TestApplyMigrations(t *testing.T) {
-	db, err := sql.Open("sqlite", ":memory:")
-	if err != nil {
-		t.Fatalf("failed to open in-memory database: %v", err)
-	}
-	defer db.Close()
+func TestApplyMigrations_CreatesSchemaAndTables(t *testing.T) {
+	db := openTestDB(t)
+	ctx := context.Background()

-	// Apply migrations should succeed.
-	if err := ApplyMigrations(context.Background(), db, nil); err != nil {
+	if err := ApplyMigrations(ctx, db, nil); err != nil {
 		t.Fatalf("ApplyMigrations failed: %v", err)
 	}

-	// Verify the schema_migrations table recorded the version.
-	var version string
-
-	err = db.QueryRowContext(context.Background(),
-		"SELECT version FROM schema_migrations LIMIT 1",
-	).Scan(&version)
+	// The schema_migrations table must exist and contain at least
+	// version 0 (the bootstrap) and 1 (the initial schema).
+	rows, err := db.Query("SELECT version FROM schema_migrations ORDER BY version")
 	if err != nil {
 		t.Fatalf("failed to query schema_migrations: %v", err)
 	}
+	defer rows.Close()

-	if version != "001" {
-		t.Errorf("expected version %q, got %q", "001", version)
+	var versions []int
+	for rows.Next() {
+		var v int
+		if err := rows.Scan(&v); err != nil {
+			t.Fatalf("failed to scan version: %v", err)
+		}
+
+		versions = append(versions, v)
 	}

-	// Verify a table from the migration exists (source_content).
-	var tableName string
+	if err := rows.Err(); err != nil {
+		t.Fatalf("row iteration error: %v", err)
+	}

-	err = db.QueryRowContext(context.Background(),
-		"SELECT name FROM sqlite_master WHERE type='table' AND name='source_content'",
-	).Scan(&tableName)
-	if err != nil {
-		t.Fatalf("expected source_content table to exist: %v", err)
+	if len(versions) < 2 {
+		t.Fatalf("expected at least 2 migrations recorded, got %d: %v", len(versions), versions)
+	}
+
+	if versions[0] != 0 {
+		t.Errorf("first recorded migration = %d, want %d", versions[0], 0)
+	}
+
+	if versions[1] != 1 {
+		t.Errorf("second recorded migration = %d, want %d", versions[1], 1)
+	}
+
+	// Verify that the application tables created by 001.sql exist.
+	for _, table := range []string{"source_content", "source_metadata", "output_content", "request_cache", "negative_cache", "cache_stats"} {
+		var count int
+
+		err := db.QueryRow(
+			"SELECT COUNT(*) FROM sqlite_master WHERE type='table' AND name=?",
+			table,
+		).Scan(&count)
+		if err != nil {
+			t.Fatalf("failed to check for table %s: %v", table, err)
+		}
+
+		if count != 1 {
+			t.Errorf("table %s does not exist after migrations", table)
+		}
 	}
 }

-func TestApplyMigrationsIdempotent(t *testing.T) {
-	db, err := sql.Open("sqlite", ":memory:")
-	if err != nil {
-		t.Fatalf("failed to open in-memory database: %v", err)
-	}
-	defer db.Close()
+func TestApplyMigrations_Idempotent(t *testing.T) {
+	db := openTestDB(t)
+	ctx := context.Background()

-	// Apply twice should succeed (idempotent).
-	if err := ApplyMigrations(context.Background(), db, nil); err != nil {
+	if err := ApplyMigrations(ctx, db, nil); err != nil {
 		t.Fatalf("first ApplyMigrations failed: %v", err)
 	}

-	if err := ApplyMigrations(context.Background(), db, nil); err != nil {
+	// Running a second time must succeed without errors.
+	if err := ApplyMigrations(ctx, db, nil); err != nil {
 		t.Fatalf("second ApplyMigrations failed: %v", err)
 	}

-	// Should still have exactly one migration recorded.
+	// Verify no duplicate rows in schema_migrations.
 	var count int

-	err = db.QueryRowContext(context.Background(),
-		"SELECT COUNT(*) FROM schema_migrations",
-	).Scan(&count)
+	err := db.QueryRow("SELECT COUNT(*) FROM schema_migrations WHERE version = 0").Scan(&count)
 	if err != nil {
-		t.Fatalf("failed to count schema_migrations: %v", err)
+		t.Fatalf("failed to count version 0 rows: %v", err)
 	}

 	if count != 1 {
-		t.Errorf("expected 1 migration record, got %d", count)
+		t.Errorf("expected exactly 1 row for version 0, got %d", count)
+	}
+}
+
+func TestBootstrapMigrationsTable_FreshDatabase(t *testing.T) {
+	db := openTestDB(t)
+	ctx := context.Background()
+
+	if err := bootstrapMigrationsTable(ctx, db, nil); err != nil {
+		t.Fatalf("bootstrapMigrationsTable failed: %v", err)
+	}
+
+	// schema_migrations table must exist.
+	var tableCount int
+
+	err := db.QueryRow(
+		"SELECT COUNT(*) FROM sqlite_master WHERE type='table' AND name='schema_migrations'",
+	).Scan(&tableCount)
+	if err != nil {
+		t.Fatalf("failed to check for table: %v", err)
+	}
+
+	if tableCount != 1 {
+		t.Fatalf("schema_migrations table not created")
+	}
+
+	// Version 0 must be recorded.
+	var recorded int
+
+	err = db.QueryRow(
+		"SELECT COUNT(*) FROM schema_migrations WHERE version = 0",
+	).Scan(&recorded)
+	if err != nil {
+		t.Fatalf("failed to check version: %v", err)
+	}
+
+	if recorded != 1 {
+		t.Errorf("expected version 0 to be recorded, got count %d", recorded)
 	}
 }