check: re-implement *Check() using Compare()

This removes all of the special handling code for both TarCheck() and
Check() so that everything now uses the new (generic) Compare() code. In
addition, the tests had to be modified to reflect the new classes of
errors.

Signed-off-by: Aleksa Sarai <asarai@suse.de>

check.go

@@ -1,194 +1,31 @@
 package mtree
 
-import (
-	"fmt"
-	"os"
-	"sort"
-	"strings"
-)
-
-// Result of a Check
-type Result struct {
-	// list of any failures in the Check
-	Failures []Failure `json:"failures"`
-	Missing  []Entry   `json:"missing,omitempty"`
-	Extra    []Entry   `json:"extra,omitempty"`
-}
-
-// Failure of a particular keyword for a path
-type Failure struct {
-	Path     string `json:"path"`
-	Keyword  string `json:"keyword"`
-	Expected string `json:"expected"`
-	Got      string `json:"got"`
-}
-
-// String returns a "pretty" formatting for a Failure
-func (f Failure) String() string {
-	return fmt.Sprintf("%q: keyword %q: expected %s; got %s", f.Path, f.Keyword, f.Expected, f.Got)
-}
-
 // Check a root directory path against the DirectoryHierarchy, regarding only
 // the available keywords from the list and each entry in the hierarchy.
 // If keywords is nil, the check all present in the DirectoryHierarchy
-func Check(root string, dh *DirectoryHierarchy, keywords []string) (*Result, error) {
+//
-	curDir, err := os.Getwd()
+// This is equivalent to creating a new DirectoryHierarchy with Walk(root, nil,
-	if err == nil {
+// keywords) and then doing a Compare(dh, newDh, keywords).
-		defer os.Chdir(curDir)
+func Check(root string, dh *DirectoryHierarchy, keywords []string) ([]InodeDelta, error) {
+	if keywords == nil {
+		keywords = CollectUsedKeywords(dh)
 	}
 
-	if err := os.Chdir(root); err != nil {
+	newDh, err := Walk(root, nil, keywords)
-		return nil, err
-	}
-	sort.Sort(byPos(dh.Entries))
-
-	var result Result
-	for _, e := range dh.Entries {
-		switch e.Type {
-		case RelativeType, FullType:
-			pathname, err := e.Path()
-			if err != nil {
-				return nil, err
-			}
-			info, err := os.Lstat(pathname)
 	if err != nil {
 		return nil, err
 	}
 
-			kvs := e.AllKeys()
+	// TODO: Handle tar_time, if necessary.
-
+	return Compare(dh, newDh, keywords)
-			for _, kv := range kvs {
-				kw := kv.Keyword()
-				// 'tar_time' keyword evaluation wins against 'time' keyword evaluation
-				if kv.Keyword() == "time" && inSlice("tar_time", keywords) {
-					kw = "tar_time"
-					tartime := fmt.Sprintf("%s.%s", (strings.Split(kv.Value(), ".")[0]), "000000000")
-					kv = KeyVal(KeyVal(kw).ChangeValue(tartime))
 }
 
-				keywordFunc, ok := KeywordFuncs[kw]
+// TarCheck is the tar equivalent of checking a file hierarchy spec against a
-				if !ok {
+// tar stream to determine if files have been changed. This is precisely
-					return nil, fmt.Errorf("Unknown keyword %q for file %q", kv.Keyword(), pathname)
+// equivalent to Compare(dh, tarDH, keywords).
+func TarCheck(tarDH, dh *DirectoryHierarchy, keywords []string) ([]InodeDelta, error) {
+	if keywords == nil {
+		keywords = CollectUsedKeywords(dh)
 	}
-				if keywords != nil && !inSlice(kv.Keyword(), keywords) {
+	return Compare(dh, tarDH, keywords)
-					continue
-				}
-
-				var curKeyVal string
-				if info.Mode().IsRegular() {
-					fh, err := os.Open(pathname)
-					if err != nil {
-						return nil, err
-					}
-					curKeyVal, err = keywordFunc(pathname, info, fh)
-					if err != nil {
-						fh.Close()
-						return nil, err
-					}
-					fh.Close()
-				} else {
-					curKeyVal, err = keywordFunc(pathname, info, nil)
-					if err != nil {
-						return nil, err
-					}
-				}
-				if string(kv) != curKeyVal {
-					failure := Failure{Path: pathname, Keyword: kv.Keyword(), Expected: kv.Value(), Got: KeyVal(curKeyVal).Value()}
-					result.Failures = append(result.Failures, failure)
-				}
-			}
-		}
-	}
-	return &result, nil
-}
-
-// TarCheck is the tar equivalent of checking a file hierarchy spec against a tar stream to
-// determine if files have been changed.
-func TarCheck(tarDH, dh *DirectoryHierarchy, keywords []string) (*Result, error) {
-	var result Result
-	var err error
-	var tarRoot *Entry
-
-	for _, e := range tarDH.Entries {
-		if e.Name == "." {
-			tarRoot = &e
-			break
-		}
-	}
-	if tarRoot == nil {
-		return nil, fmt.Errorf("root of archive could not be found")
-	}
-	tarRoot.Next = &Entry{
-		Name: "seen",
-		Type: CommentType,
-	}
-	curDir := tarRoot
-	sort.Sort(byPos(dh.Entries))
-
-	var outOfTree bool
-	for _, e := range dh.Entries {
-		switch e.Type {
-		case RelativeType, FullType:
-			pathname, err := e.Path()
-			if err != nil {
-				return nil, err
-			}
-			if outOfTree {
-				return &result, fmt.Errorf("No parent node from %s", pathname)
-			}
-			// TODO: handle the case where "." is not the first Entry to be found
-			tarEntry := curDir.Descend(e.Name)
-			if tarEntry == nil {
-				result.Missing = append(result.Missing, e)
-				continue
-			}
-
-			tarEntry.Next = &Entry{
-				Type: CommentType,
-				Name: "seen",
-			}
-
-			// expected values from file hierarchy spec
-			kvs := e.AllKeys()
-
-			// actual
-			tarkvs := tarEntry.AllKeys()
-
-			for _, kv := range kvs {
-				// TODO: keep track of symlinks
-				if _, ok := KeywordFuncs[kv.Keyword()]; !ok {
-					return nil, fmt.Errorf("Unknown keyword %q for file %q", kv.Keyword(), pathname)
-				}
-				if keywords != nil && !inSlice(kv.Keyword(), keywords) {
-					continue
-				}
-				tarpath, err := tarEntry.Path()
-				if err != nil {
-					return nil, err
-				}
-				if tarkv := tarkvs.Has(kv.Keyword()); tarkv != emptyKV {
-					if string(tarkv) != string(kv) {
-						failure := Failure{Path: tarpath, Keyword: kv.Keyword(), Expected: kv.Value(), Got: tarkv.Value()}
-						result.Failures = append(result.Failures, failure)
-					}
-				}
-			}
-			// Step into a directory
-			if tarEntry.Prev != nil {
-				curDir = tarEntry
-			}
-		case DotDotType:
-			if outOfTree {
-				return &result, fmt.Errorf("No parent node.")
-			}
-			curDir = curDir.Ascend()
-			if curDir == nil {
-				outOfTree = true
-			}
-		}
-	}
-	result.Extra = filter(tarRoot, func(e *Entry) bool {
-		return e.Next == nil
-	})
-	return &result, err
 }

check_test.go

@@ -22,7 +22,7 @@ func TestCheck(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if len(res.Failures) > 0 {
+	if len(res) > 0 {
 		t.Errorf("%#v", res)
 	}
 }
@@ -53,7 +53,7 @@ func TestCheckKeywords(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if len(res.Failures) > 0 {
+	if len(res) > 0 {
 		t.Errorf("%#v", res)
 	}
 
@@ -68,8 +68,11 @@ func TestCheckKeywords(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if len(res.Failures) == 0 {
+	if len(res) != 1 {
-		t.Errorf("expected to fail on changed mtimes, but did not")
+		t.Errorf("expected to get 1 delta on changed mtimes, but did not")
+	}
+	if res[0].Type() != Modified {
+		t.Errorf("expected to get modified delta on changed mtimes, but did not")
 	}
 
 	// Check again, but only sha1 and mode. This ought to pass.
@@ -77,7 +80,7 @@ func TestCheckKeywords(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if len(res.Failures) > 0 {
+	if len(res) > 0 {
 		t.Errorf("%#v", res)
 	}
 }
@@ -92,7 +95,7 @@ func ExampleCheck() {
 	if err != nil {
 		// handle error ...
 	}
-	if len(res.Failures) > 0 {
+	if len(res) > 0 {
 		// handle failed validity ...
 	}
 }
@@ -108,9 +111,9 @@ func TestDefaultBrokenLink(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if res != nil && len(res.Failures) > 0 {
+	if len(res) > 0 {
-		for _, f := range res.Failures {
+		for _, delta := range res {
-			t.Error(f)
+			t.Error(delta)
 		}
 	}
 }
@@ -156,8 +159,8 @@ func TestTimeComparison(t *testing.T) {
 	if err != nil {
 		t.Error(err)
 	}
-	if len(res.Failures) > 0 {
+	if len(res) > 0 {
-		t.Fatal(res.Failures)
+		t.Fatal(res)
 	}
 }
 
@@ -197,19 +200,21 @@ func TestTarTime(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	keywords := CollectUsedKeywords(dh)
+
 	// make sure "time" keyword works
-	_, err = Check(dir, dh, DefaultKeywords)
+	_, err = Check(dir, dh, keywords)
 	if err != nil {
 		t.Error(err)
 	}
 
 	// make sure tar_time wins
-	res, err := Check(dir, dh, append(DefaultKeywords, "tar_time"))
+	res, err := Check(dir, dh, append(keywords, "tar_time"))
 	if err != nil {
 		t.Error(err)
 	}
-	if len(res.Failures) > 0 {
+	if len(res) > 0 {
-		t.Fatal(res.Failures)
+		t.Fatal(res)
 	}
 }
 
@@ -254,8 +259,8 @@ func TestIgnoreComments(t *testing.T) {
 		t.Error(err)
 	}
 
-	if len(res.Failures) > 0 {
+	if len(res) > 0 {
-		t.Fatal(res.Failures)
+		t.Fatal(res)
 	}
 
 	// now change the spec to a comment that looks like an actual Entry but has
@@ -274,8 +279,8 @@ func TestIgnoreComments(t *testing.T) {
 		t.Error(err)
 	}
 
-	if len(res.Failures) > 0 {
+	if len(res) > 0 {
-		t.Fatal(res.Failures)
+		t.Fatal(res)
 	}
 }
 
@@ -309,7 +314,7 @@ func TestCheckNeedsEncoding(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if len(res.Failures) > 0 {
+	if len(res) > 0 {
-		t.Fatal(res.Failures)
+		t.Fatal(res)
 	}
 }

tar_test.go

@@ -30,7 +30,7 @@ func ExampleStreamer() {
 	if err != nil {
 		// handle error ...
 	}
-	if len(res.Failures) > 0 {
+	if len(res) > 0 {
 		// handle validation issue ...
 	}
 }
@@ -104,43 +104,17 @@ func TestTar(t *testing.T) {
 	}
 
 	res, err := TarCheck(tdh, dh, append(DefaultKeywords, "sha1"))
-
 	if err != nil {
 		t.Fatal(err)
 	}
 
 	// print any failures, and then call t.Fatal once all failures/extra/missing
 	// are outputted
-	if res != nil {
+	if len(res) > 0 {
-		errors := ""
+		for _, delta := range res {
-		switch {
+			t.Error(delta)
-		case len(res.Failures) > 0:
-			for _, f := range res.Failures {
-				t.Errorf("%s\n", f)
-			}
-			errors += "Keyword validation errors\n"
-		case len(res.Missing) > 0:
-			for _, m := range res.Missing {
-				missingpath, err := m.Path()
-				if err != nil {
-					t.Fatal(err)
-				}
-				t.Errorf("Missing file: %s\n", missingpath)
-			}
-			errors += "Missing files not expected for this test\n"
-		case len(res.Extra) > 0:
-			for _, e := range res.Extra {
-				extrapath, err := e.Path()
-				if err != nil {
-					t.Fatal(err)
-				}
-				t.Errorf("Extra file: %s\n", extrapath)
-			}
-			errors += "Extra files not expected for this test\n"
-		}
-		if errors != "" {
-			t.Fatal(errors)
 		}
+		t.Fatal("unexpected errors")
 	}
 }
 
@@ -176,16 +150,11 @@ func TestArchiveCreation(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if res != nil {
+	if len(res) > 0 {
-		for _, f := range res.Failures {
+		for _, delta := range res {
-			t.Errorf(f.String())
+			t.Error(delta)
-		}
-		for _, e := range res.Extra {
-			t.Errorf("%s extra not expected", e.Name)
-		}
-		for _, m := range res.Missing {
-			t.Errorf("%s missing not expected", m.Name)
 		}
+		t.Fatal("unexpected errors")
 	}
 
 	// Test the tar manifest against itself
@@ -193,16 +162,11 @@ func TestArchiveCreation(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if res != nil {
+	if len(res) > 0 {
-		for _, f := range res.Failures {
+		for _, delta := range res {
-			t.Errorf(f.String())
+			t.Error(delta)
-		}
-		for _, e := range res.Extra {
-			t.Errorf("%s extra not expected", e.Name)
-		}
-		for _, m := range res.Missing {
-			t.Errorf("%s missing not expected", m.Name)
 		}
+		t.Fatal("unexpected errors")
 	}
 
 	// Validate the directory manifest against the archive
@@ -214,16 +178,11 @@ func TestArchiveCreation(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if res != nil {
+	if len(res) > 0 {
-		for _, f := range res.Failures {
+		for _, delta := range res {
-			t.Errorf(f.String())
+			t.Error(delta)
-		}
-		for _, e := range res.Extra {
-			t.Errorf("%s extra not expected", e.Name)
-		}
-		for _, m := range res.Missing {
-			t.Errorf("%s missing not expected", m.Name)
 		}
+		t.Fatal("unexpected errors")
 	}
 }
 
@@ -257,16 +216,11 @@ func TestTreeTraversal(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if res != nil {
+	if len(res) > 0 {
-		for _, f := range res.Failures {
+		for _, delta := range res {
-			t.Errorf(f.String())
+			t.Error(delta)
-		}
-		for _, e := range res.Extra {
-			t.Errorf("%s extra not expected", e.Name)
-		}
-		for _, m := range res.Missing {
-			t.Errorf("%s missing not expected", m.Name)
 		}
+		t.Fatal("unexpected errors")
 	}
 
 	// top-level "." directory will contain contents of traversal.tar
@@ -274,9 +228,18 @@ func TestTreeTraversal(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if res != nil {
+	if len(res) > 0 {
-		for _, f := range res.Failures {
+		var failed bool
-			t.Errorf(f.String())
+		for _, delta := range res {
+			// We only care about missing or modified files.
+			// The original test was written using the old check code.
+			if delta.Type() != Extra {
+				failed = true
+				t.Error(delta)
+			}
+		}
+		if failed {
+			t.Fatal("unexpected errors")
 		}
 	}
 
@@ -303,9 +266,18 @@ func TestTreeTraversal(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if res != nil {
+	if len(res) > 0 {
-		for _, f := range res.Failures {
+		var failed bool
-			t.Errorf(f.String())
+		for _, delta := range res {
+			// We only care about missing or modified files.
+			// The original test was written using the old check code.
+			if delta.Type() != Extra {
+				failed = true
+				t.Error(delta)
+			}
+		}
+		if failed {
+			t.Fatal("unexpected errors")
 		}
 	}
 }