Report: Add TriggerIds property
This change is adding TriggerIds property for Report interface. It is an
array of strings, each representing id of trigger which may update given
report Readings property, due to UpdateReport action. This properly is
read-only, but it can be changed in the runtime, when:
- New Trigger is made using AddTrigger dbus method, when ReportId
argument contains id of given report.
- Trigger is deleted from dbus, and its ReportNames property included id
of given report.
- ReportNames property of any trigger is updated to include (or not) id
of given report.
When this property is modified by service, signal will be emitted on
dbus for property change.
When there is existing trigger with id of non-existing report in its
ReportNames property, its id will be added to TriggerIds property of
such report, the moment it is created by user.
Testing done:
- new UTs were made, all UTs are passing.
- manual testing on dbus interface was successful.
Signed-off-by: Szymon Dompke <szymon.dompke@intel.com>
Change-Id: I1c4c94ce751ddaee001e3cadde3ea60aa8e1c224
diff --git a/tests/src/mocks/report_factory_mock.hpp b/tests/src/mocks/report_factory_mock.hpp
index c2e4dd2..cf9047f 100644
--- a/tests/src/mocks/report_factory_mock.hpp
+++ b/tests/src/mocks/report_factory_mock.hpp
@@ -37,7 +37,7 @@
WithArgs<1>(Invoke(&ReportFactoryMock::convertToLabeled)));
ON_CALL(*this,
- make(A<const std::string&>(), _, _, _, _, _, _, _, _, _, _))
+ make(A<const std::string&>(), _, _, _, _, _, _, _, _, _, _, _))
.WillByDefault(WithArgs<0>(Invoke([](const std::string& id) {
return std::make_unique<NiceMock<ReportMock>>(id);
})));
@@ -52,7 +52,7 @@
const std::vector<ReportAction>&, Milliseconds, uint64_t,
const ReportUpdates, interfaces::ReportManager&,
interfaces::JsonStorage&, std::vector<LabeledMetricParameters>,
- bool),
+ bool, const std::vector<std::string>&),
(const, override));
auto& expectMake(
@@ -60,6 +60,7 @@
const testing::Matcher<interfaces::ReportManager&>& rm,
const testing::Matcher<interfaces::JsonStorage&>& js)
{
+ using testing::_;
if (paramsRef)
{
const ReportParams& params = *paramsRef;
@@ -68,12 +69,12 @@
params.reportingType(), params.reportActions(),
params.interval(), params.appendLimit(),
params.reportUpdates(), rm, js,
- params.metricParameters(), params.enabled()));
+ params.metricParameters(), params.enabled(), _));
}
else
{
- using testing::_;
- return EXPECT_CALL(*this, make(_, _, _, _, _, _, _, rm, js, _, _));
+ return EXPECT_CALL(*this,
+ make(_, _, _, _, _, _, _, rm, js, _, _, _));
}
}
};
diff --git a/tests/src/mocks/report_manager_mock.hpp b/tests/src/mocks/report_manager_mock.hpp
index df79e12..25f40a0 100644
--- a/tests/src/mocks/report_manager_mock.hpp
+++ b/tests/src/mocks/report_manager_mock.hpp
@@ -9,4 +9,8 @@
public:
MOCK_METHOD(void, removeReport, (const interfaces::Report*), (override));
MOCK_METHOD(void, updateReport, (const std::string& name), (override));
+ MOCK_METHOD(void, updateTriggerIds,
+ (const std::string& reportId, const std::string& triggerId,
+ TriggerIdUpdate updateType),
+ (override));
};
diff --git a/tests/src/mocks/report_mock.hpp b/tests/src/mocks/report_mock.hpp
index 758290c..6ececfc 100644
--- a/tests/src/mocks/report_mock.hpp
+++ b/tests/src/mocks/report_mock.hpp
@@ -24,5 +24,9 @@
MOCK_METHOD(std::string, getId, (), (override, const));
MOCK_METHOD(std::string, getPath, (), (override, const));
MOCK_METHOD(void, updateReadings, (), (override));
+ MOCK_METHOD(void, updateTriggerIds,
+ (const std::string& triggerId, TriggerIdUpdate updateType),
+ (override));
+
MOCK_METHOD(void, Die, ());
};
diff --git a/tests/src/mocks/trigger_manager_mock.hpp b/tests/src/mocks/trigger_manager_mock.hpp
index 9377d2a..f9f898d 100644
--- a/tests/src/mocks/trigger_manager_mock.hpp
+++ b/tests/src/mocks/trigger_manager_mock.hpp
@@ -9,4 +9,6 @@
public:
MOCK_METHOD(void, removeTrigger, (const interfaces::Trigger* trigger),
(override));
+ MOCK_METHOD(std::vector<std::string>, getTriggerIdsForReport,
+ (const std::string& reportId), (const, override));
};
diff --git a/tests/src/mocks/trigger_mock.hpp b/tests/src/mocks/trigger_mock.hpp
index cd0e5f4..bb38257 100644
--- a/tests/src/mocks/trigger_mock.hpp
+++ b/tests/src/mocks/trigger_mock.hpp
@@ -23,5 +23,7 @@
MOCK_METHOD(std::string, getId, (), (const, override));
MOCK_METHOD(std::string, getPath, (), (const, override));
+ MOCK_METHOD(const std::vector<std::string>&, getReportIds, (),
+ (const, override));
MOCK_METHOD(void, Die, ());
};
diff --git a/tests/src/params/report_params.hpp b/tests/src/params/report_params.hpp
index 19ea519..f6e547e 100644
--- a/tests/src/params/report_params.hpp
+++ b/tests/src/params/report_params.hpp
@@ -108,6 +108,17 @@
return metricParametersProperty;
}
+ ReportParams& triggerIds(std::vector<std::string> val)
+ {
+ triggerIdsProperty = std::move(val);
+ return *this;
+ }
+
+ std::vector<std::string> triggerIds() const
+ {
+ return triggerIdsProperty;
+ }
+
private:
std::string reportIdProperty = "TestId";
std::string reportNameProperty = "TestReport";
@@ -133,5 +144,6 @@
"MetricId2",
CollectionTimeScope::point,
CollectionDuration(Milliseconds(0u))}}};
+ std::vector<std::string> triggerIdsProperty{};
bool enabledProperty = true;
};
diff --git a/tests/src/params/trigger_params.hpp b/tests/src/params/trigger_params.hpp
index ea7f7dc..b17ea86 100644
--- a/tests/src/params/trigger_params.hpp
+++ b/tests/src/params/trigger_params.hpp
@@ -54,6 +54,12 @@
return reportIdsProperty;
}
+ TriggerParams& reportIds(std::vector<std::string> val)
+ {
+ reportIdsProperty = std::move(val);
+ return *this;
+ }
+
TriggerParams& thresholdParams(LabeledTriggerThresholdParams val)
{
labeledThresholdsProperty = std::move(val);
diff --git a/tests/src/test_report.cpp b/tests/src/test_report.cpp
index af22166..3e6b049 100644
--- a/tests/src/test_report.cpp
+++ b/tests/src/test_report.cpp
@@ -86,7 +86,7 @@
params.reportUpdates(), *reportManagerMock, storageMock,
utils::convContainer<std::shared_ptr<interfaces::Metric>>(
metricMocks),
- params.enabled(), std::move(clockFakePtr));
+ params.enabled(), std::move(clockFakePtr), params.triggerIds());
}
template <class T>
@@ -164,6 +164,9 @@
Eq(toReadingParameters(defaultParams.metricParameters())));
EXPECT_THAT(getProperty<std::string>(sut->getPath(), "Name"),
Eq(defaultParams.reportName()));
+ EXPECT_THAT(
+ getProperty<std::vector<std::string>>(sut->getPath(), "TriggerIds"),
+ Eq(std::vector<std::string>()));
}
TEST_F(TestReport, readingsAreInitialyEmpty)
@@ -253,6 +256,59 @@
EXPECT_THAT(ec, Eq(boost::system::errc::success));
}
+TEST_F(TestReport, triggerIdsAreUpdatedProperly)
+{
+ sut->updateTriggerIds("trigger1", TriggerIdUpdate::Add);
+ EXPECT_THAT(
+ getProperty<std::vector<std::string>>(sut->getPath(), "TriggerIds"),
+ UnorderedElementsAre("trigger1"));
+
+ sut->updateTriggerIds("trigger2", TriggerIdUpdate::Add);
+ EXPECT_THAT(
+ getProperty<std::vector<std::string>>(sut->getPath(), "TriggerIds"),
+ UnorderedElementsAre("trigger1", "trigger2"));
+
+ sut->updateTriggerIds("trigger3", TriggerIdUpdate::Add);
+ EXPECT_THAT(
+ getProperty<std::vector<std::string>>(sut->getPath(), "TriggerIds"),
+ UnorderedElementsAre("trigger1", "trigger2", "trigger3"));
+
+ sut->updateTriggerIds("trigger1", TriggerIdUpdate::Remove);
+ EXPECT_THAT(
+ getProperty<std::vector<std::string>>(sut->getPath(), "TriggerIds"),
+ UnorderedElementsAre("trigger2", "trigger3"));
+}
+
+TEST_F(TestReport, successWhenRemovingSameTriggerIdMultipleTimes)
+{
+ sut->updateTriggerIds("trigger1", TriggerIdUpdate::Add);
+ sut->updateTriggerIds("trigger2", TriggerIdUpdate::Add);
+ sut->updateTriggerIds("trigger1", TriggerIdUpdate::Remove);
+ sut->updateTriggerIds("trigger1", TriggerIdUpdate::Remove);
+ EXPECT_THAT(
+ getProperty<std::vector<std::string>>(sut->getPath(), "TriggerIds"),
+ UnorderedElementsAre("trigger2"));
+}
+
+TEST_F(TestReport, successWhenRemovingNonExistingTriggerId)
+{
+ sut->updateTriggerIds("trigger1", TriggerIdUpdate::Add);
+ sut->updateTriggerIds("notTrigger", TriggerIdUpdate::Remove);
+ EXPECT_THAT(
+ getProperty<std::vector<std::string>>(sut->getPath(), "TriggerIds"),
+ UnorderedElementsAre("trigger1"));
+}
+
+TEST_F(TestReport, noDuplicatesWhenSameTriggerIdIsAdded)
+{
+ sut->updateTriggerIds("trigger1", TriggerIdUpdate::Add);
+ sut->updateTriggerIds("trigger2", TriggerIdUpdate::Add);
+ sut->updateTriggerIds("trigger1", TriggerIdUpdate::Add);
+ EXPECT_THAT(
+ getProperty<std::vector<std::string>>(sut->getPath(), "TriggerIds"),
+ UnorderedElementsAre("trigger1", "trigger2"));
+}
+
class TestReportStore :
public TestReport,
public WithParamInterface<std::pair<std::string, nlohmann::json>>
@@ -782,3 +838,12 @@
ASSERT_THAT(storedConfiguration.at("AppendLimit"),
Eq(std::numeric_limits<uint64_t>::max()));
}
+
+TEST_F(TestReportInitialization, triggerIdsPropertyIsInitialzed)
+{
+ sut = makeReport(ReportParams().triggerIds({"trigger1", "trigger2"}));
+
+ EXPECT_THAT(
+ getProperty<std::vector<std::string>>(sut->getPath(), "TriggerIds"),
+ UnorderedElementsAre("trigger1", "trigger2"));
+}
diff --git a/tests/src/test_report_manager.cpp b/tests/src/test_report_manager.cpp
index 10b4de9..bf76efc 100644
--- a/tests/src/test_report_manager.cpp
+++ b/tests/src/test_report_manager.cpp
@@ -1,7 +1,9 @@
#include "dbus_environment.hpp"
#include "helpers.hpp"
+#include "interfaces/trigger_manager.hpp"
#include "mocks/json_storage_mock.hpp"
#include "mocks/report_factory_mock.hpp"
+#include "mocks/trigger_manager_mock.hpp"
#include "params/report_params.hpp"
#include "report.hpp"
#include "report_manager.hpp"
@@ -29,6 +31,9 @@
std::make_unique<NiceMock<ReportMock>>(reportParams.reportId());
ReportMock& reportMock = *reportMockPtr;
+ std::unique_ptr<interfaces::TriggerManager> triggerManagerMockPtr =
+ std::make_unique<NiceMock<TriggerManagerMock>>();
+
std::unique_ptr<ReportManager> sut;
MockFunction<void(std::string)> checkPoint;
@@ -38,9 +43,9 @@
EXPECT_CALL(reportFactoryMock, convertMetricParams(_, _))
.Times(AnyNumber());
- sut = std::make_unique<ReportManager>(std::move(reportFactoryMockPtr),
- std::move(storageMockPtr),
- DbusEnvironment::getObjServer());
+ sut = std::make_unique<ReportManager>(
+ std::move(reportFactoryMockPtr), std::move(storageMockPtr),
+ DbusEnvironment::getObjServer(), triggerManagerMockPtr);
}
void TearDown() override
@@ -378,6 +383,32 @@
sut->updateReport("NotAReport");
}
+TEST_F(TestReportManager, updateTriggerIdsUpdatesThemForExistReport)
+{
+ reportFactoryMock.expectMake(reportParams, Ref(*sut), Ref(storageMock))
+ .WillOnce(Return(ByMove(std::move(reportMockPtr))));
+ EXPECT_CALL(reportMock, updateTriggerIds("Trigger1", TriggerIdUpdate::Add));
+ EXPECT_CALL(reportMock,
+ updateTriggerIds("Trigger2", TriggerIdUpdate::Remove));
+
+ addReport(reportParams);
+ sut->updateTriggerIds(reportParams.reportId(), "Trigger1",
+ TriggerIdUpdate::Add);
+
+ sut->updateTriggerIds(reportParams.reportId(), "Trigger2",
+ TriggerIdUpdate::Remove);
+}
+
+TEST_F(TestReportManager, updateTriggerIdsDoNothingIfReportDoesNotExist)
+{
+ reportFactoryMock.expectMake(reportParams, Ref(*sut), Ref(storageMock))
+ .WillOnce(Return(ByMove(std::move(reportMockPtr))));
+ EXPECT_CALL(reportMock, updateTriggerIds(_, _)).Times(0);
+
+ addReport(reportParams);
+ sut->updateTriggerIds("NotAReport", "Trigger1", TriggerIdUpdate::Add);
+}
+
class TestReportManagerWithAggregationOperationType :
public TestReportManager,
public WithParamInterface<OperationType>
@@ -431,9 +462,9 @@
void makeReportManager()
{
- sut = std::make_unique<ReportManager>(std::move(reportFactoryMockPtr),
- std::move(storageMockPtr),
- DbusEnvironment::getObjServer());
+ sut = std::make_unique<ReportManager>(
+ std::move(reportFactoryMockPtr), std::move(storageMockPtr),
+ DbusEnvironment::getObjServer(), triggerManagerMockPtr);
}
nlohmann::json data = nlohmann::json{
diff --git a/tests/src/test_trigger.cpp b/tests/src/test_trigger.cpp
index a2286ee..5ad2a81 100644
--- a/tests/src/test_trigger.cpp
+++ b/tests/src/test_trigger.cpp
@@ -1,6 +1,7 @@
#include "dbus_environment.hpp"
#include "helpers.hpp"
#include "mocks/json_storage_mock.hpp"
+#include "mocks/report_manager_mock.hpp"
#include "mocks/sensor_mock.hpp"
#include "mocks/threshold_mock.hpp"
#include "mocks/trigger_factory_mock.hpp"
@@ -36,6 +37,8 @@
Milliseconds(5).count(), "32.7"},
});
+ std::unique_ptr<ReportManagerMock> reportManagerMockPtr =
+ std::make_unique<NiceMock<ReportManagerMock>>();
std::unique_ptr<TriggerManagerMock> triggerManagerMockPtr =
std::make_unique<NiceMock<TriggerManagerMock>>();
std::unique_ptr<TriggerFactoryMock> triggerFactoryMockPtr =
@@ -70,7 +73,8 @@
params.reportIds().begin(), params.reportIds().end()),
std::vector<std::shared_ptr<interfaces::Threshold>>(thresholdMocks),
*triggerManagerMockPtr, storageMock, *triggerFactoryMockPtr,
- SensorMock::makeSensorMocks(params.sensors()));
+ SensorMock::makeSensorMocks(params.sensors()),
+ *reportManagerMockPtr);
}
static interfaces::JsonStorage::FilePath to_file_path(std::string name)
@@ -132,6 +136,13 @@
triggerParams.thresholdParams())));
}
+TEST_F(TestTrigger, checkBasicGetters)
+{
+ EXPECT_THAT(sut->getId(), Eq(triggerParams.id()));
+ EXPECT_THAT(sut->getPath(), Eq(Trigger::triggerDir + triggerParams.id()));
+ EXPECT_THAT(sut->getReportIds(), Eq(triggerParams.reportIds()));
+}
+
TEST_F(TestTrigger, setPropertyNameToCorrectValue)
{
std::string name = "custom name 1234 %^#5";
@@ -148,6 +159,70 @@
EXPECT_THAT(
getProperty<std::vector<std::string>>(sut->getPath(), "ReportNames"),
Eq(newNames));
+ EXPECT_THAT(
+ getProperty<std::vector<std::string>>(sut->getPath(), "ReportNames"),
+ Eq(sut->getReportIds()));
+}
+
+TEST_F(TestTrigger, settingPropertyReportNamesUptadesTriggerIdsInReports)
+{
+ std::vector<std::string> newPropertyVal = {"abc", "one", "two"};
+
+ for (const auto& reportId : newPropertyVal)
+ {
+ EXPECT_CALL(
+ *reportManagerMockPtr,
+ updateTriggerIds(reportId, sut->getId(), TriggerIdUpdate::Add));
+ }
+ for (const auto& reportId : triggerParams.reportIds())
+ {
+ EXPECT_CALL(
+ *reportManagerMockPtr,
+ updateTriggerIds(reportId, sut->getId(), TriggerIdUpdate::Remove));
+ }
+
+ EXPECT_THAT(setProperty(sut->getPath(), "ReportNames", newPropertyVal),
+ Eq(boost::system::errc::success));
+}
+
+TEST_F(TestTrigger, settingPropertyReportNamesWillNotRemoveTriggerIdsInReports)
+{
+ std::vector<std::string> newPropertyVal = triggerParams.reportIds();
+ std::vector<std::string> newNames{"abc", "one", "two"};
+ newPropertyVal.insert(newPropertyVal.end(), newNames.begin(),
+ newNames.end());
+
+ for (const auto& reportId : newNames)
+ {
+ EXPECT_CALL(
+ *reportManagerMockPtr,
+ updateTriggerIds(reportId, sut->getId(), TriggerIdUpdate::Add));
+ }
+
+ EXPECT_THAT(setProperty(sut->getPath(), "ReportNames", newPropertyVal),
+ Eq(boost::system::errc::success));
+}
+
+TEST_F(TestTrigger,
+ settingPropertyReportNamesToSameValueWillNotUpdateTriggerIdsInReports)
+{
+ std::vector<std::string> newPropertyVal = triggerParams.reportIds();
+
+ EXPECT_CALL(*reportManagerMockPtr, updateTriggerIds(_, _, _)).Times(0);
+
+ EXPECT_THAT(setProperty(sut->getPath(), "ReportNames", newPropertyVal),
+ Eq(boost::system::errc::success));
+}
+
+TEST_F(TestTrigger,
+ DISABLED_settingPropertyReportNamesThrowsExceptionWhenDuplicateReportIds)
+{
+ std::vector<std::string> newPropertyVal{"trigger1", "trigger2", "trigger1"};
+
+ EXPECT_CALL(*reportManagerMockPtr, updateTriggerIds(_, _, _)).Times(0);
+
+ EXPECT_THAT(setProperty(sut->getPath(), "ReportNames", newPropertyVal),
+ Eq(boost::system::errc::invalid_argument));
}
TEST_F(TestTrigger, setPropertySensors)
@@ -231,6 +306,12 @@
{
EXPECT_CALL(storageMock, remove(to_file_path(sut->getId())));
EXPECT_CALL(*triggerManagerMockPtr, removeTrigger(sut.get()));
+ for (const auto& reportId : triggerParams.reportIds())
+ {
+ EXPECT_CALL(
+ *reportManagerMockPtr,
+ updateTriggerIds(reportId, sut->getId(), TriggerIdUpdate::Remove));
+ }
auto ec = deleteTrigger(sut->getPath());
EXPECT_THAT(ec, Eq(boost::system::errc::success));
}
@@ -252,7 +333,7 @@
Eq(persistent));
}
-class TestTriggerErrors : public TestTrigger
+class TestTriggerInitialization : public TestTrigger
{
public:
void SetUp() override
@@ -261,7 +342,8 @@
nlohmann::json storedConfiguration;
};
-TEST_F(TestTriggerErrors, exceptionDuringTriggerStoreDisablesPersistency)
+TEST_F(TestTriggerInitialization,
+ exceptionDuringTriggerStoreDisablesPersistency)
{
EXPECT_CALL(storageMock, store(_, _))
.WillOnce(Throw(std::runtime_error("Generic error!")));
@@ -271,7 +353,7 @@
EXPECT_THAT(getProperty<bool>(sut->getPath(), "Persistent"), Eq(false));
}
-TEST_F(TestTriggerErrors, creatingTriggerThrowsExceptionWhenIdIsInvalid)
+TEST_F(TestTriggerInitialization, creatingTriggerThrowsExceptionWhenIdIsInvalid)
{
EXPECT_CALL(storageMock, store(_, _)).Times(0);
@@ -279,6 +361,18 @@
sdbusplus::exception::SdBusError);
}
+TEST_F(TestTriggerInitialization, creatingTriggerUpdatesTriggersIdsInReports)
+{
+ for (const auto& reportId : triggerParams.reportIds())
+ {
+ EXPECT_CALL(*reportManagerMockPtr,
+ updateTriggerIds(reportId, triggerParams.id(),
+ TriggerIdUpdate::Add));
+ }
+
+ sut = makeTrigger(triggerParams);
+}
+
class TestTriggerStore : public TestTrigger
{
public:
diff --git a/tests/src/test_trigger_manager.cpp b/tests/src/test_trigger_manager.cpp
index ea4f0b9..efdac65 100644
--- a/tests/src/test_trigger_manager.cpp
+++ b/tests/src/test_trigger_manager.cpp
@@ -123,6 +123,17 @@
EXPECT_THAT(path, Eq(std::string()));
}
+TEST_F(TestTriggerManager, DISABLED_failToAddTriggerWithDuplicatesInReportsIds)
+{
+ triggerFactoryMock.expectMake(std::nullopt, Ref(*sut), Ref(storageMock))
+ .Times(0);
+
+ auto [ec, path] = addTrigger(
+ TriggerParams().reportIds({"trigger1", "trigger2", "trigger1"}));
+ EXPECT_THAT(ec.value(), Eq(boost::system::errc::invalid_argument));
+ EXPECT_THAT(path, Eq(std::string()));
+}
+
TEST_F(TestTriggerManager, addTriggerWithoutIdAndName)
{
triggerFactoryMock
@@ -266,7 +277,6 @@
sut->removeTrigger(&triggerMock);
checkPoint.Call("end");
}
-
class TestTriggerManagerStorage : public TestTriggerManager
{
public: