The file was overwritten when transferring a file that already existed in the downloads folder. With this change we now add " (x)", where x is an incrementing number, starting at 1, using the next non-existing number, to the file name, just before the first dot, or at the end if no dot. This has been applied to the GetDownloadPath method in the platform api.

PiperOrigin-RevId: 457530287

internal/platform/implementation/windows/platform_test.cc

@@ -19,18 +19,33 @@
 #include <windows.h>
 
 #include <xstring>
+#include <fstream>
 
 #include "gtest/gtest.h"
 
+namespace {
+constexpr absl::string_view kFileName("/increment_file_test.txt");
+constexpr absl::string_view kFirstIterationFileName(
+    "/increment_file_test (1).txt");
+constexpr absl::string_view kSecondIterationFileName(
+    "/increment_file_test (2).txt");
+constexpr absl::string_view kThirdIterationFileName(
+    "/increment_file_test (3).txt");
+constexpr absl::string_view kNoDotsFileName("/incrementfiletesttxt");
+constexpr absl::string_view kOneIterationNoDotsFileName(
+    "/incrementfiletesttxt (1)");
+constexpr absl::string_view kMultipleDotsFileName("/increment.file.test.txt");
+constexpr absl::string_view kOneIterationMultipleDotsFileName(
+    "/increment (1).file.test.txt");
+}  // namespace
+
 // Can't run on google 3, I presume the SHGetKnownFolderPath
 // fails.
 #if 0
-class ImplementationPlatformTests : public testing::Test
-{
+class ImplementationPlatformTests : public testing::Test {
  protected:
   // You can define per-test set-up logic as usual.
-  void SetUp() override
-  {
+  void SetUp() override {
     PWSTR basePath;
 
     SHGetKnownFolderPath(
@@ -52,18 +67,19 @@ class ImplementationPlatformTests : public testing::Test
 
     size_t bufferSize;
     wcstombs_s(&bufferSize, NULL, 0, basePath, 0);
-    std::string fullpathUTF8(bufferSize, '\0');
-    wcstombs_s(&bufferSize, fullpathUTF8.data(), bufferSize, basePath,
+    default_download_path_.resize(bufferSize - 1, '\0');
+    wcstombs_s(&bufferSize, default_download_path_.data(), bufferSize, basePath,
                _TRUNCATE);
-    default_download_path_ = fullpathUTF8;
+
+    std::replace(default_download_path_.begin(), default_download_path_.end(),
+                 '\\', '/');
   }
 
   std::string default_download_path_;
 };
 
 TEST_F(ImplementationPlatformTests,
-       GetDownloadPathWithEmptyStringArgumentsShouldReturnBaseDownloadPath)
-{
+       GetDownloadPathWithEmptyStringArgumentsShouldReturnBaseDownloadPath) {
   // Arrange
   std::string parent_folder("");
   std::string file_name("");
@@ -78,8 +94,7 @@ TEST_F(ImplementationPlatformTests,
 
 TEST_F(ImplementationPlatformTests,
        GetDownloadPathWithSlashParentFolderArgumentsShouldReturn\
-BaseDownloadPath)
-{
+BaseDownloadPath) {
   // Arrange
   std::string parent_folder("/");
   std::string file_name("");
@@ -94,8 +109,7 @@ BaseDownloadPath)
 
 TEST_F(ImplementationPlatformTests,
        GetDownloadPathWithBackslashParentFolderArgumentsShouldReturn\
-BaseDownloadPath)
-{
+BaseDownloadPath) {
   // Arrange
   std::string parent_folder("\\");
   std::string file_name("");
@@ -109,8 +123,7 @@ BaseDownloadPath)
 }
 
 TEST_F(ImplementationPlatformTests,
-       GetDownloadPathWithSlashFileNameArgumentsShouldReturnBaseDownloadPath)
-{
+       GetDownloadPathWithSlashFileNameArgumentsShouldReturnBaseDownloadPath) {
   // Arrange
   std::string parent_folder("");
   std::string file_name("/");
@@ -125,8 +138,7 @@ TEST_F(ImplementationPlatformTests,
 
 TEST_F(ImplementationPlatformTests,
        GetDownloadPathWithBackslashFileNameArgumentsShouldReturn\
-BaseDownloadPath)
-{
+BaseDownloadPath) {
   // Arrange
   std::string parent_folder("");
   std::string file_name("\\");
@@ -144,14 +156,13 @@ BaseDownloadPath)
 
 TEST_F(ImplementationPlatformTests,
        GetDownloadPathWithParentFolderShouldReturnParentFolder\
-AppendedToBaseDownloadPath)
-{
+AppendedToBaseDownloadPath) {
   // Arrange
   std::string parent_folder("test_parent_folder");
   std::string file_name("");
 
   std::stringstream path("");
-  path << default_download_path_.c_str() << "\\"
+  path << default_download_path_ << "/"
        << "test_parent_folder";
 
   std::string expected = path.str();
@@ -166,14 +177,13 @@ AppendedToBaseDownloadPath)
 
 TEST_F(ImplementationPlatformTests,
        GetDownloadPathWithParentFolderStartingWithSlashArgumentsShouldReturn\
-ParentFolderAppendedToBaseDownloadPath)
-{
+ParentFolderAppendedToBaseDownloadPath) {
   // Arrange
   std::string parent_folder("/test_parent_folder");
   std::string file_name("");
 
   std::stringstream path("");
-  path << default_download_path_.c_str() << "\\"
+  path << default_download_path_ << "/"
        << "test_parent_folder";
 
   std::string expected = path.str();
@@ -188,14 +198,13 @@ ParentFolderAppendedToBaseDownloadPath)
 
 TEST_F(ImplementationPlatformTests,
        GetDownloadPathWithParentFolderStartingWithBackslashArguments\
-ShouldReturnParentFolderAppendedToBaseDownloadPath)
-{
+ShouldReturnParentFolderAppendedToBaseDownloadPath) {
   // Arrange
   std::string parent_folder("\\test_parent_folder");
   std::string file_name("");
 
   std::stringstream path("");
-  path << default_download_path_.c_str() << "\\"
+  path << default_download_path_ << "/"
        << "test_parent_folder";
 
   std::string expected = path.str();
@@ -210,14 +219,13 @@ ShouldReturnParentFolderAppendedToBaseDownloadPath)
 
 TEST_F(ImplementationPlatformTests,
        GetDownloadPathWithParentFolderEndingWithSlashArgumentsShouldReturn\
-ParentFolderAppendedToBaseDownloadPath)
-{
+ParentFolderAppendedToBaseDownloadPath) {
   // Arrange
   std::string parent_folder("test_parent_folder/");
   std::string file_name("");
 
   std::stringstream path("");
-  path << default_download_path_.c_str() << "\\"
+  path << default_download_path_ << "/"
        << "test_parent_folder";
 
   std::string expected = path.str();
@@ -232,14 +240,13 @@ ParentFolderAppendedToBaseDownloadPath)
 
 TEST_F(ImplementationPlatformTests,
        GetDownloadPathWithParentFolderEndingWithBackslashArguments\
-ShouldReturnParentFolderAppendedToBaseDownloadPath)
-{
+ShouldReturnParentFolderAppendedToBaseDownloadPath) {
   // Arrange
   std::string parent_folder("test_parent_folder\\");
   std::string file_name("");
 
   std::stringstream path("");
-  path << default_download_path_.c_str() << "\\"
+  path << default_download_path_ << "/"
        << "test_parent_folder";
 
   std::string expected = path.str();
@@ -254,14 +261,13 @@ ShouldReturnParentFolderAppendedToBaseDownloadPath)
 
 TEST_F(ImplementationPlatformTests,
        GetDownloadPathWithFileNameBeginningWithSlashArgumentsShouldReturn\
-FileNameAppendedToBaseDownloadPath)
-{
+FileNameAppendedToBaseDownloadPath) {
   // Arrange
   std::string parent_folder("");
   std::string file_name("/test_file_name.name");
 
   std::stringstream path("");
-  path << default_download_path_.c_str() << "\\"
+  path << default_download_path_ << "/"
        << "test_file_name.name";
 
   std::string expected = path.str();
@@ -276,36 +282,32 @@ FileNameAppendedToBaseDownloadPath)
 
 TEST_F(ImplementationPlatformTests,
        GetDownloadPathWithFileNameBeginningWithBackslashArgumentsShouldReturn\
-FileNameAppendedToBaseDownloadPath)
-{
+FileNameAppendedToBaseDownloadPath) {
   // Arrange
   std::string parent_folder("");
   std::string file_name("\\test_file_name.name");
 
   std::stringstream path("");
-  path << default_download_path_.c_str() << "\\"
+  path << default_download_path_ << "/"
        << "test_file_name.name";
 
-  std::string expected = path.str();
-
   // Act
   auto result = location::nearby::api::ImplementationPlatform::GetDownloadPath(
       parent_folder, file_name);
 
   // Assert
-  EXPECT_EQ(result, expected);
+  EXPECT_EQ(result, path.str().c_str());
 }
 
 TEST_F(ImplementationPlatformTests,
        GetDownloadPathWithFileNameEndingWithSlashArgumentsShouldReturnFileName\
-AppendedToBaseDownloadPath)
-{
+AppendedToBaseDownloadPath) {
   // Arrange
   std::string parent_folder("");
   std::string file_name("test_file_name.name/");
 
   std::stringstream path("");
-  path << default_download_path_.c_str() << "\\"
+  path << default_download_path_ << "/"
        << "test_file_name.name";
 
   std::string expected = path.str();
@@ -320,14 +322,13 @@ AppendedToBaseDownloadPath)
 
 TEST_F(ImplementationPlatformTests,
        GetDownloadPathWithFileNameEndingWithBackslashArgumentsShouldReturn\
-FileNameAppendedToBaseDownloadPath)
-{
+FileNameAppendedToBaseDownloadPath) {
   // Arrange
   std::string parent_folder("");
   std::string file_name("test_file_name.name\\");
 
   std::stringstream path("");
-  path << default_download_path_.c_str() << "\\"
+  path << default_download_path_ << "/"
        << "test_file_name.name";
 
   std::string expected = path.str();
@@ -342,16 +343,15 @@ FileNameAppendedToBaseDownloadPath)
 
 TEST_F(ImplementationPlatformTests,
        GetDownloadPathWithParentFolderAndFileNameArgumentsShouldReturn\
-ParentFolderAndFileNameAppendedToBaseDownloadPath)
-{
+ParentFolderAndFileNameAppendedToBaseDownloadPath) {
   // Arrange
   std::string parent_folder("test_parent_folder");
   std::string file_name("test_file_name.name");
 
   std::stringstream path("");
-  path << default_download_path_.c_str() << "\\"
+  path << default_download_path_ << "/"
        << "test_parent_folder"
-       << "\\"
+       << "/"
        << "test_file_name.name";
 
   std::string expected = path.str();
@@ -366,16 +366,15 @@ ParentFolderAndFileNameAppendedToBaseDownloadPath)
 
 TEST_F(ImplementationPlatformTests,
        GetDownloadPathWithParentFolderEndingWithBackslashAndFileNameArguments\
-ShouldReturnParentFolderAndFileNameAppendedToBaseDownloadPath)
-{
+ShouldReturnParentFolderAndFileNameAppendedToBaseDownloadPath) {
   // Arrange
   std::string parent_folder("test_parent_folder\\");
   std::string file_name("test_file_name.name");
 
   std::stringstream path("");
-  path << default_download_path_.c_str() << "\\"
+  path << default_download_path_ << "/"
        << "test_parent_folder"
-       << "\\"
+       << "/"
        << "test_file_name.name";
 
   std::string expected = path.str();
@@ -388,19 +387,17 @@ ShouldReturnParentFolderAndFileNameAppendedToBaseDownloadPath)
   EXPECT_EQ(result, expected);
 }
 
-TEST_F(
-    ImplementationPlatformTests,
-    GetDownloadPathWithFileNameStartingWithBackslashAndParentFolderArguments\
-ShouldReturnParentFolderAndFileNameAppendedToBaseDownloadPath)
-{
+TEST_F(ImplementationPlatformTests,
+       GetDownloadPathWithFileNameStartingWithBackslashAndParentFolderArguments\
+ShouldReturnParentFolderAndFileNameAppendedToBaseDownloadPath) {
   // Arrange
   std::string parent_folder("test_parent_folder");
   std::string file_name("\\test_file_name.name");
 
   std::stringstream path("");
-  path << default_download_path_.c_str() << "\\"
+  path << default_download_path_ << "/"
        << "test_parent_folder"
-       << "\\"
+       << "/"
        << "test_file_name.name";
 
   std::string expected = path.str();
@@ -412,4 +409,279 @@ ShouldReturnParentFolderAndFileNameAppendedToBaseDownloadPath)
   // Assert
   EXPECT_EQ(result, expected);
 }
+
+TEST_F(ImplementationPlatformTests,
+       GetDownloadPath_FileDoesntExistReturnsFileWithPassedName) {
+  // Arrange
+  std::string file_name(kFileName);
+  std::string parent_folder("");
+
+  std::string expected(default_download_path_);
+  expected.append(file_name.c_str());
+
+  // Act
+  std::string actual =
+      location::nearby::api::ImplementationPlatform::GetDownloadPath(
+          parent_folder, file_name);
+
+  // Assert
+  EXPECT_EQ(actual, expected);
+}
+
+TEST_F(ImplementationPlatformTests,
+       GetDownloadPath_FileExistsReturnsFileWithIncrementedName) {
+  // Arrange
+  std::string file_name(kFileName);
+  std::string renamed_file_name(kFirstIterationFileName);
+  std::string parent_folder("");
+
+  std::string output_file_path(default_download_path_);
+  output_file_path.append(file_name);
+
+  std::string expected(default_download_path_);
+  expected.append(renamed_file_name.c_str());
+
+  std::ifstream input_file;
+  std::ofstream output_file;
+
+  output_file.open(output_file_path,
+                   std::ofstream::binary | std::ofstream::out);
+
+  ASSERT_TRUE(output_file.rdstate() == std::ofstream::goodbit);
+
+  output_file.close();
+
+  // Act
+  std::string actual =
+      location::nearby::api::ImplementationPlatform::GetDownloadPath(
+          parent_folder, file_name);
+
+  // Assert
+  EXPECT_EQ(actual, expected);
+
+  // Remove the file and check that it is removed
+  // File 1
+  std::remove(output_file_path.c_str());
+
+  input_file.open(output_file_path, std::ifstream::binary | std::ifstream::in);
+
+  ASSERT_FALSE(input_file.rdstate() == std::ifstream::goodbit);
+}
+
+TEST_F(ImplementationPlatformTests,
+       GetDownloadPath_MultipleFilesExistReturnsNextIncrementedFileName) {
+  // Arrange
+  std::ofstream output_file;
+  std::ifstream input_file;
+
+  std::string file_name(kFileName);
+  std::string first_renamed_file_name(kFirstIterationFileName);
+  std::string second_renamed_file_name(kSecondIterationFileName);
+
+  std::string parent_folder("");
+
+  std::string expected(default_download_path_);
+  expected.append(second_renamed_file_name.c_str());
+
+  std::string output_file1_path(default_download_path_);
+  output_file1_path.append(file_name);
+
+  std::string output_file2_path(default_download_path_);
+  output_file2_path.append(first_renamed_file_name);
+
+  // Create the test files
+  output_file.open(output_file1_path,
+                   std::ofstream::binary | std::ofstream::out);
+  ASSERT_TRUE(output_file.rdstate() == std::ofstream::goodbit);
+  output_file.close();
+  output_file.clear();
+
+  output_file.open(output_file2_path,
+                   std::ofstream::binary | std::ofstream::out);
+  ASSERT_TRUE(output_file.rdstate() == std::ofstream::goodbit);
+  output_file.close();
+
+  // Act
+  std::string actual =
+      location::nearby::api::ImplementationPlatform::GetDownloadPath(
+          parent_folder, file_name);
+
+  // Assert
+  EXPECT_EQ(expected, actual);
+
+  // Remove the test files and check that it is removed
+  // File 1
+  std::remove(output_file1_path.c_str());
+  input_file.open(output_file1_path, std::ifstream::binary | std::ifstream::in);
+
+  ASSERT_FALSE(input_file.rdstate() == std::ifstream::goodbit);
+
+  // File 2
+  std::remove(output_file2_path.c_str());
+
+  input_file.clear();
+  input_file.open(output_file2_path, std::ifstream::binary | std::ifstream::in);
+
+  ASSERT_FALSE(input_file.rdstate() == std::ifstream::goodbit);
+}
+
+TEST_F(
+    ImplementationPlatformTests,
+    GetDownloadPath_FileNameContainsMultipleDotsReturnsIncrementBeforeFirstDot){
+  // Arrange
+  std::ifstream input_file;
+  std::ofstream output_file;
+
+  std::string file_name(kMultipleDotsFileName);
+  std::string renamed_file_name(kOneIterationMultipleDotsFileName);
+
+  std::string parent_folder("");
+
+  std::string output_file1_path(default_download_path_);
+  output_file1_path.append(file_name);
+
+  std::string output_file2_path(default_download_path_);
+  output_file2_path.append(renamed_file_name);
+
+  std::string expected(default_download_path_);
+  expected.append(renamed_file_name);
+
+  output_file.open(output_file1_path,
+                   std::ofstream::binary | std::ofstream::out);
+  ASSERT_TRUE(output_file.rdstate() == std::ofstream::goodbit);
+  output_file.close();
+
+  // Act
+  std::string actual =
+      location::nearby::api::ImplementationPlatform::GetDownloadPath(
+          parent_folder, file_name);
+
+  // Assert
+  EXPECT_EQ(expected, actual);
+
+  std::remove(output_file1_path.c_str());
+  input_file.open(output_file1_path, std::ifstream::binary | std::ifstream::in);
+
+  ASSERT_FALSE(input_file.rdstate() == std::ifstream::goodbit);
+}
+
+TEST_F(ImplementationPlatformTests,
+       GetDownloadPath_FileNameContainsNoDotsReturnsWithIncrementAtEnd) {
+  std::ifstream input_file;
+  std::ofstream output_file;
+
+  std::string file_name(kNoDotsFileName);
+  std::string renamed_file_name(kOneIterationNoDotsFileName);
+
+  std::string parent_folder("");
+
+  std::string output_file1_path(default_download_path_);
+  output_file1_path.append(file_name);
+
+  std::string output_file2_path(default_download_path_);
+  output_file2_path.append(renamed_file_name);
+
+  std::string expected(default_download_path_);
+  expected.append(renamed_file_name);
+
+  output_file.open(output_file1_path,
+                   std::ofstream::binary | std::ofstream::out);
+  ASSERT_TRUE(output_file.rdstate() == std::ofstream::goodbit);
+  output_file.close();
+
+  // Act
+  std::string actual =
+      location::nearby::api::ImplementationPlatform::GetDownloadPath(
+          parent_folder, file_name);
+
+  // Assert
+  EXPECT_EQ(expected, actual);
+
+  std::remove(output_file1_path.c_str());
+  input_file.open(output_file1_path, std::ifstream::binary | std::ifstream::in);
+
+  ASSERT_FALSE(input_file.rdstate() == std::ifstream::goodbit);
+}
+
+TEST_F(ImplementationPlatformTests,
+       GetDownloadPath_FileNameExistsWithAHoleBetweenRenamedFiles) {
+  std::ifstream input_file;
+  std::ofstream output_file;
+
+  std::string file_name(kFileName);
+  std::string file_name1(kFirstIterationFileName);
+  std::string file_name2(kSecondIterationFileName);
+  std::string file_name3(kThirdIterationFileName);
+
+  std::string parent_folder("");
+
+  // Create the path for the original file name
+  std::string output_file_path(default_download_path_);
+  output_file_path.append(
+      file_name);  // Original file name example: "increment_file_test.txt"
+
+  // Create the path for the first iteration of the original file name
+  std::string output_file1_path(default_download_path_);
+  output_file1_path.append(file_name1);  // First iteration on original file
+                                         // name example:
+                                         // "increment_file_test (1).txt"
+
+  // Create the path for the third iteration of the original file name
+  std::string output_file3_path(default_download_path_);
+  output_file3_path.append(
+      file_name3);  // Third iteration on original file
+                    // name example: "increment_file_test (3).txt"
+
+  // Create the expected result which is the second iteration of the original
+  // file name
+  std::string expected(default_download_path_);
+  expected.append(file_name2);  // Second iteration on original file name
+                                // example: "increment_file_test (2).txt"
+
+  // Create the original file
+  output_file.open(output_file_path,
+                   std::ofstream::binary | std::ofstream::out);
+  ASSERT_TRUE(output_file.rdstate() == std::ofstream::goodbit);
+  output_file.close();
+
+  // Create the first iteration of the original file
+  output_file.clear();
+  output_file.open(output_file1_path,
+                   std::ofstream::binary | std::ofstream::out);
+  ASSERT_TRUE(output_file.rdstate() == std::ofstream::goodbit);
+  output_file.close();
+
+  // Create the third iteration of the original file
+  output_file.clear();
+  output_file.open(output_file3_path,
+                   std::ofstream::binary | std::ofstream::out);
+  ASSERT_TRUE(output_file.rdstate() == std::ofstream::goodbit);
+  output_file.close();
+
+  // Act
+  // This should return the second iteration of the original file
+  std::string actual =
+      location::nearby::api::ImplementationPlatform::GetDownloadPath(
+          parent_folder, file_name);
+
+  // Assert
+  EXPECT_EQ(expected, actual);
+
+  // Delete the original file
+  std::remove(output_file_path.c_str());
+  input_file.open(output_file_path, std::ifstream::binary | std::ifstream::in);
+  ASSERT_FALSE(input_file.rdstate() == std::ifstream::goodbit);
+
+  // Delete the first iteration of the original file
+  input_file.clear();  // Reset the input_file state
+  std::remove(output_file1_path.c_str());
+  input_file.open(output_file1_path, std::ifstream::binary | std::ifstream::in);
+  ASSERT_FALSE(input_file.rdstate() == std::ifstream::goodbit);
+
+  // Delete the third iteration of the original file
+  input_file.clear();  // Reset the input_file state
+  std::remove(output_file3_path.c_str());
+  input_file.open(output_file3_path, std::ifstream::binary | std::ifstream::in);
+  ASSERT_FALSE(input_file.rdstate() == std::ifstream::goodbit);
+}
 #endif