From d4edf0d3ff145982c6421594b5216e020e4aca4d Mon Sep 17 00:00:00 2001 From: Ibrahim Serdar Acikgoz Date: Wed, 11 Sep 2024 17:26:41 +0200 Subject: [PATCH] reflect review comments --- cmd/migration-assist/commands/mysql.go | 11 +- cmd/migration-assist/commands/postgres.go | 148 ++++++++++++---------- cmd/migration-assist/commands/util.go | 24 ++++ internal/store/store.go | 2 +- 4 files changed, 114 insertions(+), 71 deletions(-) create mode 100644 cmd/migration-assist/commands/util.go diff --git a/cmd/migration-assist/commands/mysql.go b/cmd/migration-assist/commands/mysql.go index 10dc9f4..c36023d 100644 --- a/cmd/migration-assist/commands/mysql.go +++ b/cmd/migration-assist/commands/mysql.go @@ -82,7 +82,16 @@ func runSourceCheckCmdF(cmd *cobra.Command, args []string) error { } outputFile, _ := cmd.Flags().GetString("output") - if err = os.WriteFile(outputFile, b, 0600); err != nil { + if _, err = os.Stat(outputFile); err == nil || os.IsExist(err) { + if ConfirmationPrompt("Output file already exists, do you want to overwrite it?") { + if err = os.Remove(outputFile); err != nil { + return fmt.Errorf("could not remove output file: %w", err) + } + } else { + baseLogger.Println("Output file already exists, will not overwrite it.") + } + } + if err = os.WriteFile(outputFile, b, 0666); err != nil { return fmt.Errorf("could not write to output file: %w", err) } diff --git a/cmd/migration-assist/commands/postgres.go b/cmd/migration-assist/commands/postgres.go index 4d42b3f..8fdb199 100644 --- a/cmd/migration-assist/commands/postgres.go +++ b/cmd/migration-assist/commands/postgres.go @@ -19,11 +19,12 @@ import ( func TargetCheckCmd() *cobra.Command { cmd := &cobra.Command{ - Use: "postgres", - Short: "Checks the Postgres database schema whether it is ready for the migration", - RunE: runTargetCheckCmdF, - Example: " migration-assist postgres \"postgres://mmuser:mostest@localhost:8765/mattermost_test?sslmode=disable\" \\\n--run-migrations", - Args: cobra.MinimumNArgs(1), + Use: "postgres", + Short: "Checks the Postgres database schema whether it is ready for the migration", + RunE: runTargetCheckCmdF, + Example: " migration-assist postgres \"postgres://mmuser:mostest@localhost:8765/mattermost_test?sslmode=disable\" \\\n--run-migrations" + + "\n\n--mattermost-version, --migrations-dir, --applied-migrations are mutually exclusive. Use only one of these flags.\n", + Args: cobra.MinimumNArgs(1), } amCmd := RunAfterMigration() @@ -31,9 +32,10 @@ func TargetCheckCmd() *cobra.Command { // Optional flags cmd.Flags().Bool("run-migrations", false, "Runs migrations for Postgres schema") - cmd.Flags().String("mattermost-version", "", "Mattermost version to be cloned to run migrations (example: v8.1)") - cmd.Flags().String("migrations-dir", "", "Migrations directory (should be used if mattermost-version is not supplied)") - cmd.Flags().String("git", "git", "git binary to be executed if the repository will be cloned") + cmd.Flags().String("mattermost-version", "", "Mattermost version to be cloned to run migrations (example: \"v8.1\")") + cmd.Flags().String("migrations-dir", "", "Migrations directory (should be used if the migrations are already cloned separately)") + cmd.Flags().String("applied-migrations", "", "File containing the list of applied migrations (example: \"mysql.output\")") + cmd.Flags().String("git", "git", "git binary to be executed if the repository will be cloned (ie. --mattermost-version is supplied)") cmd.Flags().Bool("check-schema-owner", true, "Check if the schema owner is the same as the user running the migration") cmd.Flags().Bool("check-tables-empty", true, "Check if tables are empty before running migrations") cmd.PersistentFlags().String("schema", "public", "the default schema to be used for the session") @@ -111,69 +113,13 @@ func runTargetCheckCmdF(cmd *cobra.Command, args []string) error { return nil } - mysqlMigrations := "mysql.output" - if len(args) > 1 { - mysqlMigrations = args[1] - } - - var src sources.Source - - // download required migrations if necessary + mysqlMigrations, _ := cmd.Flags().GetString("applied-migrations") + mmVersion, _ := cmd.Flags().GetString("mattermost-version") migrationDir, _ := cmd.Flags().GetString("migrations-dir") - if _, err = os.Stat(mysqlMigrations); !os.IsNotExist(err) { - baseLogger.Printf("loading migrations from the %s file\n", mysqlMigrations) - // load migrations from the applied migrations file - var cfg store.DBConfig - f, err2 := os.Open(mysqlMigrations) - if err2 != nil { - return fmt.Errorf("could not open file: %w", err2) - } - defer f.Close() - - err = json.NewDecoder(f).Decode(&cfg) - if err != nil { - return fmt.Errorf("could not decode file: %w", err) - } - - src, err = store.CreateSourceFromEbmedded(queries.Assets(), "migrations/postgres", cfg.AppliedMigrations) - if err != nil { - return fmt.Errorf("could not create source from embedded: %w", err) - } - } else if migrationDir == "" { - mmVersion, _ := cmd.Flags().GetString("mattermost-version") - if mmVersion == "" { - return fmt.Errorf("--mattermost-version needs to be supplied to run migrations") - } - v, err2 := semver.ParseTolerant(mmVersion) - if err2 != nil { - return fmt.Errorf("could not parse mattermost version: %w", err2) - } - tempDir, err3 := os.MkdirTemp("", "mattermost") - if err3 != nil { - return fmt.Errorf("could not create temp directory: %w", err3) - } - - baseLogger.Printf("cloning %s@%s\n", "repository", v.String()) - err = git.CloneMigrations(git.CloneOptions{ - TempRepoPath: tempDir, - Output: "postgres", - DriverType: "postgres", - Version: v, - }, verboseLogger) - if err != nil { - return fmt.Errorf("error during cloning migrations: %w", err) - } - - src, err = file.Open("postgres") - if err != nil { - return fmt.Errorf("could not read migrations: %w", err) - } - } else { - src, err = file.Open(migrationDir) - if err != nil { - return fmt.Errorf("could not read migrations: %w", err) - } + src, err := determineSource(mysqlMigrations, migrationDir, mmVersion, baseLogger, verboseLogger) + if err != nil { + return fmt.Errorf("could not determine source: %w", err) } // run the migrations @@ -221,3 +167,67 @@ func runPostMigrateCmdF(c *cobra.Command, args []string) error { return nil } + +func determineSource(appliedMigrations, userSuppliedMigrations, mmVersion string, baseLogger, verboseLogger logger.LogInterface) (sources.Source, error) { + switch { + case appliedMigrations != "": + baseLogger.Printf("loading migrations from the %s file\n", appliedMigrations) + // load migrations from the applied migrations file + var cfg store.DBConfig + f, err2 := os.Open(appliedMigrations) + if err2 != nil { + return nil, fmt.Errorf("could not open file: %w", err2) + } + defer f.Close() + + err := json.NewDecoder(f).Decode(&cfg) + if err != nil { + return nil, fmt.Errorf("could not decode file: %w", err) + } + + src, err := store.CreateSourceFromEmbedded(queries.Assets(), "migrations/postgres", cfg.AppliedMigrations) + if err != nil { + return nil, fmt.Errorf("could not create source from embedded: %w", err) + } + + return src, nil + case userSuppliedMigrations != "": + src, err := file.Open(userSuppliedMigrations) + if err != nil { + return nil, fmt.Errorf("could not read migrations: %w", err) + } + + return src, nil + default: + if mmVersion == "" { + return nil, fmt.Errorf("--mattermost-version needs to be supplied to run migrations") + } + v, err2 := semver.ParseTolerant(mmVersion) + if err2 != nil { + return nil, fmt.Errorf("could not parse mattermost version: %w", err2) + } + + tempDir, err3 := os.MkdirTemp("", "mattermost") + if err3 != nil { + return nil, fmt.Errorf("could not create temp directory: %w", err3) + } + + baseLogger.Printf("cloning %s@%s\n", "repository", v.String()) + err := git.CloneMigrations(git.CloneOptions{ + TempRepoPath: tempDir, + Output: "postgres", + DriverType: "postgres", + Version: v, + }, baseLogger) + if err != nil { + return nil, fmt.Errorf("error during cloning migrations: %w", err) + } + + src, err := file.Open("postgres") + if err != nil { + return nil, fmt.Errorf("could not read migrations: %w", err) + } + + return src, nil + } +} diff --git a/cmd/migration-assist/commands/util.go b/cmd/migration-assist/commands/util.go new file mode 100644 index 0000000..15ea36f --- /dev/null +++ b/cmd/migration-assist/commands/util.go @@ -0,0 +1,24 @@ +package commands + +import ( + "bufio" + "fmt" + "os" +) + +func ConfirmationPrompt(question string) bool { + fmt.Fprintf(os.Stderr, "%s [y/N]: ", question) + + rd := bufio.NewReader(os.Stdin) + r, err := rd.ReadString('\n') + if err != nil { + fmt.Fprintf(os.Stderr, "Error reading input: %v\n", err) + return false + } + + if len(r) > 0 && (r[0] == 'y' || r[0] == 'Y') { + return true + } + + return false +} diff --git a/internal/store/store.go b/internal/store/store.go index 01508c3..0aa2d90 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -142,7 +142,7 @@ func (db *DB) RunMigrations(src sources.Source) error { return nil } -func CreateSourceFromEbmedded(assets embed.FS, dir string, versions []int) (sources.Source, error) { +func CreateSourceFromEmbedded(assets embed.FS, dir string, versions []int) (sources.Source, error) { dirEntries, err := assets.ReadDir(dir) if err != nil { return nil, err