Skip to content

Commit

Permalink
fix: add missing c-tor for nesting Variants (#467)
Browse files Browse the repository at this point in the history
This allows nesting variants, i.e. allows a variant to contain another variant as its value. Until now, there was no such possibility, since the default generated copy constructor would be invoked which would create a copy of source variant instead of embed the source variant as a value in the destination variant. The default generated copy constructor is kept, for it makes sense too, but a new tag-based overload is added for embedding the source variant into the destination variant.
  • Loading branch information
sangelovic authored Nov 20, 2024
1 parent 02ca721 commit a1419ee
Show file tree
Hide file tree
Showing 10 changed files with 67 additions and 7 deletions.
3 changes: 3 additions & 0 deletions include/sdbus-c++/TypeTraits.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,9 @@ namespace sdbus {
// Tag denoting a call where the reply shouldn't be waited for
struct dont_expect_reply_t { explicit dont_expect_reply_t() = default; };
inline constexpr dont_expect_reply_t dont_expect_reply{};
// Tag denoting that the variant shall embed the other variant as its value, instead of creating a copy
struct embed_variant_t { explicit embed_variant_t() = default; };
inline constexpr embed_variant_t embed_variant{};

// Helper for static assert
template <class... _T> constexpr bool always_false = false;
Expand Down
8 changes: 8 additions & 0 deletions include/sdbus-c++/Types.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,14 @@ namespace sdbus {
msg_.seal();
}

Variant(const Variant& value, embed_variant_t) : Variant()
{
msg_.openVariant<Variant>();
msg_ << value;
msg_.closeVariant();
msg_.seal();
}

template <typename _Struct>
explicit Variant(const as_dictionary<_Struct>& value) : Variant()
{
Expand Down
9 changes: 9 additions & 0 deletions tests/integrationtests/DBusPropertiesTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,12 @@ TYPED_TEST(SdbusTestObject, CanAccessAssociatedPropertySetMessageInPropertySetHa
ASSERT_THAT(this->m_adaptor->m_propertySetMsg, NotNull());
ASSERT_THAT(this->m_adaptor->m_propertySetSender, Not(IsEmpty()));
}

TYPED_TEST(SdbusTestObject, WritesAndReadsReadWriteVariantPropertySuccessfully)
{
sdbus::Variant newActionValue{5678};

this->m_proxy->actionVariant(newActionValue);

ASSERT_THAT(this->m_proxy->actionVariant().template get<int>(), Eq(5678));
}
21 changes: 14 additions & 7 deletions tests/integrationtests/DBusStandardInterfacesTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,10 @@ TYPED_TEST(SdbusTestObject, GetsAllPropertiesViaPropertiesInterface)
{
const auto properties = this->m_proxy->GetAll(INTERFACE_NAME);

ASSERT_THAT(properties, SizeIs(3));
ASSERT_THAT(properties, SizeIs(4));
EXPECT_THAT(properties.at(STATE_PROPERTY).template get<std::string>(), Eq(DEFAULT_STATE_VALUE));
EXPECT_THAT(properties.at(ACTION_PROPERTY).template get<uint32_t>(), Eq(DEFAULT_ACTION_VALUE));
EXPECT_THAT(properties.at(ACTION_VARIANT_PROPERTY).template get<sdbus::Variant>().template get<std::string>(), Eq(DEFAULT_ACTION_VARIANT_VALUE));
EXPECT_THAT(properties.at(BLOCKING_PROPERTY).template get<bool>(), Eq(DEFAULT_BLOCKING_VALUE));
}

Expand All @@ -161,9 +162,10 @@ TYPED_TEST(SdbusTestObject, GetsAllPropertiesAsynchronouslyViaPropertiesInterfac
});
const auto properties = future.get();

ASSERT_THAT(properties, SizeIs(3));
ASSERT_THAT(properties, SizeIs(4));
EXPECT_THAT(properties.at(STATE_PROPERTY).get<std::string>(), Eq(DEFAULT_STATE_VALUE));
EXPECT_THAT(properties.at(ACTION_PROPERTY).get<uint32_t>(), Eq(DEFAULT_ACTION_VALUE));
EXPECT_THAT(properties.at(ACTION_VARIANT_PROPERTY).template get<sdbus::Variant>().template get<std::string>(), Eq(DEFAULT_ACTION_VARIANT_VALUE));
EXPECT_THAT(properties.at(BLOCKING_PROPERTY).get<bool>(), Eq(DEFAULT_BLOCKING_VALUE));
}

Expand All @@ -173,9 +175,10 @@ TYPED_TEST(SdbusTestObject, GetsAllPropertiesAsynchronouslyViaPropertiesInterfac

auto properties = future.get();

ASSERT_THAT(properties, SizeIs(3));
ASSERT_THAT(properties, SizeIs(4));
EXPECT_THAT(properties.at(STATE_PROPERTY).template get<std::string>(), Eq(DEFAULT_STATE_VALUE));
EXPECT_THAT(properties.at(ACTION_PROPERTY).template get<uint32_t>(), Eq(DEFAULT_ACTION_VALUE));
EXPECT_THAT(properties.at(ACTION_VARIANT_PROPERTY).template get<sdbus::Variant>().template get<std::string>(), Eq(DEFAULT_ACTION_VARIANT_VALUE));
EXPECT_THAT(properties.at(BLOCKING_PROPERTY).template get<bool>(), Eq(DEFAULT_BLOCKING_VALUE));
}

Expand Down Expand Up @@ -252,17 +255,19 @@ TYPED_TEST(SdbusTestObject, EmitsInterfacesAddedSignalForSelectedObjectInterface
EXPECT_THAT(interfacesAndProperties.count(INTERFACE_NAME), Eq(1));
#if LIBSYSTEMD_VERSION<=244
// Up to sd-bus v244, all properties are added to the list, i.e. `state', `action', and `blocking' in this case.
EXPECT_THAT(interfacesAndProperties.at(INTERFACE_NAME), SizeIs(3));
EXPECT_THAT(interfacesAndProperties.at(INTERFACE_NAME), SizeIs(4));
EXPECT_TRUE(interfacesAndProperties.at(INTERFACE_NAME).count(STATE_PROPERTY));
EXPECT_TRUE(interfacesAndProperties.at(INTERFACE_NAME).count(ACTION_PROPERTY));
EXPECT_TRUE(interfacesAndProperties.at(INTERFACE_NAME).count(ACTION_VARIANT_PROPERTY));
EXPECT_TRUE(interfacesAndProperties.at(INTERFACE_NAME).count(BLOCKING_PROPERTY));
#else
// Since v245 sd-bus does not add to the InterfacesAdded signal message the values of properties marked only
// for invalidation on change, which makes the behavior consistent with the PropertiesChangedSignal.
// So in this specific instance, `action' property is no more added to the list.
EXPECT_THAT(interfacesAndProperties.at(INTERFACE_NAME), SizeIs(2));
EXPECT_THAT(interfacesAndProperties.at(INTERFACE_NAME), SizeIs(3));
EXPECT_TRUE(interfacesAndProperties.at(INTERFACE_NAME).count(STATE_PROPERTY));
EXPECT_TRUE(interfacesAndProperties.at(INTERFACE_NAME).count(BLOCKING_PROPERTY));
EXPECT_TRUE(interfacesAndProperties.at(INTERFACE_NAME).count(ACTION_VARIANT_PROPERTY));
#endif
signalReceived = true;
};
Expand All @@ -288,17 +293,19 @@ TYPED_TEST(SdbusTestObject, EmitsInterfacesAddedSignalForAllObjectInterfaces)
#endif
#if LIBSYSTEMD_VERSION<=244
// Up to sd-bus v244, all properties are added to the list, i.e. `state', `action', and `blocking' in this case.
EXPECT_THAT(interfacesAndProperties.at(INTERFACE_NAME), SizeIs(3));
EXPECT_THAT(interfacesAndProperties.at(INTERFACE_NAME), SizeIs(4));
EXPECT_TRUE(interfacesAndProperties.at(INTERFACE_NAME).count(STATE_PROPERTY));
EXPECT_TRUE(interfacesAndProperties.at(INTERFACE_NAME).count(ACTION_PROPERTY));
EXPECT_TRUE(interfacesAndProperties.at(INTERFACE_NAME).count(ACTION_VARIANT_PROPERTY));
EXPECT_TRUE(interfacesAndProperties.at(INTERFACE_NAME).count(BLOCKING_PROPERTY));
#else
// Since v245 sd-bus does not add to the InterfacesAdded signal message the values of properties marked only
// for invalidation on change, which makes the behavior consistent with the PropertiesChangedSignal.
// So in this specific instance, `action' property is no more added to the list.
EXPECT_THAT(interfacesAndProperties.at(INTERFACE_NAME), SizeIs(2));
EXPECT_THAT(interfacesAndProperties.at(INTERFACE_NAME), SizeIs(3));
EXPECT_TRUE(interfacesAndProperties.at(INTERFACE_NAME).count(STATE_PROPERTY));
EXPECT_TRUE(interfacesAndProperties.at(INTERFACE_NAME).count(BLOCKING_PROPERTY));
EXPECT_TRUE(interfacesAndProperties.at(INTERFACE_NAME).count(ACTION_VARIANT_PROPERTY));
#endif
signalReceived = true;
};
Expand Down
2 changes: 2 additions & 0 deletions tests/integrationtests/Defs.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ const ObjectPath OBJECT_PATH {"/org/sdbuscpp/integrationtests/ObjectA1"};
const ObjectPath OBJECT_PATH_2{"/org/sdbuscpp/integrationtests/ObjectB1"};
const PropertyName STATE_PROPERTY{"state"};
const PropertyName ACTION_PROPERTY{"action"};
const PropertyName ACTION_VARIANT_PROPERTY{"actionVariant"};
const PropertyName BLOCKING_PROPERTY{"blocking"};
const std::string DIRECT_CONNECTION_SOCKET_PATH{std::filesystem::temp_directory_path() / "sdbus-cpp-direct-connection-test"};

Expand All @@ -58,6 +59,7 @@ const int UNIX_FD_VALUE = 0;

const std::string DEFAULT_STATE_VALUE{"default-state-value"};
const uint32_t DEFAULT_ACTION_VALUE{999};
const std::string DEFAULT_ACTION_VARIANT_VALUE{"ahoj"};
const bool DEFAULT_BLOCKING_VALUE{true};

constexpr const double DOUBLE_VALUE{3.24L};
Expand Down
10 changes: 10 additions & 0 deletions tests/integrationtests/TestAdaptor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,16 @@ void TestAdaptor::action(const uint32_t& value)
m_action = value;
}

sdbus::Variant TestAdaptor::actionVariant()
{
return m_actionVariant;
}

void TestAdaptor::actionVariant(const sdbus::Variant& value)
{
m_actionVariant = value;
}

bool TestAdaptor::blocking()
{
return m_blocking;
Expand Down
5 changes: 5 additions & 0 deletions tests/integrationtests/TestAdaptor.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ class TestAdaptor final : public sdbus::AdaptorInterfaces< org::sdbuscpp::integr

uint32_t action() override;
void action(const uint32_t& value) override;
sdbus::Variant actionVariant() override;
void actionVariant(const sdbus::Variant& value) override;
bool blocking() override;
void blocking(const bool& value) override;
std::string state() override;
Expand All @@ -101,6 +103,7 @@ class TestAdaptor final : public sdbus::AdaptorInterfaces< org::sdbuscpp::integr
const std::string m_state{DEFAULT_STATE_VALUE};
uint32_t m_action{DEFAULT_ACTION_VALUE};
bool m_blocking{DEFAULT_BLOCKING_VALUE};
sdbus::Variant m_actionVariant{"ahoj"};

public: // for tests
// For dont-expect-reply method call verifications
Expand Down Expand Up @@ -152,6 +155,8 @@ class DummyTestAdaptor final : public sdbus::AdaptorInterfaces< org::sdbuscpp::i

uint32_t action() override { return {}; }
void action(const uint32_t&) override {}
sdbus::Variant actionVariant() override { return {}; }
void actionVariant(const sdbus::Variant&) override {}
bool blocking() override { return {}; }
void blocking(const bool&) override {}
std::string state() override { return {}; }
Expand Down
3 changes: 3 additions & 0 deletions tests/integrationtests/integrationtests-adaptor.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ class integrationtests_adaptor
, sdbus::registerSignal("signalWithMap").withParameters<std::map<int32_t, std::string>>("aMap")
, sdbus::registerSignal("signalWithVariant").withParameters<sdbus::Variant>("aVariant")
, sdbus::registerProperty("action").withGetter([this](){ return this->action(); }).withSetter([this](const uint32_t& value){ this->action(value); }).withUpdateBehavior(sdbus::Flags::EMITS_INVALIDATION_SIGNAL)
, sdbus::registerProperty("actionVariant").withGetter([this](){ return this->actionVariant(); }).withSetter([this](const sdbus::Variant& value){ this->actionVariant(value); }).withUpdateBehavior(sdbus::Flags::EMITS_NO_SIGNAL)
, sdbus::registerProperty("blocking").withGetter([this](){ return this->blocking(); }).withSetter([this](const bool& value){ this->blocking(value); })
, sdbus::registerProperty("state").withGetter([this](){ return this->state(); }).markAsDeprecated().withUpdateBehavior(sdbus::Flags::CONST_PROPERTY_VALUE)
).forInterface(INTERFACE_NAME);
Expand Down Expand Up @@ -113,7 +114,9 @@ class integrationtests_adaptor

private:
virtual uint32_t action() = 0;
virtual sdbus::Variant actionVariant() = 0;
virtual void action(const uint32_t& value) = 0;
virtual void actionVariant(const sdbus::Variant& value) = 0;
virtual bool blocking() = 0;
virtual void blocking(const bool& value) = 0;
virtual std::string state() = 0;
Expand Down
10 changes: 10 additions & 0 deletions tests/integrationtests/integrationtests-proxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,16 @@ class integrationtests_proxy
m_proxy.setProperty("action").onInterface(INTERFACE_NAME).toValue(value);
}

sdbus::Variant actionVariant()
{
return m_proxy.getProperty("actionVariant").onInterface(INTERFACE_NAME).get<sdbus::Variant>();
}

void actionVariant(const sdbus::Variant& value)
{
m_proxy.setProperty("actionVariant").onInterface(INTERFACE_NAME).toValue({value, sdbus::embed_variant});
}

bool blocking()
{
return m_proxy.getProperty("blocking").onInterface(INTERFACE_NAME).get<bool>();
Expand Down
3 changes: 3 additions & 0 deletions tools/xml2cpp-codegen/ProxyGenerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,9 @@ std::tuple<std::string, std::string> ProxyGenerator::processProperties(const Nod

if (propertyAccess == "readwrite" || propertyAccess == "write")
{
if (propertySignature == "v")
propertyArg = "{" + propertyArg + ", sdbus::embed_variant}";

const std::string realRetType = (asyncSet ? (futureSet ? "std::future<void>" : "sdbus::PendingAsyncCall") : "void");

propertySS << tab << realRetType << " " << propertyNameSafe << "(" << propertyTypeArg << ")" << endl
Expand Down

0 comments on commit a1419ee

Please sign in to comment.