Mercurial > hg > orthanc

changeset 5226:49e906a8fea2 db-protobuf

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
integration mainline->db-protobuf
author Sebastien Jodogne <s.jodogne@gmail.com>
date Tue, 04 Apr 2023 07:09:22 +0200
parents 3a61fd50f804 (current diff) 5874e5dd9a38 (diff)
children 988dab8deb1c
files NEWS OrthancServer/Sources/Database/StatelessDatabaseOperations.cpp
diffstat 4 files changed, 117 insertions(+), 87 deletions(-) [+]
line wrap: on
line diff
--- a/NEWS	Mon Apr 03 21:14:45 2023 +0200
+++ b/NEWS	Tue Apr 04 07:09:22 2023 +0200
@@ -29,6 +29,9 @@
   to allow "/instances/../export" (the latter is now disabled by default)
 * Fix issue 214: VOILUTSequence is not returned in Wado-RS
 * Fix /tools/reset crashing when ExtraMainDicomTags were defined
+* Fix Housekeeper plugin infinite loop if Orthanc is empty.
+* Fix a crash in /tools/reconstruct triggered by the Housekeeper plugin 
+  when only changing the StorageCompression.
 
 
 Version 1.11.3 (2023-02-03)
--- a/OrthancServer/Plugins/Samples/Housekeeper/Plugin.cpp	Mon Apr 03 21:14:45 2023 +0200
+++ b/OrthancServer/Plugins/Samples/Housekeeper/Plugin.cpp	Tue Apr 04 07:09:22 2023 +0200
@@ -488,36 +488,51 @@
     OrthancPlugins::RestApiGet(changes, "/changes?since=" + boost::lexical_cast<std::string>(pluginStatus_.lastProcessedChange) + "&limit=100", false);
   }
 
-  for (Json::ArrayIndex i = 0; i < changes["Changes"].size(); i++)
+  if (changes["Changes"].size() > 0)
   {
-    const Json::Value& change = changes["Changes"][i];
-    int64_t seq = change["Seq"].asInt64();
-
-    if (change["ChangeType"] == "NewStudy") // some StableStudy might be missing if orthanc was shutdown during a StableAge -> consider only the NewStudy events that can not be missed
+    for (Json::ArrayIndex i = 0; i < changes["Changes"].size(); i++)
     {
-      Json::Value result;
-      Json::Value request;
-      if (needsReingest)
+      const Json::Value& change = changes["Changes"][i];
+      int64_t seq = change["Seq"].asInt64();
+
+      if (change["ChangeType"] == "NewStudy") // some StableStudy might be missing if orthanc was shutdown during a StableAge -> consider only the NewStudy events that can not be missed
+      {
+        Json::Value result;
+        Json::Value request;
+        if (needsReingest)
+        {
+          request["ReconstructFiles"] = true;
+        }
+        OrthancPlugins::RestApiPost(result, "/studies/" + change["ID"].asString() + "/reconstruct", request, false);
+      }
+
       {
-        request["ReconstructFiles"] = true;
+        boost::recursive_mutex::scoped_lock lock(pluginStatusMutex_);
+
+        pluginStatus_.lastProcessedChange = seq;
+
+        if (seq >= pluginStatus_.lastChangeToProcess)  // we are done !
+        {
+          return true;
+        }
       }
-      OrthancPlugins::RestApiPost(result, "/studies/" + change["ID"].asString() + "/reconstruct", request, false);
+
+      if (change["ChangeType"] == "NewStudy")
+      {
+        boost::this_thread::sleep(boost::posix_time::milliseconds(throttleDelay_ * 1000));
+      }
     }
-
+  }
+  else
+  {
+    // if the change list is empty and Done is true, it means that there is nothing to process anymore
+    if (changes["Done"].asBool())
     {
       boost::recursive_mutex::scoped_lock lock(pluginStatusMutex_);
 
-      pluginStatus_.lastProcessedChange = seq;
+      pluginStatus_.lastProcessedChange = changes["Last"].asInt64();
 
-      if (seq >= pluginStatus_.lastChangeToProcess)  // we are done !
-      {
-        return true;
-      }
-    }
-
-    if (change["ChangeType"] == "NewStudy")
-    {
-      boost::this_thread::sleep(boost::posix_time::milliseconds(throttleDelay_ * 1000));
+      return true;
     }
   }
 
--- a/OrthancServer/Sources/Database/StatelessDatabaseOperations.cpp	Mon Apr 03 21:14:45 2023 +0200
+++ b/OrthancServer/Sources/Database/StatelessDatabaseOperations.cpp	Tue Apr 04 07:09:22 2023 +0200
@@ -2967,6 +2967,10 @@
                                       MetadataType metadata,
                                       const std::string& value)
       {
+        if (metadata == 15)
+        {
+          LOG(INFO) << "toto";
+        }
         content.AddMetadata(instance, metadata, value);
         instanceMetadata[metadata] = value;
       }
@@ -3092,13 +3096,21 @@
         {
           IDatabaseWrapper::CreateInstanceResult status;
           int64_t instanceId;
+          
+          bool isNewInstance = transaction.CreateInstance(status, instanceId, hashPatient_,
+                                                          hashStudy_, hashSeries_, hashInstance_);
+
+          if (isReconstruct_ && isNewInstance)
+          {
+            // In case of reconstruct, we just want to modify the attachments and some metadata like the TransferSyntex
+            // The DicomTags and many metadata have already been updated before we get here in ReconstructInstance
+            throw OrthancException(ErrorCode_InternalError, "New instance while reconstructing; this should not happen.");
+          }
 
           // Check whether this instance is already stored
-          if (!transaction.CreateInstance(status, instanceId, hashPatient_,
-                                          hashStudy_, hashSeries_, hashInstance_))
+          if (!isNewInstance && !isReconstruct_)
           {
             // The instance already exists
-        
             if (overwrite_)
             {
               // Overwrite the old instance
@@ -3109,7 +3121,7 @@
               if (!transaction.CreateInstance(status, instanceId, hashPatient_,
                                               hashStudy_, hashSeries_, hashInstance_))
               {
-                throw OrthancException(ErrorCode_InternalError);
+                throw OrthancException(ErrorCode_InternalError, "No new instance while overwriting; this should not happen.");
               }
             }
             else
@@ -3184,14 +3196,19 @@
           for (Attachments::const_iterator it = attachments_.begin();
                it != attachments_.end(); ++it)
           {
+            if (isReconstruct_)
+            {
+              // we are replacing attachments during a reconstruction
+              transaction.DeleteAttachment(instanceId, it->GetContentType());
+            }
+
             transaction.AddAttachment(instanceId, *it, 0 /* this is the first revision */);
           }
 
-      
+          if (!isReconstruct_)
           {
             ResourcesContent content(true /* new resource, metadata can be set */);
 
-
             // Attach the user-specified metadata (in case of reconstruction, metadata_ contains all past metadata, including the system ones we want to keep)
             for (MetadataMap::const_iterator 
                    it = metadata_.begin(); it != metadata_.end(); ++it)
@@ -3220,62 +3237,52 @@
               }
             }
 
-            // Populate the tags of the newly-created resources
-
-            content.AddResource(instanceId, ResourceType_Instance, dicomSummary_);
-            SetInstanceMetadata(content, instanceMetadata_, instanceId, MetadataType_MainDicomTagsSignature, DicomMap::GetMainDicomTagsSignature(ResourceType_Instance));  // New in Orthanc 1.11.0
-            SetMainDicomSequenceMetadata(content, instanceId, dicomSummary_, ResourceType_Instance);   // new in Orthanc 1.11.1
-
-            if (status.isNewSeries_)
-            {
-              content.AddResource(status.seriesId_, ResourceType_Series, dicomSummary_);
-              content.AddMetadata(status.seriesId_, MetadataType_MainDicomTagsSignature, DicomMap::GetMainDicomTagsSignature(ResourceType_Series));  // New in Orthanc 1.11.0
-              SetMainDicomSequenceMetadata(content, status.seriesId_, dicomSummary_, ResourceType_Series);   // new in Orthanc 1.11.1
-            }
-
-            if (status.isNewStudy_)
-            {
-              content.AddResource(status.studyId_, ResourceType_Study, dicomSummary_);
-              content.AddMetadata(status.studyId_, MetadataType_MainDicomTagsSignature, DicomMap::GetMainDicomTagsSignature(ResourceType_Study));  // New in Orthanc 1.11.0
-              SetMainDicomSequenceMetadata(content, status.studyId_, dicomSummary_, ResourceType_Study);   // new in Orthanc 1.11.1
-            }
-
-            if (status.isNewPatient_)
+            if (!isReconstruct_)
             {
-              content.AddResource(status.patientId_, ResourceType_Patient, dicomSummary_);
-              content.AddMetadata(status.patientId_, MetadataType_MainDicomTagsSignature, DicomMap::GetMainDicomTagsSignature(ResourceType_Patient));  // New in Orthanc 1.11.0
-              SetMainDicomSequenceMetadata(content, status.patientId_, dicomSummary_, ResourceType_Patient);   // new in Orthanc 1.11.1
-            }
-
-            // Attach the auto-computed metadata for the patient/study/series levels
-            std::string now = SystemToolbox::GetNowIsoString(true /* use UTC time (not local time) */);
-            content.AddMetadata(status.seriesId_, MetadataType_LastUpdate, now);
-            content.AddMetadata(status.studyId_, MetadataType_LastUpdate, now);
-            content.AddMetadata(status.patientId_, MetadataType_LastUpdate, now);
-
-            if (status.isNewSeries_)
-            {
-              if (hasExpectedInstances_)
+              // Populate the tags of the newly-created resources
+              content.AddResource(instanceId, ResourceType_Instance, dicomSummary_);
+              SetInstanceMetadata(content, instanceMetadata_, instanceId, MetadataType_MainDicomTagsSignature, DicomMap::GetMainDicomTagsSignature(ResourceType_Instance));  // New in Orthanc 1.11.0
+              SetMainDicomSequenceMetadata(content, instanceId, dicomSummary_, ResourceType_Instance);   // new in Orthanc 1.11.1
+
+              if (status.isNewSeries_)
+              {
+                content.AddResource(status.seriesId_, ResourceType_Series, dicomSummary_);
+                content.AddMetadata(status.seriesId_, MetadataType_MainDicomTagsSignature, DicomMap::GetMainDicomTagsSignature(ResourceType_Series));  // New in Orthanc 1.11.0
+                SetMainDicomSequenceMetadata(content, status.seriesId_, dicomSummary_, ResourceType_Series);   // new in Orthanc 1.11.1
+              }
+
+              if (status.isNewStudy_)
+              {
+                content.AddResource(status.studyId_, ResourceType_Study, dicomSummary_);
+                content.AddMetadata(status.studyId_, MetadataType_MainDicomTagsSignature, DicomMap::GetMainDicomTagsSignature(ResourceType_Study));  // New in Orthanc 1.11.0
+                SetMainDicomSequenceMetadata(content, status.studyId_, dicomSummary_, ResourceType_Study);   // new in Orthanc 1.11.1
+              }
+
+              if (status.isNewPatient_)
               {
-                content.AddMetadata(status.seriesId_, MetadataType_Series_ExpectedNumberOfInstances,
-                                    boost::lexical_cast<std::string>(expectedInstances_));
+                content.AddResource(status.patientId_, ResourceType_Patient, dicomSummary_);
+                content.AddMetadata(status.patientId_, MetadataType_MainDicomTagsSignature, DicomMap::GetMainDicomTagsSignature(ResourceType_Patient));  // New in Orthanc 1.11.0
+                SetMainDicomSequenceMetadata(content, status.patientId_, dicomSummary_, ResourceType_Patient);   // new in Orthanc 1.11.1
               }
 
-              // New in Orthanc 1.9.0
-              content.AddMetadata(status.seriesId_, MetadataType_RemoteAet,
-                                  origin_.GetRemoteAetC());
-            }
-
-            if (hasTransferSyntax_)
-            {
-              // New in Orthanc 1.2.0
-              SetInstanceMetadata(content, instanceMetadata_, instanceId,
-                                  MetadataType_Instance_TransferSyntax,
-                                  GetTransferSyntaxUid(transferSyntax_));
-            }
-
-            if (!isReconstruct_) // don't change origin metadata
-            {        
+              // Attach the auto-computed metadata for the patient/study/series levels
+              std::string now = SystemToolbox::GetNowIsoString(true /* use UTC time (not local time) */);
+              content.AddMetadata(status.seriesId_, MetadataType_LastUpdate, now);
+              content.AddMetadata(status.studyId_, MetadataType_LastUpdate, now);
+              content.AddMetadata(status.patientId_, MetadataType_LastUpdate, now);
+
+              if (status.isNewSeries_)
+              {
+                if (hasExpectedInstances_)
+                {
+                  content.AddMetadata(status.seriesId_, MetadataType_Series_ExpectedNumberOfInstances,
+                                      boost::lexical_cast<std::string>(expectedInstances_));
+                }
+
+                // New in Orthanc 1.9.0
+                content.AddMetadata(status.seriesId_, MetadataType_RemoteAet,
+                                    origin_.GetRemoteAetC());
+              }
               // Attach the auto-computed metadata for the instance level,
               // reflecting these additions into the input metadata map
               SetInstanceMetadata(content, instanceMetadata_, instanceId,
@@ -3309,6 +3316,17 @@
               }
             }
 
+            // Following metadatas are also updated if reconstructing the instance.
+            // They might be missing since they have been introduced along Orthanc versions.
+
+            if (hasTransferSyntax_)
+            {
+              // New in Orthanc 1.2.0
+              SetInstanceMetadata(content, instanceMetadata_, instanceId,
+                                  MetadataType_Instance_TransferSyntax,
+                                  GetTransferSyntaxUid(transferSyntax_));
+            }
+
             if (hasPixelDataOffset_)
             {
               // New in Orthanc 1.9.1
--- a/OrthancServer/Sources/ServerToolbox.cpp	Mon Apr 03 21:14:45 2023 +0200
+++ b/OrthancServer/Sources/ServerToolbox.cpp	Tue Apr 04 07:09:22 2023 +0200
@@ -281,14 +281,8 @@
           std::string resultPublicId;  // ignored
           std::unique_ptr<DicomInstanceToStore> dicomInstancetoStore(DicomInstanceToStore::CreateFromParsedDicomFile(locker.GetDicom()));
 
-          context.GetIndex().GetAllMetadata(instanceMetadata, *it, ResourceType_Instance);
-          
-          for (InstanceMetadata::const_iterator itm = instanceMetadata.begin();
-              itm != instanceMetadata.end(); ++itm)
-          {
-            dicomInstancetoStore->AddMetadata(ResourceType_Instance, itm->first, itm->second);
-          }
-
+          // TODO: TranscodeAndStore and specifically ServerIndex::Store have been "poluted" by the isReconstruct parameter
+          // we should very likely refactor it
           context.TranscodeAndStore(resultPublicId, dicomInstancetoStore.get(), StoreInstanceMode_OverwriteDuplicate, true);
         }
       }