repository: return specific error on object not found, accept multiple namespace to push/pull

5 files changed, 85 insertions, 131 deletions

diff --git a/repository/gogit.go b/repository/gogit.go
index 746204df..781eec73 100644
--- a/repository/gogit.go
+++ b/repository/gogit.go
@@ -342,14 +342,18 @@ func (repo *GoGitRepo) GetIndex(name string) (Index, error) {
 // FetchRefs fetch git refs matching a directory prefix to a remote
 // Ex: prefix="foo" will fetch any remote refs matching "refs/foo/*" locally.
 // The equivalent git refspec would be "refs/foo/*:refs/remotes/<remote>/foo/*"
-func (repo *GoGitRepo) FetchRefs(remote string, prefix string) (string, error) {
-	refspec := fmt.Sprintf("refs/%s/*:refs/remotes/%s/%s/*", prefix, remote, prefix)
+func (repo *GoGitRepo) FetchRefs(remote string, prefixes ...string) (string, error) {
+	refSpecs := make([]config.RefSpec, len(prefixes))
+
+	for i, prefix := range prefixes {
+		refSpecs[i] = config.RefSpec(fmt.Sprintf("refs/%s/*:refs/remotes/%s/%s/*", prefix, remote, prefix))
+	}
 
 	buf := bytes.NewBuffer(nil)
 
 	err := repo.r.Fetch(&gogit.FetchOptions{
 		RemoteName: remote,
-		RefSpecs:   []config.RefSpec{config.RefSpec(refspec)},
+		RefSpecs:   refSpecs,
 		Progress:   buf,
 	})
 	if err == gogit.NoErrAlreadyUpToDate {
@@ -368,35 +372,41 @@ func (repo *GoGitRepo) FetchRefs(remote string, prefix string) (string, error) {
 //
 // Additionally, PushRefs will update the local references in refs/remotes/<remote>/foo to match
 // the remote state.
-func (repo *GoGitRepo) PushRefs(remote string, prefix string) (string, error) {
-	refspec := fmt.Sprintf("refs/%s/*:refs/%s/*", prefix, prefix)
-
+func (repo *GoGitRepo) PushRefs(remote string, prefixes ...string) (string, error) {
 	remo, err := repo.r.Remote(remote)
 	if err != nil {
 		return "", err
 	}
 
-	// to make sure that the push also create the corresponding refs/remotes/<remote>/... references,
-	// we need to have a default fetch refspec configured on the remote, to make our refs "track" the remote ones.
-	// This does not change the config on disk, only on memory.
-	hasCustomFetch := false
-	fetchRefspec := fmt.Sprintf("refs/%s/*:refs/remotes/%s/%s/*", prefix, remote, prefix)
-	for _, r := range remo.Config().Fetch {
-		if string(r) == fetchRefspec {
-			hasCustomFetch = true
-			break
+	refSpecs := make([]config.RefSpec, len(prefixes))
+
+	for i, prefix := range prefixes {
+		refspec := fmt.Sprintf("refs/%s/*:refs/%s/*", prefix, prefix)
+
+		// to make sure that the push also create the corresponding refs/remotes/<remote>/... references,
+		// we need to have a default fetch refspec configured on the remote, to make our refs "track" the remote ones.
+		// This does not change the config on disk, only on memory.
+		hasCustomFetch := false
+		fetchRefspec := fmt.Sprintf("refs/%s/*:refs/remotes/%s/%s/*", prefix, remote, prefix)
+		for _, r := range remo.Config().Fetch {
+			if string(r) == fetchRefspec {
+				hasCustomFetch = true
+				break
+			}
 		}
-	}
 
-	if !hasCustomFetch {
-		remo.Config().Fetch = append(remo.Config().Fetch, config.RefSpec(fetchRefspec))
+		if !hasCustomFetch {
+			remo.Config().Fetch = append(remo.Config().Fetch, config.RefSpec(fetchRefspec))
+		}
+
+		refSpecs[i] = config.RefSpec(refspec)
 	}
 
 	buf := bytes.NewBuffer(nil)
 
 	err = remo.Push(&gogit.PushOptions{
 		RemoteName: remote,
-		RefSpecs:   []config.RefSpec{config.RefSpec(refspec)},
+		RefSpecs:   refSpecs,
 		Progress:   buf,
 	})
 	if err == gogit.NoErrAlreadyUpToDate {
@@ -438,6 +448,9 @@ func (repo *GoGitRepo) ReadData(hash Hash) ([]byte, error) {
 	defer repo.rMutex.Unlock()
 
 	obj, err := repo.r.BlobObject(plumbing.NewHash(hash.String()))
+	if err == plumbing.ErrObjectNotFound {
+		return nil, ErrNotFound
+	}
 	if err != nil {
 		return nil, err
 	}
@@ -507,6 +520,9 @@ func (repo *GoGitRepo) ReadTree(hash Hash) ([]TreeEntry, error) {
 
 	// the given hash could be a tree or a commit
 	obj, err := repo.r.Storer.EncodedObject(plumbing.AnyObject, h)
+	if err == plumbing.ErrObjectNotFound {
+		return nil, ErrNotFound
+	}
 	if err != nil {
 		return nil, err
 	}
@@ -613,43 +629,11 @@ func (repo *GoGitRepo) StoreSignedCommit(treeHash Hash, signKey *openpgp.Entity,
 	return Hash(hash.String()), nil
 }
 
-// GetTreeHash return the git tree hash referenced in a commit
-func (repo *GoGitRepo) GetTreeHash(commit Hash) (Hash, error) {
-	repo.rMutex.Lock()
-	defer repo.rMutex.Unlock()
-
-	obj, err := repo.r.CommitObject(plumbing.NewHash(commit.String()))
-	if err != nil {
-		return "", err
-	}
-
-	return Hash(obj.TreeHash.String()), nil
-}
-
-// FindCommonAncestor will return the last common ancestor of two chain of commit
-func (repo *GoGitRepo) FindCommonAncestor(commit1 Hash, commit2 Hash) (Hash, error) {
-	repo.rMutex.Lock()
-	defer repo.rMutex.Unlock()
-
-	obj1, err := repo.r.CommitObject(plumbing.NewHash(commit1.String()))
-	if err != nil {
-		return "", err
-	}
-	obj2, err := repo.r.CommitObject(plumbing.NewHash(commit2.String()))
-	if err != nil {
-		return "", err
-	}
-
-	commits, err := obj1.MergeBase(obj2)
-	if err != nil {
-		return "", err
-	}
-
-	return Hash(commits[0].Hash.String()), nil
-}
-
 func (repo *GoGitRepo) ResolveRef(ref string) (Hash, error) {
 	r, err := repo.r.Reference(plumbing.ReferenceName(ref), false)
+	if err == plumbing.ErrReferenceNotFound {
+		return "", ErrNotFound
+	}
 	if err != nil {
 		return "", err
 	}
@@ -702,6 +686,9 @@ func (repo *GoGitRepo) RefExist(ref string) (bool, error) {
 // CopyRef will create a new reference with the same value as another one
 func (repo *GoGitRepo) CopyRef(source string, dest string) error {
 	r, err := repo.r.Reference(plumbing.ReferenceName(source), false)
+	if err == plumbing.ErrReferenceNotFound {
+		return ErrNotFound
+	}
 	if err != nil {
 		return err
 	}
@@ -718,6 +705,9 @@ func (repo *GoGitRepo) ReadCommit(hash Hash) (Commit, error) {
 	defer repo.rMutex.Unlock()
 
 	commit, err := repo.r.CommitObject(plumbing.NewHash(hash.String()))
+	if err == plumbing.ErrObjectNotFound {
+		return Commit{}, ErrNotFound
+	}
 	if err != nil {
 		return Commit{}, err
 	}
diff --git a/repository/hash.go b/repository/hash.go
index 6a11558f..ad0206c3 100644
--- a/repository/hash.go
+++ b/repository/hash.go
@@ -43,7 +43,7 @@ func (h *Hash) IsValid() bool {
 		return false
 	}
 	for _, r := range *h {
-		if (r < 'a' || r > 'z') && (r < '0' || r > '9') {
+		if (r < 'a' || r > 'f') && (r < '0' || r > '9') {
 			return false
 		}
 	}
diff --git a/repository/mock_repo.go b/repository/mock_repo.go
index f869795d..c2cef8ef 100644
--- a/repository/mock_repo.go
+++ b/repository/mock_repo.go
@@ -239,12 +239,12 @@ func NewMockRepoData() *mockRepoData {
 	}
 }
 
-func (r *mockRepoData) FetchRefs(remote string, prefix string) (string, error) {
+func (r *mockRepoData) FetchRefs(remote string, prefixes ...string) (string, error) {
 	panic("implement me")
 }
 
 // PushRefs push git refs to a remote
-func (r *mockRepoData) PushRefs(remote string, prefix string) (string, error) {
+func (r *mockRepoData) PushRefs(remote string, prefixes ...string) (string, error) {
 	panic("implement me")
 }
 
@@ -258,7 +258,7 @@ func (r *mockRepoData) StoreData(data []byte) (Hash, error) {
 func (r *mockRepoData) ReadData(hash Hash) ([]byte, error) {
 	data, ok := r.blobs[hash]
 	if !ok {
-		return nil, fmt.Errorf("unknown hash")
+		return nil, ErrNotFound
 	}
 
 	return data, nil
@@ -283,13 +283,13 @@ func (r *mockRepoData) ReadTree(hash Hash) ([]TreeEntry, error) {
 		commit, ok := r.commits[hash]
 
 		if !ok {
-			return nil, fmt.Errorf("unknown hash")
+			return nil, ErrNotFound
 		}
 
 		data, ok = r.trees[commit.treeHash]
 
 		if !ok {
-			return nil, fmt.Errorf("unknown hash")
+			return nil, ErrNotFound
 		}
 	}
 
@@ -327,7 +327,7 @@ func (r *mockRepoData) StoreSignedCommit(treeHash Hash, signKey *openpgp.Entity,
 func (r *mockRepoData) ReadCommit(hash Hash) (Commit, error) {
 	c, ok := r.commits[hash]
 	if !ok {
-		return Commit{}, fmt.Errorf("unknown commit")
+		return Commit{}, ErrNotFound
 	}
 
 	result := Commit{
@@ -346,19 +346,10 @@ func (r *mockRepoData) ReadCommit(hash Hash) (Commit, error) {
 	return result, nil
 }
 
-func (r *mockRepoData) GetTreeHash(commit Hash) (Hash, error) {
-	c, ok := r.commits[commit]
-	if !ok {
-		return "", fmt.Errorf("unknown commit")
-	}
-
-	return c.treeHash, nil
-}
-
 func (r *mockRepoData) ResolveRef(ref string) (Hash, error) {
 	h, ok := r.refs[ref]
 	if !ok {
-		return "", fmt.Errorf("unknown ref")
+		return "", ErrNotFound
 	}
 	return h, nil
 }
@@ -394,48 +385,13 @@ func (r *mockRepoData) CopyRef(source string, dest string) error {
 	hash, exist := r.refs[source]
 
 	if !exist {
-		return fmt.Errorf("Unknown ref")
+		return ErrNotFound
 	}
 
 	r.refs[dest] = hash
 	return nil
 }
 
-func (r *mockRepoData) FindCommonAncestor(hash1 Hash, hash2 Hash) (Hash, error) {
-	ancestor1 := []Hash{hash1}
-
-	for hash1 != "" {
-		c, ok := r.commits[hash1]
-		if !ok {
-			return "", fmt.Errorf("unknown commit %v", hash1)
-		}
-		if len(c.parents) == 0 {
-			break
-		}
-		ancestor1 = append(ancestor1, c.parents[0])
-		hash1 = c.parents[0]
-	}
-
-	for {
-		for _, ancestor := range ancestor1 {
-			if ancestor == hash2 {
-				return ancestor, nil
-			}
-		}
-
-		c, ok := r.commits[hash2]
-		if !ok {
-			return "", fmt.Errorf("unknown commit %v", hash1)
-		}
-
-		if c.parents[0] == "" {
-			return "", fmt.Errorf("no ancestor found")
-		}
-
-		hash2 = c.parents[0]
-	}
-}
-
 func (r *mockRepoData) ListCommits(ref string) ([]Hash, error) {
 	return nonNativeListCommits(r, ref)
 }
diff --git a/repository/repo.go b/repository/repo.go
index 42ed194c..66baec65 100644
--- a/repository/repo.go
+++ b/repository/repo.go
@@ -16,6 +16,8 @@ var (
 	ErrNotARepo = errors.New("not a git repository")
 	// ErrClockNotExist is the error returned when a clock can't be found
 	ErrClockNotExist = errors.New("clock doesn't exist")
+	// ErrNotFound is the error returned when a git object can't be found
+	ErrNotFound = errors.New("ref not found")
 )
 
 // Repo represents a source code repository.
@@ -122,7 +124,7 @@ type RepoData interface {
 	// FetchRefs fetch git refs matching a directory prefix to a remote
 	// Ex: prefix="foo" will fetch any remote refs matching "refs/foo/*" locally.
 	// The equivalent git refspec would be "refs/foo/*:refs/remotes/<remote>/foo/*"
-	FetchRefs(remote string, prefix string) (string, error)
+	FetchRefs(remote string, prefixes ...string) (string, error)
 
 	// PushRefs push git refs matching a directory prefix to a remote
 	// Ex: prefix="foo" will push any local refs matching "refs/foo/*" to the remote.
@@ -130,12 +132,13 @@ type RepoData interface {
 	//
 	// Additionally, PushRefs will update the local references in refs/remotes/<remote>/foo to match
 	// the remote state.
-	PushRefs(remote string, prefix string) (string, error)
+	PushRefs(remote string, prefixes ...string) (string, error)
 
 	// StoreData will store arbitrary data and return the corresponding hash
 	StoreData(data []byte) (Hash, error)
 
 	// ReadData will attempt to read arbitrary data from the given hash
+	// Returns ErrNotFound if not found.
 	ReadData(hash Hash) ([]byte, error)
 
 	// StoreTree will store a mapping key-->Hash as a Git tree
@@ -143,6 +146,7 @@ type RepoData interface {
 
 	// ReadTree will return the list of entries in a Git tree
 	// The given hash could be from either a commit or a tree
+	// Returns ErrNotFound if not found.
 	ReadTree(hash Hash) ([]TreeEntry, error)
 
 	// StoreCommit will store a Git commit with the given Git tree
@@ -153,13 +157,11 @@ type RepoData interface {
 	StoreSignedCommit(treeHash Hash, signKey *openpgp.Entity, parents ...Hash) (Hash, error)
 
 	// ReadCommit read a Git commit and returns some of its characteristic
+	// Returns ErrNotFound if not found.
 	ReadCommit(hash Hash) (Commit, error)
 
-	// GetTreeHash return the git tree hash referenced in a commit
-	// Deprecated
-	GetTreeHash(commit Hash) (Hash, error)
-
 	// ResolveRef returns the hash of the target commit of the given ref
+	// Returns ErrNotFound if not found.
 	ResolveRef(ref string) (Hash, error)
 
 	// UpdateRef will create or update a Git reference
@@ -176,12 +178,9 @@ type RepoData interface {
 	RefExist(ref string) (bool, error)
 
 	// CopyRef will create a new reference with the same value as another one
+	// Returns ErrNotFound if not found.
 	CopyRef(source string, dest string) error
 
-	// FindCommonAncestor will return the last common ancestor of two chain of commit
-	// Deprecated
-	FindCommonAncestor(commit1 Hash, commit2 Hash) (Hash, error)
-
 	// ListCommits will return the list of tree hashes of a ref, in chronological order
 	ListCommits(ref string) ([]Hash, error)
 }
diff --git a/repository/repo_testing.go b/repository/repo_testing.go
index c4ed6d77..821eb762 100644
--- a/repository/repo_testing.go
+++ b/repository/repo_testing.go
@@ -48,6 +48,15 @@ func RepoConfigTest(t *testing.T, repo RepoConfig) {
 	testConfig(t, repo.LocalConfig())
 }
 
+func randomHash() Hash {
+	var letterRunes = "abcdef0123456789"
+	b := make([]byte, idLengthSHA256)
+	for i := range b {
+		b[i] = letterRunes[rand.Intn(len(letterRunes))]
+	}
+	return Hash(b)
+}
+
 // helper to test a RepoData
 func RepoDataTest(t *testing.T, repo RepoData) {
 	// Blob
@@ -62,6 +71,9 @@ func RepoDataTest(t *testing.T, repo RepoData) {
 	require.NoError(t, err)
 	require.Equal(t, data, blob1Read)
 
+	_, err = repo.ReadData(randomHash())
+	require.ErrorIs(t, err, ErrNotFound)
+
 	// Tree
 
 	blobHash2, err := repo.StoreData(randomData())
@@ -111,25 +123,20 @@ func RepoDataTest(t *testing.T, repo RepoData) {
 	require.NoError(t, err)
 	require.ElementsMatch(t, tree2, tree2Read)
 
+	_, err = repo.ReadTree(randomHash())
+	require.ErrorIs(t, err, ErrNotFound)
+
 	// Commit
 
 	commit1, err := repo.StoreCommit(treeHash1)
 	require.NoError(t, err)
 	require.True(t, commit1.IsValid())
 
-	treeHash1Read, err := repo.GetTreeHash(commit1)
-	require.NoError(t, err)
-	require.Equal(t, treeHash1, treeHash1Read)
-
 	// commit with a parent
 	commit2, err := repo.StoreCommit(treeHash2, commit1)
 	require.NoError(t, err)
 	require.True(t, commit2.IsValid())
 
-	treeHash2Read, err := repo.GetTreeHash(commit2)
-	require.NoError(t, err)
-	require.Equal(t, treeHash2, treeHash2Read)
-
 	// ReadTree should accept tree and commit hashes
 	tree1read, err := repo.ReadTree(commit1)
 	require.NoError(t, err)
@@ -140,6 +147,9 @@ func RepoDataTest(t *testing.T, repo RepoData) {
 	c2expected := Commit{Hash: commit2, Parents: []Hash{commit1}, TreeHash: treeHash2}
 	require.Equal(t, c2expected, c2)
 
+	_, err = repo.ReadCommit(randomHash())
+	require.ErrorIs(t, err, ErrNotFound)
+
 	// Ref
 
 	exist1, err := repo.RefExist("refs/bugs/ref1")
@@ -172,14 +182,13 @@ func RepoDataTest(t *testing.T, repo RepoData) {
 	require.NoError(t, err)
 	require.Equal(t, []Hash{commit1, commit2}, commits)
 
-	// Graph
+	_, err = repo.ResolveRef("/refs/bugs/refnotexist")
+	require.ErrorIs(t, err, ErrNotFound)
 
-	commit3, err := repo.StoreCommit(treeHash1, commit1)
-	require.NoError(t, err)
+	err = repo.CopyRef("/refs/bugs/refnotexist", "refs/foo")
+	require.ErrorIs(t, err, ErrNotFound)
 
-	ancestorHash, err := repo.FindCommonAncestor(commit2, commit3)
-	require.NoError(t, err)
-	require.Equal(t, commit1, ancestorHash)
+	// Cleanup
 
 	err = repo.RemoveRef("refs/bugs/ref1")
 	require.NoError(t, err)