Mercurial > hg > orthanc

changeset 5789:40ad08b75d84 find-refactoring

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
cleanup + W005
author Alain Mazy <am@orthanc.team>
date Tue, 17 Sep 2024 17:16:42 +0200
parents 61c9e5df64d7
children a3d283f61304
files NEWS OrthancServer/Resources/Configuration.json OrthancServer/Sources/OrthancConfiguration.cpp OrthancServer/Sources/ResourceFinder.cpp OrthancServer/Sources/ResourceFinder.h OrthancServer/Sources/ServerEnumerations.h
diffstat 6 files changed, 74 insertions(+), 42 deletions(-) [+]
line wrap: on
line diff
--- a/NEWS	Tue Sep 17 16:17:53 2024 +0200
+++ b/NEWS	Tue Sep 17 17:16:42 2024 +0200
@@ -28,6 +28,8 @@
 * in /system, added a new field "Capabilities" with new values:
   - "HasExtendedChanges" for DB backend that provides this feature (the default SQLite DB
     or PostgreSQL vX.X, MySQL vX.X, ODBC vX.X).
+  - "HasExendedFind" for DB backend that provides this feature (the default SQLite DB
+    or PostgreSQL vX.X, MySQL vX.X, ODBC vX.X).
 * With DB backend with "HasExtendedChanges" support, /changes now supports 2 more options: 
   - 'type' to filter the changes returned by the query 
   - 'to' to potentially cycle through changes in reverse order.
@@ -51,7 +53,10 @@
 * Added a new fallback when trying to decode a frame: transcode the file using the plugin
   before decoding the frame.  This solves some issues with JP2K Lossy compression:
   https://discourse.orthanc-server.org/t/decoding-displaying-jpeg2000-lossy-images/5117
-* Added a new warning that can be disabled in the configuration: W003_DecoderFailure
+* Added new warnings that can be disabled in the configuration: 
+  - W003_DecoderFailure
+  - W004_NoMainDicomTagsSignature
+  - W005_RequestingTagFromLowerResourceLevel
 
 
 
--- a/OrthancServer/Resources/Configuration.json	Tue Sep 17 16:17:53 2024 +0200
+++ b/OrthancServer/Resources/Configuration.json	Tue Sep 17 17:16:42 2024 +0200
@@ -998,8 +998,13 @@
     // You should call patients|studies|series|instances/../reconstruct to rebuild
     // the DB.  You may also check for the "Housekeeper" plugin.
     // (new in Orthanc 1.12.5)
-    "W004_NoMainDicomTagsSignature": true
+    "W004_NoMainDicomTagsSignature": true,
 
+    // Display a warning when a user performs a find request and requests a tag
+    // from a lower resource level; e.g. when requesting "StudyDescription" at
+    // Patient level.
+    // (new in Orthanc 1.12.5)
+    "W005_RequestingTagFromLowerResourceLevel": true
   },
 
   // Configure Orthanc in read only mode.
--- a/OrthancServer/Sources/OrthancConfiguration.cpp	Tue Sep 17 16:17:53 2024 +0200
+++ b/OrthancServer/Sources/OrthancConfiguration.cpp	Tue Sep 17 17:16:42 2024 +0200
@@ -1165,6 +1165,10 @@
         {
           warning = Warnings_004_NoMainDicomTagsSignature;
         }
+        else if (name == "W005_RequestingTagFromLowerResourceLevel")
+        {
+          warning = Warnings_005_RequestingTagFromLowerResourceLevel;
+        }
         else
         {
           throw OrthancException(ErrorCode_BadFileFormat, name + " is not recognized as a valid warning name");
--- a/OrthancServer/Sources/ResourceFinder.cpp	Tue Sep 17 16:17:53 2024 +0200
+++ b/OrthancServer/Sources/ResourceFinder.cpp	Tue Sep 17 17:16:42 2024 +0200
@@ -484,8 +484,18 @@
     limitsCount_(0),
     expand_(expand),
     allowStorageAccess_(true),
-    hasRequestedTags_(false)
+    isWarning002Enabled_(false),
+    isWarning004Enabled_(false),
+    isWarning005Enabled_(false)
   {
+    {
+      OrthancConfiguration::ReaderLock lock;
+      isWarning002Enabled_ = lock.GetConfiguration().IsWarningEnabled(Warnings_002_InconsistentDicomTagsInDb);
+      isWarning004Enabled_ = lock.GetConfiguration().IsWarningEnabled(Warnings_004_NoMainDicomTagsSignature);
+      isWarning005Enabled_ = lock.GetConfiguration().IsWarningEnabled(Warnings_005_RequestingTagFromLowerResourceLevel);
+    }
+
+
     UpdateRequestLimits();
 
     if (expand)
@@ -621,14 +631,12 @@
   void ResourceFinder::AddRequestedTag(const DicomTag& tag)
   {
     requestedTags_.insert(tag);
-    hasRequestedTags_ = true;
 
     if (DicomMap::IsMainDicomTag(tag, ResourceType_Patient))
     {
       if (request_.GetLevel() == ResourceType_Patient)
       {
         request_.SetRetrieveMainDicomTags(true);
-        requestedPatientTags_.insert(tag);
       }
       else
       {
@@ -636,8 +644,6 @@
          * This comes from the fact that patient-level tags are copied
          * at the study level, as implemented by "ResourcesContent::AddResource()".
          **/
-        requestedStudyTags_.insert(tag);
-
         if (request_.GetLevel() == ResourceType_Study)
         {
           request_.SetRetrieveMainDicomTags(true);
@@ -652,8 +658,11 @@
     {
       if (request_.GetLevel() == ResourceType_Patient)
       {
-        LOG(WARNING) << "Requested tag " << tag.Format()
-                     << " should only be read at the study, series, or instance level";
+        if (isWarning005Enabled_)
+        {
+          LOG(WARNING) << "W005: Requested tag " << tag.Format()
+                       << " should only be read at the study, series, or instance level";
+        }
         request_.SetRetrieveOneInstanceMetadataAndAttachments(true); // we might need to get it from one instance
       }
       else
@@ -666,8 +675,6 @@
         {
           request_.GetParentSpecification(ResourceType_Study).SetRetrieveMainDicomTags(true);
         }
-
-        requestedStudyTags_.insert(tag);
       }
     }
     else if (DicomMap::IsMainDicomTag(tag, ResourceType_Series))
@@ -675,8 +682,11 @@
       if (request_.GetLevel() == ResourceType_Patient ||
           request_.GetLevel() == ResourceType_Study)
       {
-        LOG(WARNING) << "Requested tag " << tag.Format()
-                     << " should only be read at the series or instance level";
+        if (isWarning005Enabled_)
+        {
+          LOG(WARNING) << "W005: Requested tag " << tag.Format()
+                      << " should only be read at the series or instance level";
+        }
         request_.SetRetrieveOneInstanceMetadataAndAttachments(true); // we might need to get it from one instance
       }
       else
@@ -689,8 +699,6 @@
         {
           request_.GetParentSpecification(ResourceType_Series).SetRetrieveMainDicomTags(true);
         }
-
-        requestedSeriesTags_.insert(tag);
       }
     }
     else if (DicomMap::IsMainDicomTag(tag, ResourceType_Instance))
@@ -699,14 +707,16 @@
           request_.GetLevel() == ResourceType_Study ||
           request_.GetLevel() == ResourceType_Series)
       {
-        LOG(WARNING) << "Requested tag " << tag.Format()
-                     << " should only be read at the instance level";
+        if (isWarning005Enabled_)
+        {
+          LOG(WARNING) << "W005: Requested tag " << tag.Format()
+                       << " should only be read at the instance level";
+        }
         request_.SetRetrieveOneInstanceMetadataAndAttachments(true); // we might need to get it from one instance
       }
       else
       {
         request_.SetRetrieveMainDicomTags(true);
-        requestedInstanceTags_.insert(tag);
       }
     }
     else if (tag == DICOM_TAG_NUMBER_OF_PATIENT_RELATED_STUDIES)
@@ -757,7 +767,10 @@
     else
     {
       // This is neither a main DICOM tag, nor a computed DICOM tag:
-      // We might need to access the DICOM file
+      // We might need to access a DICOM file or the MainDicomSequences metadata
+      
+      request_.SetRetrieveMetadata(true);
+
       if (request_.GetLevel() != ResourceType_Instance)
       {
         request_.SetRetrieveOneInstanceMetadataAndAttachments(true);
@@ -974,17 +987,17 @@
 
       DicomMap outRequestedTags;
 
-      if (hasRequestedTags_)
+      if (HasRequestedTags())
       {
         std::set<DicomTag> remainingRequestedTags = requestedTags_; // at this point, all requested tags are "missing"
 
         InjectComputedTags(outRequestedTags, resource);
         Toolbox::RemoveSets(remainingRequestedTags, requestedComputedTags_);
 
-        InjectRequestedTags(outRequestedTags, remainingRequestedTags, resource, ResourceType_Patient /*, requestedPatientTags_*/);
-        InjectRequestedTags(outRequestedTags, remainingRequestedTags, resource, ResourceType_Study /*, requestedStudyTags_*/);
-        InjectRequestedTags(outRequestedTags, remainingRequestedTags, resource, ResourceType_Series /*, requestedSeriesTags_*/);
-        InjectRequestedTags(outRequestedTags, remainingRequestedTags, resource, ResourceType_Instance /*, requestedInstanceTags_*/);
+        InjectRequestedTags(outRequestedTags, remainingRequestedTags, resource, ResourceType_Patient);
+        InjectRequestedTags(outRequestedTags, remainingRequestedTags, resource, ResourceType_Study);
+        InjectRequestedTags(outRequestedTags, remainingRequestedTags, resource, ResourceType_Series);
+        InjectRequestedTags(outRequestedTags, remainingRequestedTags, resource, ResourceType_Instance);
 
         if (!remainingRequestedTags.empty())
         {
@@ -1005,20 +1018,20 @@
             mainDicomTagsSignature != DicomMap::GetMainDicomTagsSignature(resource.GetLevel()))
         {
           LOG(WARNING) << "W002: " << Orthanc::GetResourceTypeText(resource.GetLevel(), false , false)
-                      << " has been stored with another version of Main Dicom Tags list, you should POST to /"
-                      << Orthanc::GetResourceTypeText(resource.GetLevel(), true, false)
-                      << "/" << resource.GetIdentifier()
-                      << "/reconstruct to update the list of tags saved in DB or run the Housekeeper plugin.  Some MainDicomTags might be missing from this answer.";
+                       << " has been stored with another version of Main Dicom Tags list, you should POST to /"
+                       << Orthanc::GetResourceTypeText(resource.GetLevel(), true, false)
+                       << "/" << resource.GetIdentifier()
+                       << "/reconstruct to update the list of tags saved in DB or run the Housekeeper plugin.  Some MainDicomTags might be missing from this answer.";
         }
-        else if (isWarning004Enabled &&
+        else if (isWarning004Enabled && 
+                 request_.IsRetrieveMetadata() &&
                  !resource.LookupMetadata(mainDicomTagsSignature, resource.GetLevel(), MetadataType_MainDicomTagsSignature))
         {
-          // LOG(WARNING) << "W004: " << Orthanc::GetResourceTypeText(resource.GetLevel(), false , false)
-          //             << " has been stored a while ago and does not have a MainDicomTagsSignature, you should POST to /"
-          //             << Orthanc::GetResourceTypeText(resource.GetLevel(), true, false)
-          //             << "/" << resource.GetIdentifier()
-          //             << "/reconstruct to update the list of tags saved in DB or run the Housekeeper plugin.  Some MainDicomTags might be missing from this answer.";
-          // TODO: sometimes, we do not read the metadata at all !
+          LOG(WARNING) << "W004: " << Orthanc::GetResourceTypeText(resource.GetLevel(), false , false)
+                       << " has been stored with an old Orthanc version and does not have a MainDicomTagsSignature, you should POST to /"
+                       << Orthanc::GetResourceTypeText(resource.GetLevel(), true, false)
+                       << "/" << resource.GetIdentifier()
+                       << "/reconstruct to update the list of tags saved in DB or run the Housekeeper plugin.  Some MainDicomTags might be missing from this answer.";
         }
 
       }
@@ -1130,7 +1143,7 @@
 
     target = Json::arrayValue;
 
-    Visitor visitor(*this, context.GetIndex(), target, format, hasRequestedTags_, includeAllMetadata);
+    Visitor visitor(*this, context.GetIndex(), target, format, HasRequestedTags(), includeAllMetadata);
     Execute(visitor, context);
   }
 
--- a/OrthancServer/Sources/ResourceFinder.h	Tue Sep 17 16:17:53 2024 +0200
+++ b/OrthancServer/Sources/ResourceFinder.h	Tue Sep 17 17:16:42 2024 +0200
@@ -67,14 +67,13 @@
     uint64_t                         limitsCount_;
     bool                             expand_;
     bool                             allowStorageAccess_;
-    bool                             hasRequestedTags_;
-    std::set<DicomTag>               requestedPatientTags_;
-    std::set<DicomTag>               requestedStudyTags_;
-    std::set<DicomTag>               requestedSeriesTags_;
-    std::set<DicomTag>               requestedInstanceTags_;
     std::set<DicomTag>               requestedTags_;
     std::set<DicomTag>               requestedComputedTags_;
 
+    bool                             isWarning002Enabled_;
+    bool                             isWarning004Enabled_;
+    bool                             isWarning005Enabled_;
+
     bool IsRequestedComputedTag(const DicomTag& tag) const
     {
       return requestedComputedTags_.find(tag) != requestedComputedTags_.end();
@@ -97,6 +96,11 @@
 
     void UpdateRequestLimits();
 
+    bool HasRequestedTags() const
+    {
+      return requestedTags_.size() > 0;
+    }
+
   public:
     ResourceFinder(ResourceType level,
                    bool expand);
--- a/OrthancServer/Sources/ServerEnumerations.h	Tue Sep 17 16:17:53 2024 +0200
+++ b/OrthancServer/Sources/ServerEnumerations.h	Tue Sep 17 17:16:42 2024 +0200
@@ -218,7 +218,8 @@
     Warnings_001_TagsBeingReadFromStorage,
     Warnings_002_InconsistentDicomTagsInDb,
     Warnings_003_DecoderFailure,              // new in Orthanc 1.12.5
-    Warnings_004_NoMainDicomTagsSignature     // new in Orthanc 1.12.5
+    Warnings_004_NoMainDicomTagsSignature,    // new in Orthanc 1.12.5
+    Warnings_005_RequestingTagFromLowerResourceLevel    // new in Orthanc 1.12.5
   };