diff --git a/cpp/core/internal/client_proxy.cc b/cpp/core/internal/client_proxy.cc index 409414f3..ebb701cc 100644 --- a/cpp/core/internal/client_proxy.cc +++ b/cpp/core/internal/client_proxy.cc @@ -19,6 +19,7 @@ #include #include "platform/base/base64_utils.h" +#include "platform/base/feature_flags.h" #include "platform/base/prng.h" #include "platform/public/crypto.h" #include "platform/public/logging.h" @@ -493,6 +494,11 @@ bool ClientProxy::RemoteConnectionIsAccepted(std::string endpoint_id) const { } void ClientProxy::AddCancellationFlag(const std::string& endpoint_id) { + // Don't insert the CancellationFlag to the map if feature flag is disabled. + if (!FeatureFlags::GetInstance().GetFlags().enable_cancellation_flag) { + return; + } + auto item = cancellation_flags_.find(endpoint_id); if (item != cancellation_flags_.end()) { return; diff --git a/cpp/core/internal/client_proxy_test.cc b/cpp/core/internal/client_proxy_test.cc index 2a1b1735..af22940b 100644 --- a/cpp/core/internal/client_proxy_test.cc +++ b/cpp/core/internal/client_proxy_test.cc @@ -20,6 +20,8 @@ #include "core/options.h" #include "core/strategy.h" #include "platform/base/byte_array.h" +#include "platform/base/feature_flags.h" +#include "platform/base/medium_environment.h" #include "gmock/gmock.h" #include "gtest/gtest.h" #include "absl/container/flat_hash_set.h" @@ -30,10 +32,20 @@ namespace nearby { namespace connections { namespace { +using FeatureFlags = FeatureFlags::Flags; using ::testing::MockFunction; using ::testing::StrictMock; -class ClientProxyTest : public testing::Test { +constexpr FeatureFlags kTestCases[] = { + FeatureFlags{ + .enable_cancellation_flag = true, + }, + FeatureFlags{ + .enable_cancellation_flag = false, + }, +}; + +class ClientProxyTest : public ::testing::TestWithParam { protected: struct MockDiscoveryListener { StrictMockHasPendingConnectionToEndpoint(endpoint.id)); - // Cancellation flag has been created and added into map. - EXPECT_FALSE(client->GetCancellationFlag(endpoint.id)->Cancelled()); } void OnDiscoveryConnectionLocalAccepted(ClientProxy* client, @@ -175,8 +185,6 @@ class ClientProxyTest : public testing::Test { const Endpoint& endpoint) { EXPECT_CALL(mock_discovery_connection_.disconnected_cb, Call).Times(1); client->OnDisconnected(endpoint.id, true); - // The Cancelled is always true as the default flag being returned. - EXPECT_TRUE(client->GetCancellationFlag(endpoint.id)->Cancelled()); } void OnPayload(ClientProxy* client, const Endpoint& endpoint) { @@ -231,6 +239,106 @@ class ClientProxyTest : public testing::Test { ConnectionOptions connection_options_; }; +TEST_P(ClientProxyTest, CanCancelEndpoint) { + FeatureFlags feature_flags = GetParam(); + MediumEnvironment::Instance().SetFeatureFlags(feature_flags); + + Endpoint advertising_endpoint = + StartAdvertising(&client1_, advertising_connection_listener_); + StartDiscovery(&client2_, discovery_listener_); + OnDiscoveryEndpointFound(&client2_, advertising_endpoint); + OnDiscoveryConnectionInitiated(&client2_, advertising_endpoint); + + EXPECT_FALSE( + client2_.GetCancellationFlag(advertising_endpoint.id)->Cancelled()); + + client2_.CancelEndpoint(advertising_endpoint.id); + + // If FeatureFlag is disabled, Cancelled is false as no-op. + if (!feature_flags.enable_cancellation_flag) { + EXPECT_FALSE( + client2_.GetCancellationFlag(advertising_endpoint.id)->Cancelled()); + } else { + // The Cancelled is always true as the default flag being returned. + EXPECT_TRUE( + client2_.GetCancellationFlag(advertising_endpoint.id)->Cancelled()); + } +} + +TEST_P(ClientProxyTest, CanCancelAllEndpoints) { + FeatureFlags feature_flags = GetParam(); + MediumEnvironment::Instance().SetFeatureFlags(feature_flags); + + Endpoint advertising_endpoint = + StartAdvertising(&client1_, advertising_connection_listener_); + StartDiscovery(&client2_, discovery_listener_); + OnDiscoveryEndpointFound(&client2_, advertising_endpoint); + OnDiscoveryConnectionInitiated(&client2_, advertising_endpoint); + + EXPECT_FALSE( + client2_.GetCancellationFlag(advertising_endpoint.id)->Cancelled()); + + client2_.CancelAllEndpoints(); + + // If FeatureFlag is disabled, Cancelled is false as no-op. + if (!feature_flags.enable_cancellation_flag) { + EXPECT_FALSE( + client2_.GetCancellationFlag(advertising_endpoint.id)->Cancelled()); + } else { + // The Cancelled is always true as the default flag being returned. + EXPECT_TRUE( + client2_.GetCancellationFlag(advertising_endpoint.id)->Cancelled()); + } +} + +TEST_P(ClientProxyTest, CanCancelAllEndpointsWithDifferentEndpoint) { + FeatureFlags feature_flags = GetParam(); + MediumEnvironment::Instance().SetFeatureFlags(feature_flags); + + ConnectionListener advertising_connection_listener_2; + ConnectionListener advertising_connection_listener_3; + ClientProxy client3; + + StartDiscovery(&client1_, discovery_listener_); + Endpoint advertising_endpoint_2 = + StartAdvertising(&client2_, advertising_connection_listener_2); + Endpoint advertising_endpoint_3 = + StartAdvertising(&client3, advertising_connection_listener_3); + OnDiscoveryEndpointFound(&client1_, advertising_endpoint_2); + OnDiscoveryConnectionInitiated(&client1_, advertising_endpoint_2); + OnDiscoveryEndpointFound(&client1_, advertising_endpoint_3); + OnDiscoveryConnectionInitiated(&client1_, advertising_endpoint_3); + + // The CancellationFlag of endpoint_2 and endpoint_3 have been added. Default + // Cancelled is false. + EXPECT_FALSE( + client1_.GetCancellationFlag(advertising_endpoint_2.id)->Cancelled()); + EXPECT_FALSE( + client1_.GetCancellationFlag(advertising_endpoint_3.id)->Cancelled()); + + client1_.CancelAllEndpoints(); + + if (!feature_flags.enable_cancellation_flag) { + // The CancellationFlag of endpoint_2 and endpoint_3 will not be removed + // since it is not added. The default flag returned as Cancelled being true, + // but Cancelled requested is false since the FeatureFlag is off. + EXPECT_FALSE( + client1_.GetCancellationFlag(advertising_endpoint_2.id)->Cancelled()); + EXPECT_FALSE( + client1_.GetCancellationFlag(advertising_endpoint_3.id)->Cancelled()); + } else { + // Expect the CancellationFlag of endpoint_2 and endpoint_3 has been + // removed. The Cancelled is always true as the default flag being returned. + EXPECT_TRUE( + client1_.GetCancellationFlag(advertising_endpoint_2.id)->Cancelled()); + EXPECT_TRUE( + client1_.GetCancellationFlag(advertising_endpoint_3.id)->Cancelled()); + } +} + +INSTANTIATE_TEST_SUITE_P(ParametrisedClientProxyTest, ClientProxyTest, + ::testing::ValuesIn(kTestCases)); + TEST_F(ClientProxyTest, ConstructorDestructorWorks) { SUCCEED(); } TEST_F(ClientProxyTest, ClientIdIsUnique) { @@ -371,72 +479,6 @@ TEST_F(ClientProxyTest, OnPayloadProgressChangesState) { OnPayloadProgress(&client2_, advertising_endpoint); } -TEST_F(ClientProxyTest, CanCancelEndpoint) { - Endpoint advertising_endpoint = - StartAdvertising(&client1_, advertising_connection_listener_); - StartDiscovery(&client2_, discovery_listener_); - OnDiscoveryEndpointFound(&client2_, advertising_endpoint); - OnDiscoveryConnectionInitiated(&client2_, advertising_endpoint); - - EXPECT_FALSE( - client2_.GetCancellationFlag(advertising_endpoint.id)->Cancelled()); - - client2_.CancelEndpoint(advertising_endpoint.id); - - // The Cancelled is always true as the default flag being returned. - EXPECT_TRUE( - client2_.GetCancellationFlag(advertising_endpoint.id)->Cancelled()); -} - -TEST_F(ClientProxyTest, CanCancelAllEndpoints) { - Endpoint advertising_endpoint = - StartAdvertising(&client1_, advertising_connection_listener_); - StartDiscovery(&client2_, discovery_listener_); - OnDiscoveryEndpointFound(&client2_, advertising_endpoint); - OnDiscoveryConnectionInitiated(&client2_, advertising_endpoint); - - EXPECT_FALSE( - client2_.GetCancellationFlag(advertising_endpoint.id)->Cancelled()); - - client2_.CancelAllEndpoints(); - - // The Cancelled is always true as the default flag being returned. - EXPECT_TRUE( - client2_.GetCancellationFlag(advertising_endpoint.id)->Cancelled()); -} - -TEST_F(ClientProxyTest, CanCancelAllEndpointsWithDifferentEndpoint) { - ConnectionListener advertising_connection_listener_2; - ConnectionListener advertising_connection_listener_3; - ClientProxy client3; - - StartDiscovery(&client1_, discovery_listener_); - Endpoint advertising_endpoint_2 = - StartAdvertising(&client2_, advertising_connection_listener_2); - Endpoint advertising_endpoint_3 = - StartAdvertising(&client3, advertising_connection_listener_3); - OnDiscoveryEndpointFound(&client1_, advertising_endpoint_2); - OnDiscoveryConnectionInitiated(&client1_, advertising_endpoint_2); - OnDiscoveryEndpointFound(&client1_, advertising_endpoint_3); - OnDiscoveryConnectionInitiated(&client1_, advertising_endpoint_3); - - // The CancellationFlag of endpoint_2 and endpoint_3 have been added. Default - // Cancelled is false. - EXPECT_FALSE( - client1_.GetCancellationFlag(advertising_endpoint_2.id)->Cancelled()); - EXPECT_FALSE( - client1_.GetCancellationFlag(advertising_endpoint_3.id)->Cancelled()); - - client1_.CancelAllEndpoints(); - - // Expect the CancellationFlag of endpoint_2 and endpoint_3 has been removed. - // The Cancelled is always true as the default flag being returned. - EXPECT_TRUE( - client1_.GetCancellationFlag(advertising_endpoint_2.id)->Cancelled()); - EXPECT_TRUE( - client1_.GetCancellationFlag(advertising_endpoint_3.id)->Cancelled()); -} - } // namespace } // namespace connections } // namespace nearby diff --git a/cpp/core/internal/mediums/ble.cc b/cpp/core/internal/mediums/ble.cc index bda74756..a6a72486 100644 --- a/cpp/core/internal/mediums/ble.cc +++ b/cpp/core/internal/mediums/ble.cc @@ -335,7 +335,12 @@ BleSocket Ble::Connect(BlePeripheral& peripheral, const std::string& service_id, return socket; } - socket = medium_.Connect(peripheral, service_id); + if (cancellation_flag->Cancelled()) { + NEARBY_LOGS(INFO) << "Can't create client BLE socket due to cancel."; + return socket; + } + + socket = medium_.Connect(peripheral, service_id, cancellation_flag); if (!socket.IsValid()) { NEARBY_LOGS(INFO) << "Failed to Connect via BLE [service=" << service_id << "]"; diff --git a/cpp/core/internal/mediums/ble_test.cc b/cpp/core/internal/mediums/ble_test.cc index b420ae40..3393ac66 100644 --- a/cpp/core/internal/mediums/ble_test.cc +++ b/cpp/core/internal/mediums/ble_test.cc @@ -29,12 +29,23 @@ namespace nearby { namespace connections { namespace { +using FeatureFlags = FeatureFlags::Flags; + +constexpr FeatureFlags kTestCases[] = { + FeatureFlags{ + .enable_cancellation_flag = true, + }, + FeatureFlags{ + .enable_cancellation_flag = false, + }, +}; + constexpr absl::Duration kWaitDuration = absl::Milliseconds(1000); constexpr absl::string_view kServiceID{"com.google.location.nearby.apps.test"}; constexpr absl::string_view kAdvertisementString{"\x0a\x0b\x0c\x0d"}; constexpr absl::string_view kFastAdvertisementServiceUuid{"\xf3\xfe"}; -class BleTest : public ::testing::Test { +class BleTest : public ::testing::TestWithParam { protected: using DiscoveredPeripheralCallback = BleMedium::DiscoveredPeripheralCallback; @@ -43,6 +54,123 @@ class BleTest : public ::testing::Test { MediumEnvironment& env_{MediumEnvironment::Instance()}; }; +TEST_P(BleTest, CanStartAcceptingConnectionsAndConnect) { + FeatureFlags feature_flags = GetParam(); + env_.SetFeatureFlags(feature_flags); + env_.Start(); + BluetoothRadio radio_a; + BluetoothRadio radio_b; + Ble ble_a{radio_a}; + Ble ble_b{radio_b}; + radio_a.Enable(); + radio_b.Enable(); + std::string service_id(kServiceID); + ByteArray advertisement_bytes{std::string(kAdvertisementString)}; + std::string fast_advertisement_service_uuid(kFastAdvertisementServiceUuid); + CountDownLatch found_latch(1); + CountDownLatch accept_latch(1); + + ble_a.StartAdvertising(service_id, advertisement_bytes, + fast_advertisement_service_uuid); + ble_a.StartAcceptingConnections( + service_id, + { + .accepted_cb = [&accept_latch]( + BleSocket socket, + const std::string&) { accept_latch.CountDown(); }, + }); + BlePeripheral discovered_peripheral; + ble_b.StartScanning( + service_id, fast_advertisement_service_uuid, + { + .peripheral_discovered_cb = + [&found_latch, &discovered_peripheral]( + BlePeripheral& peripheral, const std::string& service_id, + const ByteArray& advertisement_bytes, + bool fast_advertisement) { + discovered_peripheral = peripheral; + NEARBY_LOG( + INFO, + "Discovered peripheral=%p [impl=%p], fast advertisement=%d", + &peripheral, &peripheral.GetImpl(), fast_advertisement); + found_latch.CountDown(); + }, + }); + + EXPECT_TRUE(found_latch.Await(kWaitDuration).result()); + ASSERT_TRUE(discovered_peripheral.IsValid()); + CancellationFlag flag; + BleSocket socket = ble_b.Connect(discovered_peripheral, service_id, &flag); + EXPECT_TRUE(accept_latch.Await(kWaitDuration).result()); + EXPECT_TRUE(socket.IsValid()); + ble_b.StopScanning(service_id); + ble_a.StopAdvertising(service_id); + env_.Stop(); +} + +TEST_P(BleTest, CanCancelConnect) { + FeatureFlags feature_flags = GetParam(); + env_.SetFeatureFlags(feature_flags); + env_.Start(); + BluetoothRadio radio_a; + BluetoothRadio radio_b; + Ble ble_a{radio_a}; + Ble ble_b{radio_b}; + radio_a.Enable(); + radio_b.Enable(); + std::string service_id(kServiceID); + ByteArray advertisement_bytes{std::string(kAdvertisementString)}; + std::string fast_advertisement_service_uuid(kFastAdvertisementServiceUuid); + CountDownLatch found_latch(1); + CountDownLatch accept_latch(1); + + ble_a.StartAdvertising(service_id, advertisement_bytes, + fast_advertisement_service_uuid); + ble_a.StartAcceptingConnections( + service_id, + { + .accepted_cb = [&accept_latch]( + BleSocket socket, + const std::string&) { accept_latch.CountDown(); }, + }); + BlePeripheral discovered_peripheral; + ble_b.StartScanning( + service_id, fast_advertisement_service_uuid, + { + .peripheral_discovered_cb = + [&found_latch, &discovered_peripheral]( + BlePeripheral& peripheral, const std::string& service_id, + const ByteArray& advertisement_bytes, + bool fast_advertisement) { + discovered_peripheral = peripheral; + NEARBY_LOG( + INFO, + "Discovered peripheral=%p [impl=%p], fast advertisement=%d", + &peripheral, &peripheral.GetImpl(), fast_advertisement); + found_latch.CountDown(); + }, + }); + + EXPECT_TRUE(found_latch.Await(kWaitDuration).result()); + ASSERT_TRUE(discovered_peripheral.IsValid()); + CancellationFlag flag(true); + BleSocket socket = ble_b.Connect(discovered_peripheral, service_id, &flag); + // If FeatureFlag is disabled, Cancelled is false as no-op. + if (!feature_flags.enable_cancellation_flag) { + EXPECT_TRUE(accept_latch.Await(kWaitDuration).result()); + EXPECT_TRUE(socket.IsValid()); + } else { + EXPECT_FALSE(accept_latch.Await(kWaitDuration).result()); + EXPECT_FALSE(socket.IsValid()); + } + ble_b.StopScanning(service_id); + ble_a.StopAdvertising(service_id); + env_.Stop(); +} + +INSTANTIATE_TEST_SUITE_P(ParametrisedBleTest, BleTest, + ::testing::ValuesIn(kTestCases)); + TEST_F(BleTest, CanConstructValidObject) { env_.Start(); BluetoothRadio radio_a; @@ -129,58 +257,6 @@ TEST_F(BleTest, CanStartDiscovery) { env_.Stop(); } -TEST_F(BleTest, CanStartAcceptingConnectionsAndConnect) { - env_.Start(); - BluetoothRadio radio_a; - BluetoothRadio radio_b; - Ble ble_a{radio_a}; - Ble ble_b{radio_b}; - radio_a.Enable(); - radio_b.Enable(); - std::string service_id(kServiceID); - ByteArray advertisement_bytes{std::string(kAdvertisementString)}; - std::string fast_advertisement_service_uuid(kFastAdvertisementServiceUuid); - CountDownLatch found_latch(1); - CountDownLatch accept_latch(1); - - ble_a.StartAdvertising(service_id, advertisement_bytes, - fast_advertisement_service_uuid); - ble_a.StartAcceptingConnections( - service_id, - { - .accepted_cb = [&accept_latch]( - BleSocket socket, - const std::string&) { accept_latch.CountDown(); }, - }); - BlePeripheral discovered_peripheral; - ble_b.StartScanning( - service_id, fast_advertisement_service_uuid, - { - .peripheral_discovered_cb = - [&found_latch, &discovered_peripheral]( - BlePeripheral& peripheral, const std::string& service_id, - const ByteArray& advertisement_bytes, - bool fast_advertisement) { - discovered_peripheral = peripheral; - NEARBY_LOG( - INFO, - "Discovered peripheral=%p [impl=%p], fast advertisement=%d", - &peripheral, &peripheral.GetImpl(), fast_advertisement); - found_latch.CountDown(); - }, - }); - - EXPECT_TRUE(found_latch.Await(kWaitDuration).result()); - ASSERT_TRUE(discovered_peripheral.IsValid()); - CancellationFlag flag; - BleSocket socket = ble_b.Connect(discovered_peripheral, service_id, &flag); - EXPECT_TRUE(accept_latch.Await(kWaitDuration).result()); - EXPECT_TRUE(socket.IsValid()); - ble_b.StopScanning(service_id); - ble_a.StopAdvertising(service_id); - env_.Stop(); -} - } // namespace } // namespace connections } // namespace nearby diff --git a/cpp/core/internal/mediums/bluetooth_classic.cc b/cpp/core/internal/mediums/bluetooth_classic.cc index beff1280..ad0f49ec 100644 --- a/cpp/core/internal/mediums/bluetooth_classic.cc +++ b/cpp/core/internal/mediums/bluetooth_classic.cc @@ -350,7 +350,8 @@ BluetoothSocket BluetoothClassic::Connect(BluetoothDevice& bluetooth_device, CancellationFlag* cancellation_flag) { for (int attempts_count = 0; attempts_count < kConnectAttemptsLimit; attempts_count++) { - auto wrapper_result = AttemptToConnect(bluetooth_device, service_name); + auto wrapper_result = + AttemptToConnect(bluetooth_device, service_name, cancellation_flag); if (wrapper_result.IsValid()) { return wrapper_result; } @@ -359,7 +360,8 @@ BluetoothSocket BluetoothClassic::Connect(BluetoothDevice& bluetooth_device, } BluetoothSocket BluetoothClassic::AttemptToConnect( - BluetoothDevice& bluetooth_device, const std::string& service_name) { + BluetoothDevice& bluetooth_device, const std::string& service_name, + CancellationFlag* cancellation_flag) { MutexLock lock(&mutex_); NEARBY_LOG(INFO, "BluetoothClassic::Connect: device=%p", &bluetooth_device); // Socket to return. To allow for NRVO to work, it has to be a single object. @@ -386,8 +388,14 @@ BluetoothSocket BluetoothClassic::AttemptToConnect( return socket; } + if (cancellation_flag->Cancelled()) { + NEARBY_LOGS(INFO) << "Can't create client BT socket due to cancel."; + return socket; + } + socket = medium_.ConnectToService(bluetooth_device, - GenerateUuidFromString(service_name)); + GenerateUuidFromString(service_name), + cancellation_flag); if (!socket.IsValid()) { NEARBY_LOG(INFO, "Failed to Connect via BT [service=%s]", service_name.c_str()); diff --git a/cpp/core/internal/mediums/bluetooth_classic.h b/cpp/core/internal/mediums/bluetooth_classic.h index 15a8d2d7..45c6626e 100644 --- a/cpp/core/internal/mediums/bluetooth_classic.h +++ b/cpp/core/internal/mediums/bluetooth_classic.h @@ -170,7 +170,8 @@ class BluetoothClassic { // Returns socket instance. On success, BluetoothSocket.IsValid() return true. // Called by client. BluetoothSocket AttemptToConnect(BluetoothDevice& bluetooth_device, - const std::string& service_name); + const std::string& service_name, + CancellationFlag* cancellation_flag); mutable Mutex mutex_; BluetoothRadio& radio_ ABSL_GUARDED_BY(mutex_); diff --git a/cpp/core/internal/mediums/bluetooth_classic_test.cc b/cpp/core/internal/mediums/bluetooth_classic_test.cc index 02945400..8772c578 100644 --- a/cpp/core/internal/mediums/bluetooth_classic_test.cc +++ b/cpp/core/internal/mediums/bluetooth_classic_test.cc @@ -31,9 +31,20 @@ namespace nearby { namespace connections { namespace { +using FeatureFlags = FeatureFlags::Flags; + +constexpr FeatureFlags kTestCases[] = { + FeatureFlags{ + .enable_cancellation_flag = true, + }, + FeatureFlags{ + .enable_cancellation_flag = false, + }, +}; + constexpr absl::Duration kWaitDuration = absl::Milliseconds(1000); -class BluetoothClassicTest : public ::testing::Test { +class BluetoothClassicTest : public ::testing::TestWithParam { protected: using DiscoveryCallback = BluetoothClassicMedium::DiscoveryCallback; @@ -72,6 +83,125 @@ class BluetoothClassicTest : public ::testing::Test { std::unique_ptr bt_b_; }; +TEST_P(BluetoothClassicTest, CanConnect) { + FeatureFlags feature_flags = GetParam(); + env_.SetFeatureFlags(feature_flags); + + constexpr absl::string_view kDeviceName{"Simulated BT device #1"}; + constexpr absl::string_view kServiceName{"service name"}; + + BluetoothRadio& radio_for_client = *radio_a_; + BluetoothRadio& radio_for_server = *radio_b_; + BluetoothClassic& bt_client = *bt_a_; + BluetoothClassic& bt_server = *bt_b_; + + EXPECT_TRUE(radio_for_client.IsEnabled()); + EXPECT_TRUE(radio_for_server.IsEnabled()); + + EXPECT_TRUE(bt_server.TurnOnDiscoverability(std::string(kDeviceName))); + EXPECT_EQ(radio_for_server.GetBluetoothAdapter().GetName(), + std::string(kDeviceName)); + CountDownLatch latch(1); + BluetoothDevice discovered_device; + EXPECT_TRUE(bt_client.StartDiscovery({ + .device_discovered_cb = + [&latch, &discovered_device](BluetoothDevice& device) { + discovered_device = device; + NEARBY_LOG(INFO, "Discovered device=%p [impl=%p]", &device, + &device.GetImpl()); + latch.CountDown(); + }, + })); + EXPECT_TRUE(latch.Await(kWaitDuration).result()); + EXPECT_TRUE(bt_server.TurnOffDiscoverability()); + ASSERT_TRUE(discovered_device.IsValid()); + BluetoothSocket socket_for_server; + CountDownLatch accept_latch(1); + EXPECT_TRUE(bt_server.StartAcceptingConnections( + std::string(kServiceName), + { + .accepted_cb = + [&socket_for_server, &accept_latch](BluetoothSocket socket) { + socket_for_server = std::move(socket); + accept_latch.CountDown(); + }, + })); + CancellationFlag flag; + BluetoothSocket socket_for_client = + bt_client.Connect(discovered_device, std::string(kServiceName), &flag); + EXPECT_TRUE(accept_latch.Await(kWaitDuration).result()); + EXPECT_TRUE(bt_server.StopAcceptingConnections(std::string(kServiceName))); + EXPECT_TRUE(socket_for_server.IsValid()); + EXPECT_TRUE(socket_for_client.IsValid()); + EXPECT_TRUE(socket_for_server.GetRemoteDevice().IsValid()); + EXPECT_TRUE(socket_for_client.GetRemoteDevice().IsValid()); +} + +TEST_P(BluetoothClassicTest, CanCancelConnect) { + FeatureFlags feature_flags = GetParam(); + env_.SetFeatureFlags(feature_flags); + + constexpr absl::string_view kDeviceName{"Simulated BT device #1"}; + constexpr absl::string_view kServiceName{"service name"}; + + BluetoothRadio& radio_for_client = *radio_a_; + BluetoothRadio& radio_for_server = *radio_b_; + BluetoothClassic& bt_client = *bt_a_; + BluetoothClassic& bt_server = *bt_b_; + + EXPECT_TRUE(radio_for_client.IsEnabled()); + EXPECT_TRUE(radio_for_server.IsEnabled()); + + EXPECT_TRUE(bt_server.TurnOnDiscoverability(std::string(kDeviceName))); + EXPECT_EQ(radio_for_server.GetBluetoothAdapter().GetName(), + std::string(kDeviceName)); + CountDownLatch latch(1); + BluetoothDevice discovered_device; + EXPECT_TRUE(bt_client.StartDiscovery({ + .device_discovered_cb = + [&latch, &discovered_device](BluetoothDevice& device) { + discovered_device = device; + NEARBY_LOG(INFO, "Discovered device=%p [impl=%p]", &device, + &device.GetImpl()); + latch.CountDown(); + }, + })); + EXPECT_TRUE(latch.Await(kWaitDuration).result()); + EXPECT_TRUE(bt_server.TurnOffDiscoverability()); + ASSERT_TRUE(discovered_device.IsValid()); + BluetoothSocket socket_for_server; + CountDownLatch accept_latch(1); + EXPECT_TRUE(bt_server.StartAcceptingConnections( + std::string(kServiceName), + { + .accepted_cb = + [&socket_for_server, &accept_latch](BluetoothSocket socket) { + socket_for_server = std::move(socket); + accept_latch.CountDown(); + }, + })); + CancellationFlag flag(true); + BluetoothSocket socket_for_client = + bt_client.Connect(discovered_device, std::string(kServiceName), &flag); + // If FeatureFlag is disabled, Cancelled is false as no-op. + if (!feature_flags.enable_cancellation_flag) { + EXPECT_TRUE(accept_latch.Await(kWaitDuration).result()); + EXPECT_TRUE(bt_server.StopAcceptingConnections(std::string(kServiceName))); + EXPECT_TRUE(socket_for_server.IsValid()); + EXPECT_TRUE(socket_for_client.IsValid()); + EXPECT_TRUE(socket_for_server.GetRemoteDevice().IsValid()); + EXPECT_TRUE(socket_for_client.GetRemoteDevice().IsValid()); + } else { + EXPECT_FALSE(accept_latch.Await(kWaitDuration).result()); + EXPECT_TRUE(bt_server.StopAcceptingConnections(std::string(kServiceName))); + EXPECT_FALSE(socket_for_server.IsValid()); + EXPECT_FALSE(socket_for_client.IsValid()); + } +} + +INSTANTIATE_TEST_SUITE_P(ParametrisedBluetoothClassicTest, BluetoothClassicTest, + ::testing::ValuesIn(kTestCases)); + TEST_F(BluetoothClassicTest, CanConstructValidObject) { EXPECT_TRUE(bt_a_->IsMediumValid()); EXPECT_TRUE(bt_a_->IsAdapterValid()); @@ -154,57 +284,6 @@ TEST_F(BluetoothClassicTest, CanStartAcceptingConnections) { EXPECT_TRUE(bt_server.StopAcceptingConnections(std::string(kServiceName))); } -TEST_F(BluetoothClassicTest, CanConnect) { - constexpr absl::string_view kDeviceName{"Simulated BT device #1"}; - constexpr absl::string_view kServiceName{"service name"}; - - BluetoothRadio& radio_for_client = *radio_a_; - BluetoothRadio& radio_for_server = *radio_b_; - BluetoothClassic& bt_client = *bt_a_; - BluetoothClassic& bt_server = *bt_b_; - - EXPECT_TRUE(radio_for_client.IsEnabled()); - EXPECT_TRUE(radio_for_server.IsEnabled()); - - EXPECT_TRUE(bt_server.TurnOnDiscoverability(std::string(kDeviceName))); - EXPECT_EQ(radio_for_server.GetBluetoothAdapter().GetName(), - std::string(kDeviceName)); - CountDownLatch latch(1); - BluetoothDevice discovered_device; - EXPECT_TRUE(bt_client.StartDiscovery({ - .device_discovered_cb = - [&latch, &discovered_device](BluetoothDevice& device) { - discovered_device = device; - NEARBY_LOG(INFO, "Discovered device=%p [impl=%p]", &device, - &device.GetImpl()); - latch.CountDown(); - }, - })); - EXPECT_TRUE(latch.Await(kWaitDuration).result()); - EXPECT_TRUE(bt_server.TurnOffDiscoverability()); - ASSERT_TRUE(discovered_device.IsValid()); - BluetoothSocket socket_for_server; - CountDownLatch accept_latch(1); - EXPECT_TRUE(bt_server.StartAcceptingConnections( - std::string(kServiceName), - { - .accepted_cb = - [&socket_for_server, &accept_latch](BluetoothSocket socket) { - socket_for_server = std::move(socket); - accept_latch.CountDown(); - }, - })); - CancellationFlag flag; - BluetoothSocket socket_for_client = - bt_client.Connect(discovered_device, std::string(kServiceName), &flag); - EXPECT_TRUE(accept_latch.Await(kWaitDuration).result()); - EXPECT_TRUE(bt_server.StopAcceptingConnections(std::string(kServiceName))); - EXPECT_TRUE(socket_for_server.IsValid()); - EXPECT_TRUE(socket_for_client.IsValid()); - EXPECT_TRUE(socket_for_server.GetRemoteDevice().IsValid()); - EXPECT_TRUE(socket_for_client.GetRemoteDevice().IsValid()); -} - } // namespace } // namespace connections } // namespace nearby diff --git a/cpp/core/internal/mediums/webrtc.cc b/cpp/core/internal/mediums/webrtc.cc index 8447f9ea..38d46127 100644 --- a/cpp/core/internal/mediums/webrtc.cc +++ b/cpp/core/internal/mediums/webrtc.cc @@ -212,8 +212,8 @@ WebRtcSocketWrapper WebRtc::Connect(const std::string& service_id, CancellationFlag* cancellation_flag) { for (int attempts_count = 0; attempts_count < kConnectAttemptsLimit; attempts_count++) { - auto wrapper_result = - AttemptToConnect(service_id, remote_peer_id, location_hint); + auto wrapper_result = AttemptToConnect(service_id, remote_peer_id, + location_hint, cancellation_flag); if (wrapper_result.IsValid()) { return wrapper_result; } @@ -223,7 +223,7 @@ WebRtcSocketWrapper WebRtc::Connect(const std::string& service_id, WebRtcSocketWrapper WebRtc::AttemptToConnect( const std::string& service_id, const PeerId& remote_peer_id, - const LocationHint& location_hint) { + const LocationHint& location_hint, CancellationFlag* cancellation_flag) { ConnectionRequestInfo info = ConnectionRequestInfo(); info.self_peer_id = PeerId::FromRandom(); Future socket_future = info.socket_future; @@ -238,6 +238,11 @@ WebRtcSocketWrapper WebRtc::AttemptToConnect( return WebRtcSocketWrapper(); } + if (cancellation_flag->Cancelled()) { + NEARBY_LOGS(INFO) << "Cannot connect with WebRtc due to cancel."; + return WebRtcSocketWrapper(); + } + // Create a new ConnectionFlow for this connection attempt. std::unique_ptr connection_flow = CreateConnectionFlow(service_id, remote_peer_id); diff --git a/cpp/core/internal/mediums/webrtc.h b/cpp/core/internal/mediums/webrtc.h index 494695aa..06541980 100644 --- a/cpp/core/internal/mediums/webrtc.h +++ b/cpp/core/internal/mediums/webrtc.h @@ -143,7 +143,8 @@ class WebRtc { // Runs on @MainThread. WebRtcSocketWrapper AttemptToConnect(const std::string& service_id, const PeerId& peer_id, - const LocationHint& location_hint) + const LocationHint& location_hint, + CancellationFlag* cancellation_flag) ABSL_LOCKS_EXCLUDED(mutex_); // Returns if the device is accepting connection with specific service id. diff --git a/cpp/core/internal/mediums/webrtc_test.cc b/cpp/core/internal/mediums/webrtc_test.cc index 4eb7509b..535a11b9 100644 --- a/cpp/core/internal/mediums/webrtc_test.cc +++ b/cpp/core/internal/mediums/webrtc_test.cc @@ -28,7 +28,18 @@ namespace mediums { namespace { -class WebRtcTest : public ::testing::Test { +using FeatureFlags = FeatureFlags::Flags; + +constexpr FeatureFlags kTestCases[] = { + FeatureFlags{ + .enable_cancellation_flag = true, + }, + FeatureFlags{ + .enable_cancellation_flag = false, + }, +}; + +class WebRtcTest : public ::testing::TestWithParam { protected: WebRtcTest() { MediumEnvironment::Instance().Stop(); @@ -36,6 +47,87 @@ class WebRtcTest : public ::testing::Test { } }; +// Tests the flow when the two devices exchange SDP messages and connect to each +// other but the signaling channel is closed before sending the data. +TEST_P(WebRtcTest, ConnectBothDevices_ShutdownSignaling_SendData) { + FeatureFlags feature_flags = GetParam(); + MediumEnvironment::Instance().SetFeatureFlags(feature_flags); + WebRtc receiver, sender; + WebRtcSocketWrapper receiver_socket, sender_socket; + const PeerId self_id("self_id"); + const std::string service_id("NearbySharing"); + LocationHint location_hint; + Future connected; + ByteArray message("message xyz"); + + receiver.StartAcceptingConnections( + service_id, self_id, location_hint, + {[&receiver_socket, connected](WebRtcSocketWrapper wrapper) mutable { + receiver_socket = wrapper; + connected.Set(receiver_socket.IsValid()); + }}); + + CancellationFlag flag; + sender_socket = sender.Connect(service_id, self_id, location_hint, &flag); + EXPECT_TRUE(sender_socket.IsValid()); + + ExceptionOr devices_connected = connected.Get(); + ASSERT_TRUE(devices_connected.ok()); + EXPECT_TRUE(devices_connected.result()); + + // Only shuts down signaling channel. + receiver.StopAcceptingConnections(service_id); + + sender_socket.GetOutputStream().Write(message); + ExceptionOr received_msg = + receiver_socket.GetInputStream().Read(/*size=*/32); + ASSERT_TRUE(received_msg.ok()); + EXPECT_EQ(message, received_msg.result()); +} + +TEST_P(WebRtcTest, CanCancelConnect) { + FeatureFlags feature_flags = GetParam(); + MediumEnvironment::Instance().SetFeatureFlags(feature_flags); + WebRtc receiver, sender; + WebRtcSocketWrapper receiver_socket, sender_socket; + const PeerId self_id("self_id"); + const std::string service_id("NearbySharing"); + LocationHint location_hint; + Future connected; + ByteArray message("message"); + + receiver.StartAcceptingConnections( + service_id, self_id, location_hint, + {[&receiver_socket, connected](WebRtcSocketWrapper wrapper) mutable { + receiver_socket = wrapper; + connected.Set(receiver_socket.IsValid()); + }}); + + CancellationFlag flag(true); + sender_socket = sender.Connect(service_id, self_id, location_hint, &flag); + // If FeatureFlag is disabled, Cancelled is false as no-op. + if (!feature_flags.enable_cancellation_flag) { + EXPECT_TRUE(sender_socket.IsValid()); + + ExceptionOr devices_connected = connected.Get(); + ASSERT_TRUE(devices_connected.ok()); + EXPECT_TRUE(devices_connected.result()); + + sender_socket.GetOutputStream().Write(message); + ExceptionOr received_msg = + receiver_socket.GetInputStream().Read(/*size=*/32); + ASSERT_TRUE(received_msg.ok()); + EXPECT_EQ(message, received_msg.result()); + + receiver_socket.Close(); + } else { + EXPECT_FALSE(sender_socket.IsValid()); + } +} + +INSTANTIATE_TEST_SUITE_P(ParametrisedWebRtcTest, WebRtcTest, + ::testing::ValuesIn(kTestCases)); + // Basic test to check that device is accepting connections when initialized. TEST_F(WebRtcTest, NotAcceptingConnections) { WebRtc webrtc; @@ -241,42 +333,6 @@ TEST_F(WebRtcTest, ConnectBothDevicesAndSendData) { receiver_socket.Close(); } -// Tests the flow when the two devices exchange SDP messages and connect to each -// other but the signaling channel is closed before sending the data. -TEST_F(WebRtcTest, ConnectBothDevices_ShutdownSignaling_SendData) { - WebRtc receiver, sender; - WebRtcSocketWrapper receiver_socket, sender_socket; - const PeerId self_id("self_id"); - const std::string service_id("NearbySharing"); - LocationHint location_hint; - Future connected; - ByteArray message("message xyz"); - - receiver.StartAcceptingConnections( - service_id, self_id, location_hint, - {[&receiver_socket, connected](WebRtcSocketWrapper wrapper) mutable { - receiver_socket = wrapper; - connected.Set(receiver_socket.IsValid()); - }}); - - CancellationFlag flag; - sender_socket = sender.Connect(service_id, self_id, location_hint, &flag); - EXPECT_TRUE(sender_socket.IsValid()); - - ExceptionOr devices_connected = connected.Get(); - ASSERT_TRUE(devices_connected.ok()); - EXPECT_TRUE(devices_connected.result()); - - // Only shuts down signaling channel. - receiver.StopAcceptingConnections(service_id); - - sender_socket.GetOutputStream().Write(message); - ExceptionOr received_msg = - receiver_socket.GetInputStream().Read(/*size=*/32); - ASSERT_TRUE(received_msg.ok()); - EXPECT_EQ(message, received_msg.result()); -} - TEST_F(WebRtcTest, Connect_NullPeerConnection) { using MockAcceptedCallback = testing::MockFunction; diff --git a/cpp/core/internal/mediums/wifi_lan.cc b/cpp/core/internal/mediums/wifi_lan.cc index 9f1e5487..782807f4 100644 --- a/cpp/core/internal/mediums/wifi_lan.cc +++ b/cpp/core/internal/mediums/wifi_lan.cc @@ -248,7 +248,12 @@ WifiLanSocket WifiLan::Connect(WifiLanService& wifi_lan_service, return socket; } - socket = medium_.Connect(wifi_lan_service, service_id); + if (cancellation_flag->Cancelled()) { + NEARBY_LOGS(INFO) << "Can't create client WifiLan socket due to cancel."; + return socket; + } + + socket = medium_.Connect(wifi_lan_service, service_id, cancellation_flag); if (!socket.IsValid()) { NEARBY_LOG(INFO, "Failed to Connect via WifiLan [service_id=%s]", service_id.c_str()); diff --git a/cpp/core/internal/mediums/wifi_lan_test.cc b/cpp/core/internal/mediums/wifi_lan_test.cc index 9f1bc63e..7b88ef63 100644 --- a/cpp/core/internal/mediums/wifi_lan_test.cc +++ b/cpp/core/internal/mediums/wifi_lan_test.cc @@ -29,6 +29,17 @@ namespace nearby { namespace connections { namespace { +using FeatureFlags = FeatureFlags::Flags; + +constexpr FeatureFlags kTestCases[] = { + FeatureFlags{ + .enable_cancellation_flag = true, + }, + FeatureFlags{ + .enable_cancellation_flag = false, + }, +}; + constexpr absl::Duration kWaitDuration = absl::Milliseconds(1000); constexpr absl::string_view kServiceID{"com.google.location.nearby.apps.test"}; constexpr absl::string_view kServiceInfoName{ @@ -36,7 +47,7 @@ constexpr absl::string_view kServiceInfoName{ constexpr absl::string_view kEndpointName{"Simulated endpoint name"}; constexpr absl::string_view kEndpointInfoKey{"n"}; -class WifiLanTest : public ::testing::Test { +class WifiLanTest : public ::testing::TestWithParam { protected: using DiscoveredServiceCallback = WifiLanMedium::DiscoveredServiceCallback; @@ -45,6 +56,117 @@ class WifiLanTest : public ::testing::Test { MediumEnvironment& env_{MediumEnvironment::Instance()}; }; +TEST_P(WifiLanTest, CanStartAcceptingConnectionsAndConnect) { + FeatureFlags feature_flags = GetParam(); + env_.SetFeatureFlags(feature_flags); + env_.Start(); + WifiLan wifi_lan_a; + WifiLan wifi_lan_b; + std::string service_id(kServiceID); + std::string service_info_name{kServiceInfoName}; + std::string endpoint_info_name{kEndpointName}; + CountDownLatch found_latch(1); + CountDownLatch accept_latch(1); + + NsdServiceInfo nsd_service_info; + nsd_service_info.SetServiceInfoName(service_info_name); + nsd_service_info.SetTxtRecord(std::string(kEndpointInfoKey), + endpoint_info_name); + wifi_lan_a.StartAdvertising(service_id, nsd_service_info); + wifi_lan_a.StartAcceptingConnections( + service_id, + { + .accepted_cb = [&accept_latch]( + WifiLanSocket socket, + absl::string_view) { accept_latch.CountDown(); }, + }); + WifiLanService discovered_service; + wifi_lan_b.StartDiscovery( + service_id, + { + .service_discovered_cb = + [&found_latch, &discovered_service]( + WifiLanService& service, absl::string_view service_id) { + discovered_service = service; + NEARBY_LOG(INFO, "Discovered service=%p [impl=%p]", &service, + &service.GetImpl()); + found_latch.CountDown(); + }, + }); + + EXPECT_TRUE(found_latch.Await(kWaitDuration).result()); + ASSERT_TRUE(discovered_service.IsValid()); + CancellationFlag flag; + WifiLanSocket socket = + wifi_lan_b.Connect(discovered_service, service_id, &flag); + EXPECT_TRUE(accept_latch.Await(kWaitDuration).result()); + EXPECT_TRUE(socket.IsValid()); + wifi_lan_b.StopDiscovery(service_id); + wifi_lan_a.StopAcceptingConnections(service_id); + wifi_lan_a.StopAdvertising(service_id); + env_.Stop(); +} + +TEST_P(WifiLanTest, CanCancelConnect) { + FeatureFlags feature_flags = GetParam(); + env_.SetFeatureFlags(feature_flags); + env_.Start(); + WifiLan wifi_lan_a; + WifiLan wifi_lan_b; + std::string service_id(kServiceID); + std::string service_info_name{kServiceInfoName}; + std::string endpoint_info_name{kEndpointName}; + CountDownLatch found_latch(1); + CountDownLatch accept_latch(1); + + NsdServiceInfo nsd_service_info; + nsd_service_info.SetServiceInfoName(service_info_name); + nsd_service_info.SetTxtRecord(std::string(kEndpointInfoKey), + endpoint_info_name); + wifi_lan_a.StartAdvertising(service_id, nsd_service_info); + wifi_lan_a.StartAcceptingConnections( + service_id, + { + .accepted_cb = [&accept_latch]( + WifiLanSocket socket, + absl::string_view) { accept_latch.CountDown(); }, + }); + WifiLanService discovered_service; + wifi_lan_b.StartDiscovery( + service_id, + { + .service_discovered_cb = + [&found_latch, &discovered_service]( + WifiLanService& service, absl::string_view service_id) { + discovered_service = service; + NEARBY_LOG(INFO, "Discovered service=%p [impl=%p]", &service, + &service.GetImpl()); + found_latch.CountDown(); + }, + }); + + EXPECT_TRUE(found_latch.Await(kWaitDuration).result()); + ASSERT_TRUE(discovered_service.IsValid()); + CancellationFlag flag(true); + WifiLanSocket socket = + wifi_lan_b.Connect(discovered_service, service_id, &flag); + // If FeatureFlag is disabled, Cancelled is false as no-op. + if (!feature_flags.enable_cancellation_flag) { + EXPECT_TRUE(accept_latch.Await(kWaitDuration).result()); + EXPECT_TRUE(socket.IsValid()); + } else { + EXPECT_FALSE(accept_latch.Await(kWaitDuration).result()); + EXPECT_FALSE(socket.IsValid()); + } + wifi_lan_b.StopDiscovery(service_id); + wifi_lan_a.StopAcceptingConnections(service_id); + wifi_lan_a.StopAdvertising(service_id); + env_.Stop(); +} + +INSTANTIATE_TEST_SUITE_P(ParametrisedWifiLanTest, WifiLanTest, + ::testing::ValuesIn(kTestCases)); + TEST_F(WifiLanTest, CanConstructValidObject) { env_.Start(); WifiLan wifi_lan_a; @@ -121,55 +243,6 @@ TEST_F(WifiLanTest, CanStartDiscovery) { env_.Stop(); } -TEST_F(WifiLanTest, CanStartAcceptingConnectionsAndConnect) { - env_.Start(); - WifiLan wifi_lan_a; - WifiLan wifi_lan_b; - std::string service_id(kServiceID); - std::string service_info_name{kServiceInfoName}; - std::string endpoint_info_name{kEndpointName}; - CountDownLatch found_latch(1); - CountDownLatch accept_latch(1); - - NsdServiceInfo nsd_service_info; - nsd_service_info.SetServiceInfoName(service_info_name); - nsd_service_info.SetTxtRecord(std::string(kEndpointInfoKey), - endpoint_info_name); - wifi_lan_a.StartAdvertising(service_id, nsd_service_info); - wifi_lan_a.StartAcceptingConnections( - service_id, - { - .accepted_cb = [&accept_latch]( - WifiLanSocket socket, - absl::string_view) { accept_latch.CountDown(); }, - }); - WifiLanService discovered_service; - wifi_lan_b.StartDiscovery( - service_id, - { - .service_discovered_cb = - [&found_latch, &discovered_service]( - WifiLanService& service, absl::string_view service_id) { - discovered_service = service; - NEARBY_LOG(INFO, "Discovered service=%p [impl=%p]", &service, - &service.GetImpl()); - found_latch.CountDown(); - }, - }); - - EXPECT_TRUE(found_latch.Await(kWaitDuration).result()); - ASSERT_TRUE(discovered_service.IsValid()); - CancellationFlag flag; - WifiLanSocket socket = - wifi_lan_b.Connect(discovered_service, service_id, &flag); - EXPECT_TRUE(accept_latch.Await(kWaitDuration).result()); - EXPECT_TRUE(socket.IsValid()); - wifi_lan_b.StopDiscovery(service_id); - wifi_lan_a.StopAcceptingConnections(service_id); - wifi_lan_a.StopAdvertising(service_id); - env_.Stop(); -} - } // namespace } // namespace connections } // namespace nearby diff --git a/cpp/core/internal/offline_simulation_user.cc b/cpp/core/internal/offline_simulation_user.cc index 538e31b8..e3271253 100644 --- a/cpp/core/internal/offline_simulation_user.cc +++ b/cpp/core/internal/offline_simulation_user.cc @@ -167,6 +167,7 @@ Status OfflineSimulationUser::RequestConnection(CountDownLatch* latch) { .disconnected_cb = absl::bind_front(&OfflineSimulationUser::OnEndpointDisconnect, this), }; + client_.AddCancellationFlag(discovered_.endpoint_id); return ctrl_.RequestConnection( &client_, discovered_.endpoint_id, { diff --git a/cpp/core/internal/p2p_cluster_pcp_handler_test.cc b/cpp/core/internal/p2p_cluster_pcp_handler_test.cc index 74b7e961..ec82464b 100644 --- a/cpp/core/internal/p2p_cluster_pcp_handler_test.cc +++ b/cpp/core/internal/p2p_cluster_pcp_handler_test.cc @@ -227,6 +227,7 @@ TEST_P(P2pClusterPcpHandlerTest, CanConnect) { EXPECT_TRUE(discover_latch.Await(absl::Milliseconds(1000)).result()); EXPECT_EQ(endpoint_name_a, std::string{discovered.endpoint_info}); + client_b_.AddCancellationFlag(discovered.endpoint_id); handler_b.RequestConnection( &client_b_, discovered.endpoint_id, { diff --git a/cpp/core/internal/simulation_user.cc b/cpp/core/internal/simulation_user.cc index cd66d29e..5c57a3d5 100644 --- a/cpp/core/internal/simulation_user.cc +++ b/cpp/core/internal/simulation_user.cc @@ -148,6 +148,7 @@ void SimulationUser::RequestConnection(CountDownLatch* latch) { .rejected_cb = absl::bind_front(&SimulationUser::OnConnectionRejected, this), }; + client_.AddCancellationFlag(discovered_.endpoint_id); EXPECT_TRUE( mgr_.RequestConnection(&client_, discovered_.endpoint_id, { diff --git a/cpp/platform/api/BUILD b/cpp/platform/api/BUILD index e3149177..e31a848a 100644 --- a/cpp/platform/api/BUILD +++ b/cpp/platform/api/BUILD @@ -67,6 +67,7 @@ cc_library( deps = [ "//proto/connections:offline_wire_formats_portable_proto", "//platform/base", + "//platform/base:cancellation_flag", "//absl/strings", "//absl/types:optional", "//webrtc/api:libjingle_peerconnection_api", diff --git a/cpp/platform/api/ble.h b/cpp/platform/api/ble.h index 264f7b13..c58df6db 100644 --- a/cpp/platform/api/ble.h +++ b/cpp/platform/api/ble.h @@ -17,6 +17,7 @@ #include "platform/api/bluetooth_classic.h" #include "platform/base/byte_array.h" +#include "platform/base/cancellation_flag.h" #include "platform/base/input_stream.h" #include "platform/base/output_stream.h" @@ -112,8 +113,9 @@ class BleMedium { // Connects to a BLE peripheral. // On success, returns a new BleSocket. // On error, returns nullptr. - virtual std::unique_ptr Connect(BlePeripheral& peripheral, - const std::string& service_id) = 0; + virtual std::unique_ptr Connect( + BlePeripheral& peripheral, const std::string& service_id, + CancellationFlag* cancellation_flag) = 0; }; } // namespace api diff --git a/cpp/platform/api/bluetooth_classic.h b/cpp/platform/api/bluetooth_classic.h index 29335edf..95361863 100644 --- a/cpp/platform/api/bluetooth_classic.h +++ b/cpp/platform/api/bluetooth_classic.h @@ -19,6 +19,7 @@ #include #include "platform/base/byte_array.h" +#include "platform/base/cancellation_flag.h" #include "platform/base/exception.h" #include "platform/base/input_stream.h" #include "platform/base/listeners.h" @@ -136,7 +137,8 @@ class BluetoothClassicMedium { // On success, returns a new BluetoothSocket. // On error, returns nullptr. virtual std::unique_ptr ConnectToService( - BluetoothDevice& remote_device, const std::string& service_uuid) = 0; + BluetoothDevice& remote_device, const std::string& service_uuid, + CancellationFlag* cancellation_flag) = 0; // https://developer.android.com/reference/android/bluetooth/BluetoothAdapter.html#listenUsingInsecureRfcommWithServiceRecord // diff --git a/cpp/platform/api/wifi_lan.h b/cpp/platform/api/wifi_lan.h index 27e68eb5..e4ae202b 100644 --- a/cpp/platform/api/wifi_lan.h +++ b/cpp/platform/api/wifi_lan.h @@ -18,6 +18,7 @@ #include #include "platform/base/byte_array.h" +#include "platform/base/cancellation_flag.h" #include "platform/base/input_stream.h" #include "platform/base/listeners.h" #include "platform/base/nsd_service_info.h" @@ -113,7 +114,8 @@ class WifiLanMedium { // On success, returns a new WifiLanSocket. // On error, returns nullptr. virtual std::unique_ptr Connect( - WifiLanService& wifi_lan_service, const std::string& service_id) = 0; + WifiLanService& wifi_lan_service, const std::string& service_id, + CancellationFlag* cancellation_flag) = 0; virtual WifiLanService* GetRemoteService(const std::string& ip_address, int port) = 0; diff --git a/cpp/platform/base/BUILD b/cpp/platform/base/BUILD index ac91f09a..55567885 100644 --- a/cpp/platform/base/BUILD +++ b/cpp/platform/base/BUILD @@ -176,7 +176,10 @@ cc_test( "cancellation_flag_test.cc", ], deps = [ + ":base", ":cancellation_flag", + ":test_util", + "//platform/impl/g3", # build_cleaner: keep "//testing/base/public:gunit_main", ], ) diff --git a/cpp/platform/base/cancellation_flag.cc b/cpp/platform/base/cancellation_flag.cc index 1bb6e062..f317355b 100644 --- a/cpp/platform/base/cancellation_flag.cc +++ b/cpp/platform/base/cancellation_flag.cc @@ -1,5 +1,7 @@ #include "platform/base/cancellation_flag.h" +#include "platform/base/feature_flags.h" + namespace location { namespace nearby { @@ -15,6 +17,11 @@ CancellationFlag::CancellationFlag(bool cancelled) { void CancellationFlag::Cancel() { absl::MutexLock lock(mutex_.get()); + // Return immediately as no-op if feature flag is not enabled. + if (!FeatureFlags::GetInstance().GetFlags().enable_cancellation_flag) { + return; + } + if (cancelled_) { // Someone already cancelled. Return immediately. return; @@ -25,6 +32,11 @@ void CancellationFlag::Cancel() { bool CancellationFlag::Cancelled() const { absl::MutexLock lock(mutex_.get()); + // Return falsea as no-op if feature flag is not enabled. + if (!FeatureFlags::GetInstance().GetFlags().enable_cancellation_flag) { + return false; + } + return cancelled_; } diff --git a/cpp/platform/base/cancellation_flag_test.cc b/cpp/platform/base/cancellation_flag_test.cc index 32d79e0d..80bb64a6 100644 --- a/cpp/platform/base/cancellation_flag_test.cc +++ b/cpp/platform/base/cancellation_flag_test.cc @@ -1,20 +1,71 @@ #include "platform/base/cancellation_flag.h" +#include "platform/base/feature_flags.h" +#include "platform/base/medium_environment.h" #include "gtest/gtest.h" namespace location { namespace nearby { +namespace { -TEST(CancellationFlagTest, InitialValueIsFalse) { +using FeatureFlags = FeatureFlags::Flags; + +constexpr FeatureFlags kTestCases[] = { + FeatureFlags{ + .enable_cancellation_flag = true, + }, + FeatureFlags{ + .enable_cancellation_flag = false, + }, +}; + +class CancellationFlagTest : public ::testing::TestWithParam { + protected: + void SetUp() override { + feature_flags_ = GetParam(); + env_.SetFeatureFlags(feature_flags_); + } + + FeatureFlags feature_flags_; + MediumEnvironment& env_{MediumEnvironment::Instance()}; +}; + +TEST_P(CancellationFlagTest, InitialValueIsFalse) { CancellationFlag flag; + + // No matter FeatureFlag is enabled or not, Cancelled is always false. EXPECT_FALSE(flag.Cancelled()); } -TEST(CancellationFlagTest, CanCancel) { - CancellationFlag flag; - flag.Cancel(); +TEST_P(CancellationFlagTest, InitialValueAsTrue) { + CancellationFlag flag{true}; + + // If FeatureFlag is disabled, Cancelled is false as no-op. + if (!feature_flags_.enable_cancellation_flag) { + EXPECT_FALSE(flag.Cancelled()); + return; + } + EXPECT_TRUE(flag.Cancelled()); } +TEST_P(CancellationFlagTest, CanCancel) { + CancellationFlag flag; + flag.Cancel(); + + // If FeatureFlag is disabled, return as no-op immediately and + // Cancelled is always false. + if (!feature_flags_.enable_cancellation_flag) { + EXPECT_FALSE(flag.Cancelled()); + return; + } + + EXPECT_TRUE(flag.Cancelled()); +} + +INSTANTIATE_TEST_SUITE_P(ParametrisedCancellationFlagTest, CancellationFlagTest, + ::testing::ValuesIn(kTestCases)); + +} // namespace } // namespace nearby } // namespace location diff --git a/cpp/platform/base/feature_flags.h b/cpp/platform/base/feature_flags.h index 4cca9bf5..c0d9632f 100644 --- a/cpp/platform/base/feature_flags.h +++ b/cpp/platform/base/feature_flags.h @@ -12,7 +12,7 @@ class FeatureFlags { public: // Holds for all the feature flags. struct Flags { - bool enable_cancellation_flags = false; + bool enable_cancellation_flag = false; bool resume_before_disconnect = true; }; diff --git a/cpp/platform/base/feature_flags_test.cc b/cpp/platform/base/feature_flags_test.cc index dbe4b4a1..d585a7fc 100644 --- a/cpp/platform/base/feature_flags_test.cc +++ b/cpp/platform/base/feature_flags_test.cc @@ -7,18 +7,19 @@ namespace location { namespace nearby { namespace { -FeatureFlags::Flags kTestFeatureFlags{.enable_cancellation_flags = true}; +constexpr FeatureFlags::Flags kTestFeatureFlags{.enable_cancellation_flag = + true}; -TEST(FeatureFlagsTest, ToStringWorks) { +TEST(FeatureFlagsTest, ToSetFeatureWorks) { const FeatureFlags& features = FeatureFlags::GetInstance(); - EXPECT_FALSE(features.GetFlags().enable_cancellation_flags); + EXPECT_FALSE(features.GetFlags().enable_cancellation_flag); MediumEnvironment& medium_environment = MediumEnvironment::Instance(); medium_environment.SetFeatureFlags(kTestFeatureFlags); - EXPECT_TRUE(features.GetFlags().enable_cancellation_flags); + EXPECT_TRUE(features.GetFlags().enable_cancellation_flag); const FeatureFlags& another_features_ref = FeatureFlags::GetInstance(); - EXPECT_TRUE(another_features_ref.GetFlags().enable_cancellation_flags); + EXPECT_TRUE(another_features_ref.GetFlags().enable_cancellation_flag); } } // namespace diff --git a/cpp/platform/impl/g3/BUILD b/cpp/platform/impl/g3/BUILD index 580da280..50826f70 100644 --- a/cpp/platform/impl/g3/BUILD +++ b/cpp/platform/impl/g3/BUILD @@ -69,6 +69,7 @@ cc_library( ":types", "//platform/api:comm", "//platform/base", + "//platform/base:cancellation_flag", "//platform/base:logging", "//platform/base:test_util", "//absl/base:core_headers", diff --git a/cpp/platform/impl/g3/ble.cc b/cpp/platform/impl/g3/ble.cc index f265feec..0595c80a 100644 --- a/cpp/platform/impl/g3/ble.cc +++ b/cpp/platform/impl/g3/ble.cc @@ -317,7 +317,8 @@ bool BleMedium::StopAcceptingConnections(const std::string& service_id) { } std::unique_ptr BleMedium::Connect( - api::BlePeripheral& remote_peripheral, const std::string& service_id) { + api::BlePeripheral& remote_peripheral, const std::string& service_id, + CancellationFlag* cancellation_flag) { NEARBY_LOG(INFO, "G3 Ble Connect [self]: medium=%p, adapter=%p, peripheral=%p, " "service_id=%s", @@ -346,6 +347,13 @@ std::unique_ptr BleMedium::Connect( } } + if (cancellation_flag->Cancelled()) { + NEARBY_LOGS(ERROR) << "G3 BLE Connect: Has been cancelled: " + "service_id=" + << service_id; + return {}; + } + BlePeripheral peripheral = static_cast(remote_peripheral); auto socket = std::make_unique(&peripheral); // Finally, Request to connect to this socket. diff --git a/cpp/platform/impl/g3/ble.h b/cpp/platform/impl/g3/ble.h index b743b8d0..dfa3d6fe 100644 --- a/cpp/platform/impl/g3/ble.h +++ b/cpp/platform/impl/g3/ble.h @@ -183,8 +183,9 @@ class BleMedium : public api::BleMedium { // On success, returns a new BleSocket. // On error, returns nullptr. std::unique_ptr Connect( - api::BlePeripheral& remote_peripheral, - const std::string& service_id) override ABSL_LOCKS_EXCLUDED(mutex_); + api::BlePeripheral& remote_peripheral, const std::string& service_id, + CancellationFlag* cancellation_flag) override + ABSL_LOCKS_EXCLUDED(mutex_); BluetoothAdapter& GetAdapter() { return *adapter_; } diff --git a/cpp/platform/impl/g3/bluetooth_classic.cc b/cpp/platform/impl/g3/bluetooth_classic.cc index 5f90db40..4c7c7901 100644 --- a/cpp/platform/impl/g3/bluetooth_classic.cc +++ b/cpp/platform/impl/g3/bluetooth_classic.cc @@ -199,7 +199,8 @@ bool BluetoothClassicMedium::StopDiscovery() { } std::unique_ptr BluetoothClassicMedium::ConnectToService( - api::BluetoothDevice& remote_device, const std::string& service_uuid) { + api::BluetoothDevice& remote_device, const std::string& service_uuid, + CancellationFlag* cancellation_flag) { NEARBY_LOG(INFO, "G3 ConnectToService [self]: medium=%p, adapter=%p, device=%p", this, &GetAdapter(), &GetAdapter().GetDevice()); @@ -227,6 +228,13 @@ std::unique_ptr BluetoothClassicMedium::ConnectToService( } } + if (cancellation_flag->Cancelled()) { + NEARBY_LOGS(ERROR) << "G3 Bluetooth Connect: Has been cancelled: " + "service_uuid=" + << service_uuid; + return {}; + } + auto socket = std::make_unique(&GetAdapter()); // Finally, Request to connect to this socket. if (!server_socket->Connect(*socket)) { diff --git a/cpp/platform/impl/g3/bluetooth_classic.h b/cpp/platform/impl/g3/bluetooth_classic.h index 2f523056..393e4c50 100644 --- a/cpp/platform/impl/g3/bluetooth_classic.h +++ b/cpp/platform/impl/g3/bluetooth_classic.h @@ -201,8 +201,9 @@ class BluetoothClassicMedium : public api::BluetoothClassicMedium { // On success, returns a new BluetoothSocket. // On error, returns nullptr. std::unique_ptr ConnectToService( - api::BluetoothDevice& remote_device, - const std::string& service_uuid) override ABSL_LOCKS_EXCLUDED(mutex_); + api::BluetoothDevice& remote_device, const std::string& service_uuid, + CancellationFlag* cancellation_flag) override + ABSL_LOCKS_EXCLUDED(mutex_); BluetoothAdapter& GetAdapter() { return *adapter_; } diff --git a/cpp/platform/impl/g3/wifi_lan.cc b/cpp/platform/impl/g3/wifi_lan.cc index ec8a9549..2ae2b9fe 100644 --- a/cpp/platform/impl/g3/wifi_lan.cc +++ b/cpp/platform/impl/g3/wifi_lan.cc @@ -317,8 +317,8 @@ bool WifiLanMedium::StopAcceptingConnections(const std::string& service_id) { } std::unique_ptr WifiLanMedium::Connect( - api::WifiLanService& remote_wifi_lan_service, - const std::string& service_id) { + api::WifiLanService& remote_wifi_lan_service, const std::string& service_id, + CancellationFlag* cancellation_flag) { NEARBY_LOG( INFO, "G3 WifiLan Connect: medium=%p, wifi_lan_service=%p, " @@ -354,6 +354,13 @@ std::unique_ptr WifiLanMedium::Connect( } } + if (cancellation_flag->Cancelled()) { + NEARBY_LOGS(INFO) << "G3 WifiLan Connect: Has been cancelled: " + "service_id=" + << service_id; + return {}; + } + WifiLanService wifi_lan_service = static_cast(remote_wifi_lan_service); auto socket = std::make_unique(&wifi_lan_service); diff --git a/cpp/platform/impl/g3/wifi_lan.h b/cpp/platform/impl/g3/wifi_lan.h index 8a432d4c..bfa552e7 100644 --- a/cpp/platform/impl/g3/wifi_lan.h +++ b/cpp/platform/impl/g3/wifi_lan.h @@ -204,7 +204,9 @@ class WifiLanMedium : public api::WifiLanMedium { // On error, returns nullptr. std::unique_ptr Connect( api::WifiLanService& remote_wifi_lan_service, - const std::string& service_id) override ABSL_LOCKS_EXCLUDED(mutex_); + const std::string& service_id, + CancellationFlag* cancellation_flag) override + ABSL_LOCKS_EXCLUDED(mutex_); api::WifiLanService* GetRemoteService(const std::string& ip_address, int port) override; diff --git a/cpp/platform/public/BUILD b/cpp/platform/public/BUILD index 0d7d89a3..6b394eec 100644 --- a/cpp/platform/public/BUILD +++ b/cpp/platform/public/BUILD @@ -82,6 +82,7 @@ cc_library( "//platform/api:comm", "//platform/api:platform", "//platform/base", + "//platform/base:cancellation_flag", "//absl/container:flat_hash_map", "//absl/strings", "//webrtc/api:libjingle_peerconnection_api", diff --git a/cpp/platform/public/ble.cc b/cpp/platform/public/ble.cc index 4e5670d6..41ce8d3f 100644 --- a/cpp/platform/public/ble.cc +++ b/cpp/platform/public/ble.cc @@ -127,13 +127,15 @@ bool BleMedium::StopAcceptingConnections(const std::string& service_id) { } BleSocket BleMedium::Connect(BlePeripheral& peripheral, - const std::string& service_id) { + const std::string& service_id, + CancellationFlag* cancellation_flag) { { MutexLock lock(&mutex_); NEARBY_LOG(INFO, "BleMedium::Connect: peripheral=%p [impl=%p]", &peripheral, &peripheral.GetImpl()); } - return BleSocket(impl_->Connect(peripheral.GetImpl(), service_id)); + return BleSocket( + impl_->Connect(peripheral.GetImpl(), service_id, cancellation_flag)); } } // namespace nearby diff --git a/cpp/platform/public/ble.h b/cpp/platform/public/ble.h index 777ca9e7..67be3dbb 100644 --- a/cpp/platform/public/ble.h +++ b/cpp/platform/public/ble.h @@ -18,6 +18,7 @@ #include "platform/api/ble.h" #include "platform/api/platform.h" #include "platform/base/byte_array.h" +#include "platform/base/cancellation_flag.h" #include "platform/base/input_stream.h" #include "platform/base/output_stream.h" #include "platform/public/bluetooth_adapter.h" @@ -137,7 +138,8 @@ class BleMedium final { // Returns a new BleSocket. On Success, BleSocket::IsValid() // returns true. - BleSocket Connect(BlePeripheral& peripheral, const std::string& service_id); + BleSocket Connect(BlePeripheral& peripheral, const std::string& service_id, + CancellationFlag* cancellation_flag); bool IsValid() const { return impl_ != nullptr; } diff --git a/cpp/platform/public/ble_test.cc b/cpp/platform/public/ble_test.cc index 10cbe645..9c0e248e 100644 --- a/cpp/platform/public/ble_test.cc +++ b/cpp/platform/public/ble_test.cc @@ -26,12 +26,23 @@ namespace location { namespace nearby { namespace { +using FeatureFlags = FeatureFlags::Flags; + +constexpr FeatureFlags kTestCases[] = { + FeatureFlags{ + .enable_cancellation_flag = true, + }, + FeatureFlags{ + .enable_cancellation_flag = false, + }, +}; + constexpr absl::Duration kWaitDuration = absl::Milliseconds(1000); constexpr absl::string_view kServiceID{"com.google.location.nearby.apps.test"}; constexpr absl::string_view kAdvertisementString{"\x0a\x0b\x0c\x0d"}; constexpr absl::string_view kFastAdvertisementServiceUuid{"\xf3\xfe"}; -class BleMediumTest : public ::testing::Test { +class BleMediumTest : public ::testing::TestWithParam { protected: using DiscoveredPeripheralCallback = BleMedium::DiscoveredPeripheralCallback; using AcceptedConnectionCallback = BleMedium::AcceptedConnectionCallback; @@ -41,6 +52,139 @@ class BleMediumTest : public ::testing::Test { MediumEnvironment& env_{MediumEnvironment::Instance()}; }; +TEST_P(BleMediumTest, CanStartAcceptingConnectionsAndConnect) { + FeatureFlags feature_flags = GetParam(); + env_.SetFeatureFlags(feature_flags); + env_.Start(); + BluetoothAdapter adapter_a_; + BluetoothAdapter adapter_b_; + BleMedium ble_a{adapter_a_}; + BleMedium ble_b{adapter_b_}; + std::string service_id(kServiceID); + ByteArray advertisement_bytes{std::string(kAdvertisementString)}; + std::string fast_advertisement_service_uuid(kFastAdvertisementServiceUuid); + CountDownLatch found_latch(1); + CountDownLatch accepted_latch(1); + + BlePeripheral* discovered_peripheral = nullptr; + ble_a.StartScanning( + service_id, fast_advertisement_service_uuid, + DiscoveredPeripheralCallback{ + .peripheral_discovered_cb = + [&found_latch, &discovered_peripheral]( + BlePeripheral& peripheral, const std::string& service_id, + const ByteArray& advertisement_bytes, + bool fast_advertisement) { + NEARBY_LOG( + INFO, + "Peripheral discovered: %s, %p, fast advertisement: %d", + peripheral.GetName().c_str(), &peripheral, + fast_advertisement); + discovered_peripheral = &peripheral; + found_latch.CountDown(); + }, + }); + ble_b.StartAdvertising(service_id, advertisement_bytes, + fast_advertisement_service_uuid); + ble_b.StartAcceptingConnections( + service_id, + AcceptedConnectionCallback{ + .accepted_cb = [&accepted_latch](BleSocket socket, + const std::string& service_id) { + NEARBY_LOG(INFO, "Connection accepted: socket=%p, service_id=%s", + &socket, service_id.c_str()); + accepted_latch.CountDown(); + }}); + EXPECT_TRUE(found_latch.Await(kWaitDuration).result()); + + BleSocket socket_a; + EXPECT_FALSE(socket_a.IsValid()); + { + SingleThreadExecutor client_executor; + client_executor.Execute( + [&ble_a, &socket_a, discovered_peripheral, &service_id]() { + CancellationFlag flag; + socket_a = ble_a.Connect(*discovered_peripheral, service_id, &flag); + }); + } + EXPECT_TRUE(accepted_latch.Await(kWaitDuration).result()); + EXPECT_TRUE(socket_a.IsValid()); + ble_b.StopAdvertising(service_id); + ble_a.StopScanning(service_id); + env_.Stop(); +} + +TEST_P(BleMediumTest, CanCancelConnect) { + FeatureFlags feature_flags = GetParam(); + env_.SetFeatureFlags(feature_flags); + env_.Start(); + BluetoothAdapter adapter_a_; + BluetoothAdapter adapter_b_; + BleMedium ble_a{adapter_a_}; + BleMedium ble_b{adapter_b_}; + std::string service_id(kServiceID); + ByteArray advertisement_bytes{std::string(kAdvertisementString)}; + std::string fast_advertisement_service_uuid(kFastAdvertisementServiceUuid); + CountDownLatch found_latch(1); + CountDownLatch accepted_latch(1); + + BlePeripheral* discovered_peripheral = nullptr; + ble_a.StartScanning( + service_id, fast_advertisement_service_uuid, + DiscoveredPeripheralCallback{ + .peripheral_discovered_cb = + [&found_latch, &discovered_peripheral]( + BlePeripheral& peripheral, const std::string& service_id, + const ByteArray& advertisement_bytes, + bool fast_advertisement) { + NEARBY_LOG( + INFO, + "Peripheral discovered: %s, %p, fast advertisement: %d", + peripheral.GetName().c_str(), &peripheral, + fast_advertisement); + discovered_peripheral = &peripheral; + found_latch.CountDown(); + }, + }); + ble_b.StartAdvertising(service_id, advertisement_bytes, + fast_advertisement_service_uuid); + ble_b.StartAcceptingConnections( + service_id, + AcceptedConnectionCallback{ + .accepted_cb = [&accepted_latch](BleSocket socket, + const std::string& service_id) { + NEARBY_LOG(INFO, "Connection accepted: socket=%p, service_id=%s", + &socket, service_id.c_str()); + accepted_latch.CountDown(); + }}); + EXPECT_TRUE(found_latch.Await(kWaitDuration).result()); + + BleSocket socket_a; + EXPECT_FALSE(socket_a.IsValid()); + { + SingleThreadExecutor client_executor; + client_executor.Execute( + [&ble_a, &socket_a, discovered_peripheral, &service_id]() { + CancellationFlag flag(true); + socket_a = ble_a.Connect(*discovered_peripheral, service_id, &flag); + }); + } + // If FeatureFlag is disabled, Cancelled is false as no-op. + if (!feature_flags.enable_cancellation_flag) { + EXPECT_TRUE(accepted_latch.Await(kWaitDuration).result()); + EXPECT_TRUE(socket_a.IsValid()); + } else { + EXPECT_FALSE(accepted_latch.Await(kWaitDuration).result()); + EXPECT_FALSE(socket_a.IsValid()); + } + ble_b.StopAdvertising(service_id); + ble_a.StopScanning(service_id); + env_.Stop(); +} + +INSTANTIATE_TEST_SUITE_P(ParametrisedBleMediumTest, BleMediumTest, + ::testing::ValuesIn(kTestCases)); + TEST_F(BleMediumTest, ConstructorDestructorWorks) { env_.Start(); BluetoothAdapter adapter_a_; @@ -156,65 +300,6 @@ TEST_F(BleMediumTest, CanStopDiscovery) { env_.Stop(); } -TEST_F(BleMediumTest, CanStartAcceptingConnectionsAndConnect) { - env_.Start(); - BluetoothAdapter adapter_a_; - BluetoothAdapter adapter_b_; - BleMedium ble_a{adapter_a_}; - BleMedium ble_b{adapter_b_}; - std::string service_id(kServiceID); - ByteArray advertisement_bytes{std::string(kAdvertisementString)}; - std::string fast_advertisement_service_uuid(kFastAdvertisementServiceUuid); - CountDownLatch found_latch(1); - CountDownLatch accepted_latch(1); - - BlePeripheral* discovered_peripheral = nullptr; - ble_a.StartScanning( - service_id, fast_advertisement_service_uuid, - DiscoveredPeripheralCallback{ - .peripheral_discovered_cb = - [&found_latch, &discovered_peripheral]( - BlePeripheral& peripheral, const std::string& service_id, - const ByteArray& advertisement_bytes, - bool fast_advertisement) { - NEARBY_LOG( - INFO, - "Peripheral discovered: %s, %p, fast advertisement: %d", - peripheral.GetName().c_str(), &peripheral, - fast_advertisement); - discovered_peripheral = &peripheral; - found_latch.CountDown(); - }, - }); - ble_b.StartAdvertising(service_id, advertisement_bytes, - fast_advertisement_service_uuid); - ble_b.StartAcceptingConnections( - service_id, - AcceptedConnectionCallback{ - .accepted_cb = [&accepted_latch](BleSocket socket, - const std::string& service_id) { - NEARBY_LOG(INFO, "Connection accepted: socket=%p, service_id=%s", - &socket, service_id.c_str()); - accepted_latch.CountDown(); - }}); - EXPECT_TRUE(found_latch.Await(kWaitDuration).result()); - - BleSocket socket_a; - EXPECT_FALSE(socket_a.IsValid()); - { - SingleThreadExecutor client_executor; - client_executor.Execute( - [&ble_a, &socket_a, discovered_peripheral, &service_id]() { - socket_a = ble_a.Connect(*discovered_peripheral, service_id); - }); - } - EXPECT_TRUE(accepted_latch.Await(kWaitDuration).result()); - EXPECT_TRUE(socket_a.IsValid()); - ble_b.StopAdvertising(service_id); - ble_a.StopScanning(service_id); - env_.Stop(); -} - } // namespace } // namespace nearby } // namespace location diff --git a/cpp/platform/public/bluetooth_classic.cc b/cpp/platform/public/bluetooth_classic.cc index 37bb62d4..e5e5d35d 100644 --- a/cpp/platform/public/bluetooth_classic.cc +++ b/cpp/platform/public/bluetooth_classic.cc @@ -23,12 +23,13 @@ namespace nearby { BluetoothClassicMedium::~BluetoothClassicMedium() { StopDiscovery(); } BluetoothSocket BluetoothClassicMedium::ConnectToService( - BluetoothDevice& remote_device, const std::string& service_uuid) { + BluetoothDevice& remote_device, const std::string& service_uuid, + CancellationFlag* cancellation_flag) { NEARBY_LOG(INFO, "BluetoothClassicMedium::ConnectToService: device=%p [impl=%p]", &remote_device, &remote_device.GetImpl()); - return BluetoothSocket( - impl_->ConnectToService(remote_device.GetImpl(), service_uuid)); + return BluetoothSocket(impl_->ConnectToService( + remote_device.GetImpl(), service_uuid, cancellation_flag)); } bool BluetoothClassicMedium::StartDiscovery(DiscoveryCallback callback) { diff --git a/cpp/platform/public/bluetooth_classic.h b/cpp/platform/public/bluetooth_classic.h index 2fa407a8..38743752 100644 --- a/cpp/platform/public/bluetooth_classic.h +++ b/cpp/platform/public/bluetooth_classic.h @@ -21,6 +21,7 @@ #include "platform/api/bluetooth_classic.h" #include "platform/api/platform.h" #include "platform/base/byte_array.h" +#include "platform/base/cancellation_flag.h" #include "platform/base/exception.h" #include "platform/base/input_stream.h" #include "platform/base/listeners.h" @@ -179,7 +180,8 @@ class BluetoothClassicMedium final { // Returns a new BluetoothSocket. On Success, BluetoothSocket::IsValid() // returns true. BluetoothSocket ConnectToService(BluetoothDevice& remote_device, - const std::string& service_uuid); + const std::string& service_uuid, + CancellationFlag* cancellation_flag); // https://developer.android.com/reference/android/bluetooth/BluetoothAdapter.html#listenUsingInsecureRfcommWithServiceRecord // diff --git a/cpp/platform/public/bluetooth_classic_test.cc b/cpp/platform/public/bluetooth_classic_test.cc index 2ba855f6..2dfb6c97 100644 --- a/cpp/platform/public/bluetooth_classic_test.cc +++ b/cpp/platform/public/bluetooth_classic_test.cc @@ -29,7 +29,19 @@ namespace location { namespace nearby { namespace { -class BluetoothClassicMediumTest : public ::testing::Test { +using FeatureFlags = FeatureFlags::Flags; + +constexpr FeatureFlags kTestCases[] = { + FeatureFlags{ + .enable_cancellation_flag = true, + }, + FeatureFlags{ + .enable_cancellation_flag = false, + }, +}; + +class BluetoothClassicMediumTest + : public ::testing::TestWithParam { protected: using DiscoveryCallback = BluetoothClassicMedium::DiscoveryCallback; BluetoothClassicMediumTest() { @@ -66,6 +78,114 @@ class BluetoothClassicMediumTest : public ::testing::Test { std::unique_ptr bt_b_; }; +TEST_P(BluetoothClassicMediumTest, CanConnectToService) { + FeatureFlags feature_flags = GetParam(); + env_.SetFeatureFlags(feature_flags); + + adapter_a_->SetScanMode(BluetoothAdapter::ScanMode::kConnectable); + CountDownLatch found_latch(1); + BluetoothDevice* discovered_device = nullptr; + bt_a_->StartDiscovery(DiscoveryCallback{ + .device_discovered_cb = + [this, &found_latch, &discovered_device](BluetoothDevice& device) { + NEARBY_LOG(INFO, "Device discovered: %s", device.GetName().c_str()); + EXPECT_EQ(device.GetName(), adapter_b_->GetName()); + discovered_device = &device; + found_latch.CountDown(); + }, + }); + adapter_b_->SetScanMode(BluetoothAdapter::ScanMode::kConnectableDiscoverable); + EXPECT_EQ(adapter_b_->GetScanMode(), + BluetoothAdapter::ScanMode::kConnectableDiscoverable); + EXPECT_TRUE(found_latch.Await(absl::Milliseconds(1000)).result()); + std::string service_name{"service"}; + std::string service_uuid("service-uuid"); + BluetoothServerSocket server_socket = + bt_b_->ListenForService(service_name, service_uuid); + EXPECT_TRUE(server_socket.IsValid()); + BluetoothSocket socket_a; + BluetoothSocket socket_b; + EXPECT_FALSE(socket_a.IsValid()); + EXPECT_FALSE(socket_b.IsValid()); + { + SingleThreadExecutor server_executor; + SingleThreadExecutor client_executor; + client_executor.Execute( + [this, &socket_a, discovered_device, &service_uuid, &server_socket]() { + CancellationFlag flag; + socket_a = + bt_a_->ConnectToService(*discovered_device, service_uuid, &flag); + if (!socket_a.IsValid()) server_socket.Close(); + }); + server_executor.Execute([&socket_b, &server_socket]() { + socket_b = server_socket.Accept(); + if (!socket_b.IsValid()) server_socket.Close(); + }); + } + EXPECT_TRUE(socket_a.IsValid()); + EXPECT_TRUE(socket_b.IsValid()); + server_socket.Close(); +} + +TEST_P(BluetoothClassicMediumTest, CanCancelConnect) { + FeatureFlags feature_flags = GetParam(); + env_.SetFeatureFlags(feature_flags); + + adapter_a_->SetScanMode(BluetoothAdapter::ScanMode::kConnectable); + CountDownLatch found_latch(1); + BluetoothDevice* discovered_device = nullptr; + bt_a_->StartDiscovery(DiscoveryCallback{ + .device_discovered_cb = + [this, &found_latch, &discovered_device](BluetoothDevice& device) { + NEARBY_LOG(INFO, "Device discovered: %s", device.GetName().c_str()); + EXPECT_EQ(device.GetName(), adapter_b_->GetName()); + discovered_device = &device; + found_latch.CountDown(); + }, + }); + adapter_b_->SetScanMode(BluetoothAdapter::ScanMode::kConnectableDiscoverable); + EXPECT_EQ(adapter_b_->GetScanMode(), + BluetoothAdapter::ScanMode::kConnectableDiscoverable); + EXPECT_TRUE(found_latch.Await(absl::Milliseconds(1000)).result()); + std::string service_name{"service"}; + std::string service_uuid("service-uuid"); + BluetoothServerSocket server_socket = + bt_b_->ListenForService(service_name, service_uuid); + EXPECT_TRUE(server_socket.IsValid()); + BluetoothSocket socket_a; + BluetoothSocket socket_b; + EXPECT_FALSE(socket_a.IsValid()); + EXPECT_FALSE(socket_b.IsValid()); + { + SingleThreadExecutor server_executor; + SingleThreadExecutor client_executor; + client_executor.Execute( + [this, &socket_a, discovered_device, &service_uuid, &server_socket]() { + CancellationFlag flag(true); + socket_a = + bt_a_->ConnectToService(*discovered_device, service_uuid, &flag); + if (!socket_a.IsValid()) server_socket.Close(); + }); + server_executor.Execute([&socket_b, &server_socket]() { + socket_b = server_socket.Accept(); + if (!socket_b.IsValid()) server_socket.Close(); + }); + } + // If FeatureFlag is disabled, Cancelled is false as no-op. + if (!feature_flags.enable_cancellation_flag) { + EXPECT_TRUE(socket_a.IsValid()); + EXPECT_TRUE(socket_b.IsValid()); + } else { + EXPECT_FALSE(socket_a.IsValid()); + EXPECT_FALSE(socket_b.IsValid()); + } + server_socket.Close(); +} + +INSTANTIATE_TEST_SUITE_P(ParametrisedBluetoothClassicMediumTest, + BluetoothClassicMediumTest, + ::testing::ValuesIn(kTestCases)); + TEST_F(BluetoothClassicMediumTest, ConstructorDestructorWorks) { // Make sure we can create functional adapters. ASSERT_TRUE(adapter_a_->IsValid()); @@ -163,51 +283,6 @@ TEST_F(BluetoothClassicMediumTest, CanListenForService) { server_socket.Close(); } -TEST_F(BluetoothClassicMediumTest, CanConnectToService) { - adapter_a_->SetScanMode(BluetoothAdapter::ScanMode::kConnectable); - CountDownLatch found_latch(1); - BluetoothDevice* discovered_device = nullptr; - bt_a_->StartDiscovery(DiscoveryCallback{ - .device_discovered_cb = - [this, &found_latch, &discovered_device](BluetoothDevice& device) { - NEARBY_LOG(INFO, "Device discovered: %s", device.GetName().c_str()); - EXPECT_EQ(device.GetName(), adapter_b_->GetName()); - discovered_device = &device; - found_latch.CountDown(); - }, - }); - adapter_b_->SetScanMode(BluetoothAdapter::ScanMode::kConnectableDiscoverable); - EXPECT_EQ(adapter_b_->GetScanMode(), - BluetoothAdapter::ScanMode::kConnectableDiscoverable); - EXPECT_TRUE(found_latch.Await(absl::Milliseconds(1000)).result()); - std::string service_name{"service"}; - std::string service_uuid("service-uuid"); - BluetoothServerSocket server_socket = - bt_b_->ListenForService(service_name, service_uuid); - EXPECT_TRUE(server_socket.IsValid()); - BluetoothSocket socket_a; - BluetoothSocket socket_b; - EXPECT_FALSE(socket_a.IsValid()); - EXPECT_FALSE(socket_b.IsValid()); - { - SingleThreadExecutor server_executor; - SingleThreadExecutor client_executor; - client_executor.Execute( - [this, &socket_a, discovered_device, &service_uuid, &server_socket]() { - socket_a = bt_a_->ConnectToService(*discovered_device, service_uuid); - if (!socket_a.IsValid()) server_socket.Close(); - }); - server_executor.Execute( - [&socket_b, &server_socket]() { - socket_b = server_socket.Accept(); - if (!socket_b.IsValid()) server_socket.Close(); - }); - } - EXPECT_TRUE(socket_a.IsValid()); - EXPECT_TRUE(socket_b.IsValid()); - server_socket.Close(); -} - } // namespace } // namespace nearby } // namespace location diff --git a/cpp/platform/public/wifi_lan.cc b/cpp/platform/public/wifi_lan.cc index c0f1c6f3..dd879b2c 100644 --- a/cpp/platform/public/wifi_lan.cc +++ b/cpp/platform/public/wifi_lan.cc @@ -136,13 +136,15 @@ bool WifiLanMedium::StopAcceptingConnections(const std::string& service_id) { } WifiLanSocket WifiLanMedium::Connect(WifiLanService& wifi_lan_service, - const std::string& service_id) { + const std::string& service_id, + CancellationFlag* cancellation_flag) { NEARBY_LOG( INFO, "WifiLanMedium::Connect: service=%p [impl=%p, service_info_name=%s]", &wifi_lan_service, &wifi_lan_service.GetImpl(), wifi_lan_service.GetServiceInfo().GetServiceInfoName().c_str()); - return WifiLanSocket(impl_->Connect(wifi_lan_service.GetImpl(), service_id)); + return WifiLanSocket(impl_->Connect(wifi_lan_service.GetImpl(), service_id, + cancellation_flag)); } WifiLanService WifiLanMedium::GetRemoteService(const std::string& ip_address, diff --git a/cpp/platform/public/wifi_lan.h b/cpp/platform/public/wifi_lan.h index fa381c75..fbcca37a 100644 --- a/cpp/platform/public/wifi_lan.h +++ b/cpp/platform/public/wifi_lan.h @@ -18,6 +18,7 @@ #include "platform/api/platform.h" #include "platform/api/wifi_lan.h" #include "platform/base/byte_array.h" +#include "platform/base/cancellation_flag.h" #include "platform/base/input_stream.h" #include "platform/base/nsd_service_info.h" #include "platform/base/output_stream.h" @@ -151,7 +152,8 @@ class WifiLanMedium final { // Returns a new WifiLanSocket. On Success, WifiLanSocket::IsValid() // returns true. WifiLanSocket Connect(WifiLanService& wifi_lan_service, - const std::string& service_id); + const std::string& service_id, + CancellationFlag* cancellation_flag); bool IsValid() const { return impl_ != nullptr; } diff --git a/cpp/platform/public/wifi_lan_test.cc b/cpp/platform/public/wifi_lan_test.cc index c1064440..f3992771 100644 --- a/cpp/platform/public/wifi_lan_test.cc +++ b/cpp/platform/public/wifi_lan_test.cc @@ -27,12 +27,23 @@ namespace location { namespace nearby { namespace { +using FeatureFlags = FeatureFlags::Flags; + +constexpr FeatureFlags kTestCases[] = { + FeatureFlags{ + .enable_cancellation_flag = true, + }, + FeatureFlags{ + .enable_cancellation_flag = false, + }, +}; + constexpr absl::string_view kServiceID{"com.google.location.nearby.apps.test"}; constexpr absl::string_view kServiceInfoName{"Simulated service info name"}; constexpr absl::string_view kEndpointName{"Simulated endpoint name"}; constexpr absl::string_view kEndpointInfoKey{"n"}; -class WifiLanMediumTest : public ::testing::Test { +class WifiLanMediumTest : public ::testing::TestWithParam { protected: using DiscoveredServiceCallback = WifiLanMedium::DiscoveredServiceCallback; using AcceptedConnectionCallback = WifiLanMedium::AcceptedConnectionCallback; @@ -42,6 +53,142 @@ class WifiLanMediumTest : public ::testing::Test { MediumEnvironment& env_{MediumEnvironment::Instance()}; }; +TEST_P(WifiLanMediumTest, CanStartAcceptingConnectionsAndConnect) { + FeatureFlags feature_flags = GetParam(); + env_.SetFeatureFlags(feature_flags); + env_.Start(); + WifiLanMedium wifi_a; + WifiLanMedium wifi_b; + std::string service_id(kServiceID); + std::string service_info_name{kServiceInfoName}; + std::string endpoint_info_name{kEndpointName}; + CountDownLatch found_latch(1); + CountDownLatch accepted_latch(1); + + WifiLanService* discovered_service = nullptr; + wifi_a.StartDiscovery( + service_id, + DiscoveredServiceCallback{ + .service_discovered_cb = + [&found_latch, &discovered_service]( + WifiLanService& service, const std::string& service_id) { + NEARBY_LOG( + INFO, "Service discovered: %s, %p", + service.GetServiceInfo().GetServiceInfoName().c_str(), + &service); + discovered_service = &service; + found_latch.CountDown(); + }, + }); + + NsdServiceInfo nsd_service_info; + nsd_service_info.SetServiceInfoName(service_info_name); + nsd_service_info.SetTxtRecord(std::string(kEndpointInfoKey), + endpoint_info_name); + wifi_b.StartAdvertising(service_id, nsd_service_info); + wifi_b.StartAcceptingConnections( + service_id, + AcceptedConnectionCallback{ + .accepted_cb = [&accepted_latch](WifiLanSocket socket, + const std::string& service_id) { + NEARBY_LOG(INFO, "Connection accepted: socket=%p, service_id=%s", + &socket, service_id.c_str()); + accepted_latch.CountDown(); + }}); + EXPECT_TRUE(found_latch.Await(absl::Milliseconds(1000)).result()); + + WifiLanSocket socket_a; + EXPECT_FALSE(socket_a.IsValid()); + { + SingleThreadExecutor client_executor; + client_executor.Execute( + [&wifi_a, &socket_a, discovered_service, &service_id]() { + CancellationFlag flag; + socket_a = wifi_a.Connect(*discovered_service, service_id, &flag); + }); + } + EXPECT_TRUE(accepted_latch.Await(absl::Milliseconds(1000)).result()); + EXPECT_TRUE(socket_a.IsValid()); + wifi_b.StopAcceptingConnections(service_id); + wifi_b.StopAdvertising(service_id); + wifi_a.StopDiscovery(service_id); + env_.Stop(); +} + +TEST_P(WifiLanMediumTest, CanCancelConnect) { + FeatureFlags feature_flags = GetParam(); + env_.SetFeatureFlags(feature_flags); + env_.Start(); + WifiLanMedium wifi_a; + WifiLanMedium wifi_b; + std::string service_id(kServiceID); + std::string service_info_name{kServiceInfoName}; + std::string endpoint_info_name{kEndpointName}; + CountDownLatch found_latch(1); + CountDownLatch accepted_latch(1); + + WifiLanService* discovered_service = nullptr; + wifi_a.StartDiscovery( + service_id, + DiscoveredServiceCallback{ + .service_discovered_cb = + [&found_latch, &discovered_service]( + WifiLanService& service, const std::string& service_id) { + NEARBY_LOG( + INFO, "Service discovered: %s, %p", + service.GetServiceInfo().GetServiceInfoName().c_str(), + &service); + discovered_service = &service; + found_latch.CountDown(); + }, + }); + + NsdServiceInfo nsd_service_info; + nsd_service_info.SetServiceInfoName(service_info_name); + nsd_service_info.SetTxtRecord(std::string(kEndpointInfoKey), + endpoint_info_name); + wifi_b.StartAdvertising(service_id, nsd_service_info); + wifi_b.StartAcceptingConnections( + service_id, + AcceptedConnectionCallback{ + .accepted_cb = [&accepted_latch](WifiLanSocket socket, + const std::string& service_id) { + NEARBY_LOG(INFO, "Connection accepted: socket=%p, service_id=%s", + &socket, service_id.c_str()); + accepted_latch.CountDown(); + }}); + EXPECT_TRUE(found_latch.Await(absl::Milliseconds(1000)).result()); + + WifiLanSocket socket_a; + EXPECT_FALSE(socket_a.IsValid()); + { + SingleThreadExecutor client_executor; + client_executor.Execute( + [&wifi_a, &socket_a, discovered_service, &service_id]() { + // Make it as Cancelled. + CancellationFlag flag(true); + socket_a = wifi_a.Connect(*discovered_service, service_id, &flag); + }); + } + + // If FeatureFlag is disabled, Cancelled is false as no-op. + if (!feature_flags.enable_cancellation_flag) { + EXPECT_TRUE(accepted_latch.Await(absl::Milliseconds(1000)).result()); + EXPECT_TRUE(socket_a.IsValid()); + } else { + EXPECT_FALSE(accepted_latch.Await(absl::Milliseconds(1000)).result()); + EXPECT_FALSE(socket_a.IsValid()); + } + + wifi_b.StopAcceptingConnections(service_id); + wifi_b.StopAdvertising(service_id); + wifi_a.StopDiscovery(service_id); + env_.Stop(); +} + +INSTANTIATE_TEST_SUITE_P(ParametrisedWifiLanMediumTest, WifiLanMediumTest, + ::testing::ValuesIn(kTestCases)); + TEST_F(WifiLanMediumTest, ConstructorDestructorWorks) { env_.Start(); WifiLanMedium wifi_a; @@ -158,65 +305,6 @@ TEST_F(WifiLanMediumTest, CanStopDiscovery) { env_.Stop(); } -TEST_F(WifiLanMediumTest, CanStartAcceptingConnectionsAndConnect) { - env_.Start(); - WifiLanMedium wifi_a; - WifiLanMedium wifi_b; - std::string service_id(kServiceID); - std::string service_info_name{kServiceInfoName}; - std::string endpoint_info_name{kEndpointName}; - CountDownLatch found_latch(1); - CountDownLatch accepted_latch(1); - - WifiLanService* discovered_service = nullptr; - wifi_a.StartDiscovery( - service_id, - DiscoveredServiceCallback{ - .service_discovered_cb = - [&found_latch, &discovered_service]( - WifiLanService& service, const std::string& service_id) { - NEARBY_LOG( - INFO, "Service discovered: %s, %p", - service.GetServiceInfo().GetServiceInfoName().c_str(), - &service); - discovered_service = &service; - found_latch.CountDown(); - }, - }); - - NsdServiceInfo nsd_service_info; - nsd_service_info.SetServiceInfoName(service_info_name); - nsd_service_info.SetTxtRecord(std::string(kEndpointInfoKey), - endpoint_info_name); - wifi_b.StartAdvertising(service_id, nsd_service_info); - wifi_b.StartAcceptingConnections( - service_id, - AcceptedConnectionCallback{ - .accepted_cb = [&accepted_latch](WifiLanSocket socket, - const std::string& service_id) { - NEARBY_LOG(INFO, "Connection accepted: socket=%p, service_id=%s", - &socket, service_id.c_str()); - accepted_latch.CountDown(); - }}); - EXPECT_TRUE(found_latch.Await(absl::Milliseconds(1000)).result()); - - WifiLanSocket socket_a; - EXPECT_FALSE(socket_a.IsValid()); - { - SingleThreadExecutor client_executor; - client_executor.Execute( - [&wifi_a, &socket_a, discovered_service, &service_id]() { - socket_a = wifi_a.Connect(*discovered_service, service_id); - }); - } - EXPECT_TRUE(accepted_latch.Await(absl::Milliseconds(1000)).result()); - EXPECT_TRUE(socket_a.IsValid()); - wifi_b.StopAcceptingConnections(service_id); - wifi_b.StopAdvertising(service_id); - wifi_a.StopDiscovery(service_id); - env_.Stop(); -} - } // namespace } // namespace nearby } // namespace location