commit 5e7cbf42eabe916a82a13d41199ecc0921c4cac4
author Michal Orzel <michalx.orzel@intel.com> Thu Aug 01 15:44:42 2024 +0200
committer Adrian Ambrożewicz <adrian.ambrozewicz@linux.intel.com> Tue Aug 20 08:31:07 2024 +0000
tree bdc4904d3c60d6c97863742b964842b53ad07f25
parent 53fb785568946ef22dd79dde100bb6d77a58f080

Verify reading parameters on update

Adds verification of ReadingParameter's metric count when property is
being updated. This brings consistency with behavior upon adding a new
report, where that same check is being made. Also adds a unit test for
that check.

Tested:
On platform, by updating report's ReadingParameter with object
containing metric count exceeding one specified by
`max-reading-parameters` project option. With this change applied, that
operation failed, as expected.

Change-Id: I06c8e21178d6bd554b62886e0e4f8cd0589f0d09
Signed-off-by: Michal Orzel <michalx.orzel@intel.com>

src/report.cpp

diff --git a/src/report.cpp b/src/report.cpp
index fa1da3f..c9df1e2 100644
--- a/src/report.cpp
+++ b/src/report.cpp
@@ -290,6 +290,7 @@
         sdbusplus::vtable::property_::emits_change,
         [this, &reportFactory](auto newVal, auto& oldVal) {
         auto labeledMetricParams = reportFactory.convertMetricParams(newVal);
+        ReportManager::verifyMetricParams(labeledMetricParams);
         reportFactory.updateMetrics(metrics, state.get<ReportFlags::enabled>(),
                                     labeledMetricParams);
         readingParameters = toReadingParameters(

src/report_manager.cpp

diff --git a/src/report_manager.cpp b/src/report_manager.cpp
index 3af033a..e9ccbb8 100644
--- a/src/report_manager.cpp
+++ b/src/report_manager.cpp
@@ -119,19 +119,7 @@
         throw errors::InvalidArgument("Interval");
     }
 
-    size_t metricCount = 0;
-    for (auto metricParam : readingParams)
-    {
-        auto metricParamsVec =
-            metricParam.at_label<utils::tstring::SensorPath>();
-        metricCount += metricParamsVec.size();
-    }
-
-    if (readingParams.size() > maxNumberMetrics ||
-        metricCount > maxNumberMetrics)
-    {
-        throw errors::InvalidArgument("MetricParams", "Too many.");
-    }
+    verifyMetricParams(readingParams);
 
     for (const LabeledMetricParameters& item : readingParams)
     {
@@ -237,3 +225,21 @@
         }
     }
 }
+
+void ReportManager::verifyMetricParams(
+    const std::vector<LabeledMetricParameters>& metricParams)
+{
+    size_t metricCount = 0;
+    for (const auto& metricParam : metricParams)
+    {
+        auto metricParamsVec =
+            metricParam.at_label<utils::tstring::SensorPath>();
+        metricCount += metricParamsVec.size();
+    }
+
+    if (metricParams.size() > ReportManager::maxNumberMetrics ||
+        metricCount > ReportManager::maxNumberMetrics)
+    {
+        throw errors::InvalidArgument("ReadingParameters", "Too many");
+    }
+}

src/report_manager.hpp

diff --git a/src/report_manager.hpp b/src/report_manager.hpp
index dcd2bfc..710d3ce 100644
--- a/src/report_manager.hpp
+++ b/src/report_manager.hpp
@@ -33,6 +33,9 @@
 
     void removeReport(const interfaces::Report* report) override;
 
+    static void verifyMetricParams(
+        const std::vector<LabeledMetricParameters>& metricParams);
+
   private:
     std::unique_ptr<interfaces::ReportFactory> reportFactory;
     std::unique_ptr<interfaces::JsonStorage> reportStorage;

tests/src/test_report.cpp

diff --git a/tests/src/test_report.cpp b/tests/src/test_report.cpp
index 10ba120..e4e5510 100644
--- a/tests/src/test_report.cpp
+++ b/tests/src/test_report.cpp
@@ -290,6 +290,31 @@
         Eq(newParams));
 }
 
+TEST_F(TestReport, setReadingParametersFailsWhenMetricCountExceedsAllowedValue)
+{
+    std::vector<LabeledMetricParameters> readingParams{{LabeledMetricParameters{
+        {LabeledSensorInfo{"Service", "/xyz/openbmc_project/sensors/power/psu",
+                           "NewMetadata123"}},
+        OperationType::avg,
+        CollectionTimeScope::startup,
+        CollectionDuration(250ms)}}};
+
+    auto& metricParamsVec =
+        readingParams[0].at_label<utils::tstring::SensorPath>();
+    for (size_t i = 0; i < ReportManager::maxNumberMetrics; i++)
+    {
+        metricParamsVec.emplace_back(LabeledSensorInfo{
+            "Service", "/xyz/openbmc_project/sensors/power/psu",
+            "NewMetadata123"});
+    }
+
+    ReadingParameters newParams = toReadingParameters(readingParams);
+
+    EXPECT_THAT(
+        setProperty(sut->getPath(), "ReadingParameters", newParams).value(),
+        Eq(boost::system::errc::invalid_argument));
+}
+
 TEST_F(TestReport, setReportActionsWithValidNewActions)
 {
     std::vector<std::string> newActions = {