Improve output filename validation

fleprocess/adif_process.go

@@ -23,10 +23,10 @@ import (
 //ProcessAdifCommand FIXME
 func ProcessAdifCommand(inputFilename, outputFilename string, isInterpolateTime, isWWFFcli, isSOTAcli, isOverwrite bool) {
 
-	verifiedOutputFilename, filenameWasOK := buildOutputFilename(outputFilename, inputFilename, isOverwrite, ".adi")
+	verifiedOutputFilename, err := buildOutputFilename(outputFilename, inputFilename, isOverwrite, ".adi")
 
 	// if the output file could not be parsed correctly do noting
-	if filenameWasOK {
+	if err == nil {
 		loadedLogFile, isLoadedOK := LoadFile(inputFilename, isInterpolateTime)
 
 		if isLoadedOK {

fleprocess/csv_process.go

@@ -26,18 +26,33 @@ import (
 //ProcessCsvCommand loads an FLE input to produce a SOTA CSV
 func ProcessCsvCommand(inputFilename, outputCsvFilename string, isInterpolateTime, isOverwriteCsv bool) error {
 
-	if verifiedOutputFilename, filenameWasOK := buildOutputFilename(outputCsvFilename, inputFilename, isOverwriteCsv, ".csv"); filenameWasOK == true {
+	//Validate of build the output filenaem
-		if loadedLogFile, isLoadedOK := LoadFile(inputFilename, isInterpolateTime); isLoadedOK == true {
+	var verifiedOutputFilename string
+	var err error
+
+	if verifiedOutputFilename, err = buildOutputFilename(outputCsvFilename, inputFilename, isOverwriteCsv, ".csv"); err != nil {
+		return err
+	}
+
+	//Load the input file
+	var loadedLogFile []LogLine
+	var isLoadedOK bool
+
+	if loadedLogFile, isLoadedOK = LoadFile(inputFilename, isInterpolateTime); isLoadedOK == false {
+		return fmt.Errorf("There were input file parsing errors. Could not generate CSV file")
+	}
+
+	//Check if we have all the necessary data
 	if err := validateDataForSotaCsv(loadedLogFile); err != nil {
 		return err
 	}
+
 	outputCsv(verifiedOutputFilename, loadedLogFile)
+
 	return nil
-		}
+		
-		return fmt.Errorf("There were input file parsing errors. Could not generate CSV file")
+
-	}
+	
-	//TODO: we need something more explicit here
-	return fmt.Errorf("Failed to compute or validate output file name")
 }
 
 func validateDataForSotaCsv(loadedLogFile []LogLine) error {

fleprocess/csv_write_test.go

@@ -57,6 +57,17 @@ func Test_buildCsv(t *testing.T) {
 		"V2,ON4KJM/P,ON/ON-001,24/05/20,1312,14MHz,CW,ON4LY",
 	}
 
+	sampleFilledLog2 := []LogLine{
+		{MyCall: "ON4KJM/P", Call: "S57LC", Date: "2020-05-24", Time: "1310", Band: "20m", Frequency: "14.045", Mode: "CW", RSTsent: "599", RSTrcvd: "599", MySOTA: "ON/ON-001", Operator: "ON4KJM", Nickname: "ONFF-0259-1", SOTA: "ON/ON-002"},
+		{MyCall: "ON4KJM/P", Call: "ON4LY", Date: "2020-05-24", Time: "1312", Band: "20m", Mode: "CW", RSTsent: "559", RSTrcvd: "599", MySOTA: "ON/ON-001", Operator: "ON4KJM", Comment: "QSL Message"},
+	}
+	//add case with no SOTA and with or no comment
+
+	expectedOutput2 := []string{
+		"V2,ON4KJM/P,ON/ON-001,24/05/20,1310,14MHz,CW,S57LC,ON/ON-002",
+		"V2,ON4KJM/P,ON/ON-001,24/05/20,1312,14MHz,CW,ON4LY,,QSL Message",
+	}
+
 	type args struct {
 		fullLog []LogLine
 	}
@@ -70,6 +81,11 @@ func Test_buildCsv(t *testing.T) {
 			args{fullLog: sampleFilledLog1},
 			expectedOutput1,
 		},
+		{
+			"Second happy case",
+			args{fullLog: sampleFilledLog2},
+			expectedOutput2,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

fleprocess/output_filename.go

@@ -23,12 +23,11 @@ import (
 )
 
 //buildOutputFilname will try to figure out an output filename (for the case none was provided)
-func buildOutputFilename(output string, input string, overwrite bool, newExtension string) (outputFilename string, wasOK bool) {
+func buildOutputFilename(output string, input string, overwrite bool, newExtension string) (string, error) {
-	outputFilename = ""
 
 	//validate that input is populated (should never happen if properly called)
 	if input == "" {
-		return "", false
+		return "", fmt.Errorf("Unexepected error: no input file provided")
 	}
 
 	//No output was provided, let's create one from the input file
@@ -39,36 +38,21 @@ func buildOutputFilename(output string, input string, overwrite bool, newExtensi
 		fmt.Println("No output provided, defaulting to \"" + output + "\"")
 	}
 
-	//an output was provided by the user
+	//process the computed or user-provided output filename
-	if output != "" {
 	info, err := os.Stat(output)
 	if os.IsNotExist(err) {
 		//File doesn't exist, so we're good
-			return output, true
+		return output, nil
 	}
 	//It exisits but is a directory
 	if info.IsDir() {
-			fmt.Println("Error: specified output exists and is a directory")
+		return "", fmt.Errorf("Error: specified output exists and is a directory")
-			return "", false
 	}
 	if overwrite {
 		//user accepted to overwrite the file
-			return output, true
+		return output, nil
 	}
 
-		fmt.Println("File already exists. Use --overwrite flag if necessary.")
+	return "", fmt.Errorf("File already exists. Use --overwrite flag if necessary")
-		return "", false
-	}
-
-	return outputFilename, true
 }
 
-// fileExists checks if a file exists and is not a directory before we
-// try using it to prevent further errors.
-func fileExists(filename string) bool {
-	info, err := os.Stat(filename)
-	if os.IsNotExist(err) {
-		return false
-	}
-	return !info.IsDir()
-}

fleprocess/output_filename_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 */
 
 import (
+	"errors"
 	"os"
 	"testing"
 )
@@ -52,42 +53,47 @@ func Test_buildOutputFilename(t *testing.T) {
 		name               string
 		args               args
 		wantOutputFilename string
-		wantWasOK          bool
+		wantError          error
 	}{
 		{
 			"input file not provided",
 			args{input: "", output: "xxx", overwrite: false, extension: ".adi"},
-			"", false,
+			"", errors.New("Unexepected error: no input file provided"),
 		},
 		{
 			"Output file does not exist",
 			args{input: "a file", output: "output.adi", overwrite: false, extension: ".adi"},
-			"output.adi", true,
+			"output.adi", nil,
 		},
 		{
 			"Output name is a directory",
 			args{input: "a file", output: testDir, overwrite: false, extension: ".adi"},
-			"", false,
+			"", errors.New("Error: specified output exists and is a directory"),
 		},
 		{
 			"Output exist but no overwrite",
 			args{input: "a file", output: testFile, overwrite: false, extension: ".adi"},
-			"", false,
+			"", errors.New("File already exists. Use --overwrite flag if necessary"),
+		},
+		{
+			"Output exist but user wants to overwrite",
+			args{input: "a file", output: testFile, overwrite: true, extension: ".adi"},
+			"test.adi", nil,
 		},
 		{
 			"no output, input provided with extention",
 			args{input: "/test/data/file.txt", output: "", overwrite: false, extension: ".adi"},
-			"/test/data/file.adi", true,
+			"/test/data/file.adi", nil,
 		},
 		{
 			"no output, input provided without extention",
 			args{input: "/test/data/file", output: "", overwrite: false, extension: ".adi"},
-			"/test/data/file.adi", true,
+			"/test/data/file.adi", nil,
 		},
 		{
 			"no output, input provided, enfing with a point",
 			args{input: "/test/data/file.", output: "", overwrite: false, extension: ".adi"},
-			"/test/data/file.adi", true,
+			"/test/data/file.adi", nil,
 		},
 	}
 
@@ -96,12 +102,18 @@ func Test_buildOutputFilename(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			gotOutputFilename, gotWasOK := buildOutputFilename(tt.args.output, tt.args.input, tt.args.overwrite, tt.args.extension)
+			gotOutputFilename, gotErr := buildOutputFilename(tt.args.output, tt.args.input, tt.args.overwrite, tt.args.extension)
 			if gotOutputFilename != tt.wantOutputFilename {
 				t.Errorf("buildOutputFilename() gotOutputFilename = %v, want %v", gotOutputFilename, tt.wantOutputFilename)
 			}
-			if gotWasOK != tt.wantWasOK {
+			if gotErr != nil && tt.wantError != nil {
-				t.Errorf("buildOutputFilename() gotWasOK = %v, want %v", gotWasOK, tt.wantWasOK)
+				if gotErr.Error() != tt.wantError.Error() {
+					t.Errorf("buildOutputFilename() error = %v, want %v", gotErr, tt.wantError)
+				}
+			} else {
+				if!(gotErr == nil && tt.wantError == nil) {
+					t.Errorf("buildOutputFilename() error = %v, want %v", gotErr, tt.wantError)
+				}
 			}
 		})
 	}