changeset 4505:97d103b57cd1

removed cached dicom summary from DicomInstanceToStore
author Sebastien Jodogne <s.jodogne@gmail.com>
date Wed, 10 Feb 2021 12:07:03 +0100
parents 7d1eabfac6e0
children ac69c9f76c71
files OrthancServer/Sources/DicomInstanceToStore.cpp OrthancServer/Sources/DicomInstanceToStore.h OrthancServer/Sources/OrthancRestApi/OrthancRestApi.cpp OrthancServer/Sources/ServerContext.cpp OrthancServer/Sources/ServerIndex.cpp OrthancServer/Sources/ServerIndex.h OrthancServer/Sources/main.cpp OrthancServer/UnitTestsSources/LuaServerTests.cpp OrthancServer/UnitTestsSources/ServerIndexTests.cpp
diffstat 9 files changed, 57 insertions(+), 131 deletions(-) [+]
line wrap: on
line diff
--- a/OrthancServer/Sources/DicomInstanceToStore.cpp	Wed Feb 10 11:29:53 2021 +0100
+++ b/OrthancServer/Sources/DicomInstanceToStore.cpp	Wed Feb 10 12:07:03 2021 +0100
@@ -157,7 +157,6 @@
     const void*                          bufferData_;
     size_t                               bufferSize_;
     SmartContainer<ParsedDicomFile>      parsed_;
-    SmartContainer<DicomMap>             summary_;
     MetadataMap                          metadata_;
 
     PImpl() :
@@ -168,9 +167,7 @@
     }
 
   private:
-    std::unique_ptr<DicomInstanceHasher>  hasher_;
-
-    void ParseDicomFile()
+    void ComputeParsedDicomFileIfMissing()
     {
       if (!parsed_.HasContent())
       {
@@ -190,29 +187,13 @@
       }
     }
 
-    void ComputeMissingInformation()
+    void ComputeDicomBufferIfMissing()
     {
-      if (hasBuffer_ &&
-          summary_.HasContent())
-      {
-        // Fine, everything is available
-        return; 
-      }
-    
       if (!hasBuffer_)
       {
         if (!parsed_.HasContent())
         {
-          if (!summary_.HasContent())
-          {
-            throw OrthancException(ErrorCode_NotImplemented);
-          }
-          else
-          {
-            parsed_.TakeOwnership(new ParsedDicomFile(summary_.GetConstContent(),
-                                                      GetDefaultDicomEncoding(),
-                                                      false /* be strict */));
-          }                                
+          throw OrthancException(ErrorCode_NotImplemented);
         }
 
         // Serialize the parsed DICOM file
@@ -226,21 +207,6 @@
 
         hasBuffer_ = true;
       }
-
-      if (!summary_.HasContent())
-      {
-        // At this point, we know that the DICOM file is available as a
-        // memory buffer, but that its summary or its JSON version is
-        // missing
-        
-        ParseDicomFile();
-        assert(parsed_.HasContent());
-
-        // At this point, we have parsed the DICOM file
-        
-        summary_.Allocate();
-        OrthancConfiguration::DefaultExtractDicomSummary(summary_.GetContent(), parsed_.GetContent());
-      }
     }
 
 
@@ -256,7 +222,7 @@
     
     const void* GetBufferData()
     {
-      ComputeMissingInformation();
+      ComputeDicomBufferIfMissing();
 
       if (!hasBuffer_)
       {
@@ -283,7 +249,7 @@
 
     size_t GetBufferSize()
     {
-      ComputeMissingInformation();
+      ComputeDicomBufferIfMissing();
     
       if (!hasBuffer_)
       {
@@ -301,39 +267,8 @@
     }
 
 
-    const DicomMap& GetSummary()
-    {
-      ComputeMissingInformation();
-    
-      if (!summary_.HasContent())
-      {
-        throw OrthancException(ErrorCode_InternalError);
-      }
-
-      return summary_.GetConstContent();
-    }
-
-    
-    DicomInstanceHasher& GetHasher()
-    {
-      if (hasher_.get() == NULL)
-      {
-        hasher_.reset(new DicomInstanceHasher(GetSummary()));
-      }
-
-      if (hasher_.get() == NULL)
-      {
-        throw OrthancException(ErrorCode_InternalError);
-      }
-
-      return *hasher_;
-    }
-
-    
     bool LookupTransferSyntax(std::string& result)
     {
-      ComputeMissingInformation();
-
       DicomMap header;
       if (DicomMap::ParseDicomMetaInformation(header, GetBufferData(), GetBufferSize()))
       {
@@ -365,8 +300,7 @@
 
     ParsedDicomFile& GetParsedDicomFile()
     {
-      ComputeMissingInformation();
-      ParseDicomFile();
+      ComputeParsedDicomFileIfMissing();
       
       if (parsed_.HasContent())
       {
@@ -411,12 +345,6 @@
   }
 
 
-  void DicomInstanceToStore::SetSummary(const DicomMap& summary)
-  {
-    pimpl_->summary_.SetConstReference(summary);
-  }
-
-
   const DicomInstanceToStore::MetadataMap& DicomInstanceToStore::GetMetadata() const
   {
     return pimpl_->metadata_;
@@ -449,23 +377,12 @@
   }
 
 
-  const DicomMap& DicomInstanceToStore::GetSummary()
-  {
-    return pimpl_->GetSummary();
-  }
-
-    
   bool DicomInstanceToStore::LookupTransferSyntax(std::string& result) const
   {
     return const_cast<PImpl&>(*pimpl_).LookupTransferSyntax(result);
   }
 
 
-  DicomInstanceHasher& DicomInstanceToStore::GetHasher()
-  {
-    return pimpl_->GetHasher();
-  }
-
   bool DicomInstanceToStore::HasPixelData() const
   {
     return const_cast<PImpl&>(*pimpl_).GetParsedDicomFile().HasTag(DICOM_TAG_PIXEL_DATA);
--- a/OrthancServer/Sources/DicomInstanceToStore.h	Wed Feb 10 11:29:53 2021 +0100
+++ b/OrthancServer/Sources/DicomInstanceToStore.h	Wed Feb 10 12:07:03 2021 +0100
@@ -33,7 +33,6 @@
 
 #pragma once
 
-#include "../../OrthancFramework/Sources/DicomFormat/DicomInstanceHasher.h"
 #include "../../OrthancFramework/Sources/DicomFormat/DicomMap.h"
 #include "DicomInstanceOrigin.h"
 #include "ServerEnumerations.h"
@@ -67,8 +66,6 @@
 
     void SetParsedDicomFile(ParsedDicomFile& parsed);
 
-    void SetSummary(const DicomMap& summary);
-
     const MetadataMap& GetMetadata() const;
 
     MetadataMap& GetMetadata();
@@ -81,12 +78,8 @@
 
     size_t GetBufferSize() const;
 
-    const DicomMap& GetSummary();
-
     bool LookupTransferSyntax(std::string& result) const;
 
-    DicomInstanceHasher& GetHasher();
-
     bool HasPixelData() const;
 
     ParsedDicomFile& GetParsedDicomFile() const;
--- a/OrthancServer/Sources/OrthancRestApi/OrthancRestApi.cpp	Wed Feb 10 11:29:53 2021 +0100
+++ b/OrthancServer/Sources/OrthancRestApi/OrthancRestApi.cpp	Wed Feb 10 12:07:03 2021 +0100
@@ -39,6 +39,7 @@
 #include "../../../OrthancFramework/Sources/Logging.h"
 #include "../../../OrthancFramework/Sources/MetricsRegistry.h"
 #include "../../../OrthancFramework/Sources/SerializationToolbox.h"
+#include "../OrthancConfiguration.h"
 #include "../ServerContext.h"
 
 #include <boost/algorithm/string/predicate.hpp>
@@ -69,9 +70,13 @@
   {
     SetupResourceAnswer(result, instanceId, ResourceType_Instance, status);
 
-    result["ParentPatient"] = instance.GetHasher().HashPatient();
-    result["ParentStudy"] = instance.GetHasher().HashStudy();
-    result["ParentSeries"] = instance.GetHasher().HashSeries();
+    DicomMap summary;
+    OrthancConfiguration::DefaultExtractDicomSummary(summary, instance.GetParsedDicomFile());
+
+    DicomInstanceHasher hasher(summary);
+    result["ParentPatient"] = hasher.HashPatient();
+    result["ParentStudy"] = hasher.HashStudy();
+    result["ParentSeries"] = hasher.HashSeries();
   }
 
 
--- a/OrthancServer/Sources/ServerContext.cpp	Wed Feb 10 11:29:53 2021 +0100
+++ b/OrthancServer/Sources/ServerContext.cpp	Wed Feb 10 12:07:03 2021 +0100
@@ -512,19 +512,21 @@
         throw OrthancException(ErrorCode_ParameterOutOfRange);
     }
 
-
     bool hasPixelDataOffset;
     uint64_t pixelDataOffset;
     hasPixelDataOffset = DicomStreamReader::LookupPixelDataOffset(
       pixelDataOffset, dicom.GetBufferData(), dicom.GetBufferSize());
-    
-    
+        
+    DicomMap summary;
+    OrthancConfiguration::DefaultExtractDicomSummary(summary, dicom.GetParsedDicomFile());
+
     try
     {
       MetricsRegistry::Timer timer(GetMetricsRegistry(), "orthanc_store_dicom_duration_ms");
       StorageAccessor accessor(area_, GetMetricsRegistry());
 
-      resultPublicId = dicom.GetHasher().HashInstance();
+      DicomInstanceHasher hasher(summary);
+      resultPublicId = hasher.HashInstance();
 
       Json::Value dicomAsJson;
       OrthancConfiguration::DefaultDicomDatasetToJson(dicomAsJson, dicom.GetParsedDicomFile());
@@ -588,8 +590,8 @@
 
       typedef std::map<MetadataType, std::string>  InstanceMetadata;
       InstanceMetadata  instanceMetadata;
-      StoreStatus status = index_.Store(
-        instanceMetadata, dicom, attachments, overwrite, hasPixelDataOffset, pixelDataOffset);
+      StoreStatus status = index_.Store(instanceMetadata, dicom, summary, hasher, attachments,
+                                        overwrite, hasPixelDataOffset, pixelDataOffset);
 
       // Only keep the metadata for the "instance" level
       dicom.GetMetadata().clear();
@@ -656,9 +658,9 @@
     {
       if (e.GetErrorCode() == ErrorCode_InexistentTag)
       {
-        dicom.GetSummary().LogMissingTagsForStore();
+        summary.LogMissingTagsForStore();
       }
-
+      
       throw;
     }
   }
--- a/OrthancServer/Sources/ServerIndex.cpp	Wed Feb 10 11:29:53 2021 +0100
+++ b/OrthancServer/Sources/ServerIndex.cpp	Wed Feb 10 12:07:03 2021 +0100
@@ -757,6 +757,8 @@
   
   StoreStatus ServerIndex::Store(std::map<MetadataType, std::string>& instanceMetadata,
                                  DicomInstanceToStore& instanceToStore,
+                                 const DicomMap& dicomSummary,
+                                 DicomInstanceHasher& hasher,
                                  const Attachments& attachments,
                                  bool overwrite,
                                  bool hasPixelDataOffset,
@@ -764,7 +766,6 @@
   {
     boost::mutex::scoped_lock lock(mutex_);
 
-    const DicomMap& dicomSummary = instanceToStore.GetSummary();
     const ServerIndex::MetadataMap& metadata = instanceToStore.GetMetadata();
 
     int64_t expectedInstances;
@@ -773,10 +774,10 @@
     
     instanceMetadata.clear();
 
-    const std::string hashPatient = instanceToStore.GetHasher().HashPatient();
-    const std::string hashStudy = instanceToStore.GetHasher().HashStudy();
-    const std::string hashSeries = instanceToStore.GetHasher().HashSeries();
-    const std::string hashInstance = instanceToStore.GetHasher().HashInstance();
+    const std::string hashPatient = hasher.HashPatient();
+    const std::string hashStudy = hasher.HashStudy();
+    const std::string hashSeries = hasher.HashSeries();
+    const std::string hashInstance = hasher.HashInstance();
 
     try
     {
--- a/OrthancServer/Sources/ServerIndex.h	Wed Feb 10 11:29:53 2021 +0100
+++ b/OrthancServer/Sources/ServerIndex.h	Wed Feb 10 12:07:03 2021 +0100
@@ -44,6 +44,7 @@
 namespace Orthanc
 {
   class DatabaseLookup;
+  class DicomInstanceHasher;
   class DicomInstanceToStore;
   class ParsedDicomFile;
   class ServerContext;
@@ -140,6 +141,8 @@
 
     StoreStatus Store(std::map<MetadataType, std::string>& instanceMetadata,
                       DicomInstanceToStore& instance,
+                      const DicomMap& dicomSummary,
+                      DicomInstanceHasher& hasher,
                       const Attachments& attachments,
                       bool overwrite,
                       bool hasPixelDataOffset,
--- a/OrthancServer/Sources/main.cpp	Wed Feb 10 11:29:53 2021 +0100
+++ b/OrthancServer/Sources/main.cpp	Wed Feb 10 12:07:03 2021 +0100
@@ -88,14 +88,8 @@
                       const std::string& remoteAet,
                       const std::string& calledAet) ORTHANC_OVERRIDE 
   {
-    DicomMap dicomSummary;
     std::string dicomFile;
 
-    const std::set<DicomTag> ignoreTagLength;
-
-    // TODO => Parameters in class "DicomServer"
-    FromDcmtkBridge::ExtractDicomSummary(dicomSummary, dicom, ORTHANC_MAXIMUM_TAG_LENGTH, ignoreTagLength);
-
     if (!FromDcmtkBridge::SaveToMemoryBuffer(dicomFile, dicom))
     {
       throw OrthancException(ErrorCode_InternalError, "Cannot write DICOM file to memory");
@@ -107,7 +101,6 @@
       toStore.SetOrigin(DicomInstanceOrigin::FromDicomProtocol
                         (remoteIp.c_str(), remoteAet.c_str(), calledAet.c_str()));
       toStore.SetBuffer(dicomFile.c_str(), dicomFile.size());
-      toStore.SetSummary(dicomSummary);
 
       std::string id;
       context_.Store(id, toStore, StoreInstanceMode_Default);
--- a/OrthancServer/UnitTestsSources/LuaServerTests.cpp	Wed Feb 10 11:29:53 2021 +0100
+++ b/OrthancServer/UnitTestsSources/LuaServerTests.cpp	Wed Feb 10 12:07:03 2021 +0100
@@ -151,7 +151,7 @@
 
 #if UNIT_TESTS_WITH_HTTP_CONNEXIONS == 1
   // The "http://www.orthanc-server.com/downloads/third-party/" does
-  // not automatically redirect to HTTPS, so we cas use it even if the
+  // not automatically redirect to HTTPS, so we use it even if the
   // OpenSSL/HTTPS support is disabled in curl
   const std::string BASE = "http://www.orthanc-server.com/downloads/third-party/";
 
--- a/OrthancServer/UnitTestsSources/ServerIndexTests.cpp	Wed Feb 10 11:29:53 2021 +0100
+++ b/OrthancServer/UnitTestsSources/ServerIndexTests.cpp	Wed Feb 10 12:07:03 2021 +0100
@@ -40,6 +40,7 @@
 #include "../../OrthancFramework/Sources/Logging.h"
 
 #include "../Sources/Database/SQLiteDatabaseWrapper.h"
+#include "../Sources/OrthancConfiguration.h"
 #include "../Sources/Search/DatabaseLookup.h"
 #include "../Sources/ServerContext.h"
 #include "../Sources/ServerToolbox.h"
@@ -724,11 +725,21 @@
     instance.SetValue(DICOM_TAG_SOP_INSTANCE_UID, "instance-" + id, false);
     instance.SetValue(DICOM_TAG_SOP_CLASS_UID, "1.2.840.10008.5.1.4.1.1.1", false);  // CR image
 
+    ParsedDicomFile dicom(instance, GetDefaultDicomEncoding(), false /* be strict */);
+
     std::map<MetadataType, std::string> instanceMetadata;
     DicomInstanceToStore toStore;
-    toStore.SetSummary(instance);
-    ASSERT_EQ(StoreStatus_Success, index.Store(instanceMetadata, toStore, attachments,
-                                               false /* don't overwrite */, true /* pixel data offset */, 42));
+    toStore.SetParsedDicomFile(dicom);
+
+    {
+      DicomMap summary;
+      OrthancConfiguration::DefaultExtractDicomSummary(summary, toStore.GetParsedDicomFile());
+
+      DicomInstanceHasher hasher(summary);      
+      ASSERT_EQ(StoreStatus_Success, index.Store(instanceMetadata, toStore, summary, hasher, attachments,
+                                                 false /* don't overwrite */, true /* pixel data offset */, 42));
+    }
+    
     ASSERT_EQ(6u, instanceMetadata.size());
     ASSERT_TRUE(instanceMetadata.find(MetadataType_RemoteAet) != instanceMetadata.end());
     ASSERT_TRUE(instanceMetadata.find(MetadataType_Instance_ReceptionDate) != instanceMetadata.end());
@@ -750,11 +761,6 @@
     ids.push_back(hasher.HashStudy());
     ids.push_back(hasher.HashSeries());
     ids.push_back(hasher.HashInstance());
-
-    ASSERT_EQ(hasher.HashPatient(), toStore.GetHasher().HashPatient());
-    ASSERT_EQ(hasher.HashStudy(), toStore.GetHasher().HashStudy());
-    ASSERT_EQ(hasher.HashSeries(), toStore.GetHasher().HashSeries());
-    ASSERT_EQ(hasher.HashInstance(), toStore.GetHasher().HashInstance());
   }
 
   index.GetGlobalStatistics(diskSize, uncompressedSize, countPatients, 
@@ -820,10 +826,14 @@
     ASSERT_EQ(0u, diskSize);
 
     {
+      ParsedDicomFile dicom(instance, GetDefaultDicomEncoding(), false /* be strict */);
+
+      DicomInstanceHasher hasher(instance);
+      
       DicomInstanceToStore toStore;
-      toStore.SetSummary(instance);
+      toStore.SetParsedDicomFile(dicom);
       toStore.SetOrigin(DicomInstanceOrigin::FromPlugins());
-      ASSERT_EQ(id, toStore.GetHasher().HashInstance());
+      ASSERT_EQ(id, hasher.HashInstance());
 
       std::string id2;
       ASSERT_EQ(StoreStatus_Success, context.Store(id2, toStore, StoreInstanceMode_Default));
@@ -856,8 +866,10 @@
       instance2.Assign(instance);
       instance2.SetValue(DICOM_TAG_PATIENT_NAME, "overwritten", false);
 
+      ParsedDicomFile dicom(instance2, GetDefaultDicomEncoding(), false /* be strict */);
+
       DicomInstanceToStore toStore;
-      toStore.SetSummary(instance2);
+      toStore.SetParsedDicomFile(dicom);
       toStore.SetOrigin(DicomInstanceOrigin::FromPlugins());
 
       std::string id2;