# HG changeset patch
# User Sebastien Jodogne <s.jodogne@gmail.com>
# Date 1645370973 -3600
# Node ID ea5f1c6ed07e9eb73a02e0ddeb7620883b8f2f68
# Parent  181e67f9d1297b618cc8619329aa3ada4cf6ecf2
fix cache of storage area

diff -r 181e67f9d129 -r ea5f1c6ed07e OrthancFramework/Sources/FileStorage/StorageAccessor.cpp
--- a/OrthancFramework/Sources/FileStorage/StorageAccessor.cpp	Sun Feb 20 12:00:14 2022 +0100
+++ b/OrthancFramework/Sources/FileStorage/StorageAccessor.cpp	Sun Feb 20 16:29:33 2022 +0100
@@ -133,7 +133,7 @@
           }
         }
 
-        cache_.Add(uuid, type, data, size);
+        cache_.Add(uuid, type, compressed);
         return FileInfo(uuid, type, size, md5,
                         CompressionType_ZlibWithSize, compressed.size(), compressedMD5);
       }
@@ -156,23 +156,17 @@
   void StorageAccessor::Read(std::string& content,
                              const FileInfo& info)
   {
-    if (cache_.Fetch(content, info.GetUuid(), info.GetContentType()))
-    {
-      LOG(INFO) << "Read attachment \"" << info.GetUuid() << "\" "
-                << "content type from cache";
-      return;
-    }
-
     switch (info.GetCompressionType())
     {
       case CompressionType_None:
       {
-        MetricsTimer timer(*this, METRICS_READ);
+        if (!cache_.Fetch(content, info.GetUuid(), info.GetContentType()))
+        {
+          MetricsTimer timer(*this, METRICS_READ);
+          std::unique_ptr<IMemoryBuffer> buffer(area_.Read(info.GetUuid(), info.GetContentType()));
+          buffer->MoveToString(content);
+        }
 
-        std::unique_ptr<IMemoryBuffer> buffer(area_.Read(info.GetUuid(), info.GetContentType()));
-        buffer->MoveToString(content);
-
-        cache_.Add(info.GetUuid(), info.GetContentType(), content);
         break;
       }
 
@@ -180,16 +174,23 @@
       {
         ZlibCompressor zlib;
 
-        std::unique_ptr<IMemoryBuffer> compressed;
-
+        std::string cached;
+        if (cache_.Fetch(cached, info.GetUuid(), info.GetContentType()))
+        {
+          zlib.Uncompress(content, cached.empty() ? NULL : cached.c_str(), cached.size());
+        }
+        else
         {
-          MetricsTimer timer(*this, METRICS_READ);
-          compressed.reset(area_.Read(info.GetUuid(), info.GetContentType()));
+          std::unique_ptr<IMemoryBuffer> compressed;
+          
+          {
+            MetricsTimer timer(*this, METRICS_READ);
+            compressed.reset(area_.Read(info.GetUuid(), info.GetContentType()));
+          }
+          
+          zlib.Uncompress(content, compressed->GetData(), compressed->GetSize());
         }
 
-        zlib.Uncompress(content, compressed->GetData(), compressed->GetSize());
-
-        cache_.Add(info.GetUuid(), info.GetContentType(), content);
         break;
       }
 
@@ -206,63 +207,56 @@
   void StorageAccessor::ReadRaw(std::string& content,
                                 const FileInfo& info)
   {
-    MetricsTimer timer(*this, METRICS_READ);
-
-    std::unique_ptr<IMemoryBuffer> buffer(area_.Read(info.GetUuid(), info.GetContentType()));
-    buffer->MoveToString(content);        
+    if (!cache_.Fetch(content, info.GetUuid(), info.GetContentType()))
+    {
+      MetricsTimer timer(*this, METRICS_READ);
+      std::unique_ptr<IMemoryBuffer> buffer(area_.Read(info.GetUuid(), info.GetContentType()));
+      buffer->MoveToString(content);
+    }
   }
 
 
   void StorageAccessor::Remove(const std::string& fileUuid,
                                FileContentType type)
   {
-    MetricsTimer timer(*this, METRICS_REMOVE);
-    area_.Remove(fileUuid, type);
+    cache_.Invalidate(fileUuid, type);
 
-    cache_.Invalidate(fileUuid, type);
-    
-    // in ReadStartRange, we might have cached only the start of the file -> try to remove it
-    if (type == FileContentType_Dicom)
     {
-      cache_.Invalidate(fileUuid, FileContentType_DicomUntilPixelData);
+      MetricsTimer timer(*this, METRICS_REMOVE);
+      area_.Remove(fileUuid, type);
     }
   }
+  
 
   void StorageAccessor::Remove(const FileInfo &info)
   {
     Remove(info.GetUuid(), info.GetContentType());
   }
 
-  IMemoryBuffer* StorageAccessor::ReadStartRange(const std::string& fileUuid,
-                                                 FileContentType contentType,
-                                                 uint64_t end /* exclusive */,
-                                                 FileContentType startFileContentType)
+
+  void StorageAccessor::ReadStartRange(std::string& target,
+                                       const std::string& fileUuid,
+                                       FileContentType contentType,
+                                       uint64_t end /* exclusive */)
   {
-    std::string content;
-    if (cache_.Fetch(content, fileUuid, contentType))
+    if (cache_.Fetch(target, fileUuid, contentType))
     {
-      LOG(INFO) << "Read attachment \"" << fileUuid << "\" "
-                << "(range from " << 0 << " to " << end << ") from cache";
-
-      return StringMemoryBuffer::CreateFromCopy(content, 0, end);
+      if (target.size() < end)
+      {
+        throw OrthancException(ErrorCode_CorruptedFile);
+      }
+      else
+      {
+        target.resize(end);
+      }
     }
-
-    if (cache_.Fetch(content, fileUuid, startFileContentType))
+    else
     {
-      LOG(INFO) << "Read attachment \"" << fileUuid << "\" "
-                << "(range from " << 0 << " to " << end << ") from cache";
-
-      assert(content.size() == end);
-      return StringMemoryBuffer::CreateFromCopy(content);
+      MetricsTimer timer(*this, METRICS_READ);
+      std::unique_ptr<IMemoryBuffer> buffer(area_.ReadRange(fileUuid, contentType, 0, end));
+      assert(buffer->GetSize() == end);
+      buffer->MoveToString(target);
     }
-
-    std::unique_ptr<IMemoryBuffer> buffer(area_.ReadRange(fileUuid, contentType, 0, end));
-
-    // we've read only the first part of the file -> add an entry in the cache
-    // note the uuid is still the uuid of the full file but the type is the type of the start of the file !
-    assert(buffer->GetSize() == end);
-    cache_.Add(fileUuid, startFileContentType, buffer->GetData(), buffer->GetSize());
-    return buffer.release();
   }
 
 
@@ -271,18 +265,10 @@
                                     const FileInfo& info,
                                     const std::string& mime)
   {
-    if (cache_.Fetch(sender.GetBuffer(), info.GetUuid(), info.GetContentType()))
-    {
-      LOG(INFO) << "Read attachment \"" << info.GetUuid() << "\" "
-                << "content type from cache";
-    }
-    else
     {
       MetricsTimer timer(*this, METRICS_READ);
       std::unique_ptr<IMemoryBuffer> buffer(area_.Read(info.GetUuid(), info.GetContentType()));
       buffer->MoveToString(sender.GetBuffer());
-
-      cache_.Add(info.GetUuid(), info.GetContentType(), sender.GetBuffer());
     }
 
     sender.SetContentType(mime);
diff -r 181e67f9d129 -r ea5f1c6ed07e OrthancFramework/Sources/FileStorage/StorageAccessor.h
--- a/OrthancFramework/Sources/FileStorage/StorageAccessor.h	Sun Feb 20 12:00:14 2022 +0100
+++ b/OrthancFramework/Sources/FileStorage/StorageAccessor.h	Sun Feb 20 16:29:33 2022 +0100
@@ -102,10 +102,10 @@
     void ReadRaw(std::string& content,
                  const FileInfo& info);
 
-    IMemoryBuffer* ReadStartRange(const std::string& fileUuid,
-                                  FileContentType fullFileContentType,
-                                  uint64_t end /* exclusive */,
-                                  FileContentType startFileContentType);
+    void ReadStartRange(std::string& target,
+                        const std::string& fileUuid,
+                        FileContentType fullFileContentType,
+                        uint64_t end /* exclusive */);
 
     void Remove(const std::string& fileUuid,
                 FileContentType type);
diff -r 181e67f9d129 -r ea5f1c6ed07e OrthancFramework/Sources/FileStorage/StorageCache.cpp
--- a/OrthancFramework/Sources/FileStorage/StorageCache.cpp	Sun Feb 20 12:00:14 2022 +0100
+++ b/OrthancFramework/Sources/FileStorage/StorageCache.cpp	Sun Feb 20 16:29:33 2022 +0100
@@ -25,114 +25,68 @@
 #include "StorageCache.h"
 
 #include "../Compatibility.h"
+#include "../Logging.h"
 #include "../OrthancException.h"
 
+#include <boost/lexical_cast.hpp>
 
 
 namespace Orthanc
 {
-  bool IsAcceptedContentType(FileContentType contentType)
-  {
-    return contentType == FileContentType_Dicom ||
-      contentType == FileContentType_DicomUntilPixelData ||
-      contentType == FileContentType_DicomAsJson;
-  }
-
-  const char* ToString(FileContentType contentType)
+  static std::string GetCacheKey(const std::string& uuid,
+                                 FileContentType contentType)
   {
-    switch (contentType)
-    {
-      case FileContentType_Dicom:
-        return "dicom";
-      case FileContentType_DicomUntilPixelData:
-        return "dicom-header";
-      case FileContentType_DicomAsJson:
-        return "dicom-json";
-      default:
-        throw OrthancException(ErrorCode_InternalError,
-                               "ContentType not supported in StorageCache");
-    }
+    return uuid + ":" + boost::lexical_cast<std::string>(contentType);
   }
-
-  bool GetCacheKey(std::string& key, const std::string& uuid, FileContentType contentType)
-  {
-    if (contentType == FileContentType_Unknown || contentType >= FileContentType_StartUser)
-    {
-      return false;
-    }
-
-    key = uuid + ":" + std::string(ToString(contentType));
-
-    return true;
-  }
+  
   
   void StorageCache::SetMaximumSize(size_t size)
   {
     cache_.SetMaximumSize(size);
   }
+  
 
   void StorageCache::Add(const std::string& uuid, 
                          FileContentType contentType,
                          const std::string& value)
   {
-    if (!IsAcceptedContentType(contentType))
-    {
-      return;
-    }
-
-    std::string key;
-
-    if (GetCacheKey(key, uuid, contentType))
-    {
-      cache_.Add(key, value);
-    }
+    const std::string key = GetCacheKey(uuid, contentType);
+    cache_.Add(key, value);
   }
+  
 
   void StorageCache::Add(const std::string& uuid, 
                          FileContentType contentType,
                          const void* buffer,
                          size_t size)
   {
-    if (!IsAcceptedContentType(contentType))
-    {
-      return;
-    }
-    
-    std::string key;
+    const std::string key = GetCacheKey(uuid, contentType);
+    cache_.Add(key, buffer, size);
+  }
+  
 
-    if (GetCacheKey(key, uuid, contentType))
-    {
-      cache_.Add(key, buffer, size);
-    }
-  }
-
-  void StorageCache::Invalidate(const std::string& uuid, FileContentType contentType)
+  void StorageCache::Invalidate(const std::string& uuid,
+                                FileContentType contentType)
   {
-    std::string key;
-    
-    if (GetCacheKey(key, uuid, contentType))
-    {
-      cache_.Invalidate(key);
-    }
+    const std::string key = GetCacheKey(uuid, contentType);
+    cache_.Invalidate(key);
   }
+  
 
   bool StorageCache::Fetch(std::string& value, 
                            const std::string& uuid,
                            FileContentType contentType)
   {
-    if (!IsAcceptedContentType(contentType))
+    const std::string key = GetCacheKey(uuid, contentType);
+    if (cache_.Fetch(value, key))
+    {
+      LOG(INFO) << "Read attachment \"" << uuid << "\" with content type "
+                << boost::lexical_cast<std::string>(contentType) << " from cache";
+      return true;
+    }
+    else
     {
       return false;
     }
-
-    std::string key;
-    if (GetCacheKey(key, uuid, contentType))
-    {
-      return cache_.Fetch(value, key);
-    }
-
-    return false;
   }
-
-
-}
\ No newline at end of file
+}
diff -r 181e67f9d129 -r ea5f1c6ed07e OrthancFramework/Sources/FileStorage/StorageCache.h
--- a/OrthancFramework/Sources/FileStorage/StorageCache.h	Sun Feb 20 12:00:14 2022 +0100
+++ b/OrthancFramework/Sources/FileStorage/StorageCache.h	Sun Feb 20 16:29:33 2022 +0100
@@ -37,7 +37,9 @@
    **/
    class ORTHANC_PUBLIC StorageCache : public boost::noncopyable
     {
+    private:
       MemoryStringCache   cache_;
+      
     public:
       void SetMaximumSize(size_t size);
 
@@ -50,7 +52,8 @@
                const void* buffer,
                size_t size);
 
-      void Invalidate(const std::string& uuid, FileContentType contentType);
+      void Invalidate(const std::string& uuid,
+                      FileContentType contentType);
 
       bool Fetch(std::string& value, 
                  const std::string& uuid,
diff -r 181e67f9d129 -r ea5f1c6ed07e OrthancServer/Sources/ServerContext.cpp
--- a/OrthancServer/Sources/ServerContext.cpp	Sun Feb 20 12:00:14 2022 +0100
+++ b/OrthancServer/Sources/ServerContext.cpp	Sun Feb 20 16:29:33 2022 +0100
@@ -953,23 +953,17 @@
          * "true".
          **/
       
-        std::unique_ptr<IMemoryBuffer> dicom;
+        std::string dicom;
+        
         {
           StorageAccessor accessor(area_, storageCache_, GetMetricsRegistry());
-          dicom.reset(accessor.ReadStartRange(attachment.GetUuid(), FileContentType_Dicom, pixelDataOffset, FileContentType_DicomUntilPixelData));
-        }
-
-        if (dicom.get() == NULL)
-        {
-          throw OrthancException(ErrorCode_InternalError);
+          accessor.ReadStartRange(dicom, attachment.GetUuid(), FileContentType_Dicom, pixelDataOffset);
         }
-        else
-        {
-          assert(dicom->GetSize() == pixelDataOffset);
-          ParsedDicomFile parsed(dicom->GetData(), dicom->GetSize());
-          OrthancConfiguration::DefaultDicomDatasetToJson(result, parsed, ignoreTagLength);
-          InjectEmptyPixelData(result);
-        }
+        
+        assert(dicom.size() == pixelDataOffset);
+        ParsedDicomFile parsed(dicom);
+        OrthancConfiguration::DefaultDicomDatasetToJson(result, parsed, ignoreTagLength);
+        InjectEmptyPixelData(result);
       }
       else if (ignoreTagLength.empty() &&
                index_.LookupAttachment(attachment, revision, instancePublicId, FileContentType_DicomAsJson))
@@ -1093,9 +1087,9 @@
 
         StorageAccessor accessor(area_, storageCache_, GetMetricsRegistry());
 
-        std::unique_ptr<IMemoryBuffer> buffer(
-          accessor.ReadStartRange(attachment.GetUuid(), attachment.GetContentType(), pixelDataOffset, FileContentType_DicomUntilPixelData));
-        buffer->MoveToString(dicom);
+        accessor.ReadStartRange(dicom, attachment.GetUuid(), attachment.GetContentType(), pixelDataOffset);
+        assert(dicom.size() == pixelDataOffset);
+        
         return true;   // Success
       }
       catch (boost::bad_lexical_cast&)