Fix Trigger GET functionality
Due to changes in dbus interface of TelemetryService, some modifications
were needed in bmcweb:
- Some of values in TriggerActions property were renamed.
- ReportNames property was renamed and its type was changed. It is now a
vector of object_path, each representing full dbus path of a Report
Telemetry resource. Some additional logic was added to verify it.
Additionally, 2 issues were found during testing:
- In case of error occurring during internal data parsing, GET could
return both "Internal Error" message and incomplete Trigger
representation. By swapping few lines of code, this issue is fixed:
now either error is returned or full Trigger representation, but never
both of them.
- NumericThresholds object was parsed as 'null' in case of missing
thresholds. It should be an empty json {} instead.
Testing done:
- TriggerActions are properly translated to redfish names.
- Links are now returning proper uris of existing MRD.
- Internal Error is returned for malformed data returned by service.
- Other Trigger GET functionality remained unchanged.
Signed-off-by: Szymon Dompke <szymon.dompke@intel.com>
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I81ebbf3889a152199bef7230de56a73bb112731b
diff --git a/redfish-core/lib/trigger.hpp b/redfish-core/lib/trigger.hpp
index 9955105..920a8c3 100644
--- a/redfish-core/lib/trigger.hpp
+++ b/redfish-core/lib/trigger.hpp
@@ -34,7 +34,8 @@
using TriggerGetParamsVariant =
std::variant<std::monostate, bool, std::string, TriggerThresholdParamsExt,
- TriggerSensorsParams, std::vector<std::string>>;
+ TriggerSensorsParams, std::vector<std::string>,
+ std::vector<sdbusplus::message::object_path>>;
inline std::optional<std::string>
getRedfishFromDbusAction(const std::string& dbusAction)
@@ -44,11 +45,11 @@
{
redfishAction = "RedfishMetricReport";
}
- if (dbusAction == "RedfishEvent")
+ if (dbusAction == "LogToRedfishEventLog")
{
redfishAction = "RedfishEvent";
}
- if (dbusAction == "LogToLogService")
+ if (dbusAction == "LogToJournal")
{
redfishAction = "LogToLogService";
}
@@ -72,10 +73,10 @@
triggerActions.push_back(*redfishAction);
}
- return std::make_optional(triggerActions);
+ return {std::move(triggerActions)};
}
-inline std::optional<nlohmann::json>
+inline std::optional<nlohmann::json::array_t>
getDiscreteTriggers(const TriggerThresholdParamsExt& thresholdParams)
{
const std::vector<DiscreteThresholdParams>* discreteParams =
@@ -86,7 +87,7 @@
return std::nullopt;
}
- nlohmann::json triggers = nlohmann::json::array();
+ nlohmann::json::array_t triggers;
for (const auto& [name, severity, dwellTime, value] : *discreteParams)
{
std::optional<std::string> duration =
@@ -105,7 +106,7 @@
});
}
- return std::make_optional(triggers);
+ return {std::move(triggers)};
}
inline std::optional<nlohmann::json>
@@ -119,7 +120,7 @@
return std::nullopt;
}
- nlohmann::json thresholds;
+ nlohmann::json::object_t thresholds;
for (const auto& [type, dwellTime, activation, reading] : *numericParams)
{
std::optional<std::string> duration =
@@ -135,24 +136,34 @@
threshold["DwellTime"] = *duration;
}
- return std::make_optional(thresholds);
+ return {std::move(thresholds)};
}
-inline nlohmann::json
- getMetricReportDefinitions(const std::vector<std::string>& reportNames)
+inline std::optional<nlohmann::json> getMetricReportDefinitions(
+ const std::vector<sdbusplus::message::object_path>& reportPaths)
{
nlohmann::json reports = nlohmann::json::array();
- for (const std::string& name : reportNames)
+
+ for (const sdbusplus::message::object_path& path : reportPaths)
{
+ std::string reportId = path.filename();
+ if (reportId.empty())
+ {
+ {
+ BMCWEB_LOG_ERROR << "Property Reports contains invalid value: "
+ << path.str;
+ return std::nullopt;
+ }
+ }
+
nlohmann::json::object_t report;
report["@odata.id"] =
crow::utility::urlFromPieces("redfish", "v1", "TelemetryService",
- "MetricReportDefinitions", name)
- .string();
+ "MetricReportDefinitions", reportId);
reports.push_back(std::move(report));
}
- return reports;
+ return {std::move(reports)};
}
inline std::vector<std::string>
@@ -176,7 +187,7 @@
const std::string* name = nullptr;
const bool* discrete = nullptr;
const TriggerSensorsParams* sensors = nullptr;
- const std::vector<std::string>* reports = nullptr;
+ const std::vector<sdbusplus::message::object_path>* reports = nullptr;
const std::vector<std::string>* actions = nullptr;
const TriggerThresholdParamsExt* thresholds = nullptr;
@@ -194,9 +205,10 @@
{
sensors = std::get_if<TriggerSensorsParams>(&var);
}
- else if (key == "ReportNames")
+ else if (key == "Reports")
{
- reports = std::get_if<std::vector<std::string>>(&var);
+ reports =
+ std::get_if<std::vector<sdbusplus::message::object_path>>(&var);
}
else if (key == "TriggerActions")
{
@@ -217,16 +229,26 @@
return false;
}
- json["@odata.type"] = "#Triggers.v1_2_0.Triggers";
- json["@odata.id"] = crow::utility::urlFromPieces(
- "redfish", "v1", "TelemetryService", "Triggers", id)
- .string();
- json["Id"] = id;
- json["Name"] = *name;
+ std::optional<std::vector<std::string>> triggerActions =
+ getTriggerActions(*actions);
+ if (!triggerActions)
+ {
+ BMCWEB_LOG_ERROR << "Property TriggerActions is invalid in Trigger: "
+ << id;
+ return false;
+ }
+
+ std::optional<nlohmann::json> linkedReports =
+ getMetricReportDefinitions(*reports);
+ if (!linkedReports)
+ {
+ BMCWEB_LOG_ERROR << "Property Reports is invalid in Trigger: " << id;
+ return false;
+ }
if (*discrete)
{
- std::optional<nlohmann::json> discreteTriggers =
+ std::optional<nlohmann::json::array_t> discreteTriggers =
getDiscreteTriggers(*thresholds);
if (!discreteTriggers)
@@ -259,20 +281,14 @@
json["MetricType"] = "Numeric";
}
- std::optional<std::vector<std::string>> triggerActions =
- getTriggerActions(*actions);
-
- if (!triggerActions)
- {
- BMCWEB_LOG_ERROR << "Property TriggerActions is invalid in Trigger: "
- << id;
- return false;
- }
-
+ json["@odata.type"] = "#Triggers.v1_2_0.Triggers";
+ json["@odata.id"] = crow::utility::urlFromPieces(
+ "redfish", "v1", "TelemetryService", "Triggers", id);
+ json["Id"] = id;
+ json["Name"] = *name;
json["TriggerActions"] = *triggerActions;
json["MetricProperties"] = getMetricProperties(*sensors);
- json["Links"]["MetricReportDefinitions"] =
- getMetricReportDefinitions(*reports);
+ json["Links"]["MetricReportDefinitions"] = *linkedReports;
return true;
}