Avoid http request copy in OEM handler
Initial copy was done to avoid request object going out of scope before
OEM handler are invoked.
The MR avoids the whole copy of the request object and create a sub
route object which contains elements required for OEM route handling.
Tested
- Service Validator Passes
- OpenBMC OEM properties and rendered well.
Change-Id: I3ef80a130afe6ab764a13704a8b672f5b0635126
Signed-off-by: Rohit PAI <ropai@nvidia.com>
diff --git a/redfish-core/include/redfish.hpp b/redfish-core/include/redfish.hpp
index f92e4ef..f6628eb 100644
--- a/redfish-core/include/redfish.hpp
+++ b/redfish-core/include/redfish.hpp
@@ -6,6 +6,7 @@
#include "async_resp.hpp"
#include "http_request.hpp"
#include "redfish_oem_routing.hpp"
+#include "sub_request.hpp"
#include "verb.hpp"
#include <memory>
@@ -50,7 +51,8 @@
const crow::Request& req,
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp) const
{
- oemRouter.handle(req, asyncResp);
+ auto subReq = std::make_shared<SubRequest>(req);
+ oemRouter.handle(subReq, asyncResp);
}
OemRouter oemRouter;
diff --git a/redfish-core/include/redfish_oem_routing.hpp b/redfish-core/include/redfish_oem_routing.hpp
index 366ec1e..135c0ff 100644
--- a/redfish-core/include/redfish_oem_routing.hpp
+++ b/redfish-core/include/redfish_oem_routing.hpp
@@ -1,10 +1,10 @@
#pragma once
#include "async_resp.hpp"
-#include "http_request.hpp"
#include "http_response.hpp"
#include "logging.hpp"
#include "redfishoemrule.hpp"
+#include "sub_request.hpp"
#include "sub_route_trie.hpp"
#include "utility.hpp"
#include "utils/query_param.hpp"
@@ -171,7 +171,7 @@
return route;
}
- FindRouteResponse findRoute(const crow::Request& req) const
+ FindRouteResponse findRoute(const SubRequest& req) const
{
FindRouteResponse findRoute;
std::optional<HttpVerb> verb = httpVerbFromBoost(req.method());
@@ -185,8 +185,8 @@
return findRoute;
}
- FindRoute route = findRouteByPerMethod(req.url().encoded_path(),
- perMethods[reqMethodIndex]);
+ FindRoute route =
+ findRouteByPerMethod(req.url(), perMethods[reqMethodIndex]);
if (!route.fragmentRules.empty())
{
findRoute.route = route;
@@ -194,8 +194,7 @@
else
{
BMCWEB_LOG_DEBUG(
- "No fragments for for url {}, method {}",
- req.url().encoded_path(),
+ "No fragments for url {}, method {}", req.url(),
httpVerbToString(static_cast<HttpVerb>(reqMethodIndex)));
}
@@ -220,11 +219,11 @@
}
}
- void handle(const crow::Request& req,
+ void handle(const std::shared_ptr<SubRequest>& req,
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp) const
{
BMCWEB_LOG_DEBUG("Checking OEM routes");
- FindRouteResponse foundRoute = findRoute(req);
+ FindRouteResponse foundRoute = findRoute(*req);
std::vector<OemBaseRule*> fragments =
std::move(foundRoute.route.fragmentRules);
std::vector<std::string> params = std::move(foundRoute.route.params);
@@ -241,9 +240,8 @@
auto uriParams = std::make_shared<std::vector<std::string>>(params);
asyncResp->res.setCompleteRequestHandler(std::bind_front(
- query_param::MultiAsyncResp::startMultiFragmentHandle,
- std::make_shared<crow::Request>(req.copy()), multiResp,
- uriFragments, uriParams));
+ query_param::MultiAsyncResp::startMultiFragmentHandle, req,
+ multiResp, uriFragments, uriParams));
}
else
{
diff --git a/redfish-core/include/redfishoemrule.hpp b/redfish-core/include/redfishoemrule.hpp
index 018a18e..d3f60fb 100644
--- a/redfish-core/include/redfishoemrule.hpp
+++ b/redfish-core/include/redfishoemrule.hpp
@@ -1,6 +1,6 @@
#pragma once
#include "async_resp.hpp"
-#include "http_request.hpp"
+#include "sub_request.hpp"
#include <nlohmann/json.hpp>
@@ -24,9 +24,9 @@
OemBaseRule& operator=(const OemBaseRule&) = delete;
OemBaseRule& operator=(const OemBaseRule&&) = delete;
- virtual void handle(const crow::Request& /*req*/,
+ virtual void handle(const SubRequest& req,
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
- const std::vector<std::string>& /*params*/) = 0;
+ const std::vector<std::string>& params) = 0;
std::string rule;
};
@@ -51,12 +51,12 @@
void operator()(Func&& f)
{
static_assert(
- std::is_invocable_v<Func, crow::Request,
+ std::is_invocable_v<Func, SubRequest,
std::shared_ptr<bmcweb::AsyncResp>&, Args...>,
"Handler type is mismatched with URL parameters");
static_assert(
std::is_same_v<
- void, std::invoke_result_t<Func, crow::Request,
+ void, std::invoke_result_t<Func, SubRequest,
std::shared_ptr<bmcweb::AsyncResp>&,
Args...>>,
"Handler function with response argument should have void return type");
@@ -64,7 +64,7 @@
handler = std::forward<Func>(f);
}
- void handle(const crow::Request& req,
+ void handle(const SubRequest& req,
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
const std::vector<std::string>& params) override
{
@@ -97,7 +97,7 @@
}
private:
- std::function<void(const crow::Request&,
+ std::function<void(const SubRequest&,
const std::shared_ptr<bmcweb::AsyncResp>&, Args...)>
handler;
};
diff --git a/redfish-core/include/sub_request.hpp b/redfish-core/include/sub_request.hpp
new file mode 100644
index 0000000..8c846ae
--- /dev/null
+++ b/redfish-core/include/sub_request.hpp
@@ -0,0 +1,37 @@
+// SPDX-License-Identifier: Apache-2.0
+// SPDX-FileCopyrightText: Copyright OpenBMC Authors
+#pragma once
+
+#include "http_request.hpp"
+
+#include <boost/beast/http/verb.hpp>
+
+#include <string>
+#include <string_view>
+
+namespace redfish
+{
+
+class SubRequest
+{
+ public:
+ explicit SubRequest(const crow::Request& req) :
+ url_(req.url().encoded_path()), method_(req.method())
+ {}
+
+ std::string_view url() const
+ {
+ return url_;
+ }
+
+ boost::beast::http::verb method() const
+ {
+ return method_;
+ }
+
+ private:
+ std::string url_;
+ boost::beast::http::verb method_;
+};
+
+} // namespace redfish
diff --git a/redfish-core/include/utils/query_param.hpp b/redfish-core/include/utils/query_param.hpp
index 3acb511..33c6ff9 100644
--- a/redfish-core/include/utils/query_param.hpp
+++ b/redfish-core/include/utils/query_param.hpp
@@ -16,6 +16,7 @@
#include "logging.hpp"
#include "redfishoemrule.hpp"
#include "str_utility.hpp"
+#include "sub_request.hpp"
#include "utils/json_utils.hpp"
#include <unistd.h>
@@ -820,7 +821,7 @@
}
static void startMultiFragmentHandle(
- const std::shared_ptr<crow::Request>& req,
+ const std::shared_ptr<redfish::SubRequest>& req,
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
const std::shared_ptr<std::vector<OemBaseRule*>>& fragments,
const std::shared_ptr<std::vector<std::string>>& params,
@@ -834,8 +835,9 @@
{
OemBaseRule& fragmentRule = *fragment;
auto rsp = std::make_shared<bmcweb::AsyncResp>();
- BMCWEB_LOG_DEBUG("Matched fragment GET rule '{}' {}",
- fragmentRule.rule, req->methodString());
+ BMCWEB_LOG_DEBUG("Matched fragment rule '{}' method '{}'",
+ fragmentRule.rule,
+ boost::beast::http::to_string(req->method()));
BMCWEB_LOG_DEBUG(
"Handling fragment rules: setting completion handler on {}",
logPtr(&rsp->res));
diff --git a/redfish-core/lib/openbmc/openbmc_managers.hpp b/redfish-core/lib/openbmc/openbmc_managers.hpp
index 4e726e9..ab7fb8b 100644
--- a/redfish-core/lib/openbmc/openbmc_managers.hpp
+++ b/redfish-core/lib/openbmc/openbmc_managers.hpp
@@ -6,10 +6,10 @@
#include "dbus_singleton.hpp"
#include "dbus_utility.hpp"
#include "error_messages.hpp"
-#include "http_request.hpp"
#include "io_context_singleton.hpp"
#include "logging.hpp"
#include "redfish.hpp"
+#include "sub_request.hpp"
#include "utils/dbus_utils.hpp"
#include "utils/json_utils.hpp"
#include "verb.hpp"
@@ -1479,7 +1479,7 @@
};
inline void handleGetManagerOpenBmc(
- const crow::Request& /*req*/,
+ const SubRequest& /*req*/,
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
const std::string& /*managerId*/)
{
diff --git a/test/redfish-core/include/redfish_oem_routing_test.cpp b/test/redfish-core/include/redfish_oem_routing_test.cpp
index 68dd09b..9f9bfdb 100644
--- a/test/redfish-core/include/redfish_oem_routing_test.cpp
+++ b/test/redfish-core/include/redfish_oem_routing_test.cpp
@@ -1,8 +1,8 @@
-
#include "app.hpp"
#include "async_resp.hpp"
#include "http_request.hpp"
#include "redfish.hpp"
+#include "sub_request.hpp"
#include "verb.hpp"
#include <boost/beast/http/verb.hpp>
@@ -27,7 +27,7 @@
// Callback handler that does nothing
bool oemCalled = false;
- auto oemCallback = [&oemCalled](const crow::Request&,
+ auto oemCallback = [&oemCalled](const SubRequest&,
const std::shared_ptr<bmcweb::AsyncResp>&,
const std::string& bar) {
oemCalled = true;