Add Id to Trigger
Currently, Trigger is using Name as unique identifier. By adding Id, we
can be compliant with redfish specification:
- Id will be used as unique identifier
- Name will be used as human readable, non-unique name
AddTrigger dbus method is now requiring both id and name. Each of them
can be passed as empty string and the service will fill them with
correct values. If only id is an empty string, name will be used to
generate its value.
Dbus object path and persistent storage filename are now be based on id,
instead of name.
Added validation for AddTrigger:
- correct characters in id
- max id length
Added Name property for Trigger object, which can be modified from dbus.
Testing done:
- Unit test added and passing
- Trigger was added using dbus, without errors
- Id generation is working properly
- Name property is accessible and writable from dbus
Signed-off-by: Szymon Dompke <szymon.dompke@intel.com>
Change-Id: Ibb553586eaf51943044c93a35bc0725e6ef67ce9
diff --git a/tests/src/test_trigger.cpp b/tests/src/test_trigger.cpp
index 3a9004f..a074e7a 100644
--- a/tests/src/test_trigger.cpp
+++ b/tests/src/test_trigger.cpp
@@ -4,8 +4,8 @@
#include "mocks/trigger_manager_mock.hpp"
#include "params/trigger_params.hpp"
#include "trigger.hpp"
+#include "trigger_manager.hpp"
#include "utils/conversion_trigger.hpp"
-#include "utils/set_exception.hpp"
#include "utils/transform.hpp"
#include "utils/tstring.hpp"
@@ -22,7 +22,8 @@
TriggerParams triggerParams;
TriggerParams triggerDiscreteParams =
TriggerParams()
- .name("Trigger2")
+ .id("DiscreteTrigger")
+ .name("My Discrete Trigger")
.thresholdParams(std::vector<discrete::LabeledThresholdParam>{
discrete::LabeledThresholdParam{
"userId", discrete::Severity::warning,
@@ -55,8 +56,8 @@
{
return std::make_unique<Trigger>(
DbusEnvironment::getIoc(), DbusEnvironment::getObjServer(),
- params.name(), params.triggerActions(), params.reportNames(),
- params.sensors(), params.thresholdParams(),
+ params.id(), params.name(), params.triggerActions(),
+ params.reportNames(), params.sensors(), params.thresholdParams(),
std::vector<std::shared_ptr<interfaces::Threshold>>{},
*triggerManagerMockPtr, storageMock);
}
@@ -70,20 +71,8 @@
template <class T>
static T getProperty(const std::string& path, const std::string& property)
{
- auto propertyPromise = std::promise<T>();
- auto propertyFuture = propertyPromise.get_future();
- sdbusplus::asio::getProperty<T>(
- *DbusEnvironment::getBus(), DbusEnvironment::serviceName(), path,
- Trigger::triggerIfaceName, property,
- [&propertyPromise](const boost::system::error_code& ec, T t) {
- if (ec)
- {
- utils::setException(propertyPromise, "GetProperty failed");
- return;
- }
- propertyPromise.set_value(t);
- });
- return DbusEnvironment::waitForFuture(std::move(propertyFuture));
+ return DbusEnvironment::getProperty<T>(path, Trigger::triggerIfaceName,
+ property);
}
template <class T>
@@ -91,17 +80,8 @@
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));
+ return DbusEnvironment::setProperty<T>(path, Trigger::triggerIfaceName,
+ property, newValue);
}
boost::system::error_code deleteTrigger(const std::string& path)
@@ -119,6 +99,8 @@
TEST_F(TestTrigger, checkIfPropertiesAreSet)
{
+ EXPECT_THAT(getProperty<std::string>(sut->getPath(), "Name"),
+ Eq(triggerParams.name()));
EXPECT_THAT(getProperty<bool>(sut->getPath(), "Persistent"), Eq(true));
EXPECT_THAT(
getProperty<std::vector<std::string>>(sut->getPath(), "TriggerActions"),
@@ -134,6 +116,14 @@
triggerParams.thresholdParams())));
}
+TEST_F(TestTrigger, setPropertyNameToCorrectValue)
+{
+ std::string name = "custom name 1234 %^#5";
+ EXPECT_THAT(setProperty(sut->getPath(), "Name", name),
+ Eq(boost::system::errc::success));
+ EXPECT_THAT(getProperty<std::string>(sut->getPath(), "Name"), Eq(name));
+}
+
TEST_F(TestTrigger, checkIfNumericCoversionsAreGood)
{
const auto& labeledParamsBase =
@@ -188,7 +178,7 @@
TEST_F(TestTrigger, deleteTrigger)
{
- EXPECT_CALL(storageMock, remove(to_file_path(sut->getName())));
+ EXPECT_CALL(storageMock, remove(to_file_path(sut->getId())));
EXPECT_CALL(*triggerManagerMockPtr, removeTrigger(sut.get()));
auto ec = deleteTrigger(sut->getPath());
EXPECT_THAT(ec, Eq(boost::system::errc::success));
@@ -202,7 +192,7 @@
TEST_F(TestTrigger, settingPersistencyToFalseRemovesTriggerFromStorage)
{
- EXPECT_CALL(storageMock, remove(to_file_path(sut->getName())));
+ EXPECT_CALL(storageMock, remove(to_file_path(sut->getId())));
bool persistent = false;
EXPECT_THAT(setProperty(sut->getPath(), "Persistent", persistent),
@@ -230,11 +220,11 @@
EXPECT_THAT(getProperty<bool>(sut->getPath(), "Persistent"), Eq(false));
}
-TEST_F(TestTriggerErrors, creatingTriggerThrowsExceptionWhenNameIsInvalid)
+TEST_F(TestTriggerErrors, creatingTriggerThrowsExceptionWhenIdIsInvalid)
{
EXPECT_CALL(storageMock, store(_, _)).Times(0);
- EXPECT_THROW(makeTrigger(triggerParams.name("inv?lidName")),
+ EXPECT_THROW(makeTrigger(triggerParams.id("inv?lidId")),
sdbusplus::exception::SdBusError);
}
@@ -262,6 +252,11 @@
ASSERT_THAT(storedConfiguration.at("Version"), Eq(expectedTriggerVersion));
}
+TEST_F(TestTriggerStore, settingPersistencyToTrueStoresTriggerId)
+{
+ ASSERT_THAT(storedConfiguration.at("Id"), Eq(triggerParams.id()));
+}
+
TEST_F(TestTriggerStore, settingPersistencyToTrueStoresTriggerName)
{
ASSERT_THAT(storedConfiguration.at("Name"), Eq(triggerParams.name()));