From acd78ceadcc84937984c783939b595842ee790e9 Mon Sep 17 00:00:00 2001 From: Michael Orlov Date: Mon, 22 Jul 2024 01:31:14 -0700 Subject: [PATCH] Replace create_temp_directory with the new create_temporary_directory (#198) * Replace create_temp_directory with the new create_temporary_directory - The newly added `create_temporary_directory(..)` uses std::filesystem::path and doesn't have platform-specific code. - Also deprecated `create_temp_directory(..)` and `temp_directory_path` Signed-off-by: Michael Orlov --- include/rcpputils/filesystem_helper.hpp | 24 ++++++++++++++++++- src/filesystem_helper.cpp | 32 +++++++++++++++++++++++++ test/test_filesystem_helper.cpp | 31 ++++++++++++++---------- 3 files changed, 74 insertions(+), 13 deletions(-) diff --git a/include/rcpputils/filesystem_helper.hpp b/include/rcpputils/filesystem_helper.hpp index 872b0d0..38f99fa 100644 --- a/include/rcpputils/filesystem_helper.hpp +++ b/include/rcpputils/filesystem_helper.hpp @@ -40,6 +40,7 @@ #define RCPPUTILS__FILESYSTEM_HELPER_HPP_ #include +#include #include #include @@ -269,6 +270,7 @@ RCPPUTILS_PUBLIC bool exists(const path & path_to_check); * * \return A path to a directory for storing temporary files and directories. */ +[[deprecated("Please use std::filesystem::temp_directory_path() instead")]] RCPPUTILS_PUBLIC path temp_directory_path(); /** @@ -284,9 +286,29 @@ RCPPUTILS_PUBLIC path temp_directory_path(); * * \throws std::system_error If any OS APIs do not succeed. */ +[[deprecated("Please use rcpputils::fs::create_temporary_directory(..) instead")]] RCPPUTILS_PUBLIC path create_temp_directory( const std::string & base_name, - const path & parent_path = temp_directory_path()); + const path & parent_path = path(std::filesystem::temp_directory_path().generic_string())); + +/// \brief Construct a uniquely named temporary directory, in "parent", with format base_nameXXXXXX +/// The output, if successful, is guaranteed to be a newly-created directory. +/// The underlying implementation keeps generating paths until one that does not exist is found or +/// until the number of iterations exceeded the maximum tries. +/// This guarantees that there will be no existing files in the returned directory. +/// \param[in] base_name User-specified portion of the created directory. +/// \param[in] parent_path The parent path of the directory that will be created. +/// \param[in] max_tries The maximum number of tries to find a unique directory (default 1000) +/// \return A path to a newly created directory with base_name and a 6-character unique suffix. +/// \throws std::invalid_argument If base_name contain directory-separator defined as +/// std::filesystem::path::preferred_separator. +/// \throws std::system_error If any OS APIs do not succeed. +/// \throws std::runtime_error If the number of the iterations exceeds the maximum tries and +/// a unique directory is not found. +RCPPUTILS_PUBLIC std::filesystem::path create_temporary_directory( + const std::string & base_name, + const std::filesystem::path & parent_path = std::filesystem::temp_directory_path(), + size_t max_tries = 1000); /** * \brief Return current working directory. diff --git a/src/filesystem_helper.cpp b/src/filesystem_helper.cpp index fd25697..9556fed 100644 --- a/src/filesystem_helper.cpp +++ b/src/filesystem_helper.cpp @@ -43,8 +43,10 @@ #include #include #include +#include #include #include +#include #include #ifdef _WIN32 @@ -341,6 +343,36 @@ path create_temp_directory(const std::string & base_name, const path & parent_pa return final_path; } +std::filesystem::path create_temporary_directory( + const std::string & base_name, const std::filesystem::path & parent_path, size_t max_tries) +{ + if (base_name.find(std::filesystem::path::preferred_separator) != std::string::npos) { + throw std::invalid_argument("The base_name contain directory-separator"); + } + // mersenne twister random generator engine seeded with the std::random_device + std::mt19937 random_generator(std::random_device{}()); + std::uniform_int_distribution<> distribution(0, 0xFFFFFF); + std::filesystem::path path_to_temp_dir; + constexpr size_t kSuffixLength = 7; // 6 chars + 1 null terminator + char random_suffix_str[kSuffixLength]; + size_t current_iteration = 0; + while (true) { + snprintf(random_suffix_str, kSuffixLength, "%06x", distribution(random_generator)); + const std::string random_dir_name = base_name + random_suffix_str; + path_to_temp_dir = parent_path / random_dir_name; + // true if the directory was newly created. + if (std::filesystem::create_directories(path_to_temp_dir)) { + break; + } + if (current_iteration == max_tries) { + throw std::runtime_error( + "Exceeded maximum allowed iterations to find non-existing directory"); + } + current_iteration++; + } + return path_to_temp_dir; +} + path current_path() { #ifdef _WIN32 diff --git a/test/test_filesystem_helper.cpp b/test/test_filesystem_helper.cpp index b96b678..491ebc2 100644 --- a/test/test_filesystem_helper.cpp +++ b/test/test_filesystem_helper.cpp @@ -308,7 +308,8 @@ TEST(TestFilesystemHelper, filesystem_manipulation) EXPECT_TRUE(rcpputils::fs::remove(dir)); EXPECT_FALSE(rcpputils::fs::exists(file)); EXPECT_FALSE(rcpputils::fs::exists(dir)); - auto temp_dir = rcpputils::fs::temp_directory_path(); + auto temp_dir_std = std::filesystem::temp_directory_path(); + rcpputils::fs::path temp_dir = rcpputils::fs::path(temp_dir_std.generic_string()); temp_dir = temp_dir / "rcpputils" / "test_folder"; EXPECT_FALSE(rcpputils::fs::exists(temp_dir)); EXPECT_TRUE(rcpputils::fs::create_directories(temp_dir)); @@ -446,13 +447,14 @@ TEST(TestFilesystemHelper, stream_operator) ASSERT_EQ(s.str(), "barfoo"); } -TEST(TestFilesystemHelper, create_temp_directory) +TEST(TestFilesystemHelper, create_temporary_directory) { // basic usage { const std::string basename = "test_base_name"; - const auto tmpdir1 = rcpputils::fs::create_temp_directory(basename); + const auto tmpdir1_std = rcpputils::fs::create_temporary_directory(basename); + rcpputils::fs::path tmpdir1(tmpdir1_std.generic_string()); EXPECT_TRUE(tmpdir1.exists()); EXPECT_TRUE(tmpdir1.is_directory()); @@ -464,7 +466,8 @@ TEST(TestFilesystemHelper, create_temp_directory) EXPECT_TRUE(rcpputils::fs::exists(tmp_file)); EXPECT_TRUE(rcpputils::fs::is_regular_file(tmp_file)); - const auto tmpdir2 = rcpputils::fs::create_temp_directory(basename); + const auto tmpdir2_std = rcpputils::fs::create_temporary_directory(basename); + rcpputils::fs::path tmpdir2(tmpdir2_std.generic_string()); EXPECT_TRUE(tmpdir2.exists()); EXPECT_TRUE(tmpdir2.is_directory()); @@ -477,16 +480,21 @@ TEST(TestFilesystemHelper, create_temp_directory) // bad names { if (is_win32) { - EXPECT_THROW(rcpputils::fs::create_temp_directory("illegalchar?"), std::system_error); + EXPECT_THROW(rcpputils::fs::create_temporary_directory("illegalchar?"), std::system_error); + EXPECT_THROW(rcpputils::fs::create_temporary_directory("base\\name"), std::invalid_argument); } else { - EXPECT_THROW(rcpputils::fs::create_temp_directory("base/name"), std::system_error); + EXPECT_THROW(rcpputils::fs::create_temporary_directory("base/name"), std::invalid_argument); } } // newly created paths { const auto new_relative = rcpputils::fs::current_path() / "child1" / "child2"; - const auto tmpdir = rcpputils::fs::create_temp_directory("base_name", new_relative); + const auto tmpdir_std = rcpputils::fs::create_temporary_directory( + "base_name", + std::filesystem::path(new_relative.string())); + rcpputils::fs::path tmpdir(tmpdir_std.generic_string()); + EXPECT_TRUE(tmpdir.exists()); EXPECT_TRUE(tmpdir.is_directory()); EXPECT_TRUE(rcpputils::fs::remove_all(tmpdir)); @@ -495,15 +503,14 @@ TEST(TestFilesystemHelper, create_temp_directory) // edge case inputs { // Provided no base name we should still get a path with the 6 unique template chars - const auto tmpdir_emptybase = rcpputils::fs::create_temp_directory(""); + const auto tmpdir_emptybase = rcpputils::fs::create_temporary_directory(""); EXPECT_EQ(tmpdir_emptybase.filename().string().size(), 6u); - // Empty path doesn't exist and cannot be created - EXPECT_THROW(rcpputils::fs::create_temp_directory("basename", path()), std::system_error); - // With the template string XXXXXX already in the name, it will still be there, the unique // portion is appended to the end. - const auto tmpdir_template_in_name = rcpputils::fs::create_temp_directory("base_XXXXXX"); + const auto tmpdir_template_in_name_std = + rcpputils::fs::create_temporary_directory("base_XXXXXX"); + rcpputils::fs::path tmpdir_template_in_name(tmpdir_template_in_name_std.generic_string()); EXPECT_TRUE(tmpdir_template_in_name.exists()); EXPECT_TRUE(tmpdir_template_in_name.is_directory()); // On Linux, it will not replace the base_name Xs, only the final 6 that the function appends.