health: take json_ptr instead of reference when filling status

The existing codes populates the health status on the |AsyncResponse| or
a given JSON reference. This doesn't work if we want to populates status
on an array of objects, since the array can be resized which changes the
address of each object.

This commit changed the contructor to take a JSON pointer instead.
|HealthPopulate| will populates status on
|AsyncResponse->res.jsonValue|[json_ptr]. If the point can't be resolved
in the |jsonValue|, |HealthPopulate| populates nothing.

Fixed all places where the old reference based constructor is used.

This commit is extremely useful when implementing efficient level-1
expand handler on ResourceCollections. It also prevents issues on
reference lifecycles.

Tested:
1. It builds
2. Tested DIMM/System/Storage health on real hardware, works as expected
3. Tested on Redfish Service Validator, no new failures on health
properties.

Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I305515522af50b48be92a3f4689d8166f3bc0cc0

3 files changed, 18 insertions, 7 deletions

diff --git a/redfish-core/lib/health.hpp b/redfish-core/lib/health.hpp
index 31adf7b431..254f8e9203 100644
--- a/redfish-core/lib/health.hpp
+++ b/redfish-core/lib/health.hpp
@@ -22,6 +22,7 @@
 #include <boost/container/flat_set.hpp>
 #include <dbus_singleton.hpp>
 #include <dbus_utility.hpp>
+#include <nlohmann/json.hpp>
 
 #include <variant>
 
@@ -30,14 +31,18 @@ namespace redfish
 
 struct HealthPopulate : std::enable_shared_from_this<HealthPopulate>
 {
+    // By default populate status to "/Status" of |asyncResp->res.jsonValue|.
     HealthPopulate(const std::shared_ptr<bmcweb::AsyncResp>& asyncRespIn) :
-        asyncResp(asyncRespIn), jsonStatus(asyncResp->res.jsonValue["Status"])
+        asyncResp(asyncRespIn), statusPtr("/Status")
     {}
 
+    // Takes a JSON pointer rather than a reference. This is pretty useful when
+    // the address of the status JSON might change, for example, elements in an
+    // array.
     HealthPopulate(const std::shared_ptr<bmcweb::AsyncResp>& asyncRespIn,
-                   nlohmann::json& status) :
+                   const nlohmann::json::json_pointer& ptr) :
         asyncResp(asyncRespIn),
-        jsonStatus(status)
+        statusPtr(ptr)
     {}
 
     HealthPopulate(const HealthPopulate&) = delete;
@@ -47,6 +52,7 @@ struct HealthPopulate : std::enable_shared_from_this<HealthPopulate>
 
     ~HealthPopulate()
     {
+        nlohmann::json& jsonStatus = asyncResp->res.jsonValue[statusPtr];
         nlohmann::json& health = jsonStatus["Health"];
         nlohmann::json& rollup = jsonStatus["HealthRollup"];
 
@@ -232,7 +238,9 @@ struct HealthPopulate : std::enable_shared_from_this<HealthPopulate>
     }
 
     std::shared_ptr<bmcweb::AsyncResp> asyncResp;
-    nlohmann::json& jsonStatus;
+
+    // Will populate the health status into |asyncResp_json[statusPtr]|
+    nlohmann::json::json_pointer statusPtr;
 
     // we store pointers to other HealthPopulate items so we can update their
     // members and reduce dbus calls. As we hold a shared_ptr to them, they get
diff --git a/redfish-core/lib/storage.hpp b/redfish-core/lib/storage.hpp
index c86adabe54..ab59c33cf9 100644
--- a/redfish-core/lib/storage.hpp
+++ b/redfish-core/lib/storage.hpp
@@ -248,10 +248,13 @@ inline void requestRoutesStorage(App& app)
                     // be resized, as json::array uses vector underneath and we
                     // need references to its members that won't change
                     size_t count = 0;
+                    // Pointer based on |asyncResp->res.jsonValue|
+                    nlohmann::json::json_pointer rootPtr =
+                        "/StorageControllers"_json_pointer;
                     for (const auto& [path, interfaceDict] : subtree)
                     {
                         auto subHealth = std::make_shared<HealthPopulate>(
-                            asyncResp, root[count]["Status"]);
+                            asyncResp, rootPtr / count / "Status");
                         subHealth->inventory.emplace_back(path);
                         health->inventory.emplace_back(path);
                         health->children.emplace_back(subHealth);
diff --git a/redfish-core/lib/systems.hpp b/redfish-core/lib/systems.hpp
index 58bab2ddbf..b11bb908f1 100644
--- a/redfish-core/lib/systems.hpp
+++ b/redfish-core/lib/systems.hpp
@@ -269,10 +269,10 @@ inline void
                 }
 
                 auto memoryHealth = std::make_shared<HealthPopulate>(
-                    aResp, aResp->res.jsonValue["MemorySummary"]["Status"]);
+                    aResp, "/MemorySummary/Status"_json_pointer);
 
                 auto cpuHealth = std::make_shared<HealthPopulate>(
-                    aResp, aResp->res.jsonValue["ProcessorSummary"]["Status"]);
+                    aResp, "/ProcessorSummary/Status"_json_pointer);
 
                 systemHealth->children.emplace_back(memoryHealth);
                 systemHealth->children.emplace_back(cpuHealth);