code cleanup

simgrid · Jun 3, 2024 · 654fddc · 654fddc
1 parent 824481e
commit 654fddc
Show file tree

Hide file tree

Showing 8 changed files with 64 additions and 30 deletions.
diff --git a/include/fsmod/FileSystem.hpp b/include/fsmod/FileSystem.hpp
@@ -20,9 +20,9 @@
 
 namespace simgrid::fsmod {
 
-	/**
- 	* @brief A class that implements a file system abstraction
- 	*/
+    /**
+     * @brief A class that implements a file system abstraction
+     */
     class XBT_PUBLIC FileSystem {
         const std::string name_;
         int max_num_open_files_;
@@ -40,12 +40,15 @@ namespace simgrid::fsmod {
         void create_file(const std::string& full_path, const std::string& size) const;
 
         [[nodiscard]] bool file_exists(const std::string& full_path) const;
-        [[nodiscard]] bool directory_exists(const std::string& full_dir_path) const;
-        [[nodiscard]] std::set<std::string, std::less<>> list_files_in_directory(const std::string& full_dir_path) const;
 
         void move_file(const std::string& src_full_path, const std::string& dst_full_path) const;
         void unlink_file(const std::string& full_path) const;
+
+
+        void create_directory(const std::string& full_dir_path) const;
+        [[nodiscard]] bool directory_exists(const std::string& full_dir_path) const;
         void unlink_directory(const std::string& full_dir_path) const;
+        [[nodiscard]] std::set<std::string, std::less<>> list_files_in_directory(const std::string& full_dir_path) const;
 
         [[nodiscard]] sg_size_t file_size(const std::string& full_path) const;
 
@@ -59,7 +62,7 @@ namespace simgrid::fsmod {
         [[nodiscard]] std::shared_ptr<Partition> get_partition_for_path_or_null(const std::string& full_path) const;
 
 
-            private:
+    private:
         [[nodiscard]] std::pair<std::shared_ptr<Partition>, std::string> find_path_at_mount_point(const std::string &full_path) const;
 
         std::map<std::string, std::shared_ptr<Partition>, std::less<>> partitions_;

diff --git a/include/fsmod/FileSystemException.hpp b/include/fsmod/FileSystemException.hpp
@@ -21,7 +21,7 @@
     XBT_ATTRIB_NORETURN void rethrow_nested(simgrid::xbt::ThrowPoint&& throwpoint,                                     \
                                             const std::string& message) const override                                 \
     {                                                                                                                  \
-      std::string augmented_message = std::string("CRAP: ") + message;                                                 \
+      std::string augmented_message = std::string(msg_prefix) + message;                                                 \
       std::throw_with_nested(AnyException(std::move(throwpoint), augmented_message));                                  \
     }                                                                                                                  \
   }
@@ -33,6 +33,7 @@ namespace simgrid::fsmod {
     DECLARE_FSMOD_EXCEPTION(FileNotFoundException, "File not found");
     DECLARE_FSMOD_EXCEPTION(NotEnoughSpaceException, "Not enough space");
     DECLARE_FSMOD_EXCEPTION(FileIsOpenException, "Operation not permitted on an opened file");
+    DECLARE_FSMOD_EXCEPTION(DirectoryAlreadyExistsException, "Directory already exists");
     DECLARE_FSMOD_EXCEPTION(DirectoryDoesNotExistException, "Directory does not exist");
     DECLARE_FSMOD_EXCEPTION(TooManyOpenFilesException, "Too many open files");
     DECLARE_FSMOD_EXCEPTION(FileAlreadyExistsException, "File already exists");

diff --git a/include/fsmod/Partition.hpp b/include/fsmod/Partition.hpp
@@ -100,6 +100,7 @@ namespace simgrid::fsmod {
 
         [[nodiscard]] std::shared_ptr<Storage> get_storage() const { return storage_; }
 
+        void create_new_directory(const std::string& dir_path);
         [[nodiscard]] bool directory_exists(const std::string& dir_path) const { return content_.find(dir_path) != content_.end(); }
         std::set<std::string, std::less<>> list_files_in_directory(const std::string &dir_path) const;
         void delete_directory(const std::string &dir_path);

diff --git a/src/File.cpp b/src/File.cpp
@@ -178,7 +178,7 @@ namespace simgrid::fsmod {
      */
     void File::update_current_position(sg_offset_t pos) {
         if (pos < 0) {
-            throw simgrid::fsmod::InvalidSeekException(XBT_THROW_POINT, "Cannot seek before the first byte");
+            throw InvalidSeekException(XBT_THROW_POINT, "Cannot seek before the first byte");
         }
         current_position_ = pos;
     }

diff --git a/src/FileSystem.cpp b/src/FileSystem.cpp
@@ -34,7 +34,7 @@ namespace simgrid::fsmod {
                                    return PathUtil::is_at_mount_point(simplified_path, element.first);
                                });
         if (it == this->partitions_.end()) {
-            throw simgrid::fsmod::InvalidPathException(XBT_THROW_POINT, "No path prefix matches a partition's mount point (" + simplified_path + ")");
+            throw InvalidPathException(XBT_THROW_POINT, "No path prefix matches a partition's mount point (" + simplified_path + ")");
         }
         auto path_at_mount_point = PathUtil::path_at_mount_point(simplified_path, it->first);
         auto partition = it->second;
@@ -173,7 +173,7 @@ namespace simgrid::fsmod {
         // TODO: This is weak, since if directory "a/b/c/d" exists, director "a/b" does not!
         // TODO: (we don't _really_ have directories, just prefixes before the file name)
         if (partition->directory_exists(path_at_mount_point)) {
-            throw simgrid::fsmod::InvalidPathException(XBT_THROW_POINT, "Provided file path is that of an existing directory (" + path_at_mount_point + ")");
+            throw InvalidPathException(XBT_THROW_POINT, "Provided file path is that of an existing directory (" + path_at_mount_point + ")");
         }
 
         // Split the path
@@ -192,7 +192,7 @@ namespace simgrid::fsmod {
     std::shared_ptr<File> FileSystem::open(const std::string &full_path, const std::string& access_mode) {
         // "Get a file descriptor"
         if (this->num_open_files_ >= this->max_num_open_files_) {
-            throw simgrid::fsmod::TooManyOpenFilesException(XBT_THROW_POINT);
+            throw TooManyOpenFilesException(XBT_THROW_POINT);
         }
         if (access_mode != "r" && access_mode != "w" && access_mode != "a") {
             throw std::invalid_argument("Invalid access mode. Authorized values are: 'r', 'w', or 'a'");
@@ -209,7 +209,7 @@ namespace simgrid::fsmod {
         auto metadata = partition->get_file_metadata(dir, file_name);
         if (not metadata) {
             if (access_mode == "r")
-                throw simgrid::fsmod::FileNotFoundException(XBT_THROW_POINT, full_path);
+                throw FileNotFoundException(XBT_THROW_POINT, full_path);
             create_file(full_path, "0B");
             metadata = partition->get_file_metadata(dir, file_name);
         } else {
@@ -259,14 +259,14 @@ namespace simgrid::fsmod {
         // Check that the file exist
         auto file_metadata = partition->get_file_metadata(dir, file_name);
         if (not file_metadata) {
-            throw simgrid::fsmod::FileNotFoundException(XBT_THROW_POINT, full_path);
+            throw FileNotFoundException(XBT_THROW_POINT, full_path);
         }
 
         return file_metadata->get_current_size();
     }
 
     /**
-     * @brief Unlike a file
+     * @brief Unlink a file
      * @param full_path: the file path
      */
     void FileSystem::unlink_file(const std::string &full_path) const {
@@ -292,7 +292,7 @@ namespace simgrid::fsmod {
 
         // No mv across partitions (just like in the real world)
         if (src_partition != dst_partition) {
-            throw simgrid::fsmod::InvalidMoveException(XBT_THROW_POINT, "Cannot move file across partitions");
+            throw InvalidMoveException(XBT_THROW_POINT, "Cannot move file across partitions");
         }
 
         auto [src_dir, src_file_name] = PathUtil::split_path(src_path_at_mount_point);
@@ -303,7 +303,7 @@ namespace simgrid::fsmod {
     }
 
     /**
-     * @brief Method to check that a file exists at a given path
+     * @brief Check that a file exists at a given path
      * @param full_path: the file path
      * @return true if the file exists, false otherwise
      */
@@ -319,7 +319,22 @@ namespace simgrid::fsmod {
     }
 
     /**
-     * @brief Method to check that a directory exists at a given path
+     * @brief Create a directory
+     * @param full_dir_path: the directory path
+     */
+    void FileSystem::create_directory(const std::string& full_dir_path) const {
+        std::string simplified_path = PathUtil::simplify_path_string(full_dir_path);
+        // This check cannot be performed at the partition-level
+        if (this->file_exists(simplified_path)) {
+            throw InvalidPathException(XBT_THROW_POINT, "Path is that of an existing file: " + simplified_path);
+        }
+        auto [partition, path_at_mount_point] = this->find_path_at_mount_point(simplified_path);
+        partition->create_new_directory(path_at_mount_point);
+    }
+
+
+    /**
+     * @brief Check that a directory exists at a given path
      * @param full_path: the directory path
      * @return true if the directory exists, false otherwise
      */
@@ -330,7 +345,7 @@ namespace simgrid::fsmod {
     }
 
     /**
-     * @brief Method to get the list of names of files in a directory
+     * @brief Retrieve the list of names of files in a directory
      * @param full_dir_path: the path to the directory
      * @return
      */
@@ -341,7 +356,7 @@ namespace simgrid::fsmod {
     }
 
     /**
-     * @brief Method to remove a directory and the files it contains
+     * @brief Remove a directory and the files it contains
      * @param full_dir_path: the path to the directory
      * @return
      */

diff --git a/src/Partition.cpp b/src/Partition.cpp
@@ -44,7 +44,7 @@ namespace simgrid::fsmod {
 
         // Check that the file doesn't already exit
         if (this->get_file_metadata(dir_path, file_name)) {
-            throw simgrid::fsmod::FileAlreadyExistsException(XBT_THROW_POINT, dir_path + "/" + file_name);
+            throw FileAlreadyExistsException(XBT_THROW_POINT, dir_path + "/" + file_name);
         }
 
         content_[dir_path][file_name] = std::make_unique<FileMetadata>(size, this, dir_path, file_name);
@@ -60,11 +60,11 @@ namespace simgrid::fsmod {
     void Partition::delete_file(const std::string &dir_path, const std::string &file_name) {
         auto* metadata_ptr = this->get_file_metadata(dir_path, file_name);
         if (not metadata_ptr) {
-            throw simgrid::fsmod::FileNotFoundException(XBT_THROW_POINT, dir_path + "/" + file_name);
+            throw FileNotFoundException(XBT_THROW_POINT, dir_path + "/" + file_name);
         }
 
         if (metadata_ptr->get_file_refcount() > 0) {
-            throw simgrid::fsmod::FileIsOpenException(XBT_THROW_POINT, dir_path + "/" + file_name);
+            throw FileIsOpenException(XBT_THROW_POINT, dir_path + "/" + file_name);
         }
 
         this->new_file_deletion_event(metadata_ptr);
@@ -84,7 +84,7 @@ namespace simgrid::fsmod {
         // Get the src metadata, which must exist
         const FileMetadata* src_metadata = this->get_file_metadata(src_dir_path, src_file_name);
         if (not src_metadata) {
-            throw simgrid::fsmod::FileNotFoundException(XBT_THROW_POINT, src_dir_path + "/" + src_file_name);
+            throw FileNotFoundException(XBT_THROW_POINT, src_dir_path + "/" + src_file_name);
         }
 
         // Get the dst metadata, if any
@@ -97,10 +97,10 @@ namespace simgrid::fsmod {
 
         // Sanity checks
         if (src_metadata->get_file_refcount() > 0) {
-            throw simgrid::fsmod::FileIsOpenException(XBT_THROW_POINT, src_dir_path + "/" + src_file_name);
+            throw FileIsOpenException(XBT_THROW_POINT, src_dir_path + "/" + src_file_name);
         }
         if (dst_metadata && dst_metadata->get_file_refcount()) {
-            throw simgrid::fsmod::FileIsOpenException(XBT_THROW_POINT, dst_dir_path + "/" + dst_file_name);
+            throw FileIsOpenException(XBT_THROW_POINT, dst_dir_path + "/" + dst_file_name);
         }
 
         // Update free space if needed
@@ -124,7 +124,7 @@ namespace simgrid::fsmod {
 
     std::set<std::string, std::less<>> Partition::list_files_in_directory(const std::string &dir_path) const {
         if (content_.find(dir_path) == content_.end()) {
-            throw simgrid::fsmod::DirectoryDoesNotExistException(XBT_THROW_POINT, dir_path);
+            throw DirectoryDoesNotExistException(XBT_THROW_POINT, dir_path);
         }
         std::set<std::string, std::less<>> keys;
         for (auto const &[filename, metadata]: content_.at(dir_path)) {
@@ -133,14 +133,22 @@ namespace simgrid::fsmod {
         return keys;
     }
 
+
+    void Partition::create_new_directory(const std::string &dir_path) {
+        if (content_.find(dir_path) != content_.end()) {
+            throw DirectoryAlreadyExistsException(XBT_THROW_POINT, dir_path);
+        }
+        this->content_[dir_path] = (std::unordered_map<std::string, std::unique_ptr<FileMetadata>>){};
+    }
+
     void Partition::delete_directory(const std::string &dir_path) {
         if (content_.find(dir_path) == content_.end()) {
-            throw simgrid::fsmod::DirectoryDoesNotExistException(XBT_THROW_POINT, dir_path);
+            throw DirectoryDoesNotExistException(XBT_THROW_POINT, dir_path);
         }
         // Check that no file is open
         for (const auto &[filename, metadata]: content_.at(dir_path)) {
             if (metadata->get_file_refcount() != 0) {
-                throw simgrid::fsmod::FileIsOpenException(XBT_THROW_POINT, "No content deleted in directory before file " + filename + " is open");
+                throw FileIsOpenException(XBT_THROW_POINT, "No content deleted in directory before file " + filename + " is open");
             }
         }
         // Wipe everything out
@@ -149,7 +157,7 @@ namespace simgrid::fsmod {
 
 
     void Partition::create_space(sg_size_t num_bytes) {
-        throw simgrid::fsmod::NotEnoughSpaceException(XBT_THROW_POINT);
+        throw NotEnoughSpaceException(XBT_THROW_POINT);
     }
 
     void Partition::new_file_creation_event(FileMetadata *file_metadata) {

diff --git a/src/PartitionFIFOCaching.cpp b/src/PartitionFIFOCaching.cpp
@@ -27,7 +27,7 @@ namespace simgrid::fsmod {
             }
         }
         if (space_that_can_be_created < num_bytes) {
-            throw simgrid::fsmod::NotEnoughSpaceException(XBT_THROW_POINT, "Unable to evict files to create enough space");
+            throw NotEnoughSpaceException(XBT_THROW_POINT, "Unable to evict files to create enough space");
         }
         for (auto const &victim: files_to_remove_to_create_space) {
             this->delete_file(this->priority_list_[victim]->dir_path_, this->priority_list_[victim]->file_name_);

diff --git a/test/file_system_test.cpp b/test/file_system_test.cpp
@@ -142,6 +142,12 @@ TEST_F(FileSystemTest, Directories)  {
             ASSERT_THROW(fs_->unlink_directory("/dev/a/b/d"), sgfs::DirectoryDoesNotExistException);
             ASSERT_FALSE(fs_->directory_exists("/dev/a/b/d"));
 
+            ASSERT_THROW(fs_->create_directory("/whatever/bogus"), sgfs::InvalidPathException);
+            ASSERT_THROW(fs_->create_directory("/dev/a/foo.txt"), sgfs::InvalidPathException);
+            ASSERT_NO_THROW(fs_->create_directory("/dev/a/new_dir"));
+            ASSERT_THROW(fs_->create_directory("/dev/a/new_dir"), sgfs::DirectoryAlreadyExistsException);
+            ASSERT_NO_THROW(fs_->unlink_directory("/dev/a/new_dir"));
+
 
             XBT_INFO("Try to unlink a directory in which one file is opened. This shouldn't work");
             std::shared_ptr<sgfs::File> file;