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/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(