From 9cc42abf0e7ca1d4cc83489afb4b467cb13ee72c Mon Sep 17 00:00:00 2001 From: Brad Bishop Date: Wed, 12 Dec 2018 16:36:51 -0500 Subject: 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 --- interface_ops.hpp | 157 ++++++++++++++++++++ test/Makefile.am | 6 + test/interface_ops_test.cpp | 350 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 513 insertions(+) create mode 100644 interface_ops.hpp create mode 100644 test/interface_ops_test.cpp 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 +#include +#include +#include +#include +#include + +namespace sdbusplus +{ +namespace bus +{ +class bus; +} +} // namespace sdbusplus + +namespace phosphor +{ +namespace inventory +{ +namespace manager +{ + +template +struct HasProperties +{ + private: + using yes = char; + struct no + { + char array[2]; + }; + + template + static constexpr yes test(typename U::PropertiesVariant*); + template + static constexpr no test(...); + + public: + static constexpr auto value = sizeof(test(0)) == sizeof(yes); +}; + +template +struct MakeInterface +{ + static std::any op(sdbusplus::bus::bus& bus, const char* path, + const Interface&) + { + return std::any(std::make_shared(bus, path)); + } +}; + +template +struct MakeInterface::value>> +{ + static std::any op(sdbusplus::bus::bus& bus, const char* path, + const Interface& props) + { + using InterfaceVariant = + std::map; + + InterfaceVariant v; + for (const auto& p : props) + { + v.emplace(p.first, + convertVariant(p.second)); + } + + return std::any(std::make_shared(bus, path, v)); + } +}; + +template +struct AssignInterface +{ + static void op(const Interface&, std::any&) + { + } +}; + +template +struct AssignInterface::value>> +{ + static void op(const Interface& props, std::any& holder) + { + auto& iface = *std::any_cast&>(holder); + for (const auto& p : props) + { + iface.setPropertyByName( + p.first, + convertVariant(p.second)); + } + } +}; + +template +struct SerializeInterface +{ + static void op(const std::string& path, const std::string& iface, + const std::any&) + { + Ops::serialize(path, iface); + } +}; + +template +struct SerializeInterface::value>> +{ + static void op(const std::string& path, const std::string& iface, + const std::any& holder) + { + const auto& object = *std::any_cast&>(holder); + Ops::serialize(path, iface, object); + } +}; + +template +struct DeserializeInterface +{ + static void op(const std::string& path, const std::string& iface, std::any&) + { + Ops::deserialize(path, iface); + } +}; + +template +struct DeserializeInterface::value>> +{ + static void op(const std::string& path, const std::string& iface, + std::any& holder) + { + auto& object = *std::any_cast&>(holder); + Ops::deserialize(path, iface, object); + } +}; + +struct DummyInterface +{ +}; +using MakeInterfaceType = + std::add_pointer_t::op)>; +using AssignInterfaceType = + std::add_pointer_t::op)>; +template +using SerializeInterfaceType = + std::add_pointer_t::op)>; +template +using DeserializeInterfaceType = + std::add_pointer_t::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_LDADD = ${GTEST_LIBS} ${GMOCK_LIBS} 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 + +#include + +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; + +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::value); +} + +TEST(InterfaceOpsTest, TestHasPropertiesHasProperties) +{ + EXPECT_TRUE(HasProperties::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::op(b, "foo", i); + + EXPECT_NO_THROW( + std::any_cast>(r)); +} + +TEST(InterfaceOpsTest, TestMakePropertylessInterfaceWithOneArgument) +{ + MockInterface mock; + Interface i{{"foo"s, static_cast(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::op(b, "foo", i); + + EXPECT_NO_THROW( + std::any_cast>(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::op(b, "bar", i); + + EXPECT_NO_THROW( + std::any_cast>(r)); +} + +TEST(InterfaceOpsTest, TestMakeInterfaceWithOneArgument) +{ + MockInterface mock; + Interface i{{"foo"s, static_cast(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::op(b, "foo", i); + + EXPECT_NO_THROW( + std::any_cast>(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::op(b, "foo", i); + + AssignInterface::op(i, r); +} + +TEST(InterfaceOpsTest, TestAssignPropertylessInterfaceWithOneArgument) +{ + MockInterface mock; + Interface i{{"foo"s, static_cast(1ll)}}; + sdbusplus::SdBusMock interface; + + EXPECT_CALL(mock, setPropertyByName(_, _)).Times(0); + + auto b = sdbusplus::get_mocked_new(&interface); + auto r = MakeInterface::op(b, "foo", i); + + AssignInterface::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::op(b, "foo", i); + + AssignInterface::op(i, r); +} + +TEST(InterfaceOpsTest, TestAssignInterfaceWithOneArgument) +{ + MockInterface mock; + Interface i{{"foo"s, static_cast(1ll)}}; + sdbusplus::SdBusMock interface; + + EXPECT_CALL(mock, setPropertyByName("foo"s, 1ll)).Times(1); + + auto b = sdbusplus::get_mocked_new(&interface); + auto r = MakeInterface::op(b, "bar", i); + + AssignInterface::op(i, r); +} + +TEST(InterfaceOpsTest, TestSerializePropertylessInterfaceWithoutArguments) +{ + MockInterface mock; + Interface i; + sdbusplus::SdBusMock interface; + + auto b = sdbusplus::get_mocked_new(&interface); + auto r = MakeInterface::op(b, "foo", i); + + EXPECT_CALL(mock, serializeTwoArgs("/foo"s, "bar"s)).Times(1); + + SerializeInterface::op( + "/foo"s, "bar"s, r); +} + +TEST(InterfaceOpsTest, TestSerializePropertylessInterfaceWithOneArgument) +{ + MockInterface mock; + Interface i{{"foo"s, static_cast(1ll)}}; + sdbusplus::SdBusMock interface; + + auto b = sdbusplus::get_mocked_new(&interface); + auto r = MakeInterface::op(b, "foo", i); + + EXPECT_CALL(mock, serializeTwoArgs("/foo"s, "bar"s)).Times(1); + + SerializeInterface::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::op(b, "foo", i); + + EXPECT_CALL(mock, serializeThreeArgs("/foo"s, "bar"s, _)).Times(1); + + SerializeInterface::op( + "/foo"s, "bar"s, r); +} + +TEST(InterfaceOpsTest, TestSerializeInterfaceWithOneArgument) +{ + MockInterface mock; + Interface i{{"foo"s, static_cast(1ll)}}; + sdbusplus::SdBusMock interface; + + auto b = sdbusplus::get_mocked_new(&interface); + auto r = MakeInterface::op(b, "foo", i); + + EXPECT_CALL(mock, serializeThreeArgs("/foo"s, "bar"s, _)).Times(1); + + SerializeInterface::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::op(b, "foo", i); + + EXPECT_CALL(mock, deserializeNoop()).Times(1); + + DeserializeInterface::op( + "/foo"s, "bar"s, r); +} + +TEST(InterfaceOpsTest, TestDeserializePropertylessInterfaceWithOneArgument) +{ + MockInterface mock; + Interface i{{"foo"s, static_cast(1ll)}}; + sdbusplus::SdBusMock interface; + + auto b = sdbusplus::get_mocked_new(&interface); + auto r = MakeInterface::op(b, "foo", i); + + EXPECT_CALL(mock, deserializeNoop()).Times(1); + + DeserializeInterface::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::op(b, "foo", i); + + EXPECT_CALL(mock, deserializeThreeArgs("/foo"s, "bar"s, _)).Times(1); + + DeserializeInterface::op( + "/foo"s, "bar"s, r); +} + +TEST(InterfaceOpsTest, TestDeserializeInterfaceWithOneArgument) +{ + MockInterface mock; + Interface i{{"foo"s, static_cast(1ll)}}; + sdbusplus::SdBusMock interface; + + auto b = sdbusplus::get_mocked_new(&interface); + auto r = MakeInterface::op(b, "foo", i); + + EXPECT_CALL(mock, deserializeThreeArgs("/foo"s, "bar"s, _)).Times(1); + + DeserializeInterface::op( + "/foo"s, "bar"s, r); +} -- cgit v1.2.1