diff OrthancServer/Sources/ServerJobs/ResourceModificationJob.cpp @ 5137:15109c3f0f7d

added sanity checks in DicomModificationJob + automatically reconstruct resources at the end of a DicomModificationJob
author Alain Mazy <am@osimis.io>
date Wed, 18 Jan 2023 17:58:51 +0100
parents e71b22a43c0b
children 021d7fdcb659
line wrap: on
line diff
--- a/OrthancServer/Sources/ServerJobs/ResourceModificationJob.cpp	Tue Jan 17 17:54:38 2023 +0100
+++ b/OrthancServer/Sources/ServerJobs/ResourceModificationJob.cpp	Wed Jan 18 17:58:51 2023 +0100
@@ -168,7 +168,8 @@
     boost::recursive_mutex::scoped_lock lock(mutex_);
 
     // TODO: cleanup the instances that have been generated during the previous run
-    modifiedInstances_.clear();
+    modifiedSeries_.clear();
+    instancesToReconstruct_.clear();
 
     ThreadedSetOfInstancesJob::Reset();
   }
@@ -178,12 +179,15 @@
     boost::recursive_mutex::scoped_lock lock(mutex_);
 
     // reconstruct the parents MainDicomTags in case one of them has changed
-    if (modifiedInstances_.size() > 0)
+    if (instancesToReconstruct_.size() > 0)
     {
-      ServerContext::DicomCacheLocker locker(GetContext(), *(modifiedInstances_.begin()));
-      ParsedDicomFile& modifiedDicom = locker.GetDicom();
+      for (std::set<std::string>::const_iterator it = instancesToReconstruct_.begin(); it != instancesToReconstruct_.end(); ++it)
+      {
+        ServerContext::DicomCacheLocker locker(GetContext(), *it);
+        ParsedDicomFile& modifiedDicom = locker.GetDicom();
 
-      GetContext().GetIndex().ReconstructInstance(modifiedDicom);
+        GetContext().GetIndex().ReconstructInstance(modifiedDicom);
+      }
     }
     
   }
@@ -320,7 +324,13 @@
       boost::recursive_mutex::scoped_lock lock(outputMutex_);
 
       output_->Update(modifiedHasher);
-      modifiedInstances_.insert(modifiedInstance);
+      if (modifiedSeries_.find(modifiedHasher.HashSeries()) == modifiedSeries_.end())
+      {
+        modifiedSeries_.insert(modifiedHasher.HashSeries());
+        // add an instance to reconstruct for each series
+        instancesToReconstruct_.insert(modifiedHasher.HashInstance());
+      }
+      
     }
 
     return true;
@@ -613,7 +623,7 @@
       {
         value[TRANSCODE] = GetTransferSyntaxUid(transferSyntax_);
       }
-      
+
       origin_.Serialize(value[ORIGIN]);
       
       Json::Value tmp;
@@ -630,4 +640,115 @@
       return true;
     }
   }
+
+  void ResourceModificationJob::PerformSanityChecks()
+  {
+    boost::recursive_mutex::scoped_lock lock(mutex_);  // because we access the parentResources_
+
+    std::set<DicomTag> emptyRequestedTags;
+
+    if (modification_.get() == NULL)
+    {
+      throw OrthancException(ErrorCode_BadSequenceOfCalls);
+    }
+
+    bool replacePatientMainDicomTags = false;
+    bool replaceStudyMainDicomTags = false;
+    bool replaceSeriesMainDicomTags = false;
+    bool replaceInstanceMainDicomTags = false;
+
+    ResourceType modificationLevel = modification_->GetLevel();
+    std::set<DicomTag> replacedTags;
+    modification_->GetReplacedTags(replacedTags);
+
+    for (std::set<DicomTag>::const_iterator it = replacedTags.begin(); it != replacedTags.end(); it++)
+    {
+      replacePatientMainDicomTags |= DicomMap::IsMainDicomTag(*it, ResourceType_Patient);
+      replaceStudyMainDicomTags |= DicomMap::IsMainDicomTag(*it, ResourceType_Study);
+      replaceSeriesMainDicomTags |= DicomMap::IsMainDicomTag(*it, ResourceType_Series);
+      replaceInstanceMainDicomTags |= DicomMap::IsMainDicomTag(*it, ResourceType_Instance);
+    }
+
+    if (modification_->IsKept(DICOM_TAG_SOP_INSTANCE_UID) && !IsKeepSource())
+    {
+      // note: we could refine this criteria -> this is valid only if all DicomUIDs are kept identical (but this can happen through Keep or Replace options)
+      throw OrthancException(ErrorCode_BadRequest,
+                             "When keeping SOPInstanceUID tag, you must set KeepSource to true to avoid deleting the modified files at the end of the process");
+    }
+
+    if (modificationLevel == ResourceType_Study && replacePatientMainDicomTags)
+    {
+      for (std::set<std::string>::const_iterator studyId = parentResources_.begin(); studyId != parentResources_.end(); ++studyId)
+      {
+        // When modifying a study, you may not modify patient tags as you wish.
+        // - If this is the patient's only study, you may modify all patient tags. This could be performed in 2 steps (modify the patient and then, the study) but, 
+        //   for many use cases, it's helpful to be able to do it one step (e.g, to modify a name in a study that has just been acquired)
+        // - If the patient already has other studies, you may only 'attach' the study to an existing patient by modifying 
+        //   all patient tags from the study to match those of the target patient.
+        // - Otherwise, you can't modify the patient tags
+        
+        std::string targetPatientId;
+        if (modification_->IsReplaced(DICOM_TAG_PATIENT_ID))
+        {
+          targetPatientId = modification_->GetReplacementAsString(DICOM_TAG_PATIENT_ID);
+        }
+        else
+        {
+          ExpandedResource originalStudy;
+          if (GetContext().GetIndex().ExpandResource(originalStudy, *studyId, ResourceType_Study, emptyRequestedTags, ExpandResourceDbFlags_IncludeMainDicomTags))
+          {
+            targetPatientId = originalStudy.tags_.GetStringValue(DICOM_TAG_PATIENT_ID, "", false);
+          }
+          else
+          {
+            throw OrthancException(ErrorCode_UnknownResource, "Study not found");
+          }
+        }
+
+        // try to find the targetPatient
+        std::vector<std::string> lookupPatientResult;
+        GetContext().GetIndex().LookupIdentifierExact(lookupPatientResult, ResourceType_Patient, DICOM_TAG_PATIENT_ID, targetPatientId);
+
+        // if the patient exists, check how many child studies it has.
+        if (lookupPatientResult.size() >= 1)
+        {
+          ExpandedResource targetPatient;
+          
+          if (GetContext().GetIndex().ExpandResource(targetPatient, lookupPatientResult[0], ResourceType_Patient, emptyRequestedTags, static_cast<ExpandResourceDbFlags>(ExpandResourceDbFlags_IncludeMainDicomTags | ExpandResourceDbFlags_IncludeChildren)))
+          {
+            const std::list<std::string> childrenIds = targetPatient.childrenIds_;
+            bool targetPatientHasOtherStudies = childrenIds.size() > 1;
+            if (childrenIds.size() == 1)
+            {
+              targetPatientHasOtherStudies = std::find(childrenIds.begin(), childrenIds.end(), *studyId) == childrenIds.end();  // if the patient has one study that is not the one being modified
+            }
+
+            if (targetPatientHasOtherStudies)
+            {
+              // this is allowed if all patient replacedTags do match the target patient tags
+              DicomMap targetPatientTags;
+              targetPatient.tags_.ExtractPatientInformation(targetPatientTags);
+
+              for (std::set<DicomTag>::const_iterator mainPatientTag = DicomMap::GetMainDicomTags(ResourceType_Patient).begin();
+                    mainPatientTag != DicomMap::GetMainDicomTags(ResourceType_Patient).end(); ++mainPatientTag)
+              {
+                if (targetPatientTags.HasTag(*mainPatientTag) 
+                    && (!modification_->IsReplaced(*mainPatientTag) 
+                        || modification_->GetReplacementAsString(*mainPatientTag) != targetPatientTags.GetStringValue(*mainPatientTag, "", false)))
+                {
+                  throw OrthancException(ErrorCode_BadRequest, std::string("Trying to change patient tags in a study.  The Patient already exists and has other studies.  All the 'Replace' tags should match the existing patient main dicom tags.  Try using /patients/../modify instead to modify the patient. Failing tag: ") + mainPatientTag->Format());
+                }
+                else if (!targetPatientTags.HasTag(*mainPatientTag) && modification_->IsReplaced(*mainPatientTag) )
+                {
+                  throw OrthancException(ErrorCode_BadRequest, std::string("Trying to change patient tags in a study.  The Patient already exists and has other studies.  You are trying to replace a tag that is not defined yet in this patient.  Try using /patients/../modify instead to modify the patient. Failing tag: ") + mainPatientTag->Format());
+                }
+              }
+            }
+            // TODO: need reconstruct_
+          }
+        }
+      }
+      
+    }
+  }
 }