fix/correctly parse crlf line endings (#496)

* chore: add failing tests that demonstrate the issue * fix: correct migration parsing regexps Fixes #213. * fix: satisfy linter * fix: use unique test table names in test migrations
amacneil · Nov 16, 2023 · e87887f · e87887f
1 parent d74b06f
commit e87887f
Show file tree

Hide file tree

Showing 3 changed files with 75 additions and 2 deletions.
diff --git a/pkg/dbmate/db_test.go b/pkg/dbmate/db_test.go
@@ -735,3 +735,44 @@ func TestMigrateQueryErrorMessage(t *testing.T) {
 		require.Contains(t, err.Error(), "line: 3, column: 3, position: 29:")
 	})
 }
+
+func TestMigrationContents(t *testing.T) {
+	for _, u := range testURLs() {
+		t.Run(u.Scheme, func(t *testing.T) {
+			t.Run("ensure Windows CR/LF line endings in migration files work", func(t *testing.T) {
+				db := newTestDB(t, u)
+				drv, err := db.Driver()
+				require.NoError(t, err)
+
+				err = db.Drop()
+				require.NoError(t, err)
+				err = db.Create()
+				require.NoError(t, err)
+
+				sqlDB, err := drv.Open()
+				require.NoError(t, err)
+				defer dbutil.MustClose(sqlDB)
+
+				db.FS = fstest.MapFS{
+					"db/migrations/001_win_crlf_migration_empty.sql": {
+						Data: []byte("-- migrate:up\r\n-- migrate:down\r\n"),
+					},
+					"db/migrations/002_win_crlf_migration_basic.sql": {
+						Data: []byte("-- migrate:up\r\ncreate table test_win_crlf_basic (\r\n  id integer,\r\n  name varchar(255)\r\n);\r\n-- migrate:down\r\ndrop table test_win_crlf_basic;\r\n"),
+					},
+					"db/migrations/003_win_crlf_migration_options.sql": {
+						Data: []byte("-- migrate:up transaction:true\r\ncreate table test_win_crlf_options (\r\n  id integer,\r\n  name varchar(255)\r\n);\r\n-- migrate:down transaction:true\r\ndrop table test_win_crlf_options;\r\n"),
+					},
+				}
+
+				// run migrations
+				err = db.Migrate()
+				require.NoError(t, err)
+
+				// rollback last migration
+				err = db.Rollback()
+				require.NoError(t, err)
+			})
+		})
+	}
+}
diff --git a/pkg/dbmate/migration.go b/pkg/dbmate/migration.go
@@ -60,12 +60,12 @@ func (m migrationOptions) Transaction() bool {
 
 var (
 	upRegExp              = regexp.MustCompile(`(?m)^--\s*migrate:up(\s*$|\s+\S+)`)
-	downRegExp            = regexp.MustCompile(`(?m)^--\s*migrate:down(\s*$|\s+\S+)$`)
+	downRegExp            = regexp.MustCompile(`(?m)^--\s*migrate:down(\s*$|\s+\S+)`)
 	emptyLineRegExp       = regexp.MustCompile(`^\s*$`)
 	commentLineRegExp     = regexp.MustCompile(`^\s*--`)
 	whitespaceRegExp      = regexp.MustCompile(`\s+`)
 	optionSeparatorRegExp = regexp.MustCompile(`:`)
-	blockDirectiveRegExp  = regexp.MustCompile(`^--\s*migrate:[up|down]]`)
+	blockDirectiveRegExp  = regexp.MustCompile(`^--\s*migrate:(up|down)`)
 )
 
 // Error codes

diff --git a/pkg/dbmate/migration_test.go b/pkg/dbmate/migration_test.go
@@ -167,4 +167,36 @@ DROP COLUMN status;
 		_, err := parseMigrationContents(migration)
 		require.Error(t, err, "dbmate does not support statements preceding the '-- migrate:up' block")
 	})
+
+	t.Run("ensure Windows CR/LF line endings in migration files are parsed correctly", func(t *testing.T) {
+		t.Run("without migration options", func(t *testing.T) {
+			migration := "-- migrate:up\r\ncreate table users (id serial, name text);\r\n-- migrate:down\r\ndrop table users;\r\n"
+
+			parsed, err := parseMigrationContents(migration)
+			require.Nil(t, err)
+
+			require.Equal(t, "-- migrate:up\r\ncreate table users (id serial, name text);\r\n", parsed.Up)
+			require.Equal(t, migrationOptions{}, parsed.UpOptions)
+			require.Equal(t, true, parsed.UpOptions.Transaction())
+
+			require.Equal(t, "-- migrate:down\r\ndrop table users;\r\n", parsed.Down)
+			require.Equal(t, migrationOptions{}, parsed.DownOptions)
+			require.Equal(t, true, parsed.DownOptions.Transaction())
+		})
+
+		t.Run("with migration options", func(t *testing.T) {
+			migration := "-- migrate:up transaction:true\r\ncreate table users (id serial, name text);\r\n-- migrate:down transaction:true\r\ndrop table users;\r\n"
+
+			parsed, err := parseMigrationContents(migration)
+			require.Nil(t, err)
+
+			require.Equal(t, "-- migrate:up transaction:true\r\ncreate table users (id serial, name text);\r\n", parsed.Up)
+			require.Equal(t, migrationOptions{"transaction": "true"}, parsed.UpOptions)
+			require.Equal(t, true, parsed.UpOptions.Transaction())
+
+			require.Equal(t, "-- migrate:down transaction:true\r\ndrop table users;\r\n", parsed.Down)
+			require.Equal(t, migrationOptions{"transaction": "true"}, parsed.DownOptions)
+			require.Equal(t, true, parsed.DownOptions.Transaction())
+		})
+	})
 }