diff --git a/ecal/service/ecal_service/include/ecal/service/client_manager.h b/ecal/service/ecal_service/include/ecal/service/client_manager.h index 481003bd57..0bb903c947 100644 --- a/ecal/service/ecal_service/include/ecal/service/client_manager.h +++ b/ecal/service/ecal_service/include/ecal/service/client_manager.h @@ -147,8 +147,7 @@ namespace eCAL * stopped from this central place. * * @param protocol_version The protocol version to use for the client session. If 0, the legacy buggy protocol will be used. - * @param address The address of the server to connect to TODO: Update documentation - * @param port The port of the server to connect to + * @param server_list A list of endpoints to connect to. Must not be empty. The endpoints will be tried in the given order until a working endpoint is found. * @param event_callback The callback, that will be called, when the client has connected to the server or disconnected from it. The callback will be executed in the io_context thread. * * @return A shared_ptr to the newly created ClientSession instance diff --git a/ecal/service/ecal_service/include/ecal/service/client_session.h b/ecal/service/ecal_service/include/ecal/service/client_session.h index b75b83bf45..6b9b7f52de 100644 --- a/ecal/service/ecal_service/include/ecal/service/client_session.h +++ b/ecal/service/ecal_service/include/ecal/service/client_session.h @@ -136,8 +136,7 @@ namespace eCAL * * @param io_context The io_context to use for the session and all callbacks. * @param protocol_version The protocol version to use for the session. When this is 0, the legacy buggy protocol is used. - * @param address The address of the server to connect to. May be an IP or a Hostname, IPv6 is supported. TODO: Update documentation - * @param port The port of the server to connect to. + * @param server_list A list of endpoints to connect to. Must not be empty. The endpoints will be tried in the given order until a working endpoint is found. * @param event_callback The callback to be called when the session's state changes, i.e. when the session successfully connected to a server or disconnected from it. * @param logger The logger to use for logging. * @param delete_callback The callback to be called when the session is deleted. This is useful for the eCAL::service::ClientManager to keep track of the number of active sessions. @@ -225,30 +224,40 @@ namespace eCAL */ eCAL::service::Error call_service(const std::shared_ptr& request, std::shared_ptr& response); - /** TODO: Revise documentation - * @brief Get the address that this client session has been created with. + /** + * @brief Get the host that this client is connected to. + * + * Get the host that this client is connected to. + * If the client is not connected, this function will return an empty + * string. Otherwise, it will return the hostname from the list + * server_list that the client is connected to. * - * This function returns the address that this client session has been - * created with. It will not return the address of the server that this - * client session is connected to, which would actually be the same - * address, but probably resolved to an IP. + * The host is not resolved to an IP address. Use get_remote_endpoint() + * to get the actual IP address. * - * @return The address that this client session has been created with. + * @return The host that this client is connected to. */ std::string get_host() const; - /**TODO: Revise documentation - * @brief Get the port that this client session has been created with. + /** + * @brief Get the port that this client session is connected to. * - * This function returns the port that this client session has been - * created with. It is not said, that the connection has been established - * successfully. + * Get the port that this client session is connected to. If the client + * is not connected, this function will return 0. Otherwise, it will + * return the port from the list server_list that the client is connected + * to. * - * @return The port that this client session has been created with. + * @return The port that this client is connected to */ std::uint16_t get_port() const; - // TODO: Document + /** + * @brief Get the remote endpoint that this client session is connected to. + * + * Get the remote endpoint that this client session is connected to. Only + * valid, if the client session is actually connected to a server. If a + * hostname was given, this function will return the resolved IP address. + */ asio::ip::tcp::endpoint get_remote_endpoint() const; /** diff --git a/ecal/service/ecal_service/src/client_session_impl_base.h b/ecal/service/ecal_service/src/client_session_impl_base.h index 535efe7587..6db29e96c1 100644 --- a/ecal/service/ecal_service/src/client_session_impl_base.h +++ b/ecal/service/ecal_service/src/client_session_impl_base.h @@ -75,7 +75,7 @@ namespace eCAL virtual std::string get_host() const = 0; virtual std::uint16_t get_port() const = 0; - virtual asio::ip::tcp::endpoint get_remote_endpoint() const = 0; // TODO: Document + virtual asio::ip::tcp::endpoint get_remote_endpoint() const = 0; virtual State get_state() const = 0; virtual std::uint8_t get_accepted_protocol_version() const = 0; diff --git a/ecal/service/ecal_service/src/client_session_impl_v0.cpp b/ecal/service/ecal_service/src/client_session_impl_v0.cpp index bf6c96572f..10e98ce998 100644 --- a/ecal/service/ecal_service/src/client_session_impl_v0.cpp +++ b/ecal/service/ecal_service/src/client_session_impl_v0.cpp @@ -34,6 +34,7 @@ #include #include #include +#include #include #include #include @@ -54,7 +55,13 @@ namespace eCAL { std::shared_ptr instance(new ClientSessionV0(io_context, server_list, event_callback, logger)); - instance->resolve_endpoint(0); // TODO: Write a test that checks what happens when the server_list is empty + // Throw exception, if the server list is empty + if (server_list.empty()) + { + throw std::invalid_argument("Server list must not be empty"); + } + + instance->resolve_endpoint(0); return instance; } diff --git a/ecal/service/ecal_service/src/client_session_impl_v0.h b/ecal/service/ecal_service/src/client_session_impl_v0.h index 28fa738158..326ad1b648 100644 --- a/ecal/service/ecal_service/src/client_session_impl_v0.h +++ b/ecal/service/ecal_service/src/client_session_impl_v0.h @@ -101,7 +101,7 @@ namespace eCAL public: std::string get_host() const override; std::uint16_t get_port() const override; - asio::ip::tcp::endpoint get_remote_endpoint() const override; // TODO: Document + asio::ip::tcp::endpoint get_remote_endpoint() const override; State get_state() const override; std::uint8_t get_accepted_protocol_version() const override; diff --git a/ecal/service/ecal_service/src/client_session_impl_v1.cpp b/ecal/service/ecal_service/src/client_session_impl_v1.cpp index 558079bebb..f6b44ae3b9 100644 --- a/ecal/service/ecal_service/src/client_session_impl_v1.cpp +++ b/ecal/service/ecal_service/src/client_session_impl_v1.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -63,6 +64,12 @@ namespace eCAL { std::shared_ptr instance(new ClientSessionV1(io_context, server_list, event_callback, logger)); + // Throw exception, if the server list is empty + if (server_list.empty()) + { + throw std::invalid_argument("Server list must not be empty"); + } + instance->resolve_endpoint(0); return instance; diff --git a/ecal/service/ecal_service/src/client_session_impl_v1.h b/ecal/service/ecal_service/src/client_session_impl_v1.h index d7bf18dd8f..c95d360f1e 100644 --- a/ecal/service/ecal_service/src/client_session_impl_v1.h +++ b/ecal/service/ecal_service/src/client_session_impl_v1.h @@ -105,7 +105,7 @@ namespace eCAL public: std::string get_host() const override; std::uint16_t get_port() const override; - asio::ip::tcp::endpoint get_remote_endpoint() const override; // TODO: Document + asio::ip::tcp::endpoint get_remote_endpoint() const override; State get_state() const override; std::uint8_t get_accepted_protocol_version() const override; diff --git a/ecal/service/test/src/ecal_tcp_service_test.cpp b/ecal/service/test/src/ecal_tcp_service_test.cpp index 88f0e67a3f..cd5abf2fcb 100644 --- a/ecal/service/test/src/ecal_tcp_service_test.cpp +++ b/ecal/service/test/src/ecal_tcp_service_test.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include // Should not be needed, when I use the server manager / client manager #include // Should not be needed, when I use the server manager / client manager @@ -59,8 +60,6 @@ eCAL::service::LoggerT critical_logger(const std::string& node_name) constexpr std::uint8_t min_protocol_version = 0; constexpr std::uint8_t max_protocol_version = 1; -// TODO: Add test for the new multi-host feature, where the second host is tried if the connection to the first one fails - #if 1 TEST(ecal_service, RAII_TcpServiceServer) // NOLINT { @@ -1851,6 +1850,27 @@ TEST(ecal_service, BackupHost) } #endif +#if 1 +// Test that the client create throws an exception when bein created with an empty server list +TEST(ecal_service, EmptyServerList) +{ + // Regular client + for (std::uint8_t protocol_version = min_protocol_version; protocol_version <= max_protocol_version; protocol_version++) + { + const auto io_context = std::make_shared(); + EXPECT_THROW(eCAL::service::ClientSession::create(io_context, protocol_version, {}, [](eCAL::service::ClientEventType, const std::string&) -> void {}), std::invalid_argument); + } + + // Client manager + for (std::uint8_t protocol_version = min_protocol_version; protocol_version <= max_protocol_version; protocol_version++) + { + const auto io_context = std::make_shared(); + auto client_manager = eCAL::service::ClientManager::create(io_context); + EXPECT_THROW(client_manager->create_client(protocol_version, {}, [](eCAL::service::ClientEventType, const std::string&) -> void {}), std::invalid_argument); + } +} +#endif + #if 1 TEST(ecal_service, ErrorCallback_ErrorCallbackNoServer) // NOLINT {