diff --git a/internal/dbtest/inspect_test.go b/internal/dbtest/inspect_test.go index 943846106..52afcbe15 100644 --- a/internal/dbtest/inspect_test.go +++ b/internal/dbtest/inspect_test.go @@ -537,7 +537,7 @@ func TestBunModelInspector_Inspect(t *testing.T) { Key: "name", Value: &sqlschema.BaseColumn{ SQLType: sqltype.VarChar, - DefaultValue: "'John Doe'", + DefaultValue: "John Doe", }, }, )) diff --git a/internal/dbtest/migrate_test.go b/internal/dbtest/migrate_test.go index 0705aec30..39b5ec787 100644 --- a/internal/dbtest/migrate_test.go +++ b/internal/dbtest/migrate_test.go @@ -254,8 +254,8 @@ func TestAutoMigrator_CreateSQLMigrations(t *testing.T) { require.Len(t, migrations, 2, "expected up/down migration pair") require.DirExists(t, migrationsDir) - checkMigrationFileContains(t, ".up.sql", "CREATE TABLE") - checkMigrationFileContains(t, ".down.sql", "DROP TABLE") + checkMigrationFileContains(t, "_auto.up.sql", "CREATE TABLE") + checkMigrationFileContains(t, "_auto.down.sql", "DROP TABLE") }) t.Run("transactional", func(t *testing.T) { @@ -264,8 +264,8 @@ func TestAutoMigrator_CreateSQLMigrations(t *testing.T) { require.Len(t, migrations, 2, "expected up/down migration pair") require.DirExists(t, migrationsDir) - checkMigrationFileContains(t, "tx.up.sql", "CREATE TABLE", "SET statement_timeout = 0") - checkMigrationFileContains(t, "tx.down.sql", "DROP TABLE", "SET statement_timeout = 0") + checkMigrationFileContains(t, "_auto.tx.up.sql", "CREATE TABLE", "SET statement_timeout = 0") + checkMigrationFileContains(t, "_auto.tx.down.sql", "DROP TABLE", "SET statement_timeout = 0") }) }) @@ -291,7 +291,7 @@ func checkMigrationFileContains(t *testing.T, fileSuffix string, snippets ...str t.Errorf("no *%s file in migrations directory (%s)", fileSuffix, migrationsDir) } -// checkMigrationFilesExist makes sure both up- and down- SQL migration files were created. +// checkMigrationFilesExist checks both up- and down- SQL migration files were created. func checkMigrationFilesExist(t *testing.T) { t.Helper() @@ -315,6 +315,7 @@ func checkMigrationFilesExist(t *testing.T) { } } +// runMigrations is a test helper to run AutoMigrator.Migrate(), check that it completed without error and migration files were created. func runMigrations(t *testing.T, m *migrate.AutoMigrator) { t.Helper() @@ -338,6 +339,7 @@ func TestAutoMigrator_Migrate(t *testing.T) { {testUnique}, {testUniqueRenamedTable}, {testUpdatePrimaryKeys}, + {testNothingToMigrate}, } testEachDB(t, func(t *testing.T, dbName string, db *bun.DB) { @@ -1103,3 +1105,24 @@ func testUpdatePrimaryKeys(t *testing.T, db *bun.DB) { state := inspect(ctx) cmpTables(t, db.Dialect().(sqlschema.InspectorDialect), wantTables, state.Tables) } + +func testNothingToMigrate(t *testing.T, db *bun.DB) { + type BoringThing struct { + AlwaysBlue string `bun:"colour,default:'blue'"` + } + + ctx := context.Background() + mustResetModel(t, ctx, db, (*BoringThing)(nil)) + m := newAutoMigratorOrSkip(t, db, + migrate.WithModel((*BoringThing)(nil)), + ) + + // Act + _, err := m.Migrate(ctx) // do not use runMigrations because we do not expect any files to be created + require.NoError(t, err, "auto migration failed") + + migrator := migrate.NewMigrator(db, migrate.NewMigrations(), migrate.WithTableName(migrationsTable)) + applied, err := migrator.AppliedMigrations(ctx) + require.NoError(t, err, "fetch applied migrations") + require.Empty(t, applied, "nothing to migrate, AppliedMigrations not empty") +} diff --git a/migrate/auto.go b/migrate/auto.go index e56fa23a0..16804cd99 100644 --- a/migrate/auto.go +++ b/migrate/auto.go @@ -196,6 +196,9 @@ func (am *AutoMigrator) plan(ctx context.Context) (*changeset, error) { func (am *AutoMigrator) Migrate(ctx context.Context, opts ...MigrationOption) (*MigrationGroup, error) { migrations, _, err := am.createSQLMigrations(ctx, false) if err != nil { + if err == errNothingToMigrate { + return new(MigrationGroup), nil + } return nil, fmt.Errorf("auto migrate: %w", err) } @@ -214,23 +217,37 @@ func (am *AutoMigrator) Migrate(ctx context.Context, opts ...MigrationOption) (* // CreateSQLMigration writes required changes to a new migration file. // Use migrate.Migrator to apply the generated migrations. func (am *AutoMigrator) CreateSQLMigrations(ctx context.Context) ([]*MigrationFile, error) { - _, files, err := am.createSQLMigrations(ctx, true) + _, files, err := am.createSQLMigrations(ctx, false) + if err == errNothingToMigrate { + return files, nil + } return files, err } // CreateTxSQLMigration writes required changes to a new migration file making sure they will be executed // in a transaction when applied. Use migrate.Migrator to apply the generated migrations. func (am *AutoMigrator) CreateTxSQLMigrations(ctx context.Context) ([]*MigrationFile, error) { - _, files, err := am.createSQLMigrations(ctx, false) + _, files, err := am.createSQLMigrations(ctx, true) + if err == errNothingToMigrate { + return files, nil + } return files, err } +// errNothingToMigrate is a sentinel error which means the database is already in a desired state. +// Should not be returned to the user -- return a nil-error instead. +var errNothingToMigrate = errors.New("nothing to migrate") + func (am *AutoMigrator) createSQLMigrations(ctx context.Context, transactional bool) (*Migrations, []*MigrationFile, error) { changes, err := am.plan(ctx) if err != nil { return nil, nil, fmt.Errorf("create sql migrations: %w", err) } + if changes.Len() == 0 { + return nil, nil, errNothingToMigrate + } + name, _ := genMigrationName(am.schemaName + "_auto") migrations := NewMigrations(am.migrationsOpts...) migrations.Add(Migration{ @@ -282,6 +299,10 @@ func (am *AutoMigrator) createSQL(_ context.Context, migrations *Migrations, fna return mf, nil } +func (c *changeset) Len() int { + return len(c.operations) +} + // Func creates a MigrationFunc that applies all operations all the changeset. func (c *changeset) Func(m sqlschema.Migrator) MigrationFunc { return func(ctx context.Context, db *bun.DB) error { diff --git a/migrate/sqlschema/inspector.go b/migrate/sqlschema/inspector.go index fc9af06fc..43fc66447 100644 --- a/migrate/sqlschema/inspector.go +++ b/migrate/sqlschema/inspector.go @@ -120,7 +120,7 @@ func (bmi *BunModelInspector) Inspect(ctx context.Context) (Database, error) { Name: f.Name, SQLType: strings.ToLower(sqlType), // TODO(dyma): maybe this is not necessary after Column.Eq() VarcharLen: length, - DefaultValue: exprToLower(f.SQLDefault), + DefaultValue: exprOrLiteral(f.SQLDefault), IsNullable: !f.NotNull, IsAutoIncrement: f.AutoIncrement, IsIdentity: f.Identity, @@ -211,12 +211,13 @@ func parseLen(typ string) (string, int, error) { return typ[:paren], length, nil } -// exprToLower converts string to lowercase, if it does not contain a string literal 'lit'. +// exprOrLiteral converts string to lowercase, if it does not contain a string literal 'lit' +// and trims the surrounding '' otherwise. // Use it to ensure that user-defined default values in the models are always comparable // to those returned by the database inspector, regardless of the case convention in individual drivers. -func exprToLower(s string) string { +func exprOrLiteral(s string) string { if strings.HasPrefix(s, "'") && strings.HasSuffix(s, "'") { - return s + return strings.Trim(s, "'") } return strings.ToLower(s) }