diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e7cf80983..e8c3414692 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -112,6 +112,25 @@ ServiceDescription("First", "Second", "DontCare") myServiceDescription2; ServiceDescription("Foo", "Bar", "Baz") myServiceDescription3; ``` +Instead of using a constructor a `ServiceDescription` is now deserialized via a +static method with error handling: + +```cpp +// before +iox::cxx::Serialization serializedObj; +iox::capro::ServiceDescription service(serializedObj); + +// after +iox::cxx::Serialization serialisedObj; +capro::ServiceDescription::deserialize(serialisedObj) + .and_then([](auto& value){ + // Do something with the deserialized object + }) + .or_else([](auto& error){ + // Handle the error + }); +``` + The service-related methods have been moved from `PoshRuntime` to a separate class (TBD): ```cpp diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index bd22dabfb4..883cef47fe 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -126,6 +126,9 @@ information about logging and error handling. * Public members of structs and classes do not have the `m_` prefix * Namespaces in `lower_snake_case` : `my_namespace` * Aliases have a `_t` postfix : `using FooString_t = iox::cxx::string<100>;` +* Objects created from a method returning a `cxx::optional` shall be named `maybeFoo` +* Objects created from a method returning a `cxx::expected` shall +contain the name `result` e.g. `getChunkResult` for the method `getChunk()` ### Doxygen diff --git a/iceoryx_posh/include/iceoryx_posh/capro/service_description.hpp b/iceoryx_posh/include/iceoryx_posh/capro/service_description.hpp index 0864d8c6e8..7317d7df22 100644 --- a/iceoryx_posh/include/iceoryx_posh/capro/service_description.hpp +++ b/iceoryx_posh/include/iceoryx_posh/capro/service_description.hpp @@ -90,9 +90,6 @@ class ServiceDescription uint32_t data[CLASS_HASH_ELEMENT_COUNT]; }; - /// @brief construction of the capro service description using serialized strings - ServiceDescription(const cxx::Serialization& serial) noexcept; - /// @brief default C'tor ServiceDescription() noexcept; ServiceDescription(const ServiceDescription&) noexcept = default; @@ -123,12 +120,18 @@ class ServiceDescription /// @brief serialization of the capro description. operator cxx::Serialization() const; + /// @brief de-serialization of a ServiceDescription. + /// @param[in] serialized, Serialization object from which the ServiceDescription shall be created + /// @return cxx::expected that either has a ServiceDescription or cxx::Serialization::Error stored inside + static cxx::expected + deserialize(const cxx::Serialization& serialized) noexcept; + // @brief Returns if this service description is used for an RouDi-internal channel bool isInternal() const noexcept; // @brief Set this service description to be is used for an RouDi-internal channel void setInternal() noexcept; /// @brief Returns the scope of a ServiceDescription - Scope getScope() noexcept; + Scope getScope() const noexcept; /// @brief Returns true for valid ServiceDescription /// false for ServiceDescription that contain InvalidStrings. diff --git a/iceoryx_posh/include/iceoryx_posh/internal/roudi/service_registry.hpp b/iceoryx_posh/include/iceoryx_posh/internal/roudi/service_registry.hpp index fee392d1bb..b089ad4cda 100644 --- a/iceoryx_posh/include/iceoryx_posh/internal/roudi/service_registry.hpp +++ b/iceoryx_posh/include/iceoryx_posh/internal/roudi/service_registry.hpp @@ -55,10 +55,9 @@ class ServiceRegistry /// @return ServiceRegistryError, error wrapped in cxx::expected cxx::expected add(const capro::ServiceDescription& serviceDescription) noexcept; - /// @brief Removes given service description from registry + /// @brief Removes given service description from registry if service is found /// @param[in] serviceDescription, service to be removed - /// @return true, if service description was removed, false otherwise - bool remove(const capro::ServiceDescription& serviceDescription) noexcept; + void remove(const capro::ServiceDescription& serviceDescription) noexcept; /// @brief Removes given service description from registry /// @param[in] searchResult, reference to the vector which will be filled with the results diff --git a/iceoryx_posh/include/iceoryx_posh/runtime/posh_runtime.hpp b/iceoryx_posh/include/iceoryx_posh/runtime/posh_runtime.hpp index 91e50d3088..cdbed5048b 100644 --- a/iceoryx_posh/include/iceoryx_posh/runtime/posh_runtime.hpp +++ b/iceoryx_posh/include/iceoryx_posh/runtime/posh_runtime.hpp @@ -47,6 +47,7 @@ class NodeData; enum class FindServiceError { INVALID_STATE, + DESERIALIZATION_FAILED, UNABLE_TO_WRITE_TO_ROUDI_CHANNEL, /// @todo #415 remove as IPC channel won't be used INSTANCE_CONTAINER_OVERFLOW /// @todo #415 set container to iox::MAX_NUMBER_OF_SERVICES and remove error }; diff --git a/iceoryx_posh/source/capro/service_description.cpp b/iceoryx_posh/source/capro/service_description.cpp index 8321c31eb5..9c59ed8c5b 100644 --- a/iceoryx_posh/source/capro/service_description.cpp +++ b/iceoryx_posh/source/capro/service_description.cpp @@ -69,37 +69,6 @@ bool ServiceDescription::ClassHash::operator!=(const ClassHash& rhs) const noexc return !operator==(rhs); } -ServiceDescription::ServiceDescription(const cxx::Serialization& serial) noexcept -{ - std::underlying_type::type scope = 0; - std::underlying_type::type interfaceSource = 0; - serial.extract(m_serviceString, - m_instanceString, - m_eventString, - m_classHash[0u], - m_classHash[1u], - m_classHash[2u], - m_classHash[3u], - scope, - interfaceSource); - if (scope > static_cast::type>(Scope::INVALID)) - { - m_scope = Scope::INVALID; - } - else - { - m_scope = static_cast(scope); - } - if (interfaceSource > static_cast::type>(Interfaces::INTERFACE_END)) - { - m_interfaceSource = Interfaces::INTERFACE_END; - } - else - { - m_interfaceSource = static_cast(interfaceSource); - } -} - ServiceDescription::ServiceDescription() noexcept : ServiceDescription(InvalidIdString, InvalidIdString, InvalidIdString) { @@ -181,6 +150,38 @@ ServiceDescription::operator cxx::Serialization() const interface); } +cxx::expected +ServiceDescription::deserialize(const cxx::Serialization& serialized) noexcept +{ + ServiceDescription deserializedObject; + + using ScopeUnderlyingType = std::underlying_type::type; + using InterfaceUnderlyingType = std::underlying_type::type; + + ScopeUnderlyingType scope{0}; + InterfaceUnderlyingType interfaceSource{0}; + + auto deserializationSuccessful = serialized.extract(deserializedObject.m_serviceString, + deserializedObject.m_instanceString, + deserializedObject.m_eventString, + deserializedObject.m_classHash[0u], + deserializedObject.m_classHash[1u], + deserializedObject.m_classHash[2u], + deserializedObject.m_classHash[3u], + scope, + interfaceSource); + if (!deserializationSuccessful || scope >= static_cast(Scope::INVALID) + || interfaceSource >= static_cast(Interfaces::INTERFACE_END)) + { + return cxx::error(cxx::Serialization::Error::DESERIALIZATION_FAILED); + } + + deserializedObject.m_scope = static_cast(scope); + deserializedObject.m_interfaceSource = static_cast(interfaceSource); + + return cxx::success(deserializedObject); +} + IdString_t ServiceDescription::getServiceIDString() const noexcept { return m_serviceString; @@ -206,7 +207,7 @@ void ServiceDescription::setInternal() noexcept m_scope = Scope::INTERNAL; } -Scope ServiceDescription::getScope() noexcept +Scope ServiceDescription::getScope() const noexcept { return m_scope; } diff --git a/iceoryx_posh/source/popo/client_options.cpp b/iceoryx_posh/source/popo/client_options.cpp index cd1a8c203d..d0fd9b5af7 100644 --- a/iceoryx_posh/source/popo/client_options.cpp +++ b/iceoryx_posh/source/popo/client_options.cpp @@ -46,29 +46,15 @@ ClientOptions::deserialize(const cxx::Serialization& serialized) noexcept responseQueueFullPolicy, serverTooSlowPolicy); - if (!deserializationSuccessful) + if (!deserializationSuccessful + || responseQueueFullPolicy > static_cast(QueueFullPolicy2::DISCARD_OLDEST_DATA) + || serverTooSlowPolicy > static_cast(ConsumerTooSlowPolicy::DISCARD_OLDEST_DATA)) { return cxx::error(cxx::Serialization::Error::DESERIALIZATION_FAILED); } - if (responseQueueFullPolicy > static_cast(QueueFullPolicy2::DISCARD_OLDEST_DATA)) - { - return cxx::error(cxx::Serialization::Error::DESERIALIZATION_FAILED); - } - else - { - clientOptions.responseQueueFullPolicy = static_cast(responseQueueFullPolicy); - } - - if (serverTooSlowPolicy > static_cast(ConsumerTooSlowPolicy::DISCARD_OLDEST_DATA)) - { - return cxx::error(cxx::Serialization::Error::DESERIALIZATION_FAILED); - } - else - { - clientOptions.serverTooSlowPolicy = static_cast(serverTooSlowPolicy); - } - + clientOptions.responseQueueFullPolicy = static_cast(responseQueueFullPolicy); + clientOptions.serverTooSlowPolicy = static_cast(serverTooSlowPolicy); return cxx::success(clientOptions); } } // namespace popo diff --git a/iceoryx_posh/source/roudi/roudi.cpp b/iceoryx_posh/source/roudi/roudi.cpp index 5f36d9f320..ec37801d65 100644 --- a/iceoryx_posh/source/roudi/roudi.cpp +++ b/iceoryx_posh/source/roudi/roudi.cpp @@ -231,7 +231,16 @@ void RouDi::processMessage(const runtime::IpcMessage& message, } else { - capro::ServiceDescription service(cxx::Serialization(message.getElementAtIndex(2))); + auto deserializationResult = + capro::ServiceDescription::deserialize(cxx::Serialization(message.getElementAtIndex(2))); + if (deserializationResult.has_error()) + { + LogError() << "Deserialization failed when '" << message.getElementAtIndex(2).c_str() + << "' was provided\n"; + break; + } + const auto& service = deserializationResult.value(); + cxx::Serialization portConfigInfoSerialization(message.getElementAtIndex(7)); if (!service.isValid()) @@ -283,7 +292,16 @@ void RouDi::processMessage(const runtime::IpcMessage& message, } else { - capro::ServiceDescription service(cxx::Serialization(message.getElementAtIndex(2))); + auto deserializationResult = + capro::ServiceDescription::deserialize(cxx::Serialization(message.getElementAtIndex(2))); + if (deserializationResult.has_error()) + { + LogError() << "Deserialization failed when '" << message.getElementAtIndex(2).c_str() + << "' was provided\n"; + break; + } + + const auto& service = deserializationResult.value(); cxx::Serialization portConfigInfoSerialization(message.getElementAtIndex(8)); if (!service.isValid()) diff --git a/iceoryx_posh/source/roudi/service_registry.cpp b/iceoryx_posh/source/roudi/service_registry.cpp index 7ce87d8061..b38a561c13 100644 --- a/iceoryx_posh/source/roudi/service_registry.cpp +++ b/iceoryx_posh/source/roudi/service_registry.cpp @@ -47,7 +47,7 @@ cxx::expected ServiceRegistry::add(const capro::ServiceD return cxx::success<>(); } -bool ServiceRegistry::remove(const capro::ServiceDescription& serviceDescription) noexcept +void ServiceRegistry::remove(const capro::ServiceDescription& serviceDescription) noexcept { bool removedElement{false}; uint64_t index = 0U; @@ -65,7 +65,7 @@ bool ServiceRegistry::remove(const capro::ServiceDescription& serviceDescription } else { - return true; + return; } // There can't be more than one element break; @@ -96,8 +96,6 @@ bool ServiceRegistry::remove(const capro::ServiceDescription& serviceDescription removeIndexFromMap(m_serviceMap, index); removeIndexFromMap(m_instanceMap, index); } - - return removedElement; } void ServiceRegistry::find(ServiceDescriptionVector_t& searchResult, diff --git a/iceoryx_posh/source/runtime/posh_runtime_impl.cpp b/iceoryx_posh/source/runtime/posh_runtime_impl.cpp index 6513707b0d..8378dd608e 100644 --- a/iceoryx_posh/source/runtime/posh_runtime_impl.cpp +++ b/iceoryx_posh/source/runtime/posh_runtime_impl.cpp @@ -419,8 +419,13 @@ PoshRuntimeImpl::findService(const cxx::variant s uint32_t numberOfServices = algorithm::min(capacity, numberOfElements); for (uint32_t i = 0U; i < numberOfServices; ++i) { - capro::ServiceDescription service(cxx::Serialization(requestResponse.getElementAtIndex(i))); - serviceContainer.push_back(service); + auto deserializationResult = + capro::ServiceDescription::deserialize(cxx::Serialization(requestResponse.getElementAtIndex(i))); + if (deserializationResult.has_error()) + { + return cxx::error(FindServiceError::DESERIALIZATION_FAILED); + } + serviceContainer.push_back(deserializationResult.value()); } if (numberOfElements > capacity) diff --git a/iceoryx_posh/test/moduletests/test_capro_service.cpp b/iceoryx_posh/test/moduletests/test_capro_service.cpp index cdc4f63d6c..16577d40a1 100644 --- a/iceoryx_posh/test/moduletests/test_capro_service.cpp +++ b/iceoryx_posh/test/moduletests/test_capro_service.cpp @@ -148,23 +148,29 @@ TEST_F(ServiceDescription_test, ServiceDescriptionSerializationCreatesServiceDes static_cast(testScope), static_cast(testInterfaceSource)); - ServiceDescription serviceDescription1 = ServiceDescription(serialObj); - - EXPECT_THAT(serviceDescription1.getServiceIDString(), Eq(testService)); - EXPECT_THAT(serviceDescription1.getInstanceIDString(), Eq(testInstance)); - EXPECT_THAT(serviceDescription1.getEventIDString(), Eq(testEvent)); - EXPECT_THAT((serviceDescription1.getClassHash())[0], Eq(testHash[0])); - EXPECT_THAT((serviceDescription1.getClassHash())[1], Eq(testHash[1])); - EXPECT_THAT((serviceDescription1.getClassHash())[2], Eq(testHash[2])); - EXPECT_THAT((serviceDescription1.getClassHash())[3], Eq(testHash[3])); - EXPECT_THAT(serviceDescription1.getScope(), Eq(Scope::INTERNAL)); - EXPECT_THAT(serviceDescription1.getSourceInterface(), Eq(Interfaces::INTERNAL)); + + ServiceDescription::deserialize(serialObj) + .and_then([&](const auto& service) { + EXPECT_THAT(service.getServiceIDString(), Eq(testService)); + EXPECT_THAT(service.getInstanceIDString(), Eq(testInstance)); + EXPECT_THAT(service.getEventIDString(), Eq(testEvent)); + EXPECT_THAT((service.getClassHash())[0], Eq(testHash[0])); + EXPECT_THAT((service.getClassHash())[1], Eq(testHash[1])); + EXPECT_THAT((service.getClassHash())[2], Eq(testHash[2])); + EXPECT_THAT((service.getClassHash())[3], Eq(testHash[3])); + EXPECT_THAT(service.getScope(), Eq(Scope::INTERNAL)); + EXPECT_THAT(service.getSourceInterface(), Eq(Interfaces::INTERNAL)); + }) + .or_else([](const auto& error) { + FAIL() << "Deserialization should not fail but failed with: " << static_cast(error); + }); } /// @attention The purpose of the Serialization is not to be an alternative Constructor. It is intended to send/receive /// the ServiceDescription over communication protocols which transfers strings like the MessageQueue. The testcase is /// only intended to check the functionality by injecting the valus directly. -TEST_F(ServiceDescription_test, ServiceDescriptionObjectInitialisationWithOutOfBoundaryScopeSetsTheScopeToInvalid) +TEST_F(ServiceDescription_test, + ServiceDescriptionObjectInitialisationWithOutOfBoundaryScopeLeadsToInvalidDeserialization) { ServiceDescription::ClassHash testHash = {14U, 28U, 42U, 56U}; testService = "Service"; @@ -180,16 +186,17 @@ TEST_F(ServiceDescription_test, ServiceDescriptionObjectInitialisationWithOutOfB testHash[3], invalidScope); - ServiceDescription serviceDescription1 = ServiceDescription(serialObj); + auto deserializationResult = ServiceDescription::deserialize(serialObj); - EXPECT_THAT(serviceDescription1.getScope(), Eq(Scope::INVALID)); + ASSERT_TRUE(deserializationResult.has_error()); + EXPECT_THAT(deserializationResult.get_error(), Eq(iox::cxx::Serialization::Error::DESERIALIZATION_FAILED)); } /// @attention The purpose of the Serialization is not to be an alternative Constructor. It is intended to send/receive /// the ServiceDescription over communication protocols which transfers strings like the MessageQueue. The testcase is /// only intended to check the functionality by injecting the valus directly. TEST_F(ServiceDescription_test, - ServiceDescriptionObjectInitialisationWithOutOfBoundaryInterfaceSourceSetsTheInterfaceSourceToInterfaceEnd) + ServiceDescriptionObjectInitialisationWithOutOfBoundaryInterfaceSourceLeadsToInvalidDeserialization) { ServiceDescription::ClassHash testHash = {17U, 34U, 51U, 68U}; testService = "Service"; @@ -207,12 +214,23 @@ TEST_F(ServiceDescription_test, static_cast(testScope), invalidInterfaceSource); - ServiceDescription serviceDescription1 = ServiceDescription(serialObj); + auto deserializationResult = ServiceDescription::deserialize(serialObj); + + ASSERT_TRUE(deserializationResult.has_error()); + EXPECT_THAT(deserializationResult.get_error(), Eq(iox::cxx::Serialization::Error::DESERIALIZATION_FAILED)); +} + +TEST_F(ServiceDescription_test, ServiceDescriptionObjectInitialisationWithEmptyStringLeadsToInvalidDeserialization) +{ + std::string emptyString; + iox::cxx::Serialization invalidSerialObj{emptyString}; + + auto deserializationResult = ServiceDescription::deserialize(invalidSerialObj); - EXPECT_THAT(serviceDescription1.getSourceInterface(), Eq(Interfaces::INTERFACE_END)); + ASSERT_TRUE(deserializationResult.has_error()); + EXPECT_THAT(deserializationResult.get_error(), Eq(iox::cxx::Serialization::Error::DESERIALIZATION_FAILED)); } -/// @todo remove TEST_F(ServiceDescription_test, ServiceDescriptionDefaultCtorInitializesStringsToInvalidString) { ServiceDescription serviceDescription1 = ServiceDescription(); @@ -433,7 +451,6 @@ TEST_F(ServiceDescription_test, LessThanOperatorReturnsFalseIfEventStringOfFirst EXPECT_FALSE(serviceDescription1 < serviceDescription2); } -/// @todo add new tests for service description? /// END SERVICEDESCRIPTION TESTS diff --git a/iceoryx_posh/test/moduletests/test_roudi_service_registry.cpp b/iceoryx_posh/test/moduletests/test_roudi_service_registry.cpp index ed640d0b54..9f930ccc1c 100644 --- a/iceoryx_posh/test/moduletests/test_roudi_service_registry.cpp +++ b/iceoryx_posh/test/moduletests/test_roudi_service_registry.cpp @@ -42,14 +42,14 @@ class ServiceRegistry_test : public Test std::cout << output << std::endl; } } - iox::roudi::ServiceRegistry registry; + iox::roudi::ServiceRegistry sut; iox::roudi::ServiceRegistry::ServiceDescriptionVector_t searchResults; }; TEST_F(ServiceRegistry_test, AddNoServiceDescriptionsAndWildcardSearchReturnsNothing) { - registry.find(searchResults, Wildcard, Wildcard); + sut.find(searchResults, Wildcard, Wildcard); EXPECT_THAT(searchResults.size(), Eq(0)); } @@ -66,7 +66,7 @@ TEST_F(ServiceRegistry_test, AddMaximumNumberOfServiceDescriptionsWorks) for (auto& service : services) { - auto result = registry.add(service); + auto result = sut.add(service); ASSERT_FALSE(result.has_error()); } } @@ -83,24 +83,24 @@ TEST_F(ServiceRegistry_test, AddMoreThanMaximumNumberOfServiceDescriptionsFails) for (auto& service : services) { - auto result = registry.add(service); + auto result = sut.add(service); ASSERT_FALSE(result.has_error()); } - auto result = registry.add(iox::capro::ServiceDescription("Foo", "Bar", "Baz")); + auto result = sut.add(iox::capro::ServiceDescription("Foo", "Bar", "Baz")); ASSERT_TRUE(result.has_error()); EXPECT_THAT(result.get_error(), Eq(ServiceRegistry::Error::SERVICE_REGISTRY_FULL)); } TEST_F(ServiceRegistry_test, AddServiceDescriptionsWhichWasAlreadyAddedAndReturnsOneResult) { - auto result1 = registry.add(ServiceDescription("Li", "La", "Launebaer")); + auto result1 = sut.add(ServiceDescription("Li", "La", "Launebaer")); ASSERT_FALSE(result1.has_error()); - auto result2 = registry.add(ServiceDescription("Li", "La", "Launebaer")); + auto result2 = sut.add(ServiceDescription("Li", "La", "Launebaer")); ASSERT_FALSE(result2.has_error()); - registry.find(searchResults, Wildcard, Wildcard); + sut.find(searchResults, Wildcard, Wildcard); EXPECT_THAT(searchResults.size(), Eq(1)); EXPECT_THAT(searchResults[0].serviceDescription, Eq(ServiceDescription("Li", "La", "Launebaer"))); @@ -109,15 +109,15 @@ TEST_F(ServiceRegistry_test, AddServiceDescriptionsWhichWasAlreadyAddedAndReturn TEST_F(ServiceRegistry_test, AddServiceDescriptionsTwiceAndRemoveOnceAndReturnsOneResult) { - auto result1 = registry.add(ServiceDescription("Li", "La", "Launebaerli")); + auto result1 = sut.add(ServiceDescription("Li", "La", "Launebaerli")); ASSERT_FALSE(result1.has_error()); - auto result2 = registry.add(ServiceDescription("Li", "La", "Launebaerli")); + auto result2 = sut.add(ServiceDescription("Li", "La", "Launebaerli")); ASSERT_FALSE(result2.has_error()); - registry.remove(ServiceDescription("Li", "La", "Launebaerli")); + sut.remove(ServiceDescription("Li", "La", "Launebaerli")); - registry.find(searchResults, Wildcard, Wildcard); + sut.find(searchResults, Wildcard, Wildcard); EXPECT_THAT(searchResults.size(), Eq(1)); EXPECT_THAT(searchResults[0].serviceDescription, Eq(ServiceDescription("Li", "La", "Launebaerli"))); @@ -126,25 +126,27 @@ TEST_F(ServiceRegistry_test, AddServiceDescriptionsTwiceAndRemoveOnceAndReturnsO TEST_F(ServiceRegistry_test, AddInvalidServiceDescriptionsWorks) { - auto result = registry.add(ServiceDescription()); + auto result = sut.add(ServiceDescription()); ASSERT_FALSE(result.has_error()); } TEST_F(ServiceRegistry_test, RemovingServiceDescriptionsWhichWasntAddedFails) { - EXPECT_FALSE(registry.remove(ServiceDescription("Sim", "Sa", "Lambim"))); + sut.remove(ServiceDescription("Sim", "Sa", "Lambim")); + EXPECT_THAT(sut.getServices().size(), Eq(0)); } TEST_F(ServiceRegistry_test, RemovingInvalidServiceDescriptionsWorks) { - ASSERT_FALSE(registry.add(ServiceDescription()).has_error()); - EXPECT_TRUE(registry.remove(ServiceDescription())); + ASSERT_FALSE(sut.add(ServiceDescription()).has_error()); + sut.remove(ServiceDescription()); + EXPECT_THAT(sut.getServices().size(), Eq(0)); } TEST_F(ServiceRegistry_test, SingleInvalidServiceDescriptionsCanBeFoundWithWildcardSearch) { - ASSERT_FALSE(registry.add(ServiceDescription()).has_error()); - registry.find(searchResults, Wildcard, Wildcard); + ASSERT_FALSE(sut.add(ServiceDescription()).has_error()); + sut.find(searchResults, Wildcard, Wildcard); EXPECT_THAT(searchResults.size(), Eq(1)); EXPECT_THAT(searchResults[0].serviceDescription, Eq(ServiceDescription())); @@ -152,8 +154,8 @@ TEST_F(ServiceRegistry_test, SingleInvalidServiceDescriptionsCanBeFoundWithWildc TEST_F(ServiceRegistry_test, SingleInvalidServiceDescriptionsCanBeFoundWithEmptyString) { - ASSERT_FALSE(registry.add(ServiceDescription()).has_error()); - registry.find(searchResults, "", ""); + ASSERT_FALSE(sut.add(ServiceDescription()).has_error()); + sut.find(searchResults, "", ""); EXPECT_THAT(searchResults.size(), Eq(1)); EXPECT_THAT(searchResults[0].serviceDescription, Eq(ServiceDescription())); @@ -161,9 +163,9 @@ TEST_F(ServiceRegistry_test, SingleInvalidServiceDescriptionsCanBeFoundWithEmpty TEST_F(ServiceRegistry_test, SingleServiceDescriptionCanBeFoundWithWildcardSearch) { - auto result = registry.add(ServiceDescription("Foo", "Bar", "Baz")); + auto result = sut.add(ServiceDescription("Foo", "Bar", "Baz")); ASSERT_FALSE(result.has_error()); - registry.find(searchResults, Wildcard, Wildcard); + sut.find(searchResults, Wildcard, Wildcard); EXPECT_THAT(searchResults.size(), Eq(1)); EXPECT_THAT(searchResults[0].serviceDescription, Eq(ServiceDescription("Foo", "Bar", "Baz"))); @@ -171,9 +173,9 @@ TEST_F(ServiceRegistry_test, SingleServiceDescriptionCanBeFoundWithWildcardSearc TEST_F(ServiceRegistry_test, SingleServiceDescriptionCanBeFoundWithInstanceName) { - auto result = registry.add(ServiceDescription("Baz", "Bar", "Foo")); + auto result = sut.add(ServiceDescription("Baz", "Bar", "Foo")); ASSERT_FALSE(result.has_error()); - registry.find(searchResults, Wildcard, "Bar"); + sut.find(searchResults, Wildcard, "Bar"); EXPECT_THAT(searchResults.size(), Eq(1)); EXPECT_THAT(searchResults[0].serviceDescription, Eq(ServiceDescription("Baz", "Bar", "Foo"))); @@ -182,8 +184,8 @@ TEST_F(ServiceRegistry_test, SingleServiceDescriptionCanBeFoundWithInstanceName) TEST_F(ServiceRegistry_test, SingleServiceDescriptionCanBeFoundWithServiceName) { iox::capro::ServiceDescription service1("a", "b", "c"); - ASSERT_FALSE(registry.add(service1).has_error()); - registry.find(searchResults, "a", Wildcard); + ASSERT_FALSE(sut.add(service1).has_error()); + sut.find(searchResults, "a", Wildcard); EXPECT_THAT(searchResults.size(), Eq(1)); EXPECT_THAT(searchResults[0].serviceDescription, Eq(service1)); @@ -194,9 +196,9 @@ TEST_F(ServiceRegistry_test, ValidAndInvalidServiceDescriptionsCanAllBeFoundWith ServiceDescription service1; ServiceDescription service2("alpha", "bravo", "charlie"); - ASSERT_FALSE(registry.add(service1).has_error()); - ASSERT_FALSE(registry.add(service2).has_error()); - registry.find(searchResults, Wildcard, Wildcard); + ASSERT_FALSE(sut.add(service1).has_error()); + ASSERT_FALSE(sut.add(service2).has_error()); + sut.find(searchResults, Wildcard, Wildcard); EXPECT_THAT(searchResults.size(), Eq(2)); EXPECT_THAT(searchResults[0].serviceDescription, Eq(service1)); @@ -209,10 +211,10 @@ TEST_F(ServiceRegistry_test, MultipleServiceDescriptionWithSameServiceNameCanAll iox::capro::ServiceDescription service2("a", "c", "c"); iox::capro::ServiceDescription service3("a", "d", "d"); - ASSERT_FALSE(registry.add(service1).has_error()); - ASSERT_FALSE(registry.add(service2).has_error()); - ASSERT_FALSE(registry.add(service3).has_error()); - registry.find(searchResults, "a", Wildcard); + ASSERT_FALSE(sut.add(service1).has_error()); + ASSERT_FALSE(sut.add(service2).has_error()); + ASSERT_FALSE(sut.add(service3).has_error()); + sut.find(searchResults, "a", Wildcard); EXPECT_THAT(searchResults.size(), Eq(3)); @@ -238,15 +240,15 @@ TEST_F(ServiceRegistry_test, MultipleServiceDescriptionWithDifferentServiceNameC iox::capro::ServiceDescription service1("a", "b", "b"); iox::capro::ServiceDescription service2("c", "d", "d"); - ASSERT_FALSE(registry.add(service1).has_error()); - ASSERT_FALSE(registry.add(service2).has_error()); - registry.find(searchResults, "a", Wildcard); + ASSERT_FALSE(sut.add(service1).has_error()); + ASSERT_FALSE(sut.add(service2).has_error()); + sut.find(searchResults, "a", Wildcard); EXPECT_THAT(searchResults.size(), Eq(1)); EXPECT_THAT(searchResults[0].serviceDescription, Eq(service1)); searchResults.clear(); - registry.find(searchResults, "c", Wildcard); + sut.find(searchResults, "c", Wildcard); EXPECT_THAT(searchResults.size(), Eq(1)); EXPECT_THAT(searchResults[0].serviceDescription, Eq(service2)); } @@ -257,10 +259,10 @@ TEST_F(ServiceRegistry_test, MultipleServiceDescriptionWithSameServiceNameFindsS iox::capro::ServiceDescription service2("a", "c", "c"); iox::capro::ServiceDescription service3("a", "d", "d"); - ASSERT_FALSE(registry.add(service1).has_error()); - ASSERT_FALSE(registry.add(service2).has_error()); - ASSERT_FALSE(registry.add(service3).has_error()); - registry.find(searchResults, "a", "c"); + ASSERT_FALSE(sut.add(service1).has_error()); + ASSERT_FALSE(sut.add(service2).has_error()); + ASSERT_FALSE(sut.add(service3).has_error()); + sut.find(searchResults, "a", "c"); EXPECT_THAT(searchResults.size(), Eq(1)); EXPECT_THAT(searchResults[0].serviceDescription, Eq(service2)); @@ -274,15 +276,16 @@ TEST_F(ServiceRegistry_test, MultipleServiceDescriptionAddedInNonLinearOrderFind iox::capro::ServiceDescription service4("d", "4", "moep"); iox::capro::ServiceDescription service5("e", "5", "moep"); - ASSERT_FALSE(registry.add(service5).has_error()); - ASSERT_FALSE(registry.add(service3).has_error()); - ASSERT_FALSE(registry.add(service4).has_error()); - ASSERT_FALSE(registry.add(service2).has_error()); - ASSERT_FALSE(registry.add(service1).has_error()); + ASSERT_FALSE(sut.add(service5).has_error()); + ASSERT_FALSE(sut.add(service3).has_error()); + ASSERT_FALSE(sut.add(service4).has_error()); + ASSERT_FALSE(sut.add(service2).has_error()); + ASSERT_FALSE(sut.add(service1).has_error()); - ASSERT_TRUE(registry.remove(service5)); - ASSERT_TRUE(registry.remove(service1)); - registry.find(searchResults, "a", Wildcard); + sut.remove(service5); + sut.remove(service1); + EXPECT_THAT(sut.getServices().size(), Eq(3)); + sut.find(searchResults, "a", Wildcard); EXPECT_THAT(searchResults.size(), Eq(0)); } @@ -293,10 +296,10 @@ TEST_F(ServiceRegistry_test, FindSpecificNonExistingServiceDescriptionFails) iox::capro::ServiceDescription service2("a", "c", "c"); iox::capro::ServiceDescription service3("a", "d", "d"); - ASSERT_FALSE(registry.add(service1).has_error()); - ASSERT_FALSE(registry.add(service2).has_error()); - ASSERT_FALSE(registry.add(service3).has_error()); - registry.find(searchResults, "a", "g"); + ASSERT_FALSE(sut.add(service1).has_error()); + ASSERT_FALSE(sut.add(service2).has_error()); + ASSERT_FALSE(sut.add(service3).has_error()); + sut.find(searchResults, "a", "g"); EXPECT_THAT(searchResults.size(), Eq(0)); } @@ -307,30 +310,31 @@ TEST_F(ServiceRegistry_test, AddingMultipleServiceDescriptionWithSameServicesAnd iox::capro::ServiceDescription service2("a", "c", "c"); iox::capro::ServiceDescription service3("a", "d", "d"); - ASSERT_FALSE(registry.add(service1).has_error()); - ASSERT_FALSE(registry.add(service2).has_error()); - ASSERT_FALSE(registry.add(service3).has_error()); + ASSERT_FALSE(sut.add(service1).has_error()); + ASSERT_FALSE(sut.add(service2).has_error()); + ASSERT_FALSE(sut.add(service3).has_error()); - EXPECT_TRUE(registry.remove(service2)); + sut.remove(service2); + EXPECT_THAT(sut.getServices().size(), Eq(2)); - registry.find(searchResults, "a", "c"); + sut.find(searchResults, "a", "c"); EXPECT_THAT(searchResults.size(), Eq(0)); } -TEST_F(ServiceRegistry_test, - AddingMultipleServiceDescriptionWithDifferentServicesAndRemovingSpecificDoesNotFindSpecific) +TEST_F(ServiceRegistry_test, ServiceNotFoundAfterAddingAndRemovingToServiceRegistry) { iox::capro::ServiceDescription service1("a", "b", "b"); iox::capro::ServiceDescription service2("b", "c", "c"); iox::capro::ServiceDescription service3("c", "d", "d"); - ASSERT_FALSE(registry.add(service1).has_error()); - ASSERT_FALSE(registry.add(service2).has_error()); - ASSERT_FALSE(registry.add(service3).has_error()); + ASSERT_FALSE(sut.add(service1).has_error()); + ASSERT_FALSE(sut.add(service2).has_error()); + ASSERT_FALSE(sut.add(service3).has_error()); - EXPECT_TRUE(registry.remove(service2)); + sut.remove(service2); + EXPECT_THAT(sut.getServices().size(), Eq(2)); - registry.find(searchResults, "b", "c"); + sut.find(searchResults, "b", "c"); EXPECT_THAT(searchResults.size(), Eq(0)); } @@ -340,15 +344,15 @@ TEST_F(ServiceRegistry_test, AddingMultipleServiceDescriptionAndRemovingAllDoesN iox::capro::ServiceDescription service2("a", "c", "c"); iox::capro::ServiceDescription service3("a", "d", "d"); - ASSERT_FALSE(registry.add(service1).has_error()); - ASSERT_FALSE(registry.add(service2).has_error()); - ASSERT_FALSE(registry.add(service3).has_error()); + ASSERT_FALSE(sut.add(service1).has_error()); + ASSERT_FALSE(sut.add(service2).has_error()); + ASSERT_FALSE(sut.add(service3).has_error()); - EXPECT_TRUE(registry.remove(service1)); - EXPECT_TRUE(registry.remove(service2)); - EXPECT_TRUE(registry.remove(service3)); + sut.remove(service1); + sut.remove(service2); + sut.remove(service3); - registry.find(searchResults, "a", Wildcard); + sut.find(searchResults, "a", Wildcard); EXPECT_THAT(searchResults.size(), Eq(0)); } @@ -359,14 +363,14 @@ TEST_F(ServiceRegistry_test, AddingVariousServiceDescriptionAndGetServicesDoesNo iox::capro::ServiceDescription service3("a", "d", "d"); iox::capro::ServiceDescription service4("e", "f", "f"); - ASSERT_FALSE(registry.add(service1).has_error()); - // add same service a, instance c to check if in registry only one entry is created - ASSERT_FALSE(registry.add(service2).has_error()); - ASSERT_FALSE(registry.add(service2).has_error()); - ASSERT_FALSE(registry.add(service3).has_error()); - ASSERT_FALSE(registry.add(service4).has_error()); + ASSERT_FALSE(sut.add(service1).has_error()); + // add same service a, instance c to check if in sut only one entry is created + ASSERT_FALSE(sut.add(service2).has_error()); + ASSERT_FALSE(sut.add(service2).has_error()); + ASSERT_FALSE(sut.add(service3).has_error()); + ASSERT_FALSE(sut.add(service4).has_error()); - auto serviceDescriptionVector = registry.getServices(); + auto serviceDescriptionVector = sut.getServices(); bool service1Found = false; bool service2Found = false;