Api changes in AddReportFuture version
Added support for CollectionFunction, CollectionDuration,
CollectionTimeScope, ReportUpdates, AppendLimit.
New API separates Id and Name, user can decide to pass only Name
to auto generate Id or pass Id which needs to be unique.
Tested:
- No functional changes to old API, everything works as before
- All use cases can be replaced with new API to achieve same results
- New features which require new API work as expected
Signed-off-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com>
Change-Id: I647efab36d90a548754f89968223e162a087481e
diff --git a/meson.build b/meson.build
index 7673e7e..811749f 100644
--- a/meson.build
+++ b/meson.build
@@ -91,6 +91,7 @@
'src/trigger_manager.cpp',
'src/types/report_types.cpp',
'src/utils/conversion_trigger.cpp',
+ 'src/utils/generate_id.cpp',
],
dependencies: [
boost,
diff --git a/src/discrete_threshold.cpp b/src/discrete_threshold.cpp
index efec549..89a1b7c 100644
--- a/src/discrete_threshold.cpp
+++ b/src/discrete_threshold.cpp
@@ -94,8 +94,3 @@
action->commit(sensorName, timestamp, value);
}
}
-
-const char* DiscreteThreshold::getName() const
-{
- return name.c_str();
-}
diff --git a/src/discrete_threshold.hpp b/src/discrete_threshold.hpp
index 70b2a6a..8dc2eeb 100644
--- a/src/discrete_threshold.hpp
+++ b/src/discrete_threshold.hpp
@@ -32,7 +32,6 @@
void initialize() override;
void sensorUpdated(interfaces::Sensor&, uint64_t) override;
void sensorUpdated(interfaces::Sensor&, uint64_t, double) override;
- const char* getName() const;
private:
boost::asio::io_context& ioc;
diff --git a/src/interfaces/report.hpp b/src/interfaces/report.hpp
index a299dac..4b3bd9e 100644
--- a/src/interfaces/report.hpp
+++ b/src/interfaces/report.hpp
@@ -10,7 +10,7 @@
public:
virtual ~Report() = default;
- virtual std::string getName() const = 0;
+ virtual std::string getId() const = 0;
virtual std::string getPath() const = 0;
virtual void updateReadings() = 0;
};
diff --git a/src/interfaces/report_factory.hpp b/src/interfaces/report_factory.hpp
index 613c1d5..6a255a4 100644
--- a/src/interfaces/report_factory.hpp
+++ b/src/interfaces/report_factory.hpp
@@ -27,7 +27,8 @@
const ReadingParameters& metricParams) const = 0;
virtual std::unique_ptr<interfaces::Report>
- make(const std::string& name, const ReportingType reportingType,
+ make(const std::string& id, const std::string& name,
+ const ReportingType reportingType,
const std::vector<ReportAction>& reportActions,
Milliseconds period, uint64_t appendLimit,
const ReportUpdates reportUpdates, ReportManager& reportManager,
diff --git a/src/interfaces/report_manager.hpp b/src/interfaces/report_manager.hpp
index 910e439..901c446 100644
--- a/src/interfaces/report_manager.hpp
+++ b/src/interfaces/report_manager.hpp
@@ -11,7 +11,7 @@
virtual ~ReportManager() = default;
virtual void removeReport(const interfaces::Report* report) = 0;
- virtual void updateReport(const std::string& name) = 0;
+ virtual void updateReport(const std::string& id) = 0;
};
} // namespace interfaces
diff --git a/src/interfaces/sensor.hpp b/src/interfaces/sensor.hpp
index 65d3c21..6a6bcf7 100644
--- a/src/interfaces/sensor.hpp
+++ b/src/interfaces/sensor.hpp
@@ -43,6 +43,7 @@
virtual ~Sensor() = default;
virtual Id id() const = 0;
+ virtual std::string metadata() const = 0;
virtual void registerForUpdates(const std::weak_ptr<SensorListener>&) = 0;
virtual void
unregisterFromUpdates(const std::weak_ptr<SensorListener>&) = 0;
diff --git a/src/interfaces/trigger_factory.hpp b/src/interfaces/trigger_factory.hpp
index f39d555..ee85063 100644
--- a/src/interfaces/trigger_factory.hpp
+++ b/src/interfaces/trigger_factory.hpp
@@ -22,7 +22,7 @@
virtual std::unique_ptr<interfaces::Trigger> make(
const std::string& id, const std::string& name,
const std::vector<std::string>& triggerActions,
- const std::vector<std::string>& reportNames,
+ const std::vector<std::string>& reportIds,
interfaces::TriggerManager& triggerManager,
interfaces::JsonStorage& triggerStorage,
const LabeledTriggerThresholdParams& labeledThresholdParams,
diff --git a/src/metric.cpp b/src/metric.cpp
index d76f220..ee12c74 100644
--- a/src/metric.cpp
+++ b/src/metric.cpp
@@ -5,6 +5,8 @@
#include "utils/labeled_tuple.hpp"
#include "utils/transform.hpp"
+#include <sdbusplus/exception.hpp>
+
#include <algorithm>
class Metric::CollectionData
@@ -45,7 +47,14 @@
CollectionDuration duration) :
function(std::move(function)),
duration(duration)
- {}
+ {
+ if (duration.t.count() == 0)
+ {
+ throw sdbusplus::exception::SdBusError(
+ static_cast<int>(std::errc::invalid_argument),
+ "Invalid CollectionDuration");
+ }
+ }
ReadingItem update(uint64_t timestamp) override
{
@@ -113,14 +122,10 @@
};
Metric::Metric(Sensors sensorsIn, OperationType operationTypeIn,
- std::string idIn, std::string metadataIn,
- CollectionTimeScope timeScopeIn,
+ std::string idIn, CollectionTimeScope timeScopeIn,
CollectionDuration collectionDurationIn,
std::unique_ptr<interfaces::Clock> clockIn) :
- id(idIn),
- metadata(metadataIn),
- readings(sensorsIn.size(),
- MetricValue{std::move(idIn), std::move(metadataIn), 0.0, 0u}),
+ id(std::move(idIn)),
sensors(std::move(sensorsIn)), operationType(operationTypeIn),
collectionTimeScope(timeScopeIn), collectionDuration(collectionDurationIn),
collectionAlgorithms(makeCollectionData(sensors.size(), operationType,
@@ -128,7 +133,9 @@
collectionDuration)),
clock(std::move(clockIn))
{
- attemptUnpackJsonMetadata();
+ readings = utils::transform(sensors, [this](const auto& sensor) {
+ return MetricValue{id, sensor->metadata(), 0.0, 0u};
+ });
}
Metric::~Metric() = default;
@@ -187,12 +194,12 @@
LabeledMetricParameters Metric::dumpConfiguration() const
{
auto sensorPath = utils::transform(sensors, [this](const auto& sensor) {
- return LabeledSensorParameters(sensor->id().service, sensor->id().path);
+ return LabeledSensorParameters(sensor->id().service, sensor->id().path,
+ sensor->metadata());
});
return LabeledMetricParameters(std::move(sensorPath), operationType, id,
- metadata, collectionTimeScope,
- collectionDuration);
+ collectionTimeScope, collectionDuration);
}
std::vector<std::unique_ptr<Metric::CollectionData>>
@@ -238,32 +245,3 @@
{
return sensors.size();
}
-
-void Metric::attemptUnpackJsonMetadata()
-{
- using MetricMetadata =
- utils::LabeledTuple<std::tuple<std::vector<std::string>>,
- utils::tstring::MetricProperties>;
-
- using ReadingMetadata =
- utils::LabeledTuple<std::tuple<std::string, std::string>,
- utils::tstring::SensorDbusPath,
- utils::tstring::SensorRedfishUri>;
- try
- {
- const MetricMetadata parsedMetadata =
- nlohmann::json::parse(metadata).get<MetricMetadata>();
-
- if (readings.size() == parsedMetadata.at_index<0>().size())
- {
- for (size_t i = 0; i < readings.size(); ++i)
- {
- ReadingMetadata readingMetadata{
- sensors[i]->id().path, parsedMetadata.at_index<0>()[i]};
- readings[i].metadata = readingMetadata.dump();
- }
- }
- }
- catch (const nlohmann::json::exception&)
- {}
-}
diff --git a/src/metric.hpp b/src/metric.hpp
index 7929aac..1ff5bc2 100644
--- a/src/metric.hpp
+++ b/src/metric.hpp
@@ -12,7 +12,7 @@
{
public:
Metric(Sensors sensors, OperationType operationType, std::string id,
- std::string metadata, CollectionTimeScope, CollectionDuration,
+ CollectionTimeScope, CollectionDuration,
std::unique_ptr<interfaces::Clock>);
~Metric();
@@ -34,11 +34,9 @@
makeCollectionData(size_t size, OperationType, CollectionTimeScope,
CollectionDuration);
- void attemptUnpackJsonMetadata();
CollectionData& findAssociatedData(const interfaces::Sensor& notifier);
std::string id;
- std::string metadata;
std::vector<MetricValue> readings;
Sensors sensors;
OperationType operationType;
diff --git a/src/report.cpp b/src/report.cpp
index cc981f1..c2813c0 100644
--- a/src/report.cpp
+++ b/src/report.cpp
@@ -1,6 +1,7 @@
#include "report.hpp"
#include "report_manager.hpp"
+#include "utils/clock.hpp"
#include "utils/contains.hpp"
#include "utils/transform.hpp"
@@ -12,7 +13,7 @@
Report::Report(boost::asio::io_context& ioc,
const std::shared_ptr<sdbusplus::asio::object_server>& objServer,
- const std::string& reportName,
+ const std::string& reportId, const std::string& reportName,
const ReportingType reportingTypeIn,
std::vector<ReportAction> reportActionsIn,
const Milliseconds intervalIn, const uint64_t appendLimitIn,
@@ -21,15 +22,14 @@
interfaces::JsonStorage& reportStorageIn,
std::vector<std::shared_ptr<interfaces::Metric>> metricsIn,
const bool enabledIn) :
- name(reportName),
- path(reportDir + name), reportingType(reportingTypeIn),
- interval(intervalIn), reportActions(std::move(reportActionsIn)),
+ id(reportId),
+ name(reportName), reportingType(reportingTypeIn), interval(intervalIn),
+ reportActions(std::move(reportActionsIn)),
sensorCount(getSensorCount(metricsIn)),
appendLimit(deduceAppendLimit(appendLimitIn)),
reportUpdates(reportUpdatesIn),
readingsBuffer(deduceBufferSize(reportUpdates, reportingType)),
objServer(objServer), metrics(std::move(metricsIn)), timer(ioc),
- fileName(std::to_string(std::hash<std::string>{}(name))),
reportStorage(reportStorageIn), enabled(enabledIn)
{
readingParameters =
@@ -39,17 +39,21 @@
readingParametersPastVersion =
utils::transform(readingParameters, [](const auto& item) {
+ const auto& [sensorData, operationType, id, collectionTimeScope,
+ collectionDuration] = item;
+
return ReadingParametersPastVersion::value_type(
- std::get<0>(item).front(), std::get<1>(item), std::get<2>(item),
- std::get<3>(item));
+ std::get<0>(sensorData.front()), operationType, id,
+ std::get<1>(sensorData.front()));
});
deleteIface = objServer->add_unique_interface(
- path, deleteIfaceName, [this, &ioc, &reportManager](auto& dbusIface) {
+ getPath(), deleteIfaceName,
+ [this, &ioc, &reportManager](auto& dbusIface) {
dbusIface.register_method("Delete", [this, &ioc, &reportManager] {
if (persistency)
{
- reportStorage.remove(fileName);
+ reportStorage.remove(fileName());
}
boost::asio::post(ioc, [this, &reportManager] {
reportManager.removeReport(this);
@@ -128,7 +132,8 @@
std::unique_ptr<sdbusplus::asio::dbus_interface> Report::makeReportInterface()
{
- auto dbusIface = objServer->add_unique_interface(path, reportIfaceName);
+ auto dbusIface =
+ objServer->add_unique_interface(getPath(), reportIfaceName);
dbusIface->register_property_rw(
"Enabled", enabled, sdbusplus::vtable::property_::emits_change,
[this](bool newVal, const auto&) {
@@ -189,7 +194,7 @@
}
else
{
- reportStorage.remove(fileName);
+ reportStorage.remove(fileName());
persistency = false;
}
return true;
@@ -220,6 +225,9 @@
return utils::contains(reportActions,
ReportAction::emitsReadingsUpdate);
});
+ dbusIface->register_property_r("Name", std::string{},
+ sdbusplus::vtable::property_::const_,
+ [this](const auto&) { return name; });
dbusIface->register_property_r(
"LogToMetricReportsCollection", bool{},
sdbusplus::vtable::property_::const_, [this](const auto&) {
@@ -307,8 +315,9 @@
}
}
- readings = {std::time(0), std::vector<ReadingData>(readingsBuffer.begin(),
- readingsBuffer.end())};
+ readings = {
+ Clock().timestamp(),
+ std::vector<ReadingData>(readingsBuffer.begin(), readingsBuffer.end())};
reportIface->signal_property("Readings");
}
@@ -321,6 +330,7 @@
data["Enabled"] = enabled;
data["Version"] = reportVersion;
+ data["Id"] = id;
data["Name"] = name;
data["ReportingType"] = utils::toUnderlying(reportingType);
data["ReportActions"] =
@@ -335,7 +345,7 @@
return metric->dumpConfiguration();
});
- reportStorage.store(fileName, data);
+ reportStorage.store(fileName(), data);
}
catch (const std::exception& e)
{
@@ -347,3 +357,8 @@
return true;
}
+interfaces::JsonStorage::FilePath Report::fileName() const
+{
+ return interfaces::JsonStorage::FilePath{
+ std::to_string(std::hash<std::string>{}(id))};
+}
diff --git a/src/report.hpp b/src/report.hpp
index 5d675bf..72563f5 100644
--- a/src/report.hpp
+++ b/src/report.hpp
@@ -22,7 +22,8 @@
public:
Report(boost::asio::io_context& ioc,
const std::shared_ptr<sdbusplus::asio::object_server>& objServer,
- const std::string& reportName, const ReportingType reportingType,
+ const std::string& reportId, const std::string& reportName,
+ const ReportingType reportingType,
std::vector<ReportAction> reportActions, const Milliseconds period,
const uint64_t appendLimitIn, const ReportUpdates reportUpdatesIn,
interfaces::ReportManager& reportManager,
@@ -36,14 +37,14 @@
Report& operator=(const Report&) = delete;
Report& operator=(Report&&) = delete;
- std::string getName() const override
+ std::string getId() const override
{
- return name;
+ return id;
}
std::string getPath() const override
{
- return path;
+ return reportDir + id;
}
void updateReadings() override;
@@ -59,9 +60,10 @@
void setReportUpdates(const ReportUpdates newReportUpdates);
static uint64_t getSensorCount(
std::vector<std::shared_ptr<interfaces::Metric>>& metrics);
+ interfaces::JsonStorage::FilePath fileName() const;
+ std::string id;
std::string name;
- std::string path;
ReportingType reportingType;
Milliseconds interval;
std::vector<ReportAction> reportActions;
@@ -79,7 +81,6 @@
std::vector<std::shared_ptr<interfaces::Metric>> metrics;
boost::asio::steady_timer timer;
- interfaces::JsonStorage::FilePath fileName;
interfaces::JsonStorage& reportStorage;
bool enabled;
diff --git a/src/report_factory.cpp b/src/report_factory.cpp
index 57126b7..f72a3f7 100644
--- a/src/report_factory.cpp
+++ b/src/report_factory.cpp
@@ -17,7 +17,8 @@
{}
std::unique_ptr<interfaces::Report> ReportFactory::make(
- const std::string& name, const ReportingType reportingType,
+ const std::string& id, const std::string& name,
+ const ReportingType reportingType,
const std::vector<ReportAction>& reportActions, Milliseconds period,
uint64_t appendLimit, const ReportUpdates reportUpdates,
interfaces::ReportManager& reportManager,
@@ -34,13 +35,12 @@
return std::make_shared<Metric>(
getSensors(param.at_label<ts::SensorPath>()),
param.at_label<ts::OperationType>(), param.at_label<ts::Id>(),
- param.at_label<ts::MetricMetadata>(),
param.at_label<ts::CollectionTimeScope>(),
param.at_label<ts::CollectionDuration>(),
std::make_unique<Clock>());
});
- return std::make_unique<Report>(bus->get_io_context(), objServer, name,
+ return std::make_unique<Report>(bus->get_io_context(), objServer, id, name,
reportingType, reportActions, period,
appendLimit, reportUpdates, reportManager,
reportStorage, std::move(metrics), enabled);
@@ -51,14 +51,14 @@
{
using namespace utils::tstring;
- return utils::transform(sensorPaths,
- [this](const LabeledSensorParameters& sensorPath)
- -> std::shared_ptr<interfaces::Sensor> {
- return sensorCache.makeSensor<Sensor>(
- sensorPath.at_label<Service>(),
- sensorPath.at_label<Path>(),
- bus->get_io_context(), bus);
- });
+ return utils::transform(
+ sensorPaths,
+ [this](const LabeledSensorParameters& sensorPath)
+ -> std::shared_ptr<interfaces::Sensor> {
+ return sensorCache.makeSensor<Sensor>(
+ sensorPath.at_label<Service>(), sensorPath.at_label<Path>(),
+ sensorPath.at_label<Metadata>(), bus->get_io_context(), bus);
+ });
}
std::vector<LabeledMetricParameters> ReportFactory::convertMetricParams(
@@ -68,12 +68,12 @@
auto tree = utils::getSubTreeSensors(yield, bus);
return utils::transform(metricParams, [&tree](const auto& item) {
- const auto& [sensorPaths, operationType, id, metadata,
- collectionTimeScope, collectionDuration] = item;
+ auto [sensorPaths, operationType, id, collectionTimeScope,
+ collectionDuration] = item;
std::vector<LabeledSensorParameters> sensorParameters;
- for (const auto& sensorPath : sensorPaths)
+ for (const auto& [sensorPath, metadata] : sensorPaths)
{
auto it = std::find_if(
tree.begin(), tree.end(),
@@ -82,7 +82,7 @@
if (it != tree.end() && it->second.size() == 1)
{
const auto& [service, ifaces] = it->second.front();
- sensorParameters.emplace_back(service, sensorPath);
+ sensorParameters.emplace_back(service, sensorPath, metadata);
}
}
@@ -93,9 +93,20 @@
"Could not find service for provided sensors");
}
+ if (operationType.empty())
+ {
+ operationType = utils::enumToString(OperationType::avg);
+ }
+
+ if (collectionTimeScope.empty())
+ {
+ collectionTimeScope =
+ utils::enumToString(CollectionTimeScope::point);
+ }
+
return LabeledMetricParameters(
std::move(sensorParameters), utils::toOperationType(operationType),
- id, metadata, utils::toCollectionTimeScope(collectionTimeScope),
+ id, utils::toCollectionTimeScope(collectionTimeScope),
CollectionDuration(Milliseconds(collectionDuration)));
});
}
diff --git a/src/report_factory.hpp b/src/report_factory.hpp
index 15f5898..c9209ef 100644
--- a/src/report_factory.hpp
+++ b/src/report_factory.hpp
@@ -20,7 +20,8 @@
const ReadingParameters& metricParams) const override;
std::unique_ptr<interfaces::Report>
- make(const std::string& name, const ReportingType reportingType,
+ make(const std::string& reportId, const std::string& name,
+ const ReportingType reportingType,
const std::vector<ReportAction>& reportActions,
Milliseconds period, uint64_t appendLimitIn,
const ReportUpdates reportUpdatesIn,
diff --git a/src/report_manager.cpp b/src/report_manager.cpp
index ab76a31..18636f8 100644
--- a/src/report_manager.cpp
+++ b/src/report_manager.cpp
@@ -3,6 +3,7 @@
#include "report.hpp"
#include "types/report_types.hpp"
#include "utils/conversion.hpp"
+#include "utils/generate_id.hpp"
#include "utils/transform.hpp"
#include <phosphor-logging/log.hpp>
@@ -17,10 +18,14 @@
return utils::transform(params, [](const auto& param) {
using namespace std::chrono_literals;
+ const auto& [sensorPath, operationType, id, metadata] = param;
+
return ReadingParameters::value_type(
- std::vector{{std::get<0>(param)}}, std::get<1>(param),
- std::get<2>(param), std::get<3>(param),
- utils::enumToString(CollectionTimeScope::point), 0u);
+ std::vector<
+ std::tuple<sdbusplus::message::object_path, std::string>>{
+ {sensorPath, metadata}},
+ operationType, id, utils::enumToString(CollectionTimeScope::point),
+ 0u);
});
}
@@ -41,16 +46,16 @@
"MaxReports", size_t{}, sdbusplus::vtable::property_::const_,
[](const auto&) { return maxReports; });
dbusIface.register_property_r(
- "MaxReportNameLength", size_t{},
+ "MaxReportIdLength", size_t{},
sdbusplus::vtable::property_::const_,
- [](const auto&) { return maxReportNameLength; });
+ [](const auto&) { return maxReportIdLength; });
dbusIface.register_property_r(
"MinInterval", uint64_t{}, sdbusplus::vtable::property_::const_,
[](const auto&) -> uint64_t { return minInterval.count(); });
dbusIface.register_method(
"AddReport", [this](boost::asio::yield_context& yield,
- const std::string& reportName,
+ const std::string& reportId,
const std::string& reportingType,
const bool emitsReadingsUpdate,
const bool logToMetricReportsCollection,
@@ -74,7 +79,7 @@
ReportAction::logToMetricReportsCollection);
}
- return addReport(yield, reportName,
+ return addReport(yield, reportId, reportId,
utils::toReportingType(reportingType),
reportActions, Milliseconds(interval),
appendLimitDefault, reportUpdatesDefault,
@@ -86,16 +91,16 @@
dbusIface.register_method(
"AddReportFutureVersion",
- [this](boost::asio::yield_context& yield,
- const std::string& reportName,
- const std::string& reportingType,
- const std::string& reportUpdates,
- const uint64_t appendLimit,
- const std::vector<std::string>& reportActions,
- const uint64_t interval,
- ReadingParameters metricParams) {
+ [this](
+ boost::asio::yield_context& yield,
+ const std::string& reportId, const std::string& reportName,
+ const std::string& reportingType,
+ const std::string& reportUpdates,
+ const uint64_t appendLimit,
+ const std::vector<std::string>& reportActions,
+ const uint64_t interval, ReadingParameters metricParams) {
constexpr auto enabledDefault = true;
- return addReport(yield, reportName,
+ return addReport(yield, reportId, reportName,
utils::toReportingType(reportingType),
utils::transform(
reportActions,
@@ -119,19 +124,20 @@
reports.end());
}
-void ReportManager::verifyReportNameLength(const std::string& reportName)
+void ReportManager::verifyReportIdLength(const std::string& reportId)
{
- if (reportName.length() > maxReportNameLength)
+ if (reportId.length() > maxReportIdLength)
{
throw sdbusplus::exception::SdBusError(
static_cast<int>(std::errc::invalid_argument),
- "Report name exceeds maximum length");
+ "Report id exceeds maximum length");
}
}
void ReportManager::verifyAddReport(
- const std::string& reportName, const ReportingType reportingType,
- Milliseconds interval, const ReportUpdates reportUpdates,
+ const std::string& reportId, const std::string& reportName,
+ const ReportingType reportingType, Milliseconds interval,
+ const ReportUpdates reportUpdates,
const std::vector<LabeledMetricParameters>& readingParams)
{
if (reportingType == ReportingType::onChange)
@@ -148,11 +154,12 @@
"Reached maximal report count");
}
- verifyReportNameLength(reportName);
+ verifyReportIdLength(reportId);
+ utils::verifyIdCharacters(reportId);
for (const auto& report : reports)
{
- if (report->getName() == reportName)
+ if (report->getId() == reportId)
{
throw sdbusplus::exception::SdBusError(
static_cast<int>(std::errc::file_exists), "Duplicate report");
@@ -193,8 +200,8 @@
}
interfaces::Report& ReportManager::addReport(
- boost::asio::yield_context& yield, const std::string& reportName,
- const ReportingType reportingType,
+ boost::asio::yield_context& yield, const std::string& reportId,
+ const std::string& reportName, const ReportingType reportingType,
const std::vector<ReportAction>& reportActions, Milliseconds interval,
const uint64_t appendLimit, const ReportUpdates reportUpdates,
ReadingParameters metricParams, const bool enabled)
@@ -202,23 +209,36 @@
auto labeledMetricParams =
reportFactory->convertMetricParams(yield, metricParams);
- return addReport(reportName, reportingType, reportActions, interval,
- appendLimit, reportUpdates, std::move(labeledMetricParams),
- enabled);
+ return addReport(reportId, reportName, reportingType, reportActions,
+ interval, appendLimit, reportUpdates,
+ std::move(labeledMetricParams), enabled);
}
interfaces::Report& ReportManager::addReport(
- const std::string& reportName, const ReportingType reportingType,
+ const std::string& reportId, const std::string& reportName,
+ const ReportingType reportingType,
const std::vector<ReportAction>& reportActions, Milliseconds interval,
const uint64_t appendLimit, const ReportUpdates reportUpdates,
std::vector<LabeledMetricParameters> labeledMetricParams,
const bool enabled)
{
- verifyAddReport(reportName, reportingType, interval, reportUpdates,
+ std::string name = reportName;
+ if (name.empty())
+ {
+ name = "Report";
+ }
+
+ const auto existingReportIds = utils::transform(
+ reports, [](const auto& report) { return report->getId(); });
+
+ std::string id =
+ utils::generateId(reportId, name, existingReportIds, maxReportIdLength);
+
+ verifyAddReport(id, name, reportingType, interval, reportUpdates,
labeledMetricParams);
reports.emplace_back(reportFactory->make(
- reportName, reportingType, reportActions, interval, appendLimit,
+ id, name, reportingType, reportActions, interval, appendLimit,
reportUpdates, *this, *reportStorage, labeledMetricParams, enabled));
return *reports.back();
}
@@ -233,12 +253,13 @@
std::optional<nlohmann::json> data = reportStorage->load(path);
try
{
- bool enabled = data->at("Enabled").get<bool>();
size_t version = data->at("Version").get<size_t>();
if (version != Report::reportVersion)
{
throw std::logic_error("Invalid version");
}
+ bool enabled = data->at("Enabled").get<bool>();
+ std::string& id = data->at("Id").get_ref<std::string&>();
std::string& name = data->at("Name").get_ref<std::string&>();
uint32_t reportingType = data->at("ReportingType").get<uint32_t>();
@@ -254,7 +275,7 @@
data->at("ReadingParameters")
.get<std::vector<LabeledMetricParameters>>();
- addReport(name, utils::toReportingType(reportingType),
+ addReport(id, name, utils::toReportingType(reportingType),
reportActions, Milliseconds(interval), appendLimit,
utils::toReportUpdates(reportUpdates),
std::move(readingParameters), enabled);
@@ -272,11 +293,11 @@
}
}
-void ReportManager::updateReport(const std::string& name)
+void ReportManager::updateReport(const std::string& id)
{
for (auto& report : reports)
{
- if (report->getName() == name)
+ if (report->getId() == id)
{
report->updateReadings();
return;
diff --git a/src/report_manager.hpp b/src/report_manager.hpp
index cc613e8..b12d335 100644
--- a/src/report_manager.hpp
+++ b/src/report_manager.hpp
@@ -30,7 +30,7 @@
ReportManager& operator=(ReportManager&&) = delete;
void removeReport(const interfaces::Report* report) override;
- void updateReport(const std::string& name) override;
+ void updateReport(const std::string& id) override;
private:
std::unique_ptr<interfaces::ReportFactory> reportFactory;
@@ -39,19 +39,21 @@
std::unique_ptr<sdbusplus::asio::dbus_interface> reportManagerIface;
std::vector<std::unique_ptr<interfaces::Report>> reports;
- void verifyReportNameLength(const std::string& reportName);
+ void verifyReportIdLength(const std::string& reportName);
void verifyAddReport(
- const std::string& reportName, const ReportingType reportingType,
- Milliseconds interval, const ReportUpdates reportUpdates,
+ const std::string& reportId, const std::string& reportName,
+ const ReportingType reportingType, Milliseconds interval,
+ const ReportUpdates reportUpdates,
const std::vector<LabeledMetricParameters>& readingParams);
interfaces::Report& addReport(
- boost::asio::yield_context& yield, const std::string& reportName,
- const ReportingType reportingType,
+ boost::asio::yield_context& yield, const std::string& reportId,
+ const std::string& reportName, const ReportingType reportingType,
const std::vector<ReportAction>& reportActions, Milliseconds interval,
const uint64_t appendLimit, const ReportUpdates reportUpdates,
ReadingParameters metricParams, const bool enabled);
interfaces::Report& addReport(
- const std::string& reportName, const ReportingType reportingType,
+ const std::string& reportId, const std::string& reportName,
+ const ReportingType reportingType,
const std::vector<ReportAction>& reportActions, Milliseconds interval,
const uint64_t appendLimit, const ReportUpdates reportUpdates,
std::vector<LabeledMetricParameters> metricParams, const bool enabled);
@@ -60,7 +62,7 @@
public:
static constexpr size_t maxReports{TELEMETRY_MAX_REPORTS};
static constexpr size_t maxReadingParams{TELEMETRY_MAX_READING_PARAMS};
- static constexpr size_t maxReportNameLength{
+ static constexpr size_t maxReportIdLength{
TELEMETRY_MAX_DBUS_PATH_LENGTH -
std::string_view(Report::reportDir).length()};
static constexpr Milliseconds minInterval{TELEMETRY_MIN_INTERVAL};
diff --git a/src/sensor.cpp b/src/sensor.cpp
index 644de2f..5c71910 100644
--- a/src/sensor.cpp
+++ b/src/sensor.cpp
@@ -1,15 +1,18 @@
#include "sensor.hpp"
+#include "utils/clock.hpp"
+
#include <boost/container/flat_map.hpp>
#include <phosphor-logging/log.hpp>
#include <sdbusplus/asio/property.hpp>
#include <functional>
-Sensor::Sensor(interfaces::Sensor::Id sensorId, boost::asio::io_context& ioc,
+Sensor::Sensor(interfaces::Sensor::Id sensorId,
+ const std::string& sensorMetadata, boost::asio::io_context& ioc,
const std::shared_ptr<sdbusplus::asio::connection>& bus) :
sensorId(std::move(sensorId)),
- ioc(ioc), bus(bus)
+ sensorMetadata(sensorMetadata), ioc(ioc), bus(bus)
{}
Sensor::Id Sensor::makeId(std::string_view service, std::string_view path)
@@ -22,6 +25,11 @@
return sensorId;
}
+std::string Sensor::metadata() const
+{
+ return sensorMetadata;
+}
+
void Sensor::async_read()
{
uniqueCall([this](auto lock) { async_read(std::move(lock)); });
@@ -92,7 +100,7 @@
void Sensor::updateValue(double newValue)
{
- timestamp = std::time(0);
+ timestamp = Clock().timestamp();
if (value == newValue)
{
diff --git a/src/sensor.hpp b/src/sensor.hpp
index 3393b06..3c9c746 100644
--- a/src/sensor.hpp
+++ b/src/sensor.hpp
@@ -18,7 +18,8 @@
using ValueVariant = std::variant<std::monostate, double>;
public:
- Sensor(interfaces::Sensor::Id sensorId, boost::asio::io_context& ioc,
+ Sensor(interfaces::Sensor::Id sensorId, const std::string& sensorMetadata,
+ boost::asio::io_context& ioc,
const std::shared_ptr<sdbusplus::asio::connection>& bus);
Sensor(const Sensor&) = delete;
@@ -27,6 +28,7 @@
static Id makeId(std::string_view service, std::string_view path);
Id id() const override;
+ std::string metadata() const override;
void registerForUpdates(
const std::weak_ptr<interfaces::SensorListener>& weakListener) override;
void unregisterFromUpdates(
@@ -43,6 +45,7 @@
void updateValue(double);
interfaces::Sensor::Id sensorId;
+ std::string sensorMetadata;
boost::asio::io_context& ioc;
std::shared_ptr<sdbusplus::asio::connection> bus;
Milliseconds timerInterval = Milliseconds(0);
diff --git a/src/trigger.cpp b/src/trigger.cpp
index ff229b8..0b210ae 100644
--- a/src/trigger.cpp
+++ b/src/trigger.cpp
@@ -13,7 +13,7 @@
const std::shared_ptr<sdbusplus::asio::object_server>& objServer,
const std::string& idIn, const std::string& nameIn,
const std::vector<std::string>& triggerActionsIn,
- const std::vector<std::string>& reportNamesIn,
+ const std::vector<std::string>& reportIdsIn,
const std::vector<LabeledSensorInfo>& LabeledSensorsInfoIn,
const LabeledTriggerThresholdParams& labeledThresholdParamsIn,
std::vector<std::shared_ptr<interfaces::Threshold>>&& thresholdsIn,
@@ -21,7 +21,7 @@
interfaces::JsonStorage& triggerStorageIn) :
id(idIn),
name(nameIn), triggerActions(std::move(triggerActionsIn)),
- path(triggerDir + id), reportNames(reportNamesIn),
+ path(triggerDir + id), reportIds(reportIdsIn),
labeledSensorsInfo(LabeledSensorsInfoIn),
labeledThresholdParams(labeledThresholdParamsIn),
thresholds(std::move(thresholdsIn)),
@@ -82,7 +82,7 @@
});
dbusIface.register_property_r(
- "ReportNames", reportNames,
+ "ReportNames", reportIds,
sdbusplus::vtable::property_::emits_change,
[](const auto& x) { return x; });
@@ -125,7 +125,7 @@
data["TriggerActions"] = triggerActions;
data["ThresholdParams"] =
utils::labeledThresholdParamsToJson(labeledThresholdParams);
- data["ReportNames"] = reportNames;
+ data["ReportIds"] = reportIds;
data["Sensors"] = labeledSensorsInfo;
triggerStorage.store(fileName, data);
diff --git a/src/trigger.hpp b/src/trigger.hpp
index e1874e7..54f67c5 100644
--- a/src/trigger.hpp
+++ b/src/trigger.hpp
@@ -18,7 +18,7 @@
const std::shared_ptr<sdbusplus::asio::object_server>& objServer,
const std::string& id, const std::string& name,
const std::vector<std::string>& triggerActions,
- const std::vector<std::string>& reportNames,
+ const std::vector<std::string>& reportIds,
const std::vector<LabeledSensorInfo>& LabeledSensorsInfoIn,
const LabeledTriggerThresholdParams& labeledThresholdParamsIn,
std::vector<std::shared_ptr<interfaces::Threshold>>&& thresholds,
@@ -48,7 +48,7 @@
std::vector<std::string> triggerActions;
std::string path;
bool persistent = false;
- std::vector<std::string> reportNames;
+ std::vector<std::string> reportIds;
std::vector<LabeledSensorInfo> labeledSensorsInfo;
LabeledTriggerThresholdParams labeledThresholdParams;
std::unique_ptr<sdbusplus::asio::dbus_interface> deleteIface;
diff --git a/src/trigger_actions.cpp b/src/trigger_actions.cpp
index e89bd84..e159cb1 100644
--- a/src/trigger_actions.cpp
+++ b/src/trigger_actions.cpp
@@ -99,7 +99,7 @@
std::vector<std::unique_ptr<interfaces::TriggerAction>>& actionsIf,
const std::vector<TriggerAction>& ActionsEnum, ::numeric::Type type,
double thresholdValue, interfaces::ReportManager& reportManager,
- const std::vector<std::string>& reportNames)
+ const std::vector<std::string>& reportIds)
{
actionsIf.reserve(ActionsEnum.size());
for (auto actionType : ActionsEnum)
@@ -121,7 +121,7 @@
case TriggerAction::UpdateReport:
{
actionsIf.emplace_back(
- std::make_unique<UpdateReport>(reportManager, reportNames));
+ std::make_unique<UpdateReport>(reportManager, reportIds));
break;
}
}
@@ -185,7 +185,7 @@
std::vector<std::unique_ptr<interfaces::TriggerAction>>& actionsIf,
const std::vector<TriggerAction>& ActionsEnum,
::discrete::Severity severity, interfaces::ReportManager& reportManager,
- const std::vector<std::string>& reportNames)
+ const std::vector<std::string>& reportIds)
{
actionsIf.reserve(ActionsEnum.size());
for (auto actionType : ActionsEnum)
@@ -207,7 +207,7 @@
case TriggerAction::UpdateReport:
{
actionsIf.emplace_back(
- std::make_unique<UpdateReport>(reportManager, reportNames));
+ std::make_unique<UpdateReport>(reportManager, reportIds));
break;
}
}
@@ -241,7 +241,7 @@
std::vector<std::unique_ptr<interfaces::TriggerAction>>& actionsIf,
const std::vector<TriggerAction>& ActionsEnum,
interfaces::ReportManager& reportManager,
- const std::vector<std::string>& reportNames)
+ const std::vector<std::string>& reportIds)
{
actionsIf.reserve(ActionsEnum.size());
for (auto actionType : ActionsEnum)
@@ -261,7 +261,7 @@
case TriggerAction::UpdateReport:
{
actionsIf.emplace_back(
- std::make_unique<UpdateReport>(reportManager, reportNames));
+ std::make_unique<UpdateReport>(reportManager, reportIds));
break;
}
}
@@ -272,7 +272,7 @@
void UpdateReport::commit(const std::string&, uint64_t, double)
{
- for (const auto& name : reportNames)
+ for (const auto& name : reportIds)
{
reportManager.updateReport(name);
}
diff --git a/src/trigger_actions.hpp b/src/trigger_actions.hpp
index c180c3d..d86acb8 100644
--- a/src/trigger_actions.hpp
+++ b/src/trigger_actions.hpp
@@ -45,7 +45,7 @@
std::vector<std::unique_ptr<interfaces::TriggerAction>>& actionsIf,
const std::vector<TriggerAction>& ActionsEnum, ::numeric::Type type,
double thresholdValue, interfaces::ReportManager& reportManager,
- const std::vector<std::string>& reportNames);
+ const std::vector<std::string>& reportIds);
} // namespace numeric
namespace discrete
@@ -84,7 +84,7 @@
std::vector<std::unique_ptr<interfaces::TriggerAction>>& actionsIf,
const std::vector<TriggerAction>& ActionsEnum,
::discrete::Severity severity, interfaces::ReportManager& reportManager,
- const std::vector<std::string>& reportNames);
+ const std::vector<std::string>& reportIds);
namespace onChange
{
@@ -112,7 +112,7 @@
std::vector<std::unique_ptr<interfaces::TriggerAction>>& actionsIf,
const std::vector<TriggerAction>& ActionsEnum,
interfaces::ReportManager& reportManager,
- const std::vector<std::string>& reportNames);
+ const std::vector<std::string>& reportIds);
} // namespace onChange
} // namespace discrete
@@ -121,9 +121,9 @@
{
public:
UpdateReport(interfaces::ReportManager& reportManager,
- std::vector<std::string> names) :
+ std::vector<std::string> ids) :
reportManager(reportManager),
- reportNames(std::move(names))
+ reportIds(std::move(ids))
{}
void commit(const std::string& id, uint64_t timestamp,
@@ -131,6 +131,6 @@
private:
interfaces::ReportManager& reportManager;
- std::vector<std::string> reportNames;
+ std::vector<std::string> reportIds;
};
} // namespace action
diff --git a/src/trigger_factory.cpp b/src/trigger_factory.cpp
index d5294c4..0003360 100644
--- a/src/trigger_factory.cpp
+++ b/src/trigger_factory.cpp
@@ -23,7 +23,7 @@
std::unique_ptr<interfaces::Trigger> TriggerFactory::make(
const std::string& id, const std::string& name,
const std::vector<std::string>& triggerActionsIn,
- const std::vector<std::string>& reportNames,
+ const std::vector<std::string>& reportIds,
interfaces::TriggerManager& triggerManager,
interfaces::JsonStorage& triggerStorage,
const LabeledTriggerThresholdParams& labeledThresholdParams,
@@ -57,7 +57,7 @@
labeledThresholdParam.at_label<ts::ThresholdValue>();
action::discrete::fillActions(actions, triggerActions, severity,
- reportManager, reportNames);
+ reportManager, reportIds);
thresholds.emplace_back(std::make_shared<DiscreteThreshold>(
bus->get_io_context(), sensors, sensorNames, std::move(actions),
@@ -68,7 +68,7 @@
{
std::vector<std::unique_ptr<interfaces::TriggerAction>> actions;
action::discrete::onChange::fillActions(actions, triggerActions,
- reportManager, reportNames);
+ reportManager, reportIds);
thresholds.emplace_back(std::make_shared<OnChangeThreshold>(
sensors, sensorNames, std::move(actions)));
@@ -92,7 +92,7 @@
action::numeric::fillActions(actions, triggerActions, type,
thresholdValue, reportManager,
- reportNames);
+ reportIds);
thresholds.emplace_back(std::make_shared<NumericThreshold>(
bus->get_io_context(), sensors, sensorNames, std::move(actions),
@@ -101,9 +101,9 @@
}
return std::make_unique<Trigger>(
- bus->get_io_context(), objServer, id, name, triggerActionsIn,
- reportNames, labeledSensorsInfo, labeledThresholdParams,
- std::move(thresholds), triggerManager, triggerStorage);
+ bus->get_io_context(), objServer, id, name, triggerActionsIn, reportIds,
+ labeledSensorsInfo, labeledThresholdParams, std::move(thresholds),
+ triggerManager, triggerStorage);
}
std::pair<Sensors, std::vector<std::string>> TriggerFactory::getSensors(
@@ -116,10 +116,10 @@
{
const auto& service = labeledSensorInfo.at_label<ts::Service>();
const auto& sensorPath = labeledSensorInfo.at_label<ts::SensorPath>();
- const auto& metadata = labeledSensorInfo.at_label<ts::SensorMetadata>();
+ const auto& metadata = labeledSensorInfo.at_label<ts::Metadata>();
sensors.emplace_back(sensorCache.makeSensor<Sensor>(
- service, sensorPath, bus->get_io_context(), bus));
+ service, sensorPath, metadata, bus->get_io_context(), bus));
if (metadata.empty())
{
diff --git a/src/trigger_factory.hpp b/src/trigger_factory.hpp
index 757d663..9ae2197 100644
--- a/src/trigger_factory.hpp
+++ b/src/trigger_factory.hpp
@@ -18,7 +18,7 @@
std::unique_ptr<interfaces::Trigger>
make(const std::string& id, const std::string& name,
const std::vector<std::string>& triggerActions,
- const std::vector<std::string>& reportNames,
+ const std::vector<std::string>& reportIds,
interfaces::TriggerManager& triggerManager,
interfaces::JsonStorage& triggerStorage,
const LabeledTriggerThresholdParams& labeledThresholdParams,
diff --git a/src/trigger_manager.cpp b/src/trigger_manager.cpp
index ac0fba5..358ee78 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/generate_id.hpp"
#include "utils/transform.hpp"
#include <phosphor-logging/log.hpp>
@@ -24,7 +25,7 @@
const std::string& name,
const std::vector<std::string>& triggerActions,
const SensorsInfo& sensors,
- const std::vector<std::string>& reportNames,
+ const std::vector<std::string>& reportIds,
const TriggerThresholdParamsExt& thresholds) {
LabeledTriggerThresholdParams
labeledTriggerThresholdParams = std::visit(
@@ -35,7 +36,7 @@
triggerFactory->getLabeledSensorsInfo(yield, sensors);
return addTrigger(id, name, triggerActions,
- labeledSensorsInfo, reportNames,
+ labeledSensorsInfo, reportIds,
labeledTriggerThresholdParams)
.getPath();
});
@@ -61,7 +62,7 @@
}
verifyTriggerIdLength(triggerId);
- verifyIdCharacters(triggerId);
+ utils::verifyIdCharacters(triggerId);
for (const auto& trigger : triggers)
{
@@ -83,50 +84,21 @@
}
}
-void TriggerManager::verifyIdCharacters(const std::string& triggerId)
-{
- if (triggerId.find_first_not_of(allowedCharactersInId) != std::string::npos)
- {
- throw sdbusplus::exception::SdBusError(
- static_cast<int>(std::errc::invalid_argument),
- "Invalid character in trigger id");
- }
-}
-
std::string TriggerManager::generateId(const std::string& prefix,
const std::string& triggerName) const
{
- verifyIdCharacters(prefix);
- std::string strippedId(triggerName);
- strippedId.erase(std::remove_if(strippedId.begin(), strippedId.end(),
- [](char c) {
- return c == '/' ||
- allowedCharactersInId.find(c) ==
- std::string_view::npos;
- }),
- strippedId.end());
+ const auto existingTriggerIds = utils::transform(
+ triggers, [](const auto& trigger) { return trigger->getId(); });
- strippedId = prefix + strippedId;
- strippedId = strippedId.substr(
- 0, maxTriggerIdLength - std::to_string(maxTriggers - 1).length());
-
- size_t idx = 0;
- std::string tmpId(strippedId);
- while (std::find_if(triggers.begin(), triggers.end(),
- [&tmpId](const auto& trigger) {
- return trigger->getId() == tmpId;
- }) != triggers.end())
- {
- tmpId = strippedId + std::to_string(idx++);
- }
- return tmpId;
+ return utils::generateId(prefix, triggerName, existingTriggerIds,
+ maxTriggerIdLength);
}
interfaces::Trigger& TriggerManager::addTrigger(
const std::string& triggerIdIn, const std::string& triggerNameIn,
const std::vector<std::string>& triggerActions,
const std::vector<LabeledSensorInfo>& labeledSensorsInfo,
- const std::vector<std::string>& reportNames,
+ const std::vector<std::string>& reportIds,
const LabeledTriggerThresholdParams& labeledThresholdParams)
{
std::string triggerName = triggerNameIn;
@@ -135,16 +107,12 @@
triggerName = triggerNameDefault;
}
- std::string triggerId = triggerIdIn;
- if (triggerId.empty() || triggerId.ends_with('/'))
- {
- triggerId = generateId(triggerId, triggerName);
- }
+ std::string triggerId = generateId(triggerIdIn, triggerName);
verifyAddTrigger(triggerId, triggerName);
triggers.emplace_back(triggerFactory->make(
- triggerId, triggerName, triggerActions, reportNames, *this,
+ triggerId, triggerName, triggerActions, reportIds, *this,
*triggerStorage, labeledThresholdParams, labeledSensorsInfo));
return *triggers.back();
@@ -190,14 +158,14 @@
.get<std::vector<discrete::LabeledThresholdParam>>();
}
- auto reportNames =
- data->at("ReportNames").get<std::vector<std::string>>();
+ auto reportIds =
+ data->at("ReportIds").get<std::vector<std::string>>();
auto labeledSensorsInfo =
data->at("Sensors").get<std::vector<LabeledSensorInfo>>();
- addTrigger(id, name, triggerActions, labeledSensorsInfo,
- reportNames, labeledThresholdParams);
+ addTrigger(id, name, triggerActions, labeledSensorsInfo, reportIds,
+ labeledThresholdParams);
}
catch (const std::exception& e)
{
diff --git a/src/trigger_manager.hpp b/src/trigger_manager.hpp
index 4837062..5231546 100644
--- a/src/trigger_manager.hpp
+++ b/src/trigger_manager.hpp
@@ -36,13 +36,12 @@
std::string generateId(const std::string& prefix,
const std::string& triggerName) const;
static void verifyTriggerIdLength(const std::string& triggerId);
- static void verifyIdCharacters(const std::string& triggerId);
interfaces::Trigger&
addTrigger(const std::string& triggerId, const std::string& triggerName,
const std::vector<std::string>& triggerActions,
const std::vector<LabeledSensorInfo>& labeledSensors,
- const std::vector<std::string>& reportNames,
+ const std::vector<std::string>& reportIds,
const LabeledTriggerThresholdParams& labeledThresholdParams);
void loadFromPersistent();
@@ -55,8 +54,5 @@
"xyz.openbmc_project.Telemetry.TriggerManager";
static constexpr const char* triggerManagerPath =
"/xyz/openbmc_project/Telemetry/Triggers";
-
- static constexpr std::string_view allowedCharactersInId =
- "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_/";
static constexpr const char* triggerNameDefault = "Trigger";
};
diff --git a/src/types/operation_type.hpp b/src/types/operation_type.hpp
index 5665e79..29bceb4 100644
--- a/src/types/operation_type.hpp
+++ b/src/types/operation_type.hpp
@@ -22,13 +22,13 @@
convDataOperationType = {
{std::make_pair<std::string_view, OperationType>("SINGLE",
OperationType::single),
- std::make_pair<std::string_view, OperationType>("MAX",
+ std::make_pair<std::string_view, OperationType>("Maximum",
OperationType::max),
- std::make_pair<std::string_view, OperationType>("MIN",
+ std::make_pair<std::string_view, OperationType>("Minimum",
OperationType::min),
- std::make_pair<std::string_view, OperationType>("AVG",
+ std::make_pair<std::string_view, OperationType>("Average",
OperationType::avg),
- std::make_pair<std::string_view, OperationType>("SUM",
+ std::make_pair<std::string_view, OperationType>("Summation",
OperationType::sum)}};
inline OperationType
diff --git a/src/types/report_types.cpp b/src/types/report_types.cpp
index f6979cc..47add24 100644
--- a/src/types/report_types.cpp
+++ b/src/types/report_types.cpp
@@ -13,12 +13,13 @@
utils::transform(
metricParams.at_label<ts::SensorPath>(),
[](const LabeledSensorParameters& sensorParameters) {
- return sdbusplus::message::object_path(
- sensorParameters.at_label<ts::Path>());
+ return std::tuple<sdbusplus::message::object_path,
+ std::string>(
+ sensorParameters.at_label<ts::Path>(),
+ sensorParameters.at_label<ts::Metadata>());
}),
utils::enumToString(metricParams.at_label<ts::OperationType>()),
metricParams.at_label<ts::Id>(),
- metricParams.at_label<ts::MetricMetadata>(),
utils::enumToString(
metricParams.at_label<ts::CollectionTimeScope>()),
metricParams.at_label<ts::CollectionDuration>().t.count());
diff --git a/src/types/report_types.hpp b/src/types/report_types.hpp
index 9d4a160..978fd18 100644
--- a/src/types/report_types.hpp
+++ b/src/types/report_types.hpp
@@ -18,20 +18,21 @@
std::vector<std::tuple<sdbusplus::message::object_path, std::string,
std::string, std::string>>;
-using ReadingParameters = std::vector<
- std::tuple<std::vector<sdbusplus::message::object_path>, std::string,
- std::string, std::string, std::string, uint64_t>>;
+using ReadingParameters = std::vector<std::tuple<
+ std::vector<std::tuple<sdbusplus::message::object_path, std::string>>,
+ std::string, std::string, std::string, uint64_t>>;
using LabeledSensorParameters =
- utils::LabeledTuple<std::tuple<std::string, std::string>,
- utils::tstring::Service, utils::tstring::Path>;
+ utils::LabeledTuple<std::tuple<std::string, std::string, std::string>,
+ utils::tstring::Service, utils::tstring::Path,
+ utils::tstring::Metadata>;
using LabeledMetricParameters = utils::LabeledTuple<
std::tuple<std::vector<LabeledSensorParameters>, OperationType, std::string,
- std::string, CollectionTimeScope, CollectionDuration>,
+ CollectionTimeScope, CollectionDuration>,
utils::tstring::SensorPath, utils::tstring::OperationType,
- utils::tstring::Id, utils::tstring::MetricMetadata,
- utils::tstring::CollectionTimeScope, utils::tstring::CollectionDuration>;
+ utils::tstring::Id, utils::tstring::CollectionTimeScope,
+ utils::tstring::CollectionDuration>;
using ReadingData = std::tuple<std::string, std::string, double, uint64_t>;
diff --git a/src/types/trigger_types.hpp b/src/types/trigger_types.hpp
index 0ad9087..b21e8eb 100644
--- a/src/types/trigger_types.hpp
+++ b/src/types/trigger_types.hpp
@@ -138,7 +138,7 @@
using LabeledSensorInfo =
utils::LabeledTuple<std::tuple<std::string, std::string, std::string>,
utils::tstring::Service, utils::tstring::SensorPath,
- utils::tstring::SensorMetadata>;
+ utils::tstring::Metadata>;
using TriggerThresholdParamsExt =
std::variant<std::monostate, std::vector<numeric::ThresholdParam>,
diff --git a/src/utils/conversion.hpp b/src/utils/conversion.hpp
index 222b990..fd3dbcb 100644
--- a/src/utils/conversion.hpp
+++ b/src/utils/conversion.hpp
@@ -1,5 +1,7 @@
#pragma once
+#include <sdbusplus/exception.hpp>
+
#include <algorithm>
#include <array>
#include <stdexcept>
@@ -13,7 +15,9 @@
{
[[noreturn]] static void throwConversionError()
{
- throw std::out_of_range("Value is not in range of enum");
+ throw sdbusplus::exception::SdBusError(
+ static_cast<int>(std::errc::invalid_argument),
+ "Invalid enum value");
}
};
diff --git a/src/utils/conversion_trigger.cpp b/src/utils/conversion_trigger.cpp
index aec0f28..ab06a48 100644
--- a/src/utils/conversion_trigger.cpp
+++ b/src/utils/conversion_trigger.cpp
@@ -73,7 +73,7 @@
return utils::transform(infos, [](const LabeledSensorInfo& val) {
return SensorsInfo::value_type(
sdbusplus::message::object_path(val.at_label<ts::SensorPath>()),
- val.at_label<ts::SensorMetadata>());
+ val.at_label<ts::Metadata>());
});
}
diff --git a/src/utils/generate_id.cpp b/src/utils/generate_id.cpp
new file mode 100644
index 0000000..adb1a5b
--- /dev/null
+++ b/src/utils/generate_id.cpp
@@ -0,0 +1,64 @@
+#include "utils/generate_id.hpp"
+
+#include <sdbusplus/exception.hpp>
+
+#include <system_error>
+
+namespace utils
+{
+namespace details
+{
+
+static constexpr std::string_view allowedCharactersInId =
+ "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_/";
+
+}
+
+void verifyIdCharacters(std::string_view triggerId)
+{
+ if (triggerId.find_first_not_of(details::allowedCharactersInId) !=
+ std::string::npos)
+ {
+ throw sdbusplus::exception::SdBusError(
+ static_cast<int>(std::errc::invalid_argument),
+ "Invalid character in id");
+ }
+}
+
+std::string generateId(std::string_view prefix, std::string_view name,
+ const std::vector<std::string>& conflictIds,
+ size_t maxLength)
+{
+ verifyIdCharacters(prefix);
+
+ if (!prefix.empty() && !prefix.ends_with('/'))
+ {
+ return std::string(prefix);
+ }
+
+ 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;
+ }),
+ strippedId.end());
+ strippedId = std::string(prefix) + strippedId;
+
+ size_t idx = 0;
+ std::string tmpId = strippedId.substr(0, maxLength);
+
+ while (std::find(conflictIds.begin(), conflictIds.end(), tmpId) !=
+ conflictIds.end() ||
+ tmpId.empty())
+ {
+ tmpId = strippedId.substr(0, maxLength - std::to_string(idx).length()) +
+ std::to_string(idx);
+ ++idx;
+ }
+ return tmpId;
+}
+
+} // namespace utils
diff --git a/src/utils/generate_id.hpp b/src/utils/generate_id.hpp
new file mode 100644
index 0000000..aa9508e
--- /dev/null
+++ b/src/utils/generate_id.hpp
@@ -0,0 +1,15 @@
+#pragma once
+
+#include <string>
+#include <string_view>
+#include <vector>
+
+namespace utils
+{
+
+void verifyIdCharacters(std::string_view triggerId);
+std::string generateId(std::string_view prefix, std::string_view triggerName,
+ const std::vector<std::string>& conflictIds,
+ size_t maxLength);
+
+} // namespace utils
diff --git a/src/utils/labeled_tuple.hpp b/src/utils/labeled_tuple.hpp
index 398e4be..55789e9 100644
--- a/src/utils/labeled_tuple.hpp
+++ b/src/utils/labeled_tuple.hpp
@@ -167,6 +167,8 @@
}
else
{
+ static_assert(Idx + 1 < sizeof...(Args),
+ "Label not found in LabeledTuple");
return find_item<Idx + 1, Label>(self);
}
}
diff --git a/src/utils/tstring.hpp b/src/utils/tstring.hpp
index f0bc679..e4529d8 100644
--- a/src/utils/tstring.hpp
+++ b/src/utils/tstring.hpp
@@ -24,11 +24,11 @@
}
};
-struct SensorMetadata
+struct Metadata
{
static std::string str()
{
- return "sensorMetadata";
+ return "metadata";
}
};
@@ -40,14 +40,6 @@
}
};
-struct MetricMetadata
-{
- static std::string str()
- {
- return "metricMetadata";
- }
-};
-
struct Service
{
static std::string str()
diff --git a/tests/meson.build b/tests/meson.build
index 3059953..27cdb42 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -28,12 +28,14 @@
'../src/trigger_manager.cpp',
'../src/types/report_types.cpp',
'../src/utils/conversion_trigger.cpp',
+ '../src/utils/generate_id.cpp',
'src/dbus_environment.cpp',
'src/main.cpp',
'src/stubs/dbus_sensor_object.cpp',
'src/test_conversion.cpp',
'src/test_detached_timer.cpp',
'src/test_discrete_threshold.cpp',
+ 'src/test_generate_id.cpp',
'src/test_metric.cpp',
'src/test_numeric_threshold.cpp',
'src/test_on_change_threshold.cpp',
diff --git a/tests/src/mocks/report_factory_mock.hpp b/tests/src/mocks/report_factory_mock.hpp
index cc0bc8e..58c71d6 100644
--- a/tests/src/mocks/report_factory_mock.hpp
+++ b/tests/src/mocks/report_factory_mock.hpp
@@ -15,14 +15,15 @@
return utils::transform(readingParams, [](const auto& params) {
return LabeledMetricParameters(
utils::transform(std::get<0>(params),
- [](const auto& sensorPath) {
- return LabeledSensorParameters("Service",
- sensorPath);
+ [](const auto& sensorData) {
+ return LabeledSensorParameters(
+ "Service", std::get<0>(sensorData),
+ std::get<1>(sensorData));
}),
utils::toOperationType(std::get<1>(params)),
- std::get<2>(params), std::get<3>(params),
- utils::toCollectionTimeScope(std::get<4>(params)),
- CollectionDuration(Milliseconds(std::get<5>(params))));
+ std::get<2>(params),
+ utils::toCollectionTimeScope(std::get<3>(params)),
+ CollectionDuration(Milliseconds(std::get<4>(params))));
});
}
@@ -35,9 +36,10 @@
.WillByDefault(
WithArgs<1>(Invoke(&ReportFactoryMock::convertToLabeled)));
- ON_CALL(*this, make(A<const std::string&>(), _, _, _, _, _, _, _, _, _))
- .WillByDefault(WithArgs<0>(Invoke([](const std::string& name) {
- return std::make_unique<NiceMock<ReportMock>>(name);
+ ON_CALL(*this,
+ make(A<const std::string&>(), _, _, _, _, _, _, _, _, _, _))
+ .WillByDefault(WithArgs<0>(Invoke([](const std::string& id) {
+ return std::make_unique<NiceMock<ReportMock>>(id);
})));
}
@@ -46,7 +48,7 @@
(const, override));
MOCK_METHOD(std::unique_ptr<interfaces::Report>, make,
- (const std::string&, const ReportingType,
+ (const std::string&, const std::string&, const ReportingType,
const std::vector<ReportAction>&, Milliseconds, uint64_t,
const ReportUpdates, interfaces::ReportManager&,
interfaces::JsonStorage&, std::vector<LabeledMetricParameters>,
@@ -62,15 +64,16 @@
{
const ReportParams& params = *paramsRef;
return EXPECT_CALL(
- *this, make(params.reportName(), params.reportingType(),
- params.reportActions(), params.interval(),
- params.appendLimit(), params.reportUpdates(), rm,
- js, params.metricParameters(), params.enabled()));
+ *this, make(params.reportId(), params.reportName(),
+ params.reportingType(), params.reportActions(),
+ params.interval(), params.appendLimit(),
+ params.reportUpdates(), rm, js,
+ 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_mock.hpp b/tests/src/mocks/report_mock.hpp
index 726d53c..6a6e259 100644
--- a/tests/src/mocks/report_mock.hpp
+++ b/tests/src/mocks/report_mock.hpp
@@ -7,12 +7,12 @@
class ReportMock : public interfaces::Report
{
public:
- ReportMock(std::string name)
+ ReportMock(const std::string& id)
{
using namespace testing;
- ON_CALL(*this, getName).WillByDefault([name] { return name; });
- ON_CALL(*this, getPath).WillByDefault([name] { return "/" + name; });
+ ON_CALL(*this, getId).WillByDefault([id] { return id; });
+ ON_CALL(*this, getPath).WillByDefault([id] { return "/" + id; });
EXPECT_CALL(*this, Die).Times(AnyNumber());
}
@@ -21,7 +21,7 @@
Die();
}
- MOCK_METHOD(std::string, getName, (), (override, const));
+ MOCK_METHOD(std::string, getId, (), (override, const));
MOCK_METHOD(std::string, getPath, (), (override, const));
MOCK_METHOD(void, updateReadings, (), (override));
MOCK_METHOD(void, Die, ());
diff --git a/tests/src/mocks/sensor_mock.hpp b/tests/src/mocks/sensor_mock.hpp
index b86a26e..9ff72e2 100644
--- a/tests/src/mocks/sensor_mock.hpp
+++ b/tests/src/mocks/sensor_mock.hpp
@@ -24,6 +24,7 @@
}
MOCK_METHOD(Id, id, (), (const, override));
+ MOCK_METHOD(std::string, metadata, (), (const, override));
MOCK_METHOD(void, registerForUpdates,
(const std::weak_ptr<interfaces::SensorListener>&), (override));
MOCK_METHOD(void, unregisterFromUpdates,
diff --git a/tests/src/mocks/trigger_factory_mock.hpp b/tests/src/mocks/trigger_factory_mock.hpp
index ab7510d..9fa2f48 100644
--- a/tests/src/mocks/trigger_factory_mock.hpp
+++ b/tests/src/mocks/trigger_factory_mock.hpp
@@ -24,7 +24,7 @@
MOCK_METHOD(std::unique_ptr<interfaces::Trigger>, make,
(const std::string& id, const std::string& name,
const std::vector<std::string>& triggerActions,
- const std::vector<std::string>& reportNames,
+ const std::vector<std::string>& reportIds,
interfaces::TriggerManager& triggerManager,
interfaces::JsonStorage& triggerStorage,
const LabeledTriggerThresholdParams& labeledThresholdParams,
@@ -54,7 +54,7 @@
return EXPECT_CALL(
*this, make(params.id(), params.name(), params.triggerActions(),
- params.reportNames(), tm, triggerStorage,
+ params.reportIds(), tm, triggerStorage,
params.thresholdParams(), params.sensors()));
}
else
diff --git a/tests/src/params/metric_params.hpp b/tests/src/params/metric_params.hpp
index 4654060..90db484 100644
--- a/tests/src/params/metric_params.hpp
+++ b/tests/src/params/metric_params.hpp
@@ -35,17 +35,6 @@
return idProperty;
}
- MetricParams& metadata(std::string val)
- {
- metadataProperty = std::move(val);
- return *this;
- }
-
- const std::string& metadata() const
- {
- return metadataProperty;
- }
-
MetricParams& collectionTimeScope(CollectionTimeScope val)
{
collectionTimeScopeProperty = val;
@@ -93,7 +82,6 @@
private:
OperationType operationTypeProperty = {};
std::string idProperty = "MetricId";
- std::string metadataProperty = "MetricMetadata";
CollectionTimeScope collectionTimeScopeProperty = {};
CollectionDuration collectionDurationProperty =
CollectionDuration(Milliseconds(0u));
diff --git a/tests/src/params/report_params.hpp b/tests/src/params/report_params.hpp
index 44ae901..47e74e3 100644
--- a/tests/src/params/report_params.hpp
+++ b/tests/src/params/report_params.hpp
@@ -9,6 +9,17 @@
class ReportParams final
{
public:
+ ReportParams& reportId(std::string val)
+ {
+ reportIdProperty = std::move(val);
+ return *this;
+ }
+
+ const std::string& reportId() const
+ {
+ return reportIdProperty;
+ }
+
ReportParams& reportName(std::string val)
{
reportNameProperty = std::move(val);
@@ -98,6 +109,7 @@
}
private:
+ std::string reportIdProperty = "TestId";
std::string reportNameProperty = "TestReport";
ReportingType reportingTypeProperty = ReportingType::onRequest;
std::vector<ReportAction> reportActionsProperty;
@@ -107,18 +119,18 @@
std::vector<LabeledMetricParameters> metricParametersProperty{
{LabeledMetricParameters{
{LabeledSensorParameters{"Service",
- "/xyz/openbmc_project/sensors/power/p1"}},
+ "/xyz/openbmc_project/sensors/power/p1",
+ "metadata1"}},
OperationType::single,
"MetricId1",
- "Metadata1",
CollectionTimeScope::point,
CollectionDuration(Milliseconds(0u))},
LabeledMetricParameters{
{LabeledSensorParameters{"Service",
- "/xyz/openbmc_project/sensors/power/p2"}},
+ "/xyz/openbmc_project/sensors/power/p2",
+ "metadata2"}},
OperationType::single,
"MetricId2",
- "Metadata2",
CollectionTimeScope::point,
CollectionDuration(Milliseconds(0u))}}};
bool enabledProperty = true;
diff --git a/tests/src/params/trigger_params.hpp b/tests/src/params/trigger_params.hpp
index 62acd7a..ea309e6 100644
--- a/tests/src/params/trigger_params.hpp
+++ b/tests/src/params/trigger_params.hpp
@@ -49,9 +49,9 @@
return labeledSensorsProperty;
}
- const std::vector<std::string>& reportNames() const
+ const std::vector<std::string>& reportIds() const
{
- return reportNamesProperty;
+ return reportIdsProperty;
}
TriggerParams& thresholdParams(LabeledTriggerThresholdParams val)
@@ -73,7 +73,7 @@
std::vector<LabeledSensorInfo> labeledSensorsProperty = {
{"service1", "/xyz/openbmc_project/sensors/temperature/BMC_Temp",
"metadata1"}};
- std::vector<std::string> reportNamesProperty = {"Report1"};
+ std::vector<std::string> reportIdsProperty = {"Report1"};
LabeledTriggerThresholdParams labeledThresholdsProperty =
std::vector<numeric::LabeledThresholdParam>{
numeric::LabeledThresholdParam{numeric::Type::lowerCritical,
diff --git a/tests/src/test_conversion.cpp b/tests/src/test_conversion.cpp
index a61ff1b..2b7249c 100644
--- a/tests/src/test_conversion.cpp
+++ b/tests/src/test_conversion.cpp
@@ -43,10 +43,10 @@
EXPECT_EQ(toEnum(2), Enum::two);
}
-TEST_F(TestConversion, passInvalidValueExpectToThrowOutOfRangeException)
+TEST_F(TestConversion, passInvalidValueExpectToThrowException)
{
- EXPECT_THROW(toEnum(-1), std::out_of_range);
- EXPECT_THROW(toEnum(3), std::out_of_range);
+ EXPECT_THROW(toEnum(-1), sdbusplus::exception::SdBusError);
+ EXPECT_THROW(toEnum(3), sdbusplus::exception::SdBusError);
}
TEST_F(TestConversion, convertsToUnderlyingType)
@@ -72,10 +72,11 @@
TEST_F(TestConversion, enumToStringThrowsWhenUknownEnumPassed)
{
- EXPECT_THROW(enumToString(static_cast<Enum>(77)), std::out_of_range);
+ EXPECT_THROW(enumToString(static_cast<Enum>(77)),
+ sdbusplus::exception::SdBusError);
}
TEST_F(TestConversion, toEnumThrowsWhenUknownStringPassed)
{
- EXPECT_THROW(toEnum("four"), std::out_of_range);
+ EXPECT_THROW(toEnum("four"), sdbusplus::exception::SdBusError);
}
diff --git a/tests/src/test_generate_id.cpp b/tests/src/test_generate_id.cpp
new file mode 100644
index 0000000..0bfabf5
--- /dev/null
+++ b/tests/src/test_generate_id.cpp
@@ -0,0 +1,67 @@
+#include "utils/generate_id.hpp"
+
+#include <gmock/gmock.h>
+
+using namespace testing;
+
+class TestGenerateId : public Test
+{
+ public:
+ std::vector<std::string> conflicts;
+};
+
+TEST_F(TestGenerateId, returnPrefixWhenPrefixIsId)
+{
+ constexpr size_t maxSize = 32;
+ EXPECT_THAT(utils::generateId("prefix", "name", conflicts, maxSize),
+ Eq("prefix"));
+ EXPECT_THAT(utils::generateId("pre", "name123", conflicts, maxSize),
+ Eq("pre"));
+ EXPECT_THAT(utils::generateId("prefix/abc", "name", conflicts, maxSize),
+ Eq("prefix/abc"));
+ EXPECT_THAT(utils::generateId("prefix/abc", "name",
+ {"conflicts", "prefix/abc"}, maxSize),
+ Eq("prefix/abc"));
+}
+
+TEST_F(TestGenerateId, returnsNameWithPrefixWhenPrefixIsNamesapce)
+{
+ constexpr size_t maxSize = 32;
+ EXPECT_THAT(utils::generateId("prefix/", "name", conflicts, maxSize),
+ Eq("prefix/name"));
+ EXPECT_THAT(utils::generateId("pre/", "name", conflicts, maxSize),
+ Eq("pre/name"));
+}
+
+TEST_F(TestGenerateId, returnsOriginalNameWithoutInvalidCharacters)
+{
+ constexpr size_t maxSize = 32;
+ EXPECT_THAT(utils::generateId("", "n#a$m@e", conflicts, maxSize),
+ Eq("name"));
+ EXPECT_THAT(utils::generateId("/", "n!^aŹme", conflicts, maxSize),
+ Eq("/name"));
+ EXPECT_THAT(utils::generateId("p/", "n!^aŹm*(e", conflicts, maxSize),
+ Eq("p/name"));
+}
+
+TEST_F(TestGenerateId, returnsUniqueIdWhenConflictExist)
+{
+ constexpr size_t maxSize = 32;
+
+ EXPECT_THAT(utils::generateId("p/", "name",
+ {"conflicts", "p/name", "p/name1"}, maxSize),
+ Eq("p/name0"));
+ EXPECT_THAT(utils::generateId("p/", "name",
+ {"conflicts", "p/name", "p/name0", "p/name1"},
+ maxSize),
+ Eq("p/name2"));
+}
+
+TEST_F(TestGenerateId, returnsUniqueIdWithingMaxSize)
+{
+ constexpr size_t maxSize = 4;
+
+ EXPECT_THAT(utils::generateId("", "trigger", {""}, maxSize), Eq("trig"));
+ EXPECT_THAT(utils::generateId("", "trigger", {"trig"}, maxSize),
+ Eq("tri0"));
+}
diff --git a/tests/src/test_metric.cpp b/tests/src/test_metric.cpp
index a8c3d86..e19446c 100644
--- a/tests/src/test_metric.cpp
+++ b/tests/src/test_metric.cpp
@@ -25,7 +25,9 @@
std::vector<std::shared_ptr<SensorMock>> result;
for (size_t i = 0; i < amount; ++i)
{
- result.emplace_back(std::make_shared<NiceMock<SensorMock>>());
+ auto& metricMock =
+ result.emplace_back(std::make_shared<NiceMock<SensorMock>>());
+ ON_CALL(*metricMock, metadata()).WillByDefault(Return("metadata"));
}
return result;
}
@@ -35,13 +37,12 @@
return std::make_shared<Metric>(
utils::convContainer<std::shared_ptr<interfaces::Sensor>>(
sensorMocks),
- p.operationType(), p.id(), p.metadata(), p.collectionTimeScope(),
+ p.operationType(), p.id(), p.collectionTimeScope(),
p.collectionDuration(), std::move(clockFakePtr));
}
MetricParams params = MetricParams()
.id("id")
- .metadata("metadata")
.operationType(OperationType::avg)
.collectionTimeScope(CollectionTimeScope::point)
.collectionDuration(CollectionDuration(0ms));
@@ -83,51 +84,6 @@
ElementsAre(MetricValue({"id", "metadata", 0., 0u})));
}
-TEST_F(TestMetric, parsesSensorMetadata)
-{
- using ReadingMetadata =
- utils::LabeledTuple<std::tuple<std::string, std::string>,
- utils::tstring::SensorDbusPath,
- utils::tstring::SensorRedfishUri>;
-
- nlohmann::json metadata;
- metadata["MetricProperties"] = {"sensor1", "sensor2"};
-
- sensorMocks = makeSensorMocks(2);
- sut = makeSut(params.metadata(metadata.dump()));
-
- EXPECT_THAT(
- sut->getReadings(),
- ElementsAre(
- MetricValue{"id", ReadingMetadata("", "sensor1").dump(), 0., 0u},
- MetricValue{"id", ReadingMetadata("", "sensor2").dump(), 0., 0u}));
-}
-
-TEST_F(TestMetric, parsesSensorMetadataWhenMoreMetadataThanSensors)
-{
- nlohmann::json metadata;
- metadata["MetricProperties"] = {"sensor1", "sensor2"};
-
- sensorMocks = makeSensorMocks(1);
- sut = makeSut(params.metadata(metadata.dump()));
-
- EXPECT_THAT(sut->getReadings(),
- ElementsAre(MetricValue{"id", metadata.dump(), 0., 0u}));
-}
-
-TEST_F(TestMetric, parsesSensorMetadataWhenMoreSensorsThanMetadata)
-{
- nlohmann::json metadata;
- metadata["MetricProperties"] = {"sensor1"};
-
- sensorMocks = makeSensorMocks(2);
- sut = makeSut(params.metadata(metadata.dump()));
-
- EXPECT_THAT(sut->getReadings(),
- ElementsAre(MetricValue{"id", metadata.dump(), 0., 0u},
- MetricValue{"id", metadata.dump(), 0., 0u}));
-}
-
class TestMetricAfterInitialization : public TestMetric
{
public:
@@ -167,17 +123,18 @@
ON_CALL(*sensorMocks.front(), id())
.WillByDefault(Return(SensorMock::makeId("service1", "path1")));
+ ON_CALL(*sensorMocks.front(), metadata())
+ .WillByDefault(Return("metadata1"));
const auto conf = sut->dumpConfiguration();
LabeledMetricParameters expected = {};
expected.at_label<ts::Id>() = params.id();
- expected.at_label<ts::MetricMetadata>() = params.metadata();
expected.at_label<ts::OperationType>() = params.operationType();
expected.at_label<ts::CollectionTimeScope>() = params.collectionTimeScope();
expected.at_label<ts::CollectionDuration>() = params.collectionDuration();
expected.at_label<ts::SensorPath>() = {
- LabeledSensorParameters("service1", "path1")};
+ LabeledSensorParameters("service1", "path1", "metadata1")};
EXPECT_THAT(conf, Eq(expected));
}
diff --git a/tests/src/test_report.cpp b/tests/src/test_report.cpp
index 7a56dac..1beb15f 100644
--- a/tests/src/test_report.cpp
+++ b/tests/src/test_report.cpp
@@ -6,6 +6,7 @@
#include "params/report_params.hpp"
#include "report.hpp"
#include "report_manager.hpp"
+#include "utils/clock.hpp"
#include "utils/contains.hpp"
#include "utils/conv_container.hpp"
#include "utils/transform.hpp"
@@ -58,10 +59,10 @@
sut = makeReport(ReportParams());
}
- static interfaces::JsonStorage::FilePath to_file_path(std::string name)
+ static interfaces::JsonStorage::FilePath to_file_path(std::string id)
{
return interfaces::JsonStorage::FilePath(
- std::to_string(std::hash<std::string>{}(name)));
+ std::to_string(std::hash<std::string>{}(id)));
}
std::unique_ptr<Report> makeReport(const ReportParams& params)
@@ -70,9 +71,9 @@
return std::make_unique<Report>(
DbusEnvironment::getIoc(), DbusEnvironment::getObjServer(),
- params.reportName(), params.reportingType(), params.reportActions(),
- params.interval(), params.appendLimit(), params.reportUpdates(),
- *reportManagerMock, storageMock,
+ params.reportId(), params.reportName(), params.reportingType(),
+ params.reportActions(), params.interval(), params.appendLimit(),
+ params.reportUpdates(), *reportManagerMock, storageMock,
utils::convContainer<std::shared_ptr<interfaces::Metric>>(
metricMocks),
params.enabled());
@@ -118,6 +119,11 @@
}
};
+TEST_F(TestReport, returnsId)
+{
+ EXPECT_THAT(sut->getId(), Eq(defaultParams.reportId()));
+}
+
TEST_F(TestReport, verifyIfPropertiesHaveValidValue)
{
EXPECT_THAT(getProperty<bool>(sut->getPath(), "Enabled"),
@@ -146,6 +152,8 @@
EXPECT_THAT(getProperty<ReadingParameters>(
sut->getPath(), "ReadingParametersFutureVersion"),
Eq(toReadingParameters(defaultParams.metricParameters())));
+ EXPECT_THAT(getProperty<std::string>(sut->getPath(), "Name"),
+ Eq(defaultParams.reportName()));
}
TEST_F(TestReport, readingsAreInitialyEmpty)
@@ -204,7 +212,7 @@
TEST_F(TestReport, settingPersistencyToFalseRemovesReportFromStorage)
{
- EXPECT_CALL(storageMock, remove(to_file_path(sut->getName())));
+ EXPECT_CALL(storageMock, remove(to_file_path(sut->getId())));
bool persistency = false;
EXPECT_THAT(setProperty(sut->getPath(), "Persistency", persistency).value(),
@@ -228,7 +236,7 @@
TEST_F(TestReport, deleteReportExpectThatFileIsRemoveFromStorage)
{
- EXPECT_CALL(storageMock, remove(to_file_path(sut->getName())));
+ EXPECT_CALL(storageMock, remove(to_file_path(sut->getId())));
auto ec = deleteReport(sut->getPath());
EXPECT_THAT(ec, Eq(boost::system::errc::success));
}
@@ -248,6 +256,7 @@
_, TestReportStore,
Values(std::make_pair("Enabled"s, nlohmann::json(ReportParams().enabled())),
std::make_pair("Version"s, nlohmann::json(6)),
+ std::make_pair("Id"s, nlohmann::json(ReportParams().reportId())),
std::make_pair("Name"s, nlohmann::json(ReportParams().reportName())),
std::make_pair("ReportingType",
nlohmann::json(ReportParams().reportingType())),
@@ -265,20 +274,20 @@
{{{tstring::SensorPath::str(),
{{{tstring::Service::str(), "Service"},
{tstring::Path::str(),
- "/xyz/openbmc_project/sensors/power/p1"}}}},
+ "/xyz/openbmc_project/sensors/power/p1"},
+ {tstring::Metadata::str(), "metadata1"}}}},
{tstring::OperationType::str(), OperationType::single},
{tstring::Id::str(), "MetricId1"},
- {tstring::MetricMetadata::str(), "Metadata1"},
{tstring::CollectionTimeScope::str(),
CollectionTimeScope::point},
{tstring::CollectionDuration::str(), 0}},
{{tstring::SensorPath::str(),
{{{tstring::Service::str(), "Service"},
{tstring::Path::str(),
- "/xyz/openbmc_project/sensors/power/p2"}}}},
+ "/xyz/openbmc_project/sensors/power/p2"},
+ {tstring::Metadata::str(), "metadata2"}}}},
{tstring::OperationType::str(), OperationType::single},
{tstring::Id::str(), "MetricId2"},
- {tstring::MetricMetadata::str(), "Metadata2"},
{tstring::CollectionTimeScope::str(),
CollectionTimeScope::point},
{tstring::CollectionDuration::str(), 0}}}))));
@@ -289,9 +298,9 @@
{
InSequence seq;
- EXPECT_CALL(storageMock, remove(to_file_path(sut->getName())));
+ EXPECT_CALL(storageMock, remove(to_file_path(sut->getId())));
EXPECT_CALL(checkPoint, Call());
- EXPECT_CALL(storageMock, store(to_file_path(sut->getName()), _))
+ EXPECT_CALL(storageMock, store(to_file_path(sut->getId()), _))
.WillOnce(SaveArg<1>(&storedConfiguration));
}
@@ -306,8 +315,7 @@
TEST_P(TestReportStore, reportIsSavedToStorageAfterCreated)
{
- EXPECT_CALL(storageMock,
- store(to_file_path(ReportParams().reportName()), _))
+ EXPECT_CALL(storageMock, store(to_file_path(ReportParams().reportId()), _))
.WillOnce(SaveArg<1>(&storedConfiguration));
sut = makeReport(ReportParams());
@@ -337,7 +345,7 @@
EXPECT_NO_THROW(makeReport(GetParam()));
}
-class TestReportInvalidNames :
+class TestReportInvalidIds :
public TestReport,
public WithParamInterface<ReportParams>
{
@@ -346,23 +354,18 @@
{}
};
-INSTANTIATE_TEST_SUITE_P(InvalidNames, TestReportInvalidNames,
- Values(ReportParams().reportName("/"),
- ReportParams().reportName("/Invalid"),
- ReportParams().reportName("Invalid/"),
- ReportParams().reportName("Invalid/Invalid/"),
- ReportParams().reportName("Invalid?")));
+INSTANTIATE_TEST_SUITE_P(InvalidNames, TestReportInvalidIds,
+ Values(ReportParams().reportId("/"),
+ ReportParams().reportId("/Invalid"),
+ ReportParams().reportId("Invalid/"),
+ ReportParams().reportId("Invalid/Invalid/"),
+ ReportParams().reportId("Invalid?")));
-TEST_P(TestReportInvalidNames, reportCtorThrowOnInvalidName)
-{
- EXPECT_THROW(makeReport(GetParam()), sdbusplus::exception::SdBusError);
-}
-
-TEST_F(TestReportInvalidNames, reportCtorThrowOnInvalidNameAndNoStoreIsCalled)
+TEST_P(TestReportInvalidIds, failsToCreateReportWithInvalidName)
{
EXPECT_CALL(storageMock, store).Times(0);
- EXPECT_THROW(makeReport(ReportParams().reportName("/Invalid")),
- sdbusplus::exception::SdBusError);
+
+ EXPECT_THROW(makeReport(GetParam()), sdbusplus::exception::SdBusError);
}
class TestReportAllReportTypes :
@@ -391,7 +394,7 @@
TEST_P(TestReportAllReportTypes, updateReadingsCallEnabledPropertyOff)
{
- const uint64_t expectedTime = std::time(0);
+ const uint64_t expectedTime = Clock().timestamp();
setProperty(sut->getPath(), "Enabled", false);
sut->updateReadings();
@@ -404,7 +407,7 @@
TEST_P(TestReportAllReportTypes, updateReadingsCallEnabledPropertyOn)
{
- const uint64_t expectedTime = std::time(0);
+ const uint64_t expectedTime = Clock().timestamp();
sut->updateReadings();
const auto [timestamp, readings] =
@@ -425,7 +428,7 @@
TEST_F(TestReportOnRequestType, updatesReadingTimestamp)
{
- const uint64_t expectedTime = std::time(0);
+ const uint64_t expectedTime = Clock().timestamp();
ASSERT_THAT(update(sut->getPath()), Eq(boost::system::errc::success));
@@ -504,7 +507,7 @@
TEST_F(TestReportPeriodicReport, readingTimestampIsUpdatedAfterIntervalExpires)
{
- const uint64_t expectedTime = std::time(0);
+ const uint64_t expectedTime = Clock().timestamp();
DbusEnvironment::sleepFor(ReportManager::minInterval + 1ms);
const auto [timestamp, readings] =
diff --git a/tests/src/test_report_manager.cpp b/tests/src/test_report_manager.cpp
index 1bf886c..05a00ed 100644
--- a/tests/src/test_report_manager.cpp
+++ b/tests/src/test_report_manager.cpp
@@ -26,7 +26,7 @@
StorageMock& storageMock = *storageMockPtr;
std::unique_ptr<ReportMock> reportMockPtr =
- std::make_unique<NiceMock<ReportMock>>(reportParams.reportName());
+ std::make_unique<NiceMock<ReportMock>>(reportParams.reportId());
ReportMock& reportMock = *reportMockPtr;
std::unique_ptr<ReportManager> sut;
@@ -68,14 +68,16 @@
auto addReport(const ReportParams& params)
{
- return addReport(
- params.reportName(), utils::enumToString(params.reportingType()),
- utils::enumToString(params.reportUpdates()), params.appendLimit(),
- utils::transform(
- params.reportActions(),
- [](const auto v) { return utils::enumToString(v); }),
- params.interval().count(),
- toReadingParameters(params.metricParameters()));
+ return addReport(params.reportId(), params.reportName(),
+ utils::enumToString(params.reportingType()),
+ utils::enumToString(params.reportUpdates()),
+ params.appendLimit(),
+ utils::transform(params.reportActions(),
+ [](const auto v) {
+ return utils::enumToString(v);
+ }),
+ params.interval().count(),
+ toReadingParameters(params.metricParameters()));
}
template <class T>
@@ -110,16 +112,42 @@
EXPECT_THAT(path, Eq(reportMock.getPath()));
}
-TEST_F(TestReportManager, addReportWithMaxLengthName)
+TEST_F(TestReportManager, nameIsUsedToGenerateIdWhenIdIsEmptyInAddReport)
{
- std::string reportName(ReportManager::maxReportNameLength, 'z');
- reportParams.reportName(reportName);
+ reportParams.reportId("ReportName");
+ reportParams.reportName("ReportName");
+
+ reportFactoryMock.expectMake(reportParams, Ref(*sut), Ref(storageMock));
+
+ auto [ec, path] = addReport(reportParams.reportId(""));
+
+ EXPECT_THAT(ec.value(), Eq(boost::system::errc::success));
+ EXPECT_THAT(path, Eq("/ReportName"));
+}
+
+TEST_F(TestReportManager, nameIsUsedToGenerateIdWhenIdIsNamespace)
+{
+ reportParams.reportId("Prefix/ReportName");
+ reportParams.reportName("ReportName");
+
+ reportFactoryMock.expectMake(reportParams, Ref(*sut), Ref(storageMock));
+
+ auto [ec, path] = addReport(reportParams.reportId("Prefix/"));
+
+ EXPECT_THAT(ec.value(), Eq(boost::system::errc::success));
+ EXPECT_THAT(path, Eq("/Prefix/ReportName"));
+}
+
+TEST_F(TestReportManager, addReportWithMaxLengthId)
+{
+ std::string reportId(ReportManager::maxReportIdLength, 'z');
+ reportParams.reportId(reportId);
reportFactoryMock.expectMake(reportParams, Ref(*sut), Ref(storageMock));
auto [ec, path] = addReport(reportParams);
EXPECT_THAT(ec.value(), Eq(boost::system::errc::success));
- EXPECT_THAT(path, Eq("/"s + reportName));
+ EXPECT_THAT(path, Eq("/"s + reportId));
}
TEST_F(TestReportManager, DISABLED_failToAddReportWithTooLongName)
@@ -127,8 +155,8 @@
reportFactoryMock.expectMake(std::nullopt, Ref(*sut), Ref(storageMock))
.Times(0);
- reportParams.reportName(
- std::string(ReportManager::maxReportNameLength + 1, 'z'));
+ reportParams.reportId(
+ std::string(ReportManager::maxReportIdLength + 1, 'z'));
auto [ec, path] = addReport(reportParams);
@@ -204,14 +232,14 @@
for (size_t i = 0; i < ReportManager::maxReports; i++)
{
- reportParams.reportName(reportParams.reportName() + std::to_string(i));
+ reportParams.reportId(reportParams.reportName() + std::to_string(i));
auto [ec, path] = addReport(reportParams);
EXPECT_THAT(ec.value(), Eq(boost::system::errc::success));
}
- reportParams.reportName(reportParams.reportName() +
- std::to_string(ReportManager::maxReports));
+ reportParams.reportId(reportParams.reportName() +
+ std::to_string(ReportManager::maxReports));
auto [ec, path] = addReport(reportParams);
EXPECT_THAT(ec.value(), Eq(boost::system::errc::too_many_files_open));
@@ -270,7 +298,7 @@
EXPECT_CALL(reportMock, updateReadings());
addReport(reportParams);
- sut->updateReport(reportParams.reportName());
+ sut->updateReport(reportParams.reportId());
}
TEST_F(TestReportManager, updateReportDoNothingIfReportDoesNotExist)
@@ -302,10 +330,10 @@
reportParams.metricParameters(
std::vector<LabeledMetricParameters>{{LabeledMetricParameters{
{LabeledSensorParameters{"Service",
- "/xyz/openbmc_project/sensors/power/p1"}},
+ "/xyz/openbmc_project/sensors/power/p1",
+ "Metadata1"}},
operationType,
"MetricId1",
- "Metadata1",
CollectionTimeScope::point,
CollectionDuration(Milliseconds(0u))}}});
@@ -315,7 +343,7 @@
auto [ec, path] = addReport(reportParams);
EXPECT_THAT(ec.value(), Eq(boost::system::errc::success));
- EXPECT_THAT(path, Eq("/"s + reportParams.reportName()));
+ EXPECT_THAT(path, Eq("/"s + reportParams.reportId()));
}
class TestReportManagerStorage : public TestReportManager
@@ -344,6 +372,7 @@
nlohmann::json data = nlohmann::json{
{"Enabled", reportParams.enabled()},
{"Version", Report::reportVersion},
+ {"Id", reportParams.reportId()},
{"Name", reportParams.reportName()},
{"ReportingType", utils::toUnderlying(reportParams.reportingType())},
{"ReportActions", reportParams.reportActions()},
diff --git a/tests/src/test_sensor.cpp b/tests/src/test_sensor.cpp
index 5547cf0..ffa73b1 100644
--- a/tests/src/test_sensor.cpp
+++ b/tests/src/test_sensor.cpp
@@ -4,6 +4,7 @@
#include "sensor.hpp"
#include "sensor_cache.hpp"
#include "stubs/dbus_sensor_object.hpp"
+#include "utils/clock.hpp"
#include <sdbusplus/asio/property.hpp>
@@ -51,9 +52,9 @@
std::unique_ptr<stubs::DbusSensorObject> sensorObject = makeSensorObject();
SensorCache sensorCache;
- uint64_t timestamp = std::time(0);
+ uint64_t timestamp = Clock().timestamp();
std::shared_ptr<Sensor> sut = sensorCache.makeSensor<Sensor>(
- DbusEnvironment::serviceName(), sensorObject->path(),
+ DbusEnvironment::serviceName(), sensorObject->path(), "metadata",
DbusEnvironment::getIoc(), DbusEnvironment::getBus());
std::shared_ptr<SensorListenerMock> listenerMock =
std::make_shared<StrictMock<SensorListenerMock>>();
diff --git a/tests/src/test_trigger.cpp b/tests/src/test_trigger.cpp
index a074e7a..66e9bf0 100644
--- a/tests/src/test_trigger.cpp
+++ b/tests/src/test_trigger.cpp
@@ -57,7 +57,7 @@
return std::make_unique<Trigger>(
DbusEnvironment::getIoc(), DbusEnvironment::getObjServer(),
params.id(), params.name(), params.triggerActions(),
- params.reportNames(), params.sensors(), params.thresholdParams(),
+ params.reportIds(), params.sensors(), params.thresholdParams(),
std::vector<std::shared_ptr<interfaces::Threshold>>{},
*triggerManagerMockPtr, storageMock);
}
@@ -109,7 +109,7 @@
Eq(utils::fromLabeledSensorsInfo(triggerParams.sensors())));
EXPECT_THAT(
getProperty<std::vector<std::string>>(sut->getPath(), "ReportNames"),
- Eq(triggerParams.reportNames()));
+ Eq(triggerParams.reportIds()));
EXPECT_THAT(
getProperty<TriggerThresholdParams>(sut->getPath(), "Thresholds"),
Eq(std::visit(utils::FromLabeledThresholdParamConversion(),
@@ -268,10 +268,10 @@
Eq(triggerParams.triggerActions()));
}
-TEST_F(TestTriggerStore, settingPersistencyToTrueStoresTriggerReportNames)
+TEST_F(TestTriggerStore, settingPersistencyToTrueStoresTriggerReportIds)
{
- ASSERT_THAT(storedConfiguration.at("ReportNames"),
- Eq(triggerParams.reportNames()));
+ ASSERT_THAT(storedConfiguration.at("ReportIds"),
+ Eq(triggerParams.reportIds()));
}
TEST_F(TestTriggerStore, settingPersistencyToTrueStoresTriggerSensors)
@@ -280,7 +280,7 @@
expectedItem["service"] = "service1";
expectedItem["sensorPath"] =
"/xyz/openbmc_project/sensors/temperature/BMC_Temp";
- expectedItem["sensorMetadata"] = "metadata1";
+ expectedItem["metadata"] = "metadata1";
ASSERT_THAT(storedConfiguration.at("Sensors"), ElementsAre(expectedItem));
}
diff --git a/tests/src/test_trigger_manager.cpp b/tests/src/test_trigger_manager.cpp
index c813e97..182c58b 100644
--- a/tests/src/test_trigger_manager.cpp
+++ b/tests/src/test_trigger_manager.cpp
@@ -30,7 +30,7 @@
DbusEnvironment::serviceName(), TriggerManager::triggerManagerPath,
TriggerManager::triggerManagerIfaceName, "AddTrigger", params.id(),
params.name(), params.triggerActions(), sensorInfos,
- params.reportNames(),
+ params.reportIds(),
std::visit(utils::FromLabeledThresholdParamConversion(),
params.thresholdParams()));
return DbusEnvironment::waitForFuture(addTriggerPromise.get_future());
@@ -294,7 +294,7 @@
{"TriggerActions", TriggerParams().triggerActions()},
{"ThresholdParams", utils::labeledThresholdParamsToJson(
TriggerParams().thresholdParams())},
- {"ReportNames", TriggerParams().reportNames()},
+ {"ReportIds", TriggerParams().reportIds()},
{"Sensors", TriggerParams().sensors()}};
nlohmann::json data2 = data1;