manager: refactor interface templates
Many of the templates in manager.hpp are _almost_ testable. Enable
testing by porting to a new header - interface_ops.hpp - to be activated
later.
Enhancements include:
Dropped 'PropertiesVariant' wrapper template for reduced comprehensional
complexity.
More intuitive HasProperties implementation. HasProperties is a type
traits template that simply checks for the presence of a nested
PropertiesVariant typename. These are provided by sdbusplus generated
server bindings for dbus interfaces that have properties.
Standalone templates for make, assign, serialize and deserialize
interface. There was no good reason to couple these, and the
resulting types and method names are more intuitive.
Templated serialize/deserialize operations so the dependency on
cereal can be abstracted away.
Change-Id: Ia449628eeaa9732bbbc51b0609bd449f43ebcb7c
Signed-off-by: Brad Bishop <bradleyb@fuzziesquirrel.com>
diff --git a/interface_ops.hpp b/interface_ops.hpp
new file mode 100644
index 0000000..db429b8
--- /dev/null
+++ b/interface_ops.hpp
@@ -0,0 +1,157 @@
+#pragma once
+
+#include "types.hpp"
+#include "utils.hpp"
+
+#include <any>
+#include <map>
+#include <memory>
+#include <string>
+#include <type_traits>
+#include <utility>
+
+namespace sdbusplus
+{
+namespace bus
+{
+class bus;
+}
+} // namespace sdbusplus
+
+namespace phosphor
+{
+namespace inventory
+{
+namespace manager
+{
+
+template <typename T>
+struct HasProperties
+{
+ private:
+ using yes = char;
+ struct no
+ {
+ char array[2];
+ };
+
+ template <typename U>
+ static constexpr yes test(typename U::PropertiesVariant*);
+ template <typename U>
+ static constexpr no test(...);
+
+ public:
+ static constexpr auto value = sizeof(test<T>(0)) == sizeof(yes);
+};
+
+template <typename T, typename Enable = void>
+struct MakeInterface
+{
+ static std::any op(sdbusplus::bus::bus& bus, const char* path,
+ const Interface&)
+ {
+ return std::any(std::make_shared<T>(bus, path));
+ }
+};
+
+template <typename T>
+struct MakeInterface<T, std::enable_if_t<HasProperties<T>::value>>
+{
+ static std::any op(sdbusplus::bus::bus& bus, const char* path,
+ const Interface& props)
+ {
+ using InterfaceVariant =
+ std::map<std::string, typename T::PropertiesVariant>;
+
+ InterfaceVariant v;
+ for (const auto& p : props)
+ {
+ v.emplace(p.first,
+ convertVariant<typename T::PropertiesVariant>(p.second));
+ }
+
+ return std::any(std::make_shared<T>(bus, path, v));
+ }
+};
+
+template <typename T, typename Enable = void>
+struct AssignInterface
+{
+ static void op(const Interface&, std::any&)
+ {
+ }
+};
+
+template <typename T>
+struct AssignInterface<T, std::enable_if_t<HasProperties<T>::value>>
+{
+ static void op(const Interface& props, std::any& holder)
+ {
+ auto& iface = *std::any_cast<std::shared_ptr<T>&>(holder);
+ for (const auto& p : props)
+ {
+ iface.setPropertyByName(
+ p.first,
+ convertVariant<typename T::PropertiesVariant>(p.second));
+ }
+ }
+};
+
+template <typename T, typename Ops, typename Enable = void>
+struct SerializeInterface
+{
+ static void op(const std::string& path, const std::string& iface,
+ const std::any&)
+ {
+ Ops::serialize(path, iface);
+ }
+};
+
+template <typename T, typename Ops>
+struct SerializeInterface<T, Ops, std::enable_if_t<HasProperties<T>::value>>
+{
+ static void op(const std::string& path, const std::string& iface,
+ const std::any& holder)
+ {
+ const auto& object = *std::any_cast<const std::shared_ptr<T>&>(holder);
+ Ops::serialize(path, iface, object);
+ }
+};
+
+template <typename T, typename Ops, typename Enable = void>
+struct DeserializeInterface
+{
+ static void op(const std::string& path, const std::string& iface, std::any&)
+ {
+ Ops::deserialize(path, iface);
+ }
+};
+
+template <typename T, typename Ops>
+struct DeserializeInterface<T, Ops, std::enable_if_t<HasProperties<T>::value>>
+{
+ static void op(const std::string& path, const std::string& iface,
+ std::any& holder)
+ {
+ auto& object = *std::any_cast<std::shared_ptr<T>&>(holder);
+ Ops::deserialize(path, iface, object);
+ }
+};
+
+struct DummyInterface
+{
+};
+using MakeInterfaceType =
+ std::add_pointer_t<decltype(MakeInterface<DummyInterface>::op)>;
+using AssignInterfaceType =
+ std::add_pointer_t<decltype(AssignInterface<DummyInterface>::op)>;
+template <typename Ops>
+using SerializeInterfaceType =
+ std::add_pointer_t<decltype(SerializeInterface<DummyInterface, Ops>::op)>;
+template <typename Ops>
+using DeserializeInterfaceType =
+ std::add_pointer_t<decltype(DeserializeInterface<DummyInterface, Ops>::op)>;
+
+} // namespace manager
+} // namespace inventory
+} // namespace phosphor
diff --git a/test/Makefile.am b/test/Makefile.am
index 777f11b..f213b4f 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -27,6 +27,12 @@
serialize_test_LDFLAGS = ${OESDK_TESTCASE_FLAGS}
check_PROGRAMS += serialize-test
+interface_ops_test_SOURCES = interface_ops_test.cpp
+interface_ops_test_CFLAGS = ${GTEST_CFLAGS} ${GMOCK_CFLAGS}
+interface_ops_test_LDADD = ${GTEST_LIBS} ${GMOCK_LIBS}
+interface_ops_test_LDFLAGS = ${OESDK_TESTCASE_FLAGS}
+check_PROGRAMS += interface-ops-test
+
extra_yamldir=$(top_srcdir)/example/extra_interfaces.d
phosphor_inventory_test_SOURCES = test.cpp
diff --git a/test/interface_ops_test.cpp b/test/interface_ops_test.cpp
new file mode 100644
index 0000000..68c5e95
--- /dev/null
+++ b/test/interface_ops_test.cpp
@@ -0,0 +1,350 @@
+#include "../interface_ops.hpp"
+
+#include <sdbusplus/test/sdbus_mock.hpp>
+
+#include <gtest/gtest.h>
+
+using namespace phosphor::inventory::manager;
+using namespace testing;
+using namespace std::string_literals;
+
+struct MockInterface;
+struct DummyInterfaceWithProperties;
+
+MockInterface* g_currentMock = nullptr;
+
+using FakeVariantType = int;
+using InterfaceVariant = std::map<std::string, FakeVariantType>;
+
+struct MockInterface
+{
+ MockInterface()
+ {
+ g_currentMock = this;
+ }
+ ~MockInterface()
+ {
+ g_currentMock = nullptr;
+ }
+ MockInterface(const MockInterface&) = delete;
+ MockInterface& operator=(const MockInterface&) = delete;
+ // Not supporting move semantics simply because they aren't needed.
+ MockInterface(MockInterface&&) = delete;
+ MockInterface& operator=(MockInterface&&) = delete;
+
+ // We'll be getting calls proxyed through other objects.
+ MOCK_METHOD2(constructWithProperties,
+ void(const char*, const InterfaceVariant& i));
+ MOCK_METHOD1(constructWithoutProperties, void(const char*));
+ MOCK_METHOD2(setPropertyByName, void(std::string, FakeVariantType));
+
+ MOCK_METHOD2(serializeTwoArgs,
+ void(const std::string&, const std::string&));
+ MOCK_METHOD3(serializeThreeArgs,
+ void(const std::string&, const std::string&,
+ const DummyInterfaceWithProperties&));
+
+ MOCK_METHOD0(deserializeNoop, void());
+ MOCK_METHOD3(deserializeThreeArgs,
+ void(const std::string&, const std::string&,
+ DummyInterfaceWithProperties&));
+};
+
+struct DummyInterfaceWithoutProperties
+{
+ DummyInterfaceWithoutProperties(sdbusplus::bus::bus&, const char* name)
+ {
+ g_currentMock->constructWithoutProperties(name);
+ }
+};
+
+struct DummyInterfaceWithProperties
+{
+ using PropertiesVariant = FakeVariantType;
+
+ DummyInterfaceWithProperties(sdbusplus::bus::bus&, const char* name,
+ const InterfaceVariant& i)
+ {
+ g_currentMock->constructWithProperties(name, i);
+ }
+
+ void setPropertyByName(std::string name, PropertiesVariant val)
+ {
+ g_currentMock->setPropertyByName(name, val);
+ }
+};
+
+struct SerialForwarder
+{
+ static void serialize(const std::string& path, const std::string& iface)
+ {
+ g_currentMock->serializeTwoArgs(path, iface);
+ }
+
+ static void serialize(const std::string& path, const std::string& iface,
+ const DummyInterfaceWithProperties& obj)
+ {
+ g_currentMock->serializeThreeArgs(path, iface, obj);
+ }
+
+ static void deserialize(const std::string& path, const std::string& iface)
+ {
+ g_currentMock->deserializeNoop();
+ }
+
+ static void deserialize(const std::string& path, const std::string& iface,
+ DummyInterfaceWithProperties& obj)
+ {
+ g_currentMock->deserializeThreeArgs(path, iface, obj);
+ }
+};
+
+TEST(InterfaceOpsTest, TestHasPropertiesNoProperties)
+{
+ EXPECT_FALSE(HasProperties<DummyInterfaceWithoutProperties>::value);
+}
+
+TEST(InterfaceOpsTest, TestHasPropertiesHasProperties)
+{
+ EXPECT_TRUE(HasProperties<DummyInterfaceWithProperties>::value);
+}
+
+TEST(InterfaceOpsTest, TestMakePropertylessInterfaceWithoutArguments)
+{
+ MockInterface mock;
+ Interface i;
+ sdbusplus::SdBusMock interface;
+
+ EXPECT_CALL(mock, constructWithoutProperties("foo")).Times(1);
+ EXPECT_CALL(mock, constructWithProperties(_, _)).Times(0);
+
+ auto b = sdbusplus::get_mocked_new(&interface);
+ auto r = MakeInterface<DummyInterfaceWithoutProperties>::op(b, "foo", i);
+
+ EXPECT_NO_THROW(
+ std::any_cast<std::shared_ptr<DummyInterfaceWithoutProperties>>(r));
+}
+
+TEST(InterfaceOpsTest, TestMakePropertylessInterfaceWithOneArgument)
+{
+ MockInterface mock;
+ Interface i{{"foo"s, static_cast<int64_t>(1ll)}};
+ sdbusplus::SdBusMock interface;
+
+ EXPECT_CALL(mock, constructWithoutProperties("foo")).Times(1);
+ EXPECT_CALL(mock, constructWithProperties(_, _)).Times(0);
+
+ auto b = sdbusplus::get_mocked_new(&interface);
+ auto r = MakeInterface<DummyInterfaceWithoutProperties>::op(b, "foo", i);
+
+ EXPECT_NO_THROW(
+ std::any_cast<std::shared_ptr<DummyInterfaceWithoutProperties>>(r));
+}
+
+TEST(InterfaceOpsTest, TestMakeInterfaceWithWithoutArguments)
+{
+ MockInterface mock;
+ Interface i;
+ sdbusplus::SdBusMock interface;
+
+ EXPECT_CALL(mock, constructWithoutProperties(_)).Times(0);
+ EXPECT_CALL(mock, constructWithProperties("bar", _)).Times(1);
+
+ auto b = sdbusplus::get_mocked_new(&interface);
+ auto r = MakeInterface<DummyInterfaceWithProperties>::op(b, "bar", i);
+
+ EXPECT_NO_THROW(
+ std::any_cast<std::shared_ptr<DummyInterfaceWithProperties>>(r));
+}
+
+TEST(InterfaceOpsTest, TestMakeInterfaceWithOneArgument)
+{
+ MockInterface mock;
+ Interface i{{"foo"s, static_cast<int64_t>(1ll)}};
+ sdbusplus::SdBusMock interface;
+
+ EXPECT_CALL(mock, constructWithoutProperties(_)).Times(0);
+ EXPECT_CALL(mock, constructWithProperties("foo", _)).Times(1);
+
+ auto b = sdbusplus::get_mocked_new(&interface);
+ auto r = MakeInterface<DummyInterfaceWithProperties>::op(b, "foo", i);
+
+ EXPECT_NO_THROW(
+ std::any_cast<std::shared_ptr<DummyInterfaceWithProperties>>(r));
+}
+
+TEST(InterfaceOpsTest, TestAssignPropertylessInterfaceWithoutArguments)
+{
+ MockInterface mock;
+ Interface i;
+ sdbusplus::SdBusMock interface;
+
+ EXPECT_CALL(mock, setPropertyByName(_, _)).Times(0);
+
+ auto b = sdbusplus::get_mocked_new(&interface);
+ auto r = MakeInterface<DummyInterfaceWithoutProperties>::op(b, "foo", i);
+
+ AssignInterface<DummyInterfaceWithoutProperties>::op(i, r);
+}
+
+TEST(InterfaceOpsTest, TestAssignPropertylessInterfaceWithOneArgument)
+{
+ MockInterface mock;
+ Interface i{{"foo"s, static_cast<int64_t>(1ll)}};
+ sdbusplus::SdBusMock interface;
+
+ EXPECT_CALL(mock, setPropertyByName(_, _)).Times(0);
+
+ auto b = sdbusplus::get_mocked_new(&interface);
+ auto r = MakeInterface<DummyInterfaceWithoutProperties>::op(b, "foo", i);
+
+ AssignInterface<DummyInterfaceWithoutProperties>::op(i, r);
+}
+
+TEST(InterfaceOpsTest, TestAssignInterfaceWithoutArguments)
+{
+ MockInterface mock;
+ Interface i;
+ sdbusplus::SdBusMock interface;
+
+ EXPECT_CALL(mock, setPropertyByName(_, _)).Times(0);
+
+ auto b = sdbusplus::get_mocked_new(&interface);
+ auto r = MakeInterface<DummyInterfaceWithProperties>::op(b, "foo", i);
+
+ AssignInterface<DummyInterfaceWithProperties>::op(i, r);
+}
+
+TEST(InterfaceOpsTest, TestAssignInterfaceWithOneArgument)
+{
+ MockInterface mock;
+ Interface i{{"foo"s, static_cast<int64_t>(1ll)}};
+ sdbusplus::SdBusMock interface;
+
+ EXPECT_CALL(mock, setPropertyByName("foo"s, 1ll)).Times(1);
+
+ auto b = sdbusplus::get_mocked_new(&interface);
+ auto r = MakeInterface<DummyInterfaceWithProperties>::op(b, "bar", i);
+
+ AssignInterface<DummyInterfaceWithProperties>::op(i, r);
+}
+
+TEST(InterfaceOpsTest, TestSerializePropertylessInterfaceWithoutArguments)
+{
+ MockInterface mock;
+ Interface i;
+ sdbusplus::SdBusMock interface;
+
+ auto b = sdbusplus::get_mocked_new(&interface);
+ auto r = MakeInterface<DummyInterfaceWithoutProperties>::op(b, "foo", i);
+
+ EXPECT_CALL(mock, serializeTwoArgs("/foo"s, "bar"s)).Times(1);
+
+ SerializeInterface<DummyInterfaceWithoutProperties, SerialForwarder>::op(
+ "/foo"s, "bar"s, r);
+}
+
+TEST(InterfaceOpsTest, TestSerializePropertylessInterfaceWithOneArgument)
+{
+ MockInterface mock;
+ Interface i{{"foo"s, static_cast<int64_t>(1ll)}};
+ sdbusplus::SdBusMock interface;
+
+ auto b = sdbusplus::get_mocked_new(&interface);
+ auto r = MakeInterface<DummyInterfaceWithoutProperties>::op(b, "foo", i);
+
+ EXPECT_CALL(mock, serializeTwoArgs("/foo"s, "bar"s)).Times(1);
+
+ SerializeInterface<DummyInterfaceWithoutProperties, SerialForwarder>::op(
+ "/foo"s, "bar"s, r);
+}
+
+TEST(InterfaceOpsTest, TestSerializeInterfaceWithNoArguments)
+{
+ MockInterface mock;
+ Interface i;
+ sdbusplus::SdBusMock interface;
+
+ auto b = sdbusplus::get_mocked_new(&interface);
+ auto r = MakeInterface<DummyInterfaceWithProperties>::op(b, "foo", i);
+
+ EXPECT_CALL(mock, serializeThreeArgs("/foo"s, "bar"s, _)).Times(1);
+
+ SerializeInterface<DummyInterfaceWithProperties, SerialForwarder>::op(
+ "/foo"s, "bar"s, r);
+}
+
+TEST(InterfaceOpsTest, TestSerializeInterfaceWithOneArgument)
+{
+ MockInterface mock;
+ Interface i{{"foo"s, static_cast<int64_t>(1ll)}};
+ sdbusplus::SdBusMock interface;
+
+ auto b = sdbusplus::get_mocked_new(&interface);
+ auto r = MakeInterface<DummyInterfaceWithProperties>::op(b, "foo", i);
+
+ EXPECT_CALL(mock, serializeThreeArgs("/foo"s, "bar"s, _)).Times(1);
+
+ SerializeInterface<DummyInterfaceWithProperties, SerialForwarder>::op(
+ "/foo"s, "bar"s, r);
+}
+
+TEST(InterfaceOpsTest, TestDeserializePropertylessInterfaceWithoutArguments)
+{
+ MockInterface mock;
+ Interface i;
+ sdbusplus::SdBusMock interface;
+
+ auto b = sdbusplus::get_mocked_new(&interface);
+ auto r = MakeInterface<DummyInterfaceWithoutProperties>::op(b, "foo", i);
+
+ EXPECT_CALL(mock, deserializeNoop()).Times(1);
+
+ DeserializeInterface<DummyInterfaceWithoutProperties, SerialForwarder>::op(
+ "/foo"s, "bar"s, r);
+}
+
+TEST(InterfaceOpsTest, TestDeserializePropertylessInterfaceWithOneArgument)
+{
+ MockInterface mock;
+ Interface i{{"foo"s, static_cast<int64_t>(1ll)}};
+ sdbusplus::SdBusMock interface;
+
+ auto b = sdbusplus::get_mocked_new(&interface);
+ auto r = MakeInterface<DummyInterfaceWithoutProperties>::op(b, "foo", i);
+
+ EXPECT_CALL(mock, deserializeNoop()).Times(1);
+
+ DeserializeInterface<DummyInterfaceWithoutProperties, SerialForwarder>::op(
+ "/foo"s, "bar"s, r);
+}
+
+TEST(InterfaceOpsTest, TestDeserializeInterfaceWithNoArguments)
+{
+ MockInterface mock;
+ Interface i;
+ sdbusplus::SdBusMock interface;
+
+ auto b = sdbusplus::get_mocked_new(&interface);
+ auto r = MakeInterface<DummyInterfaceWithProperties>::op(b, "foo", i);
+
+ EXPECT_CALL(mock, deserializeThreeArgs("/foo"s, "bar"s, _)).Times(1);
+
+ DeserializeInterface<DummyInterfaceWithProperties, SerialForwarder>::op(
+ "/foo"s, "bar"s, r);
+}
+
+TEST(InterfaceOpsTest, TestDeserializeInterfaceWithOneArgument)
+{
+ MockInterface mock;
+ Interface i{{"foo"s, static_cast<int64_t>(1ll)}};
+ sdbusplus::SdBusMock interface;
+
+ auto b = sdbusplus::get_mocked_new(&interface);
+ auto r = MakeInterface<DummyInterfaceWithProperties>::op(b, "foo", i);
+
+ EXPECT_CALL(mock, deserializeThreeArgs("/foo"s, "bar"s, _)).Times(1);
+
+ DeserializeInterface<DummyInterfaceWithProperties, SerialForwarder>::op(
+ "/foo"s, "bar"s, r);
+}