Check & create database dirs before writing bags (#1133)

* Check & create dir before writing bag if necessary Signed-off-by: homalozoa <nx.tardis@gmail.com> * Add .vscode & install & log in .gitignore Signed-off-by: homalozoa <nx.tardis@gmail.com> * Port dir creation to sequential_compression_writer 1. Port dir creation to sequential_compression_writer 2. Remove dir before test db creation Signed-off-by: homalozoa <nx.tardis@gmail.com> * Remove db dir creation from verb record in ros2bag * Database directories' creation will happend while writer open database file. So there is no need to create db in record.py Signed-off-by: homalozoa <nx.tardis@gmail.com> Signed-off-by: homalozoa <nx.tardis@gmail.com>
ros2 · Oct 25, 2022 · 197b094 · 197b094
1 parent 9a74ad8
commit 197b094
Show file tree

Hide file tree

Showing 6 changed files with 54 additions and 4 deletions.
diff --git a/.gitignore b/.gitignore
@@ -1,7 +1,10 @@
 .idea/
 .ipynb_checkpoints/
+.vscode/
 cmake-build-debug/
 cmake-build-release/
-build/
+build
+install
+log
 venv/
 **/.pytest_cache/
diff --git a/ros2bag/ros2bag/verb/record.py b/ros2bag/ros2bag/verb/record.py
@@ -18,7 +18,6 @@
 
 from rclpy.qos import InvalidQoSProfileException
 from ros2bag.api import convert_yaml_to_qos_profile
-from ros2bag.api import create_bag_directory
 from ros2bag.api import print_error
 from ros2bag.verb import VerbExtension
 from ros2cli.node import NODE_NAME_PREFIX
@@ -108,8 +107,6 @@ def main(self, *, args):  # noqa: D102
             except (InvalidQoSProfileException, ValueError) as e:
                 return print_error(str(e))
 
-        create_bag_directory(uri)
-
         if args.all:
             # NOTE(hidmic): in merged install workspaces on Windows, Python entrypoint lookups
             #               combined with constrained environments (as imposed by colcon test)

diff --git a/rosbag2_compression/src/rosbag2_compression/sequential_compression_writer.cpp b/rosbag2_compression/src/rosbag2_compression/sequential_compression_writer.cpp
@@ -112,6 +112,21 @@ void SequentialCompressionWriter::open(
     converter_ = std::make_unique<rosbag2_cpp::Converter>(converter_options, converter_factory_);
   }
 
+  rcpputils::fs::path db_path(base_folder_);
+  if (db_path.is_directory()) {
+    std::stringstream error;
+    error << "Database directory already exists (" << db_path.string() <<
+      "), can't overwrite existing database";
+    throw std::runtime_error{error.str()};
+  }
+
+  bool dir_created = rcpputils::fs::create_directories(db_path);
+  if (!dir_created) {
+    std::stringstream error;
+    error << "Failed to create database directory (" << db_path.string() << ").";
+    throw std::runtime_error{error.str()};
+  }
+
   const auto storage_uri = format_storage_uri(base_folder_, 0);
   storage_ = storage_factory_->open_read_write(storage_uri, storage_options.storage_id);
   if (!storage_) {

diff --git a/rosbag2_compression/test/rosbag2_compression/test_sequential_compression_writer.cpp b/rosbag2_compression/test/rosbag2_compression/test_sequential_compression_writer.cpp
@@ -19,6 +19,8 @@
 #include <utility>
 #include <vector>
 
+#include "rcpputils/filesystem_helper.hpp"
+
 #include "rosbag2_compression/compression_options.hpp"
 #include "rosbag2_compression/sequential_compression_writer.hpp"
 
@@ -44,10 +46,19 @@ class SequentialCompressionWriterTest : public Test
     storage_options_{},
     serialization_format_{"rmw_format"}
   {
+    rcpputils::fs::path dir(storage_options_.uri);
+    rcpputils::fs::remove_all(dir);
+
     ON_CALL(*storage_factory_, open_read_write(_, _)).WillByDefault(Return(storage_));
     EXPECT_CALL(*storage_factory_, open_read_write(_, _)).Times(AtLeast(0));
   }
 
+  ~SequentialCompressionWriterTest()
+  {
+    rcpputils::fs::path dir(storage_options_.uri);
+    rcpputils::fs::remove_all(dir);
+  }
+
   std::unique_ptr<StrictMock<MockStorageFactory>> storage_factory_;
   std::shared_ptr<NiceMock<MockStorage>> storage_;
   std::shared_ptr<StrictMock<MockConverterFactory>> converter_factory_;

diff --git a/rosbag2_cpp/src/rosbag2_cpp/writers/sequential_writer.cpp b/rosbag2_cpp/src/rosbag2_cpp/writers/sequential_writer.cpp
@@ -95,6 +95,21 @@ void SequentialWriter::open(
     converter_ = std::make_unique<Converter>(converter_options, converter_factory_);
   }
 
+  rcpputils::fs::path db_path(base_folder_);
+  if (db_path.is_directory()) {
+    std::stringstream error;
+    error << "Database directory already exists (" << db_path.string() <<
+      "), can't overwrite existing database";
+    throw std::runtime_error{error.str()};
+  }
+
+  bool dir_created = rcpputils::fs::create_directories(db_path);
+  if (!dir_created) {
+    std::stringstream error;
+    error << "Failed to create database directory (" << db_path.string() << ").";
+    throw std::runtime_error{error.str()};
+  }
+
   const auto storage_uri = format_storage_uri(base_folder_, 0);
 
   storage_ = storage_factory_->open_read_write(storage_uri, storage_options.storage_id);

diff --git a/rosbag2_cpp/test/rosbag2_cpp/test_sequential_writer.cpp b/rosbag2_cpp/test/rosbag2_cpp/test_sequential_writer.cpp
@@ -47,6 +47,9 @@ class SequentialWriterTest : public Test
     storage_options_ = rosbag2_cpp::StorageOptions{};
     storage_options_.uri = "uri";
 
+    rcpputils::fs::path dir(storage_options_.uri);
+    rcpputils::fs::remove_all(dir);
+
     ON_CALL(*storage_factory_, open_read_write(_, _)).WillByDefault(
       DoAll(
         Invoke(
@@ -59,6 +62,12 @@ class SequentialWriterTest : public Test
       *storage_factory_, open_read_write(_, _)).Times(AtLeast(0));
   }
 
+  ~SequentialWriterTest()
+  {
+    rcpputils::fs::path dir(storage_options_.uri);
+    rcpputils::fs::remove_all(dir);
+  }
+
   std::unique_ptr<StrictMock<MockStorageFactory>> storage_factory_;
   std::shared_ptr<NiceMock<MockStorage>> storage_;
   std::shared_ptr<StrictMock<MockConverterFactory>> converter_factory_;