Add endpoint ID to share target.

PiperOrigin-RevId: 727070543

sharing/BUILD

@@ -88,7 +88,6 @@ cc_library(
         "//sharing/common:enum",
         "//sharing/internal/public:logging",
         "//sharing/proto:enums_cc_proto",
-        "//sharing/proto:wire_format_cc_proto",
         "@com_google_absl//absl/container:flat_hash_set",
         "@com_google_absl//absl/strings",
         "@com_google_absl//absl/strings:str_format",
@@ -841,10 +840,12 @@ cc_test(
         ":transfer_metadata_matchers",
         ":types",
         "//internal/analytics:mock_event_logger",
+        "//internal/network:url",
         "//internal/platform/implementation/g3",  # fixdeps: keep
         "//internal/test",
         "//proto:sharing_enums_cc_proto",
         "//sharing/analytics",
+        "//sharing/common:enum",
         "//sharing/internal/public:logging",
         "//sharing/proto:wire_format_cc_proto",
         "//sharing/proto/analytics:sharing_log_cc_proto",

sharing/incoming_share_session.cc

@@ -65,12 +65,12 @@ using ::nearby::sharing::service::proto::WifiCredentials;
 IncomingShareSession::IncomingShareSession(
     Clock* clock, TaskRunner& service_thread,
     NearbyConnectionsManager* connections_manager,
-    analytics::AnalyticsRecorder& analytics_recorder, std::string endpoint_id,
+    analytics::AnalyticsRecorder& analytics_recorder,
     const ShareTarget& share_target,
     std::function<void(const IncomingShareSession&, const TransferMetadata&)>
         transfer_update_callback)
     : ShareSession(clock, service_thread, connections_manager,
-                   analytics_recorder, std::move(endpoint_id), share_target),
+                   analytics_recorder, share_target),
       transfer_update_callback_(std::move(transfer_update_callback)) {}
 
 IncomingShareSession::IncomingShareSession(IncomingShareSession&&) = default;

sharing/incoming_share_session.h

@@ -19,7 +19,6 @@
 #include <functional>
 #include <memory>
 #include <optional>
-#include <string>
 #include <vector>
 
 #include "absl/functional/any_invocable.h"
@@ -45,7 +44,7 @@ class IncomingShareSession : public ShareSession {
   IncomingShareSession(
       Clock* clock, TaskRunner& service_thread,
       NearbyConnectionsManager* connections_manager,
-      analytics::AnalyticsRecorder& analytics_recorder, std::string endpoint_id,
+      analytics::AnalyticsRecorder& analytics_recorder,
       const ShareTarget& share_target,
       std::function<void(const IncomingShareSession&, const TransferMetadata&)>
           transfer_update_callback);

sharing/incoming_share_session_test.cc

@@ -32,12 +32,14 @@
 #include "absl/time/time.h"
 #include "internal/analytics/mock_event_logger.h"
 #include "internal/analytics/sharing_log_matchers.h"
+#include "internal/network/url.h"
 #include "internal/test/fake_clock.h"
 #include "internal/test/fake_device_info.h"
 #include "internal/test/fake_task_runner.h"
 #include "proto/sharing_enums.pb.h"
 #include "sharing/analytics/analytics_recorder.h"
 #include "sharing/attachment_compare.h"  // IWYU pragma: keep
+#include "sharing/common/nearby_share_enums.h"
 #include "sharing/fake_nearby_connections_manager.h"
 #include "sharing/file_attachment.h"
 #include "sharing/internal/public/logging.h"
@@ -119,7 +121,7 @@ class IncomingShareSessionTest : public ::testing::Test {
   IncomingShareSessionTest()
       : connection_(device_info_),
         session_(&clock_, task_runner_, &connections_manager_,
-                 analytics_recorder_, std::string(kEndpointId), share_target_,
+                 analytics_recorder_, share_target_,
                  transfer_metadata_callback_.AsStdFunction()) {
     CHECK(
         proto2::TextFormat::ParseFromString(R"pb(
@@ -190,7 +192,10 @@ class IncomingShareSessionTest : public ::testing::Test {
   nearby::analytics::MockEventLogger mock_event_logger_;
   analytics::AnalyticsRecorder analytics_recorder_{/*vendor_id=*/0,
                                                    &mock_event_logger_};
-  ShareTarget share_target_;
+  ShareTarget share_target_ = ShareTarget::CreateShareTargetForTest(
+      "deviceName", ::nearby::network::Url(), ShareTargetType::kPhone,
+      /* is_incoming */ true, kEndpointId, "test_full_name",
+      /* is_known */ false, "test_device_id", /*for_self_share=*/false);
   MockFunction<void(const IncomingShareSession&, const TransferMetadata&)>
       transfer_metadata_callback_;
   FakeDeviceInfo device_info_;
@@ -1088,9 +1093,9 @@ TEST_F(IncomingShareSessionTest, ReadyForTransferNotSelfShare) {
 TEST_F(IncomingShareSessionTest, ReadyForTransferSelfShare) {
   ShareTarget share_target;
   share_target.for_self_share = true;
+  share_target.endpoint_id = "XYCA";
   IncomingShareSession session(&clock_, task_runner_, &connections_manager_,
-                               analytics_recorder_, std::string("XYCA"),
-                               share_target,
+                               analytics_recorder_, share_target,
                                transfer_metadata_callback_.AsStdFunction());
   session.set_session_id(1234);
   session.OnConnected(&connection_);

sharing/nearby_sharing_service_impl.cc

@@ -32,7 +32,6 @@
 #include <utility>
 #include <vector>
 
-#include "absl/algorithm/container.h"
 #include "absl/container/flat_hash_map.h"
 #include "absl/functional/any_invocable.h"
 #include "absl/functional/bind_front.h"
@@ -1038,9 +1037,10 @@ void NearbySharingServiceImpl::OnIncomingConnection(
 
   ShareTarget placeholder_share_target;
   placeholder_share_target.is_incoming = true;
+  placeholder_share_target.endpoint_id = std::string(endpoint_id);
   int64_t placeholder_share_target_id = placeholder_share_target.id;
   IncomingShareSession& session = CreateIncomingShareSession(
-      placeholder_share_target, endpoint_id, /*certificate=*/std::nullopt);
+      placeholder_share_target, /*certificate=*/std::nullopt);
   session.set_session_id(analytics_recorder_.GenerateNextId());
   session.OnConnected(connection);
   connection->SetDisconnectionListener([this, placeholder_share_target_id]() {
@@ -1786,14 +1786,13 @@ void NearbySharingServiceImpl::OnOutgoingDecryptedCertificate(
     FinishEndpointDiscoveryEvent();
     return;
   }
-  if (FindDuplicateInOutgoingShareTargets(endpoint_id, *share_target)) {
-    DeduplicateInOutgoingShareTarget(*share_target, endpoint_id,
-                                     std::move(certificate));
+  if (FindDuplicateInOutgoingShareTargets(*share_target)) {
+    DeduplicateInOutgoingShareTarget(*share_target, std::move(certificate));
     FinishEndpointDiscoveryEvent();
     return;
   }
-  if (FindDuplicateInDiscoveryCache(endpoint_id, *share_target)) {
-    DeDuplicateInDiscoveryCache(*share_target, endpoint_id,
+  if (FindDuplicateInDiscoveryCache(*share_target)) {
+    DeDuplicateInDiscoveryCache(*share_target,
                                 std::move(certificate));
     FinishEndpointDiscoveryEvent();
     return;
@@ -1802,8 +1801,7 @@ void NearbySharingServiceImpl::OnOutgoingDecryptedCertificate(
   VLOG(1) << __func__ << ": Adding (endpoint_id=" << endpoint_id
           << ", share_target_id=" << share_target->id
           << ") to outgoing share target map";
-  CreateOutgoingShareSession(*share_target, endpoint_id,
-                             std::move(certificate));
+  CreateOutgoingShareSession(*share_target, std::move(certificate));
 
   // Update the endpoint id for the share target.
   LOG(INFO) << __func__ << ": An endpoint: " << endpoint_id
@@ -1832,8 +1830,7 @@ void NearbySharingServiceImpl::OnOutgoingDecryptedCertificate(
   OnShareTargetDiscovered(*share_target);
 
   VLOG(1) << __func__ << ": Reported OnShareTargetDiscovered: share_target: "
-          << share_target->ToString() << " endpoint_id=" << endpoint_id
-          << " to all send surfaces.";
+          << share_target->ToString() << " to all send surfaces.";
 
   FinishEndpointDiscoveryEvent();
 }
@@ -2696,8 +2693,8 @@ void NearbySharingServiceImpl::OnIncomingDecryptedCertificate(
   VLOG(1) << __func__ << ": Received incoming connection from "
           << share_target_id;
 
-  IncomingShareSession& session = CreateIncomingShareSession(
-      *share_target, endpoint_id, std::move(certificate));
+  IncomingShareSession& session =
+      CreateIncomingShareSession(*share_target, std::move(certificate));
   // Copy session id from placeholder session to actual session.
   session.set_session_id(session_id);
   session.OnConnected(connection);
@@ -2960,6 +2957,7 @@ std::optional<ShareTarget> NearbySharingServiceImpl::CreateShareTarget(
   target.device_name = std::move(*device_name);
   target.is_incoming = is_incoming;
   target.device_id = GetDeviceId(endpoint_id, certificate);
+  target.endpoint_id = std::string(endpoint_id);
   target.vendor_id = advertisement.vendor_id();
   if (certificate.has_value()) {
     target.for_self_share = certificate->for_self_share();
@@ -3126,13 +3124,12 @@ void NearbySharingServiceImpl::RemoveIncomingPayloads(
 }
 
 IncomingShareSession& NearbySharingServiceImpl::CreateIncomingShareSession(
-    const ShareTarget& share_target, absl::string_view endpoint_id,
+    const ShareTarget& share_target,
     std::optional<NearbyShareDecryptedPublicCertificate> certificate) {
   DCHECK(share_target.is_incoming);
   auto [it, inserted] = incoming_share_session_map_.try_emplace(
       share_target.id, context_->GetClock(), *service_thread_,
-      nearby_connections_manager_.get(), analytics_recorder_,
-      std::string(endpoint_id), share_target,
+      nearby_connections_manager_.get(), analytics_recorder_, share_target,
       absl::bind_front(&NearbySharingServiceImpl::OnIncomingTransferUpdate,
                        this));
   if (!inserted) {
@@ -3146,7 +3143,7 @@ IncomingShareSession& NearbySharingServiceImpl::CreateIncomingShareSession(
 }
 
 void NearbySharingServiceImpl::DeduplicateInOutgoingShareTarget(
-    const ShareTarget& share_target, absl::string_view endpoint_id,
+    const ShareTarget& share_target,
     std::optional<NearbyShareDecryptedPublicCertificate> certificate) {
   // TODO(b/343764269): may need to update last_outgoing_metadata_ if the
   // deduped target id matches the one in last_outgoing_metadata_.
@@ -3164,8 +3161,8 @@ void NearbySharingServiceImpl::DeduplicateInOutgoingShareTarget(
               << " is connected, not updating outgoing_share_session_map_.";
     return;
   }
-  session_it->second.UpdateSessionForDedup(share_target, std::move(certificate),
-                                           endpoint_id);
+  session_it->second.UpdateSessionForDedup(share_target,
+                                           std::move(certificate));
 
   OnShareTargetUpdated(share_target);
 
@@ -3176,9 +3173,9 @@ void NearbySharingServiceImpl::DeduplicateInOutgoingShareTarget(
 }
 
 void NearbySharingServiceImpl::DeDuplicateInDiscoveryCache(
-    const ShareTarget& share_target, absl::string_view endpoint_id,
+    const ShareTarget& share_target,
     std::optional<NearbyShareDecryptedPublicCertificate> certificate) {
-  CreateOutgoingShareSession(share_target, endpoint_id, std::move(certificate));
+  CreateOutgoingShareSession(share_target, std::move(certificate));
   OnShareTargetUpdated(share_target);
 
   LOG(INFO) << __func__
@@ -3188,13 +3185,13 @@ void NearbySharingServiceImpl::DeDuplicateInDiscoveryCache(
 }
 
 bool NearbySharingServiceImpl::FindDuplicateInDiscoveryCache(
-    absl::string_view endpoint_id, ShareTarget& share_target) {
-  auto it = discovery_cache_.find(endpoint_id);
+    ShareTarget& share_target) {
+  auto it = discovery_cache_.find(share_target.endpoint_id);
   if (it != discovery_cache_.end()) {
     // If endpoint info changes for an endpoint ID, NC will send a rediscovery
     // event for the same endpoint id.
-    LOG(INFO) << __func__
-              << ": [Dedupped] Found duplicate endpoint_id: " << endpoint_id
+    LOG(INFO) << __func__ << ": [Dedupped] Found duplicate endpoint_id: "
+              << share_target.endpoint_id
               << ", share_target.id changed from: " << share_target.id << " to "
               << it->second.share_target.id;
     share_target.id = it->second.share_target.id;
@@ -3208,7 +3205,7 @@ bool NearbySharingServiceImpl::FindDuplicateInDiscoveryCache(
                 << ": [Dedupped] Found duplicate device_id, share_target.id "
                    "changed from: "
                 << share_target.id << " to " << it->second.share_target.id
-                << ". New endpoint_id: " << endpoint_id;
+                << ". New endpoint_id: " << share_target.endpoint_id;
       // Share targets in discovery cache have receive_disabled set to true.
       // Copy only the id field from cache entry,
       share_target.id = it->second.share_target.id;
@@ -3220,17 +3217,17 @@ bool NearbySharingServiceImpl::FindDuplicateInDiscoveryCache(
 }
 
 bool NearbySharingServiceImpl::FindDuplicateInOutgoingShareTargets(
-    absl::string_view endpoint_id, ShareTarget& share_target) {
+    ShareTarget& share_target) {
   // If the duplicate is found, share_target.id needs to be updated to the old
   // "discovered" share_target_id so OnShareTargetUpdated matches a target that
   // was discovered before.
 
-  auto it = outgoing_share_target_map_.find(endpoint_id);
+  auto it = outgoing_share_target_map_.find(share_target.endpoint_id);
   if (it != outgoing_share_target_map_.end()) {
     // If endpoint info changes for an endpoint ID, NC will send a rediscovery
     // event for the same endpoint id.
-    LOG(INFO) << __func__
-              << ": [Dedupped] Found duplicate endpoint_id: " << endpoint_id
+    LOG(INFO) << __func__ << ": [Dedupped] Found duplicate endpoint_id: "
+              << share_target.endpoint_id
               << " in outgoing_share_target_map, share_target.id changed from: "
               << share_target.id << " to " << it->second.id;
     share_target.id = it->second.id;
@@ -3245,12 +3242,13 @@ bool NearbySharingServiceImpl::FindDuplicateInOutgoingShareTargets(
           << __func__
           << ": [Dedupped] Found duplicate device_id, endpoint ID "
              "changed from: "
-          << it->first << " to " << endpoint_id
+          << it->first << " to " << share_target.endpoint_id
           << " in outgoing_share_target_map, share_target.id changed from: "
           << share_target.id << " to " << it->second.id;
       share_target.id = it->second.id;
       outgoing_share_target_map_.erase(it);
-      outgoing_share_target_map_.insert_or_assign(endpoint_id, share_target);
+      outgoing_share_target_map_.insert_or_assign(share_target.endpoint_id,
+                                                  share_target);
       return true;
     }
   }
@@ -3305,7 +3303,7 @@ void NearbySharingServiceImpl::MoveToDiscoveryCache(std::string endpoint_id,
   cache_entry.expiry_timer = std::make_unique<ThreadTimer>(
       *service_thread_, absl::StrCat("discovery_cache_timeout_", endpoint_id),
       absl::Milliseconds(expiry_ms),
-      [this, expiry_ms, endpoint_id = std::string(endpoint_id)]() {
+      [this, expiry_ms, endpoint_id = cache_entry.share_target.endpoint_id]() {
         auto cache_node = discovery_cache_.extract(endpoint_id);
         if (cache_node.empty()) {
           LOG(WARNING) << "Trying to remove endpoint_id: " << endpoint_id
@@ -3327,20 +3325,26 @@ void NearbySharingServiceImpl::MoveToDiscoveryCache(std::string endpoint_id,
       });
   // Send ShareTarget update to set receive disabled to true.
   OnShareTargetUpdated(cache_entry.share_target);
-  auto [it, inserted] =
-      discovery_cache_.insert_or_assign(endpoint_id, std::move(cache_entry));
-  LOG(INFO) << "[Dedupped] added to discovery_cache: " << endpoint_id << " by "
+  // The endpoint ID in the share target may not be the same as the endpoint ID
+  // passed into this function if endpoint ID was updated when there is an
+  // active connection to the share target.  Use the updated endpoint ID to add
+  // to the discovery cache.
+  std::string share_target_endpoint_id = cache_entry.share_target.endpoint_id;
+  auto [it, inserted] = discovery_cache_.insert_or_assign(
+      share_target_endpoint_id, std::move(cache_entry));
+  LOG(INFO) << "[Dedupped] added to discovery_cache: "
+            << share_target_endpoint_id << " by "
             << (inserted ? "insert" : "assign");
 }
 
 void NearbySharingServiceImpl::CreateOutgoingShareSession(
-    const ShareTarget& share_target, absl::string_view endpoint_id,
+    const ShareTarget& share_target,
     std::optional<NearbyShareDecryptedPublicCertificate> certificate) {
-  outgoing_share_target_map_.insert_or_assign(endpoint_id, share_target);
+  outgoing_share_target_map_.insert_or_assign(share_target.endpoint_id,
+                                              share_target);
   auto [it_out, inserted] = outgoing_share_session_map_.try_emplace(
       share_target.id, context_->GetClock(), *service_thread_,
-      nearby_connections_manager_.get(), analytics_recorder_,
-      std::string(endpoint_id), share_target,
+      nearby_connections_manager_.get(), analytics_recorder_, share_target,
       absl::bind_front(&NearbySharingServiceImpl::OnOutgoingTransferUpdate,
                        this));
   if (!inserted) {

sharing/nearby_sharing_service_impl.h

@@ -366,10 +366,10 @@ class NearbySharingServiceImpl
   void RemoveIncomingPayloads(const IncomingShareSession& session);
 
   IncomingShareSession& CreateIncomingShareSession(
-      const ShareTarget& share_target, absl::string_view endpoint_id,
+      const ShareTarget& share_target,
       std::optional<NearbyShareDecryptedPublicCertificate> certificate);
   void CreateOutgoingShareSession(
-      const ShareTarget& share_target, absl::string_view endpoint_id,
+      const ShareTarget& share_target,
       std::optional<NearbyShareDecryptedPublicCertificate> certificate);
 
   // Move the endpoint to the discovery cache with the given expiry time.
@@ -378,27 +378,25 @@ class NearbySharingServiceImpl
   // Update the entry in outgoing_share_session_map_ with the new share target
   // and OnShareTargetUpdated is called.
   void DeduplicateInOutgoingShareTarget(
-      const ShareTarget& share_target, absl::string_view endpoint_id,
+      const ShareTarget& share_target,
       std::optional<NearbyShareDecryptedPublicCertificate> certificate);
 
   // Add an entry to the outgoing_share_session_map_ and
   // outgoing_share_target_map_ and OnShareTargetUpdated is called.
   void DeDuplicateInDiscoveryCache(
-      const ShareTarget& share_target, absl::string_view endpoint_id,
+      const ShareTarget& share_target,
       std::optional<NearbyShareDecryptedPublicCertificate> certificate);
 
   // Looks for a duplicate of the share target in the outgoing share
   // target map. The share target's id is changed to match an existing target if
   // available. Returns true if the duplicate is found.
-  bool FindDuplicateInOutgoingShareTargets(absl::string_view endpoint_id,
-                                           ShareTarget& share_target);
+  bool FindDuplicateInOutgoingShareTargets(ShareTarget& share_target);
 
   // Looks for a duplicate of the share target in the discovery cache.
   // If found, the share target is removed from the discovery cache and its
   // id is copied into `share_target`.
   // Returns true if the duplicate is found.
-  bool FindDuplicateInDiscoveryCache(absl::string_view endpoint_id,
-                                     ShareTarget& share_target);
+  bool FindDuplicateInDiscoveryCache(ShareTarget& share_target);
 
   ShareSession* GetShareSession(int64_t share_target_id);
   IncomingShareSession* GetIncomingShareSession(int64_t share_target_id);

sharing/nearby_sharing_service_impl_test.cc

@@ -5021,10 +5021,10 @@ TEST_F(NearbySharingServiceImplTest, RemoveIncomingPayloads) {
                                                   &mock_event_logger};
   ShareTarget share_target;
   share_target.is_incoming = true;
+  share_target.endpoint_id = "endpoint_id";
   IncomingShareSession session(
       fake_context_.fake_clock(), *sharing_service_task_runner_,
-      fake_nearby_connections_manager_, analytics_recorder, "endpoint_id",
-      share_target,
+      fake_nearby_connections_manager_, analytics_recorder, share_target,
       [](const IncomingShareSession&, const TransferMetadata&) {});
   service_->RemoveIncomingPayloads(session);
   EXPECT_EQ(