From f1d3c2d740837d400d1b8a620cf239cb05764f22 Mon Sep 17 00:00:00 2001 From: Francis Tsui Date: Fri, 29 Mar 2024 12:40:24 -0700 Subject: [PATCH] Wrap std::filesystem calls. PiperOrigin-RevId: 620315145 --- sharing/BUILD | 8 +++- sharing/common/BUILD | 7 ++++ sharing/common/files.cc | 63 +++++++++++++++++++++++++++++ sharing/common/files.h | 41 +++++++++++++++++++ sharing/nearby_connections_types.h | 11 +++-- sharing/nearby_file_handler.cc | 24 +++-------- sharing/nearby_file_handler.h | 2 +- sharing/nearby_file_handler_test.cc | 41 ++++++------------- 8 files changed, 144 insertions(+), 53 deletions(-) create mode 100644 sharing/common/files.cc create mode 100644 sharing/common/files.h diff --git a/sharing/BUILD b/sharing/BUILD index 5dc4578a..e33c189e 100644 --- a/sharing/BUILD +++ b/sharing/BUILD @@ -7,6 +7,7 @@ cc_library( "//internal/crypto_cros", "//internal/interop:authentication_status", "//sharing/common:compatible_u8_string", + "//sharing/common:files", "@com_google_absl//absl/strings:string_view", "@com_google_absl//absl/time", ], @@ -30,7 +31,6 @@ cc_library( "file_attachment.h", "nearby_connection.h", "nearby_connections_manager.h", - "nearby_connections_types.h", "nearby_sharing_decoder.h", "share_target.h", "text_attachment.h", @@ -41,6 +41,7 @@ cc_library( "//sharing:__subpackages__", ], deps = [ + ":connection_types", "//internal/crypto_cros", "//internal/interop:authentication_status", "//internal/network:url", @@ -146,6 +147,7 @@ cc_library( "//sharing:__subpackages__", ], deps = [ + ":connection_types", ":transfer_metadata", ":types", "//connections:core", @@ -166,6 +168,7 @@ cc_library( "//sharing/common", "//sharing/common:compatible_u8_string", "//sharing/common:enum", + "//sharing/common:files", "//sharing/contacts", "//sharing/fast_initiation:nearby_fast_initiation", "//sharing/flags/generated:generated_flags", @@ -214,6 +217,7 @@ cc_library( ], visibility = ["//visibility:public"], deps = [ + ":connection_types", ":nearby_sharing_service", ":transfer_metadata", ":types", @@ -290,6 +294,7 @@ cc_test( ], shard_count = 8, deps = [ + ":connection_types", ":nearby_sharing_service", ":test_support", ":transfer_metadata", @@ -311,6 +316,7 @@ cc_test( "//sharing/common", "//sharing/common:compatible_u8_string", "//sharing/common:enum", + "//sharing/common:files", "//sharing/contacts", "//sharing/contacts:test_support", "//sharing/fast_initiation:nearby_fast_initiation", diff --git a/sharing/common/BUILD b/sharing/common/BUILD index e4a31dfa..1ce19682 100644 --- a/sharing/common/BUILD +++ b/sharing/common/BUILD @@ -61,3 +61,10 @@ cc_library( hdrs = ["compatible_u8_string.h"], visibility = ["//sharing:__subpackages__"], ) + +cc_library( + name = "files", + srcs = ["files.cc"], + hdrs = ["files.h"], + visibility = ["//visibility:public"], +) diff --git a/sharing/common/files.cc b/sharing/common/files.cc new file mode 100644 index 00000000..6198a1dc --- /dev/null +++ b/sharing/common/files.cc @@ -0,0 +1,63 @@ +// Copyright 2024 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 "sharing/common/files.h" + +#include +#include // NOLINT(build/c++17) +#include +#include // NOLINT(build/c++11) + +namespace nearby::sharing { + +bool FileExists(const std::filesystem::path& path) { + std::error_code error_code; + if (std::filesystem::exists(path, error_code) && + !std::filesystem::is_directory(path, error_code)) { + // is_directory returns false on error. + return (!error_code); + } + return false; +} + +std::optional GetFileSize(const std::filesystem::path& path) { + if (!FileExists(path)) { + return std::nullopt; + } + std::error_code error_code; + uintmax_t size = std::filesystem::file_size(path, error_code); + if (size == static_cast(-1)) { + return std::nullopt; + } + return size; +} + +bool DirectoryExists(const std::filesystem::path& path) { + std::error_code error_code; + if (std::filesystem::exists(path, error_code) && + std::filesystem::is_directory(path, error_code)) { + return true; + } + return false; +} + +bool RemoveFile(const std::filesystem::path& path) { + if (!FileExists(path)) { + return false; + } + std::error_code error_code; + return std::filesystem::remove(path, error_code); +} + +} // namespace nearby::sharing diff --git a/sharing/common/files.h b/sharing/common/files.h new file mode 100644 index 00000000..4b9672a6 --- /dev/null +++ b/sharing/common/files.h @@ -0,0 +1,41 @@ +// Copyright 2024 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 THIRD_PARTY_NEARBY_SHARING_COMMON_FILES_H_ +#define THIRD_PARTY_NEARBY_SHARING_COMMON_FILES_H_ + +#include +#include // NOLINT(build/c++17) +#include + +// This file contains exception safe wrappers to access common std::filesystem +// functions. +namespace nearby::sharing { + +// Returns true if path exists and is not a directory. +bool FileExists(const std::filesystem::path& path); + +// Returns the size of the file at path, or nullopt if not found or not a file. +std::optional GetFileSize(const std::filesystem::path& path); + +// Returns true if path exists and is a directory. +bool DirectoryExists(const std::filesystem::path& path); + +// Removes the file at path and returns true. +// Returns false if path does not exist, is not a file or cannot be removed. +bool RemoveFile(const std::filesystem::path& path); + +} // namespace nearby::sharing + +#endif // THIRD_PARTY_NEARBY_SHARING_COMMON_FILES_H_ diff --git a/sharing/nearby_connections_types.h b/sharing/nearby_connections_types.h index 903cdb8e..141f4996 100644 --- a/sharing/nearby_connections_types.h +++ b/sharing/nearby_connections_types.h @@ -29,6 +29,7 @@ #include "internal/crypto_cros/random.h" #include "internal/interop/authentication_status.h" #include "sharing/common/compatible_u8_string.h" +#include "sharing/common/files.h" namespace nearby { namespace sharing { @@ -439,8 +440,9 @@ struct Payload { id = std::hash()(GetCompatibleU8String(file.path.u8string())); content.type = PayloadContent::Type::kFile; - if (std::filesystem::exists(file.path)) { - content.file_payload.size = std::filesystem::file_size(file.path); + std::optional size = GetFileSize(file.path); + if (size.has_value()) { + content.file_payload.size = *size; } content.file_payload.file = std::move(file); @@ -456,8 +458,9 @@ struct Payload { absl::string_view parent_folder = absl::string_view()) : id(id) { content.type = PayloadContent::Type::kFile; - if (std::filesystem::exists(file.path)) { - content.file_payload.size = std::filesystem::file_size(file.path); + std::optional size = GetFileSize(file.path); + if (size.has_value()) { + content.file_payload.size = *size; } content.file_payload.file = std::move(file); diff --git a/sharing/nearby_file_handler.cc b/sharing/nearby_file_handler.cc index e2451ed4..dbf2f555 100644 --- a/sharing/nearby_file_handler.cc +++ b/sharing/nearby_file_handler.cc @@ -18,6 +18,7 @@ #include // NOLINT(build/c++17) #include #include +#include #include #include @@ -26,6 +27,7 @@ #include "absl/types/span.h" #include "internal/platform/task_runner_impl.h" #include "sharing/common/compatible_u8_string.h" +#include "sharing/common/files.h" #include "sharing/internal/public/logging.h" namespace nearby { @@ -37,16 +39,13 @@ std::vector DoOpenFiles( absl::Span file_paths) { std::vector files; for (const auto& file_path : file_paths) { - if (!std::filesystem::exists(file_path)) { + std::optional size = GetFileSize(file_path); + if (!size.has_value()) { NL_LOG(ERROR) << __func__ << ": Failed to open file. File=" << GetCompatibleU8String(file_path.u8string()); return {}; } - - int64_t size = std::filesystem::file_size(file_path); - if (size < 0) return {}; - - files.push_back({size, file_path}); + files.push_back({*size, file_path}); } return files; } @@ -85,19 +84,6 @@ void NearbyFileHandler::GetUniquePath(const std::filesystem::path& file_path, }); } -bool RemoveFile(const std::filesystem::path file) noexcept { - try { - if (!std::filesystem::remove(file)) { - return false; - } - } catch (std::exception) { - return false; - } catch (...) { - return false; - } - return true; -} - void NearbyFileHandler::DeleteFilesFromDisk( std::vector file_paths, DeleteFilesFromDiskCallback callback) { diff --git a/sharing/nearby_file_handler.h b/sharing/nearby_file_handler.h index 757febc8..337f2e59 100644 --- a/sharing/nearby_file_handler.h +++ b/sharing/nearby_file_handler.h @@ -32,7 +32,7 @@ namespace sharing { class NearbyFileHandler { public: struct FileInfo { - int64_t size; + uint64_t size; std::filesystem::path file_path; }; diff --git a/sharing/nearby_file_handler_test.cc b/sharing/nearby_file_handler_test.cc index 374b48f1..5103314a 100644 --- a/sharing/nearby_file_handler_test.cc +++ b/sharing/nearby_file_handler_test.cc @@ -22,6 +22,7 @@ #include "absl/synchronization/notification.h" #include "absl/time/clock.h" #include "absl/time/time.h" +#include "sharing/common/files.h" namespace nearby { namespace sharing { @@ -36,22 +37,6 @@ bool CreateFile(std::filesystem::path file_path) { return true; } -bool DeleteFile(std::filesystem::path file_path) { - if (std::filesystem::exists(file_path)) { - return std::filesystem::remove(file_path); - } - - return true; -} - -bool ExistFile(std::filesystem::path file_path) { - if (std::filesystem::exists(file_path)) { - return true; - } - - return false; -} - TEST(NearbyFileHandler, GetUniquePath) { NearbyFileHandler nearby_file_handler; std::filesystem::path unique_path; @@ -63,7 +48,7 @@ TEST(NearbyFileHandler, GetUniquePath) { std::filesystem::temp_directory_path() / "nearby_nfh_test_abc.jpg"; ASSERT_TRUE(CreateFile(test_file)); - ASSERT_TRUE(DeleteFile(expected_file)); + ASSERT_TRUE(RemoveFile(expected_file)); nearby_file_handler.GetUniquePath( test_file, [¬ification, &unique_path](std::filesystem::path path) { @@ -92,7 +77,7 @@ TEST(NearbyFileHandler, OpenFiles) { notification.WaitForNotificationWithTimeout(absl::Seconds(1)); EXPECT_EQ(result.size(), 1); - ASSERT_TRUE(DeleteFile(test_file)); + ASSERT_TRUE(RemoveFile(test_file)); } TEST(NearbyFileHandler, DeleteAFileFromDisk) { @@ -103,9 +88,9 @@ TEST(NearbyFileHandler, DeleteAFileFromDisk) { std::vector file_paths; file_paths.push_back(test_file); nearby_file_handler.DeleteFilesFromDisk(file_paths, []() {}); - ASSERT_TRUE(ExistFile(test_file)); + ASSERT_TRUE(FileExists(test_file)); absl::SleepFor(absl::Seconds(2)); - ASSERT_FALSE(ExistFile(test_file)); + ASSERT_FALSE(FileExists(test_file)); } TEST(NearbyFileHandler, DeleteMultipleFilesFromDisk) { @@ -120,13 +105,13 @@ TEST(NearbyFileHandler, DeleteMultipleFilesFromDisk) { file_paths = {test_file, test_file2, test_file3}; // Check it doesn't throw an exception. nearby_file_handler.DeleteFilesFromDisk(file_paths, []() {}); - ASSERT_FALSE(ExistFile(test_file)); - ASSERT_FALSE(ExistFile(test_file2)); - ASSERT_FALSE(ExistFile(test_file3)); + ASSERT_FALSE(FileExists(test_file)); + ASSERT_FALSE(FileExists(test_file2)); + ASSERT_FALSE(FileExists(test_file3)); absl::SleepFor(absl::Seconds(2)); - ASSERT_FALSE(ExistFile(test_file)); - ASSERT_FALSE(ExistFile(test_file2)); - ASSERT_FALSE(ExistFile(test_file3)); + ASSERT_FALSE(FileExists(test_file)); + ASSERT_FALSE(FileExists(test_file2)); + ASSERT_FALSE(FileExists(test_file3)); } TEST(NearbyFileHandler, TestCallback) { @@ -140,10 +125,10 @@ TEST(NearbyFileHandler, TestCallback) { nearby_file_handler.DeleteFilesFromDisk( file_paths, [&received_callback]() { received_callback = true; }); ASSERT_FALSE(received_callback); - ASSERT_TRUE(ExistFile(test_file)); + ASSERT_TRUE(FileExists(test_file)); absl::SleepFor(absl::Seconds(2)); ASSERT_TRUE(received_callback); - ASSERT_FALSE(ExistFile(test_file)); + ASSERT_FALSE(FileExists(test_file)); } } // namespace