make building handlers from json more generic
Problem: version-handler, a new feature that will be added to this
repository will also parse json files from the same directory ipmi-flash
does. Currently buildjson is a mix of some code that can be reused and
other code that is pretty specific to firmware updates; this makes it
hard to reuse the code.
Solution: factor out the generic parts and place it in bmc and then
leave the specific parts in firmware-handler. Also code changes have
been made to buildjson: wrap functions in a templated class that leaves
feature specific parsing as a pure virtual method.
Tested:
Ran the unit tests, which do test the parsing functionality.
Signed-off-by: Jason Ling <jasonling@google.com>
Change-Id: I021dc829a82d1719b4cb862cdfb224eca629a44d
diff --git a/bmc/Makefile.am b/bmc/Makefile.am
index 4592bbd..3bccc41 100644
--- a/bmc/Makefile.am
+++ b/bmc/Makefile.am
@@ -3,6 +3,7 @@
# shared functionality between firmware and version blob handlers
libbmc_common_la_SOURCES = \
+ buildjson.cpp \
file_handler.cpp \
fs.cpp \
general_systemd.cpp \
diff --git a/bmc/buildjson.cpp b/bmc/buildjson.cpp
new file mode 100644
index 0000000..c4bc4b5
--- /dev/null
+++ b/bmc/buildjson.cpp
@@ -0,0 +1,101 @@
+/*
+ * Copyright 2019 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include "buildjson.hpp"
+
+#include "file_handler.hpp"
+#include "fs.hpp"
+
+#include <nlohmann/json.hpp>
+#include <sdbusplus/bus.hpp>
+
+#include <algorithm>
+#include <cstdio>
+#include <exception>
+#include <fstream>
+#include <string>
+#include <vector>
+
+namespace ipmi_flash
+{
+
+std::unique_ptr<TriggerableActionInterface>
+ buildFileSystemd(const nlohmann::json& data)
+{
+ /* This type of action requires a path and unit, and optionally a mode. */
+ const auto& path = data.at("path");
+ const auto& unit = data.at("unit");
+
+ /* the mode parameter is optional. */
+ std::string systemdMode = "replace";
+ const auto& mode = data.find("mode");
+ if (mode != data.end())
+ {
+ systemdMode = data.at("mode").get<std::string>();
+ }
+
+ return SystemdWithStatusFile::CreateSystemdWithStatusFile(
+ sdbusplus::bus::new_default(), path, unit, systemdMode);
+}
+
+std::unique_ptr<TriggerableActionInterface>
+ buildSystemd(const nlohmann::json& data)
+{
+ /* This type of action requires a unit, and optionally a mode. */
+ const auto& unit = data.at("unit");
+
+ /* the mode parameter is optional. */
+ std::string systemdMode = "replace";
+ const auto& mode = data.find("mode");
+ if (mode != data.end())
+ {
+ systemdMode = data.at("mode").get<std::string>();
+ }
+
+ return SystemdNoFile::CreateSystemdNoFile(sdbusplus::bus::new_default(),
+ unit, systemdMode);
+}
+
+template <typename T>
+std::vector<HandlerConfig<T>>
+ HandlersBuilderIfc<T>::buildHandlerConfigs(const std::string& directory)
+{
+ std::vector<HandlerConfig<T>> output;
+
+ std::vector<std::string> jsonPaths = GetJsonList(directory);
+
+ for (const auto& path : jsonPaths)
+ {
+ std::ifstream jsonFile(path);
+ if (!jsonFile.is_open())
+ {
+ std::fprintf(stderr, "Unable to open json file: %s\n",
+ path.c_str());
+ continue;
+ }
+
+ auto data = nlohmann::json::parse(jsonFile, nullptr, false);
+ if (data.is_discarded())
+ {
+ std::fprintf(stderr, "Parsing json failed: %s\n", path.c_str());
+ }
+
+ std::vector<HandlerConfig<T>> configs = buildHandlerFromJson(data);
+ std::move(configs.begin(), configs.end(), std::back_inserter(output));
+ }
+ return output;
+}
+
+} // namespace ipmi_flash
diff --git a/bmc/buildjson.hpp b/bmc/buildjson.hpp
new file mode 100644
index 0000000..4aae00d
--- /dev/null
+++ b/bmc/buildjson.hpp
@@ -0,0 +1,97 @@
+#pragma once
+
+#include "general_systemd.hpp"
+#include "image_handler.hpp"
+
+#include <nlohmann/json.hpp>
+
+#include <memory>
+#include <string>
+#include <vector>
+
+namespace ipmi_flash
+{
+std::unique_ptr<TriggerableActionInterface>
+ buildFileSystemd(const nlohmann::json& data);
+
+std::unique_ptr<TriggerableActionInterface>
+ buildSystemd(const nlohmann::json& data);
+
+/**
+ * HandlerConfig associates a blobID with an ImageHandler and a set of
+ * supported actions of type T.
+ */
+template <typename T>
+class HandlerConfig
+{
+ public:
+ HandlerConfig() = default;
+ ~HandlerConfig() = default;
+ HandlerConfig(const HandlerConfig&) = delete;
+ HandlerConfig& operator=(const HandlerConfig&) = delete;
+ HandlerConfig(HandlerConfig&&) = default;
+ HandlerConfig& operator=(HandlerConfig&&) = default;
+
+ /* A string in the form: /flash/{unique}, s.t. unique is something like,
+ * flash, ubitar, statictar, or bios
+ */
+ std::string blobId;
+
+ /* This owns a handler interface, this is typically going to be a file
+ * writer object.
+ */
+ std::unique_ptr<ImageHandlerInterface> handler;
+
+ /* specifies actions to be taken in response to certain operations on a
+ * blob.
+ * Usually required but there are exceptions; the hashBlobId doesn't have
+ * an action pack.
+ */
+ std::unique_ptr<T> actions;
+};
+
+/* HandlersBuilderIfc is a helper class that builds Handlers from the json files
+ * found within a specified directory.
+ * The child class that inherits from HandlersBuilderIfc should implement
+ * buildHandlersConfigs to perform json validation and parsing.
+ *
+ */
+template <typename T>
+class HandlersBuilderIfc
+{
+ public:
+ HandlersBuilderIfc() = default;
+ ~HandlersBuilderIfc() = default;
+ HandlersBuilderIfc(const HandlersBuilderIfc&) = delete;
+ HandlersBuilderIfc& operator=(const HandlersBuilderIfc&) = delete;
+ HandlersBuilderIfc(HandlersBuilderIfc&&) = default;
+ HandlersBuilderIfc& operator=(HandlersBuilderIfc&&) = default;
+ /**
+ * Given a folder of json configs, build the configurations.
+ *
+ * @param[in] directory - the directory to search (recurisvely).
+ * @return list of HandlerConfig objects.
+ */
+ std::vector<HandlerConfig<T>>
+ buildHandlerConfigs(const std::string& directory);
+ /**
+ * Given a list of handlers as json data, construct the appropriate
+ * HandlerConfig objects. This method is meant to be called per json
+ * configuration file found.
+ *
+ * The list will only contain validly build HandlerConfig objects. Any
+ * invalid configuration is skipped. The hope is that the BMC firmware
+ * update configuration will never be invalid, but if another aspect is
+ * invalid, it can be fixed with a BMC firmware update once the bug is
+ * identified.
+ *
+ * This code does not validate that the blob specified is unique, that
+ * should be handled at a higher layer.
+ *
+ * @param[in] data - json data from a json file.
+ * @return list of HandlerConfig objects.
+ */
+ virtual std::vector<HandlerConfig<T>>
+ buildHandlerFromJson(const nlohmann::json& data) = 0;
+};
+} // namespace ipmi_flash
diff --git a/bmc/firmware-handler/Makefile.am b/bmc/firmware-handler/Makefile.am
index c4fe392..e7b8eb2 100644
--- a/bmc/firmware-handler/Makefile.am
+++ b/bmc/firmware-handler/Makefile.am
@@ -42,7 +42,7 @@
# firmware blob handler specific
libfirmwareblob_common_la_SOURCES = \
- buildjson.cpp \
+ firmware_handlers_builder.cpp \
firmware_handler.cpp \
lpc_handler.cpp
diff --git a/bmc/firmware-handler/buildjson.hpp b/bmc/firmware-handler/buildjson.hpp
deleted file mode 100644
index 0af4241..0000000
--- a/bmc/firmware-handler/buildjson.hpp
+++ /dev/null
@@ -1,66 +0,0 @@
-#pragma once
-
-#include "firmware_handler.hpp"
-#include "image_handler.hpp"
-
-#include <nlohmann/json.hpp>
-
-#include <memory>
-#include <string>
-#include <vector>
-
-namespace ipmi_flash
-{
-
-class HandlerConfig
-{
- public:
- HandlerConfig() = default;
- ~HandlerConfig() = default;
- HandlerConfig(const HandlerConfig&) = delete;
- HandlerConfig& operator=(const HandlerConfig&) = delete;
- HandlerConfig(HandlerConfig&&) = default;
- HandlerConfig& operator=(HandlerConfig&&) = default;
-
- /* A string in the form: /flash/{unique}, s.t. unique is something like,
- * flash, ubitar, statictar, or bios
- */
- std::string blobId;
-
- /* This owns a handler interface, this is typically going to be a file
- * writer object.
- */
- std::unique_ptr<ImageHandlerInterface> handler;
-
- /* Only the hashBlobId doesn't have an action pack, otherwise it's required.
- */
- std::unique_ptr<ActionPack> actions;
-};
-
-/**
- * Given a list of handlers as json data, construct the appropriate
- * HandlerConfig objects. This method is meant to be called per json
- * configuration file found.
- *
- * The list will only contain validly build HandlerConfig objects. Any invalid
- * configuration is skipped. The hope is that the BMC firmware update
- * configuration will never be invalid, but if another aspect is invalid, it can
- * be fixed with a BMC firmware update once the bug is identified.
- *
- * This code does not validate that the blob specified is unique, that should
- * be handled at a higher layer.
- *
- * @param[in] data - json data from a json file.
- * @return list of HandlerConfig objects.
- */
-std::vector<HandlerConfig> buildHandlerFromJson(const nlohmann::json& data);
-
-/**
- * Given a folder of json configs, build the configurations.
- *
- * @param[in] directory - the directory to search (recurisvely).
- * @return list of HandlerConfig objects.
- */
-std::vector<HandlerConfig> BuildHandlerConfigs(const std::string& directory);
-
-} // namespace ipmi_flash
diff --git a/bmc/firmware-handler/buildjson.cpp b/bmc/firmware-handler/firmware_handlers_builder.cpp
similarity index 68%
rename from bmc/firmware-handler/buildjson.cpp
rename to bmc/firmware-handler/firmware_handlers_builder.cpp
index 095161c..d5f029c 100644
--- a/bmc/firmware-handler/buildjson.cpp
+++ b/bmc/firmware-handler/firmware_handlers_builder.cpp
@@ -13,15 +13,13 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-#include "buildjson.hpp"
+#include "firmware_handlers_builder.hpp"
#include "file_handler.hpp"
#include "fs.hpp"
-#include "general_systemd.hpp"
#include "skip_action.hpp"
#include <nlohmann/json.hpp>
-#include <sdbusplus/bus.hpp>
#include <algorithm>
#include <cstdio>
@@ -33,53 +31,16 @@
namespace ipmi_flash
{
-
-std::unique_ptr<TriggerableActionInterface>
- buildFileSystemd(const nlohmann::json& data)
+std::vector<HandlerConfig<ActionPack>>
+ FirmwareHandlersBuilder::buildHandlerFromJson(const nlohmann::json& data)
{
- /* This type of action requires a path and unit, and optionally a mode. */
- const auto& path = data.at("path");
- const auto& unit = data.at("unit");
-
- /* the mode parameter is optional. */
- std::string systemdMode = "replace";
- const auto& mode = data.find("mode");
- if (mode != data.end())
- {
- systemdMode = data.at("mode").get<std::string>();
- }
-
- return SystemdWithStatusFile::CreateSystemdWithStatusFile(
- sdbusplus::bus::new_default(), path, unit, systemdMode);
-}
-
-std::unique_ptr<TriggerableActionInterface>
- buildSystemd(const nlohmann::json& data)
-{
- /* This type of action requires a unit, and optionally a mode. */
- const auto& unit = data.at("unit");
-
- /* the mode parameter is optional. */
- std::string systemdMode = "replace";
- const auto& mode = data.find("mode");
- if (mode != data.end())
- {
- systemdMode = data.at("mode").get<std::string>();
- }
-
- return SystemdNoFile::CreateSystemdNoFile(sdbusplus::bus::new_default(),
- unit, systemdMode);
-}
-
-std::vector<HandlerConfig> buildHandlerFromJson(const nlohmann::json& data)
-{
- std::vector<HandlerConfig> handlers;
+ std::vector<HandlerConfig<ActionPack>> handlers;
for (const auto& item : data)
{
try
{
- HandlerConfig output;
+ HandlerConfig<ActionPack> output;
/* at() throws an exception when the key is not present. */
item.at("blob").get_to(output.blobId);
@@ -188,34 +149,4 @@
return handlers;
}
-
-std::vector<HandlerConfig> BuildHandlerConfigs(const std::string& directory)
-{
- std::vector<HandlerConfig> output;
-
- std::vector<std::string> jsonPaths = GetJsonList(directory);
-
- for (const auto& path : jsonPaths)
- {
- std::ifstream jsonFile(path);
- if (!jsonFile.is_open())
- {
- std::fprintf(stderr, "Unable to open json file: %s\n",
- path.c_str());
- continue;
- }
-
- auto data = nlohmann::json::parse(jsonFile, nullptr, false);
- if (data.is_discarded())
- {
- std::fprintf(stderr, "Parsing json failed: %s\n", path.c_str());
- }
-
- std::vector<HandlerConfig> configs = buildHandlerFromJson(data);
- std::move(configs.begin(), configs.end(), std::back_inserter(output));
- }
-
- return output;
-}
-
} // namespace ipmi_flash
diff --git a/bmc/firmware-handler/firmware_handlers_builder.hpp b/bmc/firmware-handler/firmware_handlers_builder.hpp
new file mode 100644
index 0000000..6cfb649
--- /dev/null
+++ b/bmc/firmware-handler/firmware_handlers_builder.hpp
@@ -0,0 +1,17 @@
+#pragma once
+#include "buildjson.hpp"
+#include "firmware_handler.hpp"
+
+#include <nlohmann/json.hpp>
+
+#include <vector>
+
+namespace ipmi_flash
+{
+class FirmwareHandlersBuilder : public HandlersBuilderIfc<ActionPack>
+{
+ public:
+ std::vector<HandlerConfig<ActionPack>>
+ buildHandlerFromJson(const nlohmann::json& data) override;
+};
+} // namespace ipmi_flash
diff --git a/bmc/firmware-handler/main.cpp b/bmc/firmware-handler/main.cpp
index eeca589..32d83f0 100644
--- a/bmc/firmware-handler/main.cpp
+++ b/bmc/firmware-handler/main.cpp
@@ -16,9 +16,9 @@
#include "config.h"
-#include "buildjson.hpp"
#include "file_handler.hpp"
#include "firmware_handler.hpp"
+#include "firmware_handlers_builder.hpp"
#include "flags.hpp"
#include "general_systemd.hpp"
#include "image_handler.hpp"
@@ -113,9 +113,10 @@
#endif
ActionMap actionPacks = {};
+ FirmwareHandlersBuilder builder;
- std::vector<HandlerConfig> configsFromJson =
- BuildHandlerConfigs(jsonConfigurationPath);
+ std::vector<HandlerConfig<ActionPack>> configsFromJson =
+ builder.buildHandlerConfigs(jsonConfigurationPath);
std::vector<HandlerPack> supportedFirmware;
diff --git a/bmc/firmware-handler/test/firmware_json_unittest.cpp b/bmc/firmware-handler/test/firmware_json_unittest.cpp
index 8f3ea30..235b33c 100644
--- a/bmc/firmware-handler/test/firmware_json_unittest.cpp
+++ b/bmc/firmware-handler/test/firmware_json_unittest.cpp
@@ -1,4 +1,4 @@
-#include "buildjson.hpp"
+#include "firmware_handlers_builder.hpp"
#include "general_systemd.hpp"
#include "skip_action.hpp"
@@ -40,8 +40,7 @@
}
}]
)"_json;
-
- EXPECT_THAT(buildHandlerFromJson(j2), IsEmpty());
+ EXPECT_THAT(FirmwareHandlersBuilder().buildHandlerFromJson(j2), IsEmpty());
}
TEST(FirmwareJsonTest, InvalidPreparationType)
@@ -70,7 +69,7 @@
}]
)"_json;
- EXPECT_THAT(buildHandlerFromJson(j2), IsEmpty());
+ EXPECT_THAT(FirmwareHandlersBuilder().buildHandlerFromJson(j2), IsEmpty());
}
TEST(FirmwareJsonTest, InvalidVerificationType)
@@ -99,7 +98,7 @@
}]
)"_json;
- EXPECT_THAT(buildHandlerFromJson(j2), IsEmpty());
+ EXPECT_THAT(FirmwareHandlersBuilder().buildHandlerFromJson(j2), IsEmpty());
}
TEST(FirmwareJsonTest, InvalidUpdateType)
@@ -128,7 +127,7 @@
}]
)"_json;
- EXPECT_THAT(buildHandlerFromJson(j2), IsEmpty());
+ EXPECT_THAT(FirmwareHandlersBuilder().buildHandlerFromJson(j2), IsEmpty());
}
TEST(FirmwareJsonTest, MissingHandler)
@@ -153,7 +152,7 @@
}]
)"_json;
- EXPECT_THAT(buildHandlerFromJson(j2), IsEmpty());
+ EXPECT_THAT(FirmwareHandlersBuilder().buildHandlerFromJson(j2), IsEmpty());
}
TEST(FirmwareJsonTest, MissingActions)
@@ -168,7 +167,7 @@
}]
)"_json;
- EXPECT_THAT(buildHandlerFromJson(j2), IsEmpty());
+ EXPECT_THAT(FirmwareHandlersBuilder().buildHandlerFromJson(j2), IsEmpty());
}
TEST(FirmwareJsonTest, MissingActionPreparation)
@@ -193,7 +192,7 @@
}]
)"_json;
- EXPECT_THAT(buildHandlerFromJson(j2), IsEmpty());
+ EXPECT_THAT(FirmwareHandlersBuilder().buildHandlerFromJson(j2), IsEmpty());
}
TEST(FirmwareJsonTest, MissingActionVerification)
@@ -217,7 +216,7 @@
}]
)"_json;
- EXPECT_THAT(buildHandlerFromJson(j2), IsEmpty());
+ EXPECT_THAT(FirmwareHandlersBuilder().buildHandlerFromJson(j2), IsEmpty());
}
TEST(FirmwareJsonTest, MissingActionUpdate)
@@ -243,7 +242,7 @@
}]
)"_json;
- EXPECT_THAT(buildHandlerFromJson(j2), IsEmpty());
+ EXPECT_THAT(FirmwareHandlersBuilder().buildHandlerFromJson(j2), IsEmpty());
}
TEST(FirmwareJsonTest, TwoConfigsOneInvalidReturnsValid)
@@ -289,7 +288,7 @@
}]
)"_json;
- auto h = buildHandlerFromJson(j2);
+ auto h = FirmwareHandlersBuilder().buildHandlerFromJson(j2);
EXPECT_EQ(h[0].blobId, "/flash/image2");
EXPECT_EQ(h.size(), 1);
}
@@ -329,7 +328,7 @@
}]
)"_json;
- EXPECT_THAT(buildHandlerFromJson(j2), IsEmpty());
+ EXPECT_THAT(FirmwareHandlersBuilder().buildHandlerFromJson(j2), IsEmpty());
}
TEST(FirmwareJsonTest, VerifyMinimumBlobNameLength)
@@ -360,7 +359,7 @@
}]
)"_json;
- EXPECT_THAT(buildHandlerFromJson(j2), IsEmpty());
+ EXPECT_THAT(FirmwareHandlersBuilder().buildHandlerFromJson(j2), IsEmpty());
}
TEST(FirmwareJsonTest, VerifySystemdWithReboot)
@@ -389,7 +388,7 @@
}]
)"_json;
- auto h = buildHandlerFromJson(j2);
+ auto h = FirmwareHandlersBuilder().buildHandlerFromJson(j2);
EXPECT_EQ(h[0].blobId, "/flash/image");
EXPECT_FALSE(h[0].handler == nullptr);
EXPECT_FALSE(h[0].actions == nullptr);
@@ -447,7 +446,7 @@
}]
)"_json;
- auto h = buildHandlerFromJson(j2);
+ auto h = FirmwareHandlersBuilder().buildHandlerFromJson(j2);
EXPECT_EQ(h.size(), 2);
EXPECT_EQ(h[0].blobId, "/flash/image");
EXPECT_EQ(h[1].blobId, "/flash/bios");
@@ -480,7 +479,7 @@
}]
)"_json;
- auto h = buildHandlerFromJson(j2);
+ auto h = FirmwareHandlersBuilder().buildHandlerFromJson(j2);
EXPECT_EQ(h[0].blobId, "/flash/image");
EXPECT_FALSE(h[0].handler == nullptr);
EXPECT_FALSE(h[0].actions == nullptr);
@@ -523,7 +522,7 @@
}]
)"_json;
- auto h = buildHandlerFromJson(j2);
+ auto h = FirmwareHandlersBuilder().buildHandlerFromJson(j2);
EXPECT_EQ(h[0].blobId, "/flash/image");
EXPECT_FALSE(h[0].handler == nullptr);
EXPECT_FALSE(h[0].actions == nullptr);
@@ -567,7 +566,7 @@
}]
)"_json;
- auto h = buildHandlerFromJson(j2);
+ auto h = FirmwareHandlersBuilder().buildHandlerFromJson(j2);
EXPECT_EQ(h[0].blobId, "/flash/image");
EXPECT_FALSE(h[0].handler == nullptr);
EXPECT_FALSE(h[0].actions == nullptr);
@@ -607,7 +606,7 @@
}]
)"_json;
- auto h = buildHandlerFromJson(j2);
+ auto h = FirmwareHandlersBuilder().buildHandlerFromJson(j2);
EXPECT_EQ(h[0].blobId, "/flash/image");
EXPECT_FALSE(h[0].handler == nullptr);
EXPECT_FALSE(h[0].actions == nullptr);