From b49414cb90f255648b753be74eae7010fa1af35b Mon Sep 17 00:00:00 2001 From: hai007 Date: Tue, 10 Feb 2026 09:56:04 -0800 Subject: [PATCH] Add more test cases for InternalPayloadFactoryTest to improve test coverage. PiperOrigin-RevId: 868195322 --- connections/implementation/BUILD | 1 + .../internal_payload_factory.cc | 32 ++-- .../internal_payload_factory_test.cc | 167 +++++++++++++++++- 3 files changed, 173 insertions(+), 27 deletions(-) diff --git a/connections/implementation/BUILD b/connections/implementation/BUILD index 4f37d002..ec42c3d2 100644 --- a/connections/implementation/BUILD +++ b/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", ], ) diff --git a/connections/implementation/internal_payload_factory.cc b/connections/implementation/internal_payload_factory.cc index 8958ecc6..36d65718 100644 --- a/connections/implementation/internal_payload_factory.cc +++ b/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> 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> CreateIncomingInternalPayload( const location::nearby::connections::PayloadTransferFrame& frame, const std::string& custom_save_path) { diff --git a/connections/implementation/internal_payload_factory_test.cc b/connections/implementation/internal_payload_factory_test.cc index dc72bd5c..082b968a 100644 --- a/connections/implementation/internal_payload_factory_test.cc +++ b/connections/implementation/internal_payload_factory_test.cc @@ -14,6 +14,7 @@ #include "connections/implementation/internal_payload_factory.h" +#include #include #include #include @@ -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> result = + CreateIncomingInternalPayload(frame, path); + ASSERT_FALSE(result.has_error()); + std::unique_ptr 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> result = + CreateOutgoingInternalPayload(Payload{data}); + ASSERT_FALSE(result.has_error()); + std::unique_ptr 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> internal_payload_result = + CreateOutgoingInternalPayload(Payload(std::move(input))); + ASSERT_FALSE(internal_payload_result.has_error()); + std::unique_ptr 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> result = + CreateIncomingInternalPayload(frame, path); + ASSERT_FALSE(result.has_error()); + std::unique_ptr 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 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> result = + CreateIncomingInternalPayload(frame, path); + ASSERT_FALSE(result.has_error()); + std::unique_ptr 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 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