From 11735c3b3aaa8f789dc10739a4de7ad438196000 Mon Sep 17 00:00:00 2001 From: Máximo Cuadros Date: Mon, 5 Dec 2016 15:44:50 +0100 Subject: plumbing/protocol: paktp avoid duplication of haves, wants and shallow (#158) --- plumbing/protocol/packp/ulreq_encode.go | 51 +++++++++------- plumbing/protocol/packp/ulreq_encode_test.go | 90 ++++++++++++++++++++++------ plumbing/protocol/packp/uppackreq.go | 11 ++++ plumbing/protocol/packp/uppackreq_test.go | 27 +++++++++ 4 files changed, 137 insertions(+), 42 deletions(-) (limited to 'plumbing/protocol') diff --git a/plumbing/protocol/packp/ulreq_encode.go b/plumbing/protocol/packp/ulreq_encode.go index b2ca491..4a26e74 100644 --- a/plumbing/protocol/packp/ulreq_encode.go +++ b/plumbing/protocol/packp/ulreq_encode.go @@ -1,9 +1,9 @@ package packp import ( + "bytes" "fmt" "io" - "sort" "time" "gopkg.in/src-d/go-git.v4/plumbing" @@ -21,10 +21,9 @@ func (u *UploadRequest) Encode(w io.Writer) error { } type ulReqEncoder struct { - pe *pktline.Encoder // where to write the encoded data - data *UploadRequest // the data to encode - sortedWants []string - err error // sticky error + pe *pktline.Encoder // where to write the encoded data + data *UploadRequest // the data to encode + err error // sticky error } func newUlReqEncoder(w io.Writer) *ulReqEncoder { @@ -34,13 +33,13 @@ func newUlReqEncoder(w io.Writer) *ulReqEncoder { } func (e *ulReqEncoder) Encode(v *UploadRequest) error { + e.data = v + if len(v.Wants) == 0 { return fmt.Errorf("empty wants provided") } - e.data = v - e.sortedWants = sortHashes(v.Wants) - + plumbing.HashesSort(e.data.Wants) for state := e.encodeFirstWant; state != nil; { state = state() } @@ -48,27 +47,18 @@ func (e *ulReqEncoder) Encode(v *UploadRequest) error { return e.err } -func sortHashes(list []plumbing.Hash) []string { - sorted := make([]string, len(list)) - for i, hash := range list { - sorted[i] = hash.String() - } - sort.Strings(sorted) - - return sorted -} - func (e *ulReqEncoder) encodeFirstWant() stateFn { var err error if e.data.Capabilities.IsEmpty() { - err = e.pe.Encodef("want %s\n", e.sortedWants[0]) + err = e.pe.Encodef("want %s\n", e.data.Wants[0]) } else { err = e.pe.Encodef( "want %s %s\n", - e.sortedWants[0], + e.data.Wants[0], e.data.Capabilities.String(), ) } + if err != nil { e.err = fmt.Errorf("encoding first want line: %s", err) return nil @@ -78,23 +68,38 @@ func (e *ulReqEncoder) encodeFirstWant() stateFn { } func (e *ulReqEncoder) encodeAditionalWants() stateFn { - for _, w := range e.sortedWants[1:] { + last := e.data.Wants[0] + for _, w := range e.data.Wants[1:] { + if bytes.Compare(last[:], w[:]) == 0 { + continue + } + if err := e.pe.Encodef("want %s\n", w); err != nil { e.err = fmt.Errorf("encoding want %q: %s", w, err) return nil } + + last = w } return e.encodeShallows } func (e *ulReqEncoder) encodeShallows() stateFn { - sorted := sortHashes(e.data.Shallows) - for _, s := range sorted { + plumbing.HashesSort(e.data.Shallows) + + var last plumbing.Hash + for _, s := range e.data.Shallows { + if bytes.Compare(last[:], s[:]) == 0 { + continue + } + if err := e.pe.Encodef("shallow %s\n", s); err != nil { e.err = fmt.Errorf("encoding shallow %q: %s", s, err) return nil } + + last = s } return e.encodeDepth diff --git a/plumbing/protocol/packp/ulreq_encode_test.go b/plumbing/protocol/packp/ulreq_encode_test.go index 3b3b6c2..b414a37 100644 --- a/plumbing/protocol/packp/ulreq_encode_test.go +++ b/plumbing/protocol/packp/ulreq_encode_test.go @@ -76,11 +76,13 @@ func (s *UlReqEncodeSuite) TestOneWantWithCapabilities(c *C) { func (s *UlReqEncodeSuite) TestWants(c *C) { ur := NewUploadRequest() - ur.Wants = append(ur.Wants, plumbing.NewHash("4444444444444444444444444444444444444444")) - ur.Wants = append(ur.Wants, plumbing.NewHash("1111111111111111111111111111111111111111")) - ur.Wants = append(ur.Wants, plumbing.NewHash("3333333333333333333333333333333333333333")) - ur.Wants = append(ur.Wants, plumbing.NewHash("2222222222222222222222222222222222222222")) - ur.Wants = append(ur.Wants, plumbing.NewHash("5555555555555555555555555555555555555555")) + ur.Wants = append(ur.Wants, + plumbing.NewHash("4444444444444444444444444444444444444444"), + plumbing.NewHash("1111111111111111111111111111111111111111"), + plumbing.NewHash("3333333333333333333333333333333333333333"), + plumbing.NewHash("2222222222222222222222222222222222222222"), + plumbing.NewHash("5555555555555555555555555555555555555555"), + ) expected := []string{ "want 1111111111111111111111111111111111111111\n", @@ -94,13 +96,37 @@ func (s *UlReqEncodeSuite) TestWants(c *C) { testUlReqEncode(c, ur, expected) } +func (s *UlReqEncodeSuite) TestWantsDuplicates(c *C) { + ur := NewUploadRequest() + ur.Wants = append(ur.Wants, + plumbing.NewHash("4444444444444444444444444444444444444444"), + plumbing.NewHash("1111111111111111111111111111111111111111"), + plumbing.NewHash("3333333333333333333333333333333333333333"), + plumbing.NewHash("1111111111111111111111111111111111111111"), + plumbing.NewHash("2222222222222222222222222222222222222222"), + plumbing.NewHash("1111111111111111111111111111111111111111"), + ) + + expected := []string{ + "want 1111111111111111111111111111111111111111\n", + "want 2222222222222222222222222222222222222222\n", + "want 3333333333333333333333333333333333333333\n", + "want 4444444444444444444444444444444444444444\n", + pktline.FlushString, + } + + testUlReqEncode(c, ur, expected) +} + func (s *UlReqEncodeSuite) TestWantsWithCapabilities(c *C) { ur := NewUploadRequest() - ur.Wants = append(ur.Wants, plumbing.NewHash("4444444444444444444444444444444444444444")) - ur.Wants = append(ur.Wants, plumbing.NewHash("1111111111111111111111111111111111111111")) - ur.Wants = append(ur.Wants, plumbing.NewHash("3333333333333333333333333333333333333333")) - ur.Wants = append(ur.Wants, plumbing.NewHash("2222222222222222222222222222222222222222")) - ur.Wants = append(ur.Wants, plumbing.NewHash("5555555555555555555555555555555555555555")) + ur.Wants = append(ur.Wants, + plumbing.NewHash("4444444444444444444444444444444444444444"), + plumbing.NewHash("1111111111111111111111111111111111111111"), + plumbing.NewHash("3333333333333333333333333333333333333333"), + plumbing.NewHash("2222222222222222222222222222222222222222"), + plumbing.NewHash("5555555555555555555555555555555555555555"), + ) ur.Capabilities.Add(capability.MultiACK) ur.Capabilities.Add(capability.OFSDelta) @@ -139,10 +165,12 @@ func (s *UlReqEncodeSuite) TestManyShallows(c *C) { ur := NewUploadRequest() ur.Wants = append(ur.Wants, plumbing.NewHash("1111111111111111111111111111111111111111")) ur.Capabilities.Add(capability.MultiACK) - ur.Shallows = append(ur.Shallows, plumbing.NewHash("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb")) - ur.Shallows = append(ur.Shallows, plumbing.NewHash("dddddddddddddddddddddddddddddddddddddddd")) - ur.Shallows = append(ur.Shallows, plumbing.NewHash("cccccccccccccccccccccccccccccccccccccccc")) - ur.Shallows = append(ur.Shallows, plumbing.NewHash("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")) + ur.Shallows = append(ur.Shallows, + plumbing.NewHash("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"), + plumbing.NewHash("dddddddddddddddddddddddddddddddddddddddd"), + plumbing.NewHash("cccccccccccccccccccccccccccccccccccccccc"), + plumbing.NewHash("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"), + ) expected := []string{ "want 1111111111111111111111111111111111111111 multi_ack\n", @@ -156,6 +184,28 @@ func (s *UlReqEncodeSuite) TestManyShallows(c *C) { testUlReqEncode(c, ur, expected) } +func (s *UlReqEncodeSuite) TestShallowsDuplicate(c *C) { + ur := NewUploadRequest() + ur.Wants = append(ur.Wants, plumbing.NewHash("1111111111111111111111111111111111111111")) + ur.Capabilities.Add(capability.MultiACK) + ur.Shallows = append(ur.Shallows, + plumbing.NewHash("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"), + plumbing.NewHash("cccccccccccccccccccccccccccccccccccccccc"), + plumbing.NewHash("cccccccccccccccccccccccccccccccccccccccc"), + plumbing.NewHash("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"), + ) + + expected := []string{ + "want 1111111111111111111111111111111111111111 multi_ack\n", + "shallow aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n", + "shallow bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb\n", + "shallow cccccccccccccccccccccccccccccccccccccccc\n", + pktline.FlushString, + } + + testUlReqEncode(c, ur, expected) +} + func (s *UlReqEncodeSuite) TestDepthCommits(c *C) { ur := NewUploadRequest() ur.Wants = append(ur.Wants, plumbing.NewHash("1111111111111111111111111111111111111111")) @@ -220,11 +270,13 @@ func (s *UlReqEncodeSuite) TestDepthReference(c *C) { func (s *UlReqEncodeSuite) TestAll(c *C) { ur := NewUploadRequest() - ur.Wants = append(ur.Wants, plumbing.NewHash("4444444444444444444444444444444444444444")) - ur.Wants = append(ur.Wants, plumbing.NewHash("1111111111111111111111111111111111111111")) - ur.Wants = append(ur.Wants, plumbing.NewHash("3333333333333333333333333333333333333333")) - ur.Wants = append(ur.Wants, plumbing.NewHash("2222222222222222222222222222222222222222")) - ur.Wants = append(ur.Wants, plumbing.NewHash("5555555555555555555555555555555555555555")) + ur.Wants = append(ur.Wants, + plumbing.NewHash("4444444444444444444444444444444444444444"), + plumbing.NewHash("1111111111111111111111111111111111111111"), + plumbing.NewHash("3333333333333333333333333333333333333333"), + plumbing.NewHash("2222222222222222222222222222222222222222"), + plumbing.NewHash("5555555555555555555555555555555555555555"), + ) ur.Capabilities.Add(capability.MultiACK) ur.Capabilities.Add(capability.OFSDelta) diff --git a/plumbing/protocol/packp/uppackreq.go b/plumbing/protocol/packp/uppackreq.go index 2b1cb84..887d27a 100644 --- a/plumbing/protocol/packp/uppackreq.go +++ b/plumbing/protocol/packp/uppackreq.go @@ -1,6 +1,7 @@ package packp import ( + "bytes" "fmt" "io" @@ -68,10 +69,20 @@ type UploadHaves struct { // Encode encodes the UploadHaves into the Writer. func (u *UploadHaves) Encode(w io.Writer) error { e := pktline.NewEncoder(w) + + plumbing.HashesSort(u.Haves) + + var last plumbing.Hash for _, have := range u.Haves { + if bytes.Compare(last[:], have[:]) == 0 { + continue + } + if err := e.Encodef("have %s\n", have); err != nil { return fmt.Errorf("sending haves for %q: %s", have, err) } + + last = have } if len(u.Haves) != 0 { diff --git a/plumbing/protocol/packp/uppackreq_test.go b/plumbing/protocol/packp/uppackreq_test.go index 75b75b4..e551f45 100644 --- a/plumbing/protocol/packp/uppackreq_test.go +++ b/plumbing/protocol/packp/uppackreq_test.go @@ -4,6 +4,8 @@ import ( "gopkg.in/src-d/go-git.v4/plumbing" "gopkg.in/src-d/go-git.v4/plumbing/protocol/packp/capability" + "bytes" + . "gopkg.in/check.v1" ) @@ -40,3 +42,28 @@ func (s *UploadPackRequestSuite) TestIsEmpty(c *C) { c.Assert(r.IsEmpty(), Equals, true) } + +type UploadHavesSuite struct{} + +var _ = Suite(&UploadHavesSuite{}) + +func (s *UploadHavesSuite) TestEncode(c *C) { + uh := &UploadHaves{} + uh.Haves = append(uh.Haves, + plumbing.NewHash("1111111111111111111111111111111111111111"), + plumbing.NewHash("3333333333333333333333333333333333333333"), + plumbing.NewHash("1111111111111111111111111111111111111111"), + plumbing.NewHash("2222222222222222222222222222222222222222"), + plumbing.NewHash("1111111111111111111111111111111111111111"), + ) + + buf := bytes.NewBuffer(nil) + err := uh.Encode(buf) + c.Assert(err, IsNil) + c.Assert(buf.String(), Equals, ""+ + "0032have 1111111111111111111111111111111111111111\n"+ + "0032have 2222222222222222222222222222222222222222\n"+ + "0032have 3333333333333333333333333333333333333333\n"+ + "0000", + ) +} -- cgit