iox-eclipse-iceoryx#415 Address review finding from eclipse-iceoryx#860…

… fixing ServiceDescription deserialisation Signed-off-by: Simon Hoinkis <[email protected]>
ApexAI · Sep 1, 2021 · 0af7817 · 0af7817
1 parent 0397cec
commit 0af7817
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 15 deletions.
diff --git a/iceoryx_posh/source/capro/service_description.cpp b/iceoryx_posh/source/capro/service_description.cpp
@@ -73,26 +73,41 @@ ServiceDescription::ServiceDescription(const cxx::Serialization& serial) noexcep
 {
     std::underlying_type<Scope>::type scope = 0;
     std::underlying_type<Interfaces>::type interfaceSource = 0;
-    serial.extract(m_serviceString,
-                   m_instanceString,
-                   m_eventString,
-                   m_classHash[0u],
-                   m_classHash[1u],
-                   m_classHash[2u],
-                   m_classHash[3u],
-                   scope,
-                   interfaceSource);
+    auto deserializationSuccessful = serial.extract(m_serviceString,
+                                                    m_instanceString,
+                                                    m_eventString,
+                                                    m_classHash[0u],
+                                                    m_classHash[1u],
+                                                    m_classHash[2u],
+                                                    m_classHash[3u],
+                                                    scope,
+                                                    interfaceSource);
+    if (!deserializationSuccessful)
+    {
+        m_serviceString = iox::capro::InvalidIdString;
+        m_instanceString = iox::capro::InvalidIdString;
+        m_eventString = iox::capro::InvalidIdString;
+        return;
+    }
+
     if (scope > static_cast<std::underlying_type<Scope>::type>(Scope::INVALID))
     {
-        m_scope = Scope::INVALID;
+        m_serviceString = iox::capro::InvalidIdString;
+        m_instanceString = iox::capro::InvalidIdString;
+        m_eventString = iox::capro::InvalidIdString;
+        return;
     }
     else
     {
         m_scope = static_cast<Scope>(scope);
     }
+
     if (interfaceSource > static_cast<std::underlying_type<Interfaces>::type>(Interfaces::INTERFACE_END))
     {
-        m_interfaceSource = Interfaces::INTERFACE_END;
+        m_serviceString = iox::capro::InvalidIdString;
+        m_instanceString = iox::capro::InvalidIdString;
+        m_eventString = iox::capro::InvalidIdString;
+        return;
     }
     else
     {

diff --git a/iceoryx_posh/test/moduletests/test_capro_service.cpp b/iceoryx_posh/test/moduletests/test_capro_service.cpp
@@ -182,7 +182,7 @@ TEST_F(ServiceDescription_test, ServiceDescriptionObjectInitialisationWithOutOfB
 
     ServiceDescription serviceDescription1 = ServiceDescription(serialObj);
 
-    EXPECT_THAT(serviceDescription1.getScope(), Eq(Scope::INVALID));
+    EXPECT_FALSE(serviceDescription1.isValid());
 }
 
 /// @attention The purpose of the Serialization is not to be an alternative Constructor. It is intended to send/receive
@@ -209,10 +209,9 @@ TEST_F(ServiceDescription_test,
 
     ServiceDescription serviceDescription1 = ServiceDescription(serialObj);
 
-    EXPECT_THAT(serviceDescription1.getSourceInterface(), Eq(Interfaces::INTERFACE_END));
+    EXPECT_FALSE(serviceDescription1.isValid());
 }
 
-/// @todo remove
 TEST_F(ServiceDescription_test, ServiceDescriptionDefaultCtorInitializesStringsToInvalidString)
 {
     ServiceDescription serviceDescription1 = ServiceDescription();
@@ -433,7 +432,6 @@ TEST_F(ServiceDescription_test, LessThanOperatorReturnsFalseIfEventStringOfFirst
     EXPECT_FALSE(serviceDescription1 < serviceDescription2);
 }
 
-/// @todo add new tests for service description?
 
 /// END SERVICEDESCRIPTION TESTS