Skip to content

[HUB-4300] Introduce Versioned Migration Flow#10

Draft
tholst-d4l wants to merge 15 commits intomainfrom
feature/migration-approaches
Draft

[HUB-4300] Introduce Versioned Migration Flow#10
tholst-d4l wants to merge 15 commits intomainfrom
feature/migration-approaches

Conversation

@tholst-d4l
Copy link
Copy Markdown
Collaborator

@tholst-d4l tholst-d4l commented Feb 6, 2026

https://gesundheitscloud.atlassian.net/browse/HUB-4300

Summary

This PR introduces a versioned migration flow alongside the legacy flow and updates migration documentation/diagrams. The new flow interleaves before -> AutoMigrate(version) -> after for each version, enabling complete, version-by-version migrations.

Background / Problem

Until now, go-svc only supported a single AutoMigrate pass plus SQL migrations. That made it hard to express a full migration path across multiple versions, especially when a specific version required changes before or after AutoMigrate. The new versioned flow makes this explicit and more intuitive by letting services define migrations per version with before/auto/after steps.

Changes

  • Add a versioned migration flow that interleaves per version:
    1. versioned before script (optional)
    2. AutoMigrate(version)
    3. versioned after script (optional)
    4. record version after the full sequence completes
  • Keep legacy flow unchanged for backward compatibility (legacy AutoMigrate once + golang-migrate .up.sql).
  • Support dual naming for versioned scripts:
    • {version}_{name}.before.sql or {version}_{name}.before.up.sql
    • {version}_{name}.after.sql or {version}_{name}.after.up.sql
    • If both variants exist for the same version/hook, we fail fast to avoid ambiguous behavior.
  • Add a guard: configuring both WithMigrationFunc and WithVersionedMigrationFunc errors out early.
  • Update docs and diagrams to reflect the new flow and naming.
  • Add unit + integration tests for migration flows.

Tests Added

  • Unit: dual naming discovery for .before.sql/.after.sql and .before.up.sql/.after.up.sql (incl. conflict handling).
  • Unit: version-recording semantics (startFromZero, dirty flag, ErrNilVersion handling).
  • Integration (Postgres): end-to-end versioned flow with before/auto/after ordering, dual naming, and missing-script behavior.

User Impact

Services can now opt into version-by-version migrations with explicit before/auto/after steps while legacy users remain unaffected.

Notes

Legacy flow continues to use .up.sql files with golang-migrate.

FYI (@weese + @LRueckert )

Contrary to what we previously sketched in our call: I kept the legacy branch and the versioned migration branch separate. We originally discussed implementing legacy support by having WithMigrationFunc set WithVersionedMigrationFunc via a wrapper, but we dropped that approach.

Rationale: the two flows have different semantics (legacy: AutoMigrate once + tracked .up.sql; versioned: per-version before/auto/after + version recording). Keeping them separate makes configuration explicit, preserves legacy behavior, and avoids subtle wrapper-induced regressions. We also guard against accidental misconfiguration by erroring if both functions are set.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Feb 6, 2026

Test Results

392 tests  +14   391 ✅ +14   0s ⏱️ ±0s
 10 suites ± 0     1 💤 ± 0 
  1 files   ± 0     0 ❌ ± 0 

Results for commit 3be7264. ± Comparison against base commit 1f0ff37.

♻️ This comment has been updated with latest results.

@tholst-d4l tholst-d4l force-pushed the feature/migration-approaches branch from 7090301 to 7ccb9b5 Compare February 6, 2026 13:31
- Restore legacy MigrationFunc + golang-migrate path alongside versioned flow

- Guard against configuring both legacy and versioned functions

- Tighten before/after script discovery (conflicts error)

- Lint/test fixes
@tholst-d4l tholst-d4l changed the title Refine versioned migration flow [HUB-4300] Introduce Versioned Migration Flow Feb 15, 2026
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel this doesn't warrant its own test file? Could just be a test case in the other test file.

Comment on lines +30 to +44
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if dirty {
t.Fatalf("expected dirty=false")
}
if needsRecordTarget {
t.Fatalf("expected needsRecordTarget=false")
}
if version != 0 {
t.Fatalf("expected version 0, got %d", version)
}
if len(setter.forced) != 0 {
t.Fatalf("expected no Force calls, got %v", setter.forced)
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again bringing up require and assert

Comment thread pkg/migrate/migrate.go
}

// CreateAfterSourceFolder returns a temp folder containing only non-before migrations.
func CreateAfterSourceFolder(sourceFolder string) (string, func(), error) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is never used, only tested. I assume an old approach?

Comment thread pkg/migrate/migrate.go
}

// CreateAfterSourceFolderForVersion returns a temp folder with the after migration for a single version.
func CreateAfterSourceFolderForVersion(sourceFolder string, migrationVersion uint) (string, func(), error) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also only tested, never used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants