Merge pull request #1056 from jfontan/fix/lose-objects-shared-cache

storage/filesystem: check file object before using cache

Author: mcuadros

storage/filesystem/object.go

@@ -307,10 +307,6 @@ func (s *ObjectStorage) DeltaObject(t plumbing.ObjectType,
 }
 
 func (s *ObjectStorage) getFromUnpacked(h plumbing.Hash) (obj plumbing.EncodedObject, err error) {
-	if cacheObj, found := s.objectCache.Get(h); found {
-		return cacheObj, nil
-	}
-
 	f, err := s.dir.Object(h)
 	if err != nil {
 		if os.IsNotExist(err) {
@@ -319,9 +315,12 @@ func (s *ObjectStorage) getFromUnpacked(h plumbing.Hash) (obj plumbing.EncodedOb
 
 		return nil, err
 	}
-
 	defer ioutil.CheckClose(f, &err)
 
+	if cacheObj, found := s.objectCache.Get(h); found {
+		return cacheObj, nil
+	}
+
 	obj = s.NewEncodedObject()
 	r, err := objfile.NewReader(f)
 	if err != nil {

storage/filesystem/object_test.go

@@ -297,6 +297,23 @@ func (s *FsSuite) TestPackfileIterKeepDescriptors(c *C) {
 	})
 }
 
+func (s *FsSuite) TestGetFromObjectFileSharedCache(c *C) {
+	f1 := fixtures.ByTag("worktree").One().DotGit()
+	f2 := fixtures.ByTag("worktree").ByTag("submodule").One().DotGit()
+
+	ch := cache.NewObjectLRUDefault()
+	o1 := NewObjectStorage(dotgit.New(f1), ch)
+	o2 := NewObjectStorage(dotgit.New(f2), ch)
+
+	expected := plumbing.NewHash("af2d6a6954d532f8ffb47615169c8fdf9d383a1a")
+	obj, err := o1.EncodedObject(plumbing.CommitObject, expected)
+	c.Assert(err, IsNil)
+	c.Assert(obj.Hash(), Equals, expected)
+
+	obj, err = o2.EncodedObject(plumbing.CommitObject, expected)
+	c.Assert(err, Equals, plumbing.ErrObjectNotFound)
+}
+
 func BenchmarkPackfileIter(b *testing.B) {
 	if err := fixtures.Init(); err != nil {
 		b.Fatal(err)