Run is_plain method with the corresponding data representation (#4681)

* Refs #20742: Add regression tests for endpoints Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> * Refs #20742: Run method with the corresponding data representation Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> * Refs #20742: Set SampleLoanManager variable with property value Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> * Refs #20574: Please linter Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> * Refs #20742: Check `is_plain` for all possible DataReader's data representation Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> * Refs #20742: Fix failures in other tests Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> * Refs #20742: Fix unset datarepresentation case Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> * Refs #20742: Add Miguel' suggestion Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> * Refs #20742: Apply rev suggestions Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> * Refs #20742: Please linter Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> --------- Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>
eProsima · May 8, 2024 · aef4f43 · aef4f43
1 parent c2a4523
commit aef4f43
Show file tree

Hide file tree

Showing 7 changed files with 313 additions and 12 deletions.
diff --git a/src/cpp/fastdds/publisher/DataWriterImpl.cpp b/src/cpp/fastdds/publisher/DataWriterImpl.cpp
@@ -484,7 +484,7 @@ ReturnCode_t DataWriterImpl::loan_sample(
             microseconds(::TimeConv::Time_t2MicroSecondsInt64(qos_.reliability().max_blocking_time));
 
     // Type should be plain and have space for the representation header
-    if (!type_->is_plain() || SerializedPayload_t::representation_header_size > type_->m_typeSize)
+    if (!type_->is_plain(data_representation_) || SerializedPayload_t::representation_header_size > type_->m_typeSize)
     {
         return ReturnCode_t::RETCODE_ILLEGAL_OPERATION;
     }
@@ -570,7 +570,7 @@ ReturnCode_t DataWriterImpl::discard_loan(
         void*& sample)
 {
     // Type should be plain and have space for the representation header
-    if (!type_->is_plain() || SerializedPayload_t::representation_header_size > type_->m_typeSize)
+    if (!type_->is_plain(data_representation_) || SerializedPayload_t::representation_header_size > type_->m_typeSize)
     {
         return ReturnCode_t::RETCODE_ILLEGAL_OPERATION;
     }
@@ -2033,7 +2033,7 @@ std::shared_ptr<IPayloadPool> DataWriterImpl::get_payload_pool()
         // When the user requested PREALLOCATED_WITH_REALLOC, but we know the type cannot
         // grow, we translate the policy into bare PREALLOCATED
         if (PREALLOCATED_WITH_REALLOC_MEMORY_MODE == history_.m_att.memoryPolicy &&
-                (type_->is_bounded() || type_->is_plain()))
+                (type_->is_bounded() || type_->is_plain(data_representation_)))
         {
             history_.m_att.memoryPolicy = PREALLOCATED_MEMORY_MODE;
         }
@@ -2058,7 +2058,7 @@ std::shared_ptr<IPayloadPool> DataWriterImpl::get_payload_pool()
         }
 
         // Prepare loans collection for plain types only
-        if (type_->is_plain())
+        if (type_->is_plain(data_representation_))
         {
             loans_.reset(new LoanCollection(config));
         }

diff --git a/src/cpp/fastdds/subscriber/DataReaderImpl.cpp b/src/cpp/fastdds/subscriber/DataReaderImpl.cpp
@@ -1804,10 +1804,26 @@ DataReaderListener* DataReaderImpl::get_listener_for(
 
 std::shared_ptr<IPayloadPool> DataReaderImpl::get_payload_pool()
 {
+    // Check whether DataReader's type is plain in all its data representations
+    bool is_plain = true;
+    if (qos_.type_consistency().representation.m_value.size() > 0)
+    {
+        for (auto data_representation : qos_.type_consistency().representation.m_value)
+        {
+            is_plain = is_plain && type_->is_plain(data_representation);
+        }
+    }
+    // If data representation is not defined, consider both XCDR representations
+    else
+    {
+        is_plain = type_->is_plain(DataRepresentationId_t::XCDR_DATA_REPRESENTATION)
+                && type_->is_plain(DataRepresentationId_t::XCDR2_DATA_REPRESENTATION);
+    }
+
     // When the user requested PREALLOCATED_WITH_REALLOC, but we know the type cannot
     // grow, we translate the policy into bare PREALLOCATED
     if (PREALLOCATED_WITH_REALLOC_MEMORY_MODE == history_.m_att.memoryPolicy &&
-            (type_->is_bounded() || type_->is_plain()))
+            (type_->is_bounded() || is_plain))
     {
         history_.m_att.memoryPolicy = PREALLOCATED_MEMORY_MODE;
     }
@@ -1816,7 +1832,7 @@ std::shared_ptr<IPayloadPool> DataReaderImpl::get_payload_pool()
 
     if (!sample_pool_)
     {
-        sample_pool_ = std::make_shared<detail::SampleLoanManager>(config, type_);
+        sample_pool_ = std::make_shared<detail::SampleLoanManager>(config, type_, is_plain);
     }
     if (!is_custom_payload_pool_)
     {

diff --git a/src/cpp/fastdds/subscriber/DataReaderImpl/SampleLoanManager.hpp b/src/cpp/fastdds/subscriber/DataReaderImpl/SampleLoanManager.hpp
@@ -51,8 +51,10 @@ struct SampleLoanManager
 
     SampleLoanManager(
             const PoolConfig& pool_config,
-            const TypeSupport& type)
-        : limits_(pool_config.initial_size,
+            const TypeSupport& type,
+            const bool is_plain)
+        : is_plain_(is_plain)
+        , limits_(pool_config.initial_size,
                 pool_config.maximum_size ? pool_config.maximum_size : std::numeric_limits<size_t>::max(),
                 1)
         , free_loans_(limits_)
@@ -62,7 +64,7 @@ struct SampleLoanManager
         for (size_t n = 0; n < limits_.initial; ++n)
         {
             OutstandingLoanItem item;
-            if (!type_->is_plain())
+            if (!is_plain_)
             {
                 item.sample = type_->createData();
             }
@@ -72,7 +74,7 @@ struct SampleLoanManager
 
     ~SampleLoanManager()
     {
-        if (!type_->is_plain())
+        if (!is_plain_)
         {
             for (const OutstandingLoanItem& item : free_loans_)
             {
@@ -108,7 +110,7 @@ struct SampleLoanManager
             if (nullptr != item)
             {
                 // Create sample if necessary
-                if (!type_->is_plain())
+                if (!is_plain_)
                 {
                     item->sample = type_->createData();
                 }
@@ -139,7 +141,7 @@ struct SampleLoanManager
         tmp.serializedPayload.data = nullptr;
 
         // Perform deserialization
-        if (type_->is_plain())
+        if (is_plain_)
         {
             auto ptr = item->payload.data;
             ptr += item->payload.representation_header_size;
@@ -212,6 +214,7 @@ struct SampleLoanManager
 
     using collection_type = eprosima::fastrtps::ResourceLimitedVector<OutstandingLoanItem>;
 
+    bool is_plain_;
     eprosima::fastrtps::ResourceLimitedContainerConfig limits_;
     collection_type free_loans_;
     collection_type used_loans_;

diff --git a/test/unittest/dds/participant/ParticipantTests.cpp b/test/unittest/dds/participant/ParticipantTests.cpp
@@ -241,6 +241,12 @@ class LoanableTopicDataTypeMock : public TopicDataType
         return true;
     }
 
+    inline bool is_plain(
+            DataRepresentationId_t) const override
+    {
+        return true;
+    }
+
     bool getKey(
             void* /*data*/,
             fastrtps::rtps::InstanceHandle_t* /*ihandle*/,

diff --git a/test/unittest/dds/publisher/DataWriterTests.cpp b/test/unittest/dds/publisher/DataWriterTests.cpp
@@ -1402,6 +1402,12 @@ class LoanableTypeSupport : public TopicDataType
         return true;
     }
 
+    bool is_plain(
+            DataRepresentationId_t) const override
+    {
+        return true;
+    }
+
     bool construct_sample(
             void* sample) const override
     {
@@ -1509,6 +1515,12 @@ class LoanableTypeSupportTesting : public LoanableTypeSupport
         return is_plain_result;
     }
 
+    bool is_plain(
+            DataRepresentationId_t) const override
+    {
+        return is_plain_result;
+    }
+
     bool construct_sample(
             void* sample) const override
     {
@@ -2157,6 +2169,88 @@ TEST(DataWriterTests, history_depth_max_samples_per_instance_warning)
     Log::KillThread();
 }
 
+class DataRepresentationTestsTypeSupport : public LoanableTypeSupport
+{
+public:
+
+    bool is_bounded() const override
+    {
+        return true;
+    }
+
+    MOCK_CONST_METHOD1(custom_is_plain_with_rep, bool(DataRepresentationId_t data_representation_id));
+
+    bool is_plain(
+            DataRepresentationId_t data_representation_id) const override
+    {
+        return custom_is_plain_with_rep(data_representation_id);
+    }
+
+    MOCK_CONST_METHOD0(custom_is_plain, bool());
+
+    bool is_plain() const override
+    {
+        return custom_is_plain();
+    }
+
+};
+
+TEST(DataWriterTests, data_type_is_plain_data_representation)
+{
+    /* Create a participant, topic, and a publisher */
+    DomainParticipant* participant = DomainParticipantFactory::get_instance()->create_participant(0,
+                    PARTICIPANT_QOS_DEFAULT);
+    ASSERT_NE(participant, nullptr);
+
+    DataRepresentationTestsTypeSupport* type = new DataRepresentationTestsTypeSupport();
+    TypeSupport ts (type);
+    ts.register_type(participant);
+
+    Topic* topic = participant->create_topic("plain_topic", "LoanableType", TOPIC_QOS_DEFAULT);
+    ASSERT_NE(topic, nullptr);
+
+    Publisher* publisher = participant->create_publisher(PUBLISHER_QOS_DEFAULT);
+    ASSERT_NE(publisher, nullptr);
+
+    /* Define default data representation (XCDR1) QoS to force "is_plain" call */
+    DataWriterQos qos_xcdr = DATAWRITER_QOS_DEFAULT;
+    qos_xcdr.endpoint().history_memory_policy = PREALLOCATED_WITH_REALLOC_MEMORY_MODE;
+
+    /* Expect the "is_plain" method called with default data representation (XCDR1) */
+    EXPECT_CALL(*type, custom_is_plain()).Times(0);
+    EXPECT_CALL(*type, custom_is_plain_with_rep(DataRepresentationId_t::XCDR_DATA_REPRESENTATION)).Times(
+        testing::AtLeast(1)).WillRepeatedly(testing::Return(true));
+    EXPECT_CALL(*type, custom_is_plain_with_rep(DataRepresentationId_t::XCDR2_DATA_REPRESENTATION)).Times(0);
+
+    /* Create a datawriter will trigger the "is_plain" call */
+    DataWriter* datawriter_xcdr = publisher->create_datawriter(topic, qos_xcdr);
+    ASSERT_NE(datawriter_xcdr, nullptr);
+
+    testing::Mock::VerifyAndClearExpectations(&type);
+
+    /* Define XCDR2 data representation QoS to force "is_plain" call */
+    DataWriterQos qos_xcdr2 = DATAWRITER_QOS_DEFAULT;
+    qos_xcdr2.endpoint().history_memory_policy = PREALLOCATED_WITH_REALLOC_MEMORY_MODE;
+    qos_xcdr2.representation().m_value.clear();
+    qos_xcdr2.representation().m_value.push_back(DataRepresentationId_t::XCDR2_DATA_REPRESENTATION);
+
+    /* Expect the "is_plain" method called with XCDR2 data representation */
+    EXPECT_CALL(*type, custom_is_plain()).Times(0);
+    EXPECT_CALL(*type, custom_is_plain_with_rep(DataRepresentationId_t::XCDR_DATA_REPRESENTATION)).Times(0);
+    EXPECT_CALL(*type, custom_is_plain_with_rep(DataRepresentationId_t::XCDR2_DATA_REPRESENTATION)).Times(
+        testing::AtLeast(1)).WillRepeatedly(testing::Return(true));
+
+    /* Create a datawriter will trigger the "is_plain" call */
+    DataWriter* datawriter_xcdr2 = publisher->create_datawriter(topic, qos_xcdr2);
+    ASSERT_NE(datawriter_xcdr2, nullptr);
+
+    testing::Mock::VerifyAndClearExpectations(&type);
+
+    /* Tear down */
+    participant->delete_contained_entities();
+    DomainParticipantFactory::get_instance()->delete_participant(participant);
+}
+
 } // namespace dds
 } // namespace fastdds
 } // namespace eprosima

diff --git a/test/unittest/dds/publisher/PublisherTests.cpp b/test/unittest/dds/publisher/PublisherTests.cpp
@@ -180,6 +180,12 @@ class LoanableTopicDataTypeMock : public TopicDataType
         return true;
     }
 
+    inline bool is_plain(
+            DataRepresentationId_t) const override
+    {
+        return true;
+    }
+
     bool getKey(
             void* /*data*/,
             fastrtps::rtps::InstanceHandle_t* /*ihandle*/,