entity-manager: fixes for logDeviceAdded/Removed
Follow-up patch to fix issues found in comments of [1].
- Use explicit type rather than 'auto' for local variable
- Setup a return variable to avoid duplicate initial value "Unknown"
- Fix string value read to avoid uncaught exception
- Remove unused boost include
- Add unit tests
Tested: Unit tests pass.
References:
[1] https://gerrit.openbmc.org/c/openbmc/entity-manager/+/84340
Change-Id: I3d5540860e8ef8e590bc2685ce559c53dc8452b5
Signed-off-by: Alexander Hansen <alexander.hansen@9elements.com>
diff --git a/src/entity_manager/log_device_inventory.cpp b/src/entity_manager/log_device_inventory.cpp
index 93cf156..d47a54e 100644
--- a/src/entity_manager/log_device_inventory.cpp
+++ b/src/entity_manager/log_device_inventory.cpp
@@ -1,3 +1,5 @@
+#include "log_device_inventory.hpp"
+
#include "../utils.hpp"
#include <systemd/sd-journal.h>
@@ -8,58 +10,45 @@
#include <flat_map>
#include <string>
-struct InvAddRemoveInfo
+static void setStringIfFound(std::string& value, const std::string& key,
+ const nlohmann::json& record, bool dump = false)
{
- std::string model = "Unknown";
- std::string type = "Unknown";
- std::string sn = "Unknown";
- std::string name = "Unknown";
-};
+ const nlohmann::json::const_iterator find = record.find(key);
-static InvAddRemoveInfo queryInvInfo(const nlohmann::json& record)
-{
- auto findType = record.find("Type");
- auto findAsset = record.find(sdbusplus::common::xyz::openbmc_project::
- inventory::decorator::Asset::interface);
-
- std::string model = "Unknown";
- std::string type = "Unknown";
- std::string sn = "Unknown";
- std::string name = "Unknown";
-
- if (findType != record.end())
+ if (find == record.end())
{
- type = findType->get<std::string>();
+ return;
}
+
+ const std::string* foundValue = find->get_ptr<const std::string*>();
+ if (foundValue != nullptr)
+ {
+ value = *foundValue;
+ }
+ else if (dump)
+ {
+ value = find->dump();
+ }
+}
+
+InvAddRemoveInfo queryInvInfo(const nlohmann::json& record)
+{
+ InvAddRemoveInfo ret;
+
+ setStringIfFound(ret.type, "Type", record);
+ setStringIfFound(ret.name, "Name", record);
+
+ const nlohmann::json::const_iterator findAsset = record.find(
+ sdbusplus::common::xyz::openbmc_project::inventory::decorator::Asset::
+ interface);
+
if (findAsset != record.end())
{
- auto findModel = findAsset->find("Model");
- auto findSn = findAsset->find("SerialNumber");
- if (findModel != findAsset->end())
- {
- model = findModel->get<std::string>();
- }
- if (findSn != findAsset->end())
- {
- const std::string* getSn = findSn->get_ptr<const std::string*>();
- if (getSn != nullptr)
- {
- sn = *getSn;
- }
- else
- {
- sn = findSn->dump();
- }
- }
+ setStringIfFound(ret.model, "Model", *findAsset);
+ setStringIfFound(ret.sn, "SerialNumber", *findAsset, true);
}
- auto findName = record.find("Name");
- if (findName != record.end())
- {
- name = findName->get<std::string>();
- }
-
- return {model, type, sn, name};
+ return ret;
}
void logDeviceAdded(const nlohmann::json& record)
@@ -73,7 +62,7 @@
return;
}
- const auto info = queryInvInfo(record);
+ const InvAddRemoveInfo info = queryInvInfo(record);
sd_journal_send(
"MESSAGE=Inventory Added: %s", info.name.c_str(), "PRIORITY=%i",
@@ -89,7 +78,7 @@
return;
}
- const auto info = queryInvInfo(record);
+ const InvAddRemoveInfo info = queryInvInfo(record);
sd_journal_send(
"MESSAGE=Inventory Removed: %s", info.name.c_str(), "PRIORITY=%i",
diff --git a/src/entity_manager/log_device_inventory.hpp b/src/entity_manager/log_device_inventory.hpp
index 9ebf040..4232f14 100644
--- a/src/entity_manager/log_device_inventory.hpp
+++ b/src/entity_manager/log_device_inventory.hpp
@@ -5,6 +5,16 @@
#include <nlohmann/json.hpp>
+struct InvAddRemoveInfo
+{
+ std::string model = "Unknown";
+ std::string type = "Unknown";
+ std::string sn = "Unknown";
+ std::string name = "Unknown";
+};
+
void logDeviceAdded(const nlohmann::json& record);
void logDeviceRemoved(const nlohmann::json& record);
+
+InvAddRemoveInfo queryInvInfo(const nlohmann::json& record);
diff --git a/test/entity_manager/log_device_inventory.cpp b/test/entity_manager/log_device_inventory.cpp
new file mode 100644
index 0000000..cbf7bd4
--- /dev/null
+++ b/test/entity_manager/log_device_inventory.cpp
@@ -0,0 +1,49 @@
+
+#include "entity_manager/log_device_inventory.hpp"
+
+#include <gtest/gtest.h>
+
+TEST(LogDevicInventory, QueryInvInfoSuccess)
+{
+ nlohmann::json record = nlohmann::json::parse(R"(
+{
+ "Name": "Supermicro PWS 920P SQ 0",
+ "Type": "PowerSupply",
+ "xyz.openbmc_project.Inventory.Decorator.Asset": {
+ "Manufacturer": "Supermicro",
+ "Model": "PWS 920P SQ",
+ "PartNumber": "328923",
+ "SerialNumber": "43829239"
+ }
+}
+ )");
+
+ InvAddRemoveInfo info = queryInvInfo(record);
+
+ EXPECT_EQ(info.name, "Supermicro PWS 920P SQ 0");
+ EXPECT_EQ(info.type, "PowerSupply");
+ EXPECT_EQ(info.sn, "43829239");
+ EXPECT_EQ(info.model, "PWS 920P SQ");
+}
+
+TEST(LogDevicInventory, QueryInvInfoNoModelFound)
+{
+ nlohmann::json record = nlohmann::json::parse(R"(
+{
+ "Name": "Supermicro PWS 920P SQ 0",
+ "Type": "PowerSupply",
+ "xyz.openbmc_project.Inventory.Decorator.Asset": {
+ "Manufacturer": "Supermicro",
+ "PartNumber": "328923",
+ "SerialNumber": "43829239"
+ }
+}
+ )");
+
+ InvAddRemoveInfo info = queryInvInfo(record);
+
+ EXPECT_EQ(info.name, "Supermicro PWS 920P SQ 0");
+ EXPECT_EQ(info.type, "PowerSupply");
+ EXPECT_EQ(info.sn, "43829239");
+ EXPECT_EQ(info.model, "Unknown");
+}
diff --git a/test/entity_manager/meson.build b/test/entity_manager/meson.build
index d0ac54c..43d4689 100644
--- a/test/entity_manager/meson.build
+++ b/test/entity_manager/meson.build
@@ -16,3 +16,22 @@
include_directories: test_include_dir,
),
)
+
+test(
+ 'test_log_device_inventory',
+ executable(
+ 'test_log_device_inventory',
+ 'log_device_inventory.cpp',
+ cpp_args: test_boost_args,
+ dependencies: [
+ boost,
+ gtest,
+ nlohmann_json_dep,
+ phosphor_logging_dep,
+ sdbusplus,
+ valijson,
+ ],
+ link_with: entity_manager_lib,
+ include_directories: test_include_dir,
+ ),
+)