Filter expected files out from the delete list.

PiperOrigin-RevId: 637746917

sharing/fake_nearby_connections_manager.cc

@@ -305,20 +305,6 @@ void FakeNearbyConnectionsManager::SetCustomSavePath(
   custom_save_path_ = custom_save_path;
 }
 
-absl::flat_hash_set<std::filesystem::path>
-FakeNearbyConnectionsManager::GetUnknownFilePathsToDelete() {
-  return file_paths_to_delete_;
-}
-
-void FakeNearbyConnectionsManager::ClearUnknownFilePathsToDelete() {
-  file_paths_to_delete_.clear();
-}
-
-void FakeNearbyConnectionsManager::AddUnknownFilePathsToDeleteForTesting(
-    std::filesystem::path file_path) {
-  file_paths_to_delete_.insert(file_path);
-}
-
 absl::flat_hash_set<std::filesystem::path>
 FakeNearbyConnectionsManager::GetAndClearUnknownFilePathsToDelete() {
   absl::flat_hash_set<std::filesystem::path> file_paths_to_delete =
@@ -327,6 +313,15 @@ FakeNearbyConnectionsManager::GetAndClearUnknownFilePathsToDelete() {
   return file_paths_to_delete;
 }
 
+absl::flat_hash_set<std::filesystem::path>
+FakeNearbyConnectionsManager::GetUnknownFilePathsToDeleteForTesting() {
+  return file_paths_to_delete_;
+}
+void FakeNearbyConnectionsManager::AddUnknownFilePathsToDeleteForTesting(
+    std::filesystem::path file_path) {
+  file_paths_to_delete_.insert(file_path);
+}
+
 std::string FakeNearbyConnectionsManager::Dump() const { return ""; }
 
 }  // namespace sharing

sharing/fake_nearby_connections_manager.h

@@ -72,11 +72,8 @@ class FakeNearbyConnectionsManager : public NearbyConnectionsManager {
       absl::string_view endpoint_id) override;
   void UpgradeBandwidth(absl::string_view endpoint_id) override;
   void SetCustomSavePath(absl::string_view custom_save_path) override;
-  absl::flat_hash_set<std::filesystem::path> GetUnknownFilePathsToDelete()
-      override;
   absl::flat_hash_set<std::filesystem::path>
   GetAndClearUnknownFilePathsToDelete() override;
-  void ClearUnknownFilePathsToDelete() override;
 
   // Testing methods
   void SetRawAuthenticationToken(absl::string_view endpoint_id,
@@ -134,6 +131,8 @@ class FakeNearbyConnectionsManager : public NearbyConnectionsManager {
     return !incoming_payloads_.empty();
   }
 
+  absl::flat_hash_set<std::filesystem::path>
+  GetUnknownFilePathsToDeleteForTesting();
   void AddUnknownFilePathsToDeleteForTesting(std::filesystem::path file_path);
 
  private:

sharing/nearby_connections_manager.h

@@ -158,13 +158,6 @@ class NearbyConnectionsManager {
   // Sets a custom save path.
   virtual void SetCustomSavePath(absl::string_view custom_save_path) = 0;
 
-  // Gets the file paths to delete.
-  virtual absl::flat_hash_set<std::filesystem::path>
-  GetUnknownFilePathsToDelete() = 0;
-
-  // Deletes the file paths to delete.
-  virtual void ClearUnknownFilePathsToDelete() = 0;
-
   // Gets the file paths to delete and clear the hash set.
   virtual absl::flat_hash_set<std::filesystem::path>
   GetAndClearUnknownFilePathsToDelete() = 0;

sharing/nearby_connections_manager_impl.cc

@@ -134,12 +134,24 @@ std::string MediumSelectionToString(const MediumSelection& mediums) {
   return ss.str();
 }
 
+std::string PayloadStatusToString(PayloadStatus status) {
+  switch (status) {
+    case PayloadStatus::kSuccess:
+      return "Success";
+    case PayloadStatus::kFailure:
+      return "Failure";
+    case PayloadStatus::kInProgress:
+      return "In Progress";
+    case PayloadStatus::kCanceled:
+      return "Canceled";
+  }
+}
+
 }  // namespace
 
 NearbyConnectionsManagerImpl::NearbyConnectionsManagerImpl(
-    TaskRunner* connections_callback_task_runner,
-    Context* context, ConnectivityManager& connectivity_manager,
-    nearby::DeviceInfo& device_info,
+    TaskRunner* connections_callback_task_runner, Context* context,
+    ConnectivityManager& connectivity_manager, nearby::DeviceInfo& device_info,
     std::unique_ptr<NearbyConnectionsService> nearby_connections_service)
     : connections_callback_task_runner_(connections_callback_task_runner),
       context_(context),
@@ -797,13 +809,31 @@ void NearbyConnectionsManagerImpl::OnPayloadReceived(
   NL_DCHECK(result.second);
 }
 
+void NearbyConnectionsManagerImpl::ProcessUnknownFilePathsToDelete(
+    PayloadStatus status, PayloadContent::Type type,
+    const std::filesystem::path& path) {
+  if (NearbyFlags::GetInstance().GetBoolFlag(
+          sharing::config_package_nearby::nearby_sharing_feature::
+              kDeleteUnexpectedReceivedFile)) {
+    // Unknown payload comes with kCanceled and kFile type from
+    // NearbyConnections. Delete it.
+    if (status == PayloadStatus::kCanceled &&
+        type == PayloadContent::Type::kFile) {
+      NL_LOG(WARNING) << __func__
+                      << ": Unknown payload has been canceled, removing.";
+      MutexLock lock(&mutex_);
+      file_paths_to_delete_.insert(path);
+    }
+  }
+}
+
 void NearbyConnectionsManagerImpl::OnPayloadTransferUpdate(
     absl::string_view endpoint_id, const PayloadTransferUpdate& update) {
   MutexLock lock(&mutex_);
   NL_LOG(INFO) << "Received payload transfer update id=" << update.payload_id
-               << ",status=" << update.status << ",total=" << update.total_bytes
-               << ",bytes_transferred=" << update.bytes_transferred
-               << std::endl;
+               << ",status=" << PayloadStatusToString(update.status)
+               << ",total=" << update.total_bytes
+               << ",bytes_transferred=" << update.bytes_transferred;
 
   // If this is a payload we've registered for, then forward its status to
   // the PayloadStatusListener if it still exists. We don't need to do
@@ -838,19 +868,11 @@ void NearbyConnectionsManagerImpl::OnPayloadTransferUpdate(
 
   if (payload_it->second.content.type != PayloadContent::Type::kBytes) {
     NL_LOG(WARNING) << "Received unknown payload of file type. Cancelling.";
-    if (NearbyFlags::GetInstance().GetBoolFlag(
-            sharing::config_package_nearby::nearby_sharing_feature::
-                kDeleteUnexpectedReceivedFile)) {
-      // if we get kFile and have file_path, delete the file path.
-      if (payload_it->second.content.type == PayloadContent::Type::kFile) {
-        auto file_path = payload_it->second.content.file_payload.file.path;
-        NL_LOG(WARNING) << __func__
-                        << ": Payload is Type::kFile type. Removing.";
-        file_paths_to_delete_.insert(file_path);
-      }
-    }
     nearby_connections_service_->CancelPayload(kServiceId, payload_it->first,
                                                [](Status status) {});
+    ProcessUnknownFilePathsToDelete(
+        update.status, payload_it->second.content.type,
+        payload_it->second.content.file_payload.file.path);
     return;
   }
 
@@ -922,25 +944,36 @@ NearbyConnectionsManagerImpl::GetUnknownFilePathsToDelete() {
   return file_paths_to_delete_;
 }
 
-void NearbyConnectionsManagerImpl::ClearUnknownFilePathsToDelete() {
-  MutexLock lock(&mutex_);
-  file_paths_to_delete_.clear();
-}
-
 absl::flat_hash_set<std::filesystem::path>
 NearbyConnectionsManagerImpl::GetAndClearUnknownFilePathsToDelete() {
   MutexLock lock(&mutex_);
-  auto file_paths_to_delete = file_paths_to_delete_;
+  auto file_paths_to_delete = std::move(file_paths_to_delete_);
   file_paths_to_delete_.clear();
   return file_paths_to_delete;
 }
 
+absl::flat_hash_set<std::filesystem::path>
+NearbyConnectionsManagerImpl::GetUnknownFilePathsToDeleteForTesting() {
+  return GetUnknownFilePathsToDelete();
+}
+
 void NearbyConnectionsManagerImpl::AddUnknownFilePathsToDeleteForTesting(
     std::filesystem::path file_path) {
   MutexLock lock(&mutex_);
   file_paths_to_delete_.insert(file_path);
 }
 
+void NearbyConnectionsManagerImpl::ProcessUnknownFilePathsToDeleteForTesting(
+    PayloadStatus status, PayloadContent::Type type,
+    const std::filesystem::path& path) {
+  ProcessUnknownFilePathsToDelete(status, type, path);
+}
+
+void NearbyConnectionsManagerImpl::OnPayloadTransferUpdateForTesting(
+    absl::string_view endpoint_id, const PayloadTransferUpdate& update) {
+  OnPayloadTransferUpdate(endpoint_id, update);
+}
+
 std::string NearbyConnectionsManagerImpl::Dump() const {
   return nearby_connections_service_->Dump();
 }

sharing/nearby_connections_manager_impl.h

@@ -84,19 +84,22 @@ class NearbyConnectionsManagerImpl : public NearbyConnectionsManager {
       absl::string_view endpoint_id) override;
   void UpgradeBandwidth(absl::string_view endpoint_id) override;
   void SetCustomSavePath(absl::string_view custom_save_path) override;
-  absl::flat_hash_set<std::filesystem::path> GetUnknownFilePathsToDelete()
-      override;
   absl::flat_hash_set<std::filesystem::path>
   GetAndClearUnknownFilePathsToDelete() override;
-  void ClearUnknownFilePathsToDelete() override;
-
   std::string Dump() const override;
 
   NearbyConnectionsService* GetNearbyConnectionsService() const {
     return nearby_connections_service_.get();
   }
 
+  absl::flat_hash_set<std::filesystem::path>
+  GetUnknownFilePathsToDeleteForTesting();
   void AddUnknownFilePathsToDeleteForTesting(std::filesystem::path file_path);
+  void ProcessUnknownFilePathsToDeleteForTesting(
+      PayloadStatus status, PayloadContent::Type type,
+      const std::filesystem::path& path);
+  void OnPayloadTransferUpdateForTesting(absl::string_view endpoint_id,
+                                         const PayloadTransferUpdate& update);
 
  private:
   // EndpointDiscoveryListener:
@@ -119,6 +122,10 @@ class NearbyConnectionsManagerImpl : public NearbyConnectionsManager {
   void OnConnectionTimedOut(absl::string_view endpoint_id);
   void OnConnectionRequested(absl::string_view endpoint_id,
                              ConnectionsStatus status);
+  void ProcessUnknownFilePathsToDelete(PayloadStatus status,
+                                       PayloadContent::Type type,
+                                       const std::filesystem::path& path);
+  absl::flat_hash_set<std::filesystem::path> GetUnknownFilePathsToDelete();
 
   void Reset();
 

sharing/nearby_connections_manager_impl_test.cc

@@ -1739,21 +1739,23 @@ TEST_F(NearbyConnectionsManagerImplTest, UnknownFilePathsToDelete) {
   nearby_connections_manager_->AddUnknownFilePathsToDeleteForTesting(
       "test2.txt");
   auto unknown_file_paths =
-      nearby_connections_manager_->GetUnknownFilePathsToDelete();
+      nearby_connections_manager_->GetUnknownFilePathsToDeleteForTesting();
   nearby_connections_manager_->AddUnknownFilePathsToDeleteForTesting(
       "test3.txt");
 
   // Test if we get copy of container.
   EXPECT_NE(unknown_file_paths.size(), 3);
   EXPECT_EQ(unknown_file_paths.size(), 2);
-  EXPECT_EQ(nearby_connections_manager_->GetUnknownFilePathsToDelete().size(),
+  EXPECT_EQ(nearby_connections_manager_->GetUnknownFilePathsToDeleteForTesting()
+                .size(),
             3);
   unknown_file_paths =
-      nearby_connections_manager_->GetUnknownFilePathsToDelete();
+      nearby_connections_manager_->GetUnknownFilePathsToDeleteForTesting();
   EXPECT_THAT(unknown_file_paths,
               UnorderedElementsAre("test1.txt", "test2.txt", "test3.txt"));
-  nearby_connections_manager_->ClearUnknownFilePathsToDelete();
-  EXPECT_EQ(nearby_connections_manager_->GetUnknownFilePathsToDelete().size(),
+  nearby_connections_manager_->GetAndClearUnknownFilePathsToDelete();
+  EXPECT_EQ(nearby_connections_manager_->GetUnknownFilePathsToDeleteForTesting()
+                .size(),
             0);
 
   // Test GetAndClearUnknownFilePathsToDelete
@@ -1761,13 +1763,103 @@ TEST_F(NearbyConnectionsManagerImplTest, UnknownFilePathsToDelete) {
       "test1.txt");
   nearby_connections_manager_->AddUnknownFilePathsToDeleteForTesting(
       "test2.txt");
-  unknown_file_paths =
-      nearby_connections_manager_->GetAndClearUnknownFilePathsToDelete();
+  unknown_file_paths = nearby_connections_manager_
+                           ->GetAndClearUnknownFilePathsToDelete();
   EXPECT_EQ(unknown_file_paths.size(), 2);
-  EXPECT_EQ(nearby_connections_manager_->GetUnknownFilePathsToDelete().size(),
+  EXPECT_EQ(nearby_connections_manager_->GetUnknownFilePathsToDeleteForTesting()
+                .size(),
             0);
 }
 
+TEST_F(NearbyConnectionsManagerImplTest,
+       OnPayloadTransferUpdateForUnknownFile) {
+  NearbyConnectionsService::ConnectionListener connection_listener_remote;
+  testing::NiceMock<MockIncomingConnectionListener>
+      incoming_connection_listener;
+  StartAdvertising(connection_listener_remote, incoming_connection_listener);
+
+  NearbyConnectionsService::PayloadListener payload_listener_remote;
+  NearbyConnection* connection = OnIncomingConnection(
+      connection_listener_remote, incoming_connection_listener,
+      payload_listener_remote);
+  EXPECT_TRUE(connection);
+  std::filesystem::path file(std::filesystem::temp_directory_path() /
+                             "file.jpg");
+  payload_listener_remote.payload_cb(kRemoteEndpointId,
+                                     Payload(kPayloadId, InputFile(file)));
+
+  // Flag is off. Don't add unknown file paths to the list.
+  NearbyFlags::GetInstance().OverrideBoolFlagValue(
+      config_package_nearby::nearby_sharing_feature::
+          kDeleteUnexpectedReceivedFile,
+      false);
+  auto unknown_file_paths =
+      nearby_connections_manager_->GetUnknownFilePathsToDeleteForTesting();
+  EXPECT_EQ(unknown_file_paths.size(), 0);
+  nearby_connections_manager_->OnPayloadTransferUpdateForTesting(
+      kRemoteEndpointId,
+      PayloadTransferUpdate(kPayloadId, PayloadStatus::kCanceled, kTotalSize,
+                            /*bytes_transferred=*/kTotalSize));
+
+  // Flag is on. Add unknown file paths with kCanceled to the list.
+  NearbyFlags::GetInstance().OverrideBoolFlagValue(
+      config_package_nearby::nearby_sharing_feature::
+          kDeleteUnexpectedReceivedFile,
+      true);
+  nearby_connections_manager_->OnPayloadTransferUpdateForTesting(
+      kRemoteEndpointId,
+      PayloadTransferUpdate(kPayloadId, PayloadStatus::kCanceled, kTotalSize,
+                            /*bytes_transferred=*/kTotalSize));
+  unknown_file_paths =
+      nearby_connections_manager_->GetUnknownFilePathsToDeleteForTesting();
+  EXPECT_EQ(unknown_file_paths.size(), 1);
+  nearby_connections_manager_->GetAndClearUnknownFilePathsToDelete();
+
+  // Flag is on. Don't add unknown file paths w/o kCanceled to the list.
+  nearby_connections_manager_->OnPayloadTransferUpdateForTesting(
+      kRemoteEndpointId,
+      PayloadTransferUpdate(kPayloadId, PayloadStatus::kFailure, kTotalSize,
+                            /*bytes_transferred=*/kTotalSize));
+  unknown_file_paths =
+      nearby_connections_manager_->GetUnknownFilePathsToDeleteForTesting();
+  EXPECT_EQ(unknown_file_paths.size(), 0);
+}
+
+TEST_F(NearbyConnectionsManagerImplTest, ProcessUnknownFilePathsToDelete) {
+  std::filesystem::path file(std::filesystem::temp_directory_path() /
+                             "file.jpg");
+  // Flag is off. Don't add unknown file paths to the list.
+  NearbyFlags::GetInstance().OverrideBoolFlagValue(
+      config_package_nearby::nearby_sharing_feature::
+          kDeleteUnexpectedReceivedFile,
+      false);
+  auto unknown_file_paths =
+      nearby_connections_manager_->GetUnknownFilePathsToDeleteForTesting();
+  nearby_connections_manager_->ProcessUnknownFilePathsToDeleteForTesting(
+      PayloadStatus::kCanceled, PayloadContent::Type::kFile, file);
+  EXPECT_EQ(unknown_file_paths.size(), 0);
+
+  // Flag is on. Add unknown file paths with kCanceled to the list.
+  NearbyFlags::GetInstance().OverrideBoolFlagValue(
+      config_package_nearby::nearby_sharing_feature::
+          kDeleteUnexpectedReceivedFile,
+      true);
+  nearby_connections_manager_->ProcessUnknownFilePathsToDeleteForTesting(
+      PayloadStatus::kCanceled, PayloadContent::Type::kFile, file);
+  unknown_file_paths =
+      nearby_connections_manager_->GetUnknownFilePathsToDeleteForTesting();
+  EXPECT_EQ(unknown_file_paths.size(), 1);
+  nearby_connections_manager_->GetAndClearUnknownFilePathsToDelete();
+
+  // Flag is on. Don't add unknown file paths w/o kCanceled to the list.
+  nearby_connections_manager_->ProcessUnknownFilePathsToDeleteForTesting(
+      PayloadStatus::kFailure, PayloadContent::Type::kFile, file);
+  unknown_file_paths =
+      nearby_connections_manager_->GetUnknownFilePathsToDeleteForTesting();
+  EXPECT_EQ(unknown_file_paths.size(), 0);
+  nearby_connections_manager_->GetAndClearUnknownFilePathsToDelete();
+}
+
 }  // namespace NearbyConnectionsManagerUnitTests
 }  // namespace sharing
 }  // namespace nearby

sharing/nearby_sharing_service_impl.cc

@@ -3089,6 +3089,11 @@ void NearbySharingServiceImpl::OnIncomingTransferUpdate(
       // For any type of failure, lets make sure any pending files get cleaned
       // up.
       RemoveIncomingPayloads(share_target);
+    } else {
+      if (!nearby_connections_manager_->GetAndClearUnknownFilePathsToDelete()
+               .empty()) {
+        NL_LOG(WARNING) << __func__ << ": Unknown file paths are not empty.";
+      }
     }
   } else if (metadata.status() ==
              TransferMetadata::Status::kAwaitingLocalConfirmation) {

sharing/nearby_sharing_service_impl_test.cc

@@ -4835,7 +4835,7 @@ TEST_F(NearbySharingServiceImplTest, RemoveIncomingPayloads) {
   fake_nearby_connections_manager_->AddUnknownFilePathsToDeleteForTesting(
       "test2.txt");
   auto unknown_file_paths_to_delete =
-      fake_nearby_connections_manager_->GetUnknownFilePathsToDelete();
+      fake_nearby_connections_manager_->GetUnknownFilePathsToDeleteForTesting();
   EXPECT_EQ(unknown_file_paths_to_delete.size(), 2);
   EXPECT_THAT(unknown_file_paths_to_delete,
               UnorderedElementsAre("test1.txt", "test2.txt"));
@@ -4843,7 +4843,8 @@ TEST_F(NearbySharingServiceImplTest, RemoveIncomingPayloads) {
   share_target.is_incoming = true;
   service_->RemoveIncomingPayloads(share_target);
   EXPECT_EQ(
-      fake_nearby_connections_manager_->GetUnknownFilePathsToDelete().size(),
+      fake_nearby_connections_manager_->GetUnknownFilePathsToDeleteForTesting()
+          .size(),
       0);
 
   // Test GetAndClearUnknownFilePathsToDelete
@@ -4855,7 +4856,8 @@ TEST_F(NearbySharingServiceImplTest, RemoveIncomingPayloads) {
       fake_nearby_connections_manager_->GetAndClearUnknownFilePathsToDelete();
   EXPECT_EQ(unknown_file_paths_to_delete.size(), 2);
   EXPECT_EQ(
-      fake_nearby_connections_manager_->GetUnknownFilePathsToDelete().size(),
+      fake_nearby_connections_manager_->GetUnknownFilePathsToDeleteForTesting()
+          .size(),
       0);
 }