Skip to content

Commit

Permalink
Refactor NearbyConnection class.
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 707733296
  • Loading branch information
ftsui authored and copybara-github committed Dec 19, 2024
1 parent 0b715d2 commit b1ef67d
Show file tree
Hide file tree
Showing 22 changed files with 206 additions and 285 deletions.
3 changes: 2 additions & 1 deletion sharing/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -167,14 +167,14 @@ cc_library(
hdrs = ["paired_key_verification_runner.h"],
deps = [
":incoming_frame_reader",
":types",
"//internal/platform:types",
"//proto:sharing_enums_cc_proto",
"//sharing/certificates",
"//sharing/internal/public:logging",
"//sharing/proto:enums_cc_proto",
"//sharing/proto:share_cc_proto",
"//sharing/proto:wire_format_cc_proto",
"@com_google_absl//absl/functional:any_invocable",
"@com_google_absl//absl/time",
],
)
Expand Down Expand Up @@ -229,6 +229,7 @@ cc_library(
"//sharing/proto:wire_format_cc_proto",
"@com_google_absl//absl/container:flat_hash_map",
"@com_google_absl//absl/functional:any_invocable",
"@com_google_absl//absl/functional:bind_front",
"@com_google_absl//absl/strings:str_format",
"@com_google_absl//absl/strings:string_view",
"@com_google_absl//absl/time",
Expand Down
8 changes: 3 additions & 5 deletions sharing/fake_nearby_connections_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@
#include "sharing/nearby_connections_types.h"
#include "sharing/proto/enums.pb.h"

namespace nearby {
namespace sharing {
namespace nearby::sharing {

using DataUsage = ::nearby::sharing::proto::DataUsage;

Expand Down Expand Up @@ -119,7 +118,7 @@ void FakeNearbyConnectionsManager::Connect(
absl::MutexLock lock(&endpoints_mutex_);
connection_endpoint_infos_.emplace(endpoint_id, std::move(endpoint_info));
}
std::move(callback)(connection_, Status::kUnknown);
std::move(callback)(endpoint_id, connection_, Status::kUnknown);
}

void FakeNearbyConnectionsManager::AcceptConnection(
Expand Down Expand Up @@ -335,5 +334,4 @@ void FakeNearbyConnectionsManager::AddUnknownFilePathsToDeleteForTesting(

std::string FakeNearbyConnectionsManager::Dump() const { return ""; }

} // namespace sharing
} // namespace nearby
} // namespace nearby::sharing
12 changes: 4 additions & 8 deletions sharing/incoming_frames_reader_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,11 @@
#include "internal/test/fake_clock.h"
#include "internal/test/fake_device_info.h"
#include "internal/test/fake_task_runner.h"
#include "sharing/fake_nearby_connections_manager.h"
#include "sharing/internal/public/logging.h"
#include "sharing/nearby_connection_impl.h"
#include "sharing/proto/wire_format.pb.h"

namespace nearby {
namespace sharing {
namespace nearby::sharing {
namespace {

using ::nearby::sharing::service::proto::ConnectionResponseFrame;
Expand Down Expand Up @@ -103,7 +101,7 @@ class IncomingFramesReaderTest : public testing::Test {
public:
IncomingFramesReaderTest() {
nearby_connection_ = std::make_unique<NearbyConnectionImpl>(
fake_device_info_, &fake_nearby_connections_manager_, "endpoint_id");
fake_device_info_, "endpoint_id");
}
~IncomingFramesReaderTest() override = default;

Expand All @@ -113,7 +111,7 @@ class IncomingFramesReaderTest : public testing::Test {
}

NearbyConnectionImpl& connection() {
NL_CHECK(nearby_connection_);
CHECK(nearby_connection_);
return *nearby_connection_;
}

Expand All @@ -136,7 +134,6 @@ class IncomingFramesReaderTest : public testing::Test {
FakeClock fake_clock_;
FakeTaskRunner fake_task_runner_ {&fake_clock_, 1};
FakeDeviceInfo fake_device_info_;
FakeNearbyConnectionsManager fake_nearby_connections_manager_;
std::unique_ptr<NearbyConnectionImpl> nearby_connection_;
std::shared_ptr<IncomingFramesReader> frames_reader_ = nullptr;
};
Expand Down Expand Up @@ -369,5 +366,4 @@ TEST_F(IncomingFramesReaderTest, SkipInvalidFrame) {
}

} // namespace
} // namespace sharing
} // namespace nearby
} // namespace nearby::sharing
2 changes: 1 addition & 1 deletion sharing/incoming_share_session_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ std::unique_ptr<Payload> CreateWifiCredentialsPayload(
class IncomingShareSessionTest : public ::testing::Test {
protected:
IncomingShareSessionTest()
: connection_(device_info_, &connections_manager_, kEndpointId),
: connection_(device_info_, kEndpointId),
session_(&clock_, task_runner_, &connections_manager_,
analytics_recorder_, std::string(kEndpointId), share_target_,
transfer_metadata_callback_.AsStdFunction()) {
Expand Down
14 changes: 2 additions & 12 deletions sharing/nearby_connection.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@
#include <optional>
#include <vector>

namespace nearby {
namespace sharing {
namespace nearby::sharing {

// A socket-like wrapper around Nearby Connections that allows for asynchronous
// reads and writes.
Expand All @@ -38,21 +37,12 @@ class NearbyConnection {
std::function<void(std::optional<std::vector<uint8_t>> bytes)>
callback) = 0;

// Writes an outgoing stream of bytes to the remote device asynchronously.
// Must not be used on an already closed connection.
virtual void Write(std::vector<uint8_t> bytes) = 0;

// Closes the socket and disconnects from the remote device. This object will
// be invalidated after `listener` in SetDisconnectionListener is invoked.
virtual void Close() = 0;

// Listens to the socket being closed. Invoke `listener` when the socket is
// closed. This object will be invalidated after `listener` is invoked.
// Previously set listener will be replaced by `listener`.
virtual void SetDisconnectionListener(std::function<void()> listener) = 0;
};

} // namespace sharing
} // namespace nearby
} // namespace nearby::sharing

#endif // THIRD_PARTY_NEARBY_SHARING_NEARBY_CONNECTION_H_
33 changes: 6 additions & 27 deletions sharing/nearby_connection_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

#include <cstdint>
#include <functional>
#include <memory>
#include <optional>
#include <queue>
#include <string>
Expand All @@ -27,21 +26,15 @@
#include "absl/synchronization/mutex.h"
#include "internal/platform/device_info.h"
#include "sharing/internal/public/logging.h"
#include "sharing/nearby_connections_manager.h"
#include "sharing/nearby_connections_types.h"

namespace nearby {
namespace sharing {
namespace nearby::sharing {

NearbyConnectionImpl::NearbyConnectionImpl(
nearby::DeviceInfo& device_info,
NearbyConnectionsManager* nearby_connections_manager,
absl::string_view endpoint_id)
NearbyConnectionImpl::NearbyConnectionImpl(nearby::DeviceInfo& device_info,
absl::string_view endpoint_id)
: device_info_(device_info),
nearby_connections_manager_(nearby_connections_manager),
endpoint_id_(endpoint_id) {
if (!device_info_.PreventSleep()) {
NL_LOG(WARNING) << __func__ << ":Failed to prevent device sleep.";
LOG(WARNING) << __func__ << ":Failed to prevent device sleep.";
}
}

Expand All @@ -51,7 +44,7 @@ NearbyConnectionImpl::~NearbyConnectionImpl() {
{
absl::MutexLock lock(&mutex_);
if (!device_info_.AllowSleep()) {
NL_LOG(ERROR) << __func__ << ":Failed to allow device sleep.";
LOG(ERROR) << __func__ << ":Failed to allow device sleep.";
}
disconnect_listener = std::move(disconnect_listener_);
read_callback = std::move(read_callback_);
Expand Down Expand Up @@ -81,19 +74,6 @@ void NearbyConnectionImpl::Read(
std::move(callback)(std::move(bytes));
}

void NearbyConnectionImpl::Write(std::vector<uint8_t> bytes) {
nearby_connections_manager_->Send(
endpoint_id_, std::make_unique<Payload>(bytes),
/*listener=*/
std::weak_ptr<NearbyConnectionsManager::PayloadStatusListener>());
}

void NearbyConnectionImpl::Close() {
// As [this] therefore endpoint_id_ will be destroyed in Disconnect, make a
// copy of [endpoint_id] as the parameter is a const ref.
nearby_connections_manager_->Disconnect(endpoint_id_);
}

void NearbyConnectionImpl::SetDisconnectionListener(
std::function<void()> listener) {
absl::MutexLock lock(&mutex_);
Expand All @@ -116,5 +96,4 @@ void NearbyConnectionImpl::WriteMessage(std::vector<uint8_t> bytes) {
}
}

} // namespace sharing
} // namespace nearby
} // namespace nearby::sharing
10 changes: 2 additions & 8 deletions sharing/nearby_connection_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,24 +28,20 @@
#include "internal/platform/device_info.h"
#include "sharing/nearby_connection.h"

namespace nearby {
namespace sharing {
namespace nearby::sharing {

class NearbyConnectionsManager;

class NearbyConnectionImpl : public NearbyConnection {
public:
NearbyConnectionImpl(nearby::DeviceInfo& device_info,
NearbyConnectionsManager* nearby_connections_manager,
absl::string_view endpoint_id);
~NearbyConnectionImpl() override;

// NearbyConnection:
void Read(
std::function<void(std::optional<std::vector<uint8_t>> bytes)> callback)
ABSL_LOCKS_EXCLUDED(mutex_) override;
void Write(std::vector<uint8_t> bytes) override;
void Close() override;
void SetDisconnectionListener(std::function<void()> listener)
ABSL_LOCKS_EXCLUDED(mutex_) override;

Expand All @@ -54,7 +50,6 @@ class NearbyConnectionImpl : public NearbyConnection {

private:
nearby::DeviceInfo& device_info_;
NearbyConnectionsManager* const nearby_connections_manager_;
const std::string endpoint_id_;

absl::Mutex mutex_;
Expand All @@ -67,7 +62,6 @@ class NearbyConnectionImpl : public NearbyConnection {
std::queue<std::vector<uint8_t>> reads_ ABSL_GUARDED_BY(mutex_);
};

} // namespace sharing
} // namespace nearby
} // namespace nearby::sharing

#endif // THIRD_PARTY_NEARBY_SHARING_NEARBY_CONNECTION_IMPL_H_
15 changes: 4 additions & 11 deletions sharing/nearby_connection_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,23 +23,19 @@
#include "internal/test/fake_clock.h"
#include "internal/test/fake_device_info.h"
#include "internal/test/fake_task_runner.h"
#include "sharing/fake_nearby_connections_manager.h"
#include "sharing/incoming_frames_reader.h"
#include "sharing/proto/wire_format.pb.h"

namespace nearby {
namespace sharing {
namespace nearby::sharing {
namespace {

TEST(NearbyConnectionImpl, DestructorBeforeReaderDestructor) {
FakeNearbyConnectionsManager connection_manager;
FakeClock fake_clock;
FakeTaskRunner fake_task_runner(&fake_clock, 1);
FakeDeviceInfo device_info;
bool called = false;

auto connection = std::make_unique<NearbyConnectionImpl>(
device_info, &connection_manager, "test");
auto connection = std::make_unique<NearbyConnectionImpl>(device_info, "test");
auto frames_reader = std::make_shared<IncomingFramesReader>(fake_task_runner,
connection.get());

Expand All @@ -56,14 +52,12 @@ TEST(NearbyConnectionImpl, DestructorBeforeReaderDestructor) {
}

TEST(NearbyConnectionImpl, DestructorAfterReaderDestructor) {
FakeNearbyConnectionsManager connection_manager;
FakeClock fake_clock;
FakeTaskRunner fake_task_runner(&fake_clock, 1);
FakeDeviceInfo device_info;
std::optional<nearby::sharing::service::proto::V1Frame> frame_result;

auto connection = std::make_unique<NearbyConnectionImpl>(
device_info, &connection_manager, "test");
auto connection = std::make_unique<NearbyConnectionImpl>(device_info, "test");
auto frames_reader = std::make_shared<IncomingFramesReader>(fake_task_runner,
connection.get());

Expand All @@ -82,5 +76,4 @@ TEST(NearbyConnectionImpl, DestructorAfterReaderDestructor) {
}

} // namespace
} // namespace sharing
} // namespace nearby
} // namespace nearby::sharing
14 changes: 5 additions & 9 deletions sharing/nearby_connections_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,22 +28,19 @@
#include "absl/strings/string_view.h"
#include "absl/types/span.h"
#include "sharing/common/nearby_share_enums.h"
#include "sharing/nearby_connection.h"
#include "sharing/nearby_connections_types.h"
#include "sharing/proto/enums.pb.h"

namespace nearby {
namespace sharing {

class NearbyConnection;
class PayloadTransferUpdatePtr;
namespace nearby::sharing {

using ConnectionsStatus = nearby::sharing::Status;

class NearbyConnectionsManager {
public:
using ConnectionsCallback = std::function<void(ConnectionsStatus status)>;
using NearbyConnectionCallback =
std::function<void(NearbyConnection*, Status status)>;
using NearbyConnectionCallback = std::function<void(
absl::string_view endpoint_id, NearbyConnection*, Status status)>;

// A callback for handling incoming connections while advertising.
class IncomingConnectionListener {
Expand Down Expand Up @@ -166,7 +163,6 @@ class NearbyConnectionsManager {
virtual std::string Dump() const = 0;
};

} // namespace sharing
} // namespace nearby
} // namespace nearby::sharing

#endif // THIRD_PARTY_NEARBY_SHARING_NEARBY_CONNECTIONS_MANAGER_H_
Loading

0 comments on commit b1ef67d

Please sign in to comment.