Iox #415 remove InvalidIdString and isValid() from ServiceDescription

CHANGELOG.md

@@ -62,6 +62,7 @@
 - Remove creation pattern from `MemoryMap` and replace it with `MemoryMapBuilder` [\#1036](https://github.com/eclipse-iceoryx/iceoryx/issues/1036)
 - Fix error handling of `TypedUniqueId` and refactor it to `UniquePortId` [\#861](https://github.com/eclipse-iceoryx/iceoryx/issues/861)
 - Updating Codecov API and enforce CMake version 3.16 for building iceoryx [\#774](https://github.com/eclipse-iceoryx/iceoryx/issues/774) and [\#1031](https://github.com/eclipse-iceoryx/iceoryx/issues/1031)
+- Remove `InvalidIdString` and `isValid()` from `ServiceDescription`, replace Wildcard string with `iox::cxx::nullopt` [\#415](https://github.com/eclipse-iceoryx/iceoryx/issues/415)
 
 **API Breaking Changes:**
 
@@ -224,6 +225,10 @@ The `iox::cxx::expected` has dropped the requirement for `INVALID_STATE`. With t
 `ErrorTypeAdapter` which was necessary for non enum types was also removed. The specialization
 of `ErrorTypeAdapter` for custom types must therefore also be removed in the user code.
 
+The `InvalidIdString` was removed from `ServiceDescription` and the Wildcard string was replaced
+with a `iox::cxx::nullopt`. With this, every string is allowed within the `ServiceDescription`.
+The default `ServiceDescription` consists of empty strings.
+
 ## [v1.0.1](https://github.com/eclipse-iceoryx/iceoryx/tree/v1.0.0) (2021-06-15)
 
 [Full Changelog](https://github.com/eclipse-iceoryx/iceoryx/compare/v1.0.0...v1.0.1)

iceoryx_hoofs/include/iceoryx_hoofs/cxx/helplets.hpp

@@ -378,7 +378,6 @@ constexpr T into(const F value) noexcept;
   private:                                                                                                             \
     type m_##name = defaultValue;
 
-
 } // namespace cxx
 } // namespace iox
 

iceoryx_posh/include/iceoryx_posh/capro/service_description.hpp

@@ -1,5 +1,5 @@
 // Copyright (c) 2019, 2021 by Robert Bosch GmbH. All rights reserved.
-// Copyright (c) 2021 by Apex.AI Inc. All rights reserved.
+// Copyright (c) 2021 - 2022 by Apex.AI Inc. All rights reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -17,7 +17,6 @@
 #ifndef IOX_POSH_CAPRO_SERVICE_DESCRIPTION_HPP
 #define IOX_POSH_CAPRO_SERVICE_DESCRIPTION_HPP
 
-#include "iceoryx_hoofs/cxx/helplets.hpp"
 #include "iceoryx_hoofs/cxx/serialization.hpp"
 #include "iceoryx_hoofs/cxx/string.hpp"
 #include "iceoryx_hoofs/cxx/vector.hpp"
@@ -30,7 +29,9 @@ namespace iox
 {
 namespace capro
 {
-static const IdString_t InvalidIdString{""};
+/// @brief Used to search for any string
+constexpr iox::cxx::nullopt_t Wildcard;
+
 static constexpr int32_t MAX_NUMBER_OF_CHARS = 64;
 static constexpr size_t CLASS_HASH_ELEMENT_COUNT{4U};
 
@@ -100,11 +101,10 @@ class ServiceDescription
     ServiceDescription(const IdString_t& service,
                        const IdString_t& instance,
                        const IdString_t& event,
-                       ClassHash m_classHash = {0u, 0u, 0u, 0u},
+                       ClassHash m_classHash = {0U, 0U, 0U, 0U},
                        Interfaces interfaceSource = Interfaces::INTERNAL) noexcept;
 
-    /// @brief compare operator. If wildcards AnyServiceString, AnyInstanceString or AnyEventString are used, the
-    /// corresponding member comparisons are skipped.
+    /// @brief compare operator.
     bool operator==(const ServiceDescription& rhs) const noexcept;
 
     /// @brief negation of compare operator.
@@ -133,16 +133,12 @@ class ServiceDescription
     /// @brief Returns the scope of a ServiceDescription
     Scope getScope() const noexcept;
 
-    /// @brief Returns true for valid ServiceDescription
-    /// false for ServiceDescription that contain InvalidStrings.
-    /// @return bool, true if ServiceDescription is valid, false otherwise
-    bool isValid() const noexcept;
 
     ///@{
-    /// Getters for the integer and string IDs
-    IdString_t getServiceIDString() const noexcept;
-    IdString_t getInstanceIDString() const noexcept;
-    IdString_t getEventIDString() const noexcept;
+    /// Getters for the string IDs
+    const IdString_t& getServiceIDString() const noexcept;
+    const IdString_t& getInstanceIDString() const noexcept;
+    const IdString_t& getEventIDString() const noexcept;
     ///@}
 
     ///@{
@@ -155,11 +151,11 @@ class ServiceDescription
 
   private:
     /// @brief string representation of the service
-    IdString_t m_serviceString{InvalidIdString};
+    IdString_t m_serviceString;
     /// @brief string representation of the instance
-    IdString_t m_instanceString{InvalidIdString};
+    IdString_t m_instanceString;
     /// @brief string representation of the event
-    IdString_t m_eventString{InvalidIdString};
+    IdString_t m_eventString;
 
     /// @brief 128-Bit class hash (32-Bit * 4)
     ClassHash m_classHash{0, 0, 0, 0};

iceoryx_posh/include/iceoryx_posh/internal/roudi/port_manager.hpp

@@ -1,5 +1,5 @@
 // Copyright (c) 2019 by Robert Bosch GmbH. All rights reserved.
-// Copyright (c) 2021 by Apex.AI Inc. All rights reserved.
+// Copyright (c) 2021 - 2022 by Apex.AI Inc. All rights reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -96,7 +96,8 @@ class PortManager
     void deletePortsOfProcess(const RuntimeName_t& runtimeName) noexcept;
 
     const std::atomic<uint64_t>* serviceRegistryChangeCounter() noexcept;
-    runtime::IpcMessage findService(const capro::IdString_t& service, const capro::IdString_t& instance) noexcept;
+    runtime::IpcMessage findService(const cxx::optional<capro::IdString_t>& service,
+                                    const cxx::optional<capro::IdString_t>& instance) noexcept;
 
   protected:
     void makeAllPublisherPortsToStopOffer() noexcept;

iceoryx_posh/include/iceoryx_posh/internal/roudi/process_manager.hpp

@@ -1,5 +1,5 @@
 // Copyright (c) 2019, 2021 by Robert Bosch GmbH. All rights reserved.
-// Copyright (c) 2021 by Apex.AI Inc. All rights reserved.
+// Copyright (c) 2021 - 2022 by Apex.AI Inc. All rights reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -108,8 +108,8 @@ class ProcessManager : public ProcessManagerInterface
     void updateLivelinessOfProcess(const RuntimeName_t& name) noexcept;
 
     void findServiceForProcess(const RuntimeName_t& name,
-                               const capro::IdString_t& service,
-                               const capro::IdString_t& instance) noexcept;
+                               const cxx::optional<capro::IdString_t>& service,
+                               const cxx::optional<capro::IdString_t>& instance) noexcept;
 
     void
     addInterfaceForProcess(const RuntimeName_t& name, capro::Interfaces interface, const NodeName_t& node) noexcept;

iceoryx_posh/include/iceoryx_posh/internal/roudi/service_registry.hpp

@@ -1,5 +1,5 @@
 // Copyright (c) 2019 by Robert Bosch GmbH. All rights reserved.
-// Copyright (c) 2021 by Apex.AI Inc. All rights reserved.
+// Copyright (c) 2021 - 2022 by Apex.AI Inc. All rights reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -29,7 +29,6 @@ namespace iox
 {
 namespace roudi
 {
-static const capro::IdString_t Wildcard{"*"};
 class ServiceRegistry
 {
   public:
@@ -58,13 +57,13 @@ class ServiceRegistry
     /// @param[in] serviceDescription, service to be removed
     void remove(const capro::ServiceDescription& serviceDescription) noexcept;
 
-    /// @brief Removes given service description from registry
+    /// @brief Searches for given service description in registry
     /// @param[in] searchResult, reference to the vector which will be filled with the results
-    /// @param[in] service, string or wildcard to search for
-    /// @param[in] instance, string or wildcard to search for
+    /// @param[in] service, string or wildcard (= iox::cxx::nullopt) to search for
+    /// @param[in] instance, string or wildcard (= iox::cxx::nullopt) to search for
     void find(ServiceDescriptionVector_t& searchResult,
-              const capro::IdString_t& service = Wildcard,
-              const capro::IdString_t& instance = Wildcard) const noexcept;
+              const cxx::optional<capro::IdString_t>& service,
+              const cxx::optional<capro::IdString_t>& instance) const noexcept;
 
     /// @brief Returns all service descriptions as copy
     /// @return ServiceDescriptionVector_t, copy of complete service registry

iceoryx_posh/include/iceoryx_posh/roudi/port_pool.hpp

@@ -46,7 +46,6 @@ enum class PortPoolError : uint8_t
     NODE_DATA_LIST_FULL,
     CONDITION_VARIABLE_LIST_FULL,
     EVENT_VARIABLE_LIST_FULL,
-    SERVICE_DESCRIPTION_INVALID,
 };
 
 class PortPool

iceoryx_posh/include/iceoryx_posh/runtime/posh_runtime.hpp

@@ -50,11 +50,6 @@ enum class FindServiceError
     INSTANCE_CONTAINER_OVERFLOW       /// @todo #415 set container to iox::MAX_NUMBER_OF_SERVICES and remove error
 };
 
-/// @brief Used to search for any string
-struct Wildcard_t
-{
-};
-
 /// @brief The runtime that is needed for each application to communicate with the RouDi daemon
 class PoshRuntime
 {

iceoryx_posh/include/iceoryx_posh/runtime/service_discovery.hpp

@@ -35,14 +35,14 @@ class ServiceDiscovery
     ~ServiceDiscovery() noexcept = default;
 
     /// @brief find all services that match the provided service description
-    /// @param[in] service service string to search for (wildcards allowed)
-    /// @param[in] instance instance string to search for (wildcards allowed)
+    /// @param[in] service service string to search for, a nullopt corresponds to a wildcard
+    /// @param[in] instance instance string to search for, a nullopt corresponds to a wildcard
     /// @return cxx::expected<ServiceContainer, FindServiceError>
     /// ServiceContainer: on success, container that is filled with all matching instances
     /// FindServiceError: if any, encountered during the operation
     cxx::expected<ServiceContainer, FindServiceError>
-    findService(const cxx::variant<Wildcard_t, capro::IdString_t> service,
-                const cxx::variant<Wildcard_t, capro::IdString_t> instance) noexcept;
+    findService(const cxx::optional<capro::IdString_t>& service,
+                const cxx::optional<capro::IdString_t>& instance) noexcept;
 
     /// @brief requests the serviceRegistryChangeCounter from the shared memory
     /// @return pointer to the serviceRegistryChangeCounter

iceoryx_posh/source/capro/service_description.cpp

@@ -1,5 +1,5 @@
 // Copyright (c) 2019, 2021 by Robert Bosch GmbH. All rights reserved.
-// Copyright (c) 2021 by Apex.AI Inc. All rights reserved.
+// Copyright (c) 2021 - 2022 by Apex.AI Inc. All rights reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -29,11 +29,11 @@ ServiceDescription::ClassHash::ClassHash() noexcept
 
 ServiceDescription::ClassHash::ClassHash(const std::initializer_list<uint32_t>& values) noexcept
 {
-    uint64_t index = 0u;
+    uint64_t index = 0U;
     for (auto& v : values)
     {
         data[index++] = v;
-        if (index == 4u)
+        if (index == CLASS_HASH_ELEMENT_COUNT)
         {
             return;
         }
@@ -54,7 +54,7 @@ const uint32_t& ServiceDescription::ClassHash::operator[](
 
 bool ServiceDescription::ClassHash::operator==(const ClassHash& rhs) const noexcept
 {
-    for (size_t i = 0u; i < CLASS_HASH_ELEMENT_COUNT; ++i)
+    for (size_t i = 0U; i < CLASS_HASH_ELEMENT_COUNT; ++i)
     {
         if (data[i] != rhs[i])
         {
@@ -70,7 +70,7 @@ bool ServiceDescription::ClassHash::operator!=(const ClassHash& rhs) const noexc
 }
 
 ServiceDescription::ServiceDescription() noexcept
-    : ServiceDescription(InvalidIdString, InvalidIdString, InvalidIdString)
+    : ServiceDescription("", "", "")
 {
 }
 
@@ -142,10 +142,10 @@ ServiceDescription::operator cxx::Serialization() const noexcept
     return cxx::Serialization::create(m_serviceString,
                                       m_instanceString,
                                       m_eventString,
-                                      m_classHash[0u],
-                                      m_classHash[1u],
-                                      m_classHash[2u],
-                                      m_classHash[3u],
+                                      m_classHash[0U],
+                                      m_classHash[1U],
+                                      m_classHash[2U],
+                                      m_classHash[3U],
                                       scope,
                                       interface);
 }
@@ -164,10 +164,10 @@ ServiceDescription::deserialize(const cxx::Serialization& serialized) noexcept
     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],
+                                                        deserializedObject.m_classHash[0U],
+                                                        deserializedObject.m_classHash[1U],
+                                                        deserializedObject.m_classHash[2U],
+                                                        deserializedObject.m_classHash[3U],
                                                         scope,
                                                         interfaceSource);
     if (!deserializationSuccessful || scope >= static_cast<ScopeUnderlyingType>(Scope::INVALID)
@@ -182,17 +182,17 @@ ServiceDescription::deserialize(const cxx::Serialization& serialized) noexcept
     return cxx::success<ServiceDescription>(deserializedObject);
 }
 
-IdString_t ServiceDescription::getServiceIDString() const noexcept
+const IdString_t& ServiceDescription::getServiceIDString() const noexcept
 {
     return m_serviceString;
 }
 
-IdString_t ServiceDescription::getInstanceIDString() const noexcept
+const IdString_t& ServiceDescription::getInstanceIDString() const noexcept
 {
     return m_instanceString;
 }
 
-IdString_t ServiceDescription::getEventIDString() const noexcept
+const IdString_t& ServiceDescription::getEventIDString() const noexcept
 {
     return m_eventString;
 }
@@ -222,12 +222,6 @@ Interfaces ServiceDescription::getSourceInterface() const noexcept
     return m_interfaceSource;
 }
 
-bool ServiceDescription::isValid() const noexcept
-{
-    return !(m_serviceString == iox::capro::InvalidIdString || m_instanceString == iox::capro::InvalidIdString
-             || m_eventString == iox::capro::InvalidIdString);
-}
-
 bool serviceMatch(const ServiceDescription& first, const ServiceDescription& second) noexcept
 {
     return (first.getServiceIDString() == second.getServiceIDString());

iceoryx_posh/source/popo/ports/interface_port_data.cpp

@@ -22,10 +22,7 @@ namespace iox
 namespace popo
 {
 InterfacePortData::InterfacePortData(const RuntimeName_t& runtimeName, const capro::Interfaces interface) noexcept
-    : BasePortData(capro::ServiceDescription(
-                       capro::InvalidIdString, capro::InvalidIdString, capro::InvalidIdString, {0, 0, 0, 0}, interface),
-                   runtimeName,
-                   "")
+    : BasePortData(capro::ServiceDescription("", "", "", {0, 0, 0, 0}, interface), runtimeName, "")
 {
 }
 } // namespace popo

iceoryx_posh/source/roudi/port_manager.cpp

@@ -149,7 +149,6 @@ void PortManager::handlePublisherPorts() noexcept
 void PortManager::doDiscoveryForPublisherPort(PublisherPortRouDiType& publisherPort) noexcept
 {
     publisherPort.tryGetCaProMessage().and_then([this, &publisherPort](auto caproMessage) {
-        cxx::Expects(caproMessage.m_serviceDescription.isValid() && "invalid service description!");
         m_portIntrospection.reportMessage(caproMessage);
         if (capro::CaproMessageType::OFFER == caproMessage.m_type)
         {
@@ -192,7 +191,6 @@ void PortManager::handleSubscriberPorts() noexcept
 void PortManager::doDiscoveryForSubscriberPort(SubscriberPortType& subscriberPort) noexcept
 {
     subscriberPort.tryGetCaProMessage().and_then([this, &subscriberPort](auto caproMessage) {
-        cxx::Expects(caproMessage.m_serviceDescription.isValid() && "invalid service description!");
         if ((capro::CaproMessageType::SUB == caproMessage.m_type)
             || (capro::CaproMessageType::UNSUB == caproMessage.m_type))
         {
@@ -560,22 +558,14 @@ void PortManager::destroySubscriberPort(SubscriberPortType::MemberType_t* const
     LogDebug() << "Destroyed subscriber port";
 }
 
-runtime::IpcMessage PortManager::findService(const capro::IdString_t& service,
-                                             const capro::IdString_t& instance) noexcept
+runtime::IpcMessage PortManager::findService(const cxx::optional<capro::IdString_t>& service,
+                                             const cxx::optional<capro::IdString_t>& instance) noexcept
 {
-    // send find to all interfaces
-    capro::CaproMessage caproMessage(capro::CaproMessageType::FIND, {service, instance, roudi::Wildcard});
-
-    for (auto interfacePortData : m_portPool->getInterfacePortDataList())
-    {
-        iox::popo::InterfacePort interfacePort(interfacePortData);
-        interfacePort.dispatchCaProMessage(caproMessage);
-    }
-
     runtime::IpcMessage response;
 
     ServiceRegistry::ServiceDescriptionVector_t searchResult;
     m_serviceRegistry.find(searchResult, service, instance);
+
     for (auto& service : searchResult)
     {
         response << static_cast<cxx::Serialization>(service.serviceDescription).toString();
@@ -608,11 +598,6 @@ PortManager::acquirePublisherPortData(const capro::ServiceDescription& service,
         return cxx::error<PortPoolError>(PortPoolError::UNIQUE_PUBLISHER_PORT_ALREADY_EXISTS);
     }
 
-    if (!service.isValid())
-    {
-        return cxx::error<PortPoolError>(PortPoolError::SERVICE_DESCRIPTION_INVALID);
-    }
-
     // we can create a new port
     auto maybePublisherPortData = m_portPool->addPublisherPort(
         service, payloadDataSegmentMemoryManager, runtimeName, publisherOptions, portConfigInfo.memoryInfo);
@@ -638,11 +623,6 @@ PortManager::acquireSubscriberPortData(const capro::ServiceDescription& service,
                                        const RuntimeName_t& runtimeName,
                                        const PortConfigInfo& portConfigInfo) noexcept
 {
-    if (!service.isValid())
-    {
-        return cxx::error<PortPoolError>(PortPoolError::SERVICE_DESCRIPTION_INVALID);
-    }
-
     auto maybeSubscriberPortData =
         m_portPool->addSubscriberPort(service, runtimeName, subscriberOptions, portConfigInfo.memoryInfo);
     if (!maybeSubscriberPortData.has_error())