Functions in core.ObjectStorage interface now return an error

6 files changed, 62 insertions, 41 deletions

diff --git a/commit_test.go b/commit_test.go
index 19565fb..65f6400 100644
--- a/commit_test.go
+++ b/commit_test.go
@@ -170,8 +170,8 @@ 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, ok := storage.Get(core.NewHash(member))
-		if ok {
+		obj, err := storage.Get(core.NewHash(member))
+		if err == nil {
 			series = append(series, obj)
 		}
 	}
diff --git a/core/object.go b/core/object.go
index 62cb469..2f3b262 100644
--- a/core/object.go
+++ b/core/object.go
@@ -23,9 +23,9 @@ 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
 }
 
@@ -101,14 +101,11 @@ func (iter *ObjectLookupIter) Next() (Object, error) {
 		return nil, io.EOF
 	}
 	hash := iter.series[iter.pos]
-	obj, ok := iter.storage.Get(hash)
-	if !ok {
-		// FIXME: Consider making ObjectStorage.Get return an actual error that we
-		//        could pass back here.
-		return nil, ObjectNotFoundErr
+	obj, err := iter.storage.Get(hash)
+	if err == nil {
+		iter.pos++
 	}
-	iter.pos++
-	return obj, nil
+	return obj, err
 }
 
 // Close releases any resources used by the iterator.
@@ -184,11 +181,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
 
@@ -201,13 +198,16 @@ 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, ok
+	return obj, nil
 }
 
 func (o *RAWObjectStorage) Iter(t ObjectType) ObjectIter {
diff --git a/formats/packfile/reader.go b/formats/packfile/reader.go
index 959e411..91aef21 100644
--- a/formats/packfile/reader.go
+++ b/formats/packfile/reader.go
@@ -144,7 +144,10 @@ func (r *Reader) readObjects(count uint32) error {
 }
 
 func (r *Reader) newRAWObject() (core.Object, error) {
-	raw := r.s.New()
+	raw, err := r.s.New()
+	if err != nil {
+		return nil, err
+	}
 	var steps int64
 
 	var buf [1]byte
@@ -170,7 +173,6 @@ func (r *Reader) newRAWObject() (core.Object, error) {
 	raw.SetType(typ)
 	raw.SetSize(size)
 
-	var err error
 	switch raw.Type() {
 	case core.REFDeltaObject:
 		err = r.readREFDelta(raw)
@@ -196,9 +198,12 @@ func (r *Reader) readREFDelta(raw core.Object) error {
 		return err
 	}
 
-	referenced, ok := r.s.Get(ref)
-	if !ok {
-		return ObjectNotFoundErr.n("%s", ref)
+	referenced, err := r.s.Get(ref)
+	if err != nil {
+		if err == core.ObjectNotFoundErr {
+			return ObjectNotFoundErr.n("%s", ref)
+		}
+		return err
 	}
 
 	d, _ := ioutil.ReadAll(referenced.Reader())
@@ -231,7 +236,7 @@ func (r *Reader) readOFSDelta(raw core.Object, steps int64) error {
 		return PackEntryNotFoundErr.n("offset %d", start+offset)
 	}
 
-	referenced, _ := r.s.Get(ref)
+	referenced, _ := r.s.Get(ref) // FIXME: Handle error returned from Get()
 	d, _ := ioutil.ReadAll(referenced.Reader())
 	patched := patchDelta(d, buf.Bytes())
 	if patched == nil {
diff --git a/formats/packfile/reader_test.go b/formats/packfile/reader_test.go
index 8561a59..f460614 100644
--- a/formats/packfile/reader_test.go
+++ b/formats/packfile/reader_test.go
@@ -102,8 +102,8 @@ func (s *ReaderSuite) testReadPackfileGitFixture(c *C, file string, f Format) {
 func AssertObjects(c *C, s *core.RAWObjectStorage, expects []string) {
 	c.Assert(len(expects), Equals, len(s.Objects))
 	for _, expected := range expects {
-		obtained, ok := s.Get(core.NewHash(expected))
-		c.Assert(ok, Equals, true)
+		obtained, err := s.Get(core.NewHash(expected))
+		c.Assert(err, IsNil)
 		c.Assert(obtained.Hash().String(), Equals, expected)
 	}
 }
diff --git a/repository.go b/repository.go
index d0bc103..b8920c0 100644
--- a/repository.go
+++ b/repository.go
@@ -89,9 +89,12 @@ func (r *Repository) Pull(remoteName, branch string) (err error) {
 
 // Commit return the commit with the given hash
 func (r *Repository) Commit(h core.Hash) (*Commit, error) {
-	obj, ok := r.Storage.Get(h)
-	if !ok {
-		return nil, ObjectNotFoundErr
+	obj, err := r.Storage.Get(h)
+	if err != nil {
+		if err == core.ObjectNotFoundErr {
+			return nil, ObjectNotFoundErr
+		}
+		return nil, err
 	}
 
 	commit := &Commit{r: r}
@@ -105,9 +108,12 @@ func (r *Repository) Commits() *CommitIter {
 
 // Tree return the tree with the given hash
 func (r *Repository) Tree(h core.Hash) (*Tree, error) {
-	obj, ok := r.Storage.Get(h)
-	if !ok {
-		return nil, ObjectNotFoundErr
+	obj, err := r.Storage.Get(h)
+	if err != nil {
+		if err == core.ObjectNotFoundErr {
+			return nil, ObjectNotFoundErr
+		}
+		return nil, err
 	}
 
 	tree := &Tree{r: r}
diff --git a/tree.go b/tree.go
index 8398725..339fb7f 100644
--- a/tree.go
+++ b/tree.go
@@ -39,9 +39,12 @@ func (t *Tree) File(path string) (*File, error) {
 		return nil, ErrFileNotFound
 	}
 
-	obj, ok := t.r.Storage.Get(*hash)
-	if !ok {
-		return nil, ErrFileNotFound // a git submodule
+	obj, err := t.r.Storage.Get(*hash)
+	if err != nil {
+		if err == core.ObjectNotFoundErr {
+			return nil, ErrFileNotFound // a git submodule
+		}
+		return nil, err
 	}
 
 	if obj.Type() != core.BlobObject {
@@ -81,9 +84,12 @@ func (t *Tree) dir(baseName string) (*Tree, error) {
 		return nil, errDirNotFound
 	}
 
-	obj, ok := t.r.Storage.Get(entry.Hash)
-	if !ok { // git submodule
-		return nil, errDirNotFound
+	obj, err := t.r.Storage.Get(entry.Hash)
+	if err != nil {
+		if err == core.ObjectNotFoundErr { // git submodule
+			return nil, errDirNotFound
+		}
+		return nil, err
 	}
 
 	if obj.Type() != core.TreeObject {
@@ -120,9 +126,13 @@ func (t *Tree) Files() chan *File {
 
 func (t *Tree) walkEntries(base string, ch chan *File) {
 	for _, entry := range t.Entries {
-		obj, ok := t.r.Storage.Get(entry.Hash)
-		if !ok {
-			continue // ignore entries without hash (= submodule dirs)
+		obj, err := t.r.Storage.Get(entry.Hash)
+		if err != nil {
+			if err == core.ObjectNotFoundErr {
+				continue // ignore entries without hash (= submodule dirs)
+			}
+			//FIXME: Refactor this function to return an error. Ideally this would be
+			//       moved into a FileIter type.
 		}
 
 		if obj.Type() == core.TreeObject {