Merge pull request #20 from scjalliance/generic-object-storage

Iterable ObjectStorage interface for use in Repository struct

Author: mcuadros

commit.go

@@ -34,16 +34,7 @@ func (c *Commit) Tree() *Tree {
 }
 
 func (c *Commit) Parents() *CommitIter {
-	i := NewCommitIter(c.r)
-	go func() {
-		defer i.Close()
-		for _, hash := range c.parents {
-			obj, _ := c.r.Storage.Get(hash)
-			i.Add(obj)
-		}
-	}()
-
-	return i
+	return NewCommitIter(c.r, core.NewObjectLookupIter(c.r.Storage, c.parents))
 }
 
 // NumParents returns the number of parents in a commit.
@@ -106,52 +97,24 @@ func (c *Commit) String() string {
 }
 
 type CommitIter struct {
-	iter
+	core.ObjectIter
+	r *Repository
 }
 
-func NewCommitIter(r *Repository) *CommitIter {
-	return &CommitIter{newIter(r)}
+func NewCommitIter(r *Repository, iter core.ObjectIter) *CommitIter {
+	return &CommitIter{iter, r}
 }
 
-func (i *CommitIter) Next() (*Commit, error) {
-	obj := <-i.ch
-	if obj == nil {
-		return nil, io.EOF
+func (iter *CommitIter) Next() (*Commit, error) {
+	obj, err := iter.ObjectIter.Next()
+	if err != nil {
+		return nil, err
 	}
 
-	commit := &Commit{r: i.r}
+	commit := &Commit{r: iter.r}
 	return commit, commit.Decode(obj)
 }
 
-type iter struct {
-	ch chan core.Object
-	r  *Repository
-
-	IsClosed bool
-}
-
-func newIter(r *Repository) iter {
-	ch := make(chan core.Object, 1)
-	return iter{ch: ch, r: r}
-}
-
-func (i *iter) Add(o core.Object) {
-	if i.IsClosed {
-		return
-	}
-
-	i.ch <- o
-}
-
-func (i *iter) Close() {
-	if i.IsClosed {
-		return
-	}
-
-	defer func() { i.IsClosed = true }()
-	close(i.ch)
-}
-
 type commitSorterer struct {
 	l []*Commit
 }

commit_test.go

@@ -1,6 +1,7 @@
 package git
 
 import (
+	"io"
 	"os"
 
 	"gopkg.in/src-d/go-git.v2/core"
@@ -40,10 +41,73 @@ func (s *SuiteCommit) SetUpSuite(c *C) {
 	}
 }
 
-func (s *SuiteCommit) TestIterClose(c *C) {
-	i := &iter{ch: make(chan core.Object, 1)}
-	i.Close()
-	i.Close()
+var iterTests = []struct {
+	repo    string   // the repo name in the test suite's map of fixtures
+	commits []string // the commit hashes to iterate over in the test
+}{
+	{"https://github.com/tyba/git-fixture.git", []string{
+		"6ecf0ef2c2dffb796033e5a02219af86ec6584e5",
+		"918c48b83bd081e863dbe1b80f8998f058cd8294",
+		"af2d6a6954d532f8ffb47615169c8fdf9d383a1a",
+		"1669dce138d9b841a518c64b10914d88f5e488ea",
+		"35e85108805c84807bc66a02d91535e1e24b38b9",
+		"b029517f6300c2da0f4b651b8642506cd6aaf45d",
+		"a5b8b09e2f8fcb0bb99d3ccb0958157b40890d69",
+		"b029517f6300c2da0f4b651b8642506cd6aaf45d", // Intentional duplicate
+		"b8e471f58bcbca63b07bda20e428190409c2db47",
+		"b029517f6300c2da0f4b651b8642506cd6aaf45d"}}, // Intentional duplicate
+}
+
+func (s *SuiteCommit) TestIterSlice(c *C) {
+	for i, t := range iterTests {
+		r := s.repos[t.repo]
+		iter := NewCommitIter(r, core.NewObjectSliceIter(makeObjectSlice(t.commits, r.Storage)))
+		s.checkIter(c, r, i, iter, t.commits)
+	}
+}
+
+func (s *SuiteCommit) TestIterLookup(c *C) {
+	for i, t := range iterTests {
+		r := s.repos[t.repo]
+		iter := NewCommitIter(r, core.NewObjectLookupIter(r.Storage, makeHashSlice(t.commits)))
+		s.checkIter(c, r, i, iter, t.commits)
+	}
+}
+
+func (s *SuiteCommit) checkIter(c *C, r *Repository, subtest int, iter *CommitIter, commits []string) {
+	for k := 0; k < len(commits); k++ {
+		commit, err := iter.Next()
+		c.Assert(err, IsNil, Commentf("subtest %d, iter %d, err=%v", subtest, k, err))
+		c.Assert(commit.Hash.String(), Equals, commits[k], Commentf("subtest %d, iter %d, hash=%v, expected=%s", subtest, k, commit.Hash.String(), commits[k]))
+	}
+	_, err := iter.Next()
+	c.Assert(err, Equals, io.EOF)
+}
+
+func (s *SuiteCommit) TestIterSliceClose(c *C) {
+	for i, t := range iterTests {
+		r := s.repos[t.repo]
+		iter := NewCommitIter(r, core.NewObjectSliceIter(makeObjectSlice(t.commits, r.Storage)))
+		s.checkIterClose(c, i, iter)
+	}
+}
+
+func (s *SuiteCommit) TestIterLookupClose(c *C) {
+	for i, t := range iterTests {
+		r := s.repos[t.repo]
+		iter := NewCommitIter(r, core.NewObjectLookupIter(r.Storage, makeHashSlice(t.commits)))
+		s.checkIterClose(c, i, iter)
+	}
+}
+
+func (s *SuiteCommit) checkIterClose(c *C, subtest int, iter *CommitIter) {
+	iter.Close()
+	_, err := iter.Next()
+	c.Assert(err, Equals, io.EOF, Commentf("subtest %d, close 1, err=%v", subtest, err))
+
+	iter.Close()
+	_, err = iter.Next()
+	c.Assert(err, Equals, io.EOF, Commentf("subtest %d, close 2, err=%v", subtest, err))
 }
 
 var fileTests = []struct {
@@ -102,3 +166,22 @@ func (s *SuiteCommit) TestFile(c *C) {
 		}
 	}
 }
+
+func makeObjectSlice(hashes []string, storage core.ObjectStorage) []core.Object {
+	series := make([]core.Object, 0, len(hashes))
+	for _, member := range hashes {
+		obj, err := storage.Get(core.NewHash(member))
+		if err == nil {
+			series = append(series, obj)
+		}
+	}
+	return series
+}
+
+func makeHashSlice(hashes []string) []core.Hash {
+	series := make([]core.Hash, 0, len(hashes))
+	for _, member := range hashes {
+		series = append(series, core.NewHash(member))
+	}
+	return series
+}

core/object.go

@@ -3,9 +3,14 @@ package core
 
 import (
 	"bytes"
+	"errors"
 	"io"
 )
 
+var (
+	ObjectNotFoundErr = errors.New("object not found")
+)
+
 // Object is a generic representation of any git object
 type Object interface {
 	Type() ObjectType
@@ -19,9 +24,10 @@ type Object interface {
 
 // ObjectStorage generic storage of objects
 type ObjectStorage interface {
-	New() Object
-	Set(Object) Hash
-	Get(Hash) (Object, bool)
+	New() (Object, error)
+	Set(Object) (Hash, error)
+	Get(Hash) (Object, error)
+	Iter(ObjectType) ObjectIter
 }
 
 // ObjectType internal object type's
@@ -59,6 +65,89 @@ func (t ObjectType) Bytes() []byte {
 	return []byte(t.String())
 }
 
+// ObjectIter is a generic closable interface for iterating over objects.
+type ObjectIter interface {
+	Next() (Object, error)
+	Close()
+}
+
+// ObjectLookupIter implements ObjectIter. It iterates over a series of object
+// hashes and yields their associated objects by retrieving each one from
+// object storage. The retrievals are lazy and only occur when the iterator
+// moves forward with a call to Next().
+//
+// The ObjectLookupIter must be closed with a call to Close() when it is no
+// longer needed.
+type ObjectLookupIter struct {
+	storage ObjectStorage
+	series  []Hash
+	pos     int
+}
+
+// NewObjectLookupIter returns an object iterator given an object storage and
+// a slice of object hashes.
+func NewObjectLookupIter(storage ObjectStorage, series []Hash) *ObjectLookupIter {
+	return &ObjectLookupIter{
+		storage: storage,
+		series:  series,
+	}
+}
+
+// Next returns the next object from the iterator. If the iterator has reached
+// the end it will return io.EOF as an error. If the object can't be found in
+// the object storage, it will return ObjectNotFoundErr as an error. If the
+// object is retreieved successfully error will be nil.
+func (iter *ObjectLookupIter) Next() (Object, error) {
+	if iter.pos >= len(iter.series) {
+		return nil, io.EOF
+	}
+	hash := iter.series[iter.pos]
+	obj, err := iter.storage.Get(hash)
+	if err == nil {
+		iter.pos++
+	}
+	return obj, err
+}
+
+// Close releases any resources used by the iterator.
+func (iter *ObjectLookupIter) Close() {
+	iter.pos = len(iter.series)
+}
+
+// ObjectSliceIter implements ObjectIter. It iterates over a series of objects
+// stored in a slice and yields each one in turn when Next() is called.
+//
+// The ObjectSliceIter must be closed with a call to Close() when it is no
+// longer needed.
+type ObjectSliceIter struct {
+	series []Object
+	pos    int
+}
+
+// NewObjectSliceIter returns an object iterator for the given slice of objects.
+func NewObjectSliceIter(series []Object) *ObjectSliceIter {
+	return &ObjectSliceIter{
+		series: series,
+	}
+}
+
+// Next returns the next object from the iterator. If the iterator has reached
+// the end it will return io.EOF as an error. If the object is retreieved
+// successfully error will be nil.
+func (iter *ObjectSliceIter) Next() (Object, error) {
+	if iter.pos >= len(iter.series) {
+		return nil, io.EOF
+	}
+	obj := iter.series[iter.pos]
+	iter.pos++
+	return obj, nil
+}
+
+// Close releases any resources used by the iterator.
+func (iter *ObjectSliceIter) Close() {
+	iter.pos = len(iter.series)
+}
+
 type RAWObject struct {
 	b []byte
 	t ObjectType
@@ -93,11 +182,11 @@ func NewRAWObjectStorage() *RAWObjectStorage {
 	}
 }
 
-func (o *RAWObjectStorage) New() Object {
-	return &RAWObject{}
+func (o *RAWObjectStorage) New() (Object, error) {
+	return &RAWObject{}, nil
 }
 
-func (o *RAWObjectStorage) Set(obj Object) Hash {
+func (o *RAWObjectStorage) Set(obj Object) (Hash, error) {
 	h := obj.Hash()
 	o.Objects[h] = obj
 
@@ -110,11 +199,35 @@ func (o *RAWObjectStorage) Set(obj Object) Hash {
 		o.Blobs[h] = o.Objects[h]
 	}
 
-	return h
+	return h, nil
 }
 
-func (o *RAWObjectStorage) Get(h Hash) (Object, bool) {
+func (o *RAWObjectStorage) Get(h Hash) (Object, error) {
 	obj, ok := o.Objects[h]
+	if !ok {
+		return nil, ObjectNotFoundErr
+	}
+
+	return obj, nil
+}
 
-	return obj, ok
+func (o *RAWObjectStorage) Iter(t ObjectType) ObjectIter {
+	var series []Object
+	switch t {
+	case CommitObject:
+		series = flattenObjectMap(o.Commits)
+	case TreeObject:
+		series = flattenObjectMap(o.Trees)
+	case BlobObject:
+		series = flattenObjectMap(o.Blobs)
+	}
+	return NewObjectSliceIter(series)
+}
+
+func flattenObjectMap(m map[Hash]Object) []Object {
+	objects := make([]Object, 0, len(m))
+	for _, obj := range m {
+		objects = append(objects, obj)
+	}
+	return objects
 }