plumbing: packfile, open and close packfile on FSObject reads

Signed-off-by: Miguel Molina <miguel@erizocosmi.co>

5 files changed, 126 insertions, 56 deletions

diff --git a/plumbing/format/packfile/encoder_advanced_test.go b/plumbing/format/packfile/encoder_advanced_test.go
index 78ddc45..fc1419e 100644
--- a/plumbing/format/packfile/encoder_advanced_test.go
+++ b/plumbing/format/packfile/encoder_advanced_test.go
@@ -6,7 +6,7 @@ import (
 	"math/rand"
 	"testing"
 
-	"gopkg.in/src-d/go-billy.v3/memfs"
+	"gopkg.in/src-d/go-billy.v4/memfs"
 	"gopkg.in/src-d/go-git.v4/plumbing"
 	"gopkg.in/src-d/go-git.v4/plumbing/format/idxfile"
 	. "gopkg.in/src-d/go-git.v4/plumbing/format/packfile"
@@ -84,7 +84,8 @@ func (s *EncoderAdvancedSuite) testEncodeDecode(
 	encodeHash, err := enc.Encode(hashes, packWindow)
 	c.Assert(err, IsNil)
 
-	f, err := memfs.New().Create("packfile")
+	fs := memfs.New()
+	f, err := fs.Create("packfile")
 	c.Assert(err, IsNil)
 
 	_, err = f.Write(buf.Bytes())
@@ -105,7 +106,7 @@ func (s *EncoderAdvancedSuite) testEncodeDecode(
 	_, err = f.Seek(0, io.SeekStart)
 	c.Assert(err, IsNil)
 
-	p := NewPackfile(index, f)
+	p := NewPackfile(index, fs, f)
 
 	decodeHash, err := p.ID()
 	c.Assert(err, IsNil)
diff --git a/plumbing/format/packfile/encoder_test.go b/plumbing/format/packfile/encoder_test.go
index 24e2082..80b916d 100644
--- a/plumbing/format/packfile/encoder_test.go
+++ b/plumbing/format/packfile/encoder_test.go
@@ -5,7 +5,7 @@ import (
 	"io"
 	stdioutil "io/ioutil"
 
-	"gopkg.in/src-d/go-billy.v3/memfs"
+	"gopkg.in/src-d/go-billy.v4/memfs"
 	"gopkg.in/src-d/go-git.v4/plumbing"
 	"gopkg.in/src-d/go-git.v4/plumbing/format/idxfile"
 	"gopkg.in/src-d/go-git.v4/storage/memory"
@@ -290,7 +290,8 @@ func objectsEqual(c *C, o1, o2 plumbing.EncodedObject) {
 }
 
 func packfileFromReader(c *C, buf *bytes.Buffer) (*Packfile, func()) {
-	file, err := memfs.New().Create("packfile")
+	fs := memfs.New()
+	file, err := fs.Create("packfile")
 	c.Assert(err, IsNil)
 
 	_, err = file.Write(buf.Bytes())
@@ -311,7 +312,7 @@ func packfileFromReader(c *C, buf *bytes.Buffer) (*Packfile, func()) {
 	index, err := w.Index()
 	c.Assert(err, IsNil)
 
-	return NewPackfile(index, file), func() {
+	return NewPackfile(index, fs, file), func() {
 		c.Assert(file.Close(), IsNil)
 	}
 }
diff --git a/plumbing/format/packfile/fsobject.go b/plumbing/format/packfile/fsobject.go
index d63127e..6fd3ca5 100644
--- a/plumbing/format/packfile/fsobject.go
+++ b/plumbing/format/packfile/fsobject.go
@@ -3,17 +3,23 @@ package packfile
 import (
 	"io"
 
+	billy "gopkg.in/src-d/go-billy.v4"
 	"gopkg.in/src-d/go-git.v4/plumbing"
+	"gopkg.in/src-d/go-git.v4/plumbing/cache"
+	"gopkg.in/src-d/go-git.v4/plumbing/format/idxfile"
 )
 
 // FSObject is an object from the packfile on the filesystem.
 type FSObject struct {
-	hash     plumbing.Hash
-	h        *ObjectHeader
-	offset   int64
-	size     int64
-	typ      plumbing.ObjectType
-	packfile *Packfile
+	hash   plumbing.Hash
+	h      *ObjectHeader
+	offset int64
+	size   int64
+	typ    plumbing.ObjectType
+	index  idxfile.Index
+	fs     billy.Filesystem
+	path   string
+	cache  cache.Object
 }
 
 // NewFSObject creates a new filesystem object.
@@ -22,20 +28,42 @@ func NewFSObject(
 	finalType plumbing.ObjectType,
 	offset int64,
 	contentSize int64,
-	packfile *Packfile,
+	index idxfile.Index,
+	fs billy.Filesystem,
+	path string,
+	cache cache.Object,
 ) *FSObject {
 	return &FSObject{
-		hash:     hash,
-		offset:   offset,
-		size:     contentSize,
-		typ:      finalType,
-		packfile: packfile,
+		hash:   hash,
+		offset: offset,
+		size:   contentSize,
+		typ:    finalType,
+		index:  index,
+		fs:     fs,
+		path:   path,
+		cache:  cache,
 	}
 }
 
 // Reader implements the plumbing.EncodedObject interface.
 func (o *FSObject) Reader() (io.ReadCloser, error) {
-	return o.packfile.getObjectContent(o.offset)
+	f, err := o.fs.Open(o.path)
+	if err != nil {
+		return nil, err
+	}
+
+	p := NewPackfileWithCache(o.index, nil, f, o.cache)
+	r, err := p.getObjectContent(o.offset)
+	if err != nil {
+		_ = f.Close()
+		return nil, err
+	}
+
+	if err := f.Close(); err != nil {
+		return nil, err
+	}
+
+	return r, nil
 }
 
 // SetSize implements the plumbing.EncodedObject interface. This method
@@ -62,3 +90,17 @@ func (o *FSObject) Type() plumbing.ObjectType {
 func (o *FSObject) Writer() (io.WriteCloser, error) {
 	return nil, nil
 }
+
+type objectReader struct {
+	io.ReadCloser
+	f billy.File
+}
+
+func (r *objectReader) Close() error {
+	if err := r.ReadCloser.Close(); err != nil {
+		_ = r.f.Close()
+		return err
+	}
+
+	return r.f.Close()
+}
diff --git a/plumbing/format/packfile/packfile.go b/plumbing/format/packfile/packfile.go
index df8a3d4..5feb781 100644
--- a/plumbing/format/packfile/packfile.go
+++ b/plumbing/format/packfile/packfile.go
@@ -3,9 +3,9 @@ package packfile
 import (
 	"bytes"
 	"io"
-	stdioutil "io/ioutil"
 	"os"
 
+	billy "gopkg.in/src-d/go-billy.v4"
 	"gopkg.in/src-d/go-git.v4/plumbing"
 	"gopkg.in/src-d/go-git.v4/plumbing/cache"
 	"gopkg.in/src-d/go-git.v4/plumbing/format/idxfile"
@@ -24,21 +24,26 @@ var (
 // Packfile allows retrieving information from inside a packfile.
 type Packfile struct {
 	idxfile.Index
-	file           io.ReadSeeker
+	fs             billy.Filesystem
+	file           billy.File
 	s              *Scanner
 	deltaBaseCache cache.Object
 	offsetToType   map[int64]plumbing.ObjectType
 }
 
 // NewPackfileWithCache creates a new Packfile with the given object cache.
+// If the filesystem is provided, the packfile will return FSObjects, otherwise
+// it will return MemoryObjects.
 func NewPackfileWithCache(
 	index idxfile.Index,
-	file io.ReadSeeker,
+	fs billy.Filesystem,
+	file billy.File,
 	cache cache.Object,
 ) *Packfile {
 	s := NewScanner(file)
 	return &Packfile{
 		index,
+		fs,
 		file,
 		s,
 		cache,
@@ -48,8 +53,10 @@ func NewPackfileWithCache(
 
 // NewPackfile returns a packfile representation for the given packfile file
 // and packfile idx.
-func NewPackfile(index idxfile.Index, file io.ReadSeeker) *Packfile {
-	return NewPackfileWithCache(index, file, cache.NewObjectLRUDefault())
+// If the filesystem is provided, the packfile will return FSObjects, otherwise
+// it will return MemoryObjects.
+func NewPackfile(index idxfile.Index, fs billy.Filesystem, file billy.File) *Packfile {
+	return NewPackfileWithCache(index, fs, file, cache.NewObjectLRUDefault())
 }
 
 // Get retrieves the encoded object in the packfile with the given hash.
@@ -215,6 +222,12 @@ func (p *Packfile) nextObject() (plumbing.EncodedObject, error) {
 		return nil, err
 	}
 
+	// If we have no filesystem, we will return a MemoryObject instead
+	// of an FSObject.
+	if p.fs == nil {
+		return p.getNextObject(h)
+	}
+
 	hash, err := p.FindHash(h.Offset)
 	if err != nil {
 		return nil, err
@@ -232,7 +245,16 @@ func (p *Packfile) nextObject() (plumbing.EncodedObject, error) {
 
 	p.offsetToType[h.Offset] = typ
 
-	return NewFSObject(hash, typ, h.Offset, size, p), nil
+	return NewFSObject(
+		hash,
+		typ,
+		h.Offset,
+		size,
+		p.Index,
+		p.fs,
+		p.file.Name(),
+		p.deltaBaseCache,
+	), nil
 }
 
 func (p *Packfile) getObjectContent(offset int64) (io.ReadCloser, error) {
@@ -245,10 +267,20 @@ func (p *Packfile) getObjectContent(offset int64) (io.ReadCloser, error) {
 		return nil, err
 	}
 
+	obj, err := p.getNextObject(h)
+	if err != nil {
+		return nil, err
+	}
+
+	return obj.Reader()
+}
+
+func (p *Packfile) getNextObject(h *ObjectHeader) (plumbing.EncodedObject, error) {
 	var obj = new(plumbing.MemoryObject)
 	obj.SetSize(h.Length)
 	obj.SetType(h.Type)
 
+	var err error
 	switch h.Type {
 	case plumbing.CommitObject, plumbing.TreeObject, plumbing.BlobObject, plumbing.TagObject:
 		err = p.fillRegularObjectContent(obj)
@@ -264,7 +296,7 @@ func (p *Packfile) getObjectContent(offset int64) (io.ReadCloser, error) {
 		return nil, err
 	}
 
-	return obj.Reader()
+	return obj, nil
 }
 
 func (p *Packfile) fillRegularObjectContent(obj plumbing.EncodedObject) error {
@@ -410,29 +442,6 @@ func (p *Packfile) Close() error {
 	return closer.Close()
 }
 
-// MemoryObjectFromDisk converts a FSObject to a MemoryObject.
-func MemoryObjectFromDisk(obj plumbing.EncodedObject) (plumbing.EncodedObject, error) {
-	o2 := new(plumbing.MemoryObject)
-	o2.SetType(obj.Type())
-	o2.SetSize(obj.Size())
-
-	r, err := obj.Reader()
-	if err != nil {
-		return nil, err
-	}
-
-	data, err := stdioutil.ReadAll(r)
-	if err != nil {
-		return nil, err
-	}
-
-	if _, err := o2.Write(data); err != nil {
-		return nil, err
-	}
-
-	return o2, nil
-}
-
 type objectIter struct {
 	p    *Packfile
 	typ  plumbing.ObjectType
diff --git a/plumbing/format/packfile/packfile_test.go b/plumbing/format/packfile/packfile_test.go
index 3193bed..05dc8a7 100644
--- a/plumbing/format/packfile/packfile_test.go
+++ b/plumbing/format/packfile/packfile_test.go
@@ -109,13 +109,14 @@ var expectedEntries = map[plumbing.Hash]int64{
 func (s *PackfileSuite) SetUpTest(c *C) {
 	s.f = fixtures.Basic().One()
 
-	f, err := osfs.New("").Open(s.f.Packfile().Name())
+	fs := osfs.New("")
+	f, err := fs.Open(s.f.Packfile().Name())
 	c.Assert(err, IsNil)
 
 	s.idx = idxfile.NewMemoryIndex()
 	c.Assert(idxfile.NewDecoder(s.f.Idx()).Decode(s.idx), IsNil)
 
-	s.p = packfile.NewPackfile(s.idx, f)
+	s.p = packfile.NewPackfile(s.idx, fs, f)
 }
 
 func (s *PackfileSuite) TearDownTest(c *C) {
@@ -125,7 +126,11 @@ func (s *PackfileSuite) TearDownTest(c *C) {
 func (s *PackfileSuite) TestDecode(c *C) {
 	fixtures.Basic().ByTag("packfile").Test(c, func(f *fixtures.Fixture) {
 		index := getIndexFromIdxFile(f.Idx())
-		p := packfile.NewPackfile(index, f.Packfile())
+		fs := osfs.New("")
+		pf, err := fs.Open(f.Packfile().Name())
+		c.Assert(err, IsNil)
+
+		p := packfile.NewPackfile(index, fs, pf)
 		defer p.Close()
 
 		for _, h := range expectedHashes {
@@ -140,7 +145,11 @@ func (s *PackfileSuite) TestDecodeByTypeRefDelta(c *C) {
 	f := fixtures.Basic().ByTag("ref-delta").One()
 
 	index := getIndexFromIdxFile(f.Idx())
-	packfile := packfile.NewPackfile(index, f.Packfile())
+	fs := osfs.New("")
+	pf, err := fs.Open(f.Packfile().Name())
+	c.Assert(err, IsNil)
+
+	packfile := packfile.NewPackfile(index, fs, pf)
 	defer packfile.Close()
 
 	iter, err := packfile.GetByType(plumbing.CommitObject)
@@ -171,7 +180,11 @@ func (s *PackfileSuite) TestDecodeByType(c *C) {
 	fixtures.Basic().ByTag("packfile").Test(c, func(f *fixtures.Fixture) {
 		for _, t := range ts {
 			index := getIndexFromIdxFile(f.Idx())
-			packfile := packfile.NewPackfile(index, f.Packfile())
+			fs := osfs.New("")
+			pf, err := fs.Open(f.Packfile().Name())
+			c.Assert(err, IsNil)
+
+			packfile := packfile.NewPackfile(index, fs, pf)
 			defer packfile.Close()
 
 			iter, err := packfile.GetByType(t)
@@ -188,10 +201,14 @@ func (s *PackfileSuite) TestDecodeByType(c *C) {
 func (s *PackfileSuite) TestDecodeByTypeConstructor(c *C) {
 	f := fixtures.Basic().ByTag("packfile").One()
 	index := getIndexFromIdxFile(f.Idx())
-	packfile := packfile.NewPackfile(index, f.Packfile())
+	fs := osfs.New("")
+	pf, err := fs.Open(f.Packfile().Name())
+	c.Assert(err, IsNil)
+
+	packfile := packfile.NewPackfile(index, fs, pf)
 	defer packfile.Close()
 
-	_, err := packfile.GetByType(plumbing.OFSDeltaObject)
+	_, err = packfile.GetByType(plumbing.OFSDeltaObject)
 	c.Assert(err, Equals, plumbing.ErrInvalidType)
 
 	_, err = packfile.GetByType(plumbing.REFDeltaObject)