Save persistent triggers to storage
Create json storage file for persistent triggers.
Handle persistent dbus property.
Save/remove persistent triggers on add/delete.
Cover code with UTs.
Tested:
- Passed unit tests
- Tested on QEMU
* adding new valid and invalid trigger from cli
* verifying if valid trigger is properly stored
* deleting existed trigger from storage
Change-Id: I243326e84833a8cb22075fbf565573b62b205b4a
Signed-off-by: Cezary Zwolak <cezary.zwolak@intel.com>
Signed-off-by: Lukasz Kazmierczak <lukasz.kazmierczak@intel.com>
diff --git a/tests/src/mocks/trigger_factory_mock.hpp b/tests/src/mocks/trigger_factory_mock.hpp
index cf1be55..8249799 100644
--- a/tests/src/mocks/trigger_factory_mock.hpp
+++ b/tests/src/mocks/trigger_factory_mock.hpp
@@ -13,7 +13,7 @@
{
using namespace testing;
- ON_CALL(*this, make(_, _, _, _, _, _, _, _, _, _))
+ ON_CALL(*this, make(_, _, _, _, _, _, _, _, _, _, _))
.WillByDefault(WithArgs<1>(Invoke([](const std::string& name) {
return std::make_unique<NiceMock<TriggerMock>>(name);
})));
@@ -27,12 +27,14 @@
std::pair<sdbusplus::message::object_path, std::string>>& sensors),
const std::vector<std::string>& reportNames,
const TriggerThresholdParams& thresholdParams,
- interfaces::TriggerManager& triggerManager),
+ interfaces::TriggerManager& triggerManager,
+ interfaces::JsonStorage& triggerStorage),
(const, override));
auto& expectMake(
std::optional<std::reference_wrapper<const TriggerParams>> paramsOpt,
- const testing::Matcher<interfaces::TriggerManager&>& tm)
+ const testing::Matcher<interfaces::TriggerManager&>& tm,
+ const testing::Matcher<interfaces::JsonStorage&>& triggerStorage)
{
using namespace testing;
@@ -40,15 +42,16 @@
{
const TriggerParams& params = *paramsOpt;
return EXPECT_CALL(
- *this,
- make(_, params.name(), params.isDiscrete(),
- params.logToJournal(), params.logToRedfish(),
- params.updateReport(), params.sensors(),
- params.reportNames(), params.thresholdParams(), tm));
+ *this, make(_, params.name(), params.isDiscrete(),
+ params.logToJournal(), params.logToRedfish(),
+ params.updateReport(), params.sensors(),
+ params.reportNames(), params.thresholdParams(), tm,
+ triggerStorage));
}
else
{
- return EXPECT_CALL(*this, make(_, _, _, _, _, _, _, _, _, tm));
+ return EXPECT_CALL(
+ *this, make(_, _, _, _, _, _, _, _, _, tm, triggerStorage));
}
}
};
diff --git a/tests/src/params/trigger_params.hpp b/tests/src/params/trigger_params.hpp
index 95b19d2..08bed4b 100644
--- a/tests/src/params/trigger_params.hpp
+++ b/tests/src/params/trigger_params.hpp
@@ -42,13 +42,18 @@
return logToRedfishProperty;
}
+ TriggerParams& updateReport(bool updateReport)
+ {
+ updateReportProperty = updateReport;
+ return *this;
+ }
+
bool updateReport() const
{
return updateReportProperty;
}
- const std::vector<std::pair<sdbusplus::message::object_path, std::string>>&
- sensors() const
+ const TriggerSensors& sensors() const
{
return sensorsProperty;
}
@@ -74,12 +79,11 @@
bool discreteProperty = false;
bool logToJournalProperty = false;
bool logToRedfishProperty = false;
- bool updateReportProperty = false;
- std::vector<std::pair<sdbusplus::message::object_path, std::string>>
- sensorsProperty = {
- {sdbusplus::message::object_path(
- "/xyz/openbmc_project/sensors/temperature/BMC_Temp"),
- ""}};
+ bool updateReportProperty = true;
+ TriggerSensors sensorsProperty = {
+ {sdbusplus::message::object_path(
+ "/xyz/openbmc_project/sensors/temperature/BMC_Temp"),
+ ""}};
std::vector<std::string> reportNamesProperty = {"Report1"};
TriggerThresholdParams thresholdsProperty =
std::vector<numeric::ThresholdParam>{
diff --git a/tests/src/test_trigger.cpp b/tests/src/test_trigger.cpp
index b2b259c..f396589 100644
--- a/tests/src/test_trigger.cpp
+++ b/tests/src/test_trigger.cpp
@@ -1,5 +1,6 @@
#include "dbus_environment.hpp"
#include "helpers.hpp"
+#include "mocks/json_storage_mock.hpp"
#include "mocks/trigger_manager_mock.hpp"
#include "params/trigger_params.hpp"
#include "trigger.hpp"
@@ -8,6 +9,8 @@
using namespace testing;
using namespace std::literals::string_literals;
+static constexpr size_t expectedTriggerVersion = 0;
+
class TestTrigger : public Test
{
public:
@@ -15,18 +18,29 @@
std::unique_ptr<TriggerManagerMock> triggerManagerMockPtr =
std::make_unique<NiceMock<TriggerManagerMock>>();
+ testing::NiceMock<StorageMock> storageMock;
std::unique_ptr<Trigger> sut;
void SetUp() override
{
- sut = std::make_unique<Trigger>(
+ sut = makeTrigger(triggerParams);
+ }
+
+ std::unique_ptr<Trigger> makeTrigger(const TriggerParams& params)
+ {
+ return std::make_unique<Trigger>(
DbusEnvironment::getIoc(), DbusEnvironment::getObjServer(),
- triggerParams.name(), triggerParams.isDiscrete(),
- triggerParams.logToJournal(), triggerParams.logToRedfish(),
- triggerParams.updateReport(), triggerParams.sensors(),
- triggerParams.reportNames(), triggerParams.thresholdParams(),
+ params.name(), params.isDiscrete(), params.logToJournal(),
+ params.logToRedfish(), params.updateReport(), params.sensors(),
+ params.reportNames(), params.thresholdParams(),
std::vector<std::shared_ptr<interfaces::Threshold>>{},
- *triggerManagerMockPtr);
+ *triggerManagerMockPtr, storageMock);
+ }
+
+ static interfaces::JsonStorage::FilePath to_file_path(std::string name)
+ {
+ return interfaces::JsonStorage::FilePath(
+ std::to_string(std::hash<std::string>{}(name)));
}
template <class T>
@@ -48,6 +62,24 @@
return DbusEnvironment::waitForFuture(std::move(propertyFuture));
}
+ template <class T>
+ static boost::system::error_code setProperty(const std::string& path,
+ const std::string& property,
+ const T& newValue)
+ {
+ auto setPromise = std::promise<boost::system::error_code>();
+ auto setFuture = setPromise.get_future();
+
+ sdbusplus::asio::setProperty(
+ *DbusEnvironment::getBus(), DbusEnvironment::serviceName(), path,
+ Trigger::triggerIfaceName, property, std::move(newValue),
+ [setPromise =
+ std::move(setPromise)](boost::system::error_code ec) mutable {
+ setPromise.set_value(ec);
+ });
+ return DbusEnvironment::waitForFuture(std::move(setFuture));
+ }
+
boost::system::error_code deleteTrigger(const std::string& path)
{
std::promise<boost::system::error_code> methodPromise;
@@ -63,6 +95,7 @@
TEST_F(TestTrigger, checkIfPropertiesAreSet)
{
+ EXPECT_THAT(getProperty<bool>(sut->getPath(), "Persistent"), Eq(true));
EXPECT_THAT(getProperty<bool>(sut->getPath(), "Discrete"),
Eq(triggerParams.isDiscrete()));
EXPECT_THAT(getProperty<bool>(sut->getPath(), "LogToJournal"),
@@ -85,6 +118,7 @@
TEST_F(TestTrigger, deleteTrigger)
{
+ EXPECT_CALL(storageMock, remove(to_file_path(sut->getName())));
EXPECT_CALL(*triggerManagerMockPtr, removeTrigger(sut.get()));
auto ec = deleteTrigger(sut->getPath());
EXPECT_THAT(ec, Eq(boost::system::errc::success));
@@ -95,3 +129,125 @@
auto ec = deleteTrigger(Trigger::triggerDir + "NonExisting"s);
EXPECT_THAT(ec.value(), Eq(EBADR));
}
+
+TEST_F(TestTrigger, settingPersistencyToFalseRemovesReportFromStorage)
+{
+ EXPECT_CALL(storageMock, remove(to_file_path(sut->getName())));
+
+ bool persistent = false;
+ EXPECT_THAT(setProperty(sut->getPath(), "Persistent", persistent).value(),
+ Eq(boost::system::errc::success));
+ EXPECT_THAT(getProperty<bool>(sut->getPath(), "Persistent"),
+ Eq(persistent));
+}
+
+class TestTriggerErrors : public TestTrigger
+{
+ public:
+ void SetUp() override
+ {}
+
+ nlohmann::json storedConfiguration;
+};
+
+TEST_F(TestTriggerErrors, throwingExceptionDoesNotStoreTriggerReportNames)
+{
+ EXPECT_CALL(storageMock, store(_, _))
+ .WillOnce(Throw(std::runtime_error("Generic error!")));
+
+ sut = makeTrigger(triggerParams);
+
+ EXPECT_THAT(getProperty<bool>(sut->getPath(), "Persistent"), Eq(false));
+}
+
+TEST_F(TestTriggerErrors, creatingTriggerThrowsExceptionWhenNameIsInvalid)
+{
+ EXPECT_CALL(storageMock, store(_, _)).Times(0);
+
+ EXPECT_THROW(makeTrigger(triggerParams.name("inv?lidName")),
+ sdbusplus::exception::SdBusError);
+}
+
+class TestTriggerStore : public TestTrigger
+{
+ public:
+ void SetUp() override
+ {
+ ON_CALL(storageMock, store(_, _))
+ .WillByDefault(SaveArg<1>(&storedConfiguration));
+
+ sut = makeTrigger(triggerParams);
+ }
+
+ nlohmann::json storedConfiguration;
+};
+
+TEST_F(TestTriggerStore, settingPersistencyToTrueStoresTriggerVersion)
+{
+ ASSERT_THAT(storedConfiguration.at("Version"), Eq(expectedTriggerVersion));
+}
+
+TEST_F(TestTriggerStore, settingPersistencyToTrueStoresTriggerName)
+{
+ ASSERT_THAT(storedConfiguration.at("Name"), Eq(triggerParams.name()));
+}
+
+TEST_F(TestTriggerStore, settingPersistencyToTrueStoresTriggerIsDiscrete)
+{
+ ASSERT_THAT(storedConfiguration.at("IsDiscrete"),
+ Eq(triggerParams.isDiscrete()));
+}
+
+TEST_F(TestTriggerStore, settingPersistencyToTrueStoresTriggerLogToJournal)
+{
+ ASSERT_THAT(storedConfiguration.at("LogToJournal"),
+ Eq(triggerParams.logToRedfish()));
+}
+
+TEST_F(TestTriggerStore, settingPersistencyToTrueStoresTriggerLogToRedfish)
+{
+ ASSERT_THAT(storedConfiguration.at("LogToRedfish"),
+ Eq(triggerParams.logToRedfish()));
+}
+
+TEST_F(TestTriggerStore, settingPersistencyToTrueStoresTriggerUpdateReport)
+{
+ ASSERT_THAT(storedConfiguration.at("UpdateReport"),
+ Eq(triggerParams.updateReport()));
+}
+
+TEST_F(TestTriggerStore, settingPersistencyToTrueStoresTriggerReportNames)
+{
+ ASSERT_THAT(storedConfiguration.at("ReportNames"),
+ Eq(triggerParams.reportNames()));
+}
+
+TEST_F(TestTriggerStore, settingPersistencyToTrueStoresTriggerSensors)
+{
+ nlohmann::json expectedItem;
+ expectedItem["sensorPath"] =
+ "/xyz/openbmc_project/sensors/temperature/BMC_Temp";
+ expectedItem["sensorMetadata"] = "";
+
+ ASSERT_THAT(storedConfiguration.at("Sensors"), ElementsAre(expectedItem));
+}
+
+TEST_F(TestTriggerStore, settingPersistencyToTrueStoresTriggerThresholdParams)
+{
+ ASSERT_THAT(storedConfiguration.at("ThresholdParamsDiscriminator"), Eq(0));
+
+ nlohmann::json expectedItem0;
+ expectedItem0["type"] = 0;
+ expectedItem0["dwellTime"] = 10;
+ expectedItem0["direction"] = 1;
+ expectedItem0["thresholdValue"] = 0.0;
+
+ nlohmann::json expectedItem1;
+ expectedItem1["type"] = 3;
+ expectedItem1["dwellTime"] = 10;
+ expectedItem1["direction"] = 2;
+ expectedItem1["thresholdValue"] = 90.0;
+
+ ASSERT_THAT(storedConfiguration.at("ThresholdParams"),
+ ElementsAre(expectedItem0, expectedItem1));
+}
diff --git a/tests/src/test_trigger_manager.cpp b/tests/src/test_trigger_manager.cpp
index 8754446..1bc4f1b 100644
--- a/tests/src/test_trigger_manager.cpp
+++ b/tests/src/test_trigger_manager.cpp
@@ -1,5 +1,6 @@
#include "dbus_environment.hpp"
#include "helpers.hpp"
+#include "mocks/json_storage_mock.hpp"
#include "mocks/trigger_factory_mock.hpp"
#include "mocks/trigger_mock.hpp"
#include "params/trigger_params.hpp"
@@ -29,6 +30,9 @@
}
TriggerParams triggerParams;
+ std::unique_ptr<StorageMock> storageMockPtr =
+ std::make_unique<NiceMock<StorageMock>>();
+ StorageMock& storageMock = *storageMockPtr;
std::unique_ptr<TriggerFactoryMock> triggerFactoryMockPtr =
std::make_unique<NiceMock<TriggerFactoryMock>>();
TriggerFactoryMock& triggerFactoryMock = *triggerFactoryMockPtr;
@@ -36,14 +40,14 @@
std::make_unique<NiceMock<TriggerMock>>(triggerParams.name());
TriggerMock& triggerMock = *triggerMockPtr;
std::unique_ptr<TriggerManager> sut = std::make_unique<TriggerManager>(
- std::move(triggerFactoryMockPtr),
+ std::move(triggerFactoryMockPtr), std::move(storageMockPtr),
std::move(DbusEnvironment::getObjServer()));
MockFunction<void(std::string)> checkPoint;
};
TEST_F(TestTriggerManager, addTrigger)
{
- triggerFactoryMock.expectMake(triggerParams, Ref(*sut))
+ triggerFactoryMock.expectMake(triggerParams, Ref(*sut), Ref(storageMock))
.WillOnce(Return(ByMove(std::move(triggerMockPtr))));
auto [ec, path] = addTrigger(triggerParams);
@@ -83,7 +87,7 @@
TEST_F(TestTriggerManager, DISABLED_failToAddTriggerTwice)
{
- triggerFactoryMock.expectMake(triggerParams, Ref(*sut))
+ triggerFactoryMock.expectMake(triggerParams, Ref(*sut), Ref(storageMock))
.WillOnce(Return(ByMove(std::move(triggerMockPtr))));
addTrigger(triggerParams);
@@ -95,7 +99,7 @@
TEST_F(TestTriggerManager, DISABLED_failToAddTriggerWhenMaxTriggerIsReached)
{
- triggerFactoryMock.expectMake(std::nullopt, Ref(*sut))
+ triggerFactoryMock.expectMake(std::nullopt, Ref(*sut), Ref(storageMock))
.Times(TriggerManager::maxTriggers);
for (size_t i = 0; i < TriggerManager::maxTriggers; i++)
@@ -117,7 +121,8 @@
{
{
InSequence seq;
- triggerFactoryMock.expectMake(triggerParams, Ref(*sut))
+ triggerFactoryMock
+ .expectMake(triggerParams, Ref(*sut), Ref(storageMock))
.WillOnce(Return(ByMove(std::move(triggerMockPtr))));
EXPECT_CALL(triggerMock, Die());
EXPECT_CALL(checkPoint, Call("end"));
@@ -144,7 +149,8 @@
{
{
InSequence seq;
- triggerFactoryMock.expectMake(triggerParams, Ref(*sut))
+ triggerFactoryMock
+ .expectMake(triggerParams, Ref(*sut), Ref(storageMock))
.WillOnce(Return(ByMove(std::move(triggerMockPtr))));
EXPECT_CALL(triggerMock, Die());
EXPECT_CALL(checkPoint, Call("end"));