diff --git a/cpp/core/BUILD b/cpp/core/BUILD index 4588a291..81f1f0fc 100644 --- a/cpp/core/BUILD +++ b/cpp/core/BUILD @@ -101,6 +101,7 @@ 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 beb85969..cede3366 100644 --- a/cpp/core/core.cc +++ b/cpp/core/core.cc @@ -20,6 +20,7 @@ #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" @@ -130,6 +131,12 @@ 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::Validate(payload.GetFileName(), + parser::ILLEGAL_FILENAME_PATTERNS)); + assert(parser::Validate(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 9247f02e..5d1a309e 100644 --- a/cpp/core/internal/BUILD +++ b/cpp/core/internal/BUILD @@ -190,6 +190,7 @@ 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 3a9b4d14..b08a10b4 100644 --- a/cpp/core/internal/base_pcp_handler.cc +++ b/cpp/core/internal/base_pcp_handler.cc @@ -1478,6 +1478,7 @@ void BasePcpHandler::LogConnectionAttemptFailure( } } +// TODO(jfcarroll): FIXME!!!! void BasePcpHandler::LogConnectionAttemptSuccess( const std::string& endpoint_id, const PendingConnectionInfo& connection_info) { @@ -1497,22 +1498,26 @@ void BasePcpHandler::LogConnectionAttemptSuccess( "LogConnectionAttemptSuccess. Bail out."); return; } - 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()); - } + // 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()); + //} } bool BasePcpHandler::Cancelled(ClientProxy* client, diff --git a/cpp/core/internal/internal_payload_factory.cc b/cpp/core/internal/internal_payload_factory.cc index 8895992d..89563ba9 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 { - InputFile* file = payload_.AsFile(); + const 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; - InputFile* file = payload_.AsFile(); + const InputFile* file = payload_.AsFile(); if (!file) { return {Exception::kIo}; } @@ -236,7 +236,7 @@ class OutgoingFileInternalPayload : public InternalPayload { } void Close() override { - InputFile* file = payload_.AsFile(); + const InputFile* file = payload_.AsFile(); if (file) file->Close(); } @@ -292,10 +292,6 @@ 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)); } @@ -309,6 +305,22 @@ 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(std::to_string(id)); +} + std::unique_ptr CreateIncomingInternalPayload( const PayloadTransferFrame& frame) { if (frame.packet_type() != PayloadTransferFrame::DATA) { @@ -334,11 +346,39 @@ std::unique_ptr CreateIncomingInternalPayload( } case PayloadTransferFrame::PayloadHeader::FILE: { - std::int64_t total_size = frame.payload_header().total_size(); + 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()); + return absl::make_unique( - Payload(payload_id, InputFile(payload_id, total_size)), - OutputFile(payload_id), total_size); + Payload(payload_id, std::move(inputFile)), std::move(outputFile), + frame.payload_header().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 adfbe9b2..90acea1e 100644 --- a/cpp/core/internal/internal_payload_factory_test.cc +++ b/cpp/core/internal/internal_payload_factory_test.cc @@ -14,9 +14,13 @@ #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" @@ -30,8 +34,28 @@ namespace connections { namespace { constexpr char kText[] = "data chunk"; +#define TEST_FILE_NAME std::string("testfilename.txt") +#define TEST_FILE_PARENT_FOLDER std::string("") -TEST(InternalPayloadFActoryTest, CanCreateIternalPayloadFromBytePayload) { +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) { ByteArray data(kText); std::unique_ptr internal_payload = CreateOutgoingInternalPayload(Payload{data}); @@ -42,7 +66,7 @@ TEST(InternalPayloadFActoryTest, CanCreateIternalPayloadFromBytePayload) { EXPECT_EQ(payload.AsBytes(), ByteArray(kText)); } -TEST(InternalPayloadFActoryTest, CanCreateIternalPayloadFromStreamPayload) { +TEST_F(InternalPayloadFActoryTest, CanCreateIternalPayloadFromStreamPayload) { auto pipe = std::make_shared(); std::unique_ptr internal_payload = CreateOutgoingInternalPayload(Payload{[pipe]() -> InputStream& { @@ -55,21 +79,21 @@ TEST(InternalPayloadFActoryTest, CanCreateIternalPayloadFromStreamPayload) { EXPECT_EQ(payload.AsBytes(), ByteArray()); } -TEST(InternalPayloadFActoryTest, CanCreateIternalPayloadFromFilePayload) { - Payload::Id payload_id = Payload::GenerateId(); +TEST_F(InternalPayloadFActoryTest, CanCreateIternalPayloadFromFilePayload) { std::unique_ptr internal_payload = - CreateOutgoingInternalPayload( - Payload{payload_id, InputFile(payload_id, 512)}); + CreateOutgoingInternalPayload(Payload{ + path_.c_str(), TEST_FILE_NAME.c_str(), InputFile(path_.c_str())}); 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.GetId(), payload_id); - EXPECT_EQ(payload.AsFile()->GetPayloadId(), payload_id); + EXPECT_EQ(payload.AsFile()->GetFilePath(), path_); + payload.AsFile()->Close(); + std::filesystem::remove(path_); } -TEST(InternalPayloadFActoryTest, CanCreateIternalPayloadFromByteMessage) { +TEST_F(InternalPayloadFActoryTest, CanCreateIternalPayloadFromByteMessage) { PayloadTransferFrame frame; frame.set_packet_type(PayloadTransferFrame::DATA); std::int64_t payload_chunk_offset = 0; @@ -92,7 +116,7 @@ TEST(InternalPayloadFActoryTest, CanCreateIternalPayloadFromByteMessage) { EXPECT_EQ(payload.AsBytes(), ByteArray(kText)); } -TEST(InternalPayloadFActoryTest, CanCreateIternalPayloadFromStreamMessage) { +TEST_F(InternalPayloadFActoryTest, CanCreateIternalPayloadFromStreamMessage) { PayloadTransferFrame frame; frame.set_packet_type(PayloadTransferFrame::DATA); auto& header = *frame.mutable_payload_header(); @@ -109,7 +133,7 @@ TEST(InternalPayloadFActoryTest, CanCreateIternalPayloadFromStreamMessage) { EXPECT_EQ(payload.GetType(), Payload::Type::kStream); } -TEST(InternalPayloadFActoryTest, CanCreateIternalPayloadFromFileMessage) { +TEST_F(InternalPayloadFActoryTest, CanCreateIternalPayloadFromFileMessage) { PayloadTransferFrame frame; frame.set_packet_type(PayloadTransferFrame::DATA); auto& header = *frame.mutable_payload_header(); @@ -124,25 +148,32 @@ TEST(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(Payload::Id payload_id, const ByteArray& contents) { - OutputFile file(payload_id); - EXPECT_TRUE(file.Write(contents).Ok()); - EXPECT_TRUE(file.Close().Ok()); +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()); } -TEST(InternalPayloadFActoryTest, - SkipToOffset_FilePayloadValidOffset_SkipsOffset) { +TEST_F(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(); - CreateFileWithContents(payload_id, contents); + std::unique_ptr inputFile = + std::make_unique(path_.c_str()); std::unique_ptr internal_payload = - CreateOutgoingInternalPayload( - Payload{payload_id, InputFile(payload_id, contents.size())}); + CreateOutgoingInternalPayload(Payload{payload_id, std::move(*inputFile)}); EXPECT_NE(internal_payload, nullptr); ExceptionOr result = internal_payload->SkipToOffset(kOffset); @@ -153,10 +184,12 @@ TEST(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(InternalPayloadFActoryTest, - SkipToOffset_StreamPayloadValidOffset_SkipsOffset) { +TEST_F(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 614c65f8..83e11504 100644 --- a/cpp/core/internal/offline_frames_validator.cc +++ b/cpp/core/internal/offline_frames_validator.cc @@ -24,6 +24,13 @@ namespace location { namespace nearby { namespace connections { namespace parser { +bool Validate(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; @@ -120,6 +127,25 @@ 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 (!Validate(frame.payload_header().file_name(), + ILLEGAL_FILENAME_PATTERNS)) { + return {Exception::kFailed}; + } + } + + if (frame.payload_header().has_parent_folder()) { + if (!Validate(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 f64c32c3..019add63 100644 --- a/cpp/core/internal/offline_frames_validator.h +++ b/cpp/core/internal/offline_frames_validator.h @@ -23,8 +23,19 @@ 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 Validate(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 403be7e0..58d457aa 100644 --- a/cpp/core/internal/payload_manager.cc +++ b/cpp/core/internal/payload_manager.cc @@ -259,6 +259,7 @@ 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, @@ -354,6 +355,7 @@ 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(); @@ -392,11 +394,15 @@ 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]() { + "send-payload", + [this, client, endpoint_ids, payload_id, payload_type, resume_offset, + payload_total_size, file_name, parent_folder]() { if (shutdown_.Get()) return; PendingPayload* pending_payload = GetPayload(payload_id); if (!pending_payload) { @@ -417,10 +423,12 @@ void PayloadManager::SendPayload(ClientProxy* client, payload_type, resume_offset, internal_payload->GetTotalSize()); - PayloadTransferFrame::PayloadHeader payload_header{ - CreatePayloadHeader(*internal_payload, resume_offset)}; + NEARBY_LOG(INFO, "Creating payload header (JFC)"); + PayloadTransferFrame::PayloadHeader payload_header{CreatePayloadHeader( + *internal_payload, resume_offset, parent_folder, file_name)}; 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, @@ -610,12 +618,15 @@ int PayloadManager::GetOptimalChunkSize(EndpointIds endpoint_ids) { } PayloadTransferFrame::PayloadHeader PayloadManager::CreatePayloadHeader( - const InternalPayload& internal_payload, size_t offset) { + const InternalPayload& internal_payload, size_t offset, + std::string parent_folder, std::string file_name) { 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 @@ -1170,6 +1181,7 @@ 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 4e605ab8..934520d7 100644 --- a/cpp/core/internal/payload_manager.h +++ b/cpp/core/internal/payload_manager.h @@ -211,7 +211,8 @@ class PayloadManager : public EndpointManager::FrameProcessor { int GetOptimalChunkSize(EndpointIds endpoint_ids); PayloadTransferFrame::PayloadHeader CreatePayloadHeader( - const InternalPayload& payload, size_t offset); + const InternalPayload& payload, size_t offset, std::string parent_folder, + std::string file_name); 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 372642d3..095e675f 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,6 +266,7 @@ 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 f9c0208d..35157133 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 3843cee9..7d4bf8c3 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/public/core_config.h" +#include "platform/base/core_config.h" namespace location { namespace nearby { diff --git a/cpp/core/payload.cc b/cpp/core/payload.cc index 20acef85..aaba71e7 100644 --- a/cpp/core/payload.cc +++ b/cpp/core/payload.cc @@ -21,9 +21,24 @@ 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 = default; +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() = default; -Payload& Payload::operator=(Payload&& other) noexcept = 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; +} // Default (invalid) payload. Payload::Payload() : content_(absl::monostate()) {} @@ -33,9 +48,11 @@ Payload::Payload(ByteArray&& bytes) : content_(std::move(bytes)) {} Payload::Payload(const ByteArray& bytes) : content_(bytes) {} -Payload::Payload(InputFile file) +Payload::Payload(const char* parent_folder, const char* file_name, + InputFile&& file) : content_(std::move(file)), - id_(std::hash()(file.GetFilePath())) {} + parent_folder_(parent_folder), + file_name_(file_name) {} // TODO(jfcarroll): Convert std::function to function pointer Payload::Payload(std::function stream) @@ -47,7 +64,14 @@ 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) {} +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"; +} // TODO(jfcarroll): Convert std::function to function pointer Payload::Payload(Id id, std::function stream) @@ -69,7 +93,9 @@ InputStream* Payload::AsStream() { return result ? &(*result)() : nullptr; } // Returns InputFile* payload, if it has been defined, or nullptr. -InputFile* Payload::AsFile() { return absl::get_if(&content_); } +const InputFile* Payload::AsFile() const { + return absl::get_if(&content_); +} // Returns Payload unique ID. Payload::Id Payload::GetId() const { return id_; } @@ -80,8 +106,10 @@ 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); - InputFile* file = AsFile(); + const 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; @@ -96,6 +124,14 @@ 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 ec66062b..d5ea0896 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,13 +56,14 @@ class DLL_API Payload { explicit Payload(ByteArray&& bytes); explicit Payload(const ByteArray& bytes); - explicit Payload(InputFile file); + explicit Payload(const char* parent_folder, const char* file_name, + 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); @@ -72,7 +73,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. - InputFile* AsFile(); + const InputFile* AsFile() const; // Returns Payload unique ID. Id GetId() const; @@ -88,6 +89,9 @@ 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; @@ -95,6 +99,8 @@ 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 f6d54c76..009d8647 100644 --- a/cpp/core/payload_test.cc +++ b/cpp/core/payload_test.cc @@ -14,9 +14,13 @@ #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" @@ -24,16 +28,37 @@ #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 { -TEST(PayloadTest, DefaultPayloadHasUnknownType) { +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) { Payload payload; EXPECT_EQ(payload.GetType(), Payload::Type::kUnknown); } -TEST(PayloadTest, SupportsByteArrayType) { +TEST_F(PayloadTest, SupportsByteArrayType) { const ByteArray bytes("bytes"); Payload payload(bytes); EXPECT_EQ(payload.GetType(), Payload::Type::kBytes); @@ -42,13 +67,13 @@ TEST(PayloadTest, SupportsByteArrayType) { EXPECT_EQ(payload.AsBytes(), bytes); } -TEST(PayloadTest, SupportsFileType) { +TEST_F(PayloadTest, SupportsFileType) { constexpr size_t kOffset = 99; - const auto payload_id = Payload::GenerateId(); - InputFile file(payload_id, 100); - InputStream& stream = file.GetInputStream(); + InputFile file(path_.c_str()); + const InputStream& stream = file.GetInputStream(); - Payload payload(payload_id, std::move(file)); + Payload payload(TEST_FILE_PARENT_DIRECTORY.c_str(), TEST_FILE_PATH.c_str(), + std::move(file)); payload.SetOffset(kOffset); EXPECT_EQ(payload.GetType(), Payload::Type::kFile); @@ -58,7 +83,7 @@ TEST(PayloadTest, SupportsFileType) { EXPECT_EQ(payload.GetOffset(), kOffset); } -TEST(PayloadTest, SupportsStreamType) { +TEST_F(PayloadTest, SupportsStreamType) { constexpr size_t kOffset = 1234456; auto pipe = std::make_shared(); @@ -78,7 +103,7 @@ TEST(PayloadTest, SupportsStreamType) { EXPECT_EQ(payload.GetOffset(), kOffset); } -TEST(PayloadTest, PayloadIsMoveable) { +TEST_F(PayloadTest, PayloadIsMoveable) { Payload payload1; Payload payload2(ByteArray("bytes")); auto id = payload2.GetId(); @@ -91,13 +116,13 @@ TEST(PayloadTest, PayloadIsMoveable) { EXPECT_EQ(payload1.GetId(), id); } -TEST(PayloadTest, PayloadHasUniqueId) { +TEST_F(PayloadTest, PayloadHasUniqueId) { Payload payload1; Payload payload2; EXPECT_NE(payload1.GetId(), payload2.GetId()); } -TEST(PayloadTest, PayloadIsNotCopyable) { +TEST_F(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 f85d7d60..4268b45e 100644 --- a/cpp/core/strategy.h +++ b/cpp/core/strategy.h @@ -16,7 +16,7 @@ #include -#include "platform/public/core_config.h" +#include "platform/base/core_config.h" namespace location { namespace nearby { diff --git a/cpp/platform/api/platform.h b/cpp/platform/api/platform.h index 6cf54cae..c6873e0f 100644 --- a/cpp/platform/api/platform.h +++ b/cpp/platform/api/platform.h @@ -61,6 +61,9 @@ class ImplementationPlatform { // - file I/O // - Logging + static std::unique_ptr GetDownloadPath( + std::unique_ptr path); + // Atomics: // ======= @@ -78,12 +81,11 @@ 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(PayloadId payload_id, - std::int64_t total_size); - static std::unique_ptr CreateOutputFile(PayloadId payload_id); + Mutex *mutex); + static std::unique_ptr CreateInputFile(const char *file_path); + static std::unique_ptr CreateOutputFile(const char *file_path); 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(); @@ -94,10 +96,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 9fa908b8..89008e3b 100644 --- a/cpp/platform/base/BUILD +++ b/cpp/platform/base/BUILD @@ -29,6 +29,7 @@ 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 new file mode 100644 index 00000000..3afa60fc --- /dev/null +++ b/cpp/platform/base/core_config.h @@ -0,0 +1,38 @@ +// 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 470af01d..5dff0a0a 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 { -namespace { -std::string GetPayloadPath(PayloadId payload_id) { - return absl::StrCat("/tmp/", payload_id); +std::unique_ptr ImplementationPlatform::GetDownloadPath( + std::unique_ptr path) { + std::string basePath("/tmp/"); + return std::make_unique(basePath += *path); } -} // namespace int GetCurrentTid() { const LiveThread* my = Thread_GetMyLiveThread(); @@ -102,14 +102,13 @@ std::unique_ptr ImplementationPlatform::CreateAtomicBoolean( } std::unique_ptr ImplementationPlatform::CreateInputFile( - PayloadId payload_id, std::int64_t total_size) { - return absl::make_unique(GetPayloadPath(payload_id), - total_size); + 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)); + const char* file_path) { + return absl::make_unique(file_path); } 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 30daf0d2..607a884a 100644 --- a/cpp/platform/impl/ios/Source/Internal/GNCCoreConnection.mm +++ b/cpp/platform/impl/ios/Source/Internal/GNCCoreConnection.mm @@ -161,7 +161,10 @@ 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]; - Payload corePayload(payloadId, InputFile(payloadId, fileSize)); + + // 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())); 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 f7efb389..08759c6f 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) { - InputFile *payloadInputFile = payload.AsFile(); + const 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 d8669535..0bd3e990 100644 --- a/cpp/platform/impl/ios/Source/Internal/platform.mm +++ b/cpp/platform/impl/ios/Source/Internal/platform.mm @@ -37,18 +37,11 @@ namespace location { namespace nearby { namespace api { -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]); +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 // Atomics: std::unique_ptr ImplementationPlatform::CreateAtomicBoolean(bool initial_value) { @@ -78,22 +71,25 @@ std::unique_ptr ImplementationPlatform::CreateConditionVariab return std::make_unique(static_cast(mutex)); } -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. +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 GNCCore* core = GNCGetCore(); NSURL* url = [core extractURLWithPayloadID:payload_id]; if (url != nil) { return absl::make_unique(url); } else { - return absl::make_unique(GetPayloadPath(payload_id), total_size); + return absl::make_unique(GetDownloadPath(payload_id), total_size); } +#endif + return nullptr; } -std::unique_ptr ImplementationPlatform::CreateOutputFile(PayloadId payload_id) { - return absl::make_unique(GetPayloadPath(payload_id)); +std::unique_ptr ImplementationPlatform::CreateOutputFile(const char* file_path) { + return absl::make_unique(file_path); } std::unique_ptr ImplementationPlatform::CreateLogMessage( diff --git a/cpp/platform/impl/ios/Tests/BUILD b/cpp/platform/impl/ios/Tests/BUILD index c0f032ed..ce405aa4 100644 --- a/cpp/platform/impl/ios/Tests/BUILD +++ b/cpp/platform/impl/ios/Tests/BUILD @@ -23,7 +23,7 @@ objc_library( srcs = [ "Platform/GNCCryptoTest.mm", "Platform/GNCInputFileTest.mm", - "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 cadbadb6..ec77eb50 100644 --- a/cpp/platform/impl/shared/file.cc +++ b/cpp/platform/impl/shared/file.cc @@ -25,9 +25,21 @@ 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); -InputFile::InputFile(const std::string& path, std::int64_t size) - : file_(path, std::ios::binary), path_(path), total_size_(size) {} + // 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); +} ExceptionOr InputFile::Read(std::int64_t size) { if (!file_.is_open()) { @@ -62,8 +74,15 @@ Exception InputFile::Close() { // OutputFile -OutputFile::OutputFile(absl::string_view path) - : file_(std::string(path), std::ios::binary) {} +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); +} 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 21ed871c..dfc476cc 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 std::string& path, std::int64_t size); + explicit InputFile(const char* file_path); ~InputFile() override = default; InputFile(InputFile&&) = default; InputFile& operator=(InputFile&&) = default; - ExceptionOr Read(std::int64_t size) override; - std::string GetFilePath() const override { return path_; } - std::int64_t GetTotalSize() const override { return total_size_; } + ExceptionOr Read(int64_t size) override; + std::string GetFilePath() const override { return file_path_; } + int64_t GetTotalSize() const override { return total_size_; } Exception Close() override; private: std::ifstream file_; - std::string path_; - std::int64_t total_size_; + std::string file_path_; + int64_t total_size_; }; class OutputFile final : public api::OutputFile { public: - explicit OutputFile(absl::string_view path); + explicit OutputFile(const char* file_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 e1a55a81..69249838 100644 --- a/cpp/platform/impl/shared/file_test.cc +++ b/cpp/platform/impl/shared/file_test.cc @@ -14,7 +14,9 @@ #include "platform/impl/shared/file.h" +#include #include +#include #include #include #include @@ -24,6 +26,10 @@ #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 { @@ -32,7 +38,7 @@ class FileTest : public ::testing::Test { protected: void SetUp() override { temp_path_ = std::make_unique(TempPath::Local); - path_ = temp_path_->path() + "/file.txt"; + path_ = temp_path_->path() + "/" + TEST_FILE_NAME; std::ofstream output_file(path_); file_ = std::fstream(path_, std::fstream::in | std::fstream::out); } @@ -65,39 +71,39 @@ class FileTest : public ::testing::Test { }; TEST_F(FileTest, InputFile_NonExistentPath) { - InputFile input_file("/not/a/valid/path.txt", GetSize()); + InputFile input_file((TEST_INVALID_PARENT_FOLDER + TEST_FILE_NAME).c_str()); 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_, GetSize()); + InputFile input_file(path_.c_str()); EXPECT_EQ(input_file.GetFilePath(), path_); } TEST_F(FileTest, InputFile_EmptyFileEOF) { - InputFile input_file(path_, GetSize()); + InputFile input_file(path_.c_str()); AssertEmpty(input_file.Read(kMaxSize)); } TEST_F(FileTest, InputFile_ReadWorks) { WriteToFile("abc"); - InputFile input_file(path_, GetSize()); + InputFile input_file(path_.c_str()); input_file.Read(kMaxSize); SUCCEED(); } TEST_F(FileTest, InputFile_ReadUntilEOF) { WriteToFile("abc"); - InputFile input_file(path_, GetSize()); + InputFile input_file(path_.c_str()); AssertEquals(input_file.Read(kMaxSize), "abc"); AssertEmpty(input_file.Read(kMaxSize)); } TEST_F(FileTest, InputFile_ReadWithSize) { WriteToFile("abc"); - InputFile input_file(path_, GetSize()); + InputFile input_file(path_.c_str()); AssertEquals(input_file.Read(2), "ab"); AssertEquals(input_file.Read(1), "c"); AssertEmpty(input_file.Read(kMaxSize)); @@ -105,7 +111,7 @@ TEST_F(FileTest, InputFile_ReadWithSize) { TEST_F(FileTest, InputFile_GetTotalSize) { WriteToFile("abc"); - InputFile input_file(path_, GetSize()); + InputFile input_file(path_.c_str()); EXPECT_EQ(input_file.GetTotalSize(), 3); AssertEquals(input_file.Read(1), "a"); EXPECT_EQ(input_file.GetTotalSize(), 3); @@ -113,7 +119,7 @@ TEST_F(FileTest, InputFile_GetTotalSize) { TEST_F(FileTest, InputFile_Close) { WriteToFile("abc"); - InputFile input_file(path_, GetSize()); + InputFile input_file(path_.c_str()); input_file.Close(); ExceptionOr read_result = input_file.Read(kMaxSize); EXPECT_FALSE(read_result.ok()); @@ -121,23 +127,23 @@ TEST_F(FileTest, InputFile_Close) { } TEST_F(FileTest, OutputFile_NonExistentPath) { - OutputFile output_file("/not/a/valid/path.txt"); + OutputFile output_file((TEST_INVALID_PARENT_FOLDER + TEST_FILE_NAME).c_str()); ByteArray bytes("a", 1); EXPECT_TRUE(output_file.Write(bytes).Raised(Exception::kIo)); } TEST_F(FileTest, OutputFile_Write) { - OutputFile output_file(path_); + OutputFile output_file(path_.c_str()); 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_, GetSize()); + InputFile input_file(path_.c_str()); AssertEquals(input_file.Read(kMaxSize), "abc"); } TEST_F(FileTest, OutputFile_Close) { - OutputFile output_file(path_); + OutputFile output_file(path_.c_str()); 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 1d436944..e6994253 100644 --- a/cpp/platform/impl/windows/BUILD +++ b/cpp/platform/impl/windows/BUILD @@ -26,11 +26,9 @@ 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 fd4d6427..21376833 100644 --- a/cpp/platform/impl/windows/executor_test.cc +++ b/cpp/platform/impl/windows/executor_test.cc @@ -125,20 +125,25 @@ 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, index]() { + executor->Execute([&output, &threadIds, &crit_sec, index]() { + EnterCriticalSection(&crit_sec); 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 @@ -200,19 +205,25 @@ 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, index]() { + executor->Execute([&output, &threadIds, &crit_sec, index]() { + EnterCriticalSection(&crit_sec); 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 deleted file mode 100644 index 1ed39821..00000000 --- a/cpp/platform/impl/windows/input_file.h +++ /dev/null @@ -1,50 +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 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 eb2bf811..1a38fe93 100644 --- a/cpp/platform/impl/windows/input_file_test.cc +++ b/cpp/platform/impl/windows/input_file_test.cc @@ -12,8 +12,6 @@ // 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" @@ -24,21 +22,18 @@ class InputFileTests : public testing::Test { protected: // You can define per-test set-up logic as usual. void SetUp() override { - 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 + 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 if (hFile_ == INVALID_HANDLE_VALUE) { NEARBY_LOG(ERROR, - "Failed to create OutputFile with payloadId: %s and error: %d", - test_utils::GetPayloadPath(payloadId).c_str(), GetLastError()); + "Failed to create OutputFile with file path: %s and error: %d", + TEST_FILE_PATH.c_str(), GetLastError()); } const char* buffer = TEST_STRING; @@ -52,9 +47,8 @@ class InputFileTests : public testing::Test { // You can define per-test tear-down logic as usual. void TearDown() override { - location::nearby::PayloadId payloadId(TEST_PAYLOAD_ID); - if (FileExists(test_utils::GetPayloadPath(payloadId).c_str())) { - DeleteFileA(test_utils::GetPayloadPath(payloadId).c_str()); + if (FileExists(TEST_FILE_PATH.c_str())) { + DeleteFileA(TEST_FILE_PATH.c_str()); } } @@ -74,7 +68,7 @@ TEST_F(InputFileTests, SuccessfulCreation) { std::unique_ptr inputFile = nullptr; inputFile = location::nearby::api::ImplementationPlatform::CreateInputFile( - payloadId, strlen(TEST_STRING)); + TEST_FILE_PATH.c_str()); EXPECT_NE(inputFile, nullptr); EXPECT_EQ(inputFile->Close(), @@ -82,28 +76,27 @@ 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( - payloadId, strlen(TEST_STRING)); + TEST_FILE_PATH.c_str()); fileName = inputFile->GetFilePath(); EXPECT_EQ(inputFile->Close(), location::nearby::Exception{location::nearby::Exception::kSuccess}); - EXPECT_EQ(fileName, test_utils::GetPayloadPath(payloadId).c_str()); + EXPECT_EQ(fileName, expected); } 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( - payloadId, strlen(TEST_STRING)); + TEST_FILE_PATH.c_str()); size = inputFile->GetTotalSize(); @@ -114,11 +107,10 @@ 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( - payloadId, strlen(TEST_STRING)); + TEST_FILE_PATH.c_str()); auto fileSize = inputFile->GetTotalSize(); auto dataRead = inputFile->Read(fileSize); @@ -131,11 +123,10 @@ 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( - payloadId, strlen(TEST_STRING)); + TEST_FILE_PATH.c_str()); 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 deleted file mode 100644 index 9ad479b6..00000000 --- a/cpp/platform/impl/windows/output_file.h +++ /dev/null @@ -1,47 +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 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 18e47f93..f089f719 100644 --- a/cpp/platform/impl/windows/output_file_test.cc +++ b/cpp/platform/impl/windows/output_file_test.cc @@ -12,8 +12,6 @@ // 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" @@ -24,17 +22,15 @@ class OutputFileTests : public testing::Test { protected: // You can define per-test set-up logic as usual. void SetUp() override { - location::nearby::PayloadId payloadId(TEST_PAYLOAD_ID); - if (FileExists(test_utils::GetPayloadPath(payloadId).c_str())) { - DeleteFileA(test_utils::GetPayloadPath(payloadId).c_str()); + if (FileExists(TEST_FILE_PATH.c_str())) { + DeleteFileA(TEST_FILE_PATH.c_str()); } } // You can define per-test tear-down logic as usual. void TearDown() override { - location::nearby::PayloadId payloadId(TEST_PAYLOAD_ID); - if (FileExists(test_utils::GetPayloadPath(payloadId).c_str())) { - DeleteFileA(test_utils::GetPayloadPath(payloadId).c_str()); + if (FileExists(TEST_FILE_PATH.c_str())) { + DeleteFileA(TEST_FILE_PATH.c_str()); } } @@ -47,44 +43,41 @@ 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( - payloadId)); + TEST_FILE_PATH.c_str())); 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( - payloadId)); + TEST_FILE_PATH.c_str())); EXPECT_NO_THROW(outputFile->Close()); - DeleteFileA(test_utils::GetPayloadPath(payloadId).c_str()); + DeleteFileA(TEST_FILE_PATH.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( - payloadId)); + TEST_FILE_PATH.c_str())); EXPECT_NO_THROW(outputFile->Write(data)); EXPECT_NO_THROW(outputFile->Close()); - DeleteFileA(test_utils::GetPayloadPath(payloadId).c_str()); + DeleteFileA(TEST_FILE_PATH.c_str()); } diff --git a/cpp/platform/impl/windows/platform.cc b/cpp/platform/impl/windows/platform.cc index 5d117cf6..d50b757c 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::string GetPayloadPath(PayloadId payload_id) { +std::unique_ptr ImplementationPlatform::GetDownloadPath( + std::unique_ptr path) { PWSTR basePath; // Retrieves the full path of a known folder identified by the folder's @@ -61,13 +61,12 @@ std::string GetPayloadPath(PayloadId payload_id) { // 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; + auto basePathLength = (wcslen(basePath) + 1) * sizeof(char); + char* fullpathUTF8 = new char(basePathLength); + wcstombs(fullpathUTF8, basePath, basePathLength); + return std::make_unique(std::string(fullpathUTF8) += + "/" + *path); } -} // namespace std::unique_ptr ImplementationPlatform::CreateAtomicBoolean( bool initial_value) { @@ -94,14 +93,13 @@ ImplementationPlatform::CreateConditionVariable(Mutex* mutex) { } std::unique_ptr ImplementationPlatform::CreateInputFile( - PayloadId payload_id, std::int64_t total_size) { - return absl::make_unique(GetPayloadPath(payload_id), - total_size); + 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)); + const char* file_path) { + return absl::make_unique(file_path); } // 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 4a52c207..e62ee71d 100644 --- a/cpp/platform/impl/windows/scheduled_executor_test.cc +++ b/cpp/platform/impl/windows/scheduled_executor_test.cc @@ -31,15 +31,21 @@ 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]() { + submittableExecutor->Execute([&output, &threadIds, &crit_sec]() { + EnterCriticalSection(&crit_sec); 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 @@ -63,6 +69,9 @@ 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 = @@ -71,16 +80,19 @@ TEST(ScheduledExecutorTests, ScheduleSucceeds) { // Act submittableExecutor->Schedule( - [&output, &threadIds, &timeExecuted]() { + [&output, &threadIds, &timeExecuted, &crit_sec]() { + EnterCriticalSection(&crit_sec); 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) @@ -148,13 +160,18 @@ 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]() { + [&output, &threadIds, &crit_sec]() { + EnterCriticalSection(&crit_sec); threadIds->push_back(GetCurrentThreadId()); output.append(RUNNABLE_0_TEXT.c_str()); + LeaveCriticalSection(&crit_sec); }, absl::Milliseconds(100)); @@ -163,6 +180,7 @@ 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 779f5731..471bcd3c 100644 --- a/cpp/platform/impl/windows/submittable_executor_test.cc +++ b/cpp/platform/impl/windows/submittable_executor_test.cc @@ -164,19 +164,25 @@ 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, index]() { + submittableExecutor->Execute([&output, &threadIds, &crit_sec, index]() { + EnterCriticalSection(&crit_sec); 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 @@ -206,20 +212,27 @@ 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, index]() { + result &= submittableExecutor->DoSubmit([&output, &threadIds, &crit_sec, + index]() { + EnterCriticalSection(&crit_sec); 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 9bd73b5e..703fa690 100644 --- a/cpp/platform/impl/windows/test_utils.cc +++ b/cpp/platform/impl/windows/test_utils.cc @@ -19,39 +19,14 @@ #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 ef95b1d8..6ae3a52c 100644 --- a/cpp/platform/impl/windows/test_utils.h +++ b/cpp/platform/impl/windows/test_utils.h @@ -15,14 +15,8 @@ #ifndef PLATFORM_IMPL_WINDOWS_TEST_UTILS_H_ #define PLATFORM_IMPL_WINDOWS_TEST_UTILS_H_ -#include -#include - -#include #include -#include "platform/base/payload_id.h" - #define TEST_BUFFER_SIZE 256 #define TEST_PAYLOAD_ID 64l #define TEST_STRING \ @@ -39,9 +33,21 @@ "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); -std::string GetPayloadPath(location::nearby::PayloadId payload_id); + +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 ""; } +}; } // namespace test_utils #endif // PLATFORM_IMPL_WINDOWS_TEST_UTILS_H_ diff --git a/cpp/platform/public/BUILD b/cpp/platform/public/BUILD index 086645b1..4648ac67 100644 --- a/cpp/platform/public/BUILD +++ b/cpp/platform/public/BUILD @@ -29,7 +29,6 @@ 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 4ea9bd99..46508f7e 100644 --- a/cpp/platform/public/file.cc +++ b/cpp/platform/public/file.cc @@ -17,16 +17,19 @@ namespace location { namespace nearby { -InputFile::InputFile(PayloadId payload_id, std::int64_t size) - : impl_(Platform::CreateInputFile(payload_id, size)), id_(payload_id) {} +InputFile::InputFile(const char* file_path) + : impl_(Platform::CreateInputFile(file_path)) {} + InputFile::~InputFile() = default; -InputFile::InputFile(InputFile&&) noexcept = default; +InputFile::InputFile(InputFile&& other) noexcept { + impl_ = std::move(other.impl_); +} 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) { +ExceptionOr InputFile::Read(std::int64_t size) const { return impl_->Read(size); } @@ -36,13 +39,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) { +ExceptionOr InputFile::Skip(size_t offset) const { return impl_->Skip(offset); } // Disallows further reads from the file and frees system resources, // associated with it. -Exception InputFile::Close() { return impl_->Close(); } +Exception InputFile::Close() const { return impl_->Close(); } // Returns a handle to the underlying input stream. // @@ -51,13 +54,10 @@ Exception InputFile::Close() { 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. -InputStream& InputFile::GetInputStream() { return *impl_; } +const InputStream& InputFile::GetInputStream() const { return *impl_; } -// 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(const char* file_path) + : impl_(Platform::CreateOutputFile(file_path)) {} OutputFile::~OutputFile() = default; OutputFile::OutputFile(OutputFile&&) noexcept = default; OutputFile& OutputFile::operator=(OutputFile&&) noexcept = default; @@ -85,8 +85,5 @@ 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 80e1ce58..638e9ae0 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(PayloadId payload_id, std::int64_t size); + InputFile(const char* file_path); ~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); + ExceptionOr Read(std::int64_t size) const; // 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); + ExceptionOr Skip(size_t offset) const; // Disallows further reads from the file and frees system resources, // associated with it. - Exception Close(); + Exception Close() const; // Returns a handle to the underlying input stream. // @@ -63,20 +63,16 @@ 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. - InputStream& GetInputStream(); - - // Returns payload id of this file. The closest "file" equivalent is inode. - PayloadId GetPayloadId() const; + const InputStream& GetInputStream() const; private: std::unique_ptr impl_; - PayloadId id_; }; class DLL_API OutputFile final { public: using Platform = api::ImplementationPlatform; - explicit OutputFile(PayloadId payload_id); + explicit OutputFile(const char* file_path); ~OutputFile(); OutputFile(OutputFile&&) noexcept; OutputFile& operator=(OutputFile&&) noexcept; @@ -102,12 +98,8 @@ 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 d65a6bd5..81b3dc79 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/public/core_config.h" +#include "platform/base/core_config.h" namespace location { namespace nearby { diff --git a/windows/advertising_options.h b/windows/advertising_options.h index 80b54e13..7362ea06 100644 --- a/windows/advertising_options.h +++ b/windows/advertising_options.h @@ -19,7 +19,7 @@ #include -#include "platform/public/core_config.h" +#include "platform/base/core_config.h" namespace location { namespace nearby { diff --git a/windows/connection_options.cc b/windows/connection_options.cc index 7d299454..8dc49de4 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/public/core_config.h" +#include "platform/base/core_config.h" namespace location { namespace nearby { diff --git a/windows/connection_options.h b/windows/connection_options.h index 6d7f0cfb..a5e6830a 100644 --- a/windows/connection_options.h +++ b/windows/connection_options.h @@ -19,7 +19,7 @@ #include -#include "platform/public/core_config.h" +#include "platform/base/core_config.h" namespace location { namespace nearby { diff --git a/windows/discovery_options.cc b/windows/discovery_options.cc index caf1460c..10de6596 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/public/core_config.h" +#include "platform/base/core_config.h" namespace location { namespace nearby { diff --git a/windows/discovery_options.h b/windows/discovery_options.h index a22e9836..01d7f307 100644 --- a/windows/discovery_options.h +++ b/windows/discovery_options.h @@ -19,7 +19,7 @@ #include -#include "platform/public/core_config.h" +#include "platform/base/core_config.h" namespace location { namespace nearby {