Mercurial > hg > orthanc
changeset 5737:2fe77dfe0466
rewrote FileSystemStorage::Create knowing that we should not check the return value of boost::file_system::create_directories and that it throws on errors we were detecting manually
author | Alain Mazy <am@orthanc.team> |
---|---|
date | Thu, 01 Aug 2024 19:44:48 +0200 |
parents | 264d84c1936a |
children | 39444dccfff7 |
files | OrthancFramework/Sources/FileStorage/FilesystemStorage.cpp OrthancFramework/UnitTestsSources/FileStorageTests.cpp |
diffstat | 2 files changed, 52 insertions(+), 15 deletions(-) [+] |
line wrap: on
line diff
--- a/OrthancFramework/Sources/FileStorage/FilesystemStorage.cpp Tue Jul 30 06:06:21 2024 +0200 +++ b/OrthancFramework/Sources/FileStorage/FilesystemStorage.cpp Thu Aug 01 19:44:48 2024 +0200 @@ -139,8 +139,7 @@ throw OrthancException(ErrorCode_InternalError, "This file UUID already exists"); } - // In very unlikely case (but we've seen it !), 2 threads might enter this same piece - // of code and both try to create the same directory or a thread could be deleting a + // In very unlikely cases, a thread could be deleting a // directory while another thread needs it -> introduce 3 retries at 1 ms interval int retryCount = 0; const int maxRetryCount = 3; @@ -155,24 +154,19 @@ << "\" type"; } - if (boost::filesystem::exists(path.parent_path())) + try { - if (!boost::filesystem::is_directory(path.parent_path())) - { - throw OrthancException(ErrorCode_DirectoryOverFile); // no need to retry this error - } + boost::filesystem::create_directories(path.parent_path()); // the function ensures that the directory exists or throws } - else + catch (boost::filesystem::filesystem_error& er) { - if (!boost::filesystem::create_directories(path.parent_path())) + if (er.code() == boost::system::errc::file_exists // the last element of the parent_path is a file + || er.code() == boost::system::errc::not_a_directory) // one of the element of the parent_path is not a directory { - if (retryCount >= maxRetryCount) - { - throw OrthancException(ErrorCode_FileStorageCannotWrite); - } - - continue; // retry + throw OrthancException(ErrorCode_DirectoryOverFile, "One of the element of the path is a file"); // no need to retry this error } + + // ignore other errors and retry } try
--- a/OrthancFramework/UnitTestsSources/FileStorageTests.cpp Tue Jul 30 06:06:21 2024 +0200 +++ b/OrthancFramework/UnitTestsSources/FileStorageTests.cpp Thu Aug 01 19:44:48 2024 +0200 @@ -37,6 +37,7 @@ #include "../Sources/Logging.h" #include "../Sources/OrthancException.h" #include "../Sources/Toolbox.h" +#include "../Sources/SystemToolbox.h" #include <ctype.h> @@ -88,6 +89,48 @@ ASSERT_EQ(s.GetSize(uid), data.size()); } +TEST(FilesystemStorage, FileWithSameNameAsTopDirectory) +{ + FilesystemStorage s("UnitTestsStorageTop"); + s.Clear(); + + std::vector<uint8_t> data; + StringToVector(data, Toolbox::GenerateUuid()); + + SystemToolbox::WriteFile("toto", "UnitTestsStorageTop/12"); + ASSERT_THROW(s.Create("12345678-1234-1234-1234-1234567890ab", &data[0], data.size(), FileContentType_Unknown), OrthancException); + s.Clear(); +} + +TEST(FilesystemStorage, FileWithSameNameAsChildDirectory) +{ + FilesystemStorage s("UnitTestsStorageChild"); + s.Clear(); + + std::vector<uint8_t> data; + StringToVector(data, Toolbox::GenerateUuid()); + + SystemToolbox::MakeDirectory("UnitTestsStorageChild/12"); + SystemToolbox::WriteFile("toto", "UnitTestsStorageChild/12/34"); + ASSERT_THROW(s.Create("12345678-1234-1234-1234-1234567890ab", &data[0], data.size(), FileContentType_Unknown), OrthancException); + s.Clear(); +} + +TEST(FilesystemStorage, FileAlreadyExists) +{ + FilesystemStorage s("UnitTestsStorageFileAlreadyExists"); + s.Clear(); + + std::vector<uint8_t> data; + StringToVector(data, Toolbox::GenerateUuid()); + + SystemToolbox::MakeDirectory("UnitTestsStorageFileAlreadyExists/12/34"); + SystemToolbox::WriteFile("toto", "UnitTestsStorageFileAlreadyExists/12/34/12345678-1234-1234-1234-1234567890ab"); + ASSERT_THROW(s.Create("12345678-1234-1234-1234-1234567890ab", &data[0], data.size(), FileContentType_Unknown), OrthancException); + s.Clear(); +} + + TEST(FilesystemStorage, EndToEnd) { FilesystemStorage s("UnitTestsStorage");