Merge pull request #807 from mcserep/order_metrics_results

Properly order the metrics results

Author: mcserep

plugins/cpp_metrics/service/cxxmetrics.thrift

@@ -61,10 +61,22 @@ struct CppMetricsModuleSingle
   3:double value
 }
 
-struct CppMetricsModuleAll
+struct CppMetricsAstNodeEntry
 {
-  1:common.FileId id,
-  2:list<CppMetricsModuleSingle> metrics
+  1: common.AstNodeId astNodeId,
+  2: list<CppMetricsAstNodeSingle> metrics
+}
+
+struct CppMetricsAstNodeDetailedEntry
+{
+  1: common.AstNodeId astNodeId,
+  2: CppMetricsAstNodeDetailed details
+}
+
+struct CppMetricsModuleEntry
+{
+  1: common.FileId fileId,
+  2: list<CppMetricsModuleSingle> metrics
 }
 
 service CppMetricsService
@@ -97,7 +109,7 @@ service CppMetricsService
    *
    * The given path is a handled as a prefix.
    */
-  map<common.AstNodeId, list<CppMetricsAstNodeSingle>> getCppAstNodeMetricsForPath(
+  list<CppMetricsAstNodeEntry> getCppAstNodeMetricsForPath(
     1:string path)
 
   /**
@@ -106,7 +118,7 @@ service CppMetricsService
    *
    * The given path is a handled as a prefix.
    */
-  map<common.AstNodeId, list<CppMetricsAstNodeSingle>> getPagedCppAstNodeMetricsForPath(
+  list<CppMetricsAstNodeEntry> getPagedCppAstNodeMetricsForPath(
     1:string path
     2:i32 pageSize,
     3:common.AstNodeId previousId)
@@ -117,7 +129,7 @@ service CppMetricsService
    *
    * The given path is a handled as a prefix.
    */
-  map<common.AstNodeId, CppMetricsAstNodeDetailed> getCppAstNodeMetricsDetailedForPath(
+  list<CppMetricsAstNodeDetailedEntry> getCppAstNodeMetricsDetailedForPath(
     1:string path)
 
   /**
@@ -126,7 +138,7 @@ service CppMetricsService
    *
    * The given path is a handled as a prefix.
    */
-  map<common.AstNodeId, CppMetricsAstNodeDetailed> getPagedCppAstNodeMetricsDetailedForPath(
+  list<CppMetricsAstNodeDetailedEntry> getPagedCppAstNodeMetricsDetailedForPath(
     1:string path,
     2:i32 pageSize,
     3:common.AstNodeId previousId)
@@ -137,7 +149,7 @@ service CppMetricsService
    *
    * The given path is a handled as a prefix.
    */
-  map<common.FileId, list<CppMetricsModuleSingle>> getCppFileMetricsForPath(
+  list<CppMetricsModuleEntry> getCppFileMetricsForPath(
     1:string path)
 
   /**
@@ -146,7 +158,7 @@ service CppMetricsService
    *
    * The given path is a handled as a prefix.
    */
-  map<common.FileId, list<CppMetricsModuleSingle>> getPagedCppFileMetricsForPath(
+  list<CppMetricsModuleEntry> getPagedCppFileMetricsForPath(
     1:string path,
     2:i32 pageSize,
     3:common.FileId previousId)

plugins/cpp_metrics/service/include/service/cppmetricsservice.h

@@ -50,31 +50,31 @@ class CppMetricsServiceHandler : virtual public CppMetricsServiceIf
     const core::FileId& fileId_) override;
 
   void getCppAstNodeMetricsForPath(
-    std::map<core::AstNodeId, std::vector<CppMetricsAstNodeSingle>>& _return,
+    std::vector<CppMetricsAstNodeEntry>& _return,
     const std::string& path_) override;
 
   void getPagedCppAstNodeMetricsForPath(
-    std::map<core::AstNodeId, std::vector<CppMetricsAstNodeSingle>>& _return,
+    std::vector<CppMetricsAstNodeEntry>& _return,
     const std::string& path_,
     const std::int32_t pageSize_,
     const core::AstNodeId& previousId_) override;
 
   void getCppAstNodeMetricsDetailedForPath(
-    std::map<core::AstNodeId, CppMetricsAstNodeDetailed>& _return,
+    std::vector<CppMetricsAstNodeDetailedEntry>& _return,
     const std::string& path_) override;
 
   void getPagedCppAstNodeMetricsDetailedForPath(
-    std::map<core::AstNodeId, CppMetricsAstNodeDetailed>& _return,
+    std::vector<CppMetricsAstNodeDetailedEntry>& _return,
     const std::string& path_,
     const std::int32_t pageSize_,
     const core::AstNodeId& previousId_) override;
 
   void getCppFileMetricsForPath(
-    std::map<core::FileId, std::vector<CppMetricsModuleSingle>>& _return,
+    std::vector<CppMetricsModuleEntry>& _return,
     const std::string& path_) override;
 
   void getPagedCppFileMetricsForPath(
-    std::map<core::FileId, std::vector<CppMetricsModuleSingle>>& _return,
+    std::vector<CppMetricsModuleEntry>& _return,
     const std::string& path_,
     const std::int32_t pageSize_,
     const core::FileId& previousId_) override;
@@ -104,15 +104,15 @@ class CppMetricsServiceHandler : virtual public CppMetricsServiceIf
     const model::FileId previousId_);
 
   void queryCppAstNodeMetricsForPath(
-    std::map<core::AstNodeId, std::vector<CppMetricsAstNodeSingle>>& _return,
+    std::vector<CppMetricsAstNodeEntry>& result_,
     const odb::query<model::CppAstNodeMetricsForPathView>& query_);
 
   void queryCppAstNodeMetricsDetailedForPath(
-    std::map<core::AstNodeId, CppMetricsAstNodeDetailed>& _return,
+    std::vector<CppMetricsAstNodeDetailedEntry>& result_,
     const odb::query<model::CppAstNodeMetricsAndDataForPathView>& query_);
 
   void queryCppFileMetricsForPath(
-    std::map<core::FileId, std::vector<CppMetricsModuleSingle>>& _return,
+    std::vector<CppMetricsModuleEntry>& result_,
     const odb::query<model::CppModuleMetricsForPathView>& query_);
 };
 

plugins/cpp_metrics/service/src/cppmetricsservice.cpp

@@ -148,7 +148,7 @@ void CppMetricsServiceHandler::getCppMetricsForModule(
 }
 
 void CppMetricsServiceHandler::queryCppAstNodeMetricsForPath(
-  std::map<core::AstNodeId, std::vector<CppMetricsAstNodeSingle>>& _return,
+  std::vector<CppMetricsAstNodeEntry>& result_,
   const odb::query<model::CppAstNodeMetricsForPathView>& query_)
 {
   _transaction([&, this](){
@@ -161,30 +161,40 @@ void CppMetricsServiceHandler::queryCppAstNodeMetricsForPath(
       metric.type = static_cast<CppAstNodeMetricsType::type>(node.type);
       metric.value = node.value;
 
-      if (_return.count(std::to_string(node.astNodeId)))
+      // Try to find existing entry with same astNodeId
+      auto it = std::find_if(result_.begin(), result_.end(),
+        [&](const CppMetricsAstNodeEntry& entry) {
+          return std::stoull(entry.astNodeId) == node.astNodeId;
+        });
+
+      if (it != result_.end())
       {
-        _return[std::to_string(node.astNodeId)].push_back(metric);
+        // Found existing entry → append metric
+        it->metrics.push_back(metric);
       }
+
       else
       {
-        std::vector<CppMetricsAstNodeSingle> metricsList;
-        metricsList.push_back(metric);
-        _return.insert(std::make_pair(std::to_string(node.astNodeId), metricsList));
+        // No entry for this astNodeId → create new one
+        CppMetricsAstNodeEntry entry;
+        entry.astNodeId = std::to_string(node.astNodeId);
+        entry.metrics.push_back(metric);
+        result_.push_back(std::move(entry));
       }
     }
   });
 }
 
 void CppMetricsServiceHandler::getCppAstNodeMetricsForPath(
-  std::map<core::AstNodeId, std::vector<CppMetricsAstNodeSingle>>& _return,
+  std::vector<CppMetricsAstNodeEntry>& _return,
   const std::string& path_)
 {
   queryCppAstNodeMetricsForPath(_return,
     CppNodeMetricsQuery::LocFile::path.like(path_ + '%'));
 }
 
 void CppMetricsServiceHandler::getPagedCppAstNodeMetricsForPath(
-  std::map<core::AstNodeId, std::vector<CppMetricsAstNodeSingle>>& _return,
+  std::vector<CppMetricsAstNodeEntry>& _return,
   const std::string& path_,
   const std::int32_t pageSize_,
   const core::AstNodeId& previousId_)
@@ -197,7 +207,7 @@ void CppMetricsServiceHandler::getPagedCppAstNodeMetricsForPath(
 }
 
 void CppMetricsServiceHandler::queryCppAstNodeMetricsDetailedForPath(
-  std::map<core::AstNodeId, CppMetricsAstNodeDetailed>& _return,
+  std::vector<CppMetricsAstNodeDetailedEntry>& result_,
   const odb::query<model::CppAstNodeMetricsAndDataForPathView>& query_)
 {
   _transaction([&, this](){
@@ -206,12 +216,21 @@ void CppMetricsServiceHandler::queryCppAstNodeMetricsDetailedForPath(
     for (const auto& node : nodes)
     {
       auto pair = std::make_pair(static_cast<CppAstNodeMetricsType::type>(node.type), node.value);
-      if (_return.count(std::to_string(node.astNodeId)))
+
+      // Try to find existing entry with same astNodeId
+      auto it = std::find_if(result_.begin(), result_.end(),
+        [&](const CppMetricsAstNodeDetailedEntry& entry) {
+          return std::stoull(entry.astNodeId) == node.astNodeId;
+        });
+
+      if (it != result_.end())
       {
-        _return[std::to_string(node.astNodeId)].metrics.insert(pair);
+        // Found existing entry → append metric
+        it->details.metrics.insert(pair);
       }
       else
       {
+        // No entry for this astNodeId → create new one
         CppMetricsAstNodeDetailed metric;
         std::size_t pos = node.path.find_last_of('/');
         metric.path = node.path.substr(0, pos + 1);
@@ -223,22 +242,26 @@ void CppMetricsServiceHandler::queryCppAstNodeMetricsDetailedForPath(
         metric.astType = cc::model::astTypeToString(node.astType);
         metric.metrics.insert(pair);
 
-        _return.insert(std::make_pair(std::to_string(node.astNodeId), metric));
+        CppMetricsAstNodeDetailedEntry entry;
+        entry.astNodeId = std::to_string(node.astNodeId);
+        entry.details = metric;
+
+        result_.push_back(std::move(entry));
       }
     }
   });
 }
 
 void CppMetricsServiceHandler::getCppAstNodeMetricsDetailedForPath(
-  std::map<core::AstNodeId, CppMetricsAstNodeDetailed>& _return,
+  std::vector<CppMetricsAstNodeDetailedEntry>& _return,
   const std::string& path_)
 {
   queryCppAstNodeMetricsDetailedForPath(_return,
     CppNodeMetricsQuery::LocFile::path.like(path_ + '%'));
 }
 
 void CppMetricsServiceHandler::getPagedCppAstNodeMetricsDetailedForPath(
-  std::map<core::AstNodeId, CppMetricsAstNodeDetailed>& _return,
+  std::vector<CppMetricsAstNodeDetailedEntry>& _return,
   const std::string& path_,
   const std::int32_t pageSize_,
   const core::AstNodeId& previousId_)
@@ -247,11 +270,12 @@ void CppMetricsServiceHandler::getPagedCppAstNodeMetricsDetailedForPath(
     path_, pageSize_, previousId_.empty() ? 0 : std::stoull(previousId_));
 
   queryCppAstNodeMetricsDetailedForPath(_return,
-    CppNodeMetricsQuery::CppAstNodeMetrics::astNodeId.in_range(paged_nodes.begin(), paged_nodes.end()));
+    CppNodeMetricsQuery::CppAstNodeMetrics::astNodeId.in_range(paged_nodes.begin(), paged_nodes.end())
+    + ("ORDER BY" + odb::query<model::CppAstNodeMetrics>::astNodeId));
 }
 
 void CppMetricsServiceHandler::queryCppFileMetricsForPath(
-  std::map<core::FileId, std::vector<CppMetricsModuleSingle>>& _return,
+  std::vector<CppMetricsModuleEntry>& result_,
   const odb::query<model::CppModuleMetricsForPathView>& query_)
 {
   _transaction([&, this](){
@@ -264,38 +288,50 @@ void CppMetricsServiceHandler::queryCppFileMetricsForPath(
       metric.type = static_cast<CppModuleMetricsType::type>(file.type);
       metric.value = file.value;
 
-      if (_return.count(std::to_string(file.fileId)))
+      // Try to find existing entry with same fileId
+      auto it = std::find_if(result_.begin(), result_.end(),
+        [&](const CppMetricsModuleEntry& entry) {
+          return std::stoull(entry.fileId) == file.fileId;
+        });
+
+      if (it != result_.end())
       {
-        _return[std::to_string(file.fileId)].push_back(metric);
+        // Found existing entry → append metric
+        it->metrics.push_back(metric);
       }
+
       else
       {
-        std::vector<CppMetricsModuleSingle> metricsList;
-        metricsList.push_back(metric);
-        _return.insert(std::make_pair(std::to_string(file.fileId), metricsList));
+        // No entry for this fileId → create new one
+        CppMetricsModuleEntry entry;
+        entry.fileId = std::to_string(file.fileId);
+        entry.metrics.push_back(metric);
+        result_.push_back(std::move(entry));
       }
     }
   });
 }
 
 void CppMetricsServiceHandler::getCppFileMetricsForPath(
-  std::map<core::FileId, std::vector<CppMetricsModuleSingle>>& _return,
+  std::vector<CppMetricsModuleEntry>& _return,
   const std::string& path_)
 {
   queryCppFileMetricsForPath(_return,
     CppModuleMetricsQuery::File::path.like(path_ + '%'));
 }
 
 void CppMetricsServiceHandler::getPagedCppFileMetricsForPath(
-  std::map<core::FileId, std::vector<CppMetricsModuleSingle>>& _return,
+  std::vector<CppMetricsModuleEntry>& _return,
   const std::string& path_,
   const std::int32_t pageSize_,
   const core::FileId& previousId_)
 {
   std::vector<model::FileId> paged_files = pageFileMetrics(
     path_, pageSize_, previousId_.empty() ? 0 : std::stoull(previousId_));
+
   queryCppFileMetricsForPath(_return,
-    CppModuleMetricsQuery::CppFileMetrics::file.in_range(paged_files.begin(), paged_files.end()));
+    CppModuleMetricsQuery::CppFileMetrics::file.in_range(paged_files.begin(), paged_files.end())
+    + ("ORDER BY" + odb::query<model::CppFileMetrics>::file));
 }
 
 std::string CppMetricsServiceHandler::getLimitQuery(