Refactor Version::getValue()
Rename the function to getValues() and change the parameter and return
value types, and add a test case for it.
It will be used in future commits.
Tested: Verify the unit test case passes.
Signed-off-by: Lei YU <mine260309@gmail.com>
Change-Id: I57ccf857737ef13f4e2f27c5f2fb7400a2170e91
diff --git a/src/item_updater.cpp b/src/item_updater.cpp
index e100381..d9a502f 100644
--- a/src/item_updater.cpp
+++ b/src/item_updater.cpp
@@ -9,6 +9,11 @@
#include <phosphor-logging/log.hpp>
#include <xyz/openbmc_project/Common/error.hpp>
+namespace
+{
+constexpr auto EXTENDED_VERSION = "extended_version";
+}
+
namespace phosphor
{
namespace software
@@ -98,12 +103,14 @@
fs::path manifestPath(filePath);
manifestPath /= MANIFEST_FILE;
- std::string extendedVersion =
- (Version::getValue(
- manifestPath.string(),
- std::map<std::string, std::string>{{"extended_version", ""}}))
- .begin()
- ->second;
+ std::string extendedVersion;
+ auto values =
+ Version::getValues(manifestPath.string(), {EXTENDED_VERSION});
+ const auto it = values.find(EXTENDED_VERSION);
+ if (it != values.end())
+ {
+ extendedVersion = it->second;
+ }
auto activation = createActivationObject(
path, versionId, extendedVersion, activationState, associations);
diff --git a/src/version.cpp b/src/version.cpp
index 7940d2e..0ea6cc9 100644
--- a/src/version.cpp
+++ b/src/version.cpp
@@ -23,8 +23,8 @@
using Argument = xyz::openbmc_project::Common::InvalidArgument;
std::map<std::string, std::string>
- Version::getValue(const std::string& filePath,
- std::map<std::string, std::string> keys)
+ Version::getValues(const std::string& filePath,
+ const std::vector<std::string>& keys)
{
if (filePath.empty())
{
@@ -33,39 +33,24 @@
Argument::ARGUMENT_VALUE(filePath.c_str()));
}
- std::ifstream efile;
+ std::ifstream efile(filePath);
std::string line;
- efile.exceptions(std::ifstream::failbit | std::ifstream::badbit |
- std::ifstream::eofbit);
+ std::map<std::string, std::string> ret;
- try
+ while (getline(efile, line))
{
- efile.open(filePath);
- while (getline(efile, line))
+ for (const auto& key : keys)
{
- for (auto& key : keys)
+ auto value = key + "=";
+ auto keySize = value.length();
+ if (line.compare(0, keySize, value) == 0)
{
- auto value = key.first + "=";
- auto keySize = value.length();
- if (line.compare(0, keySize, value) == 0)
- {
- key.second = line.substr(keySize);
- break;
- }
+ ret.emplace(key, line.substr(keySize));
+ break;
}
}
- efile.close();
}
- catch (const std::exception& e)
- {
- if (!efile.eof())
- {
- log<level::ERR>("Error in reading file");
- }
- efile.close();
- }
-
- return keys;
+ return ret;
}
void Delete::delete_()
diff --git a/src/version.hpp b/src/version.hpp
index a67fc82..c3c2c54 100644
--- a/src/version.hpp
+++ b/src/version.hpp
@@ -121,13 +121,13 @@
*
* @param[in] filePath - The path to the file which contains the value
* of keys.
- * @param[in] keys - A map of keys with empty values.
+ * @param[in] keys - A vector of keys.
*
* @return The map of keys with filled values.
**/
static std::map<std::string, std::string>
- getValue(const std::string& filePath,
- std::map<std::string, std::string> keys);
+ getValues(const std::string& filePath,
+ const std::vector<std::string>& keys);
/** @brief The temUpdater's erase callback. */
eraseFunc eraseCallback;
diff --git a/test/meson.build b/test/meson.build
index 0b7b217..2de2b21 100644
--- a/test/meson.build
+++ b/test/meson.build
@@ -44,6 +44,7 @@
'../src/version.cpp',
'test_item_updater.cpp',
'test_activation.cpp',
+ 'test_version.cpp',
include_directories: [psu_inc, test_inc],
link_args: dynamic_linker,
build_rpath: oe_sdk.enabled() ? rpath : '',
diff --git a/test/test_version.cpp b/test/test_version.cpp
new file mode 100644
index 0000000..cba599f
--- /dev/null
+++ b/test/test_version.cpp
@@ -0,0 +1,67 @@
+#include "version.hpp"
+
+#include <filesystem>
+#include <fstream>
+
+#include <gtest/gtest.h>
+
+using phosphor::software::updater::Version;
+
+namespace fs = std::filesystem;
+
+namespace
+{
+constexpr auto validManifest = R"(
+purpose=xyz.openbmc_project.Software.Version.VersionPurpose.PSU
+version=psu-dummy-test.v0.1
+extended_version=model=dummy_model,manufacture=dummy_manufacture)";
+}
+
+class TestVersion : public ::testing::Test
+{
+ public:
+ TestVersion()
+ {
+ auto tmpPath = fs::temp_directory_path();
+ tmpDir = (tmpPath / "test_XXXXXX");
+ if (!mkdtemp(tmpDir.data()))
+ {
+ throw "Failed to create temp dir";
+ }
+ }
+ ~TestVersion()
+ {
+ fs::remove_all(tmpDir);
+ }
+
+ void writeFile(const fs::path& file, const char* data)
+ {
+ std::ofstream f{file};
+ f << data;
+ f.close();
+ }
+ std::string tmpDir;
+};
+
+TEST_F(TestVersion, getValuesFileNotExist)
+{
+ auto ret = Version::getValues("NotExist.file", {""});
+ EXPECT_TRUE(ret.empty());
+}
+
+TEST_F(TestVersion, getValuesOK)
+{
+ auto manifestFilePath = fs::path(tmpDir) / "MANIFEST";
+ writeFile(manifestFilePath, validManifest);
+ auto ret = Version::getValues(manifestFilePath.string(),
+ {"purpose", "version", "extended_version"});
+ EXPECT_EQ(3u, ret.size());
+ auto purpose = ret["purpose"];
+ auto version = ret["version"];
+ auto extVersion = ret["extended_version"];
+
+ EXPECT_EQ("xyz.openbmc_project.Software.Version.VersionPurpose.PSU",
+ purpose);
+ EXPECT_EQ("psu-dummy-test.v0.1", version);
+ EXPECT_EQ("model=dummy_model,manufacture=dummy_manufacture", extVersion);
+}