# HG changeset patch
# User Alain Mazy <am@orthanc.team>
# Date 1722534288 -7200
# Node ID 2fe77dfe04669af888a730f7e925657c8bfdc8ab
# Parent  264d84c1936a6e713a01b230731bfa3d6732ce93
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

diff -r 264d84c1936a -r 2fe77dfe0466 OrthancFramework/Sources/FileStorage/FilesystemStorage.cpp
--- 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 
diff -r 264d84c1936a -r 2fe77dfe0466 OrthancFramework/UnitTestsSources/FileStorageTests.cpp
--- 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");