changeset 5635:0e16e677fe64

fixed broken /instances/../tags after reconstructing studies when IngestTranscoding has changed
author Alain Mazy <am@orthanc.team>
date Tue, 21 May 2024 17:09:57 +0200
parents 5db4ed395d81
children 6ad8e2afd4fb
files NEWS OrthancServer/Plugins/Samples/Housekeeper/Plugin.cpp OrthancServer/Sources/Database/StatelessDatabaseOperations.cpp
diffstat 3 files changed, 172 insertions(+), 167 deletions(-) [+]
line wrap: on
line diff
--- a/NEWS	Tue May 21 12:28:21 2024 +0200
+++ b/NEWS	Tue May 21 17:09:57 2024 +0200
@@ -11,14 +11,19 @@
   in case you just want to update the MainDicomTags of that resource level only 
   e.g. after you have updated the 'ExtraMainDicomTags' for this level.
 * The "requestedTags" GET argument was deprecated in favor of "requested-tags".
+* Fixed broken /instances/../tags route after the calling
+  /studies/../reconstruct when changing the "IngestTranscoding". 
 
 Plugins
 -------
 
 * Multitenant DICOM plugin: added support for locales
-* Housekeeper plugin: Added an option "LimitMainDicomTagsReconstructLevel"
-  (allowed values: "Patient", "Study", "Series", "Instance").  This can greatly speed
-  up the housekeeper process e.g. if you have only update the Study level ExtraMainDicomTags.
+* Housekeeper plugin: 
+  - Added an option "LimitMainDicomTagsReconstructLevel"
+    (allowed values: "Patient", "Study", "Series", "Instance").  This can greatly speed
+    up the housekeeper process e.g. if you have only update the Study level ExtraMainDicomTags.
+  - Fixed broken /instances/../tags route after running the Housekeeper
+    after having changed the "IngestTranscoding". 
 * SDK: added OrthancPluginLogMessage() that is a new primitive for
   plugins to log messages.  This new primitive will display the plugin
   name, the plugin file name, and the plugin line number in the
@@ -43,13 +48,13 @@
 * Monitoring of stable resources now also takes into consideration the
   resource type, not only the resource identifier identifier.
 * DICOM TLS:
-  * In prior versions, when "DicomTlsRemoteCertificateRequired" was set to false, Orthanc
+  - In prior versions, when "DicomTlsRemoteCertificateRequired" was set to false, Orthanc
     was still sending a client certificate request during the TLS handshake but was not triggering
     and error if the client certificate was not trusted (equivalent to the --verify-peer-cert DCMTK option)
     From this version, if this option is set to false, Orthanc will not send a 
     client certificate request during the TLS handshake anymore (equivalent to the --ignore-peer-cert 
     DCMTK option).
-  * When working with "DicomTlsEnabled": true and "DicomTlsRemoteCertificateRequired": false,
+  - When working with "DicomTlsEnabled": true and "DicomTlsRemoteCertificateRequired": false,
     Orthanc was refusing to start if no "DicomTlsTrustedCertificates" was provided.
 * Upgraded dependencies for static builds:
   - boost 1.85.0
--- a/OrthancServer/Plugins/Samples/Housekeeper/Plugin.cpp	Tue May 21 12:28:21 2024 +0200
+++ b/OrthancServer/Plugins/Samples/Housekeeper/Plugin.cpp	Tue May 21 17:09:57 2024 +0200
@@ -565,7 +565,7 @@
         {
           Json::Value result;
 
-          if (needsReconstruct)
+          if (needsReconstruct || needsReingest)
           {
             Json::Value request;
             if (needsReingest)
@@ -657,7 +657,7 @@
     }
   }
 
-  if (!needsProcessing)
+  if (!needsProcessing && !force_)
   {
     ORTHANC_PLUGINS_LOG_WARNING("Housekeeper: everything has been processed already !");
     return;
--- a/OrthancServer/Sources/Database/StatelessDatabaseOperations.cpp	Tue May 21 12:28:21 2024 +0200
+++ b/OrthancServer/Sources/Database/StatelessDatabaseOperations.cpp	Tue May 21 17:09:57 2024 +0200
@@ -3342,193 +3342,193 @@
           transaction.AddAttachment(instanceId, *it, 0 /* this is the first revision */);
         }
 
+        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)
+        {
+          switch (it->first.first)
+          {
+            case ResourceType_Patient:
+              content.AddMetadata(status.patientId_, it->first.second, it->second);
+              break;
+
+            case ResourceType_Study:
+              content.AddMetadata(status.studyId_, it->first.second, it->second);
+              break;
+
+            case ResourceType_Series:
+              content.AddMetadata(status.seriesId_, it->first.second, it->second);
+              break;
+
+            case ResourceType_Instance:
+              SetInstanceMetadata(content, instanceMetadata_, instanceId,
+                                  it->first.second, it->second);
+              break;
+
+            default:
+              throw OrthancException(ErrorCode_ParameterOutOfRange);
+          }
+        }
+
         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)
+          // 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_)
           {
-            switch (it->first.first)
-            {
-              case ResourceType_Patient:
-                content.AddMetadata(status.patientId_, it->first.second, it->second);
-                break;
-
-              case ResourceType_Study:
-                content.AddMetadata(status.studyId_, it->first.second, it->second);
-                break;
-
-              case ResourceType_Series:
-                content.AddMetadata(status.seriesId_, it->first.second, it->second);
-                break;
-
-              case ResourceType_Instance:
-                SetInstanceMetadata(content, instanceMetadata_, instanceId,
-                                    it->first.second, it->second);
-                break;
-
-              default:
-                throw OrthancException(ErrorCode_ParameterOutOfRange);
-            }
+            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 (!isReconstruct_)
+          if (status.isNewStudy_)
           {
-            // 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.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_)
+            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.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_)
             {
-              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,
-                                MetadataType_Instance_ReceptionDate, now);
-            SetInstanceMetadata(content, instanceMetadata_, instanceId, MetadataType_RemoteAet,
-                                origin_.GetRemoteAetC());
-            SetInstanceMetadata(content, instanceMetadata_, instanceId, MetadataType_Instance_Origin, 
-                                EnumerationToString(origin_.GetRequestOrigin()));
-
-            std::string s;
-
-            if (origin_.LookupRemoteIp(s))
-            {
-              // New in Orthanc 1.4.0
-              SetInstanceMetadata(content, instanceMetadata_, instanceId,
-                                  MetadataType_Instance_RemoteIp, s);
-            }
-
-            if (origin_.LookupCalledAet(s))
-            {
-              // New in Orthanc 1.4.0
-              SetInstanceMetadata(content, instanceMetadata_, instanceId,
-                                  MetadataType_Instance_CalledAet, s);
+              content.AddMetadata(status.seriesId_, MetadataType_Series_ExpectedNumberOfInstances,
+                                  boost::lexical_cast<std::string>(expectedInstances_));
             }
 
-            if (origin_.LookupHttpUsername(s))
-            {
-              // New in Orthanc 1.4.0
-              SetInstanceMetadata(content, instanceMetadata_, instanceId,
-                                  MetadataType_Instance_HttpUsername, s);
-            }
+            // New in Orthanc 1.9.0
+            content.AddMetadata(status.seriesId_, MetadataType_RemoteAet,
+                                origin_.GetRemoteAetC());
           }
-
-          // Following metadatas are also updated if reconstructing the instance.
-          // They might be missing since they have been introduced along Orthanc versions.
-
-          if (hasTransferSyntax_)
+          // Attach the auto-computed metadata for the instance level,
+          // reflecting these additions into the input metadata map
+          SetInstanceMetadata(content, instanceMetadata_, instanceId,
+                              MetadataType_Instance_ReceptionDate, now);
+          SetInstanceMetadata(content, instanceMetadata_, instanceId, MetadataType_RemoteAet,
+                              origin_.GetRemoteAetC());
+          SetInstanceMetadata(content, instanceMetadata_, instanceId, MetadataType_Instance_Origin, 
+                              EnumerationToString(origin_.GetRequestOrigin()));
+
+          std::string s;
+
+          if (origin_.LookupRemoteIp(s))
           {
-            // New in Orthanc 1.2.0
+            // New in Orthanc 1.4.0
+            SetInstanceMetadata(content, instanceMetadata_, instanceId,
+                                MetadataType_Instance_RemoteIp, s);
+          }
+
+          if (origin_.LookupCalledAet(s))
+          {
+            // New in Orthanc 1.4.0
             SetInstanceMetadata(content, instanceMetadata_, instanceId,
-                                MetadataType_Instance_TransferSyntax,
-                                GetTransferSyntaxUid(transferSyntax_));
+                                MetadataType_Instance_CalledAet, s);
+          }
+
+          if (origin_.LookupHttpUsername(s))
+          {
+            // New in Orthanc 1.4.0
+            SetInstanceMetadata(content, instanceMetadata_, instanceId,
+                                MetadataType_Instance_HttpUsername, s);
           }
-
-          if (hasPixelDataOffset_)
+        }
+
+        // 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
+          SetInstanceMetadata(content, instanceMetadata_, instanceId,
+                              MetadataType_Instance_PixelDataOffset,
+                              boost::lexical_cast<std::string>(pixelDataOffset_));
+
+          // New in Orthanc 1.12.1
+          if (dicomSummary_.GuessPixelDataValueRepresentation(transferSyntax_) != pixelDataVR_)
           {
-            // New in Orthanc 1.9.1
+            // Store the VR of pixel data if it doesn't comply with the standard
             SetInstanceMetadata(content, instanceMetadata_, instanceId,
-                                MetadataType_Instance_PixelDataOffset,
-                                boost::lexical_cast<std::string>(pixelDataOffset_));
-
-            // New in Orthanc 1.12.1
-            if (dicomSummary_.GuessPixelDataValueRepresentation(transferSyntax_) != pixelDataVR_)
-            {
-              // Store the VR of pixel data if it doesn't comply with the standard
-              SetInstanceMetadata(content, instanceMetadata_, instanceId,
-                                  MetadataType_Instance_PixelDataVR,
-                                  EnumerationToString(pixelDataVR_));
-            }
+                                MetadataType_Instance_PixelDataVR,
+                                EnumerationToString(pixelDataVR_));
           }
-      
-          const DicomValue* value;
-          if ((value = dicomSummary_.TestAndGetValue(DICOM_TAG_SOP_CLASS_UID)) != NULL &&
-              !value->IsNull() &&
+        }
+    
+        const DicomValue* value;
+        if ((value = dicomSummary_.TestAndGetValue(DICOM_TAG_SOP_CLASS_UID)) != NULL &&
+            !value->IsNull() &&
+            !value->IsBinary())
+        {
+          SetInstanceMetadata(content, instanceMetadata_, instanceId,
+                              MetadataType_Instance_SopClassUid, value->GetContent());
+        }
+
+
+        if ((value = dicomSummary_.TestAndGetValue(DICOM_TAG_INSTANCE_NUMBER)) != NULL ||
+            (value = dicomSummary_.TestAndGetValue(DICOM_TAG_IMAGE_INDEX)) != NULL)
+        {
+          if (!value->IsNull() && 
               !value->IsBinary())
           {
             SetInstanceMetadata(content, instanceMetadata_, instanceId,
-                                MetadataType_Instance_SopClassUid, value->GetContent());
+                                MetadataType_Instance_IndexInSeries, Toolbox::StripSpaces(value->GetContent()));
           }
-
-
-          if ((value = dicomSummary_.TestAndGetValue(DICOM_TAG_INSTANCE_NUMBER)) != NULL ||
-              (value = dicomSummary_.TestAndGetValue(DICOM_TAG_IMAGE_INDEX)) != NULL)
+        }
+
+    
+        transaction.SetResourcesContent(content);
+
+
+        if (!isReconstruct_)  // a reconstruct shall not trigger any events
+        {
+          // Check whether the series of this new instance is now completed
+          int64_t expectedNumberOfInstances;
+          if (ComputeExpectedNumberOfInstances(expectedNumberOfInstances, dicomSummary_))
           {
-            if (!value->IsNull() && 
-                !value->IsBinary())
+            SeriesStatus seriesStatus = transaction.GetSeriesStatus(status.seriesId_, expectedNumberOfInstances);
+            if (seriesStatus == SeriesStatus_Complete)
             {
-              SetInstanceMetadata(content, instanceMetadata_, instanceId,
-                                  MetadataType_Instance_IndexInSeries, Toolbox::StripSpaces(value->GetContent()));
+              transaction.LogChange(status.seriesId_, ChangeType_CompletedSeries, ResourceType_Series, hashSeries_);
             }
           }
-
-      
-          transaction.SetResourcesContent(content);
+          
+          transaction.LogChange(status.seriesId_, ChangeType_NewChildInstance, ResourceType_Series, hashSeries_);
+          transaction.LogChange(status.studyId_, ChangeType_NewChildInstance, ResourceType_Study, hashStudy_);
+          transaction.LogChange(status.patientId_, ChangeType_NewChildInstance, ResourceType_Patient, hashPatient_);
+          
+          // Mark the parent resources of this instance as unstable
+          transaction.GetTransactionContext().MarkAsUnstable(ResourceType_Series, status.seriesId_, hashSeries_);
+          transaction.GetTransactionContext().MarkAsUnstable(ResourceType_Study, status.studyId_, hashStudy_);
+          transaction.GetTransactionContext().MarkAsUnstable(ResourceType_Patient, status.patientId_, hashPatient_);
         }
 
-
-        // Check whether the series of this new instance is now completed
-        int64_t expectedNumberOfInstances;
-        if (ComputeExpectedNumberOfInstances(expectedNumberOfInstances, dicomSummary_))
-        {
-          SeriesStatus seriesStatus = transaction.GetSeriesStatus(status.seriesId_, expectedNumberOfInstances);
-          if (seriesStatus == SeriesStatus_Complete)
-          {
-            transaction.LogChange(status.seriesId_, ChangeType_CompletedSeries, ResourceType_Series, hashSeries_);
-          }
-        }
-        
-        transaction.LogChange(status.seriesId_, ChangeType_NewChildInstance, ResourceType_Series, hashSeries_);
-        transaction.LogChange(status.studyId_, ChangeType_NewChildInstance, ResourceType_Study, hashStudy_);
-        transaction.LogChange(status.patientId_, ChangeType_NewChildInstance, ResourceType_Patient, hashPatient_);
-        
-        // Mark the parent resources of this instance as unstable
-        transaction.GetTransactionContext().MarkAsUnstable(ResourceType_Series, status.seriesId_, hashSeries_);
-        transaction.GetTransactionContext().MarkAsUnstable(ResourceType_Study, status.studyId_, hashStudy_);
-        transaction.GetTransactionContext().MarkAsUnstable(ResourceType_Patient, status.patientId_, hashPatient_);
         transaction.GetTransactionContext().SignalAttachmentsAdded(instanceSize);
-
-        storeStatus_ = StoreStatus_Success;          
+        storeStatus_ = StoreStatus_Success;
       }
     };