Refs #12001: apply to listeners

jparisu · jparisu · commit 36c5f3e16e1e · 2021-07-16T10:18:20.000+02:00
Signed-off-by: jparisu &lt;javierparis@eprosima.com&gt;
diff --git a/src/cpp/database/database.cpp b/src/cpp/database/database.cpp
@@ -1511,10 +1511,13 @@ std::vector<const StatisticsSample*> Database::select(
             }
             else
             {
-                if (source_entity->kind != kinds.first)
-                {
-                    continue;
-                }
+                // There are no possibility with the current kinds to arrive to this branch
+                assert(source_entity->kind == kinds.first);
+                // If in the future new kinds are added that could use this branch, use this code
+                // if (source_entity->kind != kinds.first)
+                // {
+                //     continue;
+                // }
             }
 
             if (!target_entity)
@@ -1523,10 +1526,13 @@ std::vector<const StatisticsSample*> Database::select(
             }
             else
             {
-                if (target_entity->kind != kinds.second)
-                {
-                    continue;
-                }
+                // There are no possibility with the current kinds to arrive to this branch
+                assert(target_entity->kind == kinds.second);
+                // If in the future new kinds are added that could use this branch, use this code
+                // if (target_entity->kind != kinds.second)
+                // {
+                //     continue;
+                // }
             }
         }
         catch (const std::exception& e)
@@ -2026,7 +2032,7 @@ EntityKind Database::get_entity_kind(
 const std::vector<std::shared_ptr<const Entity>> Database::get_entities(
         EntityKind entity_kind,
         const EntityId& entity_id,
-        EntityKind source_entity_kind /* = EntityKind::INVALID */) const
+        const EntityKind source_entity_kind /* = EntityKind::INVALID */) const
 {
     std::shared_ptr<const Entity> origin;
 
@@ -2061,7 +2067,7 @@ const std::vector<std::shared_ptr<const Entity>> Database::get_entities(
 std::vector<EntityId> Database::get_entity_ids(
         EntityKind entity_kind,
         const EntityId& entity_id,
-        EntityKind source_entity_kind /* = EntityKind::INVALID */) const
+        const EntityKind source_entity_kind /* = EntityKind::INVALID */) const
 {
     std::vector<EntityId> entitiesIds;
     for (const auto& entity : get_entities(entity_kind, entity_id, source_entity_kind))
diff --git a/src/cpp/database/database.hpp b/src/cpp/database/database.hpp
@@ -241,7 +241,7 @@ class Database
     const std::vector<std::shared_ptr<const Entity>> get_entities(
             EntityKind entity_kind,
             const EntityId& entity_id,
-            EntityKind source_entity_kind = EntityKind::INVALID) const;
+            const EntityKind source_entity_kind = EntityKind::INVALID) const;
 
     /**
      * Get all EntityIds of a given EntityKind related to another entity.
@@ -261,7 +261,7 @@ class Database
     std::vector<EntityId> get_entity_ids(
             EntityKind entity_type,
             const EntityId& entity_id,
-            EntityKind source_entity_kind = EntityKind::INVALID) const;
+            const EntityKind source_entity_kind = EntityKind::INVALID) const;
 
     /**
      *  @brief Generate an EntityId that is unique for the database.
diff --git a/src/cpp/database/database_queue.cpp b/src/cpp/database/database_queue.cpp
@@ -699,7 +699,7 @@ void DatabaseDataQueue::process_sample()
                 {
                     // Host name reported by Fast DDS are considered unique
                     std::shared_ptr<const Host> const_host = std::dynamic_pointer_cast<const Host>(database_->get_entity(
-                                        hosts.front().second));
+                                        hosts.front().second, EntityKind::HOST));
                     host = std::const_pointer_cast<Host>(const_host);
                 }
 
@@ -709,7 +709,7 @@ void DatabaseDataQueue::process_sample()
                 for (const auto& it : users)
                 {
                     std::shared_ptr<const User> const_user =
-                            std::dynamic_pointer_cast<const User>(database_->get_entity(it.second));
+                            std::dynamic_pointer_cast<const User>(database_->get_entity(it.second, EntityKind::USER));
 
                     // The user name is unique within the host
                     if (const_user->host == host)
@@ -731,7 +731,9 @@ void DatabaseDataQueue::process_sample()
                 for (const auto& it : processes)
                 {
                     std::shared_ptr<const Process> const_process =
-                            std::dynamic_pointer_cast<const Process>(database_->get_entity(it.second));
+                            std::dynamic_pointer_cast<const Process>(database_->get_entity(
+                                        it.second,
+                                        EntityKind::PROCESS));
 
                     // There is only one process with the same name for a given user
                     if (const_process->user == user)
diff --git a/src/cpp/subscriber/StatisticsParticipantListener.cpp b/src/cpp/subscriber/StatisticsParticipantListener.cpp
@@ -66,7 +66,7 @@ void StatisticsParticipantListener::process_endpoint_discovery(
     // This may throw if the domain does not exist
     // The database MUST contain the domain, or something went wrong upstream
     std::shared_ptr<database::Domain> domain = std::const_pointer_cast<database::Domain>(
-        std::static_pointer_cast<const database::Domain>(database_->get_entity(domain_id_)));
+        std::static_pointer_cast<const database::Domain>(database_->get_entity(domain_id_, EntityKind::DOMAIN)));
 
     // Get the participant from the database
     GUID_t endpoint_guid = info.info.guid();
@@ -86,7 +86,7 @@ void StatisticsParticipantListener::process_endpoint_discovery(
     std::shared_ptr<database::DomainParticipant> participant =
             std::const_pointer_cast<database::DomainParticipant>(
         std::static_pointer_cast<const database::DomainParticipant>(database_->get_entity(
-            participant_id.second)));
+            participant_id.second, EntityKind::PARTICIPANT)));
 
     assert(participant_id.first == domain_id_);
 
@@ -100,7 +100,9 @@ void StatisticsParticipantListener::process_endpoint_discovery(
         if (topic_id.first == domain_id_)
         {
             topic = std::const_pointer_cast<database::Topic>(
-                std::static_pointer_cast<const database::Topic>(database_->get_entity(topic_id.second)));
+                std::static_pointer_cast<const database::Topic>(database_->get_entity(
+                    topic_id.second,
+                    EntityKind::TOPIC)));
 
             if (topic->data_type == info.info.typeName().to_string())
             {
@@ -134,7 +136,6 @@ void StatisticsParticipantListener::process_endpoint_discovery(
     auto endpoint = create_endpoint(endpoint_guid, info, participant, topic);
 
     /* Start processing the locator info */
-
     // Routine to process one locator from the locator list of the endpoint
     auto process_locators = [&](const Locator_t& dds_locator)
             {
@@ -319,7 +320,9 @@ void StatisticsParticipantListener::on_participant_discovery(
         database::EntityDiscoveryInfo entity_discovery_info;
         entity_discovery_info.domain_id = domain_id_;
         entity_discovery_info.entity = std::const_pointer_cast<database::DomainParticipant>(
-            std::static_pointer_cast<const database::DomainParticipant>(database_->get_entity(participant_id)));
+            std::static_pointer_cast<const database::DomainParticipant>(database_->get_entity(
+                participant_id,
+                EntityKind::PARTICIPANT)));
 
         switch (info.status)
         {
@@ -352,7 +355,9 @@ void StatisticsParticipantListener::on_participant_discovery(
             // This may throw if the domain does not exist
             // The database MUST contain the domain, or something went wrong upstream
             std::shared_ptr<database::Domain> domain = std::const_pointer_cast<database::Domain>(
-                std::static_pointer_cast<const database::Domain>(database_->get_entity(domain_id_)));
+                std::static_pointer_cast<const database::Domain>(database_->get_entity(
+                    domain_id_,
+                    EntityKind::DOMAIN)));
 
             std::string name = info.info.m_participantName.to_string();
 
@@ -418,7 +423,9 @@ void StatisticsParticipantListener::on_subscriber_discovery(
         database::EntityDiscoveryInfo entity_discovery_info;
         entity_discovery_info.domain_id = domain_id_;
         entity_discovery_info.entity = std::const_pointer_cast<database::DataReader>(
-            std::static_pointer_cast<const database::DataReader>(database_->get_entity(datareader_id)));
+            std::static_pointer_cast<const database::DataReader>(database_->get_entity(
+                datareader_id,
+                EntityKind::DATAREADER)));
 
         switch (info.status)
         {
@@ -488,7 +495,9 @@ void StatisticsParticipantListener::on_publisher_discovery(
         database::EntityDiscoveryInfo entity_discovery_info;
         entity_discovery_info.domain_id = domain_id_;
         entity_discovery_info.entity = std::const_pointer_cast<database::DataWriter>(
-            std::static_pointer_cast<const database::DataWriter>(database_->get_entity(datawriter_id)));
+            std::static_pointer_cast<const database::DataWriter>(database_->get_entity(
+                datawriter_id,
+                EntityKind::DATAWRITER)));
 
         switch (info.status)
         {
diff --git a/test/mock/database/database/database/database.hpp b/test/mock/database/database/database/database.hpp
@@ -83,14 +83,28 @@ class Database
     MOCK_CONST_METHOD1(get_entity, const std::shared_ptr<const Entity>(
                 const EntityId& entity_id));
 
+    MOCK_CONST_METHOD2(get_entity, const std::shared_ptr<const Entity>(
+                const EntityId& entity_id,
+                const EntityKind entity_kind));
+
     MOCK_CONST_METHOD2(get_entities, const std::vector<std::shared_ptr<const Entity>>(
                 EntityKind entity_kind,
                 const EntityId& entity_id));
 
+    MOCK_CONST_METHOD3(get_entities, const std::vector<std::shared_ptr<const Entity>>(
+                EntityKind entity_kind,
+                const EntityId& entity_id,
+                const EntityKind source_entity_kind));
+
     MOCK_CONST_METHOD2(get_entity_ids, std::vector<EntityId>(
                 EntityKind entity_kind,
                 const EntityId& entity_id));
 
+    MOCK_CONST_METHOD3(get_entity_ids, std::vector<EntityId>(
+                EntityKind entity_kind,
+                const EntityId& entity_id,
+                const EntityKind source_entity_kind));
+
     MOCK_CONST_METHOD2(get_entity_by_guid, std::pair<EntityId, EntityId>(
                 EntityKind entity_kind,
                 const std::string& guid));
diff --git a/test/unittest/Database/CMakeLists.txt b/test/unittest/Database/CMakeLists.txt
@@ -300,7 +300,19 @@ set(DATABASE_TEST_LIST
     get_entity_datareader
     get_entity_datawriter
     get_entity_locator
+    get_entity_host_efficient_search
+    get_entity_process_efficient_search
+    get_entity_user_efficient_search
+    get_entity_domain_efficient_search
+    get_entity_topic_efficient_search
+    get_entity_participant_efficient_search
+    get_entity_datareader_efficient_search
+    get_entity_datawriter_efficient_search
+    get_entity_locator_efficient_search
+
     get_entity_no_existing
+    get_entity_no_correct_kind
+
     # get_entities_by_name
     get_entities_by_name_host
     get_entities_by_name_host_wrong_name
@@ -473,7 +485,7 @@ endforeach()
 add_executable(database_load_tests DatabaseLoadTests.cpp ${LIBRARY_SOURCES})
 
 if(MSVC)
-    target_compile_definitions(database_load_tests PRIVATE 
+    target_compile_definitions(database_load_tests PRIVATE
         _CRT_DECLARE_NONSTDC_NAMES=0 FASTDDS_STATISTICS_BACKEND_SOURCE)
 endif(MSVC)
 
@@ -525,7 +537,7 @@ endforeach()
 add_executable(database_load_insert_tests DatabaseLoadInsertTests.cpp ${LIBRARY_SOURCES})
 
 if(MSVC)
-    target_compile_definitions(database_load_insert_tests PRIVATE 
+    target_compile_definitions(database_load_insert_tests PRIVATE
         _CRT_DECLARE_NONSTDC_NAMES=0 FASTDDS_STATISTICS_BACKEND_SOURCE)
 endif(MSVC)
 
@@ -565,7 +577,7 @@ endforeach()
 add_executable(database_status_tests DatabaseStatusTests.cpp ${LIBRARY_SOURCES})
 
 if(MSVC)
-    target_compile_definitions(database_status_tests PRIVATE 
+    target_compile_definitions(database_status_tests PRIVATE
         _CRT_DECLARE_NONSTDC_NAMES=0 FASTDDS_STATISTICS_BACKEND_SOURCE)
 endif(MSVC)
 
@@ -615,7 +627,7 @@ endforeach()
 add_executable(database_erase_tests DatabaseEraseTests.cpp ${LIBRARY_SOURCES})
 
 if(MSVC)
-    target_compile_definitions(database_erase_tests PRIVATE 
+    target_compile_definitions(database_erase_tests PRIVATE
         _CRT_DECLARE_NONSTDC_NAMES=0 FASTDDS_STATISTICS_BACKEND_SOURCE)
 endif(MSVC)
 
diff --git a/test/unittest/Database/DatabaseTests.cpp b/test/unittest/Database/DatabaseTests.cpp
@@ -2432,9 +2432,114 @@ TEST_F(database_tests, get_entity_locator)
     ASSERT_EQ(local_writer_locator.get(), writer_locator.get());
 }
 
+TEST_F(database_tests, get_entity_host_efficient_search)
+{
+    auto local_host = db.get_entity(host_id, EntityKind::HOST);
+    ASSERT_EQ(local_host.get(), host.get());
+}
+
+TEST_F(database_tests, get_entity_process_efficient_search)
+{
+    auto local_process = db.get_entity(process_id, EntityKind::PROCESS);
+    ASSERT_EQ(local_process.get(), process.get());
+}
+
+TEST_F(database_tests, get_entity_user_efficient_search)
+{
+    auto local_user = db.get_entity(user_id, EntityKind::USER);
+    ASSERT_EQ(local_user.get(), user.get());
+}
+
+TEST_F(database_tests, get_entity_domain_efficient_search)
+{
+    auto local_domain = db.get_entity(domain_id, EntityKind::DOMAIN);
+    ASSERT_EQ(local_domain.get(), domain.get());
+}
+
+TEST_F(database_tests, get_entity_topic_efficient_search)
+{
+    auto local_topic = db.get_entity(topic_id, EntityKind::TOPIC);
+    ASSERT_EQ(local_topic.get(), topic.get());
+}
+
+TEST_F(database_tests, get_entity_participant_efficient_search)
+{
+    auto local_participant = db.get_entity(participant_id, EntityKind::PARTICIPANT);
+    ASSERT_EQ(local_participant.get(), participant.get());
+}
+
+TEST_F(database_tests, get_entity_datareader_efficient_search)
+{
+    auto local_reader = db.get_entity(reader_id, EntityKind::DATAREADER);
+    ASSERT_EQ(local_reader.get(), reader.get());
+}
+
+TEST_F(database_tests, get_entity_datawriter_efficient_search)
+{
+    auto local_writer = db.get_entity(writer_id, EntityKind::DATAWRITER);
+    ASSERT_EQ(local_writer.get(), writer.get());
+}
+
+TEST_F(database_tests, get_entity_locator_efficient_search)
+{
+    auto local_reader_locator = db.get_entity(reader_locator->id, EntityKind::LOCATOR);
+    ASSERT_EQ(local_reader_locator.get(), reader_locator.get());
+    auto local_writer_locator = db.get_entity(writer_locator->id, EntityKind::LOCATOR);
+    ASSERT_EQ(local_writer_locator.get(), writer_locator.get());
+}
+
 TEST_F(database_tests, get_entity_no_existing)
 {
     ASSERT_THROW(db.get_entity(EntityId()), BadParameter);
+    // With specific kind
+    ASSERT_THROW(db.get_entity(EntityId(), EntityKind::HOST), BadParameter);
+    ASSERT_THROW(db.get_entity(EntityId(), EntityKind::USER), BadParameter);
+    ASSERT_THROW(db.get_entity(EntityId(), EntityKind::PROCESS), BadParameter);
+    ASSERT_THROW(db.get_entity(EntityId(), EntityKind::DOMAIN), BadParameter);
+    ASSERT_THROW(db.get_entity(EntityId(), EntityKind::TOPIC), BadParameter);
+    ASSERT_THROW(db.get_entity(EntityId(), EntityKind::PARTICIPANT), BadParameter);
+    ASSERT_THROW(db.get_entity(EntityId(), EntityKind::DATAREADER), BadParameter);
+    ASSERT_THROW(db.get_entity(EntityId(), EntityKind::DATAWRITER), BadParameter);
+    ASSERT_THROW(db.get_entity(EntityId(), EntityKind::LOCATOR), BadParameter);
+}
+
+TEST_F(database_tests, get_entity_no_correct_kind)
+{
+    std::map<EntityId, EntityKind> correct_kinds = {
+        {host_id, EntityKind::HOST},
+        {process_id, EntityKind::PROCESS},
+        {user_id, EntityKind::USER},
+        {domain_id, EntityKind::DOMAIN},
+        {topic_id, EntityKind::TOPIC},
+        {participant_id, EntityKind::PARTICIPANT},
+        {reader_id, EntityKind::DATAREADER},
+        {writer_id, EntityKind::DATAWRITER},
+        {reader_locator->id, EntityKind::LOCATOR},
+        {writer_locator->id, EntityKind::LOCATOR}
+    };
+
+    std::vector<EntityKind> all_kinds = {
+        EntityKind::HOST,
+        EntityKind::PROCESS,
+        EntityKind::USER,
+        EntityKind::DOMAIN,
+        EntityKind::TOPIC,
+        EntityKind::PARTICIPANT,
+        EntityKind::DATAREADER,
+        EntityKind::DATAWRITER,
+        EntityKind::LOCATOR
+    };
+
+    for (auto correct_kind : correct_kinds)
+    {
+        for (auto kind : all_kinds)
+        {
+            if (kind != correct_kind.second)
+            {
+                ASSERT_THROW(db.get_entity(correct_kind.first, kind), BadParameter);
+            }
+        }
+    }
 }
 
 TEST_F(database_tests, get_entities_by_name_host)
diff --git a/test/unittest/DatabaseQueue/DatabaseQueueTests.cpp b/test/unittest/DatabaseQueue/DatabaseQueueTests.cpp
diff --git a/test/unittest/StatisticsParticipantListener/StatisticsParticipantListenerTests.cpp b/test/unittest/StatisticsParticipantListener/StatisticsParticipantListenerTests.cpp
diff --git a/test/unittest/StatisticsReaderListener/StatisticsReaderListenerTests.cpp b/test/unittest/StatisticsReaderListener/StatisticsReaderListenerTests.cpp

Original file line number	Diff line number	Diff line change
`@@ -1511,10 +1511,13 @@ std::vector<const StatisticsSample*> Database::select(`
`1511`	`1511`	`}`
`1512`	`1512`	`else`
`1513`	`1513`	`{`
`1514`		`- if (source_entity->kind != kinds.first)`
`1515`		`- {`
`1516`		`- continue;`
`1517`		`- }`
	`1514`	`+ // There are no possibility with the current kinds to arrive to this branch`
	`1515`	`+ assert(source_entity->kind == kinds.first);`
	`1516`	`+ // If in the future new kinds are added that could use this branch, use this code`
	`1517`	`+ // if (source_entity->kind != kinds.first)`
	`1518`	`+ // {`
	`1519`	`+ // continue;`
	`1520`	`+ // }`
`1518`	`1521`	`}`
`1519`	`1522`
`1520`	`1523`	`if (!target_entity)`
`@@ -1523,10 +1526,13 @@ std::vector<const StatisticsSample*> Database::select(`
`1523`	`1526`	`}`
`1524`	`1527`	`else`
`1525`	`1528`	`{`
`1526`		`- if (target_entity->kind != kinds.second)`
`1527`		`- {`
`1528`		`- continue;`
`1529`		`- }`
	`1529`	`+ // There are no possibility with the current kinds to arrive to this branch`
	`1530`	`+ assert(target_entity->kind == kinds.second);`
	`1531`	`+ // If in the future new kinds are added that could use this branch, use this code`
	`1532`	`+ // if (target_entity->kind != kinds.second)`
	`1533`	`+ // {`
	`1534`	`+ // continue;`
	`1535`	`+ // }`
`1530`	`1536`	`}`
`1531`	`1537`	`}`
`1532`	`1538`	`catch (const std::exception& e)`
`@@ -2026,7 +2032,7 @@ EntityKind Database::get_entity_kind(`
`2026`	`2032`	`const std::vector<std::shared_ptr<const Entity>> Database::get_entities(`
`2027`	`2033`	`EntityKind entity_kind,`
`2028`	`2034`	`const EntityId& entity_id,`
`2029`		`- EntityKind source_entity_kind /* = EntityKind::INVALID */) const`
	`2035`	`+ const EntityKind source_entity_kind /* = EntityKind::INVALID */) const`
`2030`	`2036`	`{`
`2031`	`2037`	`std::shared_ptr<const Entity> origin;`
`2032`	`2038`
`@@ -2061,7 +2067,7 @@ const std::vector<std::shared_ptr<const Entity>> Database::get_entities(`
`2061`	`2067`	`std::vector<EntityId> Database::get_entity_ids(`
`2062`	`2068`	`EntityKind entity_kind,`
`2063`	`2069`	`const EntityId& entity_id,`
`2064`		`- EntityKind source_entity_kind /* = EntityKind::INVALID */) const`
	`2070`	`+ const EntityKind source_entity_kind /* = EntityKind::INVALID */) const`
`2065`	`2071`	`{`
`2066`	`2072`	`std::vector<EntityId> entitiesIds;`
`2067`	`2073`	`for (const auto& entity : get_entities(entity_kind, entity_id, source_entity_kind))`
Original file line number	Diff line number	Diff line change
`@@ -699,7 +699,7 @@ void DatabaseDataQueue::process_sample()`
`699`	`699`	`{`
`700`	`700`	`// Host name reported by Fast DDS are considered unique`
`701`	`701`	`std::shared_ptr<const Host> const_host = std::dynamic_pointer_cast<const Host>(database_->get_entity(`
`702`		`- hosts.front().second));`
	`702`	`+ hosts.front().second, EntityKind::HOST));`
`703`	`703`	`host = std::const_pointer_cast<Host>(const_host);`
`704`	`704`	`}`
`705`	`705`
`@@ -709,7 +709,7 @@ void DatabaseDataQueue::process_sample()`
`709`	`709`	`for (const auto& it : users)`
`710`	`710`	`{`
`711`	`711`	`std::shared_ptr<const User> const_user =`
`712`		`- std::dynamic_pointer_cast<const User>(database_->get_entity(it.second));`
	`712`	`+ std::dynamic_pointer_cast<const User>(database_->get_entity(it.second, EntityKind::USER));`
`713`	`713`
`714`	`714`	`// The user name is unique within the host`
`715`	`715`	`if (const_user->host == host)`
`@@ -731,7 +731,9 @@ void DatabaseDataQueue::process_sample()`
`731`	`731`	`for (const auto& it : processes)`
`732`	`732`	`{`
`733`	`733`	`std::shared_ptr<const Process> const_process =`
`734`		`- std::dynamic_pointer_cast<const Process>(database_->get_entity(it.second));`
	`734`	`+ std::dynamic_pointer_cast<const Process>(database_->get_entity(`
	`735`	`+ it.second,`
	`736`	`+ EntityKind::PROCESS));`
`735`	`737`
`736`	`738`	`// There is only one process with the same name for a given user`
`737`	`739`	`if (const_process->user == user)`