storage: Add object type hint parameter to ObjectStorage.Get. (#69)

Some storage backends can optimize object lookup if they get
the object type that is expected. So we the signature of the Get
method is now Get(Hash, ObjectType).

Added generic tests for storage backends.

17 files changed, 171 insertions, 140 deletions

diff --git a/commit_test.go b/commit_test.go
index 886a61d..c7fc333 100644
--- a/commit_test.go
+++ b/commit_test.go
@@ -28,7 +28,7 @@ func (s *SuiteCommit) SetUpSuite(c *C) {
 
 func (s *SuiteCommit) TestDecodeNonCommit(c *C) {
 	hash := core.NewHash("9a48f23120e880dfbe41f7c9b7b708e9ee62a492")
-	blob, err := s.Repository.s.ObjectStorage().Get(hash)
+	blob, err := s.Repository.s.ObjectStorage().Get(hash, core.AnyObject)
 	c.Assert(err, IsNil)
 
 	commit := &Commit{}
diff --git a/core/object.go b/core/object.go
index 9c9a74e..7e021cb 100644
--- a/core/object.go
+++ b/core/object.go
@@ -38,17 +38,22 @@ type Object interface {
 	Writer() (ObjectWriter, error)
 }
 
-// ObjectType internal object type's
+// ObjectType internal object type
+// Integer values from 0 to 7 map to those exposed by git.
+// AnyObject is used to represent any from 0 to 7.
 type ObjectType int8
 
 const (
-	InvalidObject  ObjectType = 0
-	CommitObject   ObjectType = 1
-	TreeObject     ObjectType = 2
-	BlobObject     ObjectType = 3
-	TagObject      ObjectType = 4
+	InvalidObject ObjectType = 0
+	CommitObject  ObjectType = 1
+	TreeObject    ObjectType = 2
+	BlobObject    ObjectType = 3
+	TagObject     ObjectType = 4
+	// 5 reserved for future expansion
 	OFSDeltaObject ObjectType = 6
 	REFDeltaObject ObjectType = 7
+
+	AnyObject ObjectType = -127
 )
 
 func (t ObjectType) String() string {
@@ -132,7 +137,7 @@ func (iter *ObjectLookupIter) Next() (Object, error) {
 		return nil, io.EOF
 	}
 	hash := iter.series[iter.pos]
-	obj, err := iter.storage.Get(hash)
+	obj, err := iter.storage.Get(hash, AnyObject)
 	if err == nil {
 		iter.pos++
 	}
@@ -146,7 +151,7 @@ func (iter *ObjectLookupIter) ForEach(cb func(Object) error) error {
 	defer iter.Close()
 
 	for _, hash := range iter.series {
-		obj, err := iter.storage.Get(hash)
+		obj, err := iter.storage.Get(hash, AnyObject)
 		if err != nil {
 			return err
 		}
diff --git a/core/storage.go b/core/storage.go
index f3225d8..f3ec52b 100644
--- a/core/storage.go
+++ b/core/storage.go
@@ -9,7 +9,16 @@ var ErrStop = errors.New("stop iter")
 type ObjectStorage interface {
 	NewObject() Object
 	Set(Object) (Hash, error)
-	Get(Hash) (Object, error)
+	// Get an object by hash with the given ObjectType.
+	//
+	// Implementors should return (nil, core.ErrObjectNotFound) if an object
+	// doesn't exist with both the given hash and object type.
+	//
+	// Valid ObjectType values are CommitObject, BlobObject, TagObject, TreeObject
+	// and AnyObject.
+	//
+	// If AnyObject is given, the object must be looked up regardless of its type.
+	Get(Hash, ObjectType) (Object, error)
 	Iter(ObjectType) (ObjectIter, error)
 }
 
diff --git a/examples/object_storage/storage.go b/examples/object_storage/storage.go
index c119adc..0513654 100644
--- a/examples/object_storage/storage.go
+++ b/examples/object_storage/storage.go
@@ -86,8 +86,8 @@ func (o *AerospikeObjectStorage) Set(obj core.Object) (core.Hash, error) {
 	return obj.Hash(), err
 }
 
-func (o *AerospikeObjectStorage) Get(h core.Hash) (core.Object, error) {
-	key, err := keyFromObject(h)
+func (o *AerospikeObjectStorage) Get(h core.Hash, t core.ObjectType) (core.Object, error) {
+	key, err := keyFromObject(h, t)
 	if err != nil {
 		return nil, err
 	}
@@ -113,8 +113,8 @@ func (o *AerospikeObjectStorage) Iter(t core.ObjectType) (core.ObjectIter, error
 	return &AerospikeObjectIter{t, rs.Records}, nil
 }
 
-func keyFromObject(h core.Hash) (*aerospike.Key, error) {
-	return aerospike.NewKey("test", "objects", h.String())
+func keyFromObject(h core.Hash, t core.ObjectType) (*aerospike.Key, error) {
+	return aerospike.NewKey("test", t.String(), h.String())
 }
 
 type AerospikeObjectIter struct {
diff --git a/formats/packfile/decoder_test.go b/formats/packfile/decoder_test.go
index 31d86ad..5c8807d 100644
--- a/formats/packfile/decoder_test.go
+++ b/formats/packfile/decoder_test.go
@@ -104,7 +104,7 @@ func AssertObjects(c *C, s *memory.Storage, expects []string) {
 
 	c.Assert(len(expects), Equals, len(o.Objects))
 	for _, exp := range expects {
-		obt, err := o.Get(core.NewHash(exp))
+		obt, err := o.Get(core.NewHash(exp), core.AnyObject)
 		c.Assert(err, IsNil)
 		c.Assert(obt.Hash().String(), Equals, exp)
 	}
diff --git a/remote.go b/remote.go
index f918f3d..3af713b 100644
--- a/remote.go
+++ b/remote.go
@@ -123,7 +123,7 @@ func (r *Remote) getWantedReferences(spec []config.RefSpec) ([]*core.Reference,
 			return nil
 		}
 
-		_, err := r.s.ObjectStorage().Get(ref.Hash())
+		_, err := r.s.ObjectStorage().Get(ref.Hash(), core.CommitObject)
 		if err == core.ErrObjectNotFound {
 			refs = append(refs, ref)
 			return nil
diff --git a/repository.go b/repository.go
index fc871c7..1a5f4a7 100644
--- a/repository.go
+++ b/repository.go
@@ -226,7 +226,7 @@ func (r *Repository) Pull(o *PullOptions) error {
 
 // Commit return the commit with the given hash
 func (r *Repository) Commit(h core.Hash) (*Commit, error) {
-	obj, err := r.s.ObjectStorage().Get(h)
+	obj, err := r.s.ObjectStorage().Get(h, core.CommitObject)
 	if err != nil {
 		if err == core.ErrObjectNotFound {
 			return nil, ErrObjectNotFound
@@ -250,7 +250,7 @@ func (r *Repository) Commits() (*CommitIter, error) {
 
 // Tree return the tree with the given hash
 func (r *Repository) Tree(h core.Hash) (*Tree, error) {
-	obj, err := r.s.ObjectStorage().Get(h)
+	obj, err := r.s.ObjectStorage().Get(h, core.TreeObject)
 	if err != nil {
 		if err == core.ErrObjectNotFound {
 			return nil, ErrObjectNotFound
@@ -264,7 +264,7 @@ func (r *Repository) Tree(h core.Hash) (*Tree, error) {
 
 // Blob returns the blob with the given hash
 func (r *Repository) Blob(h core.Hash) (*Blob, error) {
-	obj, err := r.s.ObjectStorage().Get(h)
+	obj, err := r.s.ObjectStorage().Get(h, core.BlobObject)
 	if err != nil {
 		if err == core.ErrObjectNotFound {
 			return nil, ErrObjectNotFound
@@ -278,7 +278,7 @@ func (r *Repository) Blob(h core.Hash) (*Blob, error) {
 
 // Tag returns a tag with the given hash.
 func (r *Repository) Tag(h core.Hash) (*Tag, error) {
-	obj, err := r.s.ObjectStorage().Get(h)
+	obj, err := r.s.ObjectStorage().Get(h, core.TagObject)
 	if err != nil {
 		if err == core.ErrObjectNotFound {
 			return nil, ErrObjectNotFound
@@ -302,8 +302,8 @@ func (r *Repository) Tags() (*TagIter, error) {
 }
 
 // Object returns an object with the given hash.
-func (r *Repository) Object(h core.Hash) (Object, error) {
-	obj, err := r.s.ObjectStorage().Get(h)
+func (r *Repository) Object(h core.Hash, t core.ObjectType) (Object, error) {
+	obj, err := r.s.ObjectStorage().Get(h, t)
 	if err != nil {
 		if err == core.ErrObjectNotFound {
 			return nil, ErrObjectNotFound
diff --git a/repository_test.go b/repository_test.go
index bfb0298..dd4539c 100644
--- a/repository_test.go
+++ b/repository_test.go
@@ -303,7 +303,7 @@ func (s *RepositorySuite) TestObject(c *C) {
 			com := fmt.Sprintf("subtest %d, tag %d", i, k)
 			info := t.objs[k]
 			hash := core.NewHash(info.Hash)
-			obj, err := r.Object(hash)
+			obj, err := r.Object(hash, core.AnyObject)
 			c.Assert(err, IsNil, Commentf(com))
 			c.Assert(obj.Type(), Equals, info.Kind, Commentf(com))
 			c.Assert(obj.ID(), Equals, hash, Commentf(com))
diff --git a/storage/filesystem/object.go b/storage/filesystem/object.go
index f3a1dda..6024ae0 100644
--- a/storage/filesystem/object.go
+++ b/storage/filesystem/object.go
@@ -38,7 +38,7 @@ func (s *ObjectStorage) Set(core.Object) (core.Hash, error) {
 
 // Get returns the object with the given hash, by searching for it in
 // the packfile.
-func (s *ObjectStorage) Get(h core.Hash) (core.Object, error) {
+func (s *ObjectStorage) Get(h core.Hash, t core.ObjectType) (core.Object, error) {
 	offset, err := s.index.Get(h)
 	if err != nil {
 		return nil, err
@@ -71,7 +71,14 @@ func (s *ObjectStorage) Get(h core.Hash) (core.Object, error) {
 	p := packfile.NewParser(r)
 
 	obj := s.NewObject()
-	return obj, p.FillObject(obj)
+	err = p.FillObject(obj)
+	if err != nil {
+		return nil, err
+	}
+	if core.AnyObject != t && obj.Type() != t {
+		return nil, core.ErrObjectNotFound
+	}
+	return obj, nil
 }
 
 // Iter returns an iterator for all the objects in the packfile with the
@@ -80,11 +87,11 @@ func (s *ObjectStorage) Iter(t core.ObjectType) (core.ObjectIter, error) {
 	var objects []core.Object
 
 	for hash := range s.index {
-		object, err := s.Get(hash)
+		object, err := s.Get(hash, core.AnyObject)
 		if err != nil {
 			return nil, err
 		}
-		if object.Type() == t {
+		if t == core.AnyObject || object.Type() == t {
 			objects = append(objects, object)
 		}
 	}
diff --git a/storage/filesystem/object_test.go b/storage/filesystem/object_test.go
index 692a69b..956fdeb 100644
--- a/storage/filesystem/object_test.go
+++ b/storage/filesystem/object_test.go
@@ -64,9 +64,11 @@ func (s *FsSuite) TearDownSuite(c *C) {
 
 func (s *FsSuite) TestHashNotFound(c *C) {
 	sto := s.newObjectStorage(c, "binary-relations")
-
-	_, err := sto.Get(core.ZeroHash)
-	c.Assert(err, Equals, core.ErrObjectNotFound)
+	types := []core.ObjectType{core.AnyObject, core.TagObject, core.CommitObject, core.BlobObject, core.TreeObject}
+	for t := range types {
+		_, err := sto.Get(core.ZeroHash, core.ObjectType(t))
+		c.Assert(err, Equals, core.ErrObjectNotFound)
+	}
 }
 
 func (s *FsSuite) newObjectStorage(c *C, fixtureName string) core.ObjectStorage {
@@ -156,7 +158,7 @@ func equalsStorages(a, b core.ObjectStorage) (bool, string, error) {
 				break
 			}
 
-			bo, err := b.Get(ao.Hash())
+			bo, err := b.Get(ao.Hash(), core.AnyObject)
 			if err != nil {
 				return false, "", fmt.Errorf("getting object with hash %s: %s",
 					ao.Hash(), err)
diff --git a/storage/memory/storage.go b/storage/memory/storage.go
index d67368f..8033541 100644
--- a/storage/memory/storage.go
+++ b/storage/memory/storage.go
@@ -130,12 +130,11 @@ func (o *ObjectStorage) Set(obj core.Object) (core.Hash, error) {
 }
 
 // Get returns a object with the given hash
-func (o *ObjectStorage) Get(h core.Hash) (core.Object, error) {
+func (o *ObjectStorage) Get(h core.Hash, t core.ObjectType) (core.Object, error) {
 	obj, ok := o.Objects[h]
-	if !ok {
+	if !ok || (core.AnyObject != t && obj.Type() != t) {
 		return nil, core.ErrObjectNotFound
 	}
-
 	return obj, nil
 }
 
@@ -143,6 +142,8 @@ func (o *ObjectStorage) Get(h core.Hash) (core.Object, error) {
 func (o *ObjectStorage) Iter(t core.ObjectType) (core.ObjectIter, error) {
 	var series []core.Object
 	switch t {
+	case core.AnyObject:
+		series = flattenObjectMap(o.Objects)
 	case core.CommitObject:
 		series = flattenObjectMap(o.Commits)
 	case core.TreeObject:
diff --git a/storage/memory/storage_test.go b/storage/memory/storage_test.go
index ac97584..e291609 100644
--- a/storage/memory/storage_test.go
+++ b/storage/memory/storage_test.go
@@ -6,14 +6,20 @@ import (
 
 	. "gopkg.in/check.v1"
 	"gopkg.in/src-d/go-git.v4/core"
+	. "gopkg.in/src-d/go-git.v4/storage/test"
 )
 
 func Test(t *testing.T) { TestingT(t) }
 
-type StorageSuite struct{}
+type StorageSuite struct { }
 
 var _ = Suite(&StorageSuite{})
 
+func (s *StorageSuite) TestObjectStorage(c *C) {
+	storage := NewStorage()
+	RunObjectStorageSuite(c, storage.ObjectStorage())
+}
+
 func (s *StorageSuite) TestStorageObjectStorage(c *C) {
 	storage := NewStorage()
 	o := storage.ObjectStorage()
@@ -30,102 +36,6 @@ func (s *StorageSuite) TestStorageReferenceStorage(c *C) {
 	c.Assert(o == e, Equals, true)
 }
 
-func (s *StorageSuite) TestObjectStorageSetAndGet(c *C) {
-	storage := NewStorage()
-	os := storage.ObjectStorage()
-
-	commit := &core.MemoryObject{}
-	commit.SetType(core.CommitObject)
-
-	h, err := os.Set(commit)
-	c.Assert(err, IsNil)
-	c.Assert(h.String(), Equals, "dcf5b16e76cce7425d0beaef62d79a7d10fce1f5")
-
-	e, err := os.Get(h)
-	c.Assert(commit == e, Equals, true)
-
-	tree := &core.MemoryObject{}
-	tree.SetType(core.TreeObject)
-
-	h, err = os.Set(tree)
-	c.Assert(err, IsNil)
-	c.Assert(h.String(), Equals, "4b825dc642cb6eb9a060e54bf8d69288fbee4904")
-
-	e, err = os.Get(h)
-	c.Assert(tree == e, Equals, true)
-
-	blob := &core.MemoryObject{}
-	blob.SetType(core.BlobObject)
-
-	h, err = os.Set(blob)
-	c.Assert(err, IsNil)
-	c.Assert(h.String(), Equals, "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391")
-
-	e, err = os.Get(h)
-	c.Assert(blob == e, Equals, true)
-
-	tag := &core.MemoryObject{}
-	tag.SetType(core.TagObject)
-
-	h, err = os.Set(tag)
-	c.Assert(err, IsNil)
-	c.Assert(h.String(), Equals, "d994c6bb648123a17e8f70a966857c546b2a6f94")
-
-	e, err = os.Get(h)
-	c.Assert(tag == e, Equals, true)
-}
-
-func (s *StorageSuite) TestObjectStorageIter(c *C) {
-	commit := &core.MemoryObject{}
-	commit.SetType(core.CommitObject)
-	tree := &core.MemoryObject{}
-	tree.SetType(core.TreeObject)
-	blob := &core.MemoryObject{}
-	blob.SetType(core.BlobObject)
-	tag := &core.MemoryObject{}
-	tag.SetType(core.TagObject)
-
-	storage := NewStorage()
-	os := storage.ObjectStorage()
-
-	os.Set(commit)
-	os.Set(tree)
-	os.Set(blob)
-	os.Set(tag)
-
-	i, err := os.Iter(core.CommitObject)
-	c.Assert(err, IsNil)
-
-	e, err := i.Next()
-	c.Assert(err, IsNil)
-	c.Assert(commit == e, Equals, true)
-
-	i, err = os.Iter(core.TreeObject)
-	c.Assert(err, IsNil)
-
-	e, err = i.Next()
-	c.Assert(err, IsNil)
-	c.Assert(tree == e, Equals, true)
-
-	i, err = os.Iter(core.BlobObject)
-	c.Assert(err, IsNil)
-
-	e, err = i.Next()
-	c.Assert(err, IsNil)
-	c.Assert(blob == e, Equals, true)
-
-	i, err = os.Iter(core.TagObject)
-	c.Assert(err, IsNil)
-
-	e, err = i.Next()
-	c.Assert(err, IsNil)
-	c.Assert(tag == e, Equals, true)
-
-	e, err = i.Next()
-	c.Assert(e, IsNil)
-	c.Assert(err, Equals, io.EOF)
-}
-
 func (s *StorageSuite) TestReferenceStorageSetAndGet(c *C) {
 	storage := NewStorage()
 	rs := storage.ReferenceStorage()
diff --git a/storage/test/storage_suite.go b/storage/test/storage_suite.go
new file mode 100644
index 0000000..2463d9d
--- /dev/null
+++ b/storage/test/storage_suite.go
@@ -0,0 +1,92 @@
+package test
+
+import (
+	. "gopkg.in/check.v1"
+	"gopkg.in/src-d/go-git.v4/core"
+	"io"
+)
+
+type TestObject struct {
+	Object core.Object
+	Hash   string
+	Type   core.ObjectType
+}
+
+func RunObjectStorageSuite(c *C, os core.ObjectStorage) {
+	commit := &core.MemoryObject{}
+	commit.SetType(core.CommitObject)
+	tree := &core.MemoryObject{}
+	tree.SetType(core.TreeObject)
+	blob := &core.MemoryObject{}
+	blob.SetType(core.BlobObject)
+	tag := &core.MemoryObject{}
+	tag.SetType(core.TagObject)
+
+	testObjects := map[core.ObjectType]TestObject{
+		core.CommitObject: TestObject{commit, "dcf5b16e76cce7425d0beaef62d79a7d10fce1f5", core.CommitObject},
+		core.TreeObject:   TestObject{tree, "4b825dc642cb6eb9a060e54bf8d69288fbee4904", core.TreeObject},
+		core.BlobObject:   TestObject{blob, "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391", core.BlobObject},
+		core.TagObject:    TestObject{tag, "d994c6bb648123a17e8f70a966857c546b2a6f94", core.TagObject},
+	}
+
+	validTypes := []core.ObjectType{core.CommitObject, core.BlobObject, core.TagObject, core.TreeObject}
+
+	for _, to := range testObjects {
+		comment := Commentf("failed for type %s", to.Type.String())
+
+		h, err := os.Set(to.Object)
+		c.Assert(err, IsNil)
+		c.Assert(h.String(), Equals, to.Hash, comment)
+
+		o, err := os.Get(h, to.Type)
+		c.Assert(err, IsNil)
+		c.Assert(o, Equals, to.Object)
+
+		o, err = os.Get(h, core.AnyObject)
+		c.Assert(err, IsNil)
+		c.Assert(o, Equals, to.Object)
+
+		for _, validType := range validTypes {
+			if validType == to.Type {
+				continue
+			}
+			o, err = os.Get(h, validType)
+			c.Assert(o, IsNil)
+			c.Assert(err, Equals, core.ErrObjectNotFound)
+		}
+	}
+
+	for _, validType := range validTypes {
+		comment := Commentf("failed for type %s)", validType.String())
+		i, err := os.Iter(validType)
+		c.Assert(err, IsNil, comment)
+
+		o, err := i.Next()
+		c.Assert(err, IsNil)
+		c.Assert(o, Equals, testObjects[validType].Object, comment)
+
+		o, err = i.Next()
+		c.Assert(o, IsNil)
+		c.Assert(err, Equals, io.EOF, comment)
+	}
+
+	i, err := os.Iter(core.AnyObject)
+	c.Assert(err, IsNil)
+
+	foundObjects := []core.Object{}
+	i.ForEach(func(o core.Object) error {
+		foundObjects = append(foundObjects, o)
+		return nil
+	})
+	c.Assert(foundObjects, HasLen, len(testObjects))
+	for _, to := range testObjects {
+		found := false
+		for _, o := range foundObjects {
+			if to.Object == o {
+				found = true
+				break
+			}
+		}
+		c.Assert(found, Equals, true, Commentf("Object of type %s not found", to.Type.String()))
+	}
+}
diff --git a/tag.go b/tag.go
index 1391fad..b4b2d20 100644
--- a/tag.go
+++ b/tag.go
@@ -169,7 +169,7 @@ func (t *Tag) Blob() (*Blob, error) {
 
 // Object returns the object pointed to by the tag.
 func (t *Tag) Object() (Object, error) {
-	return t.r.Object(t.Target)
+	return t.r.Object(t.Target, t.TargetType)
 }
 
 // String returns the meta information contained in the tag as a formatted
diff --git a/tree.go b/tree.go
index 3058231..de9765b 100644
--- a/tree.go
+++ b/tree.go
@@ -47,7 +47,7 @@ func (t *Tree) File(path string) (*File, error) {
 		return nil, ErrFileNotFound
 	}
 
-	obj, err := t.r.s.ObjectStorage().Get(e.Hash)
+	obj, err := t.r.s.ObjectStorage().Get(e.Hash, core.BlobObject)
 	if err != nil {
 		if err == core.ErrObjectNotFound {
 			return nil, ErrFileNotFound // a git submodule
@@ -87,7 +87,7 @@ func (t *Tree) dir(baseName string) (*Tree, error) {
 		return nil, errDirNotFound
 	}
 
-	obj, err := t.r.s.ObjectStorage().Get(entry.Hash)
+	obj, err := t.r.s.ObjectStorage().Get(entry.Hash, core.TreeObject)
 	if err != nil {
 		if err == core.ErrObjectNotFound { // git submodule
 			return nil, errDirNotFound
diff --git a/tree_test.go b/tree_test.go
index 5f285af..bc9686d 100644
--- a/tree_test.go
+++ b/tree_test.go
@@ -37,7 +37,7 @@ func (s *SuiteTree) TestDecode(c *C) {
 
 func (s *SuiteTree) TestDecodeNonTree(c *C) {
 	hash := core.NewHash("9a48f23120e880dfbe41f7c9b7b708e9ee62a492")
-	blob, err := s.Repository.s.ObjectStorage().Get(hash)
+	blob, err := s.Repository.s.ObjectStorage().Get(hash, core.BlobObject)
 	c.Assert(err, IsNil)
 
 	tree := &Tree{}
diff --git a/tree_walker.go b/tree_walker.go
index 5568e1b..d4aa01a 100644
--- a/tree_walker.go
+++ b/tree_walker.go
@@ -9,6 +9,9 @@ const (
 	startingStackSize = 8
 )
 
+const submoduleMode = 0160000
+const directoryMode = 0040000
+
 // TreeWalker provides a means of walking through all of the entries in a Tree.
 type TreeWalker struct {
 	stack []treeEntryIter
@@ -66,12 +69,14 @@ func (w *TreeWalker) Next() (name string, entry TreeEntry, obj Object, err error
 			return
 		}
 
-		obj, err = w.r.Object(entry.Hash)
-		if err == ErrObjectNotFound {
-			// FIXME: Avoid doing this here in case the caller actually cares about
-			//        missing objects.
+		if entry.Mode == submoduleMode {
 			err = nil
-			continue // ignore entries without hash (= submodule dirs)
+			continue
+		}
+		if entry.Mode.IsDir() {
+			obj, err = w.r.Tree(entry.Hash)
+		} else {
+			obj, err = w.r.Blob(entry.Hash)
 		}
 
 		name = path.Join(w.base, entry.Name)