refactor: rename connection creation methods (#406)

This PR makes things around connection factories a little more consistent and more intuitive:

    * createConnection() has been removed. One shall call more expressive createSystemConnection() instead to get a connection to the system bus.
    * createDefaultBusConnection() has been renamed to createBusConnection(), so as not to be confused with libsystemd's default_bus, which is a different thing (a reusable thread-local bus).

Proxies still by default call createBusConnection() to get a connection when the connection is not provided explicitly by the caller, but now createBusConnection() does a different thing, so now the proxies connect to either session bus or system bus depending on the context (as opposed to always to system bus like before).

The integration tests were modified to use createBusConnection().

ChangeLog

@@ -242,26 +242,22 @@ v1.4.0
 
 v2.x
 - Breaking changes in API/ABI/behavior:
-  - In *synchronous* D-Bus calls, the proxy now **always** blocks the connection for concurrent use until the call finishes (with either a received reply,
-    an error, or time out). If this creates a connection contention issue in your multi-threaded design, use short-lived, light-weight proxies, or call
-    the method in an asynchronous way.
-  - Signatures of callbacks `async_reply_handler`, `signal_handler`, `message_handler` and `property_set_callback` were modified to take input message objects
-    by value, as opposed to non-const ref. Callee assumes ownership of the message. This API is more idiomatic, cleaner and safer.
+  - In *synchronous* D-Bus calls, the proxy now **always** blocks the connection for concurrent use until the call finishes (with either a received reply, an error, or time out). If this creates a connection contention issue in your multi-threaded design, use short-lived, light-weight proxies, or call the method in an asynchronous way.
+  - Signatures of callbacks `async_reply_handler`, `signal_handler`, `message_handler` and `property_set_callback` were modified to take input message objects by value, as opposed to non-const ref. Callee assumes ownership of the message. This API is more idiomatic, cleaner and safer.
   - The `PollData` struct has been extended with a new data member: `eventFd`. All hooks with external event loops shall be modified to poll on this fd as well.
-  - `PollData::timeout_usec` was renamed to `PollData::timeout` and its type has been changed to `std::chrono::microseconds`. This member now holds directly
-    what before had to be obtained through `PollData::getAbsoluteTimeout()` call.
+  - `PollData::timeout_usec` was renamed to `PollData::timeout` and its type has been changed to `std::chrono::microseconds`. This member now holds directly what before had to be obtained through `PollData::getAbsoluteTimeout()` call.
   - `PollData::getRelativeTimeout()` return type was changed to `std::chrono::microseconds`.
   - `IConnection::processPendingRequest()` was renamed to `IConnection::processPendingEvent()`.
   - `Variant` constructor is now explicit.
   - `IProxy::getCurrentlyProcessedMessage()` now returns `Message` by value instead of a raw pointer to it. The caller assumes ownership of the message.
   - Object D-Bus API registration is now done through `IObject::addVTable()` method. The registration holds immediately; no `finishRegistration()` call is needed anymore.
-  - Subscription to signals has been simplified. The subscription is active right after the `registerSignalHandler`/`uponSignal()` call. No need for the final
-    call to `finishRegistration()`. `IProxy::muteSignal()` has been removed in favor of the RAII-based slot object returned by the slot-returning variant of the
-    registration method. Destroying the slot object means unsubscribing from the signal.
+  - Subscription to signals has been simplified. The subscription is active right after the `registerSignalHandler`/`uponSignal()` call. No need for the final call to `finishRegistration()`. `IProxy::muteSignal()` has been removed in favor of the RAII-based slot object returned by the slot-returning variant of the registration method. Destroying the slot object means unsubscribing from the signal.
   - `request_slot` tag was renamed to `return_slot`.
-  - `[[nodiscard]]` attribute has been added to relevant API methods.
   - `ProxyInterfaces::getObjectPath()` was removed, it can be replaced with `ProxyInterfaces::getProxy().getObjectPath()`.
   - `AdaptorInterfaces::getObjectPath()` was removed, it can be replaced with `AdaptorInterfaces::getObject().getObjectPath()`.
+  - `createConnection()` has been removed, to create a connection to the system bus use `createSystemConnection()` instead.
+  - `createDefaultBusConnection()` has been renamed to `createBusConnection()`.
+  - `Proxy`s now by default call `createBusConnection()` to get a connection when the connection is not provided explicitly by the caller, so they connect to either the session bus or the system bus depending on the context (as opposed to always to the system bus like before).
   - Callbacks taking `const sdbus::Error* error` were changed to take `std::optional<sdbus::Error>`, which better expresses the intent and meaning.
   - Types and methods marked deprecated in sdbus-c++ v1 were removed completely.
   - CMake options got `SDBUSCPP_` prefix for better usability and minimal risk of conflicts in downstream CMake projects.
@@ -271,4 +267,5 @@ v2.x
 - Fix for external event loops in which the event loop thread ID was not correctly initialized (now fixed and simplified by not needing the thread ID anymore)
 - Introduce native integration for sd-event
 - Add method to get currently processed message also to `IConnection`
+- `[[nodiscard]]` attribute has been added to relevant API methods.
 - Other simplifications, improvements and fixes springing out from the above refactoring

docs/using-sdbus-c++.md

@@ -390,7 +390,7 @@ int main(int argc, char *argv[])
 }
 ```
 
-In simple cases, we don't need to create D-Bus connection explicitly for our proxies. Unless a connection is provided to a proxy object explicitly via factory parameter, the proxy will create a connection of his own (unless it is a light-weight, short-lived proxy created with `dont_run_event_loop_thread_t`), and it will be a system bus connection. This is the case in the example above. (This approach is not scalable and resource-saving if we have plenty of proxies; see section [Working with D-Bus connections](#working-with-d-bus-connections-in-sdbus-c) for elaboration.) So, in the example, we create a proxy for object `/org/sdbuscpp/concatenator` publicly available at bus `org.sdbuscpp.concatenator`. We register handlers for signals we are interested in (if any).
+In simple cases, we don't need to create D-Bus connection explicitly for our proxies. We either pass a connection object to the proxy upon creation, or otherwise the proxy will create a connection of his own (to either the session bus or the system bus, depending on the context, see `man sd_bus_open`). This is the case in the example above. (This approach is not scalable and resource-saving if we have plenty of proxies; see section [Working with D-Bus connections](#working-with-d-bus-connections-in-sdbus-c) for elaboration.) So, in the example, we create a proxy for object `/org/sdbuscpp/concatenator` publicly available at bus `org.sdbuscpp.concatenator`. We register handlers for signals we are interested in (if any).
 
 The callback for a D-Bus signal handler on this level is any callable of signature `void(sdbus::Signal signal)`. The one and only parameter `signal` is the incoming signal message. We need to deserialize arguments from it, and then we can do our business logic with it.
 
@@ -404,10 +404,8 @@ Please note that we can create and destroy D-Bus object proxies dynamically, at
 
 There are several factory methods to create a bus connection object in sdbus-c++:
 
-* `createConnection()` - opens a connection to the system bus
-* `createConnection(const std::string& name)` - opens a connection with the given name to the system bus
-* `createDefaultBusConnection()` - opens a connection to the session bus when in a user context, and a connection to the system bus, otherwise
-* `createDefaultBusConnection(const std::string& name)` - opens a connection with the given name to the session bus when in a user context, and a connection with the given name to the system bus, otherwise
+* `createBusConnection()` - opens a connection to the session bus when in a user context, and a connection to the system bus, otherwise
+* `createBusConnection(const std::string& name)` - opens a connection with the given name to the session bus when in a user context, and a connection with the given name to the system bus, otherwise
 * `createSystemBusConnection()` - opens a connection to the system bus
 * `createSystemBusConnection(const std::string& name)` - opens a connection with the given name to the system bus
 * `createSessionBusConnection()` - opens a connection to the session bus
@@ -467,7 +465,7 @@ On the **client** side we likewise need a connection -- just that unlike on the
       * when created **without** `dont_run_event_loop_thread_t` tag, the proxy **will start** a dedicated event loop thread on that connection;
       * or, when created **with** `dont_run_event_loop_thread_t` tag, the proxy will start **no** event loop thread on that connection.
 
-    * Or we don't care about connnections at all (proxy factory overloads with no connection parameter). Under the hood, the proxy creates its own *system bus* connection. Additionally:
+    * Or we don't care about connnections at all (proxy factory overloads with no connection parameter). Under the hood, the proxy creates its own connection, to either the session bus (when in a user context) or the system bus otherwise. Additionally:
       * when created **without** `dont_run_event_loop_thread_t` tag, the proxy **will start** a dedicated event loop thread on that connection;
       * or, when created **with** `dont_run_event_loop_thread_t` tag, the proxy will start **no** event loop thread on that connection.
 
@@ -926,7 +924,7 @@ protected:
 
 > **_Tip_:** By inheriting from `sdbus::ProxyInterfaces`, we get access to the protected `getProxy()` method. We can call this method inside our proxy implementation class to access the underlying `IProxy` object.
 
-In the above example, a proxy is created that creates and maintains its own system bus connection. However, there are `ProxyInterfaces` class template constructor overloads that also take the connection from the user as the first parameter, and pass that connection over to the underlying proxy. The connection instance is used by all interfaces listed in the `ProxyInterfaces` template parameter list.
+In the above example, a proxy is created that creates and maintains its own bus connection (the bus is either session bus or system bus depending on the context, see `man sd_bus_open`). However, there are `ProxyInterfaces` class template constructor overloads that also take the connection from the user as the first parameter, and pass that connection over to the underlying proxy. The connection instance is used by all interfaces listed in the `ProxyInterfaces` template parameter list.
 
 Note however that there are multiple `ProxyInterfaces` constructor overloads, and they differ in how the proxy behaves towards the D-Bus connection. These overloads precisely map the `sdbus::createProxy` overloads, as they are actually implemented on top of them. See [Proxy and D-Bus connection](#Proxy-and-D-Bus-connection) for more info. We can even create a `IProxy` instance on our own, and inject it into our proxy class -- there is a constructor overload for it in `ProxyInterfaces`. This can help if we need to provide mocked implementations in our unit tests.
 

include/sdbus-c++/IConnection.h

@@ -395,33 +395,14 @@ namespace sdbus {
         return setMethodCallTimeout(microsecs.count());
     }
 
-    /*!
-     * @brief Creates/opens D-Bus system bus connection
-     *
-     * @return Connection instance
-     *
-     * @throws sdbus::Error in case of failure
-     */
-    [[nodiscard]] std::unique_ptr<sdbus::IConnection> createConnection();
-
-    /*!
-     * @brief Creates/opens D-Bus system bus connection with a name
-     *
-     * @param[in] name Name to request on the connection after its opening
-     * @return Connection instance
-     *
-     * @throws sdbus::Error in case of failure
-     */
-    [[nodiscard]] std::unique_ptr<sdbus::IConnection> createConnection(const std::string& name);
-
     /*!
      * @brief Creates/opens D-Bus session bus connection when in a user context, and a system bus connection, otherwise.
      *
      * @return Connection instance
      *
      * @throws sdbus::Error in case of failure
      */
-    [[nodiscard]] std::unique_ptr<sdbus::IConnection> createDefaultBusConnection();
+    [[nodiscard]] std::unique_ptr<sdbus::IConnection> createBusConnection();
 
     /*!
      * @brief Creates/opens D-Bus session bus connection with a name when in a user context, and a system bus connection with a name, otherwise.
@@ -431,7 +412,7 @@ namespace sdbus {
      *
      * @throws sdbus::Error in case of failure
      */
-    [[nodiscard]] std::unique_ptr<sdbus::IConnection> createDefaultBusConnection(const std::string& name);
+    [[nodiscard]] std::unique_ptr<sdbus::IConnection> createBusConnection(const std::string& name);
 
     /*!
      * @brief Creates/opens D-Bus system bus connection

src/Connection.cpp

@@ -872,14 +872,6 @@ int IConnection::PollData::getPollTimeout() const
 
 namespace sdbus::internal {
 
-std::unique_ptr<sdbus::internal::IConnection> createConnection()
-{
-    auto connection = sdbus::createConnection();
-    SCOPE_EXIT{ connection.release(); };
-    auto connectionInternal = dynamic_cast<sdbus::internal::IConnection*>(connection.get());
-    return std::unique_ptr<sdbus::internal::IConnection>(connectionInternal);
-}
-
 std::unique_ptr<sdbus::internal::IConnection> createPseudoConnection()
 {
     auto interface = std::make_unique<sdbus::internal::SdBus>();
@@ -892,25 +884,15 @@ namespace sdbus {
 
 using internal::Connection;
 
-std::unique_ptr<sdbus::IConnection> createConnection()
-{
-    return createSystemBusConnection();
-}
-
-std::unique_ptr<sdbus::IConnection> createConnection(const std::string& name)
-{
-    return createSystemBusConnection(name);
-}
-
-std::unique_ptr<sdbus::IConnection> createDefaultBusConnection()
+std::unique_ptr<sdbus::IConnection> createBusConnection()
 {
     auto interface = std::make_unique<sdbus::internal::SdBus>();
     return std::make_unique<sdbus::internal::Connection>(std::move(interface), Connection::default_bus);
 }
 
-std::unique_ptr<sdbus::IConnection> createDefaultBusConnection(const std::string& name)
+std::unique_ptr<sdbus::IConnection> createBusConnection(const std::string& name)
 {
-    auto conn = createDefaultBusConnection();
+    auto conn = createBusConnection();
     conn->requestName(name);
     return conn;
 }

src/IConnection.h

@@ -98,7 +98,6 @@ namespace sdbus::internal {
                                                         , void* userData ) = 0;
     };
 
-    [[nodiscard]] std::unique_ptr<sdbus::internal::IConnection> createConnection();
     [[nodiscard]] std::unique_ptr<sdbus::internal::IConnection> createPseudoConnection();
 
 }

src/Proxy.cpp

@@ -310,7 +310,7 @@ std::unique_ptr<sdbus::IProxy> createProxy( std::unique_ptr<IConnection>&& conne
 std::unique_ptr<sdbus::IProxy> createProxy( std::string destination
                                           , std::string objectPath )
 {
-    auto connection = sdbus::createConnection();
+    auto connection = sdbus::createBusConnection();
 
     auto sdbusConnection = std::unique_ptr<sdbus::internal::IConnection>(dynamic_cast<sdbus::internal::IConnection*>(connection.release()));
     assert(sdbusConnection != nullptr);
@@ -324,7 +324,7 @@ std::unique_ptr<sdbus::IProxy> createProxy( std::string destination
                                           , std::string objectPath
                                           , dont_run_event_loop_thread_t )
 {
-    auto connection = sdbus::createConnection();
+    auto connection = sdbus::createBusConnection();
 
     auto sdbusConnection = std::unique_ptr<sdbus::internal::IConnection>(dynamic_cast<sdbus::internal::IConnection*>(connection.release()));
     assert(sdbusConnection != nullptr);

tests/integrationtests/DBusConnectionTests.cpp

@@ -48,40 +48,40 @@ using namespace sdbus::test;
 
 TEST(Connection, CanBeDefaultConstructed)
 {
-    ASSERT_NO_THROW(auto con = sdbus::createConnection());
+    ASSERT_NO_THROW(auto con = sdbus::createBusConnection());
 }
 
-TEST(Connection, CanRequestRegisteredDbusName)
+TEST(SystemBusConnection, CanRequestRegisteredDbusName)
 {
-    auto connection = sdbus::createConnection();
+    auto connection = sdbus::createSystemBusConnection();
 
     ASSERT_NO_THROW(connection->requestName(BUS_NAME))
         << "Perhaps you've forgotten to copy `org.sdbuscpp.integrationtests.conf` file to `/etc/dbus-1/system.d` directory before running the tests?";
 }
 
-TEST(Connection, CannotRequestNonregisteredDbusName)
+TEST(SystemBusConnection, CannotRequestNonregisteredDbusName)
 {
-    auto connection = sdbus::createConnection();
+    auto connection = sdbus::createSystemBusConnection();
     ASSERT_THROW(connection->requestName("some.random.not.supported.dbus.name"), sdbus::Error);
 }
 
 TEST(Connection, CanReleasedRequestedName)
 {
-    auto connection = sdbus::createConnection();
+    auto connection = sdbus::createBusConnection();
 
     connection->requestName(BUS_NAME);
     ASSERT_NO_THROW(connection->releaseName(BUS_NAME));
 }
 
 TEST(Connection, CannotReleaseNonrequestedName)
 {
-    auto connection = sdbus::createConnection();
+    auto connection = sdbus::createBusConnection();
     ASSERT_THROW(connection->releaseName("some.random.nonrequested.name"), sdbus::Error);
 }
 
 TEST(Connection, CanEnterAndLeaveInternalEventLoop)
 {
-    auto connection = sdbus::createConnection();
+    auto connection = sdbus::createBusConnection();
     connection->requestName(BUS_NAME);
 
     std::thread t([&](){ connection->enterEventLoop(); });

tests/integrationtests/DBusGeneralTests.cpp

@@ -53,7 +53,7 @@ using ADirectConnection = TestFixtureWithDirectConnection;
 
 TEST(AdaptorAndProxy, CanBeConstructedSuccesfully)
 {
-    auto connection = sdbus::createConnection();
+    auto connection = sdbus::createBusConnection();
     connection->requestName(BUS_NAME);
 
     ASSERT_NO_THROW(TestAdaptor adaptor(*connection, OBJECT_PATH));
@@ -130,7 +130,7 @@ TYPED_TEST(AConnection, CanAddFloatingMatchRule)
 {
     auto matchRule = "sender='" + BUS_NAME + "',path='" + OBJECT_PATH + "'";
     std::atomic<bool> matchingMessageReceived{false};
-    auto con = sdbus::createSystemBusConnection();
+    auto con = sdbus::createBusConnection();
     con->enterEventLoopAsync();
     auto callback = [&](sdbus::Message msg)
     {

tests/integrationtests/DBusSignalsTests.cpp

@@ -70,7 +70,7 @@ TYPED_TEST(SdbusTestObject, EmitsSimpleSignalToMultipleProxiesSuccesfully)
 TYPED_TEST(SdbusTestObject, ProxyDoesNotReceiveSignalFromOtherBusName)
 {
     auto otherBusName = BUS_NAME + "2";
-    auto connection2 = sdbus::createConnection(otherBusName);
+    auto connection2 = sdbus::createBusConnection(otherBusName);
     auto adaptor2 = std::make_unique<TestAdaptor>(*connection2, OBJECT_PATH);
 
     adaptor2->emitSimpleSignal();

tests/integrationtests/TestFixture.cpp

@@ -28,8 +28,8 @@
 
 namespace sdbus { namespace test {
 
-std::unique_ptr<sdbus::IConnection> BaseTestFixture::s_adaptorConnection = sdbus::createSystemBusConnection();
-std::unique_ptr<sdbus::IConnection> BaseTestFixture::s_proxyConnection = sdbus::createSystemBusConnection();
+std::unique_ptr<sdbus::IConnection> BaseTestFixture::s_adaptorConnection = sdbus::createBusConnection();
+std::unique_ptr<sdbus::IConnection> BaseTestFixture::s_proxyConnection = sdbus::createBusConnection();
 
 #ifndef SDBUS_basu // sd_event integration is not supported in basu-based sdbus-c++