From 9e6a03b7956464ccd9d2fbacedd8e5cc23572d02 Mon Sep 17 00:00:00 2001
From: Joshua Sjoding <joshua.sjoding@scjalliance.com>
Date: Thu, 3 Mar 2016 00:33:00 -0800
Subject: 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
---
 commit.go           |  26 ++++++++-
 commit_test.go      |  23 +-------
 file.go             |  12 +---
 file_test.go        |  25 ++------
 objects.go          |  47 ++++++++++++++-
 objects_test.go     |   1 +
 repository.go       |  30 ++++++++++
 repository_test.go  |  32 ++++++++--
 tag.go              |  72 ++++++++++++++---------
 tag_test.go         |  42 +++++++++++--
 tree.go             |  20 +++++--
 tree_test.go        |  25 ++------
 tree_walker.go      |  16 ++---
 tree_walker_test.go | 165 ++++++++++++++++++++++++----------------------------
 14 files changed, 316 insertions(+), 220 deletions(-)

diff --git a/commit.go b/commit.go
index e96ae5d..5b826ca 100644
--- a/commit.go
+++ b/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 {
diff --git a/commit_test.go b/commit_test.go
index 62af890..a620e8c 100644
--- a/commit_test.go
+++ b/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 {
diff --git a/file.go b/file.go
index e3e8e93..76ec962 100644
--- a/file.go
+++ b/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
 	}
 }
 
diff --git a/file_test.go b/file_test.go
index d9024b0..aa3369a 100644
--- a/file_test.go
+++ b/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))
 		}
diff --git a/objects.go b/objects.go
index 961502a..b84909c 100644
--- a/objects.go
+++ b/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
diff --git a/objects_test.go b/objects_test.go
index 6826957..c48b768 100644
--- a/objects_test.go
+++ b/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")
 
diff --git a/repository.go b/repository.go
index 451bb8f..ae583fd 100644
--- a/repository.go
+++ b/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
+	}
+}
diff --git a/repository_test.go b/repository_test.go
index 202c931..9e81334 100644
--- a/repository_test.go
+++ b/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, "daniel@lordran.local")
 }
@@ -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{}
diff --git a/tag.go b/tag.go
index 77a47b3..41349f6 100644
--- a/tag.go
+++ b/tag.go
@@ -18,14 +18,36 @@ import (
 //
 // https://git-scm.com/book/en/v2/Git-Internals-Git-References#Tags
 type Tag struct {
-	Hash    core.Hash
-	Type    core.ObjectType
-	Name    string
-	Tagger  Signature
-	Message string
-
-	object core.Hash
-	r      *Repository
+	Hash       core.Hash
+	Name       string
+	Tagger     Signature
+	Message    string
+	TargetType core.ObjectType
+	Target     core.Hash
+
+	r *Repository
+}
+
+// Type returns the type of object. It always returns core.TreeObject.
+/*
+func (t *Tag) Type() core.ObjectType {
+	return core.TagObject
+}
+*/
+
+// ID returns the object ID of the tag, not the object that the tag references.
+// The returned value will always match the current value of Tag.Hash.
+//
+// ID is present to fufill the Object interface.
+func (t *Tag) ID() core.Hash {
+	return t.Hash
+}
+
+// Type returns the type of object. It always returns core.TagObject.
+//
+// Type is present to fufill the Object interface.
+func (t *Tag) Type() core.ObjectType {
+	return core.TagObject
 }
 
 // Decode transforms a core.Object into a Tag struct.
@@ -57,9 +79,9 @@ func (t *Tag) Decode(o core.Object) (err error) {
 		split := bytes.SplitN(line, []byte{' '}, 2)
 		switch string(split[0]) {
 		case "object":
-			t.object = core.NewHash(string(split[1]))
+			t.Target = core.NewHash(string(split[1]))
 		case "type":
-			t.Type, err = core.ParseObjectType(string(split[1]))
+			t.TargetType, err = core.ParseObjectType(string(split[1]))
 			if err != nil {
 				return err
 			}
@@ -86,10 +108,10 @@ func (t *Tag) Decode(o core.Object) (err error) {
 // Commit returns the commit pointed to by the tag. If the tag points to a
 // different type of object ErrUnsupportedObject will be returned.
 func (t *Tag) Commit() (*Commit, error) {
-	if t.Type != core.CommitObject {
+	if t.TargetType != core.CommitObject {
 		return nil, ErrUnsupportedObject
 	}
-	return t.r.Commit(t.object)
+	return t.r.Commit(t.Target)
 }
 
 // Tree returns the tree pointed to by the tag. If the tag points to a commit
@@ -97,15 +119,15 @@ func (t *Tag) Commit() (*Commit, error) {
 // to a commit or tree object ErrUnsupportedObject will be returned.
 func (t *Tag) Tree() (*Tree, error) {
 	// TODO: If the tag is of type commit, follow the commit to its tree?
-	switch t.Type {
+	switch t.TargetType {
 	case core.CommitObject:
-		commit, err := t.r.Commit(t.object)
+		commit, err := t.r.Commit(t.Target)
 		if err != nil {
 			return nil, err
 		}
 		return commit.Tree(), nil
 	case core.TreeObject:
-		return t.r.Tree(t.object)
+		return t.r.Tree(t.Target)
 	default:
 		return nil, ErrUnsupportedObject
 	}
@@ -114,15 +136,15 @@ func (t *Tag) Tree() (*Tree, error) {
 // Blob returns the blob pointed to by the tag. If the tag points to a
 // different type of object ErrUnsupportedObject will be returned.
 func (t *Tag) Blob() (*Blob, error) {
-	if t.Type != core.BlobObject {
+	if t.TargetType != core.BlobObject {
 		return nil, ErrUnsupportedObject
 	}
-	return t.r.Blob(t.object)
+	return t.r.Blob(t.Target)
 }
 
 // Object returns the object pointed to by the tag.
-func (t *Tag) Object() (core.Object, error) {
-	return t.r.Storage.Get(t.object)
+func (t *Tag) Object() (Object, error) {
+	return t.r.Object(t.Target)
 }
 
 // String returns the meta information contained in the tag as a formatted
@@ -130,7 +152,7 @@ func (t *Tag) Object() (core.Object, error) {
 func (t *Tag) String() string {
 	return fmt.Sprintf(
 		"%s %s\nObject: %s\nType: %s\nTag: %s\nTagger: %s\nDate:   %s\n",
-		core.TagObject, t.Hash, t.object, t.Type, t.Name, t.Tagger.String(), t.Tagger.When,
+		core.TagObject, t.Hash, t.Target, t.TargetType, t.Name, t.Tagger.String(), t.Tagger.When,
 	)
 }
 
@@ -140,7 +162,10 @@ type TagIter struct {
 	r *Repository
 }
 
-// NewTagIter returns a new TagIter for the given Repository and ObjectIter.
+// NewTagIter returns a TagIter for the given repository and underlying
+// object iterator.
+//
+// The returned TagIter will automatically skip over non-tag objects.
 func NewTagIter(r *Repository, iter core.ObjectIter) *TagIter {
 	return &TagIter{iter, r}
 }
@@ -156,8 +181,3 @@ func (iter *TagIter) Next() (*Tag, error) {
 	tag := &Tag{r: iter.r}
 	return tag, tag.Decode(obj)
 }
-
-// Close releases any resources used by the iterator.
-func (iter *TagIter) Close() {
-	iter.Close()
-}
diff --git a/tag_test.go b/tag_test.go
index 1874c11..46d53fc 100644
--- a/tag_test.go
+++ b/tag_test.go
@@ -68,6 +68,7 @@ func (s *SuiteTag) TestCommit(c *C) {
 			c.Assert(err, IsNil)
 			commit, err := tag.Commit()
 			c.Assert(err, IsNil)
+			c.Assert(commit.Type(), Equals, core.CommitObject)
 			c.Assert(commit.Hash.String(), Equals, expected.Object)
 			k++
 		}
@@ -87,6 +88,7 @@ func (s *SuiteTag) TestTree(c *C) {
 			c.Assert(err, IsNil)
 			tree, err := tag.Tree()
 			c.Assert(err, IsNil)
+			c.Assert(tree.Type(), Equals, core.TreeObject)
 			c.Assert(tree.Hash.String(), Equals, expected.Object)
 			k++
 		}
@@ -98,27 +100,55 @@ func (s *SuiteTag) TestBlob(c *C) {
 		r, ok := s.repos[t.repo]
 		c.Assert(ok, Equals, true)
 		k := 0
-		for hash, expected := range t.tags {
+		for hashString, expected := range t.tags {
 			if expected.Type != core.BlobObject {
 				continue
 			}
-			tag, err := r.Tag(core.NewHash(hash))
+			hash := core.NewHash(hashString)
+			tag, err := r.Tag(hash)
 			c.Assert(err, IsNil)
+			testTagExpected(c, tag, hash, expected, "")
 			blob, err := tag.Blob()
 			c.Assert(err, IsNil)
+			c.Assert(blob.Type(), Equals, core.BlobObject)
 			c.Assert(blob.Hash.String(), Equals, expected.Object)
 			k++
 		}
 	}
 }
 
-func testTagExpected(c *C, tag *Tag, expected expectedTag, comment string) {
+func (s *SuiteTag) TestObject(c *C) {
+	for _, t := range tagTests {
+		r, ok := s.repos[t.repo]
+		c.Assert(ok, Equals, true)
+		k := 0
+		for hashString, expected := range t.tags {
+			if expected.Type != core.BlobObject {
+				continue
+			}
+			hash := core.NewHash(hashString)
+			tag, err := r.Tag(hash)
+			c.Assert(err, IsNil)
+			testTagExpected(c, tag, hash, expected, "")
+			obj, err := tag.Object()
+			c.Assert(err, IsNil)
+			c.Assert(obj.Type(), Equals, expected.Type)
+			c.Assert(obj.ID().String(), Equals, expected.Object)
+			k++
+		}
+	}
+}
+
+func testTagExpected(c *C, tag *Tag, hash core.Hash, expected expectedTag, comment string) {
 	when, err := time.Parse(time.RFC3339, expected.When)
 	c.Assert(err, IsNil)
 	c.Assert(tag, NotNil)
 	c.Assert(tag.Hash.IsZero(), Equals, false)
-	c.Assert(tag.Type, Equals, expected.Type, Commentf("%stype=%v, expected=%v", comment, tag.Type, expected.Type))
-	c.Assert(tag.object.String(), Equals, expected.Object, Commentf("%sobject=%v, expected=%s", comment, tag.object, expected.Object))
+	c.Assert(tag.Hash, Equals, tag.ID())
+	c.Assert(tag.Hash, Equals, hash)
+	c.Assert(tag.Type(), Equals, core.TagObject)
+	c.Assert(tag.TargetType, Equals, expected.Type, Commentf("%stype=%v, expected=%v", comment, tag.TargetType, expected.Type))
+	c.Assert(tag.Target.String(), Equals, expected.Object, Commentf("%sobject=%v, expected=%s", comment, tag.Target, expected.Object))
 	c.Assert(tag.Name, Equals, expected.Tag, Commentf("subtest %d, iter %d, tag=%s, expected=%s", comment, tag.Name, expected.Tag))
 	c.Assert(tag.Tagger.Name, Equals, expected.TaggerName, Commentf("subtest %d, iter %d, tagger.name=%s, expected=%s", comment, tag.Tagger.Name, expected.TaggerName))
 	c.Assert(tag.Tagger.Email, Equals, expected.TaggerEmail, Commentf("subtest %d, iter %d, tagger.email=%s, expected=%s", comment, tag.Tagger.Email, expected.TaggerEmail))
@@ -138,7 +168,7 @@ func testTagIter(c *C, iter *TagIter, tags map[string]expectedTag, comment strin
 		expected, ok := tags[tag.Hash.String()]
 		c.Assert(ok, Equals, true, Commentf("%sunexpected tag hash=%v", comment, tag.Hash))
 
-		testTagExpected(c, tag, expected, comment)
+		testTagExpected(c, tag, tag.Hash, expected, comment)
 	}
 	_, err := iter.Next()
 	c.Assert(err, Equals, io.EOF)
diff --git a/tree.go b/tree.go
index c8bebd8..246bf8c 100644
--- a/tree.go
+++ b/tree.go
@@ -127,6 +127,19 @@ func (t *Tree) Files() *FileIter {
 	return NewFileIter(t.r, t)
 }
 
+// ID returns the object ID of the tree. The returned value will always match
+// the current value of Tree.Hash.
+//
+// ID is present to fufill the Object interface.
+func (t *Tree) ID() core.Hash {
+	return t.Hash
+}
+
+// Type returns the type of object. It always returns core.TreeObject.
+func (t *Tree) Type() core.ObjectType {
+	return core.TreeObject
+}
+
 // Decode transform an core.Object into a Tree struct
 func (t *Tree) Decode(o core.Object) (err error) {
 	if o.Type() != core.TreeObject {
@@ -229,12 +242,9 @@ func (iter *TreeIter) Next() (*Tree, error) {
 			return nil, err
 		}
 
-		if obj.Type() != core.TreeObject {
-			// Skip non-tree objects
-			continue
+		if tree, ok := obj.(*Tree); ok {
+			return tree, nil
 		}
-
-		return iter.w.Tree(), nil
 	}
 }
 
diff --git a/tree_test.go b/tree_test.go
index 57af166..09e255c 100644
--- a/tree_test.go
+++ b/tree_test.go
@@ -1,11 +1,9 @@
 package git
 
 import (
-	"os"
 	"sort"
 
 	"gopkg.in/src-d/go-git.v3/core"
-	"gopkg.in/src-d/go-git.v3/formats/packfile"
 
 	. "gopkg.in/check.v1"
 )
@@ -18,10 +16,7 @@ var _ = Suite(&SuiteTree{})
 
 // create the repositories of the fixtures
 func (s *SuiteTree) SetUpSuite(c *C) {
-	fixtureRepos := [...]struct {
-		url      string
-		packfile string
-	}{
+	treeFixtures := []packedFixture{
 		{"https://github.com/tyba/git-fixture.git", "formats/packfile/fixtures/git-fixture.ofs-delta"},
 		{"https://github.com/cpcs499/Final_Pres_P.git", "formats/packfile/fixtures/Final_Pres_P.ofs-delta"},
 		{"https://github.com/jamesob/desk.git", "formats/packfile/fixtures/jamesob-desk.pack"},
@@ -29,21 +24,7 @@ func (s *SuiteTree) SetUpSuite(c *C) {
 		{"https://github.com/alcortesm/binary-relations.git", "formats/packfile/fixtures/alcortesm-binary-relations.pack"},
 		{"https://github.com/Tribler/dispersy.git", "formats/packfile/fixtures/tribler-dispersy.pack"},
 	}
-	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, treeFixtures)
 }
 
 func (s *SuiteTree) TestFile(c *C) {
@@ -217,6 +198,8 @@ func (s *SuiteTree) TestFile(c *C) {
 		}
 
 		c.Assert(file.Size, Equals, t.size, comment)
+		c.Assert(file.Hash.IsZero(), Equals, false, comment)
+		c.Assert(file.Hash, Equals, file.ID(), comment)
 		c.Assert(file.Hash.String(), Equals, t.blobHash, comment)
 	}
 }
diff --git a/tree_walker.go b/tree_walker.go
index 3272718..ff44e67 100644
--- a/tree_walker.go
+++ b/tree_walker.go
@@ -3,8 +3,6 @@ package git
 import (
 	"io"
 	"path"
-
-	"gopkg.in/src-d/go-git.v3/core"
 )
 
 const (
@@ -40,7 +38,7 @@ func NewTreeWalker(r *Repository, t *Tree) *TreeWalker {
 // In the current implementation any objects which cannot be found in the
 // underlying repository will be skipped automatically. It is possible that this
 // may change in future versions.
-func (w *TreeWalker) Next() (name string, entry TreeEntry, obj core.Object, err error) {
+func (w *TreeWalker) Next() (name string, entry TreeEntry, obj Object, err error) {
 	for {
 		current := len(w.stack) - 1
 		if current < 0 {
@@ -66,10 +64,11 @@ func (w *TreeWalker) Next() (name string, entry TreeEntry, obj core.Object, err
 			return
 		}
 
-		obj, err = w.r.Storage.Get(entry.Hash)
-		if err == core.ObjectNotFoundErr {
+		obj, err = w.r.Object(entry.Hash)
+		if err == ObjectNotFoundErr {
 			// FIXME: Avoid doing this here in case the caller actually cares about
 			//        missing objects.
+			err = nil
 			continue // ignore entries without hash (= submodule dirs)
 		}
 
@@ -82,12 +81,7 @@ func (w *TreeWalker) Next() (name string, entry TreeEntry, obj core.Object, err
 		break
 	}
 
-	if obj.Type() == core.TreeObject {
-		tree := &Tree{r: w.r}
-		err = tree.Decode(obj)
-		if err != nil {
-			return
-		}
+	if tree, ok := obj.(*Tree); ok {
 		w.stack = append(w.stack, *NewTreeEntryIter(tree))
 		w.base = path.Join(w.base, entry.Name)
 	}
diff --git a/tree_walker_test.go b/tree_walker_test.go
index bdae79f..44ddf50 100644
--- a/tree_walker_test.go
+++ b/tree_walker_test.go
@@ -6,11 +6,82 @@ import (
 	"strconv"
 
 	"gopkg.in/src-d/go-git.v3/core"
-	"gopkg.in/src-d/go-git.v3/formats/packfile"
 
 	. "gopkg.in/check.v1"
 )
 
+type expectedTreeWalkerEntry struct {
+	Kind core.ObjectType
+	Mode string
+	Name string
+	Hash string
+}
+
+var treeWalkerFixtures = []packedFixture{
+	{"https://github.com/alcortesm/binary-relations.git", "formats/packfile/fixtures/alcortesm-binary-relations.pack"},
+	{"https://github.com/Tribler/dispersy.git", "formats/packfile/fixtures/tribler-dispersy.pack"},
+}
+
+var treeWalkerTests = []struct {
+	repo   string                    // the repo name as in localRepos
+	commit string                    // the commit to search for the file
+	objs   []expectedTreeWalkerEntry // the expected objects in the commit
+}{
+	// https://api.github.com/repos/alcortesm/binary-relations/git/trees/b373f85fa2594d7dcd9989f4a5858a81647fb8ea
+	{"https://github.com/alcortesm/binary-relations.git", "b373f85fa2594d7dcd9989f4a5858a81647fb8ea", []expectedTreeWalkerEntry{
+		{core.BlobObject, "100644", ".gitignore", "7f41905b4d77ab4a9a2d334fcd0fb5db6e8e2183"},
+		{core.BlobObject, "100644", "Makefile", "d441e4e769b53cbd4b1215a1387f8c3108bac97d"},
+		{core.BlobObject, "100644", "binary-relations.tex", "cb50b067cc8cd9f639611d41416575c991ad8e97"},
+		{core.TreeObject, "040000", "imgs-gen", "b33007b7e83a738576c3f44369fe2f674bb23d5d"},
+		{core.TreeObject, "040000", "imgs-gen/simple-graph", "056633542b8ee990d6c89b7a812209dba13d6766"},
+		{core.BlobObject, "100644", "imgs-gen/simple-graph/Makefile", "49560402c1707f29c159ad14f369027250fb154a"},
+		{core.BlobObject, "100644", "imgs-gen/simple-graph/fig.fig", "2c414eb36f0c2e9a2f9c6382d85e63355752170c"},
+		{core.TreeObject, "040000", "src", "ec9d27c4df99caec3a817e9c018812a6c56c1b00"},
+		{core.TreeObject, "040000", "src/map-slice", "00cefb8e77f7a8c61b99dd2491ff48a3b0b16679"},
+		{core.BlobObject, "100644", "src/map-slice/map-slice.go", "12431e98381dd5097e1a19fe53429c72ef1f328e"},
+		{core.TreeObject, "040000", "src/simple-arrays", "9a3781b7fd9d2851e2a4488c035ed9ac905aec79"},
+		{core.BlobObject, "100644", "src/simple-arrays/simple-arrays.go", "104fbb4b0520c192f2e207a2dfd39162f6cdabf7"},
+	}},
+	// https://api.github.com/repos/Tribler/dispersy/git/trees/f5a1fca709f760bf75a7adaa480bf0f0e1a547ee
+	{"https://github.com/Tribler/dispersy.git", "f5a1fca709f760bf75a7adaa480bf0f0e1a547ee", []expectedTreeWalkerEntry{
+		{core.BlobObject, "100644", "__init__.py", "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391"},
+		{core.BlobObject, "100644", "authentication.py", "ca2fb017dce4506c4144ba81d3cbb70563487718"},
+		{core.BlobObject, "100644", "bloomfilter.py", "944e8ccc76779aad923f88f4a73b0d4e8999b6ea"},
+		{core.BlobObject, "100644", "bootstrap.py", "379a4400b992310f54ea56a4691760bdea8b1592"},
+		{core.BlobObject, "100644", "cache.py", "744d48dce50703e7d4ff14531ab2ab77e6b54685"},
+		{core.BlobObject, "100644", "callback.py", "f3a380cbe9eb1c02fb305f2bd2fc0fcfb103892f"},
+		{core.BlobObject, "100644", "candidate.py", "87309a51d3681bf6c46b22ce044dad41c97d32d2"},
+		{core.BlobObject, "100644", "community.py", "38226ffc2139a2349edaf016747d02b199508d41"},
+		{core.BlobObject, "100644", "conversion.py", "4e2fcefba40d99c2a6237768ed0fbb8e2e770c83"},
+		{core.BlobObject, "100644", "crypto.py", "8a6bb00df982fa806ce18838673ab1ef3fd52fed"},
+		{core.BlobObject, "100644", "database.py", "bb484bfd31a92f7775dbd3acf8740abf00bb3d74"},
+		{core.BlobObject, "100644", "debug.py", "3743f20d321f7b2b6d3b47211f93317818c3673e"},
+		{core.BlobObject, "100644", "debugcommunity.py", "1598ec5a773cc561430c5bb9b87157ef7d3e1c7c"},
+		{core.BlobObject, "100644", "decorator.py", "a1e913e674aec5402cc7b4e9fc0801e8155d2cec"},
+		{core.BlobObject, "100644", "destination.py", "d5c02588117d260e728d5c64aba885522ba508c5"},
+		{core.BlobObject, "100644", "dispersy.py", "63a08602e2ac8294b20543f0c89c75c740bf6c1c"},
+		{core.BlobObject, "100644", "dispersydatabase.py", "76dd222444c308c470efabde7ed511825004b4d3"},
+		{core.BlobObject, "100644", "distribution.py", "55a11beca7c09013f5b8ff46baa85b15948c756a"},
+		{core.BlobObject, "100644", "dprint.py", "fd6a8608d62bf415a65e78c9e1ca8df97513e598"},
+		{core.BlobObject, "100644", "encoding.py", "f29b0ebf65f06a0aa7b2ff1aea364f7889c58d56"},
+		{core.BlobObject, "100644", "endpoint.py", "5aa76efd3501de522dbdf2e6374440cf64131423"},
+		{core.BlobObject, "100644", "member.py", "c114c73f710b4c291305e353b4aa0106fafabd52"},
+		{core.BlobObject, "100644", "message.py", "e55bfe0efa851c4e94264dc745141f7f65b1d239"},
+		{core.BlobObject, "100644", "meta.py", "0f62db0fb93326daad6b4925a7d12155a1687f67"},
+		{core.BlobObject, "100644", "payload.py", "0aef13194c51dab3624665340b33dd4040516c86"},
+		{core.BlobObject, "100644", "requestcache.py", "7772c7d81b4b205970cac1a3cdabc2c2deb48b12"},
+		{core.BlobObject, "100644", "resolution.py", "525d6ec81c1fb098d2fe12ae0d5b10a368bfcace"},
+		{core.BlobObject, "100644", "script.py", "ef64e12cc1a4c0b3a5d42ff1b33adef202f30da3"},
+		{core.BlobObject, "100644", "singleton.py", "34662093edf45bbffa91125c13735e37410a185b"},
+		{core.BlobObject, "100644", "timeline.py", "826bb5e1802fb5eaf3144a9a195a994920101880"},
+		{core.TreeObject, "040000", "tool", "da97281af01b5b2dad1de6c84c5acb44da60ef7a"},
+		{core.BlobObject, "100644", "tool/__init__.py", "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391"},
+		{core.BlobObject, "100644", "tool/callbackscript.py", "eb9f8184ef08d9e031936e61bfa86fb9b45b965c"},
+		{core.BlobObject, "100644", "tool/scenarioscript.py", "245c41af66aab8f0a6fd00259b30a47f4d6c00dd"},
+	}},
+	{"https://github.com/Tribler/dispersy.git", "9d38ff85ca03adcf68dc14f5b68b8994f15229f4", []expectedTreeWalkerEntry(nil)},
+}
+
 type SuiteTreeWalker struct {
 	repos map[string]*Repository
 }
@@ -19,97 +90,11 @@ var _ = Suite(&SuiteTreeWalker{})
 
 // create the repositories of the fixtures
 func (s *SuiteTreeWalker) SetUpSuite(c *C) {
-	fixtureRepos := [...]struct {
-		url      string
-		packfile string
-	}{
-		{"https://github.com/alcortesm/binary-relations.git", "formats/packfile/fixtures/alcortesm-binary-relations.pack"},
-		{"https://github.com/Tribler/dispersy.git", "formats/packfile/fixtures/tribler-dispersy.pack"},
-	}
-	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)
-	}
-}
-
-type treeWalkerExpectedEntry struct {
-	Kind core.ObjectType
-	Mode string
-	Name string
-	Hash string
+	s.repos = unpackFixtures(c, treeWalkerFixtures)
 }
 
 func (s *SuiteTreeWalker) TestNext(c *C) {
-	for i, t := range []struct {
-		repo   string                    // the repo name as in localRepos
-		commit string                    // the commit to search for the file
-		objs   []treeWalkerExpectedEntry // the expected objects in the commit
-	}{
-		// https://api.github.com/repos/alcortesm/binary-relations/git/trees/b373f85fa2594d7dcd9989f4a5858a81647fb8ea
-		{"https://github.com/alcortesm/binary-relations.git", "b373f85fa2594d7dcd9989f4a5858a81647fb8ea", []treeWalkerExpectedEntry{
-			{core.BlobObject, "100644", ".gitignore", "7f41905b4d77ab4a9a2d334fcd0fb5db6e8e2183"},
-			{core.BlobObject, "100644", "Makefile", "d441e4e769b53cbd4b1215a1387f8c3108bac97d"},
-			{core.BlobObject, "100644", "binary-relations.tex", "cb50b067cc8cd9f639611d41416575c991ad8e97"},
-			{core.TreeObject, "040000", "imgs-gen", "b33007b7e83a738576c3f44369fe2f674bb23d5d"},
-			{core.TreeObject, "040000", "imgs-gen/simple-graph", "056633542b8ee990d6c89b7a812209dba13d6766"},
-			{core.BlobObject, "100644", "imgs-gen/simple-graph/Makefile", "49560402c1707f29c159ad14f369027250fb154a"},
-			{core.BlobObject, "100644", "imgs-gen/simple-graph/fig.fig", "2c414eb36f0c2e9a2f9c6382d85e63355752170c"},
-			{core.TreeObject, "040000", "src", "ec9d27c4df99caec3a817e9c018812a6c56c1b00"},
-			{core.TreeObject, "040000", "src/map-slice", "00cefb8e77f7a8c61b99dd2491ff48a3b0b16679"},
-			{core.BlobObject, "100644", "src/map-slice/map-slice.go", "12431e98381dd5097e1a19fe53429c72ef1f328e"},
-			{core.TreeObject, "040000", "src/simple-arrays", "9a3781b7fd9d2851e2a4488c035ed9ac905aec79"},
-			{core.BlobObject, "100644", "src/simple-arrays/simple-arrays.go", "104fbb4b0520c192f2e207a2dfd39162f6cdabf7"},
-		}},
-		// https://api.github.com/repos/Tribler/dispersy/git/trees/f5a1fca709f760bf75a7adaa480bf0f0e1a547ee
-		{"https://github.com/Tribler/dispersy.git", "f5a1fca709f760bf75a7adaa480bf0f0e1a547ee", []treeWalkerExpectedEntry{
-			{core.BlobObject, "100644", "__init__.py", "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391"},
-			{core.BlobObject, "100644", "authentication.py", "ca2fb017dce4506c4144ba81d3cbb70563487718"},
-			{core.BlobObject, "100644", "bloomfilter.py", "944e8ccc76779aad923f88f4a73b0d4e8999b6ea"},
-			{core.BlobObject, "100644", "bootstrap.py", "379a4400b992310f54ea56a4691760bdea8b1592"},
-			{core.BlobObject, "100644", "cache.py", "744d48dce50703e7d4ff14531ab2ab77e6b54685"},
-			{core.BlobObject, "100644", "callback.py", "f3a380cbe9eb1c02fb305f2bd2fc0fcfb103892f"},
-			{core.BlobObject, "100644", "candidate.py", "87309a51d3681bf6c46b22ce044dad41c97d32d2"},
-			{core.BlobObject, "100644", "community.py", "38226ffc2139a2349edaf016747d02b199508d41"},
-			{core.BlobObject, "100644", "conversion.py", "4e2fcefba40d99c2a6237768ed0fbb8e2e770c83"},
-			{core.BlobObject, "100644", "crypto.py", "8a6bb00df982fa806ce18838673ab1ef3fd52fed"},
-			{core.BlobObject, "100644", "database.py", "bb484bfd31a92f7775dbd3acf8740abf00bb3d74"},
-			{core.BlobObject, "100644", "debug.py", "3743f20d321f7b2b6d3b47211f93317818c3673e"},
-			{core.BlobObject, "100644", "debugcommunity.py", "1598ec5a773cc561430c5bb9b87157ef7d3e1c7c"},
-			{core.BlobObject, "100644", "decorator.py", "a1e913e674aec5402cc7b4e9fc0801e8155d2cec"},
-			{core.BlobObject, "100644", "destination.py", "d5c02588117d260e728d5c64aba885522ba508c5"},
-			{core.BlobObject, "100644", "dispersy.py", "63a08602e2ac8294b20543f0c89c75c740bf6c1c"},
-			{core.BlobObject, "100644", "dispersydatabase.py", "76dd222444c308c470efabde7ed511825004b4d3"},
-			{core.BlobObject, "100644", "distribution.py", "55a11beca7c09013f5b8ff46baa85b15948c756a"},
-			{core.BlobObject, "100644", "dprint.py", "fd6a8608d62bf415a65e78c9e1ca8df97513e598"},
-			{core.BlobObject, "100644", "encoding.py", "f29b0ebf65f06a0aa7b2ff1aea364f7889c58d56"},
-			{core.BlobObject, "100644", "endpoint.py", "5aa76efd3501de522dbdf2e6374440cf64131423"},
-			{core.BlobObject, "100644", "member.py", "c114c73f710b4c291305e353b4aa0106fafabd52"},
-			{core.BlobObject, "100644", "message.py", "e55bfe0efa851c4e94264dc745141f7f65b1d239"},
-			{core.BlobObject, "100644", "meta.py", "0f62db0fb93326daad6b4925a7d12155a1687f67"},
-			{core.BlobObject, "100644", "payload.py", "0aef13194c51dab3624665340b33dd4040516c86"},
-			{core.BlobObject, "100644", "requestcache.py", "7772c7d81b4b205970cac1a3cdabc2c2deb48b12"},
-			{core.BlobObject, "100644", "resolution.py", "525d6ec81c1fb098d2fe12ae0d5b10a368bfcace"},
-			{core.BlobObject, "100644", "script.py", "ef64e12cc1a4c0b3a5d42ff1b33adef202f30da3"},
-			{core.BlobObject, "100644", "singleton.py", "34662093edf45bbffa91125c13735e37410a185b"},
-			{core.BlobObject, "100644", "timeline.py", "826bb5e1802fb5eaf3144a9a195a994920101880"},
-			{core.TreeObject, "040000", "tool", "da97281af01b5b2dad1de6c84c5acb44da60ef7a"},
-			{core.BlobObject, "100644", "tool/__init__.py", "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391"},
-			{core.BlobObject, "100644", "tool/callbackscript.py", "eb9f8184ef08d9e031936e61bfa86fb9b45b965c"},
-			{core.BlobObject, "100644", "tool/scenarioscript.py", "245c41af66aab8f0a6fd00259b30a47f4d6c00dd"},
-		}},
-		{"https://github.com/Tribler/dispersy.git", "9d38ff85ca03adcf68dc14f5b68b8994f15229f4", []treeWalkerExpectedEntry(nil)},
-	} {
+	for i, t := range treeWalkerTests {
 		r := s.repos[t.repo]
 		commit, err := r.Commit(core.NewHash(t.commit))
 		c.Assert(err, IsNil, Commentf("subtest %d: %v (%s)", i, err, t.commit))
@@ -126,7 +111,7 @@ func (s *SuiteTreeWalker) TestNext(c *C) {
 			c.Assert(entry.Mode, Equals, os.FileMode(mode), Commentf("subtest %d, iter %d, entry.Mode=%v expected=%v", i, k, entry.Mode, mode))
 			c.Assert(obj.Type(), Equals, info.Kind, Commentf("subtest %d, iter %d, obj.Type()=%v expected=%v", i, k, obj.Type(), info.Kind))
 			c.Assert(entry.Hash.String(), Equals, info.Hash, Commentf("subtest %d, iter %d, entry.Hash=%v, expected=%s", i, k, entry.Hash, info.Hash))
-			c.Assert(obj.Hash().String(), Equals, info.Hash, Commentf("subtest %d, iter %d, obj.Hash()=%v, expected=%s", i, k, obj.Hash(), info.Hash))
+			c.Assert(obj.ID().String(), Equals, info.Hash, Commentf("subtest %d, iter %d, obj.ID()=%v, expected=%s", i, k, obj.ID(), info.Hash))
 		}
 		_, _, _, err = walker.Next()
 		c.Assert(err, Equals, io.EOF)
-- 
cgit