Use proper dbus path when possible.
Following methods and properties were updated to use full dbus path,
instead of internal telemetry id:
- TriggerManager.AddTrigger() - 'reportIds' arg
- Trigger.ReportIds - renamed to 'Reports'
- Report.TriggerIds - renamed to 'Triggers'
Testing done:
- UTs were updated and are passing.
Signed-off-by: Szymon Dompke <szymon.dompke@intel.com>
Change-Id: I78d812d38289fac575d25b48503cc8b9c6f736fe
diff --git a/meson.build b/meson.build
index 63e9f04..e66248e 100644
--- a/meson.build
+++ b/meson.build
@@ -90,6 +90,7 @@
'src/types/readings.cpp',
'src/types/report_types.cpp',
'src/utils/conversion_trigger.cpp',
+ 'src/utils/dbus_path_utils.cpp',
'src/utils/generate_id.cpp',
'src/utils/messanger_service.cpp',
],
diff --git a/src/report.cpp b/src/report.cpp
index be98c6d..cf2b389 100644
--- a/src/report.cpp
+++ b/src/report.cpp
@@ -6,6 +6,7 @@
#include "report_manager.hpp"
#include "utils/clock.hpp"
#include "utils/contains.hpp"
+#include "utils/dbus_path_utils.hpp"
#include "utils/ensure.hpp"
#include "utils/transform.hpp"
@@ -30,6 +31,7 @@
const bool enabledIn, std::unique_ptr<interfaces::Clock> clock,
Readings readingsIn) :
id(reportId),
+ path(utils::pathAppend(utils::constants::reportDirPath, id)),
name(reportName), reportingType(reportingTypeIn), interval(intervalIn),
reportActions(reportActionsIn.begin(), reportActionsIn.end()),
sensorCount(getSensorCount(metricsIn)),
@@ -104,7 +106,7 @@
if (triggerIds.size() != oldSize)
{
- reportIface->signal_property("TriggerIds");
+ reportIface->signal_property("Triggers");
}
});
@@ -391,10 +393,13 @@
},
[this](const auto&) { return utils::enumToString(reportUpdates); });
dbusIface->register_property_r(
- "TriggerIds", std::vector<std::string>{},
+ "Triggers", std::vector<sdbusplus::message::object_path>{},
sdbusplus::vtable::property_::emits_change, [this](const auto&) {
- return std::vector<std::string>(triggerIds.begin(),
- triggerIds.end());
+ return utils::transform<std::vector>(
+ triggerIds, [](const auto& triggerId) {
+ return utils::pathAppend(utils::constants::triggerDirPath,
+ triggerId);
+ });
});
dbusIface->register_method("Update", [this] {
if (reportingType == ReportingType::onRequest)
diff --git a/src/report.hpp b/src/report.hpp
index 7609297..74c979b 100644
--- a/src/report.hpp
+++ b/src/report.hpp
@@ -15,6 +15,7 @@
#include "types/report_updates.hpp"
#include "types/reporting_type.hpp"
#include "utils/circular_vector.hpp"
+#include "utils/dbus_path_utils.hpp"
#include "utils/ensure.hpp"
#include "utils/messanger.hpp"
@@ -78,7 +79,7 @@
std::string getPath() const override
{
- return reportDir + id;
+ return path.str;
}
void metricUpdated() override;
@@ -113,6 +114,7 @@
std::vector<ErrorMessage> verify() const;
std::string id;
+ const sdbusplus::message::object_path path;
std::string name;
ReportingType reportingType;
Milliseconds interval;
@@ -144,8 +146,6 @@
public:
static constexpr const char* reportIfaceName =
"xyz.openbmc_project.Telemetry.Report";
- static constexpr const char* reportDir =
- "/xyz/openbmc_project/Telemetry/Reports/";
static constexpr const char* deleteIfaceName =
"xyz.openbmc_project.Object.Delete";
static constexpr size_t reportVersion = 6;
diff --git a/src/report_manager.hpp b/src/report_manager.hpp
index d5653b2..88dc3e0 100644
--- a/src/report_manager.hpp
+++ b/src/report_manager.hpp
@@ -6,6 +6,7 @@
#include "interfaces/report_manager.hpp"
#include "interfaces/trigger_manager.hpp"
#include "report.hpp"
+#include "utils/dbus_path_utils.hpp"
#include <systemd/sd-bus-protocol.h>
@@ -65,7 +66,7 @@
static constexpr size_t maxNumberMetrics{TELEMETRY_MAX_READING_PARAMS};
static constexpr size_t maxReportIdLength{
TELEMETRY_MAX_DBUS_PATH_LENGTH -
- std::string_view(Report::reportDir).length()};
+ utils::constants::reportDirStr.length()};
static constexpr Milliseconds minInterval{TELEMETRY_MIN_INTERVAL};
static constexpr size_t maxAppendLimit{TELEMETRY_MAX_APPEND_LIMIT};
static constexpr const char* reportManagerIfaceName =
diff --git a/src/trigger.cpp b/src/trigger.cpp
index ab48281..c0935c3 100644
--- a/src/trigger.cpp
+++ b/src/trigger.cpp
@@ -7,6 +7,7 @@
#include "types/trigger_types.hpp"
#include "utils/contains.hpp"
#include "utils/conversion_trigger.hpp"
+#include "utils/dbus_path_utils.hpp"
#include "utils/transform.hpp"
#include <phosphor-logging/log.hpp>
@@ -22,9 +23,9 @@
interfaces::JsonStorage& triggerStorageIn,
const interfaces::TriggerFactory& triggerFactory, Sensors sensorsIn) :
id(std::move(idIn)),
+ path(utils::pathAppend(utils::constants::triggerDirPath, *id)),
name(nameIn), triggerActions(std::move(triggerActionsIn)),
- path(triggerDir + *id), reportIds(std::move(reportIdsIn)),
- thresholds(std::move(thresholdsIn)),
+ reportIds(std::move(reportIdsIn)), thresholds(std::move(thresholdsIn)),
fileName(std::to_string(std::hash<std::string>{}(*id))),
triggerStorage(triggerStorageIn), sensors(std::move(sensorsIn)),
messanger(ioc)
@@ -104,17 +105,27 @@
});
dbusIface.register_property_rw(
- "ReportNames", *reportIds,
+ "Reports", std::vector<sdbusplus::message::object_path>(),
sdbusplus::vtable::property_::emits_change,
[this](auto newVal, auto& oldVal) {
- TriggerManager::verifyReportIds(newVal);
- *reportIds = newVal;
+ auto newReportIds = utils::transform<std::vector>(
+ newVal, [](const auto& path) {
+ return utils::reportPathToId(path);
+ });
+ TriggerManager::verifyReportIds(newReportIds);
+ *reportIds = newReportIds;
messanger.send(messages::TriggerPresenceChangedInd{
messages::Presence::Exist, *id, *reportIds});
oldVal = std::move(newVal);
return 1;
},
- [this](const auto&) { return *reportIds; });
+ [this](const auto&) {
+ return utils::transform<std::vector>(
+ *reportIds, [](const auto& id) {
+ return utils::pathAppend(
+ utils::constants::reportDirPath, id);
+ });
+ });
dbusIface.register_property_r(
"Discrete", isDiscreate(), sdbusplus::vtable::property_::const_,
diff --git a/src/trigger.hpp b/src/trigger.hpp
index 832a277..e636334 100644
--- a/src/trigger.hpp
+++ b/src/trigger.hpp
@@ -10,6 +10,7 @@
#include <boost/asio/io_context.hpp>
#include <sdbusplus/asio/object_server.hpp>
+#include <sdbusplus/message.hpp>
#include <memory>
@@ -39,7 +40,7 @@
std::string getPath() const override
{
- return path;
+ return path.str;
}
bool storeConfiguration() const;
@@ -50,9 +51,9 @@
bool isDiscreate() const;
const TriggerId id;
+ const sdbusplus::message::object_path path;
std::string name;
std::vector<TriggerAction> triggerActions;
- std::string path;
bool persistent = false;
std::shared_ptr<std::vector<std::string>> reportIds;
std::unique_ptr<sdbusplus::asio::dbus_interface> deleteIface;
@@ -67,8 +68,6 @@
public:
static constexpr const char* triggerIfaceName =
"xyz.openbmc_project.Telemetry.Trigger";
- static constexpr const char* triggerDir =
- "/xyz/openbmc_project/Telemetry/Triggers/";
static constexpr const char* deleteIfaceName =
"xyz.openbmc_project.Object.Delete";
static constexpr size_t triggerVersion = 2;
diff --git a/src/trigger_manager.cpp b/src/trigger_manager.cpp
index 3c06b11..df8dea9 100644
--- a/src/trigger_manager.cpp
+++ b/src/trigger_manager.cpp
@@ -3,6 +3,7 @@
#include "trigger.hpp"
#include "types/trigger_types.hpp"
#include "utils/conversion_trigger.hpp"
+#include "utils/dbus_path_utils.hpp"
#include "utils/generate_id.hpp"
#include "utils/transform.hpp"
@@ -23,12 +24,13 @@
triggerManagerPath, triggerManagerIfaceName, [this](auto& iface) {
iface.register_method(
"AddTrigger",
- [this](boost::asio::yield_context& yield, const std::string& id,
- const std::string& name,
- const std::vector<std::string>& triggerActions,
- const SensorsInfo& sensors,
- const std::vector<std::string>& reportIds,
- const TriggerThresholdParamsExt& thresholds) {
+ [this](
+ boost::asio::yield_context& yield, const std::string& id,
+ const std::string& name,
+ const std::vector<std::string>& triggerActions,
+ const SensorsInfo& sensors,
+ const std::vector<sdbusplus::message::object_path>& reports,
+ const TriggerThresholdParamsExt& thresholds) {
LabeledTriggerThresholdParams
labeledTriggerThresholdParams = std::visit(
utils::ToLabeledThresholdParamConversion(),
@@ -37,6 +39,11 @@
std::vector<LabeledSensorInfo> labeledSensorsInfo =
triggerFactory->getLabeledSensorsInfo(yield, sensors);
+ auto reportIds = utils::transform<std::vector>(
+ reports, [](const auto& item) {
+ return utils::reportPathToId(item);
+ });
+
return addTrigger(id, name, triggerActions,
labeledSensorsInfo, reportIds,
labeledTriggerThresholdParams)
diff --git a/src/trigger_manager.hpp b/src/trigger_manager.hpp
index bbdba8e..4db6941 100644
--- a/src/trigger_manager.hpp
+++ b/src/trigger_manager.hpp
@@ -4,6 +4,7 @@
#include "interfaces/trigger_factory.hpp"
#include "interfaces/trigger_manager.hpp"
#include "trigger.hpp"
+#include "utils/dbus_path_utils.hpp"
#include <sdbusplus/asio/object_server.hpp>
@@ -49,7 +50,7 @@
static constexpr size_t maxTriggers{TELEMETRY_MAX_TRIGGERS};
static constexpr size_t maxTriggerIdLength{
TELEMETRY_MAX_DBUS_PATH_LENGTH -
- std::string_view(Trigger::triggerDir).length()};
+ utils::constants::triggerDirStr.length()};
static constexpr const char* triggerManagerIfaceName =
"xyz.openbmc_project.Telemetry.TriggerManager";
static constexpr const char* triggerManagerPath =
diff --git a/src/utils/dbus_path_utils.cpp b/src/utils/dbus_path_utils.cpp
new file mode 100644
index 0000000..9d37cfc
--- /dev/null
+++ b/src/utils/dbus_path_utils.cpp
@@ -0,0 +1,50 @@
+#include "utils/dbus_path_utils.hpp"
+
+namespace utils
+{
+sdbusplus::message::object_path pathAppend(sdbusplus::message::object_path path,
+ const std::string& appended)
+{
+ if (appended.starts_with('/') || !isValidDbusPath(appended))
+ {
+ throw sdbusplus::exception::SdBusError(
+ static_cast<int>(std::errc::invalid_argument),
+ "Invalid appended string");
+ }
+
+ size_t pos_start = 0;
+ size_t pos_end = 0;
+ while ((pos_end = appended.find('/', pos_start)) != std::string::npos)
+ {
+ if (pos_start == pos_end)
+ {
+ throw sdbusplus::exception::SdBusError(
+ static_cast<int>(std::errc::invalid_argument),
+ "Invalid appended string");
+ }
+ path /= std::string_view(appended.begin() + pos_start,
+ appended.begin() + pos_end);
+ pos_start = pos_end + 1;
+ }
+ path /= std::string_view(appended.begin() + pos_start, appended.end());
+ return path;
+}
+
+std::string reportPathToId(const sdbusplus::message::object_path& path)
+{
+ if (path.str.starts_with(constants::reportDirStr))
+ {
+ auto id = path.str.substr(constants::reportDirStr.length());
+ if (std::cmp_greater(std::count(id.begin(), id.end(), '/'),
+ constants::maxPrefixesInId))
+ {
+ throw sdbusplus::exception::SdBusError(
+ static_cast<int>(std::errc::invalid_argument),
+ "Too many prefixes in id");
+ }
+ return id;
+ }
+ throw sdbusplus::exception::SdBusError(
+ static_cast<int>(std::errc::invalid_argument), "Invalid path prefix");
+}
+} // namespace utils
diff --git a/src/utils/dbus_path_utils.hpp b/src/utils/dbus_path_utils.hpp
new file mode 100644
index 0000000..a1b5709
--- /dev/null
+++ b/src/utils/dbus_path_utils.hpp
@@ -0,0 +1,41 @@
+#pragma once
+
+#include <sdbusplus/message.hpp>
+
+#include <algorithm>
+#include <ranges>
+#include <string_view>
+
+namespace utils
+{
+
+namespace constants
+{
+constexpr std::string_view triggerDirStr =
+ "/xyz/openbmc_project/Telemetry/Triggers/";
+constexpr std::string_view reportDirStr =
+ "/xyz/openbmc_project/Telemetry/Reports/";
+
+constexpr std::string_view allowedCharactersInPath =
+ "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_/";
+constexpr size_t maxPrefixesInId = 1;
+
+const sdbusplus::message::object_path triggerDirPath =
+ sdbusplus::message::object_path(std::string(triggerDirStr));
+const sdbusplus::message::object_path reportDirPath =
+ sdbusplus::message::object_path(std::string(reportDirStr));
+} // namespace constants
+
+inline bool isValidDbusPath(const std::string& path)
+{
+ return (path.find_first_not_of(constants::allowedCharactersInPath) ==
+ std::string::npos) &&
+ !path.ends_with('/');
+}
+
+sdbusplus::message::object_path pathAppend(sdbusplus::message::object_path path,
+ const std::string& appended);
+
+std::string reportPathToId(const sdbusplus::message::object_path& path);
+
+} // namespace utils
diff --git a/src/utils/generate_id.cpp b/src/utils/generate_id.cpp
index c648481..42c9155 100644
--- a/src/utils/generate_id.cpp
+++ b/src/utils/generate_id.cpp
@@ -1,5 +1,7 @@
#include "utils/generate_id.hpp"
+#include "utils/dbus_path_utils.hpp"
+
#include <sdbusplus/exception.hpp>
#include <algorithm>
@@ -10,9 +12,6 @@
namespace details
{
-static constexpr std::string_view allowedCharactersInId =
- "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_/";
-
size_t countDigits(size_t value)
{
size_t result = 1;
@@ -55,12 +54,13 @@
std::string strippedId(name);
strippedId.erase(
- std::remove_if(strippedId.begin(), strippedId.end(),
- [](char c) {
- return c == '/' ||
- details::allowedCharactersInId.find(c) ==
- std::string_view::npos;
- }),
+ std::remove_if(
+ strippedId.begin(), strippedId.end(),
+ [](char c) {
+ return c == '/' ||
+ utils::constants::allowedCharactersInPath.find(c) ==
+ std::string_view::npos;
+ }),
strippedId.end());
strippedId = std::string(id) + strippedId;
@@ -92,7 +92,7 @@
void verifyIdCharacters(std::string_view id)
{
- if (id.find_first_not_of(details::allowedCharactersInId) !=
+ if (id.find_first_not_of(utils::constants::allowedCharactersInPath) !=
std::string::npos)
{
throw sdbusplus::exception::SdBusError(
diff --git a/tests/meson.build b/tests/meson.build
index 70de004..25d5085 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -30,6 +30,7 @@
'../src/types/readings.cpp',
'../src/types/report_types.cpp',
'../src/utils/conversion_trigger.cpp',
+ '../src/utils/dbus_path_utils.cpp',
'../src/utils/generate_id.cpp',
'../src/utils/messanger_service.cpp',
'src/dbus_environment.cpp',
@@ -43,6 +44,7 @@
'src/test_metric.cpp',
'src/test_numeric_threshold.cpp',
'src/test_on_change_threshold.cpp',
+ 'src/test_path_append.cpp',
'src/test_persistent_json_storage.cpp',
'src/test_report.cpp',
'src/test_report_manager.cpp',
diff --git a/tests/src/dbus_environment.cpp b/tests/src/dbus_environment.cpp
index 07e0aff..a0f454e 100644
--- a/tests/src/dbus_environment.cpp
+++ b/tests/src/dbus_environment.cpp
@@ -1,5 +1,7 @@
#include "dbus_environment.hpp"
+#include "helpers.hpp"
+
#include <future>
#include <thread>
diff --git a/tests/src/helpers.hpp b/tests/src/helpers.hpp
index 8ae0b7f..1c4905c 100644
--- a/tests/src/helpers.hpp
+++ b/tests/src/helpers.hpp
@@ -5,3 +5,4 @@
#include "helpers/labeled_tuple_helpers.hpp"
#include "helpers/metric_value_helpers.hpp"
#include "helpers/types/duration_types_helpers.hpp"
+#include "helpers/types/object_path_helpers.hpp"
diff --git a/tests/src/helpers/types/object_path_helpers.hpp b/tests/src/helpers/types/object_path_helpers.hpp
new file mode 100644
index 0000000..799a402
--- /dev/null
+++ b/tests/src/helpers/types/object_path_helpers.hpp
@@ -0,0 +1,12 @@
+#pragma once
+#include <sdbusplus/message.hpp>
+
+#include <gmock/gmock.h>
+
+namespace sdbusplus::message::details
+{
+inline void PrintTo(const string_path_wrapper& path, std::ostream* os)
+{
+ *os << path.str;
+}
+} // namespace sdbusplus::message::details
diff --git a/tests/src/main.cpp b/tests/src/main.cpp
index 6e29319..39508c8 100644
--- a/tests/src/main.cpp
+++ b/tests/src/main.cpp
@@ -1,4 +1,5 @@
#include "dbus_environment.hpp"
+#include "helpers.hpp"
#include <gmock/gmock.h>
diff --git a/tests/src/params/trigger_params.hpp b/tests/src/params/trigger_params.hpp
index b17ea86..65f423e 100644
--- a/tests/src/params/trigger_params.hpp
+++ b/tests/src/params/trigger_params.hpp
@@ -1,16 +1,29 @@
#pragma once
+#include "report.hpp"
#include "types/duration_types.hpp"
#include "types/trigger_types.hpp"
+#include "utils/dbus_path_utils.hpp"
+#include "utils/transform.hpp"
#include <sdbusplus/message.hpp>
#include <chrono>
#include <utility>
+using sdbusplus::message::object_path;
+
class TriggerParams
{
public:
+ TriggerParams()
+ {
+ reportsProperty =
+ utils::transform(reportIdsProperty, [](const auto& id) {
+ return utils::pathAppend(utils::constants::reportDirPath, id);
+ });
+ }
+
TriggerParams& id(std::string val)
{
idProperty = std::move(val);
@@ -57,6 +70,21 @@
TriggerParams& reportIds(std::vector<std::string> val)
{
reportIdsProperty = std::move(val);
+ reportsProperty = utils::transform<std::vector>(
+ reportIdsProperty, [](const auto& id) {
+ return utils::pathAppend(utils::constants::reportDirPath, id);
+ });
+ return *this;
+ }
+
+ const std::vector<object_path>& reports() const
+ {
+ return reportsProperty;
+ }
+
+ TriggerParams& reports(std::vector<object_path> val)
+ {
+ reportsProperty = std::move(val);
return *this;
}
@@ -79,7 +107,9 @@
std::vector<LabeledSensorInfo> labeledSensorsProperty = {
{"service1", "/xyz/openbmc_project/sensors/temperature/BMC_Temp",
"metadata1"}};
- std::vector<std::string> reportIdsProperty = {"Report1"};
+ std::vector<std::string> reportIdsProperty = {"Report1",
+ "Prefixed/Report2"};
+ std::vector<object_path> reportsProperty;
LabeledTriggerThresholdParams labeledThresholdsProperty =
std::vector<numeric::LabeledThresholdParam>{
numeric::LabeledThresholdParam{numeric::Type::lowerCritical,
diff --git a/tests/src/stubs/dbus_sensor_object.cpp b/tests/src/stubs/dbus_sensor_object.cpp
index 9d6f185..b73478e 100644
--- a/tests/src/stubs/dbus_sensor_object.cpp
+++ b/tests/src/stubs/dbus_sensor_object.cpp
@@ -1,5 +1,7 @@
#include "dbus_sensor_object.hpp"
+#include "helpers.hpp"
+
#include <boost/asio.hpp>
#include <sdbusplus/asio/connection.hpp>
#include <sdbusplus/asio/object_server.hpp>
diff --git a/tests/src/test_conversion.cpp b/tests/src/test_conversion.cpp
index 2b7249c..6a69e39 100644
--- a/tests/src/test_conversion.cpp
+++ b/tests/src/test_conversion.cpp
@@ -1,3 +1,4 @@
+#include "helpers.hpp"
#include "utils/conversion.hpp"
#include <gmock/gmock.h>
diff --git a/tests/src/test_ensure.cpp b/tests/src/test_ensure.cpp
index b604538..d252bc3 100644
--- a/tests/src/test_ensure.cpp
+++ b/tests/src/test_ensure.cpp
@@ -1,3 +1,4 @@
+#include "helpers.hpp"
#include "utils/ensure.hpp"
#include <gmock/gmock.h>
diff --git a/tests/src/test_generate_id.cpp b/tests/src/test_generate_id.cpp
index 412fc9b..dc534dd 100644
--- a/tests/src/test_generate_id.cpp
+++ b/tests/src/test_generate_id.cpp
@@ -1,3 +1,4 @@
+#include "helpers.hpp"
#include "utils/generate_id.hpp"
#include <sdbusplus/exception.hpp>
diff --git a/tests/src/test_path_append.cpp b/tests/src/test_path_append.cpp
new file mode 100644
index 0000000..f667ef2
--- /dev/null
+++ b/tests/src/test_path_append.cpp
@@ -0,0 +1,59 @@
+#include "helpers.hpp"
+#include "utils/dbus_path_utils.hpp"
+
+#include <sdbusplus/exception.hpp>
+
+#include <gmock/gmock.h>
+
+using namespace testing;
+using sdbusplus::message::object_path;
+using utils::pathAppend;
+
+class TestPathAppend :
+ public Test,
+ public WithParamInterface<std::tuple<object_path, std::string, object_path>>
+{};
+
+INSTANTIATE_TEST_SUITE_P(
+ _, TestPathAppend,
+ Values(std::make_tuple(object_path("/Base/Path"), "one",
+ object_path("/Base/Path/one")),
+ std::make_tuple(object_path("/Base/Path"), "one/two",
+ object_path("/Base/Path/one/two")),
+ std::make_tuple(object_path("/Base/Path"), "one/two/foobar",
+ object_path("/Base/Path/one/two/foobar")),
+ std::make_tuple(object_path("/Base/Path/"), "one",
+ object_path("/Base/Path/one")),
+ std::make_tuple(object_path("/Base/Path/"), "one/two",
+ object_path("/Base/Path/one/two")),
+ std::make_tuple(object_path("/Base/Path/"), "one/two/foobar",
+ object_path("/Base/Path/one/two/foobar")),
+ std::make_tuple(object_path("/Base/Path"), "",
+ object_path("/Base/Path/"))));
+
+TEST_P(TestPathAppend, pathAppendsCorrectly)
+{
+ auto [basePath, extension, expectedPath] = GetParam();
+ EXPECT_EQ(pathAppend(basePath, extension), expectedPath);
+}
+
+class TestPathAppendFail :
+ public Test,
+ public WithParamInterface<std::tuple<object_path, std::string>>
+{};
+
+INSTANTIATE_TEST_SUITE_P(
+ _, TestPathAppendFail,
+ Values(std::make_tuple(object_path("/Base/Path"), "/one"),
+ std::make_tuple(object_path("/Base/Path"), "one/"),
+ std::make_tuple(object_path("/Base/Path"), "one/two/"),
+ std::make_tuple(object_path("/Base/Path"), "one//two"),
+ std::make_tuple(object_path("/Base/Path"), "/"),
+ std::make_tuple(object_path("/Base/Path"), "//")));
+
+TEST_P(TestPathAppendFail, pathAppendsCorrectly)
+{
+ auto [basePath, extension] = GetParam();
+ EXPECT_THROW(pathAppend(basePath, extension),
+ sdbusplus::exception::SdBusError);
+}
diff --git a/tests/src/test_report.cpp b/tests/src/test_report.cpp
index 8226f66..ac2419d 100644
--- a/tests/src/test_report.cpp
+++ b/tests/src/test_report.cpp
@@ -14,6 +14,7 @@
#include "utils/clock.hpp"
#include "utils/contains.hpp"
#include "utils/conv_container.hpp"
+#include "utils/dbus_path_utils.hpp"
#include "utils/messanger.hpp"
#include "utils/transform.hpp"
#include "utils/tstring.hpp"
@@ -23,6 +24,7 @@
using namespace testing;
using namespace std::literals::string_literals;
using namespace std::chrono_literals;
+using sdbusplus::message::object_path;
namespace tstring = utils::tstring;
using ErrorMessageDbusType = std::tuple<std::string, std::string>;
@@ -238,7 +240,7 @@
EXPECT_THAT(getProperty<std::string>(sut->getPath(), "Name"),
Eq(defaultParams().reportName()));
EXPECT_THAT(
- getProperty<std::vector<std::string>>(sut->getPath(), "TriggerIds"),
+ getProperty<std::vector<object_path>>(sut->getPath(), "Triggers"),
IsEmpty());
EXPECT_THAT(
getProperty<ErrorMessagesDbusType>(sut->getPath(), "ErrorMessages"),
@@ -551,7 +553,8 @@
TEST_F(TestReport, deletingNonExistingReportReturnInvalidRequestDescriptor)
{
- auto ec = deleteReport(Report::reportDir + "NonExisting"s);
+ auto ec =
+ deleteReport(utils::constants::reportDirPath.str + "NonExisting"s);
EXPECT_THAT(ec.value(), Eq(EBADR));
}
@@ -581,8 +584,9 @@
{"someOtherReport", defaultParams().reportId()}});
EXPECT_THAT(
- getProperty<std::vector<std::string>>(sut->getPath(), "TriggerIds"),
- UnorderedElementsAre("trigger1", "trigger3"));
+ getProperty<std::vector<object_path>>(sut->getPath(), "Triggers"),
+ UnorderedElementsAre(utils::constants::triggerDirPath / "trigger1",
+ utils::constants::triggerDirPath / "trigger3"));
}
TEST_F(TestReport, updatesTriggerIdWhenTriggerIsRemoved)
@@ -604,8 +608,8 @@
messages::Presence::Removed, "trigger1", {defaultParams().reportId()}});
EXPECT_THAT(
- getProperty<std::vector<std::string>>(sut->getPath(), "TriggerIds"),
- UnorderedElementsAre("trigger3"));
+ getProperty<std::vector<object_path>>(sut->getPath(), "Triggers"),
+ UnorderedElementsAre(utils::constants::triggerDirPath / "trigger3"));
}
TEST_F(TestReport, updatesTriggerIdWhenTriggerIsModified)
@@ -627,8 +631,9 @@
messages::Presence::Exist, "trigger3", {defaultParams().reportId()}});
EXPECT_THAT(
- getProperty<std::vector<std::string>>(sut->getPath(), "TriggerIds"),
- UnorderedElementsAre("trigger1", "trigger3"));
+ getProperty<std::vector<object_path>>(sut->getPath(), "Triggers"),
+ UnorderedElementsAre(utils::constants::triggerDirPath / "trigger1",
+ utils::constants::triggerDirPath / "trigger3"));
}
class TestReportStore :
@@ -1217,8 +1222,9 @@
sut = makeReport(ReportParams());
EXPECT_THAT(
- getProperty<std::vector<std::string>>(sut->getPath(), "TriggerIds"),
- UnorderedElementsAre("trigger1", "trigger2"));
+ getProperty<std::vector<object_path>>(sut->getPath(), "Triggers"),
+ UnorderedElementsAre(utils::constants::triggerDirPath / "trigger1",
+ utils::constants::triggerDirPath / "trigger2"));
}
TEST_F(TestReportInitialization,
diff --git a/tests/src/test_trigger.cpp b/tests/src/test_trigger.cpp
index 9032d05..5c41d97 100644
--- a/tests/src/test_trigger.cpp
+++ b/tests/src/test_trigger.cpp
@@ -20,6 +20,7 @@
using namespace testing;
using namespace std::literals::string_literals;
+using sdbusplus::message::object_path;
static constexpr size_t expectedTriggerVersion = 2;
@@ -138,8 +139,8 @@
EXPECT_THAT((getProperty<SensorsInfo>(sut->getPath(), "Sensors")),
Eq(utils::fromLabeledSensorsInfo(triggerParams.sensors())));
EXPECT_THAT(
- getProperty<std::vector<std::string>>(sut->getPath(), "ReportNames"),
- Eq(triggerParams.reportIds()));
+ getProperty<std::vector<object_path>>(sut->getPath(), "Reports"),
+ Eq(triggerParams.reports()));
EXPECT_THAT(
getProperty<bool>(sut->getPath(), "Discrete"),
Eq(isTriggerThresholdDiscrete(triggerParams.thresholdParams())));
@@ -152,7 +153,8 @@
TEST_F(TestTrigger, checkBasicGetters)
{
EXPECT_THAT(sut->getId(), Eq(triggerParams.id()));
- EXPECT_THAT(sut->getPath(), Eq(Trigger::triggerDir + triggerParams.id()));
+ EXPECT_THAT(sut->getPath(),
+ Eq(utils::constants::triggerDirPath.str + triggerParams.id()));
}
TEST_F(TestTrigger, setPropertyNameToCorrectValue)
@@ -165,47 +167,81 @@
TEST_F(TestTrigger, setPropertyReportNames)
{
- std::vector<std::string> newNames = {"abc", "one", "two"};
- EXPECT_THAT(setProperty(sut->getPath(), "ReportNames", newNames),
+ std::vector<object_path> newNames = {
+ utils::constants::reportDirPath / "abc",
+ utils::constants::reportDirPath / "one",
+ utils::constants::reportDirPath / "prefix" / "two"};
+ EXPECT_THAT(setProperty(sut->getPath(), "Reports", newNames),
Eq(boost::system::errc::success));
EXPECT_THAT(
- getProperty<std::vector<std::string>>(sut->getPath(), "ReportNames"),
+ getProperty<std::vector<object_path>>(sut->getPath(), "Reports"),
Eq(newNames));
}
TEST_F(TestTrigger, sendsUpdateWhenReportNamesChanges)
{
- const std::vector<std::string> newPropertyVal = {"abc", "one", "two"};
+ std::vector<object_path> newPropertyVal = {
+ utils::constants::reportDirPath / "abc",
+ utils::constants::reportDirPath / "one",
+ utils::constants::reportDirPath / "two"};
EXPECT_CALL(triggerPresenceChanged,
Call(FieldsAre(messages::Presence::Exist, triggerParams.id(),
- UnorderedElementsAreArray(newPropertyVal))));
+ UnorderedElementsAre("abc", "one", "two"))));
- EXPECT_THAT(setProperty(sut->getPath(), "ReportNames", newPropertyVal),
+ EXPECT_THAT(setProperty(sut->getPath(), "Reports", newPropertyVal),
Eq(boost::system::errc::success));
}
TEST_F(TestTrigger, sendsUpdateWhenReportNamesChangesToSameValue)
{
- const std::vector<std::string> newPropertyVal = triggerParams.reportIds();
+ const std::vector<object_path> newPropertyVal = triggerParams.reports();
EXPECT_CALL(
triggerPresenceChanged,
Call(FieldsAre(messages::Presence::Exist, triggerParams.id(),
UnorderedElementsAreArray(triggerParams.reportIds()))));
- EXPECT_THAT(setProperty(sut->getPath(), "ReportNames", newPropertyVal),
+ EXPECT_THAT(setProperty(sut->getPath(), "Reports", newPropertyVal),
Eq(boost::system::errc::success));
}
TEST_F(TestTrigger,
DISABLED_settingPropertyReportNamesThrowsExceptionWhenDuplicateReportIds)
{
- std::vector<std::string> newPropertyVal{"report1", "report2", "report1"};
+ std::vector<object_path> newPropertyVal{
+ utils::constants::reportDirPath / "report1",
+ utils::constants::reportDirPath / "report2",
+ utils::constants::reportDirPath / "report1"};
EXPECT_CALL(triggerPresenceChanged, Call(_)).Times(0);
- EXPECT_THAT(setProperty(sut->getPath(), "ReportNames", newPropertyVal),
+ EXPECT_THAT(setProperty(sut->getPath(), "Reports", newPropertyVal),
+ Eq(boost::system::errc::invalid_argument));
+}
+
+TEST_F(
+ TestTrigger,
+ DISABLED_settingPropertyReportNamesThrowsExceptionWhenReportWithTooManyPrefixes)
+{
+ std::vector<object_path> newPropertyVal{
+ object_path("/xyz/openbmc_project/Telemetry/Reports/P1/P2/MyReport")};
+
+ EXPECT_CALL(triggerPresenceChanged, Call(_)).Times(0);
+
+ EXPECT_THAT(setProperty(sut->getPath(), "Reports", newPropertyVal),
+ Eq(boost::system::errc::invalid_argument));
+}
+
+TEST_F(TestTrigger,
+ DISABLED_settingPropertyReportNamesThrowsExceptionWhenReportWithBadPath)
+{
+ std::vector<object_path> newPropertyVal{
+ object_path("/xyz/openbmc_project/Telemetry/NotReports/MyReport")};
+
+ EXPECT_CALL(triggerPresenceChanged, Call(_)).Times(0);
+
+ EXPECT_THAT(setProperty(sut->getPath(), "Reports", newPropertyVal),
Eq(boost::system::errc::invalid_argument));
}
@@ -218,8 +254,8 @@
std::dynamic_pointer_cast<NiceMock<ThresholdMock>>(threshold);
EXPECT_CALL(*thresholdMockPtr, updateSensors(_));
}
- SensorsInfo newSensors({std::make_pair(
- sdbusplus::message::object_path("/abc/def"), "metadata")});
+ SensorsInfo newSensors(
+ {std::make_pair(object_path("/abc/def"), "metadata")});
EXPECT_THAT(setProperty(sut->getPath(), "Sensors", newSensors),
Eq(boost::system::errc::success));
}
@@ -307,7 +343,8 @@
TEST_F(TestTrigger, deletingNonExistingTriggerReturnInvalidRequestDescriptor)
{
- auto ec = deleteTrigger(Trigger::triggerDir + "NonExisting"s);
+ auto ec =
+ deleteTrigger(utils::constants::triggerDirPath.str + "NonExisting"s);
EXPECT_THAT(ec.value(), Eq(EBADR));
}
diff --git a/tests/src/test_trigger_actions.cpp b/tests/src/test_trigger_actions.cpp
index e8c48d4..42488aa 100644
--- a/tests/src/test_trigger_actions.cpp
+++ b/tests/src/test_trigger_actions.cpp
@@ -1,4 +1,5 @@
#include "dbus_environment.hpp"
+#include "helpers.hpp"
#include "messages/update_report_ind.hpp"
#include "trigger_actions.hpp"
#include "utils/messanger.hpp"
diff --git a/tests/src/test_trigger_manager.cpp b/tests/src/test_trigger_manager.cpp
index efdac65..58d0df1 100644
--- a/tests/src/test_trigger_manager.cpp
+++ b/tests/src/test_trigger_manager.cpp
@@ -10,6 +10,7 @@
#include "utils/transform.hpp"
using namespace testing;
+using sdbusplus::message::object_path;
class TestTriggerManager : public Test
{
@@ -33,7 +34,7 @@
utils::transform(
params.triggerActions(),
[](const auto& action) { return actionToString(action); }),
- sensorInfos, params.reportIds(),
+ sensorInfos, params.reports(),
std::visit(utils::FromLabeledThresholdParamConversion(),
params.thresholdParams()));
return DbusEnvironment::waitForFuture(addTriggerPromise.get_future());
@@ -134,6 +135,38 @@
EXPECT_THAT(path, Eq(std::string()));
}
+TEST_F(TestTriggerManager, addTriggerWithProperReportPaths)
+{
+ auto [ec, path] = addTrigger(TriggerParams().reports(
+ {object_path("/xyz/openbmc_project/Telemetry/Reports/MyReport"),
+ object_path(
+ "/xyz/openbmc_project/Telemetry/Reports/MyPrefix/MyReport")}));
+ EXPECT_THAT(ec.value(), Eq(boost::system::errc::success));
+ EXPECT_THAT(path, Eq(triggerMock.getPath()));
+}
+
+TEST_F(TestTriggerManager, DISABLED_failToAddTriggerWithBadReportsPath)
+{
+ triggerFactoryMock.expectMake(std::nullopt, Ref(*sut), Ref(storageMock))
+ .Times(0);
+
+ auto [ec, path] = addTrigger(TriggerParams().reports(
+ {object_path("/xyz/openbmc_project/Telemetry/NotReports/MyReport")}));
+ EXPECT_THAT(ec.value(), Eq(boost::system::errc::invalid_argument));
+ EXPECT_THAT(path, Eq(std::string()));
+}
+
+TEST_F(TestTriggerManager, DISABLED_failToAddTriggerWithTooManyReportPrefixes)
+{
+ triggerFactoryMock.expectMake(std::nullopt, Ref(*sut), Ref(storageMock))
+ .Times(0);
+
+ auto [ec, path] = addTrigger(TriggerParams().reports({object_path(
+ "/xyz/openbmc_project/Telemetry/Reports/P1/P2/MyReport")}));
+ EXPECT_THAT(ec.value(), Eq(boost::system::errc::invalid_argument));
+ EXPECT_THAT(path, Eq(std::string()));
+}
+
TEST_F(TestTriggerManager, addTriggerWithoutIdAndName)
{
triggerFactoryMock
diff --git a/tests/src/utils/generate_unique_mock_id.cpp b/tests/src/utils/generate_unique_mock_id.cpp
index f3e3f01..7ce4040 100644
--- a/tests/src/utils/generate_unique_mock_id.cpp
+++ b/tests/src/utils/generate_unique_mock_id.cpp
@@ -1,5 +1,7 @@
#include "generate_unique_mock_id.hpp"
+#include "helpers.hpp"
+
uint64_t generateUniqueMockId()
{
static uint64_t id = 0u;