From ddee2793963e139fb02bb9bff5305b56c77bd4bb Mon Sep 17 00:00:00 2001 From: Tobias Peters Date: Sat, 8 Jun 2024 14:37:49 +0000 Subject: [PATCH 01/28] prepare gtest for naming service test --- .gitignore | 2 +- CMakeLists.txt | 11 ++ nelns/CMakeLists.txt | 3 + nelns/naming_service/CMakeLists.txt | 6 +- nelns/tests/CMakeLists.txt | 14 ++ nelns/tests/naming_service.test.cpp | 6 + .../ryzom_naming_service.cpp | 161 +----------------- 7 files changed, 39 insertions(+), 164 deletions(-) create mode 100644 nelns/tests/CMakeLists.txt create mode 100644 nelns/tests/naming_service.test.cpp diff --git a/.gitignore b/.gitignore index dfc254b44a..dacdb0c16c 100644 --- a/.gitignore +++ b/.gitignore @@ -160,11 +160,11 @@ snowballs/build/* ryzom/build/* build/* build-2010/* -build/* install/* build_* install_* nel/tools/build_gamedata/configuration/buildsite.py +cmake-build-* # Linux nel compile nel/build/nel-config diff --git a/CMakeLists.txt b/CMakeLists.txt index a4fc01bd3c..05097e4108 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -323,6 +323,17 @@ ENDIF() IF(WITH_NEL) IF(WITH_NEL_TESTS) FIND_PACKAGE(CppTest) + + ENABLE_TESTING() + INCLUDE(FetchContent) + FetchContent_Declare( + googletest + URL https://github.com/google/googletest/archive/a7f443b80b105f940225332ed3c31f2790092f47.zip + EXCLUDE_FROM_ALL + ) + SET(gtest_force_shared_crt ON CACHE BOOL "" FORCE) + FetchContent_MakeAvailable(googletest) + INCLUDE(GoogleTest) ENDIF() IF(HUNTER_ENABLED) diff --git a/nelns/CMakeLists.txt b/nelns/CMakeLists.txt index d451111185..36f8f27599 100644 --- a/nelns/CMakeLists.txt +++ b/nelns/CMakeLists.txt @@ -4,6 +4,9 @@ IF(WITH_NELNS_SERVER) ADD_SUBDIRECTORY(admin_executor_service) ADD_SUBDIRECTORY(admin_service) ADD_SUBDIRECTORY(naming_service) + IF (WITH_NEL_TESTS) + ADD_SUBDIRECTORY(tests) + ENDIF () ADD_SUBDIRECTORY(login_service) ADD_SUBDIRECTORY(welcome_service) ENDIF() diff --git a/nelns/naming_service/CMakeLists.txt b/nelns/naming_service/CMakeLists.txt index 596bb5b48f..f0f728663d 100644 --- a/nelns/naming_service/CMakeLists.txt +++ b/nelns/naming_service/CMakeLists.txt @@ -1,6 +1,6 @@ -FILE(GLOB SRC *.cpp *.h) - -ADD_EXECUTABLE(naming_service WIN32 ${SRC}) +ADD_EXECUTABLE(naming_service WIN32 + naming_service.cpp +) TARGET_LINK_LIBRARIES(naming_service nelmisc diff --git a/nelns/tests/CMakeLists.txt b/nelns/tests/CMakeLists.txt new file mode 100644 index 0000000000..ad0f8a5eb2 --- /dev/null +++ b/nelns/tests/CMakeLists.txt @@ -0,0 +1,14 @@ +set(TEST_NAME "naming_service_test") + +add_executable("${TEST_NAME}" + naming_service.test.cpp +) +target_link_libraries("${TEST_NAME}" + GTest::gtest_main + nelmisc + nelnet +) +gtest_discover_tests("${TEST_NAME}" + EXTRA_ARGS "--gtest_output=xml:${CMAKE_CURRENT_BINARY_DIR}/reports/junit-${TEST_NAME}.xml" +) + diff --git a/nelns/tests/naming_service.test.cpp b/nelns/tests/naming_service.test.cpp new file mode 100644 index 0000000000..32d91842af --- /dev/null +++ b/nelns/tests/naming_service.test.cpp @@ -0,0 +1,6 @@ +#include + +TEST(CNamingService, BasicAssertions) { + EXPECT_STRNE("hello", "world"); + EXPECT_EQ(7 * 6, 42); +} diff --git a/ryzom/server/src/ryzom_naming_service/ryzom_naming_service.cpp b/ryzom/server/src/ryzom_naming_service/ryzom_naming_service.cpp index be8a34e9d1..5feb7e19c0 100644 --- a/ryzom/server/src/ryzom_naming_service/ryzom_naming_service.cpp +++ b/ryzom/server/src/ryzom_naming_service/ryzom_naming_service.cpp @@ -97,167 +97,8 @@ void foo() admin_modules_forceLink(); } -/** - * Manager for services instances - * (Moved from the TICKS to the NS) - * Implementable with layer 5, here implemented in NS (layer 3) - * \author Olivier Cado - * \author Nevrax France - * \date 2003 - */ -class CServiceInstanceManager -{ -public: - - /// Constructor - CServiceInstanceManager(); - - /** Add the name of a service which must not be duplicated - * If uniqueOnShard is true, only one service is allowed. - * If uniqueOnShard is false, one service is allowed by physical machine. - */ - void addUniqueService( const std::string& serviceName, bool uniqueOnShard ) - { - _UniqueServices.insert( std::make_pair( serviceName, uniqueOnShard ) ); - } - - /// Check if a service is allowed to start (if so, add it) - bool queryStartService( const std::string& serviceName, TServiceId serviceId, const std::vector &addr, string& reason ); - - /// Release a service instance - void releaseService( NLNET::TServiceId serviceId ); - - /// Display information - void displayInfo( NLMISC::CLog *log = NLMISC::InfoLog ) const; - - /// Make all controlled services quit - void killAllServices(); - -private: - - /// List of restricted services - std::map< std::string, bool > _UniqueServices; - - /// List of granted (online) services - std::set< TServiceId > _OnlineServices; -}; - - CServiceInstanceManager *SIMInstance = NULL; - -/* - * Constructor - */ -CServiceInstanceManager::CServiceInstanceManager() -{ - nlassert( ! SIMInstance ); - SIMInstance = this; - - // Note: addCallbackArray() done in CRangeMirrorManager::init() -} - - -/* - * Check if a service is allowed to start. Answer with a GSTS (Grant Start Service) message - */ -bool CServiceInstanceManager::queryStartService( const std::string& serviceName, TServiceId serviceId, const vector &addr, string& reason ) -{ - bool grantStarting = true; - std::map< std::string, bool >::iterator ius = _UniqueServices.find( serviceName ); - if ( ius != _UniqueServices.end() ) - { - // Service is restricted - set< TServiceId >::iterator ios; - bool uniqueOnShard = (*ius).second; - for ( ios=_OnlineServices.begin(); ios!=_OnlineServices.end(); ++ios ) - { - string name = getServiceName( *ios ); - if ( name == serviceName ) - { - if ( uniqueOnShard ) - { - // Only one service by shard is allowed => deny - grantStarting = false; - reason = toString( "Service %s already found as %hu, must be unique on shard", serviceName.c_str(), ios->get() ); - nlinfo( reason.c_str() ); - break; - } - else - { - // Only one service by physical machine is allowed - - // Implementation for layer5 - //TSockId hostid1, hostid2; - /*CCallbackNetBase *cnb1 = CUnifiedNetwork::getInstance()->getNetBase( serviceId, hostid1 ); - CCallbackNetBase *cnb2 = CUnifiedNetwork::getInstance()->getNetBase( *ios, hostid2 ); - if ( cnb1->hostAddress( hostid1 ).internalIPAddress() == cnb2->hostAddress( hostid2 ).internalIPAddress() )*/ - - // Implementation for NS - if ( addr[0].getAddress() == getHostAddress( *ios ).getAddress() ) - { - grantStarting = false; - reason = toString( "Service %s already found as %hu on same machine", serviceName.c_str(), ios->get() ); - nlinfo( reason.c_str() ); - break; - } - } - } - } - } - - if ( grantStarting ) - { - _OnlineServices.insert( serviceId ); - } - return grantStarting; -} - - -/* - * Release a service instance - */ -void CServiceInstanceManager::releaseService( NLNET::TServiceId serviceId ) -{ - _OnlineServices.erase( serviceId ); // not a problem if not found -} - - -/* - * Display information - */ -void CServiceInstanceManager::displayInfo( NLMISC::CLog *log ) const -{ - log->displayNL( "Restricted services:" ); - std::map< std::string, bool >::const_iterator ius; - for ( ius=_UniqueServices.begin(); ius!=_UniqueServices.end(); ++ius ) - { - log->displayNL( "%s -> only one per %s", (*ius).first.c_str(), (*ius).second?"shard":"machine" ); - } - log->displayNL( "Online registered services:" ); - std::set< TServiceId >::const_iterator ios; - for ( ios=_OnlineServices.begin(); ios!=_OnlineServices.end(); ++ios ) - { - log->displayNL( "%s", CUnifiedNetwork::getInstance()->getServiceUnifiedName( *ios ).c_str() ); - } -} - - -/* - * Make all controlled services quit - */ -void CServiceInstanceManager::killAllServices() -{ - // Send to all known online services - std::set< TServiceId >::const_iterator ios; - for ( ios=_OnlineServices.begin(); ios!=_OnlineServices.end(); ++ios ) - { - doUnregisterService( (TServiceId)(*ios) ); - } -} - - - // // Variables // @@ -267,7 +108,7 @@ list RegisteredServices; /// List of all registred services uint16 MinBasePort = 30000; /// Ports begin at 51000 uint16 MaxBasePort = 30100; /// (note: in this implementation there can be no more than 100 services) -const TServiceId BaseSId(128); /// Allocated SIds begin at 128 (except for Agent Service) +/// Allocated SIds begin at 128 (except for Agent Service) const TTime UnregisterTimeout = 10000; /// After 10s we remove an unregister service if every server didn't ACK the message From 1a452285dc8663a14a0e930920763a04a2b9aede Mon Sep 17 00:00:00 2001 From: Tobias Peters Date: Sat, 8 Jun 2024 15:31:02 +0000 Subject: [PATCH 02/28] extract CServiceEntry into separate file --- nelns/naming_service/naming_service.cpp | 23 ++-------------- nelns/naming_service/service_entry.h | 35 +++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 21 deletions(-) create mode 100644 nelns/naming_service/service_entry.h diff --git a/nelns/naming_service/naming_service.cpp b/nelns/naming_service/naming_service.cpp index b10856088d..6dc4a6afda 100644 --- a/nelns/naming_service/naming_service.cpp +++ b/nelns/naming_service/naming_service.cpp @@ -48,6 +48,8 @@ #include "nel/net/service.h" #include "nel/net/module_manager.h" +#include "service_entry.h" + // // Namespaces // @@ -70,27 +72,6 @@ NLMISC_COMMAND(test, "none", "none") } -// -// Structures -// - -struct CServiceEntry -{ - CServiceEntry (TSockId sock, const vector &a, const string &n, TServiceId s) : SockId(sock), Addr(a), Name(n), SId (s), WaitingUnregistration(false) { } - - TSockId SockId; // the connection between the service and the naming service - vector Addr; // address to send to the service who wants to lookup this service - // it s possible to have more than one addr, anyway, the naming service - // will send good address depending of the sub net address of the service - string Name; // name of the service - TServiceId SId; // id of the service - - bool WaitingUnregistration; // true if this service is in unregistration process (wait other service ACK) - TTime WaitingUnregistrationTime; // time of the beginning of the inregistration process - list WaitingUnregistrationServices; // list of service that we wait the answer -}; - - // Helper that emulates layer5's send() //void sendToService( uint16 sid, CMessage& msgout ); diff --git a/nelns/naming_service/service_entry.h b/nelns/naming_service/service_entry.h new file mode 100644 index 0000000000..3a440d4b67 --- /dev/null +++ b/nelns/naming_service/service_entry.h @@ -0,0 +1,35 @@ +#ifndef NL_SERVICE_ENTRY_H +#define NL_SERVICE_ENTRY_H + +#include +#include + +#include +#include +#include +#include + +struct CServiceEntry +{ + CServiceEntry(NLNET::TSockId sock, const std::vector &a, const std::string &n, NLNET::TServiceId s) + : SockId(sock) + , Addr(a) + , Name(n) + , SId(s) + , WaitingUnregistration(false) + { + } + + NLNET::TSockId SockId; // the connection between the service and the naming service + std::vector Addr; // address to send to the service who wants to lookup this service + // it s possible to have more than one addr, anyway, the naming service + // will send good address depending of the sub net address of the service + std::string Name; // name of the service + NLNET::TServiceId SId; // id of the service + + bool WaitingUnregistration; // true if this service is in unregistration process (wait other service ACK) + NLMISC::TTime WaitingUnregistrationTime; // time of the beginning of the inregistration process + std::list WaitingUnregistrationServices; // list of service that we wait the answer +}; + +#endif // NL_SERVICE_ENTRY_H From b0fe35e11f6fec69317563a7b62973608ffff19c Mon Sep 17 00:00:00 2001 From: Tobias Peters Date: Sat, 8 Jun 2024 16:01:27 +0000 Subject: [PATCH 03/28] revert changes to unrelated files --- .../ryzom_naming_service.cpp | 161 +++++++++++++++++- 1 file changed, 160 insertions(+), 1 deletion(-) diff --git a/ryzom/server/src/ryzom_naming_service/ryzom_naming_service.cpp b/ryzom/server/src/ryzom_naming_service/ryzom_naming_service.cpp index 5feb7e19c0..be8a34e9d1 100644 --- a/ryzom/server/src/ryzom_naming_service/ryzom_naming_service.cpp +++ b/ryzom/server/src/ryzom_naming_service/ryzom_naming_service.cpp @@ -97,8 +97,167 @@ void foo() admin_modules_forceLink(); } +/** + * Manager for services instances + * (Moved from the TICKS to the NS) + * Implementable with layer 5, here implemented in NS (layer 3) + * \author Olivier Cado + * \author Nevrax France + * \date 2003 + */ +class CServiceInstanceManager +{ +public: + + /// Constructor + CServiceInstanceManager(); + + /** Add the name of a service which must not be duplicated + * If uniqueOnShard is true, only one service is allowed. + * If uniqueOnShard is false, one service is allowed by physical machine. + */ + void addUniqueService( const std::string& serviceName, bool uniqueOnShard ) + { + _UniqueServices.insert( std::make_pair( serviceName, uniqueOnShard ) ); + } + + /// Check if a service is allowed to start (if so, add it) + bool queryStartService( const std::string& serviceName, TServiceId serviceId, const std::vector &addr, string& reason ); + + /// Release a service instance + void releaseService( NLNET::TServiceId serviceId ); + + /// Display information + void displayInfo( NLMISC::CLog *log = NLMISC::InfoLog ) const; + + /// Make all controlled services quit + void killAllServices(); + +private: + + /// List of restricted services + std::map< std::string, bool > _UniqueServices; + + /// List of granted (online) services + std::set< TServiceId > _OnlineServices; +}; + + CServiceInstanceManager *SIMInstance = NULL; + +/* + * Constructor + */ +CServiceInstanceManager::CServiceInstanceManager() +{ + nlassert( ! SIMInstance ); + SIMInstance = this; + + // Note: addCallbackArray() done in CRangeMirrorManager::init() +} + + +/* + * Check if a service is allowed to start. Answer with a GSTS (Grant Start Service) message + */ +bool CServiceInstanceManager::queryStartService( const std::string& serviceName, TServiceId serviceId, const vector &addr, string& reason ) +{ + bool grantStarting = true; + std::map< std::string, bool >::iterator ius = _UniqueServices.find( serviceName ); + if ( ius != _UniqueServices.end() ) + { + // Service is restricted + set< TServiceId >::iterator ios; + bool uniqueOnShard = (*ius).second; + for ( ios=_OnlineServices.begin(); ios!=_OnlineServices.end(); ++ios ) + { + string name = getServiceName( *ios ); + if ( name == serviceName ) + { + if ( uniqueOnShard ) + { + // Only one service by shard is allowed => deny + grantStarting = false; + reason = toString( "Service %s already found as %hu, must be unique on shard", serviceName.c_str(), ios->get() ); + nlinfo( reason.c_str() ); + break; + } + else + { + // Only one service by physical machine is allowed + + // Implementation for layer5 + //TSockId hostid1, hostid2; + /*CCallbackNetBase *cnb1 = CUnifiedNetwork::getInstance()->getNetBase( serviceId, hostid1 ); + CCallbackNetBase *cnb2 = CUnifiedNetwork::getInstance()->getNetBase( *ios, hostid2 ); + if ( cnb1->hostAddress( hostid1 ).internalIPAddress() == cnb2->hostAddress( hostid2 ).internalIPAddress() )*/ + + // Implementation for NS + if ( addr[0].getAddress() == getHostAddress( *ios ).getAddress() ) + { + grantStarting = false; + reason = toString( "Service %s already found as %hu on same machine", serviceName.c_str(), ios->get() ); + nlinfo( reason.c_str() ); + break; + } + } + } + } + } + + if ( grantStarting ) + { + _OnlineServices.insert( serviceId ); + } + return grantStarting; +} + + +/* + * Release a service instance + */ +void CServiceInstanceManager::releaseService( NLNET::TServiceId serviceId ) +{ + _OnlineServices.erase( serviceId ); // not a problem if not found +} + + +/* + * Display information + */ +void CServiceInstanceManager::displayInfo( NLMISC::CLog *log ) const +{ + log->displayNL( "Restricted services:" ); + std::map< std::string, bool >::const_iterator ius; + for ( ius=_UniqueServices.begin(); ius!=_UniqueServices.end(); ++ius ) + { + log->displayNL( "%s -> only one per %s", (*ius).first.c_str(), (*ius).second?"shard":"machine" ); + } + log->displayNL( "Online registered services:" ); + std::set< TServiceId >::const_iterator ios; + for ( ios=_OnlineServices.begin(); ios!=_OnlineServices.end(); ++ios ) + { + log->displayNL( "%s", CUnifiedNetwork::getInstance()->getServiceUnifiedName( *ios ).c_str() ); + } +} + + +/* + * Make all controlled services quit + */ +void CServiceInstanceManager::killAllServices() +{ + // Send to all known online services + std::set< TServiceId >::const_iterator ios; + for ( ios=_OnlineServices.begin(); ios!=_OnlineServices.end(); ++ios ) + { + doUnregisterService( (TServiceId)(*ios) ); + } +} + + + // // Variables // @@ -108,7 +267,7 @@ list RegisteredServices; /// List of all registred services uint16 MinBasePort = 30000; /// Ports begin at 51000 uint16 MaxBasePort = 30100; /// (note: in this implementation there can be no more than 100 services) -/// Allocated SIds begin at 128 (except for Agent Service) +const TServiceId BaseSId(128); /// Allocated SIds begin at 128 (except for Agent Service) const TTime UnregisterTimeout = 10000; /// After 10s we remove an unregister service if every server didn't ACK the message From 4050df987784cd628d167889f4a939f7c2c3da0e Mon Sep 17 00:00:00 2001 From: Tobias Peters Date: Sat, 8 Jun 2024 16:32:40 +0000 Subject: [PATCH 04/28] apply clang tidy recommendation --- nelns/naming_service/service_entry.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nelns/naming_service/service_entry.h b/nelns/naming_service/service_entry.h index 3a440d4b67..72b4c3964d 100644 --- a/nelns/naming_service/service_entry.h +++ b/nelns/naming_service/service_entry.h @@ -11,7 +11,7 @@ struct CServiceEntry { - CServiceEntry(NLNET::TSockId sock, const std::vector &a, const std::string &n, NLNET::TServiceId s) + CServiceEntry(NLNET::TSockId sock, const std::vector &a, const std::string &n, const NLNET::TServiceId& s) : SockId(sock) , Addr(a) , Name(n) From d4e7e6696e4b9240056ec55833771671922e567c Mon Sep 17 00:00:00 2001 From: Tobias Peters Date: Sat, 8 Jun 2024 17:02:50 +0000 Subject: [PATCH 05/28] extract some helper and variables into separate files --- nelns/naming_service/CMakeLists.txt | 19 ++++++- nelns/naming_service/naming_service.cpp | 55 ++++--------------- .../nelns/naming_service/helper.cpp | 27 +++++++++ .../nelns/naming_service/helper.h | 11 ++++ .../naming_service}/service_entry.h | 2 +- .../nelns/naming_service/variables.cpp | 18 ++++++ .../nelns/naming_service/variables.h | 26 +++++++++ 7 files changed, 110 insertions(+), 48 deletions(-) create mode 100644 nelns/naming_service/nelns/naming_service/helper.cpp create mode 100644 nelns/naming_service/nelns/naming_service/helper.h rename nelns/naming_service/{ => nelns/naming_service}/service_entry.h (95%) create mode 100644 nelns/naming_service/nelns/naming_service/variables.cpp create mode 100644 nelns/naming_service/nelns/naming_service/variables.h diff --git a/nelns/naming_service/CMakeLists.txt b/nelns/naming_service/CMakeLists.txt index f0f728663d..af86f56b62 100644 --- a/nelns/naming_service/CMakeLists.txt +++ b/nelns/naming_service/CMakeLists.txt @@ -1,10 +1,23 @@ +include_directories(${CMAKE_CURRENT_SOURCE_DIR}) + +add_library(nelns_naming_service + nelns/naming_service/helper.cpp + nelns/naming_service/variables.cpp +) +add_library(nelns::ns ALIAS nelns_naming_service) + + ADD_EXECUTABLE(naming_service WIN32 - naming_service.cpp + naming_service.cpp + nelns/naming_service/helper.cpp + nelns/naming_service/helper.h ) TARGET_LINK_LIBRARIES(naming_service - nelmisc - nelnet) + nelmisc + nelnet + nelns::ns +) NL_DEFAULT_PROPS(naming_service "NeLNS, Services: Naming Service") NL_ADD_RUNTIME_FLAGS(naming_service) diff --git a/nelns/naming_service/naming_service.cpp b/nelns/naming_service/naming_service.cpp index 6dc4a6afda..ae6cfbd809 100644 --- a/nelns/naming_service/naming_service.cpp +++ b/nelns/naming_service/naming_service.cpp @@ -34,21 +34,23 @@ // Includes // -#include "nel/misc/types_nl.h" +#include #include #include -#include "nel/misc/debug.h" -#include "nel/misc/command.h" -#include "nel/misc/variable.h" -#include "nel/misc/displayer.h" +#include +#include +#include +#include -#include "nel/net/callback_server.h" -#include "nel/net/service.h" -#include "nel/net/module_manager.h" +#include +#include +#include -#include "service_entry.h" +#include +#include +#include // // Namespaces @@ -76,9 +78,6 @@ NLMISC_COMMAND(test, "none", "none") // Helper that emulates layer5's send() //void sendToService( uint16 sid, CMessage& msgout ); -// Helper that emulate layer5's getServiceName() -string getServiceName( TServiceId sid ); - // Helper that returns the first address of a service CInetAddress getHostAddress( TServiceId sid ); @@ -247,21 +246,6 @@ void CServiceInstanceManager::killAllServices() -// -// Variables -// - -list RegisteredServices; /// List of all registred services - -uint16 MinBasePort = 51000; /// Ports begin at 51000 -uint16 MaxBasePort = 52000; /// (note: in this implementation there can be no more than 1000 services) - -const TServiceId BaseSId(128); /// Allocated SIds begin at 128 (except for Agent Service) - -const TTime UnregisterTimeout = 10000; /// After 10s we remove an unregister service if every server didn't ACK the message - -CCallbackServer *CallbackServer = NULL; - // // Functions // @@ -885,23 +869,6 @@ static void cbRegisteredServices(CMessage& msgin, TSockId from, CCallbackNetBase }*/ -/* - * Helper that emulate layer5's getServiceName() - */ -string getServiceName( TServiceId sid ) -{ - list::iterator it; - for (it = RegisteredServices.begin(); it != RegisteredServices.end (); it++) - { - if ((*it).SId == sid) - { - return (*it).Name; - } - } - return ""; // not found -} - - /* * Helper that returns the first address of a service */ diff --git a/nelns/naming_service/nelns/naming_service/helper.cpp b/nelns/naming_service/nelns/naming_service/helper.cpp new file mode 100644 index 0000000000..5d1f73ef87 --- /dev/null +++ b/nelns/naming_service/nelns/naming_service/helper.cpp @@ -0,0 +1,27 @@ +#include + +#include +#include +#include + +using std::list; +using std::string; + +using NLNET::TServiceId; + +/* + * Helper that emulate layer5's getServiceName() + */ +string getServiceName( TServiceId sid ) +{ + list::iterator it; + for (it = RegisteredServices.begin(); it != RegisteredServices.end (); it++) + { + if ((*it).SId == sid) + { + return (*it).Name; + } + } + return ""; // not found +} + diff --git a/nelns/naming_service/nelns/naming_service/helper.h b/nelns/naming_service/nelns/naming_service/helper.h new file mode 100644 index 0000000000..471be8a9d1 --- /dev/null +++ b/nelns/naming_service/nelns/naming_service/helper.h @@ -0,0 +1,11 @@ +#ifndef NELNS_NAMING_SERVICE_HELPER_H +#define NELNS_NAMING_SERVICE_HELPER_H + +#include + +#include + +// Helper that emulate layer5's getServiceName() +std::string getServiceName(NLNET::TServiceId sid); + +#endif // NELNS_NAMING_SERVICE_HELPER_H diff --git a/nelns/naming_service/service_entry.h b/nelns/naming_service/nelns/naming_service/service_entry.h similarity index 95% rename from nelns/naming_service/service_entry.h rename to nelns/naming_service/nelns/naming_service/service_entry.h index 72b4c3964d..8bb59b4068 100644 --- a/nelns/naming_service/service_entry.h +++ b/nelns/naming_service/nelns/naming_service/service_entry.h @@ -11,7 +11,7 @@ struct CServiceEntry { - CServiceEntry(NLNET::TSockId sock, const std::vector &a, const std::string &n, const NLNET::TServiceId& s) + CServiceEntry(NLNET::TSockId sock, const std::vector &a, const std::string &n, const NLNET::TServiceId &s) : SockId(sock) , Addr(a) , Name(n) diff --git a/nelns/naming_service/nelns/naming_service/variables.cpp b/nelns/naming_service/nelns/naming_service/variables.cpp new file mode 100644 index 0000000000..a525acef21 --- /dev/null +++ b/nelns/naming_service/nelns/naming_service/variables.cpp @@ -0,0 +1,18 @@ +#include + +using std::list; + +using NLMISC::TTime; +using NLNET::CCallbackServer; +using NLNET::TServiceId; + +list RegisteredServices; /// List of all registred services + +uint16 MinBasePort = 51000; /// Ports begin at 51000 +uint16 MaxBasePort = 52000; /// (note: in this implementation there can be no more than 1000 services) + +const TServiceId BaseSId(128); /// Allocated SIds begin at 128 (except for Agent Service) + +const TTime UnregisterTimeout = 10000; /// After 10s we remove an unregister service if every server didn't ACK the message + +CCallbackServer *CallbackServer = nullptr; diff --git a/nelns/naming_service/nelns/naming_service/variables.h b/nelns/naming_service/nelns/naming_service/variables.h new file mode 100644 index 0000000000..7fe547e92e --- /dev/null +++ b/nelns/naming_service/nelns/naming_service/variables.h @@ -0,0 +1,26 @@ +#ifndef NELNS_NAMING_SERVICE_VARIABLES_H +#define NELNS_NAMING_SERVICE_VARIABLES_H + +#include + +#include +#include +#include +#include + +// +// Variables +// + +extern std::list RegisteredServices; /// List of all registred services + +extern uint16 MinBasePort; /// Ports begin at 51000 +extern uint16 MaxBasePort; /// (note: in this implementation there can be no more than 1000 services) + +extern const NLNET::TServiceId BaseSId; /// Allocated SIds begin at 128 (except for Agent Service) + +extern const NLMISC::TTime UnregisterTimeout; /// After 10s we remove an unregister service if every server didn't ACK the message + +extern NLNET::CCallbackServer *CallbackServer; + +#endif // NELNS_NAMING_SERVICE_VARIABLES_H From 04cdfc240f67dc8324ac744f0dcda6b65b226781 Mon Sep 17 00:00:00 2001 From: Tobias Peters Date: Sat, 8 Jun 2024 18:56:07 +0000 Subject: [PATCH 06/28] extract some functions into separate files --- nelns/naming_service/CMakeLists.txt | 6 +- nelns/naming_service/naming_service.cpp | 258 +----------------- .../nelns/naming_service/can_access.cpp | 38 +++ .../nelns/naming_service/can_access.h | 12 + .../nelns/naming_service/do_remove.cpp | 93 +++++++ .../nelns/naming_service/do_remove.h | 14 + .../naming_service/do_unregister_service.cpp | 45 +++ .../naming_service/do_unregister_service.h | 12 + .../naming_service/effectively_remove.cpp | 12 + .../nelns/naming_service/effectively_remove.h | 10 + .../nelns/naming_service/helper.cpp | 20 +- .../nelns/naming_service/helper.h | 6 +- .../naming_service/service_instance_manager.h | 64 +++++ .../ryzom_naming_service.cpp | 2 +- 14 files changed, 344 insertions(+), 248 deletions(-) create mode 100644 nelns/naming_service/nelns/naming_service/can_access.cpp create mode 100644 nelns/naming_service/nelns/naming_service/can_access.h create mode 100644 nelns/naming_service/nelns/naming_service/do_remove.cpp create mode 100644 nelns/naming_service/nelns/naming_service/do_remove.h create mode 100644 nelns/naming_service/nelns/naming_service/do_unregister_service.cpp create mode 100644 nelns/naming_service/nelns/naming_service/do_unregister_service.h create mode 100644 nelns/naming_service/nelns/naming_service/effectively_remove.cpp create mode 100644 nelns/naming_service/nelns/naming_service/effectively_remove.h create mode 100644 nelns/naming_service/nelns/naming_service/service_instance_manager.h diff --git a/nelns/naming_service/CMakeLists.txt b/nelns/naming_service/CMakeLists.txt index af86f56b62..afca1b379e 100644 --- a/nelns/naming_service/CMakeLists.txt +++ b/nelns/naming_service/CMakeLists.txt @@ -1,6 +1,10 @@ include_directories(${CMAKE_CURRENT_SOURCE_DIR}) add_library(nelns_naming_service + nelns/naming_service/can_access.cpp + nelns/naming_service/do_remove.cpp + nelns/naming_service/do_unregister_service.cpp + nelns/naming_service/effectively_remove.cpp nelns/naming_service/helper.cpp nelns/naming_service/variables.cpp ) @@ -9,8 +13,6 @@ add_library(nelns::ns ALIAS nelns_naming_service) ADD_EXECUTABLE(naming_service WIN32 naming_service.cpp - nelns/naming_service/helper.cpp - nelns/naming_service/helper.h ) TARGET_LINK_LIBRARIES(naming_service diff --git a/nelns/naming_service/naming_service.cpp b/nelns/naming_service/naming_service.cpp index ae6cfbd809..5d6c3f6d01 100644 --- a/nelns/naming_service/naming_service.cpp +++ b/nelns/naming_service/naming_service.cpp @@ -48,8 +48,12 @@ #include #include +#include +#include +#include #include #include +#include #include // @@ -74,73 +78,13 @@ NLMISC_COMMAND(test, "none", "none") } - -// Helper that emulates layer5's send() -//void sendToService( uint16 sid, CMessage& msgout ); - -// Helper that returns the first address of a service -CInetAddress getHostAddress( TServiceId sid ); - -// Asks a service to stop and tell every one -void doUnregisterService (TServiceId sid); - - -/** - * Manager for services instances - * (Moved from the TICKS to the NS) - * Implementable with layer 5, here implemented in NS (layer 3) - * \author Olivier Cado - * \author Nevrax France - * \date 2003 - */ -class CServiceInstanceManager -{ -public: - - /// Constructor - CServiceInstanceManager(); - - /** Add the name of a service which must not be duplicated - * If uniqueOnShard is true, only one service is allowed. - * If uniqueOnShard is false, one service is allowed by physical machine. - */ - void addUniqueService( const std::string& serviceName, bool uniqueOnShard ) - { - _UniqueServices.insert( std::make_pair( serviceName, uniqueOnShard ) ); - } - - /// Check if a service is allowed to start (if so, add it) - bool queryStartService( const std::string& serviceName, TServiceId serviceId, const std::vector &addr, string& reason ); - - /// Release a service instance - void releaseService( NLNET::TServiceId serviceId ); - - /// Display information - void displayInfo( NLMISC::CLog *log = NLMISC::InfoLog ) const; - - /// Make all controlled services quit - void killAllServices(); - -private: - - /// List of restricted services - std::map< std::string, bool > _UniqueServices; - - /// List of granted (online) services - std::set< TServiceId > _OnlineServices; -}; - - -CServiceInstanceManager *SIMInstance = NULL; - - /* * Constructor */ CServiceInstanceManager::CServiceInstanceManager() { - nlassert( ! SIMInstance ); - SIMInstance = this; + nlassert( ! _Instance ); + _Instance = this; // Note: addCallbackArray() done in CRangeMirrorManager::init() } @@ -244,42 +188,18 @@ void CServiceInstanceManager::killAllServices() } } +CServiceInstanceManager * CServiceInstanceManager::_Instance = nullptr; +CServiceInstanceManager *CServiceInstanceManager::getInstance() +{ + nlassert( _Instance ); + return _Instance; +} // // Functions // -bool canAccess (const vector &addr, const CServiceEntry &entry, vector &accessibleAddr) -{ - accessibleAddr.clear (); - - if (entry.WaitingUnregistration) - return false; - - for (uint i = 0; i < addr.size(); i++) - { - uint32 net = addr[i].internalNetAddress(); - for (uint j = 0; j < entry.Addr.size(); j++) - { - if (net == entry.Addr[j].internalNetAddress()) - { - accessibleAddr.push_back (entry.Addr[j]); - } - } - } - - if (accessibleAddr.empty()) - { - nldebug ("service %s-%hu is not accessible by '%s'", entry.Name.c_str(), entry.SId.get(), vectorCInetAddressToString (addr).c_str ()); - } - else - { - nldebug ("service %s-%hu is accessible by '%s'", entry.Name.c_str(), entry.SId.get(), vectorCInetAddressToString (accessibleAddr).c_str ()); - } - - return !accessibleAddr.empty (); -} void displayRegisteredServices (CLog *log = InfoLog) { @@ -304,137 +224,6 @@ void displayRegisteredServices (CLog *log = InfoLog) } -list::iterator effectivelyRemove (list::iterator &it) -{ - // remove the service from the registered service list - nlinfo ("Effectively remove the service %s-%hu", (*it).Name.c_str(), it->SId.get()); - return RegisteredServices.erase (it); -} - -/* - * Helper procedure for cbLookupAlternate and cbUnregister. - * Note: name is used for a LOGS. - */ -list::iterator doRemove (list::iterator it) -{ - nldebug ("Unregister the service %s-%hu '%s'", (*it).Name.c_str(), it->SId.get(), (*it).Addr[0].asString().c_str()); - - // tell to everybody that this service is unregistered - - CMessage msgout ("UNB"); - msgout.serial ((*it).Name); - msgout.serial ((*it).SId); - - vector accessibleAddress; - nlinfo ("Broadcast the Unregistration of %s-%hu to all registered services", (*it).Name.c_str(), it->SId.get()); - for (list::iterator it3 = RegisteredServices.begin(); it3 != RegisteredServices.end (); it3++) - { - if (canAccess((*it).Addr, (*it3), accessibleAddress)) - { - CallbackServer->send (msgout, (*it3).SockId); - //CNetManager::send ("NS", msgout, (*it3).SockId); - nldebug ("Broadcast to %s-%hu", (*it3).Name.c_str(), it3->SId.get()); - } - } - - // new system, after the unregistation broadcast, we wait ACK from all services before really remove - // the service, before, we tag the service as 'wait before unregister' - // if everybody didn't answer before the time out, we remove it - - (*it).SockId = NULL; - - (*it).WaitingUnregistration = true; - (*it).WaitingUnregistrationTime = CTime::getLocalTime(); - - // we remove all services awaiting his ACK because this service is down so it'll never ACK - for (list::iterator itr = RegisteredServices.begin(); itr != RegisteredServices.end (); itr++) - { - for (list::iterator itw = (*itr).WaitingUnregistrationServices.begin(); itw != (*itr).WaitingUnregistrationServices.end ();) - { - if ((*itw) == (*it).SId) - { - itw = (*itr).WaitingUnregistrationServices.erase (itw); - } - else - { - itw++; - } - } - } - - string res; - for (list::iterator it2 = RegisteredServices.begin(); it2 != RegisteredServices.end (); it2++) - { - if (!(*it2).WaitingUnregistration) - { - (*it).WaitingUnregistrationServices.push_back ((*it2).SId); - res += toString((*it2).SId.get()) + " "; - } - } - - nlinfo ("Before removing the service %s-%hu, we wait the ACK of '%s'", (*it).Name.c_str(), (*it).SId.get(), res.c_str()); - - if ((*it).WaitingUnregistrationServices.empty()) - { - return effectivelyRemove (it); - } - else - { - return ++it; - } - - // Release from the service instance manager - SIMInstance->releaseService( (*it).SId ); -} - -void doUnregisterService (TServiceId sid) -{ - list::iterator it; - for (it = RegisteredServices.begin(); it != RegisteredServices.end (); it++) - { - if ((*it).SId == sid) - { - // found it, remove it - doRemove (it); - return; - } - } - nlwarning ("Service %hu not found", sid.get()); -} - -void doUnregisterService (TSockId from) -{ - list::iterator it; - for (it = RegisteredServices.begin(); it != RegisteredServices.end ();) - { - if ((*it).SockId == from) - { - // it's possible that one "from" have more than one registred service, so we have to find in all the list - // found it, remove it - it = doRemove (it); - } - else - { - it++; - } - } -} - -/*void doUnregisterService (const CInetAddress &addr) -{ - list::iterator it; - for (it = RegisteredServices.begin(); it != RegisteredServices.end (); it++) - { - if ((*it).Addr == addr) - { - // found it, remove it - doRemove (it); - return; - } - } - nlwarning ("Service %s not found", addr.asString().c_str()); -}*/ - /* * Helper function for cbRegister. * If alloc_sid is true, sid is ignored @@ -527,7 +316,7 @@ bool doRegister (const string &name, const vector &addr, TServiceI if (ok) { // Check if the instance is allowed to start, according to the restriction in the config file - if ( SIMInstance->queryStartService( name, sid, addr, reason ) ) + if ( CServiceInstanceManager::getInstance()->queryStartService( name, sid, addr, reason ) ) { // add him in the registered list RegisteredServices.push_back (CServiceEntry(from, addr, name, sid)); @@ -869,23 +658,6 @@ static void cbRegisteredServices(CMessage& msgin, TSockId from, CCallbackNetBase }*/ -/* - * Helper that returns the first address of a service - */ -CInetAddress getHostAddress( TServiceId sid ) -{ - list::iterator it; - for (it = RegisteredServices.begin(); it != RegisteredServices.end (); it++) - { - if ((*it).SId == sid) - { - return (*it).Addr[0]; - } - } - return CInetAddress(); -} - - // // Callback array // @@ -1095,12 +867,12 @@ NLMISC_DYNVARIABLE(uint32, NbRegisteredServices, "display the number of service NLMISC_COMMAND( displayServiceInstances, "SIM: Display info on service instances", "" ) { - SIMInstance->displayInfo( &log ); + CServiceInstanceManager::getInstance()->displayInfo( &log ); return true; } NLMISC_COMMAND( killAllServices, "SIM: Make all the controlled services quit", "" ) { - SIMInstance->killAllServices(); + CServiceInstanceManager::getInstance()->killAllServices(); return true; } diff --git a/nelns/naming_service/nelns/naming_service/can_access.cpp b/nelns/naming_service/nelns/naming_service/can_access.cpp new file mode 100644 index 0000000000..0dc610dc49 --- /dev/null +++ b/nelns/naming_service/nelns/naming_service/can_access.cpp @@ -0,0 +1,38 @@ +#include + +#include + +using std::vector; + +using NLNET::CInetAddress; + +bool canAccess (const vector &addr, const CServiceEntry &entry, vector &accessibleAddr) +{ + accessibleAddr.clear (); + + if (entry.WaitingUnregistration) + return false; + + for (uint i = 0; i < addr.size(); i++) + { + uint32 net = addr[i].internalNetAddress(); + for (uint j = 0; j < entry.Addr.size(); j++) + { + if (net == entry.Addr[j].internalNetAddress()) + { + accessibleAddr.push_back (entry.Addr[j]); + } + } + } + + if (accessibleAddr.empty()) + { + nldebug ("service %s-%hu is not accessible by '%s'", entry.Name.c_str(), entry.SId.get(), vectorCInetAddressToString (addr).c_str ()); + } + else + { + nldebug ("service %s-%hu is accessible by '%s'", entry.Name.c_str(), entry.SId.get(), vectorCInetAddressToString (accessibleAddr).c_str ()); + } + + return !accessibleAddr.empty (); +} diff --git a/nelns/naming_service/nelns/naming_service/can_access.h b/nelns/naming_service/nelns/naming_service/can_access.h new file mode 100644 index 0000000000..31bcd68804 --- /dev/null +++ b/nelns/naming_service/nelns/naming_service/can_access.h @@ -0,0 +1,12 @@ +#ifndef NELNS_NAMING_SERVICE_CAN_ACCESS_H +#define NELNS_NAMING_SERVICE_CAN_ACCESS_H + +#include + +#include +#include +#include + +bool canAccess(const std::vector &addr, const CServiceEntry &entry, std::vector &accessibleAddr); + +#endif // NELNS_NAMING_SERVICE_CAN_ACCESS_H diff --git a/nelns/naming_service/nelns/naming_service/do_remove.cpp b/nelns/naming_service/nelns/naming_service/do_remove.cpp new file mode 100644 index 0000000000..caffc4ea3a --- /dev/null +++ b/nelns/naming_service/nelns/naming_service/do_remove.cpp @@ -0,0 +1,93 @@ +#include + +#include +#include +#include +#include +#include + +using std::list; +using std::string; +using std::vector; + +using NLMISC::CTime; +using NLMISC::toString; +using NLNET::CInetAddress; +using NLNET::CMessage; +using NLNET::TServiceId; + +/* + * Helper procedure for cbLookupAlternate and cbUnregister. + * Note: name is used for a LOGS. + */ +list::iterator doRemove(list::iterator it) +{ + nldebug("Unregister the service %s-%hu '%s'", (*it).Name.c_str(), it->SId.get(), (*it).Addr[0].asString().c_str()); + + // tell to everybody that this service is unregistered + + CMessage msgout("UNB"); + msgout.serial((*it).Name); + msgout.serial((*it).SId); + + vector accessibleAddress; + nlinfo("Broadcast the Unregistration of %s-%hu to all registered services", (*it).Name.c_str(), it->SId.get()); + for (list::iterator it3 = RegisteredServices.begin(); it3 != RegisteredServices.end(); it3++) + { + if (canAccess((*it).Addr, (*it3), accessibleAddress)) + { + CallbackServer->send(msgout, (*it3).SockId); + // CNetManager::send ("NS", msgout, (*it3).SockId); + nldebug("Broadcast to %s-%hu", (*it3).Name.c_str(), it3->SId.get()); + } + } + + // new system, after the unregistation broadcast, we wait ACK from all services before really remove + // the service, before, we tag the service as 'wait before unregister' + // if everybody didn't answer before the time out, we remove it + + (*it).SockId = NULL; + + (*it).WaitingUnregistration = true; + (*it).WaitingUnregistrationTime = CTime::getLocalTime(); + + // we remove all services awaiting his ACK because this service is down so it'll never ACK + for (list::iterator itr = RegisteredServices.begin(); itr != RegisteredServices.end(); itr++) + { + for (list::iterator itw = (*itr).WaitingUnregistrationServices.begin(); itw != (*itr).WaitingUnregistrationServices.end();) + { + if ((*itw) == (*it).SId) + { + itw = (*itr).WaitingUnregistrationServices.erase(itw); + } + else + { + itw++; + } + } + } + + string res; + for (list::iterator it2 = RegisteredServices.begin(); it2 != RegisteredServices.end(); it2++) + { + if (!(*it2).WaitingUnregistration) + { + (*it).WaitingUnregistrationServices.push_back((*it2).SId); + res += toString((*it2).SId.get()) + " "; + } + } + + nlinfo("Before removing the service %s-%hu, we wait the ACK of '%s'", (*it).Name.c_str(), (*it).SId.get(), res.c_str()); + + if ((*it).WaitingUnregistrationServices.empty()) + { + return effectivelyRemove(it); + } + else + { + return ++it; + } + + // Release from the service instance manager + CServiceInstanceManager::getInstance()->releaseService((*it).SId); +} diff --git a/nelns/naming_service/nelns/naming_service/do_remove.h b/nelns/naming_service/nelns/naming_service/do_remove.h new file mode 100644 index 0000000000..6f0a79bff2 --- /dev/null +++ b/nelns/naming_service/nelns/naming_service/do_remove.h @@ -0,0 +1,14 @@ +#ifndef NELNS_NAMING_SERVICE_DO_REMOVE_H +#define NELNS_NAMING_SERVICE_DO_REMOVE_H + +#include + +#include + +/* + * Helper procedure for cbLookupAlternate and cbUnregister. + * Note: name is used for a LOGS. + */ +std::list::iterator doRemove(std::list::iterator it); + +#endif // NELNS_NAMING_SERVICE_DO_REMOVE_H diff --git a/nelns/naming_service/nelns/naming_service/do_unregister_service.cpp b/nelns/naming_service/nelns/naming_service/do_unregister_service.cpp new file mode 100644 index 0000000000..9ed14d1601 --- /dev/null +++ b/nelns/naming_service/nelns/naming_service/do_unregister_service.cpp @@ -0,0 +1,45 @@ +#include + +#include +#include +#include +#include + +using std::list; + +using NLNET::CInetAddress; +using NLNET::TServiceId; +using NLNET::TSockId; + +void doUnregisterService(const TServiceId &sid) +{ + list::iterator it; + for (it = RegisteredServices.begin(); it != RegisteredServices.end(); it++) + { + if ((*it).SId == sid) + { + // found it, remove it + doRemove(it); + return; + } + } + nlwarning("Service %hu not found", sid.get()); +} + +void doUnregisterService(const NLNET::TSockId &from) +{ + list::iterator it; + for (it = RegisteredServices.begin(); it != RegisteredServices.end();) + { + if ((*it).SockId == from) + { + // it's possible that one "from" have more than one registred service, so we have to find in all the list + // found it, remove it + it = doRemove(it); + } + else + { + it++; + } + } +} diff --git a/nelns/naming_service/nelns/naming_service/do_unregister_service.h b/nelns/naming_service/nelns/naming_service/do_unregister_service.h new file mode 100644 index 0000000000..a0112a4b93 --- /dev/null +++ b/nelns/naming_service/nelns/naming_service/do_unregister_service.h @@ -0,0 +1,12 @@ +#ifndef NELNS_NAMING_SERVICE_DO_UNREGISTER_SERVICE_H +#define NELNS_NAMING_SERVICE_DO_UNREGISTER_SERVICE_H + +#include +#include + +// Asks a service to stop and tell every one +void doUnregisterService(const NLNET::TServiceId &sid); + +void doUnregisterService(const NLNET::TSockId &from); + +#endif // NELNS_NAMING_SERVICE_DO_UNREGISTER_SERVICE_H diff --git a/nelns/naming_service/nelns/naming_service/effectively_remove.cpp b/nelns/naming_service/nelns/naming_service/effectively_remove.cpp new file mode 100644 index 0000000000..4b7f754565 --- /dev/null +++ b/nelns/naming_service/nelns/naming_service/effectively_remove.cpp @@ -0,0 +1,12 @@ +#include + +#include + +using std::list; + +list::iterator effectivelyRemove(list::iterator &it) +{ + // remove the service from the registered service list + nlinfo ("Effectively remove the service %s-%hu", (*it).Name.c_str(), it->SId.get()); + return RegisteredServices.erase (it); +} diff --git a/nelns/naming_service/nelns/naming_service/effectively_remove.h b/nelns/naming_service/nelns/naming_service/effectively_remove.h new file mode 100644 index 0000000000..af67fd348a --- /dev/null +++ b/nelns/naming_service/nelns/naming_service/effectively_remove.h @@ -0,0 +1,10 @@ +#ifndef NELNS_NAMING_SERVICE_EFFECTIVELY_REMOVE_H +#define NELNS_NAMING_SERVICE_EFFECTIVELY_REMOVE_H + +#include + +#include + +std::list::iterator effectivelyRemove (std::list::iterator &it); + +#endif // NELNS_NAMING_SERVICE_EFFECTIVELY_REMOVE_H diff --git a/nelns/naming_service/nelns/naming_service/helper.cpp b/nelns/naming_service/nelns/naming_service/helper.cpp index 5d1f73ef87..984f10941d 100644 --- a/nelns/naming_service/nelns/naming_service/helper.cpp +++ b/nelns/naming_service/nelns/naming_service/helper.cpp @@ -1,18 +1,20 @@ #include #include + #include #include using std::list; using std::string; +using NLNET::CInetAddress; using NLNET::TServiceId; /* * Helper that emulate layer5's getServiceName() */ -string getServiceName( TServiceId sid ) +string getServiceName( const TServiceId& sid ) { list::iterator it; for (it = RegisteredServices.begin(); it != RegisteredServices.end (); it++) @@ -25,3 +27,19 @@ string getServiceName( TServiceId sid ) return ""; // not found } + +/* + * Helper that returns the first address of a service + */ +CInetAddress getHostAddress( const TServiceId& sid ) +{ + list::iterator it; + for (it = RegisteredServices.begin(); it != RegisteredServices.end (); it++) + { + if ((*it).SId == sid) + { + return (*it).Addr[0]; + } + } + return {}; +} diff --git a/nelns/naming_service/nelns/naming_service/helper.h b/nelns/naming_service/nelns/naming_service/helper.h index 471be8a9d1..09b4981279 100644 --- a/nelns/naming_service/nelns/naming_service/helper.h +++ b/nelns/naming_service/nelns/naming_service/helper.h @@ -3,9 +3,13 @@ #include +#include #include // Helper that emulate layer5's getServiceName() -std::string getServiceName(NLNET::TServiceId sid); +std::string getServiceName(const NLNET::TServiceId& sid); + +// Helper that returns the first address of a service +NLNET::CInetAddress getHostAddress( const NLNET::TServiceId& sid ); #endif // NELNS_NAMING_SERVICE_HELPER_H diff --git a/nelns/naming_service/nelns/naming_service/service_instance_manager.h b/nelns/naming_service/nelns/naming_service/service_instance_manager.h new file mode 100644 index 0000000000..1800ddf18b --- /dev/null +++ b/nelns/naming_service/nelns/naming_service/service_instance_manager.h @@ -0,0 +1,64 @@ +#ifndef NELNS_NAMING_SERVICE_SERVICE_INSTANCE_MANAGER_H +#define NELNS_NAMING_SERVICE_SERVICE_INSTANCE_MANAGER_H + +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include + +/** + * Manager for services instances + * (Moved from the TICKS to the NS) + * Implementable with layer 5, here implemented in NS (layer 3) + * \author Olivier Cado + * \author Nevrax France + * \date 2003 + */ +class CServiceInstanceManager +{ +private: + static CServiceInstanceManager *_Instance; + +public: + static CServiceInstanceManager *getInstance(); + + /// Constructor + CServiceInstanceManager(); + + /** Add the name of a service which must not be duplicated + * If uniqueOnShard is true, only one service is allowed. + * If uniqueOnShard is false, one service is allowed by physical machine. + */ + void addUniqueService(const std::string &serviceName, bool uniqueOnShard) + { + _UniqueServices.insert(std::make_pair(serviceName, uniqueOnShard)); + } + + /// Check if a service is allowed to start (if so, add it) + bool queryStartService(const std::string &serviceName, NLNET::TServiceId serviceId, const std::vector &addr, std::string &reason); + + /// Release a service instance + void releaseService(NLNET::TServiceId serviceId); + + /// Display information + void displayInfo(NLMISC::CLog *log = NLMISC::InfoLog) const; + + /// Make all controlled services quit + void killAllServices(); + +private: + /// List of restricted services + std::map _UniqueServices; + + /// List of granted (online) services + std::set _OnlineServices; +}; + +#endif // NELNS_NAMING_SERVICE_SERVICE_INSTANCE_MANAGER_H diff --git a/ryzom/server/src/ryzom_naming_service/ryzom_naming_service.cpp b/ryzom/server/src/ryzom_naming_service/ryzom_naming_service.cpp index be8a34e9d1..3905f63524 100644 --- a/ryzom/server/src/ryzom_naming_service/ryzom_naming_service.cpp +++ b/ryzom/server/src/ryzom_naming_service/ryzom_naming_service.cpp @@ -471,7 +471,7 @@ void doUnregisterService (TServiceId sid) nlwarning ("Service %hu not found", sid.get()); } -void doUnregisterService (TSockId from) +void doUnregisterService (const NLNET::TSockId &from) { list::iterator it; for (it = RegisteredServices.begin(); it != RegisteredServices.end ();) From 01d6b98efb98277475894b493d9413d0fc11b9a3 Mon Sep 17 00:00:00 2001 From: Tobias Peters Date: Sat, 8 Jun 2024 18:59:54 +0000 Subject: [PATCH 07/28] removed clutter --- nelns/naming_service/naming_service.cpp | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/nelns/naming_service/naming_service.cpp b/nelns/naming_service/naming_service.cpp index 5d6c3f6d01..052e25c708 100644 --- a/nelns/naming_service/naming_service.cpp +++ b/nelns/naming_service/naming_service.cpp @@ -236,27 +236,6 @@ bool doRegister (const string &name, const vector &addr, TServiceI string reason; uint8 ok = true; bool needRegister = true; - /*for (list::iterator it = RegisteredServices.begin(); it != RegisteredServices.end (); it++) - { - if ((*it).Addr.asIPString() == addr.asIPString() ) - { - // we already have a service on this address, remplace it if it's the same name - if ((*it).Name == name) - { - // it's the same service, replace it - (*it).SockId = from; - sid = (*it).SId; - nlinfo ("Replace the service %s", name.c_str()); - } - else - { - nlwarning ("Try to register %s to %s but the service %s already on this address. ignore it!", name.c_str(), addr.asIPString().c_str(), (*it).Name.c_str()); - ok = false; - } - needRegister = false; - break; - } - }*/ if (needRegister) { From b8066665fa2a7b08e966f88eb72d058b16db6f41 Mon Sep 17 00:00:00 2001 From: Tobias Peters Date: Sat, 8 Jun 2024 19:23:32 +0000 Subject: [PATCH 08/28] simplify code --- nelns/naming_service/naming_service.cpp | 210 ++++++++++++------------ 1 file changed, 103 insertions(+), 107 deletions(-) diff --git a/nelns/naming_service/naming_service.cpp b/nelns/naming_service/naming_service.cpp index 052e25c708..fe7a6caaa0 100644 --- a/nelns/naming_service/naming_service.cpp +++ b/nelns/naming_service/naming_service.cpp @@ -235,149 +235,145 @@ bool doRegister (const string &name, const vector &addr, TServiceI // Find if the service is not already registered string reason; uint8 ok = true; - bool needRegister = true; - if (needRegister) + if (sid.get() == 0) { - if (sid.get() == 0) + // we have to find a sid + sid = BaseSId; + bool found = false; + while (!found) { - // we have to find a sid - sid = BaseSId; - bool found = false; - while (!found) + list::iterator it; + for (it = RegisteredServices.begin(); it != RegisteredServices.end (); it++) { - list::iterator it; - for (it = RegisteredServices.begin(); it != RegisteredServices.end (); it++) - { - if ((*it).SId == sid) - { - break; - } - } - if (it == RegisteredServices.end ()) - { - // ok, we have an empty sid - found = true; - } - else + if ((*it).SId == sid) { - sid.set(sid.get()+1); - if (sid.get() == 0) // round the clock - { - nlwarning ("Service identifier allocation overflow"); - ok = false; - break; - } + break; } } - - } - else - { - // we have to check that the user provided sid is available - list::iterator it; - for (it = RegisteredServices.begin(); it != RegisteredServices.end (); it++) + if (it == RegisteredServices.end ()) { - if ((*it).SId == sid) + // ok, we have an empty sid + found = true; + } + else + { + sid.set(sid.get()+1); + if (sid.get() == 0) // round the clock { - nlwarning ("Sid %d already used by another service", sid.get()); + nlwarning ("Service identifier allocation overflow"); ok = false; break; } } - if (it != RegisteredServices.end ()) + } + + } + else + { + // we have to check that the user provided sid is available + list::iterator it; + for (it = RegisteredServices.begin(); it != RegisteredServices.end (); it++) + { + if ((*it).SId == sid) { - ok = true; + nlwarning ("Sid %d already used by another service", sid.get()); + ok = false; + break; } } + if (it != RegisteredServices.end ()) + { + ok = true; + } + } - // if ok, register the service and send a broadcast to other people - if (ok) + // if ok, register the service and send a broadcast to other people + if (ok) + { + // Check if the instance is allowed to start, according to the restriction in the config file + if ( CServiceInstanceManager::getInstance()->queryStartService( name, sid, addr, reason ) ) { - // Check if the instance is allowed to start, according to the restriction in the config file - if ( CServiceInstanceManager::getInstance()->queryStartService( name, sid, addr, reason ) ) + // add him in the registered list + RegisteredServices.push_back (CServiceEntry(from, addr, name, sid)); + + // tell to everybody but not him that this service is registered + if (!reconnection) { - // add him in the registered list - RegisteredServices.push_back (CServiceEntry(from, addr, name, sid)); + CMessage msgout ("RGB"); + TServiceId::size_type s = 1; + msgout.serial (s); + msgout.serial (const_cast(name)); + msgout.serial (sid); + // we need to send all addr to all services even if the service can't access because we use the address index + // to know which connection comes. + msgout.serialCont (const_cast &>(addr)); + nlinfo ("The service is %s-%d, broadcast the Registration to everybody", name.c_str(), sid.get()); - // tell to everybody but not him that this service is registered - if (!reconnection) + vector accessibleAddress; + for (list::iterator it3 = RegisteredServices.begin(); it3 != RegisteredServices.end (); it3++) { - CMessage msgout ("RGB"); - TServiceId::size_type s = 1; - msgout.serial (s); - msgout.serial (const_cast(name)); - msgout.serial (sid); - // we need to send all addr to all services even if the service can't access because we use the address index - // to know which connection comes. - msgout.serialCont (const_cast &>(addr)); - nlinfo ("The service is %s-%d, broadcast the Registration to everybody", name.c_str(), sid.get()); - - vector accessibleAddress; - for (list::iterator it3 = RegisteredServices.begin(); it3 != RegisteredServices.end (); it3++) + // send only services that can be accessed and not itself + if ((*it3).SId != sid && canAccess(addr, (*it3), accessibleAddress)) { - // send only services that can be accessed and not itself - if ((*it3).SId != sid && canAccess(addr, (*it3), accessibleAddress)) - { - CallbackServer->send (msgout, (*it3).SockId); - //CNetManager::send ("NS", msgout, (*it3).SockId); - nldebug ("Broadcast to %s-%hu", (*it3).Name.c_str(), it3->SId.get()); - } + CallbackServer->send (msgout, (*it3).SockId); + //CNetManager::send ("NS", msgout, (*it3).SockId); + nldebug ("Broadcast to %s-%hu", (*it3).Name.c_str(), it3->SId.get()); } } - - // set the sid only if it s ok - from->setAppId (sid.get()); - } - else - { - // Reply "startup denied", and do not send registration to other services - ok = false; } + + // set the sid only if it s ok + from->setAppId (sid.get()); } + else + { + // Reply "startup denied", and do not send registration to other services + ok = false; + } + } - // send the message to the service to say if it s ok or not - if (!reconnection) + // send the message to the service to say if it s ok or not + if (!reconnection) + { + // send the answer to the client + CMessage msgout ("RG"); + msgout.serial (ok); + if (ok) { - // send the answer to the client - CMessage msgout ("RG"); - msgout.serial (ok); - if (ok) - { - msgout.serial (sid); + msgout.serial (sid); - // send him all services available (also itself) - TServiceId::size_type nb = 0; + // send him all services available (also itself) + TServiceId::size_type nb = 0; - vector accessibleAddress; + vector accessibleAddress; - for (list::iterator it2 = RegisteredServices.begin(); it2 != RegisteredServices.end (); it2++) - { - // send only services that are available - if (canAccess(addr, (*it2), accessibleAddress)) - nb++; - } - msgout.serial (nb); + for (list::iterator it2 = RegisteredServices.begin(); it2 != RegisteredServices.end (); it2++) + { + // send only services that are available + if (canAccess(addr, (*it2), accessibleAddress)) + nb++; + } + msgout.serial (nb); - for (list::iterator it = RegisteredServices.begin(); it != RegisteredServices.end (); it++) + for (list::iterator it = RegisteredServices.begin(); it != RegisteredServices.end (); it++) + { + // send only services that are available + if (canAccess(addr, (*it), accessibleAddress)) { - // send only services that are available - if (canAccess(addr, (*it), accessibleAddress)) - { - msgout.serial ((*it).Name); - msgout.serial ((*it).SId); - msgout.serialCont ((*it).Addr); - } + msgout.serial ((*it).Name); + msgout.serial ((*it).SId); + msgout.serialCont ((*it).Addr); } } - else - { - msgout.serial( reason ); - } - - netbase.send (msgout, from); - netbase.flush (from); } + else + { + msgout.serial( reason ); + } + + netbase.send (msgout, from); + netbase.flush (from); } //displayRegisteredServices (); From 10933b9a8145e132900eaf80586854e2ec342a75 Mon Sep 17 00:00:00 2001 From: Tobias Peters Date: Sat, 8 Jun 2024 19:38:53 +0000 Subject: [PATCH 09/28] move naming service tests into corresponding subfolder --- nelns/CMakeLists.txt | 3 -- nelns/naming_service/CMakeLists.txt | 4 +++ nelns/naming_service/naming_service.cpp | 6 ++-- nelns/naming_service/tests/CMakeLists.txt | 30 +++++++++++++++++++ .../tests/naming_service.test.cpp | 0 .../tests/service_instance_manager.test.cpp | 9 ++++++ nelns/tests/CMakeLists.txt | 14 --------- 7 files changed, 46 insertions(+), 20 deletions(-) create mode 100644 nelns/naming_service/tests/CMakeLists.txt rename nelns/{ => naming_service}/tests/naming_service.test.cpp (100%) create mode 100644 nelns/naming_service/tests/service_instance_manager.test.cpp delete mode 100644 nelns/tests/CMakeLists.txt diff --git a/nelns/CMakeLists.txt b/nelns/CMakeLists.txt index 36f8f27599..d451111185 100644 --- a/nelns/CMakeLists.txt +++ b/nelns/CMakeLists.txt @@ -4,9 +4,6 @@ IF(WITH_NELNS_SERVER) ADD_SUBDIRECTORY(admin_executor_service) ADD_SUBDIRECTORY(admin_service) ADD_SUBDIRECTORY(naming_service) - IF (WITH_NEL_TESTS) - ADD_SUBDIRECTORY(tests) - ENDIF () ADD_SUBDIRECTORY(login_service) ADD_SUBDIRECTORY(welcome_service) ENDIF() diff --git a/nelns/naming_service/CMakeLists.txt b/nelns/naming_service/CMakeLists.txt index afca1b379e..6ce0fc4864 100644 --- a/nelns/naming_service/CMakeLists.txt +++ b/nelns/naming_service/CMakeLists.txt @@ -26,3 +26,7 @@ NL_ADD_RUNTIME_FLAGS(naming_service) INSTALL(TARGETS naming_service RUNTIME DESTINATION sbin COMPONENT ns) INSTALL(FILES naming_service.cfg common.cfg DESTINATION ${NL_ETC_PREFIX}/nelns COMPONENT ns) + +IF (WITH_NEL_TESTS) + ADD_SUBDIRECTORY(tests) +ENDIF () diff --git a/nelns/naming_service/naming_service.cpp b/nelns/naming_service/naming_service.cpp index fe7a6caaa0..a84e87b111 100644 --- a/nelns/naming_service/naming_service.cpp +++ b/nelns/naming_service/naming_service.cpp @@ -83,8 +83,8 @@ NLMISC_COMMAND(test, "none", "none") */ CServiceInstanceManager::CServiceInstanceManager() { - nlassert( ! _Instance ); - _Instance = this; + nlassert( !_SIMInstance); + _SIMInstance = this; // Note: addCallbackArray() done in CRangeMirrorManager::init() } @@ -192,7 +192,7 @@ CServiceInstanceManager * CServiceInstanceManager::_Instance = nullptr; CServiceInstanceManager *CServiceInstanceManager::getInstance() { - nlassert( _Instance ); + nlassert(_Instance); return _Instance; } diff --git a/nelns/naming_service/tests/CMakeLists.txt b/nelns/naming_service/tests/CMakeLists.txt new file mode 100644 index 0000000000..d95c8dbb76 --- /dev/null +++ b/nelns/naming_service/tests/CMakeLists.txt @@ -0,0 +1,30 @@ +set(TEST_NAME "naming_service_test") + +add_executable("${TEST_NAME}" + naming_service.test.cpp +) +target_link_libraries("${TEST_NAME}" + GTest::gtest_main + nelmisc + nelnet + nelns::ns +) +gtest_discover_tests("${TEST_NAME}" + EXTRA_ARGS "--gtest_output=xml:${CMAKE_CURRENT_BINARY_DIR}/reports/junit-${TEST_NAME}.xml" +) + +set(TEST_NAME "service_instance_manager_test") + +add_executable("${TEST_NAME}" + service_instance_manager.test.cpp +) +target_link_libraries("${TEST_NAME}" + GTest::gtest_main + nelmisc + nelnet + nelns::ns +) +gtest_discover_tests("${TEST_NAME}" + EXTRA_ARGS "--gtest_output=xml:${CMAKE_CURRENT_BINARY_DIR}/reports/junit-${TEST_NAME}.xml" +) + diff --git a/nelns/tests/naming_service.test.cpp b/nelns/naming_service/tests/naming_service.test.cpp similarity index 100% rename from nelns/tests/naming_service.test.cpp rename to nelns/naming_service/tests/naming_service.test.cpp diff --git a/nelns/naming_service/tests/service_instance_manager.test.cpp b/nelns/naming_service/tests/service_instance_manager.test.cpp new file mode 100644 index 0000000000..4630caf4ae --- /dev/null +++ b/nelns/naming_service/tests/service_instance_manager.test.cpp @@ -0,0 +1,9 @@ +#include + +#include + +TEST(CServiceInstanceManager, ShouldWork) { + EXPECT_STRNE("hello", "world"); + EXPECT_EQ(7 * 6, 42); + CServiceInstanceManager instance; +} diff --git a/nelns/tests/CMakeLists.txt b/nelns/tests/CMakeLists.txt deleted file mode 100644 index ad0f8a5eb2..0000000000 --- a/nelns/tests/CMakeLists.txt +++ /dev/null @@ -1,14 +0,0 @@ -set(TEST_NAME "naming_service_test") - -add_executable("${TEST_NAME}" - naming_service.test.cpp -) -target_link_libraries("${TEST_NAME}" - GTest::gtest_main - nelmisc - nelnet -) -gtest_discover_tests("${TEST_NAME}" - EXTRA_ARGS "--gtest_output=xml:${CMAKE_CURRENT_BINARY_DIR}/reports/junit-${TEST_NAME}.xml" -) - From 1b698b22482365ca9a3ab4c9ea771e9855e09dee Mon Sep 17 00:00:00 2001 From: Tobias Peters Date: Sat, 8 Jun 2024 20:05:46 +0000 Subject: [PATCH 10/28] move CServiceInstanceManager into separate file and add first test for it --- nelns/naming_service/CMakeLists.txt | 8 +- nelns/naming_service/naming_service.cpp | 118 ---------------- .../service_instance_manager.cpp | 131 ++++++++++++++++++ 3 files changed, 136 insertions(+), 121 deletions(-) create mode 100644 nelns/naming_service/nelns/naming_service/service_instance_manager.cpp diff --git a/nelns/naming_service/CMakeLists.txt b/nelns/naming_service/CMakeLists.txt index 6ce0fc4864..55ea2d3083 100644 --- a/nelns/naming_service/CMakeLists.txt +++ b/nelns/naming_service/CMakeLists.txt @@ -6,18 +6,20 @@ add_library(nelns_naming_service nelns/naming_service/do_unregister_service.cpp nelns/naming_service/effectively_remove.cpp nelns/naming_service/helper.cpp + nelns/naming_service/service_instance_manager.cpp nelns/naming_service/variables.cpp ) add_library(nelns::ns ALIAS nelns_naming_service) - +target_link_libraries(nelns_naming_service + nelmisc + nelnet +) ADD_EXECUTABLE(naming_service WIN32 naming_service.cpp ) TARGET_LINK_LIBRARIES(naming_service - nelmisc - nelnet nelns::ns ) diff --git a/nelns/naming_service/naming_service.cpp b/nelns/naming_service/naming_service.cpp index a84e87b111..bab0053c22 100644 --- a/nelns/naming_service/naming_service.cpp +++ b/nelns/naming_service/naming_service.cpp @@ -78,124 +78,6 @@ NLMISC_COMMAND(test, "none", "none") } -/* - * Constructor - */ -CServiceInstanceManager::CServiceInstanceManager() -{ - nlassert( !_SIMInstance); - _SIMInstance = this; - - // Note: addCallbackArray() done in CRangeMirrorManager::init() -} - - -/* - * Check if a service is allowed to start. Answer with a GSTS (Grant Start Service) message - */ -bool CServiceInstanceManager::queryStartService( const std::string& serviceName, TServiceId serviceId, const vector &addr, string& reason ) -{ - bool grantStarting = true; - std::map< std::string, bool >::iterator ius = _UniqueServices.find( serviceName ); - if ( ius != _UniqueServices.end() ) - { - // Service is restricted - set< TServiceId >::iterator ios; - bool uniqueOnShard = (*ius).second; - for ( ios=_OnlineServices.begin(); ios!=_OnlineServices.end(); ++ios ) - { - string name = getServiceName( *ios ); - if ( name == serviceName ) - { - if ( uniqueOnShard ) - { - // Only one service by shard is allowed => deny - grantStarting = false; - reason = toString( "Service %s already found as %hu, must be unique on shard", serviceName.c_str(), ios->get() ); - nlinfo( reason.c_str() ); - break; - } - else - { - // Only one service by physical machine is allowed - - // Implementation for layer5 - //TSockId hostid1, hostid2; - /*CCallbackNetBase *cnb1 = CUnifiedNetwork::getInstance()->getNetBase( serviceId, hostid1 ); - CCallbackNetBase *cnb2 = CUnifiedNetwork::getInstance()->getNetBase( *ios, hostid2 ); - if ( cnb1->hostAddress( hostid1 ).internalIPAddress() == cnb2->hostAddress( hostid2 ).internalIPAddress() )*/ - - // Implementation for NS - if ( addr[0].getAddress() == getHostAddress( *ios ).getAddress() ) - { - grantStarting = false; - reason = toString( "Service %s already found as %hu on same machine", serviceName.c_str(), ios->get() ); - nlinfo( reason.c_str() ); - break; - } - } - } - } - } - - if ( grantStarting ) - { - _OnlineServices.insert( serviceId ); - } - return grantStarting; -} - - -/* - * Release a service instance - */ -void CServiceInstanceManager::releaseService( NLNET::TServiceId serviceId ) -{ - _OnlineServices.erase( serviceId ); // not a problem if not found -} - - -/* - * Display information - */ -void CServiceInstanceManager::displayInfo( NLMISC::CLog *log ) const -{ - log->displayNL( "Restricted services:" ); - std::map< std::string, bool >::const_iterator ius; - for ( ius=_UniqueServices.begin(); ius!=_UniqueServices.end(); ++ius ) - { - log->displayNL( "%s -> only one per %s", (*ius).first.c_str(), (*ius).second?"shard":"machine" ); - } - log->displayNL( "Online registered services:" ); - std::set< TServiceId >::const_iterator ios; - for ( ios=_OnlineServices.begin(); ios!=_OnlineServices.end(); ++ios ) - { - log->displayNL( "%s", CUnifiedNetwork::getInstance()->getServiceUnifiedName( *ios ).c_str() ); - } -} - - -/* - * Make all controlled services quit - */ -void CServiceInstanceManager::killAllServices() -{ - // Send to all known online services - std::set< TServiceId >::const_iterator ios; - for ( ios=_OnlineServices.begin(); ios!=_OnlineServices.end(); ++ios ) - { - doUnregisterService( (TServiceId)(*ios) ); - } -} - -CServiceInstanceManager * CServiceInstanceManager::_Instance = nullptr; - -CServiceInstanceManager *CServiceInstanceManager::getInstance() -{ - nlassert(_Instance); - return _Instance; -} - // // Functions // diff --git a/nelns/naming_service/nelns/naming_service/service_instance_manager.cpp b/nelns/naming_service/nelns/naming_service/service_instance_manager.cpp new file mode 100644 index 0000000000..bb7a20481f --- /dev/null +++ b/nelns/naming_service/nelns/naming_service/service_instance_manager.cpp @@ -0,0 +1,131 @@ +#include + +#include +#include +#include +#include +#include + +using std::map; +using std::set; +using std::string; +using std::vector; + +using NLMISC::toString; +using NLNET::CInetAddress; +using NLNET::CUnifiedNetwork; +using NLNET::TServiceId; + +CServiceInstanceManager *CServiceInstanceManager::_Instance = nullptr; + +CServiceInstanceManager *CServiceInstanceManager::getInstance() +{ + nlassert(_Instance); + return _Instance; +} + +/* + * Constructor + */ +CServiceInstanceManager::CServiceInstanceManager() +{ + nlassert(!_Instance); + _Instance = this; + + // Note: addCallbackArray() done in CRangeMirrorManager::init() +} + +/* + * Check if a service is allowed to start. Answer with a GSTS (Grant Start Service) message + */ +bool CServiceInstanceManager::queryStartService(const string &serviceName, TServiceId serviceId, const vector &addr, string &reason) +{ + bool grantStarting = true; + map::iterator ius = _UniqueServices.find(serviceName); + if (ius != _UniqueServices.end()) + { + // Service is restricted + set::iterator ios; + bool uniqueOnShard = (*ius).second; + for (ios = _OnlineServices.begin(); ios != _OnlineServices.end(); ++ios) + { + string name = getServiceName(*ios); + if (name == serviceName) + { + if (uniqueOnShard) + { + // Only one service by shard is allowed => deny + grantStarting = false; + reason = toString("Service %s already found as %hu, must be unique on shard", serviceName.c_str(), ios->get()); + nlinfo(reason.c_str()); + break; + } + else + { + // Only one service by physical machine is allowed + + // Implementation for layer5 + // TSockId hostid1, hostid2; + /*CCallbackNetBase *cnb1 = CUnifiedNetwork::getInstance()->getNetBase( serviceId, hostid1 ); + CCallbackNetBase *cnb2 = CUnifiedNetwork::getInstance()->getNetBase( *ios, hostid2 ); + if ( cnb1->hostAddress( hostid1 ).internalIPAddress() == cnb2->hostAddress( hostid2 ).internalIPAddress() )*/ + + // Implementation for NS + if (addr[0].getAddress() == getHostAddress(*ios).getAddress()) + { + grantStarting = false; + reason = toString("Service %s already found as %hu on same machine", serviceName.c_str(), ios->get()); + nlinfo(reason.c_str()); + break; + } + } + } + } + } + + if (grantStarting) + { + _OnlineServices.insert(serviceId); + } + return grantStarting; +} + +/* + * Release a service instance + */ +void CServiceInstanceManager::releaseService(NLNET::TServiceId serviceId) +{ + _OnlineServices.erase(serviceId); // not a problem if not found +} + +/* + * Display information + */ +void CServiceInstanceManager::displayInfo(NLMISC::CLog *log) const +{ + log->displayNL("Restricted services:"); + map::const_iterator ius; + for (ius = _UniqueServices.begin(); ius != _UniqueServices.end(); ++ius) + { + log->displayNL("%s -> only one per %s", (*ius).first.c_str(), (*ius).second ? "shard" : "machine"); + } + log->displayNL("Online registered services:"); + set::const_iterator ios; + for (ios = _OnlineServices.begin(); ios != _OnlineServices.end(); ++ios) + { + log->displayNL("%s", CUnifiedNetwork::getInstance()->getServiceUnifiedName(*ios).c_str()); + } +} + +/* + * Make all controlled services quit + */ +void CServiceInstanceManager::killAllServices() +{ + // Send to all known online services + std::set::const_iterator ios; + for (ios = _OnlineServices.begin(); ios != _OnlineServices.end(); ++ios) + { + doUnregisterService((TServiceId)(*ios)); + } +} From 3e94cc55fbf16401d0e1931638ccd95e7faaa89d Mon Sep 17 00:00:00 2001 From: Tobias Peters Date: Sat, 8 Jun 2024 20:26:34 +0000 Subject: [PATCH 11/28] add test case for service instance manager singleton --- nelns/naming_service/tests/CMakeLists.txt | 1 + .../tests/service_instance_manager.test.cpp | 14 ++++++++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/nelns/naming_service/tests/CMakeLists.txt b/nelns/naming_service/tests/CMakeLists.txt index d95c8dbb76..b9793e319f 100644 --- a/nelns/naming_service/tests/CMakeLists.txt +++ b/nelns/naming_service/tests/CMakeLists.txt @@ -20,6 +20,7 @@ add_executable("${TEST_NAME}" ) target_link_libraries("${TEST_NAME}" GTest::gtest_main + GTest::gmock nelmisc nelnet nelns::ns diff --git a/nelns/naming_service/tests/service_instance_manager.test.cpp b/nelns/naming_service/tests/service_instance_manager.test.cpp index 4630caf4ae..58e61205ac 100644 --- a/nelns/naming_service/tests/service_instance_manager.test.cpp +++ b/nelns/naming_service/tests/service_instance_manager.test.cpp @@ -2,8 +2,14 @@ #include -TEST(CServiceInstanceManager, ShouldWork) { - EXPECT_STRNE("hello", "world"); - EXPECT_EQ(7 * 6, 42); - CServiceInstanceManager instance; +TEST(CServiceInstanceManager, getInstanceShouldThrowIfNotInitialized) { + ASSERT_DEATH({ + CServiceInstanceManager::getInstance(); + }, ""); +} + +TEST(CServiceInstanceManager, ConstructorShouldNotThrow) { + EXPECT_NO_THROW({ + CServiceInstanceManager instance; + }); } From a85a166f055db73a223c263e1097afddfad16959 Mon Sep 17 00:00:00 2001 From: Tobias Peters Date: Sun, 9 Jun 2024 06:31:42 +0000 Subject: [PATCH 12/28] add test for singleton handling in CServiceInstanceManager --- .../service_instance_manager.cpp | 9 +++++++-- .../naming_service/service_instance_manager.h | 2 ++ .../tests/service_instance_manager.test.cpp | 20 +++++++++++++++++-- 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/nelns/naming_service/nelns/naming_service/service_instance_manager.cpp b/nelns/naming_service/nelns/naming_service/service_instance_manager.cpp index bb7a20481f..582156e47f 100644 --- a/nelns/naming_service/nelns/naming_service/service_instance_manager.cpp +++ b/nelns/naming_service/nelns/naming_service/service_instance_manager.cpp @@ -20,7 +20,7 @@ CServiceInstanceManager *CServiceInstanceManager::_Instance = nullptr; CServiceInstanceManager *CServiceInstanceManager::getInstance() { - nlassert(_Instance); + nlassertex(_Instance, ("No Singleton Instance existing")); return _Instance; } @@ -29,12 +29,17 @@ CServiceInstanceManager *CServiceInstanceManager::getInstance() */ CServiceInstanceManager::CServiceInstanceManager() { - nlassert(!_Instance); + nlassertex(!_Instance, ("Singleton Instance already existing")); _Instance = this; // Note: addCallbackArray() done in CRangeMirrorManager::init() } +CServiceInstanceManager::~CServiceInstanceManager() +{ + _Instance = nullptr; +} + /* * Check if a service is allowed to start. Answer with a GSTS (Grant Start Service) message */ diff --git a/nelns/naming_service/nelns/naming_service/service_instance_manager.h b/nelns/naming_service/nelns/naming_service/service_instance_manager.h index 1800ddf18b..25d2370d9b 100644 --- a/nelns/naming_service/nelns/naming_service/service_instance_manager.h +++ b/nelns/naming_service/nelns/naming_service/service_instance_manager.h @@ -32,6 +32,8 @@ class CServiceInstanceManager /// Constructor CServiceInstanceManager(); + virtual ~CServiceInstanceManager(); + /** Add the name of a service which must not be duplicated * If uniqueOnShard is true, only one service is allowed. * If uniqueOnShard is false, one service is allowed by physical machine. diff --git a/nelns/naming_service/tests/service_instance_manager.test.cpp b/nelns/naming_service/tests/service_instance_manager.test.cpp index 58e61205ac..559ff8dfd8 100644 --- a/nelns/naming_service/tests/service_instance_manager.test.cpp +++ b/nelns/naming_service/tests/service_instance_manager.test.cpp @@ -1,15 +1,31 @@ +#include #include #include -TEST(CServiceInstanceManager, getInstanceShouldThrowIfNotInitialized) { +using ::testing::Eq; +using ::testing::NotNull; + +TEST(CServiceInstanceManager, getInstanceShouldAssertIfNotInitialized) { ASSERT_DEATH({ CServiceInstanceManager::getInstance(); }, ""); } -TEST(CServiceInstanceManager, ConstructorShouldNotThrow) { +TEST(CServiceInstanceManager, getInstanceShouldReturnSingleton) { EXPECT_NO_THROW({ CServiceInstanceManager instance; + + EXPECT_THAT(CServiceInstanceManager::getInstance(), NotNull()); + EXPECT_THAT(CServiceInstanceManager::getInstance(), Eq(&instance)); }); } + +TEST(CServiceInstanceManager, DestructorShouldCleanupSingletonInstance) { + { + CServiceInstanceManager instance; + } + ASSERT_DEATH({ + CServiceInstanceManager::getInstance(); + }, ""); +} From e65ef0ba7de80b0f00f1b9252c3a90f9aaced252 Mon Sep 17 00:00:00 2001 From: Tobias Peters Date: Sun, 9 Jun 2024 07:01:21 +0000 Subject: [PATCH 13/28] add tests for addind unique services --- .../tests/service_instance_manager.test.cpp | 77 ++++++++++++++++--- 1 file changed, 68 insertions(+), 9 deletions(-) diff --git a/nelns/naming_service/tests/service_instance_manager.test.cpp b/nelns/naming_service/tests/service_instance_manager.test.cpp index 559ff8dfd8..558a3269dc 100644 --- a/nelns/naming_service/tests/service_instance_manager.test.cpp +++ b/nelns/naming_service/tests/service_instance_manager.test.cpp @@ -1,18 +1,26 @@ #include #include +#include +#include #include +using ::testing::ElementsAre; using ::testing::Eq; using ::testing::NotNull; +using ::testing::StartsWith; +using ::testing::StrEq; -TEST(CServiceInstanceManager, getInstanceShouldAssertIfNotInitialized) { - ASSERT_DEATH({ - CServiceInstanceManager::getInstance(); - }, ""); +using ::NLMISC::CLightMemDisplayer; +using ::NLMISC::CLog; + +TEST(CServiceInstanceManager, getInstanceShouldAssertIfNotInitialized) +{ + ASSERT_DEATH({ CServiceInstanceManager::getInstance(); }, ""); } -TEST(CServiceInstanceManager, getInstanceShouldReturnSingleton) { +TEST(CServiceInstanceManager, getInstanceShouldReturnSingleton) +{ EXPECT_NO_THROW({ CServiceInstanceManager instance; @@ -21,11 +29,62 @@ TEST(CServiceInstanceManager, getInstanceShouldReturnSingleton) { }); } -TEST(CServiceInstanceManager, DestructorShouldCleanupSingletonInstance) { +TEST(CServiceInstanceManager, DestructorShouldCleanupSingletonInstance) +{ { CServiceInstanceManager instance; } - ASSERT_DEATH({ - CServiceInstanceManager::getInstance(); - }, ""); + ASSERT_DEATH({ CServiceInstanceManager::getInstance(); }, ""); +} + +TEST(CServiceInstanceManager, displayInfoShouldBeEmptyIfNothingRegistered) +{ + CServiceInstanceManager instance; + CLightMemDisplayer displayer; + CLog log = { NLMISC::CLog::LOG_ASSERT }; + log.addDisplayer(&displayer); + + instance.displayInfo(&log); + + EXPECT_THAT( + displayer.lockStrings(), + ElementsAre( + StrEq("Restricted services:\n"), + StrEq("Online registered services:\n"))); +} + +TEST(CServiceInstanceManager, addUniqueServiceShouldAddShardUniqueService) +{ + CServiceInstanceManager instance; + CLightMemDisplayer displayer; + CLog log = { NLMISC::CLog::LOG_ASSERT }; + log.addDisplayer(&displayer); + + instance.addUniqueService("unique-shard-service-name", true); + + instance.displayInfo(&log); + EXPECT_THAT( + displayer.lockStrings(), + ElementsAre( + StartsWith("Restricted services:"), + StartsWith("unique-shard-service-name -> only one per shard"), + StartsWith("Online registered services:"))); +} + +TEST(CServiceInstanceManager, addUniqueServiceShouldAddMachineUniqueService) +{ + CServiceInstanceManager instance; + CLightMemDisplayer displayer; + CLog log = { NLMISC::CLog::LOG_ASSERT }; + log.addDisplayer(&displayer); + + instance.addUniqueService("unique-machine-service-name", false); + + instance.displayInfo(&log); + EXPECT_THAT( + displayer.lockStrings(), + ElementsAre( + StartsWith("Restricted services:"), + StartsWith("unique-machine-service-name -> only one per machine"), + StartsWith("Online registered services:"))); } From c1c3e1fcc48aee97455be28f9f7b857985eb507b Mon Sep 17 00:00:00 2001 From: Tobias Peters Date: Sun, 9 Jun 2024 07:06:08 +0000 Subject: [PATCH 14/28] add test results --- .github/workflows/cmake-multi-platform.yml | 6 ++++++ .github/workflows/test-report.yml | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+) create mode 100644 .github/workflows/test-report.yml diff --git a/.github/workflows/cmake-multi-platform.yml b/.github/workflows/cmake-multi-platform.yml index cee502506c..3cc93c136d 100644 --- a/.github/workflows/cmake-multi-platform.yml +++ b/.github/workflows/cmake-multi-platform.yml @@ -104,6 +104,12 @@ jobs: # See https://cmake.org/cmake/help/latest/manual/ctest.1.html for more detail run: ctest --build-config ${{ matrix.build_type }} + - uses: actions/upload-artifact@v4 + if: (success() || failure()) && matrix.os == 'ubuntu-22.04' + with: + name: test-results + path: "${{ steps.strings.outputs.build-output-dir }}/**/reports/junit-*.xml" + - name: Package run: cmake --build ${{ steps.strings.outputs.build-output-dir }} --config ${{ matrix.build_type }} --target package diff --git a/.github/workflows/test-report.yml b/.github/workflows/test-report.yml new file mode 100644 index 0000000000..70d2946d09 --- /dev/null +++ b/.github/workflows/test-report.yml @@ -0,0 +1,20 @@ +name: "Test Report" +on: + workflow_run: + workflows: ["CMake on multiple platforms"] + types: + - completed +permissions: + contents: read + actions: read + checks: write +jobs: + report: + runs-on: ubuntu-latest + steps: + - uses: dorny/test-reporter@v1 + with: + artifact: test-results + name: GTests + path: '**/*.xml' + reporter: java-junit \ No newline at end of file From db740b5be1ccb1e3798e099195e00f85c9f5f553 Mon Sep 17 00:00:00 2001 From: Tobias Peters Date: Sun, 9 Jun 2024 07:59:42 +0000 Subject: [PATCH 15/28] add additional test coverage --- .../tests/service_instance_manager.test.cpp | 151 +++++++++++++++++- 1 file changed, 147 insertions(+), 4 deletions(-) diff --git a/nelns/naming_service/tests/service_instance_manager.test.cpp b/nelns/naming_service/tests/service_instance_manager.test.cpp index 558a3269dc..b9b448142c 100644 --- a/nelns/naming_service/tests/service_instance_manager.test.cpp +++ b/nelns/naming_service/tests/service_instance_manager.test.cpp @@ -3,16 +3,27 @@ #include #include +#include +#include #include +#include +using ::std::string; +using ::std::vector; using ::testing::ElementsAre; using ::testing::Eq; +using ::testing::IsFalse; +using ::testing::IsSupersetOf; +using ::testing::IsTrue; using ::testing::NotNull; using ::testing::StartsWith; using ::testing::StrEq; using ::NLMISC::CLightMemDisplayer; using ::NLMISC::CLog; +using ::NLNET::CInetAddress; +using ::NLNET::InvalidSockId; +using ::NLNET::TServiceId; TEST(CServiceInstanceManager, getInstanceShouldAssertIfNotInitialized) { @@ -65,10 +76,8 @@ TEST(CServiceInstanceManager, addUniqueServiceShouldAddShardUniqueService) instance.displayInfo(&log); EXPECT_THAT( displayer.lockStrings(), - ElementsAre( - StartsWith("Restricted services:"), - StartsWith("unique-shard-service-name -> only one per shard"), - StartsWith("Online registered services:"))); + IsSupersetOf({ StartsWith("Restricted services:"), + StartsWith("unique-shard-service-name -> only one per shard") })); } TEST(CServiceInstanceManager, addUniqueServiceShouldAddMachineUniqueService) @@ -88,3 +97,137 @@ TEST(CServiceInstanceManager, addUniqueServiceShouldAddMachineUniqueService) StartsWith("unique-machine-service-name -> only one per machine"), StartsWith("Online registered services:"))); } + +TEST(CServiceInstanceManager, queryStartServiceShouldAddOnlineSercie) +{ + CServiceInstanceManager instance; + TServiceId serviceId(123); + vector addresses = { "localhost:12345" }; + string reason; + CLightMemDisplayer displayer; + CLog log(NLMISC::CLog::LOG_ASSERT); + log.addDisplayer(&displayer); + + EXPECT_THAT( + instance.queryStartService("online-service", serviceId, addresses, reason), + IsTrue()); + + instance.displayInfo(&log); + EXPECT_THAT( + displayer.lockStrings(), + IsSupersetOf({ StartsWith("Online registered services:"), + StartsWith(serviceId.toString()) })); +} + +TEST(CServiceInstanceManager, queryStartServiceShouldAddSingleShardUnqiueService) +{ + CServiceInstanceManager instance; + TServiceId serviceId(123); + vector addresses = { "localhost:12345" }; + string reason; + CLightMemDisplayer displayer; + CLog log(NLMISC::CLog::LOG_ASSERT); + log.addDisplayer(&displayer); + string serviceName = "unique-shard-service-name"; + instance.addUniqueService(serviceName, true); + + EXPECT_THAT( + instance.queryStartService(serviceName, serviceId, addresses, reason), + IsTrue()); + + instance.displayInfo(&log); + EXPECT_THAT( + displayer.lockStrings(), + IsSupersetOf({ StartsWith("Online registered services:"), + StartsWith(serviceId.toString()) })); +} + +TEST(CServiceInstanceManager, queryStartServiceShouldNotAddAdditionalShardUnqiueService) +{ + RegisteredServices.clear(); + CServiceInstanceManager instance; + TServiceId serviceId(123); + vector addresses = { "localhost:12345" }; + string reason; + CLightMemDisplayer displayer; + CLog log(NLMISC::CLog::LOG_ASSERT); + log.addDisplayer(&displayer); + string serviceName = "unique-shard-service-name"; + RegisteredServices.push_back(CServiceEntry(InvalidSockId, addresses, serviceName, serviceId)); + instance.addUniqueService(serviceName, true); + instance.queryStartService(serviceName, serviceId, addresses, reason); + + EXPECT_THAT( + instance.queryStartService(serviceName, serviceId, addresses, reason), + IsFalse()); + + EXPECT_THAT( + reason, + StrEq("Service unique-shard-service-name already found as 123, must be unique on shard")); +} + +TEST(CServiceInstanceManager, queryStartServiceShouldAddAdditionalMachineUnqiueServiceOnDifferentMachine) +{ + RegisteredServices.clear(); + CServiceInstanceManager instance; + TServiceId serviceId(123); + vector firstAddresses = { "127.0.0.1:12345" }; + vector secondAddresses = { "127.0.0.2:12345" }; + string reason; + CLightMemDisplayer displayer; + CLog log(NLMISC::CLog::LOG_ASSERT); + log.addDisplayer(&displayer); + string serviceName = "unique-machine-service-name"; + RegisteredServices.push_back(CServiceEntry(InvalidSockId, firstAddresses, serviceName, serviceId)); + instance.addUniqueService(serviceName, false); + instance.queryStartService(serviceName, serviceId, firstAddresses, reason); + + EXPECT_THAT( + instance.queryStartService(serviceName, serviceId, secondAddresses, reason), + IsTrue()); +} + +TEST(CServiceInstanceManager, queryStartServiceShouldNotAddAdditionalMachineUnqiueService) +{ + RegisteredServices.clear(); + CServiceInstanceManager instance; + TServiceId serviceId(123); + vector addresses = { "localhost:12345" }; + string reason; + CLightMemDisplayer displayer; + CLog log(NLMISC::CLog::LOG_ASSERT); + log.addDisplayer(&displayer); + string serviceName = "unique-machine-service-name"; + RegisteredServices.push_back(CServiceEntry(InvalidSockId, addresses, serviceName, serviceId)); + instance.addUniqueService(serviceName, false); + instance.queryStartService(serviceName, serviceId, addresses, reason); + + EXPECT_THAT( + instance.queryStartService(serviceName, serviceId, addresses, reason), + IsFalse()); + + EXPECT_THAT( + reason, + StrEq("Service unique-machine-service-name already found as 123 on same machine")); +} + +TEST(CServiceInstanceManager, releaseServiceShouldRemoveOnlineSercie) +{ + CServiceInstanceManager instance; + TServiceId serviceId(123); + vector addresses = { "localhost:12345" }; + string reason; + CLightMemDisplayer displayer; + CLog log(NLMISC::CLog::LOG_ASSERT); + log.addDisplayer(&displayer); + instance.queryStartService("online-service", serviceId, addresses, reason); + + instance.releaseService(serviceId); + + instance.displayInfo(&log); + EXPECT_THAT( + displayer.lockStrings(), + ElementsAre( + StartsWith("Restricted services:"), + StartsWith("Online registered services:"))); +} From 47bac25f5130b95f305cf73d1b4c767a29818f6a Mon Sep 17 00:00:00 2001 From: Tobias Peters Date: Sun, 9 Jun 2024 08:00:45 +0000 Subject: [PATCH 16/28] remove unused import --- .../nelns/naming_service/service_instance_manager.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/nelns/naming_service/nelns/naming_service/service_instance_manager.cpp b/nelns/naming_service/nelns/naming_service/service_instance_manager.cpp index 582156e47f..eb0dbaefe3 100644 --- a/nelns/naming_service/nelns/naming_service/service_instance_manager.cpp +++ b/nelns/naming_service/nelns/naming_service/service_instance_manager.cpp @@ -4,7 +4,6 @@ #include #include #include -#include using std::map; using std::set; From 8a06ea7fb940a1eece48bdc13b6b1c4bd2526999 Mon Sep 17 00:00:00 2001 From: Tobias Peters Date: Sun, 9 Jun 2024 09:54:06 +0000 Subject: [PATCH 17/28] extract service class and dependent functions into separate files --- nelns/naming_service/CMakeLists.txt | 2 + nelns/naming_service/naming_service.cpp | 134 +----------------- .../nelns/naming_service/functions.cpp | 32 +++++ .../nelns/naming_service/functions.h | 21 +++ .../nelns/naming_service/naming_service.cpp | 116 +++++++++++++++ .../nelns/naming_service/naming_service.h | 32 +++++ .../nelns/naming_service/variables.h | 3 + 7 files changed, 208 insertions(+), 132 deletions(-) create mode 100644 nelns/naming_service/nelns/naming_service/functions.cpp create mode 100644 nelns/naming_service/nelns/naming_service/functions.h create mode 100644 nelns/naming_service/nelns/naming_service/naming_service.cpp create mode 100644 nelns/naming_service/nelns/naming_service/naming_service.h diff --git a/nelns/naming_service/CMakeLists.txt b/nelns/naming_service/CMakeLists.txt index 55ea2d3083..a648c72435 100644 --- a/nelns/naming_service/CMakeLists.txt +++ b/nelns/naming_service/CMakeLists.txt @@ -5,7 +5,9 @@ add_library(nelns_naming_service nelns/naming_service/do_remove.cpp nelns/naming_service/do_unregister_service.cpp nelns/naming_service/effectively_remove.cpp + nelns/naming_service/functions.cpp nelns/naming_service/helper.cpp + nelns/naming_service/naming_service.cpp nelns/naming_service/service_instance_manager.cpp nelns/naming_service/variables.cpp ) diff --git a/nelns/naming_service/naming_service.cpp b/nelns/naming_service/naming_service.cpp index bab0053c22..3c325bb859 100644 --- a/nelns/naming_service/naming_service.cpp +++ b/nelns/naming_service/naming_service.cpp @@ -51,7 +51,8 @@ #include #include #include -#include +#include +#include #include #include #include @@ -435,31 +436,6 @@ static void cbQueryPort (CMessage& msgin, TSockId from, CCallbackNetBase &netbas } -/* - * Unregisters a service if it has not been done before. - * Note: this callback is called whenever someone disconnects from the NS. - * May be there are too many calls if many clients perform many transactional lookups. - */ -static void cbDisconnect /*(const string &serviceName, TSockId from, void *arg)*/ ( TSockId from, void *arg ) -{ - doUnregisterService (from); - //displayRegisteredServices (); -} - -/* - * a service is connected, send him all services infos - */ -static void cbConnect /*(const string &serviceName, TSockId from, void *arg)*/ ( TSockId from, void *arg ) -{ - // we have to wait the registred services message to send all services because it this points, we can't know which sub net - // the service can use - - //displayRegisteredServices (); - - // set the appid with a bad id (-1) - from->setAppId (~0); -} - /*// returns the list of accessible services with a list of address static void cbRegisteredServices(CMessage& msgin, TSockId from, CCallbackNetBase &netbase) { @@ -530,112 +506,6 @@ TCallbackItem CallbackArray[] = }; -// -// Service -// - -class CNamingService : public NLNET::IService -{ -public: - - /** - * Init - */ - void init() - { - // if a baseport is available in the config file, get it - CConfigFile::CVar *var; - if ((var = ConfigFile.getVarPtr ("BasePort")) != NULL) - { - uint16 newBasePort = var->asInt (); - nlinfo ("Changing the MinBasePort number from %hu to %hu", MinBasePort, newBasePort); - sint32 delta = MaxBasePort - MinBasePort; - nlassert (delta > 0); - MinBasePort = newBasePort; - MaxBasePort = MinBasePort + uint16 (delta); - } - - // Parameters for the service instance manager - try - { - CConfigFile::CVar& uniqueServices = ConfigFile.getVar("UniqueOnShardServices"); - for ( uint i=0; i!=uniqueServices.size(); ++i ) - { - _ServiceInstances.addUniqueService( uniqueServices.asString(i), true ); - } - } - catch(Exception &) - {} - try - { - CConfigFile::CVar& uniqueServicesM = ConfigFile.getVar("UniqueByMachineServices"); - for ( uint i=0; i!=uniqueServicesM.size(); ++i ) - { - _ServiceInstances.addUniqueService( uniqueServicesM.asString(i), false ); - } - } - catch(Exception &) - {} - -/* - // we don't try to associate message from client - CNetManager::getNetBase ("NS")->ignoreAllUnknownId (true); - - // add the callback in case of disconnection - CNetManager::setConnectionCallback ("NS", cbConnect, NULL); - - // add the callback in case of disconnection - CNetManager::setDisconnectionCallback ("NS", cbDisconnect, NULL); -*/ - // DEBUG - // DebugLog->addDisplayer( new CStdDisplayer() ); - - vector v = CInetAddress::localAddresses(); - nlinfo ("%d detected local addresses:", v.size()); - for (uint i = 0; i < v.size(); i++) - { - nlinfo (" %d - '%s'",i, v[i].asString().c_str()); - } - - uint16 nsport = 50000; - if ((var = ConfigFile.getVarPtr ("NSPort")) != NULL) - { - nsport = var->asInt (); - } - - CallbackServer = new CCallbackServer; - CallbackServer->init(nsport); - CallbackServer->addCallbackArray(CallbackArray, sizeof(CallbackArray)/sizeof(CallbackArray[0])); - CallbackServer->setConnectionCallback(cbConnect, NULL); - CallbackServer->setDisconnectionCallback(cbDisconnect, NULL); - } - - /** - * Update - */ - bool update () - { - checkWaitingUnregistrationServices (); - - CallbackServer->update (); - - return true; - } - - void release() - { - if (CallbackServer != NULL) - delete CallbackServer; - CallbackServer = NULL; - } - -private: - - /// Service instance manager singleton - CServiceInstanceManager _ServiceInstances; -}; - - static const char* getCompleteServiceName(const IService* theService) { static std::string s; diff --git a/nelns/naming_service/nelns/naming_service/functions.cpp b/nelns/naming_service/nelns/naming_service/functions.cpp new file mode 100644 index 0000000000..e5bddbec1d --- /dev/null +++ b/nelns/naming_service/nelns/naming_service/functions.cpp @@ -0,0 +1,32 @@ +#include + +#include + +using NLNET::CInetAddress; +using NLNET::TServiceId; +using NLNET::TSockId; + +/* + * Unregisters a service if it has not been done before. + * Note: this callback is called whenever someone disconnects from the NS. + * May be there are too many calls if many clients perform many transactional lookups. + */ +void cbDisconnect(TSockId from, void *arg) +{ + doUnregisterService(from); + // displayRegisteredServices (); +} + +/* + * a service is connected, send him all services infos + */ +void cbConnect(TSockId from, void *arg) +{ + // we have to wait the registred services message to send all services because it this points, we can't know which sub net + // the service can use + + // displayRegisteredServices (); + + // set the appid with a bad id (-1) + from->setAppId(~0); +} diff --git a/nelns/naming_service/nelns/naming_service/functions.h b/nelns/naming_service/nelns/naming_service/functions.h new file mode 100644 index 0000000000..c1424addd7 --- /dev/null +++ b/nelns/naming_service/nelns/naming_service/functions.h @@ -0,0 +1,21 @@ +#ifndef NELNS_NAMING_SERVICE_FUNCTIONS_H +#define NELNS_NAMING_SERVICE_FUNCTIONS_H + +#include +#include + +/* + * Unregisters a service if it has not been done before. + * Note: this callback is called whenever someone disconnects from the NS. + * May be there are too many calls if many clients perform many transactional lookups. + */ +void cbDisconnect(NLNET::TSockId from, void *arg); + +/* + * a service is connected, send him all services infos + */ +void cbConnect(NLNET::TSockId from, void *arg); + +void checkWaitingUnregistrationServices(); + +#endif // NELNS_NAMING_SERVICE_FUNCTIONS_H diff --git a/nelns/naming_service/nelns/naming_service/naming_service.cpp b/nelns/naming_service/nelns/naming_service/naming_service.cpp new file mode 100644 index 0000000000..e65919b8be --- /dev/null +++ b/nelns/naming_service/nelns/naming_service/naming_service.cpp @@ -0,0 +1,116 @@ +#include + +#include +#include +#include +#include +#include +#include +#include + +using std::map; +using std::set; +using std::string; +using std::vector; + +using NLMISC::CConfigFile; +using NLMISC::Exception; +using NLMISC::toString; +using NLNET::CCallbackServer; +using NLNET::CInetAddress; +using NLNET::CUnifiedNetwork; +using NLNET::TServiceId; +using NLNET::TSockId; + +/** + * Init + */ +void CNamingService::init() +{ + // if a baseport is available in the config file, get it + CConfigFile::CVar *var; + if ((var = ConfigFile.getVarPtr("BasePort")) != NULL) + { + uint16 newBasePort = var->asInt(); + nlinfo("Changing the MinBasePort number from %hu to %hu", MinBasePort, newBasePort); + sint32 delta = MaxBasePort - MinBasePort; + nlassert(delta > 0); + MinBasePort = newBasePort; + MaxBasePort = MinBasePort + uint16(delta); + } + + // Parameters for the service instance manager + try + { + CConfigFile::CVar &uniqueServices = ConfigFile.getVar("UniqueOnShardServices"); + for (uint i = 0; i != uniqueServices.size(); ++i) + { + _ServiceInstances.addUniqueService(uniqueServices.asString(i), true); + } + } + catch (Exception &) + { + } + try + { + CConfigFile::CVar &uniqueServicesM = ConfigFile.getVar("UniqueByMachineServices"); + for (uint i = 0; i != uniqueServicesM.size(); ++i) + { + _ServiceInstances.addUniqueService(uniqueServicesM.asString(i), false); + } + } + catch (Exception &) + { + } + + /* + // we don't try to associate message from client + CNetManager::getNetBase ("NS")->ignoreAllUnknownId (true); + + // add the callback in case of disconnection + CNetManager::setConnectionCallback ("NS", cbConnect, NULL); + + // add the callback in case of disconnection + CNetManager::setDisconnectionCallback ("NS", cbDisconnect, NULL); + */ + // DEBUG + // DebugLog->addDisplayer( new CStdDisplayer() ); + + vector v = CInetAddress::localAddresses(); + nlinfo("%d detected local addresses:", v.size()); + for (uint i = 0; i < v.size(); i++) + { + nlinfo(" %d - '%s'", i, v[i].asString().c_str()); + } + + uint16 nsport = 50000; + if ((var = ConfigFile.getVarPtr("NSPort")) != NULL) + { + nsport = var->asInt(); + } + + CallbackServer = new CCallbackServer; + CallbackServer->init(nsport); + CallbackServer->addCallbackArray(CallbackArray, sizeof(CallbackArray) / sizeof(CallbackArray[0])); + CallbackServer->setConnectionCallback(cbConnect, NULL); + CallbackServer->setDisconnectionCallback(cbDisconnect, NULL); +} + +/** + * Update + */ +bool CNamingService::update() +{ + checkWaitingUnregistrationServices(); + + CallbackServer->update(); + + return true; +} + +void CNamingService::release() +{ + if (CallbackServer != NULL) + delete CallbackServer; + CallbackServer = NULL; +} diff --git a/nelns/naming_service/nelns/naming_service/naming_service.h b/nelns/naming_service/nelns/naming_service/naming_service.h new file mode 100644 index 0000000000..1d4d92b69e --- /dev/null +++ b/nelns/naming_service/nelns/naming_service/naming_service.h @@ -0,0 +1,32 @@ +#ifndef NELNS_NAMING_SERVICE_SERVICE_NAMING_SERVICE_H +#define NELNS_NAMING_SERVICE_SERVICE_NAMING_SERVICE_H + +#include +#include +#include + +// +// Service +// + +class CNamingService : public NLNET::IService +{ +public: + /** + * Init + */ + void init(); + + /** + * Update + */ + bool update(); + + void release(); + +private: + /// Service instance manager singleton + CServiceInstanceManager _ServiceInstances; +}; + +#endif // NELNS_NAMING_SERVICE_SERVICE_NAMING_SERVICE_H diff --git a/nelns/naming_service/nelns/naming_service/variables.h b/nelns/naming_service/nelns/naming_service/variables.h index 7fe547e92e..53c7659c73 100644 --- a/nelns/naming_service/nelns/naming_service/variables.h +++ b/nelns/naming_service/nelns/naming_service/variables.h @@ -5,6 +5,7 @@ #include #include +#include #include #include @@ -23,4 +24,6 @@ extern const NLMISC::TTime UnregisterTimeout; /// After 10s we remove an unregis extern NLNET::CCallbackServer *CallbackServer; +extern NLNET::TCallbackItem CallbackArray[5]; + #endif // NELNS_NAMING_SERVICE_VARIABLES_H From 80afc237ba43e209c079e924ccd3321c7811fcd6 Mon Sep 17 00:00:00 2001 From: Tobias Peters Date: Sun, 9 Jun 2024 10:33:58 +0000 Subject: [PATCH 18/28] extract functions into separate files and organize imports --- nelns/naming_service/naming_service.cpp | 431 ------------------ .../nelns/naming_service/can_access.h | 1 + .../nelns/naming_service/do_remove.cpp | 1 + .../naming_service/do_unregister_service.cpp | 1 + .../nelns/naming_service/functions.cpp | 368 ++++++++++++++- .../nelns/naming_service/functions.h | 73 ++- .../nelns/naming_service/naming_service.cpp | 21 +- .../nelns/naming_service/naming_service.h | 3 +- .../nelns/naming_service/service_entry.h | 1 + .../service_instance_manager.cpp | 1 + .../naming_service/service_instance_manager.h | 1 + .../nelns/naming_service/variables.h | 4 +- nelns/naming_service/tests/CMakeLists.txt | 1 + .../tests/naming_service.test.cpp | 13 +- 14 files changed, 476 insertions(+), 444 deletions(-) diff --git a/nelns/naming_service/naming_service.cpp b/nelns/naming_service/naming_service.cpp index 3c325bb859..57e550c4fa 100644 --- a/nelns/naming_service/naming_service.cpp +++ b/nelns/naming_service/naming_service.cpp @@ -44,13 +44,9 @@ #include #include -#include #include -#include -#include #include -#include #include #include #include @@ -79,433 +75,6 @@ NLMISC_COMMAND(test, "none", "none") } -// -// Functions -// - - -void displayRegisteredServices (CLog *log = InfoLog) -{ - log->displayNL ("Display the %d registered services :", RegisteredServices.size()); - for (list::iterator it = RegisteredServices.begin(); it != RegisteredServices.end (); it++) - { - TSockId id = (*it).SockId; - if (id == NULL) - { - log->displayNL ("> %s-%hu %s '%s' %s %d addr", (*it).Name.c_str(), it->SId.get(), "", "", (*it).WaitingUnregistration?"WaitUnreg":"", (*it).Addr.size()); - for(uint i = 0; i < (*it).Addr.size(); i++) - log->displayNL (" '%s'", (*it).Addr[i].asString().c_str()); - } - else - { - log->displayNL ("> %s-%hu %s '%s' %s %d addr", (*it).Name.c_str(), it->SId.get(), (*it).SockId->asString().c_str(), CallbackServer->hostAddress((*it).SockId).asString().c_str(), (*it).WaitingUnregistration?"WaitUnreg":"", (*it).Addr.size()); - for(uint i = 0; i < (*it).Addr.size(); i++) - log->displayNL (" '%s'", (*it).Addr[i].asString().c_str()); - } - } - log->displayNL ("End of the list"); -} - - -/* - * Helper function for cbRegister. - * If alloc_sid is true, sid is ignored - * Returns false in case of failure of sid allocation or bad sid provided - * Note: the reply is included in this function, because it must be done before things such as syncUniTime() - */ -bool doRegister (const string &name, const vector &addr, TServiceId sid, TSockId from, CCallbackNetBase &netbase, bool reconnection = false) -{ - // Find if the service is not already registered - string reason; - uint8 ok = true; - - if (sid.get() == 0) - { - // we have to find a sid - sid = BaseSId; - bool found = false; - while (!found) - { - list::iterator it; - for (it = RegisteredServices.begin(); it != RegisteredServices.end (); it++) - { - if ((*it).SId == sid) - { - break; - } - } - if (it == RegisteredServices.end ()) - { - // ok, we have an empty sid - found = true; - } - else - { - sid.set(sid.get()+1); - if (sid.get() == 0) // round the clock - { - nlwarning ("Service identifier allocation overflow"); - ok = false; - break; - } - } - } - - } - else - { - // we have to check that the user provided sid is available - list::iterator it; - for (it = RegisteredServices.begin(); it != RegisteredServices.end (); it++) - { - if ((*it).SId == sid) - { - nlwarning ("Sid %d already used by another service", sid.get()); - ok = false; - break; - } - } - if (it != RegisteredServices.end ()) - { - ok = true; - } - } - - // if ok, register the service and send a broadcast to other people - if (ok) - { - // Check if the instance is allowed to start, according to the restriction in the config file - if ( CServiceInstanceManager::getInstance()->queryStartService( name, sid, addr, reason ) ) - { - // add him in the registered list - RegisteredServices.push_back (CServiceEntry(from, addr, name, sid)); - - // tell to everybody but not him that this service is registered - if (!reconnection) - { - CMessage msgout ("RGB"); - TServiceId::size_type s = 1; - msgout.serial (s); - msgout.serial (const_cast(name)); - msgout.serial (sid); - // we need to send all addr to all services even if the service can't access because we use the address index - // to know which connection comes. - msgout.serialCont (const_cast &>(addr)); - nlinfo ("The service is %s-%d, broadcast the Registration to everybody", name.c_str(), sid.get()); - - vector accessibleAddress; - for (list::iterator it3 = RegisteredServices.begin(); it3 != RegisteredServices.end (); it3++) - { - // send only services that can be accessed and not itself - if ((*it3).SId != sid && canAccess(addr, (*it3), accessibleAddress)) - { - CallbackServer->send (msgout, (*it3).SockId); - //CNetManager::send ("NS", msgout, (*it3).SockId); - nldebug ("Broadcast to %s-%hu", (*it3).Name.c_str(), it3->SId.get()); - } - } - } - - // set the sid only if it s ok - from->setAppId (sid.get()); - } - else - { - // Reply "startup denied", and do not send registration to other services - ok = false; - } - } - - // send the message to the service to say if it s ok or not - if (!reconnection) - { - // send the answer to the client - CMessage msgout ("RG"); - msgout.serial (ok); - if (ok) - { - msgout.serial (sid); - - // send him all services available (also itself) - TServiceId::size_type nb = 0; - - vector accessibleAddress; - - for (list::iterator it2 = RegisteredServices.begin(); it2 != RegisteredServices.end (); it2++) - { - // send only services that are available - if (canAccess(addr, (*it2), accessibleAddress)) - nb++; - } - msgout.serial (nb); - - for (list::iterator it = RegisteredServices.begin(); it != RegisteredServices.end (); it++) - { - // send only services that are available - if (canAccess(addr, (*it), accessibleAddress)) - { - msgout.serial ((*it).Name); - msgout.serial ((*it).SId); - msgout.serialCont ((*it).Addr); - } - } - } - else - { - msgout.serial( reason ); - } - - netbase.send (msgout, from); - netbase.flush (from); - } - - //displayRegisteredServices (); - - return ok!=0; -} - -void checkWaitingUnregistrationServices () -{ - for (list::iterator it = RegisteredServices.begin(); it != RegisteredServices.end ();) - { - if ((*it).WaitingUnregistration && ((*it).WaitingUnregistrationServices.empty() || CTime::getLocalTime() > (*it).WaitingUnregistrationTime + UnregisterTimeout)) - { - if ((*it).WaitingUnregistrationServices.empty()) - { - nlinfo ("Removing the service %s-%hu because all services ACKd the removal", (*it).Name.c_str(), (*it).SId.get()); - } - else - { - string res; - for (list::iterator it2 = (*it).WaitingUnregistrationServices.begin(); it2 != (*it).WaitingUnregistrationServices.end (); it2++) - { - res += toString(it2->get()) + " "; - } - nlwarning ("Removing the service %s-%hu because time out occurs (service numbers %s didn't ACK)", (*it).Name.c_str(), (*it).SId.get(), res.c_str()); - } - it = effectivelyRemove (it); - } - else - { - it++; - } - } -} - - -/** - * Callback for service unregistration ACK. Mean that a service was ACK the unregistration broadcast - */ -static void cbACKUnregistration (CMessage& msgin, TSockId from, CCallbackNetBase &netbase) -{ - TServiceId sid; - msgin.serial (sid); - - for (list::iterator it = RegisteredServices.begin(); it != RegisteredServices.end (); it++) - { - if ((*it).SId == sid && (*it).WaitingUnregistration) - { - for (list::iterator it2 = (*it).WaitingUnregistrationServices.begin(); it2 != (*it).WaitingUnregistrationServices.end (); it2++) - { - if (*it2 == TServiceId(uint16(from->appId()))) - { - // remove the acked service - (*it).WaitingUnregistrationServices.erase (it2); - checkWaitingUnregistrationServices (); - return; - } - } - } - } -} - - -/** - * Callback for service registration when the naming service goes down and up (don't need to broadcast) - */ -static void cbResendRegisteration (CMessage& msgin, TSockId from, CCallbackNetBase &netbase) -{ - string name; - vector addr; - TServiceId sid; - msgin.serial (name); - msgin.serialCont (addr); - msgin.serial (sid); - - doRegister (name, addr, sid, from, netbase, true); -} - - - -/** - * Callback for service registration. - * - * Message expected : RG - * - Name of service to register (string) - * - Address of service (CInetAddress) - * - * Message emitted : RG - * - Allocated service identifier (TServiceId) or 0 if failed - */ -static void cbRegister (CMessage& msgin, TSockId from, CCallbackNetBase &netbase) -{ - string name; - vector addr; - TServiceId sid; - msgin.serial (name); - msgin.serialCont (addr); - msgin.serial (sid); - - doRegister (name, addr, sid, from, netbase); -} - - -/** - * Callback for service unregistration. - * - * Message expected : UNI - * - Service identifier (TServiceId) - */ -static void cbUnregisterSId (CMessage& msgin, TSockId from, CCallbackNetBase &netbase) -{ - TServiceId sid; - msgin.serial( sid ); - - doUnregisterService (sid); - //displayRegisteredServices (); -} - - -/* - * Helper function for cbQueryPort - * - * \warning QueryPort + Registration is not atomic so more than one service could ask a port before register - */ -uint16 doAllocatePort (const CInetAddress &addr) -{ - static uint16 nextAvailablePort = MinBasePort; - - // check if nextavailableport is free - - if (nextAvailablePort >= MaxBasePort) nextAvailablePort = MinBasePort; - - bool ok; - do - { - ok = true; - list::iterator it; - for (it = RegisteredServices.begin(); it != RegisteredServices.end (); it++) - { - if ((*it).Addr[0].port () == nextAvailablePort) - { - nextAvailablePort++; - ok = false; - break; - } - } - } - while (!ok); - - return nextAvailablePort++; -} - - -/** - * Callback for port allocation - * Note: if a service queries a port but does not register itself to the naming service, the - * port will remain allocated and unused. - * - * Message expected : QP - * - Name of service to register (string) - * - Address of service (CInetAddress) (its port can be 0) - * - * Message emitted : QP - * - Allocated port number (uint16) - */ -static void cbQueryPort (CMessage& msgin, TSockId from, CCallbackNetBase &netbase) -{ - // Allocate port - uint16 port = doAllocatePort (netbase.hostAddress (from)); - - // Send port back - CMessage msgout ("QP"); - msgout.serial (port); - netbase.send (msgout, from); - - nlinfo ("The service got port %hu", port); -} - - -/*// returns the list of accessible services with a list of address -static void cbRegisteredServices(CMessage& msgin, TSockId from, CCallbackNetBase &netbase) -{ - vector addr; - msgin.serialCont (addr); - - nlinfo ("New service ask me the available services, sending him all services available"); - // send to the new service the list of all services that this service can access (depending of his sub net) - - CMessage msgout ("RGB"); - - uint8 nb = 0; - - vector accessibleAddress; - - for (list::iterator it2 = RegisteredServices.begin(); it2 != RegisteredServices.end (); it2++) - { - // send only services that are available - if (canAccess(addr, (*it2), accessibleAddress)) - nb++; - } - - msgout.serial (nb); - - for (list::iterator it = RegisteredServices.begin(); it != RegisteredServices.end (); it++) - { - // send only services that are available - if (canAccess(addr, (*it), accessibleAddress)) - { - msgout.serial ((*it).Name); - msgout.serial ((*it).SId); - msgout.serialCont (accessibleAddress); - } - } - - CNetManager::send ("NS", msgout, from); -}*/ - - -/* - * Helper that emulates layer5 send() - */ -/*void sendToService( uint16 sid, CMessage& msgout ) -{ - list::iterator it; - for (it = RegisteredServices.begin(); it != RegisteredServices.end (); it++) - { - if ((*it).SId == sid) - { - CallbackServer->send (msgout, (*it).SockId); - } - } -}*/ - - -// -// Callback array -// - -TCallbackItem CallbackArray[] = -{ - { "RG", cbRegister }, - { "RRG", cbResendRegisteration }, - { "QP", cbQueryPort }, - { "UNI", cbUnregisterSId }, - { "ACK_UNI", cbACKUnregistration }, -// { "RS", cbRegisteredServices }, -}; - - static const char* getCompleteServiceName(const IService* theService) { static std::string s; diff --git a/nelns/naming_service/nelns/naming_service/can_access.h b/nelns/naming_service/nelns/naming_service/can_access.h index 31bcd68804..2cadd81358 100644 --- a/nelns/naming_service/nelns/naming_service/can_access.h +++ b/nelns/naming_service/nelns/naming_service/can_access.h @@ -5,6 +5,7 @@ #include #include + #include bool canAccess(const std::vector &addr, const CServiceEntry &entry, std::vector &accessibleAddr); diff --git a/nelns/naming_service/nelns/naming_service/do_remove.cpp b/nelns/naming_service/nelns/naming_service/do_remove.cpp index caffc4ea3a..bd9e3d82fa 100644 --- a/nelns/naming_service/nelns/naming_service/do_remove.cpp +++ b/nelns/naming_service/nelns/naming_service/do_remove.cpp @@ -1,6 +1,7 @@ #include #include + #include #include #include diff --git a/nelns/naming_service/nelns/naming_service/do_unregister_service.cpp b/nelns/naming_service/nelns/naming_service/do_unregister_service.cpp index 9ed14d1601..ccde9a9ced 100644 --- a/nelns/naming_service/nelns/naming_service/do_unregister_service.cpp +++ b/nelns/naming_service/nelns/naming_service/do_unregister_service.cpp @@ -1,6 +1,7 @@ #include #include + #include #include #include diff --git a/nelns/naming_service/nelns/naming_service/functions.cpp b/nelns/naming_service/nelns/naming_service/functions.cpp index e5bddbec1d..671f7083bb 100644 --- a/nelns/naming_service/nelns/naming_service/functions.cpp +++ b/nelns/naming_service/nelns/naming_service/functions.cpp @@ -1,17 +1,381 @@ #include +#include + +#include +#include + +#include #include +#include +#include +#include + +using std::list; +using std::string; +using std::vector; +using NLMISC::CLog; +using NLMISC::CTime; +using NLMISC::InfoLog; +using NLMISC::toString; +using NLNET::CCallbackNetBase; using NLNET::CInetAddress; +using NLNET::CMessage; using NLNET::TServiceId; using NLNET::TSockId; +// +// Functions +// + +void displayRegisteredServices(CLog *log) +{ + log->displayNL("Display the %d registered services :", RegisteredServices.size()); + for (list::iterator it = RegisteredServices.begin(); it != RegisteredServices.end(); it++) + { + TSockId id = (*it).SockId; + if (id == NULL) + { + log->displayNL("> %s-%hu %s '%s' %s %d addr", (*it).Name.c_str(), it->SId.get(), "", "", (*it).WaitingUnregistration ? "WaitUnreg" : "", (*it).Addr.size()); + for (uint i = 0; i < (*it).Addr.size(); i++) + log->displayNL(" '%s'", (*it).Addr[i].asString().c_str()); + } + else + { + log->displayNL("> %s-%hu %s '%s' %s %d addr", (*it).Name.c_str(), it->SId.get(), (*it).SockId->asString().c_str(), CallbackServer->hostAddress((*it).SockId).asString().c_str(), (*it).WaitingUnregistration ? "WaitUnreg" : "", (*it).Addr.size()); + for (uint i = 0; i < (*it).Addr.size(); i++) + log->displayNL(" '%s'", (*it).Addr[i].asString().c_str()); + } + } + log->displayNL("End of the list"); +} + +/* + * Helper function for cbRegister. + * If alloc_sid is true, sid is ignored + * Returns false in case of failure of sid allocation or bad sid provided + * Note: the reply is included in this function, because it must be done before things such as syncUniTime() + */ +bool doRegister(const string &name, const vector &addr, TServiceId sid, TSockId from, CCallbackNetBase &netbase, bool reconnection) +{ + // Find if the service is not already registered + string reason; + uint8 ok = true; + + if (sid.get() == 0) + { + // we have to find a sid + sid = BaseSId; + bool found = false; + while (!found) + { + list::iterator it; + for (it = RegisteredServices.begin(); it != RegisteredServices.end(); it++) + { + if ((*it).SId == sid) + { + break; + } + } + if (it == RegisteredServices.end()) + { + // ok, we have an empty sid + found = true; + } + else + { + sid.set(sid.get() + 1); + if (sid.get() == 0) // round the clock + { + nlwarning("Service identifier allocation overflow"); + ok = false; + break; + } + } + } + } + else + { + // we have to check that the user provided sid is available + list::iterator it; + for (it = RegisteredServices.begin(); it != RegisteredServices.end(); it++) + { + if ((*it).SId == sid) + { + nlwarning("Sid %d already used by another service", sid.get()); + ok = false; + break; + } + } + if (it != RegisteredServices.end()) + { + ok = true; + } + } + + // if ok, register the service and send a broadcast to other people + if (ok) + { + // Check if the instance is allowed to start, according to the restriction in the config file + if (CServiceInstanceManager::getInstance()->queryStartService(name, sid, addr, reason)) + { + // add him in the registered list + RegisteredServices.push_back(CServiceEntry(from, addr, name, sid)); + + // tell to everybody but not him that this service is registered + if (!reconnection) + { + CMessage msgout("RGB"); + TServiceId::size_type s = 1; + msgout.serial(s); + msgout.serial(const_cast(name)); + msgout.serial(sid); + // we need to send all addr to all services even if the service can't access because we use the address index + // to know which connection comes. + msgout.serialCont(const_cast &>(addr)); + nlinfo("The service is %s-%d, broadcast the Registration to everybody", name.c_str(), sid.get()); + + vector accessibleAddress; + for (list::iterator it3 = RegisteredServices.begin(); it3 != RegisteredServices.end(); it3++) + { + // send only services that can be accessed and not itself + if ((*it3).SId != sid && canAccess(addr, (*it3), accessibleAddress)) + { + CallbackServer->send(msgout, (*it3).SockId); + // CNetManager::send ("NS", msgout, (*it3).SockId); + nldebug("Broadcast to %s-%hu", (*it3).Name.c_str(), it3->SId.get()); + } + } + } + + // set the sid only if it s ok + from->setAppId(sid.get()); + } + else + { + // Reply "startup denied", and do not send registration to other services + ok = false; + } + } + + // send the message to the service to say if it s ok or not + if (!reconnection) + { + // send the answer to the client + CMessage msgout("RG"); + msgout.serial(ok); + if (ok) + { + msgout.serial(sid); + + // send him all services available (also itself) + TServiceId::size_type nb = 0; + + vector accessibleAddress; + + for (list::iterator it2 = RegisteredServices.begin(); it2 != RegisteredServices.end(); it2++) + { + // send only services that are available + if (canAccess(addr, (*it2), accessibleAddress)) + nb++; + } + msgout.serial(nb); + + for (list::iterator it = RegisteredServices.begin(); it != RegisteredServices.end(); it++) + { + // send only services that are available + if (canAccess(addr, (*it), accessibleAddress)) + { + msgout.serial((*it).Name); + msgout.serial((*it).SId); + msgout.serialCont((*it).Addr); + } + } + } + else + { + msgout.serial(reason); + } + + netbase.send(msgout, from); + netbase.flush(from); + } + + // displayRegisteredServices (); + + return ok != 0; +} + +void checkWaitingUnregistrationServices() +{ + for (list::iterator it = RegisteredServices.begin(); it != RegisteredServices.end();) + { + if ((*it).WaitingUnregistration && ((*it).WaitingUnregistrationServices.empty() || CTime::getLocalTime() > (*it).WaitingUnregistrationTime + UnregisterTimeout)) + { + if ((*it).WaitingUnregistrationServices.empty()) + { + nlinfo("Removing the service %s-%hu because all services ACKd the removal", (*it).Name.c_str(), (*it).SId.get()); + } + else + { + string res; + for (list::iterator it2 = (*it).WaitingUnregistrationServices.begin(); it2 != (*it).WaitingUnregistrationServices.end(); it2++) + { + res += toString(it2->get()) + " "; + } + nlwarning("Removing the service %s-%hu because time out occurs (service numbers %s didn't ACK)", (*it).Name.c_str(), (*it).SId.get(), res.c_str()); + } + it = effectivelyRemove(it); + } + else + { + it++; + } + } +} + +/** + * Callback for service unregistration ACK. Mean that a service was ACK the unregistration broadcast + */ +void cbACKUnregistration(CMessage &msgin, TSockId from, CCallbackNetBase &netbase) +{ + TServiceId sid; + msgin.serial(sid); + + for (list::iterator it = RegisteredServices.begin(); it != RegisteredServices.end(); it++) + { + if ((*it).SId == sid && (*it).WaitingUnregistration) + { + for (list::iterator it2 = (*it).WaitingUnregistrationServices.begin(); it2 != (*it).WaitingUnregistrationServices.end(); it2++) + { + if (*it2 == TServiceId(uint16(from->appId()))) + { + // remove the acked service + (*it).WaitingUnregistrationServices.erase(it2); + checkWaitingUnregistrationServices(); + return; + } + } + } + } +} + +/** + * Callback for service registration when the naming service goes down and up (don't need to broadcast) + */ +void cbResendRegisteration(CMessage &msgin, TSockId from, CCallbackNetBase &netbase) +{ + string name; + vector addr; + TServiceId sid; + msgin.serial(name); + msgin.serialCont(addr); + msgin.serial(sid); + + doRegister(name, addr, sid, from, netbase, true); +} + +/** + * Callback for service registration. + * + * Message expected : RG + * - Name of service to register (string) + * - Address of service (CInetAddress) + * + * Message emitted : RG + * - Allocated service identifier (TServiceId) or 0 if failed + */ +void cbRegister(CMessage &msgin, TSockId from, CCallbackNetBase &netbase) +{ + string name; + vector addr; + TServiceId sid; + msgin.serial(name); + msgin.serialCont(addr); + msgin.serial(sid); + + doRegister(name, addr, sid, from, netbase); +} + +/** + * Callback for service unregistration. + * + * Message expected : UNI + * - Service identifier (TServiceId) + */ +void cbUnregisterSId(CMessage &msgin, TSockId from, CCallbackNetBase &netbase) +{ + TServiceId sid; + msgin.serial(sid); + + doUnregisterService(sid); + // displayRegisteredServices (); +} + +/* + * Helper function for cbQueryPort + * + * \warning QueryPort + Registration is not atomic so more than one service could ask a port before register + */ +uint16 doAllocatePort(const CInetAddress &addr) +{ + static uint16 nextAvailablePort = MinBasePort; + + // check if nextavailableport is free + + if (nextAvailablePort >= MaxBasePort) nextAvailablePort = MinBasePort; + + bool ok; + do + { + ok = true; + list::iterator it; + for (it = RegisteredServices.begin(); it != RegisteredServices.end(); it++) + { + if ((*it).Addr[0].port() == nextAvailablePort) + { + nextAvailablePort++; + ok = false; + break; + } + } + } while (!ok); + + return nextAvailablePort++; +} + +/** + * Callback for port allocation + * Note: if a service queries a port but does not register itself to the naming service, the + * port will remain allocated and unused. + * + * Message expected : QP + * - Name of service to register (string) + * - Address of service (CInetAddress) (its port can be 0) + * + * Message emitted : QP + * - Allocated port number (uint16) + */ +void cbQueryPort(CMessage &msgin, TSockId from, CCallbackNetBase &netbase) +{ + // Allocate port + uint16 port = doAllocatePort(netbase.hostAddress(from)); + + // Send port back + CMessage msgout("QP"); + msgout.serial(port); + netbase.send(msgout, from); + + nlinfo("The service got port %hu", port); +} + /* * Unregisters a service if it has not been done before. * Note: this callback is called whenever someone disconnects from the NS. * May be there are too many calls if many clients perform many transactional lookups. */ -void cbDisconnect(TSockId from, void *arg) +void cbDisconnect /*(const string &serviceName, TSockId from, void *arg)*/ (TSockId from, void *arg) { doUnregisterService(from); // displayRegisteredServices (); @@ -20,7 +384,7 @@ void cbDisconnect(TSockId from, void *arg) /* * a service is connected, send him all services infos */ -void cbConnect(TSockId from, void *arg) +void cbConnect /*(const string &serviceName, TSockId from, void *arg)*/ (TSockId from, void *arg) { // we have to wait the registred services message to send all services because it this points, we can't know which sub net // the service can use diff --git a/nelns/naming_service/nelns/naming_service/functions.h b/nelns/naming_service/nelns/naming_service/functions.h index c1424addd7..bdced47771 100644 --- a/nelns/naming_service/nelns/naming_service/functions.h +++ b/nelns/naming_service/nelns/naming_service/functions.h @@ -1,8 +1,77 @@ #ifndef NELNS_NAMING_SERVICE_FUNCTIONS_H #define NELNS_NAMING_SERVICE_FUNCTIONS_H -#include +#include +#include + #include +#include +#include +#include +#include + +void displayRegisteredServices(NLMISC::CLog *log = NLMISC::InfoLog); + +/* + * Helper function for cbRegister. + * If alloc_sid is true, sid is ignored + * Returns false in case of failure of sid allocation or bad sid provided + * Note: the reply is included in this function, because it must be done before things such as syncUniTime() + */ +bool doRegister(const std::string &name, const std::vector &addr, NLNET::TServiceId sid, NLNET::TSockId from, NLNET::CCallbackNetBase &netbase, bool reconnection = false); + +void checkWaitingUnregistrationServices(); + +/** + * Callback for service unregistration ACK. Mean that a service was ACK the unregistration broadcast + */ +void cbACKUnregistration(NLNET::CMessage &msgin, NLNET::TSockId from, NLNET::CCallbackNetBase &netbase); + +/** + * Callback for service registration when the naming service goes down and up (don't need to broadcast) + */ +void cbResendRegisteration(NLNET::CMessage &msgin, NLNET::TSockId from, NLNET::CCallbackNetBase &netbase); + +/** + * Callback for service registration. + * + * Message expected : RG + * - Name of service to register (string) + * - Address of service (CInetAddress) + * + * Message emitted : RG + * - Allocated service identifier (TServiceId) or 0 if failed + */ +void cbRegister(NLNET::CMessage &msgin, NLNET::TSockId from, NLNET::CCallbackNetBase &netbase); + +/** + * Callback for service unregistration. + * + * Message expected : UNI + * - Service identifier (TServiceId) + */ +void cbUnregisterSId(NLNET::CMessage &msgin, NLNET::TSockId from, NLNET::CCallbackNetBase &netbase); + +/* + * Helper function for cbQueryPort + * + * \warning QueryPort + Registration is not atomic so more than one service could ask a port before register + */ +uint16 doAllocatePort(const NLNET::CInetAddress &addr); + +/** + * Callback for port allocation + * Note: if a service queries a port but does not register itself to the naming service, the + * port will remain allocated and unused. + * + * Message expected : QP + * - Name of service to register (string) + * - Address of service (CInetAddress) (its port can be 0) + * + * Message emitted : QP + * - Allocated port number (uint16) + */ +void cbQueryPort(NLNET::CMessage &msgin, NLNET::TSockId from, NLNET::CCallbackNetBase &netbase); /* * Unregisters a service if it has not been done before. @@ -16,6 +85,4 @@ void cbDisconnect(NLNET::TSockId from, void *arg); */ void cbConnect(NLNET::TSockId from, void *arg); -void checkWaitingUnregistrationServices(); - #endif // NELNS_NAMING_SERVICE_FUNCTIONS_H diff --git a/nelns/naming_service/nelns/naming_service/naming_service.cpp b/nelns/naming_service/nelns/naming_service/naming_service.cpp index e65919b8be..bed4afa4ad 100644 --- a/nelns/naming_service/nelns/naming_service/naming_service.cpp +++ b/nelns/naming_service/nelns/naming_service/naming_service.cpp @@ -1,9 +1,12 @@ #include -#include -#include #include +#include +#include + +#include #include + #include #include #include @@ -19,9 +22,23 @@ using NLMISC::toString; using NLNET::CCallbackServer; using NLNET::CInetAddress; using NLNET::CUnifiedNetwork; +using NLNET::TCallbackItem; using NLNET::TServiceId; using NLNET::TSockId; +// +// Callback array +// + +TCallbackItem CallbackArray[] = { + { "RG", cbRegister }, + { "RRG", cbResendRegisteration }, + { "QP", cbQueryPort }, + { "UNI", cbUnregisterSId }, + { "ACK_UNI", cbACKUnregistration }, + // { "RS", cbRegisteredServices }, +}; + /** * Init */ diff --git a/nelns/naming_service/nelns/naming_service/naming_service.h b/nelns/naming_service/nelns/naming_service/naming_service.h index 1d4d92b69e..c4c41babe3 100644 --- a/nelns/naming_service/nelns/naming_service/naming_service.h +++ b/nelns/naming_service/nelns/naming_service/naming_service.h @@ -1,8 +1,9 @@ #ifndef NELNS_NAMING_SERVICE_SERVICE_NAMING_SERVICE_H #define NELNS_NAMING_SERVICE_SERVICE_NAMING_SERVICE_H -#include #include +#include + #include // diff --git a/nelns/naming_service/nelns/naming_service/service_entry.h b/nelns/naming_service/nelns/naming_service/service_entry.h index 8bb59b4068..87ca9ac2e2 100644 --- a/nelns/naming_service/nelns/naming_service/service_entry.h +++ b/nelns/naming_service/nelns/naming_service/service_entry.h @@ -5,6 +5,7 @@ #include #include + #include #include #include diff --git a/nelns/naming_service/nelns/naming_service/service_instance_manager.cpp b/nelns/naming_service/nelns/naming_service/service_instance_manager.cpp index eb0dbaefe3..07e1acd354 100644 --- a/nelns/naming_service/nelns/naming_service/service_instance_manager.cpp +++ b/nelns/naming_service/nelns/naming_service/service_instance_manager.cpp @@ -1,6 +1,7 @@ #include #include + #include #include #include diff --git a/nelns/naming_service/nelns/naming_service/service_instance_manager.h b/nelns/naming_service/nelns/naming_service/service_instance_manager.h index 25d2370d9b..0bfaf32121 100644 --- a/nelns/naming_service/nelns/naming_service/service_instance_manager.h +++ b/nelns/naming_service/nelns/naming_service/service_instance_manager.h @@ -10,6 +10,7 @@ #include #include #include + #include #include diff --git a/nelns/naming_service/nelns/naming_service/variables.h b/nelns/naming_service/nelns/naming_service/variables.h index 53c7659c73..8f3cf72602 100644 --- a/nelns/naming_service/nelns/naming_service/variables.h +++ b/nelns/naming_service/nelns/naming_service/variables.h @@ -5,8 +5,10 @@ #include #include + #include #include + #include // @@ -24,6 +26,4 @@ extern const NLMISC::TTime UnregisterTimeout; /// After 10s we remove an unregis extern NLNET::CCallbackServer *CallbackServer; -extern NLNET::TCallbackItem CallbackArray[5]; - #endif // NELNS_NAMING_SERVICE_VARIABLES_H diff --git a/nelns/naming_service/tests/CMakeLists.txt b/nelns/naming_service/tests/CMakeLists.txt index b9793e319f..f65464ed52 100644 --- a/nelns/naming_service/tests/CMakeLists.txt +++ b/nelns/naming_service/tests/CMakeLists.txt @@ -5,6 +5,7 @@ add_executable("${TEST_NAME}" ) target_link_libraries("${TEST_NAME}" GTest::gtest_main + GTest::gmock nelmisc nelnet nelns::ns diff --git a/nelns/naming_service/tests/naming_service.test.cpp b/nelns/naming_service/tests/naming_service.test.cpp index 32d91842af..d0c6722368 100644 --- a/nelns/naming_service/tests/naming_service.test.cpp +++ b/nelns/naming_service/tests/naming_service.test.cpp @@ -1,6 +1,13 @@ +#include #include -TEST(CNamingService, BasicAssertions) { - EXPECT_STRNE("hello", "world"); - EXPECT_EQ(7 * 6, 42); +#include + +using ::testing::NotNull; + + +TEST(CNamingService, ShouldInstantiateSingleton) { + CNamingService instance; + + EXPECT_THAT(CNamingService::getInstance(), NotNull()); } From 85371e34417063d6ef697efd0de42efca752fe12 Mon Sep 17 00:00:00 2001 From: Tobias Peters Date: Sun, 9 Jun 2024 14:36:08 +0000 Subject: [PATCH 19/28] cleanup singleton in IService on destruction --- nel/src/net/service.cpp | 3 +++ .../tests/naming_service.test.cpp | 25 +++++++++++++++++-- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/nel/src/net/service.cpp b/nel/src/net/service.cpp index 925075f08f..4131da3170 100644 --- a/nel/src/net/service.cpp +++ b/nel/src/net/service.cpp @@ -375,6 +375,9 @@ IService::IService() : IService::~IService() { + // Singleton + _Instance = nullptr; + // unregister the singleton INelContext::getInstance().releaseSingletonPointer("IService", this); } diff --git a/nelns/naming_service/tests/naming_service.test.cpp b/nelns/naming_service/tests/naming_service.test.cpp index d0c6722368..fab85c989a 100644 --- a/nelns/naming_service/tests/naming_service.test.cpp +++ b/nelns/naming_service/tests/naming_service.test.cpp @@ -3,11 +3,32 @@ #include +using ::testing::Eq; +using ::testing::IsFalse; +using ::testing::IsNull; +using ::testing::IsTrue; using ::testing::NotNull; - -TEST(CNamingService, ShouldInstantiateSingleton) { +TEST(CNamingService, ShouldInstantiateSingleton) +{ CNamingService instance; + EXPECT_THAT(CNamingService::isServiceInitialized(), IsTrue()); EXPECT_THAT(CNamingService::getInstance(), NotNull()); + EXPECT_THAT(CNamingService::getInstance(), Eq(&instance)); +} + +TEST(CNamingService, shouldNotAllowMultipleSimultaneousInstances) +{ + CNamingService first; + ASSERT_DEATH({ CNamingService second; }, ""); +} + +TEST(CNamingService, shouldCleanupSingleton) +{ + { + CNamingService instance; + }; + EXPECT_THAT(CNamingService::isServiceInitialized(), IsFalse()); + EXPECT_THAT(CNamingService::getInstance(), IsNull()); } From 279ff117d8e3053ae490862e319abe7d425b1846 Mon Sep 17 00:00:00 2001 From: Tobias Peters Date: Sun, 9 Jun 2024 14:59:16 +0000 Subject: [PATCH 20/28] prepare integration test for naming service --- nelns/naming_service/tests/CMakeLists.txt | 18 +++++++++++++---- .../tests/naming_service.it.cpp | 20 +++++++++++++++++++ .../tests/naming_service.test.cpp | 19 +++++++++++++++++- 3 files changed, 52 insertions(+), 5 deletions(-) create mode 100644 nelns/naming_service/tests/naming_service.it.cpp diff --git a/nelns/naming_service/tests/CMakeLists.txt b/nelns/naming_service/tests/CMakeLists.txt index f65464ed52..58a433cd11 100644 --- a/nelns/naming_service/tests/CMakeLists.txt +++ b/nelns/naming_service/tests/CMakeLists.txt @@ -6,8 +6,20 @@ add_executable("${TEST_NAME}" target_link_libraries("${TEST_NAME}" GTest::gtest_main GTest::gmock - nelmisc - nelnet + nelns::ns +) +gtest_discover_tests("${TEST_NAME}" + EXTRA_ARGS "--gtest_output=xml:${CMAKE_CURRENT_BINARY_DIR}/reports/junit-${TEST_NAME}.xml" +) + +set(TEST_NAME "naming_service_it") + +add_executable("${TEST_NAME}" + naming_service.it.cpp +) +target_link_libraries("${TEST_NAME}" + GTest::gtest_main + GTest::gmock nelns::ns ) gtest_discover_tests("${TEST_NAME}" @@ -22,8 +34,6 @@ add_executable("${TEST_NAME}" target_link_libraries("${TEST_NAME}" GTest::gtest_main GTest::gmock - nelmisc - nelnet nelns::ns ) gtest_discover_tests("${TEST_NAME}" diff --git a/nelns/naming_service/tests/naming_service.it.cpp b/nelns/naming_service/tests/naming_service.it.cpp new file mode 100644 index 0000000000..6803be919d --- /dev/null +++ b/nelns/naming_service/tests/naming_service.it.cpp @@ -0,0 +1,20 @@ +#include +#include + +#include + +using ::testing::Eq; +using ::testing::IsFalse; +using ::testing::IsNull; +using ::testing::IsTrue; +using ::testing::NotNull; + +TEST(CNamingService, shouldStartService) +{ + CNamingService instance; + uint16 port = 0; + + auto exitcode = instance.main("short name", "long name", port, "config-dir", "log-dir", "compilation date"); + + EXPECT_THAT(exitcode, Eq(0)); +} diff --git a/nelns/naming_service/tests/naming_service.test.cpp b/nelns/naming_service/tests/naming_service.test.cpp index fab85c989a..b9e500fd62 100644 --- a/nelns/naming_service/tests/naming_service.test.cpp +++ b/nelns/naming_service/tests/naming_service.test.cpp @@ -9,7 +9,7 @@ using ::testing::IsNull; using ::testing::IsTrue; using ::testing::NotNull; -TEST(CNamingService, ShouldInstantiateSingleton) +TEST(CNamingService, shouldInstantiateSingleton) { CNamingService instance; @@ -21,6 +21,7 @@ TEST(CNamingService, ShouldInstantiateSingleton) TEST(CNamingService, shouldNotAllowMultipleSimultaneousInstances) { CNamingService first; + ASSERT_DEATH({ CNamingService second; }, ""); } @@ -29,6 +30,22 @@ TEST(CNamingService, shouldCleanupSingleton) { CNamingService instance; }; + EXPECT_THAT(CNamingService::isServiceInitialized(), IsFalse()); EXPECT_THAT(CNamingService::getInstance(), IsNull()); } + +TEST(CNamingService, shouldAllowNewInstanceAfterDestruction) +{ + { + CNamingService first; + }; + + { + CNamingService second; + + EXPECT_THAT(CNamingService::isServiceInitialized(), IsTrue()); + EXPECT_THAT(CNamingService::getInstance(), NotNull()); + EXPECT_THAT(CNamingService::getInstance(), Eq(&second)); + }; +} From 73c850f7ee09a4a8f25e54006ff1abb7da4e6f6e Mon Sep 17 00:00:00 2001 From: Tobias Peters Date: Sun, 9 Jun 2024 16:44:45 +0000 Subject: [PATCH 21/28] remove integration test --- nelns/naming_service/tests/CMakeLists.txt | 13 ------------ .../tests/naming_service.it.cpp | 20 ------------------- 2 files changed, 33 deletions(-) delete mode 100644 nelns/naming_service/tests/naming_service.it.cpp diff --git a/nelns/naming_service/tests/CMakeLists.txt b/nelns/naming_service/tests/CMakeLists.txt index 58a433cd11..72c4c539d9 100644 --- a/nelns/naming_service/tests/CMakeLists.txt +++ b/nelns/naming_service/tests/CMakeLists.txt @@ -12,19 +12,6 @@ gtest_discover_tests("${TEST_NAME}" EXTRA_ARGS "--gtest_output=xml:${CMAKE_CURRENT_BINARY_DIR}/reports/junit-${TEST_NAME}.xml" ) -set(TEST_NAME "naming_service_it") - -add_executable("${TEST_NAME}" - naming_service.it.cpp -) -target_link_libraries("${TEST_NAME}" - GTest::gtest_main - GTest::gmock - nelns::ns -) -gtest_discover_tests("${TEST_NAME}" - EXTRA_ARGS "--gtest_output=xml:${CMAKE_CURRENT_BINARY_DIR}/reports/junit-${TEST_NAME}.xml" -) set(TEST_NAME "service_instance_manager_test") diff --git a/nelns/naming_service/tests/naming_service.it.cpp b/nelns/naming_service/tests/naming_service.it.cpp deleted file mode 100644 index 6803be919d..0000000000 --- a/nelns/naming_service/tests/naming_service.it.cpp +++ /dev/null @@ -1,20 +0,0 @@ -#include -#include - -#include - -using ::testing::Eq; -using ::testing::IsFalse; -using ::testing::IsNull; -using ::testing::IsTrue; -using ::testing::NotNull; - -TEST(CNamingService, shouldStartService) -{ - CNamingService instance; - uint16 port = 0; - - auto exitcode = instance.main("short name", "long name", port, "config-dir", "log-dir", "compilation date"); - - EXPECT_THAT(exitcode, Eq(0)); -} From b6f91f53671cc3de271b46c7e08d41a2e624e4bf Mon Sep 17 00:00:00 2001 From: Tobias Peters Date: Sun, 9 Jun 2024 17:23:26 +0000 Subject: [PATCH 22/28] add basic test for running a naming service --- .../tests/naming_service.test.cpp | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/nelns/naming_service/tests/naming_service.test.cpp b/nelns/naming_service/tests/naming_service.test.cpp index b9e500fd62..92a999bd13 100644 --- a/nelns/naming_service/tests/naming_service.test.cpp +++ b/nelns/naming_service/tests/naming_service.test.cpp @@ -1,6 +1,8 @@ #include #include +#include + #include using ::testing::Eq; @@ -9,6 +11,8 @@ using ::testing::IsNull; using ::testing::IsTrue; using ::testing::NotNull; +using ::NLMISC::CConfigFile; + TEST(CNamingService, shouldInstantiateSingleton) { CNamingService instance; @@ -49,3 +53,18 @@ TEST(CNamingService, shouldAllowNewInstanceAfterDestruction) EXPECT_THAT(CNamingService::getInstance(), Eq(&second)); }; } + +TEST(CNamingService, shouldInitialize) +{ + CNamingService instance; + CConfigFile::CVar var; + var.Type = CConfigFile::CVar::T_STRING; + + instance.ConfigFile.insertVar("UniqueOnShardServices", var); + instance.ConfigFile.insertVar("UniqueByMachineServices", var); + + instance.init(); + instance.update(); + + instance.release(); +} From f45c85fa188d886d4dc99d255c73bcc6f4188128 Mon Sep 17 00:00:00 2001 From: Tobias Peters Date: Sun, 9 Jun 2024 17:46:56 +0000 Subject: [PATCH 23/28] reformat code --- nelns/naming_service/tests/service_instance_manager.test.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/nelns/naming_service/tests/service_instance_manager.test.cpp b/nelns/naming_service/tests/service_instance_manager.test.cpp index b9b448142c..4d074b3285 100644 --- a/nelns/naming_service/tests/service_instance_manager.test.cpp +++ b/nelns/naming_service/tests/service_instance_manager.test.cpp @@ -3,13 +3,16 @@ #include #include + #include + #include #include #include using ::std::string; using ::std::vector; + using ::testing::ElementsAre; using ::testing::Eq; using ::testing::IsFalse; @@ -21,6 +24,7 @@ using ::testing::StrEq; using ::NLMISC::CLightMemDisplayer; using ::NLMISC::CLog; + using ::NLNET::CInetAddress; using ::NLNET::InvalidSockId; using ::NLNET::TServiceId; From 80853517b50841789ea918ae47d3ebeac1c4815f Mon Sep 17 00:00:00 2001 From: Tobias Peters Date: Sun, 9 Jun 2024 19:17:33 +0000 Subject: [PATCH 24/28] remove integration test --- .../tests/naming_service.test.cpp | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/nelns/naming_service/tests/naming_service.test.cpp b/nelns/naming_service/tests/naming_service.test.cpp index 92a999bd13..b9e500fd62 100644 --- a/nelns/naming_service/tests/naming_service.test.cpp +++ b/nelns/naming_service/tests/naming_service.test.cpp @@ -1,8 +1,6 @@ #include #include -#include - #include using ::testing::Eq; @@ -11,8 +9,6 @@ using ::testing::IsNull; using ::testing::IsTrue; using ::testing::NotNull; -using ::NLMISC::CConfigFile; - TEST(CNamingService, shouldInstantiateSingleton) { CNamingService instance; @@ -53,18 +49,3 @@ TEST(CNamingService, shouldAllowNewInstanceAfterDestruction) EXPECT_THAT(CNamingService::getInstance(), Eq(&second)); }; } - -TEST(CNamingService, shouldInitialize) -{ - CNamingService instance; - CConfigFile::CVar var; - var.Type = CConfigFile::CVar::T_STRING; - - instance.ConfigFile.insertVar("UniqueOnShardServices", var); - instance.ConfigFile.insertVar("UniqueByMachineServices", var); - - instance.init(); - instance.update(); - - instance.release(); -} From eeed8e0429611b20bfb926f4aa88999c5016b7be Mon Sep 17 00:00:00 2001 From: Tobias Peters Date: Sun, 9 Jun 2024 19:49:37 +0000 Subject: [PATCH 25/28] arrange code into preious sections --- nelns/naming_service/CMakeLists.txt | 5 - nelns/naming_service/naming_service.cpp | 1 - .../nelns/naming_service/can_access.cpp | 38 ---- .../nelns/naming_service/can_access.h | 13 -- .../nelns/naming_service/do_remove.cpp | 94 --------- .../nelns/naming_service/do_remove.h | 14 -- .../naming_service/do_unregister_service.cpp | 46 ----- .../naming_service/do_unregister_service.h | 12 -- .../naming_service/effectively_remove.cpp | 12 -- .../nelns/naming_service/effectively_remove.h | 10 - .../nelns/naming_service/functions.cpp | 182 +++++++++++++++++- .../nelns/naming_service/functions.h | 24 +++ .../nelns/naming_service/helper.cpp | 45 ----- .../nelns/naming_service/helper.h | 15 -- .../nelns/naming_service/naming_service.cpp | 1 - .../service_instance_manager.cpp | 4 +- 16 files changed, 204 insertions(+), 312 deletions(-) delete mode 100644 nelns/naming_service/nelns/naming_service/can_access.cpp delete mode 100644 nelns/naming_service/nelns/naming_service/can_access.h delete mode 100644 nelns/naming_service/nelns/naming_service/do_remove.cpp delete mode 100644 nelns/naming_service/nelns/naming_service/do_remove.h delete mode 100644 nelns/naming_service/nelns/naming_service/do_unregister_service.cpp delete mode 100644 nelns/naming_service/nelns/naming_service/do_unregister_service.h delete mode 100644 nelns/naming_service/nelns/naming_service/effectively_remove.cpp delete mode 100644 nelns/naming_service/nelns/naming_service/effectively_remove.h delete mode 100644 nelns/naming_service/nelns/naming_service/helper.cpp delete mode 100644 nelns/naming_service/nelns/naming_service/helper.h diff --git a/nelns/naming_service/CMakeLists.txt b/nelns/naming_service/CMakeLists.txt index a648c72435..26cfde7110 100644 --- a/nelns/naming_service/CMakeLists.txt +++ b/nelns/naming_service/CMakeLists.txt @@ -1,12 +1,7 @@ include_directories(${CMAKE_CURRENT_SOURCE_DIR}) add_library(nelns_naming_service - nelns/naming_service/can_access.cpp - nelns/naming_service/do_remove.cpp - nelns/naming_service/do_unregister_service.cpp - nelns/naming_service/effectively_remove.cpp nelns/naming_service/functions.cpp - nelns/naming_service/helper.cpp nelns/naming_service/naming_service.cpp nelns/naming_service/service_instance_manager.cpp nelns/naming_service/variables.cpp diff --git a/nelns/naming_service/naming_service.cpp b/nelns/naming_service/naming_service.cpp index 57e550c4fa..4787c958c5 100644 --- a/nelns/naming_service/naming_service.cpp +++ b/nelns/naming_service/naming_service.cpp @@ -46,7 +46,6 @@ #include -#include #include #include #include diff --git a/nelns/naming_service/nelns/naming_service/can_access.cpp b/nelns/naming_service/nelns/naming_service/can_access.cpp deleted file mode 100644 index 0dc610dc49..0000000000 --- a/nelns/naming_service/nelns/naming_service/can_access.cpp +++ /dev/null @@ -1,38 +0,0 @@ -#include - -#include - -using std::vector; - -using NLNET::CInetAddress; - -bool canAccess (const vector &addr, const CServiceEntry &entry, vector &accessibleAddr) -{ - accessibleAddr.clear (); - - if (entry.WaitingUnregistration) - return false; - - for (uint i = 0; i < addr.size(); i++) - { - uint32 net = addr[i].internalNetAddress(); - for (uint j = 0; j < entry.Addr.size(); j++) - { - if (net == entry.Addr[j].internalNetAddress()) - { - accessibleAddr.push_back (entry.Addr[j]); - } - } - } - - if (accessibleAddr.empty()) - { - nldebug ("service %s-%hu is not accessible by '%s'", entry.Name.c_str(), entry.SId.get(), vectorCInetAddressToString (addr).c_str ()); - } - else - { - nldebug ("service %s-%hu is accessible by '%s'", entry.Name.c_str(), entry.SId.get(), vectorCInetAddressToString (accessibleAddr).c_str ()); - } - - return !accessibleAddr.empty (); -} diff --git a/nelns/naming_service/nelns/naming_service/can_access.h b/nelns/naming_service/nelns/naming_service/can_access.h deleted file mode 100644 index 2cadd81358..0000000000 --- a/nelns/naming_service/nelns/naming_service/can_access.h +++ /dev/null @@ -1,13 +0,0 @@ -#ifndef NELNS_NAMING_SERVICE_CAN_ACCESS_H -#define NELNS_NAMING_SERVICE_CAN_ACCESS_H - -#include - -#include -#include - -#include - -bool canAccess(const std::vector &addr, const CServiceEntry &entry, std::vector &accessibleAddr); - -#endif // NELNS_NAMING_SERVICE_CAN_ACCESS_H diff --git a/nelns/naming_service/nelns/naming_service/do_remove.cpp b/nelns/naming_service/nelns/naming_service/do_remove.cpp deleted file mode 100644 index bd9e3d82fa..0000000000 --- a/nelns/naming_service/nelns/naming_service/do_remove.cpp +++ /dev/null @@ -1,94 +0,0 @@ -#include - -#include - -#include -#include -#include -#include - -using std::list; -using std::string; -using std::vector; - -using NLMISC::CTime; -using NLMISC::toString; -using NLNET::CInetAddress; -using NLNET::CMessage; -using NLNET::TServiceId; - -/* - * Helper procedure for cbLookupAlternate and cbUnregister. - * Note: name is used for a LOGS. - */ -list::iterator doRemove(list::iterator it) -{ - nldebug("Unregister the service %s-%hu '%s'", (*it).Name.c_str(), it->SId.get(), (*it).Addr[0].asString().c_str()); - - // tell to everybody that this service is unregistered - - CMessage msgout("UNB"); - msgout.serial((*it).Name); - msgout.serial((*it).SId); - - vector accessibleAddress; - nlinfo("Broadcast the Unregistration of %s-%hu to all registered services", (*it).Name.c_str(), it->SId.get()); - for (list::iterator it3 = RegisteredServices.begin(); it3 != RegisteredServices.end(); it3++) - { - if (canAccess((*it).Addr, (*it3), accessibleAddress)) - { - CallbackServer->send(msgout, (*it3).SockId); - // CNetManager::send ("NS", msgout, (*it3).SockId); - nldebug("Broadcast to %s-%hu", (*it3).Name.c_str(), it3->SId.get()); - } - } - - // new system, after the unregistation broadcast, we wait ACK from all services before really remove - // the service, before, we tag the service as 'wait before unregister' - // if everybody didn't answer before the time out, we remove it - - (*it).SockId = NULL; - - (*it).WaitingUnregistration = true; - (*it).WaitingUnregistrationTime = CTime::getLocalTime(); - - // we remove all services awaiting his ACK because this service is down so it'll never ACK - for (list::iterator itr = RegisteredServices.begin(); itr != RegisteredServices.end(); itr++) - { - for (list::iterator itw = (*itr).WaitingUnregistrationServices.begin(); itw != (*itr).WaitingUnregistrationServices.end();) - { - if ((*itw) == (*it).SId) - { - itw = (*itr).WaitingUnregistrationServices.erase(itw); - } - else - { - itw++; - } - } - } - - string res; - for (list::iterator it2 = RegisteredServices.begin(); it2 != RegisteredServices.end(); it2++) - { - if (!(*it2).WaitingUnregistration) - { - (*it).WaitingUnregistrationServices.push_back((*it2).SId); - res += toString((*it2).SId.get()) + " "; - } - } - - nlinfo("Before removing the service %s-%hu, we wait the ACK of '%s'", (*it).Name.c_str(), (*it).SId.get(), res.c_str()); - - if ((*it).WaitingUnregistrationServices.empty()) - { - return effectivelyRemove(it); - } - else - { - return ++it; - } - - // Release from the service instance manager - CServiceInstanceManager::getInstance()->releaseService((*it).SId); -} diff --git a/nelns/naming_service/nelns/naming_service/do_remove.h b/nelns/naming_service/nelns/naming_service/do_remove.h deleted file mode 100644 index 6f0a79bff2..0000000000 --- a/nelns/naming_service/nelns/naming_service/do_remove.h +++ /dev/null @@ -1,14 +0,0 @@ -#ifndef NELNS_NAMING_SERVICE_DO_REMOVE_H -#define NELNS_NAMING_SERVICE_DO_REMOVE_H - -#include - -#include - -/* - * Helper procedure for cbLookupAlternate and cbUnregister. - * Note: name is used for a LOGS. - */ -std::list::iterator doRemove(std::list::iterator it); - -#endif // NELNS_NAMING_SERVICE_DO_REMOVE_H diff --git a/nelns/naming_service/nelns/naming_service/do_unregister_service.cpp b/nelns/naming_service/nelns/naming_service/do_unregister_service.cpp deleted file mode 100644 index ccde9a9ced..0000000000 --- a/nelns/naming_service/nelns/naming_service/do_unregister_service.cpp +++ /dev/null @@ -1,46 +0,0 @@ -#include - -#include - -#include -#include -#include - -using std::list; - -using NLNET::CInetAddress; -using NLNET::TServiceId; -using NLNET::TSockId; - -void doUnregisterService(const TServiceId &sid) -{ - list::iterator it; - for (it = RegisteredServices.begin(); it != RegisteredServices.end(); it++) - { - if ((*it).SId == sid) - { - // found it, remove it - doRemove(it); - return; - } - } - nlwarning("Service %hu not found", sid.get()); -} - -void doUnregisterService(const NLNET::TSockId &from) -{ - list::iterator it; - for (it = RegisteredServices.begin(); it != RegisteredServices.end();) - { - if ((*it).SockId == from) - { - // it's possible that one "from" have more than one registred service, so we have to find in all the list - // found it, remove it - it = doRemove(it); - } - else - { - it++; - } - } -} diff --git a/nelns/naming_service/nelns/naming_service/do_unregister_service.h b/nelns/naming_service/nelns/naming_service/do_unregister_service.h deleted file mode 100644 index a0112a4b93..0000000000 --- a/nelns/naming_service/nelns/naming_service/do_unregister_service.h +++ /dev/null @@ -1,12 +0,0 @@ -#ifndef NELNS_NAMING_SERVICE_DO_UNREGISTER_SERVICE_H -#define NELNS_NAMING_SERVICE_DO_UNREGISTER_SERVICE_H - -#include -#include - -// Asks a service to stop and tell every one -void doUnregisterService(const NLNET::TServiceId &sid); - -void doUnregisterService(const NLNET::TSockId &from); - -#endif // NELNS_NAMING_SERVICE_DO_UNREGISTER_SERVICE_H diff --git a/nelns/naming_service/nelns/naming_service/effectively_remove.cpp b/nelns/naming_service/nelns/naming_service/effectively_remove.cpp deleted file mode 100644 index 4b7f754565..0000000000 --- a/nelns/naming_service/nelns/naming_service/effectively_remove.cpp +++ /dev/null @@ -1,12 +0,0 @@ -#include - -#include - -using std::list; - -list::iterator effectivelyRemove(list::iterator &it) -{ - // remove the service from the registered service list - nlinfo ("Effectively remove the service %s-%hu", (*it).Name.c_str(), it->SId.get()); - return RegisteredServices.erase (it); -} diff --git a/nelns/naming_service/nelns/naming_service/effectively_remove.h b/nelns/naming_service/nelns/naming_service/effectively_remove.h deleted file mode 100644 index af67fd348a..0000000000 --- a/nelns/naming_service/nelns/naming_service/effectively_remove.h +++ /dev/null @@ -1,10 +0,0 @@ -#ifndef NELNS_NAMING_SERVICE_EFFECTIVELY_REMOVE_H -#define NELNS_NAMING_SERVICE_EFFECTIVELY_REMOVE_H - -#include - -#include - -std::list::iterator effectivelyRemove (std::list::iterator &it); - -#endif // NELNS_NAMING_SERVICE_EFFECTIVELY_REMOVE_H diff --git a/nelns/naming_service/nelns/naming_service/functions.cpp b/nelns/naming_service/nelns/naming_service/functions.cpp index 671f7083bb..32d533070a 100644 --- a/nelns/naming_service/nelns/naming_service/functions.cpp +++ b/nelns/naming_service/nelns/naming_service/functions.cpp @@ -5,9 +5,6 @@ #include #include -#include -#include -#include #include #include @@ -29,6 +26,37 @@ using NLNET::TSockId; // Functions // +bool canAccess(const vector &addr, const CServiceEntry &entry, vector &accessibleAddr) +{ + accessibleAddr.clear(); + + if (entry.WaitingUnregistration) + return false; + + for (uint i = 0; i < addr.size(); i++) + { + uint32 net = addr[i].internalNetAddress(); + for (uint j = 0; j < entry.Addr.size(); j++) + { + if (net == entry.Addr[j].internalNetAddress()) + { + accessibleAddr.push_back(entry.Addr[j]); + } + } + } + + if (accessibleAddr.empty()) + { + nldebug("service %s-%hu is not accessible by '%s'", entry.Name.c_str(), entry.SId.get(), vectorCInetAddressToString(addr).c_str()); + } + else + { + nldebug("service %s-%hu is accessible by '%s'", entry.Name.c_str(), entry.SId.get(), vectorCInetAddressToString(accessibleAddr).c_str()); + } + + return !accessibleAddr.empty(); +} + void displayRegisteredServices(CLog *log) { log->displayNL("Display the %d registered services :", RegisteredServices.size()); @@ -51,6 +79,122 @@ void displayRegisteredServices(CLog *log) log->displayNL("End of the list"); } +list::iterator effectivelyRemove(list::iterator &it) +{ + // remove the service from the registered service list + nlinfo("Effectively remove the service %s-%hu", (*it).Name.c_str(), it->SId.get()); + return RegisteredServices.erase(it); +} + +/* + * Helper procedure for cbLookupAlternate and cbUnregister. + * Note: name is used for a LOGS. + */ +list::iterator doRemove(list::iterator it) +{ + nldebug("Unregister the service %s-%hu '%s'", (*it).Name.c_str(), it->SId.get(), (*it).Addr[0].asString().c_str()); + + // tell to everybody that this service is unregistered + + CMessage msgout("UNB"); + msgout.serial((*it).Name); + msgout.serial((*it).SId); + + vector accessibleAddress; + nlinfo("Broadcast the Unregistration of %s-%hu to all registered services", (*it).Name.c_str(), it->SId.get()); + for (list::iterator it3 = RegisteredServices.begin(); it3 != RegisteredServices.end(); it3++) + { + if (canAccess((*it).Addr, (*it3), accessibleAddress)) + { + CallbackServer->send(msgout, (*it3).SockId); + // CNetManager::send ("NS", msgout, (*it3).SockId); + nldebug("Broadcast to %s-%hu", (*it3).Name.c_str(), it3->SId.get()); + } + } + + // new system, after the unregistation broadcast, we wait ACK from all services before really remove + // the service, before, we tag the service as 'wait before unregister' + // if everybody didn't answer before the time out, we remove it + + (*it).SockId = NULL; + + (*it).WaitingUnregistration = true; + (*it).WaitingUnregistrationTime = CTime::getLocalTime(); + + // we remove all services awaiting his ACK because this service is down so it'll never ACK + for (list::iterator itr = RegisteredServices.begin(); itr != RegisteredServices.end(); itr++) + { + for (list::iterator itw = (*itr).WaitingUnregistrationServices.begin(); itw != (*itr).WaitingUnregistrationServices.end();) + { + if ((*itw) == (*it).SId) + { + itw = (*itr).WaitingUnregistrationServices.erase(itw); + } + else + { + itw++; + } + } + } + + string res; + for (list::iterator it2 = RegisteredServices.begin(); it2 != RegisteredServices.end(); it2++) + { + if (!(*it2).WaitingUnregistration) + { + (*it).WaitingUnregistrationServices.push_back((*it2).SId); + res += toString((*it2).SId.get()) + " "; + } + } + + nlinfo("Before removing the service %s-%hu, we wait the ACK of '%s'", (*it).Name.c_str(), (*it).SId.get(), res.c_str()); + + if ((*it).WaitingUnregistrationServices.empty()) + { + return effectivelyRemove(it); + } + else + { + return ++it; + } + + // Release from the service instance manager + CServiceInstanceManager::getInstance()->releaseService((*it).SId); +} + +void doUnregisterService(const TServiceId &sid) +{ + list::iterator it; + for (it = RegisteredServices.begin(); it != RegisteredServices.end(); it++) + { + if ((*it).SId == sid) + { + // found it, remove it + doRemove(it); + return; + } + } + nlwarning("Service %hu not found", sid.get()); +} + +void doUnregisterService(const NLNET::TSockId &from) +{ + list::iterator it; + for (it = RegisteredServices.begin(); it != RegisteredServices.end();) + { + if ((*it).SockId == from) + { + // it's possible that one "from" have more than one registred service, so we have to find in all the list + // found it, remove it + it = doRemove(it); + } + else + { + it++; + } + } +} + /* * Helper function for cbRegister. * If alloc_sid is true, sid is ignored @@ -394,3 +538,35 @@ void cbConnect /*(const string &serviceName, TSockId from, void *arg)*/ (TSockId // set the appid with a bad id (-1) from->setAppId(~0); } + +/* + * Helper that emulate layer5's getServiceName() + */ +string getServiceName(const TServiceId &sid) +{ + list::iterator it; + for (it = RegisteredServices.begin(); it != RegisteredServices.end(); it++) + { + if ((*it).SId == sid) + { + return (*it).Name; + } + } + return ""; // not found +} + +/* + * Helper that returns the first address of a service + */ +CInetAddress getHostAddress(const TServiceId &sid) +{ + list::iterator it; + for (it = RegisteredServices.begin(); it != RegisteredServices.end(); it++) + { + if ((*it).SId == sid) + { + return (*it).Addr[0]; + } + } + return {}; +} diff --git a/nelns/naming_service/nelns/naming_service/functions.h b/nelns/naming_service/nelns/naming_service/functions.h index bdced47771..9959349a9e 100644 --- a/nelns/naming_service/nelns/naming_service/functions.h +++ b/nelns/naming_service/nelns/naming_service/functions.h @@ -1,6 +1,7 @@ #ifndef NELNS_NAMING_SERVICE_FUNCTIONS_H #define NELNS_NAMING_SERVICE_FUNCTIONS_H +#include #include #include @@ -10,8 +11,25 @@ #include #include +#include + +bool canAccess(const std::vector &addr, const CServiceEntry &entry, std::vector &accessibleAddr); + void displayRegisteredServices(NLMISC::CLog *log = NLMISC::InfoLog); +std::list::iterator effectivelyRemove (std::list::iterator &it); + +/* + * Helper procedure for cbLookupAlternate and cbUnregister. + * Note: name is used for a LOGS. + */ +std::list::iterator doRemove(std::list::iterator it); + +// Asks a service to stop and tell every one +void doUnregisterService(const NLNET::TServiceId &sid); + +void doUnregisterService(const NLNET::TSockId &from); + /* * Helper function for cbRegister. * If alloc_sid is true, sid is ignored @@ -85,4 +103,10 @@ void cbDisconnect(NLNET::TSockId from, void *arg); */ void cbConnect(NLNET::TSockId from, void *arg); +// Helper that emulate layer5's getServiceName() +std::string getServiceName(const NLNET::TServiceId& sid); + +// Helper that returns the first address of a service +NLNET::CInetAddress getHostAddress( const NLNET::TServiceId& sid ); + #endif // NELNS_NAMING_SERVICE_FUNCTIONS_H diff --git a/nelns/naming_service/nelns/naming_service/helper.cpp b/nelns/naming_service/nelns/naming_service/helper.cpp deleted file mode 100644 index 984f10941d..0000000000 --- a/nelns/naming_service/nelns/naming_service/helper.cpp +++ /dev/null @@ -1,45 +0,0 @@ -#include - -#include - -#include -#include - -using std::list; -using std::string; - -using NLNET::CInetAddress; -using NLNET::TServiceId; - -/* - * Helper that emulate layer5's getServiceName() - */ -string getServiceName( const TServiceId& sid ) -{ - list::iterator it; - for (it = RegisteredServices.begin(); it != RegisteredServices.end (); it++) - { - if ((*it).SId == sid) - { - return (*it).Name; - } - } - return ""; // not found -} - - -/* - * Helper that returns the first address of a service - */ -CInetAddress getHostAddress( const TServiceId& sid ) -{ - list::iterator it; - for (it = RegisteredServices.begin(); it != RegisteredServices.end (); it++) - { - if ((*it).SId == sid) - { - return (*it).Addr[0]; - } - } - return {}; -} diff --git a/nelns/naming_service/nelns/naming_service/helper.h b/nelns/naming_service/nelns/naming_service/helper.h deleted file mode 100644 index 09b4981279..0000000000 --- a/nelns/naming_service/nelns/naming_service/helper.h +++ /dev/null @@ -1,15 +0,0 @@ -#ifndef NELNS_NAMING_SERVICE_HELPER_H -#define NELNS_NAMING_SERVICE_HELPER_H - -#include - -#include -#include - -// Helper that emulate layer5's getServiceName() -std::string getServiceName(const NLNET::TServiceId& sid); - -// Helper that returns the first address of a service -NLNET::CInetAddress getHostAddress( const NLNET::TServiceId& sid ); - -#endif // NELNS_NAMING_SERVICE_HELPER_H diff --git a/nelns/naming_service/nelns/naming_service/naming_service.cpp b/nelns/naming_service/nelns/naming_service/naming_service.cpp index bed4afa4ad..757d2ad959 100644 --- a/nelns/naming_service/nelns/naming_service/naming_service.cpp +++ b/nelns/naming_service/nelns/naming_service/naming_service.cpp @@ -7,7 +7,6 @@ #include #include -#include #include #include diff --git a/nelns/naming_service/nelns/naming_service/service_instance_manager.cpp b/nelns/naming_service/nelns/naming_service/service_instance_manager.cpp index 07e1acd354..342de2697b 100644 --- a/nelns/naming_service/nelns/naming_service/service_instance_manager.cpp +++ b/nelns/naming_service/nelns/naming_service/service_instance_manager.cpp @@ -2,9 +2,7 @@ #include -#include -#include -#include +#include using std::map; using std::set; From 5b9fe947c4195dd3b08b89a91f11e3be5daf8ac3 Mon Sep 17 00:00:00 2001 From: Tobias Peters Date: Tue, 11 Jun 2024 16:05:39 +0000 Subject: [PATCH 26/28] make library parts of naming service static --- nelns/naming_service/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nelns/naming_service/CMakeLists.txt b/nelns/naming_service/CMakeLists.txt index 26cfde7110..29e81c7487 100644 --- a/nelns/naming_service/CMakeLists.txt +++ b/nelns/naming_service/CMakeLists.txt @@ -1,6 +1,6 @@ include_directories(${CMAKE_CURRENT_SOURCE_DIR}) -add_library(nelns_naming_service +add_library(nelns_naming_service STATIC nelns/naming_service/functions.cpp nelns/naming_service/naming_service.cpp nelns/naming_service/service_instance_manager.cpp From 5de513a0f45439012ad9fffb54bba77be95caf32 Mon Sep 17 00:00:00 2001 From: Tobias Peters Date: Tue, 11 Jun 2024 16:06:55 +0000 Subject: [PATCH 27/28] revert unwanted changes --- ryzom/server/src/ryzom_naming_service/ryzom_naming_service.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ryzom/server/src/ryzom_naming_service/ryzom_naming_service.cpp b/ryzom/server/src/ryzom_naming_service/ryzom_naming_service.cpp index 3905f63524..be8a34e9d1 100644 --- a/ryzom/server/src/ryzom_naming_service/ryzom_naming_service.cpp +++ b/ryzom/server/src/ryzom_naming_service/ryzom_naming_service.cpp @@ -471,7 +471,7 @@ void doUnregisterService (TServiceId sid) nlwarning ("Service %hu not found", sid.get()); } -void doUnregisterService (const NLNET::TSockId &from) +void doUnregisterService (TSockId from) { list::iterator it; for (it = RegisteredServices.begin(); it != RegisteredServices.end ();) From ebd74a1b1f06e984097a383f16542b45920c4784 Mon Sep 17 00:00:00 2001 From: Tobias Peters Date: Tue, 11 Jun 2024 17:55:52 +0000 Subject: [PATCH 28/28] remove some unused variables in tests --- .../tests/service_instance_manager.test.cpp | 45 ++++--------------- 1 file changed, 9 insertions(+), 36 deletions(-) diff --git a/nelns/naming_service/tests/service_instance_manager.test.cpp b/nelns/naming_service/tests/service_instance_manager.test.cpp index 4d074b3285..7d8cebc2ae 100644 --- a/nelns/naming_service/tests/service_instance_manager.test.cpp +++ b/nelns/naming_service/tests/service_instance_manager.test.cpp @@ -102,62 +102,41 @@ TEST(CServiceInstanceManager, addUniqueServiceShouldAddMachineUniqueService) StartsWith("Online registered services:"))); } -TEST(CServiceInstanceManager, queryStartServiceShouldAddOnlineSercie) +TEST(CServiceInstanceManager, queryStartServiceShouldGrantSingleService) { CServiceInstanceManager instance; TServiceId serviceId(123); vector addresses = { "localhost:12345" }; string reason; - CLightMemDisplayer displayer; - CLog log(NLMISC::CLog::LOG_ASSERT); - log.addDisplayer(&displayer); EXPECT_THAT( - instance.queryStartService("online-service", serviceId, addresses, reason), + instance.queryStartService("single-service", serviceId, addresses, reason), IsTrue()); - - instance.displayInfo(&log); - EXPECT_THAT( - displayer.lockStrings(), - IsSupersetOf({ StartsWith("Online registered services:"), - StartsWith(serviceId.toString()) })); } -TEST(CServiceInstanceManager, queryStartServiceShouldAddSingleShardUnqiueService) +TEST(CServiceInstanceManager, queryStartServiceShouldGrantSingleShardUnqiueService) { CServiceInstanceManager instance; TServiceId serviceId(123); vector addresses = { "localhost:12345" }; string reason; - CLightMemDisplayer displayer; - CLog log(NLMISC::CLog::LOG_ASSERT); - log.addDisplayer(&displayer); string serviceName = "unique-shard-service-name"; instance.addUniqueService(serviceName, true); EXPECT_THAT( instance.queryStartService(serviceName, serviceId, addresses, reason), IsTrue()); - - instance.displayInfo(&log); - EXPECT_THAT( - displayer.lockStrings(), - IsSupersetOf({ StartsWith("Online registered services:"), - StartsWith(serviceId.toString()) })); } -TEST(CServiceInstanceManager, queryStartServiceShouldNotAddAdditionalShardUnqiueService) +TEST(CServiceInstanceManager, queryStartServiceShouldNotGrantAdditionalShardUnqiueService) { RegisteredServices.clear(); CServiceInstanceManager instance; TServiceId serviceId(123); vector addresses = { "localhost:12345" }; string reason; - CLightMemDisplayer displayer; - CLog log(NLMISC::CLog::LOG_ASSERT); - log.addDisplayer(&displayer); string serviceName = "unique-shard-service-name"; - RegisteredServices.push_back(CServiceEntry(InvalidSockId, addresses, serviceName, serviceId)); + RegisteredServices.emplace_back(InvalidSockId, addresses, serviceName, serviceId); instance.addUniqueService(serviceName, true); instance.queryStartService(serviceName, serviceId, addresses, reason); @@ -170,7 +149,7 @@ TEST(CServiceInstanceManager, queryStartServiceShouldNotAddAdditionalShardUnqiue StrEq("Service unique-shard-service-name already found as 123, must be unique on shard")); } -TEST(CServiceInstanceManager, queryStartServiceShouldAddAdditionalMachineUnqiueServiceOnDifferentMachine) +TEST(CServiceInstanceManager, queryStartServiceShouldGrantAdditionalMachineUnqiueServiceOnDifferentMachine) { RegisteredServices.clear(); CServiceInstanceManager instance; @@ -178,11 +157,8 @@ TEST(CServiceInstanceManager, queryStartServiceShouldAddAdditionalMachineUnqiueS vector firstAddresses = { "127.0.0.1:12345" }; vector secondAddresses = { "127.0.0.2:12345" }; string reason; - CLightMemDisplayer displayer; - CLog log(NLMISC::CLog::LOG_ASSERT); - log.addDisplayer(&displayer); string serviceName = "unique-machine-service-name"; - RegisteredServices.push_back(CServiceEntry(InvalidSockId, firstAddresses, serviceName, serviceId)); + RegisteredServices.emplace_back(InvalidSockId, firstAddresses, serviceName, serviceId); instance.addUniqueService(serviceName, false); instance.queryStartService(serviceName, serviceId, firstAddresses, reason); @@ -191,18 +167,15 @@ TEST(CServiceInstanceManager, queryStartServiceShouldAddAdditionalMachineUnqiueS IsTrue()); } -TEST(CServiceInstanceManager, queryStartServiceShouldNotAddAdditionalMachineUnqiueService) +TEST(CServiceInstanceManager, queryStartServiceShouldNotGrantAdditionalMachineUnqiueServiceOnSameMachine) { RegisteredServices.clear(); CServiceInstanceManager instance; TServiceId serviceId(123); vector addresses = { "localhost:12345" }; string reason; - CLightMemDisplayer displayer; - CLog log(NLMISC::CLog::LOG_ASSERT); - log.addDisplayer(&displayer); string serviceName = "unique-machine-service-name"; - RegisteredServices.push_back(CServiceEntry(InvalidSockId, addresses, serviceName, serviceId)); + RegisteredServices.emplace_back(InvalidSockId, addresses, serviceName, serviceId); instance.addUniqueService(serviceName, false); instance.queryStartService(serviceName, serviceId, addresses, reason);