From 1e08b2d886849b60beb51fe3672ac8e44de5cee9 Mon Sep 17 00:00:00 2001 From: Francis Tsui Date: Fri, 29 Mar 2024 17:31:39 -0700 Subject: [PATCH] Fixed remaining filesystem calls that throws exceptions. PiperOrigin-RevId: 620379472 --- internal/base/BUILD | 7 ++++ {sharing/common => internal/base}/files.cc | 36 ++++++++++++++++++- {sharing/common => internal/base}/files.h | 20 +++++++++-- internal/platform/BUILD | 1 + internal/platform/device_info_impl.cc | 15 +++++--- .../platform/implementation/windows/BUILD | 2 ++ .../implementation/windows/device_info.cc | 5 +-- .../windows/preferences_manager.cc | 4 ++- .../windows/preferences_repository.cc | 20 +++++++---- sharing/BUILD | 6 ++-- sharing/common/BUILD | 7 ---- sharing/nearby_connections_types.h | 2 +- sharing/nearby_file_handler.cc | 2 +- sharing/nearby_file_handler_test.cc | 2 +- 14 files changed, 98 insertions(+), 31 deletions(-) rename {sharing/common => internal/base}/files.cc (66%) rename {sharing/common => internal/base}/files.h (67%) diff --git a/internal/base/BUILD b/internal/base/BUILD index 99450c74..4bc8608d 100644 --- a/internal/base/BUILD +++ b/internal/base/BUILD @@ -52,6 +52,13 @@ cc_library( ], ) +cc_library( + name = "files", + srcs = ["files.cc"], + hdrs = ["files.h"], + visibility = ["//visibility:public"], +) + cc_test( name = "base_test", size = "small", diff --git a/sharing/common/files.cc b/internal/base/files.cc similarity index 66% rename from sharing/common/files.cc rename to internal/base/files.cc index 6198a1dc..83620d85 100644 --- a/sharing/common/files.cc +++ b/internal/base/files.cc @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "sharing/common/files.h" +#include "internal/base/files.h" #include #include // NOLINT(build/c++17) @@ -60,4 +60,38 @@ bool RemoveFile(const std::filesystem::path& path) { return std::filesystem::remove(path, error_code); } +std::optional GetTemporaryDirectory() { + std::error_code error_code; + std::filesystem::path temp_dir = + std::filesystem::temp_directory_path(error_code); + if (temp_dir.empty()) { + return std::nullopt; + } + return temp_dir; +} + +std::filesystem::path CurrentDirectory() { + // temp_directory_path() returns empty path on error. + std::error_code error_code; + return std::filesystem::current_path(error_code); +} + +bool Rename(std::filesystem::path old_path, std::filesystem::path new_path) { + std::error_code error_code; + std::filesystem::rename(old_path, new_path, error_code); + if (error_code) { + return false; + } + return true; +} + +bool CreateDirectories(std::filesystem::path path) { + std::error_code error_code; + std::filesystem::create_directories(path, error_code); + if (error_code) { + return false; + } + return true; +} + } // namespace nearby::sharing diff --git a/sharing/common/files.h b/internal/base/files.h similarity index 67% rename from sharing/common/files.h rename to internal/base/files.h index 4b9672a6..8970834f 100644 --- a/sharing/common/files.h +++ b/internal/base/files.h @@ -12,8 +12,8 @@ // 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_ +#ifndef THIRD_PARTY_NEARBY_INTERNAL_BASE_FILES_H_ +#define THIRD_PARTY_NEARBY_INTERNAL_BASE_FILES_H_ #include #include // NOLINT(build/c++17) @@ -36,6 +36,20 @@ bool DirectoryExists(const std::filesystem::path& path); // Returns false if path does not exist, is not a file or cannot be removed. bool RemoveFile(const std::filesystem::path& path); +// Returns path to a temporary directory if available. +std::optional GetTemporaryDirectory(); + +// Returns path to the current directory. On failure returns an empty path. +std::filesystem::path CurrentDirectory(); + +// Renames the file at old_path to new_path. +// Returns true on success. +bool Rename(std::filesystem::path old_path, std::filesystem::path new_path); + +// Creates all directory leading to path. +// Returns true on success. +bool CreateDirectories(std::filesystem::path path); + } // namespace nearby::sharing -#endif // THIRD_PARTY_NEARBY_SHARING_COMMON_FILES_H_ +#endif // THIRD_PARTY_NEARBY_INTERNAL_BASE_FILES_H_ diff --git a/internal/platform/BUILD b/internal/platform/BUILD index a978dc61..7a2571ec 100644 --- a/internal/platform/BUILD +++ b/internal/platform/BUILD @@ -352,6 +352,7 @@ cc_library( deps = [ ":base", ":util", + "//internal/base:files", "//internal/crypto_cros", "//internal/platform/implementation:platform", "//internal/platform/implementation:types", diff --git a/internal/platform/device_info_impl.cc b/internal/platform/device_info_impl.cc index 4b18f66a..ff2bab20 100644 --- a/internal/platform/device_info_impl.cc +++ b/internal/platform/device_info_impl.cc @@ -14,10 +14,14 @@ #include "internal/platform/device_info_impl.h" +#include +#include // NOLINT #include #include #include -#include +#include "absl/strings/string_view.h" +#include "internal/base/files.h" +#include "internal/platform/implementation/device_info.h" namespace nearby { @@ -61,7 +65,8 @@ std::filesystem::path DeviceInfoImpl::GetDownloadPath() const { if (path.has_value()) { return *path; } - return std::filesystem::temp_directory_path(); + return nearby::sharing::GetTemporaryDirectory().value_or( + nearby::sharing::CurrentDirectory()); } std::filesystem::path DeviceInfoImpl::GetAppDataPath() const { @@ -70,7 +75,8 @@ std::filesystem::path DeviceInfoImpl::GetAppDataPath() const { if (path.has_value()) { return *path; } - return std::filesystem::temp_directory_path(); + return nearby::sharing::GetTemporaryDirectory().value_or( + nearby::sharing::CurrentDirectory()); } std::filesystem::path DeviceInfoImpl::GetTemporaryPath() const { @@ -79,7 +85,8 @@ std::filesystem::path DeviceInfoImpl::GetTemporaryPath() const { if (path.has_value()) { return *path; } - return std::filesystem::temp_directory_path(); + return nearby::sharing::GetTemporaryDirectory().value_or( + nearby::sharing::CurrentDirectory()); } std::optional DeviceInfoImpl::GetAvailableDiskSpaceInBytes( diff --git a/internal/platform/implementation/windows/BUILD b/internal/platform/implementation/windows/BUILD index 9d148304..e13c221a 100644 --- a/internal/platform/implementation/windows/BUILD +++ b/internal/platform/implementation/windows/BUILD @@ -53,6 +53,7 @@ cc_library( "//base", "//base:stringprintf", "//internal/base:bluetooth_address", + "//internal/base:files", "//internal/platform:base", "//internal/platform:types", "//internal/platform:uuid", @@ -214,6 +215,7 @@ cc_library( ":crypto", # build_cleaner: keep ":types", "//internal/account", + "//internal/base:files", "//internal/flags:nearby_flags", "//internal/platform:base", "//internal/platform:cancellation_flag", diff --git a/internal/platform/implementation/windows/device_info.cc b/internal/platform/implementation/windows/device_info.cc index eb21b2b9..01aa4d6a 100644 --- a/internal/platform/implementation/windows/device_info.cc +++ b/internal/platform/implementation/windows/device_info.cc @@ -18,13 +18,14 @@ #include #include -#include +#include // NOLINT #include #include #include #include "absl/strings/string_view.h" #include "absl/synchronization/mutex.h" +#include "internal/base/files.h" #include "internal/platform/implementation/device_info.h" #include "internal/platform/implementation/windows/generated/winrt/base.h" #include "internal/platform/logging.h" @@ -300,7 +301,7 @@ std::optional DeviceInfo::GetCommonAppDataPath() const { } std::optional DeviceInfo::GetTemporaryPath() const { - return std::filesystem::temp_directory_path(); + return nearby::sharing::GetTemporaryDirectory(); } std::optional DeviceInfo::GetLogPath() const { diff --git a/internal/platform/implementation/windows/preferences_manager.cc b/internal/platform/implementation/windows/preferences_manager.cc index 0e89d26d..3d0e95ac 100644 --- a/internal/platform/implementation/windows/preferences_manager.cc +++ b/internal/platform/implementation/windows/preferences_manager.cc @@ -24,6 +24,7 @@ #include "absl/strings/string_view.h" #include "nlohmann/json.hpp" #include "nlohmann/json_fwd.hpp" +#include "internal/base/files.h" #include "internal/platform/implementation/windows/preferences_repository.h" #include "internal/platform/logging.h" @@ -39,7 +40,8 @@ PreferencesManager::PreferencesManager(absl::string_view file_path) nearby::api::ImplementationPlatform::CreateDeviceInfo() ->GetLocalAppDataPath(); if (!path.has_value()) { - path = std::filesystem::temp_directory_path(); + path = nearby::sharing::GetTemporaryDirectory().value_or( + nearby::sharing::CurrentDirectory()); } std::filesystem::path full_path = *path / std::string(file_path); diff --git a/internal/platform/implementation/windows/preferences_repository.cc b/internal/platform/implementation/windows/preferences_repository.cc index 3a5220e2..1bf94500 100644 --- a/internal/platform/implementation/windows/preferences_repository.cc +++ b/internal/platform/implementation/windows/preferences_repository.cc @@ -21,6 +21,7 @@ #include "nlohmann/json.hpp" #include "nlohmann/json_fwd.hpp" +#include "internal/base/files.h" #include "internal/platform/logging.h" namespace nearby { @@ -68,8 +69,8 @@ bool PreferencesRepository::SavePreferences(json preferences) { absl::MutexLock lock(&mutex_); try { std::filesystem::path path = path_; - if (!std::filesystem::exists(path) && - !std::filesystem::create_directories(path)) { + if (!nearby::sharing::FileExists(path) && + !nearby::sharing::CreateDirectories(path)) { NEARBY_LOGS(ERROR) << "Failed to create preferences path."; return false; } @@ -78,9 +79,11 @@ bool PreferencesRepository::SavePreferences(json preferences) { std::filesystem::path full_name_backup = path / kPreferencesBackupFileName; // Create a backup without moving the bytes on disk - if (std::filesystem::exists(full_name)) { + if (nearby::sharing::FileExists(full_name)) { NEARBY_LOGS(INFO) << "Making backup of preferences file."; - std::filesystem::rename(full_name, full_name_backup); + if (!nearby::sharing::Rename(full_name, full_name_backup)) { + NEARBY_LOGS(ERROR) << "Failed to rename preferences backup file."; + } } std::ofstream preferences_file(full_name.c_str()); @@ -111,7 +114,8 @@ bool PreferencesRepository::SavePreferences(json preferences) { std::optional PreferencesRepository::AttemptLoad() { std::filesystem::path path = path_; std::filesystem::path full_name = path / kPreferencesFileName; - if (!std::filesystem::exists(path) || !std::filesystem::exists(full_name)) { + if (!nearby::sharing::FileExists(path) || + !nearby::sharing::FileExists(full_name)) { return std::nullopt; } @@ -144,13 +148,15 @@ std::optional PreferencesRepository::RestoreFromBackup() { std::filesystem::path full_name = path / kPreferencesFileName; std::filesystem::path full_name_backup = path / kPreferencesBackupFileName; - if (!std::filesystem::exists(full_name_backup)) { + if (!nearby::sharing::FileExists(full_name_backup)) { NEARBY_LOGS(WARNING) << "Backup requested but no backup preferences file found."; return std::nullopt; } - std::filesystem::rename(full_name_backup, full_name); + if (!nearby::sharing::Rename(full_name_backup, full_name)) { + NEARBY_LOGS(ERROR) << "Failed to rename preferences backup file."; + } NEARBY_LOGS(INFO) << "Attempting load from backup preferences."; return AttemptLoad(); diff --git a/sharing/BUILD b/sharing/BUILD index e33c189e..4f53c76c 100644 --- a/sharing/BUILD +++ b/sharing/BUILD @@ -4,10 +4,10 @@ cc_library( name = "connection_types", hdrs = ["nearby_connections_types.h"], deps = [ + "//internal/base:files", "//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", ], @@ -156,6 +156,7 @@ cc_library( "//internal/analytics:event_logger", "//internal/base", "//internal/base:bluetooth_address", + "//internal/base:files", "//internal/flags:nearby_flags", "//internal/network:url", "//internal/platform:base", @@ -168,7 +169,6 @@ 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", @@ -303,6 +303,7 @@ cc_test( "//connections:core_types", "//internal/account", "//internal/analytics:event_logger", + "//internal/base:files", "//internal/flags:nearby_flags", "//internal/network:types", "//internal/network:url", @@ -316,7 +317,6 @@ 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 1ce19682..e4a31dfa 100644 --- a/sharing/common/BUILD +++ b/sharing/common/BUILD @@ -61,10 +61,3 @@ 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/nearby_connections_types.h b/sharing/nearby_connections_types.h index 141f4996..83a34a99 100644 --- a/sharing/nearby_connections_types.h +++ b/sharing/nearby_connections_types.h @@ -26,10 +26,10 @@ #include "absl/strings/string_view.h" #include "absl/time/time.h" +#include "internal/base/files.h" #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 { diff --git a/sharing/nearby_file_handler.cc b/sharing/nearby_file_handler.cc index dbf2f555..7502d153 100644 --- a/sharing/nearby_file_handler.cc +++ b/sharing/nearby_file_handler.cc @@ -25,9 +25,9 @@ #include "absl/time/clock.h" #include "absl/time/time.h" #include "absl/types/span.h" +#include "internal/base/files.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 { diff --git a/sharing/nearby_file_handler_test.cc b/sharing/nearby_file_handler_test.cc index 5103314a..c694d73f 100644 --- a/sharing/nearby_file_handler_test.cc +++ b/sharing/nearby_file_handler_test.cc @@ -22,7 +22,7 @@ #include "absl/synchronization/notification.h" #include "absl/time/clock.h" #include "absl/time/time.h" -#include "sharing/common/files.h" +#include "internal/base/files.h" namespace nearby { namespace sharing {