diff --git a/internal/platform/file.cc b/internal/platform/file.cc index 12fe4c1d..14f1b1a4 100644 --- a/internal/platform/file.cc +++ b/internal/platform/file.cc @@ -68,10 +68,6 @@ Exception OutputFile::Write(const ByteArray& data) { return impl_->Write(data); } -// Ensures that all data written by previous calls to Write() is passed -// down to the applicable transport layer. -Exception OutputFile::Flush() { return impl_->Flush(); } - // Disallows further writes to the file and frees system resources, // associated with it. Exception OutputFile::Close() { return impl_->Close(); } diff --git a/internal/platform/file.h b/internal/platform/file.h index 44af1d3b..dd3c8e17 100644 --- a/internal/platform/file.h +++ b/internal/platform/file.h @@ -85,10 +85,6 @@ class OutputFile final { // Returns Exception::kIo on error, Exception::kSuccess otherwise. Exception Write(const ByteArray& data); - // Ensures that all data written by previous calls to Write() is passed - // down to the applicable transport layer. - Exception Flush(); - // Disallows further writes to the file and frees system resources, // associated with it. Exception Close(); diff --git a/internal/platform/implementation/output_file.h b/internal/platform/implementation/output_file.h index f82502ce..434a57bc 100644 --- a/internal/platform/implementation/output_file.h +++ b/internal/platform/implementation/output_file.h @@ -15,7 +15,6 @@ #ifndef PLATFORM_API_OUTPUT_FILE_H_ #define PLATFORM_API_OUTPUT_FILE_H_ -#include "internal/platform/byte_array.h" #include "internal/platform/exception.h" #include "internal/platform/output_stream.h" @@ -26,6 +25,8 @@ namespace api { class OutputFile : public OutputStream { public: ~OutputFile() override = default; + // File flush is a no-op. + Exception Flush() override { return {Exception::kSuccess}; } }; } // namespace api diff --git a/internal/platform/implementation/windows/BUILD b/internal/platform/implementation/windows/BUILD index 56b8a517..00942c69 100644 --- a/internal/platform/implementation/windows/BUILD +++ b/internal/platform/implementation/windows/BUILD @@ -366,3 +366,18 @@ cc_test( "@com_google_googletest//:gtest_main", ], ) + +cc_test( + name = "file_test", + size = "small", + timeout = "short", + srcs = [ + "file_test.cc", + ], + tags = ["nozapfhahn"], + deps = [ + ":windows", + "@com_github_protobuf_matchers//protobuf-matchers", + "@com_google_googletest//:gtest_main", + ], +) diff --git a/internal/platform/implementation/windows/file.cc b/internal/platform/implementation/windows/file.cc index 8f129883..489875c5 100644 --- a/internal/platform/implementation/windows/file.cc +++ b/internal/platform/implementation/windows/file.cc @@ -14,10 +14,11 @@ #include "internal/platform/implementation/windows/file.h" +#include + #include #include -#include -#include +#include #include #include @@ -28,8 +29,7 @@ #include "internal/platform/implementation/windows/string_utils.h" #include "internal/platform/logging.h" -namespace nearby { -namespace windows { +namespace nearby::windows { // InputFile std::unique_ptr IOFile::CreateInputFile(absl::string_view file_path, @@ -39,91 +39,93 @@ std::unique_ptr IOFile::CreateInputFile(absl::string_view file_path, IOFile::IOFile(absl::string_view file_path, size_t size) : path_(file_path) { // Always open input file path as wide string on Windows platform. - std::wstring wide_path = - string_utils::StringToWideString(std::string(file_path)); - file_.open(wide_path, std::ios::binary | std::ios::in | std::ios::ate); - - total_size_ = file_.tellg(); - if (total_size_ == -1) { - // Unsure why it consistently returns -1 when the file size exceeds 2GB. If - // obtaining the file size through tellg fails, use the size provided - // in the parameters. - total_size_ = size; + std::wstring wide_path = string_utils::StringToWideString(path_); + file_ = ::CreateFileW(wide_path.data(), GENERIC_READ, FILE_SHARE_READ, + /*lpSecurityAttributes=*/nullptr, OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL | FILE_FLAG_SEQUENTIAL_SCAN, + /*hTemplateFile=*/nullptr); + if (file_ == INVALID_HANDLE_VALUE) { + LOG(ERROR) << "Failed to open input file: " << file_path + << " with error: " << ::GetLastError(); + return; } - file_.seekg(0); + LARGE_INTEGER file_size; + if (::GetFileSizeEx(file_, &file_size) == 0) { + LOG(ERROR) << "Failed to get file size: " << file_path + << " with error: " << ::GetLastError(); + return; + } + total_size_ = file_size.QuadPart; } std::unique_ptr IOFile::CreateOutputFile(absl::string_view path) { return std::unique_ptr(new IOFile(path)); } -IOFile::IOFile(absl::string_view file_path) - : file_(), path_(file_path), total_size_(0) { +IOFile::IOFile(absl::string_view file_path) : path_(file_path), total_size_(0) { // Always open input file path as wide string on Windows platform. std::wstring wide_path = string_utils::StringToWideString(path_); - file_.open(wide_path, std::ios::binary | std::ios::out); -} - -ExceptionOr IOFile::Read(std::int64_t size) { - try { - if (!file_.is_open()) { - return ExceptionOr{Exception::kIo}; - } - - if (file_.peek() == EOF) { - return ExceptionOr{ByteArray{}}; - } - - if (!file_.good()) { - return ExceptionOr{Exception::kIo}; - } - - if (buffer_.size() < size) { - buffer_.resize(size); - } - - file_.read(buffer_.data(), static_cast(size)); - auto num_bytes_read = file_.gcount(); - if (num_bytes_read == 0) { - return ExceptionOr{Exception::kIo}; - } - return ExceptionOr(ByteArray(buffer_.data(), num_bytes_read)); - } catch (...) { - LOG(ERROR) << "Fail to read"; - return ExceptionOr{Exception::kIo}; + file_ = ::CreateFileW(wide_path.data(), GENERIC_WRITE, /*dwShareMode=*/0, + /*lpSecurityAttributes=*/nullptr, CREATE_NEW, + FILE_ATTRIBUTE_NORMAL, + /*hTemplateFile=*/nullptr); + if (file_ == INVALID_HANDLE_VALUE) { + LOG(ERROR) << "Failed to open output file: " << file_path + << " with error: " << ::GetLastError(); + return; } } +ExceptionOr IOFile::Read(std::int64_t size) { + if (file_ == INVALID_HANDLE_VALUE) { + return ExceptionOr{Exception::kIo}; + } + // ReadFile API only supports int32_t size. + if (size > std::numeric_limits::max()) { + return ExceptionOr{Exception::kIo}; + } + if (buffer_.size() < size) { + buffer_.resize(size); + } + DWORD bytes_read = 0; + if (::ReadFile(file_, buffer_.data(), size, &bytes_read, + /*lpOverlapped=*/nullptr) == 0) { + LOG(ERROR) << "Failed to read file: " << path_ + << " with error: " << ::GetLastError(); + return ExceptionOr{Exception::kIo}; + } + if (bytes_read == 0) { + return ExceptionOr{ByteArray{}}; + } + return ExceptionOr(ByteArray(buffer_.data(), bytes_read)); +} + Exception IOFile::Close() { - if (file_.is_open()) { - file_.close(); + if (file_ != INVALID_HANDLE_VALUE) { + ::CloseHandle(file_); + file_ = INVALID_HANDLE_VALUE; } return {Exception::kSuccess}; } Exception IOFile::Write(const ByteArray& data) { - try { - if (!file_.is_open()) { - return {Exception::kIo}; - } - - if (!file_.good()) { - return {Exception::kIo}; - } - - file_.write(data.data(), data.size()); - return {file_.good() ? Exception::kSuccess : Exception::kIo}; - } catch (...) { - LOG(ERROR) << "Fail to write"; + if (file_ == INVALID_HANDLE_VALUE) { return {Exception::kIo}; } + // WriteFile API only supports int32_t size. + if (data.size() > std::numeric_limits::max()) { + return {Exception::kIo}; + } + DWORD bytes_written = 0; + if (::WriteFile(file_, data.data(), data.size(), &bytes_written, + /*lpOverlapped=*/nullptr) == 0 || + bytes_written != data.size()) { + LOG(ERROR) << "Failed to write file: " << path_ + << " with error: " << ::GetLastError(); + return {Exception::kIo}; + } + return {Exception::kSuccess}; } -Exception IOFile::Flush() { - file_.flush(); - return {file_.good() ? Exception::kSuccess : Exception::kIo}; -} - -} // namespace windows -} // namespace nearby +} // namespace nearby::windows diff --git a/internal/platform/implementation/windows/file.h b/internal/platform/implementation/windows/file.h index 56d3b294..1afa649e 100644 --- a/internal/platform/implementation/windows/file.h +++ b/internal/platform/implementation/windows/file.h @@ -15,9 +15,10 @@ #ifndef PLATFORM_IMPL_WINDOWS_FILE_H_ #define PLATFORM_IMPL_WINDOWS_FILE_H_ +#include + #include #include -#include #include #include @@ -27,8 +28,7 @@ #include "internal/platform/implementation/input_file.h" #include "internal/platform/implementation/output_file.h" -namespace nearby { -namespace windows { +namespace nearby::windows { class IOFile final : public api::InputFile, public api::OutputFile { public: @@ -37,6 +37,10 @@ class IOFile final : public api::InputFile, public api::OutputFile { static std::unique_ptr CreateOutputFile(absl::string_view path); + ~IOFile() override { + Close(); + } + ExceptionOr Read(std::int64_t size) override; std::string GetFilePath() const override { return path_; } @@ -45,19 +49,17 @@ class IOFile final : public api::InputFile, public api::OutputFile { Exception Close() override; Exception Write(const ByteArray& data) override; - Exception Flush() override; private: explicit IOFile(absl::string_view file_path, size_t size); explicit IOFile(absl::string_view file_path); - std::fstream file_; + HANDLE file_ = INVALID_HANDLE_VALUE; std::string path_; std::string buffer_; - std::int64_t total_size_; + std::int64_t total_size_ = 0; }; -} // namespace windows -} // namespace nearby +} // namespace nearby::windows #endif // PLATFORM_IMPL_WINDOWS_FILE_H_ diff --git a/internal/platform/implementation/windows/file_test.cc b/internal/platform/implementation/windows/file_test.cc new file mode 100644 index 00000000..767de341 --- /dev/null +++ b/internal/platform/implementation/windows/file_test.cc @@ -0,0 +1,191 @@ +// Copyright 2025 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. + +#include "internal/platform/implementation/windows/file.h" + +#include + +#include +#include +#include + +#include "gtest/gtest.h" +#include "absl/strings/str_cat.h" +#include "absl/strings/string_view.h" +#include "internal/platform/byte_array.h" +#include "internal/platform/exception.h" +#include "internal/platform/logging.h" + +namespace nearby::windows { +namespace { + +std::string GetTempDir() { + std::string temp_path; + temp_path.resize(MAX_PATH); + if (::GetTempPathA(temp_path.size(), temp_path.data()) == 0) { + LOG(ERROR) << "Failed to get temp path: " << ::GetLastError(); + return ""; + } + return std::string(temp_path.data()); +} + +std::string GetTempFileName(absl::string_view prefix) { + std::string temp_path = GetTempDir(); + if (temp_path.empty()) { + return ""; + } + std::string file_path; + file_path.resize(MAX_PATH); + UINT id = ::GetTempFileNameA(temp_path.data(), prefix.data(), /*uUnique=*/0, + file_path.data()); + if (id == 0) { + LOG(ERROR) << "Failed to get temp file name with path: " << temp_path + << ", error: " << ::GetLastError(); + return ""; + } + return absl::StrCat(temp_path, prefix, id, ".tmp"); +} + +std::string CreateTempFile(absl::string_view prefix, size_t size) { + std::string temp_file = GetTempFileName(prefix); + if (temp_file.empty()) { + return ""; + } + HANDLE file = ::CreateFileA( + temp_file.data(), GENERIC_WRITE, /*dwShareMode=*/0, + /*lpSecurityAttributes=*/nullptr, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, + /*hTemplateFile=*/nullptr); + if (file == INVALID_HANDLE_VALUE) { + LOG(ERROR) << "Failed to open temp file: " << temp_file + << " with error: " << ::GetLastError(); + return ""; + } + LARGE_INTEGER file_size; + file_size.QuadPart = size; + if (::SetFilePointerEx(file, file_size, /*lpNewFilePointer=*/nullptr, + FILE_BEGIN) == 0) { + LOG(ERROR) << "Failed to set file pointer: " << temp_file + << " with error: " << ::GetLastError(); + return ""; + } + if (::SetEndOfFile(file) == 0) { + LOG(ERROR) << "Failed to set end of file: " << temp_file + << " with error: " << ::GetLastError(); + return ""; + } + ::CloseHandle(file); + return temp_file; +} + +TEST(IOFileTest, InputFileNonexistentPathHasZeroSize) { + std::unique_ptr input_file = + IOFile::CreateInputFile(/*file_path=*/"", /*size=*/0); + ASSERT_NE(input_file, nullptr); + + EXPECT_EQ(input_file->GetTotalSize(), 0); +} + +TEST(IOFileTest, InputFileNonexistentPathFailsToRead) { + std::unique_ptr input_file = + IOFile::CreateInputFile(/*file_path=*/"", /*size=*/0); + ASSERT_NE(input_file, nullptr); + + ExceptionOr read_result = input_file->Read(/*size=*/1); + + EXPECT_FALSE(read_result.ok()); + EXPECT_EQ(read_result.exception(), Exception::kIo); +} + +TEST(IOFileTest, InputFileNonexistentPathCloseSucceeds) { + std::unique_ptr input_file = + IOFile::CreateInputFile(/*file_path=*/"", /*size=*/0); + ASSERT_NE(input_file, nullptr); + + ExceptionOr close_result = input_file->Close(); + + EXPECT_TRUE(close_result.ok()); +} + +TEST(IOFileTest, InputFileLargeFileSize) { + constexpr size_t kLargeFileSize = 3LL * 1024LL * 1024LL * 1024LL; + std::string temp_file = CreateTempFile("LargeFileTest", kLargeFileSize); + ASSERT_FALSE(temp_file.empty()); + + std::unique_ptr input_file = + IOFile::CreateInputFile(temp_file, /*size=*/0); + ASSERT_NE(input_file, nullptr); + + EXPECT_EQ(input_file->GetTotalSize(), kLargeFileSize); + ::DeleteFileA(temp_file.data()); +} + +TEST(IOFileTest, InputFileReadToEnd) { + constexpr size_t kFileSize = 100; + std::string temp_file = CreateTempFile("ReadToEnd", kFileSize); + ASSERT_FALSE(temp_file.empty()); + + std::unique_ptr input_file = + IOFile::CreateInputFile(temp_file, /*size=*/0); + ASSERT_NE(input_file, nullptr); + + EXPECT_EQ(input_file->GetTotalSize(), kFileSize); + ExceptionOr read_result = input_file->Read(kFileSize); + EXPECT_TRUE(read_result.ok()); + EXPECT_EQ(read_result.result().size(), kFileSize); + + read_result = input_file->Read(kFileSize); + EXPECT_TRUE(read_result.ok()); + EXPECT_EQ(read_result.result().size(), 0); + + ::DeleteFileA(temp_file.data()); +} + +TEST(IOFileTest, OutputFileAlreadyExists) { + constexpr size_t kFileSize = 100; + std::string temp_file = CreateTempFile("OutputFileExists", kFileSize); + ASSERT_FALSE(temp_file.empty()); + + std::unique_ptr output_file = IOFile::CreateOutputFile(temp_file); + ASSERT_NE(output_file, nullptr); + + EXPECT_EQ(output_file->GetTotalSize(), 0); + ExceptionOr write_result = output_file->Write(ByteArray("test")); + EXPECT_FALSE(write_result.ok()); + EXPECT_TRUE(write_result.GetException().Raised(Exception::kIo)); + + ::DeleteFileA(temp_file.data()); +} + +TEST(IOFileTest, OutputFileWrite) { + std::string temp_file = GetTempFileName("WriteFileTest"); + std::unique_ptr output_file = IOFile::CreateOutputFile(temp_file); + ASSERT_NE(output_file, nullptr); + + ExceptionOr write_result = output_file->Write(ByteArray("test1")); + EXPECT_TRUE(write_result.ok()); + write_result = output_file->Write(ByteArray("test2")); + EXPECT_TRUE(write_result.ok()); + EXPECT_TRUE(output_file->Close().Ok()); + + std::unique_ptr input_file = + IOFile::CreateInputFile(temp_file, /*size=*/0); + ExceptionOr read_result = input_file->Read(10); + EXPECT_TRUE(read_result.ok()); + EXPECT_EQ(read_result.result(), ByteArray("test1test2")); + + ::DeleteFileA(temp_file.data()); +} + +} // namespace +} // namespace nearby::windows