Fix problem with checking for missing files.

Some commands allow files to be missing. A condition has been added to the CheckFullPath function that disables checking for the existence of a file. It is also explicitly indicated for each command whether to check for files, where necessary. For history-migrate command, checking the presence of the history db file is disabled. For report-info command for plugin-report-file-path flag, checking the presence of the history db file is disabled. Checking for history yaml files is always performed.
woblerr · Mar 4, 2024 · 7825067 · 7825067
1 parent 93b20de
commit 7825067
Show file tree

Hide file tree

Showing 10 changed files with 55 additions and 40 deletions.
diff --git a/cmd/backup_clean.go b/cmd/backup_clean.go
@@ -47,7 +47,7 @@ If no --history-file or --history-db options are specified, the history database
 Only --history-file or --history-db option can be specified, not both.`,
 	Args: cobra.NoArgs,
 	Run: func(cmd *cobra.Command, args []string) {
-		doRootFlagValidation(cmd.Flags())
+		doRootFlagValidation(cmd.Flags(), checkFileExistsConst)
 		doRootBackupFlagValidation(cmd.Flags())
 		doCleanBackupFlagValidation(cmd.Flags())
 		doCleanBackup()
@@ -86,7 +86,7 @@ func init() {
 // These flag checks are applied only for backup-clean command.
 func doCleanBackupFlagValidation(flags *pflag.FlagSet) {
 	var err error
-	// If before-timestamp are specified and have correct values.
+	// If before-timestamp flag is specified and have correct values.
 	if flags.Changed(beforeTimestampFlagName) {
 		err = gpbckpconfig.CheckTimestamp(backupCleanBeforeTimestamp)
 		if err != nil {
@@ -98,9 +98,9 @@ func doCleanBackupFlagValidation(flags *pflag.FlagSet) {
 	if flags.Changed(olderThenDaysFlagName) {
 		beforeTimestamp = gpbckpconfig.GetTimestampOlderThen(backupCleanOlderThenDays)
 	}
-	// If plugin-config flag is specified and full path.
+	// If plugin-config flag is specified and it exists and the full path is specified.
 	if flags.Changed(pluginConfigFileFlagName) {
-		err = gpbckpconfig.CheckFullPath(backupCleanPluginConfigFile)
+		err = gpbckpconfig.CheckFullPath(backupCleanPluginConfigFile, checkFileExistsConst)
 		if err != nil {
 			gplog.Error(textmsg.ErrorTextUnableValidateFlag(backupCleanPluginConfigFile, pluginConfigFileFlagName, err))
 			execOSExit(exitErrorCode)

diff --git a/cmd/backup_delete.go b/cmd/backup_delete.go
@@ -48,7 +48,7 @@ If no --history-file or --history-db options are specified, the history database
 Only --history-file or --history-db option can be specified, not both.`,
 	Args: cobra.NoArgs,
 	Run: func(cmd *cobra.Command, args []string) {
-		doRootFlagValidation(cmd.Flags())
+		doRootFlagValidation(cmd.Flags(), checkFileExistsConst)
 		doRootBackupFlagValidation(cmd.Flags())
 		doDeleteBackupFlagValidation(cmd.Flags())
 		doDeleteBackup()
@@ -99,9 +99,9 @@ func doDeleteBackupFlagValidation(flags *pflag.FlagSet) {
 			}
 		}
 	}
-	// If plugin-config flag is specified and full path.
+	// If the plugin-config flag is specified and it exists and the full path is specified.
 	if flags.Changed(pluginConfigFileFlagName) {
-		err = gpbckpconfig.CheckFullPath(backupDeletePluginConfigFile)
+		err = gpbckpconfig.CheckFullPath(backupDeletePluginConfigFile, checkFileExistsConst)
 		if err != nil {
 			gplog.Error(textmsg.ErrorTextUnableValidateFlag(backupDeletePluginConfigFile, pluginConfigFileFlagName, err))
 			execOSExit(exitErrorCode)

diff --git a/cmd/backup_info.go b/cmd/backup_info.go
@@ -58,7 +58,7 @@ If no --history-file or --history-db options are specified, the history database
 Only --history-file or --history-db option can be specified, not both.`,
 	Args: cobra.NoArgs,
 	Run: func(cmd *cobra.Command, args []string) {
-		doRootFlagValidation(cmd.Flags())
+		doRootFlagValidation(cmd.Flags(), checkFileExistsConst)
 		doRootBackupFlagValidation(cmd.Flags())
 		doBackupInfoFlagValidation(cmd.Flags())
 		doBackupInfo()

diff --git a/cmd/constants.go b/cmd/constants.go
@@ -39,6 +39,9 @@ const (
 
 	exitErrorCode = 1
 
+	// Default for checking the existence of the file.
+	checkFileExistsConst = true
+
 	// Batch size for deleting from sqlite3.
 	// This is to prevent problem with sqlite3.
 	sqliteDeleteBatchSize = 1000

diff --git a/cmd/history-clean.go b/cmd/history-clean.go
@@ -42,7 +42,7 @@ If no --history-file or --history-db options are specified, the history database
 Only --history-file or --history-db option can be specified, not both.`,
 	Args: cobra.NoArgs,
 	Run: func(cmd *cobra.Command, args []string) {
-		doRootFlagValidation(cmd.Flags())
+		doRootFlagValidation(cmd.Flags(), checkFileExistsConst)
 		doRootBackupFlagValidation(cmd.Flags())
 		doCleanHistoryFlagValidation(cmd.Flags())
 		doCleanHistory()

diff --git a/cmd/history_migrate.go b/cmd/history_migrate.go
@@ -26,7 +26,8 @@ Can be specified multiple times. The full path to the file is required.
 If no --history-file and/or --history-db options are specified, the files will be searched in the current directory.`,
 	Args: cobra.NoArgs,
 	Run: func(cmd *cobra.Command, args []string) {
-		doRootFlagValidation(cmd.Flags())
+		// No need to check historyDB existence.
+		doRootFlagValidation(cmd.Flags(), false)
 		doMigrateHistory()
 	},
 }

diff --git a/cmd/report_info.go b/cmd/report_info.go
@@ -47,7 +47,7 @@ If no --history-file or --history-db options are specified, the history database
 Only --history-file or --history-db option can be specified, not both.`,
 	Args: cobra.NoArgs,
 	Run: func(cmd *cobra.Command, args []string) {
-		doRootFlagValidation(cmd.Flags())
+		doRootFlagValidation(cmd.Flags(), checkFileExistsConst)
 		doRootBackupFlagValidation(cmd.Flags())
 		doReportInfoFlagValidation(cmd.Flags())
 		doReportInfo()
@@ -89,17 +89,18 @@ func doReportInfoFlagValidation(flags *pflag.FlagSet) {
 		}
 
 	}
-	// If plugin-config flag is specified and full path.
+	// If plugin-config flag is specified and it exists and the full path is specified.
 	if flags.Changed(pluginConfigFileFlagName) {
-		err = gpbckpconfig.CheckFullPath(reportInfoPluginConfigFile)
+		err = gpbckpconfig.CheckFullPath(reportInfoPluginConfigFile, checkFileExistsConst)
 		if err != nil {
 			gplog.Error(textmsg.ErrorTextUnableValidateFlag(reportInfoPluginConfigFile, pluginConfigFileFlagName, err))
 			execOSExit(exitErrorCode)
 		}
 	}
-	// If plugin-report-file-pat flag is specified and full path.
+	// If plugin-report-file-path flag is specified and full path.
 	if flags.Changed(reportFilePluginPathFlagName) {
-		err = gpbckpconfig.CheckFullPath(reportInfoReportFilePluginPath)
+		// No need to check path existence.
+		err = gpbckpconfig.CheckFullPath(reportInfoReportFilePluginPath, false)
 		if err != nil {
 			gplog.Error(textmsg.ErrorTextUnableValidateFlag(reportInfoReportFilePluginPath, reportFilePluginPathFlagName, err))
 			execOSExit(exitErrorCode)

diff --git a/cmd/root.go b/cmd/root.go
@@ -73,20 +73,23 @@ func getVersion() string {
 }
 
 // These flag checks are applied for all commands:
-func doRootFlagValidation(flags *pflag.FlagSet) {
+func doRootFlagValidation(flags *pflag.FlagSet, checkFileExists bool) {
 	var err error
 	// If history-db flag is specified and full path.
+	// The existence of the file is checked by condition from each specific command.
+	// Not all commands (see history-migrate command, report-info command flags) require a history db file to exist.
 	if flags.Changed(historyDBFlagName) {
-		err = gpbckpconfig.CheckFullPath(rootHistoryDB)
+		err = gpbckpconfig.CheckFullPath(rootHistoryDB, checkFileExists)
 		if err != nil {
 			gplog.Error(textmsg.ErrorTextUnableValidateFlag(rootHistoryDB, historyDBFlagName, err))
 			execOSExit(exitErrorCode)
 		}
 	}
-	// If history-file flag is specified and full path.
+	// If the plugin-config flag is specified and it exists and the full path is specified.
 	if flags.Changed(historyFilesFlagName) {
 		for _, hFile := range rootHistoryFiles {
-			err = gpbckpconfig.CheckFullPath(hFile)
+			// Always check the existence of the file.
+			err = gpbckpconfig.CheckFullPath(hFile, checkFileExistsConst)
 			if err != nil {
 				gplog.Error(textmsg.ErrorTextUnableValidateFlag(hFile, historyFilesFlagName, err))
 				execOSExit(exitErrorCode)

diff --git a/gpbckpconfig/utils.go b/gpbckpconfig/utils.go
@@ -26,14 +26,16 @@ func GetTimestampOlderThen(value uint) string {
 
 // CheckFullPath Returns error if path is not an absolute path or
 // file does not exist.
-func CheckFullPath(path string) error {
+func CheckFullPath(path string, checkFileExists bool) error {
 	if !filepath.IsAbs(path) {
 		return textmsg.ErrorValidationFullPath()
 	}
-	// It's better to check if the file exists as early as possible.
-	// This simple check will resolve errors  with non-existent files.
-	if _, err := os.Stat(path); errors.Is(err, os.ErrNotExist) {
-		return textmsg.ErrorFileNotExist()
+	// In most cases this check should be mandatory.
+	// But there are commands, that allows the history db file to be missing.
+	if checkFileExists {
+		if _, err := os.Stat(path); errors.Is(err, os.ErrNotExist) {
+			return textmsg.ErrorFileNotExist()
+		}
 	}
 	return nil
 }

diff --git a/gpbckpconfig/utils_test.go b/gpbckpconfig/utils_test.go
@@ -49,35 +49,40 @@ func TestCheckFullPath(t *testing.T) {
 	defer os.Remove(tempFile.Name())
 
 	tests := []struct {
-		name    string
-		value   string
-		wantErr bool
+		name            string
+		value           string
+		checkFileExists bool
+		wantErr         bool
 	}{
 		{
-			name:    "Test exist file and full path",
-			value:   tempFile.Name(),
-			wantErr: false,
+			name:            "Test exist file and full path",
+			value:           tempFile.Name(),
+			checkFileExists: true,
+			wantErr:         false,
 		},
 		{
-			name:    "Test full path and not exist file",
-			value:   "/some/path/test.txt",
-			wantErr: true,
+			name:            "Test full path and not exist file",
+			value:           "/some/path/test.txt",
+			checkFileExists: true,
+			wantErr:         true,
 		},
 		{
-			name:    "Test zero length path",
-			value:   "",
-			wantErr: true,
+			name:            "Test zero length path",
+			value:           "",
+			checkFileExists: false,
+			wantErr:         true,
 		},
 		{
-			name:    "Test not full path",
-			value:   "test.txt",
-			wantErr: true,
+			name:            "Test not full path",
+			value:           "test.txt",
+			checkFileExists: false,
+			wantErr:         true,
 		},
 	}
 	fmt.Print(tempFile.Name())
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			err := CheckFullPath(tt.value)
+			err := CheckFullPath(tt.value, tt.checkFileExists)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("\nVariables do not match:\n%v\nwantErr:\n%v", err, tt.wantErr)
 			}