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>
diff --git a/redfish-core/include/utils/json_utils.hpp b/redfish-core/include/utils/json_utils.hpp
index 21701c7..9aac1dc 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 @@
};
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 @@
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 @@
}
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 @@
}
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,71 +333,66 @@
return ret;
}
-
-template <size_t Count, size_t Index>
-bool readJsonValues(const std::string& key, nlohmann::json& /*jsonValue*/,
- crow::Response& res, std::bitset<Count>& /*handled*/)
-{
- BMCWEB_LOG_DEBUG << "Unable to find variable for key" << key;
- messages::propertyUnknown(res, key);
- return 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)
-{
- bool ret = true;
- if (key != keyToCheck)
- {
- // 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;
- }
-
- handled.set(Index);
-
- return unpackValue<ValueType>(jsonValue, key, res, valueToFill) && ret;
-}
-
-template <size_t Index = 0, size_t Count>
-bool handleMissing(std::bitset<Count>& /*handled*/, crow::Response& /*res*/)
-{
- return true;
-}
-
-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);
- }
-
- // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-array-to-pointer-decay)
- return details::handleMissing<Index + 1, Count>(handled, res, in...) && ret;
-}
-
} // namespace details
-template <typename... UnpackTypes>
-bool readJson(nlohmann::json& jsonRequest, crow::Response& res, const char* key,
- UnpackTypes&... in)
+// 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
+{
+ std::string_view key;
+ UnpackVariant value;
+ bool complete = false;
+};
+
+inline bool readJsonHelper(nlohmann::json& jsonRequest, crow::Response& res,
+ std::span<PerUnpack> toUnpack)
{
bool result = true;
if (!jsonRequest.is_object())
@@ -406,45 +401,162 @@
messages::unrecognizedRequestBody(res);
return false;
}
-
- std::bitset<(sizeof...(in) + 1) / 2> handled(0);
- for (const auto& item : jsonRequest.items())
+ for (auto& item : jsonRequest.items())
{
- result =
- details::readJsonValues<(sizeof...(in) + 1) / 2, 0, UnpackTypes...>(
- item.key(), item.value(), res, handled, key, in...) &&
- result;
+ 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);
+ }
+
+ if (key != item.key() || unpackSpec.complete)
+ {
+ continue;
+ }
+
+ // 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;
+ }
+
+ 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;
+ }
+
+ if (unpackIndex == toUnpack.size())
+ {
+ messages::propertyUnknown(res, item.key());
+ result = false;
+ }
}
- BMCWEB_LOG_DEBUG << "JSON result is: " << result;
-
- return details::handleMissing(handled, res, key, in...) && result;
+ 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;
}
-template <typename... UnpackTypes>
-bool readJsonPatch(const crow::Request& req, crow::Response& res,
- const char* key, UnpackTypes&... in)
+inline void packVariant(std::span<PerUnpack> /*toPack*/)
+{}
+
+template <typename FirstType, typename... UnpackTypes>
+void packVariant(std::span<PerUnpack> toPack, std::string_view key,
+ FirstType& first, UnpackTypes&&... in)
+{
+ if (toPack.empty())
+ {
+ 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)...);
+}
+
+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);
+}
+
+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 std::nullopt;
+ }
+ return {std::move(jsonRequest)};
+}
+
+template <typename... UnpackTypes>
+bool readJsonPatch(const crow::Request& req, crow::Response& res,
+ std::string_view key, UnpackTypes&&... in)
+{
+ std::optional<nlohmann::json> jsonRequest = readJsonPatchHelper(req, res);
+ if (jsonRequest == std::nullopt)
+ {
return false;
}
- return readJson(jsonRequest, res, key, in...);
+ return readJson(*jsonRequest, res, key, std::forward<UnpackTypes&&>(in)...);
}
template <typename... UnpackTypes>
bool readJsonAction(const crow::Request& req, crow::Response& res,
- const char* key, UnpackTypes&... in)
+ const char* key, UnpackTypes&&... in)
{
nlohmann::json jsonRequest;
if (!json_util::processJsonFromRequest(res, req, jsonRequest))
@@ -452,22 +564,8 @@
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)
-{
- nlohmann::json::iterator it = jsonData.find(key);
- if (it == jsonData.end())
- {
- BMCWEB_LOG_DEBUG << "Key " << key << " not exist";
- return false;
- }
-
- return details::unpackValue(*it, key, value);
-}
} // namespace json_util
} // namespace redfish
diff --git a/redfish-core/lib/event_service.hpp b/redfish-core/lib/event_service.hpp
index b2b81fa..bcb30aa 100644
--- a/redfish-core/lib/event_service.hpp
+++ b/redfish-core/lib/event_service.hpp
@@ -476,22 +476,14 @@
for (nlohmann::json& mrdObj : *mrdJsonArray)
{
std::string mrdUri;
- if (json_util::getValueFromJsonObject(
- mrdObj, "@odata.id", mrdUri))
+
+ if (!json_util::readJson(mrdObj, asyncResp->res,
+ "@odata.id", mrdUri))
+
{
- subValue->metricReportDefinitions.emplace_back(
- mrdUri);
- }
- else
- {
- 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 1c11755..9312f1a 100644
--- a/redfish-core/ut/json_utils_test.cpp
+++ b/redfish-core/ut/json_utils_test.cpp
@@ -102,6 +102,65 @@
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;