Add tests for TypeLookup service (#4339)

* Refs #20165: Created basic communication Signed-off-by: adriancampo <[email protected]> rebased Signed-off-by: adriancampo <[email protected]> * Refs #20165: Changes to work with multiple types Signed-off-by: adriancampo <[email protected]> * Refs #20165: Renamed discovery methods. Improved typelookup tests. Signed-off-by: adriancampo <[email protected]> * Refs #20165: Added check_registered_type. Created idl file for each type. Signed-off-by: adriancampo <[email protected]> * Refs #20165: Changed type creation. Signed-off-by: adriancampo <[email protected]> * Refs #20165: Added all types. Signed-off-by: adriancampo <[email protected]> * Refs #20165: Fixes for TypeBig. Signed-off-by: adriancampo <[email protected]> * Refs #20165: Fixes for json files. Signed-off-by: adriancampo <[email protected]> * Refs #20165: Removed code for debug. Signed-off-by: adriancampo <[email protected]> * Refs #20165: Created unittest Signed-off-by: adriancampo <[email protected]> * Refs #20165: Implemented unittests. Added Case0 for no TypeObject Signed-off-by: adriancampo <[email protected]> * Refs #20165: Removed sending and receiving samples Signed-off-by: adriancampo <[email protected]> * Refs #20165: Removed descriptions from json tests case files Signed-off-by: adriancampo <[email protected]> * Refs #20165: Added communication with dynamic types. Signed-off-by: adriancampo <[email protected]> * Refs #20165: Fixed Log macro namespace. And TypeObjectRegistry::register_type_object. Signed-off-by: adriancampo <[email protected]> * Refs #20165: Fix for type with no TypeObject. Signed-off-by: adriancampo <[email protected]> * Refs #20165: Update TypeLookupManager mock. Signed-off-by: adriancampo <[email protected]> * Refs #20165: Added testing for dds-types-tests. Signed-off-by: adriancampo <[email protected]> * Refs #20165: Changed TypeObject consistency checks when typelookup service registers types. Fix for indirect hash typeidentifiers. Signed-off-by: adriancampo <[email protected]> * Refs #20165: Fix for modules in update_header_and_create_cases.py. Ignored relative_paths idl Signed-off-by: adriancampo <[email protected]> * Refs #20165: Reduced number of Cases.json files. Signed-off-by: adriancampo <[email protected]> * Refs #20165: Fix for modules. Signed-off-by: adriancampo <[email protected]> * Refs #20165: Changes to use take method. Fixed bug with get_map_dependencies. Signed-off-by: adriancampo <[email protected]> * Refs #20165: Removed willdcards from cmake. Signed-off-by: adriancampo <[email protected]> * Refs #20165: Updated types. Signed-off-by: adriancampo <[email protected]> * Refs #20165: Fix for custom annotations name hash. Signed-off-by: adriancampo <[email protected]> * Refs #20165. Return typelookupservice unit tests Signed-off-by: Ricardo González Moreno <[email protected]> * Refs #20165. Fix for future rebase Signed-off-by: Ricardo González Moreno <[email protected]> * Refs #20165. Improve tests Signed-off-by: Ricardo González Moreno <[email protected]> * Refs #20165. Fix typelookupservice builin endpoing discovery Signed-off-by: Ricardo González Moreno <[email protected]> * Refs #20165. Removed member_id testing Signed-off-by: Ricardo González Moreno <[email protected]> * Refs #20165. Fix and improve tests Signed-off-by: Ricardo González Moreno <[email protected]> * Apply suggestions from code review * Refs #20165. Fix identation Signed-off-by: Ricardo González Moreno <[email protected]> * Refs #20165. Fix compilation error in mac Signed-off-by: Ricardo González Moreno <[email protected]> * Refs #20165. Fix compilation errors Signed-off-by: Ricardo González Moreno <[email protected]> * Refs #20165. Remove trace Signed-off-by: Ricardo González Moreno <[email protected]> * Refs #20165. Fixes on windows Signed-off-by: Ricardo González <[email protected]> * Refs #20165. Fixes after rebase Signed-off-by: Ricardo González Moreno <[email protected]> * Refs #20165. Fixes on mac Signed-off-by: Ricardo González Moreno <[email protected]> * Refs #20165. Fixes asan Signed-off-by: Ricardo González Moreno <[email protected]> * Refs #20165. Apply suggestions Signed-off-by: Ricardo González Moreno <[email protected]> * Refs #20165. Fix minimal type_id built Signed-off-by: Ricardo González Moreno <[email protected]> * Refs #20165. Apply suggestions Signed-off-by: Ricardo González Moreno <[email protected]> * Refs #20165. Apply suggestions Signed-off-by: Ricardo González Moreno <[email protected]> --------- Signed-off-by: adriancampo <[email protected]> Signed-off-by: Ricardo González Moreno <[email protected]> Signed-off-by: Ricardo González <[email protected]> Co-authored-by: Ricardo González Moreno <[email protected]>
eProsima · Sep 25, 2024 · 309ae6f · 309ae6f
1 parent 0bf7481
commit 309ae6f
Show file tree

Hide file tree

Showing 120 changed files with 90,461 additions and 3,182 deletions.
diff --git a/src/cpp/fastdds/builtin/type_lookup_service/TypeLookupManager.cpp b/src/cpp/fastdds/builtin/type_lookup_service/TypeLookupManager.cpp
@@ -365,7 +365,7 @@ ReturnCode_t TypeLookupManager::check_type_identifier_received(
                     is_type_identifier_known(type_identifier_with_size))
     {
         // The type is already known, invoke the callback
-        callback(temp_proxy_data.get());
+        callback(RETCODE_OK, temp_proxy_data.get());
         return RETCODE_OK;
     }
 
@@ -408,7 +408,8 @@ ReturnCode_t TypeLookupManager::check_type_identifier_received(
 }
 
 void TypeLookupManager::notify_callbacks(
-        xtypes::TypeIdentfierWithSize type_identifier_with_size)
+        ReturnCode_t request_ret_status,
+        const xtypes::TypeIdentfierWithSize& type_identifier_with_size)
 {
     bool removed = false;
     // Check that type is pending to be resolved
@@ -417,7 +418,7 @@ void TypeLookupManager::notify_callbacks(
     {
         for (auto& proxy_callback_pair : writer_callbacks_it->second)
         {
-            proxy_callback_pair.second(proxy_callback_pair.first);
+            proxy_callback_pair.second(request_ret_status, proxy_callback_pair.first);
         }
         removed = true;
     }
@@ -427,7 +428,7 @@ void TypeLookupManager::notify_callbacks(
     {
         for (auto& proxy_callback_pair : reader_callbacks_it->second)
         {
-            proxy_callback_pair.second(proxy_callback_pair.first);
+            proxy_callback_pair.second(request_ret_status, proxy_callback_pair.first);
         }
         removed = true;
     }

diff --git a/src/cpp/fastdds/builtin/type_lookup_service/TypeLookupManager.hpp b/src/cpp/fastdds/builtin/type_lookup_service/TypeLookupManager.hpp
@@ -105,9 +105,9 @@ namespace builtin {
 const SampleIdentity INVALID_SAMPLE_IDENTITY;
 
 using AsyncGetTypeWriterCallback = std::function<
-    void (eprosima::fastdds::rtps::WriterProxyData*)>;
+    void (eprosima::fastdds::dds::ReturnCode_t, eprosima::fastdds::rtps::WriterProxyData*)>;
 using AsyncGetTypeReaderCallback = std::function<
-    void (eprosima::fastdds::rtps::ReaderProxyData*)>;
+    void (eprosima::fastdds::dds::ReturnCode_t, eprosima::fastdds::rtps::ReaderProxyData*)>;
 
 /**
  * Class TypeLookupManager that implements the TypeLookup Service described in the DDS-XTYPES 1.3 specification.
@@ -229,7 +229,8 @@ class TypeLookupManager
      * @param type_identifier_with_size[in] TypeIdentfierWithSize of the callbacks to notify.
      */
     void notify_callbacks(
-            xtypes::TypeIdentfierWithSize type_identifier_with_size);
+            ReturnCode_t request_ret_status,
+            const xtypes::TypeIdentfierWithSize& type_identifier_with_size);
 
     /**
      * Adds a callback to the async_get_type_callbacks_ entry of the TypeIdentfierWithSize, or creates a new one if

diff --git a/src/cpp/fastdds/builtin/type_lookup_service/TypeLookupReplyListener.cpp b/src/cpp/fastdds/builtin/type_lookup_service/TypeLookupReplyListener.cpp
@@ -18,7 +18,6 @@
  */
 
 #include <fastdds/builtin/type_lookup_service/TypeLookupReplyListener.hpp>
-
 #include <fastdds/dds/log/Log.hpp>
 
 #include <fastdds/builtin/type_lookup_service/TypeLookupManager.hpp>
@@ -101,21 +100,43 @@ void TypeLookupReplyListener::process_reply()
         if (!replies_queue_.empty())
         {
             TypeLookup_Reply& reply = replies_queue_.front().reply;
+
+            // Check if the received reply SampleIdentity corresponds to an outstanding request
+            auto& request_id {reply.header().relatedRequestId()};
+            auto request_it = typelookup_manager_->async_get_type_requests_.find(request_id);
+            if (request_it != typelookup_manager_->async_get_type_requests_.end())
             {
+                xtypes::TypeIdentfierWithSize type_id {request_it->second};
                 // Process the TypeLookup_Reply based on its type
                 switch (reply.return_value()._d())
                 {
                     case TypeLookup_getTypes_HashId:
                     {
-                        check_get_types_reply(reply.header().relatedRequestId(),
-                                reply.return_value().getType().result(), reply.header().relatedRequestId());
+                        if (RETCODE_OK == reply.return_value().getType()._d())
+                        {
+                            check_get_types_reply(request_id, type_id,
+                                    reply.return_value().getType().result(), reply.header().relatedRequestId());
+                        }
+                        else
+                        {
+                            typelookup_manager_->notify_callbacks(RETCODE_NO_DATA, type_id);
+                            typelookup_manager_->remove_async_get_type_request(request_id);
+                        }
                         break;
                     }
                     case TypeLookup_getDependencies_HashId:
                     {
-                        check_get_type_dependencies_reply(
-                            reply.header().relatedRequestId(), replies_queue_.front().type_server,
-                            reply.return_value().getTypeDependencies().result());
+                        if (RETCODE_OK == reply.return_value().getTypeDependencies()._d())
+                        {
+                            check_get_type_dependencies_reply(
+                                request_id, type_id, replies_queue_.front().type_server,
+                                reply.return_value().getTypeDependencies().result());
+                        }
+                        else
+                        {
+                            typelookup_manager_->notify_callbacks(RETCODE_NO_DATA, type_id);
+                            typelookup_manager_->remove_async_get_type_request(request_id);
+                        }
                         break;
                     }
                     default:
@@ -133,14 +154,14 @@ void TypeLookupReplyListener::process_reply()
 
 void TypeLookupReplyListener::check_get_types_reply(
         const SampleIdentity& request_id,
+        const xtypes::TypeIdentfierWithSize& type_id,
         const TypeLookup_getTypes_Out& reply,
         SampleIdentity related_request)
 {
-    // Check if the received reply SampleIdentity corresponds to an outstanding request
-    auto requests_it = typelookup_manager_->async_get_type_requests_.find(request_id);
-    if (requests_it != typelookup_manager_->async_get_type_requests_.end())
+    ReturnCode_t register_result = RETCODE_OK;
+
+    if (0 != reply.types().size())
     {
-        ReturnCode_t register_result = RETCODE_OK;
         for (xtypes::TypeIdentifierTypeObjectPair pair : reply.types())
         {
             xtypes::TypeIdentifierPair type_ids;
@@ -150,7 +171,7 @@ void TypeLookupReplyListener::check_get_types_reply(
             {
                 // If any of the types is not registered, log error
                 EPROSIMA_LOG_WARNING(TYPELOOKUP_SERVICE_REPLY_LISTENER,
-                        "Error registering remote type");
+                        "Error registering remote type.");
                 register_result = RETCODE_ERROR;
             }
         }
@@ -159,7 +180,8 @@ void TypeLookupReplyListener::check_get_types_reply(
         {
             // Check if the get_type_dependencies related to this reply required a continuation_point
             std::unique_lock<std::mutex> guard(replies_with_continuation_mutex_);
-            auto it = std::find(replies_with_continuation_.begin(), replies_with_continuation_.end(), related_request);
+            auto it = std::find(replies_with_continuation_.begin(),
+                            replies_with_continuation_.end(), related_request);
             if (it != replies_with_continuation_.end())
             {
                 // If it did, remove it from the list and continue
@@ -173,17 +195,18 @@ void TypeLookupReplyListener::check_get_types_reply(
                 {
                     xtypes::TypeObject type_object;
                     fastdds::rtps::RTPSDomainImpl::get_instance()->type_object_registry_observer().get_type_object(
-                        requests_it->second.type_id(), type_object);
+                        type_id.type_id(), type_object);
                     xtypes::TypeObjectUtils::type_object_consistency(type_object);
                     xtypes::TypeIdentifierPair type_ids;
-                    if (RETCODE_OK != fastdds::rtps::RTPSDomainImpl::get_instance()->type_object_registry_observer().
+                    if (RETCODE_OK !=
+                            fastdds::rtps::RTPSDomainImpl::get_instance()->type_object_registry_observer().
                                     register_type_object(type_object, type_ids, true))
                     {
                         EPROSIMA_LOG_WARNING(TYPELOOKUP_SERVICE_REPLY_LISTENER,
                                 "Cannot register minimal of remote type");
                     }
 
-                    typelookup_manager_->notify_callbacks(requests_it->second);
+                    typelookup_manager_->notify_callbacks(RETCODE_OK, type_id);
                 }
                 catch (const std::exception& exception)
                 {
@@ -192,25 +215,25 @@ void TypeLookupReplyListener::check_get_types_reply(
                 }
             }
         }
-
-        // Remove the processed SampleIdentity from the outstanding requests
-        typelookup_manager_->remove_async_get_type_request(request_id);
     }
+    else
+    {
+        typelookup_manager_->notify_callbacks(RETCODE_NO_DATA, type_id);
+        EPROSIMA_LOG_WARNING(TYPELOOKUP_SERVICE_REPLY_LISTENER,
+                "Received reply with no types.");
+        register_result = RETCODE_ERROR;
+    }
+
+    // Remove the processed SampleIdentity from the outstanding requests
+    typelookup_manager_->remove_async_get_type_request(request_id);
 }
 
 void TypeLookupReplyListener::check_get_type_dependencies_reply(
         const SampleIdentity& request_id,
+        const xtypes::TypeIdentfierWithSize& type_id,
         const fastdds::rtps::GUID_t type_server,
         const TypeLookup_getTypeDependencies_Out& reply)
 {
-    // Check if the received reply SampleIdentity corresponds to an outstanding request
-    auto requests_it = typelookup_manager_->async_get_type_requests_.find(request_id);
-    if (requests_it == typelookup_manager_->async_get_type_requests_.end())
-    {
-        // The reply is not associated with any outstanding request, ignore it
-        return;
-    }
-
     // Add the dependent types to the list for the get_type request
     xtypes::TypeIdentifierSeq needed_types;
     std::unordered_set<xtypes::TypeIdentifier> unique_types;
@@ -234,18 +257,18 @@ void TypeLookupReplyListener::check_get_type_dependencies_reply(
     // If there is no continuation point, add the parent type
     if (reply.continuation_point().empty())
     {
-        needed_types.push_back(requests_it->second.type_id());
+        needed_types.push_back(type_id.type_id());
     }
     // Make a new request with the continuation point
     else
     {
         SampleIdentity next_request_id = typelookup_manager_->
-                        get_type_dependencies({requests_it->second.type_id()}, type_server,
+                        get_type_dependencies({type_id.type_id()}, type_server,
                         reply.continuation_point());
         if (INVALID_SAMPLE_IDENTITY != next_request_id)
         {
             // Store the sent requests and associated TypeIdentfierWithSize
-            typelookup_manager_->add_async_get_type_request(next_request_id, requests_it->second);
+            typelookup_manager_->add_async_get_type_request(next_request_id, type_id);
         }
         else
         {
@@ -260,7 +283,7 @@ void TypeLookupReplyListener::check_get_type_dependencies_reply(
     if (INVALID_SAMPLE_IDENTITY != get_types_request)
     {
         // Store the type request
-        typelookup_manager_->add_async_get_type_request(get_types_request, requests_it->second);
+        typelookup_manager_->add_async_get_type_request(get_types_request, type_id);
 
         // If this get_types request has a continuation_point, store it in the list
         if (!reply.continuation_point().empty())

diff --git a/src/cpp/fastdds/builtin/type_lookup_service/TypeLookupReplyListener.hpp b/src/cpp/fastdds/builtin/type_lookup_service/TypeLookupReplyListener.hpp
@@ -100,6 +100,7 @@ class TypeLookupReplyListener : public fastdds::rtps::ReaderListener, public fas
      */
     void check_get_types_reply(
             const SampleIdentity& request_id,
+            const xtypes::TypeIdentfierWithSize& type_id,
             const TypeLookup_getTypes_Out& reply,
             SampleIdentity related_request);
 
@@ -114,6 +115,7 @@ class TypeLookupReplyListener : public fastdds::rtps::ReaderListener, public fas
      */
     void check_get_type_dependencies_reply(
             const SampleIdentity& request_id,
+            const xtypes::TypeIdentfierWithSize& type_id,
             const fastdds::rtps::GUID_t type_server,
             const TypeLookup_getTypeDependencies_Out& reply);
 

diff --git a/src/cpp/fastdds/builtin/type_lookup_service/TypeLookupRequestListener.cpp b/src/cpp/fastdds/builtin/type_lookup_service/TypeLookupRequestListener.cpp
@@ -207,6 +207,12 @@ void TypeLookupRequestListener::check_get_types_request(
     xtypes::TypeIdentifier complete_id;
     xtypes::TypeIdentifier minimal_id;
 
+    if (0 == request.type_ids().size())
+    {
+        EPROSIMA_LOG_WARNING(TYPELOOKUP_SERVICE_REQUEST_LISTENER,
+                "Received request with no type identifiers.");
+    }
+
     // Iterate through requested type_ids
     for (const xtypes::TypeIdentifier& type_id : request.type_ids())
     {
@@ -280,19 +286,19 @@ void TypeLookupRequestListener::check_get_type_dependencies_request(
         SampleIdentity request_id,
         const TypeLookup_getTypeDependencies_In& request)
 {
+    std::unordered_set<xtypes::TypeIdentfierWithSize>* type_dependencies_ptr {nullptr};
     std::unordered_set<xtypes::TypeIdentfierWithSize> type_dependencies;
     ReturnCode_t type_dependencies_result = RETCODE_ERROR;
     if (!request.type_ids().empty())
     {
         // Check if the received request has been done before and needed a continuation point
-        std::lock_guard<std::mutex> lock(requests_with_continuation_mutex_);
         if (!request.continuation_point().empty())
         {
             auto requests_it = requests_with_continuation_.find(request.type_ids());
             if (requests_it != requests_with_continuation_.end())
             {
                 // Get the dependencies without checking the registry
-                type_dependencies = requests_it->second;
+                type_dependencies_ptr = &requests_it->second;
                 type_dependencies_result = RETCODE_OK;
             }
             else
@@ -313,17 +319,26 @@ void TypeLookupRequestListener::check_get_type_dependencies_request(
             // If there are too many dependent types, store the type dependencies for future requests
             if (type_dependencies_result == RETCODE_OK && type_dependencies.size() > MAX_DEPENDENCIES_PER_REPLY)
             {
-                requests_with_continuation_.emplace(request.type_ids(), type_dependencies);
+                auto ret = requests_with_continuation_.emplace(request.type_ids(), std::move(type_dependencies));
+                type_dependencies_ptr = &ret.first->second;
+            }
+            else
+            {
+                type_dependencies_ptr = &type_dependencies;
             }
         }
     }
+    else
+    {
+        EPROSIMA_LOG_WARNING(TYPELOOKUP_SERVICE_REQUEST_LISTENER, "Type dependencies request is empty.");
+    }
 
     // Handle the result based on the type_dependencies_result
     if (RETCODE_OK == type_dependencies_result)
     {
         // Prepare and send the reply for successful operation
         TypeLookup_getTypeDependencies_Out out = prepare_get_type_dependencies_response(
-            request.type_ids(), type_dependencies, request.continuation_point());
+            request.type_ids(), *type_dependencies_ptr, request.continuation_point());
         answer_request(request_id, rpc::RemoteExceptionCode_t::REMOTE_EX_OK, out);
     }
     else if (RETCODE_NO_DATA == type_dependencies_result)
@@ -378,7 +393,6 @@ TypeLookup_getTypeDependencies_Out TypeLookupRequestListener::prepare_get_type_d
         if ((start_index + MAX_DEPENDENCIES_PER_REPLY) > type_dependencies.size())
         {
             // If all dependent types have been sent, remove from map
-            std::lock_guard<std::mutex> lock(requests_with_continuation_mutex_);
             auto requests_it = requests_with_continuation_.find(id_seq);
             if (requests_it != requests_with_continuation_.end())
             {

diff --git a/src/cpp/fastdds/builtin/type_lookup_service/TypeLookupRequestListener.hpp b/src/cpp/fastdds/builtin/type_lookup_service/TypeLookupRequestListener.hpp
@@ -176,9 +176,6 @@ class TypeLookupRequestListener : public fastdds::rtps::ReaderListener, public f
     //! A pointer to the typelookup manager.
     TypeLookupManager* typelookup_manager_;
 
-    //! Mutex to protect access to requests_with_continuation_.
-    std::mutex requests_with_continuation_mutex_;
-
     //! Collection of the requests that needed continuation points.
     std::unordered_map<xtypes::TypeIdentifierSeq, std::unordered_set<xtypes::TypeIdentfierWithSize>>
     requests_with_continuation_;