Skip to content

Commit

Permalink
Add tests for TypeLookup service (#4339)
Browse files Browse the repository at this point in the history
* Refs #20165: Created basic communication

Signed-off-by: adriancampo <adriancampo@eprosima.com>

rebased

Signed-off-by: adriancampo <adriancampo@eprosima.com>

* Refs #20165: Changes to work with multiple types

Signed-off-by: adriancampo <adriancampo@eprosima.com>

* Refs #20165: Renamed discovery methods. Improved typelookup tests.

Signed-off-by: adriancampo <adriancampo@eprosima.com>

* Refs #20165: Added check_registered_type. Created idl file for each type.

Signed-off-by: adriancampo <adriancampo@eprosima.com>

* Refs #20165: Changed type creation.

Signed-off-by: adriancampo <adriancampo@eprosima.com>

* Refs #20165: Added all types.

Signed-off-by: adriancampo <adriancampo@eprosima.com>

* Refs #20165: Fixes for TypeBig.

Signed-off-by: adriancampo <adriancampo@eprosima.com>

* Refs #20165: Fixes for json files.

Signed-off-by: adriancampo <adriancampo@eprosima.com>

* Refs #20165: Removed code for debug.

Signed-off-by: adriancampo <adriancampo@eprosima.com>

* Refs #20165: Created unittest

Signed-off-by: adriancampo <adriancampo@eprosima.com>

* Refs #20165: Implemented unittests. Added Case0 for no TypeObject

Signed-off-by: adriancampo <adriancampo@eprosima.com>

* Refs #20165: Removed sending and receiving samples

Signed-off-by: adriancampo <adriancampo@eprosima.com>

* Refs #20165: Removed descriptions from json tests case files

Signed-off-by: adriancampo <adriancampo@eprosima.com>

* Refs #20165: Added communication with dynamic types.

Signed-off-by: adriancampo <adriancampo@eprosima.com>

* Refs #20165: Fixed Log macro namespace. And TypeObjectRegistry::register_type_object.

Signed-off-by: adriancampo <adriancampo@eprosima.com>

* Refs #20165: Fix for type with no TypeObject.

Signed-off-by: adriancampo <adriancampo@eprosima.com>

* Refs #20165: Update TypeLookupManager mock.

Signed-off-by: adriancampo <adriancampo@eprosima.com>

* Refs #20165: Added testing for dds-types-tests.

Signed-off-by: adriancampo <adriancampo@eprosima.com>

* Refs #20165: Changed TypeObject consistency checks when typelookup service registers types. Fix for indirect hash typeidentifiers.

Signed-off-by: adriancampo <adriancampo@eprosima.com>

* Refs #20165: Fix for modules in update_header_and_create_cases.py. Ignored relative_paths idl

Signed-off-by: adriancampo <adriancampo@eprosima.com>

* Refs #20165: Reduced number of Cases.json files.

Signed-off-by: adriancampo <adriancampo@eprosima.com>

* Refs #20165: Fix for modules.

Signed-off-by: adriancampo <adriancampo@eprosima.com>

* Refs #20165: Changes to use take method. Fixed bug with get_map_dependencies.

Signed-off-by: adriancampo <adriancampo@eprosima.com>

* Refs #20165: Removed willdcards from cmake.

Signed-off-by: adriancampo <adriancampo@eprosima.com>

* Refs #20165: Updated types.

Signed-off-by: adriancampo <adriancampo@eprosima.com>

* Refs #20165: Fix for custom annotations name hash.

Signed-off-by: adriancampo <adriancampo@eprosima.com>

* Refs #20165. Return typelookupservice unit tests

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Refs #20165. Fix for future rebase

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Refs #20165. Improve tests

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Refs #20165. Fix typelookupservice builin endpoing discovery

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Refs #20165. Removed member_id testing

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Refs #20165. Fix and improve tests

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Apply suggestions from code review

* Refs #20165. Fix identation

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Refs #20165. Fix compilation error in mac

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Refs #20165. Fix compilation errors

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Refs #20165. Remove trace

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Refs #20165. Fixes on windows

Signed-off-by: Ricardo González <ricardo@richiware.dev>

* Refs #20165. Fixes after rebase

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Refs #20165. Fixes on mac

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Refs #20165. Fixes asan

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Refs #20165. Apply suggestions

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Refs #20165. Fix minimal type_id built

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Refs #20165. Apply suggestions

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Refs #20165. Apply suggestions

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

---------

Signed-off-by: adriancampo <adriancampo@eprosima.com>
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
Signed-off-by: Ricardo González <ricardo@richiware.dev>
Co-authored-by: Ricardo González Moreno <ricardo@richiware.dev>
  • Loading branch information
adriancampo and richiware authored Sep 25, 2024
1 parent 0bf7481 commit 309ae6f
Show file tree
Hide file tree
Showing 120 changed files with 90,461 additions and 3,182 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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:
Expand All @@ -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;
Expand All @@ -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;
}
}
Expand All @@ -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
Expand All @@ -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)
{
Expand All @@ -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;
Expand All @@ -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
{
Expand All @@ -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())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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())
{
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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())
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
Expand Down
Loading

0 comments on commit 309ae6f

Please sign in to comment.