From cedbafeb286dafcfb076de109af6ff8b43b89f9c Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Mon, 4 Sep 2017 22:01:59 -0700 Subject: [PATCH] Add INetworkConnection interface for unit testing. --- src/main/native/cpp/Dispatcher.cpp | 7 ++-- src/main/native/cpp/Dispatcher.h | 10 +++-- src/main/native/cpp/IDispatcher.h | 6 +-- src/main/native/cpp/INetworkConnection.h | 41 +++++++++++++++++++++ src/main/native/cpp/IStorage.h | 11 +++--- src/main/native/cpp/NetworkConnection.cpp | 6 +++ src/main/native/cpp/NetworkConnection.h | 19 +++++----- src/main/native/cpp/Storage.cpp | 30 +++++++-------- src/main/native/cpp/Storage.h | 25 +++++++------ src/test/native/cpp/MockDispatcher.h | 4 +- src/test/native/cpp/MockNetworkConnection.h | 33 +++++++++++++++++ src/test/native/cpp/StorageTest.cpp | 10 ++--- 12 files changed, 143 insertions(+), 59 deletions(-) create mode 100644 src/main/native/cpp/INetworkConnection.h create mode 100644 src/test/native/cpp/MockNetworkConnection.h diff --git a/src/main/native/cpp/Dispatcher.cpp b/src/main/native/cpp/Dispatcher.cpp index 9984944904..a170311927 100644 --- a/src/main/native/cpp/Dispatcher.cpp +++ b/src/main/native/cpp/Dispatcher.cpp @@ -16,6 +16,7 @@ #include "IConnectionNotifier.h" #include "Log.h" #include "IStorage.h" +#include "NetworkConnection.h" using namespace nt; @@ -178,7 +179,7 @@ void DispatcherBase::Stop() { if (m_dispatch_thread.joinable()) m_dispatch_thread.join(); if (m_clientserver_thread.joinable()) m_clientserver_thread.join(); - std::vector> conns; + std::vector> conns; { std::lock_guard lock(m_user_mutex); conns.swap(m_connections); @@ -315,8 +316,8 @@ void DispatcherBase::DispatchThreadMain() { } void DispatcherBase::QueueOutgoing(std::shared_ptr msg, - NetworkConnection* only, - NetworkConnection* except) { + INetworkConnection* only, + INetworkConnection* except) { std::lock_guard user_lock(m_user_mutex); for (auto& conn : m_connections) { if (conn.get() == except) continue; diff --git a/src/main/native/cpp/Dispatcher.h b/src/main/native/cpp/Dispatcher.h index 3d4151b22a..5f7299ee68 100644 --- a/src/main/native/cpp/Dispatcher.h +++ b/src/main/native/cpp/Dispatcher.h @@ -15,12 +15,13 @@ #include #include #include +#include #include #include "llvm/StringRef.h" #include "IDispatcher.h" -#include "NetworkConnection.h" +#include "INetworkConnection.h" namespace wpi { class Logger; @@ -32,6 +33,7 @@ namespace nt { class IConnectionNotifier; class IStorage; +class NetworkConnection; class DispatcherBase : public IDispatcher { friend class DispatcherTest; @@ -79,8 +81,8 @@ class DispatcherBase : public IDispatcher { void ClientReconnect(unsigned int proto_rev = 0x0300); - void QueueOutgoing(std::shared_ptr msg, NetworkConnection* only, - NetworkConnection* except) override; + void QueueOutgoing(std::shared_ptr msg, INetworkConnection* only, + INetworkConnection* except) override; IStorage& m_storage; IConnectionNotifier& m_notifier; @@ -96,7 +98,7 @@ class DispatcherBase : public IDispatcher { // Mutex for user-accessible items mutable std::mutex m_user_mutex; - std::vector> m_connections; + std::vector> m_connections; std::string m_identity; std::atomic_bool m_active; // set to false to terminate threads diff --git a/src/main/native/cpp/IDispatcher.h b/src/main/native/cpp/IDispatcher.h index fc7d14301d..bbe4aa488c 100644 --- a/src/main/native/cpp/IDispatcher.h +++ b/src/main/native/cpp/IDispatcher.h @@ -14,7 +14,7 @@ namespace nt { -class NetworkConnection; +class INetworkConnection; // Interface for generation of outgoing messages to break a dependency loop // between Storage and Dispatcher. @@ -25,8 +25,8 @@ class IDispatcher { IDispatcher& operator=(const IDispatcher&) = delete; virtual ~IDispatcher() = default; virtual void QueueOutgoing(std::shared_ptr msg, - NetworkConnection* only, - NetworkConnection* except) = 0; + INetworkConnection* only, + INetworkConnection* except) = 0; }; } // namespace nt diff --git a/src/main/native/cpp/INetworkConnection.h b/src/main/native/cpp/INetworkConnection.h new file mode 100644 index 0000000000..e7deba5917 --- /dev/null +++ b/src/main/native/cpp/INetworkConnection.h @@ -0,0 +1,41 @@ +/*----------------------------------------------------------------------------*/ +/* Copyright (c) FIRST 2017. All Rights Reserved. */ +/* Open Source Software - may be modified and shared by FRC teams. The code */ +/* must be accompanied by the FIRST BSD license file in the root directory of */ +/* the project. */ +/*----------------------------------------------------------------------------*/ + +#ifndef NT_INETWORKCONNECTION_H_ +#define NT_INETWORKCONNECTION_H_ + +#include + +#include "Message.h" +#include "ntcore_cpp.h" + +namespace nt { + +class INetworkConnection { + public: + enum State { kCreated, kInit, kHandshake, kSynchronized, kActive, kDead }; + + INetworkConnection() = default; + INetworkConnection(const INetworkConnection&) = delete; + INetworkConnection& operator=(const INetworkConnection&) = delete; + virtual ~INetworkConnection() = default; + + virtual ConnectionInfo info() const = 0; + + virtual void QueueOutgoing(std::shared_ptr msg) = 0; + virtual void PostOutgoing(bool keep_alive) = 0; + + virtual unsigned int proto_rev() const = 0; + virtual void set_proto_rev(unsigned int proto_rev) = 0; + + virtual State state() const = 0; + virtual void set_state(State state) = 0; +}; + +} // namespace nt + +#endif // NT_INETWORKCONNECTION_H_ diff --git a/src/main/native/cpp/IStorage.h b/src/main/native/cpp/IStorage.h index 7ee9d05bf9..5e5a4d4e79 100644 --- a/src/main/native/cpp/IStorage.h +++ b/src/main/native/cpp/IStorage.h @@ -21,7 +21,7 @@ namespace nt { class IDispatcher; -class NetworkConnection; +class INetworkConnection; class IStorage { public: @@ -42,12 +42,13 @@ class IStorage { virtual NT_Type GetMessageEntryType(unsigned int id) const = 0; virtual void ProcessIncoming(std::shared_ptr msg, - NetworkConnection* conn, - std::weak_ptr conn_weak) = 0; + INetworkConnection* conn, + std::weak_ptr conn_weak) = 0; virtual void GetInitialAssignments( - NetworkConnection& conn, std::vector>* msgs) = 0; + INetworkConnection& conn, + std::vector>* msgs) = 0; virtual void ApplyInitialAssignments( - NetworkConnection& conn, llvm::ArrayRef> msgs, + INetworkConnection& conn, llvm::ArrayRef> msgs, bool new_server, std::vector>* out_msgs) = 0; // Filename-based save/load functions. Used both by periodic saves and diff --git a/src/main/native/cpp/NetworkConnection.cpp b/src/main/native/cpp/NetworkConnection.cpp index dbffc38ecc..eb33abd09b 100644 --- a/src/main/native/cpp/NetworkConnection.cpp +++ b/src/main/native/cpp/NetworkConnection.cpp @@ -97,6 +97,12 @@ ConnectionInfo NetworkConnection::info() const { m_last_update, m_proto_rev}; } +unsigned int NetworkConnection::proto_rev() const { return m_proto_rev; } + +void NetworkConnection::set_proto_rev(unsigned int proto_rev) { + m_proto_rev = proto_rev; +} + NetworkConnection::State NetworkConnection::state() const { std::lock_guard lock(m_state_mutex); return m_state; diff --git a/src/main/native/cpp/NetworkConnection.h b/src/main/native/cpp/NetworkConnection.h index 01edafd7c7..91d82ccdfa 100644 --- a/src/main/native/cpp/NetworkConnection.h +++ b/src/main/native/cpp/NetworkConnection.h @@ -14,6 +14,7 @@ #include #include "support/ConcurrentQueue.h" +#include "INetworkConnection.h" #include "Message.h" #include "ntcore_cpp.h" @@ -26,10 +27,8 @@ namespace nt { class IConnectionNotifier; -class NetworkConnection { +class NetworkConnection : public INetworkConnection { public: - enum State { kCreated, kInit, kHandshake, kSynchronized, kActive, kDead }; - typedef std::function()> get_msg, @@ -56,21 +55,21 @@ class NetworkConnection { void Start(); void Stop(); - ConnectionInfo info() const; + ConnectionInfo info() const override; bool active() const { return m_active; } wpi::NetworkStream& stream() { return *m_stream; } - void QueueOutgoing(std::shared_ptr msg); - void PostOutgoing(bool keep_alive); + void QueueOutgoing(std::shared_ptr msg) override; + void PostOutgoing(bool keep_alive) override; unsigned int uid() const { return m_uid; } - unsigned int proto_rev() const { return m_proto_rev; } - void set_proto_rev(unsigned int proto_rev) { m_proto_rev = proto_rev; } + unsigned int proto_rev() const override; + void set_proto_rev(unsigned int proto_rev) override; - State state() const; - void set_state(State state); + State state() const override; + void set_state(State state) override; std::string remote_id() const; void set_remote_id(StringRef remote_id); diff --git a/src/main/native/cpp/Storage.cpp b/src/main/native/cpp/Storage.cpp index 34abfcbe6f..433e00ebf1 100644 --- a/src/main/native/cpp/Storage.cpp +++ b/src/main/native/cpp/Storage.cpp @@ -12,9 +12,9 @@ #include "Handle.h" #include "IDispatcher.h" #include "IEntryNotifier.h" +#include "INetworkConnection.h" #include "IRpcServer.h" #include "Log.h" -#include "NetworkConnection.h" using namespace nt; @@ -46,8 +46,8 @@ NT_Type Storage::GetMessageEntryType(unsigned int id) const { } void Storage::ProcessIncoming(std::shared_ptr msg, - NetworkConnection* conn, - std::weak_ptr conn_weak) { + INetworkConnection* conn, + std::weak_ptr conn_weak) { switch (msg->type()) { case Message::kKeepAlive: break; // ignore @@ -85,7 +85,7 @@ void Storage::ProcessIncoming(std::shared_ptr msg, } void Storage::ProcessIncomingEntryAssign(std::shared_ptr msg, - NetworkConnection* conn) { + INetworkConnection* conn) { std::unique_lock lock(m_mutex); unsigned int id = msg->id(); StringRef name = msg->str(); @@ -209,7 +209,7 @@ void Storage::ProcessIncomingEntryAssign(std::shared_ptr msg, } void Storage::ProcessIncomingEntryUpdate(std::shared_ptr msg, - NetworkConnection* conn) { + INetworkConnection* conn) { std::unique_lock lock(m_mutex); unsigned int id = msg->id(); if (id >= m_idmap.size() || !m_idmap[id]) { @@ -246,7 +246,7 @@ void Storage::ProcessIncomingEntryUpdate(std::shared_ptr msg, } void Storage::ProcessIncomingFlagsUpdate(std::shared_ptr msg, - NetworkConnection* conn) { + INetworkConnection* conn) { std::unique_lock lock(m_mutex); unsigned int id = msg->id(); if (id >= m_idmap.size() || !m_idmap[id]) { @@ -270,7 +270,7 @@ void Storage::ProcessIncomingFlagsUpdate(std::shared_ptr msg, } void Storage::ProcessIncomingEntryDelete(std::shared_ptr msg, - NetworkConnection* conn) { + INetworkConnection* conn) { std::unique_lock lock(m_mutex); unsigned int id = msg->id(); if (id >= m_idmap.size() || !m_idmap[id]) { @@ -294,7 +294,7 @@ void Storage::ProcessIncomingEntryDelete(std::shared_ptr msg, } void Storage::ProcessIncomingClearEntries(std::shared_ptr msg, - NetworkConnection* conn) { + INetworkConnection* conn) { std::unique_lock lock(m_mutex); // update local DeleteAllEntriesImpl(false); @@ -309,8 +309,8 @@ void Storage::ProcessIncomingClearEntries(std::shared_ptr msg, } void Storage::ProcessIncomingExecuteRpc( - std::shared_ptr msg, NetworkConnection* conn, - std::weak_ptr conn_weak) { + std::shared_ptr msg, INetworkConnection* conn, + std::weak_ptr conn_weak) { std::unique_lock lock(m_mutex); if (!m_server) return; // only process on server unsigned int id = msg->id(); @@ -349,7 +349,7 @@ void Storage::ProcessIncomingExecuteRpc( } void Storage::ProcessIncomingRpcResponse(std::shared_ptr msg, - NetworkConnection* conn) { + INetworkConnection* conn) { std::unique_lock lock(m_mutex); if (m_server) return; // only process on client unsigned int id = msg->id(); @@ -372,9 +372,9 @@ void Storage::ProcessIncomingRpcResponse(std::shared_ptr msg, } void Storage::GetInitialAssignments( - NetworkConnection& conn, std::vector>* msgs) { + INetworkConnection& conn, std::vector>* msgs) { std::lock_guard lock(m_mutex); - conn.set_state(NetworkConnection::kSynchronized); + conn.set_state(INetworkConnection::kSynchronized); for (auto& i : m_entries) { Entry* entry = i.getValue(); msgs->emplace_back(Message::EntryAssign(i.getKey(), entry->id, @@ -384,12 +384,12 @@ void Storage::GetInitialAssignments( } void Storage::ApplyInitialAssignments( - NetworkConnection& conn, llvm::ArrayRef> msgs, + INetworkConnection& conn, llvm::ArrayRef> msgs, bool new_server, std::vector>* out_msgs) { std::unique_lock lock(m_mutex); if (m_server) return; // should not do this on server - conn.set_state(NetworkConnection::kSynchronized); + conn.set_state(INetworkConnection::kSynchronized); std::vector> update_msgs; diff --git a/src/main/native/cpp/Storage.h b/src/main/native/cpp/Storage.h index 611ed2747a..ea2240fad5 100644 --- a/src/main/native/cpp/Storage.h +++ b/src/main/native/cpp/Storage.h @@ -36,6 +36,7 @@ class raw_istream; namespace nt { class IEntryNotifier; +class INetworkConnection; class IRpcServer; class IStorageTest; @@ -60,13 +61,13 @@ class Storage : public IStorage { // message itself). Not used in wire protocol 3.0. NT_Type GetMessageEntryType(unsigned int id) const override; - void ProcessIncoming(std::shared_ptr msg, NetworkConnection* conn, - std::weak_ptr conn_weak) override; + void ProcessIncoming(std::shared_ptr msg, INetworkConnection* conn, + std::weak_ptr conn_weak) override; void GetInitialAssignments( - NetworkConnection& conn, + INetworkConnection& conn, std::vector>* msgs) override; void ApplyInitialAssignments( - NetworkConnection& conn, llvm::ArrayRef> msgs, + INetworkConnection& conn, llvm::ArrayRef> msgs, bool new_server, std::vector>* out_msgs) override; @@ -197,20 +198,20 @@ class Storage : public IStorage { wpi::Logger& m_logger; void ProcessIncomingEntryAssign(std::shared_ptr msg, - NetworkConnection* conn); + INetworkConnection* conn); void ProcessIncomingEntryUpdate(std::shared_ptr msg, - NetworkConnection* conn); + INetworkConnection* conn); void ProcessIncomingFlagsUpdate(std::shared_ptr msg, - NetworkConnection* conn); + INetworkConnection* conn); void ProcessIncomingEntryDelete(std::shared_ptr msg, - NetworkConnection* conn); + INetworkConnection* conn); void ProcessIncomingClearEntries(std::shared_ptr msg, - NetworkConnection* conn); + INetworkConnection* conn); void ProcessIncomingExecuteRpc(std::shared_ptr msg, - NetworkConnection* conn, - std::weak_ptr conn_weak); + INetworkConnection* conn, + std::weak_ptr conn_weak); void ProcessIncomingRpcResponse(std::shared_ptr msg, - NetworkConnection* conn); + INetworkConnection* conn); bool GetPersistentEntries( bool periodic, diff --git a/src/test/native/cpp/MockDispatcher.h b/src/test/native/cpp/MockDispatcher.h index 7966ccbc1d..20551f254a 100644 --- a/src/test/native/cpp/MockDispatcher.h +++ b/src/test/native/cpp/MockDispatcher.h @@ -17,8 +17,8 @@ namespace nt { class MockDispatcher : public IDispatcher { public: MOCK_METHOD3(QueueOutgoing, - void(std::shared_ptr msg, NetworkConnection* only, - NetworkConnection* except)); + void(std::shared_ptr msg, INetworkConnection* only, + INetworkConnection* except)); }; } // namespace nt diff --git a/src/test/native/cpp/MockNetworkConnection.h b/src/test/native/cpp/MockNetworkConnection.h new file mode 100644 index 0000000000..51d3ecddde --- /dev/null +++ b/src/test/native/cpp/MockNetworkConnection.h @@ -0,0 +1,33 @@ +/*----------------------------------------------------------------------------*/ +/* Copyright (c) FIRST 2017. All Rights Reserved. */ +/* Open Source Software - may be modified and shared by FRC teams. The code */ +/* must be accompanied by the FIRST BSD license file in the root directory of */ +/* the project. */ +/*----------------------------------------------------------------------------*/ + +#ifndef NT_TEST_MOCKNETWORKCONNECTION_H_ +#define NT_TEST_MOCKNETWORKCONNECTION_H_ + +#include "gmock/gmock.h" + +#include "INetworkConnection.h" + +namespace nt { + +class MockNetworkConnection : public INetworkConnection { + public: + MOCK_CONST_METHOD0(info, ConnectionInfo()); + + MOCK_METHOD1(QueueOutgoing, void(std::shared_ptr msg)); + MOCK_METHOD1(PostOutgoing, void(bool keep_alive)); + + MOCK_CONST_METHOD0(proto_rev, unsigned int()); + MOCK_METHOD1(set_proto_rev, void(unsigned int proto_rev)); + + MOCK_CONST_METHOD0(state, State()); + MOCK_METHOD1(set_state, void(State state)); +}; + +} // namespace nt + +#endif // NT_TEST_MOCKNETWORKCONNECTION_H_ diff --git a/src/test/native/cpp/StorageTest.cpp b/src/test/native/cpp/StorageTest.cpp index 5b5aa7f0c6..f61b9735d8 100644 --- a/src/test/native/cpp/StorageTest.cpp +++ b/src/test/native/cpp/StorageTest.cpp @@ -8,12 +8,13 @@ #include "StorageTest.h" #include "Storage.h" -#include "gtest/gtest.h" #include "gmock/gmock.h" +#include "gtest/gtest.h" #include "llvm/raw_ostream.h" #include "support/raw_istream.h" #include "MessageMatcher.h" +#include "MockNetworkConnection.h" #include "TestPrinters.h" #include "ValueMatcher.h" @@ -553,7 +554,7 @@ TEST_P(StorageTestPersistent, SavePersistent) { llvm::raw_svector_ostream oss(buf); storage.SavePersistent(oss, false); llvm::StringRef out = oss.str(); - //fputs(out.c_str(), stderr); + // fputs(out.c_str(), stderr); llvm::StringRef line, rem = out; std::tie(line, rem) = rem.split('\n'); ASSERT_EQ("[NetworkTables Storage 3.0]", line); @@ -650,8 +651,7 @@ TEST_P(StorageTestEmpty, LoadPersistentEmptyName) { warn.Warn(line, msg); }; - wpi::raw_mem_istream iss( - "[NetworkTables Storage 3.0]\nboolean \"\"=true\n"); + wpi::raw_mem_istream iss("[NetworkTables Storage 3.0]\nboolean \"\"=true\n"); EXPECT_TRUE(storage.LoadPersistent(iss, warn_func)); EXPECT_TRUE(entries().empty()); EXPECT_TRUE(idmap().empty()); @@ -873,7 +873,7 @@ TEST_P(StorageTestEmpty, LoadPersistentWarn) { } TEST_P(StorageTestEmpty, ProcessIncomingEntryAssign) { - std::shared_ptr conn; + auto conn = std::make_shared(); auto value = Value::MakeDouble(1.0); if (GetParam()) { // id assign message reply generated on the server