Refs #20733: Properly fix key annotation handling

Signed-off-by: eduponz <[email protected]>
eProsima · Apr 29, 2024 · b955060 · b955060
1 parent f3feee5
commit b955060
Show file tree

Hide file tree

Showing 5 changed files with 53 additions and 28 deletions.
diff --git a/include/fastrtps/types/MemberDescriptor.h b/include/fastrtps/types/MemberDescriptor.h
@@ -47,6 +47,8 @@ class MemberDescriptor
 
     bool is_type_name_consistent(const std::string& sName) const;
 
+    void copy_annotations_from_type(const DynamicType_ptr& type);
+
 public:
     RTPS_DllAPI MemberDescriptor();
 
@@ -57,18 +59,18 @@ class MemberDescriptor
     RTPS_DllAPI MemberDescriptor(
             MemberId id,
             const std::string& name,
-            DynamicType_ptr type_);
+            DynamicType_ptr type);
 
     RTPS_DllAPI MemberDescriptor(
             MemberId id,
             const std::string& name,
-            DynamicType_ptr type_,
+            DynamicType_ptr type,
             const std::string& defaultValue);
 
     RTPS_DllAPI MemberDescriptor(
             MemberId id,
             const std::string& name,
-            DynamicType_ptr type_,
+            DynamicType_ptr type,
             const std::string& defaultValue,
             const std::vector<uint64_t>& unionLabels,
             bool isDefaultLabel);

diff --git a/src/cpp/dynamic-types/AnnotationDescriptor.cpp b/src/cpp/dynamic-types/AnnotationDescriptor.cpp
@@ -90,12 +90,23 @@ bool AnnotationDescriptor::equals(
 
 bool AnnotationDescriptor::key_annotation() const
 {
-    auto it = value_.find(ANNOTATION_KEY_ID);
-    if (it == value_.end())
+    bool ret = false;
+
+    // Annotations @key and @Key have names "key" and "Key" respectively.
+    if (type_ && (type_->get_name() == ANNOTATION_KEY_ID || type_->get_name() == ANNOTATION_EPKEY_ID))
     {
-        it = value_.find(ANNOTATION_EPKEY_ID); // Legacy "@Key"
+        // When an annotation is a key annotation, there is only one entry in value_.
+        // Its map key is "value" and its value is either "true" of "false".
+        // We cannot call get_value() directly because it is not const-qualified
+        auto it = value_.find("value");
+
+        if (it != value_.end())
+        {
+            ret = it->second == CONST_TRUE;
+        }
     }
-    return (it != value_.end() && it->second == CONST_TRUE);
+
+    return ret;
 }
 
 ReturnCode_t AnnotationDescriptor::get_value(

diff --git a/src/cpp/dynamic-types/DynamicType.cpp b/src/cpp/dynamic-types/DynamicType.cpp
@@ -191,7 +191,7 @@ ReturnCode_t DynamicType::copy_from_builder(
         {
             DynamicTypeMember* newMember = new DynamicTypeMember(it->second);
             newMember->set_parent(this);
-            is_key_defined_ = newMember->key_annotation();
+            is_key_defined_ |= newMember->key_annotation();
             member_by_id_.insert(std::make_pair(newMember->get_id(), newMember));
             member_by_name_.insert(std::make_pair(newMember->get_name(), newMember));
         }
@@ -245,14 +245,7 @@ TypeDescriptor* DynamicType::get_descriptor()
 
 bool DynamicType::key_annotation() const
 {
-    for (auto anIt = descriptor_->annotation_.begin(); anIt != descriptor_->annotation_.end(); ++anIt)
-    {
-        if ((*anIt)->key_annotation())
-        {
-            return true;
-        }
-    }
-    return false;
+    return descriptor_->annotation_get_key();
 }
 
 bool DynamicType::equals(

diff --git a/src/cpp/dynamic-types/MemberDescriptor.cpp b/src/cpp/dynamic-types/MemberDescriptor.cpp
@@ -31,6 +31,7 @@ MemberDescriptor::MemberDescriptor()
     , index_(INDEX_INVALID)
     , default_label_(false)
 {
+    copy_annotations_from_type(type_);
 }
 
 MemberDescriptor::MemberDescriptor(
@@ -43,6 +44,7 @@ MemberDescriptor::MemberDescriptor(
     , index_(index)
     , default_label_(false)
 {
+    copy_annotations_from_type(type_);
 }
 
 MemberDescriptor::MemberDescriptor(
@@ -55,51 +57,54 @@ MemberDescriptor::MemberDescriptor(
     , default_label_(false)
 {
     copy_from(descriptor);
+    copy_annotations_from_type(type_);
 }
 
 MemberDescriptor::MemberDescriptor(
         MemberId id,
         const std::string& name,
-        DynamicType_ptr type_)
+        DynamicType_ptr type)
     : name_(name)
     , id_(id)
-    , type_(type_)
+    , type_(type)
     , default_value_("")
     , index_(INDEX_INVALID)
     , default_label_(false)
 {
-
+    copy_annotations_from_type(type_);
 }
 
 MemberDescriptor::MemberDescriptor(
         MemberId id,
         const std::string& name,
-        DynamicType_ptr type_,
+        DynamicType_ptr type,
         const std::string& defaultValue)
     : name_(name)
     , id_(id)
-    , type_(type_)
+    , type_(type)
     , default_value_(defaultValue)
     , index_(INDEX_INVALID)
     , default_label_(false)
 {
+    copy_annotations_from_type(type_);
 }
 
 MemberDescriptor::MemberDescriptor(
         MemberId id,
         const std::string& name,
-        DynamicType_ptr type_,
+        DynamicType_ptr type,
         const std::string& defaultValue,
         const std::vector<uint64_t>& unionLabels,
         bool isDefaultLabel)
     : name_(name)
     , id_(id)
-    , type_(type_)
+    , type_(type)
     , default_value_(defaultValue)
     , index_(INDEX_INVALID)
     , default_label_(isDefaultLabel)
 {
     labels_ = unionLabels;
+    copy_annotations_from_type(type_);
 }
 
 MemberDescriptor::~MemberDescriptor()
@@ -411,6 +416,24 @@ bool MemberDescriptor::is_type_name_consistent(
     return TypeDescriptor::is_type_name_consistent(sName);
 }
 
+void MemberDescriptor::copy_annotations_from_type(const DynamicType_ptr& type)
+{
+    if (type)
+    {
+        // Copy annotation from type
+        uint32_t num_annotations = type->get_annotation_count();
+        for (uint32_t i = 0; i < num_annotations; ++i)
+        {
+            AnnotationDescriptor ann;
+            type->get_annotation(ann, i);
+            AnnotationDescriptor* pNewDescriptor = new AnnotationDescriptor();
+            pNewDescriptor->copy_from(&ann);
+            annotation_.push_back(pNewDescriptor);
+        }
+    }
+
+}
+
 void MemberDescriptor::set_id(
         MemberId id)
 {

diff --git a/src/cpp/rtps/xmlparser/XMLDynamicParser.cpp b/src/cpp/rtps/xmlparser/XMLDynamicParser.cpp
@@ -1464,11 +1464,7 @@ p_dynamictypebuilder_t XMLParser::parseXMLMemberDynamicType(
     {
         if (strncmp(memberTopicKey, "true", 5) == 0)
         {
-            memberBuilder->apply_annotation(types::ANNOTATION_KEY_ID, "key", "true");
-            if (p_dynamictype != nullptr)
-            {
-                p_dynamictype->apply_annotation(types::ANNOTATION_KEY_ID, "key", "true");
-            }
+            memberBuilder->apply_annotation(types::ANNOTATION_KEY_ID, "value", "true");
         }
     }