diff --git a/docs/using-sdbus-c++.md b/docs/using-sdbus-c++.md index cfaa43c..7a3528d 100644 --- a/docs/using-sdbus-c++.md +++ b/docs/using-sdbus-c++.md @@ -132,11 +132,15 @@ The following diagram illustrates the major entities in sdbus-c++. * invoking remote methods of the corresponding object, in both synchronous and asynchronous way, * registering handlers for signals, -`Message` class represents a message, which is the fundamental DBus concept. There are three distinctive types of message that derive from the `Message` class: +`Message` class represents a message, which is the fundamental DBus concept. There are three distinctive types of message that are derived from the `Message` class: * `MethodCall` (with serialized parameters), + * `AsyncMethodCall` (with serialized parameters), * `MethodReply` (with serialized return values), - * or a `Signal` (with serialized parameters). + * `Signal` (with serialized parameters), + * `PropertySetCall` (with serialized parameter value to be set) + * `PropertyGetReply` (where property value shall be stored) + * `PlainMessage` (for internal purposes). ### Thread safety in sdbus-c++ diff --git a/include/sdbus-c++/ConvenienceApiClasses.inl b/include/sdbus-c++/ConvenienceApiClasses.inl index ee3c0b7..833f3c3 100644 --- a/include/sdbus-c++/ConvenienceApiClasses.inl +++ b/include/sdbus-c++/ConvenienceApiClasses.inl @@ -253,10 +253,10 @@ namespace sdbus { if (propertySignature_.empty()) propertySignature_ = signature_of_function_output_arguments<_Function>::str(); - getter_ = [callback = std::forward<_Function>(callback)](Message& msg) + getter_ = [callback = std::forward<_Function>(callback)](PropertyGetReply& reply) { - // Get the propety value and serialize it into the message - msg << callback(); + // Get the propety value and serialize it into the pre-constructed reply message + reply << callback(); }; return *this; @@ -271,14 +271,14 @@ namespace sdbus { if (propertySignature_.empty()) propertySignature_ = signature_of_function_input_arguments<_Function>::str(); - setter_ = [callback = std::forward<_Function>(callback)](Message& msg) + setter_ = [callback = std::forward<_Function>(callback)](PropertySetCall& call) { // Default-construct property value using property_type = function_argument_t<_Function, 0>; std::decay_t property; - // Deserialize property value from the message - msg >> property; + // Deserialize property value from the incoming call message + call >> property; // Invoke setter with the value callback(property); diff --git a/include/sdbus-c++/Message.h b/include/sdbus-c++/Message.h index fc834d3..7ff447e 100644 --- a/include/sdbus-c++/Message.h +++ b/include/sdbus-c++/Message.h @@ -77,16 +77,6 @@ namespace sdbus { class Message { public: - Message() = default; - Message(internal::ISdBus* sdbus) noexcept; - Message(void *msg, internal::ISdBus* sdbus) noexcept; - Message(void *msg, internal::ISdBus* sdbus, adopt_message_t) noexcept; - Message(const Message&) noexcept; - Message& operator=(const Message&) noexcept; - Message(Message&& other) noexcept; - Message& operator=(Message&& other) noexcept; - ~Message(); - Message& operator<<(bool item); Message& operator<<(int16_t item); Message& operator<<(int32_t item); @@ -150,6 +140,23 @@ namespace sdbus { void seal(); void rewind(bool complete); + class Factory; + + protected: + Message() = default; + Message(internal::ISdBus* sdbus) noexcept; + Message(void *msg, internal::ISdBus* sdbus) noexcept; + Message(void *msg, internal::ISdBus* sdbus, adopt_message_t) noexcept; + + Message(const Message&) noexcept; + Message& operator=(const Message&) noexcept; + Message(Message&& other) noexcept; + Message& operator=(Message&& other) noexcept; + + ~Message(); + + friend Factory; + protected: void* msg_{}; internal::ISdBus* sdbus_{}; @@ -158,8 +165,10 @@ namespace sdbus { class MethodCall : public Message { - public: using Message::Message; + + public: + MethodCall() = default; MethodReply send() const; MethodReply createReply() const; MethodReply createErrorReply(const sdbus::Error& error) const; @@ -173,29 +182,58 @@ namespace sdbus { class AsyncMethodCall : public Message { + using Message::Message; + public: using Slot = std::unique_ptr>; - using Message::Message; - AsyncMethodCall() = default; // Fixes gcc 6.3 error (default c-tor is not imported in above using declaration) + AsyncMethodCall() = default; AsyncMethodCall(MethodCall&& call) noexcept; Slot send(void* callback, void* userData) const; }; class MethodReply : public Message { - public: using Message::Message; + + public: + MethodReply() = default; void send() const; }; class Signal : public Message { - public: using Message::Message; + + public: + Signal() = default; void send() const; }; + class PropertySetCall : public Message + { + using Message::Message; + + public: + PropertySetCall() = default; + }; + + class PropertyGetReply : public Message + { + using Message::Message; + + public: + PropertyGetReply() = default; + }; + + class PlainMessage : public Message + { + using Message::Message; + + public: + PlainMessage() = default; + }; + template inline Message& operator<<(Message& msg, const std::vector<_Element>& items) { diff --git a/include/sdbus-c++/TypeTraits.h b/include/sdbus-c++/TypeTraits.h index f7bde84..1f085b1 100644 --- a/include/sdbus-c++/TypeTraits.h +++ b/include/sdbus-c++/TypeTraits.h @@ -41,10 +41,11 @@ namespace sdbus { class ObjectPath; class Signature; struct UnixFd; - class Message; class MethodCall; class MethodReply; class Signal; + class PropertySetCall; + class PropertyGetReply; template class Result; class Error; } @@ -54,8 +55,8 @@ namespace sdbus { using method_callback = std::function; using async_reply_handler = std::function; using signal_handler = std::function; - using property_set_callback = std::function; - using property_get_callback = std::function; + using property_set_callback = std::function; + using property_get_callback = std::function; template struct signature_of diff --git a/include/sdbus-c++/Types.h b/include/sdbus-c++/Types.h index fe19f81..d122758 100644 --- a/include/sdbus-c++/Types.h +++ b/include/sdbus-c++/Types.h @@ -94,7 +94,7 @@ namespace sdbus { std::string peekValueType() const; private: - mutable Message msg_{}; + mutable PlainMessage msg_{}; }; /********************************************//** diff --git a/src/Connection.cpp b/src/Connection.cpp index 42fa192..b8147ca 100644 --- a/src/Connection.cpp +++ b/src/Connection.cpp @@ -25,6 +25,7 @@ #include "Connection.h" #include "SdBus.h" +#include "MessageUtils.h" #include #include #include "ScopeGuard.h" @@ -169,7 +170,7 @@ MethodCall Connection::createMethodCall( const std::string& destination SDBUS_THROW_ERROR_IF(r < 0, "Failed to create method call", -r); - return MethodCall{sdbusMsg, iface_.get(), adopt_message}; + return Message::Factory::create(sdbusMsg, iface_.get(), adopt_message); } Signal Connection::createSignal( const std::string& objectPath @@ -186,7 +187,7 @@ Signal Connection::createSignal( const std::string& objectPath SDBUS_THROW_ERROR_IF(r < 0, "Failed to create signal", -r); - return Signal{sdbusSignal, iface_.get(), adopt_message}; + return Message::Factory::create(sdbusSignal, iface_.get(), adopt_message); } void Connection::emitPropertiesChangedSignal( const std::string& objectPath diff --git a/src/Message.cpp b/src/Message.cpp index f4c11d2..3396e2b 100644 --- a/src/Message.cpp +++ b/src/Message.cpp @@ -639,7 +639,7 @@ MethodReply MethodCall::sendWithReply() const SDBUS_THROW_ERROR_IF(r < 0, "Failed to call method", -r); - return MethodReply{sdbusReply, sdbus_, adopt_message}; + return Factory::create(sdbusReply, sdbus_, adopt_message); } MethodReply MethodCall::sendWithNoReply() const @@ -647,7 +647,7 @@ MethodReply MethodCall::sendWithNoReply() const auto r = sdbus_->sd_bus_send(nullptr, (sd_bus_message*)msg_, nullptr); SDBUS_THROW_ERROR_IF(r < 0, "Failed to call method with no reply", -r); - return MethodReply{}; // No reply + return Factory::create(); // No reply } MethodReply MethodCall::createReply() const @@ -656,7 +656,7 @@ MethodReply MethodCall::createReply() const auto r = sdbus_->sd_bus_message_new_method_return((sd_bus_message*)msg_, &sdbusReply); SDBUS_THROW_ERROR_IF(r < 0, "Failed to create method reply", -r); - return MethodReply{sdbusReply, sdbus_, adopt_message}; + return Factory::create(sdbusReply, sdbus_, adopt_message); } MethodReply MethodCall::createErrorReply(const Error& error) const @@ -669,7 +669,7 @@ MethodReply MethodCall::createErrorReply(const Error& error) const auto r = sdbus_->sd_bus_message_new_method_error((sd_bus_message*)msg_, &sdbusErrorReply, &sdbusError); SDBUS_THROW_ERROR_IF(r < 0, "Failed to create method error reply", -r); - return MethodReply{sdbusErrorReply, sdbus_, adopt_message}; + return Factory::create(sdbusErrorReply, sdbus_, adopt_message); } AsyncMethodCall::AsyncMethodCall(MethodCall&& call) noexcept @@ -699,7 +699,7 @@ void Signal::send() const SDBUS_THROW_ERROR_IF(r < 0, "Failed to emit signal", -r); } -Message createPlainMessage() +PlainMessage createPlainMessage() { int r; @@ -736,7 +736,7 @@ Message createPlainMessage() r = sd_bus_message_new(bus, &sdbusMsg, _SD_BUS_MESSAGE_TYPE_INVALID); SDBUS_THROW_ERROR_IF(r < 0, "Failed to create a new message", -r); - return Message{sdbusMsg, &sdbus, adopt_message}; + return Message::Factory::create(sdbusMsg, &sdbus, adopt_message); } } diff --git a/src/MessageUtils.h b/src/MessageUtils.h index d4cc361..6f31d35 100644 --- a/src/MessageUtils.h +++ b/src/MessageUtils.h @@ -30,7 +30,35 @@ namespace sdbus { - Message createPlainMessage(); + class Message::Factory + { + public: + template + static _Msg create() + { + return _Msg{}; + } + + template + static _Msg create(void *msg) + { + return _Msg{msg}; + } + + template + static _Msg create(void *msg, internal::ISdBus* sdbus) + { + return _Msg{msg, sdbus}; + } + + template + static _Msg create(void *msg, internal::ISdBus* sdbus, adopt_message_t) + { + return _Msg{msg, sdbus, adopt_message}; + } + }; + + PlainMessage createPlainMessage(); } #endif /* SDBUS_CXX_INTERNAL_MESSAGEUTILS_H_ */ diff --git a/src/Object.cpp b/src/Object.cpp index c3db8e2..295783e 100644 --- a/src/Object.cpp +++ b/src/Object.cpp @@ -24,6 +24,7 @@ */ #include "Object.h" +#include "MessageUtils.h" #include #include #include @@ -260,7 +261,7 @@ int Object::sdbus_method_callback(sd_bus_message *sdbusMessage, void *userData, auto* object = static_cast(userData); assert(object != nullptr); - MethodCall message{sdbusMessage, &object->connection_.getSdBusInterface()}; + auto message = Message::Factory::create(sdbusMessage, &object->connection_.getSdBusInterface()); // Note: The lookup can be optimized by using sorted vectors instead of associative containers auto& callback = object->interfaces_[message.getInterfaceName()].methods_[message.getMemberName()].callback_; @@ -298,7 +299,7 @@ int Object::sdbus_property_get_callback( sd_bus */*bus*/ return 1; } - Message reply{sdbusReply, &object->connection_.getSdBusInterface()}; + auto reply = Message::Factory::create(sdbusReply, &object->connection_.getSdBusInterface()); try { @@ -327,7 +328,7 @@ int Object::sdbus_property_set_callback( sd_bus */*bus*/ auto& callback = object->interfaces_[interface].properties_[property].setCallback_; assert(callback); - Message value{sdbusValue, &object->connection_.getSdBusInterface()}; + auto value = Message::Factory::create(sdbusValue, &object->connection_.getSdBusInterface()); try { diff --git a/src/Proxy.cpp b/src/Proxy.cpp index f64f7ea..c6c29a0 100644 --- a/src/Proxy.cpp +++ b/src/Proxy.cpp @@ -25,6 +25,7 @@ #include "Proxy.h" #include "IConnection.h" +#include "MessageUtils.h" #include "sdbus-c++/Message.h" #include "sdbus-c++/IConnection.h" #include "sdbus-c++/Error.h" @@ -137,7 +138,7 @@ int Proxy::sdbus_async_reply_handler(sd_bus_message *sdbusMessage, void *userDat SCOPE_EXIT{ proxy.pendingAsyncCalls_.removeCall(asyncCallData->slot.get()); }; - MethodReply message{sdbusMessage, &proxy.connection_->getSdBusInterface()}; + auto message = Message::Factory::create(sdbusMessage, &proxy.connection_->getSdBusInterface()); const auto* error = sd_bus_message_get_error(sdbusMessage); if (error == nullptr) @@ -158,7 +159,7 @@ int Proxy::sdbus_signal_callback(sd_bus_message *sdbusMessage, void *userData, s auto* proxy = static_cast(userData); assert(proxy != nullptr); - Signal message{sdbusMessage, &proxy->connection_->getSdBusInterface()}; + auto message = Message::Factory::create(sdbusMessage, &proxy->connection_->getSdBusInterface()); // Note: The lookup can be optimized by using sorted vectors instead of associative containers auto& callback = proxy->interfaces_[message.getInterfaceName()].signals_[message.getMemberName()].callback_; diff --git a/tests/unittests/Message_test.cpp b/tests/unittests/Message_test.cpp index 8d96d7e..88a3773 100644 --- a/tests/unittests/Message_test.cpp +++ b/tests/unittests/Message_test.cpp @@ -36,7 +36,7 @@ using namespace std::string_literals; namespace { - std::string deserializeString(sdbus::Message& msg) + std::string deserializeString(sdbus::PlainMessage& msg) { std::string str; msg >> str; @@ -50,29 +50,29 @@ namespace TEST(AMessage, CanBeDefaultConstructed) { - ASSERT_NO_THROW(sdbus::Message()); + ASSERT_NO_THROW(sdbus::PlainMessage()); } TEST(AMessage, IsInvalidAfterDefaultConstructed) { - sdbus::Message msg; + sdbus::PlainMessage msg; ASSERT_FALSE(msg.isValid()); } TEST(AMessage, IsValidWhenConstructedAsRealMessage) { - sdbus::Message msg{sdbus::createPlainMessage()}; + auto msg = sdbus::createPlainMessage(); ASSERT_TRUE(msg.isValid()); } TEST(AMessage, CreatesShallowCopyWhenCopyConstructed) { - sdbus::Message msg{sdbus::createPlainMessage()}; + auto msg = sdbus::createPlainMessage(); msg << "I am a string"s; msg.seal(); - sdbus::Message msgCopy = msg; + sdbus::PlainMessage msgCopy = msg; std::string str; msgCopy >> str; @@ -83,11 +83,11 @@ TEST(AMessage, CreatesShallowCopyWhenCopyConstructed) TEST(AMessage, CreatesDeepCopyWhenEplicitlyCopied) { - sdbus::Message msg{sdbus::createPlainMessage()}; + auto msg = sdbus::createPlainMessage(); msg << "I am a string"s; msg.seal(); - sdbus::Message msgCopy{sdbus::createPlainMessage()}; + auto msgCopy = sdbus::createPlainMessage(); msg.copyTo(msgCopy, true); msgCopy.seal(); // Seal to be able to read from it subsequently msg.rewind(true); // Rewind to the beginning after copying @@ -98,14 +98,14 @@ TEST(AMessage, CreatesDeepCopyWhenEplicitlyCopied) TEST(AMessage, IsEmptyWhenContainsNoValue) { - sdbus::Message msg{sdbus::createPlainMessage()}; + auto msg = sdbus::createPlainMessage(); ASSERT_TRUE(msg.isEmpty()); } TEST(AMessage, IsNotEmptyWhenContainsAValue) { - sdbus::Message msg{sdbus::createPlainMessage()}; + auto msg = sdbus::createPlainMessage(); msg << "I am a string"s; ASSERT_FALSE(msg.isEmpty()); @@ -113,7 +113,7 @@ TEST(AMessage, IsNotEmptyWhenContainsAValue) TEST(AMessage, CanCarryASimpleInteger) { - sdbus::Message msg{sdbus::createPlainMessage()}; + auto msg = sdbus::createPlainMessage(); int dataWritten = 5; @@ -128,7 +128,7 @@ TEST(AMessage, CanCarryASimpleInteger) TEST(AMessage, CanCarryAUnixFd) { - sdbus::Message msg{sdbus::createPlainMessage()}; + auto msg = sdbus::createPlainMessage(); sdbus::UnixFd dataWritten = 0; msg << dataWritten; @@ -143,7 +143,7 @@ TEST(AMessage, CanCarryAUnixFd) TEST(AMessage, CanCarryAVariant) { - sdbus::Message msg{sdbus::createPlainMessage()}; + auto msg = sdbus::createPlainMessage(); auto dataWritten = sdbus::Variant((double)3.14); @@ -158,7 +158,7 @@ TEST(AMessage, CanCarryAVariant) TEST(AMessage, CanCarryACollectionOfEmbeddedVariants) { - sdbus::Message msg{sdbus::createPlainMessage()}; + auto msg = sdbus::createPlainMessage(); auto value = std::vector{"hello"s, (double)3.14}; auto dataWritten = sdbus::Variant{value}; @@ -175,7 +175,7 @@ TEST(AMessage, CanCarryACollectionOfEmbeddedVariants) TEST(AMessage, CanCarryAnArray) { - sdbus::Message msg{sdbus::createPlainMessage()}; + auto msg = sdbus::createPlainMessage(); std::vector dataWritten{3545342, 43643532, 324325}; @@ -190,7 +190,7 @@ TEST(AMessage, CanCarryAnArray) TEST(AMessage, CanCarryADictionary) { - sdbus::Message msg{sdbus::createPlainMessage()}; + auto msg = sdbus::createPlainMessage(); std::map dataWritten{{1, "one"}, {2, "two"}}; @@ -205,7 +205,7 @@ TEST(AMessage, CanCarryADictionary) TEST(AMessage, CanCarryAComplexType) { - sdbus::Message msg{sdbus::createPlainMessage()}; + auto msg = sdbus::createPlainMessage(); using ComplexType = std::map< uint64_t, diff --git a/tests/unittests/Types_test.cpp b/tests/unittests/Types_test.cpp index 77301c3..e9d5fbb 100644 --- a/tests/unittests/Types_test.cpp +++ b/tests/unittests/Types_test.cpp @@ -152,7 +152,7 @@ TEST(ANonEmptyVariant, SerializesSuccessfullyToAMessage) { sdbus::Variant variant("a string"); - sdbus::Message msg = sdbus::createPlainMessage(); + auto msg = sdbus::createPlainMessage(); ASSERT_NO_THROW(variant.serializeTo(msg)); } @@ -161,7 +161,7 @@ TEST(AnEmptyVariant, ThrowsWhenBeingSerializedToAMessage) { sdbus::Variant variant; - sdbus::Message msg = sdbus::createPlainMessage(); + auto msg = sdbus::createPlainMessage(); ASSERT_THROW(variant.serializeTo(msg), sdbus::Error); } @@ -172,7 +172,7 @@ TEST(ANonEmptyVariant, SerializesToAndDeserializesFromAMessageSuccessfully) ComplexType value{ {ANY_UINT64, ComplexType::mapped_type{sdbus::make_struct("hello", ANY_DOUBLE), sdbus::make_struct("world", ANY_DOUBLE)}} }; sdbus::Variant variant(value); - sdbus::Message msg = sdbus::createPlainMessage(); + auto msg = sdbus::createPlainMessage(); variant.serializeTo(msg); msg.seal(); sdbus::Variant variant2; @@ -189,7 +189,7 @@ TEST(CopiesOfVariant, SerializeToAndDeserializeFromMessageSuccessfully) auto variantCopy1{variant}; auto variantCopy2 = variant; - sdbus::Message msg = sdbus::createPlainMessage(); + auto msg = sdbus::createPlainMessage(); variant.serializeTo(msg); variantCopy1.serializeTo(msg); variantCopy2.serializeTo(msg);