From 1400b45d9d33fc7e07068c001f972e5116f54659 Mon Sep 17 00:00:00 2001 From: Andrey Smirnov Date: Tue, 25 Feb 2014 00:40:32 +0400 Subject: [PATCH] Fix PackageCollection.Update to detect conflicts and skip updates when it's not necessary. --- debian/package.go | 41 ++++++++++++++++++++++++++++++++++++----- debian/package_test.go | 40 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 75 insertions(+), 6 deletions(-) diff --git a/debian/package.go b/debian/package.go index a3e75f05..9feedc02 100644 --- a/debian/package.go +++ b/debian/package.go @@ -205,14 +205,18 @@ func (p *Package) Key() []byte { return []byte("P" + p.Architecture + " " + p.Name + " " + p.Version) } -// Encode does msgpack encoding of Package -func (p *Package) Encode() []byte { - var buf bytes.Buffer +// Internal buffer reused by all Package.Encode operations +var encodeBuf bytes.Buffer - encoder := codec.NewEncoder(&buf, &codec.MsgpackHandle{}) +// Encode does msgpack encoding of Package, []byte should be copied, as buffer would +// be used for the next call to Encode +func (p *Package) Encode() []byte { + encodeBuf.Reset() + + encoder := codec.NewEncoder(&encodeBuf, &codec.MsgpackHandle{}) encoder.Encode(p) - return buf.Bytes() + return encodeBuf.Bytes() } // Decode decodes msgpack representation into Package @@ -499,6 +503,33 @@ func (collection *PackageCollection) ByKey(key []byte) (*Package, error) { // Update adds or updates information about package in DB func (collection *PackageCollection) Update(p *Package) error { + existing, err := collection.ByKey(p.Key()) + if err == nil { + // check for conflict + if existing.Equals(p) { + // packages are the same, no need to update + return nil + } + + // if .Files is different, consider to be conflict + if len(p.Files) != len(existing.Files) { + return fmt.Errorf("unable to save: %s, conflict with existing packge", p) + } + + for i, f := range p.Files { + if existing.Files[i] != f { + return fmt.Errorf("unable to save: %s, conflict with existing packge", p) + } + } + + // ok, .Files are the same, but some meta-data is different, proceed to saving + } else { + if err != database.ErrNotFound { + return err + } + // ok, package doesn't exist yet + } + return collection.db.Put(p.Key(), p.Encode()) } diff --git a/debian/package_test.go b/debian/package_test.go index 6424e99a..b1c3545e 100644 --- a/debian/package_test.go +++ b/debian/package_test.go @@ -306,7 +306,45 @@ func (s *PackageCollectionSuite) TearDownTest(c *C) { s.db.Close() } -func (s *PackageCollectionSuite) TestUpdateByKey(c *C) { +func (s *PackageCollectionSuite) TestUpdate(c *C) { + // package doesn't exist, update ok + err := s.collection.Update(s.p) + c.Assert(err, IsNil) + res, err := s.collection.ByKey(s.p.Key()) + c.Assert(err, IsNil) + c.Assert(res.Equals(s.p), Equals, true) + + // same package, ok + p2 := NewPackageFromControlFile(packageStanza.Copy()) + err = s.collection.Update(p2) + c.Assert(err, IsNil) + res, err = s.collection.ByKey(p2.Key()) + c.Assert(err, IsNil) + c.Assert(res.Equals(s.p), Equals, true) + + // change some metadata + p2.Source = "lala" + err = s.collection.Update(p2) + c.Assert(err, IsNil) + res, err = s.collection.ByKey(p2.Key()) + c.Assert(err, IsNil) + c.Assert(res.Equals(s.p), Equals, false) + c.Assert(res.Equals(p2), Equals, true) + + // change file info + p2 = NewPackageFromControlFile(packageStanza.Copy()) + p2.Files = nil + res, err = s.collection.ByKey(p2.Key()) + err = s.collection.Update(p2) + c.Assert(err, ErrorMatches, ".*conflict with existing packge") + p2 = NewPackageFromControlFile(packageStanza.Copy()) + p2.Files[0].Checksums.MD5 = "abcdef" + res, err = s.collection.ByKey(p2.Key()) + err = s.collection.Update(p2) + c.Assert(err, ErrorMatches, ".*conflict with existing packge") +} + +func (s *PackageCollectionSuite) TestByKey(c *C) { err := s.collection.Update(s.p) c.Assert(err, IsNil)