diff options
author | Willy Tu <wltu@google.com> | 2022-02-01 02:38:48 +0300 |
---|---|---|
committer | Ed Tanous <ed@tanous.net> | 2022-03-01 03:00:11 +0300 |
commit | ea2e6eeca15f4019923466f7c8ccc52c53a5ea94 (patch) | |
tree | de20cf21801883ace3ef3b887253c7dd407411e5 /redfish-core | |
parent | ad5d7e7130db7c99d6d4d3c3a465b28c33f9be21 (diff) | |
download | bmcweb-ea2e6eeca15f4019923466f7c8ccc52c53a5ea94.tar.xz |
json_utils: Add support for multiple level json read
Added support for multiple level direct read. For example, we can now
access `abc/xyz` directly instead of getting `abc` and then abc[`xyz`].
For extra element error, it will only be triggered if the element at the
root level is not a parent of any of the requested elements.
For example,
{
"abc": {
"xyz": 12
}
}
Getting "abc/xyz" will satisfy the condition so it does not throw an
error.
This is accomplished in a reasonable way by moving the previously
variadic templated code to a std::span<variant> that contains all
possible types. This is a trick learned from the fmt library to reduce
compile sizes, as the majority of the code doesn't get duplicated at
template level, and is instead operating on the fixed variant type.
This commit drops 7316 bytes (about half a percent of total) from the
bmcweb binary size from the reduction in template usage. Later patches
build on this patchset to simplify call sites even more and reduce the
binary size further, but as is, this is still a win.
Note: now that the UnpackVariant lists all possible unpack types, it was
found that readJson would fail to compile for vector<bool>. This is a
well known C++ deficiency in the std::vector<bool> type when compared to
all other types, and will need special handling in the future. The two
types for vector<bool> are left commented out in the typelist.
Tested: Unit tests passing with reasonable coverage. Functional use in
next commit in this series.
Change-Id: Ifb247c9121c41ad8f1de26eb4bfc3d71484e6bd6
Signed-off-by: Willy Tu <wltu@google.com>
Signed-off-by: Ed Tanous <edtanous@google.com>
Diffstat (limited to 'redfish-core')
-rw-r--r-- | redfish-core/include/utils/json_utils.hpp | 284 | ||||
-rw-r--r-- | redfish-core/lib/event_service.hpp | 18 | ||||
-rw-r--r-- | redfish-core/ut/json_utils_test.cpp | 59 |
3 files changed, 255 insertions, 106 deletions
diff --git a/redfish-core/include/utils/json_utils.hpp b/redfish-core/include/utils/json_utils.hpp index 21701c7418..9aac1dcaf9 100644 --- a/redfish-core/include/utils/json_utils.hpp +++ b/redfish-core/include/utils/json_utils.hpp @@ -20,7 +20,7 @@ #include <http_response.hpp> #include <nlohmann/json.hpp> -#include <bitset> +#include <span> namespace redfish { @@ -76,7 +76,7 @@ enum class UnpackErrorCode }; template <typename ToType, typename FromType> -bool checkRange(const FromType& from, const std::string& key) +bool checkRange(const FromType& from, std::string_view key) { if (from > std::numeric_limits<ToType>::max()) { @@ -104,7 +104,7 @@ bool checkRange(const FromType& from, const std::string& key) template <typename Type> UnpackErrorCode unpackValueWithErrorCode(nlohmann::json& jsonValue, - const std::string& key, Type& value) + std::string_view key, Type& value) { UnpackErrorCode ret = UnpackErrorCode::success; @@ -191,7 +191,7 @@ UnpackErrorCode unpackValueWithErrorCode(nlohmann::json& jsonValue, } template <typename Type> -bool unpackValue(nlohmann::json& jsonValue, const std::string& key, +bool unpackValue(nlohmann::json& jsonValue, std::string_view key, crow::Response& res, Type& value) { bool ret = true; @@ -280,7 +280,7 @@ bool unpackValue(nlohmann::json& jsonValue, const std::string& key, } template <typename Type> -bool unpackValue(nlohmann::json& jsonValue, const std::string& key, Type& value) +bool unpackValue(nlohmann::json& jsonValue, std::string_view key, Type& value) { bool ret = true; if constexpr (IsOptional<Type>::value) @@ -333,141 +333,239 @@ bool unpackValue(nlohmann::json& jsonValue, const std::string& key, Type& value) return ret; } +} // namespace details -template <size_t Count, size_t Index> -bool readJsonValues(const std::string& key, nlohmann::json& /*jsonValue*/, - crow::Response& res, std::bitset<Count>& /*handled*/) +// clang-format off +using UnpackVariant = std::variant< + uint8_t*, + uint16_t*, + int16_t*, + uint32_t*, + int32_t*, + uint64_t*, + int64_t*, + bool*, + double*, + std::string*, + nlohmann::json*, + std::vector<uint8_t>*, + std::vector<uint16_t>*, + std::vector<int16_t>*, + std::vector<uint32_t>*, + std::vector<int32_t>*, + std::vector<uint64_t>*, + std::vector<int64_t>*, + //std::vector<bool>*, + std::vector<double>*, + std::vector<std::string>*, + std::vector<nlohmann::json>*, + std::optional<uint8_t>*, + std::optional<uint16_t>*, + std::optional<int16_t>*, + std::optional<uint32_t>*, + std::optional<int32_t>*, + std::optional<uint64_t>*, + std::optional<int64_t>*, + std::optional<bool>*, + std::optional<double>*, + std::optional<std::string>*, + std::optional<nlohmann::json>*, + std::optional<std::vector<uint8_t>>*, + std::optional<std::vector<uint16_t>>*, + std::optional<std::vector<int16_t>>*, + std::optional<std::vector<uint32_t>>*, + std::optional<std::vector<int32_t>>*, + std::optional<std::vector<uint64_t>>*, + std::optional<std::vector<int64_t>>*, + //std::optional<std::vector<bool>>*, + std::optional<std::vector<double>>*, + std::optional<std::vector<std::string>>*, + std::optional<std::vector<nlohmann::json>>* +>; +// clang-format on + +struct PerUnpack { - BMCWEB_LOG_DEBUG << "Unable to find variable for key" << key; - messages::propertyUnknown(res, key); - return false; -} + std::string_view key; + UnpackVariant value; + bool complete = false; +}; -template <size_t Count, size_t Index, typename ValueType, - typename... UnpackTypes> -bool readJsonValues(const std::string& key, nlohmann::json& jsonValue, - crow::Response& res, std::bitset<Count>& handled, - const char* keyToCheck, ValueType& valueToFill, - UnpackTypes&... in) +inline bool readJsonHelper(nlohmann::json& jsonRequest, crow::Response& res, + std::span<PerUnpack> toUnpack) { - bool ret = true; - if (key != keyToCheck) + bool result = true; + if (!jsonRequest.is_object()) { - // key is an element at root and should cause extra element error. - // If we are requesting elements that is under key like key/other, - // ignore the extra element error. - ret = - readJsonValues<Count, Index + 1>( - key, jsonValue, res, handled, - // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-array-to-pointer-decay) - in...) && - ret; - return ret; + BMCWEB_LOG_DEBUG << "Json value is not an object"; + messages::unrecognizedRequestBody(res); + return false; } + for (auto& item : jsonRequest.items()) + { + size_t unpackIndex = 0; + for (; unpackIndex < toUnpack.size(); unpackIndex++) + { + PerUnpack& unpackSpec = toUnpack[unpackIndex]; + std::string_view key = unpackSpec.key; + size_t keysplitIndex = key.find('/'); + std::string_view leftover; + if (keysplitIndex != std::string_view::npos) + { + leftover = key.substr(keysplitIndex + 1); + key = key.substr(0, keysplitIndex); + } - handled.set(Index); + if (key != item.key() || unpackSpec.complete) + { + continue; + } - return unpackValue<ValueType>(jsonValue, key, res, valueToFill) && ret; -} + // Sublevel key + if (!leftover.empty()) + { + // Include the slash in the key so we can compare later + key = unpackSpec.key.substr(0, keysplitIndex + 1); + nlohmann::json j; + result = details::unpackValue<nlohmann::json>(item.value(), key, + res, j) && + result; + if (result == false) + { + return result; + } + + std::vector<PerUnpack> nextLevel; + for (PerUnpack& p : toUnpack) + { + if (!p.key.starts_with(key)) + { + continue; + } + std::string_view thisLeftover = p.key.substr(key.size()); + nextLevel.push_back({thisLeftover, p.value, false}); + p.complete = true; + } + + result = readJsonHelper(j, res, nextLevel) && result; + break; + } -template <size_t Index = 0, size_t Count> -bool handleMissing(std::bitset<Count>& /*handled*/, crow::Response& /*res*/) -{ - return true; -} + result = + std::visit( + [&item, &unpackSpec, &res](auto&& val) { + using ContainedT = + std::remove_pointer_t<std::decay_t<decltype(val)>>; + return details::unpackValue<ContainedT>( + item.value(), unpackSpec.key, res, *val); + }, + unpackSpec.value) && + result; + + unpackSpec.complete = true; + break; + } -template <size_t Index = 0, size_t Count, typename ValueType, - typename... UnpackTypes> -bool handleMissing(std::bitset<Count>& handled, crow::Response& res, - const char* key, ValueType& /*unusedValue*/, - UnpackTypes&... in) -{ - bool ret = true; - if (!handled.test(Index) && !IsOptional<ValueType>::value) - { - ret = false; - messages::propertyMissing(res, key); + if (unpackIndex == toUnpack.size()) + { + messages::propertyUnknown(res, item.key()); + result = false; + } } - // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-array-to-pointer-decay) - return details::handleMissing<Index + 1, Count>(handled, res, in...) && ret; + for (PerUnpack& perUnpack : toUnpack) + { + if (perUnpack.complete == false) + { + bool isOptional = std::visit( + [](auto&& val) { + using ContainedType = + std::remove_pointer_t<std::decay_t<decltype(val)>>; + return details::IsOptional<ContainedType>::value; + }, + perUnpack.value); + if (isOptional) + { + continue; + } + messages::propertyMissing(res, perUnpack.key); + result = false; + } + } + return result; } -} // namespace details +inline void packVariant(std::span<PerUnpack> /*toPack*/) +{} -template <typename... UnpackTypes> -bool readJson(nlohmann::json& jsonRequest, crow::Response& res, const char* key, - UnpackTypes&... in) +template <typename FirstType, typename... UnpackTypes> +void packVariant(std::span<PerUnpack> toPack, std::string_view key, + FirstType& first, UnpackTypes&&... in) { - bool result = true; - if (!jsonRequest.is_object()) + if (toPack.empty()) { - BMCWEB_LOG_DEBUG << "Json value is not an object"; - messages::unrecognizedRequestBody(res); - return false; + return; } + toPack[0].key = key; + toPack[0].value = &first; + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-array-to-pointer-decay) + packVariant(toPack.subspan(1), std::forward<UnpackTypes&&>(in)...); +} - std::bitset<(sizeof...(in) + 1) / 2> handled(0); - for (const auto& item : jsonRequest.items()) - { - result = - details::readJsonValues<(sizeof...(in) + 1) / 2, 0, UnpackTypes...>( - item.key(), item.value(), res, handled, key, in...) && - result; - } - - BMCWEB_LOG_DEBUG << "JSON result is: " << result; - - return details::handleMissing(handled, res, key, in...) && result; +template <typename FirstType, typename... UnpackTypes> +bool readJson(nlohmann::json& jsonRequest, crow::Response& res, + std::string_view key, FirstType&& first, UnpackTypes&&... in) +{ + const std::size_t n = sizeof...(UnpackTypes) + 2; + std::array<PerUnpack, n / 2> toUnpack2; + packVariant(toUnpack2, key, first, std::forward<UnpackTypes&&>(in)...); + return readJsonHelper(jsonRequest, res, toUnpack2); } -template <typename... UnpackTypes> -bool readJsonPatch(const crow::Request& req, crow::Response& res, - const char* key, UnpackTypes&... in) +inline std::optional<nlohmann::json> + readJsonPatchHelper(const crow::Request& req, crow::Response& res) { nlohmann::json jsonRequest; if (!json_util::processJsonFromRequest(res, req, jsonRequest)) { BMCWEB_LOG_DEBUG << "Json value not readable"; - return false; + return std::nullopt; } if (jsonRequest.empty()) { BMCWEB_LOG_DEBUG << "Json value is empty"; messages::emptyJSON(res); - return false; + return std::nullopt; } - - return readJson(jsonRequest, res, key, in...); + return {std::move(jsonRequest)}; } template <typename... UnpackTypes> -bool readJsonAction(const crow::Request& req, crow::Response& res, - const char* key, UnpackTypes&... in) +bool readJsonPatch(const crow::Request& req, crow::Response& res, + std::string_view key, UnpackTypes&&... in) { - nlohmann::json jsonRequest; - if (!json_util::processJsonFromRequest(res, req, jsonRequest)) + std::optional<nlohmann::json> jsonRequest = readJsonPatchHelper(req, res); + if (jsonRequest == std::nullopt) { - BMCWEB_LOG_DEBUG << "Json value not readable"; return false; } - return readJson(jsonRequest, res, key, in...); + return readJson(*jsonRequest, res, key, std::forward<UnpackTypes&&>(in)...); } -template <typename Type> -bool getValueFromJsonObject(nlohmann::json& jsonData, const std::string& key, - Type& value) +template <typename... UnpackTypes> +bool readJsonAction(const crow::Request& req, crow::Response& res, + const char* key, UnpackTypes&&... in) { - nlohmann::json::iterator it = jsonData.find(key); - if (it == jsonData.end()) + nlohmann::json jsonRequest; + if (!json_util::processJsonFromRequest(res, req, jsonRequest)) { - BMCWEB_LOG_DEBUG << "Key " << key << " not exist"; + BMCWEB_LOG_DEBUG << "Json value not readable"; return false; } - - return details::unpackValue(*it, key, value); + return readJson(jsonRequest, res, key, std::forward<UnpackTypes&&>(in)...); } + } // namespace json_util } // namespace redfish diff --git a/redfish-core/lib/event_service.hpp b/redfish-core/lib/event_service.hpp index b2b81fa428..bcb30aa79d 100644 --- a/redfish-core/lib/event_service.hpp +++ b/redfish-core/lib/event_service.hpp @@ -476,22 +476,14 @@ inline void requestRoutesEventDestinationCollection(App& app) for (nlohmann::json& mrdObj : *mrdJsonArray) { std::string mrdUri; - if (json_util::getValueFromJsonObject( - mrdObj, "@odata.id", mrdUri)) - { - subValue->metricReportDefinitions.emplace_back( - mrdUri); - } - else + + if (!json_util::readJson(mrdObj, asyncResp->res, + "@odata.id", mrdUri)) + { - messages::propertyValueFormatError( - asyncResp->res, - mrdObj.dump( - 2, ' ', true, - nlohmann::json::error_handler_t::replace), - "MetricReportDefinitions"); return; } + subValue->metricReportDefinitions.emplace_back(mrdUri); } } diff --git a/redfish-core/ut/json_utils_test.cpp b/redfish-core/ut/json_utils_test.cpp index 1c11755114..9312f1abd4 100644 --- a/redfish-core/ut/json_utils_test.cpp +++ b/redfish-core/ut/json_utils_test.cpp @@ -102,6 +102,65 @@ TEST(readJson, JsonVector) EXPECT_TRUE(res.jsonValue.empty()); } +TEST(readJson, JsonSubElementValue) +{ + crow::Response res; + nlohmann::json jsonRequest = R"( + { + "json": {"integer": 42} + } + )"_json; + + int integer = 0; + EXPECT_TRUE(readJson(jsonRequest, res, "json/integer", integer)); + EXPECT_EQ(integer, 42); + EXPECT_EQ(res.result(), boost::beast::http::status::ok); + EXPECT_TRUE(res.jsonValue.empty()); +} + +TEST(readJson, JsonSubElementValueDepth2) +{ + crow::Response res; + nlohmann::json jsonRequest = R"( + { + "json": { + "json2": {"string": "foobar"} + } + } + )"_json; + + std::string foobar; + EXPECT_TRUE(readJson(jsonRequest, res, "json/json2/string", foobar)); + EXPECT_EQ(foobar, "foobar"); + EXPECT_EQ(res.result(), boost::beast::http::status::ok); + EXPECT_TRUE(res.jsonValue.empty()); +} + +TEST(readJson, JsonSubElementValueMultiple) +{ + crow::Response res; + nlohmann::json jsonRequest = R"( + { + "json": { + "integer": 42, + "string": "foobar" + }, + "string": "bazbar" + } + )"_json; + + int integer = 0; + std::string foobar; + std::string bazbar; + EXPECT_TRUE(readJson(jsonRequest, res, "json/integer", integer, + "json/string", foobar, "string", bazbar)); + EXPECT_EQ(integer, 42); + EXPECT_EQ(foobar, "foobar"); + EXPECT_EQ(bazbar, "bazbar"); + EXPECT_EQ(res.result(), boost::beast::http::status::ok); + EXPECT_TRUE(res.jsonValue.empty()); +} + TEST(readJson, ExtraElement) { crow::Response res; |