From e87887fcb436bfe9d29c4048ef8b83bee8e9205f Mon Sep 17 00:00:00 2001 From: Dossy Shiobara Date: Thu, 16 Nov 2023 11:29:21 -0500 Subject: [PATCH] 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 --- pkg/dbmate/db_test.go | 41 ++++++++++++++++++++++++++++++++++++ pkg/dbmate/migration.go | 4 ++-- pkg/dbmate/migration_test.go | 32 ++++++++++++++++++++++++++++ 3 files changed, 75 insertions(+), 2 deletions(-) diff --git a/pkg/dbmate/db_test.go b/pkg/dbmate/db_test.go index d0ba5c95..9bbbaf1c 100644 --- 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 index 9d43b156..ec67f7cf 100644 --- 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 index adde73dc..fe53cb22 100644 --- 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()) + }) + }) }