diff --git a/cpp/core/BUILD b/cpp/core/BUILD index 81f1f0fc..4588a291 100644 --- a/cpp/core/BUILD +++ b/cpp/core/BUILD @@ -101,7 +101,6 @@ cc_test( deps = [ ":core", ":core_types", - "//file/util:temp_path", "//testing/base/public:gunit_main", "//absl/strings", "//absl/time", diff --git a/cpp/core/core.cc b/cpp/core/core.cc index 526e9977..beb85969 100644 --- a/cpp/core/core.cc +++ b/cpp/core/core.cc @@ -20,7 +20,6 @@ #include #include "absl/time/clock.h" -#include "core/internal/offline_frames_validator.h" #include "core/options.h" #include "platform/base/feature_flags.h" #include "platform/public/count_down_latch.h" @@ -131,12 +130,6 @@ void Core::SendPayload(absl::Span endpoint_ids, Payload payload, ResultCallback callback) { assert(payload.GetType() != Payload::Type::kUnknown); assert(!endpoint_ids.empty()); - if (payload.GetType() == Payload::Type::kFile) { - assert(!parser::HasIllegalCharacters(payload.GetFileName(), - parser::ILLEGAL_FILENAME_PATTERNS)); - assert(!parser::HasIllegalCharacters( - payload.GetParentFolder(), parser::ILLEGAL_PARENT_FOLDER_PATTERNS)); - } router_->SendPayload(&client_, endpoint_ids, std::move(payload), callback); } diff --git a/cpp/core/internal/BUILD b/cpp/core/internal/BUILD index 5d1a309e..9247f02e 100644 --- a/cpp/core/internal/BUILD +++ b/cpp/core/internal/BUILD @@ -190,7 +190,6 @@ cc_test( deps = [ ":internal", ":internal_test", - "//file/util:temp_path", "//testing/base/public:gunit", "//testing/base/public:gunit_main", "//absl/container:flat_hash_set", diff --git a/cpp/core/internal/base_pcp_handler.cc b/cpp/core/internal/base_pcp_handler.cc index f371df35..f258cd33 100644 --- a/cpp/core/internal/base_pcp_handler.cc +++ b/cpp/core/internal/base_pcp_handler.cc @@ -1478,7 +1478,6 @@ void BasePcpHandler::LogConnectionAttemptFailure( } } -// TODO(jfcarroll): FIXME!!!! void BasePcpHandler::LogConnectionAttemptSuccess( const std::string& endpoint_id, const PendingConnectionInfo& connection_info) { @@ -1498,26 +1497,22 @@ void BasePcpHandler::LogConnectionAttemptSuccess( "LogConnectionAttemptSuccess. Bail out."); return; } - // TODO(jfcarroll): Something in the below code is coming up null - // causing a crash. I can't debug this locally, and as a TVC I'm - // not able to debug using ciderd. - // if (connection_info.is_incoming) { - // connection_info.client->GetAnalyticsRecorder().OnIncomingConnectionAttempt( - // proto::connections::INITIAL, - // connection_info.channel->GetMedium(), - // proto::connections::RESULT_SUCCESS, - // SystemClock::ElapsedRealtime() - connection_info.start_time, - // connection_info.connection_token, - // connections_attempt_metadata_params.get()); - //} else { - // connection_info.client->GetAnalyticsRecorder().OnOutgoingConnectionAttempt( - // endpoint_id, proto::connections::INITIAL, - // connection_info.channel->GetMedium(), - // proto::connections::RESULT_SUCCESS, - // SystemClock::ElapsedRealtime() - connection_info.start_time, - // connection_info.connection_token, - // connections_attempt_metadata_params.get()); - //} + if (connection_info.is_incoming) { + connection_info.client->GetAnalyticsRecorder().OnIncomingConnectionAttempt( + proto::connections::INITIAL, connection_info.channel->GetMedium(), + proto::connections::RESULT_SUCCESS, + SystemClock::ElapsedRealtime() - connection_info.start_time, + connection_info.connection_token, + connections_attempt_metadata_params.get()); + } else { + connection_info.client->GetAnalyticsRecorder().OnOutgoingConnectionAttempt( + endpoint_id, proto::connections::INITIAL, + connection_info.channel->GetMedium(), + proto::connections::RESULT_SUCCESS, + SystemClock::ElapsedRealtime() - connection_info.start_time, + connection_info.connection_token, + connections_attempt_metadata_params.get()); + } } bool BasePcpHandler::Cancelled(ClientProxy* client, diff --git a/cpp/core/internal/internal_payload_factory.cc b/cpp/core/internal/internal_payload_factory.cc index e554acc3..8895992d 100644 --- a/cpp/core/internal/internal_payload_factory.cc +++ b/cpp/core/internal/internal_payload_factory.cc @@ -189,7 +189,7 @@ class OutgoingFileInternalPayload : public InternalPayload { std::int64_t GetTotalSize() const override { return total_size_; } ByteArray DetachNextChunk(int chunk_size) override { - const InputFile* file = payload_.AsFile(); + InputFile* file = payload_.AsFile(); if (!file) return {}; ExceptionOr bytes_read = file->Read(chunk_size); @@ -215,7 +215,7 @@ class OutgoingFileInternalPayload : public InternalPayload { ExceptionOr SkipToOffset(size_t offset) override { NEARBY_LOGS(INFO) << "SkipToOffset " << offset; - const InputFile* file = payload_.AsFile(); + InputFile* file = payload_.AsFile(); if (!file) { return {Exception::kIo}; } @@ -236,7 +236,7 @@ class OutgoingFileInternalPayload : public InternalPayload { } void Close() override { - const InputFile* file = payload_.AsFile(); + InputFile* file = payload_.AsFile(); if (file) file->Close(); } @@ -292,6 +292,10 @@ std::unique_ptr CreateOutgoingInternalPayload( return absl::make_unique(std::move(payload)); case Payload::Type::kFile: { + InputFile* file = payload.AsFile(); + const PayloadId file_payload_id = file ? file->GetPayloadId() : 0; + const PayloadId payload_id = payload.GetId(); + CHECK(payload_id == file_payload_id); return absl::make_unique(std::move(payload)); } @@ -305,22 +309,6 @@ std::unique_ptr CreateOutgoingInternalPayload( } } -std::string make_path(std::string parent_folder, std::string file_name) { - if (parent_folder.find_last_of('/') == std::string::npos) { - parent_folder.append("/"); - } - - return parent_folder.append(file_name); -} - -std::string make_path(std::string parent_folder, int64_t id) { - if (parent_folder.find_last_of('/') == std::string::npos) { - parent_folder.append("/"); - } - - return parent_folder.append(".nearby/" + std::to_string(id)); -} - std::unique_ptr CreateIncomingInternalPayload( const PayloadTransferFrame& frame) { if (frame.packet_type() != PayloadTransferFrame::DATA) { @@ -346,39 +334,11 @@ std::unique_ptr CreateIncomingInternalPayload( } case PayloadTransferFrame::PayloadHeader::FILE: { - std::string file_path; - int64_t total_size; - - if (frame.payload_header().has_parent_folder()) { - file_path = frame.payload_header().parent_folder(); - } - - if (!frame.payload_header().has_file_name()) { - file_path = make_path(file_path, frame.payload_header().id()); - } else { - file_path = make_path(file_path, frame.payload_header().file_name()); - } - - if (frame.payload_header().has_total_size()) { - total_size = frame.payload_header().total_size(); - } - - // These are ordered, the output file must be created first otherwise - // there will be no input file to open. - OutputFile outputFile( - location::nearby::api::ImplementationPlatform::GetDownloadPath( - std::make_unique(file_path)) - ->c_str()); - InputFile inputFile( - location::nearby::api::ImplementationPlatform::GetDownloadPath( - std::make_unique(file_path)) - ->c_str()); - + std::int64_t total_size = frame.payload_header().total_size(); return absl::make_unique( - Payload(payload_id, std::move(inputFile)), std::move(outputFile), - frame.payload_header().total_size()); + Payload(payload_id, InputFile(payload_id, total_size)), + OutputFile(payload_id), total_size); } - default: DCHECK(false); // This should never happen. return {}; diff --git a/cpp/core/internal/internal_payload_factory_test.cc b/cpp/core/internal/internal_payload_factory_test.cc index 90acea1e..adfbe9b2 100644 --- a/cpp/core/internal/internal_payload_factory_test.cc +++ b/cpp/core/internal/internal_payload_factory_test.cc @@ -14,13 +14,9 @@ #include "core/internal/internal_payload_factory.h" -#include -#include -#include #include #include -#include "file/util/temp_path.h" #include "gmock/gmock.h" #include "gtest/gtest.h" #include "core/internal/offline_frames.h" @@ -34,28 +30,8 @@ namespace connections { namespace { constexpr char kText[] = "data chunk"; -#define TEST_FILE_NAME std::string("testfilename.txt") -#define TEST_FILE_PARENT_FOLDER std::string("") -class InternalPayloadFActoryTest : public ::testing::Test { - protected: - void SetUp() override { - temp_path_ = std::make_unique(TempPath::Local); - path_ = temp_path_->path() + "/" + TEST_FILE_NAME; - file_ = std::fstream(path_, std::fstream::out | std::fstream::trunc); - file_ << "This is a test file with a minimum of 101 characters. This is " - "used to verify the InputFile in the payload_test google test."; - file_.close(); - } - - void TearDown() override { std::filesystem::remove(path_.c_str()); } - - std::fstream file_; - std::unique_ptr temp_path_; - std::string path_; -}; - -TEST_F(InternalPayloadFActoryTest, CanCreateIternalPayloadFromBytePayload) { +TEST(InternalPayloadFActoryTest, CanCreateIternalPayloadFromBytePayload) { ByteArray data(kText); std::unique_ptr internal_payload = CreateOutgoingInternalPayload(Payload{data}); @@ -66,7 +42,7 @@ TEST_F(InternalPayloadFActoryTest, CanCreateIternalPayloadFromBytePayload) { EXPECT_EQ(payload.AsBytes(), ByteArray(kText)); } -TEST_F(InternalPayloadFActoryTest, CanCreateIternalPayloadFromStreamPayload) { +TEST(InternalPayloadFActoryTest, CanCreateIternalPayloadFromStreamPayload) { auto pipe = std::make_shared(); std::unique_ptr internal_payload = CreateOutgoingInternalPayload(Payload{[pipe]() -> InputStream& { @@ -79,21 +55,21 @@ TEST_F(InternalPayloadFActoryTest, CanCreateIternalPayloadFromStreamPayload) { EXPECT_EQ(payload.AsBytes(), ByteArray()); } -TEST_F(InternalPayloadFActoryTest, CanCreateIternalPayloadFromFilePayload) { +TEST(InternalPayloadFActoryTest, CanCreateIternalPayloadFromFilePayload) { + Payload::Id payload_id = Payload::GenerateId(); std::unique_ptr internal_payload = - CreateOutgoingInternalPayload(Payload{ - path_.c_str(), TEST_FILE_NAME.c_str(), InputFile(path_.c_str())}); + CreateOutgoingInternalPayload( + Payload{payload_id, InputFile(payload_id, 512)}); EXPECT_NE(internal_payload, nullptr); Payload payload = internal_payload->ReleasePayload(); EXPECT_NE(payload.AsFile(), nullptr); EXPECT_EQ(payload.AsStream(), nullptr); EXPECT_EQ(payload.AsBytes(), ByteArray()); - EXPECT_EQ(payload.AsFile()->GetFilePath(), path_); - payload.AsFile()->Close(); - std::filesystem::remove(path_); + EXPECT_EQ(payload.GetId(), payload_id); + EXPECT_EQ(payload.AsFile()->GetPayloadId(), payload_id); } -TEST_F(InternalPayloadFActoryTest, CanCreateIternalPayloadFromByteMessage) { +TEST(InternalPayloadFActoryTest, CanCreateIternalPayloadFromByteMessage) { PayloadTransferFrame frame; frame.set_packet_type(PayloadTransferFrame::DATA); std::int64_t payload_chunk_offset = 0; @@ -116,7 +92,7 @@ TEST_F(InternalPayloadFActoryTest, CanCreateIternalPayloadFromByteMessage) { EXPECT_EQ(payload.AsBytes(), ByteArray(kText)); } -TEST_F(InternalPayloadFActoryTest, CanCreateIternalPayloadFromStreamMessage) { +TEST(InternalPayloadFActoryTest, CanCreateIternalPayloadFromStreamMessage) { PayloadTransferFrame frame; frame.set_packet_type(PayloadTransferFrame::DATA); auto& header = *frame.mutable_payload_header(); @@ -133,7 +109,7 @@ TEST_F(InternalPayloadFActoryTest, CanCreateIternalPayloadFromStreamMessage) { EXPECT_EQ(payload.GetType(), Payload::Type::kStream); } -TEST_F(InternalPayloadFActoryTest, CanCreateIternalPayloadFromFileMessage) { +TEST(InternalPayloadFActoryTest, CanCreateIternalPayloadFromFileMessage) { PayloadTransferFrame frame; frame.set_packet_type(PayloadTransferFrame::DATA); auto& header = *frame.mutable_payload_header(); @@ -148,32 +124,25 @@ TEST_F(InternalPayloadFActoryTest, CanCreateIternalPayloadFromFileMessage) { EXPECT_EQ(payload.AsStream(), nullptr); EXPECT_EQ(payload.AsBytes(), ByteArray()); EXPECT_EQ(payload.GetType(), Payload::Type::kFile); + EXPECT_EQ(payload.GetId(), payload.AsFile()->GetPayloadId()); } -void CreateFileWithContents(const char* file_path, const ByteArray& contents) { - std::unique_ptr file = std::make_unique(file_path); - EXPECT_TRUE(file->Write(contents).Ok()); - EXPECT_TRUE(file->Close().Ok()); +void CreateFileWithContents(Payload::Id payload_id, const ByteArray& contents) { + OutputFile file(payload_id); + EXPECT_TRUE(file.Write(contents).Ok()); + EXPECT_TRUE(file.Close().Ok()); } -TEST_F(InternalPayloadFActoryTest, - SkipToOffset_FilePayloadValidOffset_SkipsOffset) { +TEST(InternalPayloadFActoryTest, + SkipToOffset_FilePayloadValidOffset_SkipsOffset) { ByteArray contents("0123456789"); constexpr size_t kOffset = 4; size_t size_after_skip = contents.size() - kOffset; - NEARBY_LOGS(INFO) - << "SkipToOffset_FilePayloadValidOffset_SkipsOffset: file path = " - << path_.c_str() << "\n"; - NEARBY_LOGS(INFO) - << "SkipToOffset_FilePayloadValidOffset_SkipsOffset: contents = " - << contents.data() << "\n"; - - CreateFileWithContents(path_.c_str(), contents); Payload::Id payload_id = Payload::GenerateId(); - std::unique_ptr inputFile = - std::make_unique(path_.c_str()); + CreateFileWithContents(payload_id, contents); std::unique_ptr internal_payload = - CreateOutgoingInternalPayload(Payload{payload_id, std::move(*inputFile)}); + CreateOutgoingInternalPayload( + Payload{payload_id, InputFile(payload_id, contents.size())}); EXPECT_NE(internal_payload, nullptr); ExceptionOr result = internal_payload->SkipToOffset(kOffset); @@ -184,12 +153,10 @@ TEST_F(InternalPayloadFActoryTest, ByteArray contents_after_skip = internal_payload->DetachNextChunk(size_after_skip); EXPECT_EQ(contents_after_skip, ByteArray("456789")); - internal_payload = nullptr; - std::filesystem::remove(path_); } -TEST_F(InternalPayloadFActoryTest, - SkipToOffset_StreamPayloadValidOffset_SkipsOffset) { +TEST(InternalPayloadFActoryTest, + SkipToOffset_StreamPayloadValidOffset_SkipsOffset) { ByteArray contents("0123456789"); constexpr size_t kOffset = 6; auto pipe = std::make_shared(); diff --git a/cpp/core/internal/offline_frames_validator.cc b/cpp/core/internal/offline_frames_validator.cc index b2af1bfb..614c65f8 100644 --- a/cpp/core/internal/offline_frames_validator.cc +++ b/cpp/core/internal/offline_frames_validator.cc @@ -24,13 +24,6 @@ namespace location { namespace nearby { namespace connections { namespace parser { -bool HasIllegalCharacters(std::string toBeValidated, - std::vector illegalPatterns) { - return std::any_of(illegalPatterns.begin(), illegalPatterns.end(), - [&toBeValidated](const auto& s) { - return toBeValidated.find(s) != std::string::npos; - }); -} namespace { using PayloadChunk = PayloadTransferFrame::PayloadChunk; @@ -127,25 +120,6 @@ Exception EnsureValidPayloadTransferFrame(const PayloadTransferFrame& frame) { frame.payload_header().total_size() != InternalPayload::kIndeterminateSize)) return {Exception::kInvalidProtocolBuffer}; - - if (frame.payload_header().has_type() && - frame.payload_header().type() == - PayloadTransferFrame::PayloadHeader::FILE) { - if (frame.payload_header().has_file_name()) { - if (HasIllegalCharacters(frame.payload_header().file_name(), - ILLEGAL_FILENAME_PATTERNS)) { - return {Exception::kFailed}; - } - } - - if (frame.payload_header().has_parent_folder()) { - if (HasIllegalCharacters(frame.payload_header().file_name(), - ILLEGAL_PARENT_FOLDER_PATTERNS)) { - return {Exception::kFailed}; - } - } - } - if (!frame.has_packet_type()) return {Exception::kInvalidProtocolBuffer}; switch (frame.packet_type()) { diff --git a/cpp/core/internal/offline_frames_validator.h b/cpp/core/internal/offline_frames_validator.h index 2c8db738..f64c32c3 100644 --- a/cpp/core/internal/offline_frames_validator.h +++ b/cpp/core/internal/offline_frames_validator.h @@ -23,19 +23,8 @@ namespace nearby { namespace connections { namespace parser { -const std::vector ILLEGAL_FILENAME_PATTERNS{ - "/", "\\", "?", "*", "\"", "<", ">", "|", "[", "]", - ":", ",", ";", "..", "\0", "\n", "\r", "\t", "\f"}; - -const std::vector ILLEGAL_PARENT_FOLDER_PATTERNS{ - "\\", "?", "*", "\"", "<", ">", "|", "[", "]", - ":", ",", ";", "..", "\0", "\n", "\r", "\t", "\f"}; - Exception EnsureValidOfflineFrame(const OfflineFrame& offline_frame); -bool HasIllegalCharacters(std::string toBeValidated, - std::vector illegalPatterns); - } // namespace parser } // namespace connections } // namespace nearby diff --git a/cpp/core/internal/payload_manager.cc b/cpp/core/internal/payload_manager.cc index 58d457aa..403be7e0 100644 --- a/cpp/core/internal/payload_manager.cc +++ b/cpp/core/internal/payload_manager.cc @@ -259,7 +259,6 @@ Payload::Id PayloadManager::CreateOutgoingPayload( Payload::Id payload_id = internal_payload->GetId(); NEARBY_LOGS(INFO) << "CreateOutgoingPayload: payload_id=" << payload_id; MutexLock lock(&mutex_); - pending_payloads_.StartTrackingPayload( payload_id, absl::make_unique(std::move(internal_payload), endpoint_ids, @@ -355,7 +354,6 @@ void PayloadManager::SendPayload(ClientProxy* client, // Before transfer to internal payload, retrieves the Payload size for // analytics. std::int64_t payload_total_size; - switch (payload.GetType()) { case connections::Payload::Type::kBytes: payload_total_size = payload.AsBytes().size(); @@ -394,15 +392,11 @@ void PayloadManager::SendPayload(ClientProxy* client, ? payload.GetOffset() : 0; - std::string file_name(""); - std::string parent_folder(""); - Payload::Id payload_id = CreateOutgoingPayload(std::move(payload), endpoint_ids); executor->Execute( - "send-payload", - [this, client, endpoint_ids, payload_id, payload_type, resume_offset, - payload_total_size, file_name, parent_folder]() { + "send-payload", [this, client, endpoint_ids, payload_id, payload_type, + resume_offset, payload_total_size]() { if (shutdown_.Get()) return; PendingPayload* pending_payload = GetPayload(payload_id); if (!pending_payload) { @@ -423,12 +417,10 @@ void PayloadManager::SendPayload(ClientProxy* client, payload_type, resume_offset, internal_payload->GetTotalSize()); - NEARBY_LOG(INFO, "Creating payload header (JFC)"); - PayloadTransferFrame::PayloadHeader payload_header{CreatePayloadHeader( - *internal_payload, resume_offset, parent_folder, file_name)}; + PayloadTransferFrame::PayloadHeader payload_header{ + CreatePayloadHeader(*internal_payload, resume_offset)}; bool should_continue = true; std::int64_t next_chunk_offset = 0; - NEARBY_LOG(INFO, "Entering send payload loop (JFC)"); while (should_continue && !shutdown_.Get()) { should_continue = SendPayloadLoop(client, *pending_payload, payload_header, @@ -618,15 +610,12 @@ int PayloadManager::GetOptimalChunkSize(EndpointIds endpoint_ids) { } PayloadTransferFrame::PayloadHeader PayloadManager::CreatePayloadHeader( - const InternalPayload& internal_payload, size_t offset, - std::string parent_folder, std::string file_name) { + const InternalPayload& internal_payload, size_t offset) { PayloadTransferFrame::PayloadHeader payload_header; size_t payload_size = internal_payload.GetTotalSize(); payload_header.set_id(internal_payload.GetId()); payload_header.set_type(internal_payload.GetType()); - payload_header.set_file_name(file_name); - payload_header.set_parent_folder(parent_folder); payload_header.set_total_size(payload_size == InternalPayload::kIndeterminateSize ? InternalPayload::kIndeterminateSize @@ -1181,7 +1170,6 @@ PayloadManager::PendingPayload::PendingPayload( // Later on some may become canceled, some may experience data transfer // failures. Any of these situations will cause endpoint to be marked as // unavailable. - for (const auto& id : endpoint_ids) { EndpointInfo endpoint_info{}; endpoint_info.id = id; diff --git a/cpp/core/internal/payload_manager.h b/cpp/core/internal/payload_manager.h index 934520d7..4e605ab8 100644 --- a/cpp/core/internal/payload_manager.h +++ b/cpp/core/internal/payload_manager.h @@ -211,8 +211,7 @@ class PayloadManager : public EndpointManager::FrameProcessor { int GetOptimalChunkSize(EndpointIds endpoint_ids); PayloadTransferFrame::PayloadHeader CreatePayloadHeader( - const InternalPayload& payload, size_t offset, std::string parent_folder, - std::string file_name); + const InternalPayload& payload, size_t offset); PayloadTransferFrame::PayloadChunk CreatePayloadChunk(std::int64_t offset, ByteArray body); diff --git a/cpp/core/internal/payload_manager_test.cc b/cpp/core/internal/payload_manager_test.cc index 095e675f..372642d3 100644 --- a/cpp/core/internal/payload_manager_test.cc +++ b/cpp/core/internal/payload_manager_test.cc @@ -258,7 +258,7 @@ TEST_P(PayloadManagerTest, CanCancelPayloadOnSenderSide) { PayloadSimulationUser user_a(kDeviceA, GetParam()); PayloadSimulationUser user_b(kDeviceB, GetParam()); ASSERT_TRUE(SetupConnection(user_a, user_b)); - NEARBY_LOG(INFO, "User a and user b have been setup. JFC"); + auto pipe = std::make_shared(); OutputStream& tx = pipe->GetOutputStream(); @@ -266,7 +266,6 @@ TEST_P(PayloadManagerTest, CanCancelPayloadOnSenderSide) { const ByteArray message{std::string(kMessage)}; tx.Write(message); - NEARBY_LOG(INFO, "Sending payload from user_b. JFC"); user_b.SendPayload(Payload([pipe]() -> InputStream& { return pipe->GetInputStream(); // NOLINT })); diff --git a/cpp/core/listeners.h b/cpp/core/listeners.h index 35157133..f9c0208d 100644 --- a/cpp/core/listeners.h +++ b/cpp/core/listeners.h @@ -32,8 +32,8 @@ #include "core/status.h" #include "platform/base/byte_array.h" #include "platform/base/byte_utils.h" -#include "platform/base/core_config.h" #include "platform/base/listeners.h" +#include "platform/public/core_config.h" namespace location { namespace nearby { diff --git a/cpp/core/params.h b/cpp/core/params.h index 7d4bf8c3..3843cee9 100644 --- a/cpp/core/params.h +++ b/cpp/core/params.h @@ -19,7 +19,7 @@ #include "core/listeners.h" #include "platform/base/byte_array.h" -#include "platform/base/core_config.h" +#include "platform/public/core_config.h" namespace location { namespace nearby { diff --git a/cpp/core/payload.cc b/cpp/core/payload.cc index aaba71e7..20acef85 100644 --- a/cpp/core/payload.cc +++ b/cpp/core/payload.cc @@ -21,24 +21,9 @@ namespace connections { // Payload is default-constructible, and moveable, but not copyable container // that holds at most one instance of one of: // ByteArray, InputStream, or InputFile. -Payload::Payload(Payload&& other) noexcept { - file_name_ = other.file_name_; - parent_folder_ = other.parent_folder_; - content_ = std::move(other.content_); - id_ = other.id_; - offset_ = other.offset_; - type_ = other.type_; -} +Payload::Payload(Payload&& other) noexcept = default; Payload::~Payload() = default; -Payload& Payload::operator=(Payload&& other) noexcept { - file_name_ = other.file_name_; - parent_folder_ = other.parent_folder_; - content_ = std::move(other.content_); - id_ = other.id_; - offset_ = other.offset_; - type_ = other.type_; - return *this; -} +Payload& Payload::operator=(Payload&& other) noexcept = default; // Default (invalid) payload. Payload::Payload() : content_(absl::monostate()) {} @@ -48,11 +33,9 @@ Payload::Payload(ByteArray&& bytes) : content_(std::move(bytes)) {} Payload::Payload(const ByteArray& bytes) : content_(bytes) {} -Payload::Payload(const char* parent_folder, const char* file_name, - InputFile&& file) +Payload::Payload(InputFile file) : content_(std::move(file)), - parent_folder_(parent_folder), - file_name_(file_name) {} + id_(std::hash()(file.GetFilePath())) {} // TODO(jfcarroll): Convert std::function to function pointer Payload::Payload(std::function stream) @@ -64,14 +47,7 @@ Payload::Payload(Id id, ByteArray&& bytes) Payload::Payload(Id id, const ByteArray& bytes) : content_(bytes), id_(id) {} -Payload::Payload(Id id, InputFile&& file) - : content_(std::move(file)), id_(id), parent_folder_("") { - auto fileName = std::to_string(id); - file_name_ = fileName.c_str(); - - NEARBY_LOGS(INFO) << "Payload(Id,InputFile): parent folder =" - << parent_folder_ << " file name = " << file_name_ << "\n"; -} +Payload::Payload(Id id, InputFile file) : content_(std::move(file)), id_(id) {} // TODO(jfcarroll): Convert std::function to function pointer Payload::Payload(Id id, std::function stream) @@ -93,9 +69,7 @@ InputStream* Payload::AsStream() { return result ? &(*result)() : nullptr; } // Returns InputFile* payload, if it has been defined, or nullptr. -const InputFile* Payload::AsFile() const { - return absl::get_if(&content_); -} +InputFile* Payload::AsFile() { return absl::get_if(&content_); } // Returns Payload unique ID. Payload::Id Payload::GetId() const { return id_; } @@ -106,10 +80,8 @@ Payload::Type Payload::GetType() const { return type_; } // Sets the payload offset in bytes void Payload::SetOffset(size_t offset) { CHECK(type_ == Type::kFile || type_ == Type::kStream); - const InputFile* file = AsFile(); + InputFile* file = AsFile(); if (file != nullptr) { - NEARBY_LOGS(INFO) << "Payload::SetOffset: offset: " << offset - << " file total size : " << file->GetTotalSize() << "\n"; CHECK(file->GetTotalSize() > 0 && offset < (size_t)file->GetTotalSize()); } offset_ = offset; @@ -124,14 +96,6 @@ Payload::Type Payload::FindType() const { return static_cast(content_.index()); } -const std::string Payload::GetParentFolder() const { - return std::string(parent_folder_); -} - -const std::string Payload::GetFileName() const { - return std::string(file_name_); -} - } // namespace connections } // namespace nearby } // namespace location diff --git a/cpp/core/payload.h b/cpp/core/payload.h index d5ea0896..ec66062b 100644 --- a/cpp/core/payload.h +++ b/cpp/core/payload.h @@ -22,10 +22,10 @@ #include "absl/types/variant.h" #include "platform/base/byte_array.h" -#include "platform/base/core_config.h" #include "platform/base/input_stream.h" #include "platform/base/payload_id.h" #include "platform/base/prng.h" +#include "platform/public/core_config.h" #include "platform/public/file.h" #include "platform/public/logging.h" @@ -56,14 +56,13 @@ class DLL_API Payload { explicit Payload(ByteArray&& bytes); explicit Payload(const ByteArray& bytes); - explicit Payload(const char* parent_folder, const char* file_name, - InputFile&& file); + explicit Payload(InputFile file); explicit Payload(std::function stream); // Constructors for incoming payloads. Payload(Id id, ByteArray&& bytes); Payload(Id id, const ByteArray& bytes); - Payload(Id id, InputFile&& file); + Payload(Id id, InputFile file); Payload(Id id, std::function stream); @@ -73,7 +72,7 @@ class DLL_API Payload { // Returns InputStream* payload, if it has been defined, or nullptr. InputStream* AsStream(); // Returns InputFile* payload, if it has been defined, or nullptr. - const InputFile* AsFile() const; + InputFile* AsFile(); // Returns Payload unique ID. Id GetId() const; @@ -89,9 +88,6 @@ class DLL_API Payload { // Generate Payload Id; to be passed to outgoing file constructor. static Id GenerateId(); - const std::string GetFileName() const; - const std::string GetParentFolder() const; - private: Type FindType() const; @@ -99,8 +95,6 @@ class DLL_API Payload { Id id_{GenerateId()}; Type type_{FindType()}; size_t offset_{0}; - const char* parent_folder_; - const char* file_name_; }; } // namespace connections diff --git a/cpp/core/payload_test.cc b/cpp/core/payload_test.cc index 009d8647..f6d54c76 100644 --- a/cpp/core/payload_test.cc +++ b/cpp/core/payload_test.cc @@ -14,13 +14,9 @@ #include "core/payload.h" -#include - -#include #include #include -#include "file/util/temp_path.h" #include "gmock/gmock.h" #include "gtest/gtest.h" #include "platform/base/byte_array.h" @@ -28,37 +24,16 @@ #include "platform/public/file.h" #include "platform/public/pipe.h" -#define TEST_FILE_PARENT_DIRECTORY std::string("") -#define TEST_FILE_NAME std::string("testfilename.txt") -#define TEST_FILE_PATH TEST_FILE_NAME - namespace location { namespace nearby { namespace connections { -class PayloadTest : public ::testing::Test { - protected: - void SetUp() override { - temp_path_ = std::make_unique(TempPath::Local); - path_ = temp_path_->path() + "/" + TEST_FILE_NAME; - file_ = std::fstream(path_, std::fstream::out | std::fstream::trunc); - file_ << "This is a test file with a minimum of 101 characters. This is " - "used to verify the InputFile in the payload_test google test."; - file_.close(); - } - - void TearDown() override { std::remove(path_.c_str()); } - - std::fstream file_; - std::unique_ptr temp_path_; - std::string path_; -}; -TEST_F(PayloadTest, DefaultPayloadHasUnknownType) { +TEST(PayloadTest, DefaultPayloadHasUnknownType) { Payload payload; EXPECT_EQ(payload.GetType(), Payload::Type::kUnknown); } -TEST_F(PayloadTest, SupportsByteArrayType) { +TEST(PayloadTest, SupportsByteArrayType) { const ByteArray bytes("bytes"); Payload payload(bytes); EXPECT_EQ(payload.GetType(), Payload::Type::kBytes); @@ -67,13 +42,13 @@ TEST_F(PayloadTest, SupportsByteArrayType) { EXPECT_EQ(payload.AsBytes(), bytes); } -TEST_F(PayloadTest, SupportsFileType) { +TEST(PayloadTest, SupportsFileType) { constexpr size_t kOffset = 99; - InputFile file(path_.c_str()); - const InputStream& stream = file.GetInputStream(); + const auto payload_id = Payload::GenerateId(); + InputFile file(payload_id, 100); + InputStream& stream = file.GetInputStream(); - Payload payload(TEST_FILE_PARENT_DIRECTORY.c_str(), TEST_FILE_PATH.c_str(), - std::move(file)); + Payload payload(payload_id, std::move(file)); payload.SetOffset(kOffset); EXPECT_EQ(payload.GetType(), Payload::Type::kFile); @@ -83,7 +58,7 @@ TEST_F(PayloadTest, SupportsFileType) { EXPECT_EQ(payload.GetOffset(), kOffset); } -TEST_F(PayloadTest, SupportsStreamType) { +TEST(PayloadTest, SupportsStreamType) { constexpr size_t kOffset = 1234456; auto pipe = std::make_shared(); @@ -103,7 +78,7 @@ TEST_F(PayloadTest, SupportsStreamType) { EXPECT_EQ(payload.GetOffset(), kOffset); } -TEST_F(PayloadTest, PayloadIsMoveable) { +TEST(PayloadTest, PayloadIsMoveable) { Payload payload1; Payload payload2(ByteArray("bytes")); auto id = payload2.GetId(); @@ -116,13 +91,13 @@ TEST_F(PayloadTest, PayloadIsMoveable) { EXPECT_EQ(payload1.GetId(), id); } -TEST_F(PayloadTest, PayloadHasUniqueId) { +TEST(PayloadTest, PayloadHasUniqueId) { Payload payload1; Payload payload2; EXPECT_NE(payload1.GetId(), payload2.GetId()); } -TEST_F(PayloadTest, PayloadIsNotCopyable) { +TEST(PayloadTest, PayloadIsNotCopyable) { EXPECT_FALSE(std::is_copy_constructible_v); EXPECT_FALSE(std::is_copy_assignable_v); } diff --git a/cpp/core/strategy.h b/cpp/core/strategy.h index 4268b45e..f85d7d60 100644 --- a/cpp/core/strategy.h +++ b/cpp/core/strategy.h @@ -16,7 +16,7 @@ #include -#include "platform/base/core_config.h" +#include "platform/public/core_config.h" namespace location { namespace nearby { diff --git a/cpp/platform/api/platform.h b/cpp/platform/api/platform.h index c6873e0f..6cf54cae 100644 --- a/cpp/platform/api/platform.h +++ b/cpp/platform/api/platform.h @@ -61,9 +61,6 @@ class ImplementationPlatform { // - file I/O // - Logging - static std::unique_ptr GetDownloadPath( - std::unique_ptr path); - // Atomics: // ======= @@ -81,11 +78,12 @@ class ImplementationPlatform { std::int32_t count); static std::unique_ptr CreateMutex(Mutex::Mode mode); static std::unique_ptr CreateConditionVariable( - Mutex *mutex); - static std::unique_ptr CreateInputFile(const char *file_path); - static std::unique_ptr CreateOutputFile(const char *file_path); + Mutex* mutex); + static std::unique_ptr CreateInputFile(PayloadId payload_id, + std::int64_t total_size); + static std::unique_ptr CreateOutputFile(PayloadId payload_id); static std::unique_ptr CreateLogMessage( - const char *file, int line, LogMessage::Severity severity); + const char* file, int line, LogMessage::Severity severity); // Java-like Executors static std::unique_ptr CreateSingleThreadExecutor(); @@ -96,10 +94,10 @@ class ImplementationPlatform { // Protocol implementations, domain-specific support static std::unique_ptr CreateBluetoothAdapter(); static std::unique_ptr CreateBluetoothClassicMedium( - BluetoothAdapter &); - static std::unique_ptr CreateBleMedium(BluetoothAdapter &); + BluetoothAdapter&); + static std::unique_ptr CreateBleMedium(BluetoothAdapter&); static std::unique_ptr CreateBleV2Medium( - BluetoothAdapter &); + BluetoothAdapter&); static std::unique_ptr CreateServerSyncMedium(); static std::unique_ptr CreateWifiMedium(); static std::unique_ptr CreateWifiLanMedium(); diff --git a/cpp/platform/base/BUILD b/cpp/platform/base/BUILD index 89008e3b..9fa908b8 100644 --- a/cpp/platform/base/BUILD +++ b/cpp/platform/base/BUILD @@ -29,7 +29,6 @@ cc_library( "bluetooth_utils.h", "byte_array.h", "callable.h", - "core_config.h", "exception.h", "feature_flags.h", "input_stream.h", diff --git a/cpp/platform/base/core_config.h b/cpp/platform/base/core_config.h deleted file mode 100644 index 3afa60fc..00000000 --- a/cpp/platform/base/core_config.h +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright 2020 Google LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// https://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -#ifndef CORE_CONFIG_H_ -#define CORE_CONFIG_H_ - -namespace location { -namespace nearby { -namespace connections { - -#ifdef _WIN32 // These storage class specifiers only matter to win32 dll - // builds. -#ifdef CORE_ADAPTER_DLL -#define DLL_API \ - __declspec(dllexport) // If we're building the core, we're exporting. -#else // !CORE_ADAPTER_DLL -#define DLL_API \ - __declspec(dllimport) // If we're not building the core, we're importing. -#endif // CORE_ADAPTER_DLL -#else // !_WIN32 -#define DLL_API // We're not building a win32 dll, leave the source unchanged. -#endif // _WIN32 - -} // namespace connections -} // namespace nearby -} // namespace location - -#endif // CORE_CONFIG_H_ diff --git a/cpp/platform/impl/g3/platform.cc b/cpp/platform/impl/g3/platform.cc index 5dff0a0a..470af01d 100644 --- a/cpp/platform/impl/g3/platform.cc +++ b/cpp/platform/impl/g3/platform.cc @@ -55,11 +55,11 @@ namespace location { namespace nearby { namespace api { -std::unique_ptr ImplementationPlatform::GetDownloadPath( - std::unique_ptr path) { - std::string basePath("/tmp/"); - return std::make_unique(basePath += *path); +namespace { +std::string GetPayloadPath(PayloadId payload_id) { + return absl::StrCat("/tmp/", payload_id); } +} // namespace int GetCurrentTid() { const LiveThread* my = Thread_GetMyLiveThread(); @@ -102,13 +102,14 @@ std::unique_ptr ImplementationPlatform::CreateAtomicBoolean( } std::unique_ptr ImplementationPlatform::CreateInputFile( - const char* file_path) { - return absl::make_unique(file_path); + PayloadId payload_id, std::int64_t total_size) { + return absl::make_unique(GetPayloadPath(payload_id), + total_size); } std::unique_ptr ImplementationPlatform::CreateOutputFile( - const char* file_path) { - return absl::make_unique(file_path); + PayloadId payload_id) { + return absl::make_unique(GetPayloadPath(payload_id)); } std::unique_ptr ImplementationPlatform::CreateLogMessage( diff --git a/cpp/platform/impl/ios/Source/Internal/GNCCoreConnection.mm b/cpp/platform/impl/ios/Source/Internal/GNCCoreConnection.mm index 607a884a..30daf0d2 100644 --- a/cpp/platform/impl/ios/Source/Internal/GNCCoreConnection.mm +++ b/cpp/platform/impl/ios/Source/Internal/GNCCoreConnection.mm @@ -161,10 +161,7 @@ class GNCInputStreamFromNSStream : public InputStream { PayloadId payloadId = Payload::GenerateId(); // Add the pair of payloadId and fileURL to the map in the GNCCore. [_core insertURLToMapWithPayloadID:payloadId urlToSend:fileURL]; - - // TODO(edwinwu): Need someone familiar with iOS to fix this - std::string path("FIXME, WE NEED A PATH HERE"); - Payload corePayload(payloadId, InputFile(path.c_str())); + Payload corePayload(payloadId, InputFile(payloadId, fileSize)); progress.totalUnitCount = fileSize; return [self sendPayload:std::move(corePayload) size:fileSize diff --git a/cpp/platform/impl/ios/Source/Internal/GNCPayloadListener.mm b/cpp/platform/impl/ios/Source/Internal/GNCPayloadListener.mm index 08759c6f..f7efb389 100644 --- a/cpp/platform/impl/ios/Source/Internal/GNCPayloadListener.mm +++ b/cpp/platform/impl/ios/Source/Internal/GNCPayloadListener.mm @@ -146,7 +146,7 @@ void GNCPayloadListener::OnPayload(const std::string &endpoint_id, Payload paylo case Payload::Type::kFile: if (handlers.filePayloadHandler) { - const InputFile *payloadInputFile = payload.AsFile(); + InputFile *payloadInputFile = payload.AsFile(); NSURL *fileURL = [NSURL URLWithString:ObjCStringFromCppString(payloadInputFile->GetFilePath())]; int64_t fileSize = payloadInputFile->GetTotalSize(); diff --git a/cpp/platform/impl/ios/Source/Internal/platform.mm b/cpp/platform/impl/ios/Source/Internal/platform.mm index 0bd3e990..21f64426 100644 --- a/cpp/platform/impl/ios/Source/Internal/platform.mm +++ b/cpp/platform/impl/ios/Source/Internal/platform.mm @@ -37,11 +37,18 @@ namespace location { namespace nearby { namespace api { -std::unique_ptr ImplementationPlatform::GetDownloadPath( - std::unique_ptr path) { - // TODO(jfcarroll): Fixme, we need to modulate the path the the system download path - return path; +namespace { +std::string GetPayloadPath(PayloadId payload_id) { + // This is to get a file path, e.g. /tmp/[payload_id], for the storage of payload file. + // NOTE: Per + // https://developer.apple.com/library/content/documentation/FileManagement/Conceptual/FileSystemProgrammingGuide/FileSystemOverview/FileSystemOverview.html + // Files saved in the /tmp directory will be deleted by the system. Callers should be responsible + // for copying the files to the permanent storage. + NSString* payloadIdString = ObjCStringFromCppString(std::to_string(payload_id)); + return CppStringFromObjCString( + [NSTemporaryDirectory() stringByAppendingPathComponent:payloadIdString]); } +} // namespace // Atomics: std::unique_ptr ImplementationPlatform::CreateAtomicBoolean(bool initial_value) { @@ -71,25 +78,22 @@ std::unique_ptr ImplementationPlatform::CreateConditionVariab return std::make_unique(static_cast(mutex)); } -std::unique_ptr ImplementationPlatform::CreateInputFile(const char* file_path) { -// Extract the NSURL object with payload_id from |GNCCore| which stores the maps. If the retrieved -// NSURL object is not nil, we create InputFile by ios::InputFile. The difference is -// that ios::InputFile implements to read bytes from local real file for sending. -// TODO(jfcarroll): Need someone familiar with iOS to fix this -#if 0 +std::unique_ptr ImplementationPlatform::CreateInputFile(PayloadId payload_id, + std::int64_t total_size) { + // Extract the NSURL object with payload_id from |GNCCore| which stores the maps. If the retrieved + // NSURL object is not nil, we create InputFile by ios::InputFile. The difference is + // that ios::InputFile implements to read bytes from local real file for sending. GNCCore* core = GNCGetCore(); NSURL* url = [core extractURLWithPayloadID:payload_id]; if (url != nil) { return absl::make_unique(url); } else { - return absl::make_unique(GetDownloadPath(payload_id), total_size); + return absl::make_unique(GetPayloadPath(payload_id), total_size); } -#endif - return nullptr; } -std::unique_ptr ImplementationPlatform::CreateOutputFile(const char* file_path) { - return absl::make_unique(file_path); +std::unique_ptr ImplementationPlatform::CreateOutputFile(PayloadId payload_id) { + return absl::make_unique(GetPayloadPath(payload_id)); } std::unique_ptr ImplementationPlatform::CreateLogMessage( diff --git a/cpp/platform/impl/ios/Tests/BUILD b/cpp/platform/impl/ios/Tests/BUILD index 5890a682..c0f032ed 100644 --- a/cpp/platform/impl/ios/Tests/BUILD +++ b/cpp/platform/impl/ios/Tests/BUILD @@ -23,8 +23,7 @@ objc_library( srcs = [ "Platform/GNCCryptoTest.mm", "Platform/GNCInputFileTest.mm", - # TODO(edwinwu): This test is failing, needs fixing. - #"Platform/GNCMultiThreadExecutorTest.mm", + "Platform/GNCMultiThreadExecutorTest.mm", "Platform/GNCScheduledExecutorTest.mm", "Platform/GNCSingleThreadExecutorTest.mm", ], diff --git a/cpp/platform/impl/shared/file.cc b/cpp/platform/impl/shared/file.cc index ec77eb50..cadbadb6 100644 --- a/cpp/platform/impl/shared/file.cc +++ b/cpp/platform/impl/shared/file.cc @@ -25,21 +25,9 @@ namespace nearby { namespace shared { // InputFile -InputFile::InputFile(const char* file_path) : file_path_(file_path) { - // Open the file with the current location at eof (std::ios::ate) - // std::ios::binary - specifies binary access mode - // std::ios::in - allows input (read operations) from a stream - // std::ios::ate - sets the stream's position indicator to the - // end of the stream on opening. - file_.open(std::string(file_path), - std::ios::binary | std::ios::in | std::ios::ate); - // Read the current position in the file and use that for total size. - total_size_ = file_.tellg(); - - // Reset to the beginning of the file. - file_.seekg(0); -} +InputFile::InputFile(const std::string& path, std::int64_t size) + : file_(path, std::ios::binary), path_(path), total_size_(size) {} ExceptionOr InputFile::Read(std::int64_t size) { if (!file_.is_open()) { @@ -74,15 +62,8 @@ Exception InputFile::Close() { // OutputFile -OutputFile::OutputFile(const char* file_path) { - // std::ios::binary - specifies binary access mode - // std::ios::out - allows output (read operations) from - // a stream - // std::ios::trunc - when the file is opened, the old - // contents are immediately removed. - file_ = std::ofstream(file_path, - std::ios::binary | std::ios::out | std::ios::trunc); -} +OutputFile::OutputFile(absl::string_view path) + : file_(std::string(path), std::ios::binary) {} Exception OutputFile::Write(const ByteArray& data) { if (!file_.is_open()) { diff --git a/cpp/platform/impl/shared/file.h b/cpp/platform/impl/shared/file.h index dfc476cc..21ed871c 100644 --- a/cpp/platform/impl/shared/file.h +++ b/cpp/platform/impl/shared/file.h @@ -29,25 +29,25 @@ namespace shared { class InputFile final : public api::InputFile { public: - explicit InputFile(const char* file_path); + explicit InputFile(const std::string& path, std::int64_t size); ~InputFile() override = default; InputFile(InputFile&&) = default; InputFile& operator=(InputFile&&) = default; - ExceptionOr Read(int64_t size) override; - std::string GetFilePath() const override { return file_path_; } - int64_t GetTotalSize() const override { return total_size_; } + ExceptionOr Read(std::int64_t size) override; + std::string GetFilePath() const override { return path_; } + std::int64_t GetTotalSize() const override { return total_size_; } Exception Close() override; private: std::ifstream file_; - std::string file_path_; - int64_t total_size_; + std::string path_; + std::int64_t total_size_; }; class OutputFile final : public api::OutputFile { public: - explicit OutputFile(const char* file_path); + explicit OutputFile(absl::string_view path); ~OutputFile() override = default; OutputFile(OutputFile&&) = default; OutputFile& operator=(OutputFile&&) = default; diff --git a/cpp/platform/impl/shared/file_test.cc b/cpp/platform/impl/shared/file_test.cc index 69249838..e1a55a81 100644 --- a/cpp/platform/impl/shared/file_test.cc +++ b/cpp/platform/impl/shared/file_test.cc @@ -14,9 +14,7 @@ #include "platform/impl/shared/file.h" -#include #include -#include #include #include #include @@ -26,10 +24,6 @@ #include "absl/strings/string_view.h" #include "platform/base/byte_array.h" -#define TEST_FILE_NAME std::string("testfilename.txt") -#define TEST_INVALID_PARENT_FOLDER \ - std::string("fake/path/that/has/not/been/created/") - namespace location { namespace nearby { namespace shared { @@ -38,7 +32,7 @@ class FileTest : public ::testing::Test { protected: void SetUp() override { temp_path_ = std::make_unique(TempPath::Local); - path_ = temp_path_->path() + "/" + TEST_FILE_NAME; + path_ = temp_path_->path() + "/file.txt"; std::ofstream output_file(path_); file_ = std::fstream(path_, std::fstream::in | std::fstream::out); } @@ -71,39 +65,39 @@ class FileTest : public ::testing::Test { }; TEST_F(FileTest, InputFile_NonExistentPath) { - InputFile input_file((TEST_INVALID_PARENT_FOLDER + TEST_FILE_NAME).c_str()); + InputFile input_file("/not/a/valid/path.txt", GetSize()); ExceptionOr read_result = input_file.Read(kMaxSize); EXPECT_FALSE(read_result.ok()); EXPECT_TRUE(read_result.GetException().Raised(Exception::kIo)); } TEST_F(FileTest, InputFile_GetFilePath) { - InputFile input_file(path_.c_str()); + InputFile input_file(path_, GetSize()); EXPECT_EQ(input_file.GetFilePath(), path_); } TEST_F(FileTest, InputFile_EmptyFileEOF) { - InputFile input_file(path_.c_str()); + InputFile input_file(path_, GetSize()); AssertEmpty(input_file.Read(kMaxSize)); } TEST_F(FileTest, InputFile_ReadWorks) { WriteToFile("abc"); - InputFile input_file(path_.c_str()); + InputFile input_file(path_, GetSize()); input_file.Read(kMaxSize); SUCCEED(); } TEST_F(FileTest, InputFile_ReadUntilEOF) { WriteToFile("abc"); - InputFile input_file(path_.c_str()); + InputFile input_file(path_, GetSize()); AssertEquals(input_file.Read(kMaxSize), "abc"); AssertEmpty(input_file.Read(kMaxSize)); } TEST_F(FileTest, InputFile_ReadWithSize) { WriteToFile("abc"); - InputFile input_file(path_.c_str()); + InputFile input_file(path_, GetSize()); AssertEquals(input_file.Read(2), "ab"); AssertEquals(input_file.Read(1), "c"); AssertEmpty(input_file.Read(kMaxSize)); @@ -111,7 +105,7 @@ TEST_F(FileTest, InputFile_ReadWithSize) { TEST_F(FileTest, InputFile_GetTotalSize) { WriteToFile("abc"); - InputFile input_file(path_.c_str()); + InputFile input_file(path_, GetSize()); EXPECT_EQ(input_file.GetTotalSize(), 3); AssertEquals(input_file.Read(1), "a"); EXPECT_EQ(input_file.GetTotalSize(), 3); @@ -119,7 +113,7 @@ TEST_F(FileTest, InputFile_GetTotalSize) { TEST_F(FileTest, InputFile_Close) { WriteToFile("abc"); - InputFile input_file(path_.c_str()); + InputFile input_file(path_, GetSize()); input_file.Close(); ExceptionOr read_result = input_file.Read(kMaxSize); EXPECT_FALSE(read_result.ok()); @@ -127,23 +121,23 @@ TEST_F(FileTest, InputFile_Close) { } TEST_F(FileTest, OutputFile_NonExistentPath) { - OutputFile output_file((TEST_INVALID_PARENT_FOLDER + TEST_FILE_NAME).c_str()); + OutputFile output_file("/not/a/valid/path.txt"); ByteArray bytes("a", 1); EXPECT_TRUE(output_file.Write(bytes).Raised(Exception::kIo)); } TEST_F(FileTest, OutputFile_Write) { - OutputFile output_file(path_.c_str()); + OutputFile output_file(path_); ByteArray bytes1("a"); ByteArray bytes2("bc"); EXPECT_EQ(output_file.Write(bytes1), Exception{Exception::kSuccess}); EXPECT_EQ(output_file.Write(bytes2), Exception{Exception::kSuccess}); - InputFile input_file(path_.c_str()); + InputFile input_file(path_, GetSize()); AssertEquals(input_file.Read(kMaxSize), "abc"); } TEST_F(FileTest, OutputFile_Close) { - OutputFile output_file(path_.c_str()); + OutputFile output_file(path_); output_file.Close(); ByteArray bytes("a"); EXPECT_EQ(output_file.Write(bytes), Exception{Exception::kIo}); diff --git a/cpp/platform/impl/windows/BUILD b/cpp/platform/impl/windows/BUILD index e6994253..1d436944 100644 --- a/cpp/platform/impl/windows/BUILD +++ b/cpp/platform/impl/windows/BUILD @@ -26,9 +26,11 @@ cc_library( "condition_variable.h", "executor.h", "future.h", + "input_file.h", "listenable_future.h", "log_message.h", "mutex.h", + "output_file.h", "scheduled_executor.h", "settable_future.h", "submittable_executor.h", diff --git a/cpp/platform/impl/windows/executor_test.cc b/cpp/platform/impl/windows/executor_test.cc index 21376833..fd4d6427 100644 --- a/cpp/platform/impl/windows/executor_test.cc +++ b/cpp/platform/impl/windows/executor_test.cc @@ -125,25 +125,20 @@ TEST(ExecutorTests, SingleThreadedExecutorMultipleTasksSucceeds) { // Container to note threads that ran std::unique_ptr> threadIds = std::make_unique>(); - CRITICAL_SECTION crit_sec; - InitializeCriticalSection(&crit_sec); threadIds->push_back(GetCurrentThreadId()); // Act for (int index = 0; index < 5; index++) { - executor->Execute([&output, &threadIds, &crit_sec, index]() { - EnterCriticalSection(&crit_sec); + executor->Execute([&output, &threadIds, index]() { threadIds->push_back(GetCurrentThreadId()); char buffer[128]; snprintf(buffer, sizeof(buffer), "%s%d, ", RUNNABLE_TEXT.c_str(), index); output.append(std::string(buffer)); - LeaveCriticalSection(&crit_sec); }); } executor->Shutdown(); - DeleteCriticalSection(&crit_sec); // Assert // We should've run 1 time on the main thread, and 5 times on the @@ -205,25 +200,19 @@ TEST(ExecutorTests, MultiThreadedExecutorMultipleTasksSucceeds) { std::shared_ptr output = std::make_shared(); - CRITICAL_SECTION crit_sec; - InitializeCriticalSection(&crit_sec); - threadIds->push_back(GetCurrentThreadId()); // Act for (int index = 0; index < 5; index++) { - executor->Execute([&output, &threadIds, &crit_sec, index]() { - EnterCriticalSection(&crit_sec); + executor->Execute([&output, &threadIds, index]() { threadIds->push_back(GetCurrentThreadId()); char buffer[128]; snprintf(buffer, sizeof(buffer), "%s %d, ", RUNNABLE_TEXT.c_str(), index); output->append(std::string(buffer)); - LeaveCriticalSection(&crit_sec); }); } executor->Shutdown(); - DeleteCriticalSection(&crit_sec); // Assert // We should've run 1 time on the main thread, and 5 times on the diff --git a/cpp/platform/impl/windows/input_file.h b/cpp/platform/impl/windows/input_file.h new file mode 100644 index 00000000..1ed39821 --- /dev/null +++ b/cpp/platform/impl/windows/input_file.h @@ -0,0 +1,50 @@ +// Copyright 2020 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef PLATFORM_IMPL_WINDOWS_INPUT_FILE_H_ +#define PLATFORM_IMPL_WINDOWS_INPUT_FILE_H_ + +#include "platform/api/input_file.h" +#include "platform/base/byte_array.h" +#include "platform/base/exception.h" + +namespace location { +namespace nearby { +namespace windows { + +// An InputFile represents a readable file on the system. +class InputFile : public api::InputFile { + public: + // TODO(b/184975123): replace with real implementation. + ~InputFile() override = default; + // TODO(b/184975123): replace with real implementation. + std::string GetFilePath() const override { return "Un-implemented"; } + // TODO(b/184975123): replace with real implementation. + std::int64_t GetTotalSize() const override { return 0; } + + // throws Exception::kIo + // TODO(b/184975123): replace with real implementation. + ExceptionOr Read(std::int64_t size) override { + return ExceptionOr(Exception::kFailed); + } + // throws Exception::kIo + // TODO(b/184975123): replace with real implementation. + Exception Close() override { return Exception{}; } +}; + +} // namespace windows +} // namespace nearby +} // namespace location + +#endif // PLATFORM_IMPL_WINDOWS_INPUT_FILE_H_ diff --git a/cpp/platform/impl/windows/input_file_test.cc b/cpp/platform/impl/windows/input_file_test.cc index 1a38fe93..eb2bf811 100644 --- a/cpp/platform/impl/windows/input_file_test.cc +++ b/cpp/platform/impl/windows/input_file_test.cc @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include "platform/impl/windows/input_file.h" + #include "gtest/gtest.h" #include "platform/base/exception.h" #include "platform/base/payload_id.h" @@ -22,18 +24,21 @@ class InputFileTests : public testing::Test { protected: // You can define per-test set-up logic as usual. void SetUp() override { - hFile_ = CreateFileA(TEST_FILE_PATH.c_str(), // name of the write - GENERIC_WRITE, // open for writing - 0, // do not share - NULL, // default security - CREATE_ALWAYS, // create new file only - FILE_ATTRIBUTE_NORMAL, // normal file - NULL); // no attr. template + location::nearby::PayloadId payloadId(TEST_PAYLOAD_ID); + + hFile_ = CreateFileA( + test_utils::GetPayloadPath(payloadId).c_str(), // name of the write + GENERIC_WRITE, // open for writing + 0, // do not share + NULL, // default security + CREATE_ALWAYS, // create new file only + FILE_ATTRIBUTE_NORMAL, // normal file + NULL); // no attr. template if (hFile_ == INVALID_HANDLE_VALUE) { NEARBY_LOG(ERROR, - "Failed to create OutputFile with file path: %s and error: %d", - TEST_FILE_PATH.c_str(), GetLastError()); + "Failed to create OutputFile with payloadId: %s and error: %d", + test_utils::GetPayloadPath(payloadId).c_str(), GetLastError()); } const char* buffer = TEST_STRING; @@ -47,8 +52,9 @@ class InputFileTests : public testing::Test { // You can define per-test tear-down logic as usual. void TearDown() override { - if (FileExists(TEST_FILE_PATH.c_str())) { - DeleteFileA(TEST_FILE_PATH.c_str()); + location::nearby::PayloadId payloadId(TEST_PAYLOAD_ID); + if (FileExists(test_utils::GetPayloadPath(payloadId).c_str())) { + DeleteFileA(test_utils::GetPayloadPath(payloadId).c_str()); } } @@ -68,7 +74,7 @@ TEST_F(InputFileTests, SuccessfulCreation) { std::unique_ptr inputFile = nullptr; inputFile = location::nearby::api::ImplementationPlatform::CreateInputFile( - TEST_FILE_PATH.c_str()); + payloadId, strlen(TEST_STRING)); EXPECT_NE(inputFile, nullptr); EXPECT_EQ(inputFile->Close(), @@ -76,27 +82,28 @@ TEST_F(InputFileTests, SuccessfulCreation) { } TEST_F(InputFileTests, SuccessfulGetFilePath) { + location::nearby::PayloadId payloadId(TEST_PAYLOAD_ID); std::unique_ptr inputFile = nullptr; std::string fileName; - std::string expected(TEST_FILE_PATH.c_str()); inputFile = location::nearby::api::ImplementationPlatform::CreateInputFile( - TEST_FILE_PATH.c_str()); + payloadId, strlen(TEST_STRING)); fileName = inputFile->GetFilePath(); EXPECT_EQ(inputFile->Close(), location::nearby::Exception{location::nearby::Exception::kSuccess}); - EXPECT_EQ(fileName, expected); + EXPECT_EQ(fileName, test_utils::GetPayloadPath(payloadId).c_str()); } TEST_F(InputFileTests, SuccessfulGetTotalSize) { + location::nearby::PayloadId payloadId(TEST_PAYLOAD_ID); std::unique_ptr inputFile = nullptr; int64_t size = -1; inputFile = location::nearby::api::ImplementationPlatform::CreateInputFile( - TEST_FILE_PATH.c_str()); + payloadId, strlen(TEST_STRING)); size = inputFile->GetTotalSize(); @@ -107,10 +114,11 @@ TEST_F(InputFileTests, SuccessfulGetTotalSize) { } TEST_F(InputFileTests, SuccessfulRead) { + location::nearby::PayloadId payloadId(TEST_PAYLOAD_ID); std::unique_ptr inputFile = nullptr; inputFile = location::nearby::api::ImplementationPlatform::CreateInputFile( - TEST_FILE_PATH.c_str()); + payloadId, strlen(TEST_STRING)); auto fileSize = inputFile->GetTotalSize(); auto dataRead = inputFile->Read(fileSize); @@ -123,10 +131,11 @@ TEST_F(InputFileTests, SuccessfulRead) { } TEST_F(InputFileTests, FailedRead) { + location::nearby::PayloadId payloadId(TEST_PAYLOAD_ID); std::unique_ptr inputFile = nullptr; inputFile = location::nearby::api::ImplementationPlatform::CreateInputFile( - TEST_FILE_PATH.c_str()); + payloadId, strlen(TEST_STRING)); auto fileSize = inputFile->GetTotalSize(); EXPECT_NE(fileSize, -1); diff --git a/cpp/platform/impl/windows/output_file.h b/cpp/platform/impl/windows/output_file.h new file mode 100644 index 00000000..9ad479b6 --- /dev/null +++ b/cpp/platform/impl/windows/output_file.h @@ -0,0 +1,47 @@ +// Copyright 2020 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef PLATFORM_IMPL_WINDOWS_OUTPUT_FILE_H_ +#define PLATFORM_IMPL_WINDOWS_OUTPUT_FILE_H_ + +#include "platform/api/output_file.h" +#include "platform/base/byte_array.h" +#include "platform/base/exception.h" + +namespace location { +namespace nearby { +namespace windows { + +// An OutputFile represents a writable file on the system. +class OutputFile : public api::OutputFile { + public: + // TODO(b/184975123): replace with real implementation. + ~OutputFile() override = default; + + // throws Exception::kIo + // TODO(b/184975123): replace with real implementation. + Exception Write(const ByteArray& data) override { return Exception{}; } + // throws Exception::kIo + // TODO(b/184975123): replace with real implementation. + Exception Flush() override { return Exception{}; } + // throws Exception::kIo + // TODO(b/184975123): replace with real implementation. + Exception Close() override { return Exception{}; } +}; + +} // namespace windows +} // namespace nearby +} // namespace location + +#endif // PLATFORM_IMPL_WINDOWS_OUTPUT_FILE_H_ diff --git a/cpp/platform/impl/windows/output_file_test.cc b/cpp/platform/impl/windows/output_file_test.cc index f089f719..18e47f93 100644 --- a/cpp/platform/impl/windows/output_file_test.cc +++ b/cpp/platform/impl/windows/output_file_test.cc @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include "platform/impl/windows/output_file.h" + #include "gtest/gtest.h" #include "platform/api/platform.h" #include "platform/base/exception.h" @@ -22,15 +24,17 @@ class OutputFileTests : public testing::Test { protected: // You can define per-test set-up logic as usual. void SetUp() override { - if (FileExists(TEST_FILE_PATH.c_str())) { - DeleteFileA(TEST_FILE_PATH.c_str()); + location::nearby::PayloadId payloadId(TEST_PAYLOAD_ID); + if (FileExists(test_utils::GetPayloadPath(payloadId).c_str())) { + DeleteFileA(test_utils::GetPayloadPath(payloadId).c_str()); } } // You can define per-test tear-down logic as usual. void TearDown() override { - if (FileExists(TEST_FILE_PATH.c_str())) { - DeleteFileA(TEST_FILE_PATH.c_str()); + location::nearby::PayloadId payloadId(TEST_PAYLOAD_ID); + if (FileExists(test_utils::GetPayloadPath(payloadId).c_str())) { + DeleteFileA(test_utils::GetPayloadPath(payloadId).c_str()); } } @@ -43,41 +47,44 @@ class OutputFileTests : public testing::Test { }; TEST_F(OutputFileTests, SuccessfulCreation) { + location::nearby::PayloadId payloadId(TEST_PAYLOAD_ID); std::unique_ptr outputFile = nullptr; EXPECT_NO_THROW( outputFile = location::nearby::api::ImplementationPlatform::CreateOutputFile( - TEST_FILE_PATH.c_str())); + payloadId)); EXPECT_NE(outputFile, nullptr); EXPECT_NO_THROW(outputFile->Close()); } TEST_F(OutputFileTests, SuccessfulClose) { + location::nearby::PayloadId payloadId(TEST_PAYLOAD_ID); std::unique_ptr outputFile = nullptr; EXPECT_NO_THROW( outputFile = location::nearby::api::ImplementationPlatform::CreateOutputFile( - TEST_FILE_PATH.c_str())); + payloadId)); EXPECT_NO_THROW(outputFile->Close()); - DeleteFileA(TEST_FILE_PATH.c_str()); + DeleteFileA(test_utils::GetPayloadPath(payloadId).c_str()); } TEST_F(OutputFileTests, SuccessfulWrite) { + location::nearby::PayloadId payloadId(TEST_PAYLOAD_ID); location::nearby::ByteArray data(std::string(TEST_STRING)); std::unique_ptr outputFile = nullptr; EXPECT_NO_THROW( outputFile = location::nearby::api::ImplementationPlatform::CreateOutputFile( - TEST_FILE_PATH.c_str())); + payloadId)); EXPECT_NO_THROW(outputFile->Write(data)); EXPECT_NO_THROW(outputFile->Close()); - DeleteFileA(TEST_FILE_PATH.c_str()); + DeleteFileA(test_utils::GetPayloadPath(payloadId).c_str()); } diff --git a/cpp/platform/impl/windows/platform.cc b/cpp/platform/impl/windows/platform.cc index d50b757c..5d117cf6 100644 --- a/cpp/platform/impl/windows/platform.cc +++ b/cpp/platform/impl/windows/platform.cc @@ -41,9 +41,9 @@ namespace location { namespace nearby { namespace api { +namespace { -std::unique_ptr ImplementationPlatform::GetDownloadPath( - std::unique_ptr path) { +std::string GetPayloadPath(PayloadId payload_id) { PWSTR basePath; // Retrieves the full path of a known folder identified by the folder's @@ -61,12 +61,13 @@ std::unique_ptr ImplementationPlatform::GetDownloadPath( // is no longer needed by calling CoTaskMemFree, whether // SHGetKnownFolderPath succeeds or not. - auto basePathLength = (wcslen(basePath) + 1) * sizeof(char); - char* fullpathUTF8 = new char(basePathLength); - wcstombs(fullpathUTF8, basePath, basePathLength); - return std::make_unique(std::string(fullpathUTF8) += - "/" + *path); + char* fullpathUTF8 = new char((wcslen(basePath) + 1) * sizeof(char)); + wcstombs(fullpathUTF8, basePath, (wcslen(basePath) + 1) * sizeof(char)); + std::string fullPath = std::string(fullpathUTF8); + auto retval = absl::StrCat(fullPath += "/", payload_id); + return retval; } +} // namespace std::unique_ptr ImplementationPlatform::CreateAtomicBoolean( bool initial_value) { @@ -93,13 +94,14 @@ ImplementationPlatform::CreateConditionVariable(Mutex* mutex) { } std::unique_ptr ImplementationPlatform::CreateInputFile( - const char* file_path) { - return absl::make_unique(file_path); + PayloadId payload_id, std::int64_t total_size) { + return absl::make_unique(GetPayloadPath(payload_id), + total_size); } std::unique_ptr ImplementationPlatform::CreateOutputFile( - const char* file_path) { - return absl::make_unique(file_path); + PayloadId payload_id) { + return absl::make_unique(GetPayloadPath(payload_id)); } // TODO(b/184975123): replace with real implementation. diff --git a/cpp/platform/impl/windows/scheduled_executor_test.cc b/cpp/platform/impl/windows/scheduled_executor_test.cc index e62ee71d..4a52c207 100644 --- a/cpp/platform/impl/windows/scheduled_executor_test.cc +++ b/cpp/platform/impl/windows/scheduled_executor_test.cc @@ -31,21 +31,15 @@ TEST(ScheduledExecutorTests, ExecuteSucceeds) { std::unique_ptr> threadIds = std::make_unique>(); - CRITICAL_SECTION crit_sec; - InitializeCriticalSection(&crit_sec); - threadIds->push_back(GetCurrentThreadId()); // Act - submittableExecutor->Execute([&output, &threadIds, &crit_sec]() { - EnterCriticalSection(&crit_sec); + submittableExecutor->Execute([&output, &threadIds]() { threadIds->push_back(GetCurrentThreadId()); output.append(RUNNABLE_0_TEXT.c_str()); - LeaveCriticalSection(&crit_sec); }); submittableExecutor->Shutdown(); - DeleteCriticalSection(&crit_sec); // Assert // We should've run 1 time on the main thread, and 1 times on the @@ -69,9 +63,6 @@ TEST(ScheduledExecutorTests, ScheduleSucceeds) { std::unique_ptr> threadIds = std::make_unique>(); - CRITICAL_SECTION crit_sec; - InitializeCriticalSection(&crit_sec); - threadIds->push_back(GetCurrentThreadId()); std::chrono::system_clock::time_point timeNow = @@ -80,19 +71,16 @@ TEST(ScheduledExecutorTests, ScheduleSucceeds) { // Act submittableExecutor->Schedule( - [&output, &threadIds, &timeExecuted, &crit_sec]() { - EnterCriticalSection(&crit_sec); + [&output, &threadIds, &timeExecuted]() { timeExecuted = std::chrono::system_clock::now(); threadIds->push_back(GetCurrentThreadId()); output.append(RUNNABLE_0_TEXT.c_str()); - LeaveCriticalSection(&crit_sec); }, absl::Milliseconds(50)); SleepEx(100, true); // Yield the thread submittableExecutor->Shutdown(); - DeleteCriticalSection(&crit_sec); auto difference = std::chrono::duration_cast( timeExecuted - timeNow) @@ -160,18 +148,13 @@ TEST(ScheduledExecutorTests, CancelAfterStartedFails) { std::unique_ptr> threadIds = std::make_unique>(); - CRITICAL_SECTION crit_sec; - InitializeCriticalSection(&crit_sec); - threadIds->push_back(GetCurrentThreadId()); // Act auto cancelable = submittableExecutor->Schedule( - [&output, &threadIds, &crit_sec]() { - EnterCriticalSection(&crit_sec); + [&output, &threadIds]() { threadIds->push_back(GetCurrentThreadId()); output.append(RUNNABLE_0_TEXT.c_str()); - LeaveCriticalSection(&crit_sec); }, absl::Milliseconds(100)); @@ -180,7 +163,6 @@ TEST(ScheduledExecutorTests, CancelAfterStartedFails) { auto actual = cancelable->Cancel(); submittableExecutor->Shutdown(); - DeleteCriticalSection(&crit_sec); // Assert ASSERT_FALSE(actual); diff --git a/cpp/platform/impl/windows/submittable_executor_test.cc b/cpp/platform/impl/windows/submittable_executor_test.cc index 471bcd3c..779f5731 100644 --- a/cpp/platform/impl/windows/submittable_executor_test.cc +++ b/cpp/platform/impl/windows/submittable_executor_test.cc @@ -164,25 +164,19 @@ TEST(SubmittableExecutorTests, SingleThreadedExecuteMultipleTasksSucceeds) { std::unique_ptr> threadIds = std::make_unique>(); - CRITICAL_SECTION crit_sec; - InitializeCriticalSection(&crit_sec); - threadIds->push_back(GetCurrentThreadId()); // Act for (int index = 0; index < 5; index++) { - submittableExecutor->Execute([&output, &threadIds, &crit_sec, index]() { - EnterCriticalSection(&crit_sec); + submittableExecutor->Execute([&output, &threadIds, index]() { threadIds->push_back(GetCurrentThreadId()); char buffer[128]; snprintf(buffer, sizeof(buffer), "%s%d, ", RUNNABLE_TEXT.c_str(), index); output->append(std::string(buffer)); - LeaveCriticalSection(&crit_sec); }); } submittableExecutor->Shutdown(); - DeleteCriticalSection(&crit_sec); // Assert // We should've run 1 time on the main thread, and 5 times on the @@ -212,27 +206,20 @@ TEST(SubmittableExecutorTests, SingleThreadedDoSubmitMultipleTasksSucceeds) { std::unique_ptr> threadIds = std::make_unique>(); - CRITICAL_SECTION crit_sec; - InitializeCriticalSection(&crit_sec); - threadIds->push_back(GetCurrentThreadId()); // Act bool result = true; for (int index = 0; index < 5; index++) { - result &= submittableExecutor->DoSubmit([&output, &threadIds, &crit_sec, - index]() { - EnterCriticalSection(&crit_sec); + result &= submittableExecutor->DoSubmit([&output, &threadIds, index]() { threadIds->push_back(GetCurrentThreadId()); char buffer[128]; snprintf(buffer, sizeof(buffer), "%s%d, ", RUNNABLE_TEXT.c_str(), index); output->append(std::string(buffer)); - LeaveCriticalSection(&crit_sec); }); } submittableExecutor->Shutdown(); - DeleteCriticalSection(&crit_sec); // Assert // All of these should have submitted diff --git a/cpp/platform/impl/windows/test_utils.cc b/cpp/platform/impl/windows/test_utils.cc index 703fa690..9bd73b5e 100644 --- a/cpp/platform/impl/windows/test_utils.cc +++ b/cpp/platform/impl/windows/test_utils.cc @@ -19,14 +19,39 @@ #include "absl/strings/str_cat.h" namespace test_utils { -std::wstring StringToWideString(const std::string &s) { +std::wstring StringToWideString(const std::string& s) { int len; int slength = (int)s.length() + 1; len = MultiByteToWideChar(CP_ACP, 0, s.c_str(), slength, 0, 0); - wchar_t *buf = new wchar_t[len]; + wchar_t* buf = new wchar_t[len]; MultiByteToWideChar(CP_ACP, 0, s.c_str(), slength, buf, len); std::wstring r(buf); delete[] buf; return r; } + +std::string GetPayloadPath(location::nearby::PayloadId payload_id) { + PWSTR basePath; + + // Retrieves the full path of a known folder identified by the folder's + // KNOWNFOLDERID. + // https://docs.microsoft.com/en-us/windows/win32/api/shlobj_core/nf-shlobj_core-shgetknownfolderpath + SHGetKnownFolderPath( + FOLDERID_Downloads, // rfid: A reference to the KNOWNFOLDERID that + // identifies the folder. + 0, // dwFlags: Flags that specify special retrieval options. + NULL, // hToken: An access token that represents a particular user. + &basePath); // ppszPath: When this method returns, contains the address + // of a pointer to a null-terminated Unicode string that + // specifies the path of the known folder. The calling + // process is responsible for freeing this resource once it + // is no longer needed by calling CoTaskMemFree, whether + // SHGetKnownFolderPath succeeds or not. + + char* fullpathUTF8 = new char((wcslen(basePath) + 1) * sizeof(char)); + wcstombs(fullpathUTF8, basePath, (wcslen(basePath) + 1) * sizeof(char)); + std::string fullPath = std::string(fullpathUTF8); + auto retval = absl::StrCat(fullPath += "/", payload_id); + return retval; +} } // namespace test_utils diff --git a/cpp/platform/impl/windows/test_utils.h b/cpp/platform/impl/windows/test_utils.h index 6ae3a52c..6e5f89b8 100644 --- a/cpp/platform/impl/windows/test_utils.h +++ b/cpp/platform/impl/windows/test_utils.h @@ -17,6 +17,8 @@ #include +#include "platform/base/payload_id.h" + #define TEST_BUFFER_SIZE 256 #define TEST_PAYLOAD_ID 64l #define TEST_STRING \ @@ -33,21 +35,9 @@ "eu tellus. Cras feugiat ornare vestibulum. Nullam at ipsum vestibulum " \ "sapien luctus dictum ac vel ligula." -#define TEST_FILE_PATH std::string("testfilename.txt") - namespace test_utils { std::wstring StringToWideString(const std::string& s); - -class TempPath { - public: - enum Location { - Local, // Some local directory, works for unittest and on borglets. - CNSTest, // Creates a directory on the cns test cell. - }; - TempPath(Location location) : location_(location) {} - Location location_; - const std::string path() { return ""; } -}; +std::string GetPayloadPath(location::nearby::PayloadId payload_id); } // namespace test_utils #endif // PLATFORM_IMPL_WINDOWS_TEST_UTILS_H_ diff --git a/cpp/platform/public/BUILD b/cpp/platform/public/BUILD index 4648ac67..086645b1 100644 --- a/cpp/platform/public/BUILD +++ b/cpp/platform/public/BUILD @@ -29,6 +29,7 @@ cc_library( "cancelable_alarm.h", "cancellable_task.h", "condition_variable.h", + "core_config.h", "count_down_latch.h", "crypto.h", "file.h", diff --git a/cpp/platform/public/file.cc b/cpp/platform/public/file.cc index 46508f7e..4ea9bd99 100644 --- a/cpp/platform/public/file.cc +++ b/cpp/platform/public/file.cc @@ -17,19 +17,16 @@ namespace location { namespace nearby { -InputFile::InputFile(const char* file_path) - : impl_(Platform::CreateInputFile(file_path)) {} - +InputFile::InputFile(PayloadId payload_id, std::int64_t size) + : impl_(Platform::CreateInputFile(payload_id, size)), id_(payload_id) {} InputFile::~InputFile() = default; -InputFile::InputFile(InputFile&& other) noexcept { - impl_ = std::move(other.impl_); -} +InputFile::InputFile(InputFile&&) noexcept = default; InputFile& InputFile::operator=(InputFile&&) noexcept = default; // Reads up to size bytes and returns as a ByteArray object wrapped by // ExceptionOr. // Returns Exception::kIo on error, or end of file. -ExceptionOr InputFile::Read(std::int64_t size) const { +ExceptionOr InputFile::Read(std::int64_t size) { return impl_->Read(size); } @@ -39,13 +36,13 @@ std::string InputFile::GetFilePath() const { return impl_->GetFilePath(); } // Returns total size of this file in bytes. std::int64_t InputFile::GetTotalSize() const { return impl_->GetTotalSize(); } -ExceptionOr InputFile::Skip(size_t offset) const { +ExceptionOr InputFile::Skip(size_t offset) { return impl_->Skip(offset); } // Disallows further reads from the file and frees system resources, // associated with it. -Exception InputFile::Close() const { return impl_->Close(); } +Exception InputFile::Close() { return impl_->Close(); } // Returns a handle to the underlying input stream. // @@ -54,10 +51,13 @@ Exception InputFile::Close() const { return impl_->Close(); } // Side effects of any non-const operation invoked for InputFile (such as // Read, or Close will be observable through InputStream& handle, and vice // versa. -const InputStream& InputFile::GetInputStream() const { return *impl_; } +InputStream& InputFile::GetInputStream() { return *impl_; } -OutputFile::OutputFile(const char* file_path) - : impl_(Platform::CreateOutputFile(file_path)) {} +// Returns payload id of this file. The closest "file" equivalent is inode. +PayloadId InputFile::GetPayloadId() const { return id_; } + +OutputFile::OutputFile(PayloadId payload_id) + : impl_(Platform::CreateOutputFile(payload_id)), id_(payload_id) {} OutputFile::~OutputFile() = default; OutputFile::OutputFile(OutputFile&&) noexcept = default; OutputFile& OutputFile::operator=(OutputFile&&) noexcept = default; @@ -85,5 +85,8 @@ Exception OutputFile::Close() { return impl_->Close(); } // versa. OutputStream& OutputFile::GetOutputStream() { return *impl_; } +// Returns payload id of this file. The closest "file" equivalent is inode. +PayloadId OutputFile::GetPayloadId() const { return id_; } + } // namespace nearby } // namespace location diff --git a/cpp/platform/public/file.h b/cpp/platform/public/file.h index 638e9ae0..80e1ce58 100644 --- a/cpp/platform/public/file.h +++ b/cpp/platform/public/file.h @@ -23,10 +23,10 @@ #include "platform/api/output_file.h" #include "platform/api/platform.h" #include "platform/base/byte_array.h" -#include "platform/base/core_config.h" #include "platform/base/exception.h" #include "platform/base/input_stream.h" #include "platform/base/output_stream.h" +#include "platform/public/core_config.h" namespace location { namespace nearby { @@ -34,7 +34,7 @@ namespace nearby { class DLL_API InputFile final { public: using Platform = api::ImplementationPlatform; - InputFile(const char* file_path); + InputFile(PayloadId payload_id, std::int64_t size); ~InputFile(); InputFile(InputFile&&) noexcept; InputFile& operator=(InputFile&&) noexcept; @@ -42,7 +42,7 @@ class DLL_API InputFile final { // Reads up to size bytes and returns as a ByteArray object wrapped by // ExceptionOr. // Returns Exception::kIo on error, or end of file. - ExceptionOr Read(std::int64_t size) const; + ExceptionOr Read(std::int64_t size); // Returns a string that uniqely identifies this file. std::string GetFilePath() const; @@ -50,11 +50,11 @@ class DLL_API InputFile final { // Returns total size of this file in bytes. std::int64_t GetTotalSize() const; - ExceptionOr Skip(size_t offset) const; + ExceptionOr Skip(size_t offset); // Disallows further reads from the file and frees system resources, // associated with it. - Exception Close() const; + Exception Close(); // Returns a handle to the underlying input stream. // @@ -63,16 +63,20 @@ class DLL_API InputFile final { // Side effects of any non-const operation invoked for InputFile (such as // Read, or Close will be observable through InputStream& handle, and vice // versa. - const InputStream& GetInputStream() const; + InputStream& GetInputStream(); + + // Returns payload id of this file. The closest "file" equivalent is inode. + PayloadId GetPayloadId() const; private: std::unique_ptr impl_; + PayloadId id_; }; class DLL_API OutputFile final { public: using Platform = api::ImplementationPlatform; - explicit OutputFile(const char* file_path); + explicit OutputFile(PayloadId payload_id); ~OutputFile(); OutputFile(OutputFile&&) noexcept; OutputFile& operator=(OutputFile&&) noexcept; @@ -98,8 +102,12 @@ class DLL_API OutputFile final { // versa. OutputStream& GetOutputStream(); + // Returns payload id of this file. The closest "file" equivalent is inode. + PayloadId GetPayloadId() const; + private: std::unique_ptr impl_; + PayloadId id_; }; } // namespace nearby diff --git a/windows/advertising_options.cc b/windows/advertising_options.cc index 81b3dc79..d65a6bd5 100644 --- a/windows/advertising_options.cc +++ b/windows/advertising_options.cc @@ -17,7 +17,7 @@ #include "core/options.h" #include "core/strategy.h" #include "platform/base/byte_array.h" -#include "platform/base/core_config.h" +#include "platform/public/core_config.h" namespace location { namespace nearby { diff --git a/windows/advertising_options.h b/windows/advertising_options.h index 7362ea06..80b54e13 100644 --- a/windows/advertising_options.h +++ b/windows/advertising_options.h @@ -19,7 +19,7 @@ #include -#include "platform/base/core_config.h" +#include "platform/public/core_config.h" namespace location { namespace nearby { diff --git a/windows/connection_options.cc b/windows/connection_options.cc index 8dc49de4..7d299454 100644 --- a/windows/connection_options.cc +++ b/windows/connection_options.cc @@ -17,7 +17,7 @@ #include "core/options.h" #include "core/strategy.h" #include "platform/base/byte_array.h" -#include "platform/base/core_config.h" +#include "platform/public/core_config.h" namespace location { namespace nearby { diff --git a/windows/connection_options.h b/windows/connection_options.h index a5e6830a..6d7f0cfb 100644 --- a/windows/connection_options.h +++ b/windows/connection_options.h @@ -19,7 +19,7 @@ #include -#include "platform/base/core_config.h" +#include "platform/public/core_config.h" namespace location { namespace nearby { diff --git a/windows/discovery_options.cc b/windows/discovery_options.cc index 10de6596..caf1460c 100644 --- a/windows/discovery_options.cc +++ b/windows/discovery_options.cc @@ -17,7 +17,7 @@ #include "core/options.h" #include "core/strategy.h" #include "platform/base/byte_array.h" -#include "platform/base/core_config.h" +#include "platform/public/core_config.h" namespace location { namespace nearby { diff --git a/windows/discovery_options.h b/windows/discovery_options.h index 01d7f307..a22e9836 100644 --- a/windows/discovery_options.h +++ b/windows/discovery_options.h @@ -19,7 +19,7 @@ #include -#include "platform/base/core_config.h" +#include "platform/public/core_config.h" namespace location { namespace nearby {