entityName: move pieces into handler object
Move different items used by the handler into the handler and enable
unit-testing by parameterizing different aspects of the code.
Tested: Only ran unit-tests (added new ones).
Change-Id: Ia3b4b5792c0ac1ae5bc6513eadfc9ee35f7a369f
Signed-off-by: Patrick Venture <venture@google.com>
diff --git a/Makefile.am b/Makefile.am
index b2645ef..59adc1d 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -47,7 +47,8 @@
psu.cpp \
pcie_i2c.cpp \
entity_name.cpp \
- handler.cpp
+ handler.cpp \
+ util.cpp
libsyscmds_common_la_CXXFLAGS = \
$(SDBUSPLUS_CFLAGS) \
$(PHOSPHOR_LOGGING_CFLAGS) \
diff --git a/handler.cpp b/handler.cpp
index 93b3c21..041f77b 100644
--- a/handler.cpp
+++ b/handler.cpp
@@ -17,6 +17,7 @@
#include "handler.hpp"
#include "errors.hpp"
+#include "util.hpp"
#include <ipmid/api.h>
@@ -196,43 +197,48 @@
}
}
-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::string Handler::getEntityName(std::uint8_t id, std::uint8_t instance)
{
- std::ifstream jsonFile(configFile);
- if (!jsonFile.is_open())
+ // Check if we support this Entity ID.
+ auto it = _entityIdToName.find(id);
+ if (it == _entityIdToName.end())
{
- log<level::ERR>("Entity association JSON file not found");
- elog<InternalFailure>();
+ log<level::ERR>("Unknown Entity ID", entry("ENTITY_ID=%d", id));
+ throw IpmiException(IPMI_CC_INVALID_FIELD_REQUEST);
}
- auto data = Json::parse(jsonFile, nullptr, false);
- if (data.is_discarded())
+ std::string entityName;
+ try
{
- log<level::ERR>("Entity association JSON parser failure");
- elog<InternalFailure>();
+ // Parse the JSON config file.
+ if (!_entityConfigParsed)
+ {
+ _entityConfig = parseConfig(_configFile);
+ _entityConfigParsed = true;
+ }
+
+ // Find the "entity id:entity instance" mapping to entity name.
+ entityName = readNameFromConfig(it->second, instance, _entityConfig);
+ if (entityName.empty())
+ {
+ throw IpmiException(IPMI_CC_INVALID_FIELD_REQUEST);
+ }
+ }
+ catch (InternalFailure& e)
+ {
+ throw IpmiException(IPMI_CC_UNSPECIFIED_ERROR);
}
- return data;
+ return entityName;
}
-std::string read(const std::string& type, uint8_t instance, const Json& config)
+std::string readNameFromConfig(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);
@@ -247,41 +253,12 @@
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;
-}
+const std::string defaultConfigFile =
+ "/usr/share/ipmi-entity-association/entity_association_map.json";
Handler handlerImpl;
diff --git a/handler.hpp b/handler.hpp
index be5f4d0..ec77657 100644
--- a/handler.hpp
+++ b/handler.hpp
@@ -1,6 +1,8 @@
#pragma once
#include <cstdint>
+#include <map>
+#include <nlohmann/json.hpp>
#include <string>
#include <tuple>
@@ -12,6 +14,8 @@
using VersionTuple =
std::tuple<std::uint8_t, std::uint8_t, std::uint8_t, std::uint8_t>;
+extern const std::string defaultConfigFile;
+
class HandlerInterface
{
public:
@@ -68,7 +72,8 @@
class Handler : public HandlerInterface
{
public:
- Handler() = default;
+ explicit Handler(const std::string& entityConfigPath = defaultConfigFile) :
+ _configFile(entityConfigPath){};
~Handler() = default;
std::tuple<std::uint8_t, std::string> getEthDetails() const override;
@@ -76,8 +81,35 @@
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;
+
+ private:
+ std::string _configFile;
+
+ bool _entityConfigParsed = false;
+
+ 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"}};
+
+ nlohmann::json _entityConfig{};
};
+/**
+ * Given a type, entity instance, and a configuration, return the name.
+ *
+ * @param[in] type - the entity type
+ * @param[in] instance - the entity instance
+ * @param[in] config - the json object holding the entity mapping
+ * @return the name of the entity from the map
+ */
+std::string readNameFromConfig(const std::string& type, uint8_t instance,
+ const nlohmann::json& config);
+
extern Handler handlerImpl;
} // namespace ipmi
diff --git a/test/handler_unittest.cpp b/test/handler_unittest.cpp
index c1f4093..a4f9281 100644
--- a/test/handler_unittest.cpp
+++ b/test/handler_unittest.cpp
@@ -1,6 +1,8 @@
#include "errors.hpp"
#include "handler.hpp"
+#include <fstream>
+#include <nlohmann/json.hpp>
#include <string>
#include <tuple>
@@ -27,6 +29,70 @@
EXPECT_THROW(h.getRxPackets("eth0/../../"), IpmiException);
}
+TEST(HandlerTest, readNameFromConfigInstanceVariety)
+{
+ // Make sure it handles the failures and successes as we expect.
+ struct testCase
+ {
+ std::string type;
+ std::uint8_t instance;
+ std::string expectedName;
+ };
+
+ std::vector<testCase> tests = {
+ {"cpu", 5, ""},
+ {"cpu", 3, "CPU2"},
+ };
+
+ auto j2 = R"(
+ {
+ "cpu": [
+ {"instance": 1, "name": "CPU0"},
+ {"instance": 2, "name": "CPU1"},
+ {"instance": 3, "name": "CPU2"},
+ {"instance": 4, "name": "CPU3"}
+ ]
+ }
+ )"_json;
+
+ for (const auto& test : tests)
+ {
+ EXPECT_STREQ(test.expectedName.c_str(),
+ readNameFromConfig(test.type, test.instance, j2).c_str());
+ }
+}
+
+// TODO: If we can test with phosphor-logging in the future, there are more
+// failure cases.
+
+TEST(HandlerTest, getEntityNameWithNameNotFoundExcepts)
+{
+ const char* testFilename = "test.json";
+ std::string contents = R"({"cpu": [{"instance": 1, "name": "CPU0"}]})";
+ std::ofstream outputJson(testFilename);
+ outputJson << contents;
+ outputJson.flush();
+ outputJson.close();
+
+ Handler h(testFilename);
+ EXPECT_THROW(h.getEntityName(0x03, 2), IpmiException);
+ (void)std::remove(testFilename);
+}
+
+TEST(HandlerTest, getEntityNameWithNameFoundReturnsIt)
+{
+ const char* testFilename = "test.json";
+ std::string contents = R"({"cpu": [{"instance": 1, "name": "CPU0"}]})";
+ std::ofstream outputJson(testFilename);
+ outputJson << contents;
+ outputJson.flush();
+ outputJson.close();
+
+ Handler h(testFilename);
+ EXPECT_STREQ("CPU0", h.getEntityName(0x03, 1).c_str());
+ (void)std::remove(testFilename);
+}
+
// TODO: Add checks for other functions of handler.
} // namespace ipmi
diff --git a/util.cpp b/util.cpp
new file mode 100644
index 0000000..02dab83
--- /dev/null
+++ b/util.cpp
@@ -0,0 +1,38 @@
+#include "util.hpp"
+
+#include <fstream>
+#include <nlohmann/json.hpp>
+#include <phosphor-logging/elog-errors.hpp>
+#include <string>
+#include <xyz/openbmc_project/Common/error.hpp>
+
+namespace google
+{
+namespace ipmi
+{
+
+using namespace phosphor::logging;
+using InternalFailure =
+ sdbusplus::xyz::openbmc_project::Common::Error::InternalFailure;
+
+nlohmann::json parseConfig(const std::string& file)
+{
+ std::ifstream jsonFile(file);
+ if (!jsonFile.is_open())
+ {
+ log<level::ERR>("Entity association JSON file not found");
+ elog<InternalFailure>();
+ }
+
+ auto data = nlohmann::json::parse(jsonFile, nullptr, false);
+ if (data.is_discarded())
+ {
+ log<level::ERR>("Entity association JSON parser failure");
+ elog<InternalFailure>();
+ }
+
+ return data;
+}
+
+} // namespace ipmi
+} // namespace google
diff --git a/util.hpp b/util.hpp
new file mode 100644
index 0000000..d98a6cc
--- /dev/null
+++ b/util.hpp
@@ -0,0 +1,21 @@
+#pragma once
+
+#include <nlohmann/json.hpp>
+#include <string>
+
+namespace google
+{
+namespace ipmi
+{
+
+/**
+ * Parse a file and return the json object.
+ *
+ * @param[in] file - the path to the file to parse.
+ * @return the json object if valid.
+ * @throw elog<InternalFailure> on failures.
+ */
+nlohmann::json parseConfig(const std::string& file);
+
+} // namespace ipmi
+} // namespace google