Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[20165] Add tests for TypeLookup service #4339

Merged
merged 45 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
170bfc6
Refs #20165: Created basic communication
adriancampo Dec 20, 2023
a395be7
Refs #20165: Changes to work with multiple types
adriancampo Jan 11, 2024
053d80f
Refs #20165: Renamed discovery methods. Improved typelookup tests.
adriancampo Jan 14, 2024
730a90d
Refs #20165: Added check_registered_type. Created idl file for each t…
adriancampo Jan 16, 2024
aafbb83
Refs #20165: Changed type creation.
adriancampo Jan 21, 2024
13e6804
Refs #20165: Added all types.
adriancampo Jan 21, 2024
9fd7b1f
Refs #20165: Fixes for TypeBig.
adriancampo Jan 22, 2024
75d9611
Refs #20165: Fixes for json files.
adriancampo Jan 23, 2024
b7afbf7
Refs #20165: Removed code for debug.
adriancampo Jan 23, 2024
283ad16
Refs #20165: Created unittest
adriancampo Jan 28, 2024
8e3c121
Refs #20165: Implemented unittests. Added Case0 for no TypeObject
adriancampo Jan 30, 2024
46b467a
Refs #20165: Removed sending and receiving samples
adriancampo Jan 31, 2024
03994eb
Refs #20165: Removed descriptions from json tests case files
adriancampo Feb 1, 2024
f435495
Refs #20165: Added communication with dynamic types.
adriancampo Feb 8, 2024
359837d
Refs #20165: Fixed Log macro namespace. And TypeObjectRegistry::regis…
adriancampo Feb 12, 2024
2b99f80
Refs #20165: Fix for type with no TypeObject.
adriancampo Feb 12, 2024
5a72018
Refs #20165: Update TypeLookupManager mock.
adriancampo Feb 13, 2024
6e2d5f8
Refs #20165: Added testing for dds-types-tests.
adriancampo Feb 18, 2024
5319f75
Refs #20165: Changed TypeObject consistency checks when typelookup se…
adriancampo Feb 18, 2024
ed818b5
Refs #20165: Fix for modules in update_header_and_create_cases.py. Ig…
adriancampo Feb 19, 2024
5a93b52
Refs #20165: Reduced number of Cases.json files.
adriancampo Feb 20, 2024
c95ce07
Refs #20165: Fix for modules.
adriancampo Feb 20, 2024
b82bb45
Refs #20165: Changes to use take method. Fixed bug with get_map_depen…
adriancampo Feb 22, 2024
6c6bddd
Refs #20165: Removed willdcards from cmake.
adriancampo Feb 22, 2024
8834124
Refs #20165: Updated types.
adriancampo Feb 28, 2024
dec5d7e
Refs #20165: Fix for custom annotations name hash.
adriancampo Mar 4, 2024
b28b9fb
Refs #20165. Return typelookupservice unit tests
richiware May 30, 2024
46c747e
Refs #20165. Fix for future rebase
richiware Jun 12, 2024
3825749
Refs #20165. Improve tests
richiware Sep 6, 2024
0ee77c7
Refs #20165. Fix typelookupservice builin endpoing discovery
richiware Sep 9, 2024
1b861d0
Refs #20165. Removed member_id testing
richiware Sep 9, 2024
0cf0837
Refs #20165. Fix and improve tests
richiware Sep 10, 2024
2313a11
Apply suggestions from code review
richiware Sep 10, 2024
f8ccba6
Refs #20165. Fix identation
richiware Sep 10, 2024
5c9c223
Refs #20165. Fix compilation error in mac
richiware Sep 10, 2024
8bb5fe6
Refs #20165. Fix compilation errors
richiware Sep 10, 2024
80a2c1f
Refs #20165. Remove trace
richiware Sep 11, 2024
0a2b7c4
Refs #20165. Fixes on windows
richiware Sep 11, 2024
5f46f42
Refs #20165. Fixes after rebase
richiware Sep 11, 2024
9b85819
Refs #20165. Fixes on mac
richiware Sep 12, 2024
e73a278
Refs #20165. Fixes asan
richiware Sep 17, 2024
e7c0a00
Refs #20165. Apply suggestions
richiware Sep 24, 2024
5b5eaeb
Refs #20165. Fix minimal type_id built
richiware Sep 24, 2024
b548118
Refs #20165. Apply suggestions
richiware Sep 24, 2024
29daa05
Refs #20165. Apply suggestions
richiware Sep 24, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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;
MiguelCompany marked this conversation as resolved.
Show resolved Hide resolved
}

// 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
Loading