From ce8c9645cc8ddb87abcf29c178ad6a784d43cbf3 Mon Sep 17 00:00:00 2001 From: Máximo Cuadros Date: Wed, 23 Nov 2016 10:23:32 +0100 Subject: plumbing: format, packfile fix issue #129, related #124, and documentation improvements (#130) * plumbing: format, packfile fix issue #129, related #124 * plumbing: format, packfile documentation improvements --- plumbing/format/packfile/decoder.go | 102 +++++++++++++++++++++--------- plumbing/format/packfile/decoder_test.go | 103 ++++++++++++++++++++++++------- 2 files changed, 154 insertions(+), 51 deletions(-) (limited to 'plumbing') diff --git a/plumbing/format/packfile/decoder.go b/plumbing/format/packfile/decoder.go index 5c103a0..4ab8eb0 100644 --- a/plumbing/format/packfile/decoder.go +++ b/plumbing/format/packfile/decoder.go @@ -34,29 +34,46 @@ var ( // ErrCannotRecall is returned by RecallByOffset or RecallByHash if the object // to recall cannot be returned. ErrCannotRecall = NewError("cannot recall object") - // ErrNonSeekable is returned if a NewDecoder is used with a non-seekable - // reader and without a plumbing.ObjectStorage or ReadObjectAt method is called - // without a seekable scanner + // ErrResolveDeltasNotSupported is returned if a NewDecoder is used with a + // non-seekable scanner and without a plumbing.ObjectStorage + ErrResolveDeltasNotSupported = NewError("resolve delta is not supported") + // ErrNonSeekable is returned if a ReadObjectAt method is called without a + // seekable scanner ErrNonSeekable = NewError("non-seekable scanner") // ErrRollback error making Rollback over a transaction after an error ErrRollback = NewError("rollback error, during set error") + // ErrAlreadyDecoded is returned if NewDecoder is called for a second time + ErrAlreadyDecoded = NewError("packfile was already decoded") ) -// Decoder reads and decodes packfiles from an input stream. +// Decoder reads and decodes packfiles from an input Scanner, if an ObjectStorer +// was provided the decoded objects are store there. If not the decode object +// is destroyed. The Offsets and CRCs are calculated independand if the an +// ObjectStorer was provided or not. type Decoder struct { s *Scanner o storer.ObjectStorer tx storer.Transaction + isDecoded bool offsetToHash map[int64]plumbing.Hash hashToOffset map[plumbing.Hash]int64 crcs map[plumbing.Hash]uint32 } -// NewDecoder returns a new Decoder that reads from r. +// NewDecoder returns a new Decoder that decodes a Packfile using the given +// s and store the objects in the provided o. ObjectStorer can be nil, in this +// case the objects are not stored but objects offsets on the Packfile and the +// CRCs are calculated. +// +// If ObjectStorer is nil and the Scanner is not Seekable, ErrNonSeekable is +// returned. +// +// If the ObjectStorer implements storer.Transactioner, a transaction is created +// during the Decode execution, if something fails the Rollback is called func NewDecoder(s *Scanner, o storer.ObjectStorer) (*Decoder, error) { - if !s.IsSeekable && o == nil { - return nil, ErrNonSeekable + if !canResolveDeltas(s, o) { + return nil, ErrResolveDeltasNotSupported } return &Decoder{ @@ -69,8 +86,19 @@ func NewDecoder(s *Scanner, o storer.ObjectStorer) (*Decoder, error) { }, nil } -// Decode reads a packfile and stores it in the value pointed to by s. +func canResolveDeltas(s *Scanner, o storer.ObjectStorer) bool { + return s.IsSeekable || o != nil +} + +// Decode reads a packfile and stores it in the value pointed to by s. The +// offsets and the CRCs are calculated by this method func (d *Decoder) Decode() (checksum plumbing.Hash, err error) { + defer func() { d.isDecoded = true }() + + if d.isDecoded { + return plumbing.ZeroHash, ErrAlreadyDecoded + } + if err := d.doDecode(); err != nil { return plumbing.ZeroHash, err } @@ -87,17 +115,17 @@ func (d *Decoder) doDecode() error { _, isTxStorer := d.o.(storer.Transactioner) switch { case d.o == nil: - return d.readObjects(int(count)) + return d.decodeObjects(int(count)) case isTxStorer: - return d.readObjectsWithObjectStorerTx(int(count)) + return d.decodeObjectsWithObjectStorerTx(int(count)) default: - return d.readObjectsWithObjectStorer(int(count)) + return d.decodeObjectsWithObjectStorer(int(count)) } } -func (d *Decoder) readObjects(count int) error { +func (d *Decoder) decodeObjects(count int) error { for i := 0; i < count; i++ { - if _, err := d.ReadObject(); err != nil { + if _, err := d.DecodeObject(); err != nil { return err } } @@ -105,9 +133,9 @@ func (d *Decoder) readObjects(count int) error { return nil } -func (d *Decoder) readObjectsWithObjectStorer(count int) error { +func (d *Decoder) decodeObjectsWithObjectStorer(count int) error { for i := 0; i < count; i++ { - obj, err := d.ReadObject() + obj, err := d.DecodeObject() if err != nil { return err } @@ -120,11 +148,11 @@ func (d *Decoder) readObjectsWithObjectStorer(count int) error { return nil } -func (d *Decoder) readObjectsWithObjectStorerTx(count int) error { +func (d *Decoder) decodeObjectsWithObjectStorerTx(count int) error { d.tx = d.o.(storer.Transactioner).Begin() for i := 0; i < count; i++ { - obj, err := d.ReadObject() + obj, err := d.DecodeObject() if err != nil { return err } @@ -144,8 +172,10 @@ func (d *Decoder) readObjectsWithObjectStorerTx(count int) error { return d.tx.Commit() } -// ReadObject reads a object from the stream and return it -func (d *Decoder) ReadObject() (plumbing.Object, error) { +// DecodeObject reads the next object from the scanner and returns it. This +// method can be used in replacement of the Decode method, to work in a +// interative way +func (d *Decoder) DecodeObject() (plumbing.Object, error) { h, err := d.s.NextObjectHeader() if err != nil { return nil, err @@ -185,8 +215,9 @@ func (d *Decoder) newObject() plumbing.Object { return d.o.NewObject() } -// ReadObjectAt reads an object at the given location -func (d *Decoder) ReadObjectAt(offset int64) (plumbing.Object, error) { +// DecodeObjectAt reads an object at the given location, if Decode wasn't called +// previously objects offset should provided using the SetOffsets method +func (d *Decoder) DecodeObjectAt(offset int64) (plumbing.Object, error) { if !d.s.IsSeekable { return nil, ErrNonSeekable } @@ -203,7 +234,7 @@ func (d *Decoder) ReadObjectAt(offset int64) (plumbing.Object, error) { } }() - return d.ReadObject() + return d.DecodeObject() } func (d *Decoder) fillRegularObjectContent(obj plumbing.Object) (uint32, error) { @@ -259,11 +290,11 @@ func (d *Decoder) setCRC(h plumbing.Hash, crc uint32) { func (d *Decoder) recallByOffset(o int64) (plumbing.Object, error) { if d.s.IsSeekable { - return d.ReadObjectAt(o) + return d.DecodeObjectAt(o) } if h, ok := d.offsetToHash[o]; ok { - return d.tx.Object(plumbing.AnyObject, h) + return d.recallByHashNonSeekable(h) } return nil, plumbing.ErrObjectNotFound @@ -272,11 +303,22 @@ func (d *Decoder) recallByOffset(o int64) (plumbing.Object, error) { func (d *Decoder) recallByHash(h plumbing.Hash) (plumbing.Object, error) { if d.s.IsSeekable { if o, ok := d.hashToOffset[h]; ok { - return d.ReadObjectAt(o) + return d.DecodeObjectAt(o) } } - obj, err := d.tx.Object(plumbing.AnyObject, h) + return d.recallByHashNonSeekable(h) +} + +// recallByHashNonSeekable if we are in a transaction the objects are read from +// the transaction, if not are directly read from the ObjectStorer +func (d *Decoder) recallByHashNonSeekable(h plumbing.Hash) (obj plumbing.Object, err error) { + if d.tx != nil { + obj, err = d.tx.Object(plumbing.AnyObject, h) + } else { + obj, err = d.o.Object(plumbing.AnyObject, h) + } + if err != plumbing.ErrObjectNotFound { return obj, err } @@ -284,18 +326,20 @@ func (d *Decoder) recallByHash(h plumbing.Hash) (plumbing.Object, error) { return nil, plumbing.ErrObjectNotFound } -// SetOffsets sets the offsets, required when using the method ReadObjectAt, +// SetOffsets sets the offsets, required when using the method DecodeObjectAt, // without decoding the full packfile func (d *Decoder) SetOffsets(offsets map[plumbing.Hash]int64) { d.hashToOffset = offsets } -// Offsets returns the objects read offset +// Offsets returns the objects read offset, Decode method should be called +// before to calculate the Offsets func (d *Decoder) Offsets() map[plumbing.Hash]int64 { return d.hashToOffset } -// CRCs returns the CRC-32 for each objected read +// CRCs returns the CRC-32 for each objected read,Decode method should be called +// before to calculate the CRCs func (d *Decoder) CRCs() map[plumbing.Hash]uint32 { return d.crcs } diff --git a/plumbing/format/packfile/decoder_test.go b/plumbing/format/packfile/decoder_test.go index 936f589..865cdae 100644 --- a/plumbing/format/packfile/decoder_test.go +++ b/plumbing/format/packfile/decoder_test.go @@ -1,4 +1,4 @@ -package packfile +package packfile_test import ( "io" @@ -7,7 +7,11 @@ import ( "gopkg.in/src-d/go-git.v4/fixtures" "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" + "gopkg.in/src-d/go-git.v4/plumbing/storer" + "gopkg.in/src-d/go-git.v4/storage/filesystem" "gopkg.in/src-d/go-git.v4/storage/memory" + fs "gopkg.in/src-d/go-git.v4/utils/fs/memory" . "gopkg.in/check.v1" ) @@ -21,8 +25,8 @@ type ReaderSuite struct { var _ = Suite(&ReaderSuite{}) func (s *ReaderSuite) TestNewDecodeNonSeekable(c *C) { - scanner := NewScanner(nil) - d, err := NewDecoder(scanner, nil) + scanner := packfile.NewScanner(nil) + d, err := packfile.NewDecoder(scanner, nil) c.Assert(d, IsNil) c.Assert(err, NotNil) @@ -30,10 +34,10 @@ func (s *ReaderSuite) TestNewDecodeNonSeekable(c *C) { func (s *ReaderSuite) TestDecode(c *C) { fixtures.Basic().ByTag("packfile").Test(c, func(f *fixtures.Fixture) { - scanner := NewScanner(f.Packfile()) + scanner := packfile.NewScanner(f.Packfile()) storage := memory.NewStorage() - d, err := NewDecoder(scanner, storage) + d, err := packfile.NewDecoder(scanner, storage) c.Assert(err, IsNil) defer d.Close() @@ -45,10 +49,28 @@ func (s *ReaderSuite) TestDecode(c *C) { }) } +func (s *ReaderSuite) TestDecodeMultipleTimes(c *C) { + f := fixtures.Basic().ByTag("packfile").One() + scanner := packfile.NewScanner(f.Packfile()) + storage := memory.NewStorage() + + d, err := packfile.NewDecoder(scanner, storage) + c.Assert(err, IsNil) + defer d.Close() + + ch, err := d.Decode() + c.Assert(err, IsNil) + c.Assert(ch, Equals, f.PackfileHash) + + ch, err = d.Decode() + c.Assert(err, Equals, packfile.ErrAlreadyDecoded) + c.Assert(ch, Equals, plumbing.ZeroHash) +} + func (s *ReaderSuite) TestDecodeInMemory(c *C) { fixtures.Basic().ByTag("packfile").Test(c, func(f *fixtures.Fixture) { - scanner := NewScanner(f.Packfile()) - d, err := NewDecoder(scanner, nil) + scanner := packfile.NewScanner(f.Packfile()) + d, err := packfile.NewDecoder(scanner, nil) c.Assert(err, IsNil) ch, err := d.Decode() @@ -65,15 +87,44 @@ func (nsr nonSeekableReader) Read(b []byte) (int, error) { return nsr.r.Read(b) } -func (s *ReaderSuite) TestDecodeNoSeekable(c *C) { +func (s *ReaderSuite) TestDecodeNoSeekableWithTxStorer(c *C) { fixtures.Basic().ByTag("packfile").Test(c, func(f *fixtures.Fixture) { reader := nonSeekableReader{ r: f.Packfile(), } - scanner := NewScanner(reader) - storage := memory.NewStorage() - d, err := NewDecoder(scanner, storage) + scanner := packfile.NewScanner(reader) + + var storage storer.ObjectStorer = memory.NewStorage() + _, isTxStorer := storage.(storer.Transactioner) + c.Assert(isTxStorer, Equals, true) + + d, err := packfile.NewDecoder(scanner, storage) + c.Assert(err, IsNil) + defer d.Close() + + ch, err := d.Decode() + c.Assert(err, IsNil) + c.Assert(ch, Equals, f.PackfileHash) + + assertObjects(c, storage, expectedHashes) + }) +} + +func (s *ReaderSuite) TestDecodeNoSeekableWithoutTxStorer(c *C) { + fixtures.Basic().ByTag("packfile").Test(c, func(f *fixtures.Fixture) { + reader := nonSeekableReader{ + r: f.Packfile(), + } + + scanner := packfile.NewScanner(reader) + + var storage storer.ObjectStorer + storage, _ = filesystem.NewStorage(fs.New()) + _, isTxStorer := storage.(storer.Transactioner) + c.Assert(isTxStorer, Equals, false) + + d, err := packfile.NewDecoder(scanner, storage) c.Assert(err, IsNil) defer d.Close() @@ -122,10 +173,10 @@ var expectedHashes = []string{ func (s *ReaderSuite) TestDecodeCRCs(c *C) { f := fixtures.Basic().ByTag("ofs-delta").One() - scanner := NewScanner(f.Packfile()) + scanner := packfile.NewScanner(f.Packfile()) storage := memory.NewStorage() - d, err := NewDecoder(scanner, storage) + d, err := packfile.NewDecoder(scanner, storage) c.Assert(err, IsNil) _, err = d.Decode() c.Assert(err, IsNil) @@ -140,8 +191,8 @@ func (s *ReaderSuite) TestDecodeCRCs(c *C) { func (s *ReaderSuite) TestReadObjectAt(c *C) { f := fixtures.Basic().One() - scanner := NewScanner(f.Packfile()) - d, err := NewDecoder(scanner, nil) + scanner := packfile.NewScanner(f.Packfile()) + d, err := packfile.NewDecoder(scanner, nil) c.Assert(err, IsNil) // when the packfile is ref-delta based, the offsets are required @@ -152,15 +203,15 @@ func (s *ReaderSuite) TestReadObjectAt(c *C) { // the objects at reference 186, is a delta, so should be recall, // without being read before. - obj, err := d.ReadObjectAt(186) + obj, err := d.DecodeObjectAt(186) c.Assert(err, IsNil) c.Assert(obj.Hash().String(), Equals, "6ecf0ef2c2dffb796033e5a02219af86ec6584e5") } func (s *ReaderSuite) TestOffsets(c *C) { f := fixtures.Basic().One() - scanner := NewScanner(f.Packfile()) - d, err := NewDecoder(scanner, nil) + scanner := packfile.NewScanner(f.Packfile()) + d, err := packfile.NewDecoder(scanner, nil) c.Assert(err, IsNil) c.Assert(d.Offsets(), HasLen, 0) @@ -173,8 +224,8 @@ func (s *ReaderSuite) TestOffsets(c *C) { func (s *ReaderSuite) TestSetOffsets(c *C) { f := fixtures.Basic().One() - scanner := NewScanner(f.Packfile()) - d, err := NewDecoder(scanner, nil) + scanner := packfile.NewScanner(f.Packfile()) + d, err := packfile.NewDecoder(scanner, nil) c.Assert(err, IsNil) h := plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5") @@ -185,8 +236,16 @@ func (s *ReaderSuite) TestSetOffsets(c *C) { c.Assert(o[h], Equals, int64(42)) } -func assertObjects(c *C, s *memory.Storage, expects []string) { - c.Assert(len(expects), Equals, len(s.Objects)) +func assertObjects(c *C, s storer.ObjectStorer, expects []string) { + + i, err := s.IterObjects(plumbing.AnyObject) + c.Assert(err, IsNil) + + var count int + err = i.ForEach(func(plumbing.Object) error { count++; return nil }) + c.Assert(err, IsNil) + c.Assert(count, Equals, len(expects)) + for _, exp := range expects { obt, err := s.Object(plumbing.AnyObject, plumbing.NewHash(exp)) c.Assert(err, IsNil) -- cgit