From 59f179a0eb04d30a4520bd7fc6f66af8fdeebcf4 Mon Sep 17 00:00:00 2001 From: Chien-Chih Liao Date: Fri, 26 Jul 2019 15:18:38 -0700 Subject: [PATCH 1/7] Add method to RelayFrame to reveal Arg2 offset Add a method "Arg2EndOffset" to RelayFrame interface so that we could know how much space Arg2 is used. This is the first step to know if it's possible to add more methods in Relay code to allow Relayer to inject more headers or read the headers to decide which peer to forward to. NOTE: this is a breaking change to RelayFrame interface. Related to #745. Will use this change to know Arg2 usage. --- relay/relay.go | 3 +++ relay_messages.go | 12 ++++++++++++ relay_messages_test.go | 19 +++++++++++++++++++ testutils/call.go | 6 ++++++ 4 files changed, 40 insertions(+) diff --git a/relay/relay.go b/relay/relay.go index 1ac0375e..567101b2 100644 --- a/relay/relay.go +++ b/relay/relay.go @@ -37,6 +37,9 @@ type CallFrame interface { // RoutingKey may refer to an alternate traffic group instead of the // traffic group identified by the service name. RoutingKey() []byte + // Arg2EndOffset returns the offset from start of frame to the end of Arg2 + // in bytes. + Arg2EndOffset() int } // Conn contains information about the underlying connection. diff --git a/relay_messages.go b/relay_messages.go index e990cb55..dbb3d216 100644 --- a/relay_messages.go +++ b/relay_messages.go @@ -88,6 +88,8 @@ type lazyCallReq struct { *Frame caller, method, delegate, key []byte + + arg2EndOffset int } // TODO: Consider pooling lazyCallReq and using pointers to the struct. @@ -132,6 +134,10 @@ func newLazyCallReq(f *Frame) lazyCallReq { arg1Len := int(binary.BigEndian.Uint16(f.Payload[cur : cur+2])) cur += 2 cr.method = f.Payload[cur : cur+arg1Len] + + // arg2~2 + cur += arg1Len + cr.arg2EndOffset = cur + 2 /*arg2 len*/ + int(binary.BigEndian.Uint16(f.Payload[cur:cur+2])) return cr } @@ -183,6 +189,12 @@ func (f lazyCallReq) HasMoreFragments() bool { return f.Payload[_flagsIndex]&hasMoreFragmentsFlag != 0 } +// Arg2EndOffset returns the offset from start of frame to the end of Arg2 +// in bytes. +func (f lazyCallReq) Arg2EndOffset() int { + return f.arg2EndOffset +} + // finishesCall checks whether this frame is the last one we should expect for // this RPC req-res. func finishesCall(f *Frame) bool { diff --git a/relay_messages_test.go b/relay_messages_test.go index 64c52f7c..f468c3ef 100644 --- a/relay_messages_test.go +++ b/relay_messages_test.go @@ -79,9 +79,18 @@ func (cr testCallReq) req() lazyCallReq { payload.WriteUint32(0) // checksum contents } payload.WriteLen16String("moneys") // method + + // add Arg2 into frame + arg2Buf := buildArg2Buffer() + payload.WriteUint16(uint16(len(arg2Buf))) + payload.WriteBytes(arg2Buf) return newLazyCallReq(f) } +func buildArg2Buffer() []byte { + return []byte("test arg2 buf") +} + func withLazyCallReqCombinations(f func(cr testCallReq)) { for cr := testCallReq(0); cr < reqTotalCombinations; cr++ { f(cr) @@ -272,6 +281,16 @@ func TestLazyCallReqSetTTL(t *testing.T) { }) } +func TestLazyCallArg2EndOffset(t *testing.T) { + wantArg2Buf := buildArg2Buffer() + withLazyCallReqCombinations(func(crt testCallReq) { + cr := crt.req() + arg2EndOffset := cr.Arg2EndOffset() + arg2Payload := cr.Payload[arg2EndOffset-len(wantArg2Buf) : arg2EndOffset] + assert.Equal(t, wantArg2Buf, arg2Payload) + }) +} + func TestLazyCallResRejectsOtherFrames(t *testing.T) { assertWrappingPanics( t, diff --git a/testutils/call.go b/testutils/call.go index 9a3cec57..42d37a27 100644 --- a/testutils/call.go +++ b/testutils/call.go @@ -94,6 +94,7 @@ func NewIncomingCall(callerName string) tchannel.IncomingCall { // FakeCallFrame is a stub implementation of the CallFrame interface. type FakeCallFrame struct { ServiceF, MethodF, CallerF, RoutingKeyF, RoutingDelegateF string + Arg2EndOffsetVal int } var _ relay.CallFrame = FakeCallFrame{} @@ -122,3 +123,8 @@ func (f FakeCallFrame) RoutingKey() []byte { func (f FakeCallFrame) RoutingDelegate() []byte { return []byte(f.RoutingDelegateF) } + +// Arg2EndOffset returns the offset from start of frame to the end of Arg2. +func (f FakeCallFrame) Arg2EndOffset() int { + return f.Arg2EndOffsetVal +} From 8f74f33ffc6292eb23c97f0b0725692c1775d068 Mon Sep 17 00:00:00 2001 From: Chien-Chih Liao Date: Mon, 29 Jul 2019 11:47:51 -0700 Subject: [PATCH 2/7] CR: add Arg2StartOffset method; add isFragmented flag; improve tests --- relay/relay.go | 10 +++++--- relay_messages.go | 18 ++++++++++--- relay_messages_test.go | 58 +++++++++++++++++++++++++++++++++--------- testutils/call.go | 14 +++++++--- 4 files changed, 77 insertions(+), 23 deletions(-) diff --git a/relay/relay.go b/relay/relay.go index 567101b2..fe32abbf 100644 --- a/relay/relay.go +++ b/relay/relay.go @@ -37,9 +37,13 @@ type CallFrame interface { // RoutingKey may refer to an alternate traffic group instead of the // traffic group identified by the service name. RoutingKey() []byte - // Arg2EndOffset returns the offset from start of frame to the end of Arg2 - // in bytes. - Arg2EndOffset() int + // Arg2StartOffset returns the offset from start of frame to the + // beginning of Arg2 in bytes. + Arg2StartOffset() int + // Arg2EndOffset returns the offset from start of frame to the end of + // Arg2 in bytes, and isFragmented to indicate if Arg2 is fragmented + // (i.e. spans multiple frames). + Arg2EndOffset() (_ int, isFragmented bool) } // Conn contains information about the underlying connection. diff --git a/relay_messages.go b/relay_messages.go index dbb3d216..88fac811 100644 --- a/relay_messages.go +++ b/relay_messages.go @@ -89,7 +89,8 @@ type lazyCallReq struct { caller, method, delegate, key []byte - arg2EndOffset int + arg2StartOffset, arg2EndOffset int + isArg2Fragmented bool } // TODO: Consider pooling lazyCallReq and using pointers to the struct. @@ -137,7 +138,10 @@ func newLazyCallReq(f *Frame) lazyCallReq { // arg2~2 cur += arg1Len + cr.arg2StartOffset = cur + 2 cr.arg2EndOffset = cur + 2 /*arg2 len*/ + int(binary.BigEndian.Uint16(f.Payload[cur:cur+2])) + // arg2 is fragmented if we don't see arg3 in this frame. + cr.isArg2Fragmented = int(cr.Header.FrameSize()) <= (FrameHeaderSize+cr.arg2EndOffset) && f.Payload[_flagsIndex]&hasMoreFragmentsFlag != 0 return cr } @@ -190,9 +194,15 @@ func (f lazyCallReq) HasMoreFragments() bool { } // Arg2EndOffset returns the offset from start of frame to the end of Arg2 -// in bytes. -func (f lazyCallReq) Arg2EndOffset() int { - return f.arg2EndOffset +// in bytes, and whether arg2 is fragmented or not. +func (f lazyCallReq) Arg2EndOffset() (_ int, isFragmented bool) { + return f.arg2EndOffset, f.isArg2Fragmented +} + +// Arg2StartOffset returns the offset from start of frame to the beginning +// of Arg2 in bytes. +func (f lazyCallReq) Arg2StartOffset() int { + return f.arg2StartOffset } // finishesCall checks whether this frame is the last one we should expect for diff --git a/relay_messages_test.go b/relay_messages_test.go index f468c3ef..c4334e03 100644 --- a/relay_messages_test.go +++ b/relay_messages_test.go @@ -41,17 +41,27 @@ const ( reqHasAll testCallReq = reqTotalCombinations - 1 ) +type testCallReqParams struct { + flags byte + arg2Buf []byte +} + func (cr testCallReq) req() lazyCallReq { + return cr.reqWithParams(testCallReqParams{}) +} + +func (cr testCallReq) reqWithParams(p testCallReqParams) lazyCallReq { // TODO: Constructing a frame is ugly because the initial flags byte is // written in reqResWriter instead of callReq. We should instead handle that // in callReq, which will allow our tests to be sane. f := NewFrame(200) fh := fakeHeader() + fh.size = 0xD8 // 200 + 16 bytes of header = 216 (0xD8) f.Header = fh fh.write(typed.NewWriteBuffer(f.headerBuffer)) payload := typed.NewWriteBuffer(f.Payload) - payload.WriteSingleByte(0) // flags + payload.WriteSingleByte(p.flags) // flags payload.WriteUint32(42) // TTL payload.WriteBytes(make([]byte, 25)) // tracing payload.WriteLen8String("bankmoji") // service @@ -80,10 +90,8 @@ func (cr testCallReq) req() lazyCallReq { } payload.WriteLen16String("moneys") // method - // add Arg2 into frame - arg2Buf := buildArg2Buffer() - payload.WriteUint16(uint16(len(arg2Buf))) - payload.WriteBytes(arg2Buf) + payload.WriteUint16(uint16(len(p.arg2Buf))) + payload.WriteBytes(p.arg2Buf) return newLazyCallReq(f) } @@ -281,13 +289,39 @@ func TestLazyCallReqSetTTL(t *testing.T) { }) } -func TestLazyCallArg2EndOffset(t *testing.T) { - wantArg2Buf := buildArg2Buffer() - withLazyCallReqCombinations(func(crt testCallReq) { - cr := crt.req() - arg2EndOffset := cr.Arg2EndOffset() - arg2Payload := cr.Payload[arg2EndOffset-len(wantArg2Buf) : arg2EndOffset] - assert.Equal(t, wantArg2Buf, arg2Payload) +func TestLazyCallArg2Offset(t *testing.T) { + t.Run("arg2 is fully contained in frame", func(t *testing.T) { + wantArg2Buf := buildArg2Buffer() + withLazyCallReqCombinations(func(crt testCallReq) { + cr := crt.reqWithParams(testCallReqParams{arg2Buf: wantArg2Buf}) + arg2EndOffset, isFragmented := cr.Arg2EndOffset() + assert.False(t, isFragmented) + arg2Payload := cr.Payload[cr.Arg2StartOffset():arg2EndOffset] + assert.Equal(t, wantArg2Buf, arg2Payload) + }) + }) + t.Run("has no arg2", func(t *testing.T) { + withLazyCallReqCombinations(func(crt testCallReq) { + cr := crt.req() + endOffset, isFragmented := cr.Arg2EndOffset() + assert.False(t, isFragmented) + assert.Zero(t, endOffset-cr.Arg2StartOffset()) + }) + }) + t.Run("arg2 has been fragmented", func(t *testing.T) { + withLazyCallReqCombinations(func(crt testCallReq) { + // For each CallReq, we first get the remaining space left, and + // fill up the remaining space with arg2. + crNoArg2 := crt.req() + arg2Size := int(crNoArg2.Header.FrameSize()) - crNoArg2.Arg2StartOffset() + cr := crt.reqWithParams(testCallReqParams{ + flags: hasMoreFragmentsFlag, + arg2Buf: make([]byte, arg2Size), + }) + endOffset, isFragmented := cr.Arg2EndOffset() + assert.True(t, isFragmented) + assert.EqualValues(t, crNoArg2.Header.FrameSize(), endOffset) + }) }) } diff --git a/testutils/call.go b/testutils/call.go index 42d37a27..a69b37cd 100644 --- a/testutils/call.go +++ b/testutils/call.go @@ -94,7 +94,6 @@ func NewIncomingCall(callerName string) tchannel.IncomingCall { // FakeCallFrame is a stub implementation of the CallFrame interface. type FakeCallFrame struct { ServiceF, MethodF, CallerF, RoutingKeyF, RoutingDelegateF string - Arg2EndOffsetVal int } var _ relay.CallFrame = FakeCallFrame{} @@ -124,7 +123,14 @@ func (f FakeCallFrame) RoutingDelegate() []byte { return []byte(f.RoutingDelegateF) } -// Arg2EndOffset returns the offset from start of frame to the end of Arg2. -func (f FakeCallFrame) Arg2EndOffset() int { - return f.Arg2EndOffsetVal +// Arg2StartOffset returns the offset from start of frame to +// the beginning of Arg2. +func (f FakeCallFrame) Arg2StartOffset() int { + return 0 +} + +// Arg2EndOffset returns the offset from start of frame to the end +// of Arg2 and whether Arg2 is fragmented. +func (f FakeCallFrame) Arg2EndOffset() (int, bool) { + return 0, false } From 538814970992f091b1019aac9663e8713c1315ba Mon Sep 17 00:00:00 2001 From: Chien-Chih Liao Date: Mon, 29 Jul 2019 12:00:29 -0700 Subject: [PATCH 3/7] s/isFragmented/hasMore/gc --- relay/relay.go | 6 +++--- relay_messages.go | 4 ++-- relay_messages_test.go | 12 ++++++------ 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/relay/relay.go b/relay/relay.go index fe32abbf..db133699 100644 --- a/relay/relay.go +++ b/relay/relay.go @@ -41,9 +41,9 @@ type CallFrame interface { // beginning of Arg2 in bytes. Arg2StartOffset() int // Arg2EndOffset returns the offset from start of frame to the end of - // Arg2 in bytes, and isFragmented to indicate if Arg2 is fragmented - // (i.e. spans multiple frames). - Arg2EndOffset() (_ int, isFragmented bool) + // Arg2 in bytes, and hasMore to indicate if there are mote data from + // other frames (i.e. arg2 is fragmented). + Arg2EndOffset() (_ int, hasMore bool) } // Conn contains information about the underlying connection. diff --git a/relay_messages.go b/relay_messages.go index 88fac811..fed29d20 100644 --- a/relay_messages.go +++ b/relay_messages.go @@ -194,8 +194,8 @@ func (f lazyCallReq) HasMoreFragments() bool { } // Arg2EndOffset returns the offset from start of frame to the end of Arg2 -// in bytes, and whether arg2 is fragmented or not. -func (f lazyCallReq) Arg2EndOffset() (_ int, isFragmented bool) { +// in bytes, and whether there are more data from other frames. +func (f lazyCallReq) Arg2EndOffset() (_ int, hasMore bool) { return f.arg2EndOffset, f.isArg2Fragmented } diff --git a/relay_messages_test.go b/relay_messages_test.go index c4334e03..9c4ce301 100644 --- a/relay_messages_test.go +++ b/relay_messages_test.go @@ -294,8 +294,8 @@ func TestLazyCallArg2Offset(t *testing.T) { wantArg2Buf := buildArg2Buffer() withLazyCallReqCombinations(func(crt testCallReq) { cr := crt.reqWithParams(testCallReqParams{arg2Buf: wantArg2Buf}) - arg2EndOffset, isFragmented := cr.Arg2EndOffset() - assert.False(t, isFragmented) + arg2EndOffset, hasMore := cr.Arg2EndOffset() + assert.False(t, hasMore) arg2Payload := cr.Payload[cr.Arg2StartOffset():arg2EndOffset] assert.Equal(t, wantArg2Buf, arg2Payload) }) @@ -303,8 +303,8 @@ func TestLazyCallArg2Offset(t *testing.T) { t.Run("has no arg2", func(t *testing.T) { withLazyCallReqCombinations(func(crt testCallReq) { cr := crt.req() - endOffset, isFragmented := cr.Arg2EndOffset() - assert.False(t, isFragmented) + endOffset, hasMore := cr.Arg2EndOffset() + assert.False(t, hasMore) assert.Zero(t, endOffset-cr.Arg2StartOffset()) }) }) @@ -318,8 +318,8 @@ func TestLazyCallArg2Offset(t *testing.T) { flags: hasMoreFragmentsFlag, arg2Buf: make([]byte, arg2Size), }) - endOffset, isFragmented := cr.Arg2EndOffset() - assert.True(t, isFragmented) + endOffset, hasMore := cr.Arg2EndOffset() + assert.True(t, hasMore) assert.EqualValues(t, crNoArg2.Header.FrameSize(), endOffset) }) }) From 071c6eaba95f96a0a77e8cf3c36b2b0395d6d4df Mon Sep 17 00:00:00 2001 From: Chien-Chih Liao Date: Mon, 29 Jul 2019 13:10:08 -0700 Subject: [PATCH 4/7] simplify test --- relay_messages_test.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/relay_messages_test.go b/relay_messages_test.go index 9c4ce301..3055ff8d 100644 --- a/relay_messages_test.go +++ b/relay_messages_test.go @@ -95,10 +95,6 @@ func (cr testCallReq) reqWithParams(p testCallReqParams) lazyCallReq { return newLazyCallReq(f) } -func buildArg2Buffer() []byte { - return []byte("test arg2 buf") -} - func withLazyCallReqCombinations(f func(cr testCallReq)) { for cr := testCallReq(0); cr < reqTotalCombinations; cr++ { f(cr) @@ -291,7 +287,7 @@ func TestLazyCallReqSetTTL(t *testing.T) { func TestLazyCallArg2Offset(t *testing.T) { t.Run("arg2 is fully contained in frame", func(t *testing.T) { - wantArg2Buf := buildArg2Buffer() + wantArg2Buf := []byte("test arg2 buf") withLazyCallReqCombinations(func(crt testCallReq) { cr := crt.reqWithParams(testCallReqParams{arg2Buf: wantArg2Buf}) arg2EndOffset, hasMore := cr.Arg2EndOffset() From 5256d8de252477869ef71d8603b98d12f820b3a6 Mon Sep 17 00:00:00 2001 From: Chien-Chih Liao Date: Mon, 29 Jul 2019 13:23:05 -0700 Subject: [PATCH 5/7] test: consolidate tests with table test --- relay_messages_test.go | 56 +++++++++++++++++++++++++++++------------- 1 file changed, 39 insertions(+), 17 deletions(-) diff --git a/relay_messages_test.go b/relay_messages_test.go index 3055ff8d..9017e922 100644 --- a/relay_messages_test.go +++ b/relay_messages_test.go @@ -286,24 +286,46 @@ func TestLazyCallReqSetTTL(t *testing.T) { } func TestLazyCallArg2Offset(t *testing.T) { - t.Run("arg2 is fully contained in frame", func(t *testing.T) { - wantArg2Buf := []byte("test arg2 buf") - withLazyCallReqCombinations(func(crt testCallReq) { - cr := crt.reqWithParams(testCallReqParams{arg2Buf: wantArg2Buf}) - arg2EndOffset, hasMore := cr.Arg2EndOffset() - assert.False(t, hasMore) - arg2Payload := cr.Payload[cr.Arg2StartOffset():arg2EndOffset] - assert.Equal(t, wantArg2Buf, arg2Payload) - }) - }) - t.Run("has no arg2", func(t *testing.T) { - withLazyCallReqCombinations(func(crt testCallReq) { - cr := crt.req() - endOffset, hasMore := cr.Arg2EndOffset() - assert.False(t, hasMore) - assert.Zero(t, endOffset-cr.Arg2StartOffset()) + wantArg2Buf := []byte("test arg2 buf") + tests := []struct { + msg string + flags byte + arg2Buf []byte + }{ + { + msg: "arg2 is fully contained in frame", + arg2Buf: wantArg2Buf, + }, + { + msg: "has no arg2", + }, + { + msg: "frame fragmented but arg2 is fully contained", + flags: hasMoreFragmentsFlag, + arg2Buf: wantArg2Buf, + }, + } + + for _, tt := range tests { + t.Run(tt.msg, func(t *testing.T) { + withLazyCallReqCombinations(func(crt testCallReq) { + cr := crt.reqWithParams(testCallReqParams{ + flags: tt.flags, + arg2Buf: tt.arg2Buf, + }) + arg2EndOffset, hasMore := cr.Arg2EndOffset() + assert.False(t, hasMore) + if len(tt.arg2Buf) == 0 { + assert.Zero(t, arg2EndOffset-cr.Arg2StartOffset()) + return + } + + arg2Payload := cr.Payload[cr.Arg2StartOffset():arg2EndOffset] + assert.Equal(t, tt.arg2Buf, arg2Payload) + }) }) - }) + } + t.Run("arg2 has been fragmented", func(t *testing.T) { withLazyCallReqCombinations(func(crt testCallReq) { // For each CallReq, we first get the remaining space left, and From aa6735afcee19931cd772262b532a89048e13871 Mon Sep 17 00:00:00 2001 From: Chien-Chih Liao Date: Tue, 30 Jul 2019 10:25:32 -0700 Subject: [PATCH 6/7] CR: simplify code; correct comment; update tests --- relay/relay.go | 4 ++-- relay_messages.go | 8 ++++---- relay_messages_test.go | 35 ++++++++++++++++++++++------------- testutils/call.go | 11 +++++++---- 4 files changed, 35 insertions(+), 23 deletions(-) diff --git a/relay/relay.go b/relay/relay.go index db133699..eaae1e7c 100644 --- a/relay/relay.go +++ b/relay/relay.go @@ -37,10 +37,10 @@ type CallFrame interface { // RoutingKey may refer to an alternate traffic group instead of the // traffic group identified by the service name. RoutingKey() []byte - // Arg2StartOffset returns the offset from start of frame to the + // Arg2StartOffset returns the offset from start of payload to the // beginning of Arg2 in bytes. Arg2StartOffset() int - // Arg2EndOffset returns the offset from start of frame to the end of + // Arg2EndOffset returns the offset from start of payload to the end of // Arg2 in bytes, and hasMore to indicate if there are mote data from // other frames (i.e. arg2 is fragmented). Arg2EndOffset() (_ int, hasMore bool) diff --git a/relay_messages.go b/relay_messages.go index fed29d20..b2be1194 100644 --- a/relay_messages.go +++ b/relay_messages.go @@ -139,9 +139,9 @@ func newLazyCallReq(f *Frame) lazyCallReq { // arg2~2 cur += arg1Len cr.arg2StartOffset = cur + 2 - cr.arg2EndOffset = cur + 2 /*arg2 len*/ + int(binary.BigEndian.Uint16(f.Payload[cur:cur+2])) + cr.arg2EndOffset = cr.arg2StartOffset + int(binary.BigEndian.Uint16(f.Payload[cur:cur+2])) // arg2 is fragmented if we don't see arg3 in this frame. - cr.isArg2Fragmented = int(cr.Header.FrameSize()) <= (FrameHeaderSize+cr.arg2EndOffset) && f.Payload[_flagsIndex]&hasMoreFragmentsFlag != 0 + cr.isArg2Fragmented = int(cr.Header.PayloadSize()) <= cr.arg2EndOffset && cr.HasMoreFragments() return cr } @@ -193,13 +193,13 @@ func (f lazyCallReq) HasMoreFragments() bool { return f.Payload[_flagsIndex]&hasMoreFragmentsFlag != 0 } -// Arg2EndOffset returns the offset from start of frame to the end of Arg2 +// Arg2EndOffset returns the offset from start of payload to the end of Arg2 // in bytes, and whether there are more data from other frames. func (f lazyCallReq) Arg2EndOffset() (_ int, hasMore bool) { return f.arg2EndOffset, f.isArg2Fragmented } -// Arg2StartOffset returns the offset from start of frame to the beginning +// Arg2StartOffset returns the offset from start of payload to the beginning // of Arg2 in bytes. func (f lazyCallReq) Arg2StartOffset() int { return f.arg2StartOffset diff --git a/relay_messages_test.go b/relay_messages_test.go index 9017e922..fd948d07 100644 --- a/relay_messages_test.go +++ b/relay_messages_test.go @@ -21,6 +21,7 @@ package tchannel import ( + "fmt" "testing" "time" @@ -326,20 +327,28 @@ func TestLazyCallArg2Offset(t *testing.T) { }) } - t.Run("arg2 has been fragmented", func(t *testing.T) { - withLazyCallReqCombinations(func(crt testCallReq) { - // For each CallReq, we first get the remaining space left, and - // fill up the remaining space with arg2. - crNoArg2 := crt.req() - arg2Size := int(crNoArg2.Header.FrameSize()) - crNoArg2.Arg2StartOffset() - cr := crt.reqWithParams(testCallReqParams{ - flags: hasMoreFragmentsFlag, - arg2Buf: make([]byte, arg2Size), + t.Run("no arg3 set", func(t *testing.T) { + for _, testHasMore := range []bool{true, false} { + t.Run(fmt.Sprintf("hasMore flag is set=%v", testHasMore), func(t *testing.T) { + withLazyCallReqCombinations(func(crt testCallReq) { + // For each CallReq, we first get the remaining space left, and + // fill up the remaining space with arg2. + crNoArg2 := crt.req() + arg2Size := int(crNoArg2.Header.PayloadSize()) - crNoArg2.Arg2StartOffset() + var flags byte + if testHasMore { + flags |= hasMoreFragmentsFlag + } + cr := crt.reqWithParams(testCallReqParams{ + flags: flags, + arg2Buf: make([]byte, arg2Size), + }) + endOffset, hasMore := cr.Arg2EndOffset() + assert.Equal(t, hasMore, testHasMore) + assert.EqualValues(t, crNoArg2.Header.PayloadSize(), endOffset) + }) }) - endOffset, hasMore := cr.Arg2EndOffset() - assert.True(t, hasMore) - assert.EqualValues(t, crNoArg2.Header.FrameSize(), endOffset) - }) + } }) } diff --git a/testutils/call.go b/testutils/call.go index a69b37cd..f2cd1823 100644 --- a/testutils/call.go +++ b/testutils/call.go @@ -94,6 +94,9 @@ func NewIncomingCall(callerName string) tchannel.IncomingCall { // FakeCallFrame is a stub implementation of the CallFrame interface. type FakeCallFrame struct { ServiceF, MethodF, CallerF, RoutingKeyF, RoutingDelegateF string + + Arg2StartOffsetVal, Arg2EndOffsetVal int + IsArg2Fragmented bool } var _ relay.CallFrame = FakeCallFrame{} @@ -123,14 +126,14 @@ func (f FakeCallFrame) RoutingDelegate() []byte { return []byte(f.RoutingDelegateF) } -// Arg2StartOffset returns the offset from start of frame to +// Arg2StartOffset returns the offset from start of payload to // the beginning of Arg2. func (f FakeCallFrame) Arg2StartOffset() int { - return 0 + return f.Arg2StartOffsetVal } -// Arg2EndOffset returns the offset from start of frame to the end +// Arg2EndOffset returns the offset from start of payload to the end // of Arg2 and whether Arg2 is fragmented. func (f FakeCallFrame) Arg2EndOffset() (int, bool) { - return 0, false + return f.Arg2EndOffsetVal, f.IsArg2Fragmented } From 023b52b15c95f891c1d90cfc5677e9c1b02309cb Mon Sep 17 00:00:00 2001 From: Chien-Chih Liao Date: Tue, 30 Jul 2019 16:44:12 -0700 Subject: [PATCH 7/7] CR: update comment --- relay/relay.go | 4 ++-- relay_messages.go | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/relay/relay.go b/relay/relay.go index eaae1e7c..da173172 100644 --- a/relay/relay.go +++ b/relay/relay.go @@ -41,8 +41,8 @@ type CallFrame interface { // beginning of Arg2 in bytes. Arg2StartOffset() int // Arg2EndOffset returns the offset from start of payload to the end of - // Arg2 in bytes, and hasMore to indicate if there are mote data from - // other frames (i.e. arg2 is fragmented). + // Arg2 in bytes, and hasMore to indicate if there are more frames and + // Arg3 has not started (i.e. Arg2 is fragmented). Arg2EndOffset() (_ int, hasMore bool) } diff --git a/relay_messages.go b/relay_messages.go index b2be1194..530fddd8 100644 --- a/relay_messages.go +++ b/relay_messages.go @@ -194,7 +194,8 @@ func (f lazyCallReq) HasMoreFragments() bool { } // Arg2EndOffset returns the offset from start of payload to the end of Arg2 -// in bytes, and whether there are more data from other frames. +// in bytes, and hasMore to be true if there are more frames and arg3 has +// not started. func (f lazyCallReq) Arg2EndOffset() (_ int, hasMore bool) { return f.arg2EndOffset, f.isArg2Fragmented }