From 0f867595c564d97f4c0bfea48751a25c48483c5f Mon Sep 17 00:00:00 2001 From: Evan Jarrett Date: Thu, 9 Oct 2025 10:54:03 -0500 Subject: [PATCH] fix db migration logic --- docker-compose.yml | 4 +- pkg/appview/db/migrations/0001_example.yaml | 3 + ...move_star_count_from_repository_stats.yaml | 8 -- pkg/appview/db/migrations/README.md | 40 ++++++---- pkg/appview/db/schema.go | 78 ++++++++++++------- 5 files changed, 80 insertions(+), 53 deletions(-) create mode 100644 pkg/appview/db/migrations/0001_example.yaml delete mode 100644 pkg/appview/db/migrations/0001_remove_star_count_from_repository_stats.yaml diff --git a/docker-compose.yml b/docker-compose.yml index 983db20..75f34ad 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -55,8 +55,8 @@ services: container_name: atcr-hold ports: - "8080:8080" - volumes: - - atcr-hold:/var/lib/atcr/hold + # volumes: + # - atcr-hold:/var/lib/atcr/hold restart: unless-stopped dns: - 8.8.8.8 diff --git a/pkg/appview/db/migrations/0001_example.yaml b/pkg/appview/db/migrations/0001_example.yaml new file mode 100644 index 0000000..24b093b --- /dev/null +++ b/pkg/appview/db/migrations/0001_example.yaml @@ -0,0 +1,3 @@ +description: Example migrarion query +query: | + SELECT COUNT(*) FROM schema_migrations; \ No newline at end of file diff --git a/pkg/appview/db/migrations/0001_remove_star_count_from_repository_stats.yaml b/pkg/appview/db/migrations/0001_remove_star_count_from_repository_stats.yaml deleted file mode 100644 index e929b40..0000000 --- a/pkg/appview/db/migrations/0001_remove_star_count_from_repository_stats.yaml +++ /dev/null @@ -1,8 +0,0 @@ -version: 1 -name: remove_star_count_from_repository_stats -up: | - -- Drop star_count column if it exists (SQLite 3.35.0+) - ALTER TABLE repository_stats DROP COLUMN IF EXISTS star_count; - - -- Drop the old star_count index if it exists - DROP INDEX IF EXISTS idx_repository_stats_star_count; diff --git a/pkg/appview/db/migrations/README.md b/pkg/appview/db/migrations/README.md index 5c23bfb..6d05b9f 100644 --- a/pkg/appview/db/migrations/README.md +++ b/pkg/appview/db/migrations/README.md @@ -7,45 +7,53 @@ This directory contains database migrations for the ATCR AppView database. Each migration is a YAML file with the following structure: ```yaml -version: 1 -name: descriptive_migration_name -up: | +description: Optional human-readable description of what this migration does +query: | SQL commands to apply the migration ``` +**Version and name are parsed from the filename**, so you don't need to specify them in the YAML. + ## Naming Convention -Migration files should be named: `{version:04d}_{name}.yaml` +Migration files **must** be named: `{version:04d}_{migration_name}.yaml` + +The filename determines: +- **Version**: Numeric prefix (e.g., `0001` → version 1) +- **Name**: Everything after first underscore (e.g., `add_repository_labels` → "add repository labels") Examples: -- `0001_remove_star_count_from_repository_stats.yaml` -- `0002_add_repository_labels.yaml` -- `0003_create_webhooks_table.yaml` +- `0001_remove_star_count_from_repository_stats.yaml` → version 1, name "remove star count from repository stats" +- `0002_add_repository_labels.yaml` → version 2, name "add repository labels" +- `0003_create_webhooks_table.yaml` → version 3, name "create webhooks table" ## Creating a New Migration 1. **Choose the next version number** - Look at existing migrations and increment by 1 -2. **Create a new YAML file** with the naming convention above -3. **Write your SQL** - Use the `|` block scalar for clean multi-line SQL -4. **Use `IF EXISTS` / `IF NOT EXISTS`** where possible for idempotency +2. **Create a new YAML file** with format `000N_descriptive_name.yaml` +3. **Add description** (optional) - Explain what the migration does +4. **Write your SQL in `query`** - Use the `|` block scalar for clean multi-line SQL +5. **Use `IF EXISTS` / `IF NOT EXISTS`** where possible for idempotency (note: not supported for `DROP COLUMN`) ## Examples ### Simple single-statement migration: +Filename: `0002_add_repository_description_index.yaml` + ```yaml -version: 2 -name: add_repository_description_index -up: | +description: Add index on manifests description field for faster searches +query: | CREATE INDEX IF NOT EXISTS idx_manifests_description ON manifests(description); ``` ### Complex multi-statement migration: +Filename: `0003_create_webhooks_table.yaml` + ```yaml -version: 3 -name: create_webhooks_table -up: | +description: Create webhooks table for repository event notifications +query: | -- Create webhooks table CREATE TABLE IF NOT EXISTS webhooks ( id INTEGER PRIMARY KEY AUTOINCREMENT, diff --git a/pkg/appview/db/schema.go b/pkg/appview/db/schema.go index 372e4af..601f884 100644 --- a/pkg/appview/db/schema.go +++ b/pkg/appview/db/schema.go @@ -2,15 +2,21 @@ package db import ( "database/sql" + "embed" "fmt" - "os" + "io/fs" "path/filepath" "sort" + "strconv" + "strings" _ "github.com/mattn/go-sqlite3" "go.yaml.in/yaml/v4" ) +//go:embed migrations/*.yaml +var migrationsFS embed.FS + const schema = ` CREATE TABLE IF NOT EXISTS schema_migrations ( version INTEGER PRIMARY KEY, @@ -204,9 +210,10 @@ func InitDB(path string) (*sql.DB, error) { // Migration represents a database migration type Migration struct { - Version int `yaml:"version"` - Name string `yaml:"name"` - Up string `yaml:"up"` + Version int + Name string + Description string `yaml:"description"` + Query string `yaml:"query"` } // runMigrations applies any pending database migrations @@ -236,8 +243,8 @@ func runMigrations(db *sql.DB) error { } // Apply migration - fmt.Printf("Applying migration %d: %s\n", m.Version, m.Name) - if _, err := db.Exec(m.Up); err != nil { + fmt.Printf("Applying migration %d: %s\n%s\n", m.Version, m.Name, m.Description) + if _, err := db.Exec(m.Query); err != nil { return fmt.Errorf("failed to apply migration %d (%s): %w", m.Version, m.Name, err) } @@ -252,25 +259,25 @@ func runMigrations(db *sql.DB) error { return nil } -// loadMigrations loads all migration files from the migrations directory +// loadMigrations loads all migration files from embedded filesystem func loadMigrations() ([]Migration, error) { - // Get the path to the migrations directory - // Try relative to working directory first, then relative to this file - migrationsDir := "pkg/appview/db/migrations" - if _, err := os.Stat(migrationsDir); os.IsNotExist(err) { - // Try embedded path (when running from different directory) - migrationsDir = filepath.Join(".", "migrations") - } - - // Read all .yaml files in the migrations directory - files, err := filepath.Glob(filepath.Join(migrationsDir, "*.yaml")) + // Read all migration files from embedded FS + entries, err := fs.Glob(migrationsFS, "migrations/[0-9][0-9][0-9][0-9]_*.yaml") if err != nil { return nil, fmt.Errorf("failed to list migration files: %w", err) } var migrations []Migration - for _, file := range files { - data, err := os.ReadFile(file) + for _, file := range entries { + // Parse version and name from filename + basename := filepath.Base(file) + version, name, err := parseMigrationFilename(basename) + if err != nil { + return nil, fmt.Errorf("invalid migration filename %s: %w", basename, err) + } + + // Read YAML content from embedded FS + data, err := migrationsFS.ReadFile(file) if err != nil { return nil, fmt.Errorf("failed to read migration file %s: %w", file, err) } @@ -280,15 +287,13 @@ func loadMigrations() ([]Migration, error) { return nil, fmt.Errorf("failed to parse migration file %s: %w", file, err) } + // Set version and name from filename + m.Version = version + m.Name = name + // Validate migration - if m.Version <= 0 { - return nil, fmt.Errorf("invalid migration version in %s: %d", file, m.Version) - } - if m.Name == "" { - return nil, fmt.Errorf("missing migration name in %s", file) - } - if m.Up == "" { - return nil, fmt.Errorf("missing migration 'up' SQL in %s", file) + if m.Query == "" { + return nil, fmt.Errorf("missing migration 'query' in %s", file) } migrations = append(migrations, m) @@ -296,3 +301,22 @@ func loadMigrations() ([]Migration, error) { return migrations, nil } + +// parseMigrationFilename extracts version and name from migration filename +// Expected format: 0001_migration_name.yaml +// Returns: version (int), name (string), error +// Note: Glob pattern ensures format is valid, so minimal validation needed +func parseMigrationFilename(filename string) (int, string, error) { + // Remove extension (.yaml or .yml) + ext := filepath.Ext(filename) + fileNameWithoutExt := filename[:len(filename)-len(ext)] + + // First 4 characters are the version (glob guarantees they're digits) + version, _ := strconv.Atoi(fileNameWithoutExt[:4]) + + // Remainder after position 5 is the name (glob guarantees it exists) + name := strings.ReplaceAll(fileNameWithoutExt[5:], "_", " ") + name = strings.TrimSpace(name) + + return version, name, nil +}