migrate: remove query construction with format string

brunotm · Mar 17, 2022 · bc2947c · bc2947c
1 parent 1f868f6
commit bc2947c
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 15 deletions.
diff --git a/migrate/migrate.go b/migrate/migrate.go
@@ -11,6 +11,8 @@ import (
 	"strconv"
 	"strings"
 	"time"
+
+	"github.com/brunotm/norm/statement"
 )
 
 var (
@@ -238,9 +240,13 @@ func (m *Migrate) Down(ctx context.Context) (err error) {
 }
 
 func (m *Migrate) set(ctx context.Context, tx *sql.Tx, mig *Migration) (err error) {
-	stmt := fmt.Sprintf(
-		"INSERT INTO migrations(version, date, name) values(%d, NOW(), '%s')",
-		mig.Version, mig.Name)
+	stmt, err := statement.Insert().Into("migrations").
+		Columns("version", "date", "name").
+		Values(mig.Version, statement.Ident("NOW()"), mig.Name).String()
+
+	if err != nil {
+		return err
+	}
 
 	m.logger(`migrate: update version, statement: %s`, stmt)
 	_, err = tx.ExecContext(ctx, stmt)

diff --git a/migrate/migrate_down_test.go b/migrate/migrate_down_test.go
@@ -29,7 +29,7 @@ func TestMigrationDown(t *testing.T) {
 			AddRow(migration4.Version, time.Now(), migration4.Name),
 	)
 	mock.ExpectExec(migration4.Discard.Statements[0]).WillReturnResult(sqlmock.NewResult(0, 1))
-	mock.ExpectExec(`INSERT INTO migrations(version, date, name) values(3, NOW(), 'roles_table')`).
+	mock.ExpectExec(`INSERT INTO migrations(version,date,name) VALUES (3,NOW(),'roles_table')`).
 		WillReturnResult(sqlmock.NewResult(0, 1))
 	mock.ExpectCommit()
 
@@ -39,7 +39,7 @@ func TestMigrationDown(t *testing.T) {
 			AddRow(migration3.Version, time.Now(), migration3.Name),
 	)
 	mock.ExpectExec(migration3.Discard.Statements[0]).WillReturnResult(sqlmock.NewResult(0, 1))
-	mock.ExpectExec(`INSERT INTO migrations(version, date, name) values(2, NOW(), 'users_email_index')`).
+	mock.ExpectExec(`INSERT INTO migrations(version,date,name) VALUES (2,NOW(),'users_email_index')`).
 		WillReturnResult(sqlmock.NewResult(0, 1))
 	mock.ExpectCommit()
 
@@ -49,7 +49,7 @@ func TestMigrationDown(t *testing.T) {
 			AddRow(migration2.Version, time.Now(), migration2.Name),
 	)
 	mock.ExpectExec(migration2.Discard.Statements[0]).WillReturnResult(sqlmock.NewResult(0, 1))
-	mock.ExpectExec(`INSERT INTO migrations(version, date, name) values(1, NOW(), 'users_table')`).
+	mock.ExpectExec(`INSERT INTO migrations(version,date,name) VALUES (1,NOW(),'users_table')`).
 		WillReturnResult(sqlmock.NewResult(0, 1))
 	mock.ExpectCommit()
 
@@ -59,7 +59,7 @@ func TestMigrationDown(t *testing.T) {
 			AddRow(migration1.Version, time.Now(), migration1.Name),
 	)
 	mock.ExpectExec(migration1.Discard.Statements[0]).WillReturnResult(sqlmock.NewResult(0, 1))
-	mock.ExpectExec(`INSERT INTO migrations(version, date, name) values(0, NOW(), 'create_migrations_table')`).
+	mock.ExpectExec(`INSERT INTO migrations(version,date,name) VALUES (0,NOW(),'create_migrations_table')`).
 		WillReturnResult(sqlmock.NewResult(0, 1))
 	mock.ExpectCommit()
 

diff --git a/migrate/migrate_up_test.go b/migrate/migrate_up_test.go
@@ -27,7 +27,7 @@ func TestMigrationUp(t *testing.T) {
 	mock.ExpectRollback()
 	mock.ExpectBegin()
 	mock.ExpectExec(migration0.Apply.Statements[0]).WillReturnResult(sqlmock.NewResult(0, 1))
-	mock.ExpectExec(`INSERT INTO migrations(version, date, name) values(0, NOW(), 'create_migrations_table')`).
+	mock.ExpectExec(`INSERT INTO migrations(version,date,name) VALUES (0,NOW(),'create_migrations_table')`).
 		WillReturnResult(sqlmock.NewResult(0, 1))
 	mock.ExpectCommit()
 
@@ -39,7 +39,7 @@ func TestMigrationUp(t *testing.T) {
 	)
 	mock.ExpectExec(migration1.Apply.Statements[0]).
 		WillReturnResult(sqlmock.NewResult(0, 1))
-	mock.ExpectExec(`INSERT INTO migrations(version, date, name) values(1, NOW(), 'users_table')`).
+	mock.ExpectExec(`INSERT INTO migrations(version,date,name) VALUES (1,NOW(),'users_table')`).
 		WillReturnResult(sqlmock.NewResult(0, 1))
 	mock.ExpectCommit()
 
@@ -51,7 +51,7 @@ func TestMigrationUp(t *testing.T) {
 	)
 	mock.ExpectExec(migration2.Apply.Statements[0]).
 		WillReturnResult(sqlmock.NewResult(0, 1))
-	mock.ExpectExec(`INSERT INTO migrations(version, date, name) values(2, NOW(), 'users_email_index')`).
+	mock.ExpectExec(`INSERT INTO migrations(version,date,name) VALUES (2,NOW(),'users_email_index')`).
 		WillReturnResult(sqlmock.NewResult(0, 1))
 	mock.ExpectCommit()
 
@@ -63,7 +63,7 @@ func TestMigrationUp(t *testing.T) {
 	)
 	mock.ExpectExec(migration3.Apply.Statements[0]).
 		WillReturnResult(sqlmock.NewResult(0, 1))
-	mock.ExpectExec(`INSERT INTO migrations(version, date, name) values(3, NOW(), 'roles_table')`).
+	mock.ExpectExec(`INSERT INTO migrations(version,date,name) VALUES (3,NOW(),'roles_table')`).
 		WillReturnResult(sqlmock.NewResult(0, 1))
 	mock.ExpectCommit()
 
@@ -75,7 +75,7 @@ func TestMigrationUp(t *testing.T) {
 	)
 	mock.ExpectExec(migration4.Apply.Statements[0]).
 		WillReturnResult(sqlmock.NewResult(0, 1))
-	mock.ExpectExec(`INSERT INTO migrations(version, date, name) values(4, NOW(), 'user_roles_fk')`).
+	mock.ExpectExec(`INSERT INTO migrations(version,date,name) VALUES (4,NOW(),'user_roles_fk')`).
 		WillReturnResult(sqlmock.NewResult(0, 1))
 	mock.ExpectCommit()
 

diff --git a/migrate/parse_test.go b/migrate/parse_test.go
@@ -6,7 +6,7 @@ import (
 )
 
 func TestParseSimple(t *testing.T) {
-	stmt, err := parseStatement(statement)
+	stmt, err := parseStatement(stmt)
 	if err != nil {
 		t.Fatalf("failed to parse statement: %s", err)
 	}
@@ -17,15 +17,15 @@ func TestParseSimple(t *testing.T) {
 }
 
 func TestParseMultiNoTx(t *testing.T) {
-	notx := append([]byte(`-- migrate: NoTransaction`), statement...)
+	notx := append([]byte(`-- migrate: NoTransaction`), stmt...)
 	_, err := parseStatement(notx)
 
 	if err != ErrInvalidNoTx {
 		t.Fatalf("failed to parse statement: %s", err)
 	}
 }
 
-var statement = []byte(`
+var stmt = []byte(`
 CREATE TABLE IF NOT EXISTS users (
 	created_at timestamptz NOT NULL DEFAULT now(),
 	updated_at timestamptz NOT NULL DEFAULT now(),