From 741888a88ce44f2400d3c434e608ba1d18184bef Mon Sep 17 00:00:00 2001 From: vibhatsu Date: Sun, 27 Apr 2025 03:48:56 +0530 Subject: [PATCH] refactor(file-operations): update get_file_info and list_directory to use FileInfo struct for improved file metadata handling Signed-off-by: vibhatsu --- include/common/file_operations.hpp | 5 +- src/common/file_operations.cpp | 69 ++++++++++++++++--- .../unittests/common/file_operations_test.cpp | 60 ++++++++++------ 3 files changed, 101 insertions(+), 33 deletions(-) diff --git a/include/common/file_operations.hpp b/include/common/file_operations.hpp index 6f6c4e0..82b2eea 100644 --- a/include/common/file_operations.hpp +++ b/include/common/file_operations.hpp @@ -1,6 +1,7 @@ #ifndef FENRIS_COMMON_FILE_OPERATIONS_HPP #define FENRIS_COMMON_FILE_OPERATIONS_HPP +#include "fenris.pb.h" #include #include #include @@ -88,7 +89,7 @@ FileOperationResult delete_file(const std::string &filepath); * @param filepath Path to the file * @return Pair of (filesystem::file_status, FileOperationResult) */ -std::pair +std::pair get_file_info(const std::string &filepath); /** @@ -132,7 +133,7 @@ FileOperationResult delete_directory(const std::string &dirpath, * @param dirpath Path of the directory to list * @return Pair of (vector of entry names, FileOperationResult) */ -std::pair, FileOperationResult> +std::pair, FileOperationResult> list_directory(const std::string &dirpath); /** diff --git a/src/common/file_operations.cpp b/src/common/file_operations.cpp index 1760fcd..cfc729e 100644 --- a/src/common/file_operations.cpp +++ b/src/common/file_operations.cpp @@ -249,17 +249,60 @@ FileOperationResult delete_file(const std::string &filepath) return FileOperationResult::SUCCESS; } -std::pair +std::pair get_file_info(const std::string &filepath) { std::error_code ec; - fs::file_status status = fs::status(filepath, ec); + FileInfo file_info; + + // Check if file exists + if (!fs::exists(filepath, ec)) { + return {file_info, FileOperationResult::FILE_NOT_FOUND}; + } + + // Set file name + file_info.set_name(filepath); + // Set file size (for regular files) + if (fs::is_regular_file(filepath, ec)) { + if (ec) { + return {file_info, system_error_to_file_operation_result(ec)}; + } + + uintmax_t size = fs::file_size(filepath, ec); + if (ec) { + return {file_info, system_error_to_file_operation_result(ec)}; + } + file_info.set_size(size); + } else { + // For directories or other types, size is 0 + file_info.set_size(0); + } + + // Check if it's a directory + file_info.set_is_directory(fs::is_directory(filepath, ec)); + if (ec) { + return {file_info, system_error_to_file_operation_result(ec)}; + } + + // Get last modified time + auto last_write = fs::last_write_time(filepath, ec); if (ec) { - return {status, system_error_to_file_operation_result(ec)}; + return {file_info, system_error_to_file_operation_result(ec)}; } + file_info.set_modified_time(last_write.time_since_epoch().count()); - return {status, FileOperationResult::SUCCESS}; + // Get permissions + fs::file_status status = fs::status(filepath, ec); + if (ec) { + return {file_info, system_error_to_file_operation_result(ec)}; + } + auto perms = status.permissions(); + file_info.set_permissions(static_cast( + perms & + (fs::perms::owner_all | fs::perms::group_all | fs::perms::others_all))); + + return {file_info, FileOperationResult::SUCCESS}; } bool file_exists(const std::string &filepath) @@ -334,29 +377,35 @@ FileOperationResult delete_directory(const std::string &dirpath, bool recursive) return FileOperationResult::SUCCESS; } -std::pair, FileOperationResult> +std::pair, FileOperationResult> list_directory(const std::string &dirpath) { std::vector entries; std::error_code ec; + std::vector file_info_list; if (!fs::exists(dirpath, ec)) { - return {entries, FileOperationResult::FILE_NOT_FOUND}; + return {file_info_list, FileOperationResult::FILE_NOT_FOUND}; } if (!fs::is_directory(dirpath, ec)) { - return {entries, FileOperationResult::INVALID_PATH}; + return {file_info_list, FileOperationResult::INVALID_PATH}; } for (const auto &entry : fs::directory_iterator(dirpath, ec)) { entries.push_back(entry.path().filename().string()); } - if (ec) { - return {entries, system_error_to_file_operation_result(ec)}; + for (const auto &entry : entries) { + std::string full_path = (fs::path(dirpath) / entry).string(); + auto [file_info, result] = get_file_info(full_path); + if (result != FileOperationResult::SUCCESS) { + return {file_info_list, result}; + } + file_info_list.push_back(file_info); } - return {entries, FileOperationResult::SUCCESS}; + return {file_info_list, FileOperationResult::SUCCESS}; } FileOperationResult change_directory(const std::string &dirpath) diff --git a/tests/unittests/common/file_operations_test.cpp b/tests/unittests/common/file_operations_test.cpp index 3806f10..8e208ce 100644 --- a/tests/unittests/common/file_operations_test.cpp +++ b/tests/unittests/common/file_operations_test.cpp @@ -204,17 +204,20 @@ TEST_F(FileOperationsTest, GetFileInfo) create_test_file(filename, "Test file for info check"); - auto [status, error] = get_file_info(filepath); + auto [file_info, error] = get_file_info(filepath); EXPECT_EQ(error, FileOperationResult::SUCCESS); - EXPECT_TRUE(fs::is_regular_file(status)); + EXPECT_EQ(file_info.name(), filepath); + EXPECT_FALSE(file_info.is_directory()); + EXPECT_EQ(file_info.size(), 24); // "Test file for info check" length // Get info for directory - auto [dir_status, dir_error] = get_file_info(test_dir.string()); + auto [dir_info, dir_error] = get_file_info(test_dir.string()); EXPECT_EQ(dir_error, FileOperationResult::SUCCESS); - EXPECT_TRUE(fs::is_directory(dir_status)); + EXPECT_EQ(dir_info.name(), test_dir.string()); + EXPECT_TRUE(dir_info.is_directory()); // Get info for non-existent file - auto [invalid_status, invalid_error] = + auto [invalid_info, invalid_error] = get_file_info(filepath + ".nonexistent"); EXPECT_EQ(invalid_error, FileOperationResult::FILE_NOT_FOUND); } @@ -308,31 +311,46 @@ TEST_F(FileOperationsTest, ListDirectory) fs::create_directory(test_dir / "subdir2"); // List directory contents - auto [entries, error] = list_directory(test_dir.string()); + auto [file_infos, error] = list_directory(test_dir.string()); EXPECT_EQ(error, FileOperationResult::SUCCESS); - EXPECT_EQ(entries.size(), 4); - - // Verify entries - EXPECT_TRUE(std::find(entries.begin(), entries.end(), "file1.txt") != - entries.end()); - EXPECT_TRUE(std::find(entries.begin(), entries.end(), "file2.txt") != - entries.end()); - EXPECT_TRUE(std::find(entries.begin(), entries.end(), "subdir1") != - entries.end()); - EXPECT_TRUE(std::find(entries.begin(), entries.end(), "subdir2") != - entries.end()); + EXPECT_EQ(file_infos.size(), 4); + + // Verify entries by checking names in the FileInfo objects + std::vector file_names; + for (const auto &info : file_infos) { + file_names.push_back(fs::path(info.name()).filename().string()); + } + + EXPECT_TRUE(std::find(file_names.begin(), file_names.end(), "file1.txt") != + file_names.end()); + EXPECT_TRUE(std::find(file_names.begin(), file_names.end(), "file2.txt") != + file_names.end()); + EXPECT_TRUE(std::find(file_names.begin(), file_names.end(), "subdir1") != + file_names.end()); + EXPECT_TRUE(std::find(file_names.begin(), file_names.end(), "subdir2") != + file_names.end()); + + // Check that directory flag is correctly set + for (const auto &info : file_infos) { + std::string name = fs::path(info.name()).filename().string(); + if (name == "subdir1" || name == "subdir2") { + EXPECT_TRUE(info.is_directory()); + } else { + EXPECT_FALSE(info.is_directory()); + } + } // List contents of non-existent directory - auto [invalid_entries, invalid_error] = + auto [invalid_infos, invalid_error] = list_directory((test_dir / "nonexistent").string()); EXPECT_EQ(invalid_error, FileOperationResult::FILE_NOT_FOUND); - EXPECT_TRUE(invalid_entries.empty()); + EXPECT_TRUE(invalid_infos.empty()); // List a file as a directory - auto [file_entries, file_error] = + auto [file_infos_error, file_error] = list_directory((test_dir / "file1.txt").string()); EXPECT_EQ(file_error, FileOperationResult::INVALID_PATH); - EXPECT_TRUE(file_entries.empty()); + EXPECT_TRUE(file_infos_error.empty()); } // Test changing directory