Fix updating OutgoingShareSession for dedup.

PiperOrigin-RevId: 725347879

Author: ftsui

sharing/outgoing_share_session.cc

@@ -25,6 +25,7 @@
 #include <utility>
 #include <vector>
 
+#include "absl/functional/any_invocable.h"
 #include "absl/strings/string_view.h"
 #include "absl/time/time.h"
 #include "internal/platform/clock.h"
@@ -116,7 +117,7 @@ OutgoingShareSession::OutgoingShareSession(
     NearbyConnectionsManager* connections_manager,
     analytics::AnalyticsRecorder& analytics_recorder, std::string endpoint_id,
     const ShareTarget& share_target,
-    std::function<void(OutgoingShareSession&, const TransferMetadata&)>
+    absl::AnyInvocable<void(OutgoingShareSession&, const TransferMetadata&)>
         transfer_update_callback)
     : ShareSession(clock, service_thread, connections_manager,
                    analytics_recorder, std::move(endpoint_id), share_target),
@@ -489,18 +490,23 @@ void OutgoingShareSession::DelayComplete(
       });
 }
 
-void OutgoingShareSession::UpdateSessionForDedup(
+bool OutgoingShareSession::UpdateSessionForDedup(
     const ShareTarget& share_target,
     std::optional<NearbyShareDecryptedPublicCertificate> certificate,
     absl::string_view endpoint_id) {
-  if (IsConnected()) return;
+  LOG_IF(DFATAL, share_target.id != this->share_target().id)
+      << "Share target id cannot be changed during deduplication.";
   set_share_target(share_target);
+  if (IsConnected()) {
+    return false;
+  }
   set_endpoint_id(endpoint_id);
   if (certificate.has_value()) {
     set_certificate(std::move(certificate.value()));
   } else {
     clear_certificate();
   }
+  return true;
 }
 
 void OutgoingShareSession::Connect(

sharing/outgoing_share_session.h

@@ -24,6 +24,7 @@
 #include <utility>
 #include <vector>
 
+#include "absl/functional/any_invocable.h"
 #include "absl/strings/string_view.h"
 #include "absl/time/time.h"
 #include "internal/platform/clock.h"
@@ -53,7 +54,7 @@ class OutgoingShareSession : public ShareSession {
       NearbyConnectionsManager* connections_manager,
       analytics::AnalyticsRecorder& analytics_recorder, std::string endpoint_id,
       const ShareTarget& share_target,
-      std::function<void(OutgoingShareSession&, const TransferMetadata&)>
+      absl::AnyInvocable<void(OutgoingShareSession&, const TransferMetadata&)>
           transfer_update_callback);
   OutgoingShareSession(OutgoingShareSession&&);
   ~OutgoingShareSession() override;
@@ -137,8 +138,11 @@ class OutgoingShareSession : public ShareSession {
   // A 1 min timer is setup so that if we do not receive disconnect from
   // receiver, we assume the transfer has failed.
   void DelayComplete(const TransferMetadata& complete_metadata);
-  // Used only for OutgoingShareSession De-duplication.
-  void UpdateSessionForDedup(
+  // Updates the share target in the session to `share_target`.
+  // If the session is not connected, also updates the `certificate` and
+  // `endpoint_id`.
+  // Returns true if the session is not connected and updated successfully.
+  bool UpdateSessionForDedup(
       const ShareTarget& share_target,
       std::optional<NearbyShareDecryptedPublicCertificate> certificate,
       absl::string_view endpoint_id);
@@ -177,7 +181,7 @@ class OutgoingShareSession : public ShareSession {
   std::vector<Payload> file_payloads_;
   std::vector<Payload> wifi_credentials_payloads_;
   Status connection_layer_status_ = Status::kUnknown;
-  std::function<void(OutgoingShareSession&, const TransferMetadata&)>
+  absl::AnyInvocable<void(OutgoingShareSession&, const TransferMetadata&)>
       transfer_update_callback_;
   bool ready_for_accept_ = false;
   // This alarm is used to disconnect the sharing connection if both sides do

sharing/outgoing_share_session_test.cc

@@ -78,6 +78,7 @@ using ::testing::IsFalse;
 using ::testing::IsTrue;
 using ::testing::Matcher;
 using ::testing::MockFunction;
+using ::testing::Not;
 using ::testing::Property;
 using ::testing::SizeIs;
 using ::testing::StrictMock;
@@ -876,45 +877,67 @@ TEST_F(OutgoingShareSessionTest, DelayCompleteDisconnectTimeout) {
 
 TEST_F(OutgoingShareSessionTest, UpdateSessionForDedupWithCertificate) {
   EXPECT_FALSE(session_.certificate().has_value());
-  EXPECT_FALSE(session_.self_share());
-  ShareTarget share_target2{
-      "test_update_name",     ::nearby::network::Url(), ShareTargetType::kPhone,
-      /* is_incoming */ true, "test_update_full_name",
-      /* is_known */ true,    "test_update_device_id",  true};
+  ShareTarget share_target2{"test_update_name",      ::nearby::network::Url(),
+                            ShareTargetType::kPhone,
+                            /* is_incoming */ true,  "test_update_full_name",
+                            /* is_known */ true,     "test_update_device_id",
+                            /*for_self_share=*/true};
+  share_target2.id = session_.share_target().id;
+  EXPECT_THAT(session_.share_target(), Not(Eq(share_target2)));
+
   session_.UpdateSessionForDedup(share_target2,
                                  GetNearbyShareTestDecryptedPublicCertificate(),
                                  "test_update_endpoint_id");
-  EXPECT_THAT(session_.share_target().ToString(), Eq(share_target2.ToString()));
+
+  EXPECT_THAT(session_.share_target(), Eq(share_target2));
   EXPECT_TRUE(session_.certificate().has_value());
   EXPECT_THAT(session_.endpoint_id(), Eq("test_update_endpoint_id"));
   EXPECT_TRUE(session_.self_share());
 }
 
 TEST_F(OutgoingShareSessionTest, UpdateSessionForDedupWithoutCertificate) {
   session_.set_certificate(GetNearbyShareTestDecryptedPublicCertificate());
-  ShareTarget share_target2{
-      "test_update_name",     ::nearby::network::Url(), ShareTargetType::kPhone,
-      /* is_incoming */ true, "test_update_full_name",
-      /* is_known */ true,    "test_update_device_id",  true};
+  EXPECT_TRUE(session_.certificate().has_value());
+  ShareTarget share_target2{"test_update_name",      ::nearby::network::Url(),
+                            ShareTargetType::kPhone,
+                            /* is_incoming */ true,  "test_update_full_name",
+                            /* is_known */ true,     "test_update_device_id",
+                            /*for_self_share=*/true};
+  share_target2.id = session_.share_target().id;
+  EXPECT_THAT(session_.share_target(), Not(Eq(share_target2)));
+
   session_.UpdateSessionForDedup(share_target2, std::nullopt,
                                  "test_update_endpoint_id");
+
+  EXPECT_THAT(session_.share_target(), Eq(share_target2));
   // Certificate is cleared.
   EXPECT_FALSE(session_.certificate().has_value());
+  EXPECT_THAT(session_.endpoint_id(), Eq("test_update_endpoint_id"));
+  EXPECT_TRUE(session_.self_share());
 }
 
-TEST_F(OutgoingShareSessionTest, UpdateSessionForDedupConnectedIsNoOp) {
-  auto share_target_org = session_.share_target();
+TEST_F(OutgoingShareSessionTest,
+       UpdateSessionForDedupConnectedDoesNotUpdateCertAndEndpointId) {
+  auto endpoint_id_org = session_.endpoint_id();
   NearbyConnectionImpl connection(device_info_);
   session_.set_session_id(1234);
   ConnectionSuccess(&connection);
-  ShareTarget share_target2{
-      "test_update_name",     ::nearby::network::Url(), ShareTargetType::kPhone,
-      /* is_incoming */ true, "test_update_full_name",
-      /* is_known */ true,    "test_update_device_id",  true};
+  session_.set_certificate(GetNearbyShareTestDecryptedPublicCertificate());
+  EXPECT_TRUE(session_.certificate().has_value());
+  ShareTarget share_target2{"test_update_name",      ::nearby::network::Url(),
+                            ShareTargetType::kPhone,
+                            /* is_incoming */ true,  "test_update_full_name",
+                            /* is_known */ true,     "test_update_device_id",
+                            /*for_self_share=*/true};
+  share_target2.id = session_.share_target().id;
+  EXPECT_THAT(session_.share_target(), Not(Eq(share_target2)));
+
   session_.UpdateSessionForDedup(share_target2, std::nullopt,
                                  "test_update_endpoint_id");
-  EXPECT_THAT(session_.share_target().ToString(),
-              Eq(share_target_org.ToString()));
+
+  EXPECT_THAT(session_.share_target(), Eq(share_target2));
+  EXPECT_TRUE(session_.certificate().has_value());
+  EXPECT_THAT(session_.endpoint_id(), Eq(endpoint_id_org));
 }
 }  // namespace
 }  // namespace nearby::sharing

sharing/share_target.cc

@@ -87,5 +87,15 @@ std::string ShareTarget::ToString() const {
   return absl::StrCat("ShareTarget<", absl::StrJoin(fmt, ", "), ">");
 }
 
+bool ShareTarget::operator==(const ShareTarget& other) const {
+  return id == other.id && device_name == other.device_name &&
+         image_url == other.image_url && type == other.type &&
+         is_incoming == other.is_incoming && full_name == other.full_name &&
+         is_known == other.is_known && device_id == other.device_id &&
+         for_self_share == other.for_self_share &&
+         vendor_id == other.vendor_id &&
+         receive_disabled == other.receive_disabled;
+}
+
 }  // namespace sharing
 }  // namespace nearby

sharing/share_target.h

@@ -42,6 +42,8 @@ struct ShareTarget {
 
   std::string ToString() const;
 
+  bool operator==(const ShareTarget& other) const;
+
   int64_t id;
   std::string device_name;
   // Uri that points to an image of the ShareTarget, if one exists.

sharing/share_target_test.cc

@@ -64,10 +64,21 @@ using ShareTargetToStringTest =
     testing::TestWithParam<ShareTargetToStringTestData>;
 
 TEST_P(ShareTargetToStringTest, ToStringResultMatches) {
-  ShareTarget test_share_target = GetParam().share_target;
+  const ShareTarget& test_share_target = GetParam().share_target;
   EXPECT_EQ(GetParam().expected_string_result, test_share_target.ToString());
 }
 
+TEST_P(ShareTargetToStringTest, EqualsResultMatches) {
+  const ShareTarget& test_share_target = GetParam().share_target;
+  EXPECT_EQ(test_share_target, test_share_target);
+}
+
+TEST_P(ShareTargetToStringTest, EqualsCopyConstructionResultMatches) {
+  const ShareTarget& test_share_target = GetParam().share_target;
+  ShareTarget copy_share_target = test_share_target;
+  EXPECT_EQ(test_share_target, copy_share_target);
+}
+
 INSTANTIATE_TEST_SUITE_P(ShareTargetToStringTest, ShareTargetToStringTest,
                          testing::ValuesIn(GetTestData()));