Added Object interface for Commit, Tree, Blob and Tag

* New Object interface is distinct from core.Object
* New Object interface is used in places where returned object could be of any type
* Object is implemented by Commit, Tree, Blob, File and Tag
* Added Repository.Object function for retrieving objects of any type
* Tag.Object now returns Object instead of core.Object
* Tag target hash is now publicly accessible
* Renamed Tag.Type field to Tag.TargetType, making it distinct from Tag.Type function
* Fixed infinite recursive loop in TagIter.Close
* TreeWalker.Next now returns Object instead of core.Object
* Removed some duplicate test setup code

Author: JoshuaSjoding

commit.go

@@ -43,13 +43,28 @@ func (c *Commit) NumParents() int {
 }
 
 // File returns the file with the specified "path" in the commit and a
-// nil error if the file exists. If the file does not exists, it returns
+// nil error if the file exists. If the file does not exist, it returns
 // a nil file and the ErrFileNotFound error.
 func (c *Commit) File(path string) (file *File, err error) {
 	return c.Tree().File(path)
 }
 
-// Decode transform an core.Object into a Blob struct
+// ID returns the object ID of the commit. The returned value will always match
+// the current value of Commit.Hash.
+//
+// ID is present to fufill the Object interface.
+func (c *Commit) ID() core.Hash {
+	return c.Hash
+}
+
+// Type returns the type of object. It always returns core.CommitObject.
+//
+// Type is present to fufill the Object interface.
+func (c *Commit) Type() core.ObjectType {
+	return core.CommitObject
+}
+
+// Decode transforms a core.Object into a Commit struct.
 func (c *Commit) Decode(o core.Object) (err error) {
 	if o.Type() != core.CommitObject {
 		return ErrUnsupportedObject
@@ -107,15 +122,22 @@ func (c *Commit) String() string {
 	)
 }
 
+// CommitIter provides an iterator for a set of commits.
 type CommitIter struct {
 	core.ObjectIter
 	r *Repository
 }
 
+// NewCommitIter returns a CommitIter for the given repository and underlying
+// object iterator.
+//
+// The returned CommitIter will automatically skip over non-commit objects.
 func NewCommitIter(r *Repository, iter core.ObjectIter) *CommitIter {
 	return &CommitIter{iter, r}
 }
 
+// Next moves the iterator to the next commit and returns a pointer to it. If it
+// has reached the end of the set it will return io.EOF.
 func (iter *CommitIter) Next() (*Commit, error) {
 	obj, err := iter.ObjectIter.Next()
 	if err != nil {

commit_test.go

@@ -2,10 +2,8 @@ package git
 
 import (
 	"io"
-	"os"
 
 	"gopkg.in/src-d/go-git.v3/core"
-	"gopkg.in/src-d/go-git.v3/formats/packfile"
 
 	. "gopkg.in/check.v1"
 )
@@ -18,27 +16,10 @@ var _ = Suite(&SuiteCommit{})
 
 // create the repositories of the fixtures
 func (s *SuiteCommit) SetUpSuite(c *C) {
-	fixtureRepos := [...]struct {
-		url      string
-		packfile string
-	}{
+	commitFixtures := []packedFixture{
 		{"https://github.com/tyba/git-fixture.git", "formats/packfile/fixtures/git-fixture.ofs-delta"},
 	}
-	s.repos = make(map[string]*Repository, 0)
-	for _, fixRepo := range fixtureRepos {
-		s.repos[fixRepo.url] = NewPlainRepository()
-
-		d, err := os.Open(fixRepo.packfile)
-		c.Assert(err, IsNil)
-
-		r := packfile.NewReader(d)
-		r.Format = packfile.OFSDeltaFormat // TODO: how to know the format of a pack file ahead of time?
-
-		_, err = r.Read(s.repos[fixRepo.url].Storage)
-		c.Assert(err, IsNil)
-
-		c.Assert(d.Close(), IsNil)
-	}
+	s.repos = unpackFixtures(c, commitFixtures)
 }
 
 var commitIterTests = []struct {

file.go

@@ -3,8 +3,6 @@ package git
 import (
 	"bytes"
 	"strings"
-
-	"gopkg.in/src-d/go-git.v3/core"
 )
 
 // File represents git file objects.
@@ -64,15 +62,9 @@ func (iter *FileIter) Next() (*File, error) {
 			return nil, err
 		}
 
-		if obj.Type() != core.BlobObject {
-			// Skip non-blob objects
-			continue
+		if blob, ok := obj.(*Blob); ok {
+			return newFile(name, blob), nil
 		}
-
-		blob := &Blob{}
-		blob.Decode(obj)
-
-		return newFile(name, blob), nil
 	}
 }
 

file_test.go

@@ -2,10 +2,8 @@ package git
 
 import (
 	"io"
-	"os"
 
 	"gopkg.in/src-d/go-git.v3/core"
-	"gopkg.in/src-d/go-git.v3/formats/packfile"
 
 	. "gopkg.in/check.v1"
 )
@@ -18,28 +16,11 @@ var _ = Suite(&SuiteFile{})
 
 // create the repositories of the fixtures
 func (s *SuiteFile) SetUpSuite(c *C) {
-	fixtureRepos := [...]struct {
-		url      string
-		packfile string
-	}{
+	fileFixtures := []packedFixture{
 		{"https://github.com/tyba/git-fixture.git", "formats/packfile/fixtures/git-fixture.ofs-delta"},
 		{"https://github.com/cpcs499/Final_Pres_P", "formats/packfile/fixtures/Final_Pres_P.ofs-delta"},
 	}
-	s.repos = make(map[string]*Repository, 0)
-	for _, fixRepo := range fixtureRepos {
-		s.repos[fixRepo.url] = NewPlainRepository()
-
-		d, err := os.Open(fixRepo.packfile)
-		c.Assert(err, IsNil)
-
-		r := packfile.NewReader(d)
-		r.Format = packfile.OFSDeltaFormat
-
-		_, err = r.Read(s.repos[fixRepo.url].Storage)
-		c.Assert(err, IsNil)
-
-		c.Assert(d.Close(), IsNil)
-	}
+	s.repos = unpackFixtures(c, fileFixtures)
 }
 
 type fileIterExpectedEntry struct {
@@ -77,6 +58,8 @@ func (s *SuiteFile) TestIter(c *C) {
 			expected := t.files[k]
 			file, err := iter.Next()
 			c.Assert(err, IsNil, Commentf("subtest %d, iter %d, err=%v", i, k, err))
+			c.Assert(file.Hash.IsZero(), Equals, false)
+			c.Assert(file.Hash, Equals, file.ID())
 			c.Assert(file.Name, Equals, expected.Name, Commentf("subtest %d, iter %d, name=%s, expected=%s", i, k, file.Name, expected.Hash))
 			c.Assert(file.Hash.String(), Equals, expected.Hash, Commentf("subtest %d, iter %d, hash=%v, expected=%s", i, k, file.Hash.String(), expected.Hash))
 		}

objects.go

@@ -10,6 +10,36 @@ import (
 	"gopkg.in/src-d/go-git.v3/core"
 )
 
+var ErrUnsupportedObject = errors.New("unsupported object type")
+
+// Object is a generic representation of any git object. It is implemented by
+// Commit, Tree, Blob and Tag, and includes the functions that are common to
+// them.
+//
+// Object is returned when an object could of any type. It is frequently used
+// with a type cast to acquire the specific type of object:
+//
+//   func process(obj Object) {
+//   	switch o := obj.(type) {
+//   	case *Commit:
+//   		// o is a Commit
+//   	case *Tree:
+//   		// o is a Tree
+//   	case *Blob:
+//   		// o is a Blob
+//   	case *Tag:
+//   		// o is a Tag
+//   	}
+//   }
+//
+// This interface is intentionally different from core.Object, which is a lower
+// level interface used by storage implementations to read and write objects.
+type Object interface {
+	ID() core.Hash
+	Type() core.ObjectType
+	Decode(core.Object) error
+}
+
 // Blob is used to store file data - it is generally a file.
 type Blob struct {
 	Hash core.Hash
@@ -18,9 +48,22 @@ type Blob struct {
 	obj core.Object
 }
 
-var ErrUnsupportedObject = errors.New("unsupported object type")
+// ID returns the object ID of the blob. The returned value will always match
+// the current value of Blob.Hash.
+//
+// ID is present to fufill the Object interface.
+func (b *Blob) ID() core.Hash {
+	return b.Hash
+}
+
+// Type returns the type of object. It always returns core.BlobObject.
+//
+// Type is present to fufill the Object interface.
+func (b *Blob) Type() core.ObjectType {
+	return core.BlobObject
+}
 
-// Decode transform an core.Object into a Blob struct
+// Decode transforms a core.Object into a Blob struct.
 func (b *Blob) Decode(o core.Object) error {
 	if o.Type() != core.BlobObject {
 		return ErrUnsupportedObject

objects_test.go

@@ -29,6 +29,7 @@ func (s *ObjectsSuite) TestNewCommit(c *C) {
 	commit, err := s.r.Commit(hash)
 	c.Assert(err, IsNil)
 
+	c.Assert(commit.Hash, Equals, commit.ID())
 	c.Assert(commit.Hash.String(), Equals, "a5b8b09e2f8fcb0bb99d3ccb0958157b40890d69")
 	c.Assert(commit.Tree().Hash.String(), Equals, "c2d30fa8ef288618f65f6eed6e168e0d514886f4")
 

repository.go

@@ -75,6 +75,8 @@ func (r *Repository) Pull(remoteName, branch string) (err error) {
 	req := &common.GitUploadPackRequest{}
 	req.Want(ref)
 
+	// TODO: Provide "haves" for what's already in the repository's storage
+
 	reader, err := remote.Fetch(req)
 	if err != nil {
 		return err
@@ -155,3 +157,31 @@ func (r *Repository) Tag(h core.Hash) (*Tag, error) {
 func (r *Repository) Tags() *TagIter {
 	return NewTagIter(r, r.Storage.Iter(core.TagObject))
 }
+
+// Object returns an object with the given hash.
+func (r *Repository) Object(h core.Hash) (Object, error) {
+	obj, err := r.Storage.Get(h)
+	if err != nil {
+		if err == core.ObjectNotFoundErr {
+			return nil, ObjectNotFoundErr
+		}
+		return nil, err
+	}
+
+	switch obj.Type() {
+	case core.CommitObject:
+		commit := &Commit{r: r}
+		return commit, commit.Decode(obj)
+	case core.TreeObject:
+		tree := &Tree{r: r}
+		return tree, tree.Decode(obj)
+	case core.BlobObject:
+		blob := &Blob{}
+		return blob, blob.Decode(obj)
+	case core.TagObject:
+		tag := &Tag{r: r}
+		return tag, tag.Decode(obj)
+	default:
+		return nil, core.ErrInvalidType
+	}
+}

repository_test.go

@@ -15,8 +15,8 @@ type SuiteRepository struct {
 
 var _ = Suite(&SuiteRepository{})
 
-func (s *SuiteRepository) SetUpTest(c *C) {
-	s.repos = unpackFixtures(c, tagFixtures)
+func (s *SuiteRepository) SetUpSuite(c *C) {
+	s.repos = unpackFixtures(c, tagFixtures, treeWalkerFixtures)
 }
 
 func (s *SuiteRepository) TestNewRepository(c *C) {
@@ -58,6 +58,9 @@ func (s *SuiteRepository) TestCommit(c *C) {
 	c.Assert(err, IsNil)
 
 	c.Assert(commit.Hash.IsZero(), Equals, false)
+	c.Assert(commit.Hash, Equals, commit.ID())
+	c.Assert(commit.Hash, Equals, hash)
+	c.Assert(commit.Type(), Equals, core.CommitObject)
 	c.Assert(commit.Tree().Hash.IsZero(), Equals, false)
 	c.Assert(commit.Author.Email, Equals, "[email protected]")
 }
@@ -79,6 +82,8 @@ func (s *SuiteRepository) TestCommits(c *C) {
 
 		count++
 		c.Assert(commit.Hash.IsZero(), Equals, false)
+		c.Assert(commit.Hash, Equals, commit.ID())
+		c.Assert(commit.Type(), Equals, core.CommitObject)
 		//c.Assert(commit.Tree.IsZero(), Equals, false)
 	}
 
@@ -90,10 +95,11 @@ func (s *SuiteRepository) TestTag(c *C) {
 		r, ok := s.repos[t.repo]
 		c.Assert(ok, Equals, true)
 		k := 0
-		for hash, expected := range t.tags {
-			tag, err := r.Tag(core.NewHash(hash))
+		for hashString, expected := range t.tags {
+			hash := core.NewHash(hashString)
+			tag, err := r.Tag(hash)
 			c.Assert(err, IsNil)
-			testTagExpected(c, tag, expected, fmt.Sprintf("subtest %d, tag %d: ", i, k))
+			testTagExpected(c, tag, hash, expected, fmt.Sprintf("subtest %d, tag %d: ", i, k))
 			k++
 		}
 	}
@@ -107,6 +113,22 @@ func (s *SuiteRepository) TestTags(c *C) {
 	}
 }
 
+func (s *SuiteRepository) TestObject(c *C) {
+	for i, t := range treeWalkerTests {
+		r, ok := s.repos[t.repo]
+		c.Assert(ok, Equals, true)
+		for k := 0; k < len(t.objs); k++ {
+			comment := fmt.Sprintf("subtest %d, tag %d", i, k)
+			info := t.objs[k]
+			hash := core.NewHash(info.Hash)
+			obj, err := r.Object(hash)
+			c.Assert(err, IsNil, Commentf(comment))
+			c.Assert(obj.Type(), Equals, info.Kind, Commentf(comment))
+			c.Assert(obj.ID(), Equals, hash, Commentf(comment))
+		}
+	}
+}
+
 func (s *SuiteRepository) TestCommitIterClosePanic(c *C) {
 	r, err := NewRepository(RepositoryFixture, nil)
 	r.Remotes["origin"].upSrv = &MockGitUploadPackService{}