Skip to content

Commit

Permalink
Fix DynamicData union deserialization when no member is selected (#5279)
Browse files Browse the repository at this point in the history 
* Refs #21733. Add regression test

Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>

* Refs #21733. Refine DynamicDataImpl::equals unions case

Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>

* Refs #21733. Fix DynamicData union deserialization when no member is selected

Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>

* Refs #21733. Fix printer

Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>

* Refs #21733. Abort deserialization right away when no member selected

Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>

* Refs #21733. Generate type lookup service tests for new type

Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>

* Refs #21733. Fix early deserialization abortion

Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>

* Refs #21733. Update dds-types-test commit

Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>

---------

Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>
  • Loading branch information
@juanlofer-eprosima
juanlofer-eprosima authored Oct 3, 2024
1 parent 2d1e793 commit 4b968bc
Show file tree
Hide file tree
Showing 14 changed files with 746 additions and 15 deletions.
24 changes: 18 additions & 6 deletions src/cpp/fastdds/xtypes/dynamic_types/DynamicDataImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -373,11 +373,12 @@ bool DynamicDataImpl::equals(
}
else if (TK_UNION == type_kind)
{
return std::static_pointer_cast<DynamicDataImpl>(value_.at(0))->equals(
std::static_pointer_cast<DynamicDataImpl>(other_data->value_.at(0))) &&
(MEMBER_ID_INVALID == selected_union_member_ ||
std::static_pointer_cast<DynamicDataImpl>(value_.at(selected_union_member_))->equals(
std::static_pointer_cast<DynamicDataImpl>(other_data->value_.at(selected_union_member_))));
return (MEMBER_ID_INVALID == selected_union_member_ &&
MEMBER_ID_INVALID == other_data->selected_union_member()) ||
(std::static_pointer_cast<DynamicDataImpl>(value_.at(0))->equals(std::static_pointer_cast<DynamicDataImpl>(
other_data->value_.at(0))) &&
std::static_pointer_cast<DynamicDataImpl>(value_.at(selected_union_member_))->equals(std::
static_pointer_cast<DynamicDataImpl>(other_data->value_.at(selected_union_member_))));
}
else if (TK_ARRAY == type_kind ||
TK_SEQUENCE == type_kind)
Expand Down Expand Up @@ -6478,6 +6479,12 @@ bool DynamicDataImpl::deserialize(
if (MEMBER_ID_INVALID == selected_union_member_)
{
selected_union_member_ = type->default_union_member();

// Check again after attempting to assign the default member
if (MEMBER_ID_INVALID == selected_union_member_)
{
ret_value = false;
}
}
}
else
Expand All @@ -6488,7 +6495,12 @@ bool DynamicDataImpl::deserialize(
break;
default:
{
if (1 == value_.count(selected_union_member_))
if (MEMBER_ID_INVALID == selected_union_member_)
{
// Do nothing
return false;
}
else if (1 == value_.count(selected_union_member_))
{
// Check MemberId in mutable case.
auto member_data {std::static_pointer_cast<DynamicDataImpl>(value_.at(
Expand Down
26 changes: 18 additions & 8 deletions src/cpp/fastdds/xtypes/serializers/json/dynamic_data_json.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,19 +199,29 @@ ReturnCode_t json_serialize_member(

// Fill JSON object with loaned value
nlohmann::json j_union;
DynamicTypeMember::_ref_type active_type_member;
ReturnCode_t ret = st_data->enclosing_type()->get_member(active_type_member,
st_data->selected_union_member());
if (RETCODE_OK != ret)
ReturnCode_t ret = RETCODE_OK;
MemberId selected_member = st_data->selected_union_member();

if (MEMBER_ID_INVALID == selected_member)
{
EPROSIMA_LOG_ERROR(XTYPES_UTILS,
"Error encountered while serializing union member to JSON: get_member failed.");
// No member selected, insert empty JSON object
json_insert(member_name, j_union, output);
}
else
{
if (RETCODE_OK == (ret = json_serialize_member(st_data, active_type_member, j_union, format)))
DynamicTypeMember::_ref_type active_type_member;
ret = st_data->enclosing_type()->get_member(active_type_member, selected_member);
if (RETCODE_OK != ret)
{
json_insert(member_name, j_union, output);
EPROSIMA_LOG_ERROR(XTYPES_UTILS,
"Error encountered while serializing union member to JSON: get_member failed.");
}
else
{
if (RETCODE_OK == (ret = json_serialize_member(st_data, active_type_member, j_union, format)))
{
json_insert(member_name, j_union, output);
}
}
}

Expand Down
170 changes: 170 additions & 0 deletions test/dds-types-test/unions.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19159,6 +19159,176 @@ class DefaultAnnotationExternalValue

std::function<void()> member_destructor_;
};
/*!
* @brief This class represents the structure UnionShortExtraMember defined by the user in the IDL file.
* @ingroup unions
*/
class UnionShortExtraMember
{
public:

/*!
* @brief Default constructor.
*/
eProsima_user_DllExport UnionShortExtraMember()
{
}

/*!
* @brief Default destructor.
*/
eProsima_user_DllExport ~UnionShortExtraMember()
{
}

/*!
* @brief Copy constructor.
* @param x Reference to the object UnionShortExtraMember that will be copied.
*/
eProsima_user_DllExport UnionShortExtraMember(
const UnionShortExtraMember& x)
{
m_var_union_short = x.m_var_union_short;

m_var_long = x.m_var_long;

}

/*!
* @brief Move constructor.
* @param x Reference to the object UnionShortExtraMember that will be copied.
*/
eProsima_user_DllExport UnionShortExtraMember(
UnionShortExtraMember&& x) noexcept
{
m_var_union_short = std::move(x.m_var_union_short);
m_var_long = x.m_var_long;
}

/*!
* @brief Copy assignment.
* @param x Reference to the object UnionShortExtraMember that will be copied.
*/
eProsima_user_DllExport UnionShortExtraMember& operator =(
const UnionShortExtraMember& x)
{

m_var_union_short = x.m_var_union_short;

m_var_long = x.m_var_long;

return *this;
}

/*!
* @brief Move assignment.
* @param x Reference to the object UnionShortExtraMember that will be copied.
*/
eProsima_user_DllExport UnionShortExtraMember& operator =(
UnionShortExtraMember&& x) noexcept
{

m_var_union_short = std::move(x.m_var_union_short);
m_var_long = x.m_var_long;
return *this;
}

/*!
* @brief Comparison operator.
* @param x UnionShortExtraMember object to compare.
*/
eProsima_user_DllExport bool operator ==(
const UnionShortExtraMember& x) const
{
return (m_var_union_short == x.m_var_union_short &&
m_var_long == x.m_var_long);
}

/*!
* @brief Comparison operator.
* @param x UnionShortExtraMember object to compare.
*/
eProsima_user_DllExport bool operator !=(
const UnionShortExtraMember& x) const
{
return !(*this == x);
}

/*!
* @brief This function copies the value in member var_union_short
* @param _var_union_short New value to be copied in member var_union_short
*/
eProsima_user_DllExport void var_union_short(
const Union_Short& _var_union_short)
{
m_var_union_short = _var_union_short;
}

/*!
* @brief This function moves the value in member var_union_short
* @param _var_union_short New value to be moved in member var_union_short
*/
eProsima_user_DllExport void var_union_short(
Union_Short&& _var_union_short)
{
m_var_union_short = std::move(_var_union_short);
}

/*!
* @brief This function returns a constant reference to member var_union_short
* @return Constant reference to member var_union_short
*/
eProsima_user_DllExport const Union_Short& var_union_short() const
{
return m_var_union_short;
}

/*!
* @brief This function returns a reference to member var_union_short
* @return Reference to member var_union_short
*/
eProsima_user_DllExport Union_Short& var_union_short()
{
return m_var_union_short;
}


/*!
* @brief This function sets a value in member var_long
* @param _var_long New value for member var_long
*/
eProsima_user_DllExport void var_long(
int32_t _var_long)
{
m_var_long = _var_long;
}

/*!
* @brief This function returns the value of member var_long
* @return Value of member var_long
*/
eProsima_user_DllExport int32_t var_long() const
{
return m_var_long;
}

/*!
* @brief This function returns a reference to member var_long
* @return Reference to member var_long
*/
eProsima_user_DllExport int32_t& var_long()
{
return m_var_long;
}



private:

Union_Short m_var_union_short;
int32_t m_var_long{0};

};

#endif // _FAST_DDS_GENERATED_UNIONS_HPP_

Expand Down
7 changes: 7 additions & 0 deletions test/dds-types-test/unionsCdrAux.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,9 @@ constexpr uint32_t UnionInnerUnionHelper_max_key_cdr_typesize {0UL};



constexpr uint32_t UnionShortExtraMember_max_cdr_typesize {20UL};
constexpr uint32_t UnionShortExtraMember_max_key_cdr_typesize {0UL};



constexpr uint32_t UnionDiscriminatorULongLong_max_cdr_typesize {24UL};
Expand Down Expand Up @@ -378,6 +381,10 @@ eProsima_user_DllExport void serialize_key(



eProsima_user_DllExport void serialize_key(
eprosima::fastcdr::Cdr& scdr,
const UnionShortExtraMember& data);


} // namespace fastcdr
} // namespace eprosima
Expand Down
89 changes: 89 additions & 0 deletions test/dds-types-test/unionsCdrAux.ipp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8474,6 +8474,95 @@ eProsima_user_DllExport void deserialize(
});
}

template<>
eProsima_user_DllExport size_t calculate_serialized_size(
eprosima::fastcdr::CdrSizeCalculator& calculator,
const UnionShortExtraMember& data,
size_t& current_alignment)
{
static_cast<void>(data);

eprosima::fastcdr::EncodingAlgorithmFlag previous_encoding = calculator.get_encoding();
size_t calculated_size {calculator.begin_calculate_type_serialized_size(
eprosima::fastcdr::CdrVersion::XCDRv2 == calculator.get_cdr_version() ?
eprosima::fastcdr::EncodingAlgorithmFlag::DELIMIT_CDR2 :
eprosima::fastcdr::EncodingAlgorithmFlag::PLAIN_CDR,
current_alignment)};


calculated_size += calculator.calculate_member_serialized_size(eprosima::fastcdr::MemberId(0),
data.var_union_short(), current_alignment);

calculated_size += calculator.calculate_member_serialized_size(eprosima::fastcdr::MemberId(1),
data.var_long(), current_alignment);


calculated_size += calculator.end_calculate_type_serialized_size(previous_encoding, current_alignment);

return calculated_size;
}

template<>
eProsima_user_DllExport void serialize(
eprosima::fastcdr::Cdr& scdr,
const UnionShortExtraMember& data)
{
eprosima::fastcdr::Cdr::state current_state(scdr);
scdr.begin_serialize_type(current_state,
eprosima::fastcdr::CdrVersion::XCDRv2 == scdr.get_cdr_version() ?
eprosima::fastcdr::EncodingAlgorithmFlag::DELIMIT_CDR2 :
eprosima::fastcdr::EncodingAlgorithmFlag::PLAIN_CDR);

scdr
<< eprosima::fastcdr::MemberId(0) << data.var_union_short()
<< eprosima::fastcdr::MemberId(1) << data.var_long()
;
scdr.end_serialize_type(current_state);
}

template<>
eProsima_user_DllExport void deserialize(
eprosima::fastcdr::Cdr& cdr,
UnionShortExtraMember& data)
{
cdr.deserialize_type(eprosima::fastcdr::CdrVersion::XCDRv2 == cdr.get_cdr_version() ?
eprosima::fastcdr::EncodingAlgorithmFlag::DELIMIT_CDR2 :
eprosima::fastcdr::EncodingAlgorithmFlag::PLAIN_CDR,
[&data](eprosima::fastcdr::Cdr& dcdr, const eprosima::fastcdr::MemberId& mid) -> bool
{
bool ret_value = true;
switch (mid.id)
{
case 0:
dcdr >> data.var_union_short();
break;

case 1:
dcdr >> data.var_long();
break;

default:
ret_value = false;
break;
}
return ret_value;
});
}

void serialize_key(
eprosima::fastcdr::Cdr& scdr,
const UnionShortExtraMember& data)
{

static_cast<void>(scdr);
static_cast<void>(data);
scdr << data.var_union_short();

scdr << data.var_long();

}



} // namespace fastcdr
} // namespace eprosima
Expand Down
Loading

0 comments on commit 4b968bc

Please sign in to comment.