format: packfile fix ReadObjectAt without decode

4 files changed, 87 insertions, 47 deletions

diff --git a/formats/packfile/decoder.go b/formats/packfile/decoder.go
index 23a8e1a..c4b9182 100644
--- a/formats/packfile/decoder.go
+++ b/formats/packfile/decoder.go
@@ -43,8 +43,9 @@ type Decoder struct {
 	o  core.ObjectStorage
 	tx core.TxObjectStorage
 
-	offsets map[int64]core.Hash
-	crcs    map[core.Hash]uint32
+	offsetToHash map[int64]core.Hash
+	hashToOffset map[core.Hash]int64
+	crcs         map[core.Hash]uint32
 }
 
 // NewDecoder returns a new Decoder that reads from r.
@@ -54,8 +55,9 @@ func NewDecoder(s *Scanner, o core.ObjectStorage) *Decoder {
 		o:  o,
 		tx: o.Begin(),
 
-		offsets: make(map[int64]core.Hash, 0),
-		crcs:    make(map[core.Hash]uint32, 0),
+		offsetToHash: make(map[int64]core.Hash, 0),
+		hashToOffset: make(map[core.Hash]int64, 0),
+		crcs:         make(map[core.Hash]uint32, 0),
 	}
 }
 
@@ -82,11 +84,7 @@ func (d *Decoder) doDecode() error {
 		return err
 	}
 
-	if err := d.tx.Commit(); err != nil {
-		return err
-	}
-
-	return nil
+	return d.tx.Commit()
 }
 
 func (d *Decoder) readObjects(count uint32) error {
@@ -126,7 +124,9 @@ func (d *Decoder) ReadObject() (core.Object, error) {
 		return obj, err
 	}
 
-	d.remember(obj, h.Offset, crc)
+	hash := obj.Hash()
+	d.setOffset(hash, h.Offset)
+	d.setCRC(hash, crc)
 
 	if _, err := d.tx.Set(obj); err != nil {
 		return nil, err
@@ -194,34 +194,45 @@ func (d *Decoder) fillOFSDeltaObjectContent(obj core.Object, offset int64) (uint
 	return crc, ApplyDelta(obj, base, buf.Bytes())
 }
 
-func (d *Decoder) remember(obj core.Object, offset int64, crc uint32) {
-	h := obj.Hash()
+func (d *Decoder) setOffset(h core.Hash, offset int64) {
+	d.offsetToHash[offset] = h
+	d.hashToOffset[h] = offset
+}
 
-	d.offsets[offset] = h
+func (d *Decoder) setCRC(h core.Hash, crc uint32) {
 	d.crcs[h] = crc
 }
 
 func (d *Decoder) recallByOffset(o int64) (core.Object, error) {
-	h, ok := d.offsets[o]
-	if ok {
-		return d.recallByHash(h)
+	if h, ok := d.offsetToHash[o]; ok {
+		return d.tx.Get(core.AnyObject, h)
 	}
 
 	return d.ReadObjectAt(o)
 }
 
 func (d *Decoder) recallByHash(h core.Hash) (core.Object, error) {
-	return d.tx.Get(core.AnyObject, h)
+	obj, err := d.tx.Get(core.AnyObject, h)
+	if err != core.ErrObjectNotFound {
+		return obj, err
+	}
+
+	if o, ok := d.hashToOffset[h]; ok {
+		return d.ReadObjectAt(o)
+	}
+
+	return nil, core.ErrObjectNotFound
+}
+
+// SetOffsets sets the offsets, required when using the method ReadObjectAt,
+// without decoding the full packfile
+func (d *Decoder) SetOffsets(offsets map[core.Hash]int64) {
+	d.hashToOffset = offsets
 }
 
 // Offsets returns the objects read offset
 func (d *Decoder) Offsets() map[core.Hash]int64 {
-	i := make(map[core.Hash]int64, len(d.offsets))
-	for o, h := range d.offsets {
-		i[h] = o
-	}
-
-	return i
+	return d.hashToOffset
 }
 
 // CRCs returns the CRC-32 for each objected read
diff --git a/formats/packfile/decoder_test.go b/formats/packfile/decoder_test.go
index 7baab44..d85f3bf 100644
--- a/formats/packfile/decoder_test.go
+++ b/formats/packfile/decoder_test.go
@@ -1,10 +1,12 @@
 package packfile
 
 import (
+	"io"
 	"testing"
 
 	"gopkg.in/src-d/go-git.v4/core"
 	"gopkg.in/src-d/go-git.v4/fixtures"
+	"gopkg.in/src-d/go-git.v4/formats/idxfile"
 	"gopkg.in/src-d/go-git.v4/storage/memory"
 
 	. "gopkg.in/check.v1"
@@ -68,7 +70,7 @@ func (s *ReaderSuite) TestDecode(c *C) {
 	})
 }
 func (s *ReaderSuite) TestDecodeCRCs(c *C) {
-	f := fixtures.Basic().ByTag("ofs-delta")
+	f := fixtures.Basic().ByTag("ofs-delta").One()
 
 	scanner := NewScanner(f.Packfile())
 	storage := memory.NewStorage()
@@ -86,18 +88,24 @@ func (s *ReaderSuite) TestDecodeCRCs(c *C) {
 }
 
 func (s *ReaderSuite) TestReadObjectAt(c *C) {
-	f := fixtures.Basic().One()
+	fixtures.Basic().Test(c, func(f *fixtures.Fixture) {
+		scanner := NewScanner(f.Packfile())
+		storage := memory.NewStorage()
 
-	scanner := NewScanner(f.Packfile())
-	storage := memory.NewStorage()
+		d := NewDecoder(scanner, storage.ObjectStorage())
 
-	d := NewDecoder(scanner, storage.ObjectStorage())
+		// when the packfile is ref-delta based, the offsets are required
+		if f.Is("ref-delta") {
+			offsets := getOffsetsFromIdx(f.Idx())
+			d.SetOffsets(offsets)
+		}
 
-	// the objects at reference 186, is a delta, so should be recall, without
-	// being read before.
-	obj, err := d.ReadObjectAt(186)
-	c.Assert(err, IsNil)
-	c.Assert(obj.Hash().String(), Equals, "6ecf0ef2c2dffb796033e5a02219af86ec6584e5")
+		// the objects at reference 186, is a delta, so should be recall,
+		// without being read before.
+		obj, err := d.ReadObjectAt(186)
+		c.Assert(err, IsNil)
+		c.Assert(obj.Hash().String(), Equals, "6ecf0ef2c2dffb796033e5a02219af86ec6584e5")
+	})
 }
 
 func AssertObjects(c *C, s *memory.Storage, expects []string) {
@@ -110,3 +118,18 @@ func AssertObjects(c *C, s *memory.Storage, expects []string) {
 		c.Assert(obt.Hash().String(), Equals, exp)
 	}
 }
+
+func getOffsetsFromIdx(r io.Reader) map[core.Hash]int64 {
+	idx := &idxfile.Idxfile{}
+	err := idxfile.NewDecoder(r).Decode(idx)
+	if err != nil {
+		panic(err)
+	}
+
+	offsets := make(map[core.Hash]int64)
+	for _, e := range idx.Entries {
+		offsets[e.Hash] = int64(e.Offset)
+	}
+
+	return offsets
+}
diff --git a/formats/packfile/parser.go b/formats/packfile/scanner.go
index 6fa2f42..86092a1 100644
--- a/formats/packfile/parser.go
+++ b/formats/packfile/scanner.go
@@ -41,8 +41,6 @@ type ObjectHeader struct {
 	OffsetReference int64
 }
 
-// A Parser is a collection of functions to read and process data form a packfile.
-// Values from this type are not zero-value safe. See the NewParser function bellow.
 type Scanner struct {
 	r   reader
 	crc hash.Hash32
@@ -53,18 +51,22 @@ type Scanner struct {
 	version, objects uint32
 }
 
-// NewParser returns a new Parser that reads from the packfile represented by r.
-func NewScannerFromReader(r io.Reader) *Scanner {
-	s := &trackableReader{Reader: r}
-	return NewScanner(s)
-}
+// NewScanner returns a new Scanner based on a reader, if the given reader
+// implements io.ReadSeeker the Scanner will be also Seekable
+func NewScanner(r io.Reader) *Scanner {
+	seeker, ok := r.(io.ReadSeeker)
+	if !ok {
+		seeker = &trackableReader{Reader: r}
+	}
 
-func NewScanner(r io.ReadSeeker) *Scanner {
 	crc := crc32.NewIEEE()
-	seeker := newByteReadSeeker(r)
-	tee := &teeReader{seeker, crc}
-
-	return &Scanner{r: tee, crc: crc}
+	return &Scanner{
+		r: &teeReader{
+			newByteReadSeeker(seeker),
+			crc,
+		},
+		crc: crc,
+	}
 }
 
 // Header reads the whole packfile header (signature, version and object count).
@@ -265,6 +267,8 @@ func (s *Scanner) readLength(first byte) (int64, error) {
 	return length, nil
 }
 
+// NextObject writes the content of the next object into the reader, returns
+// the number of bytes written, the CRC32 of the content and an error, if any
 func (s *Scanner) NextObject(w io.Writer) (written int64, crc32 uint32, err error) {
 	defer s.crc.Reset()
 
@@ -308,6 +312,7 @@ func (s *Scanner) Seek(offset int64) (previous int64, err error) {
 	return previous, err
 }
 
+// Checksum returns the checksum of the packfile
 func (s *Scanner) Checksum() (core.Hash, error) {
 	err := s.discardObjectIfNeeded()
 	if err != nil {
@@ -383,6 +388,7 @@ func (s *Scanner) readByte() (byte, error) {
 	return b, err
 }
 
+// Close reads the reader until io.EOF
 func (s *Scanner) Close() error {
 	_, err := io.Copy(ioutil.Discard, s.r)
 	return err
diff --git a/formats/packfile/parser_test.go b/formats/packfile/scanner_test.go
index 2ff2887..6161fdb 100644
--- a/formats/packfile/parser_test.go
+++ b/formats/packfile/scanner_test.go
@@ -49,7 +49,7 @@ func (s *ScannerSuite) TestNextObjectHeaderOFSDelta(c *C) {
 }
 
 func (s *ScannerSuite) testNextObjectHeader(c *C, tag string, expected []ObjectHeader) {
-	r := fixtures.Basic().ByTag(tag).Packfile()
+	r := fixtures.Basic().ByTag(tag).One().Packfile()
 	p := NewScanner(r)
 
 	_, objects, err := p.Header()
@@ -72,7 +72,7 @@ func (s *ScannerSuite) testNextObjectHeader(c *C, tag string, expected []ObjectH
 }
 
 func (s *ScannerSuite) TestNextObjectHeaderWithOutReadObject(c *C) {
-	f := fixtures.Basic().ByTag("ref-delta")
+	f := fixtures.Basic().ByTag("ref-delta").One()
 	r := f.Packfile()
 	p := NewScanner(r)