diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 3da8331f5..3054948f3 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -47,7 +47,7 @@ set(memgraph_src_files transactions/engine_master.cpp transactions/engine_single_node.cpp transactions/engine_worker.cpp - utils/filesystem.cpp + utils/file.cpp utils/network.cpp utils/watchdog.cpp ) diff --git a/src/communication/raft/storage/file.hpp b/src/communication/raft/storage/file.hpp index 774621bd8..e45b7011a 100644 --- a/src/communication/raft/storage/file.hpp +++ b/src/communication/raft/storage/file.hpp @@ -4,7 +4,7 @@ * Raft log is stored inside a folder. Each log entry is stored in a file named * by its index. There is a special file named "metadata" which stores Raft * metadata and also the last log index, which is used on startup to identify - * which log entry files are valid. + * which log entry files are valid. */ #pragma once @@ -17,7 +17,7 @@ #include "communication/raft/raft.hpp" #include "communication/raft/storage/memory.hpp" -#include "utils/filesystem.hpp" +#include "utils/file.hpp" namespace communication::raft { @@ -43,14 +43,14 @@ class SimpleFileStorage : public RaftStorageInterface { } auto md = utils::TryOpenFile(dir_, "metadata", O_RDONLY); - if (md.Empty()) { + if (!md) { LOG(WARNING) << fmt::format("No metadata file found in directory '{}'", parent_dir); return; } boost::iostreams::file_descriptor_source src( - md.Handle(), + md->Handle(), boost::iostreams::file_descriptor_flags::never_close_handle); boost::iostreams::stream is(src); boost::archive::binary_iarchive iar(is); @@ -96,22 +96,11 @@ class SimpleFileStorage : public RaftStorageInterface { LOG(FATAL) << fmt::format("Failed to deserialize log entry {}: {}", idx, e.what()); } - - try { - utils::Close(entry_file); - } catch (std::system_error &e) { - LOG(FATAL) << fmt::format("Failed to close entry file {}: {}", idx, - e.what()); - } } LOG(INFO) << fmt::format("Read {} log entries", metadata.last_log_index); - - utils::Close(md); } - ~SimpleFileStorage() { utils::Close(dir_); } - void WriteTermAndVotedFor( TermId term, const std::experimental::optional &voted_for) override { @@ -160,7 +149,6 @@ class SimpleFileStorage : public RaftStorageInterface { try { utils::Fsync(entry_file); - utils::Close(entry_file); } catch (std::system_error &e) { LOG(FATAL) << fmt::format("Failed to write log entry file to disk: {}", e.what()); @@ -234,7 +222,6 @@ class SimpleFileStorage : public RaftStorageInterface { try { utils::Fsync(md_tmp); - utils::Close(md_tmp); } catch (std::system_error &e) { LOG(FATAL) << fmt::format( "Failed to write temporary metadata file to disk: {}", e.what()); diff --git a/src/utils/filesystem.cpp b/src/utils/file.cpp similarity index 50% rename from src/utils/filesystem.cpp rename to src/utils/file.cpp index 92bd9405b..820c99915 100644 --- a/src/utils/filesystem.cpp +++ b/src/utils/file.cpp @@ -1,4 +1,4 @@ -#include "filesystem.hpp" +#include "utils/file.hpp" #include #include @@ -11,21 +11,65 @@ namespace utils { +std::vector LoadFilePaths(const fs::path &directory, + const std::string &extension) { + // result container + std::vector file_paths; + + for (auto &directory_entry : fs::recursive_directory_iterator(directory)) { + auto path = directory_entry.path().string(); + + // skip directories + if (!fs::is_regular_file(directory_entry)) continue; + + // if extension isn't defined then put all file paths from the directory + // to the result set + if (!extension.empty()) { + // skip paths that don't have appropriate extension + auto file_extension = path.substr(path.find_last_of(".") + 1); + if (file_extension != extension) continue; + } + + file_paths.emplace_back(path); + + // skip paths that don't have appropriate extension + auto file_extension = path.substr(path.find_last_of(".") + 1); + if (file_extension != extension) continue; + + // path has the right extension and can be placed in the result + // container + file_paths.emplace_back(path); + } + + return file_paths; +} + +std::vector ReadLines(const fs::path &path) { + std::vector lines; + + std::ifstream stream(path.c_str()); + std::string line; + while (std::getline(stream, line)) { + lines.emplace_back(line); + } + + return lines; +} + +void Write(const std::string &text, const fs::path &path) { + std::ofstream stream; + stream.open(path.c_str()); + stream << text; + stream.close(); +} + File::File() : fd_(-1), path_() {} File::File(int fd, fs::path path) : fd_(fd), path_(std::move(path)) {} -File::~File() { - CHECK(fd_ == -1) << fmt::format( - "Underlying file descriptor should be released or closed " - "before destructing (fd = {}, path = {})", - fd_, path_); -} +File::~File() { Close(); } -File::File(File &&rhs) : fd_(rhs.fd_), path_(rhs.path_) { - LOG(INFO) << "Move constructor"; - rhs.Release(); -} +File::File(File &&rhs) : fd_(rhs.fd_), path_(rhs.path_) { rhs.Release(); } File &File::operator=(File &&rhs) { if (this != &rhs) { @@ -45,6 +89,17 @@ void File::Release() { path_ = fs::path(); } +void File::Close() { + if (fd_ == -1) { + return; + } + if (::close(fd_) == -1) { + throw std::system_error(errno, std::generic_category(), + fmt::format("cannot close {}", path_)); + } + Release(); +} + File OpenFile(const fs::path &path, int flags, ::mode_t mode) { int fd = ::open(path.c_str(), flags, mode); if (fd == -1) { @@ -55,9 +110,11 @@ File OpenFile(const fs::path &path, int flags, ::mode_t mode) { return File(fd, path); } -File TryOpenFile(const fs::path &path, int flags, ::mode_t mode) { +std::experimental::optional TryOpenFile(const fs::path &path, int flags, + ::mode_t mode) { int fd = ::open(path.c_str(), flags, mode); - return fd == -1 ? File() : File(fd, path); + return fd == -1 ? std::experimental::nullopt + : std::experimental::make_optional(File(fd, path)); } File OpenFile(const File &dir, const fs::path &path, int flags, ::mode_t mode) { @@ -69,18 +126,13 @@ File OpenFile(const File &dir, const fs::path &path, int flags, ::mode_t mode) { return File(fd, dir.Path() / path); } -File TryOpenFile(const File &dir, const fs::path &path, int flags, - ::mode_t mode) { +std::experimental::optional TryOpenFile(const File &dir, + const fs::path &path, int flags, + ::mode_t mode) { int fd = ::openat(dir.Handle(), path.c_str(), flags, mode); - return fd == -1 ? File() : File(fd, dir.Path() / path); -} - -void Close(File &file) { - if (::close(file.Handle()) == -1) { - throw std::system_error(errno, std::generic_category(), - fmt::format("cannot close {}", file.Path())); - } - file.Release(); + return fd == -1 + ? std::experimental::nullopt + : std::experimental::make_optional(File(fd, dir.Path() / path)); } void Fsync(const File &file) { @@ -104,7 +156,6 @@ void Rename(const File &dir1, const fs::path &path1, const File &dir2, void SyncDir(const fs::path &path) { File dir = OpenFile(path, O_DIRECTORY | O_RDONLY); Fsync(dir); - Close(dir); } File OpenDir(const fs::path &path) { @@ -113,11 +164,9 @@ File OpenDir(const fs::path &path) { if (path.has_parent_path()) { SyncDir(path.parent_path()); } - } else { - if (errno != EEXIST) { - throw std::system_error(errno, std::generic_category(), - fmt::format("cannot create directory {}", path)); - } + } else if (errno != EEXIST) { + throw std::system_error(errno, std::generic_category(), + fmt::format("cannot create directory {}", path)); } int fd = ::open(path.c_str(), O_RDONLY | O_DIRECTORY); diff --git a/src/utils/file.hpp b/src/utils/file.hpp index cf3c3aafe..1ebac821d 100644 --- a/src/utils/file.hpp +++ b/src/utils/file.hpp @@ -1,11 +1,21 @@ +/** + * @file + * + * This file contains utilities for operations with files. Other than utility + * funtions, a `File` class is provided which wraps a system file handle. + */ #pragma once #include +#include #include + namespace fs = std::experimental::filesystem; namespace utils { +// Higher level utility operations follow. + /** * Loads all file paths in the specified directory. Optionally * the paths are filtered by extension. @@ -18,38 +28,8 @@ namespace utils { * * @return std::vector of paths founded in the directory */ -inline auto LoadFilePaths(const fs::path &directory, - const std::string &extension = "") { - // result container - std::vector file_paths; - - for (auto &directory_entry : fs::recursive_directory_iterator(directory)) { - auto path = directory_entry.path().string(); - - // skip directories - if (!fs::is_regular_file(directory_entry)) continue; - - // if extension isn't defined then put all file paths from the directory - // to the result set - if (!extension.empty()) { - // skip paths that don't have appropriate extension - auto file_extension = path.substr(path.find_last_of(".") + 1); - if (file_extension != extension) continue; - } - - file_paths.emplace_back(path); - - // skip paths that don't have appropriate extension - auto file_extension = path.substr(path.find_last_of(".") + 1); - if (file_extension != extension) continue; - - // path has the right extension and can be placed in the result - // container - file_paths.emplace_back(path); - } - - return file_paths; -} +std::vector LoadFilePaths(const fs::path &directory, + const std::string &extension = ""); // TODO: add error checking /** @@ -58,28 +38,164 @@ inline auto LoadFilePaths(const fs::path &directory, * @param path file path. * @return vector of all lines from the file. */ -std::vector ReadLines(const fs::path &path) { - std::vector lines; - - std::ifstream stream(path.c_str()); - std::string line; - while (std::getline(stream, line)) { - lines.emplace_back(line); - } - - return lines; -} +std::vector ReadLines(const fs::path &path); /** - * Writes test into the file specified by path. + * Writes text into the file specified by path. * * @param text content which will be written in the file. * @param path a path to the file. */ -void Write(const std::string &text, const fs::path &path) { - std::ofstream stream; - stream.open(path.c_str()); - stream << text; - stream.close(); -} -} +void Write(const std::string &text, const fs::path &path); + +// End higher level operations. + +// Lower level wrappers around C system calls follow. + +/** + * Thin wrapper around system file handle. + */ +class File { + public: + /** Constructs an empty file handle. */ + File(); + + /** + * Take ownership of the given system file handle. + * + * @param fd System file handle. + * @param path Pathname naming the file, used only for error handling. + */ + File(int fd, fs::path path); + + File(File &&); + File &operator=(File &&); + + File(const File &) = delete; + File &operator=(const File &) = delete; + + /** + * Closes the underlying file handle. + * + * @throws std::system_error + */ + ~File(); + + /** Gets the path to the underlying file. */ + fs::path Path() const; + + /** Gets the underlying file handle. */ + int Handle() const; + + /** Checks if there's an underlying file handle. */ + bool Empty() const; + + /** + * Closes the underlying file handle. + * + * Wrapper around `close` system call (see `man 2 close`). File will + * be empty after the call returns. You may call Close multiple times, all + * calls after the first one are no-op. + * + * @throws std::system_error + */ + void Close(); + + private: + int fd_; + fs::path path_; + + void Release(); +}; + +/** + * Opens a file descriptor. + * Wrapper around `open` system call (see `man 2 open`). + * + * @param path Pathname naming the file. + * @param flags Opening flags. + * @param mode File mode bits to apply for creation of new file. + * @return File descriptor referring to the opened file. + * + * @throws std::system_error + */ +File OpenFile(const fs::path &path, int flags, ::mode_t mode = 0666); + +/** + * Same as OpenFile, but returns `nullopt` instead of throwing an exception. + */ +std::experimental::optional TryOpenFile(const fs::path &path, int flags, + ::mode_t mode = 0666); + +/** Calls File::Close */ +inline void Close(File &file) { file.Close(); } + +/** + * Opens a file descriptor for a file inside a directory. + * Wrapper around `openat` system call (see `man 2 openat`). + * + * @param dir Directory file descriptor. + * @param path Pathname naming the file. + * @param flags Opening flags. + * @param mode File mode bits to apply for creation of new file. + * @return File descriptor referring to the opened file. + * + * @throws std::system_error + */ +File OpenFile(const File &dir, const fs::path &path, int flags, + ::mode_t mode = 0666); + +/** + * Same as OpenFile, but returns `nullopt` instead of throwing an exception. + */ +std::experimental::optional TryOpenFile(const File &dir, + const fs::path &path, int flags, + ::mode_t mode = 0666); + +/** + * Synchronizes file with the underlying storage device. + * Wrapper around `fsync` system call (see `man 2 fsync`). + * + * @param file File descriptor referring to the file to be synchronized. + * + * @throws std::system_error + */ +void Fsync(const File &file); + +/** + * Moves a file from one directory to another. + * + * Wrapper around `renameat` system call (see `man 2 renameat`). + * + * @param dir1 Source directory. + * @param path1 Pathname naming the source file. + * @param dir2 Destination directory. + * @param path2 Pathname naming the destination file. + * + * @throws std::system_error + */ +void Rename(const File &dir1, const fs::path &path1, const File &dir2, + const fs::path &path2); + +/** + * Synchronizes directory with the underlying storage device. + * + * @param path Pathname naming the directory. + * + * @throws std::system_error + */ +void SyncDir(const fs::path &path); + +/** + * Opens a directory, creating it if it doesn't exist. + * + * @param path Pathname naming the directory. + * @return File descriptor referring to the opened directory. + * + * @throws std::system_error + */ +File OpenDir(const fs::path &path); + +// End lower level wrappers. + +} // namespace utils diff --git a/src/utils/filesystem.hpp b/src/utils/filesystem.hpp deleted file mode 100644 index 19bf12d2c..000000000 --- a/src/utils/filesystem.hpp +++ /dev/null @@ -1,169 +0,0 @@ -/** - * @file - * @brief This file contains C++ wrappers around some C system calls. - */ -#pragma once - -#include - -namespace fs = std::experimental::filesystem; - -namespace utils { - -/** - * @brief Thin wrapper around system file handle. - */ -class File { - public: - /** - * Constructs an empty file handle. - */ - File(); - - /** - * Wraps the given system file handle. - * - * This class doesn't take ownership of the file handle, it won't close it - * on destruction. - * - * @param fd System file handle. - * @param path Pathname naming the file, used only for error handling. - */ - File(int fd, fs::path path); - - File(File &&); - File &operator=(File &&); - - File(const File &) = delete; - File &operator=(const File &) = delete; - - /*** - * Destructor -- crashes the program if the underlying file descriptor is not - * released or closed. - */ - ~File(); - - /** - * Gets the path to the underlying file. - */ - fs::path Path() const; - - /** - * Gets the underlying file handle. - */ - int Handle() const; - - /** - * Checks if there's an underlying file handle. - */ - bool Empty() const; - - /** - * Releases the underlying file handle. - * - * File descriptor will be empty after the call returns. - */ - void Release(); - - private: - int fd_; - fs::path path_; -}; - -/** - * Opens a file descriptor. - * Wrapper around `open` system call (see `man 2 open`). - * - * @param path Pathname naming the file. - * @param flags Opening flags. - * @param mode File mode bits to apply for creation of new file. - * @return File descriptor referring to the opened file. - * - * @throws std::system_error - */ -File OpenFile(const fs::path &path, int flags, ::mode_t mode = 0666); - -/** - * Same as OpenFile, but returns an empty File object instead of - * throwing an exception. - */ -File TryOpenFile(const fs::path &path, int flags, ::mode_t mode = 0666); - -/** - * Opens a file descriptor for a file inside a directory. - * Wrapper around `openat` system call (see `man 2 openat`). - * - * @param dir Directory file descriptor. - * @param path Pathname naming the file. - * @param flags Opening flags. - * @param mode File mode bits to apply for creation of new file. - * @return File descriptor referring to the opened file. - * - * @throws std::system_error - */ -File OpenFile(const File &dir, const fs::path &path, int flags, - ::mode_t mode = 0666); - -/** - * Same as OpenFile, but returns an empty File object instead of - * throwing an exception. - */ -File TryOpenFile(const File &dir, const fs::path &path, int flags, - ::mode_t mode = 0666); - -/** - * Closes a file descriptor. - * Wrapper around `close` system call (see `man 2 close`). File descriptor will - * be empty after the call returns. - * - * @param file File descriptor to be closed. - * - * @throws std::system_error - */ -void Close(File &file); - -/** - * Synchronizes file with the underlying storage device. - * Wrapper around `fsync` system call (see `man 2 fsync`). - * - * @param file File descriptor referring to the file to be synchronized. - * - * @throws std::system_error - */ -void Fsync(const File &file); - -/** - * Moves a file from one directory to another. - * - * Wrapper around `renameat` system call (see `man 2 renameat`). - * - * @param dir1 Source directory. - * @param path1 Pathname naming the source file. - * @param dir2 Destination directory. - * @param path2 Pathname naming the destination file. - * - * @throws std::system_error - */ -void Rename(const File &dir1, const fs::path &path1, const File &dir2, - const fs::path &path2); - -/** - * Synchronizes directory with the underlying storage device. - * - * @param path Pathname naming the directory. - * - * @throws std::system_error - */ -void SyncDir(const fs::path &path); - -/** - * Opens a directory, creating it if it doesn't exist. - * - * @param path Pathname naming the directory. - * @return File descriptor referring to the opened directory. - * - * @throws std::system_error - */ -File OpenDir(const fs::path &path); - -} // namespace utils diff --git a/tests/unit/raft_storage.cpp b/tests/unit/raft_storage.cpp index b24099a8b..0d101dced 100644 --- a/tests/unit/raft_storage.cpp +++ b/tests/unit/raft_storage.cpp @@ -4,7 +4,6 @@ #include "communication/raft/storage/file.hpp" #include "communication/raft/test_utils.hpp" -#include "utils/filesystem.hpp" using communication::raft::LogEntry; using communication::raft::SimpleFileStorage;