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 <santosh.puranik@in.ibm.com>
Change-Id: I54080392297a8f0276161da8b48d8f9518cbdcfe
diff --git a/redfish-core/include/utils/json_utils.hpp b/redfish-core/include/utils/json_utils.hpp
index 4c35b0c..775de64 100644
--- a/redfish-core/include/utils/json_utils.hpp
+++ b/redfish-core/include/utils/json_utils.hpp
@@ -118,9 +118,11 @@
}
template <typename Type>
-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<Type>)
{
double helper = 0;
@@ -137,7 +139,7 @@
}
if (!checkRange<Type>(jsonPtr, key, jsonValue, res))
{
- return;
+ return false;
}
value = static_cast<Type>(*jsonPtr);
}
@@ -147,7 +149,7 @@
int64_t* jsonPtr = jsonValue.get_ptr<int64_t*>();
if (!checkRange<Type>(jsonPtr, key, jsonValue, res))
{
- return;
+ return false;
}
value = static_cast<Type>(*jsonPtr);
}
@@ -158,7 +160,7 @@
uint64_t* jsonPtr = jsonValue.get_ptr<uint64_t*>();
if (!checkRange<Type>(jsonPtr, key, jsonValue, res))
{
- return;
+ return false;
}
value = static_cast<Type>(*jsonPtr);
}
@@ -166,7 +168,9 @@
else if constexpr (is_optional_v<Type>)
{
value.emplace();
- unpackValue<typename Type::value_type>(jsonValue, key, res, *value);
+ ret = unpackValue<typename Type::value_type>(jsonValue, key, res,
+ *value) &&
+ ret;
}
else if constexpr (std::is_same_v<nlohmann::json, Type>)
{
@@ -175,7 +179,7 @@
if (!jsonValue.is_object() && !jsonValue.is_array())
{
messages::propertyValueTypeError(res, jsonValue.dump(), key);
- return;
+ return false;
}
value = std::move(jsonValue);
@@ -185,18 +189,19 @@
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<typename Type::value_type>(val.value(), key, res,
- value[index++]);
+ ret = unpackValue<typename Type::value_type>(val.value(), key, res,
+ value[index++]) &&
+ ret;
}
}
else if constexpr (is_vector_v<Type>)
@@ -204,14 +209,15 @@
if (!jsonValue.is_array())
{
messages::propertyValueTypeError(res, res.jsonValue.dump(), key);
- return;
+ return false;
}
for (const auto& val : jsonValue.items())
{
value.emplace_back();
- unpackValue<typename Type::value_type>(val.value(), key, res,
- value.back());
+ ret = unpackValue<typename Type::value_type>(val.value(), key, res,
+ value.back()) &&
+ ret;
}
}
else
@@ -224,53 +230,61 @@
<< "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 <size_t Count, size_t Index>
-void readJsonValues(const std::string& key, nlohmann::json& jsonValue,
+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>
-void readJsonValues(const std::string& key, nlohmann::json& jsonValue,
+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)
{
- readJsonValues<Count, Index + 1>(key, jsonValue, res, handled, in...);
- return;
+ ret = readJsonValues<Count, Index + 1>(key, jsonValue, res, handled,
+ in...) &&
+ ret;
+ return ret;
}
handled.set(Index);
- unpackValue<ValueType>(jsonValue, key, res, valueToFill);
+ return unpackValue<ValueType>(jsonValue, key, res, valueToFill) && ret;
}
template <size_t Index = 0, size_t Count>
-void handleMissing(std::bitset<Count>& handled, crow::Response& res)
+bool handleMissing(std::bitset<Count>& handled, crow::Response& res)
{
+ return true;
}
template <size_t Index = 0, size_t Count, typename ValueType,
typename... UnpackTypes>
-void handleMissing(std::bitset<Count>& handled, crow::Response& res,
+bool handleMissing(std::bitset<Count>& handled, crow::Response& res,
const char* key, ValueType& unused, UnpackTypes&... in)
{
+ bool ret = true;
if (!handled.test(Index) && !is_optional_v<ValueType>)
{
+ ret = false;
messages::propertyMissing(res, key);
}
- details::handleMissing<Index + 1, Count>(handled, res, in...);
+ return details::handleMissing<Index + 1, Count>(handled, res, in...) && ret;
}
} // namespace details
@@ -278,6 +292,7 @@
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 @@
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 <typename... UnpackTypes>
diff --git a/redfish-core/lib/managers.hpp b/redfish-core/lib/managers.hpp
index 9c250e8..4ebff71 100644
--- a/redfish-core/lib/managers.hpp
+++ b/redfish-core/lib/managers.hpp
@@ -1654,14 +1654,14 @@
{
std::optional<nlohmann::json> oem;
std::optional<std::string> datetime;
+ std::shared_ptr<AsyncResp> response = std::make_shared<AsyncResp>(res);
- if (!json_util::readJson(req, res, "Oem", oem, "DateTime", datetime))
+ if (!json_util::readJson(req, response->res, "Oem", oem, "DateTime",
+ datetime))
{
return;
}
- std::shared_ptr<AsyncResp> response = std::make_shared<AsyncResp>(res);
-
if (oem)
{
std::optional<nlohmann::json> openbmc;
diff --git a/redfish-core/lib/systems.hpp b/redfish-core/lib/systems.hpp
index c092379..038387c 100644
--- a/redfish-core/lib/systems.hpp
+++ b/redfish-core/lib/systems.hpp
@@ -1453,13 +1453,14 @@
{
std::optional<std::string> indicatorLed;
std::optional<nlohmann::json> bootProps;
- if (!json_util::readJson(req, res, "IndicatorLED", indicatorLed, "Boot",
- bootProps))
+ auto asyncResp = std::make_shared<AsyncResp>(res);
+
+ if (!json_util::readJson(req, asyncResp->res, "IndicatorLED",
+ indicatorLed, "Boot", bootProps))
{
return;
}
- auto asyncResp = std::make_shared<AsyncResp>(res);
asyncResp->res.result(boost::beast::http::status::no_content);
if (bootProps)