Check listening options as well when we want to enforce topology constraints

PiperOrigin-RevId: 543497497

connections/implementation/base_pcp_handler.cc

@@ -34,6 +34,7 @@
 #include "connections/implementation/proto/offline_wire_formats.pb.h"
 #include "connections/medium_selector.h"
 #include "connections/status.h"
+#include "connections/v3/connection_listening_options.h"
 #include "connections/v3/connections_device.h"
 #include "connections/v3/listeners.h"
 #include "internal/flags/nearby_flags.h"
@@ -611,7 +612,7 @@ Status BasePcpHandler::RequestConnection(
 
         // If our child class says we can't send any more outgoing connections,
         // listen to them.
-        if (ShouldEnforceTopologyConstraints(client->GetAdvertisingOptions()) &&
+        if (client->ShouldEnforceTopologyConstraints() &&
             !CanSendOutgoingConnection(client)) {
           NEARBY_LOGS(INFO)
               << "In requestConnection(), client=" << client->GetClientId()
@@ -974,17 +975,6 @@ void BasePcpHandler::ProcessPreConnectionResultFailure(
   client->OnConnectionRejected(endpoint_id, {Status::kError});
 }
 
-bool BasePcpHandler::ShouldEnforceTopologyConstraints(
-    const AdvertisingOptions& local_advertising_options) const {
-  // Topology constraints only matter for the advertiser.
-  // For discoverers, we'll always enforce them.
-  if (local_advertising_options.strategy.IsNone()) {
-    return true;
-  }
-
-  return local_advertising_options.enforce_topology_constraints;
-}
-
 bool BasePcpHandler::AutoUpgradeBandwidth(
     const AdvertisingOptions& local_advertising_options) const {
   if (local_advertising_options.strategy.IsNone()) {
@@ -1388,7 +1378,7 @@ Exception BasePcpHandler::OnIncomingConnection(
 
   // If our child class says we can't accept any more incoming connections,
   // listen to them.
-  if (ShouldEnforceTopologyConstraints(client->GetAdvertisingOptions()) &&
+  if (client->ShouldEnforceTopologyConstraints() &&
       !CanReceiveIncomingConnection(client)) {
     NEARBY_LOGS(ERROR) << "Incoming connections are currently disallowed.";
     return {Exception::kIo};

connections/implementation/base_pcp_handler.h

@@ -36,6 +36,7 @@
 #include "connections/listeners.h"
 #include "connections/medium_selector.h"
 #include "connections/status.h"
+#include "connections/v3/connection_listening_options.h"
 #include "connections/v3/listeners.h"
 #include "internal/platform/atomic_boolean.h"
 #include "internal/platform/byte_array.h"
@@ -435,10 +436,6 @@ class BasePcpHandler : public PcpHandler,
   bool IsPreferred(const BasePcpHandler::DiscoveredEndpoint& new_endpoint,
                    const BasePcpHandler::DiscoveredEndpoint& old_endpoint);
 
-  // Returns true, if connection party should respect the specified topology.
-  bool ShouldEnforceTopologyConstraints(
-      const AdvertisingOptions& local_advertising_options) const;
-
   // Returns true, if connection party should attempt to upgrade itself to
   // use a higher bandwidth medium, if it is available.
   bool AutoUpgradeBandwidth(

connections/implementation/client_proxy.cc

@@ -711,6 +711,21 @@ bool ClientProxy::RemoteConnectionIsAccepted(std::string endpoint_id) const {
       endpoint_id, ClientProxy::Connection::kRemoteEndpointAccepted);
 }
 
+bool ClientProxy::ShouldEnforceTopologyConstraints() const {
+  bool result = false;
+  if (IsAdvertising() &&
+      (GetAdvertisingOptions().strategy.IsNone() ||
+       GetAdvertisingOptions().enforce_topology_constraints)) {
+    result |= true;
+  }
+  if (IsListeningForIncomingConnections() &&
+      (GetListeningOptions().strategy.IsNone() ||
+       GetListeningOptions().enforce_topology_constraints)) {
+    result |= true;
+  }
+  return result;
+}
+
 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) {

connections/implementation/client_proxy.h

@@ -197,6 +197,8 @@ class ClientProxy final {
   // Returns true if either the local endpoint or the remote endpoint has
   // rejected the connection.
   bool IsConnectionRejected(const std::string& endpoint_id) const;
+  // Returns true if the client should enforce topology constraints.
+  bool ShouldEnforceTopologyConstraints() const;
 
   // Proxies to the client's PayloadListener::OnPayload() callback.
   void OnPayload(const std::string& endpoint_id, Payload payload);

connections/implementation/client_proxy_test.cc

@@ -18,6 +18,7 @@
 #include <memory>
 #include <optional>
 #include <string>
+#include <utility>
 
 #include "gmock/gmock.h"
 #include "protobuf-matchers/protocol-buffer-matchers.h"
@@ -144,6 +145,23 @@ class ClientProxyTest : public ::testing::TestWithParam<FeatureFlags::Flags> {
     EXPECT_TRUE(client->HasPendingConnectionToEndpoint(endpoint.id));
   }
 
+  Endpoint StartListeningForIncomingConnections(
+      ClientProxy* client, v3::ConnectionListener listener,
+      v3::ConnectionListeningOptions options = {}) {
+    Endpoint endpoint{
+        .info = ByteArray{"advertising endpoint name"},
+        .id = client->GetLocalEndpointId(),
+    };
+    client->StartedListeningForIncomingConnections(
+        service_id_, strategy_, std::move(listener), options);
+    return endpoint;
+  }
+
+  void StopListeningForIncomingConnections(ClientProxy* client) {
+    client->StoppedListeningForIncomingConnections();
+    EXPECT_FALSE(client->IsListeningForIncomingConnections());
+  }
+
   Endpoint StartDiscovery(ClientProxy* client, DiscoveryListener listener) {
     Endpoint endpoint{
         .info = ByteArray{"discovery endpoint name"},
@@ -1064,6 +1082,30 @@ TEST_F(ClientProxyTest, TestGetIncomingConnectionListener) {
   EXPECT_TRUE(disconnect_latch.Await().Ok());
 }
 
+TEST_F(ClientProxyTest, EnforceTopologyWhenRequestedAdvertising) {
+  EXPECT_FALSE(client1_.ShouldEnforceTopologyConstraints());
+  StartAdvertising(&client1_, advertising_connection_listener_,
+                   {.enforce_topology_constraints = true});
+  EXPECT_TRUE(client1_.ShouldEnforceTopologyConstraints());
+}
+
+TEST_F(ClientProxyTest, EnforceTopologyWhenRequestedListeningWithStrategy) {
+  EXPECT_FALSE(client1_.ShouldEnforceTopologyConstraints());
+  StartListeningForIncomingConnections(&client1_,
+                                       {},
+                                       {.strategy = Strategy::kP2pCluster,
+                                        .enforce_topology_constraints = true});
+  EXPECT_TRUE(client1_.ShouldEnforceTopologyConstraints());
+}
+
+TEST_F(ClientProxyTest, DontEnforceTopologyWhenRequestedWithNoStrategy) {
+  EXPECT_FALSE(client1_.ShouldEnforceTopologyConstraints());
+  StartListeningForIncomingConnections(&client1_,
+                                       {},
+                                       {.strategy = Strategy::kNone});
+  EXPECT_TRUE(client1_.ShouldEnforceTopologyConstraints());
+}
+
 }  // namespace
 }  // namespace connections
 }  // namespace nearby