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/processor.hpp b/redfish-core/lib/processor.hpp
index 49e04e2..8127510 100644
--- a/redfish-core/lib/processor.hpp
+++ b/redfish-core/lib/processor.hpp
@@ -17,8 +17,8 @@
#include "health.hpp"
+#include <app.hpp>
#include <boost/container/flat_map.hpp>
-#include <node.hpp>
#include <sdbusplus/message/native_types.hpp>
#include <sdbusplus/utility/dedup_variant.hpp>
#include <utils/collection.hpp>
@@ -981,281 +981,212 @@
std::variant<sdbusplus::message::object_path>(std::move(configPath)));
}
-class OperatingConfigCollection : public Node
+inline void requestRoutesOperatingConfigCollection(App& app)
{
- public:
- OperatingConfigCollection(App& app) :
- Node(app,
- "/redfish/v1/Systems/system/Processors/<str>/OperatingConfigs/",
- std::string())
- {
- // Defined by Redfish spec privilege registry
- entityPrivileges = {
- {boost::beast::http::verb::get, {{"Login"}}},
- {boost::beast::http::verb::head, {{"Login"}}},
- {boost::beast::http::verb::patch, {{"ConfigureComponents"}}},
- {boost::beast::http::verb::put, {{"ConfigureComponents"}}},
- {boost::beast::http::verb::delete_, {{"ConfigureComponents"}}},
- {boost::beast::http::verb::post, {{"ConfigureComponents"}}}};
- }
- private:
- void doGet(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/Systems/system/Processors/<str>/OperatingConfigs/")
+ .privileges({"Login"})
+ .methods(boost::beast::http::verb::get)(
+ [](const crow::Request& req,
+ const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
+ const std::string& cpuName) {
+ asyncResp->res.jsonValue["@odata.type"] =
+ "#OperatingConfigCollection.OperatingConfigCollection";
+ asyncResp->res.jsonValue["@odata.id"] = req.url;
+ asyncResp->res.jsonValue["Name"] =
+ "Operating Config Collection";
- const std::string& cpuName = params[0];
- asyncResp->res.jsonValue["@odata.type"] =
- "#OperatingConfigCollection.OperatingConfigCollection";
- asyncResp->res.jsonValue["@odata.id"] = req.url;
- asyncResp->res.jsonValue["Name"] = "Operating Config Collection";
+ // First find the matching CPU object so we know how to
+ // constrain our search for related Config objects.
+ crow::connections::systemBus->async_method_call(
+ [asyncResp,
+ cpuName](const boost::system::error_code ec,
+ const std::vector<std::string>& objects) {
+ if (ec)
+ {
+ BMCWEB_LOG_WARNING << "D-Bus error: " << ec << ", "
+ << ec.message();
+ messages::internalError(asyncResp->res);
+ return;
+ }
- // First find the matching CPU object so we know how to constrain our
- // search for related Config objects.
- crow::connections::systemBus->async_method_call(
- [asyncResp, cpuName](const boost::system::error_code ec,
- const std::vector<std::string>& objects) {
- if (ec)
- {
- BMCWEB_LOG_WARNING << "D-Bus error: " << ec << ", "
- << ec.message();
- messages::internalError(asyncResp->res);
- return;
- }
+ for (const std::string& object : objects)
+ {
+ if (!boost::ends_with(object, cpuName))
+ {
+ continue;
+ }
- for (const std::string& object : objects)
- {
- if (!boost::ends_with(object, cpuName))
+ // Not expected that there will be multiple matching
+ // CPU objects, but if there are just use the first
+ // one.
+
+ // Use the common search routine to construct the
+ // Collection of all Config objects under this CPU.
+ collection_util::getCollectionMembers(
+ asyncResp,
+ "/redfish/v1/Systems/system/Processors/" +
+ cpuName + "/OperatingConfigs",
+ {"xyz.openbmc_project.Inventory.Item.Cpu."
+ "OperatingConfig"},
+ object.c_str());
+ return;
+ }
+ },
+ "xyz.openbmc_project.ObjectMapper",
+ "/xyz/openbmc_project/object_mapper",
+ "xyz.openbmc_project.ObjectMapper", "GetSubTreePaths",
+ "/xyz/openbmc_project/inventory", 0,
+ std::array<const char*, 1>{
+ "xyz.openbmc_project.Control.Processor."
+ "CurrentOperatingConfig"});
+ });
+}
+
+inline void requestRoutesOperatingConfig(App& app)
+{
+ BMCWEB_ROUTE(
+ app,
+ "/redfish/v1/Systems/system/Processors/<str>/OperatingConfigs/<str>/")
+ .privileges({"Login"})
+ .methods(
+ boost::beast::http::verb::get)([](const crow::Request& req,
+ const std::shared_ptr<
+ bmcweb::AsyncResp>& asyncResp,
+ const std::string& cpuName,
+ const std::string& configName) {
+ // Ask for all objects implementing OperatingConfig so we can search
+ // for one with a matching name
+ crow::connections::systemBus->async_method_call(
+ [asyncResp, cpuName, configName,
+ reqUrl{req.url}](boost::system::error_code ec,
+ const MapperGetSubTreeResponse& subtree) {
+ if (ec)
{
- continue;
+ BMCWEB_LOG_WARNING << "D-Bus error: " << ec << ", "
+ << ec.message();
+ messages::internalError(asyncResp->res);
+ return;
}
-
- // Not expected that there will be multiple matching CPU
- // objects, but if there are just use the first one.
-
- // Use the common search routine to construct the Collection
- // of all Config objects under this CPU.
- collection_util::getCollectionMembers(
- asyncResp,
- "/redfish/v1/Systems/system/Processors/" + cpuName +
- "/OperatingConfigs",
- {"xyz.openbmc_project.Inventory.Item.Cpu."
- "OperatingConfig"},
- object.c_str());
- return;
- }
- },
- "xyz.openbmc_project.ObjectMapper",
- "/xyz/openbmc_project/object_mapper",
- "xyz.openbmc_project.ObjectMapper", "GetSubTreePaths",
- "/xyz/openbmc_project/inventory", 0,
- std::array<const char*, 1>{"xyz.openbmc_project.Control.Processor."
- "CurrentOperatingConfig"});
- }
-};
-
-class OperatingConfig : public Node
-{
- public:
- OperatingConfig(App& app) :
- Node(app,
- "/redfish/v1/Systems/system/Processors/<str>/OperatingConfigs/"
- "<str>/",
- std::string(), std::string())
- {
- // Defined by Redfish spec privilege registry
- entityPrivileges = {
- {boost::beast::http::verb::get, {{"Login"}}},
- {boost::beast::http::verb::head, {{"Login"}}},
- {boost::beast::http::verb::patch, {{"ConfigureComponents"}}},
- {boost::beast::http::verb::put, {{"ConfigureComponents"}}},
- {boost::beast::http::verb::delete_, {{"ConfigureComponents"}}},
- {boost::beast::http::verb::post, {{"ConfigureComponents"}}}};
- }
-
- private:
- void doGet(const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
- const crow::Request& req,
- const std::vector<std::string>& params) override
- {
- if (params.size() != 2)
- {
- messages::internalError(asyncResp->res);
- return;
- }
-
- const std::string& cpuName = params[0];
- const std::string& configName = params[1];
-
- // Ask for all objects implementing OperatingConfig so we can search for
- // one with a matching name
- crow::connections::systemBus->async_method_call(
- [asyncResp, cpuName, configName,
- reqUrl{req.url}](boost::system::error_code ec,
- const MapperGetSubTreeResponse& subtree) {
- if (ec)
- {
- BMCWEB_LOG_WARNING << "D-Bus error: " << ec << ", "
- << ec.message();
- messages::internalError(asyncResp->res);
- return;
- }
- const std::string expectedEnding = cpuName + '/' + configName;
- for (const auto& [objectPath, serviceMap] : subtree)
- {
- // Ignore any configs without matching cpuX/configY
- if (!boost::ends_with(objectPath, expectedEnding) ||
- serviceMap.empty())
+ const std::string expectedEnding =
+ cpuName + '/' + configName;
+ for (const auto& [objectPath, serviceMap] : subtree)
{
- continue;
+ // Ignore any configs without matching cpuX/configY
+ if (!boost::ends_with(objectPath, expectedEnding) ||
+ serviceMap.empty())
+ {
+ continue;
+ }
+
+ nlohmann::json& json = asyncResp->res.jsonValue;
+ json["@odata.type"] =
+ "#OperatingConfig.v1_0_0.OperatingConfig";
+ json["@odata.id"] = reqUrl;
+ json["Name"] = "Processor Profile";
+ json["Id"] = configName;
+
+ // Just use the first implementation of the object - not
+ // expected that there would be multiple matching
+ // services
+ getOperatingConfigData(
+ asyncResp, serviceMap.begin()->first, objectPath);
+ return;
}
+ messages::resourceNotFound(asyncResp->res,
+ "OperatingConfig", configName);
+ },
+ "xyz.openbmc_project.ObjectMapper",
+ "/xyz/openbmc_project/object_mapper",
+ "xyz.openbmc_project.ObjectMapper", "GetSubTree",
+ "/xyz/openbmc_project/inventory", 0,
+ std::array<const char*, 1>{
+ "xyz.openbmc_project.Inventory.Item.Cpu.OperatingConfig"});
+ });
+}
- nlohmann::json& json = asyncResp->res.jsonValue;
- json["@odata.type"] =
- "#OperatingConfig.v1_0_0.OperatingConfig";
- json["@odata.id"] = reqUrl;
- json["Name"] = "Processor Profile";
- json["Id"] = configName;
-
- // Just use the first implementation of the object - not
- // expected that there would be multiple matching services
- getOperatingConfigData(asyncResp, serviceMap.begin()->first,
- objectPath);
- return;
- }
- messages::resourceNotFound(asyncResp->res, "OperatingConfig",
- configName);
- },
- "xyz.openbmc_project.ObjectMapper",
- "/xyz/openbmc_project/object_mapper",
- "xyz.openbmc_project.ObjectMapper", "GetSubTree",
- "/xyz/openbmc_project/inventory", 0,
- std::array<const char*, 1>{
- "xyz.openbmc_project.Inventory.Item.Cpu.OperatingConfig"});
- }
-};
-
-class ProcessorCollection : public Node
+inline void requestRoutesProcessorCollection(App& app)
{
- public:
- /*
- * Default Constructor
- */
- ProcessorCollection(App& app) :
- Node(app, "/redfish/v1/Systems/system/Processors/")
- {
- entityPrivileges = {
- {boost::beast::http::verb::get, {{"Login"}}},
- {boost::beast::http::verb::head, {{"Login"}}},
- {boost::beast::http::verb::patch, {{"ConfigureComponents"}}},
- {boost::beast::http::verb::put, {{"ConfigureComponents"}}},
- {boost::beast::http::verb::delete_, {{"ConfigureComponents"}}},
- {boost::beast::http::verb::post, {{"ConfigureComponents"}}}};
- }
-
- private:
/**
* Functions triggers appropriate requests on DBus
*/
- void doGet(const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
- const crow::Request&, const std::vector<std::string>&) override
- {
- asyncResp->res.jsonValue["@odata.type"] =
- "#ProcessorCollection.ProcessorCollection";
- asyncResp->res.jsonValue["Name"] = "Processor Collection";
+ BMCWEB_ROUTE(app, "/redfish/v1/Systems/system/Processors/")
+ .privileges({"Login"})
+ .methods(boost::beast::http::verb::get)(
+ [](const crow::Request&,
+ const std::shared_ptr<bmcweb::AsyncResp>& asyncResp) {
+ asyncResp->res.jsonValue["@odata.type"] =
+ "#ProcessorCollection.ProcessorCollection";
+ asyncResp->res.jsonValue["Name"] = "Processor Collection";
- asyncResp->res.jsonValue["@odata.id"] =
- "/redfish/v1/Systems/system/Processors";
+ asyncResp->res.jsonValue["@odata.id"] =
+ "/redfish/v1/Systems/system/Processors";
- collection_util::getCollectionMembers(
- asyncResp, "/redfish/v1/Systems/system/Processors",
- std::vector<const char*>(processorInterfaces.begin(),
- processorInterfaces.end()));
- }
-};
+ collection_util::getCollectionMembers(
+ asyncResp, "/redfish/v1/Systems/system/Processors",
+ std::vector<const char*>(processorInterfaces.begin(),
+ processorInterfaces.end()));
+ });
+}
-class Processor : public Node
+inline void requestRoutesProcessor(App& app)
{
- public:
- /*
- * Default Constructor
- */
- Processor(App& app) :
- Node(app, "/redfish/v1/Systems/system/Processors/<str>/", std::string())
- {
- entityPrivileges = {
- {boost::beast::http::verb::get, {{"Login"}}},
- {boost::beast::http::verb::head, {{"Login"}}},
- {boost::beast::http::verb::patch, {{"ConfigureComponents"}}},
- {boost::beast::http::verb::put, {{"ConfigureComponents"}}},
- {boost::beast::http::verb::delete_, {{"ConfigureComponents"}}},
- {boost::beast::http::verb::post, {{"ConfigureComponents"}}}};
- }
-
- private:
/**
* Functions triggers appropriate requests on DBus
*/
- void doGet(const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
- const crow::Request&,
- const std::vector<std::string>& params) override
- {
- // Check if there is required param, truly entering this shall be
- // impossible
- if (params.size() != 1)
- {
- messages::internalError(asyncResp->res);
- return;
- }
- const std::string& processorId = params[0];
- asyncResp->res.jsonValue["@odata.type"] =
- "#Processor.v1_11_0.Processor";
- asyncResp->res.jsonValue["@odata.id"] =
- "/redfish/v1/Systems/system/Processors/" + processorId;
- getProcessorObject(asyncResp, processorId, getProcessorData);
- }
+ BMCWEB_ROUTE(app, "/redfish/v1/Systems/system/Processors/<str>/")
+ .privileges({"Login"})
+ .methods(boost::beast::http::verb::get)(
+ [](const crow::Request&,
+ const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
+ const std::string& processorId) {
+ asyncResp->res.jsonValue["@odata.type"] =
+ "#Processor.v1_11_0.Processor";
+ asyncResp->res.jsonValue["@odata.id"] =
+ "/redfish/v1/Systems/system/Processors/" + processorId;
- void doPatch(const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
- const crow::Request& req,
- const std::vector<std::string>& params) override
- {
- std::optional<nlohmann::json> appliedConfigJson;
- if (!json_util::readJson(req, asyncResp->res, "AppliedOperatingConfig",
- appliedConfigJson))
- {
- return;
- }
+ getProcessorObject(asyncResp, processorId, getProcessorData);
+ });
- std::string appliedConfigUri;
- if (appliedConfigJson)
- {
- if (!json_util::readJson(*appliedConfigJson, asyncResp->res,
- "@odata.id", appliedConfigUri))
- {
- return;
- }
- // Check for 404 and find matching D-Bus object, then run property
- // patch handlers if that all succeeds.
- getProcessorObject(
- asyncResp, params[0],
- [appliedConfigUri = std::move(appliedConfigUri)](
- const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
- const std::string& processorId,
- const std::string& objectPath,
- const MapperServiceMap& serviceMap) {
- patchAppliedOperatingConfig(asyncResp, processorId,
- appliedConfigUri, objectPath,
- serviceMap);
- });
- }
- }
-};
+ BMCWEB_ROUTE(app, "/redfish/v1/Systems/system/Processors/<str>/")
+ .privileges({"ConfigureComponents"})
+ .methods(boost::beast::http::verb::patch)(
+ [](const crow::Request& req,
+ const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
+ const std::string& processorId) {
+ std::optional<nlohmann::json> appliedConfigJson;
+ if (!json_util::readJson(req, asyncResp->res,
+ "AppliedOperatingConfig",
+ appliedConfigJson))
+ {
+ return;
+ }
+
+ std::string appliedConfigUri;
+ if (appliedConfigJson)
+ {
+ if (!json_util::readJson(*appliedConfigJson, asyncResp->res,
+ "@odata.id", appliedConfigUri))
+ {
+ return;
+ }
+ // Check for 404 and find matching D-Bus object, then run
+ // property patch handlers if that all succeeds.
+ getProcessorObject(
+ asyncResp, processorId,
+ [appliedConfigUri = std::move(appliedConfigUri)](
+ const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
+ const std::string& processorId,
+ const std::string& objectPath,
+ const MapperServiceMap& serviceMap) {
+ patchAppliedOperatingConfig(asyncResp, processorId,
+ appliedConfigUri,
+ objectPath, serviceMap);
+ });
+ }
+ });
+}
} // namespace redfish