From 085624be20d042b5994df51ca80fd734a48b7a6e Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Thu, 7 Dec 2017 18:19:11 +0700 Subject: [PATCH] replace stream.LenOfDataForWriting by HasDataForWriting The return value (the length of data for writing) was only used to determine if the stream has data for writing. Therefore it's easier to just return a bool. No functional change expected. --- internal/mocks/stream.go | 24 ++++++++++++------------ stream.go | 12 +++++------- stream_framer.go | 4 ++-- stream_framer_test.go | 28 ++++++++++++++-------------- stream_test.go | 14 +++++++------- 5 files changed, 40 insertions(+), 42 deletions(-) diff --git a/internal/mocks/stream.go b/internal/mocks/stream.go index 5592282c..9d5230d1 100644 --- a/internal/mocks/stream.go +++ b/internal/mocks/stream.go @@ -130,6 +130,18 @@ func (_mr *MockStreamIMockRecorder) GetWriteOffset() *gomock.Call { return _mr.mock.ctrl.RecordCallWithMethodType(_mr.mock, "GetWriteOffset", reflect.TypeOf((*MockStreamI)(nil).GetWriteOffset)) } +// HasDataForWriting mocks base method +func (_m *MockStreamI) HasDataForWriting() bool { + ret := _m.ctrl.Call(_m, "HasDataForWriting") + ret0, _ := ret[0].(bool) + return ret0 +} + +// HasDataForWriting indicates an expected call of HasDataForWriting +func (_mr *MockStreamIMockRecorder) HasDataForWriting() *gomock.Call { + return _mr.mock.ctrl.RecordCallWithMethodType(_mr.mock, "HasDataForWriting", reflect.TypeOf((*MockStreamI)(nil).HasDataForWriting)) +} + // IsFlowControlBlocked mocks base method func (_m *MockStreamI) IsFlowControlBlocked() bool { ret := _m.ctrl.Call(_m, "IsFlowControlBlocked") @@ -142,18 +154,6 @@ func (_mr *MockStreamIMockRecorder) IsFlowControlBlocked() *gomock.Call { return _mr.mock.ctrl.RecordCallWithMethodType(_mr.mock, "IsFlowControlBlocked", reflect.TypeOf((*MockStreamI)(nil).IsFlowControlBlocked)) } -// LenOfDataForWriting mocks base method -func (_m *MockStreamI) LenOfDataForWriting() protocol.ByteCount { - ret := _m.ctrl.Call(_m, "LenOfDataForWriting") - ret0, _ := ret[0].(protocol.ByteCount) - return ret0 -} - -// LenOfDataForWriting indicates an expected call of LenOfDataForWriting -func (_mr *MockStreamIMockRecorder) LenOfDataForWriting() *gomock.Call { - return _mr.mock.ctrl.RecordCallWithMethodType(_mr.mock, "LenOfDataForWriting", reflect.TypeOf((*MockStreamI)(nil).LenOfDataForWriting)) -} - // Read mocks base method func (_m *MockStreamI) Read(_param0 []byte) (int, error) { ret := _m.ctrl.Call(_m, "Read", _param0) diff --git a/stream.go b/stream.go index 3fe9a8d1..f8d9a629 100644 --- a/stream.go +++ b/stream.go @@ -19,7 +19,7 @@ type streamI interface { AddStreamFrame(*wire.StreamFrame) error RegisterRemoteError(error, protocol.ByteCount) error - LenOfDataForWriting() protocol.ByteCount + HasDataForWriting() bool GetDataForWriting(maxBytes protocol.ByteCount) []byte GetWriteOffset() protocol.ByteCount Finished() bool @@ -263,14 +263,12 @@ func (s *stream) GetWriteOffset() protocol.ByteCount { return s.writeOffset } -func (s *stream) LenOfDataForWriting() protocol.ByteCount { +// HasDataForWriting says if there's stream available to be dequeued for writing +func (s *stream) HasDataForWriting() bool { s.mutex.Lock() - var l protocol.ByteCount - if s.err == nil { - l = protocol.ByteCount(len(s.dataForWriting)) - } + hasData := s.err == nil && len(s.dataForWriting) > 0 s.mutex.Unlock() - return l + return hasData } func (s *stream) GetDataForWriting(maxBytes protocol.ByteCount) []byte { diff --git a/stream_framer.go b/stream_framer.go index e16a01e9..8e886cac 100644 --- a/stream_framer.go +++ b/stream_framer.go @@ -54,7 +54,7 @@ func (f *streamFramer) HasFramesForRetransmission() bool { } func (f *streamFramer) HasCryptoStreamFrame() bool { - return f.cryptoStream.LenOfDataForWriting() > 0 + return f.cryptoStream.HasDataForWriting() } // TODO(lclemente): This is somewhat duplicate with the normal path for generating frames. @@ -116,7 +116,7 @@ func (f *streamFramer) maybePopNormalFrames(maxBytes protocol.ByteCount) (res [] maxLen := maxBytes - currentLen - frameHeaderBytes var data []byte - if s.LenOfDataForWriting() > 0 { + if s.HasDataForWriting() { data = s.GetDataForWriting(maxLen) } diff --git a/stream_framer_test.go b/stream_framer_test.go index 84897b9b..97dcb11e 100644 --- a/stream_framer_test.go +++ b/stream_framer_test.go @@ -50,7 +50,7 @@ var _ = Describe("Stream Framer", func() { }) setNoData := func(str *mocks.MockStreamI) { - str.EXPECT().LenOfDataForWriting().Return(protocol.ByteCount(0)).AnyTimes() + str.EXPECT().HasDataForWriting().Return(false).AnyTimes() str.EXPECT().GetDataForWriting(gomock.Any()).Return(nil).AnyTimes() str.EXPECT().ShouldSendFin().Return(false).AnyTimes() str.EXPECT().GetWriteOffset().AnyTimes() @@ -75,7 +75,7 @@ var _ = Describe("Stream Framer", func() { connFC.EXPECT().IsBlocked() setNoData(stream2) stream1.EXPECT().GetWriteOffset() - stream1.EXPECT().LenOfDataForWriting().Return(protocol.ByteCount(8)) + stream1.EXPECT().HasDataForWriting().Return(true) stream1.EXPECT().GetDataForWriting(gomock.Any()).Return([]byte("foobar")) stream1.EXPECT().IsFlowControlBlocked() stream1.EXPECT().ShouldSendFin() @@ -112,7 +112,7 @@ var _ = Describe("Stream Framer", func() { It("returns normal frames", func() { stream1.EXPECT().GetDataForWriting(gomock.Any()).Return([]byte("foobar")) - stream1.EXPECT().LenOfDataForWriting().Return(protocol.ByteCount(6)) + stream1.EXPECT().HasDataForWriting().Return(true) stream1.EXPECT().GetWriteOffset() stream1.EXPECT().ShouldSendFin() setNoData(stream2) @@ -125,11 +125,11 @@ var _ = Describe("Stream Framer", func() { It("returns multiple normal frames", func() { stream1.EXPECT().GetDataForWriting(gomock.Any()).Return([]byte("foobar")) - stream1.EXPECT().LenOfDataForWriting().Return(protocol.ByteCount(6)) + stream1.EXPECT().HasDataForWriting().Return(true) stream1.EXPECT().GetWriteOffset() stream1.EXPECT().ShouldSendFin() stream2.EXPECT().GetDataForWriting(gomock.Any()).Return([]byte("foobaz")) - stream2.EXPECT().LenOfDataForWriting().Return(protocol.ByteCount(6)) + stream2.EXPECT().HasDataForWriting().Return(true) stream2.EXPECT().GetWriteOffset() stream2.EXPECT().ShouldSendFin() fs := framer.PopStreamFrames(1000) @@ -146,7 +146,7 @@ var _ = Describe("Stream Framer", func() { It("returns retransmission frames before normal frames", func() { stream1.EXPECT().GetDataForWriting(gomock.Any()).Return([]byte("foobar")) - stream1.EXPECT().LenOfDataForWriting().Return(protocol.ByteCount(6)) + stream1.EXPECT().HasDataForWriting().Return(true) stream1.EXPECT().GetWriteOffset() stream1.EXPECT().ShouldSendFin() setNoData(stream2) @@ -158,7 +158,7 @@ var _ = Describe("Stream Framer", func() { }) It("does not pop empty frames", func() { - stream1.EXPECT().LenOfDataForWriting().Return(protocol.ByteCount(0)) + stream1.EXPECT().HasDataForWriting().Return(false) stream1.EXPECT().ShouldSendFin() stream1.EXPECT().GetWriteOffset() setNoData(stream2) @@ -169,11 +169,11 @@ var _ = Describe("Stream Framer", func() { It("uses the round-robin scheduling", func() { streamFrameHeaderLen := protocol.ByteCount(4) stream1.EXPECT().GetDataForWriting(10 - streamFrameHeaderLen).Return(bytes.Repeat([]byte("f"), int(10-streamFrameHeaderLen))) - stream1.EXPECT().LenOfDataForWriting().Return(protocol.ByteCount(100)) + stream1.EXPECT().HasDataForWriting().Return(true) stream1.EXPECT().GetWriteOffset() stream1.EXPECT().ShouldSendFin() stream2.EXPECT().GetDataForWriting(protocol.ByteCount(10 - streamFrameHeaderLen)).Return(bytes.Repeat([]byte("e"), int(10-streamFrameHeaderLen))) - stream2.EXPECT().LenOfDataForWriting().Return(protocol.ByteCount(100)) + stream2.EXPECT().HasDataForWriting().Return(true) stream2.EXPECT().GetWriteOffset() stream2.EXPECT().ShouldSendFin() fs := framer.PopStreamFrames(10) @@ -279,7 +279,7 @@ var _ = Describe("Stream Framer", func() { Context("sending FINs", func() { It("sends FINs when streams are closed", func() { offset := protocol.ByteCount(42) - stream1.EXPECT().LenOfDataForWriting().Return(protocol.ByteCount(0)) + stream1.EXPECT().HasDataForWriting().Return(false) stream1.EXPECT().GetWriteOffset().Return(offset) stream1.EXPECT().ShouldSendFin().Return(true) stream1.EXPECT().SentFin() @@ -296,7 +296,7 @@ var _ = Describe("Stream Framer", func() { It("bundles FINs with data", func() { offset := protocol.ByteCount(42) stream1.EXPECT().GetDataForWriting(gomock.Any()).Return([]byte("foobar")) - stream1.EXPECT().LenOfDataForWriting().Return(protocol.ByteCount(6)) + stream1.EXPECT().HasDataForWriting().Return(true) stream1.EXPECT().GetWriteOffset().Return(offset) stream1.EXPECT().ShouldSendFin().Return(true) stream1.EXPECT().SentFin() @@ -319,7 +319,7 @@ var _ = Describe("Stream Framer", func() { It("queues and pops BLOCKED frames for individually blocked streams", func() { connFC.EXPECT().IsBlocked() stream1.EXPECT().GetDataForWriting(gomock.Any()).Return([]byte("foobar")) - stream1.EXPECT().LenOfDataForWriting().Return(protocol.ByteCount(6)) + stream1.EXPECT().HasDataForWriting().Return(true) stream1.EXPECT().GetWriteOffset() stream1.EXPECT().ShouldSendFin() stream1.EXPECT().IsFlowControlBlocked().Return(true) @@ -336,7 +336,7 @@ var _ = Describe("Stream Framer", func() { It("does not queue a stream-level BLOCKED frame after sending the FinBit frame", func() { connFC.EXPECT().IsBlocked() stream1.EXPECT().GetDataForWriting(gomock.Any()).Return([]byte("foo")) - stream1.EXPECT().LenOfDataForWriting().Return(protocol.ByteCount(3)) + stream1.EXPECT().HasDataForWriting().Return(true) stream1.EXPECT().GetWriteOffset() stream1.EXPECT().ShouldSendFin().Return(true) stream1.EXPECT().SentFin() @@ -352,7 +352,7 @@ var _ = Describe("Stream Framer", func() { It("queues and pops BLOCKED frames for connection blocked streams", func() { connFC.EXPECT().IsBlocked().Return(true) stream1.EXPECT().GetDataForWriting(gomock.Any()).Return([]byte("foo")) - stream1.EXPECT().LenOfDataForWriting().Return(protocol.ByteCount(3)) + stream1.EXPECT().HasDataForWriting().Return(true) stream1.EXPECT().GetWriteOffset() stream1.EXPECT().ShouldSendFin() stream1.EXPECT().IsFlowControlBlocked().Return(false) diff --git a/stream_test.go b/stream_test.go index 0fec4f02..fbb67f1e 100644 --- a/stream_test.go +++ b/stream_test.go @@ -774,7 +774,7 @@ var _ = Describe("Stream", func() { }).Should(Equal([]byte("foobar"))) Consistently(done).ShouldNot(BeClosed()) Expect(onDataCalled).To(BeTrue()) - Expect(str.LenOfDataForWriting()).To(Equal(protocol.ByteCount(6))) + Expect(str.HasDataForWriting()).To(BeTrue()) data := str.GetDataForWriting(1000) Expect(data).To(Equal([]byte("foobar"))) Expect(str.writeOffset).To(Equal(protocol.ByteCount(6))) @@ -799,17 +799,17 @@ var _ = Describe("Stream", func() { return str.dataForWriting }).Should(Equal([]byte("foobar"))) Consistently(done).ShouldNot(BeClosed()) - Expect(str.LenOfDataForWriting()).To(Equal(protocol.ByteCount(6))) + Expect(str.HasDataForWriting()).To(BeTrue()) data := str.GetDataForWriting(3) Expect(data).To(Equal([]byte("foo"))) Expect(str.writeOffset).To(Equal(protocol.ByteCount(3))) Expect(str.dataForWriting).ToNot(BeNil()) - Expect(str.LenOfDataForWriting()).To(Equal(protocol.ByteCount(3))) + Expect(str.HasDataForWriting()).To(BeTrue()) data = str.GetDataForWriting(3) Expect(data).To(Equal([]byte("bar"))) Expect(str.writeOffset).To(Equal(protocol.ByteCount(6))) Expect(str.dataForWriting).To(BeNil()) - Expect(str.LenOfDataForWriting()).To(Equal(protocol.ByteCount(0))) + Expect(str.HasDataForWriting()).To(BeFalse()) Eventually(done).Should(BeClosed()) }) @@ -827,7 +827,7 @@ var _ = Describe("Stream", func() { Expect(err).ToNot(HaveOccurred()) Expect(n).To(Equal(3)) }() - Eventually(func() protocol.ByteCount { return str.LenOfDataForWriting() }).ShouldNot(BeZero()) + Eventually(func() bool { return str.HasDataForWriting() }).Should(BeTrue()) s[0] = 'v' Expect(str.GetDataForWriting(3)).To(Equal([]byte("foo"))) }) @@ -961,11 +961,11 @@ var _ = Describe("Stream", func() { Expect(err).To(MatchError(testErr)) }() Eventually(func() []byte { return str.dataForWriting }).ShouldNot(BeNil()) - Expect(str.LenOfDataForWriting()).ToNot(BeZero()) + Expect(str.HasDataForWriting()).To(BeTrue()) str.Cancel(testErr) data := str.GetDataForWriting(6) Expect(data).To(BeNil()) - Expect(str.LenOfDataForWriting()).To(BeZero()) + Expect(str.HasDataForWriting()).To(BeFalse()) }) }) })