Cleanups to update service schema

This commit moves update service to a number of better practices, and
avoids a few copies along the way.

Change-Id: I3cea25a8f804b8e1109a1e5bb8becf69a4b77e71
Signed-off-by: Ed Tanous <ed.tanous@intel.com>
diff --git a/redfish-core/lib/update_service.hpp b/redfish-core/lib/update_service.hpp
index efa0fbf..677b6d1 100644
--- a/redfish-core/lib/update_service.hpp
+++ b/redfish-core/lib/update_service.hpp
@@ -21,43 +21,6 @@
 namespace redfish {
 static std::unique_ptr<sdbusplus::bus::match::match> fwUpdateMatcher;
 
-class OnDemandSoftwareInventoryProvider {
- public:
-  template <typename CallbackFunc>
-  void getAllSoftwareInventoryObject(CallbackFunc &&callback) {
-    crow::connections::systemBus->async_method_call(
-        [callback{std::move(callback)}](
-            const boost::system::error_code error_code,
-            const std::vector<std::pair<
-                std::string,
-                std::vector<std::pair<std::string, std::vector<std::string>>>>>
-                &subtree) {
-          BMCWEB_LOG_DEBUG << "get all software inventory object callback...";
-          if (error_code) {
-            // Something wrong on DBus, the error_code is not important at this
-            // moment, just return success=false, and empty output. Since size
-            // of vector may vary depending on information from Entity Manager,
-            // and empty output could not be treated same way as error.
-            callback(false, subtree);
-            return;
-          }
-
-          if (subtree.empty()) {
-            BMCWEB_LOG_DEBUG << "subtree empty";
-            callback(false, subtree);
-          } else {
-            BMCWEB_LOG_DEBUG << "subtree has something";
-            callback(true, subtree);
-          }
-        },
-        "xyz.openbmc_project.ObjectMapper",
-        "/xyz/openbmc_project/object_mapper",
-        "xyz.openbmc_project.ObjectMapper", "GetSubTree",
-        "/xyz/openbmc_project/software", int32_t(1),
-        std::array<const char *, 1>{"xyz.openbmc_project.Software.Activation"});
-  }
-};
-
 class UpdateService : public Node {
  public:
   UpdateService(CrowApp &app) : Node(app, "/redfish/v1/UpdateService/") {
@@ -69,7 +32,8 @@
     Node::json["Description"] = "Service for Software Update";
     Node::json["Name"] = "Update Service";
     Node::json["HttpPushUri"] = "/redfish/v1/UpdateService";
-    Node::json["ServiceEnabled"] = true;  // UpdateService cannot be disabled
+    // UpdateService cannot be disabled
+    Node::json["ServiceEnabled"] = true;
     Node::json["FirmwareInventory"] = {
         {"@odata.id", "/redfish/v1/UpdateService/FirmwareInventory"}};
 
@@ -88,15 +52,15 @@
     res.jsonValue = Node::json;
     res.end();
   }
-  static void activateImage(const std::string &obj_path) {
+  static void activateImage(const std::string &objPath) {
     crow::connections::systemBus->async_method_call(
-        [obj_path](const boost::system::error_code error_code) {
+        [objPath](const boost::system::error_code error_code) {
           if (error_code) {
             BMCWEB_LOG_DEBUG << "error_code = " << error_code;
             BMCWEB_LOG_DEBUG << "error msg = " << error_code.message();
           }
         },
-        "xyz.openbmc_project.Software.BMC.Updater", obj_path,
+        "xyz.openbmc_project.Software.BMC.Updater", objPath,
         "org.freedesktop.DBus.Properties", "Set",
         "xyz.openbmc_project.Software.Activation", "RequestedActivation",
         sdbusplus::message::variant<std::string>(
@@ -154,13 +118,13 @@
                                 sdbusplus::message::variant<std::string>>>>>
           interfaces_properties;
 
-      sdbusplus::message::object_path obj_path;
+      sdbusplus::message::object_path objPath;
 
-      m.read(obj_path, interfaces_properties);  // Read in the object path
-                                                // that was just created
-      // std::string str_objpath = obj_path.str;  // keep a copy for
+      m.read(objPath, interfaces_properties);  // Read in the object path
+                                               // that was just created
+      // std::string str_objpath = objPath.str;  // keep a copy for
       // constructing response message
-      BMCWEB_LOG_DEBUG << "obj path = " << obj_path.str;  // str_objpath;
+      BMCWEB_LOG_DEBUG << "obj path = " << objPath.str;  // str_objpath;
       for (auto &interface : interfaces_properties) {
         BMCWEB_LOG_DEBUG << "interface = " << interface.first;
 
@@ -172,7 +136,7 @@
           if (ec) {
             BMCWEB_LOG_ERROR << "error canceling timer " << ec;
           }
-          UpdateService::activateImage(obj_path.str);  // str_objpath);
+          UpdateService::activateImage(objPath.str);  // str_objpath);
           res.jsonValue = redfish::messages::success();
           BMCWEB_LOG_DEBUG << "ending response";
           res.end();
@@ -201,9 +165,6 @@
 
 class SoftwareInventoryCollection : public Node {
  public:
-  /*
-   * Default Constructor
-   */
   template <typename CrowApp>
   SoftwareInventoryCollection(CrowApp &app)
       : Node(app, "/redfish/v1/UpdateService/FirmwareInventory/") {
@@ -225,107 +186,83 @@
   }
 
  private:
-  /**
-   * Functions triggers appropriate requests on DBus
-   */
   void doGet(crow::Response &res, const crow::Request &req,
              const std::vector<std::string> &params) override {
+    std::shared_ptr<AsyncResp> asyncResp = std::make_shared<AsyncResp>(res);
     res.jsonValue = Node::json;
-    softwareInventoryProvider.getAllSoftwareInventoryObject(
-        [&](const bool &success,
+
+    crow::connections::systemBus->async_method_call(
+        [asyncResp](
+            const boost::system::error_code ec,
             const std::vector<std::pair<
                 std::string,
                 std::vector<std::pair<std::string, std::vector<std::string>>>>>
                 &subtree) {
-          if (!success) {
-            res.result(boost::beast::http::status::internal_server_error);
-            res.jsonValue = messages::internalError();
-            res.end();
+          if (ec) {
+            asyncResp->res.result(
+                boost::beast::http::status::internal_server_error);
             return;
           }
-
-          if (subtree.empty()) {
-            BMCWEB_LOG_DEBUG << "subtree empty!!";
-            res.result(boost::beast::http::status::internal_server_error);
-            res.jsonValue = messages::internalError();
-            res.end();
-            return;
-          }
-
-          res.jsonValue["Members"] = nlohmann::json::array();
-          res.jsonValue["Members@odata.count"] = 0;
-
-          std::shared_ptr<AsyncResp> asyncResp =
-              std::make_shared<AsyncResp>(res);
+          asyncResp->res.jsonValue["Members"] = nlohmann::json::array();
+          asyncResp->res.jsonValue["Members@odata.count"] = 0;
 
           for (auto &obj : subtree) {
             const std::vector<std::pair<std::string, std::vector<std::string>>>
                 &connections = obj.second;
 
-            // if can't parse fw id then return
-            std::size_t id_pos;
-            if ((id_pos = obj.first.rfind("/")) == std::string::npos) {
-              res.result(boost::beast::http::status::internal_server_error);
-              res.jsonValue = messages::internalError();
-              BMCWEB_LOG_DEBUG << "Can't parse firmware ID!!";
-              res.end();
-              return;
-            }
-            std::string fw_id = obj.first.substr(id_pos + 1);
-
-            for (const auto &conn : connections) {
-              const std::string connectionName = conn.first;
+            for (auto &conn : connections) {
+              const std::string &connectionName = conn.first;
               BMCWEB_LOG_DEBUG << "connectionName = " << connectionName;
               BMCWEB_LOG_DEBUG << "obj.first = " << obj.first;
 
               crow::connections::systemBus->async_method_call(
-                  [asyncResp, fw_id](
-                      const boost::system::error_code error_code,
-                      const sdbusplus::message::variant<std::string>
-                          &activation_status) {
+                  [asyncResp](const boost::system::error_code error_code,
+                              const VariantType &activation) {
                     BMCWEB_LOG_DEBUG << "safe returned in lambda function";
                     if (error_code) {
                       asyncResp->res.result(
                           boost::beast::http::status::internal_server_error);
-                      asyncResp->res.jsonValue = messages::internalError();
-                      asyncResp->res.end();
                       return;
                     }
-                    const std::string *activation_status_str =
-                        mapbox::getPtr<const std::string>(activation_status);
-                    if (activation_status_str != nullptr &&
-                        *activation_status_str !=
-                            "xyz.openbmc_project.Software.Activation."
-                            "Activations.Active") {
-                      // The activation status of this software is not currently
-                      // active, so does not need to be listed in the response
+
+                    const std::string *sw_inv_purpose =
+                        mapbox::getPtr<const std::string>(activation);
+                    if (sw_inv_purpose == nullptr) {
+                      asyncResp->res.result(
+                          boost::beast::http::status::internal_server_error);
                       return;
                     }
-                    asyncResp->res.jsonValue["Members"].push_back(
+                    std::size_t last_pos = sw_inv_purpose->rfind(".");
+                    if (last_pos == std::string::npos) {
+                      asyncResp->res.result(
+                          boost::beast::http::status::internal_server_error);
+                      return;
+                    }
+                    nlohmann::json &members =
+                        asyncResp->res.jsonValue["Members"];
+                    members.push_back(
                         {{"@odata.id",
                           "/redfish/v1/UpdateService/FirmwareInventory/" +
-                              fw_id}});
+                              sw_inv_purpose->substr(last_pos + 1)}});
                     asyncResp->res.jsonValue["Members@odata.count"] =
-                        asyncResp->res.jsonValue["Members"].size();
+                        members.size();
                   },
                   connectionName, obj.first, "org.freedesktop.DBus.Properties",
                   "Get", "xyz.openbmc_project.Software.Activation",
                   "Activation");
             }
           }
-        });
+        },
+        "xyz.openbmc_project.ObjectMapper",
+        "/xyz/openbmc_project/object_mapper",
+        "xyz.openbmc_project.ObjectMapper", "GetSubTree",
+        "/xyz/openbmc_project/software", int32_t(1),
+        std::array<const char *, 1>{"xyz.openbmc_project.Software.Version"});
   }
-
-  OnDemandSoftwareInventoryProvider softwareInventoryProvider;
 };
-/**
- * Chassis override class for delivering Chassis Schema
- */
+
 class SoftwareInventory : public Node {
  public:
-  /*
-   * Default Constructor
-   */
   template <typename CrowApp>
   SoftwareInventory(CrowApp &app)
       : Node(app, "/redfish/v1/UpdateService/FirmwareInventory/<str>/",
@@ -348,11 +285,9 @@
   }
 
  private:
-  /**
-   * Functions triggers appropriate requests on DBus
-   */
   void doGet(crow::Response &res, const crow::Request &req,
              const std::vector<std::string> &params) override {
+    std::shared_ptr<AsyncResp> asyncResp = std::make_shared<AsyncResp>(res);
     res.jsonValue = Node::json;
 
     if (params.size() != 1) {
@@ -362,122 +297,100 @@
       return;
     }
 
-    const std::string &fw_id = params[0];
-    res.jsonValue["Id"] = fw_id;
+    std::shared_ptr<std::string> sw_id =
+        std::make_shared<std::string>(params[0]);
+
     res.jsonValue["@odata.id"] =
-        "/redfish/v1/UpdateService/FirmwareInventory/" + fw_id;
-    softwareInventoryProvider.getAllSoftwareInventoryObject([
-      &res, id{std::string(fw_id)}
-    ](const bool &success,
-      const std::vector<std::pair<
-          std::string,
-          std::vector<std::pair<std::string, std::vector<std::string>>>>>
-          &subtree) {
-      BMCWEB_LOG_DEBUG << "doGet callback...";
-      if (!success) {
-        res.result(boost::beast::http::status::internal_server_error);
-        res.jsonValue = messages::internalError();
-        res.end();
-        return;
-      }
+        "/redfish/v1/UpdateService/FirmwareInventory/" + *sw_id;
 
-      if (subtree.empty()) {
-        BMCWEB_LOG_ERROR << "subtree empty!!";
-        res.result(boost::beast::http::status::not_found);
-        res.end();
-        return;
-      }
+    crow::connections::systemBus->async_method_call(
+        [asyncResp, sw_id](
+            const boost::system::error_code ec,
+            const std::vector<std::pair<
+                std::string,
+                std::vector<std::pair<std::string, std::vector<std::string>>>>>
+                &subtree) {
+          BMCWEB_LOG_DEBUG << "doGet callback...";
+          if (ec) {
+            asyncResp->res.result(
+                boost::beast::http::status::internal_server_error);
+            return;
+          }
 
-      bool fw_id_found = false;
+          for (const std::pair<std::string,
+                               std::vector<std::pair<std::string,
+                                                     std::vector<std::string>>>>
+                   &obj : subtree) {
+            if (boost::ends_with(obj.first, *sw_id) != true) {
+              continue;
+            }
 
-      for (auto &obj : subtree) {
-        if (boost::ends_with(obj.first, id) != true) {
-          continue;
-        }
-        fw_id_found = true;
+            if (obj.second.size() <= 1) {
+              continue;
+            }
 
-        const std::vector<std::pair<std::string, std::vector<std::string>>>
-            &connections = obj.second;
+            crow::connections::systemBus->async_method_call(
+                [asyncResp, sw_id](
+                    const boost::system::error_code error_code,
+                    const boost::container::flat_map<std::string, VariantType>
+                        &propertiesList) {
+                  if (error_code) {
+                    asyncResp->res.result(
+                        boost::beast::http::status::internal_server_error);
+                    return;
+                  }
+                  boost::container::flat_map<std::string,
+                                             VariantType>::const_iterator it =
+                      propertiesList.find("Purpose");
+                  if (it == propertiesList.end()) {
+                    BMCWEB_LOG_DEBUG << "Can't find property \"Purpose\"!";
+                    asyncResp->res.result(
+                        boost::beast::http::status::internal_server_error);
+                    return;
+                  }
+                  const std::string *sw_inv_purpose =
+                      mapbox::getPtr<const std::string>(it->second);
+                  if (sw_inv_purpose == nullptr) {
+                    BMCWEB_LOG_DEBUG << "wrong types for property\"Purpose\"!";
+                    asyncResp->res.result(
+                        boost::beast::http::status::internal_server_error);
+                    return;
+                  }
 
-        if (connections.size() <= 0) {
-          continue;
-        }
-        const std::pair<std::string, std::vector<std::string>> &conn =
-            connections[0];
-        const std::string &connectionName = conn.first;
-        BMCWEB_LOG_DEBUG << "connectionName = " << connectionName;
-        BMCWEB_LOG_DEBUG << "obj.first = " << obj.first;
+                  BMCWEB_LOG_DEBUG << "sw_inv_purpose = " << *sw_inv_purpose;
+                  if (boost::ends_with(*sw_inv_purpose, "." + *sw_id)) {
+                    it = propertiesList.find("Version");
+                    if (it == propertiesList.end()) {
+                      BMCWEB_LOG_DEBUG << "Can't find property \"Version\"!";
+                      asyncResp->res.result(
+                          boost::beast::http::status::internal_server_error);
+                      return;
+                    }
 
-        crow::connections::systemBus->async_method_call(
-            [&res, id](
-                const boost::system::error_code error_code,
-                const boost::container::flat_map<std::string, VariantType>
-                    &propertiesList) {
-              if (error_code) {
-                res.result(boost::beast::http::status::internal_server_error);
-                res.jsonValue = messages::internalError();
-                res.end();
-                return;
-              }
+                    const std::string *version =
+                        mapbox::getPtr<const std::string>(it->second);
 
-              boost::container::flat_map<std::string,
-                                         VariantType>::const_iterator it =
-                  propertiesList.find("Purpose");
-              if (it == propertiesList.end()) {
-                BMCWEB_LOG_ERROR << "Can't find property \"Purpose\"!";
-                res.result(boost::beast::http::status::internal_server_error);
-                res.jsonValue = messages::internalError();
-                res.end();
-                return;
-              }
-
-              // SoftwareId
-              const std::string *sw_inv_purpose =
-                  mapbox::getPtr<const std::string>(it->second);
-              if (sw_inv_purpose == nullptr) {
-                res.jsonValue = redfish::messages::internalError();
-                res.jsonValue = messages::internalError();
-                return;
-              }
-              BMCWEB_LOG_DEBUG << "sw_inv_purpose = " << sw_inv_purpose;
-              std::size_t last_pos = sw_inv_purpose->rfind(".");
-              if (last_pos != std::string::npos) {
-                res.jsonValue["SoftwareId"] =
-                    sw_inv_purpose->substr(last_pos + 1);
-              } else {
-                BMCWEB_LOG_ERROR << "Can't parse software purpose!";
-              }
-
-              // Version
-              it = propertiesList.find("Version");
-              if (it != propertiesList.end()) {
-                const std::string *version =
-                    mapbox::getPtr<const std::string>(it->second);
-                if (version == nullptr) {
-                  res.jsonValue = redfish::messages::internalError();
-                  res.jsonValue = messages::internalError();
-                  return;
-                }
-                res.jsonValue["Version"] =
-                    *(mapbox::getPtr<const std::string>(it->second));
-              } else {
-                BMCWEB_LOG_DEBUG << "Can't find version info!";
-              }
-
-              res.end();
-            },
-            connectionName, obj.first, "org.freedesktop.DBus.Properties",
-            "GetAll", "xyz.openbmc_project.Software.Version");
-      }
-      if (!fw_id_found) {
-        res.result(boost::beast::http::status::not_found);
-        res.end();
-        return;
-      }
-    });
+                    if (version != nullptr) {
+                      BMCWEB_LOG_DEBUG << "Can't find property \"Version\"!";
+                      asyncResp->res.result(
+                          boost::beast::http::status::internal_server_error);
+                      return;
+                    }
+                    asyncResp->res.jsonValue["Version"] = *version;
+                    asyncResp->res.jsonValue["Id"] = *sw_id;
+                  }
+                },
+                obj.second[0].first, obj.first,
+                "org.freedesktop.DBus.Properties", "GetAll",
+                "xyz.openbmc_project.Software.Version");
+          }
+        },
+        "xyz.openbmc_project.ObjectMapper",
+        "/xyz/openbmc_project/object_mapper",
+        "xyz.openbmc_project.ObjectMapper", "GetSubTree",
+        "/xyz/openbmc_project/software", int32_t(1),
+        std::array<const char *, 1>{"xyz.openbmc_project.Software.Version"});
   }
-
-  OnDemandSoftwareInventoryProvider softwareInventoryProvider;
 };
 
 }  // namespace redfish