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.

Author: smola

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{}

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
 		}

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)
 }
 

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 {

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)
 	}

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

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

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))

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)
 		}
 	}

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)

storage/memory/storage.go

@@ -130,19 +130,20 @@ 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
 }
 
 // Iter returns a core.ObjectIter for the given core.ObjectTybe
 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:

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()