Plumb updateDiscoveryOptions into pcp handler.

PiperOrigin-RevId: 544431325

connections/implementation/base_pcp_handler.cc

@@ -1292,6 +1292,27 @@ Status BasePcpHandler::UpdateAdvertisingOptions(
   return status.Get().GetResult();
 }
 
+Status BasePcpHandler::UpdateDiscoveryOptions(
+    ClientProxy* client, absl::string_view service_id,
+    const DiscoveryOptions& discovery_options) {
+  Future<Status> status;
+  RunOnPcpHandlerThread(
+      "update-discovery-options",
+      [this, client, service_id, discovery_options, &status]()
+          RUN_ON_PCP_HANDLER_THREAD() mutable {
+            StartOperationResult result = UpdateDiscoveryOptionsImpl(
+                client, service_id, client->GetLocalEndpointId(),
+                client->GetLocalEndpointInfo(), discovery_options);
+            if (!result.status.Ok()) {
+              status.Set(result.status);
+              return;
+            }
+            client->UpdateDiscoveryOptions(discovery_options);
+            status.Set({Status::kSuccess});
+          });
+  return status.Get().GetResult();
+}
+
 bool BasePcpHandler::NeedsToTurnOffAdvertisingMedium(
     Medium medium, const AdvertisingOptions& old_options,
     const AdvertisingOptions& new_options) {
@@ -1303,6 +1324,17 @@ bool BasePcpHandler::NeedsToTurnOffAdvertisingMedium(
                     medium) != new_disabled_mediums.end());
 }
 
+bool BasePcpHandler::NeedsToTurnOffDiscoveryMedium(
+    Medium medium, const DiscoveryOptions& old_options,
+    const DiscoveryOptions& new_options) {
+  auto old_enabled_mediums = old_options.allowed.GetMediums(/*value=*/true);
+  auto new_disabled_mediums = new_options.allowed.GetMediums(/*value=*/false);
+  return (std::find(old_enabled_mediums.begin(), old_enabled_mediums.end(),
+                    medium) != old_enabled_mediums.end()) &&
+         (std::find(new_disabled_mediums.begin(), new_disabled_mediums.end(),
+                    medium) != new_disabled_mediums.end());
+}
+
 bool BasePcpHandler::IsPreferred(
     const BasePcpHandler::DiscoveredEndpoint& new_endpoint,
     const BasePcpHandler::DiscoveredEndpoint& old_endpoint) {

connections/implementation/base_pcp_handler.h

@@ -154,6 +154,10 @@ class BasePcpHandler : public PcpHandler,
       ClientProxy* client, absl::string_view service_id,
       const AdvertisingOptions& advertising_options) override;
 
+  Status UpdateDiscoveryOptions(
+      ClientProxy* client, absl::string_view service_id,
+      const DiscoveryOptions& discovery_options) override;
+
   Pcp GetPcp() const override { return pcp_; }
   Strategy GetStrategy() const override { return strategy_; }
   void DisconnectFromEndpointManager();
@@ -313,11 +317,22 @@ class BasePcpHandler : public PcpHandler,
       const AdvertisingOptions& advertising_options)
       RUN_ON_PCP_HANDLER_THREAD() = 0;
 
+  virtual StartOperationResult UpdateDiscoveryOptionsImpl(
+      ClientProxy* client, absl::string_view service_id,
+      absl::string_view local_endpoint_id,
+      absl::string_view local_endpoint_info,
+      const DiscoveryOptions& discovery_options)
+      RUN_ON_PCP_HANDLER_THREAD() = 0;
+
   bool NeedsToTurnOffAdvertisingMedium(
       location::nearby::proto::connections::Medium medium,
       const AdvertisingOptions& old_options,
       const AdvertisingOptions& new_options);
 
+  bool NeedsToTurnOffDiscoveryMedium(
+      location::nearby::proto::connections::Medium medium,
+      const DiscoveryOptions& old_options, const DiscoveryOptions& new_options);
+
   virtual std::vector<location::nearby::proto::connections::Medium>
   GetConnectionMediumsByPriority() = 0;
   virtual location::nearby::proto::connections::Medium

connections/implementation/base_pcp_handler_test.cc

@@ -24,6 +24,7 @@
 #include "gtest/gtest.h"
 #include "absl/strings/string_view.h"
 #include "absl/time/time.h"
+#include "connections/discovery_options.h"
 #include "connections/implementation/base_endpoint_channel.h"
 #include "connections/implementation/bwu_manager.h"
 #include "connections/implementation/client_proxy.h"
@@ -177,6 +178,10 @@ class MockPcpHandler : public BasePcpHandler {
               (ClientProxy*, absl::string_view, absl::string_view,
                absl::string_view, const AdvertisingOptions&),
               (override));
+  MOCK_METHOD(StartOperationResult, UpdateDiscoveryOptionsImpl,
+              (ClientProxy*, absl::string_view, absl::string_view,
+               absl::string_view, const DiscoveryOptions&),
+              (override));
 
   std::vector<location::nearby::proto::connections::Medium>
   GetConnectionMediumsByPriority() override {
@@ -250,6 +255,14 @@ class MockPcpHandler : public BasePcpHandler {
     return BasePcpHandler::NeedsToTurnOffAdvertisingMedium(medium, old_options,
                                                            new_options);
   }
+
+  bool NeedsToTurnOffDiscoveryMedium(
+      location::nearby::proto::connections::Medium medium,
+      const DiscoveryOptions& old_options,
+      const DiscoveryOptions& new_options) {
+    return BasePcpHandler::NeedsToTurnOffDiscoveryMedium(medium, old_options,
+                                                         new_options);
+  }
 };
 
 class MockContext {
@@ -356,7 +369,6 @@ class BasePcpHandlerTest
 
   void StartDiscovery(ClientProxy* client, MockPcpHandler* pcp_handler,
                       BooleanMediumSelector allowed = GetParam()) {
-    std::string service_id{"service"};
     DiscoveryOptions discovery_options{
         {
             Strategy::kP2pCluster,
@@ -365,10 +377,18 @@ class BasePcpHandlerTest
         true,  // auto_upgrade_bandwidth
         true,  // enforce_topology_constraints
     };
+    StartDiscoveryWithOptions(client, pcp_handler, discovery_options);
+  }
+
+  void StartDiscoveryWithOptions(ClientProxy* client,
+                                 MockPcpHandler* pcp_handler,
+                                 const DiscoveryOptions& discovery_options) {
+    std::string service_id("service");
     EXPECT_CALL(*pcp_handler, StartDiscoveryImpl(client, service_id, _))
         .WillOnce(Return(MockPcpHandler::StartOperationResult{
             .status = {Status::kSuccess},
-            .mediums = pcp_handler->GetMediumsFromSelector(allowed),
+            .mediums =
+                pcp_handler->GetMediumsFromSelector(discovery_options.allowed),
         }));
     EXPECT_EQ(pcp_handler->StartDiscovery(client, service_id, discovery_options,
                                           discovery_listener_),
@@ -376,6 +396,17 @@ class BasePcpHandlerTest
     EXPECT_TRUE(client->IsDiscovering());
   }
 
+  void UpdateDiscoveryOptions(ClientProxy* client, MockPcpHandler* pcp_handler,
+                              DiscoveryOptions new_options,
+                              Status expected_status) {
+    EXPECT_CALL(*pcp_handler, UpdateDiscoveryOptionsImpl)
+        .WillOnce(Return(MockPcpHandler::StartOperationResult{
+            .status = expected_status,
+            .mediums = pcp_handler->GetMediumsFromSelector(new_options.allowed),
+        }));
+    pcp_handler->UpdateDiscoveryOptions(client, "service", new_options);
+  }
+
   std::pair<std::unique_ptr<MockEndpointChannel>,
             std::unique_ptr<MockEndpointChannel>>
   SetupConnection(
@@ -1470,6 +1501,141 @@ TEST_F(BasePcpHandlerTest, TestUpdateAdvertisingOptionsFailsWithBadStatus) {
   env_.Stop();
 }
 
+TEST_F(BasePcpHandlerTest, TestNeedsToTurnOffDiscoveryMedium) {
+  Mediums m;
+  EndpointChannelManager ecm;
+  EndpointManager em(&ecm);
+  BwuManager bwu(m, em, ecm, {}, {});
+  MockPcpHandler pcp_handler(&m, &em, &ecm, &bwu);
+  BooleanMediumSelector old_meds{
+      .bluetooth = true,
+      .ble = true,
+      .wifi_lan = false,
+  };
+  DiscoveryOptions old_opts, new_opts;
+  old_opts.allowed = old_meds;
+  BooleanMediumSelector new_meds{
+      .bluetooth = false,
+      .ble = true,
+      .wifi_lan = false,
+  };
+  new_opts.allowed = new_meds;
+  EXPECT_TRUE(pcp_handler.NeedsToTurnOffDiscoveryMedium(Medium::BLUETOOTH,
+                                                        old_opts, new_opts));
+  EXPECT_FALSE(pcp_handler.NeedsToTurnOffDiscoveryMedium(Medium::BLE, old_opts,
+                                                         new_opts));
+  EXPECT_FALSE(pcp_handler.NeedsToTurnOffDiscoveryMedium(Medium::WIFI_LAN,
+                                                         old_opts, new_opts));
+}
+
+TEST_F(BasePcpHandlerTest, TestUpdateDiscoveryOptionsWorks) {
+  env_.Start();
+  DiscoveryOptions old_options{
+      {},
+      true,   // auto_upgrade_bandwidth
+      true,   // enforce_topology_constraints
+      false,  // is_out_of_band_connection,
+      "",     // fast_advertisement_service_uuid
+      false,  // low_power
+  };
+  DiscoveryOptions new_options{
+      {},
+      true,   // auto_upgrade_bandwidth
+      true,   // enforce_topology_constraints
+      false,  // is_out_of_band_connection,
+      "",     // fast_advertisement_service_uuid
+      true,   // low_power
+  };
+  ClientProxy client;
+  Mediums m;
+  EndpointChannelManager ecm;
+  EndpointManager em(&ecm);
+  BwuManager bwu(m, em, ecm, {}, {});
+  MockPcpHandler pcp_handler(&m, &em, &ecm, &bwu);
+  StartDiscoveryWithOptions(&client, &pcp_handler, old_options);
+  EXPECT_TRUE(client.IsDiscovering());
+  auto current_client_opts = client.GetDiscoveryOptions();
+  // check custom option parameters
+  EXPECT_EQ(current_client_opts.auto_upgrade_bandwidth,
+            old_options.auto_upgrade_bandwidth);
+  EXPECT_EQ(current_client_opts.enforce_topology_constraints,
+            old_options.enforce_topology_constraints);
+  EXPECT_EQ(current_client_opts.low_power, old_options.low_power);
+  EXPECT_EQ(current_client_opts.is_out_of_band_connection,
+            old_options.is_out_of_band_connection);
+  EXPECT_EQ(current_client_opts.fast_advertisement_service_uuid,
+            old_options.fast_advertisement_service_uuid);
+  UpdateDiscoveryOptions(&client, &pcp_handler, new_options,
+                         {Status::kSuccess});
+  EXPECT_TRUE(client.IsDiscovering());
+  current_client_opts = client.GetDiscoveryOptions();
+  // check new option parameters
+  EXPECT_EQ(current_client_opts.auto_upgrade_bandwidth,
+            new_options.auto_upgrade_bandwidth);
+  EXPECT_EQ(current_client_opts.enforce_topology_constraints,
+            new_options.enforce_topology_constraints);
+  EXPECT_EQ(current_client_opts.low_power, new_options.low_power);
+  EXPECT_EQ(current_client_opts.is_out_of_band_connection,
+            new_options.is_out_of_band_connection);
+  EXPECT_EQ(current_client_opts.fast_advertisement_service_uuid,
+            new_options.fast_advertisement_service_uuid);
+  env_.Stop();
+}
+
+TEST_F(BasePcpHandlerTest, TestUpdateDiscoveryOptionsFailsWithBadStatus) {
+  env_.Start();
+  DiscoveryOptions old_options{
+      {},
+      true,   // auto_upgrade_bandwidth
+      true,   // enforce_topology_constraints
+      false,  // is_out_of_band_connection,
+      "",     // fast_advertisement_service_uuid
+      false,  // low_power
+  };
+  DiscoveryOptions new_options{
+      {},
+      true,   // auto_upgrade_bandwidth
+      true,   // enforce_topology_constraints
+      false,  // is_out_of_band_connection,
+      "",     // fast_advertisement_service_uuid
+      true,   // low_power
+  };
+  ClientProxy client;
+  Mediums m;
+  EndpointChannelManager ecm;
+  EndpointManager em(&ecm);
+  BwuManager bwu(m, em, ecm, {}, {});
+  MockPcpHandler pcp_handler(&m, &em, &ecm, &bwu);
+  StartDiscoveryWithOptions(&client, &pcp_handler, old_options);
+  EXPECT_TRUE(client.IsDiscovering());
+  auto current_client_opts = client.GetDiscoveryOptions();
+  // check custom option parameters
+  EXPECT_EQ(current_client_opts.auto_upgrade_bandwidth,
+            old_options.auto_upgrade_bandwidth);
+  EXPECT_EQ(current_client_opts.enforce_topology_constraints,
+            old_options.enforce_topology_constraints);
+  EXPECT_EQ(current_client_opts.low_power, old_options.low_power);
+  EXPECT_EQ(current_client_opts.is_out_of_band_connection,
+            old_options.is_out_of_band_connection);
+  EXPECT_EQ(current_client_opts.fast_advertisement_service_uuid,
+            old_options.fast_advertisement_service_uuid);
+  UpdateDiscoveryOptions(&client, &pcp_handler, new_options,
+                         {Status::kBleError});
+  EXPECT_TRUE(client.IsDiscovering());
+  current_client_opts = client.GetDiscoveryOptions();
+  // check new option parameters
+  EXPECT_EQ(current_client_opts.auto_upgrade_bandwidth,
+            old_options.auto_upgrade_bandwidth);
+  EXPECT_EQ(current_client_opts.enforce_topology_constraints,
+            old_options.enforce_topology_constraints);
+  EXPECT_EQ(current_client_opts.low_power, old_options.low_power);
+  EXPECT_EQ(current_client_opts.is_out_of_band_connection,
+            old_options.is_out_of_band_connection);
+  EXPECT_EQ(current_client_opts.fast_advertisement_service_uuid,
+            old_options.fast_advertisement_service_uuid);
+  env_.Stop();
+}
+
 }  // namespace
 }  // namespace connections
 }  // namespace nearby

connections/implementation/client_proxy.h

@@ -129,6 +129,11 @@ class ClientProxy final {
     advertising_options_ = advertising_options;
   }
 
+  void UpdateDiscoveryOptions(const DiscoveryOptions& discovery_options) {
+    MutexLock lock(&mutex_);
+    discovery_options_ = discovery_options;
+  }
+
   // Proxies to the client's DiscoveryListener::OnEndpointFound() callback.
   void OnEndpointFound(const std::string& service_id,
                        const std::string& endpoint_id,

connections/implementation/offline_service_controller.cc

@@ -169,8 +169,9 @@ Status OfflineServiceController::UpdateAdvertisingOptions(
 Status OfflineServiceController::UpdateDiscoveryOptions(
     ClientProxy* client, absl::string_view service_id,
     const DiscoveryOptions& discovery_options) {
-  // TODO(b/284048592): Implement.
+  if (stop_) return {Status::kOutOfOrderApiCall};
-  return {Status::kError};
+  return pcp_manager_.UpdateDiscoveryOptions(client, service_id,
+                                             discovery_options);
 }
 
 void OfflineServiceController::SetCustomSavePath(ClientProxy* client,

connections/implementation/offline_service_controller_test.cc

@@ -428,6 +428,58 @@ TEST_P(OfflineServiceControllerTest, TestNoUpdateAdvertisingOptionsAfterStop) {
   env_.Stop();
 }
 
+TEST_P(OfflineServiceControllerTest, TestUpdateDiscoveryOptions) {
+  env_.Start();
+  OfflineSimulationUser user_a(kDeviceA, GetParam());
+  EXPECT_FALSE(user_a.IsDiscovering());
+  EXPECT_THAT(user_a.StartDiscovery(std::string(kServiceId), nullptr),
+              Eq(Status{Status::kSuccess}));
+  EXPECT_TRUE(user_a.IsDiscovering());
+  DiscoveryOptions new_options = {
+      {
+          Strategy::kP2pCluster,
+          GetParam(),
+      },
+      false,  // auto_upgrade_bandwidth
+      false,  // enforce_topology_constraints
+      true,   // is_out_of_band_connection
+      "",     // fast_advertisement_service_uuid
+      true,   // low_power
+  };
+  // TODO(b/284048592): Change to kSuccess when implemented.
+  EXPECT_THAT(user_a.UpdateDiscoveryOptions(kServiceId, new_options),
+              Eq(Status{Status::kError}));
+  EXPECT_TRUE(user_a.IsDiscovering());
+  user_a.Stop();
+  env_.Stop();
+}
+
+TEST_P(OfflineServiceControllerTest, TestNoUpdateDiscoveryOptionsAfterStop) {
+  env_.Start();
+  OfflineSimulationUser user_a(kDeviceA, GetParam());
+  EXPECT_FALSE(user_a.IsDiscovering());
+  EXPECT_THAT(user_a.StartDiscovery(std::string(kServiceId), nullptr),
+              Eq(Status{Status::kSuccess}));
+  EXPECT_TRUE(user_a.IsDiscovering());
+  DiscoveryOptions new_options = {
+      {
+          Strategy::kP2pCluster,
+          GetParam(),
+      },
+      false,  // auto_upgrade_bandwidth
+      false,  // enforce_topology_constraints
+      true,   // is_out_of_band_connection
+      "",     // fast_advertisement_service_uuid
+      true,   // low_power
+  };
+  user_a.Stop();
+  EXPECT_FALSE(user_a.IsDiscovering());
+  EXPECT_THAT(user_a.UpdateDiscoveryOptions(kServiceId, new_options),
+              Eq(Status{Status::kOutOfOrderApiCall}));
+  EXPECT_FALSE(user_a.IsDiscovering());
+  env_.Stop();
+}
+
 INSTANTIATE_TEST_SUITE_P(ParametrisedOfflineServiceControllerTest,
                          OfflineServiceControllerTest,
                          ::testing::ValuesIn(kTestCases));

connections/implementation/offline_simulation_user.cc

@@ -152,6 +152,11 @@ Status OfflineSimulationUser::StartDiscovery(const std::string& service_id,
 
 void OfflineSimulationUser::StopDiscovery() { ctrl_.StopDiscovery(&client_); }
 
+Status OfflineSimulationUser::UpdateDiscoveryOptions(
+    absl::string_view service_id, const DiscoveryOptions& options) {
+  return ctrl_.UpdateDiscoveryOptions(&client_, service_id, options);
+}
+
 void OfflineSimulationUser::InjectEndpoint(
     const std::string& service_id,
     const OutOfBandConnectionMetadata& metadata) {

connections/implementation/offline_simulation_user.h

@@ -94,6 +94,10 @@ class OfflineSimulationUser {
   // Calls PcpManager::StopDiscovery().
   void StopDiscovery();
 
+  // Calls PcpManager::UpdateDiscoveryOptions().
+  Status UpdateDiscoveryOptions(absl::string_view service_id,
+                                const DiscoveryOptions& options);
+
   // Calls PcpManager::InjectEndpoint();
   void InjectEndpoint(const std::string& service_id,
                       const OutOfBandConnectionMetadata& metadata);

connections/implementation/p2p_cluster_pcp_handler.h

@@ -108,6 +108,18 @@ class P2pClusterPcpHandler : public BasePcpHandler {
       absl::string_view local_endpoint_info,
       const AdvertisingOptions& advertising_options) override;
 
+  // @PCPHandlerThread
+  BasePcpHandler::StartOperationResult UpdateDiscoveryOptionsImpl(
+      ClientProxy* client, absl::string_view service_id,
+      absl::string_view local_endpoint_id,
+      absl::string_view local_endpoint_info,
+      const DiscoveryOptions& discovery_options) override {
+    // TODO(b/284048592): Implement.
+    return StartOperationResult {
+      .status = {Status::kError},
+    };
+  }
+
  private:
   // Holds the state required to re-create a BleEndpoint we see on a
   // BlePeripheral, so BlePeripheralLostHandler can call

connections/implementation/pcp_handler.h

@@ -125,6 +125,10 @@ class PcpHandler {
   virtual Status UpdateAdvertisingOptions(
       ClientProxy* client, absl::string_view service_id,
       const AdvertisingOptions& advertising_options) = 0;
+
+  virtual Status UpdateDiscoveryOptions(
+      ClientProxy* client, absl::string_view service_id,
+      const DiscoveryOptions& discovery_options) = 0;
 };
 
 }  // namespace connections

connections/implementation/pcp_manager.cc

@@ -157,6 +157,16 @@ Status PcpManager::UpdateAdvertisingOptions(
                                             advertising_options);
 }
 
+Status PcpManager::UpdateDiscoveryOptions(
+    ClientProxy* client, absl::string_view service_id,
+    const DiscoveryOptions& discovery_options) {
+  if (!current_) {
+    return {Status::kOutOfOrderApiCall};
+  }
+  return current_->UpdateDiscoveryOptions(client, service_id,
+                                          discovery_options);
+}
+
 bool PcpManager::SetCurrentPcpHandler(Strategy strategy) {
   current_ = GetPcpHandler(StrategyToPcp(strategy));
 

connections/implementation/pcp_manager.h

@@ -79,6 +79,10 @@ class PcpManager {
       ClientProxy* client, absl::string_view service_id,
       const AdvertisingOptions& advertising_options);
 
+  Status UpdateDiscoveryOptions(ClientProxy* client,
+                                absl::string_view service_id,
+                                const DiscoveryOptions& discovery_options);
+
   location::nearby::proto::connections::Medium GetBandwidthUpgradeMedium();
   void DisconnectFromEndpointManager();
 

connections/implementation/pcp_manager_test.cc

@@ -221,6 +221,16 @@ TEST_F(PcpManagerTest, InjectEndpoint) {
   env_.Stop();
 }
 
+TEST_F(PcpManagerTest, TestUpdateDiscoveryFailsWithoutPcpHandler) {
+  BooleanMediumSelector selector;
+  selector.SetAll(true);
+  env_.Start();
+  SimulationUser user_a(kDeviceA, selector);
+  EXPECT_EQ(user_a.UpdateDiscoveryOptions(kServiceId).value,
+            Status::Value::kOutOfOrderApiCall);
+  env_.Stop();
+}
+
 }  // namespace
 }  // namespace connections
 }  // namespace nearby

connections/implementation/simulation_user.cc

@@ -129,6 +129,10 @@ void SimulationUser::StartDiscovery(const std::string& service_id,
           .Ok());
 }
 
+Status SimulationUser::UpdateDiscoveryOptions(absl::string_view service_id) {
+  return mgr_.UpdateDiscoveryOptions(&client_, service_id, discovery_options_);
+}
+
 void SimulationUser::InjectEndpoint(
     const std::string& service_id,
     const OutOfBandConnectionMetadata& metadata) {

connections/implementation/simulation_user.h

@@ -91,6 +91,9 @@ class SimulationUser {
   // callback.
   void StartDiscovery(const std::string& service_id, CountDownLatch* latch);
 
+  // Calls PcpManager::UpdateDiscoveryOptions.
+  Status UpdateDiscoveryOptions(absl::string_view service_id);
+
   // Calls PcpManager::InjectEndpoint.
   void InjectEndpoint(const std::string& service_id,
                       const OutOfBandConnectionMetadata& metadata);