Output File Exists Error

Signed-off-by: Julian Castrence <juliancastrence@ibm.com>

cmd/ledgerutil/main.go

@@ -16,7 +16,7 @@ import (
 )
 
 const (
-	resultFilename = "result.json"
+	compareErrorMessage = "Ledger Compare Error: "
 )
 
 var (
@@ -41,30 +41,42 @@ func main() {
 		return
 	}
 
-	// Determine result json file location
-	var resultFilepath string
-	if outputDir != nil {
-		resultFilepath = *outputDir
-	} else {
-		resultFilepath, err = os.Getwd()
-		if err != nil {
-			fmt.Println(err)
-			return
-		}
-	}
-	resultFilepath = filepath.Join(resultFilepath, resultFilename)
-
 	// Command logic
 	switch command {
 
 	case compare.FullCommand():
 
-		count, err := ledgerutil.Compare(*snapshotPath1, *snapshotPath2, resultFilepath)
+		cleanpath := filepath.Clean(*outputDir)
+
+		// Determine result json file location
+		if *outputDir == "" {
+			*outputDir, err = os.Getwd()
+			if err != nil {
+				fmt.Printf("%s%s\n", compareErrorMessage, err)
+				return
+			}
+			cleanpath = "the current directory"
+		}
+		// Clean output
+		switch cleanpath {
+		case ".":
+			cleanpath = "the current directory"
+		case "..":
+			cleanpath = "the parent directory"
+		}
+
+		count, err := ledgerutil.Compare(*snapshotPath1, *snapshotPath2, *outputDir)
 		if err != nil {
-			fmt.Println(err)
+			fmt.Printf("%s%s\n", compareErrorMessage, err)
 			return
 		}
-		fmt.Printf("\nSuccessfully compared snapshots. Result saved to %s. Total differences found: %d\n", resultFilepath, count)
+
+		fmt.Print("\nSuccessfully compared snapshots. ")
+		if count == -1 {
+			fmt.Println("Both snapshot public state hashes were the same. No results were generated.")
+		} else {
+			fmt.Printf("Results saved to %s. Total differences found: %d\n", cleanpath, count)
+		}
 
 	case troubleshoot.FullCommand():
 

internal/ledgerutil/compare.go

@@ -10,6 +10,7 @@ import (
 	"bufio"
 	"bytes"
 	"encoding/json"
+	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -23,24 +24,48 @@ import (
 	"github.com/pkg/errors"
 )
 
+const (
+	// AllDiffsByKey - Filename for the json output that contains all differences ordered by key
+	AllDiffsByKey = "all_diffs_by_key.json"
+)
+
 // Compare - Compares two ledger snapshots and outputs the differences in snapshot records
-// This function will overwrite the file at outputPath if it already exists
-func Compare(snapshotDir1 string, snapshotDir2 string, outputFile string) (count int, err error) {
+// This function will throw an error if the output directory already exist in the outputDirLoc
+// Function will return count of -1 if the public state hashes are the same
+func Compare(snapshotDir1 string, snapshotDir2 string, outputDirLoc string) (count int, err error) {
 	// Check the hashes between two files
 	hashPath1 := filepath.Join(snapshotDir1, kvledger.SnapshotSignableMetadataFileName)
 	hashPath2 := filepath.Join(snapshotDir2, kvledger.SnapshotSignableMetadataFileName)
 
-	equal, err := snapshotsComparable(hashPath1, hashPath2)
+	equal, channelName, blockHeight, err := snapshotsComparable(hashPath1, hashPath2)
 	if err != nil {
 		return 0, err
 	}
 
 	if equal {
-		return 0, errors.New("both snapshots public state hashes are same. Aborting compare")
+		return -1, nil
+	}
+
+	// Output directory creation
+	outputDirName := fmt.Sprintf("%s_%d_comparison", channelName, blockHeight)
+	outputDirPath := filepath.Join(outputDirLoc, outputDirName)
+
+	empty, err := fileutil.CreateDirIfMissing(outputDirPath)
+	if err != nil {
+		return 0, err
+	}
+	if !empty {
+		switch outputDirLoc {
+		case ".":
+			outputDirLoc = "the current directory"
+		case "..":
+			outputDirLoc = "the parent directory"
+		}
+		return 0, errors.Errorf("%s already exists in %s. Choose a different location or remove the existing results. Aborting compare", outputDirName, outputDirLoc)
 	}
 
 	// Create the output file
-	jsonOutputFile, err := newJSONFileWriter(outputFile)
+	jsonOutputFile, err := newJSONFileWriter(filepath.Join(outputDirPath, AllDiffsByKey))
 	if err != nil {
 		return 0, err
 	}
@@ -57,7 +82,7 @@ func Compare(snapshotDir1 string, snapshotDir2 string, outputFile string) (count
 		return 0, err
 	}
 
-	// Read each snapshot record  to begin looking for divergences
+	// Read each snapshot record  to begin looking for differences
 	namespace1, snapshotRecord1, err := snapshotReader1.Next()
 	if err != nil {
 		return 0, err
@@ -278,49 +303,55 @@ func nsKeyCompare(k1, k2 *nsKey) int {
 	return bytes.Compare(k1.key, k2.key)
 }
 
-// Compares hashes of snapshots
-func snapshotsComparable(fpath1 string, fpath2 string) (bool, error) {
-	var mdata1, mdata2 kvledger.SnapshotSignableMetadata
+// Extracts metadata from provided filepath
+func readMetadata(fpath string) (*kvledger.SnapshotSignableMetadata, error) {
+	var mdata kvledger.SnapshotSignableMetadata
 
-	// Open the first file
-	f, err := ioutil.ReadFile(fpath1)
+	// Open file
+	f, err := ioutil.ReadFile(fpath)
 	if err != nil {
-		return false, err
+		return nil, err
 	}
-
-	err = json.Unmarshal([]byte(f), &mdata1)
+	// Unmarshal bytes
+	err = json.Unmarshal([]byte(f), &mdata)
 	if err != nil {
-		return false, err
+		return nil, err
 	}
 
-	// Open the second file
-	f, err = ioutil.ReadFile(fpath2)
+	return &mdata, nil
+}
+
+// Compares hashes of snapshots
+func snapshotsComparable(fpath1 string, fpath2 string) (bool, string, uint64, error) {
+	var mdata1, mdata2 *kvledger.SnapshotSignableMetadata
+
+	// Read metadata from snapshot metadata filepaths
+	mdata1, err := readMetadata(fpath1)
 	if err != nil {
-		return false, err
+		return false, "", 0, err
 	}
-
-	err = json.Unmarshal([]byte(f), &mdata2)
+	mdata2, err = readMetadata(fpath2)
 	if err != nil {
-		return false, err
+		return false, "", 0, err
 	}
 
 	if mdata1.ChannelName != mdata2.ChannelName {
-		return false, errors.Errorf("the supplied snapshots appear to be non-comparable. Channel names do not match."+
+		return false, "", 0, errors.Errorf("the supplied snapshots appear to be non-comparable. Channel names do not match."+
 			"\nSnapshot1 channel name: %s\nSnapshot2 channel name: %s", mdata1.ChannelName, mdata2.ChannelName)
 	}
 
 	if mdata1.LastBlockNumber != mdata2.LastBlockNumber {
-		return false, errors.Errorf("the supplied snapshots appear to be non-comparable. Last block numbers do not match."+
+		return false, "", 0, errors.Errorf("the supplied snapshots appear to be non-comparable. Last block numbers do not match."+
 			"\nSnapshot1 last block number: %v\nSnapshot2 last block number: %v", mdata1.LastBlockNumber, mdata2.LastBlockNumber)
 	}
 
 	if mdata1.LastBlockHashInHex != mdata2.LastBlockHashInHex {
-		return false, errors.Errorf("the supplied snapshots appear to be non-comparable. Last block hashes do not match."+
+		return false, "", 0, errors.Errorf("the supplied snapshots appear to be non-comparable. Last block hashes do not match."+
 			"\nSnapshot1 last block hash: %s\nSnapshot2 last block hash: %s", mdata1.LastBlockHashInHex, mdata2.LastBlockHashInHex)
 	}
 
 	if mdata1.StateDBType != mdata2.StateDBType {
-		return false, errors.Errorf("the supplied snapshots appear to be non-comparable. State db types do not match."+
+		return false, "", 0, errors.Errorf("the supplied snapshots appear to be non-comparable. State db types do not match."+
 			"\nSnapshot1 state db type: %s\nSnapshot2 state db type: %s", mdata1.StateDBType, mdata2.StateDBType)
 	}
 
@@ -330,7 +361,7 @@ func snapshotsComparable(fpath1 string, fpath2 string) (bool, error) {
 	pubDataHash2 := mdata2.FilesAndHashes[privacyenabledstate.PubStateDataFileName]
 	pubMdataHash2 := mdata2.FilesAndHashes[privacyenabledstate.PubStateMetadataFileName]
 
-	return (pubDataHash1 == pubDataHash2 && pubMdataHash1 == pubMdataHash2), nil
+	return (pubDataHash1 == pubDataHash2 && pubMdataHash1 == pubMdataHash2), mdata1.ChannelName, mdata1.LastBlockNumber, nil
 }
 
 // jsonArrayFileWriter writes a list of diffRecords to a json file

internal/ledgerutil/compare_test.go

@@ -236,7 +236,6 @@ func TestCompare(t *testing.T) {
 				}
 			}
 		]`
-	expectedSamePubStateError := "both snapshots public state hashes are same. Aborting compare"
 	expectedDiffDatabaseError := "the supplied snapshots appear to be non-comparable. State db types do not match." +
 		"\nSnapshot1 state db type: testdatabase\nSnapshot2 state db type: testdatabase2"
 
@@ -305,9 +304,9 @@ func TestCompare(t *testing.T) {
 			inputSignableMetadata1: sampleSignableMetadata1,
 			inputTestRecords2:      sampleRecords1,
 			inputSignableMetadata2: sampleSignableMetadata1,
-			expectedOutput:         expectedSamePubStateError,
-			expectedOutputType:     "error",
-			expectedDiffCount:      0,
+			expectedOutput:         "",
+			expectedOutputType:     "same-hash",
+			expectedDiffCount:      -1,
 		},
 		// Snapshots contain different metadata (different databases in this case) that makes them non-comparable
 		"different-database": {
@@ -319,6 +318,15 @@ func TestCompare(t *testing.T) {
 			expectedOutputType:     "error",
 			expectedDiffCount:      0,
 		},
+		// Output directory file already exists
+		"output-dir-exists": {
+			inputTestRecords1:      sampleRecords1,
+			inputSignableMetadata1: sampleSignableMetadata1,
+			inputTestRecords2:      sampleRecords2,
+			inputSignableMetadata2: sampleSignableMetadata2,
+			expectedOutputType:     "exists-error",
+			expectedDiffCount:      0,
+		},
 	}
 
 	// Run test cases individually
@@ -342,14 +350,23 @@ func TestCompare(t *testing.T) {
 			require.NoError(t, err)
 
 			// Compare snapshots and check the output
-			count, out, err := compareSnapshots(snapshotDir1, snapshotDir2, filepath.Join(resultsDir, "results.json"))
-			require.Equal(t, testCase.expectedDiffCount, count)
+			count, out, err := compareSnapshots(snapshotDir1, snapshotDir2, resultsDir)
 			switch testCase.expectedOutputType {
 			case "error":
+				require.Equal(t, testCase.expectedDiffCount, count)
 				require.ErrorContains(t, err, testCase.expectedOutput)
+			case "exists-error":
+				require.NoError(t, err)
+				count, _, err = compareSnapshots(snapshotDir1, snapshotDir2, resultsDir)
+				require.Equal(t, testCase.expectedDiffCount, count)
+				require.ErrorContains(t, err, "testchannel_2_comparison already exists in "+resultsDir+". Choose a different location or remove the existing results. Aborting compare")
 			case "json":
+				require.Equal(t, testCase.expectedDiffCount, count)
 				require.NoError(t, err)
 				require.JSONEq(t, testCase.expectedOutput, out)
+			case "same-hash":
+				require.Equal(t, testCase.expectedDiffCount, count)
+				require.NoError(t, err)
 			default:
 				panic("unexpected code path: bug")
 			}
@@ -407,11 +424,17 @@ func createSnapshot(dir string, pubStateRecords []*testRecord, signableMetadata
 func compareSnapshots(ss1 string, ss2 string, res string) (int, string, error) {
 	// Run compare tool on snapshots
 	count, err := Compare(ss1, ss2, res)
+	if err != nil || count == -1 {
+		return count, "", err
+	}
+
+	// Read results of output
+	md, err := readMetadata(filepath.Join(ss1, kvledger.SnapshotSignableMetadataFileName))
 	if err != nil {
 		return 0, "", err
 	}
-	// Read results of output
-	resBytes, err := ioutil.ReadFile(res)
+	odir := fmt.Sprintf("%s_%d_comparison", md.ChannelName, md.LastBlockNumber)
+	resBytes, err := ioutil.ReadFile(filepath.Join(res, odir, AllDiffsByKey))
 	if err != nil {
 		return 0, "", err
 	}