sqldb/v2: limit MigrationExecutor interface

This commit limits the MigrationExecutor interface due to the following
reasoning:

1. SkipMigrations() and DefaultTarget() should not be on the interface
Both are only used by ApplyAllMigrations, which immediately passes the
results back into the same executor. They are internal implementation
details and should be folded into ExecuteMigrations itself.

2. SetSchemaVersion and GetSchemaVersion are test-only but on the
production interface Every caller of these in sqldb/v2 is in test files.
The SetSchemaVersion comment even says "USE WITH CAUTION" — dangerous
test utilities should not be on an interface that every real consumer
must implement. They should be accessible on the concrete types only and
used directly in tests without going through the interface.

3. ExecuteMigrations should not take a MigrationTarget parameter for the
normal path On the normal startup path, callers just do
executor.ExecuteMigrations(executor.DefaultTarget(), stream) — asking
the executor for its default and handing it straight back. The method
should run to latest by default; a version override for tests can live
on the concrete type instead.

Author: ViktorT-11

sqldb/v2/migrations.go

@@ -96,29 +96,11 @@ type MigrationTarget func(mig *migrate.Migrate,
 
 // MigrationExecutor is an interface that abstracts the migration functionality.
 type MigrationExecutor interface {
-	// ExecuteMigrations runs database migrations up to the specified target
-	// version or all migrations if no target is specified. A migration may
-	// include a schema change, a custom migration function, or both.
-	// Developers must ensure that migrations are defined in the correct
-	// order. Migration details are stored in the global variable
-	// migrationConfig.
-	ExecuteMigrations(target MigrationTarget, set MigrationSet) error
-
-	// GetSchemaVersion returns the current schema version of the database.
-	GetSchemaVersion() (int, bool, error)
-
-	// SetSchemaVersion sets the schema version of the database.
-	//
-	// NOTE: This alters the internal database schema tracker. USE WITH
-	// CAUTION!!!
-	SetSchemaVersion(version int, dirty bool) error
-
-	// DefaultTarget returns the default migration target.
-	DefaultTarget() MigrationTarget
-
-	// SkipMigrations indicates if the SQL and corresponding code migrations
-	// will be skipped.
-	SkipMigrations() bool
+	// ExecuteMigrations runs database migrations for the given migration
+	// set using the executor's default production migration target. A
+	// migration may include a schema change, a custom migration function,
+	// or both.
+	ExecuteMigrations(set MigrationSet) error
 }
 
 var (
@@ -403,13 +385,8 @@ func (t *replacerFile) Close() error {
 // ApplyAllMigrations applies both the SQLC and custom in-code migrations to the
 // SQLite database.
 func ApplyAllMigrations(executor MigrationExecutor, sets []MigrationSet) error {
-	// Execute migrations unless configured to skip them.
-	if executor.SkipMigrations() {
-		return nil
-	}
-
 	for _, set := range sets {
-		err := executor.ExecuteMigrations(executor.DefaultTarget(), set)
+		err := executor.ExecuteMigrations(set)
 		if err != nil {
 			return fmt.Errorf("error applying migrations: %w", err)
 		}

sqldb/v2/postgres.go

@@ -167,9 +167,19 @@ func errPostgresMigration(err error) error {
 	return fmt.Errorf("error creating postgres migration: %w", err)
 }
 
-// ExecuteMigrations runs migrations for the Postgres database, depending on the
-// target given, either all migrations or up to a given version.
-func (s *PostgresStore) ExecuteMigrations(target MigrationTarget,
+// ExecuteMigrations runs migrations for the Postgres database using the
+// default production migration target.
+func (s *PostgresStore) ExecuteMigrations(set MigrationSet) error {
+	if s.cfg.SkipMigrations {
+		return nil
+	}
+
+	return s.executeMigrations(TargetLatest, set)
+}
+
+// executeMigrations runs migrations for the Postgres database, depending on
+// the target given, either all migrations or up to a given version.
+func (s *PostgresStore) executeMigrations(target MigrationTarget,
 	set MigrationSet) error {
 
 	dbName, err := getDatabaseNameFromDSN(s.cfg.Dsn)
@@ -231,11 +241,3 @@ func (s *PostgresStore) SetSchemaVersion(version int, dirty bool) error {
 
 	return driver.SetVersion(version, dirty)
 }
-
-func (s *PostgresStore) DefaultTarget() MigrationTarget {
-	return TargetLatest
-}
-
-func (s *PostgresStore) SkipMigrations() bool {
-	return s.cfg.SkipMigrations
-}

sqldb/v2/postgres_fixture.go

@@ -197,7 +197,7 @@ func NewTestPostgresDBWithVersion(t testing.TB, fixture *TestPgFixture,
 	store, err := NewPostgresStore(storeCfg)
 	require.NoError(t, err)
 
-	err = store.ExecuteMigrations(TargetVersion(version), sets)
+	err = store.executeMigrations(TargetVersion(version), sets)
 	require.NoError(t, err)
 
 	t.Cleanup(func() {

sqldb/v2/sqlite.go

@@ -251,9 +251,19 @@ func (s *SqliteStore) backupAndMigrate(mig *migrate.Migrate,
 	return mig.Up()
 }
 
-// ExecuteMigrations runs migrations for the sqlite database, depending on the
+// ExecuteMigrations runs migrations for the sqlite database using the default
+// production migration target.
+func (s *SqliteStore) ExecuteMigrations(set MigrationSet) error {
+	if s.Config.SkipMigrations {
+		return nil
+	}
+
+	return s.executeMigrations(s.backupAndMigrate, set)
+}
+
+// executeMigrations runs migrations for the sqlite database, depending on the
 // target given, either all migrations or up to a given version.
-func (s *SqliteStore) ExecuteMigrations(target MigrationTarget,
+func (s *SqliteStore) executeMigrations(target MigrationTarget,
 	set MigrationSet) error {
 
 	driver, err := sqlite_migrate.WithInstance(
@@ -285,14 +295,6 @@ func (s *SqliteStore) ExecuteMigrations(target MigrationTarget,
 	)
 }
 
-func (s *SqliteStore) DefaultTarget() MigrationTarget {
-	return s.backupAndMigrate
-}
-
-func (s *SqliteStore) SkipMigrations() bool {
-	return s.Config.SkipMigrations
-}
-
 // GetSchemaVersion returns the current schema version of the SQLite database.
 func (s *SqliteStore) GetSchemaVersion() (int, bool, error) {
 	driver, err := sqlite_migrate.WithInstance(

sqldb/v2/sqlite_test_utils.go

@@ -75,7 +75,7 @@ func NewTestSqliteDBWithVersion(t *testing.T, set MigrationSet,
 	}, dbFileName)
 	require.NoError(t, err)
 
-	err = sqlDB.ExecuteMigrations(TargetVersion(version), set)
+	err = sqlDB.executeMigrations(TargetVersion(version), set)
 	require.NoError(t, err)
 
 	t.Cleanup(func() {