Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove rosbag2 filesystem helper #249

Merged
merged 2 commits into from
Jan 10, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@
#include <string>
#include <vector>

#include "rosbag2_compression/zstd_compressor.hpp"
#include "rcutils/filesystem.h"

#include "rosbag2_storage/filesystem_helper.hpp"
#include "rosbag2_compression/zstd_compressor.hpp"

#include "logging.hpp"

Expand Down Expand Up @@ -65,8 +65,7 @@ std::vector<uint8_t> get_input_buffer(const std::string & uri)
throw std::runtime_error{"Error opening file"};
}

const auto decompressed_buffer_length =
rosbag2_storage::FilesystemHelper::get_file_size(uri);
const auto decompressed_buffer_length = rcutils_get_file_size(uri.c_str());

if (decompressed_buffer_length == 0) {
fclose(file_pointer);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@
#include <string>
#include <vector>

#include "rcutils/filesystem.h"

#include "rcpputils/filesystem_helper.hpp"

#include "rosbag2_compression/zstd_decompressor.hpp"

#include "rosbag2_storage/filesystem_helper.hpp"

#include "logging.hpp"

namespace
Expand Down Expand Up @@ -68,7 +68,7 @@ std::vector<uint8_t> get_input_buffer(const std::string & uri)
throw std::runtime_error{errmsg.str()};
}

const auto compressed_buffer_length = rosbag2_storage::FilesystemHelper::get_file_size(uri);
const auto compressed_buffer_length = rcutils_get_file_size(uri.c_str());
if (compressed_buffer_length == 0) {
fclose(file_pointer);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,14 @@
#include <vector>

#include "rclcpp/rclcpp.hpp"

#include "rcpputils/filesystem_helper.hpp"

#include "rcutils/filesystem.h"

#include "rosbag2_compression/zstd_compressor.hpp"
#include "rosbag2_compression/zstd_decompressor.hpp"
#include "rosbag2_storage/filesystem_helper.hpp"

#include "rosbag2_test_common/temporary_directory_fixture.hpp"

#include "gmock/gmock.h"
Expand Down Expand Up @@ -84,28 +89,27 @@ class CompressionHelperFixture : public rosbag2_test_common::TemporaryDirectoryF

TEST_F(CompressionHelperFixture, zstd_compress_file_uri)
{
const auto uri = rosbag2_storage::FilesystemHelper::concat({temporary_dir_path_, "file1.txt"});
const auto uri = (rcpputils::fs::path(temporary_dir_path_) / "file1.txt").string();
create_garbage_file(uri);
auto zstd_compressor = rosbag2_compression::ZstdCompressor{};
const auto compressed_uri = zstd_compressor.compress_uri(uri);

const auto expected_compressed_uri = uri + "." + zstd_compressor.get_compression_identifier();
const auto uncompressed_file_size = rosbag2_storage::FilesystemHelper::get_file_size(uri);
const auto compressed_file_size =
rosbag2_storage::FilesystemHelper::get_file_size(compressed_uri);
const auto uncompressed_file_size = rcutils_get_file_size(uri.c_str());
const auto compressed_file_size = rcutils_get_file_size(compressed_uri.c_str());

EXPECT_NE(compressed_uri, uri);
EXPECT_EQ(compressed_uri, expected_compressed_uri);
EXPECT_LT(compressed_file_size, uncompressed_file_size);
EXPECT_GT(compressed_file_size, 0u);
EXPECT_TRUE(rosbag2_storage::FilesystemHelper::file_exists(compressed_uri));
EXPECT_TRUE(rcpputils::fs::path(compressed_uri).exists());
}

TEST_F(CompressionHelperFixture, zstd_decompress_file_uri)
{
const auto uri = rosbag2_storage::FilesystemHelper::concat({temporary_dir_path_, "file1.txt"});
const auto uri = (rcpputils::fs::path(temporary_dir_path_) / "file1.txt").string();
create_garbage_file(uri);
const auto initial_file_size = rosbag2_storage::FilesystemHelper::get_file_size(uri);
const auto initial_file_size = rcutils_get_file_size(uri.c_str());

auto zstd_compressor = rosbag2_compression::ZstdCompressor{};
const auto compressed_uri = zstd_compressor.compress_uri(uri);
Expand All @@ -116,19 +120,18 @@ TEST_F(CompressionHelperFixture, zstd_decompress_file_uri)
const auto decompressed_uri = zstd_decompressor.decompress_uri(compressed_uri);

const auto expected_decompressed_uri = uri;
const auto decompressed_file_size =
rosbag2_storage::FilesystemHelper::get_file_size(decompressed_uri);
const auto decompressed_file_size = rcutils_get_file_size(decompressed_uri.c_str());

EXPECT_NE(compressed_uri, uri);
EXPECT_NE(decompressed_uri, compressed_uri);
EXPECT_EQ(uri, expected_decompressed_uri);
EXPECT_EQ(initial_file_size, decompressed_file_size);
EXPECT_TRUE(rosbag2_storage::FilesystemHelper::file_exists(decompressed_uri));
EXPECT_TRUE(rcpputils::fs::path(decompressed_uri).exists());
}

TEST_F(CompressionHelperFixture, zstd_decompress_file_contents)
{
const auto uri = rosbag2_storage::FilesystemHelper::concat({temporary_dir_path_, "file2.txt"});
const auto uri = (rcpputils::fs::path(temporary_dir_path_) / "file2.txt").string();
create_garbage_file(uri);

auto compressor = rosbag2_compression::ZstdCompressor{};
Expand All @@ -138,10 +141,9 @@ TEST_F(CompressionHelperFixture, zstd_decompress_file_contents)
const auto decompressed_uri = decompressor.decompress_uri(compressed_uri);

const auto initial_data = read_file(uri);
const auto initial_file_size = rosbag2_storage::FilesystemHelper::get_file_size(uri);
const auto initial_file_size = rcutils_get_file_size(uri.c_str());
const auto decompressed_data = read_file(decompressed_uri);
const auto decompressed_file_size =
rosbag2_storage::FilesystemHelper::get_file_size(decompressed_uri);
const auto decompressed_file_size = rcutils_get_file_size(decompressed_uri.c_str());

EXPECT_EQ(
initial_data.size() * sizeof(decltype(initial_data)::value_type),
Expand All @@ -154,7 +156,7 @@ TEST_F(CompressionHelperFixture, zstd_decompress_file_contents)

TEST_F(CompressionHelperFixture, zstd_decompress_fails_on_bad_file)
{
const auto uri = rosbag2_storage::FilesystemHelper::concat({temporary_dir_path_, "file3.txt"});
const auto uri = (rcpputils::fs::path(temporary_dir_path_) / "file3.txt").string();
create_garbage_file(uri);

auto decompressor = rosbag2_compression::ZstdDecompressor{};
Expand All @@ -163,8 +165,7 @@ TEST_F(CompressionHelperFixture, zstd_decompress_fails_on_bad_file)

TEST_F(CompressionHelperFixture, zstd_decompress_fails_on_bad_uri)
{
const auto bad_uri =
rosbag2_storage::FilesystemHelper::concat({temporary_dir_path_, "bad_uri.txt"});
const auto bad_uri = (rcpputils::fs::path(temporary_dir_path_) / "bad_uri.txt").string();
auto decompressor = rosbag2_compression::ZstdDecompressor{};

EXPECT_THROW(decompressor.decompress_uri(bad_uri), std::runtime_error);
Expand Down
1 change: 0 additions & 1 deletion rosbag2_cpp/src/rosbag2_cpp/info.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
#include <stdexcept>
#include <string>

#include "rosbag2_storage/filesystem_helper.hpp"
#include "rosbag2_storage/metadata_io.hpp"
#include "rosbag2_storage/storage_factory.hpp"

Expand Down
1 change: 0 additions & 1 deletion rosbag2_cpp/src/rosbag2_cpp/writer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
#include "rosbag2_cpp/storage_options.hpp"
#include "rosbag2_cpp/writer_interfaces/base_writer_interface.hpp"

#include "rosbag2_storage/filesystem_helper.hpp"
#include "rosbag2_storage/serialized_bag_message.hpp"
#include "rosbag2_storage/topic_metadata.hpp"

Expand Down
13 changes: 7 additions & 6 deletions rosbag2_cpp/src/rosbag2_cpp/writers/sequential_writer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,13 @@
#include <string>
#include <utility>

#include "rcpputils/filesystem_helper.hpp"

#include "rcutils/filesystem.h"

#include "rosbag2_cpp/info.hpp"
#include "rosbag2_cpp/storage_options.hpp"

#include "rosbag2_storage/filesystem_helper.hpp"

namespace rosbag2_cpp
{
namespace writers
Expand All @@ -39,10 +41,9 @@ std::string format_storage_uri(const std::string & base_folder, uint64_t storage
// The name of the folder needs to be queried in case
// SequentialWriter is opened with a relative path.
std::stringstream storage_file_name;
storage_file_name << rosbag2_storage::FilesystemHelper::get_folder_name(base_folder) <<
"_" << storage_count;
storage_file_name << rcpputils::fs::path(base_folder).filename().string() << "_" << storage_count;

return rosbag2_storage::FilesystemHelper::concat({base_folder, storage_file_name.str()});
return (rcpputils::fs::path(base_folder) / storage_file_name.str()).string();
}
} // namespace

Expand Down Expand Up @@ -221,7 +222,7 @@ void SequentialWriter::finalize_metadata()
metadata_.bag_size = 0;

for (const auto & path : metadata_.relative_file_paths) {
metadata_.bag_size += rosbag2_storage::FilesystemHelper::get_file_size(path);
metadata_.bag_size += rcutils_get_file_size(path.c_str());
}

metadata_.topics_with_message_count.clear();
Expand Down
18 changes: 8 additions & 10 deletions rosbag2_cpp/test/rosbag2_cpp/test_info.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,12 @@
#include <string>
#include <vector>

#include "rcpputils/filesystem_helper.hpp"

#include "rosbag2_cpp/info.hpp"
#include "rosbag2_cpp/writer.hpp"

#include "rosbag2_storage/bag_metadata.hpp"
#include "rosbag2_storage/filesystem_helper.hpp"
#include "rosbag2_storage/metadata_io.hpp"

#include "rosbag2_test_common/temporary_directory_fixture.hpp"
Expand Down Expand Up @@ -56,11 +57,9 @@ TEST_F(TemporaryDirectoryFixture, read_metadata_supports_version_2) {
" message_count: 200\n";

{
const auto bagfile_path = rosbag2_storage::FilesystemHelper::concat({
temporary_dir_path_, rosbag2_storage::MetadataIo::metadata_filename
});

std::ofstream fout {bagfile_path};
std::ofstream fout {
(rcpputils::fs::path(temporary_dir_path_) / rosbag2_storage::MetadataIo::metadata_filename)
.string()};
fout << bagfile;
}

Expand Down Expand Up @@ -127,10 +126,9 @@ TEST_F(TemporaryDirectoryFixture, read_metadata_makes_appropriate_call_to_metada
" compression_format: \"zstd\"\n"
" compression_mode: \"FILE\"\n");

std::ofstream fout(
rosbag2_storage::FilesystemHelper::concat({
temporary_dir_path_, rosbag2_storage::MetadataIo::metadata_filename
}));
std::ofstream fout {
(rcpputils::fs::path(temporary_dir_path_) / rosbag2_storage::MetadataIo::metadata_filename)
.string()};
fout << bagfile;
fout.close();

Expand Down
7 changes: 4 additions & 3 deletions rosbag2_cpp/test/rosbag2_cpp/test_sequential_writer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,12 @@
#include <utility>
#include <vector>

#include "rcpputils/filesystem_helper.hpp"

#include "rosbag2_cpp/writers/sequential_writer.hpp"
#include "rosbag2_cpp/writer.hpp"

#include "rosbag2_storage/bag_metadata.hpp"
#include "rosbag2_storage/filesystem_helper.hpp"
#include "rosbag2_storage/topic_metadata.hpp"

#include "mock_converter.hpp"
Expand Down Expand Up @@ -237,8 +238,8 @@ TEST_F(SequentialWriterTest, writer_splits_when_storage_bagfile_size_gt_max_bagf
static_cast<unsigned int>(expected_splits)) <<
"Storage should have split bagfile " << (expected_splits - 1);

const auto base_path = rosbag2_storage::FilesystemHelper::concat({
storage_options_.uri, storage_options_.uri});
const auto base_path =
(rcpputils::fs::path(storage_options_.uri) / storage_options_.uri).string();
int counter = 0;
for (const auto & path : fake_metadata_.relative_file_paths) {
std::stringstream ss;
Expand Down
10 changes: 2 additions & 8 deletions rosbag2_storage/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ endif()

find_package(ament_cmake REQUIRED)
find_package(pluginlib REQUIRED)
find_package(rcpputils REQUIRED)
find_package(rcutils REQUIRED)
find_package(yaml_cpp_vendor REQUIRED)

Expand All @@ -36,6 +37,7 @@ target_include_directories(rosbag2_storage PUBLIC include)
ament_target_dependencies(
rosbag2_storage
pluginlib
rcpputils
rcutils
yaml_cpp_vendor)

Expand Down Expand Up @@ -105,14 +107,6 @@ if(BUILD_TESTING)
target_link_libraries(test_metadata_serialization rosbag2_storage)
ament_target_dependencies(test_metadata_serialization rosbag2_test_common)
endif()

ament_add_gmock(test_filesystem_helper
test/rosbag2_storage/test_filesystem_helper.cpp)
if(TARGET test_filesystem_helper)
target_include_directories(test_filesystem_helper PRIVATE include)
target_link_libraries(test_filesystem_helper rosbag2_storage)
ament_target_dependencies(test_filesystem_helper rosbag2_test_common)
endif()
endif()

ament_package(
Expand Down
Loading