plumbing/cache: use more explicit interface

* renamed Add to Put
* Get returns a second bool value to indicate if there
  was hit or miss.

4 files changed, 44 insertions, 26 deletions

diff --git a/plumbing/cache/common.go b/plumbing/cache/common.go
index 7b90c55..9efc26c 100644
--- a/plumbing/cache/common.go
+++ b/plumbing/cache/common.go
@@ -11,8 +11,14 @@ const (
 
 type FileSize int64
 
+// Object is an interface to a object cache.
 type Object interface {
-	Add(o plumbing.EncodedObject)
-	Get(k plumbing.Hash) plumbing.EncodedObject
+	// Put puts the given object into the cache. Whether this object will
+	// actually be put into the cache or not is implementation specific.
+	Put(o plumbing.EncodedObject)
+	// Get gets an object from the cache given its hash. The second return value
+	// is true if the object was returned, and false otherwise.
+	Get(k plumbing.Hash) (plumbing.EncodedObject, bool)
+	// Clear clears every object from the cache.
 	Clear()
 }
diff --git a/plumbing/cache/object.go b/plumbing/cache/object.go
index 44238ce..44e0d32 100644
--- a/plumbing/cache/object.go
+++ b/plumbing/cache/object.go
@@ -1,6 +1,8 @@
 package cache
 
-import "gopkg.in/src-d/go-git.v4/plumbing"
+import (
+	"gopkg.in/src-d/go-git.v4/plumbing"
+)
 
 const (
 	initialQueueSize = 20
@@ -25,12 +27,14 @@ func NewObjectFIFO(size FileSize) *ObjectFIFO {
 	}
 }
 
-// Add adds a new object to the cache. If the object size is greater than the
+// Put puts a new object to the cache. If the object size is greater than the
 // cache size, the object is not added.
-func (c *ObjectFIFO) Add(o plumbing.EncodedObject) {
+func (c *ObjectFIFO) Put(o plumbing.EncodedObject) {
+	objSize := FileSize(o.Size())
+
 	// if the size of the object is bigger or equal than the cache size,
 	// skip it
-	if FileSize(o.Size()) >= c.maxSize {
+	if objSize >= c.maxSize {
 		return
 	}
 
@@ -56,8 +60,9 @@ func (c *ObjectFIFO) Add(o plumbing.EncodedObject) {
 
 // Get returns an object by his hash. If the object is not found in the cache, it
 // returns nil
-func (c *ObjectFIFO) Get(k plumbing.Hash) plumbing.EncodedObject {
-	return c.objects[k]
+func (c *ObjectFIFO) Get(k plumbing.Hash) (plumbing.EncodedObject, bool) {
+	obj, ok := c.objects[k]
+	return obj, ok
 }
 
 // Clear the content of this object cache
diff --git a/plumbing/cache/object_test.go b/plumbing/cache/object_test.go
index 80cd17b..2a55acf 100644
--- a/plumbing/cache/object_test.go
+++ b/plumbing/cache/object_test.go
@@ -31,14 +31,14 @@ func (s *ObjectSuite) SetUpTest(c *C) {
 }
 
 func (s *ObjectSuite) TestAdd_SameObject(c *C) {
-	s.c.Add(s.aObject)
+	s.c.Put(s.aObject)
 	c.Assert(s.c.actualSize, Equals, 1*Byte)
-	s.c.Add(s.aObject)
+	s.c.Put(s.aObject)
 	c.Assert(s.c.actualSize, Equals, 1*Byte)
 }
 
 func (s *ObjectSuite) TestAdd_BigObject(c *C) {
-	s.c.Add(s.bObject)
+	s.c.Put(s.bObject)
 	c.Assert(s.c.actualSize, Equals, 0*Byte)
 	c.Assert(s.c.actualSize, Equals, 0*KiByte)
 	c.Assert(s.c.actualSize, Equals, 0*MiByte)
@@ -47,24 +47,32 @@ func (s *ObjectSuite) TestAdd_BigObject(c *C) {
 }
 
 func (s *ObjectSuite) TestAdd_CacheOverflow(c *C) {
-	s.c.Add(s.aObject)
+	s.c.Put(s.aObject)
 	c.Assert(s.c.actualSize, Equals, 1*Byte)
-	s.c.Add(s.cObject)
+	s.c.Put(s.cObject)
 	c.Assert(len(s.c.objects), Equals, 2)
-	s.c.Add(s.dObject)
+	s.c.Put(s.dObject)
 	c.Assert(len(s.c.objects), Equals, 2)
 
-	c.Assert(s.c.Get(s.aObject.Hash()), IsNil)
-	c.Assert(s.c.Get(s.cObject.Hash()), NotNil)
-	c.Assert(s.c.Get(s.dObject.Hash()), NotNil)
+	obj, ok := s.c.Get(s.aObject.Hash())
+	c.Assert(ok, Equals, false)
+	c.Assert(obj, IsNil)
+	obj, ok = s.c.Get(s.cObject.Hash())
+	c.Assert(ok, Equals, true)
+	c.Assert(obj, NotNil)
+	obj, ok = s.c.Get(s.dObject.Hash())
+	c.Assert(ok, Equals, true)
+	c.Assert(obj, NotNil)
 }
 
 func (s *ObjectSuite) TestClear(c *C) {
-	s.c.Add(s.aObject)
+	s.c.Put(s.aObject)
 	c.Assert(s.c.actualSize, Equals, 1*Byte)
 	s.c.Clear()
 	c.Assert(s.c.actualSize, Equals, 0*Byte)
-	c.Assert(s.c.Get(s.aObject.Hash()), IsNil)
+	obj, ok := s.c.Get(s.aObject.Hash())
+	c.Assert(ok, Equals, false)
+	c.Assert(obj, IsNil)
 }
 
 type dummyObject struct {
diff --git a/plumbing/format/packfile/decoder.go b/plumbing/format/packfile/decoder.go
index 39680a3..3d2eb3b 100644
--- a/plumbing/format/packfile/decoder.go
+++ b/plumbing/format/packfile/decoder.go
@@ -355,9 +355,8 @@ func (d *Decoder) fillREFDeltaObjectContent(obj plumbing.EncodedObject, ref plum
 		return 0, err
 	}
 
-	base := d.cache.Get(ref)
-
-	if base == nil {
+	base, ok := d.cache.Get(ref)
+	if !ok {
 		base, err = d.recallByHash(ref)
 		if err != nil {
 			return 0, err
@@ -366,7 +365,7 @@ func (d *Decoder) fillREFDeltaObjectContent(obj plumbing.EncodedObject, ref plum
 
 	obj.SetType(base.Type())
 	err = ApplyDelta(obj, base, buf.Bytes())
-	d.cache.Add(obj)
+	d.cache.Put(obj)
 
 	return crc, err
 }
@@ -381,10 +380,10 @@ func (d *Decoder) fillOFSDeltaObjectContent(obj plumbing.EncodedObject, offset i
 	e, ok := d.idx.LookupOffset(uint64(offset))
 	var base plumbing.EncodedObject
 	if ok {
-		base = d.cache.Get(e.Hash)
+		base, ok = d.cache.Get(e.Hash)
 	}
 
-	if base == nil {
+	if !ok {
 		base, err = d.recallByOffset(offset)
 		if err != nil {
 			return 0, err
@@ -393,7 +392,7 @@ func (d *Decoder) fillOFSDeltaObjectContent(obj plumbing.EncodedObject, offset i
 
 	obj.SetType(base.Type())
 	err = ApplyDelta(obj, base, buf.Bytes())
-	d.cache.Add(obj)
+	d.cache.Put(obj)
 
 	return crc, err
 }