From 02c28c3a59bd517ccccc1d6314a128aecc6454cf Mon Sep 17 00:00:00 2001 From: Lucas Clemente Date: Mon, 8 Aug 2016 15:35:31 +0200 Subject: [PATCH] remove unneeded streamsMap.{Put,Get}Stream --- session_test.go | 36 +++++++++++++++++----------------- stream_framer_test.go | 4 ++-- streams_map.go | 20 ++----------------- streams_map_test.go | 45 +++++-------------------------------------- 4 files changed, 27 insertions(+), 78 deletions(-) diff --git a/session_test.go b/session_test.go index b3a98f9f0..65196f662 100644 --- a/session_test.go +++ b/session_test.go @@ -126,7 +126,7 @@ var _ = Describe("Session", func() { Expect(session.streamsMap.NumberOfStreams()).To(Equal(2)) Expect(streamCallbackCalled).To(BeTrue()) p := make([]byte, 4) - str, _ := session.streamsMap.GetStream(5) + str, _ := session.streamsMap.GetOrOpenStream(5) Expect(str).ToNot(BeNil()) _, err := str.Read(p) Expect(err).ToNot(HaveOccurred()) @@ -165,7 +165,7 @@ var _ = Describe("Session", func() { }) Expect(session.streamsMap.NumberOfStreams()).To(Equal(2)) p := make([]byte, 4) - str, _ := session.streamsMap.GetStream(5) + str, _ := session.streamsMap.GetOrOpenStream(5) Expect(str).ToNot(BeNil()) _, err := str.Read(p) Expect(err).ToNot(HaveOccurred()) @@ -178,7 +178,7 @@ var _ = Describe("Session", func() { str.Close() session.garbageCollectStreams() Expect(session.streamsMap.NumberOfStreams()).To(Equal(2)) - str, _ = session.streamsMap.GetStream(5) + str, _ = session.streamsMap.GetOrOpenStream(5) Expect(str).ToNot(BeNil()) }) @@ -189,7 +189,7 @@ var _ = Describe("Session", func() { FinBit: true, }) Expect(session.streamsMap.NumberOfStreams()).To(Equal(2)) - str, _ := session.streamsMap.GetStream(5) + str, _ := session.streamsMap.GetOrOpenStream(5) Expect(str).ToNot(BeNil()) Expect(streamCallbackCalled).To(BeTrue()) p := make([]byte, 4) @@ -198,7 +198,7 @@ var _ = Describe("Session", func() { Expect(p).To(Equal([]byte{0xde, 0xca, 0xfb, 0xad})) session.garbageCollectStreams() Expect(session.streamsMap.NumberOfStreams()).To(Equal(2)) - str, _ = session.streamsMap.GetStream(5) + str, _ = session.streamsMap.GetOrOpenStream(5) Expect(str).ToNot(BeNil()) }) @@ -209,7 +209,7 @@ var _ = Describe("Session", func() { FinBit: true, }) Expect(session.streamsMap.NumberOfStreams()).To(Equal(2)) - str, _ := session.streamsMap.GetStream(5) + str, _ := session.streamsMap.GetOrOpenStream(5) Expect(str).ToNot(BeNil()) Expect(streamCallbackCalled).To(BeTrue()) p := make([]byte, 4) @@ -218,7 +218,7 @@ var _ = Describe("Session", func() { Expect(p).To(Equal([]byte{0xde, 0xca, 0xfb, 0xad})) session.garbageCollectStreams() Expect(session.streamsMap.NumberOfStreams()).To(Equal(2)) - str, _ = session.streamsMap.GetStream(5) + str, _ = session.streamsMap.GetOrOpenStream(5) Expect(str).ToNot(BeNil()) // We still need to close the stream locally str.Close() @@ -226,8 +226,8 @@ var _ = Describe("Session", func() { str.sentFin() session.garbageCollectStreams() Expect(session.streamsMap.NumberOfStreams()).To(Equal(1)) - str, strExists := session.streamsMap.GetStream(5) - Expect(strExists).To(BeTrue()) + str, err = session.streamsMap.GetOrOpenStream(5) + Expect(err).NotTo(HaveOccurred()) Expect(str).To(BeNil()) // flow controller should have been notified _, err = session.flowControlManager.SendWindowSize(5) @@ -241,7 +241,7 @@ var _ = Describe("Session", func() { Data: []byte{0xde, 0xca, 0xfb, 0xad}, }) Expect(session.streamsMap.NumberOfStreams()).To(Equal(2)) - str, _ := session.streamsMap.GetStream(5) + str, _ := session.streamsMap.GetOrOpenStream(5) Expect(str).ToNot(BeNil()) Expect(streamCallbackCalled).To(BeTrue()) p := make([]byte, 4) @@ -251,8 +251,8 @@ var _ = Describe("Session", func() { Expect(err).To(MatchError(testErr)) session.garbageCollectStreams() Expect(session.streamsMap.NumberOfStreams()).To(Equal(1)) - str, strExists := session.streamsMap.GetStream(5) - Expect(strExists).To(BeTrue()) + str, err = session.streamsMap.GetOrOpenStream(5) + Expect(err).NotTo(HaveOccurred()) Expect(str).To(BeNil()) }) @@ -260,14 +260,14 @@ var _ = Describe("Session", func() { testErr := errors.New("test") session.newStreamImpl(5) Expect(session.streamsMap.NumberOfStreams()).To(Equal(2)) - str, _ := session.streamsMap.GetStream(5) + str, _ := session.streamsMap.GetOrOpenStream(5) Expect(str).ToNot(BeNil()) session.closeStreamsWithError(testErr) _, err := str.Read([]byte{0}) Expect(err).To(MatchError(testErr)) session.garbageCollectStreams() - str, strExists := session.streamsMap.GetStream(5) - Expect(strExists).To(BeTrue()) + str, err = session.streamsMap.GetOrOpenStream(5) + Expect(err).NotTo(HaveOccurred()) Expect(str).To(BeNil()) }) @@ -286,7 +286,7 @@ var _ = Describe("Session", func() { Data: []byte{}, FinBit: true, }) - str, _ := session.streamsMap.GetStream(5) + str, _ := session.streamsMap.GetOrOpenStream(5) Expect(str).ToNot(BeNil()) _, err := str.Read([]byte{0}) Expect(err).To(MatchError(io.EOF)) @@ -361,8 +361,8 @@ var _ = Describe("Session", func() { ByteOffset: 1337, }) Expect(err).ToNot(HaveOccurred()) - str, strExists := session.streamsMap.GetStream(5) - Expect(strExists).To(BeTrue()) + str, err := session.streamsMap.GetOrOpenStream(5) + Expect(err).NotTo(HaveOccurred()) Expect(str).ToNot(BeNil()) }) diff --git a/stream_framer_test.go b/stream_framer_test.go index ffddb1a4a..32067fd90 100644 --- a/stream_framer_test.go +++ b/stream_framer_test.go @@ -32,8 +32,8 @@ var _ = Describe("Stream Framer", func() { stream2 = &stream{streamID: 11} streamsMap = newStreamsMap(nil) - streamsMap.PutStream(stream1) - streamsMap.PutStream(stream2) + streamsMap.putStream(stream1) + streamsMap.putStream(stream2) fcm = newMockFlowControlHandler() fcm.sendWindowSizes[stream1.streamID] = protocol.MaxByteCount diff --git a/streams_map.go b/streams_map.go index bc4ce3bb3..2e7574770 100644 --- a/streams_map.go +++ b/streams_map.go @@ -37,16 +37,6 @@ func newStreamsMap(newStream newStreamLambda) *streamsMap { } } -func (m *streamsMap) GetStream(id protocol.StreamID) (*stream, bool) { - m.mutex.RLock() - s, ok := m.streams[id] - m.mutex.RUnlock() - if !ok { - return nil, false - } - return s, true -} - // GetOrOpenStream either returns an existing stream, a newly opened stream, or nil if a stream with the provided ID is already closed. func (m *streamsMap) GetOrOpenStream(id protocol.StreamID) (*stream, error) { m.mutex.RLock() @@ -70,7 +60,7 @@ func (m *streamsMap) GetOrOpenStream(id protocol.StreamID) (*stream, error) { if err != nil { return nil, err } - m.putStreamImpl(s) + m.putStream(s) return s, nil } @@ -125,13 +115,7 @@ func (m *streamsMap) RoundRobinIterate(fn streamLambda) error { return nil } -func (m *streamsMap) PutStream(s *stream) error { - m.mutex.Lock() - defer m.mutex.Unlock() - return m.putStreamImpl(s) -} - -func (m *streamsMap) putStreamImpl(s *stream) error { +func (m *streamsMap) putStream(s *stream) error { id := s.StreamID() if _, ok := m.streams[id]; ok { return fmt.Errorf("a stream with ID %d already exists", id) diff --git a/streams_map_test.go b/streams_map_test.go index 220a80c3e..6c2f7f8ac 100644 --- a/streams_map_test.go +++ b/streams_map_test.go @@ -18,41 +18,6 @@ var _ = Describe("Streams Map", func() { m = newStreamsMap(nil) }) - It("returns an error for non-existent streams", func() { - _, exists := m.GetStream(1) - Expect(exists).To(BeFalse()) - }) - - It("returns nil for previously existing streams", func() { - err := m.PutStream(&stream{streamID: 1}) - Expect(err).NotTo(HaveOccurred()) - err = m.RemoveStream(1) - Expect(err).NotTo(HaveOccurred()) - s, exists := m.GetStream(1) - Expect(exists).To(BeTrue()) - Expect(s).To(BeNil()) - }) - - Context("putting streams", func() { - It("stores streams", func() { - err := m.PutStream(&stream{streamID: 5}) - Expect(err).NotTo(HaveOccurred()) - s, exists := m.GetStream(5) - Expect(exists).To(BeTrue()) - Expect(s.streamID).To(Equal(protocol.StreamID(5))) - Expect(m.openStreams).To(HaveLen(1)) - Expect(m.openStreams[0]).To(Equal(protocol.StreamID(5))) - }) - - It("does not store multiple streams with the same ID", func() { - err := m.PutStream(&stream{streamID: 5}) - Expect(err).NotTo(HaveOccurred()) - err = m.PutStream(&stream{streamID: 5}) - Expect(err).To(MatchError("a stream with ID 5 already exists")) - Expect(m.openStreams).To(HaveLen(1)) - }) - }) - Context("getting and creating streams", func() { BeforeEach(func() { m.newStream = func(id protocol.StreamID) (*stream, error) { @@ -107,7 +72,7 @@ var _ = Describe("Streams Map", func() { Context("deleting streams", func() { BeforeEach(func() { for i := 1; i <= 5; i++ { - err := m.PutStream(&stream{streamID: protocol.StreamID(i)}) + err := m.putStream(&stream{streamID: protocol.StreamID(i)}) Expect(err).ToNot(HaveOccurred()) } Expect(m.openStreams).To(Equal([]protocol.StreamID{1, 2, 3, 4, 5})) @@ -154,13 +119,13 @@ var _ = Describe("Streams Map", func() { }) It("increases the counter when a new stream is added", func() { - err := m.PutStream(&stream{streamID: 5}) + err := m.putStream(&stream{streamID: 5}) Expect(err).ToNot(HaveOccurred()) Expect(m.NumberOfStreams()).To(Equal(1)) }) It("decreases the counter when removing a stream", func() { - err := m.PutStream(&stream{streamID: 5}) + err := m.putStream(&stream{streamID: 5}) Expect(err).ToNot(HaveOccurred()) err = m.RemoveStream(5) Expect(err).ToNot(HaveOccurred()) @@ -172,7 +137,7 @@ var _ = Describe("Streams Map", func() { // create 3 streams, ids 1 to 3 BeforeEach(func() { for i := 1; i <= 3; i++ { - err := m.PutStream(&stream{streamID: protocol.StreamID(i)}) + err := m.putStream(&stream{streamID: protocol.StreamID(i)}) Expect(err).NotTo(HaveOccurred()) } }) @@ -228,7 +193,7 @@ var _ = Describe("Streams Map", func() { lambdaCalledForStream = lambdaCalledForStream[:0] numIterations = 0 for i := 1; i <= 5; i++ { - err := m.PutStream(&stream{streamID: protocol.StreamID(i)}) + err := m.putStream(&stream{streamID: protocol.StreamID(i)}) Expect(err).NotTo(HaveOccurred()) } })