From 0abe535e16d2eed653e907649b44690b4bd346a8 Mon Sep 17 00:00:00 2001 From: "hta@webrtc.org" Date: Fri, 20 Apr 2012 08:23:16 +0000 Subject: [PATCH] Refactored udp_transport to take socket manager as dependency injection This avoids having to deal with the socket manager in the unittest. Extended tests to cover one case where sockets got allocated. BUG= TEST=unittest Review URL: https://webrtc-codereview.appspot.com/496001 git-svn-id: http://webrtc.googlecode.com/svn/trunk@2078 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../udp_transport/source/udp_transport.gypi | 1 + .../source/udp_transport_impl.cc | 58 +++++---- .../udp_transport/source/udp_transport_impl.h | 25 ++-- .../source/udp_transport_unittest.cc | 120 ++++++++++++++++-- 4 files changed, 161 insertions(+), 43 deletions(-) diff --git a/src/modules/udp_transport/source/udp_transport.gypi b/src/modules/udp_transport/source/udp_transport.gypi index aeec3b318..829428ed5 100644 --- a/src/modules/udp_transport/source/udp_transport.gypi +++ b/src/modules/udp_transport/source/udp_transport.gypi @@ -102,6 +102,7 @@ 'dependencies': [ 'udp_transport', '<(webrtc_root)/../testing/gtest.gyp:gtest', + '<(webrtc_root)/../testing/gmock.gyp:gmock', '<(webrtc_root)/../test/test.gyp:test_support_main', ], 'sources': [ diff --git a/src/modules/udp_transport/source/udp_transport_impl.cc b/src/modules/udp_transport/source/udp_transport_impl.cc index 16aa575ff..bbf41f062 100644 --- a/src/modules/udp_transport/source/udp_transport_impl.cc +++ b/src/modules/udp_transport/source/udp_transport_impl.cc @@ -66,11 +66,27 @@ #endif // defined(WEBRTC_LINUX) || defined(WEBRTC_MAC) namespace webrtc { + +class SocketFactory : public UdpTransportImpl::SocketFactoryInterface { + public: + UdpSocketWrapper* CreateSocket(const WebRtc_Word32 id, + UdpSocketManager* mgr, + CallbackObj obj, + IncomingSocketCallback cb, + bool ipV6Enable, + bool disableGQOS) { + return UdpSocketWrapper::CreateSocket(id, mgr, obj, cb, ipV6Enable, + disableGQOS); + } +}; + + UdpTransport* UdpTransport::Create(const WebRtc_Word32 id, WebRtc_UWord8& numSocketThreads) { - return new UdpTransportImpl(id, numSocketThreads, - &UdpSocketWrapper::CreateSocket); + return new UdpTransportImpl(id, + new SocketFactory(), + UdpSocketManager::Create(id, numSocketThreads)); } void UdpTransport::Destroy(UdpTransport* module) @@ -82,14 +98,14 @@ void UdpTransport::Destroy(UdpTransport* module) } UdpTransportImpl::UdpTransportImpl(const WebRtc_Word32 id, - WebRtc_UWord8& numSocketThreads, - SocketMaker* maker) + SocketFactoryInterface* maker, + UdpSocketManager* socket_manager) : _id(id), _socket_creator(maker), _crit(CriticalSectionWrapper::CreateCriticalSection()), _critFilter(CriticalSectionWrapper::CreateCriticalSection()), _critPacketCallback(CriticalSectionWrapper::CreateCriticalSection()), - _mgr(UdpSocketManager::Create(id, numSocketThreads)), + _mgr(socket_manager), _lastError(kNoSocketError), _destPort(0), _destPortRTCP(0), @@ -141,10 +157,6 @@ UdpTransportImpl::UdpTransportImpl(const WebRtc_Word32 id, memset(_localMulticastIP, 0, sizeof(_localMulticastIP)); memset(&_filterIPAddress, 0, sizeof(_filterIPAddress)); - if(_mgr == NULL) - { - _mgr = UdpSocketManager::Create(id, numSocketThreads); - } WEBRTC_TRACE(kTraceMemory, kTraceTransport, id, "%s created", __FUNCTION__); } @@ -352,11 +364,11 @@ WebRtc_Word32 UdpTransportImpl::InitializeReceiveSockets( _tos=0; _pcp=0; - _ptrRtpSocket = _socket_creator(_id, _mgr, this, + _ptrRtpSocket = _socket_creator->CreateSocket(_id, _mgr, this, IncomingRTPCallback, IpV6Enabled(), false); - _ptrRtcpSocket = _socket_creator(_id, _mgr, this, + _ptrRtcpSocket = _socket_creator->CreateSocket(_id, _mgr, this, IncomingRTCPCallback, IpV6Enabled(), false); @@ -832,11 +844,11 @@ WebRtc_Word32 UdpTransportImpl::SetToS(WebRtc_Word32 DSCP, bool useSetSockOpt) { CloseSendSockets(); _ptrSendRtpSocket = - _socket_creator(_id, _mgr, NULL, + _socket_creator->CreateSocket(_id, _mgr, NULL, NULL, IpV6Enabled(), true); _ptrSendRtcpSocket = - _socket_creator(_id, _mgr, NULL, + _socket_creator->CreateSocket(_id, _mgr, NULL, NULL, IpV6Enabled(), true); rtpSock=_ptrSendRtpSocket; @@ -867,12 +879,12 @@ WebRtc_Word32 UdpTransportImpl::SetToS(WebRtc_Word32 DSCP, bool useSetSockOpt) } } CloseReceiveSockets(); - _ptrRtpSocket = _socket_creator(_id, _mgr, this, - IncomingRTPCallback, - IpV6Enabled(), true); - _ptrRtcpSocket = _socket_creator(_id, _mgr, this, - IncomingRTCPCallback, - IpV6Enabled(),true); + _ptrRtpSocket = _socket_creator->CreateSocket( + _id, _mgr, this, IncomingRTPCallback, IpV6Enabled(), + true); + _ptrRtcpSocket = _socket_creator->CreateSocket( + _id, _mgr, this, IncomingRTCPCallback, IpV6Enabled(), + true); rtpSock=_ptrRtpSocket; rtcpSock=_ptrRtcpSocket; ErrorCode retVal = BindLocalRTPSocket(); @@ -1530,9 +1542,9 @@ WebRtc_Word32 UdpTransportImpl::InitializeSourcePorts(WebRtc_UWord16 rtpPort, _tos=0; _pcp=0; - _ptrSendRtpSocket = _socket_creator(_id, _mgr, NULL, NULL, + _ptrSendRtpSocket = _socket_creator->CreateSocket(_id, _mgr, NULL, NULL, IpV6Enabled(), false); - _ptrSendRtcpSocket = _socket_creator(_id, _mgr, NULL, NULL, + _ptrSendRtcpSocket = _socket_creator->CreateSocket(_id, _mgr, NULL, NULL, IpV6Enabled(), false); ErrorCode retVal = BindRTPSendSocket(); @@ -1983,7 +1995,7 @@ int UdpTransportImpl::SendPacket(int /*channel*/, const void* data, int length) "Creating RTP socket since no receive or source socket is\ configured"); - _ptrRtpSocket = _socket_creator(_id, _mgr, this, + _ptrRtpSocket = _socket_creator->CreateSocket(_id, _mgr, this, IncomingRTPCallback, IpV6Enabled(), false); @@ -2049,7 +2061,7 @@ int UdpTransportImpl::SendRTCPPacket(int /*channel*/, const void* data, "Creating RTCP socket since no receive or source socket is\ configured"); - _ptrRtcpSocket = _socket_creator(_id, _mgr, this, + _ptrRtcpSocket = _socket_creator->CreateSocket(_id, _mgr, this, IncomingRTCPCallback, IpV6Enabled(), false); diff --git a/src/modules/udp_transport/source/udp_transport_impl.h b/src/modules/udp_transport/source/udp_transport_impl.h index 827cb15b7..0d9391423 100644 --- a/src/modules/udp_transport/source/udp_transport_impl.h +++ b/src/modules/udp_transport/source/udp_transport_impl.h @@ -22,17 +22,22 @@ class UdpSocketManager; class UdpTransportImpl : public UdpTransport { public: - // A function that returns a wrapped UDP socket or equivalent. - typedef UdpSocketWrapper* (SocketMaker)(const WebRtc_Word32 id, - UdpSocketManager* mgr, - CallbackObj obj, - IncomingSocketCallback cb, - bool ipV6Enable, - bool disableGQOS); + // A factory that returns a wrapped UDP socket or equivalent. + class SocketFactoryInterface { + public: + virtual UdpSocketWrapper* CreateSocket(const WebRtc_Word32 id, + UdpSocketManager* mgr, + CallbackObj obj, + IncomingSocketCallback cb, + bool ipV6Enable, + bool disableGQOS) = 0; + }; // Constructor, only called by UdpTransport::Create and tests. - UdpTransportImpl(const WebRtc_Word32 id, WebRtc_UWord8& numSocketThreads, - SocketMaker* maker); + // The constructor takes ownership of the factory. + UdpTransportImpl(const WebRtc_Word32 id, + SocketFactoryInterface* maker, + UdpSocketManager* socket_manager); virtual ~UdpTransportImpl(); // Module functions @@ -183,7 +188,7 @@ private: WebRtc_UWord16& sourcePort); WebRtc_Word32 _id; - SocketMaker* _socket_creator; + SocketFactoryInterface* _socket_creator; // Protects the sockets from being re-configured while receiving packets. CriticalSectionWrapper* _crit; CriticalSectionWrapper* _critFilter; diff --git a/src/modules/udp_transport/source/udp_transport_unittest.cc b/src/modules/udp_transport/source/udp_transport_unittest.cc index 594e74f39..25a9b9d5f 100644 --- a/src/modules/udp_transport/source/udp_transport_unittest.cc +++ b/src/modules/udp_transport/source/udp_transport_unittest.cc @@ -8,28 +8,128 @@ * be found in the AUTHORS file in the root of the source tree. */ -/* - * Empty test just to get code coverage metrics for this dir. - */ +#include + #include "udp_transport.h" +#include "gmock/gmock.h" #include "gtest/gtest.h" // We include the implementation header file to get at the dependency-injecting // constructor. #include "udp_transport_impl.h" +// We must mock the socket manager, for which we need its definition. +#include "udp_socket_manager_wrapper.h" -TEST(UDPTransportTest, CreateTransport) { +using ::testing::_; +using ::testing::Return; + +class MockUdpSocketWrapper : public webrtc::UdpSocketWrapper { + public: + // The following methods have to be mocked because they are pure. + MOCK_METHOD1(ChangeUniqueId, WebRtc_Word32(WebRtc_Word32)); + MOCK_METHOD2(SetCallback, bool(webrtc::CallbackObj, + webrtc::IncomingSocketCallback)); + MOCK_METHOD1(Bind, bool(const webrtc::SocketAddress&)); + MOCK_METHOD0(ValidHandle, bool()); + MOCK_METHOD4(SetSockopt, bool(WebRtc_Word32, WebRtc_Word32, + const WebRtc_Word8*, + WebRtc_Word32)); + MOCK_METHOD1(SetTOS, WebRtc_Word32(WebRtc_Word32)); + MOCK_METHOD3(SendTo, WebRtc_Word32(const WebRtc_Word8*, WebRtc_Word32, + const webrtc::SocketAddress&)); + MOCK_METHOD8(SetQos, bool(WebRtc_Word32, WebRtc_Word32, + WebRtc_Word32, WebRtc_Word32, + WebRtc_Word32, WebRtc_Word32, + const webrtc::SocketAddress &, + WebRtc_Word32)); +}; + +class MockUdpSocketManager : public webrtc::UdpSocketManager { + public: + MOCK_METHOD2(Init, bool(WebRtc_Word32, WebRtc_UWord8&)); + MOCK_METHOD1(ChangeUniqueId, WebRtc_Word32(const WebRtc_Word32)); + MOCK_METHOD0(Start, bool()); + MOCK_METHOD0(Stop, bool()); + MOCK_METHOD1(AddSocket, bool(webrtc::UdpSocketWrapper*)); + MOCK_METHOD1(RemoveSocket, bool(webrtc::UdpSocketWrapper*)); +}; + +class MockSocketFactory : + public webrtc::UdpTransportImpl::SocketFactoryInterface { + public: + MockSocketFactory(std::vector* socket_counter) + : socket_counter_(socket_counter) { + } + webrtc::UdpSocketWrapper* CreateSocket(const WebRtc_Word32 id, + webrtc::UdpSocketManager* mgr, + webrtc::CallbackObj obj, + webrtc::IncomingSocketCallback cb, + bool ipV6Enable, + bool disableGQOS) { + MockUdpSocketWrapper* socket = new MockUdpSocketWrapper(); + // We instrument the socket with calls that are expected, but do + // not matter for any specific test, in order to avoid warning messages. + EXPECT_CALL(*socket, ValidHandle()).WillRepeatedly(Return(true)); + EXPECT_CALL(*socket, Bind(_)).WillOnce(Return(true)); + socket_counter_->push_back(socket); + return socket; + } + std::vector* socket_counter_; +}; + +class UDPTransportTest : public ::testing::Test { + public: + UDPTransportTest() + : sockets_created_(0) { + } + + ~UDPTransportTest() { + // In production, sockets register themselves at creation time with + // an UdpSocketManager, and the UdpSocketManager is responsible for + // deleting them. In this test, we just delete them after the test. + while (!sockets_created_.empty()) { + delete sockets_created_.back(); + sockets_created_.pop_back(); + } + } + + int NumSocketsCreated() { + return sockets_created_.size(); + } + + std::vector* sockets_created() { + return &sockets_created_; + } +private: + std::vector sockets_created_; +}; + +TEST_F(UDPTransportTest, CreateTransport) { WebRtc_Word32 id = 0; - WebRtc_UWord8 threads = 0; + WebRtc_UWord8 threads = 1; webrtc::UdpTransport* transport = webrtc::UdpTransport::Create(id, threads); webrtc::UdpTransport::Destroy(transport); } // This test verifies that the mock_socket is not called from the constructor. -TEST(UDPTransportTest, ConstructorDoesNotCreateSocket) { +TEST_F(UDPTransportTest, ConstructorDoesNotCreateSocket) { WebRtc_Word32 id = 0; - WebRtc_UWord8 threads = 0; - webrtc::UdpTransportImpl::SocketMaker* null_maker = NULL; - webrtc::UdpTransport* transport = new webrtc::UdpTransportImpl(id, threads, - null_maker); + webrtc::UdpTransportImpl::SocketFactoryInterface* null_maker = NULL; + webrtc::UdpSocketManager* null_manager = NULL; + webrtc::UdpTransport* transport = new webrtc::UdpTransportImpl(id, + null_maker, + null_manager); + webrtc::UdpTransport::Destroy(transport); +} + +TEST_F(UDPTransportTest, InitializeSourcePorts) { + WebRtc_Word32 id = 0; + webrtc::UdpTransportImpl::SocketFactoryInterface* mock_maker + = new MockSocketFactory(sockets_created()); + webrtc::UdpSocketManager* mock_manager = new MockUdpSocketManager(); + webrtc::UdpTransport* transport = new webrtc::UdpTransportImpl(id, + mock_maker, + mock_manager); + EXPECT_EQ(0, transport->InitializeSourcePorts(4711, 4712)); + EXPECT_EQ(2, NumSocketsCreated()); webrtc::UdpTransport::Destroy(transport); }