version-handler: Refactor handler construction
Instead of requiring callers to build maps and info blobs, take the
minimal required amount of information about the blob configuration and
build required datastructures internally.
This reduces the amount of code, and nearly eliminates all of the
untested code in main.cpp.
Change-Id: Iaa398eb404814e9263e6707b71b38a9831d96697
Signed-off-by: William A. Kennington III <wak@google.com>
diff --git a/bmc/version-handler/Makefile.am b/bmc/version-handler/Makefile.am
index 7230193..dec3ff1 100644
--- a/bmc/version-handler/Makefile.am
+++ b/bmc/version-handler/Makefile.am
@@ -11,13 +11,9 @@
libversionblob_common_la_CXXFLAGS = \
-I$(top_srcdir) \
-I$(top_srcdir)/bmc \
- $(SDBUSPLUS_CFLAGS) \
- $(PHOSPHOR_LOGGING_CFLAGS) \
$(CODE_COVERAGE_CXXFLAGS) \
-flto
libversionblob_common_la_LDFLAGS = \
- $(SDBUSPLUS_LIBS) \
- $(PHOSPHOR_LOGGING_LIBS) \
$(CODE_COVERAGE_LIBS) \
-lstdc++fs
libversionblob_common_la_LIBADD = $(top_builddir)/libfirmware_common.la
@@ -29,16 +25,13 @@
main.cpp
libversionblob_la_LIBADD = libversionblob_common.la
libversionblob_la_LDFLAGS = \
- $(SDBUSPLUS_LIBS) \
- $(PHOSPHOR_LOGGING_LIBS) \
$(CODE_COVERAGE_LIBS) \
-lstdc++fs \
-version-info 0:0:0 -shared
libversionblob_la_CXXFLAGS = \
-I$(top_srcdir) \
-I$(top_srcdir)/bmc \
- $(SDBUSPLUS_CFLAGS) \
- $(PHOSPHOR_LOGGING_CFLAGS) \
$(CODE_COVERAGE_CXXFLAGS) \
-flto
+
SUBDIRS = . test
diff --git a/bmc/version-handler/main.cpp b/bmc/version-handler/main.cpp
index 8946fb3..51df8e1 100644
--- a/bmc/version-handler/main.cpp
+++ b/bmc/version-handler/main.cpp
@@ -14,64 +14,14 @@
* limitations under the License.
*/
-#include "config.h"
-
-#include "file_handler.hpp"
-#include "general_systemd.hpp"
-#include "image_handler.hpp"
-#include "status.hpp"
#include "version_handler.hpp"
#include "version_handlers_builder.hpp"
-#include <phosphor-logging/log.hpp>
-#include <sdbusplus/bus.hpp>
-
-#include <cstdint>
#include <memory>
-#include <string>
-#include <unordered_map>
-#include <vector>
-
-namespace ipmi_flash
-{
-
-namespace
-{
-
-static constexpr const char* jsonConfigurationPath =
- "/usr/share/phosphor-ipmi-flash/";
-} // namespace
-
-std::unique_ptr<blobs::GenericBlobInterface>
- createHandlerFromJsons(VersionHandlersBuilder& builder,
- const char* configPath)
-{
- std::vector<HandlerConfig<VersionActionPack>> configsFromJson =
- builder.buildHandlerConfigs(configPath);
-
- VersionInfoMap handlerMap;
- for (auto& config : configsFromJson)
- {
- auto [it, inserted] = handlerMap.try_emplace(
- config.blobId, config.blobId, std::move(config.actions),
- std::move(config.handler));
- if (inserted == false)
- {
- std::fprintf(stderr, "duplicate blob id %s, discarding\n",
- config.blobId.c_str());
- }
- else
- {
- std::fprintf(stderr, "config loaded: %s\n", config.blobId.c_str());
- }
- }
- return VersionBlobHandler::create(std::move(handlerMap));
-}
-} // namespace ipmi_flash
extern "C" std::unique_ptr<blobs::GenericBlobInterface> createHandler()
{
- ipmi_flash::VersionHandlersBuilder builder;
- return ipmi_flash::createHandlerFromJsons(
- builder, ipmi_flash::jsonConfigurationPath);
+ return std::make_unique<ipmi_flash::VersionBlobHandler>(
+ ipmi_flash::VersionHandlersBuilder().buildHandlerConfigs(
+ "/usr/share/phosphor-ipmi-flash/"));
}
diff --git a/bmc/version-handler/test/Makefile.am b/bmc/version-handler/test/Makefile.am
index 0c02ff0..93daa37 100644
--- a/bmc/version-handler/test/Makefile.am
+++ b/bmc/version-handler/test/Makefile.am
@@ -9,16 +9,12 @@
$(GMOCK_CFLAGS) \
$(CODE_COVERAGE_CPPFLAGS)
AM_CXXFLAGS = \
- $(SDBUSPLUS_CFLAGS) \
- $(PHOSPHOR_LOGGING_CFLAGS) \
$(CODE_COVERAGE_CXXFLAGS)
AM_LDFLAGS = \
$(GTEST_LIBS) \
$(GMOCK_LIBS) \
-lgmock_main \
$(OESDK_TESTCASE_FLAGS) \
- $(SDBUSPLUS_LIBS) \
- $(PHOSPHOR_LOGGING_LIBS) \
$(CODE_COVERAGE_LIBS)
# Run all 'check' test programs
diff --git a/bmc/version-handler/test/version_canhandle_enumerate_unittest.cpp b/bmc/version-handler/test/version_canhandle_enumerate_unittest.cpp
index 3637c32..2816318 100644
--- a/bmc/version-handler/test/version_canhandle_enumerate_unittest.cpp
+++ b/bmc/version-handler/test/version_canhandle_enumerate_unittest.cpp
@@ -1,52 +1,29 @@
-#include "flags.hpp"
-#include "image_mock.hpp"
-#include "triggerable_mock.hpp"
-#include "util.hpp"
#include "version_handler.hpp"
+#include "version_mock.hpp"
#include <array>
-#include <string>
-#include <vector>
#include <gtest/gtest.h>
namespace ipmi_flash
{
+
TEST(VersionHandlerCanHandleTest, VerifyGoodInfoMap)
{
- VersionInfoMap test;
- std::array blobNames{"blob0", "blob1", "blob2", "blob3"};
+ constexpr std::array blobNames{"blob0", "blob1", "blob2", "blob3"};
+ VersionBlobHandler handler(createMockVersionConfigs(blobNames));
for (const auto& blobName : blobNames)
{
- test.try_emplace(blobName,
- VersionInfoPack(blobName,
- std::make_unique<VersionActionPack>(
- CreateTriggerMock()),
- CreateImageMock()));
- }
- auto handler = VersionBlobHandler::create(std::move(test));
- ASSERT_NE(handler, nullptr);
- for (const auto& blobName : blobNames)
- {
- EXPECT_TRUE(handler->canHandleBlob(blobName));
+ EXPECT_TRUE(handler.canHandleBlob(blobName));
}
}
TEST(VersionHandlerEnumerateTest, VerifyGoodInfoMap)
{
- VersionInfoMap test;
- std::vector<std::string> blobNames{"blob0", "blob1", "blob2", "blob3"};
- for (const auto& blobName : blobNames)
- {
- test.try_emplace(blobName,
- VersionInfoPack(blobName,
- std::make_unique<VersionActionPack>(
- CreateTriggerMock()),
- CreateImageMock()));
- }
- auto handler = VersionBlobHandler::create(std::move(test));
- ASSERT_NE(handler, nullptr);
- EXPECT_THAT(handler->getBlobIds(),
+ constexpr std::array blobNames{"blob0", "blob1", "blob2", "blob3"};
+ VersionBlobHandler handler(createMockVersionConfigs(blobNames));
+ EXPECT_THAT(handler.getBlobIds(),
::testing::UnorderedElementsAreArray(blobNames));
}
+
} // namespace ipmi_flash
diff --git a/bmc/version-handler/test/version_close_unittest.cpp b/bmc/version-handler/test/version_close_unittest.cpp
index 68e6add..83b4a43 100644
--- a/bmc/version-handler/test/version_close_unittest.cpp
+++ b/bmc/version-handler/test/version_close_unittest.cpp
@@ -1,16 +1,15 @@
-#include "flags.hpp"
-#include "image_mock.hpp"
-#include "triggerable_mock.hpp"
-#include "util.hpp"
#include "version_handler.hpp"
+#include "version_mock.hpp"
-#include <array>
+#include <memory>
#include <string>
#include <vector>
#include <gtest/gtest.h>
+
using ::testing::_;
using ::testing::Return;
+
namespace ipmi_flash
{
@@ -19,26 +18,10 @@
protected:
void SetUp() override
{
- VersionInfoMap vim;
- for (const auto& blobName : blobNames)
- {
- auto t = CreateTriggerMock();
- auto i = CreateImageMock();
- tm[blobName] = reinterpret_cast<TriggerMock*>(t.get());
- im[blobName] = reinterpret_cast<ImageHandlerMock*>(i.get());
- vim.try_emplace(
- blobName,
- VersionInfoPack(
- blobName, std::make_unique<VersionActionPack>(std::move(t)),
- std::move(i)));
- }
- h = VersionBlobHandler::create(std::move(vim));
- ASSERT_NE(h, nullptr);
- for (const auto& [key, val] : tm)
- {
- ON_CALL(*val, trigger()).WillByDefault(Return(true));
- }
+ h = std::make_unique<VersionBlobHandler>(
+ createMockVersionConfigs(blobNames, &im, &tm));
}
+
std::unique_ptr<blobs::GenericBlobInterface> h;
std::vector<std::string> blobNames{"blob0", "blob1", "blob2", "blob3"};
std::unordered_map<std::string, TriggerMock*> tm;
@@ -47,27 +30,23 @@
TEST_F(VersionCloseExpireBlobTest, VerifyOpenThenClose)
{
+ EXPECT_CALL(*tm.at("blob0"), trigger()).WillOnce(Return(true));
EXPECT_CALL(*tm.at("blob0"), status())
- .Times(1)
.WillOnce(Return(ActionStatus::success));
- EXPECT_CALL(*tm.at("blob0"), abort()).Times(0);
EXPECT_TRUE(h->open(0, blobs::read, "blob0"));
EXPECT_TRUE(h->close(0));
}
TEST_F(VersionCloseExpireBlobTest, VerifyUnopenedBlobCloseFails)
{
- EXPECT_CALL(*tm.at("blob0"), status()).Times(0);
- EXPECT_CALL(*tm.at("blob0"), abort()).Times(0);
EXPECT_FALSE(h->close(0));
}
TEST_F(VersionCloseExpireBlobTest, VerifyDoubleCloseFails)
{
+ EXPECT_CALL(*tm.at("blob0"), trigger()).WillOnce(Return(true));
EXPECT_CALL(*tm.at("blob0"), status())
- .Times(1)
.WillOnce(Return(ActionStatus::success));
- EXPECT_CALL(*tm.at("blob0"), abort()).Times(0);
EXPECT_TRUE(h->open(0, blobs::read, "blob0"));
EXPECT_TRUE(h->close(0));
EXPECT_FALSE(h->close(0));
@@ -75,10 +54,9 @@
TEST_F(VersionCloseExpireBlobTest, VerifyBadSessionNumberCloseFails)
{
+ EXPECT_CALL(*tm.at("blob0"), trigger()).WillOnce(Return(true));
EXPECT_CALL(*tm.at("blob0"), status())
- .Times(1)
.WillOnce(Return(ActionStatus::success));
- EXPECT_CALL(*tm.at("blob0"), abort()).Times(0);
EXPECT_TRUE(h->open(0, blobs::read, "blob0"));
EXPECT_FALSE(h->close(1));
EXPECT_TRUE(h->close(0));
@@ -86,8 +64,8 @@
TEST_F(VersionCloseExpireBlobTest, VerifyRunningActionIsAborted)
{
+ EXPECT_CALL(*tm.at("blob0"), trigger()).WillOnce(Return(true));
EXPECT_CALL(*tm.at("blob0"), status())
- .Times(1)
.WillOnce(Return(ActionStatus::running));
EXPECT_CALL(*tm.at("blob0"), abort()).Times(1);
EXPECT_TRUE(h->open(0, blobs::read, "blob0"));
diff --git a/bmc/version-handler/test/version_createhandler_unittest.cpp b/bmc/version-handler/test/version_createhandler_unittest.cpp
index c748f7f..849d1a4 100644
--- a/bmc/version-handler/test/version_createhandler_unittest.cpp
+++ b/bmc/version-handler/test/version_createhandler_unittest.cpp
@@ -1,12 +1,8 @@
-#include "flags.hpp"
-#include "image_mock.hpp"
-#include "triggerable_mock.hpp"
-#include "util.hpp"
#include "version_handler.hpp"
+#include "version_mock.hpp"
#include <array>
-#include <string>
-#include <vector>
+#include <utility>
#include <gtest/gtest.h>
@@ -15,18 +11,20 @@
TEST(VersionHandlerCanHandleTest, VerifyGoodInfoMapPasses)
{
- VersionInfoMap test;
- std::array blobNames{"blob0", "blob1", "blob2", "blob3"};
- for (const auto& blobName : blobNames)
- {
- test.try_emplace(blobName,
- VersionInfoPack(blobName,
- std::make_unique<VersionActionPack>(
- CreateTriggerMock()),
- CreateImageMock()));
- }
- auto handler = VersionBlobHandler::create(std::move(test));
- EXPECT_NE(handler, nullptr);
+ constexpr std::array blobs{"blob0", "blob1"};
+ VersionBlobHandler handler(createMockVersionConfigs(blobs));
+ EXPECT_THAT(handler.getBlobIds(),
+ testing::UnorderedElementsAreArray(blobs));
+}
+
+TEST(VersionHandlerCanHandleTest, VerifyDuplicatesIgnored)
+{
+ constexpr std::array blobs{"blob0"};
+ auto configs = createMockVersionConfigs(blobs);
+ configs.push_back(createMockVersionConfig(blobs[0]));
+ VersionBlobHandler handler(std::move(configs));
+ EXPECT_THAT(handler.getBlobIds(),
+ testing::UnorderedElementsAreArray(blobs));
}
} // namespace ipmi_flash
diff --git a/bmc/version-handler/test/version_mock.hpp b/bmc/version-handler/test/version_mock.hpp
new file mode 100644
index 0000000..80d3748
--- /dev/null
+++ b/bmc/version-handler/test/version_mock.hpp
@@ -0,0 +1,49 @@
+#include "buildjson.hpp"
+#include "image_mock.hpp"
+#include "triggerable_mock.hpp"
+#include "version_handler.hpp"
+
+#include <memory>
+#include <string>
+#include <utility>
+
+namespace ipmi_flash
+{
+
+auto createMockVersionConfig(const std::string& id,
+ ImageHandlerMock** im = nullptr,
+ TriggerMock** tm = nullptr)
+{
+ HandlerConfig<VersionBlobHandler::ActionPack> ret;
+ ret.blobId = id;
+ auto handler = std::make_unique<testing::StrictMock<ImageHandlerMock>>();
+ if (im != nullptr)
+ {
+ *im = handler.get();
+ }
+ ret.handler = std::move(handler);
+ ret.actions = std::make_unique<VersionBlobHandler::ActionPack>();
+ auto trigger = std::make_unique<testing::StrictMock<TriggerMock>>();
+ if (tm != nullptr)
+ {
+ *tm = trigger.get();
+ }
+ ret.actions->onOpen = std::move(trigger);
+ return ret;
+}
+
+template <typename C, typename Im = std::map<std::string, ImageHandlerMock*>,
+ typename Tm = std::map<std::string, TriggerMock*>>
+auto createMockVersionConfigs(const C& ids, Im* im = nullptr, Tm* tm = nullptr)
+{
+ std::vector<HandlerConfig<VersionBlobHandler::ActionPack>> ret;
+ for (const auto& id : ids)
+ {
+ ret.push_back(
+ createMockVersionConfig(id, im == nullptr ? nullptr : &(*im)[id],
+ tm == nullptr ? nullptr : &(*tm)[id]));
+ }
+ return ret;
+}
+
+} // namespace ipmi_flash
diff --git a/bmc/version-handler/test/version_open_unittest.cpp b/bmc/version-handler/test/version_open_unittest.cpp
index 7e287e5..7c7e5d2 100644
--- a/bmc/version-handler/test/version_open_unittest.cpp
+++ b/bmc/version-handler/test/version_open_unittest.cpp
@@ -1,16 +1,16 @@
-#include "flags.hpp"
-#include "image_mock.hpp"
-#include "triggerable_mock.hpp"
-#include "util.hpp"
#include "version_handler.hpp"
+#include "version_mock.hpp"
-#include <array>
+#include <memory>
#include <string>
+#include <unordered_map>
#include <vector>
#include <gtest/gtest.h>
+
using ::testing::_;
using ::testing::Return;
+
namespace ipmi_flash
{
@@ -19,32 +19,15 @@
protected:
void SetUp() override
{
- VersionInfoMap vim;
- for (const auto& blobName : blobNames)
+ h = std::make_unique<VersionBlobHandler>(
+ createMockVersionConfigs(blobNames, &im, &tm));
+ for (const auto& blob : blobNames)
{
- auto t = CreateTriggerMock();
- auto i = CreateImageMock();
- tm[blobName] = reinterpret_cast<TriggerMock*>(t.get());
- im[blobName] = reinterpret_cast<ImageHandlerMock*>(i.get());
- vim.try_emplace(
- blobName,
- VersionInfoPack(
- blobName, std::make_unique<VersionActionPack>(std::move(t)),
- std::move(i)));
- }
- h = VersionBlobHandler::create(std::move(vim));
- ASSERT_NE(h, nullptr);
- for (const auto& [key, val] : tm)
- {
- /* by default no action triggers expected to be called */
- EXPECT_CALL(*val, trigger()).Times(0);
- }
- for (const auto& [key, val] : im)
- {
- /* by default no image handler open is expected to be called */
- EXPECT_CALL(*val, open(_, _)).Times(0);
+ EXPECT_CALL(*tm.at(blob), status())
+ .WillRepeatedly(Return(ActionStatus::unknown));
}
}
+
std::unique_ptr<blobs::GenericBlobInterface> h;
std::vector<std::string> blobNames{"blob0", "blob1", "blob2", "blob3"};
std::unordered_map<std::string, TriggerMock*> tm;
diff --git a/bmc/version-handler/test/version_read_unittest.cpp b/bmc/version-handler/test/version_read_unittest.cpp
index 2f15e03..e1c4618 100644
--- a/bmc/version-handler/test/version_read_unittest.cpp
+++ b/bmc/version-handler/test/version_read_unittest.cpp
@@ -1,10 +1,7 @@
-#include "flags.hpp"
-#include "image_mock.hpp"
-#include "triggerable_mock.hpp"
-#include "util.hpp"
#include "version_handler.hpp"
+#include "version_mock.hpp"
-#include <array>
+#include <memory>
#include <string>
#include <vector>
@@ -20,25 +17,8 @@
protected:
void SetUp() override
{
- VersionInfoMap vim;
- for (const auto& blobName : blobNames)
- {
- auto t = CreateTriggerMock();
- auto i = CreateImageMock();
- tm[blobName] = reinterpret_cast<TriggerMock*>(t.get());
- im[blobName] = reinterpret_cast<ImageHandlerMock*>(i.get());
- vim.try_emplace(
- blobName,
- VersionInfoPack(
- blobName, std::make_unique<VersionActionPack>(std::move(t)),
- std::move(i)));
- }
- h = VersionBlobHandler::create(std::move(vim));
- ASSERT_NE(h, nullptr);
- for (const auto& [key, val] : tm)
- {
- ON_CALL(*val, trigger()).WillByDefault(Return(true));
- }
+ h = std::make_unique<VersionBlobHandler>(
+ createMockVersionConfigs(blobNames, &im, &tm));
}
std::unique_ptr<blobs::GenericBlobInterface> h;
std::vector<std::string> blobNames{"blob0", "blob1", "blob2", "blob3"};
@@ -51,6 +31,7 @@
TEST_F(VersionReadBlobTest, VerifyValidRead)
{
+ EXPECT_CALL(*tm.at("blob0"), trigger()).WillOnce(Return(true));
EXPECT_CALL(*tm.at("blob0"), status())
.Times(2)
.WillRepeatedly(Return(ActionStatus::success));
@@ -71,37 +52,28 @@
TEST_F(VersionReadBlobTest, VerifyUnopenedReadFails)
{
- EXPECT_CALL(*tm.at("blob0"), status()).Times(0);
- EXPECT_CALL(*im.at("blob0"), open(_, _)).Times(0);
- EXPECT_CALL(*im.at("blob0"), read(_, _)).Times(0);
-
EXPECT_THAT(h->read(defaultSessionNumber, 0, 10), IsEmpty());
}
TEST_F(VersionReadBlobTest, VerifyTriggerFailureReadFails)
{
+ EXPECT_CALL(*tm.at("blob0"), trigger()).WillOnce(Return(true));
EXPECT_CALL(*tm.at("blob0"), status())
- .Times(1)
.WillOnce(Return(ActionStatus::failed));
- EXPECT_CALL(*im.at("blob0"), open(_, _)).Times(0);
EXPECT_TRUE(h->open(defaultSessionNumber, blobs::read, "blob0"));
EXPECT_THAT(h->read(defaultSessionNumber, 0, 10), IsEmpty());
}
TEST_F(VersionReadBlobTest, VerifyReadFailsOnFileReadFailure)
{
+ EXPECT_CALL(*tm.at("blob0"), trigger()).WillOnce(Return(true));
EXPECT_CALL(*tm.at("blob0"), status())
- .Times(1)
.WillOnce(Return(ActionStatus::success));
/* file path gets bound to file_handler on creation so path parameter
* doesn't actually matter
*/
- EXPECT_CALL(*im.at("blob0"), open(_, std::ios::in))
- .Times(1)
- .WillOnce(Return(true));
- EXPECT_CALL(*im.at("blob0"), read(_, _))
- .Times(1)
- .WillOnce(Return(std::nullopt));
+ EXPECT_CALL(*im.at("blob0"), open(_, std::ios::in)).WillOnce(Return(true));
+ EXPECT_CALL(*im.at("blob0"), read(_, _)).WillOnce(Return(std::nullopt));
EXPECT_CALL(*im.at("blob0"), close()).Times(1);
EXPECT_TRUE(h->open(defaultSessionNumber, blobs::read, "blob0"));
@@ -110,16 +82,14 @@
TEST_F(VersionReadBlobTest, VerifyReadFailsOnFileOpenFailure)
{
+ EXPECT_CALL(*tm.at("blob0"), trigger()).WillOnce(Return(true));
/* first call to trigger status fails, second succeeds */
EXPECT_CALL(*tm.at("blob0"), status())
- .Times(1)
.WillOnce(Return(ActionStatus::success));
/* file path gets bound to file_handler on creation so path parameter
* doesn't actually matter
*/
- EXPECT_CALL(*im.at("blob0"), open(_, std::ios::in))
- .Times(1)
- .WillOnce(Return(false));
+ EXPECT_CALL(*im.at("blob0"), open(_, std::ios::in)).WillOnce(Return(false));
EXPECT_TRUE(h->open(defaultSessionNumber, blobs::read, "blob0"));
EXPECT_THAT(h->read(defaultSessionNumber, 0, 10), IsEmpty());
diff --git a/bmc/version-handler/version_handler.cpp b/bmc/version-handler/version_handler.cpp
index 8bf76bd..6cb8368 100644
--- a/bmc/version-handler/version_handler.cpp
+++ b/bmc/version-handler/version_handler.cpp
@@ -1,28 +1,40 @@
#include "version_handler.hpp"
#include <stdexcept>
+#include <utility>
+#include <vector>
+
namespace ipmi_flash
{
-std::unique_ptr<blobs::GenericBlobInterface>
- VersionBlobHandler::create(VersionInfoMap&& versionMap)
+
+VersionBlobHandler::VersionBlobHandler(
+ std::vector<HandlerConfig<ActionPack>>&& configs)
{
- std::vector<std::string> blobList;
- for (const auto& [key, val] : versionMap)
+ for (auto& config : configs)
{
- blobList.push_back(key);
+ BlobInfo info = {std::move(config.blobId), std::move(config.actions),
+ std::move(config.handler)};
+ if (!blobInfoMap.try_emplace(info.blobId, std::move(info)).second)
+ {
+ fprintf(stderr, "Ignoring duplicate config for %s\n",
+ info.blobId.c_str());
+ }
}
- return std::make_unique<VersionBlobHandler>(std::move(blobList),
- std::move(versionMap));
}
bool VersionBlobHandler::canHandleBlob(const std::string& path)
{
- return (std::find(blobIds.begin(), blobIds.end(), path) != blobIds.end());
+ return blobInfoMap.find(path) != blobInfoMap.end();
}
std::vector<std::string> VersionBlobHandler::getBlobIds()
{
- return blobIds;
+ std::vector<std::string> ret;
+ for (const auto& [key, _] : blobInfoMap)
+ {
+ ret.push_back(key);
+ }
+ return ret;
}
/**
@@ -64,7 +76,7 @@
try
{
- auto& v = versionInfoMap.at(path);
+ auto& v = blobInfoMap.at(path);
if (v.blobState == blobs::StateFlags::open_read)
{
cleanup(session);
@@ -72,7 +84,7 @@
path.c_str());
return false;
}
- if (v.actionPack->onOpen->trigger() == false)
+ if (v.actions->onOpen->trigger() == false)
{
fprintf(stderr, "open %s fail: onOpen trigger failed\n",
path.c_str());
@@ -94,11 +106,11 @@
uint32_t requestedSize)
{
std::string* blobName;
- VersionInfoPack* pack;
+ BlobInfo* pack;
try
{
blobName = &sessionToBlob.at(session);
- pack = &versionInfoMap.at(*blobName);
+ pack = &blobInfoMap.at(*blobName);
}
catch (const std::out_of_range& e)
{
@@ -107,26 +119,26 @@
/* onOpen trigger must be successful, otherwise potential
* for stale data to be read
*/
- if (pack->actionPack->onOpen->status() != ActionStatus::success)
+ if (pack->actions->onOpen->status() != ActionStatus::success)
{
fprintf(stderr, "read failed: onOpen trigger not successful\n");
return {};
}
- if (!pack->imageHandler->open("don't care", std::ios::in))
+ if (!pack->handler->open("don't care", std::ios::in))
{
fprintf(stderr, "read failed: file open unsuccessful blob=%s\n",
blobName->c_str());
return {};
}
- auto d = pack->imageHandler->read(offset, requestedSize);
+ auto d = pack->handler->read(offset, requestedSize);
if (!d)
{
fprintf(stderr, "read failed: unable to read file for blob %s\n",
blobName->c_str());
- pack->imageHandler->close();
+ pack->handler->close();
return {};
}
- pack->imageHandler->close();
+ pack->handler->close();
return *d;
}
@@ -150,10 +162,10 @@
try
{
const auto& blobName = sessionToBlob.at(session);
- auto& pack = versionInfoMap.at(blobName);
- if (pack.actionPack->onOpen->status() == ActionStatus::running)
+ auto& pack = blobInfoMap.at(blobName);
+ if (pack.actions->onOpen->status() == ActionStatus::running)
{
- pack.actionPack->onOpen->abort();
+ pack.actions->onOpen->abort();
}
pack.blobState = static_cast<blobs::StateFlags>(0);
sessionToBlob.erase(session);
diff --git a/bmc/version-handler/version_handler.hpp b/bmc/version-handler/version_handler.hpp
index 336a869..ad380b8 100644
--- a/bmc/version-handler/version_handler.hpp
+++ b/bmc/version-handler/version_handler.hpp
@@ -6,77 +6,32 @@
#include <blobs-ipmid/blobs.hpp>
-#include <algorithm>
#include <cstdint>
#include <map>
#include <memory>
#include <string>
#include <unordered_map>
#include <vector>
+
namespace ipmi_flash
{
-struct VersionActionPack
-{
- public:
- VersionActionPack(std::unique_ptr<TriggerableActionInterface> openAction) :
- onOpen(std::move(openAction)){};
- VersionActionPack() = default;
- /** Only file operation action supported currently */
- std::unique_ptr<TriggerableActionInterface> onOpen;
-};
-
-/*
- * All the information associated with a version blob
- */
-struct VersionInfoPack
-{
- public:
- VersionInfoPack(const std::string& blobId,
- std::unique_ptr<VersionActionPack> actionPackIn,
- std::unique_ptr<ImageHandlerInterface> imageHandler) :
- blobId(blobId),
- actionPack(std::move(actionPackIn)),
- imageHandler(std::move(imageHandler)),
- blobState(static_cast<blobs::StateFlags>(0)){};
- VersionInfoPack() = default;
-
- std::string blobId;
- std::unique_ptr<VersionActionPack> actionPack;
- std::unique_ptr<ImageHandlerInterface> imageHandler;
- blobs::StateFlags blobState;
-};
-
-/* convenience alias: the key (std::string) is blobId, same as
- * VersionInfoPack.blobId */
-using VersionInfoMap = std::unordered_map<std::string, VersionInfoPack>;
class VersionBlobHandler : public blobs::GenericBlobInterface
{
public:
- /**
- * Factory to create a BlobHandler for use by phosphor-ipmi-blobs
- *
- * @param[in] versionMap - blob names to VersionInfoPack which contains
- * triggers, file handlers, state and blobID.
- *
- * @returns a unique_ptr to a GenericBlobInterface which is used by
- * phosphor-ipmi-blobs. The underlying implementation (VersionBlobHandler)
- * implements all the blob operations for the blobs owned by
- * VersionBlobHandler.
- */
- static std::unique_ptr<blobs::GenericBlobInterface>
- create(VersionInfoMap&& versionMap);
+ struct ActionPack
+ {
+ /** Only file operation action supported currently */
+ std::unique_ptr<TriggerableActionInterface> onOpen;
+ };
+
/**
* Create a VersionBlobHandler.
*
- * @param[in] blobs - list of blobs_ids to support
- * @param[in] actions - a map of blobId to VersionInfoPack
+ * @param[in] configs - list of blob configurations to support
*/
- VersionBlobHandler(std::vector<std::string>&& blobs,
- VersionInfoMap&& handlerMap) :
- blobIds(blobs),
- versionInfoMap(std::move(handlerMap))
- {}
+ VersionBlobHandler(std::vector<HandlerConfig<ActionPack>>&& configs);
+
~VersionBlobHandler() = default;
VersionBlobHandler(const VersionBlobHandler&) = delete;
VersionBlobHandler& operator=(const VersionBlobHandler&) = delete;
@@ -111,8 +66,15 @@
bool cleanup(uint16_t session);
private:
- std::vector<std::string> blobIds;
- VersionInfoMap versionInfoMap;
+ struct BlobInfo
+ {
+ std::string blobId;
+ std::unique_ptr<ActionPack> actions;
+ std::unique_ptr<ImageHandlerInterface> handler;
+ blobs::StateFlags blobState = static_cast<blobs::StateFlags>(0);
+ };
+
+ std::unordered_map<std::string, BlobInfo> blobInfoMap;
std::unordered_map<uint16_t, std::string> sessionToBlob;
};
} // namespace ipmi_flash
diff --git a/bmc/version-handler/version_handlers_builder.cpp b/bmc/version-handler/version_handlers_builder.cpp
index d6ed016..f27fa54 100644
--- a/bmc/version-handler/version_handlers_builder.cpp
+++ b/bmc/version-handler/version_handlers_builder.cpp
@@ -32,16 +32,17 @@
namespace ipmi_flash
{
-std::vector<HandlerConfig<VersionActionPack>>
+
+std::vector<HandlerConfig<VersionBlobHandler::ActionPack>>
VersionHandlersBuilder::buildHandlerFromJson(const nlohmann::json& data)
{
- std::vector<HandlerConfig<VersionActionPack>> handlers;
+ std::vector<HandlerConfig<VersionBlobHandler::ActionPack>> handlers;
for (const auto& item : data)
{
try
{
- HandlerConfig<VersionActionPack> output;
+ HandlerConfig<VersionBlobHandler::ActionPack> output;
/* at() throws an exception when the key is not present. */
item.at("blob").get_to(output.blobId);
@@ -75,8 +76,7 @@
/* actions are required (presently). */
const auto& a = v.at("actions");
- std::unique_ptr<VersionActionPack> pack =
- std::make_unique<VersionActionPack>();
+ auto pack = std::make_unique<VersionBlobHandler::ActionPack>();
/* to make an action optional, assign type "skip" */
const auto& onOpen = a.at("open");
@@ -111,4 +111,5 @@
return handlers;
}
+
} // namespace ipmi_flash
diff --git a/bmc/version-handler/version_handlers_builder.hpp b/bmc/version-handler/version_handlers_builder.hpp
index 5055098..0ec85ae 100644
--- a/bmc/version-handler/version_handlers_builder.hpp
+++ b/bmc/version-handler/version_handlers_builder.hpp
@@ -12,10 +12,11 @@
* provide the method to parse and validate blob entries from json and produce
* something that is usable by the version handler.
*/
-class VersionHandlersBuilder : public HandlersBuilderIfc<VersionActionPack>
+class VersionHandlersBuilder :
+ public HandlersBuilderIfc<VersionBlobHandler::ActionPack>
{
public:
- std::vector<HandlerConfig<VersionActionPack>>
+ std::vector<HandlerConfig<VersionBlobHandler::ActionPack>>
buildHandlerFromJson(const nlohmann::json& data) override;
};
} // namespace ipmi_flash