Mercurial > hg > orthanc

changeset 5058:d4e5ca0c9307

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Fix the "Never" option of the "StorageAccessOnFind" that was sill accessing files (bug introduced in 1.11.0)
author Alain Mazy <am@osimis.io>
date Wed, 03 Aug 2022 10:49:50 +0200
parents e6f26be401fa
children 5c997c72603c
files NEWS OrthancServer/Sources/OrthancFindRequestHandler.cpp OrthancServer/Sources/OrthancRestApi/OrthancRestResources.cpp OrthancServer/Sources/OrthancWebDav.cpp OrthancServer/Sources/ServerContext.cpp OrthancServer/Sources/ServerContext.h OrthancServer/Sources/ServerEnumerations.cpp OrthancServer/Sources/ServerEnumerations.h
diffstat 8 files changed, 96 insertions(+), 39 deletions(-) [+]
line wrap: on
line diff
--- a/NEWS	Tue Aug 02 11:38:31 2022 +0200
+++ b/NEWS	Wed Aug 03 10:49:50 2022 +0200
@@ -6,6 +6,13 @@
 
 * Added support for RGBA64 images in tools/create-dicom and /preview
 
+Bug Fixes
+---------
+
+* Fix the "Never" option of the "StorageAccessOnFind" that was sill accessing
+  files (bug introduced in 1.11.0).
+
+
 Maintenance
 -----------
 
--- a/OrthancServer/Sources/OrthancFindRequestHandler.cpp	Tue Aug 02 11:38:31 2022 +0200
+++ b/OrthancServer/Sources/OrthancFindRequestHandler.cpp	Wed Aug 03 10:49:50 2022 +0200
@@ -49,7 +49,8 @@
                         const std::list<DicomTag>& sequencesToReturn,
                         const std::string& defaultPrivateCreator,
                         const std::map<uint16_t, std::string>& privateCreators,
-                        const std::string& retrieveAet)
+                        const std::string& retrieveAet,
+                        bool allowStorageAccess)
   {
     ExpandedResource resource;
     std::set<DicomTag> requestedTags;
@@ -58,7 +59,7 @@
     requestedTags.erase(DICOM_TAG_QUERY_RETRIEVE_LEVEL); // this is not part of the answer
 
     // reuse ExpandResource to get missing tags and computed tags (ModalitiesInStudy ...).  This code is therefore shared between C-Find, tools/find, list-resources and QIDO-RS
-    context.ExpandResource(resource, publicId, mainDicomTags, instanceId, dicomAsJson, level, requestedTags, ExpandResourceDbFlags_IncludeMainDicomTags);
+    context.ExpandResource(resource, publicId, mainDicomTags, instanceId, dicomAsJson, level, requestedTags, ExpandResourceDbFlags_IncludeMainDicomTags, allowStorageAccess);
 
     DicomMap result;
 
@@ -246,6 +247,7 @@
     std::string                 defaultPrivateCreator_;       // the private creator to use if the group is not defined in the query itself
     const std::map<uint16_t, std::string>& privateCreators_;  // the private creators defined in the query itself
     std::string                 retrieveAet_;
+    FindStorageAccessMode       findStorageAccessMode_;
 
   public:
     LookupVisitor(DicomFindAnswers&  answers,
@@ -253,14 +255,16 @@
                   ResourceType level,
                   const DicomMap& query,
                   const std::list<DicomTag>& sequencesToReturn,
-                  const std::map<uint16_t, std::string>& privateCreators) :
+                  const std::map<uint16_t, std::string>& privateCreators,
+                  FindStorageAccessMode findStorageAccessMode) :
       answers_(answers),
       context_(context),
       level_(level),
       query_(query),
       queryAsArray_(query),
       sequencesToReturn_(sequencesToReturn),
-      privateCreators_(privateCreators)
+      privateCreators_(privateCreators),
+      findStorageAccessMode_(findStorageAccessMode)
     {
       answers_.SetComplete(false);
 
@@ -308,7 +312,7 @@
                        const Json::Value* dicomAsJson) ORTHANC_OVERRIDE
     {
       AddAnswer(answers_, context_, publicId, instanceId, mainDicomTags, dicomAsJson, level_, queryAsArray_, sequencesToReturn_,
-                defaultPrivateCreator_, privateCreators_, retrieveAet_);
+                defaultPrivateCreator_, privateCreators_, retrieveAet_, IsStorageAccessAllowedForAnswers(findStorageAccessMode_));
     }
   };
 
@@ -478,7 +482,7 @@
     size_t limit = (level == ResourceType_Instance) ? maxInstances_ : maxResults_;
 
 
-    LookupVisitor visitor(answers, context_, level, *filteredInput, sequencesToReturn, privateCreators);
+    LookupVisitor visitor(answers, context_, level, *filteredInput, sequencesToReturn, privateCreators, context_.GetFindStorageAccessMode());
     context_.Apply(visitor, lookup, level, 0 /* "since" is not relevant to C-FIND */, limit);
   }
 
--- a/OrthancServer/Sources/OrthancRestApi/OrthancRestResources.cpp	Tue Aug 02 11:38:31 2022 +0200
+++ b/OrthancServer/Sources/OrthancRestApi/OrthancRestResources.cpp	Wed Aug 03 10:49:50 2022 +0200
@@ -131,11 +131,12 @@
                                     const std::list<std::string>& resources,
                                     const std::map<std::string, std::string>& instancesIds, // optional: the id of an instance for each found resource.
                                     const std::map<std::string, boost::shared_ptr<DicomMap> >& resourcesMainDicomTags,  // optional: all tags read from DB for a resource (current level and upper levels)
-                                    const std::map<std::string, Json::Value>& resourcesDicomAsJson, // optional: the dicom-as-json for each resource
+                                    const std::map<std::string, const Json::Value*>& resourcesDicomAsJson, // optional: the dicom-as-json for each resource
                                     ResourceType level,
                                     bool expand,
                                     DicomToJsonFormat format,
-                                    const std::set<DicomTag>& requestedTags)
+                                    const std::set<DicomTag>& requestedTags,
+                                    bool allowStorageAccess)
   {
     Json::Value answer = Json::arrayValue;
 
@@ -145,7 +146,26 @@
       if (expand)
       {
         Json::Value expanded;
-        if (context.ExpandResource(expanded, *resource, level, format, requestedTags))
+
+        std::map<std::string, std::string>::const_iterator instanceId = instancesIds.find(*resource);
+        if (instanceId != instancesIds.end())  // if it is found in instancesIds, it is also in resourcesDicomAsJson and mainDicomTags
+        {
+          // reuse data already collected before (e.g during lookup)
+          std::map<std::string, boost::shared_ptr<DicomMap> >::const_iterator mainDicomTags = resourcesMainDicomTags.find(*resource);
+          std::map<std::string, const Json::Value*>::const_iterator dicomAsJson = resourcesDicomAsJson.find(*resource);
+
+          context.ExpandResource(expanded, *resource, 
+                                 *(mainDicomTags->second.get()),
+                                 instanceId->second,
+                                 dicomAsJson->second,
+                                 level, format, requestedTags, allowStorageAccess);
+        }
+        else
+        {
+          context.ExpandResource(expanded, *resource, level, format, requestedTags, allowStorageAccess);
+        }
+
+        if (expanded.type() == Json::objectValue)
         {
           answer.append(expanded);
         }
@@ -166,13 +186,14 @@
                                     ResourceType level,
                                     bool expand,
                                     DicomToJsonFormat format,
-                                    const std::set<DicomTag>& requestedTags)
+                                    const std::set<DicomTag>& requestedTags,
+                                    bool allowStorageAccess)
   {
     std::map<std::string, std::string> unusedInstancesIds;
     std::map<std::string, boost::shared_ptr<DicomMap> > unusedResourcesMainDicomTags;
-    std::map<std::string, Json::Value> unusedResourcesDicomAsJson;
-
-    AnswerListOfResources(output, context, resources, unusedInstancesIds, unusedResourcesMainDicomTags, unusedResourcesDicomAsJson, level, expand, format, requestedTags);
+    std::map<std::string, const Json::Value* > unusedResourcesDicomAsJson;
+
+    AnswerListOfResources(output, context, resources, unusedInstancesIds, unusedResourcesMainDicomTags, unusedResourcesDicomAsJson, level, expand, format, requestedTags, allowStorageAccess);
   }
 
 
@@ -235,7 +256,8 @@
 
     AnswerListOfResources(call.GetOutput(), context, result, resourceType, call.HasArgument("expand"),
                           OrthancRestApi::GetDicomFormat(call, DicomToJsonFormat_Human),
-                          requestedTags);
+                          requestedTags,
+                          true /* allowStorageAccess */);
   }
 
 
@@ -266,7 +288,7 @@
 
     Json::Value json;
     if (OrthancRestApi::GetContext(call).ExpandResource(
-          json, call.GetUriComponent("id", ""), resourceType, format, requestedTags))
+          json, call.GetUriComponent("id", ""), resourceType, format, requestedTags, true /* allowStorageAccess */))
     {
       call.GetOutput().AnswerJson(json);
     }
@@ -2848,17 +2870,19 @@
     private:
       bool                    isComplete_;
       std::list<std::string>  resources_;
+      FindStorageAccessMode   findStorageAccessMode_;
       
       // cache the data we used during lookup and that we could reuse when building the answers
       std::map<std::string, std::string> instancesIds_;         // the id of an instance for each found resource.
       std::map<std::string, boost::shared_ptr<DicomMap> > resourcesMainDicomTags_;  // all tags read from DB for a resource (current level and upper levels)
-      std::map<std::string, Json::Value> resourcesDicomAsJson_; // the dicom-as-json for a resource
+      std::map<std::string, const Json::Value* > resourcesDicomAsJson_; // the dicom-as-json for a resource
 
       DicomToJsonFormat       format_;
 
     public:
-      explicit FindVisitor(DicomToJsonFormat format) :
+      explicit FindVisitor(DicomToJsonFormat format, FindStorageAccessMode findStorageAccessMode) :
         isComplete_(false),
+        findStorageAccessMode_(findStorageAccessMode),
         format_(format)
       {
       }
@@ -2890,7 +2914,7 @@
                   bool expand,
                   const std::set<DicomTag>& requestedTags) const
       {
-        AnswerListOfResources(output, context, resources_, instancesIds_, resourcesMainDicomTags_, resourcesDicomAsJson_, level, expand, format_, requestedTags);
+        AnswerListOfResources(output, context, resources_, instancesIds_, resourcesMainDicomTags_, resourcesDicomAsJson_, level, expand, format_, requestedTags, IsStorageAccessAllowedForAnswers(findStorageAccessMode_));
       }
     };
   }
@@ -3056,7 +3080,7 @@
         }
       }
 
-      FindVisitor visitor(OrthancRestApi::GetDicomFormat(request, DicomToJsonFormat_Human));
+      FindVisitor visitor(OrthancRestApi::GetDicomFormat(request, DicomToJsonFormat_Human), context.GetFindStorageAccessMode());
       context.Apply(visitor, query, level, since, limit);
       visitor.Answer(call.GetOutput(), context, level, expand, requestedTags);
     }
@@ -3119,7 +3143,7 @@
            it = a.begin(); it != a.end(); ++it)
     {
       Json::Value resource;
-      if (OrthancRestApi::GetContext(call).ExpandResource(resource, *it, end, format, requestedTags))
+      if (OrthancRestApi::GetContext(call).ExpandResource(resource, *it, end, format, requestedTags, true /* allowStorageAccess */))
       {
         result.append(resource);
       }
@@ -3238,7 +3262,7 @@
     const DicomToJsonFormat format = OrthancRestApi::GetDicomFormat(call, DicomToJsonFormat_Human);
 
     Json::Value resource;
-    if (OrthancRestApi::GetContext(call).ExpandResource(resource, current, end, format, requestedTags))
+    if (OrthancRestApi::GetContext(call).ExpandResource(resource, current, end, format, requestedTags, true /* allowStorageAccess */))
     {
       call.GetOutput().AnswerJson(resource);
     }
@@ -3645,7 +3669,7 @@
           Json::Value item;
           std::set<DicomTag> emptyRequestedTags;  // not supported for bulk content
 
-          if (OrthancRestApi::GetContext(call).ExpandResource(item, *it, level, format, emptyRequestedTags))
+          if (OrthancRestApi::GetContext(call).ExpandResource(item, *it, level, format, emptyRequestedTags, true /* allowStorageAccess */))
           {
             if (metadata)
             {
@@ -3670,7 +3694,7 @@
           std::set<DicomTag> emptyRequestedTags;  // not supported for bulk content
 
           if (index.LookupResourceType(level, *it) &&
-              OrthancRestApi::GetContext(call).ExpandResource(item, *it, level, format, emptyRequestedTags))
+              OrthancRestApi::GetContext(call).ExpandResource(item, *it, level, format, emptyRequestedTags, true /* allowStorageAccess */))
           {
             if (metadata)
             {
--- a/OrthancServer/Sources/OrthancWebDav.cpp	Tue Aug 02 11:38:31 2022 +0200
+++ b/OrthancServer/Sources/OrthancWebDav.cpp	Wed Aug 03 10:49:50 2022 +0200
@@ -261,7 +261,7 @@
       Json::Value resource;
       std::set<DicomTag> emptyRequestedTags;  // not supported for webdav
 
-      if (context_.ExpandResource(resource, publicId, level_, DicomToJsonFormat_Human, emptyRequestedTags))
+      if (context_.ExpandResource(resource, publicId, level_, DicomToJsonFormat_Human, emptyRequestedTags, true /* allowStorageAccess */))
       {
         if (success_)
         {
--- a/OrthancServer/Sources/ServerContext.cpp	Tue Aug 02 11:38:31 2022 +0200
+++ b/OrthancServer/Sources/ServerContext.cpp	Wed Aug 03 10:49:50 2022 +0200
@@ -1442,15 +1442,14 @@
       // Optimization in Orthanc 1.5.1 - Don't read the full JSON from
       // the disk if only "main DICOM tags" are to be returned
 
-      std::unique_ptr<Json::Value> dicomAsJson;
+      boost::shared_ptr<Json::Value> dicomAsJson;
 
       bool hasOnlyMainDicomTags;
       DicomMap dicom;
       DicomMap allMainDicomTagsFromDB;
       
-      if (findStorageAccessMode_ == FindStorageAccessMode_DatabaseOnly ||
-          findStorageAccessMode_ == FindStorageAccessMode_DiskOnAnswer ||
-          fastLookup->HasOnlyMainDicomTags())
+      if (!IsStorageAccessAllowedForAnswers(findStorageAccessMode_) 
+          || fastLookup->HasOnlyMainDicomTags())
       {
         // Case (1): The main DICOM tags, as stored in the database,
         // are sufficient to look for match
@@ -1538,8 +1537,7 @@
           }
           else
           {
-            if ((findStorageAccessMode_ == FindStorageAccessMode_DiskOnLookupAndAnswer ||
-                findStorageAccessMode_ == FindStorageAccessMode_DiskOnAnswer) &&
+            if (IsStorageAccessAllowedForAnswers(findStorageAccessMode_) &&
                 dicomAsJson.get() == NULL &&
                 isDicomAsJsonNeeded)
             {
@@ -2336,13 +2334,14 @@
                                      const std::string& publicId,
                                      ResourceType level,
                                      DicomToJsonFormat format,
-                                     const std::set<DicomTag>& requestedTags)
+                                     const std::set<DicomTag>& requestedTags,
+                                     bool allowStorageAccess)
   {
     std::string unusedInstanceId;
     Json::Value* unusedDicomAsJson = NULL;
     DicomMap unusedMainDicomTags;
 
-    return ExpandResource(target, publicId, unusedMainDicomTags, unusedInstanceId, unusedDicomAsJson, level, format, requestedTags);
+    return ExpandResource(target, publicId, unusedMainDicomTags, unusedInstanceId, unusedDicomAsJson, level, format, requestedTags, allowStorageAccess);
   }
 
   bool ServerContext::ExpandResource(Json::Value& target,
@@ -2352,11 +2351,12 @@
                                       const Json::Value* dicomAsJson,  // optional: the dicom-as-json for the resource (if already available)
                                      ResourceType level,
                                      DicomToJsonFormat format,
-                                     const std::set<DicomTag>& requestedTags)
+                                     const std::set<DicomTag>& requestedTags,
+                                     bool allowStorageAccess)
   {
     ExpandedResource resource;
 
-    if (ExpandResource(resource, publicId, mainDicomTags, instanceId, dicomAsJson, level, requestedTags, ExpandResourceDbFlags_Default))
+    if (ExpandResource(resource, publicId, mainDicomTags, instanceId, dicomAsJson, level, requestedTags, ExpandResourceDbFlags_Default, allowStorageAccess))
     {
       SerializeExpandedResource(target, resource, format, requestedTags);
       return true;
@@ -2372,7 +2372,8 @@
                                      const Json::Value* dicomAsJson,   // optional: the dicom-as-json for the resource (if already available)
                                      ResourceType level,
                                      const std::set<DicomTag>& requestedTags,
-                                     ExpandResourceDbFlags expandFlags)
+                                     ExpandResourceDbFlags expandFlags,
+                                     bool allowStorageAccess)
   {
     // first try to get the tags from what is already available
     
@@ -2420,7 +2421,8 @@
       }
 
       // possibly merge missing requested tags from dicom-as-json
-      if (!resource.missingRequestedTags_.empty() && !DicomMap::HasOnlyComputedTags(resource.missingRequestedTags_))
+      if (allowStorageAccess
+          && !resource.missingRequestedTags_.empty() && !DicomMap::HasOnlyComputedTags(resource.missingRequestedTags_))
       {
         OrthancConfiguration::ReaderLock lock;
         if (lock.GetConfiguration().IsWarningEnabled(Warnings_001_TagsBeingReadFromStorage))
--- a/OrthancServer/Sources/ServerContext.h	Tue Aug 02 11:38:31 2022 +0200
+++ b/OrthancServer/Sources/ServerContext.h	Wed Aug 03 10:49:50 2022 +0200
@@ -545,7 +545,8 @@
                         const std::string& publicId,
                         ResourceType level,
                         DicomToJsonFormat format,
-                        const std::set<DicomTag>& requestedTags);
+                        const std::set<DicomTag>& requestedTags,
+                        bool allowStorageAccess);
 
     bool ExpandResource(Json::Value& target,
                         const std::string& publicId,
@@ -554,7 +555,8 @@
                         const Json::Value* dicomAsJson,   // optional: the dicom-as-json for the resource
                         ResourceType level,
                         DicomToJsonFormat format,
-                        const std::set<DicomTag>& requestedTags);
+                        const std::set<DicomTag>& requestedTags,
+                        bool allowStorageAccess);
 
     bool ExpandResource(ExpandedResource& target,
                         const std::string& publicId,
@@ -563,7 +565,12 @@
                         const Json::Value* dicomAsJson,   // optional: the dicom-as-json for the resource
                         ResourceType level,
                         const std::set<DicomTag>& requestedTags,
-                        ExpandResourceDbFlags expandFlags);
+                        ExpandResourceDbFlags expandFlags,
+                        bool allowStorageAccess);
 
+    FindStorageAccessMode GetFindStorageAccessMode() const
+    {
+      return findStorageAccessMode_;
+    }
   };
 }
--- a/OrthancServer/Sources/ServerEnumerations.cpp	Tue Aug 02 11:38:31 2022 +0200
+++ b/OrthancServer/Sources/ServerEnumerations.cpp	Wed Aug 03 10:49:50 2022 +0200
@@ -208,6 +208,15 @@
     }    
   }
 
+  bool IsStorageAccessAllowedForAnswers(FindStorageAccessMode mode)
+  {
+    return mode != FindStorageAccessMode_DatabaseOnly;
+  }
+
+  bool IsStorageAccessAllowedForLookup(FindStorageAccessMode mode)
+  {
+    return mode == FindStorageAccessMode_DiskOnLookupAndAnswer;
+  }
 
   BuiltinDecoderTranscoderOrder StringToBuiltinDecoderTranscoderOrder(const std::string& value)
   {
--- a/OrthancServer/Sources/ServerEnumerations.h	Tue Aug 02 11:38:31 2022 +0200
+++ b/OrthancServer/Sources/ServerEnumerations.h	Wed Aug 03 10:49:50 2022 +0200
@@ -217,6 +217,10 @@
 
   FindStorageAccessMode StringToFindStorageAccessMode(const std::string& str);
 
+  bool IsStorageAccessAllowedForAnswers(FindStorageAccessMode mode);
+
+  bool IsStorageAccessAllowedForLookup(FindStorageAccessMode mode);
+
   BuiltinDecoderTranscoderOrder StringToBuiltinDecoderTranscoderOrder(const std::string& str);
 
   Verbosity StringToVerbosity(const std::string& str);