created AddReportFutureVersion dbus method
New method will support CollectionTimeScope, CollectionDuration
In order to make not breaking interface changes bmcweb will switch to
AddReportFutureVersion, then AddReport will be changed to match
AddReportFutureVersion, then redfish will switch back to use AddReport,
then AddReportFutureVersion will be removed.
Tested:
- Verified that current version of bmcweb works fine with old API
Signed-off-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com>
Change-Id: I51a9b7fb2f4da5b8d2f688ccd5e93710352b1ac7
diff --git a/tests/src/test_report_manager.cpp b/tests/src/test_report_manager.cpp
index 4ad5479..2287707 100644
--- a/tests/src/test_report_manager.cpp
+++ b/tests/src/test_report_manager.cpp
@@ -36,6 +36,9 @@
void SetUp() override
{
+ EXPECT_CALL(reportFactoryMock, convertMetricParams(_, _))
+ .Times(AnyNumber());
+
sut = std::make_unique<ReportManager>(std::move(reportFactoryMockPtr),
std::move(storageMockPtr),
DbusEnvironment::getObjServer());
@@ -57,11 +60,11 @@
addReportPromise.set_value({ec, path});
},
DbusEnvironment::serviceName(), ReportManager::reportManagerPath,
- ReportManager::reportManagerIfaceName, "AddReport",
+ ReportManager::reportManagerIfaceName, "AddReportFutureVersion",
params.reportName(), params.reportingType(),
params.emitReadingUpdate(), params.logToMetricReportCollection(),
- static_cast<uint64_t>(params.interval().count()),
- params.readingParameters());
+ params.interval().count(),
+ toReadingParameters(params.metricParameters()));
return DbusEnvironment::waitForFuture(addReportPromise.get_future());
}
@@ -74,10 +77,10 @@
*DbusEnvironment::getBus(), DbusEnvironment::serviceName(),
ReportManager::reportManagerPath,
ReportManager::reportManagerIfaceName, property,
- [&propertyPromise](boost::system::error_code ec, T t) {
+ [&propertyPromise](const boost::system::error_code& ec, T t) {
if (ec)
{
- utils::setException(propertyPromise, "GetProperty failed");
+ utils::setException(propertyPromise, "Get property failed");
return;
}
propertyPromise.set_value(t);
@@ -99,7 +102,7 @@
TEST_F(TestReportManager, minInterval)
{
EXPECT_THAT(getProperty<uint64_t>("MinInterval"),
- Eq(static_cast<uint64_t>(ReportManager::minInterval.count())));
+ Eq(ReportManager::minInterval.count()));
}
TEST_F(TestReportManager, maxReports)
@@ -110,7 +113,8 @@
TEST_F(TestReportManager, addReport)
{
- reportFactoryMock.expectMake(_, reportParams, Ref(*sut), Ref(storageMock))
+ EXPECT_CALL(reportFactoryMock, convertMetricParams(_, _));
+ reportFactoryMock.expectMake(reportParams, Ref(*sut), Ref(storageMock))
.WillOnce(Return(ByMove(std::move(reportMockPtr))));
auto [ec, path] = addReport(reportParams);
@@ -123,7 +127,7 @@
std::string reportName =
prepareReportNameWithLength(ReportManager::maxReportNameLength);
reportParams.reportName(reportName);
- reportFactoryMock.expectMake(_, reportParams, Ref(*sut), Ref(storageMock));
+ reportFactoryMock.expectMake(reportParams, Ref(*sut), Ref(storageMock));
auto [ec, path] = addReport(reportParams);
@@ -133,9 +137,7 @@
TEST_F(TestReportManager, DISABLED_failToAddReportWithTooLongName)
{
- reportFactoryMock.expectMake(_, std::nullopt, Ref(*sut), Ref(storageMock))
- .Times(0);
- reportFactoryMock.expectMake(std::nullopt, Ref(*sut), Ref(storageMock), _)
+ reportFactoryMock.expectMake(std::nullopt, Ref(*sut), Ref(storageMock))
.Times(0);
reportParams.reportName(
@@ -149,7 +151,7 @@
TEST_F(TestReportManager, DISABLED_failToAddReportTwice)
{
- reportFactoryMock.expectMake(_, reportParams, Ref(*sut), Ref(storageMock))
+ reportFactoryMock.expectMake(reportParams, Ref(*sut), Ref(storageMock))
.WillOnce(Return(ByMove(std::move(reportMockPtr))));
addReport(reportParams);
@@ -162,9 +164,7 @@
TEST_F(TestReportManager, DISABLED_failToAddReportWithInvalidInterval)
{
- reportFactoryMock.expectMake(_, std::nullopt, Ref(*sut), Ref(storageMock))
- .Times(0);
- reportFactoryMock.expectMake(std::nullopt, Ref(*sut), Ref(storageMock), _)
+ reportFactoryMock.expectMake(std::nullopt, Ref(*sut), Ref(storageMock))
.Times(0);
reportParams.reportingType("Periodic");
@@ -178,9 +178,7 @@
TEST_F(TestReportManager, DISABLED_failToAddReportWithInvalidReportingType)
{
- reportFactoryMock.expectMake(_, std::nullopt, Ref(*sut), Ref(storageMock))
- .Times(0);
- reportFactoryMock.expectMake(std::nullopt, Ref(*sut), Ref(storageMock), _)
+ reportFactoryMock.expectMake(std::nullopt, Ref(*sut), Ref(storageMock))
.Times(0);
reportParams.reportingType("Invalid");
@@ -193,17 +191,15 @@
TEST_F(TestReportManager, DISABLED_failToAddReportWithMoreSensorsThanExpected)
{
- reportFactoryMock.expectMake(_, std::nullopt, Ref(*sut), Ref(storageMock))
- .Times(0);
- reportFactoryMock.expectMake(std::nullopt, Ref(*sut), Ref(storageMock), _)
+ reportFactoryMock.expectMake(std::nullopt, Ref(*sut), Ref(storageMock))
.Times(0);
- auto readingParams = reportParams.readingParameters();
+ auto metricParams = reportParams.metricParameters();
for (size_t i = 0; i < ReportManager::maxReadingParams + 1; i++)
{
- readingParams.push_back(readingParams.front());
+ metricParams.push_back(metricParams.front());
}
- reportParams.readingParameters(std::move(readingParams));
+ reportParams.metricParameters(std::move(metricParams));
auto [ec, path] = addReport(reportParams);
@@ -213,7 +209,7 @@
TEST_F(TestReportManager, DISABLED_failToAddReportWhenMaxReportIsReached)
{
- reportFactoryMock.expectMake(_, std::nullopt, Ref(*sut), Ref(storageMock))
+ reportFactoryMock.expectMake(std::nullopt, Ref(*sut), Ref(storageMock))
.Times(ReportManager::maxReports);
for (size_t i = 0; i < ReportManager::maxReports; i++)
@@ -236,8 +232,8 @@
{
{
InSequence seq;
- reportFactoryMock
- .expectMake(_, reportParams, Ref(*sut), Ref(storageMock))
+ EXPECT_CALL(reportFactoryMock, convertMetricParams(_, _));
+ reportFactoryMock.expectMake(reportParams, Ref(*sut), Ref(storageMock))
.WillOnce(Return(ByMove(std::move(reportMockPtr))));
EXPECT_CALL(reportMock, Die());
EXPECT_CALL(checkPoint, Call("end"));
@@ -264,8 +260,8 @@
{
{
InSequence seq;
- reportFactoryMock
- .expectMake(_, reportParams, Ref(*sut), Ref(storageMock))
+ EXPECT_CALL(reportFactoryMock, convertMetricParams(_, _));
+ reportFactoryMock.expectMake(reportParams, Ref(*sut), Ref(storageMock))
.WillOnce(Return(ByMove(std::move(reportMockPtr))));
EXPECT_CALL(reportMock, Die());
EXPECT_CALL(checkPoint, Call("end"));
@@ -279,7 +275,7 @@
TEST_F(TestReportManager, updateReportCallsUpdateReadingsForExistReport)
{
- reportFactoryMock.expectMake(_, reportParams, Ref(*sut), Ref(storageMock))
+ reportFactoryMock.expectMake(reportParams, Ref(*sut), Ref(storageMock))
.WillOnce(Return(ByMove(std::move(reportMockPtr))));
EXPECT_CALL(reportMock, updateReadings());
@@ -289,7 +285,7 @@
TEST_F(TestReportManager, updateReportDoNothingIfReportDoesNotExist)
{
- reportFactoryMock.expectMake(_, reportParams, Ref(*sut), Ref(storageMock))
+ reportFactoryMock.expectMake(reportParams, Ref(*sut), Ref(storageMock))
.WillOnce(Return(ByMove(std::move(reportMockPtr))));
EXPECT_CALL(reportMock, updateReadings()).Times(0);
@@ -313,14 +309,17 @@
TEST_P(TestReportManagerWithAggregationOperationType,
addReportWithDifferentOperationTypes)
{
- reportParams.readingParameters(
- {{{sdbusplus::message::object_path(
- "/xyz/openbmc_project/sensors/power/p1")},
- utils::enumToString(operationType),
- "MetricId1",
- "Metadata1"}});
+ reportParams.metricParameters(
+ std::vector<LabeledMetricParameters>{{LabeledMetricParameters{
+ {LabeledSensorParameters{"Service",
+ "/xyz/openbmc_project/sensors/power/p1"}},
+ operationType,
+ "MetricId1",
+ "Metadata1",
+ CollectionTimeScope::point,
+ CollectionDuration(Milliseconds(0u))}}});
- reportFactoryMock.expectMake(_, reportParams, Ref(*sut), Ref(storageMock))
+ reportFactoryMock.expectMake(reportParams, Ref(*sut), Ref(storageMock))
.WillOnce(Return(ByMove(std::move(reportMockPtr))));
auto [ec, path] = addReport(reportParams);
@@ -337,6 +336,8 @@
void SetUp() override
{
+ EXPECT_CALL(reportFactoryMock, convertMetricParams(_, _)).Times(0);
+
ON_CALL(storageMock, list())
.WillByDefault(Return(std::vector<FilePath>{FilePath("report1")}));
ON_CALL(storageMock, load(FilePath("report1")))
@@ -350,17 +351,6 @@
DbusEnvironment::getObjServer());
}
- static std::vector<LabeledMetricParameters>
- convertToLabeled(const ReadingParameters& params)
- {
- return utils::transform(params, [](const auto& item) {
- return LabeledMetricParameters(
- LabeledSensorParameters("service", std::get<0>(item)),
- utils::stringToOperationType(std::get<1>(item)),
- std::get<2>(item), std::get<3>(item));
- });
- }
-
nlohmann::json data = nlohmann::json{
{"Version", Report::reportVersion},
{"Name", reportParams.reportName()},
@@ -369,15 +359,12 @@
{"LogToMetricReportsCollection",
reportParams.logToMetricReportCollection()},
{"Interval", reportParams.interval().count()},
- {"ReadingParameters",
- convertToLabeled(reportParams.readingParameters())}};
+ {"ReadingParameters", reportParams.metricParameters()}};
};
TEST_F(TestReportManagerStorage, reportManagerCtorAddReportFromStorage)
{
- reportFactoryMock.expectMake(
- reportParams, _, Ref(storageMock),
- ElementsAreArray(convertToLabeled(reportParams.readingParameters())));
+ reportFactoryMock.expectMake(reportParams, _, Ref(storageMock));
makeReportManager();
}