add handler logic to handle SysEntityName

Add handler logic to handler for SysEntityName such that it splits the
true IPMI processing from the business logic.

Tested: Only ran unit-tests (added new ones).
Change-Id: I6d672a80f85843ff98c2c7e5daf4689932ff96f9
Signed-off-by: Patrick Venture <venture@google.com>
diff --git a/entity_name.cpp b/entity_name.cpp
index c070c84..ac04275 100644
--- a/entity_name.cpp
+++ b/entity_name.cpp
@@ -16,25 +16,19 @@
 
 #include "entity_name.hpp"
 
+#include "errors.hpp"
+#include "handler.hpp"
 #include "main.hpp"
 
 #include <cstdint>
 #include <cstring>
-#include <filesystem>
-#include <fstream>
-#include <map>
-#include <nlohmann/json.hpp>
-#include <phosphor-logging/elog-errors.hpp>
-#include <phosphor-logging/log.hpp>
 #include <string>
 #include <vector>
-#include <xyz/openbmc_project/Common/error.hpp>
 
 namespace google
 {
 namespace ipmi
 {
-namespace fs = std::filesystem;
 
 namespace
 {
@@ -44,65 +38,6 @@
 #define MAX_IPMI_BUFFER 64
 #endif
 
-using Json = nlohmann::json;
-
-using namespace phosphor::logging;
-using InternalFailure =
-    sdbusplus::xyz::openbmc_project::Common::Error::InternalFailure;
-
-static constexpr auto configFile =
-    "/usr/share/ipmi-entity-association/entity_association_map.json";
-
-static const std::map<uint8_t, std::string> entityIdToName{
-    {0x03, "cpu"},
-    {0x04, "storage_device"},
-    {0x06, "system_management_module"},
-    {0x08, "memory_module"},
-    {0x0B, "add_in_card"},
-    {0x17, "system_chassis"},
-    {0x20, "memory_device"}};
-
-Json parse_config()
-{
-    std::ifstream jsonFile(configFile);
-    if (!jsonFile.is_open())
-    {
-        log<level::ERR>("Entity association JSON file not found");
-        elog<InternalFailure>();
-    }
-
-    auto data = Json::parse(jsonFile, nullptr, false);
-    if (data.is_discarded())
-    {
-        log<level::ERR>("Entity association JSON parser failure");
-        elog<InternalFailure>();
-    }
-
-    return data;
-}
-
-std::string read(const std::string& type, uint8_t instance, const Json& config)
-{
-    static const std::vector<Json> empty{};
-    std::vector<Json> readings = config.value(type, empty);
-    std::string name = "";
-    for (const auto& j : readings)
-    {
-        uint8_t instanceNum = j.value("instance", 0);
-        // Not the instance we're interested in
-        if (instanceNum != instance)
-        {
-            continue;
-        }
-
-        // Found the instance we're interested in
-        name = j.value("name", "");
-
-        break;
-    }
-    return name;
-}
-
 } // namespace
 
 struct GetEntityNameRequest
@@ -120,7 +55,7 @@
 } __attribute__((packed));
 
 ipmi_ret_t GetEntityName(const uint8_t* reqBuf, uint8_t* replyBuf,
-                         size_t* dataLen)
+                         size_t* dataLen, HandlerInterface* handler)
 {
     struct GetEntityNameRequest request;
 
@@ -132,37 +67,15 @@
     }
 
     std::memcpy(&request, &reqBuf[0], sizeof(request));
-
-    // Check if we support this Entity ID.
-    auto it = entityIdToName.find(request.entity_id);
-    if (it == entityIdToName.end())
-    {
-        log<level::ERR>("Unknown Entity ID",
-                        entry("ENTITY_ID=%d", request.entity_id));
-        return IPMI_CC_INVALID_FIELD_REQUEST;
-    }
-
-    static Json config{};
-    static bool parsed = false;
-
     std::string entityName;
     try
     {
-        // Parse the JSON config file.
-        if (!parsed)
-        {
-            config = parse_config();
-            parsed = true;
-        }
-
-        // Find the "entity id:entity instance" mapping to entity name.
-        entityName = read(it->second, request.entity_instance, config);
-        if (entityName.empty())
-            return IPMI_CC_INVALID_FIELD_REQUEST;
+        entityName =
+            handler->getEntityName(request.entity_id, request.entity_instance);
     }
-    catch (InternalFailure& e)
+    catch (const IpmiException& e)
     {
-        return IPMI_CC_UNSPECIFIED_ERROR;
+        return e.getIpmiError();
     }
 
     int length = sizeof(struct GetEntityNameReply) + entityName.length();
diff --git a/entity_name.hpp b/entity_name.hpp
index a1bc9f4..cf35e0d 100644
--- a/entity_name.hpp
+++ b/entity_name.hpp
@@ -1,5 +1,7 @@
 #pragma once
 
+#include "handler.hpp"
+
 #include <ipmid/api.h>
 
 namespace google
@@ -10,7 +12,8 @@
 // Handle the "entity id:entity instance" to entity name mapping command.
 // Sys can query the entity name for a particular "entity id:entity instance".
 ipmi_ret_t GetEntityName(const uint8_t* reqBuf, uint8_t* replyBuf,
-                         size_t* dataLen);
+                         size_t* dataLen,
+                         HandlerInterface* handler = &handlerImpl);
 
 } // namespace ipmi
 } // namespace google
diff --git a/handler.cpp b/handler.cpp
index 8a076f2..2018ef6 100644
--- a/handler.cpp
+++ b/handler.cpp
@@ -24,11 +24,15 @@
 #include <cstdio>
 #include <filesystem>
 #include <fstream>
+#include <map>
+#include <nlohmann/json.hpp>
+#include <phosphor-logging/elog-errors.hpp>
 #include <phosphor-logging/log.hpp>
 #include <sdbusplus/bus.hpp>
 #include <sstream>
 #include <string>
 #include <tuple>
+#include <xyz/openbmc_project/Common/error.hpp>
 
 // The phosphor-host-ipmi daemon requires a configuration that maps
 // the if_name to the IPMI LAN channel.  However, that doesn't strictly
@@ -51,6 +55,10 @@
 namespace ipmi
 {
 namespace fs = std::filesystem;
+using Json = nlohmann::json;
+using namespace phosphor::logging;
+using InternalFailure =
+    sdbusplus::xyz::openbmc_project::Common::Error::InternalFailure;
 
 std::tuple<std::uint8_t, std::string> Handler::getEthDetails() const
 {
@@ -190,6 +198,93 @@
     }
 }
 
+static Json config{};
+static bool parsed = false;
+static const std::map<uint8_t, std::string> entityIdToName{
+    {0x03, "cpu"},
+    {0x04, "storage_device"},
+    {0x06, "system_management_module"},
+    {0x08, "memory_module"},
+    {0x0B, "add_in_card"},
+    {0x17, "system_chassis"},
+    {0x20, "memory_device"}};
+static constexpr auto configFile =
+    "/usr/share/ipmi-entity-association/entity_association_map.json";
+
+Json parse_config()
+{
+    std::ifstream jsonFile(configFile);
+    if (!jsonFile.is_open())
+    {
+        log<level::ERR>("Entity association JSON file not found");
+        elog<InternalFailure>();
+    }
+
+    auto data = Json::parse(jsonFile, nullptr, false);
+    if (data.is_discarded())
+    {
+        log<level::ERR>("Entity association JSON parser failure");
+        elog<InternalFailure>();
+    }
+
+    return data;
+}
+
+std::string read(const std::string& type, uint8_t instance, const Json& config)
+{
+    static const std::vector<Json> empty{};
+    std::vector<Json> readings = config.value(type, empty);
+    std::string name = "";
+    for (const auto& j : readings)
+    {
+        uint8_t instanceNum = j.value("instance", 0);
+        // Not the instance we're interested in
+        if (instanceNum != instance)
+        {
+            continue;
+        }
+
+        // Found the instance we're interested in
+        name = j.value("name", "");
+
+        break;
+    }
+    return name;
+}
+
+std::string Handler::getEntityName(std::uint8_t id, std::uint8_t instance)
+{
+    // Check if we support this Entity ID.
+    auto it = entityIdToName.find(id);
+    if (it == entityIdToName.end())
+    {
+        log<level::ERR>("Unknown Entity ID", entry("ENTITY_ID=%d", id));
+        throw IpmiException(IPMI_CC_INVALID_FIELD_REQUEST);
+    }
+
+    std::string entityName;
+    try
+    {
+        // Parse the JSON config file.
+        if (!parsed)
+        {
+            config = parse_config();
+            parsed = true;
+        }
+
+        // Find the "entity id:entity instance" mapping to entity name.
+        entityName = read(it->second, instance, config);
+        if (entityName.empty())
+            throw IpmiException(IPMI_CC_INVALID_FIELD_REQUEST);
+    }
+    catch (InternalFailure& e)
+    {
+        throw IpmiException(IPMI_CC_UNSPECIFIED_ERROR);
+    }
+
+    return entityName;
+}
+
 Handler handlerImpl;
 
 } // namespace ipmi
diff --git a/handler.hpp b/handler.hpp
index a068fce..be5f4d0 100644
--- a/handler.hpp
+++ b/handler.hpp
@@ -49,6 +49,20 @@
      * @throw IpmiException on failure.
      */
     virtual void psuResetDelay(std::uint32_t delay) const = 0;
+
+    /**
+     * Return the entity name.
+     * On the first call to this method it'll build the list of entities.
+     * @todo Consider moving the list building to construction time (and ignore
+     * failures).
+     *
+     * @param[in] id - the entity id value
+     * @param[in] instance - the entity instance
+     * @return the entity's name
+     * @throw IpmiException on failure.
+     */
+    virtual std::string getEntityName(std::uint8_t id,
+                                      std::uint8_t instance) = 0;
 };
 
 class Handler : public HandlerInterface
@@ -61,6 +75,7 @@
     std::int64_t getRxPackets(const std::string& name) const override;
     VersionTuple getCpldVersion(unsigned int id) const override;
     void psuResetDelay(std::uint32_t delay) const override;
+    std::string getEntityName(std::uint8_t id, std::uint8_t instance) override;
 };
 
 extern Handler handlerImpl;
diff --git a/test/Makefile.am b/test/Makefile.am
index 04b6836..0243d44 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -34,3 +34,7 @@
 check_PROGRAMS += psu_unittest
 psu_unittest_SOURCES = psu_unittest.cpp
 psu_unittest_LDADD = $(top_builddir)/libsyscmds_common.la
+
+check_PROGRAMS += entity_unittest
+entity_unittest_SOURCES = entity_unittest.cpp
+entity_unittest_LDADD = $(top_builddir)/libsyscmds_common.la
diff --git a/test/entity_unittest.cpp b/test/entity_unittest.cpp
new file mode 100644
index 0000000..74495c6
--- /dev/null
+++ b/test/entity_unittest.cpp
@@ -0,0 +1,53 @@
+#include "entity_name.hpp"
+#include "handler_mock.hpp"
+#include "main.hpp"
+
+#include <cstdint>
+#include <cstring>
+#include <string>
+#include <vector>
+
+#include <gtest/gtest.h>
+
+#define MAX_IPMI_BUFFER 64
+
+using ::testing::Return;
+
+namespace google
+{
+namespace ipmi
+{
+
+TEST(EntityNameCommandTest, InvalidCommandLength)
+{
+    // GetEntityNameRequest is three bytes, let's send 2.
+    std::vector<std::uint8_t> request = {SysOEMCommands::SysEntityName, 0x01};
+    size_t dataLen = request.size();
+    std::uint8_t reply[MAX_IPMI_BUFFER];
+
+    HandlerMock hMock;
+    EXPECT_EQ(IPMI_CC_REQ_DATA_LEN_INVALID,
+              GetEntityName(request.data(), reply, &dataLen, &hMock));
+}
+
+TEST(EntityNameCommandTest, ValidRequest)
+{
+    std::uint8_t entityId = 3;
+    std::uint8_t entityInstance = 5;
+    std::vector<std::uint8_t> request = {SysOEMCommands::SysEntityName,
+                                         entityId, entityInstance};
+    size_t dataLen = request.size();
+    std::uint8_t reply[MAX_IPMI_BUFFER];
+    std::string entityName = "asdf";
+
+    HandlerMock hMock;
+    EXPECT_CALL(hMock, getEntityName(entityId, entityInstance))
+        .WillOnce(Return(entityName));
+    EXPECT_EQ(IPMI_CC_OK,
+              GetEntityName(request.data(), reply, &dataLen, &hMock));
+    EXPECT_EQ(reply[1], entityName.length());
+    EXPECT_EQ(0, std::memcmp(&reply[2], entityName.c_str(), reply[1]));
+}
+
+} // namespace ipmi
+} // namespace google
diff --git a/test/handler_mock.hpp b/test/handler_mock.hpp
index 6eb929a..6394f68 100644
--- a/test/handler_mock.hpp
+++ b/test/handler_mock.hpp
@@ -21,6 +21,7 @@
                        std::tuple<std::uint8_t, std::uint8_t, std::uint8_t,
                                   std::uint8_t>(unsigned int));
     MOCK_CONST_METHOD1(psuResetDelay, void(std::uint32_t));
+    MOCK_METHOD2(getEntityName, std::string(std::uint8_t, std::uint8_t));
 };
 
 } // namespace ipmi