Properly clean/normalize user specified directory for create command

Addresses: golang-migrate#238
bodyguard-ai · Jun 21, 2019 · 9f6c7e5 · 9f6c7e5
1 parent 7c76166
commit 9f6c7e5
Show file tree

Hide file tree

Showing 3 changed files with 63 additions and 17 deletions.
diff --git a/internal/cli/commands.go b/internal/cli/commands.go
@@ -49,7 +49,22 @@ func nextSeq(matches []string, dir string, seqDigits int) (string, error) {
 	return nextSeqStr, nil
 }
 
+// cleanDir normalizes the provided directory
+func cleanDir(dir string) string {
+	dir = filepath.Clean(dir)
+	switch dir {
+	case ".":
+		return ""
+	case "/":
+		return dir
+	default:
+		return dir + "/"
+	}
+}
+
+// createCmd (meant to be called via a CLI command) creates a new migration
 func createCmd(dir string, startTime time.Time, format string, name string, ext string, seq bool, seqDigits int) {
+	dir = cleanDir(dir)
 	var base string
 	if seq && format != defaultTimeFormat {
 		log.fatalErr(errors.New("The seq and format options are mutually exclusive"))

diff --git a/internal/cli/commands_test.go b/internal/cli/commands_test.go
@@ -4,6 +4,37 @@ import (
 	"testing"
 )
 
+func TestCleanDir(t *testing.T) {
+	cases := []struct {
+		dir              string
+		expectedCleanDir string
+	}{
+		{dir: "", expectedCleanDir: ""},
+		{dir: ".", expectedCleanDir: ""},
+		{dir: "/", expectedCleanDir: "/"},
+		{dir: "./", expectedCleanDir: ""},
+		{dir: ".test", expectedCleanDir: ".test/"},
+		{dir: ".test/", expectedCleanDir: ".test/"},
+		{dir: "test", expectedCleanDir: "test/"},
+		{dir: "test/", expectedCleanDir: "test/"},
+		{dir: "./test", expectedCleanDir: "test/"},
+		{dir: "./test/", expectedCleanDir: "test/"},
+		{dir: "test/test", expectedCleanDir: "test/test/"},
+		{dir: "test/test/", expectedCleanDir: "test/test/"},
+		{dir: "./test/test", expectedCleanDir: "test/test/"},
+		{dir: "./test/test/", expectedCleanDir: "test/test/"},
+	}
+
+	for _, c := range cases {
+		t.Run(c.dir, func(t *testing.T) {
+			cleanedDir := cleanDir(c.dir)
+			if cleanedDir != c.expectedCleanDir {
+				t.Error("Incorrectly cleaned dir: " + cleanedDir + " != " + c.expectedCleanDir)
+			}
+		})
+	}
+}
+
 func TestNextSeq(t *testing.T) {
 	cases := []struct {
 		name           string
@@ -13,19 +44,23 @@ func TestNextSeq(t *testing.T) {
 		expected       string
 		expectedErrStr string
 	}{
-		{"Bad digits", []string{}, "migrationDir", 0, "", "Digits must be positive"},
-		{"Single digit initialize", []string{}, "migrationDir", 1, "1", ""},
-		{"Single digit malformed", []string{"bad"}, "migrationDir", 1, "", "Malformed migration filename: bad"},
-		{"Single digit no int", []string{"bad_bad"}, "migrationDir", 1, "", "strconv.Atoi: parsing \"bad\": invalid syntax"},
-		{"Single digit negative seq", []string{"-5_test"}, "migrationDir", 1, "", "Next sequence number must be positive"},
-		{"Single digit increment", []string{"3_test", "4_test"}, "migrationDir", 1, "5", ""},
-		{"Single digit overflow", []string{"9_test"}, "migrationDir", 1, "", "Next sequence number 10 too large. At most 1 digits are allowed"},
-		{"Zero-pad initialize", []string{}, "migrationDir", 6, "000001", ""},
-		{"Zero-pad malformed", []string{"bad"}, "migrationDir", 6, "", "Malformed migration filename: bad"},
-		{"Zero-pad no int", []string{"bad_bad"}, "migrationDir", 6, "", "strconv.Atoi: parsing \"bad\": invalid syntax"},
-		{"Zero-pad negative seq", []string{"-000005_test"}, "migrationDir", 6, "", "Next sequence number must be positive"},
-		{"Zero-pad increment", []string{"000003_test", "000004_test"}, "migrationDir", 6, "000005", ""},
-		{"Zero-pad overflow", []string{"999999_test"}, "migrationDir", 6, "", "Next sequence number 1000000 too large. At most 6 digits are allowed"},
+		{"Bad digits", []string{}, "migrationDir/", 0, "", "Digits must be positive"},
+		{"Single digit initialize", []string{}, "migrationDir/", 1, "1", ""},
+		{"Single digit malformed", []string{"bad"}, "migrationDir/", 1, "", "Malformed migration filename: bad"},
+		{"Single digit no int", []string{"bad_bad"}, "migrationDir/", 1, "", "strconv.Atoi: parsing \"bad\": invalid syntax"},
+		{"Single digit negative seq", []string{"-5_test"}, "migrationDir/", 1, "", "Next sequence number must be positive"},
+		{"Single digit increment", []string{"3_test", "4_test"}, "migrationDir/", 1, "5", ""},
+		{"Single digit overflow", []string{"9_test"}, "migrationDir/", 1, "", "Next sequence number 10 too large. At most 1 digits are allowed"},
+		{"Zero-pad initialize", []string{}, "migrationDir/", 6, "000001", ""},
+		{"Zero-pad malformed", []string{"bad"}, "migrationDir/", 6, "", "Malformed migration filename: bad"},
+		{"Zero-pad no int", []string{"bad_bad"}, "migrationDir/", 6, "", "strconv.Atoi: parsing \"bad\": invalid syntax"},
+		{"Zero-pad negative seq", []string{"-000005_test"}, "migrationDir/", 6, "", "Next sequence number must be positive"},
+		{"Zero-pad increment", []string{"000003_test", "000004_test"}, "migrationDir/", 6, "000005", ""},
+		{"Zero-pad overflow", []string{"999999_test"}, "migrationDir/", 6, "", "Next sequence number 1000000 too large. At most 6 digits are allowed"},
+		{"dir - no trailing slash", []string{"migrationDir/000001_test"}, "migrationDir", 6, "", `strconv.Atoi: parsing "/000001": invalid syntax`},
+		{"dir - with dot prefix success", []string{"migrationDir/000001_test"}, "./migrationDir/", 6, "", `strconv.Atoi: parsing "migrationDir/000001": invalid syntax`},
+		{"dir - no dir prefix", []string{"000001_test"}, "migrationDir/", 6, "000002", ""},
+		{"dir - strip success", []string{"migrationDir/000001_test"}, "migrationDir/", 6, "000002", ""},
 	}
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {

diff --git a/internal/cli/main.go b/internal/cli/main.go
@@ -139,10 +139,6 @@ Database drivers: `+strings.Join(database.List(), ", ")+"\n")
 		}
 		*extPtr = "." + strings.TrimPrefix(*extPtr, ".")
 
-		if *dirPtr != "" {
-			*dirPtr = strings.Trim(*dirPtr, "/") + "/"
-		}
-
 		createCmd(*dirPtr, startTime, *formatPtr, name, *extPtr, seq, seqDigits)
 
 	case "goto":