Add more test cases for InternalPayloadFactoryTest to improve test coverage.

PiperOrigin-RevId: 868195322

connections/implementation/BUILD

@@ -582,6 +582,7 @@ cc_test(
         "//internal/platform/implementation/g3",  # build_cleaner: keep
         "@com_github_protobuf_matchers//protobuf-matchers",
         "@com_google_absl//absl/strings:string_view",
+        "@com_google_absl//absl/time",
         "@com_google_googletest//:gtest_main",
     ],
 )

connections/implementation/internal_payload_factory.cc

@@ -46,6 +46,17 @@ namespace {
 using ::location::nearby::connections::PayloadTransferFrame;
 using ::location::nearby::proto::connections::OperationResultCode;
 
+// if custom_save_path is empty, default download path is used
+std::string make_path(const std::string& custom_save_path,
+                      const std::string& parent_folder,
+                      const std::string& file_name) {
+  if (!custom_save_path.empty()) {
+    std::string path = absl::StrCat(custom_save_path, "/", parent_folder);
+    return api::ImplementationPlatform::GetCustomSavePath(path, file_name);
+  }
+  return api::ImplementationPlatform::GetDownloadPath(parent_folder, file_name);
+}
+
 class BytesInternalPayload : public InternalPayload {
  public:
   explicit BytesInternalPayload(Payload payload)
@@ -338,27 +349,6 @@ ErrorOr<std::unique_ptr<InternalPayload>> CreateOutgoingInternalPayload(
   }
 }
 
-// if custom_save_path is empty, default download path is used
-std::string make_path(const std::string& custom_save_path,
-                      std::string& parent_folder, std::string& file_name) {
-  if (!custom_save_path.empty()) {
-    std::string path = absl::StrCat(custom_save_path, "/", parent_folder);
-    return api::ImplementationPlatform::GetCustomSavePath(path, file_name);
-  }
-  return api::ImplementationPlatform::GetDownloadPath(parent_folder, file_name);
-}
-
-// if custom_save_path is empty, default download path is used
-std::string make_path(const std::string& custom_save_path,
-                      std::string& parent_folder, int64_t id) {
-  std::string file_name(std::to_string(id));
-  if (!custom_save_path.empty()) {
-    std::string path = absl::StrCat(custom_save_path, "/", parent_folder);
-    return api::ImplementationPlatform::GetCustomSavePath(path, file_name);
-  }
-  return api::ImplementationPlatform::GetDownloadPath(parent_folder, file_name);
-}
-
 ErrorOr<std::unique_ptr<InternalPayload>> CreateIncomingInternalPayload(
     const location::nearby::connections::PayloadTransferFrame& frame,
     const std::string& custom_save_path) {

connections/implementation/internal_payload_factory_test.cc

@@ -14,6 +14,7 @@
 
 #include "connections/implementation/internal_payload_factory.h"
 
+#include <cmath>
 #include <cstddef>
 #include <cstdint>
 #include <memory>
@@ -22,6 +23,8 @@
 
 #include "gtest/gtest.h"
 #include "absl/strings/string_view.h"
+#include "absl/time/clock.h"
+#include "absl/time/time.h"
 #include "connections/implementation/internal_payload.h"
 #include "connections/implementation/proto/offline_wire_formats.pb.h"
 #include "connections/payload.h"
@@ -30,6 +33,7 @@
 #include "internal/platform/exception.h"
 #include "internal/platform/expected.h"
 #include "internal/platform/file.h"
+#include "internal/platform/input_stream.h"
 #include "internal/platform/pipe.h"
 
 namespace nearby {
@@ -82,7 +86,7 @@ TEST(InternalPayloadFactoryTest, CanCreateInternalPayloadFromFilePayload) {
 
 TEST(InternalPayloadFactoryTest, CanCreateInternalPayloadFromByteMessage) {
   PayloadTransferFrame frame;
-  std::string path = "C:\\Downloads";
+  std::string path = ::testing::TempDir();
   frame.set_packet_type(PayloadTransferFrame::DATA);
   std::int64_t payload_chunk_offset = 0;
   ByteArray data(kText);
@@ -108,7 +112,7 @@ TEST(InternalPayloadFactoryTest, CanCreateInternalPayloadFromByteMessage) {
 
 TEST(InternalPayloadFactoryTest, CanCreateInternalPayloadFromStreamMessage) {
   PayloadTransferFrame frame;
-  std::string path = "C:\\Downloads";
+  std::string path = ::testing::TempDir();
   frame.set_packet_type(PayloadTransferFrame::DATA);
   auto& header = *frame.mutable_payload_header();
   header.set_type(PayloadTransferFrame::PayloadHeader::STREAM);
@@ -133,7 +137,7 @@ TEST(InternalPayloadFactoryTest, CanCreateInternalPayloadFromStreamMessage) {
 
 TEST(InternalPayloadFactoryTest, CanCreateInternalPayloadFromFileMessage) {
   PayloadTransferFrame frame;
-  std::string path = "/tmp/Downloads";
+  std::string path = ::testing::TempDir();
   frame.set_packet_type(PayloadTransferFrame::DATA);
   auto& header = *frame.mutable_payload_header();
   header.set_type(PayloadTransferFrame::PayloadHeader::FILE);
@@ -154,7 +158,7 @@ TEST(InternalPayloadFactoryTest, CanCreateInternalPayloadFromFileMessage) {
 TEST(InternalPayloadFactoryTest,
      InternalPayloadFromFileMessageWithoutIdReturnsNullptr) {
   PayloadTransferFrame frame;
-  std::string path = "/tmp/Downloads";
+  std::string path = ::testing::TempDir();
   frame.set_packet_type(PayloadTransferFrame::DATA);
   auto& header = *frame.mutable_payload_header();
   header.set_type(PayloadTransferFrame::PayloadHeader::FILE);
@@ -167,7 +171,7 @@ TEST(InternalPayloadFactoryTest,
 TEST(InternalPayloadFactoryTest,
      CanCreateInternalPayloadFromFileMessageWithFileNameNotSet) {
   PayloadTransferFrame frame;
-  std::string path = "/tmp/Downloads";
+  std::string path = ::testing::TempDir();
   frame.set_packet_type(PayloadTransferFrame::DATA);
   auto& header = *frame.mutable_payload_header();
   header.set_type(PayloadTransferFrame::PayloadHeader::FILE);
@@ -185,7 +189,7 @@ TEST(InternalPayloadFactoryTest,
 TEST(InternalPayloadFactoryTest,
      CanCreateInternalPayloadFromFileMessageWithFileNameSet) {
   PayloadTransferFrame frame;
-  std::string path = "/tmp/Downloads";
+  std::string path = ::testing::TempDir();
   frame.set_packet_type(PayloadTransferFrame::DATA);
   auto& header = *frame.mutable_payload_header();
   header.set_type(PayloadTransferFrame::PayloadHeader::FILE);
@@ -202,6 +206,34 @@ TEST(InternalPayloadFactoryTest,
   EXPECT_EQ(payload.GetFileName(), "test.file.name");
 }
 
+TEST(InternalPayloadFactoryTest,
+     VerifyFilePayloadFileNameParentFolderAndLastModifiedTime) {
+  PayloadTransferFrame frame;
+  std::string path = ::testing::TempDir();
+  frame.set_packet_type(PayloadTransferFrame::DATA);
+  auto& header = *frame.mutable_payload_header();
+  header.set_type(PayloadTransferFrame::PayloadHeader::FILE);
+  header.set_id(12345);
+  header.set_total_size(512);
+  header.set_file_name("test_file_name");
+  header.set_parent_folder("test_parent_folder");
+  int64_t time_millis = absl::ToUnixMillis(absl::Now());
+  header.set_last_modified_timestamp_millis(time_millis);
+  ErrorOr<std::unique_ptr<InternalPayload>> result =
+      CreateIncomingInternalPayload(frame, path);
+  ASSERT_FALSE(result.has_error());
+  std::unique_ptr<InternalPayload> internal_payload = std::move(result.value());
+  EXPECT_NE(internal_payload, nullptr);
+  EXPECT_EQ(internal_payload->GetFileName(), "test_file_name");
+  EXPECT_EQ(internal_payload->GetParentFolder(), "test_parent_folder");
+  // Allow for a 1ms error in the timestamp. This is due to the time being
+  // converted to a double for the proto and then back to a time.
+  EXPECT_LE(
+      std::abs(absl::ToUnixMillis(internal_payload->GetLastModifiedTime()) -
+             time_millis),
+      1);
+}
+
 TEST(InternalPayloadFactoryTest,
      CreateInternalPayloadFailsIfFileCannotBeCreated) {
   PayloadTransferFrame frame;
@@ -251,6 +283,30 @@ TEST(InternalPayloadFactoryTest,
   EXPECT_EQ(contents_after_skip, ByteArray("456789"));
 }
 
+TEST(InternalPayloadFactoryTest,
+     SkipToOffsetForBytesPayloadFailsIfOffsetIsTooLarge) {
+  ByteArray data(kText);
+  ErrorOr<std::unique_ptr<InternalPayload>> result =
+      CreateOutgoingInternalPayload(Payload{data});
+  ASSERT_FALSE(result.has_error());
+  std::unique_ptr<InternalPayload> internal_payload = std::move(result.value());
+  ASSERT_NE(internal_payload, nullptr);
+  EXPECT_EQ(internal_payload->SkipToOffset(1024).exception(), Exception::kIo);
+}
+
+TEST(InternalPayloadFactoryTest,
+     AttachNextChunkForOutgoingStreamPayloadFails) {
+  auto [input, output] = CreatePipe();
+  ErrorOr<std::unique_ptr<InternalPayload>> internal_payload_result =
+      CreateOutgoingInternalPayload(Payload(std::move(input)));
+  ASSERT_FALSE(internal_payload_result.has_error());
+  std::unique_ptr<InternalPayload> internal_payload =
+      std::move(internal_payload_result.value());
+  EXPECT_NE(internal_payload, nullptr);
+  EXPECT_EQ(internal_payload->AttachNextChunk("data"),
+            Exception{Exception::kIo});
+}
+
 TEST(InternalPayloadFactoryTest,
      SkipToOffset_StreamPayloadValidOffset_SkipsOffset) {
   absl::string_view contents("0123456789");
@@ -273,6 +329,105 @@ TEST(InternalPayloadFactoryTest,
   EXPECT_EQ(contents_after_skip, ByteArray("6789"));
 }
 
+TEST(InternalPayloadFactoryTest, IncomingFilePayloadBehavesCorrectly) {
+  PayloadTransferFrame frame;
+  std::string path = ::testing::TempDir();
+  frame.set_packet_type(PayloadTransferFrame::DATA);
+  auto& header = *frame.mutable_payload_header();
+  header.set_type(PayloadTransferFrame::PayloadHeader::FILE);
+  header.set_id(12345);
+  const int64_t total_size = 512;
+  header.set_total_size(total_size);
+  header.set_file_name("test_file_name");
+  header.set_parent_folder("test_parent_folder");
+  header.set_last_modified_timestamp_millis(1234567890);
+  ErrorOr<std::unique_ptr<InternalPayload>> result =
+      CreateIncomingInternalPayload(frame, path);
+  ASSERT_FALSE(result.has_error());
+  std::unique_ptr<InternalPayload> internal_payload = std::move(result.value());
+  ASSERT_NE(internal_payload, nullptr);
+
+  EXPECT_EQ(internal_payload->GetType(),
+            PayloadTransferFrame::PayloadHeader::FILE);
+  EXPECT_EQ(internal_payload->GetTotalSize(), total_size);
+  EXPECT_TRUE(internal_payload->DetachNextChunk(1024).Empty());
+  EXPECT_EQ(internal_payload->SkipToOffset(1024).exception(),
+            Exception::kIo);
+
+  // Attach a chunk.
+  std::string chunk1 = "chunk1";
+  ASSERT_TRUE(internal_payload->AttachNextChunk(chunk1).Ok());
+
+  // Attach another chunk.
+  std::string chunk2 = "chunk2";
+  ASSERT_TRUE(internal_payload->AttachNextChunk(chunk2).Ok());
+
+  // Close payload by attaching empty chunk.
+  ASSERT_TRUE(internal_payload->AttachNextChunk("").Ok());
+
+  // Verify file content.
+  Payload payload = internal_payload->ReleasePayload();
+  InputFile* input_file = payload.AsFile();
+  ASSERT_NE(input_file, nullptr);
+  std::string expected_content_str = chunk1 + chunk2;
+  ByteArray expected_content(expected_content_str);
+  ExceptionOr<ByteArray> file_content =
+      input_file->Read(expected_content.size());
+  input_file->Close();
+  ASSERT_TRUE(file_content.ok());
+  EXPECT_EQ(file_content.result(), expected_content);
+}
+
+TEST(InternalPayloadFactoryTest, IncomingStreamPayloadBehavesCorrectly) {
+  PayloadTransferFrame frame;
+  std::string path = ::testing::TempDir();
+  frame.set_packet_type(PayloadTransferFrame::DATA);
+  auto& header = *frame.mutable_payload_header();
+  header.set_type(PayloadTransferFrame::PayloadHeader::STREAM);
+  header.set_id(12345);
+  header.set_total_size(0);
+  ErrorOr<std::unique_ptr<InternalPayload>> result =
+      CreateIncomingInternalPayload(frame, path);
+  ASSERT_FALSE(result.has_error());
+  std::unique_ptr<InternalPayload> internal_payload = std::move(result.value());
+  ASSERT_NE(internal_payload, nullptr);
+
+  EXPECT_EQ(internal_payload->GetType(),
+            PayloadTransferFrame::PayloadHeader::STREAM);
+  EXPECT_EQ(internal_payload->GetTotalSize(), -1);
+  EXPECT_TRUE(internal_payload->DetachNextChunk(1024).Empty());
+  EXPECT_EQ(internal_payload->SkipToOffset(1024).exception(), Exception::kIo);
+
+  // Attach a chunk.
+  std::string chunk1 = "chunk1";
+  ASSERT_TRUE(internal_payload->AttachNextChunk(chunk1).Ok());
+
+  // Attach another chunk.
+  std::string chunk2 = "chunk2";
+  ASSERT_TRUE(internal_payload->AttachNextChunk(chunk2).Ok());
+
+  // Close payload by attaching empty chunk.
+  ASSERT_TRUE(internal_payload->AttachNextChunk("").Ok());
+
+  Payload payload = internal_payload->ReleasePayload();
+  InputStream* input_stream = payload.AsStream();
+  ASSERT_NE(input_stream, nullptr);
+
+  // Read from input stream to verify.
+  std::string result_str;
+  while (true) {
+    ExceptionOr<ByteArray> chunk = input_stream->Read(1024);
+    ASSERT_TRUE(chunk.ok());
+    if (chunk.result().Empty()) break;
+    result_str.append(chunk.result().data(), chunk.result().size());
+  }
+  ByteArray result_bytes(result_str);
+  std::string expected_content_str = chunk1 + chunk2;
+  ByteArray expected_content(expected_content_str);
+  EXPECT_EQ(result_bytes, expected_content);
+  input_stream->Close();
+}
+
 }  // namespace
 }  // namespace connections
 }  // namespace nearby