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

1 files changed, 43 insertions, 26 deletions

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 <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 @@ void unpackValue(nlohmann::json& jsonValue, const std::string& key,
         }
         if (!checkRange<Type>(jsonPtr, key, jsonValue, res))
         {
-            return;
+            return false;
         }
         value = static_cast<Type>(*jsonPtr);
     }
@@ -147,7 +149,7 @@ void unpackValue(nlohmann::json& jsonValue, const std::string& key,
         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 @@ void unpackValue(nlohmann::json& jsonValue, const std::string& key,
         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 @@ void unpackValue(nlohmann::json& jsonValue, const std::string& key,
     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 @@ 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<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 @@ 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<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 @@ 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 <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 @@ template <typename... UnpackTypes>
 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 <typename... UnpackTypes>