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 <[email protected]>

Author: MichaelOrlov

include/rcpputils/filesystem_helper.hpp

@@ -40,6 +40,7 @@
 #define RCPPUTILS__FILESYSTEM_HELPER_HPP_
 
 #include <cstdint>
+#include <filesystem>
 #include <string>
 #include <vector>
 
@@ -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.

src/filesystem_helper.cpp

@@ -43,8 +43,10 @@
 #include <algorithm>
 #include <climits>
 #include <cstring>
+#include <random>
 #include <string>
 #include <system_error>
+#include <stdexcept>
 #include <vector>
 
 #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

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.