diff --git a/src/Connection.cpp b/src/Connection.cpp index d7cde68..e88e201 100644 --- a/src/Connection.cpp +++ b/src/Connection.cpp @@ -35,54 +35,33 @@ #include #include -namespace { - -std::vector to_strv(const std::vector& strings) -{ - std::vector strv; - for (auto& str : strings) - strv.push_back(const_cast(str.c_str())); - strv.push_back(nullptr); - return strv; -} - -} - namespace sdbus { namespace internal { -Connection::Connection(Connection::BusType type, std::unique_ptr&& interface) +Connection::Connection(std::unique_ptr&& interface, const BusFactory& busFactory) : iface_(std::move(interface)) - , busType_(type) + , bus_(openBus(busFactory)) { assert(iface_ != nullptr); - - auto bus = openBus(busType_); - bus_.reset(bus); - - finishHandshake(bus); - - loopExitFd_ = createProcessingLoopExitDescriptor(); } -Connection::Connection(const std::string& host, std::unique_ptr &&interface) - : iface_(std::move(interface)) - , busType_(BusType::eRemoteSystem) - , host_(host) +Connection::Connection(std::unique_ptr&& interface, system_bus_t) + : Connection(std::move(interface), [this](sd_bus** bus){ return iface_->sd_bus_open_system(bus); }) { - assert(iface_ != nullptr); +} - auto bus = openBus(busType_); - bus_.reset(bus); +Connection::Connection(std::unique_ptr&& interface, session_bus_t) + : Connection(std::move(interface), [this](sd_bus** bus){ return iface_->sd_bus_open_user(bus); }) +{ +} - finishHandshake(bus); - - loopExitFd_ = createProcessingLoopExitDescriptor(); +Connection::Connection(std::unique_ptr&& interface, remote_system_bus_t, const std::string& host) + : Connection(std::move(interface), [this, &host](sd_bus** bus){ return iface_->sd_bus_open_system_remote(bus, host.c_str()); }) +{ } Connection::~Connection() { leaveProcessingLoop(); - closeProcessingLoopExitDescriptor(loopExitFd_); } void Connection::requestName(const std::string& name) @@ -301,23 +280,15 @@ SlotPtr Connection::registerSignalHandler( const std::string& objectPath return {slot, [this](void *slot){ iface_->sd_bus_slot_unref((sd_bus_slot*)slot); }}; } -sd_bus* Connection::openBus(Connection::BusType type) +Connection::BusPtr Connection::openBus(const BusFactory& busFactory) { sd_bus* bus{}; - int r = 0; - if (type == BusType::eSystem) - r = iface_->sd_bus_open_system(&bus); - else if (type == BusType::eSession) - r = iface_->sd_bus_open_user(&bus); - else if (type == BusType::eRemoteSystem) - r = iface_->sd_bus_open_system_remote(&bus, host_.c_str()); - else - assert(false); - + int r = busFactory(&bus); SDBUS_THROW_ERROR_IF(r < 0, "Failed to open bus", -r); - assert(bus != nullptr); - return bus; + BusPtr busPtr{bus, [this](sd_bus* bus){ return iface_->sd_bus_flush_close_unref(bus); }}; + finishHandshake(busPtr.get()); + return busPtr; } void Connection::finishHandshake(sd_bus* bus) @@ -333,33 +304,19 @@ void Connection::finishHandshake(sd_bus* bus) SDBUS_THROW_ERROR_IF(r < 0, "Failed to flush bus on opening", -r); } -int Connection::createProcessingLoopExitDescriptor() -{ - auto r = eventfd(0, EFD_SEMAPHORE | EFD_CLOEXEC | EFD_NONBLOCK); - - SDBUS_THROW_ERROR_IF(r < 0, "Failed to create event object", -errno); - - return r; -} - -void Connection::closeProcessingLoopExitDescriptor(int fd) -{ - close(fd); -} - void Connection::notifyProcessingLoopToExit() { - assert(loopExitFd_ >= 0); + assert(loopExitFd_.fd >= 0); uint64_t value = 1; - auto r = write(loopExitFd_, &value, sizeof(value)); + auto r = write(loopExitFd_.fd, &value, sizeof(value)); SDBUS_THROW_ERROR_IF(r < 0, "Failed to notify processing loop", -errno); } void Connection::clearExitNotification() { uint64_t value{}; - auto r = read(loopExitFd_, &value, sizeof(value)); + auto r = read(loopExitFd_.fd, &value, sizeof(value)); SDBUS_THROW_ERROR_IF(r < 0, "Failed to read from the event descriptor", -errno); } @@ -385,10 +342,10 @@ bool Connection::waitForNextRequest() auto bus = bus_.get(); assert(bus != nullptr); - assert(loopExitFd_ != 0); + assert(loopExitFd_.fd != 0); auto sdbusPollData = getProcessLoopPollData(); - struct pollfd fds[] = {{sdbusPollData.fd, sdbusPollData.events, 0}, {loopExitFd_, POLLIN, 0}}; + struct pollfd fds[] = {{sdbusPollData.fd, sdbusPollData.events, 0}, {loopExitFd_.fd, POLLIN, 0}}; auto fdsCount = sizeof(fds)/sizeof(fds[0]); auto timeout = sdbusPollData.timeout_usec == (uint64_t) -1 ? (uint64_t)-1 : (sdbusPollData.timeout_usec+999)/1000; @@ -422,6 +379,27 @@ std::string Connection::composeSignalMatchFilter( const std::string& objectPath return filter; } +std::vector Connection::to_strv(const std::vector& strings) +{ + std::vector strv; + for (auto& str : strings) + strv.push_back(const_cast(str.c_str())); + strv.push_back(nullptr); + return strv; +} + +Connection::LoopExitEventFd::LoopExitEventFd() +{ + fd = eventfd(0, EFD_SEMAPHORE | EFD_CLOEXEC | EFD_NONBLOCK); + SDBUS_THROW_ERROR_IF(fd < 0, "Failed to create event object", -errno); +} + +Connection::LoopExitEventFd::~LoopExitEventFd() +{ + assert(fd >= 0); + close(fd); +} + }} namespace sdbus { @@ -440,8 +418,8 @@ std::unique_ptr createSystemBusConnection() { auto interface = std::make_unique(); assert(interface != nullptr); - return std::make_unique( sdbus::internal::Connection::BusType::eSystem - , std::move(interface)); + constexpr sdbus::internal::Connection::system_bus_t system_bus; + return std::make_unique(std::move(interface), system_bus); } std::unique_ptr createSystemBusConnection(const std::string& name) @@ -455,8 +433,8 @@ std::unique_ptr createSessionBusConnection() { auto interface = std::make_unique(); assert(interface != nullptr); - return std::make_unique( sdbus::internal::Connection::BusType::eSession - , std::move(interface)); + constexpr sdbus::internal::Connection::session_bus_t session_bus; + return std::make_unique(std::move(interface), session_bus); } std::unique_ptr createSessionBusConnection(const std::string& name) @@ -470,8 +448,8 @@ std::unique_ptr createRemoteSystemBusConnection(const std::s { auto interface = std::make_unique(); assert(interface != nullptr); - return std::make_unique( host - , std::move(interface)); + 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 9d2a7e3..cecacd4 100644 --- a/src/Connection.h +++ b/src/Connection.h @@ -35,6 +35,8 @@ #include #include #include +#include +#include namespace sdbus { namespace internal { @@ -43,15 +45,14 @@ namespace sdbus { namespace internal { , public sdbus::internal::IConnection // Internal, private interface { public: - enum class BusType - { - eSystem, - eSession, - eRemoteSystem, - }; + // Bus type tags + struct system_bus_t{}; + struct session_bus_t{}; + struct remote_system_bus_t{}; - Connection(BusType type, std::unique_ptr&& interface); - Connection(const std::string& host, std::unique_ptr&& interface); + 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); ~Connection() override; void requestName(const std::string& name) override; @@ -102,10 +103,12 @@ namespace sdbus { namespace internal { , void* userData ) override; private: - sd_bus* openBus(Connection::BusType type); + using BusFactory = std::function; + using BusPtr = std::unique_ptr>; + Connection(std::unique_ptr&& interface, const BusFactory& busFactory); + + BusPtr openBus(const std::function& busFactory); void finishHandshake(sd_bus* bus); - static int createProcessingLoopExitDescriptor(); - static void closeProcessingLoopExitDescriptor(int fd); bool waitForNextRequest(); static std::string composeSignalMatchFilter( const std::string& objectPath , const std::string& interfaceName @@ -113,18 +116,20 @@ namespace sdbus { namespace internal { void notifyProcessingLoopToExit(); void clearExitNotification(); void joinWithProcessingLoop(); + static std::vector to_strv(const std::vector& strings); + + struct LoopExitEventFd + { + LoopExitEventFd(); + ~LoopExitEventFd(); + int fd; + }; private: std::unique_ptr iface_; - std::unique_ptr> bus_ {nullptr, [this](sd_bus* bus) - { - return iface_->sd_bus_flush_close_unref(bus); - }}; - BusType busType_; - + BusPtr bus_; std::thread asyncLoopThread_; - int loopExitFd_{-1}; - std::string host_; + LoopExitEventFd loopExitFd_; }; }} diff --git a/tests/unittests/Connection_test.cpp b/tests/unittests/Connection_test.cpp index 28a6335..f148ce2 100644 --- a/tests/unittests/Connection_test.cpp +++ b/tests/unittests/Connection_test.cpp @@ -35,17 +35,18 @@ using ::testing::DoAll; using ::testing::SetArgPointee; using ::testing::Return; using ::testing::NiceMock; - -using BusType = sdbus::internal::Connection::BusType; - +using sdbus::internal::Connection; +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; class ConnectionCreationTest : public ::testing::Test { protected: ConnectionCreationTest() = default; - std::unique_ptr> mock_ { std::make_unique>() }; - sd_bus* STUB_ { reinterpret_cast(1) }; + std::unique_ptr> sdBusIntfMock_ = std::make_unique>(); + sd_bus* fakeBusPtr_ = reinterpret_cast(1); }; using ASystemBusConnection = ConnectionCreationTest; @@ -53,99 +54,119 @@ using ASessionBusConnection = ConnectionCreationTest; TEST_F(ASystemBusConnection, OpensAndFlushesBusWhenCreated) { - EXPECT_CALL(*mock_, sd_bus_open_system(_)).WillOnce(DoAll(SetArgPointee<0>(STUB_), Return(1))); - EXPECT_CALL(*mock_, sd_bus_flush(_)).Times(1); - sdbus::internal::Connection(BusType::eSystem, std::move(mock_)); + EXPECT_CALL(*sdBusIntfMock_, sd_bus_open_system(_)).WillOnce(DoAll(SetArgPointee<0>(fakeBusPtr_), Return(1))); + EXPECT_CALL(*sdBusIntfMock_, sd_bus_flush(_)).Times(1); + Connection(std::move(sdBusIntfMock_), system_bus); } TEST_F(ASessionBusConnection, OpensAndFlushesBusWhenCreated) { - EXPECT_CALL(*mock_, sd_bus_open_user(_)).WillOnce(DoAll(SetArgPointee<0>(STUB_), Return(1))); - EXPECT_CALL(*mock_, sd_bus_flush(_)).Times(1); - sdbus::internal::Connection(BusType::eSession, std::move(mock_)); + EXPECT_CALL(*sdBusIntfMock_, sd_bus_open_user(_)).WillOnce(DoAll(SetArgPointee<0>(fakeBusPtr_), Return(1))); + EXPECT_CALL(*sdBusIntfMock_, sd_bus_flush(_)).Times(1); + Connection(std::move(sdBusIntfMock_), session_bus); } TEST_F(ASystemBusConnection, ClosesAndUnrefsBusWhenDestructed) { - ON_CALL(*mock_, sd_bus_open_user(_)).WillByDefault(DoAll(SetArgPointee<0>(STUB_), Return(1))); - EXPECT_CALL(*mock_, sd_bus_flush_close_unref(_)).Times(1); - sdbus::internal::Connection(BusType::eSession, std::move(mock_)); + ON_CALL(*sdBusIntfMock_, sd_bus_open_user(_)).WillByDefault(DoAll(SetArgPointee<0>(fakeBusPtr_), Return(1))); + EXPECT_CALL(*sdBusIntfMock_, sd_bus_flush_close_unref(_)).Times(1); + Connection(std::move(sdBusIntfMock_), session_bus); } TEST_F(ASessionBusConnection, ClosesAndUnrefsBusWhenDestructed) { - ON_CALL(*mock_, sd_bus_open_user(_)).WillByDefault(DoAll(SetArgPointee<0>(STUB_), Return(1))); - EXPECT_CALL(*mock_, sd_bus_flush_close_unref(_)).Times(1); - sdbus::internal::Connection(BusType::eSession, std::move(mock_)); + ON_CALL(*sdBusIntfMock_, sd_bus_open_user(_)).WillByDefault(DoAll(SetArgPointee<0>(fakeBusPtr_), Return(1))); + EXPECT_CALL(*sdBusIntfMock_, sd_bus_flush_close_unref(_)).Times(1); + Connection(std::move(sdBusIntfMock_), session_bus); } TEST_F(ASystemBusConnection, ThrowsErrorWhenOpeningTheBusFailsDuringConstruction) { - ON_CALL(*mock_, sd_bus_open_system(_)).WillByDefault(DoAll(SetArgPointee<0>(STUB_), Return(-1))); - ASSERT_THROW(sdbus::internal::Connection(BusType::eSystem, std::move(mock_)), sdbus::Error); + ON_CALL(*sdBusIntfMock_, sd_bus_open_system(_)).WillByDefault(DoAll(SetArgPointee<0>(fakeBusPtr_), Return(-1))); + ASSERT_THROW(Connection(std::move(sdBusIntfMock_), system_bus), sdbus::Error); } TEST_F(ASessionBusConnection, ThrowsErrorWhenOpeningTheBusFailsDuringConstruction) { - ON_CALL(*mock_, sd_bus_open_user(_)).WillByDefault(DoAll(SetArgPointee<0>(STUB_), Return(-1))); - ASSERT_THROW(sdbus::internal::Connection(BusType::eSession, std::move(mock_)), sdbus::Error); + ON_CALL(*sdBusIntfMock_, sd_bus_open_user(_)).WillByDefault(DoAll(SetArgPointee<0>(fakeBusPtr_), Return(-1))); + ASSERT_THROW(Connection(std::move(sdBusIntfMock_), session_bus), sdbus::Error); } TEST_F(ASystemBusConnection, ThrowsErrorWhenFlushingTheBusFailsDuringConstruction) { - ON_CALL(*mock_, sd_bus_open_system(_)).WillByDefault(DoAll(SetArgPointee<0>(STUB_), Return(1))); - ON_CALL(*mock_, sd_bus_flush(_)).WillByDefault(Return(-1)); - ASSERT_THROW(sdbus::internal::Connection(BusType::eSystem, std::move(mock_)), sdbus::Error); + ON_CALL(*sdBusIntfMock_, sd_bus_open_system(_)).WillByDefault(DoAll(SetArgPointee<0>(fakeBusPtr_), Return(1))); + ON_CALL(*sdBusIntfMock_, sd_bus_flush(_)).WillByDefault(Return(-1)); + ASSERT_THROW(Connection(std::move(sdBusIntfMock_), system_bus), sdbus::Error); } TEST_F(ASessionBusConnection, ThrowsErrorWhenFlushingTheBusFailsDuringConstruction) { - ON_CALL(*mock_, sd_bus_open_user(_)).WillByDefault(DoAll(SetArgPointee<0>(STUB_), Return(1))); - ON_CALL(*mock_, sd_bus_flush(_)).WillByDefault(Return(-1)); - ASSERT_THROW(sdbus::internal::Connection(BusType::eSession, std::move(mock_)), sdbus::Error); + ON_CALL(*sdBusIntfMock_, sd_bus_open_user(_)).WillByDefault(DoAll(SetArgPointee<0>(fakeBusPtr_), Return(1))); + ON_CALL(*sdBusIntfMock_, sd_bus_flush(_)).WillByDefault(Return(-1)); + ASSERT_THROW(Connection(std::move(sdBusIntfMock_), session_bus), sdbus::Error); } -class ConnectionRequestTest : public ::testing::TestWithParam +namespace +{ +template +class AConnectionNameRequest : public ::testing::Test { protected: - ConnectionRequestTest() = default; + void setUpBusOpenExpectation(); + std::unique_ptr makeConnection(); + void SetUp() override { - switch (GetParam()) - { - case BusType::eSystem: - EXPECT_CALL(*mock_, sd_bus_open_system(_)).WillOnce(DoAll(SetArgPointee<0>(STUB_), Return(1))); - break; - case BusType::eSession: - EXPECT_CALL(*mock_, sd_bus_open_user(_)).WillOnce(DoAll(SetArgPointee<0>(STUB_), Return(1))); - break; - default: - break; - } - ON_CALL(*mock_, sd_bus_flush(_)).WillByDefault(Return(1)); - ON_CALL(*mock_, sd_bus_flush_close_unref(_)).WillByDefault(Return(STUB_)); + setUpBusOpenExpectation(); + ON_CALL(*sdBusIntfMock_, sd_bus_flush(_)).WillByDefault(Return(1)); + ON_CALL(*sdBusIntfMock_, sd_bus_flush_close_unref(_)).WillByDefault(Return(fakeBusPtr_)); + con_ = makeConnection(); } - std::unique_ptr> mock_ { std::make_unique>() }; - sd_bus* STUB_ { reinterpret_cast(1) }; + NiceMock* sdBusIntfMock_ = new NiceMock(); // con_ below will assume ownership + sd_bus* fakeBusPtr_ = reinterpret_cast(1); + std::unique_ptr con_; }; -using AConnectionNameRequest = ConnectionRequestTest; - -TEST_P(AConnectionNameRequest, DoesNotThrowOnSuccess) +template<> void AConnectionNameRequest::setUpBusOpenExpectation() { - EXPECT_CALL(*mock_, sd_bus_request_name(_, _, _)).WillOnce(Return(1)); - sdbus::internal::Connection(GetParam(), std::move(mock_)).requestName(""); + EXPECT_CALL(*sdBusIntfMock_, sd_bus_open_system(_)).WillOnce(DoAll(SetArgPointee<0>(fakeBusPtr_), Return(1))); +} +template<> void AConnectionNameRequest::setUpBusOpenExpectation() +{ + EXPECT_CALL(*sdBusIntfMock_, sd_bus_open_user(_)).WillOnce(DoAll(SetArgPointee<0>(fakeBusPtr_), Return(1))); +} +template<> void AConnectionNameRequest::setUpBusOpenExpectation() +{ + EXPECT_CALL(*sdBusIntfMock_, sd_bus_open_system_remote(_, _)).WillOnce(DoAll(SetArgPointee<0>(fakeBusPtr_), Return(1))); +} +template +std::unique_ptr AConnectionNameRequest<_BusTypeTag>::makeConnection() +{ + return std::make_unique(std::unique_ptr>(sdBusIntfMock_), _BusTypeTag{}); +} +template<> std::unique_ptr AConnectionNameRequest::makeConnection() +{ + return std::make_unique(std::unique_ptr>(sdBusIntfMock_), remote_system_bus, "some host"); } -TEST_P(AConnectionNameRequest, ThrowsOnFail) -{ - EXPECT_CALL(*mock_, sd_bus_request_name(_, _, _)).WillOnce(Return(-1)); +typedef ::testing::Types< Connection::system_bus_t + , Connection::session_bus_t + , Connection::remote_system_bus_t + > BusTypeTags; - sdbus::internal::Connection conn_(GetParam(), std::move(mock_)); - ASSERT_THROW(conn_.requestName(""), sdbus::Error); +TYPED_TEST_SUITE(AConnectionNameRequest, BusTypeTags); } -// INSTANTIATE_TEST_SUITE_P is defined in googletest master, but not in googletest v1.8.1 that we are using now -INSTANTIATE_TEST_CASE_P(Request, AConnectionNameRequest, ::testing::Values(BusType::eSystem, BusType::eSession)); -//INSTANTIATE_TEST_SUITE_P(Request, AConnectionNameRequest, ::testing::Values(BusType::eSystem, BusType::eSession)) +TYPED_TEST(AConnectionNameRequest, DoesNotThrowOnSuccess) +{ + EXPECT_CALL(*this->sdBusIntfMock_, sd_bus_request_name(_, _, _)).WillOnce(Return(1)); + this->con_->requestName("org.sdbuscpp.somename"); +} + +TYPED_TEST(AConnectionNameRequest, ThrowsOnFail) +{ + EXPECT_CALL(*this->sdBusIntfMock_, sd_bus_request_name(_, _, _)).WillOnce(Return(-1)); + + ASSERT_THROW(this->con_->requestName("org.sdbuscpp.somename"), sdbus::Error); +} diff --git a/tests/unittests/TypeTraits_test.cpp b/tests/unittests/TypeTraits_test.cpp index 0aa9e64..a1c1739 100644 --- a/tests/unittests/TypeTraits_test.cpp +++ b/tests/unittests/TypeTraits_test.cpp @@ -121,7 +121,7 @@ namespace , ComplexType > DBusSupportedTypes; - TYPED_TEST_CASE(Type2DBusTypeSignatureConversion, DBusSupportedTypes); + TYPED_TEST_SUITE(Type2DBusTypeSignatureConversion, DBusSupportedTypes); } /*-------------------------------------*/ diff --git a/tests/unittests/mocks/SdBusMock.h b/tests/unittests/mocks/SdBusMock.h index 830ff31..df8e48c 100644 --- a/tests/unittests/mocks/SdBusMock.h +++ b/tests/unittests/mocks/SdBusMock.h @@ -58,6 +58,7 @@ public: 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)); MOCK_METHOD3(sd_bus_request_name, int(sd_bus *bus, const char *name, uint64_t flags)); MOCK_METHOD2(sd_bus_release_name, int(sd_bus *bus, const char *name)); MOCK_METHOD6(sd_bus_add_object_vtable, int(sd_bus *bus, sd_bus_slot **slot, const char *path, const char *interface, const sd_bus_vtable *vtable, void *userdata));