diff --git a/connections/core.cc b/connections/core.cc index cbca2a82..0e8c00b4 100644 --- a/connections/core.cc +++ b/connections/core.cc @@ -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(*(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)); diff --git a/connections/core_test.cc b/connections/core_test.cc index 7fa4dfdd..45f22d37 100644 --- a/connections/core_test.cc +++ b/connections/core_test.cc @@ -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);