Remove Redfish Node class
Reduces the total number of lines and will allow for easier testing of
the redfish responses.
A main purpose of the node class was to set app.routeDynamic(). However
now app.routeDynamic can handle the complexity that was once in critical
to node. The macro app.routeDynamic() provides a shorter cleaner
interface to the unerlying app.routeDyanic call. The old pattern set
permissions for 6 interfaces (get, head, patch, put, delete_, and post)
even if only one interface is created. That pattern creates unneeded
code that can be safely removed with no effect.
Unit test for the responses would have to mock the node the class in
order to fully test responses.
see https://github.com/openbmc/bmcweb/issues/181
The following files still need node to be extracted.
virtual_media.hpp
account_service.hpp
redfish_sessions.hpp
ethernet.hpp
The files above use a pattern that is not trivial to address. Often their
responses call an async lambda capturing the inherited class. ie
(https://github.com/openbmc/bmcweb/blob/ffed87b5ad1797ca966d030e7f979770
28d258fa/redfish-core/lib/account_service.hpp#L1393)
At a later point I plan to remove node from the files above.
Tested:
I ran the docker unit test with the following command.
WORKSPACE=$(pwd) UNIT_TEST_PKG=bmcweb
./openbmc-build-scripts/run-unit-test-docker.sh
I ran the validator and this change did not create any issues.
python3 RedfishServiceValidator.py -c config.ini
Signed-off-by: John Edward Broadbent <jebr@google.com>
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I147a0289c52cb4198345b1ad9bfe6fdddf57f3df
diff --git a/redfish-core/lib/power.hpp b/redfish-core/lib/power.hpp
index 4173ce8..3cc0224 100644
--- a/redfish-core/lib/power.hpp
+++ b/redfish-core/lib/power.hpp
@@ -16,35 +16,19 @@
*/
#pragma once
-#include "node.hpp"
#include "sensors.hpp"
+#include <app.hpp>
+
namespace redfish
{
-
-class Power : public Node
+void setPowerCapOverride(
+ const std::shared_ptr<SensorsAsyncResp>& sensorsAsyncResp,
+ std::vector<nlohmann::json>& powerControlCollections)
{
- public:
- Power(App& app) :
- Node((app), "/redfish/v1/Chassis/<str>/Power/", std::string())
- {
- entityPrivileges = {
- {boost::beast::http::verb::get, {{"Login"}}},
- {boost::beast::http::verb::head, {{"Login"}}},
- {boost::beast::http::verb::patch, {{"ConfigureManager"}}},
- {boost::beast::http::verb::put, {{"ConfigureManager"}}},
- {boost::beast::http::verb::delete_, {{"ConfigureManager"}}},
- {boost::beast::http::verb::post, {{"ConfigureManager"}}}};
- }
-
- private:
- void setPowerCapOverride(
- const std::shared_ptr<SensorsAsyncResp>& sensorsAsyncResp,
- std::vector<nlohmann::json>& powerControlCollections)
- {
- auto getChassisPath = [sensorsAsyncResp, powerControlCollections](
- const std::optional<std::string>&
- chassisPath) mutable {
+ auto getChassisPath =
+ [sensorsAsyncResp, powerControlCollections](
+ const std::optional<std::string>& chassisPath) mutable {
if (!chassisPath)
{
BMCWEB_LOG_ERROR << "Don't find valid chassis path ";
@@ -138,235 +122,236 @@
"org.freedesktop.DBus.Properties", "Get",
"xyz.openbmc_project.Control.Power.Cap", "PowerCapEnable");
};
- getValidChassisPath(sensorsAsyncResp, std::move(getChassisPath));
- }
- void doGet(const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
- const crow::Request&,
- const std::vector<std::string>& params) override
- {
- if (params.size() != 1)
- {
- asyncResp->res.result(
- boost::beast::http::status::internal_server_error);
- return;
- }
- const std::string& chassisName = params[0];
+ getValidChassisPath(sensorsAsyncResp, std::move(getChassisPath));
+}
+inline void requestRoutesPower(App& app)
+{
- asyncResp->res.jsonValue["PowerControl"] = nlohmann::json::array();
+ BMCWEB_ROUTE(app, "/redfish/v1/Chassis/<str>/Power/")
+ .privileges({"Login"})
+ .methods(boost::beast::http::verb::get)(
+ [](const crow::Request&,
+ const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
+ const std::string& chassisName) {
+ asyncResp->res.jsonValue["PowerControl"] =
+ nlohmann::json::array();
- auto sensorAsyncResp = std::make_shared<SensorsAsyncResp>(
- asyncResp, chassisName,
- sensors::dbus::paths.at(sensors::node::power),
- sensors::node::power);
+ auto sensorAsyncResp = std::make_shared<SensorsAsyncResp>(
+ asyncResp, chassisName,
+ sensors::dbus::paths.at(sensors::node::power),
+ sensors::node::power);
- getChassisData(sensorAsyncResp);
+ getChassisData(sensorAsyncResp);
- // This callback verifies that the power limit is only provided for the
- // chassis that implements the Chassis inventory item. This prevents
- // things like power supplies providing the chassis power limit
- auto chassisHandler = [sensorAsyncResp](
- const boost::system::error_code e,
- const std::vector<std::string>&
- chassisPaths) {
- if (e)
- {
- BMCWEB_LOG_ERROR
- << "Power Limit GetSubTreePaths handler Dbus error " << e;
- return;
- }
-
- bool found = false;
- for (const std::string& chassis : chassisPaths)
- {
- size_t len = std::string::npos;
- size_t lastPos = chassis.rfind('/');
- if (lastPos == std::string::npos)
- {
- continue;
- }
-
- if (lastPos == chassis.size() - 1)
- {
- size_t end = lastPos;
- lastPos = chassis.rfind('/', lastPos - 1);
- if (lastPos == std::string::npos)
+ // This callback verifies that the power limit is only provided
+ // for the chassis that implements the Chassis inventory item.
+ // This prevents things like power supplies providing the
+ // chassis power limit
+ auto chassisHandler = [sensorAsyncResp](
+ const boost::system::error_code e,
+ const std::vector<std::string>&
+ chassisPaths) {
+ if (e)
{
- continue;
- }
-
- len = end - (lastPos + 1);
- }
-
- std::string interfaceChassisName =
- chassis.substr(lastPos + 1, len);
- if (!interfaceChassisName.compare(sensorAsyncResp->chassisId))
- {
- found = true;
- break;
- }
- }
-
- if (!found)
- {
- BMCWEB_LOG_DEBUG << "Power Limit not present for "
- << sensorAsyncResp->chassisId;
- return;
- }
-
- auto valueHandler =
- [sensorAsyncResp](
- const boost::system::error_code ec,
- const std::vector<std::pair<std::string, SensorVariant>>&
- properties) {
- if (ec)
- {
- messages::internalError(
- sensorAsyncResp->asyncResp->res);
BMCWEB_LOG_ERROR
- << "Power Limit GetAll handler: Dbus error " << ec;
+ << "Power Limit GetSubTreePaths handler Dbus error "
+ << e;
return;
}
- nlohmann::json& tempArray = sensorAsyncResp->asyncResp->res
- .jsonValue["PowerControl"];
-
- // Put multiple "sensors" into a single PowerControl, 0, so
- // only create the first one
- if (tempArray.empty())
+ bool found = false;
+ for (const std::string& chassis : chassisPaths)
{
- // Mandatory properties odata.id and MemberId
- // A warning without a odata.type
- tempArray.push_back(
- {{"@odata.type", "#Power.v1_0_0.PowerControl"},
- {"@odata.id", "/redfish/v1/Chassis/" +
- sensorAsyncResp->chassisId +
- "/Power#/PowerControl/0"},
- {"Name", "Chassis Power Control"},
- {"MemberId", "0"}});
- }
-
- nlohmann::json& sensorJson = tempArray.back();
- bool enabled = false;
- double powerCap = 0.0;
- int64_t scale = 0;
-
- for (const std::pair<std::string, SensorVariant>& property :
- properties)
- {
- if (!property.first.compare("Scale"))
+ size_t len = std::string::npos;
+ size_t lastPos = chassis.rfind('/');
+ if (lastPos == std::string::npos)
{
- const int64_t* i =
- std::get_if<int64_t>(&property.second);
-
- if (i)
- {
- scale = *i;
- }
+ continue;
}
- else if (!property.first.compare("PowerCap"))
- {
- const double* d =
- std::get_if<double>(&property.second);
- const int64_t* i =
- std::get_if<int64_t>(&property.second);
- const uint32_t* u =
- std::get_if<uint32_t>(&property.second);
- if (d)
+ if (lastPos == chassis.size() - 1)
+ {
+ size_t end = lastPos;
+ lastPos = chassis.rfind('/', lastPos - 1);
+ if (lastPos == std::string::npos)
{
- powerCap = *d;
+ continue;
}
- else if (i)
- {
- powerCap = static_cast<double>(*i);
- }
- else if (u)
- {
- powerCap = *u;
- }
+
+ len = end - (lastPos + 1);
}
- else if (!property.first.compare("PowerCapEnable"))
- {
- const bool* b = std::get_if<bool>(&property.second);
- if (b)
- {
- enabled = *b;
- }
+ std::string interfaceChassisName =
+ chassis.substr(lastPos + 1, len);
+ if (!interfaceChassisName.compare(
+ sensorAsyncResp->chassisId))
+ {
+ found = true;
+ break;
}
}
- nlohmann::json& value =
- sensorJson["PowerLimit"]["LimitInWatts"];
-
- // LimitException is Mandatory attribute as per OCP Baseline
- // Profile – v1.0.0, so currently making it "NoAction"
- // as default value to make it OCP Compliant.
- sensorJson["PowerLimit"]["LimitException"] = "NoAction";
-
- if (enabled)
+ if (!found)
{
- // Redfish specification indicates PowerLimit should be
- // null if the limit is not enabled.
- value = powerCap * std::pow(10, scale);
+ BMCWEB_LOG_DEBUG << "Power Limit not present for "
+ << sensorAsyncResp->chassisId;
+ return;
}
+
+ auto valueHandler = [sensorAsyncResp](
+ const boost::system::error_code ec,
+ const std::vector<std::pair<
+ std::string, SensorVariant>>&
+ properties) {
+ if (ec)
+ {
+ messages::internalError(
+ sensorAsyncResp->asyncResp->res);
+ BMCWEB_LOG_ERROR
+ << "Power Limit GetAll handler: Dbus error "
+ << ec;
+ return;
+ }
+
+ nlohmann::json& tempArray =
+ sensorAsyncResp->asyncResp->res
+ .jsonValue["PowerControl"];
+
+ // Put multiple "sensors" into a single PowerControl, 0,
+ // so only create the first one
+ if (tempArray.empty())
+ {
+ // Mandatory properties odata.id and MemberId
+ // A warning without a odata.type
+ tempArray.push_back(
+ {{"@odata.type", "#Power.v1_0_0.PowerControl"},
+ {"@odata.id", "/redfish/v1/Chassis/" +
+ sensorAsyncResp->chassisId +
+ "/Power#/PowerControl/0"},
+ {"Name", "Chassis Power Control"},
+ {"MemberId", "0"}});
+ }
+
+ nlohmann::json& sensorJson = tempArray.back();
+ bool enabled = false;
+ double powerCap = 0.0;
+ int64_t scale = 0;
+
+ for (const std::pair<std::string, SensorVariant>&
+ property : properties)
+ {
+ if (!property.first.compare("Scale"))
+ {
+ const int64_t* i =
+ std::get_if<int64_t>(&property.second);
+
+ if (i)
+ {
+ scale = *i;
+ }
+ }
+ else if (!property.first.compare("PowerCap"))
+ {
+ const double* d =
+ std::get_if<double>(&property.second);
+ const int64_t* i =
+ std::get_if<int64_t>(&property.second);
+ const uint32_t* u =
+ std::get_if<uint32_t>(&property.second);
+
+ if (d)
+ {
+ powerCap = *d;
+ }
+ else if (i)
+ {
+ powerCap = static_cast<double>(*i);
+ }
+ else if (u)
+ {
+ powerCap = *u;
+ }
+ }
+ else if (!property.first.compare("PowerCapEnable"))
+ {
+ const bool* b =
+ std::get_if<bool>(&property.second);
+
+ if (b)
+ {
+ enabled = *b;
+ }
+ }
+ }
+
+ nlohmann::json& value =
+ sensorJson["PowerLimit"]["LimitInWatts"];
+
+ // LimitException is Mandatory attribute as per OCP
+ // Baseline Profile – v1.0.0, so currently making it
+ // "NoAction" as default value to make it OCP Compliant.
+ sensorJson["PowerLimit"]["LimitException"] = "NoAction";
+
+ if (enabled)
+ {
+ // Redfish specification indicates PowerLimit should
+ // be null if the limit is not enabled.
+ value = powerCap * std::pow(10, scale);
+ }
+ };
+
+ crow::connections::systemBus->async_method_call(
+ std::move(valueHandler), "xyz.openbmc_project.Settings",
+ "/xyz/openbmc_project/control/host0/power_cap",
+ "org.freedesktop.DBus.Properties", "GetAll",
+ "xyz.openbmc_project.Control.Power.Cap");
};
- crow::connections::systemBus->async_method_call(
- std::move(valueHandler), "xyz.openbmc_project.Settings",
- "/xyz/openbmc_project/control/host0/power_cap",
- "org.freedesktop.DBus.Properties", "GetAll",
- "xyz.openbmc_project.Control.Power.Cap");
- };
+ crow::connections::systemBus->async_method_call(
+ std::move(chassisHandler),
+ "xyz.openbmc_project.ObjectMapper",
+ "/xyz/openbmc_project/object_mapper",
+ "xyz.openbmc_project.ObjectMapper", "GetSubTreePaths",
+ "/xyz/openbmc_project/inventory", 0,
+ std::array<const char*, 2>{
+ "xyz.openbmc_project.Inventory.Item.Board",
+ "xyz.openbmc_project.Inventory.Item.Chassis"});
+ });
- crow::connections::systemBus->async_method_call(
- std::move(chassisHandler), "xyz.openbmc_project.ObjectMapper",
- "/xyz/openbmc_project/object_mapper",
- "xyz.openbmc_project.ObjectMapper", "GetSubTreePaths",
- "/xyz/openbmc_project/inventory", 0,
- std::array<const char*, 2>{
- "xyz.openbmc_project.Inventory.Item.Board",
- "xyz.openbmc_project.Inventory.Item.Chassis"});
- }
- void doPatch(const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
- const crow::Request& req,
- const std::vector<std::string>& params) override
- {
- if (params.size() != 1)
- {
- messages::internalError(asyncResp->res);
- return;
- }
+ BMCWEB_ROUTE(app, "/redfish/v1/Chassis/<str>/Power/")
+ .privileges({"ConfigureManager"})
+ .methods(boost::beast::http::verb::patch)(
+ [](const crow::Request& req,
+ const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
+ const std::string& chassisName) {
+ auto sensorAsyncResp = std::make_shared<SensorsAsyncResp>(
+ asyncResp, chassisName,
+ sensors::dbus::paths.at(sensors::node::power),
+ sensors::node::power);
- const std::string& chassisName = params[0];
+ std::optional<std::vector<nlohmann::json>> voltageCollections;
+ std::optional<std::vector<nlohmann::json>> powerCtlCollections;
- auto sensorAsyncResp = std::make_shared<SensorsAsyncResp>(
- asyncResp, chassisName,
- sensors::dbus::paths.at(sensors::node::power),
- sensors::node::power);
+ if (!json_util::readJson(req, sensorAsyncResp->asyncResp->res,
+ "PowerControl", powerCtlCollections,
+ "Voltages", voltageCollections))
+ {
+ return;
+ }
- std::optional<std::vector<nlohmann::json>> voltageCollections;
- std::optional<std::vector<nlohmann::json>> powerCtlCollections;
-
- if (!json_util::readJson(req, sensorAsyncResp->asyncResp->res,
- "PowerControl", powerCtlCollections,
- "Voltages", voltageCollections))
- {
- return;
- }
-
- if (powerCtlCollections)
- {
- setPowerCapOverride(sensorAsyncResp, *powerCtlCollections);
- }
- if (voltageCollections)
- {
- std::unordered_map<std::string, std::vector<nlohmann::json>>
- allCollections;
- allCollections.emplace("Voltages", *std::move(voltageCollections));
- checkAndDoSensorsOverride(sensorAsyncResp, allCollections);
- }
- }
-};
+ if (powerCtlCollections)
+ {
+ setPowerCapOverride(sensorAsyncResp, *powerCtlCollections);
+ }
+ if (voltageCollections)
+ {
+ std::unordered_map<std::string, std::vector<nlohmann::json>>
+ allCollections;
+ allCollections.emplace("Voltages",
+ *std::move(voltageCollections));
+ checkAndDoSensorsOverride(sensorAsyncResp, allCollections);
+ }
+ });
+}
} // namespace redfish