Browse Source

Cleanup the repo API to make the stored objects (blobs and trees) immutable

Omar Jarjur 10 months ago
parent
commit
534d891190
3 changed files with 97 additions and 40 deletions
  1. 24 22
      repository/git.go
  2. 8 2
      repository/mock_repo.go
  3. 65 16
      repository/repo.go

+ 24 - 22
repository/git.go

@@ -123,6 +123,20 @@ func (repo *GitRepo) HasRef(ref string) (bool, error) {
 	return false, err
 }
 
+// HasObject returns whether or not the repo contains an object with the given hash.
+func (repo *GitRepo) HasObject(hash string) (bool, error) {
+	_, err := repo.runGitCommand("cat-file", "-e", hash)
+	if err == nil {
+		// We verified the object exists
+		return true, nil
+	}
+	if _, ok := err.(*exec.ExitError); ok {
+		return false, nil
+	}
+	// Got an unexpected error
+	return false, err
+}
+
 // GetPath returns the path to the repo.
 func (repo *GitRepo) GetPath() string {
 	return repo.Path
@@ -512,14 +526,8 @@ func (repo *GitRepo) ListCommitsBetween(from, to string) ([]string, error) {
 }
 
 // StoreBlob writes the given file to the repository and returns its hash.
-func (repo *GitRepo) StoreBlob(b *Blob) (string, error) {
-	if b.savedHash != "" {
-		if _, existsErr := repo.runGitCommand("cat-file", "-e", b.savedHash); existsErr == nil {
-			// We verified the blob already exists
-			return b.savedHash, nil
-		}
-	}
-	stdin := strings.NewReader(b.Contents)
+func (repo *GitRepo) StoreBlob(contents string) (string, error) {
+	stdin := strings.NewReader(contents)
 	var stdout bytes.Buffer
 	var stderr bytes.Buffer
 	args := []string{"hash-object", "-w", "-t", "blob", "--stdin"}
@@ -532,15 +540,9 @@ func (repo *GitRepo) StoreBlob(b *Blob) (string, error) {
 }
 
 // StoreTree writes the given file tree to the repository and returns its hash.
-func (repo *GitRepo) StoreTree(t *Tree) (string, error) {
-	if t.savedHash != "" {
-		if _, existsErr := repo.runGitCommand("cat-file", "-e", t.savedHash); existsErr == nil {
-			// We verified the tree already exists
-			return t.savedHash, nil
-		}
-	}
+func (repo *GitRepo) StoreTree(contents map[string]TreeChild) (string, error) {
 	var lines []string
-	for path, obj := range t.contents {
+	for path, obj := range contents {
 		objHash, err := obj.Store(repo)
 		if err != nil {
 			return "", err
@@ -569,7 +571,7 @@ func (repo *GitRepo) readBlob(objHash string) (*Blob, error) {
 	if err != nil {
 		return nil, fmt.Errorf("failure reading the file contents of %q: %v", objHash, err)
 	}
-	return &Blob{Contents: out, savedHash: objHash}, nil
+	return &Blob{contents: out, savedHashes: map[Repo]string{repo: objHash}}, nil
 }
 
 func (repo *GitRepo) ReadTree(ref string) (*Tree, error) {
@@ -581,12 +583,11 @@ func (repo *GitRepo) readTreeWithHash(ref, hash string) (*Tree, error) {
 	if err != nil {
 		return nil, fmt.Errorf("failure listing the file contents of %q: %v", ref, err)
 	}
-	t := NewTree()
+	contents := make(map[string]TreeChild)
 	if len(out) == 0 {
 		// This is possible if the tree is empty
-		return t, nil
+		return NewTree(contents), nil
 	}
-	contents := t.Contents()
 	for _, line := range strings.Split(out, "\n") {
 		lineParts := strings.Split(line, "\t")
 		if len(lineParts) != 2 {
@@ -612,7 +613,8 @@ func (repo *GitRepo) readTreeWithHash(ref, hash string) (*Tree, error) {
 		}
 		contents[path] = child
 	}
-	t.savedHash = hash
+	t := NewTree(contents)
+	t.savedHashes[repo] = hash
 	return t, nil
 }
 
@@ -646,7 +648,7 @@ func (repo *GitRepo) CreateCommit(details *CommitDetails) (string, error) {
 
 // CreateCommitWithTree creates a commit object with the given tree and returns its hash.
 func (repo *GitRepo) CreateCommitWithTree(details *CommitDetails, t *Tree) (string, error) {
-	treeHash, err := repo.StoreTree(t)
+	treeHash, err := repo.StoreTree(t.Contents())
 	if err != nil {
 		return "", fmt.Errorf("failure storing a tree: %v", err)
 	}

+ 8 - 2
repository/mock_repo.go

@@ -19,6 +19,7 @@ package repository
 import (
 	"crypto/sha1"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"strings"
 )
@@ -230,6 +231,11 @@ func (r *mockRepoForTest) HasRef(ref string) (bool, error) {
 	return true, nil
 }
 
+// HasObject reports whether or not the repo contains an object with the given hash
+func (r *mockRepoForTest) HasObject(hash string) (bool, error) {
+	return false, errors.New("Not implemented")
+}
+
 // VerifyCommit verifies that the supplied hash points to a known commit.
 func (r *mockRepoForTest) VerifyCommit(hash string) error {
 	if _, ok := r.Commits[hash]; !ok {
@@ -525,12 +531,12 @@ func (r *mockRepoForTest) ListCommitsBetween(from, to string) ([]string, error)
 }
 
 // StoreBlob writes the given file to the repository and returns its hash.
-func (r *mockRepoForTest) StoreBlob(b *Blob) (string, error) {
+func (r *mockRepoForTest) StoreBlob(contents string) (string, error) {
 	return "", fmt.Errorf("not implemented")
 }
 
 // StoreTree writes the given file tree to the repository and returns its hash.
-func (r *mockRepoForTest) StoreTree(t *Tree) (string, error) {
+func (r *mockRepoForTest) StoreTree(contents map[string]TreeChild) (string, error) {
 	return "", fmt.Errorf("not implemented")
 }
 

+ 65 - 16
repository/repo.go

@@ -47,15 +47,25 @@ type TreeChild interface {
 	// Type returns the type of the child object (e.g. "blob" vs. "tree").
 	Type() string
 
-	// Store writes the object to the given repository and returns its hash.
+	// Store writes the object to the repository and returns its hash.
 	Store(repo Repo) (string, error)
 }
 
 // Blob represents a (non-directory) file stored in a repository.
+//
+// Blob objects are immutable.
 type Blob struct {
-	Contents string
+	savedHashes map[Repo]string
+	contents    string
+}
 
-	savedHash string
+// NewBlob returns a new *Blob object tied to the given repo with the given contents.
+func NewBlob(contents string) *Blob {
+	savedHashes := make(map[Repo]string)
+	return &Blob{
+		savedHashes: savedHashes,
+		contents:    contents,
+	}
 }
 
 func (b *Blob) Type() string {
@@ -63,17 +73,40 @@ func (b *Blob) Type() string {
 }
 
 func (b *Blob) Store(repo Repo) (string, error) {
-	return repo.StoreBlob(b)
+	if savedHash := b.savedHashes[repo]; savedHash != "" {
+		return savedHash, nil
+	}
+	savedHash, err := repo.StoreBlob(b.Contents())
+	if err == nil && savedHash != "" {
+		b.savedHashes[repo] = savedHash
+	}
+	return savedHash, nil
+}
+
+// Contents returns the contents of the blob
+func (b *Blob) Contents() string {
+	return b.contents
 }
 
 // Tree represents a directory stored in a repository.
+//
+// Tree objects are immutable.
 type Tree struct {
-	contents  map[string]TreeChild
-	savedHash string
+	savedHashes map[Repo]string
+	contents    map[string]TreeChild
 }
 
-func NewTree() *Tree {
-	return &Tree{contents: make(map[string]TreeChild)}
+// NewTree constructs a new *Tree object tied to the given repo with the given contents.
+func NewTree(contents map[string]TreeChild) *Tree {
+	immutableContents := make(map[string]TreeChild)
+	for k, v := range contents {
+		immutableContents[k] = v
+	}
+	savedHashes := make(map[Repo]string)
+	return &Tree{
+		savedHashes: savedHashes,
+		contents:    immutableContents,
+	}
 }
 
 func (t *Tree) Type() string {
@@ -81,13 +114,26 @@ func (t *Tree) Type() string {
 }
 
 func (t *Tree) Store(repo Repo) (string, error) {
-	return repo.StoreTree(t)
+	if savedHash := t.savedHashes[repo]; savedHash != "" {
+		return savedHash, nil
+	}
+	savedHash, err := repo.StoreTree(t.Contents())
+	if err == nil && savedHash != "" {
+		t.savedHashes[repo] = savedHash
+	}
+	return savedHash, nil
 }
 
+// Contents returns a map of the child elements of the tree.
+//
+// The returned map is mutable, but changes made to it have no
+// effect on the underly Tree object.
 func (t *Tree) Contents() map[string]TreeChild {
-	// Since the returned contents are mutable, we have to assume the hash could change.
-	t.savedHash = ""
-	return t.contents
+	result := make(map[string]TreeChild)
+	for k, v := range t.contents {
+		result[k] = v
+	}
+	return result
 }
 
 // Repo represents a source code repository.
@@ -117,6 +163,9 @@ type Repo interface {
 	// HasRef checks whether the specified ref exists in the repo.
 	HasRef(ref string) (bool, error)
 
+	// HasObject returns whether or not the repo contains an object with the given hash.
+	HasObject(hash string) (bool, error)
+
 	// VerifyCommit verifies that the supplied hash points to a known commit.
 	VerifyCommit(hash string) error
 
@@ -225,11 +274,11 @@ type Repo interface {
 	// The generated list is in chronological order (with the oldest commit first).
 	ListCommitsBetween(from, to string) ([]string, error)
 
-	// StoreBlob writes the given file to the repository and returns its hash.
-	StoreBlob(b *Blob) (string, error)
+	// StoreBlob writes the given file contents to the repository and returns its hash.
+	StoreBlob(contents string) (string, error)
 
-	// StoreTree writes the given file tree to the repository and returns its hash.
-	StoreTree(t *Tree) (string, error)
+	// StoreTree writes the given file tree contents to the repository and returns its hash.
+	StoreTree(contents map[string]TreeChild) (string, error)
 
 	// ReadTree reads the file tree pointed to by the given ref or hash from the repository.
 	ReadTree(ref string) (*Tree, error)