Fix support for @key annotation in Dynamic types (#4694) (#4748)

* Refs #20733: Fix support for key annotation in XML types Signed-off-by: eduponz <eduardoponz@eprosima.com> * Refs #20733: Add test Signed-off-by: eduponz <eduardoponz@eprosima.com> * Refs #20733: Move implementations to cpp Signed-off-by: eduponz <eduardoponz@eprosima.com> * Refs #20733: Properly fix key annotation handling Signed-off-by: eduponz <eduardoponz@eprosima.com> * Refs #20733: Apply Miguel's suggestions Signed-off-by: eduponz <eduardoponz@eprosima.com> --------- Signed-off-by: eduponz <eduardoponz@eprosima.com> (cherry picked from commit eaa23db) Co-authored-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
eProsima · May 7, 2024 · 5eb48f6 · 5eb48f6
1 parent 8252d15
commit 5eb48f6
Show file tree

Hide file tree

Showing 13 changed files with 189 additions and 73 deletions.
diff --git a/include/fastrtps/types/MemberDescriptor.h b/include/fastrtps/types/MemberDescriptor.h
@@ -18,16 +18,17 @@
 #include <fastrtps/types/TypesBase.h>
 #include <fastrtps/types/DynamicTypePtr.h>
 
-namespace eprosima{
-namespace fastrtps{
-namespace types{
+namespace eprosima {
+namespace fastrtps {
+namespace types {
 
 class DynamicType;
 class AnnotationDescriptor;
 
 class MemberDescriptor
 {
 protected:
+
     std::string name_;                  // Name of the member
     MemberId id_;                       // MemberId, it should be filled automatically when the member is added.
     DynamicType_ptr type_;              // Member's Type.
@@ -43,11 +44,17 @@ class MemberDescriptor
     friend class DynamicTypeMember;
     friend class TypeObjectFactory;
 
-    bool is_default_value_consistent(const std::string& sDefaultValue) const;
+    bool is_default_value_consistent(
+            const std::string& sDefaultValue) const;
+
+    bool is_type_name_consistent(
+            const std::string& sName) const;
 
-    bool is_type_name_consistent(const std::string& sName) const;
+    void copy_annotations_from_type(
+            const DynamicType_ptr& type);
 
 public:
+
     RTPS_DllAPI MemberDescriptor();
 
     RTPS_DllAPI MemberDescriptor(
@@ -57,37 +64,41 @@ 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);
 
-    RTPS_DllAPI MemberDescriptor(const MemberDescriptor* descriptor);
+    RTPS_DllAPI MemberDescriptor(
+            const MemberDescriptor* descriptor);
 
     RTPS_DllAPI ~MemberDescriptor();
 
-    bool check_union_labels(const std::vector<uint64_t>& labels) const;
+    bool check_union_labels(
+            const std::vector<uint64_t>& labels) const;
 
-    RTPS_DllAPI ReturnCode_t copy_from(const MemberDescriptor* other);
+    RTPS_DllAPI ReturnCode_t copy_from(
+            const MemberDescriptor* other);
 
-    RTPS_DllAPI bool equals(const MemberDescriptor* other) const;
+    RTPS_DllAPI bool equals(
+            const MemberDescriptor* other) const;
 
     RTPS_DllAPI TypeKind get_kind() const;
 
     RTPS_DllAPI MemberId get_id() const;
 
-    RTPS_DllAPI  uint32_t get_index() const;
+    RTPS_DllAPI uint32_t get_index() const;
 
     RTPS_DllAPI std::string get_name() const;
 
@@ -105,39 +116,49 @@ class MemberDescriptor
 
     RTPS_DllAPI bool is_default_union_value() const;
 
-    RTPS_DllAPI bool is_consistent(TypeKind parentKind) const;
+    RTPS_DllAPI bool is_consistent(
+            TypeKind parentKind) const;
 
-    RTPS_DllAPI void add_union_case_index(uint64_t value);
+    RTPS_DllAPI void add_union_case_index(
+            uint64_t value);
 
-    RTPS_DllAPI void set_id(MemberId id);
+    RTPS_DllAPI void set_id(
+            MemberId id);
 
-    RTPS_DllAPI void set_index(uint32_t index);
+    RTPS_DllAPI void set_index(
+            uint32_t index);
 
-    RTPS_DllAPI void set_name(const std::string& name);
+    RTPS_DllAPI void set_name(
+            const std::string& name);
 
-    RTPS_DllAPI void set_type(DynamicType_ptr type);
+    RTPS_DllAPI void set_type(
+            DynamicType_ptr type);
 
     RTPS_DllAPI DynamicType_ptr get_type() const
     {
         return type_;
     }
 
-    RTPS_DllAPI void set_default_union_value(bool bDefault);
+    RTPS_DllAPI void set_default_union_value(
+            bool bDefault);
 
-    RTPS_DllAPI void set_default_value(const std::string& value)
+    RTPS_DllAPI void set_default_value(
+            const std::string& value)
     {
         default_value_ = value;
     }
 
     // Annotations
-    ReturnCode_t apply_annotation(AnnotationDescriptor& descriptor);
+    ReturnCode_t apply_annotation(
+            AnnotationDescriptor& descriptor);
 
     ReturnCode_t apply_annotation(
             const std::string& annotation_name,
             const std::string& key,
             const std::string& value);
 
-    AnnotationDescriptor* get_annotation(const std::string& name) const;
+    AnnotationDescriptor* get_annotation(
+            const std::string& name) const;
 
     // Annotations application
     RTPS_DllAPI bool annotation_is_optional() const;
@@ -168,23 +189,31 @@ class MemberDescriptor
     RTPS_DllAPI uint16_t annotation_get_bit_bound() const;
 
     // Annotations setters
-    RTPS_DllAPI void annotation_set_optional(bool optional);
+    RTPS_DllAPI void annotation_set_optional(
+            bool optional);
 
-    RTPS_DllAPI void annotation_set_key(bool key);
+    RTPS_DllAPI void annotation_set_key(
+            bool key);
 
-    RTPS_DllAPI void annotation_set_must_understand(bool must_understand);
+    RTPS_DllAPI void annotation_set_must_understand(
+            bool must_understand);
 
-    RTPS_DllAPI void annotation_set_non_serialized(bool non_serialized);
+    RTPS_DllAPI void annotation_set_non_serialized(
+            bool non_serialized);
 
-    RTPS_DllAPI void annotation_set_value(const std::string& value);
+    RTPS_DllAPI void annotation_set_value(
+            const std::string& value);
 
-    RTPS_DllAPI void annotation_set_default(const std::string& default_value);
+    RTPS_DllAPI void annotation_set_default(
+            const std::string& default_value);
 
     RTPS_DllAPI void annotation_set_default_literal();
 
-    RTPS_DllAPI void annotation_set_position(uint16_t position);
+    RTPS_DllAPI void annotation_set_position(
+            uint16_t position);
 
-    RTPS_DllAPI void annotation_set_bit_bound(uint16_t bit_bound);
+    RTPS_DllAPI void annotation_set_bit_bound(
+            uint16_t bit_bound);
 };
 
 } // namespace types

diff --git a/include/fastrtps/xmlparser/XMLProfileManager.h b/include/fastrtps/xmlparser/XMLProfileManager.h
@@ -243,24 +243,14 @@ class XMLProfileManager
      * XMLProfileManager::DeleteDynamicPubSubType method.
      */
     RTPS_DllAPI static types::DynamicPubSubType* CreateDynamicPubSubType(
-            const std::string& type_name)
-    {
-        if (dynamic_types_.find(type_name) != dynamic_types_.end())
-        {
-            return new types::DynamicPubSubType(dynamic_types_[type_name]->build());
-        }
-        return nullptr;
-    }
+            const std::string& type_name);
 
     /**
      * Deletes the given DynamicPubSubType previously created by calling
      * XMLProfileManager::CreateDynamicPubSubType method.
      */
     RTPS_DllAPI static void DeleteDynamicPubSubType(
-            types::DynamicPubSubType* type)
-    {
-        delete type;
-    }
+            types::DynamicPubSubType* type);
 
 private:
 

diff --git a/resources/xsd/fastRTPS_profiles.xsd b/resources/xsd/fastRTPS_profiles.xsd
@@ -516,6 +516,7 @@
         <xs:attribute name="sequenceMaxLength" type="int32" use="optional"/>
         <xs:attribute name="arrayDimensions" type="arrayDim" use="optional"/>
         <xs:attribute name="key_type" type="string" use="optional"/>
+        <xs:attribute name="key" type="string" use="optional"/>
         <xs:attribute name="mapMaxLength" type="int32" use="optional"/>
     </xs:complexType>
 

diff --git a/src/cpp/dynamic-types/AnnotationDescriptor.cpp b/src/cpp/dynamic-types/AnnotationDescriptor.cpp
@@ -12,10 +12,11 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include <fastdds/dds/log/Log.hpp>
 #include <fastrtps/types/AnnotationDescriptor.h>
 #include <fastrtps/types/DynamicType.h>
 #include <fastrtps/types/DynamicTypeBuilderFactory.h>
-#include <fastdds/dds/log/Log.hpp>
+#include <fastrtps/types/TypesBase.h>
 
 namespace eprosima {
 namespace fastrtps {
@@ -90,18 +91,29 @@ 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 ANNOTATION_VALUE_ID and its value is either "true" of "false".
+        // We cannot call get_value() directly because it is not const-qualified
+        auto it = value_.find(ANNOTATION_VALUE_ID);
+
+        if (it != value_.end())
+        {
+            ret = it->second == CONST_TRUE;
+        }
     }
-    return (it != value_.end() && it->second == CONST_TRUE);
+
+    return ret;
 }
 
 ReturnCode_t AnnotationDescriptor::get_value(
         std::string& value)
 {
-    return get_value(value, "value");
+    return get_value(value, ANNOTATION_VALUE_ID);
 }
 
 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(