From 75ea1273749d47b7274ee93fc7616320f0adc369 Mon Sep 17 00:00:00 2001 From: David Leeds Date: Sun, 9 May 2021 01:50:57 -0700 Subject: [PATCH] connection: add createDefaultBusConnection() This internally calls sd_bus_open(), which automatically selects the system or session bus connection based on the presence and content of a DBUS_STARTER_BUS_TYPE environment variable and whether the calling process has root privileges. This option is very helpful when creating services and clients that will use the system bus in production, but connect to a session for testing. Additional changes: * Removed assertions null-checking make_unique() return values. make_unique() calls new, and new is expected to throw or abort on failure, making the assertions unhelpful. * Corrected a typo in the ClosesAndUnrefsBusWhenDestructed unit test for the system bus (tested the wrong function). --- include/sdbus-c++/IConnection.h | 21 +++++++++++++++ src/Connection.cpp | 22 +++++++++++++--- src/Connection.h | 2 ++ src/ISdBus.h | 1 + src/SdBus.cpp | 5 ++++ src/SdBus.h | 1 + tests/unittests/Connection_test.cpp | 40 ++++++++++++++++++++++++++--- tests/unittests/mocks/SdBusMock.h | 1 + 8 files changed, 87 insertions(+), 6 deletions(-) diff --git a/include/sdbus-c++/IConnection.h b/include/sdbus-c++/IConnection.h index cd0ed29..efa3341 100644 --- a/include/sdbus-c++/IConnection.h +++ b/include/sdbus-c++/IConnection.h @@ -265,6 +265,27 @@ namespace sdbus { */ [[nodiscard]] std::unique_ptr createConnection(const std::string& name); + /*! + * @brief Creates/opens D-Bus user connection when in a user context, + * and a system connection, otherwise. + * + * @return Connection instance + * + * @throws sdbus::Error in case of failure + */ + [[nodiscard]] std::unique_ptr createDefaultBusConnection(); + + /*! + * @brief Creates/opens D-Bus user connection with a name when in a user + * context, and a system connection with a name, otherwise. + * + * @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 createDefaultBusConnection(const std::string& name); + /*! * @brief Creates/opens D-Bus system connection * diff --git a/src/Connection.cpp b/src/Connection.cpp index 7c8a5bb..12c3b00 100644 --- a/src/Connection.cpp +++ b/src/Connection.cpp @@ -44,6 +44,11 @@ Connection::Connection(std::unique_ptr&& interface, const BusFactory& bu assert(iface_ != nullptr); } +Connection::Connection(std::unique_ptr&& interface, default_bus_t) + : Connection(std::move(interface), [this](sd_bus** bus){ return iface_->sd_bus_open(bus); }) +{ +} + Connection::Connection(std::unique_ptr&& interface, system_bus_t) : Connection(std::move(interface), [this](sd_bus** bus){ return iface_->sd_bus_open_system(bus); }) { @@ -484,10 +489,23 @@ std::unique_ptr createConnection(const std::string& name) return createSystemBusConnection(name); } +std::unique_ptr createDefaultBusConnection() +{ + auto interface = std::make_unique(); + constexpr sdbus::internal::Connection::default_bus_t default_bus; + return std::make_unique(std::move(interface), default_bus); +} + +std::unique_ptr createDefaultBusConnection(const std::string& name) +{ + auto conn = createDefaultBusConnection(); + conn->requestName(name); + return conn; +} + std::unique_ptr createSystemBusConnection() { auto interface = std::make_unique(); - assert(interface != nullptr); constexpr sdbus::internal::Connection::system_bus_t system_bus; return std::make_unique(std::move(interface), system_bus); } @@ -502,7 +520,6 @@ std::unique_ptr createSystemBusConnection(const std::string& std::unique_ptr createSessionBusConnection() { auto interface = std::make_unique(); - assert(interface != nullptr); constexpr sdbus::internal::Connection::session_bus_t session_bus; return std::make_unique(std::move(interface), session_bus); } @@ -517,7 +534,6 @@ std::unique_ptr createSessionBusConnection(const std::string std::unique_ptr createRemoteSystemBusConnection(const std::string& host) { auto interface = std::make_unique(); - assert(interface != nullptr); constexpr sdbus::internal::Connection::remote_system_bus_t remote_system_bus; return std::make_unique(std::move(interface), remote_system_bus, host); } diff --git a/src/Connection.h b/src/Connection.h index 8539bad..4a54715 100644 --- a/src/Connection.h +++ b/src/Connection.h @@ -48,10 +48,12 @@ namespace sdbus::internal { { public: // Bus type tags + struct default_bus_t{}; struct system_bus_t{}; struct session_bus_t{}; struct remote_system_bus_t{}; + Connection(std::unique_ptr&& interface, default_bus_t); Connection(std::unique_ptr&& interface, system_bus_t); Connection(std::unique_ptr&& interface, session_bus_t); Connection(std::unique_ptr&& interface, remote_system_bus_t, const std::string& host); diff --git a/src/ISdBus.h b/src/ISdBus.h index 800ba92..b7474ae 100644 --- a/src/ISdBus.h +++ b/src/ISdBus.h @@ -66,6 +66,7 @@ namespace sdbus::internal { virtual int sd_bus_emit_interfaces_added_strv(sd_bus *bus, const char *path, char **interfaces) = 0; virtual int sd_bus_emit_interfaces_removed_strv(sd_bus *bus, const char *path, char **interfaces) = 0; + virtual int sd_bus_open(sd_bus **ret) = 0; virtual int sd_bus_open_user(sd_bus **ret) = 0; virtual int sd_bus_open_system(sd_bus **ret) = 0; virtual int sd_bus_open_system_remote(sd_bus **ret, const char* host) = 0; diff --git a/src/SdBus.cpp b/src/SdBus.cpp index a60f664..75c7bf7 100644 --- a/src/SdBus.cpp +++ b/src/SdBus.cpp @@ -161,6 +161,11 @@ int SdBus::sd_bus_emit_interfaces_removed_strv(sd_bus *bus, const char *path, ch return ::sd_bus_emit_interfaces_removed_strv(bus, path, interfaces); } +int SdBus::sd_bus_open(sd_bus **ret) +{ + return ::sd_bus_open(ret); +} + int SdBus::sd_bus_open_user(sd_bus **ret) { return ::sd_bus_open_user(ret); diff --git a/src/SdBus.h b/src/SdBus.h index 1693112..b9201cd 100644 --- a/src/SdBus.h +++ b/src/SdBus.h @@ -58,6 +58,7 @@ public: virtual int sd_bus_emit_interfaces_added_strv(sd_bus *bus, const char *path, char **interfaces) override; virtual int sd_bus_emit_interfaces_removed_strv(sd_bus *bus, const char *path, char **interfaces) override; + virtual int sd_bus_open(sd_bus **ret) override; virtual int sd_bus_open_user(sd_bus **ret) override; virtual int sd_bus_open_system(sd_bus **ret) override; virtual int sd_bus_open_system_remote(sd_bus **ret, const char* hsot) override; diff --git a/tests/unittests/Connection_test.cpp b/tests/unittests/Connection_test.cpp index f148ce2..a241728 100644 --- a/tests/unittests/Connection_test.cpp +++ b/tests/unittests/Connection_test.cpp @@ -36,6 +36,7 @@ using ::testing::SetArgPointee; using ::testing::Return; using ::testing::NiceMock; using sdbus::internal::Connection; +constexpr sdbus::internal::Connection::default_bus_t default_bus; constexpr sdbus::internal::Connection::system_bus_t system_bus; constexpr sdbus::internal::Connection::session_bus_t session_bus; constexpr sdbus::internal::Connection::remote_system_bus_t remote_system_bus; @@ -49,9 +50,17 @@ protected: sd_bus* fakeBusPtr_ = reinterpret_cast(1); }; +using ADefaultBusConnection = ConnectionCreationTest; using ASystemBusConnection = ConnectionCreationTest; using ASessionBusConnection = ConnectionCreationTest; +TEST_F(ADefaultBusConnection, OpensAndFlushesBusWhenCreated) +{ + EXPECT_CALL(*sdBusIntfMock_, sd_bus_open(_)).WillOnce(DoAll(SetArgPointee<0>(fakeBusPtr_), Return(1))); + EXPECT_CALL(*sdBusIntfMock_, sd_bus_flush(_)).Times(1); + Connection(std::move(sdBusIntfMock_), default_bus); +} + TEST_F(ASystemBusConnection, OpensAndFlushesBusWhenCreated) { EXPECT_CALL(*sdBusIntfMock_, sd_bus_open_system(_)).WillOnce(DoAll(SetArgPointee<0>(fakeBusPtr_), Return(1))); @@ -66,11 +75,18 @@ TEST_F(ASessionBusConnection, OpensAndFlushesBusWhenCreated) Connection(std::move(sdBusIntfMock_), session_bus); } +TEST_F(ADefaultBusConnection, ClosesAndUnrefsBusWhenDestructed) +{ + ON_CALL(*sdBusIntfMock_, sd_bus_open(_)).WillByDefault(DoAll(SetArgPointee<0>(fakeBusPtr_), Return(1))); + EXPECT_CALL(*sdBusIntfMock_, sd_bus_flush_close_unref(_)).Times(1); + Connection(std::move(sdBusIntfMock_), default_bus); +} + TEST_F(ASystemBusConnection, ClosesAndUnrefsBusWhenDestructed) { - ON_CALL(*sdBusIntfMock_, sd_bus_open_user(_)).WillByDefault(DoAll(SetArgPointee<0>(fakeBusPtr_), Return(1))); + ON_CALL(*sdBusIntfMock_, sd_bus_open_system(_)).WillByDefault(DoAll(SetArgPointee<0>(fakeBusPtr_), Return(1))); EXPECT_CALL(*sdBusIntfMock_, sd_bus_flush_close_unref(_)).Times(1); - Connection(std::move(sdBusIntfMock_), session_bus); + Connection(std::move(sdBusIntfMock_), system_bus); } TEST_F(ASessionBusConnection, ClosesAndUnrefsBusWhenDestructed) @@ -80,6 +96,12 @@ TEST_F(ASessionBusConnection, ClosesAndUnrefsBusWhenDestructed) Connection(std::move(sdBusIntfMock_), session_bus); } +TEST_F(ADefaultBusConnection, ThrowsErrorWhenOpeningTheBusFailsDuringConstruction) +{ + ON_CALL(*sdBusIntfMock_, sd_bus_open(_)).WillByDefault(DoAll(SetArgPointee<0>(fakeBusPtr_), Return(-1))); + ASSERT_THROW(Connection(std::move(sdBusIntfMock_), default_bus), sdbus::Error); +} + TEST_F(ASystemBusConnection, ThrowsErrorWhenOpeningTheBusFailsDuringConstruction) { ON_CALL(*sdBusIntfMock_, sd_bus_open_system(_)).WillByDefault(DoAll(SetArgPointee<0>(fakeBusPtr_), Return(-1))); @@ -92,6 +114,13 @@ TEST_F(ASessionBusConnection, ThrowsErrorWhenOpeningTheBusFailsDuringConstructio ASSERT_THROW(Connection(std::move(sdBusIntfMock_), session_bus), sdbus::Error); } +TEST_F(ADefaultBusConnection, ThrowsErrorWhenFlushingTheBusFailsDuringConstruction) +{ + ON_CALL(*sdBusIntfMock_, sd_bus_open(_)).WillByDefault(DoAll(SetArgPointee<0>(fakeBusPtr_), Return(1))); + ON_CALL(*sdBusIntfMock_, sd_bus_flush(_)).WillByDefault(Return(-1)); + ASSERT_THROW(Connection(std::move(sdBusIntfMock_), default_bus), sdbus::Error); +} + TEST_F(ASystemBusConnection, ThrowsErrorWhenFlushingTheBusFailsDuringConstruction) { ON_CALL(*sdBusIntfMock_, sd_bus_open_system(_)).WillByDefault(DoAll(SetArgPointee<0>(fakeBusPtr_), Return(1))); @@ -128,6 +157,10 @@ protected: std::unique_ptr con_; }; +template<> void AConnectionNameRequest::setUpBusOpenExpectation() +{ + EXPECT_CALL(*sdBusIntfMock_, sd_bus_open(_)).WillOnce(DoAll(SetArgPointee<0>(fakeBusPtr_), Return(1))); +} template<> void AConnectionNameRequest::setUpBusOpenExpectation() { EXPECT_CALL(*sdBusIntfMock_, sd_bus_open_system(_)).WillOnce(DoAll(SetArgPointee<0>(fakeBusPtr_), Return(1))); @@ -150,7 +183,8 @@ template<> std::unique_ptr AConnectionNameRequest(std::unique_ptr>(sdBusIntfMock_), remote_system_bus, "some host"); } -typedef ::testing::Types< Connection::system_bus_t +typedef ::testing::Types< Connection::default_bus_t + , Connection::system_bus_t , Connection::session_bus_t , Connection::remote_system_bus_t > BusTypeTags; diff --git a/tests/unittests/mocks/SdBusMock.h b/tests/unittests/mocks/SdBusMock.h index e51c4bc..f8f4356 100644 --- a/tests/unittests/mocks/SdBusMock.h +++ b/tests/unittests/mocks/SdBusMock.h @@ -57,6 +57,7 @@ public: MOCK_METHOD3(sd_bus_emit_interfaces_added_strv, int(sd_bus *bus, const char *path, char **interfaces)); MOCK_METHOD3(sd_bus_emit_interfaces_removed_strv, int(sd_bus *bus, const char *path, char **interfaces)); + MOCK_METHOD1(sd_bus_open, int(sd_bus **ret)); MOCK_METHOD1(sd_bus_open_user, int(sd_bus **ret)); MOCK_METHOD1(sd_bus_open_system, int(sd_bus **ret)); MOCK_METHOD2(sd_bus_open_system_remote, int(sd_bus **ret, const char *host));