Remove endpoint ID CHECKs from Core

PiperOrigin-RevId: 631535509
This commit is contained in:
Anay Wadhera
2024-05-07 13:47:36 -07:00
committed by Copybara-Service
parent 346999639c
commit bf23ee5851
2 changed files with 144 additions and 57 deletions
+36 -9
View File
@@ -121,7 +121,10 @@ void Core::RequestConnection(absl::string_view endpoint_id,
ConnectionRequestInfo info,
ConnectionOptions connection_options,
ResultCallback callback) {
CHECK(!endpoint_id.empty());
if (endpoint_id.empty()) {
callback(Status{.value = Status::kEndpointUnknown});
return;
}
// Assign the default from feature flags for the keep-alive frame interval and
// timeout values if client don't mind them or has the unexpected ones.
@@ -147,7 +150,10 @@ void Core::RequestConnection(absl::string_view endpoint_id,
void Core::AcceptConnection(absl::string_view endpoint_id,
PayloadListener listener, ResultCallback callback) {
CHECK(!endpoint_id.empty());
if (endpoint_id.empty()) {
callback(Status{.value = Status::kEndpointUnknown});
return;
}
router_->AcceptConnection(&client_, endpoint_id, std::move(listener),
std::move(callback));
@@ -155,7 +161,10 @@ void Core::AcceptConnection(absl::string_view endpoint_id,
void Core::RejectConnection(absl::string_view endpoint_id,
ResultCallback callback) {
CHECK(!endpoint_id.empty());
if (endpoint_id.empty()) {
callback(Status{.value = Status::kEndpointUnknown});
return;
}
router_->RejectConnection(&client_, endpoint_id, std::move(callback));
}
@@ -182,7 +191,10 @@ void Core::CancelPayload(std::int64_t payload_id, ResultCallback callback) {
void Core::DisconnectFromEndpoint(absl::string_view endpoint_id,
ResultCallback callback) {
CHECK(!endpoint_id.empty());
if (endpoint_id.empty()) {
callback(Status{.value = Status::kEndpointUnknown});
return;
}
router_->DisconnectFromEndpoint(&client_, endpoint_id, std::move(callback));
}
@@ -399,7 +411,10 @@ void Core::RequestConnectionV3(const NearbyDevice& remote_device,
.local_device = const_cast<NearbyDevice&>(*(client_.GetLocalDevice())),
.listener = std::move(connection_cb),
};
CHECK(!remote_device.GetEndpointId().empty());
if (remote_device.GetEndpointId().empty()) {
result_cb(Status{.value = Status::kEndpointUnknown});
return;
}
// Assign the default from feature flags for the keep-alive frame interval and
// timeout values if client don't mind them or has the unexpected ones.
@@ -425,7 +440,10 @@ void Core::RequestConnectionV3(const NearbyDevice& remote_device,
void Core::AcceptConnectionV3(const NearbyDevice& remote_device,
v3::PayloadListener listener_cb,
ResultCallback result_cb) {
CHECK(!remote_device.GetEndpointId().empty());
if (remote_device.GetEndpointId().empty()) {
result_cb(Status{.value = Status::kEndpointUnknown});
return;
}
router_->AcceptConnectionV3(&client_, remote_device, std::move(listener_cb),
std::move(result_cb));
@@ -433,7 +451,10 @@ void Core::AcceptConnectionV3(const NearbyDevice& remote_device,
void Core::RejectConnectionV3(const NearbyDevice& remote_device,
ResultCallback result_cb) {
CHECK(!remote_device.GetEndpointId().empty());
if (remote_device.GetEndpointId().empty()) {
result_cb(Status{.value = Status::kEndpointUnknown});
return;
}
router_->RejectConnectionV3(&client_, remote_device, std::move(result_cb));
}
@@ -441,7 +462,10 @@ void Core::RejectConnectionV3(const NearbyDevice& remote_device,
void Core::SendPayloadV3(const NearbyDevice& remote_device, Payload payload,
ResultCallback result_cb) {
CHECK(payload.GetType() != PayloadType::kUnknown);
CHECK(!remote_device.GetEndpointId().empty());
if (remote_device.GetEndpointId().empty()) {
result_cb(Status{.value = Status::kEndpointUnknown});
return;
}
router_->SendPayloadV3(&client_, remote_device, std::move(payload),
std::move(result_cb));
@@ -457,7 +481,10 @@ void Core::CancelPayloadV3(const NearbyDevice& remote_device,
void Core::DisconnectFromDeviceV3(const NearbyDevice& remote_device,
ResultCallback result_cb) {
CHECK(!remote_device.GetEndpointId().empty());
if (remote_device.GetEndpointId().empty()) {
result_cb(Status{.value = Status::kEndpointUnknown});
return;
}
router_->DisconnectFromDeviceV3(&client_, remote_device,
std::move(result_cb));
+108 -48
View File
@@ -64,70 +64,130 @@ class FakeNearbyDeviceProvider : public NearbyDeviceProvider {
};
TEST(CoreTest, ConstructorDestructorWorks) {
MockServiceControllerRouter mock;
MockServiceControllerRouter mock_controller;
// Called when Core is destroyed.
EXPECT_CALL(mock, StopAllEndpoints)
EXPECT_CALL(mock_controller, StopAllEndpoints)
.WillOnce([&](ClientProxy* client, ResultCallback callback) {
callback({Status::kSuccess});
});
Core core{&mock};
Core core{&mock_controller};
}
TEST(CoreTest, DestructorReportsFatalFailure) {
ASSERT_DEATH(
{
MockServiceControllerRouter mock;
MockServiceControllerRouter mock_controller;
// Never invoke the result callback so ~Core will time out.
EXPECT_CALL(mock, StopAllEndpoints);
Core core{&mock};
EXPECT_CALL(mock_controller, StopAllEndpoints);
Core core{&mock_controller};
},
"Unable to shutdown");
}
TEST(CoreTest, RequestConnectionCallsScRouter) {
MockServiceControllerRouter mock;
MockServiceControllerRouter mock_controller;
// Called when Core is destroyed.
EXPECT_CALL(mock, StopAllEndpoints)
EXPECT_CALL(mock_controller, StopAllEndpoints)
.WillOnce([&](ClientProxy* client, ResultCallback callback) {
callback({Status::kSuccess});
});
EXPECT_CALL(mock, RequestConnection);
Core core{&mock};
EXPECT_CALL(mock_controller, RequestConnection);
Core core{&mock_controller};
core.RequestConnection("TEST", {}, {}, {});
}
TEST(CoreTest, AcceptConnectionCallsScRouter) {
MockServiceControllerRouter mock;
TEST(CoreTest, RequestConnectionFailsWithEmptyEndpoint) {
MockServiceControllerRouter mock_controller;
// Called when Core is destroyed.
EXPECT_CALL(mock, StopAllEndpoints)
EXPECT_CALL(mock_controller, StopAllEndpoints)
.WillOnce([&](ClientProxy* client, ResultCallback callback) {
callback({Status::kSuccess});
});
EXPECT_CALL(mock, AcceptConnection);
Core core{&mock};
Status final_status;
Core core{&mock_controller};
core.RequestConnection(
"", {}, {}, [&](Status result_status) { final_status = result_status; });
EXPECT_EQ(final_status.value, Status::kEndpointUnknown);
}
TEST(CoreTest, AcceptConnectionCallsScRouter) {
MockServiceControllerRouter mock_controller;
// Called when Core is destroyed.
EXPECT_CALL(mock_controller, StopAllEndpoints)
.WillOnce([&](ClientProxy* client, ResultCallback callback) {
callback({Status::kSuccess});
});
EXPECT_CALL(mock_controller, AcceptConnection);
Core core{&mock_controller};
core.AcceptConnection("TEST", {}, {});
}
TEST(CoreTest, SendPayloadCallsScRouter) {
MockServiceControllerRouter mock;
TEST(CoreTest, AcceptConnectionFailsWithEmptyEndpoint) {
MockServiceControllerRouter mock_controller;
// Called when Core is destroyed.
EXPECT_CALL(mock, StopAllEndpoints)
EXPECT_CALL(mock_controller, StopAllEndpoints)
.WillOnce([&](ClientProxy* client, ResultCallback callback) {
callback({Status::kSuccess});
});
EXPECT_CALL(mock, SendPayload);
Core core{&mock};
Status final_status;
Core core{&mock_controller};
core.AcceptConnection(
"", {}, [&](Status result_status) { final_status = result_status; });
EXPECT_EQ(final_status.value, Status::kEndpointUnknown);
}
TEST(CoreTest, RejectConnectionFailsWithEmptyEndpoint) {
MockServiceControllerRouter mock_controller;
// Called when Core is destroyed.
EXPECT_CALL(mock_controller, StopAllEndpoints)
.WillOnce([&](ClientProxy* client, ResultCallback callback) {
callback({Status::kSuccess});
});
Status final_status;
Core core{&mock_controller};
core.RejectConnection(
"", [&](Status result_status) { final_status = result_status; });
EXPECT_EQ(final_status.value, Status::kEndpointUnknown);
}
TEST(CoreTest, DisconnectFailsWithEmptyEndpoint) {
MockServiceControllerRouter mock_controller;
// Called when Core is destroyed.
EXPECT_CALL(mock_controller, StopAllEndpoints)
.WillOnce([&](ClientProxy* client, ResultCallback callback) {
callback({Status::kSuccess});
});
Status final_status;
Core core{&mock_controller};
core.DisconnectFromEndpoint(
"", [&](Status result_status) { final_status = result_status; });
EXPECT_EQ(final_status.value, Status::kEndpointUnknown);
}
TEST(CoreTest, SendPayloadCallsScRouter) {
MockServiceControllerRouter mock_controller;
// Called when Core is destroyed.
EXPECT_CALL(mock_controller, StopAllEndpoints)
.WillOnce([&](ClientProxy* client, ResultCallback callback) {
callback({Status::kSuccess});
});
EXPECT_CALL(mock_controller, SendPayload);
Core core{&mock_controller};
core.SendPayload({"TEST"}, Payload(ByteArray("Hello world")), {});
}
TEST(CoreV3Test, TestAdvertisingOptionsConversionWorks) {
MockServiceControllerRouter mock;
MockServiceControllerRouter mock_controller;
// Called when Core is destroyed.
EXPECT_CALL(mock, StopAllEndpoints)
EXPECT_CALL(mock_controller, StopAllEndpoints)
.WillOnce([&](ClientProxy* client, ResultCallback callback) {
callback({Status::kSuccess});
});
EXPECT_CALL(mock, StartAdvertising)
EXPECT_CALL(mock_controller, StartAdvertising)
.WillOnce([](ClientProxy*, absl::string_view,
const AdvertisingOptions& options,
const ConnectionRequestInfo& info, ResultCallback) {
@@ -137,7 +197,7 @@ TEST(CoreV3Test, TestAdvertisingOptionsConversionWorks) {
EXPECT_FALSE(options.auto_upgrade_bandwidth);
EXPECT_EQ(options.fast_advertisement_service_uuid, "NearbyConnections");
});
Core core{&mock};
Core core{&mock_controller};
v3::AdvertisingOptions advertising_options = {
.strategy = Strategy::kP2pCluster,
.power_level = PowerLevel::kHighPower,
@@ -150,13 +210,13 @@ TEST(CoreV3Test, TestAdvertisingOptionsConversionWorks) {
}
TEST(CoreV3Test, TestDiscoveryOptionsConversionWorks) {
MockServiceControllerRouter mock;
MockServiceControllerRouter mock_controller;
// Called when Core is destroyed.
EXPECT_CALL(mock, StopAllEndpoints)
EXPECT_CALL(mock_controller, StopAllEndpoints)
.WillOnce([&](ClientProxy* client, ResultCallback callback) {
callback({Status::kSuccess});
});
EXPECT_CALL(mock, StartDiscovery)
EXPECT_CALL(mock_controller, StartDiscovery)
.WillOnce([](ClientProxy*, absl::string_view,
const DiscoveryOptions& options, DiscoveryListener,
ResultCallback) {
@@ -165,7 +225,7 @@ TEST(CoreV3Test, TestDiscoveryOptionsConversionWorks) {
EXPECT_TRUE(options.auto_upgrade_bandwidth);
EXPECT_EQ(options.fast_advertisement_service_uuid, "NearbyConnections");
});
Core core{&mock};
Core core{&mock_controller};
v3::DiscoveryOptions discovery_options = {
.strategy = Strategy::kP2pCluster,
.power_level = PowerLevel::kHighPower,
@@ -176,8 +236,8 @@ TEST(CoreV3Test, TestDiscoveryOptionsConversionWorks) {
}
TEST(CoreV3Test, TestCallbackWrapWorksStartAdvertisingV3FourArgs) {
MockServiceControllerRouter mock;
EXPECT_CALL(mock, StartAdvertising)
MockServiceControllerRouter mock_controller;
EXPECT_CALL(mock_controller, StartAdvertising)
.WillOnce([&](ClientProxy*, absl::string_view, const AdvertisingOptions&,
const ConnectionRequestInfo& info, const ResultCallback&) {
NEARBY_LOGS(INFO) << "StartAdvertising called";
@@ -189,12 +249,12 @@ TEST(CoreV3Test, TestCallbackWrapWorksStartAdvertisingV3FourArgs) {
info.listener.bandwidth_changed_cb("FAKE", Medium::BLUETOOTH);
info.listener.disconnected_cb("FAKE");
});
EXPECT_CALL(mock, StopAllEndpoints)
EXPECT_CALL(mock_controller, StopAllEndpoints)
.WillOnce([&](ClientProxy* client, ResultCallback callback) {
NEARBY_LOGS(INFO) << "StopAllEndpoints called";
callback({Status::kSuccess});
});
Core core{&mock};
Core core{&mock_controller};
CountDownLatch result_latch(2);
CountDownLatch bandwidth_changed_latch(1);
CountDownLatch disconnected_latch(1);
@@ -233,8 +293,8 @@ TEST(CoreV3Test, TestCallbackWrapWorksStartAdvertisingV3FourArgs) {
}
TEST(CoreV3Test, TestStartAdvertisingV3NonConnectionsDeviceProvider) {
MockServiceControllerRouter mock;
EXPECT_CALL(mock, StartAdvertising)
MockServiceControllerRouter mock_controller;
EXPECT_CALL(mock_controller, StartAdvertising)
.WillOnce([&](ClientProxy*, absl::string_view, const AdvertisingOptions&,
const ConnectionRequestInfo& info, ResultCallback) {
NEARBY_LOGS(INFO) << "StartAdvertising called";
@@ -246,12 +306,12 @@ TEST(CoreV3Test, TestStartAdvertisingV3NonConnectionsDeviceProvider) {
info.listener.bandwidth_changed_cb("FAKE", Medium::BLUETOOTH);
info.listener.disconnected_cb("FAKE");
});
EXPECT_CALL(mock, StopAllEndpoints)
EXPECT_CALL(mock_controller, StopAllEndpoints)
.WillOnce([&](ClientProxy* client, ResultCallback callback) {
NEARBY_LOGS(INFO) << "StopAllEndpoints called";
callback({Status::kSuccess});
});
Core core{&mock};
Core core{&mock_controller};
CountDownLatch result_latch(2);
CountDownLatch bandwidth_changed_latch(1);
CountDownLatch disconnected_latch(1);
@@ -292,8 +352,8 @@ TEST(CoreV3Test, TestStartAdvertisingV3NonConnectionsDeviceProvider) {
}
TEST(CoreV3Test, TestStartAdvertisingV3NonConnectionsDevice) {
MockServiceControllerRouter mock;
EXPECT_CALL(mock, StartAdvertising)
MockServiceControllerRouter mock_controller;
EXPECT_CALL(mock_controller, StartAdvertising)
.WillOnce([&](ClientProxy*, absl::string_view, const AdvertisingOptions&,
const ConnectionRequestInfo& info, const ResultCallback&) {
NEARBY_LOGS(INFO) << "StartAdvertising called";
@@ -305,12 +365,12 @@ TEST(CoreV3Test, TestStartAdvertisingV3NonConnectionsDevice) {
info.listener.bandwidth_changed_cb("FAKE", Medium::BLUETOOTH);
info.listener.disconnected_cb("FAKE");
});
EXPECT_CALL(mock, StopAllEndpoints)
EXPECT_CALL(mock_controller, StopAllEndpoints)
.WillOnce([&](ClientProxy* client, ResultCallback callback) {
NEARBY_LOGS(INFO) << "StopAllEndpoints called";
callback({Status::kSuccess});
});
Core core{&mock};
Core core{&mock_controller};
CountDownLatch result_latch(2);
CountDownLatch bandwidth_changed_latch(1);
CountDownLatch disconnected_latch(1);
@@ -350,8 +410,8 @@ TEST(CoreV3Test, TestStartAdvertisingV3NonConnectionsDevice) {
}
TEST(CoreV3Test, TestCallbackWrapWorksStartAdvertisingV3FiveArgs) {
MockServiceControllerRouter mock;
EXPECT_CALL(mock, StartAdvertising)
MockServiceControllerRouter mock_controller;
EXPECT_CALL(mock_controller, StartAdvertising)
.WillOnce([&](ClientProxy*, absl::string_view, const AdvertisingOptions&,
const ConnectionRequestInfo& info, const ResultCallback&) {
NEARBY_LOGS(INFO) << "StartAdvertising called";
@@ -363,12 +423,12 @@ TEST(CoreV3Test, TestCallbackWrapWorksStartAdvertisingV3FiveArgs) {
info.listener.bandwidth_changed_cb("FAKE", Medium::BLUETOOTH);
info.listener.disconnected_cb("FAKE");
});
EXPECT_CALL(mock, StopAllEndpoints)
EXPECT_CALL(mock_controller, StopAllEndpoints)
.WillOnce([&](ClientProxy* client, ResultCallback callback) {
NEARBY_LOGS(INFO) << "StopAllEndpoints called";
callback({Status::kSuccess});
});
Core core{&mock};
Core core{&mock_controller};
CountDownLatch result_latch(2);
CountDownLatch bandwidth_changed_latch(1);
CountDownLatch disconnected_latch(1);
@@ -408,8 +468,8 @@ TEST(CoreV3Test, TestCallbackWrapWorksStartAdvertisingV3FiveArgs) {
}
TEST(CoreV3Test, TestCallbackWrapWorksStartDiscoveryV3) {
MockServiceControllerRouter mock;
EXPECT_CALL(mock, StartDiscovery)
MockServiceControllerRouter mock_controller;
EXPECT_CALL(mock_controller, StartDiscovery)
.WillOnce([&](ClientProxy*, absl::string_view, const DiscoveryOptions&,
DiscoveryListener listener, const ResultCallback&) {
// call all callbacks to make sure it all gets called correctly.
@@ -418,14 +478,14 @@ TEST(CoreV3Test, TestCallbackWrapWorksStartDiscoveryV3) {
listener.endpoint_found_cb("FAKE", ByteArray(), "");
listener.endpoint_lost_cb("FAKE");
});
EXPECT_CALL(mock, StopAllEndpoints)
EXPECT_CALL(mock_controller, StopAllEndpoints)
.WillOnce([&](ClientProxy* client, ResultCallback callback) {
NEARBY_LOGS(INFO) << "StopAllEndpoints called";
callback({Status::kSuccess});
});
v3::DiscoveryOptions options;
options.strategy = Strategy::kP2pCluster;
Core core{&mock};
Core core{&mock_controller};
CountDownLatch endpoint_distance_latch(1);
CountDownLatch endpoint_found_latch(1);
CountDownLatch endpoint_lost_latch(1);