From 41352c2412afb93f0f1749f70c96584696197e19 Mon Sep 17 00:00:00 2001 From: Santosh Puranik Date: Wed, 3 Jul 2019 05:35:49 -0500 Subject: Fix Regression in PATCH on system node Commit d573bb2a regressed the PATCH operation on the system redfish node. By initializing result to no_content, the commit broke the subsequent json_util::readJson, which checks if the result is "ok" at the end of the function. This commit fixes it by making readJsonValues() and its descendants return a bool status. readJson() no longer relies on the response object result to indicate success/ failure. Tested: -- Verified that PATCH operations on the system node now work. -- Verified PATCH operations work on manager, accountservice and ethernetinterfaces nodes. -- No new errors from the Redfish validator. Signed-off-by: Santosh Puranik Change-Id: I54080392297a8f0276161da8b48d8f9518cbdcfe --- redfish-core/include/utils/json_utils.hpp | 69 +++++++++++++++++++------------ 1 file changed, 43 insertions(+), 26 deletions(-) (limited to 'redfish-core/include/utils/json_utils.hpp') diff --git a/redfish-core/include/utils/json_utils.hpp b/redfish-core/include/utils/json_utils.hpp index 4c35b0c00b..775de64a90 100644 --- a/redfish-core/include/utils/json_utils.hpp +++ b/redfish-core/include/utils/json_utils.hpp @@ -118,9 +118,11 @@ bool checkRange(const FromType* from, const std::string& key, } template -void unpackValue(nlohmann::json& jsonValue, const std::string& key, +bool unpackValue(nlohmann::json& jsonValue, const std::string& key, crow::Response& res, Type& value) { + bool ret = true; + if constexpr (std::is_floating_point_v) { double helper = 0; @@ -137,7 +139,7 @@ void unpackValue(nlohmann::json& jsonValue, const std::string& key, } if (!checkRange(jsonPtr, key, jsonValue, res)) { - return; + return false; } value = static_cast(*jsonPtr); } @@ -147,7 +149,7 @@ void unpackValue(nlohmann::json& jsonValue, const std::string& key, int64_t* jsonPtr = jsonValue.get_ptr(); if (!checkRange(jsonPtr, key, jsonValue, res)) { - return; + return false; } value = static_cast(*jsonPtr); } @@ -158,7 +160,7 @@ void unpackValue(nlohmann::json& jsonValue, const std::string& key, uint64_t* jsonPtr = jsonValue.get_ptr(); if (!checkRange(jsonPtr, key, jsonValue, res)) { - return; + return false; } value = static_cast(*jsonPtr); } @@ -166,7 +168,9 @@ void unpackValue(nlohmann::json& jsonValue, const std::string& key, else if constexpr (is_optional_v) { value.emplace(); - unpackValue(jsonValue, key, res, *value); + ret = unpackValue(jsonValue, key, res, + *value) && + ret; } else if constexpr (std::is_same_v) { @@ -175,7 +179,7 @@ void unpackValue(nlohmann::json& jsonValue, const std::string& key, if (!jsonValue.is_object() && !jsonValue.is_array()) { messages::propertyValueTypeError(res, jsonValue.dump(), key); - return; + return false; } value = std::move(jsonValue); @@ -185,18 +189,19 @@ void unpackValue(nlohmann::json& jsonValue, const std::string& key, if (!jsonValue.is_array()) { messages::propertyValueTypeError(res, res.jsonValue.dump(), key); - return; + return false; } if (jsonValue.size() != value.size()) { messages::propertyValueTypeError(res, res.jsonValue.dump(), key); - return; + return false; } size_t index = 0; for (const auto& val : jsonValue.items()) { - unpackValue(val.value(), key, res, - value[index++]); + ret = unpackValue(val.value(), key, res, + value[index++]) && + ret; } } else if constexpr (is_vector_v) @@ -204,14 +209,15 @@ void unpackValue(nlohmann::json& jsonValue, const std::string& key, if (!jsonValue.is_array()) { messages::propertyValueTypeError(res, res.jsonValue.dump(), key); - return; + return false; } for (const auto& val : jsonValue.items()) { value.emplace_back(); - unpackValue(val.value(), key, res, - value.back()); + ret = unpackValue(val.value(), key, res, + value.back()) && + ret; } } else @@ -224,53 +230,61 @@ void unpackValue(nlohmann::json& jsonValue, const std::string& key, << "Value for key " << key << " was incorrect type: " << jsonValue.type_name(); messages::propertyValueTypeError(res, jsonValue.dump(), key); - return; + return false; } value = std::move(*jsonPtr); } + return ret; } template -void readJsonValues(const std::string& key, nlohmann::json& jsonValue, +bool readJsonValues(const std::string& key, nlohmann::json& jsonValue, crow::Response& res, std::bitset& handled) { BMCWEB_LOG_DEBUG << "Unable to find variable for key" << key; messages::propertyUnknown(res, key); + return false; } template -void readJsonValues(const std::string& key, nlohmann::json& jsonValue, +bool readJsonValues(const std::string& key, nlohmann::json& jsonValue, crow::Response& res, std::bitset& handled, const char* keyToCheck, ValueType& valueToFill, UnpackTypes&... in) { + bool ret = true; if (key != keyToCheck) { - readJsonValues(key, jsonValue, res, handled, in...); - return; + ret = readJsonValues(key, jsonValue, res, handled, + in...) && + ret; + return ret; } handled.set(Index); - unpackValue(jsonValue, key, res, valueToFill); + return unpackValue(jsonValue, key, res, valueToFill) && ret; } template -void handleMissing(std::bitset& handled, crow::Response& res) +bool handleMissing(std::bitset& handled, crow::Response& res) { + return true; } template -void handleMissing(std::bitset& handled, crow::Response& res, +bool handleMissing(std::bitset& handled, crow::Response& res, const char* key, ValueType& unused, UnpackTypes&... in) { + bool ret = true; if (!handled.test(Index) && !is_optional_v) { + ret = false; messages::propertyMissing(res, key); } - details::handleMissing(handled, res, in...); + return details::handleMissing(handled, res, in...) && ret; } } // namespace details @@ -278,6 +292,7 @@ template bool readJson(nlohmann::json& jsonRequest, crow::Response& res, const char* key, UnpackTypes&... in) { + bool result = true; if (!jsonRequest.is_object()) { BMCWEB_LOG_DEBUG << "Json value is not an object"; @@ -295,13 +310,15 @@ bool readJson(nlohmann::json& jsonRequest, crow::Response& res, const char* key, std::bitset<(sizeof...(in) + 1) / 2> handled(0); for (const auto& item : jsonRequest.items()) { - details::readJsonValues<(sizeof...(in) + 1) / 2, 0, UnpackTypes...>( - item.key(), item.value(), res, handled, key, in...); + result = + details::readJsonValues<(sizeof...(in) + 1) / 2, 0, UnpackTypes...>( + item.key(), item.value(), res, handled, key, in...) && + result; } - details::handleMissing(handled, res, key, in...); + BMCWEB_LOG_DEBUG << "JSON result is: " << result; - return res.result() == boost::beast::http::status::ok; + return details::handleMissing(handled, res, key, in...) && result; } template -- cgit v1.2.3