From 3d95a28e52c56df1d39a28274778d85ce81d46e9 Mon Sep 17 00:00:00 2001
From: Nicolas Pope <nwpope@utu.fi>
Date: Wed, 3 Apr 2019 19:33:55 +0300
Subject: [PATCH] Update socket to use mockable wrappers

---
 CMakeLists.txt               | 36 ++++++++---------------
 cmake/ftl_paths.cmake        | 16 ++++++++++
 net/cpp/src/net_internal.hpp | 25 ++++++++++++++++
 net/cpp/src/socket.cpp       | 27 +++--------------
 net/cpp/test/CMakeLists.txt  | 57 ++++++++++++++++++++----------------
 net/cpp/test/socket_unit.cpp | 12 +++++---
 6 files changed, 97 insertions(+), 76 deletions(-)
 create mode 100644 cmake/ftl_paths.cmake
 create mode 100644 net/cpp/src/net_internal.hpp

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 02b574f18..4b9c3dd6f 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -20,6 +20,8 @@ find_package( URIParser REQUIRED )
 find_package( MsgPack REQUIRED )
 find_package( LibSGM )
 
+# Readline library is not required on Windows
+# May also entirely remove dependence on this... it should be optional at least.
 if (NOT WIN32)
 find_library( READLINE_LIBRARY NAMES readline libreadline )
 if (NOT READLINE_LIBRARY)
@@ -43,12 +45,14 @@ set(CMAKE_CUDA_FLAGS_RELEASE "")
 set(HAVE_CUDA TRUE)
 include_directories(${CMAKE_CUDA_TOOLKIT_INCLUDE_DIRECTORIES})
 
+# Some kind of fix for nvcc and -pthread problem on Linux
 set_property(TARGET Threads::Threads
                  PROPERTY INTERFACE_COMPILE_OPTIONS $<$<COMPILE_LANGUAGE:CUDA>:-Xcompiler -pthread>
                                                     "$<$<NOT:$<COMPILE_LANGUAGE:CUDA>>:-pthread>")
 
 endif ()
 
+# Incase find_package failed, manually look for msgpack
 if (NOT MsgPack_FOUND)
 	check_include_file_cxx("msgpack.hpp" MSGPACK_FOUND)
 	if (NOT MSGPACK_FOUND)
@@ -63,9 +67,11 @@ else()
 		    PATH_SUFFIXES include
 		)
 	endif()
+	# TODO(nick) Create imported target.
 	include_directories(${MSGPACK_INCLUDE_DIRS})
 endif()
 
+# Only need libuuid on Linux, Windows has its own
 if (NOT WIN32)
 check_include_file("uuid/uuid.h" UUID_FOUND)
 if (NOT UUID_FOUND)
@@ -73,9 +79,9 @@ if (NOT UUID_FOUND)
 endif()
 find_library(UUID_LIBRARIES NAMES uuid libuuid)
 else()
-
 endif()
 
+# Optional source problem check
 find_program(CPPCHECK_FOUND cppcheck)
 if (CPPCHECK_FOUND)
 	message(STATUS "Found cppcheck: will perform source checks")
@@ -85,43 +91,27 @@ endif()
 include_directories(${PROJECT_SOURCE_DIR}/common/cpp/include)
 
 include(git_version)
+include(ftl_paths)
 
-if (WIN32)
-	file(TO_CMAKE_PATH "$ENV{USERPROFILE}" ENV_USERPROFILE)
-	set(FTL_LOCAL_CONFIG_ROOT "\"${ENV_USERPROFILE}/AppData/ftl\"")
-	message(STATUS "Config root is ${FTL_LOCAL_CONFIG_ROOT}")
-	set(FTL_LOCAL_CACHE_ROOT "\"$ENV{USERPROFILE}/AppData/ftl\"")
-	set(FTL_LOCAL_DATA_ROOT "\"$ENV{USERPROFILE}/AppData/ftl\"")
+if (WIN32) # TODO(nick) Should do based upon compiler (VS)
 	add_definitions(-DWIN32)
-
 	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /std:c++17")
 	set(CMAKE_CXX_FLAGS_DEBUG "-D_DEBUG -Wall")
 	set(CMAKE_CXX_FLAGS_RELEASE "/O2")
-endif (WIN32)
-
-if (UNIX)
+else()
 	add_definitions(-DUNIX)
-	
-	set(FTL_LOCAL_CONFIG_ROOT "\"$ENV{HOME}/.config/ftl\"")
-	message(STATUS "Config root is ${FTL_LOCAL_CONFIG_ROOT}")
-	set(FTL_LOCAL_CACHE_ROOT "\"$ENV{HOME}/.cache/ftl\"")
-	set(FTL_LOCAL_DATA_ROOT "\"$ENV{HOME}/.local/share/ftl\"")
-	
-	set(FTL_GLOBAL_CONFIG_ROOT "\"${CMAKE_INSTALL_PREFIX}/share/ftl\"")
-	message(STATUS "Global config root is ${FTL_GLOBAL_CONFIG_ROOT}")
-	set(FTL_GLOBAL_CACHE_ROOT "\"${CMAKE_INSTALL_PREFIX}/share/ftl\"")
-	set(FTL_GLOBAL_DATA_ROOT "\"${CMAKE_INSTALL_PREFIX}/share/ftl\"")
-
 	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++17")
 	set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -D_DEBUG -pg -Wall")
 	set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -O3 -msse3 -mfpmath=sse")
-endif (UNIX)
+endif()
 
 SET(CMAKE_USE_RELATIVE_PATHS ON)
 
 add_subdirectory(net)
 add_subdirectory(cv-node)
 
+### Generate Build Configuration Files =========================================
+
 configure_file(${CMAKE_SOURCE_DIR}/common/cpp/include/ftl/config.h.in
                ${CMAKE_SOURCE_DIR}/common/cpp/include/ftl/config.h
 )
diff --git a/cmake/ftl_paths.cmake b/cmake/ftl_paths.cmake
new file mode 100644
index 000000000..c8c82cbcf
--- /dev/null
+++ b/cmake/ftl_paths.cmake
@@ -0,0 +1,16 @@
+if (WIN32)
+	file(TO_CMAKE_PATH "$ENV{USERPROFILE}" ENV_USERPROFILE)
+	set(FTL_LOCAL_CONFIG_ROOT "\"${ENV_USERPROFILE}/AppData/ftl\"")
+	message(STATUS "Config root is ${FTL_LOCAL_CONFIG_ROOT}")
+	set(FTL_LOCAL_CACHE_ROOT "\"$ENV{USERPROFILE}/AppData/ftl\"")
+	set(FTL_LOCAL_DATA_ROOT "\"$ENV{USERPROFILE}/AppData/ftl\"")
+else()
+	set(FTL_LOCAL_CONFIG_ROOT "\"$ENV{HOME}/.config/ftl\"")
+	message(STATUS "Config root is ${FTL_LOCAL_CONFIG_ROOT}")
+	set(FTL_LOCAL_CACHE_ROOT "\"$ENV{HOME}/.cache/ftl\"")
+	set(FTL_LOCAL_DATA_ROOT "\"$ENV{HOME}/.local/share/ftl\"")
+	set(FTL_GLOBAL_CONFIG_ROOT "\"${CMAKE_INSTALL_PREFIX}/share/ftl\"")
+	message(STATUS "Global config root is ${FTL_GLOBAL_CONFIG_ROOT}")
+	set(FTL_GLOBAL_CACHE_ROOT "\"${CMAKE_INSTALL_PREFIX}/share/ftl\"")
+	set(FTL_GLOBAL_DATA_ROOT "\"${CMAKE_INSTALL_PREFIX}/share/ftl\"")
+endif()
diff --git a/net/cpp/src/net_internal.hpp b/net/cpp/src/net_internal.hpp
new file mode 100644
index 000000000..d6bcfd739
--- /dev/null
+++ b/net/cpp/src/net_internal.hpp
@@ -0,0 +1,25 @@
+#ifndef _FTL_NET_INTERNAL_HPP_
+#define _FTL_NET_INTERNAL_HPP_
+
+namespace ftl { namespace net { namespace internal {
+#ifdef TEST_MOCKS
+#ifdef WIN32
+	int recv(SOCKET sd, char *buf, int n, int f);
+	int send(SOCKET sd, const char *v, int cnt, int flags);
+#else
+	ssize_t recv(int sd, void *buf, size_t n, int f);
+	ssize_t writev(int sd, const struct iovec *v, int cnt);
+#endif
+#else
+#ifdef WIN32
+	inline int recv(SOCKET sd, char *buf, int n, int f) { return ::recv(sd,buf,n,f); }
+	inline int send(SOCKET sd, const char *v, int cnt, int flags) { return ::send(sd,v,cnt,flags); }
+#else
+	inline ssize_t recv(int sd, void *buf, size_t n, int f) { return ::recv(sd,buf,n,f); }
+	inline ssize_t writev(int sd, const struct iovec *v, int cnt) { return ::writev(sd,v,cnt); }
+#endif
+#endif
+}}}
+
+#endif  // _FTL_NET_INTERNAL_HPP_
+
diff --git a/net/cpp/src/socket.cpp b/net/cpp/src/socket.cpp
index 914806b66..3e4d18220 100644
--- a/net/cpp/src/socket.cpp
+++ b/net/cpp/src/socket.cpp
@@ -6,6 +6,7 @@
 #include <ftl/uri.hpp>
 #include <ftl/net/socket.hpp>
 #include <ftl/net/ws_internal.hpp>
+#include "net_internal.hpp"
 
 #ifndef WIN32
 #include <unistd.h>
@@ -35,26 +36,6 @@ using ftl::URI;
 using ftl::net::ws_connect;
 using namespace std;
 
-namespace ftl { namespace net { namespace internal {
-#ifdef TEST_MOCKS
-#ifdef WIN32
-	extern int recv(SOCKET sd, char *buf, int n, int f);
-	extern int send(SOCKET sd, const char *v, int cnt, int flags);
-#else
-	extern ssize_t recv(int sd, void *buf, size_t n, int f);
-	extern ssize_t writev(int sd, const struct iovec *v, int cnt);
-#endif
-#else
-#ifdef WIN32
-	inline int recv(SOCKET sd, char *buf, int n, int f) { return ::recv(sd,buf,n,f); }
-	inline int send(SOCKET sd, const char *v, int cnt, int flags) { return ::send(sd,v,cnt,flags); }
-#else
-	inline ssize_t recv(int sd, void *buf, size_t n, int f) { return ::recv(sd,buf,n,f); }
-	inline ssize_t writev(int sd, const struct iovec *v, int cnt) { return ::writev(sd,v,cnt); }
-#endif
-#endif
-}}}
-
 /*static std::string hexStr(const std::string &s)
 {
 	const char *data = s.data();
@@ -310,7 +291,7 @@ bool Socket::data() {
 			return false;
 		}
 
-		const int rc = recv(sock_, buffer_+pos_, n, 0);
+		const int rc = ftl::net::internal::recv(sock_, buffer_+pos_, n, 0);
 
 		if (rc > 0) {
 			pos_ += static_cast<size_t>(rc);
@@ -460,10 +441,10 @@ int Socket::_send() {
 	// TODO(nick) Use WSASend instead
 	int c = 0;
 	for (auto v : send_vec_) {
-		c += ::send(sock_, (char*)v.iov_base, v.iov_len, 0);
+		c += ftl::net::internal::send(sock_, (char*)v.iov_base, v.iov_len, 0);
 	}
 #else
-	int c = ::writev(sock_, send_vec_.data(), send_vec_.size());
+	int c = ftl::net::internal::writev(sock_, send_vec_.data(), send_vec_.size());
 #endif
 	send_vec_.clear();
 	return c;
diff --git a/net/cpp/test/CMakeLists.txt b/net/cpp/test/CMakeLists.txt
index 391ad05e3..4b5777180 100644
--- a/net/cpp/test/CMakeLists.txt
+++ b/net/cpp/test/CMakeLists.txt
@@ -1,49 +1,54 @@
+### Protocol Unit ##############################################################
 add_executable(protocol_unit
 	./tests.cpp
 	./protocol_unit.cpp
 )
 target_link_libraries(protocol_unit glog::glog)
 
+### Socket Unit ################################################################
 add_executable(socket_unit
 	./tests.cpp
 	../src/ws_internal.cpp
 	./socket_unit.cpp
 )
-target_include_directories(socket_unit PUBLIC ${PROJECT_SOURCE_DIR}/include)
-target_link_libraries(socket_unit ${URIPARSER_LIBRARIES} glog::glog)
+target_link_libraries(socket_unit
+	${URIPARSER_LIBRARIES}
+	glog::glog)
 
+### URI ########################################################################
 add_executable(uri_unit
 	./tests.cpp
-	./uri_unit.cpp
-)
+	./uri_unit.cpp)
+target_link_libraries(uri_unit
+	${URIPARSER_LIBRARIES})
 
-target_link_libraries(uri_unit ${URIPARSER_LIBRARIES})
+### P2P Base Unit ##############################################################
+# TODO(nick) Actually make this a unit test
 
 add_executable(p2p_base_unit
 	./tests.cpp
-	./p2p_base_unit.cpp
-	../src/p2p.cpp
-	../src/socket.cpp
-	../src/dispatcher.cpp
-	../src/protocol.cpp
-	../src/net.cpp
-	../src/listener.cpp
-	../src/ws_internal.cpp
-)
-
-target_link_libraries(p2p_base_unit ${URIPARSER_LIBRARIES} gflags glog::glog ${UUID_LIBRARIES})
-
+	./p2p_base_unit.cpp)
+add_dependencies(p2p_base_unit ftlnet)
+target_link_libraries(p2p_base_unit
+	ftlnet
+	${URIPARSER_LIBRARIES}
+	gflags
+	glog::glog
+	${UUID_LIBRARIES})
+
+### Net Integration ############################################################
 add_executable(net_integration
 	./tests.cpp
-	./net_integration.cpp
-	../src/socket.cpp
-	../src/dispatcher.cpp
-	../src/listener.cpp
-	../src/protocol.cpp
-	../src/net.cpp
-	../src/ws_internal.cpp
-)
-target_link_libraries(net_integration ${URIPARSER_LIBRARIES} glog::glog)
+	./net_integration.cpp)
+add_dependencies(net_integration ftlnet)
+target_link_libraries(net_integration
+	ftlnet
+	${URIPARSER_LIBRARIES}
+	glog::glog)
+
+
+
+
 
 add_test(URIUnitTest uri_unit)
 add_test(ProtocolUnitTest protocol_unit)
diff --git a/net/cpp/test/socket_unit.cpp b/net/cpp/test/socket_unit.cpp
index fc76725e7..36737e3f2 100644
--- a/net/cpp/test/socket_unit.cpp
+++ b/net/cpp/test/socket_unit.cpp
@@ -6,6 +6,10 @@
 #include <ftl/net/protocol.hpp>
 #include <ftl/net/socket.hpp>
 
+/* Allow socket functions to be mocked */
+#define TEST_MOCKS
+#include "../src/net_internal.hpp"
+
 using ftl::net::Socket;
 
 #ifdef WIN32
@@ -59,9 +63,9 @@ void fake_send(int sd, uint32_t  service, const std::string &data) {
 }
 
 #ifdef WIN32
-extern int recv(SOCKET sd, char *buf, int n, int f) {
+int ftl::net::internal::recv(SOCKET sd, char *buf, int n, int f) {
 #else
-extern ssize_t recv(int sd, void *buf, size_t n, int f) {
+ssize_t ftl::net::internal::recv(int sd, void *buf, size_t n, int f) {
 #endif
 	if (fakedata.count(sd) == 0) {
 		std::cout << "Unrecognised socket" << std::endl;
@@ -76,14 +80,14 @@ extern ssize_t recv(int sd, void *buf, size_t n, int f) {
 }
 
 #ifdef WIN32
-extern int send(SOCKET sd, const char *v, int cnt, int flags) {
+int ftl::net::internal::send(SOCKET sd, const char *v, int cnt, int flags) {
 	int len = cnt;
 	// TODO(nick) merge multiple sends
 	fakedata[sd] = std::string(v, len);
 	return len;
 }
 #else
-extern ssize_t writev(int sd, const struct iovec *v, int cnt) {
+ssize_t ftl::net::internal::writev(int sd, const struct iovec *v, int cnt) {
 	size_t len = 0; //v[0].iov_len+v[1].iov_len;
 	char buf[1000];
 	char *bufp = &buf[0];
-- 
GitLab