changeset 5690:708952bd869c find-refactoring

integration tests are passing with ResourceFinder
author Sebastien Jodogne <s.jodogne@gmail.com>
date Tue, 09 Jul 2024 18:05:54 +0200
parents c14776d25491
children c352d762177c
files OrthancServer/Sources/Database/FindResponse.cpp OrthancServer/Sources/Database/FindResponse.h OrthancServer/Sources/OrthancRestApi/OrthancRestResources.cpp OrthancServer/Sources/ResourceFinder.cpp OrthancServer/Sources/Search/DatabaseConstraint.cpp OrthancServer/Sources/Search/DatabaseConstraint.h OrthancServer/Sources/Search/DicomTagConstraint.cpp OrthancServer/Sources/ServerContext.cpp
diffstat 8 files changed, 192 insertions(+), 32 deletions(-) [+]
line wrap: on
line diff
--- a/OrthancServer/Sources/Database/FindResponse.cpp	Tue Jul 09 15:56:53 2024 +0200
+++ b/OrthancServer/Sources/Database/FindResponse.cpp	Tue Jul 09 18:05:54 2024 +0200
@@ -313,6 +313,30 @@
   }
 
 
+  void FindResponse::Resource::GetAllMainDicomTags(DicomMap& target) const
+  {
+    switch (level_)
+    {
+      // Don't reorder or add "break" below
+      case ResourceType_Instance:
+        mainDicomTagsInstance_.Export(target);
+
+      case ResourceType_Series:
+        mainDicomTagsSeries_.Export(target);
+
+      case ResourceType_Study:
+        mainDicomTagsStudy_.Export(target);
+
+      case ResourceType_Patient:
+        mainDicomTagsPatient_.Export(target);
+        break;
+
+      default:
+        throw OrthancException(ErrorCode_ParameterOutOfRange);
+    }
+  }
+
+
   void FindResponse::Resource::AddMetadata(ResourceType level,
                                            MetadataType metadata,
                                            const std::string& value)
--- a/OrthancServer/Sources/Database/FindResponse.h	Tue Jul 09 15:56:53 2024 +0200
+++ b/OrthancServer/Sources/Database/FindResponse.h	Tue Jul 09 18:05:54 2024 +0200
@@ -190,6 +190,8 @@
         GetMainDicomTagsAtLevel(level).Export(target);
       }
 
+      void GetAllMainDicomTags(DicomMap& target) const;
+
       void AddMetadata(ResourceType level,
                        MetadataType metadata,
                        const std::string& value);
--- a/OrthancServer/Sources/OrthancRestApi/OrthancRestResources.cpp	Tue Jul 09 15:56:53 2024 +0200
+++ b/OrthancServer/Sources/OrthancRestApi/OrthancRestResources.cpp	Tue Jul 09 18:05:54 2024 +0200
@@ -3329,7 +3329,7 @@
       throw OrthancException(ErrorCode_BadRequest, 
                              "Field \"" + std::string(KEY_LABELS_CONSTRAINT) + "\" must be an array of strings");
     }
-    else if (false)
+    else if (true)
     {
       /**
        * EXPERIMENTAL VERSION
@@ -3379,6 +3379,39 @@
         finder.SetLimits(since, limit);
       }
 
+      {
+        bool caseSensitive = false;
+        if (request.isMember(KEY_CASE_SENSITIVE))
+        {
+          caseSensitive = request[KEY_CASE_SENSITIVE].asBool();
+        }
+
+        DatabaseLookup query;
+
+        Json::Value::Members members = request[KEY_QUERY].getMemberNames();
+        for (size_t i = 0; i < members.size(); i++)
+        {
+          if (request[KEY_QUERY][members[i]].type() != Json::stringValue)
+          {
+            throw OrthancException(ErrorCode_BadRequest,
+                                   "Tag \"" + members[i] + "\" must be associated with a string");
+          }
+
+          const std::string value = request[KEY_QUERY][members[i]].asString();
+
+          if (!value.empty())
+          {
+            // An empty string corresponds to an universal constraint,
+            // so we ignore it. This mimics the behavior of class
+            // "OrthancFindRequestHandler"
+            query.AddRestConstraint(FromDcmtkBridge::ParseTag(members[i]),
+                                    value, caseSensitive, true);
+          }
+        }
+
+        finder.SetDatabaseLookup(query);
+      }
+
       if (request.isMember(KEY_REQUESTED_TAGS))
       {
         std::set<DicomTag> requestedTags;
--- a/OrthancServer/Sources/ResourceFinder.cpp	Tue Jul 09 15:56:53 2024 +0200
+++ b/OrthancServer/Sources/ResourceFinder.cpp	Tue Jul 09 18:05:54 2024 +0200
@@ -424,28 +424,28 @@
 
   void ResourceFinder::UpdateRequestLimits()
   {
-    // TODO-FIND: Check this
+    // By default, use manual paging
+    isDatabasePaging_ = false;
+
+    if (databaseLimits_ != 0)
+    {
+      request_.SetLimits(0, databaseLimits_ + 1);
+    }
 
     if (lookup_.get() == NULL ||
         lookup_->HasOnlyMainDicomTags())
     {
-      isDatabasePaging_ = true;
+      // TODO-FIND: Understand why this doesn't work:
+      // $ ./Start.sh --force Orthanc.test_rest_find_limit Orthanc.test_resources_since_limit Orthanc.test_rest_find_limit
 
-      if (hasLimits_)
+      /*if (hasLimits_)
       {
+        isDatabasePaging_ = true;
         request_.SetLimits(limitsSince_, limitsCount_);
-      }
+        }*/
     }
-    else if (databaseLimits_ != 0)
-    {
-      // The "+ 1" below is used to test the completeness of the response
-      request_.SetLimits(0, databaseLimits_ + 1);
-      isDatabasePaging_ = false;
-    }
-    else
-    {
-      isDatabasePaging_ = false;
-    }
+
+    // TODO-FIND: More cases could be added, depending on "GetDatabaseCapabilities()"
   }
 
 
@@ -541,11 +541,29 @@
     MainDicomTagsRegistry registry;
     registry.NormalizeLookup(request_.GetDicomTagConstraints(), lookup, request_.GetLevel());
 
-    // Sanity check
+    // "request_.GetDicomTagConstraints()" only contains constraints on main DICOM tags
+
     for (size_t i = 0; i < request_.GetDicomTagConstraints().GetSize(); i++)
     {
-      if (IsComputedTag(request_.GetDicomTagConstraints().GetConstraint(i).GetTag()))
+      const DatabaseConstraint& constraint = request_.GetDicomTagConstraints().GetConstraint(i);
+      if (constraint.GetLevel() == request_.GetLevel())
+      {
+        request_.SetRetrieveMainDicomTags(true);
+      }
+      else if (IsResourceLevelAboveOrEqual(constraint.GetLevel(), request_.GetLevel()))
       {
+        request_.GetParentSpecification(constraint.GetLevel()).SetRetrieveMainDicomTags(true);
+      }
+      else
+      {
+        LOG(WARNING) << "Executing a database lookup at level " << EnumerationToString(request_.GetLevel())
+                     << " on main DICOM tag " << constraint.GetTag().Format() << " from an inferior level ("
+                     << EnumerationToString(constraint.GetLevel()) << "), this will return no result";
+      }
+
+      if (IsComputedTag(constraint.GetTag()))
+      {
+        // Sanity check
         throw OrthancException(ErrorCode_InternalError);
       }
     }
@@ -824,8 +842,7 @@
                   response.GetSize() <= databaseLimits_);
     }
 
-    if (lookup_.get() != NULL &&
-        !lookup_->HasOnlyMainDicomTags())
+    if (lookup_.get() != NULL)
     {
       LOG(INFO) << "Number of candidate resources after fast DB filtering on main DICOM tags: " << response.GetSize();
     }
@@ -868,7 +885,7 @@
       if (lookup_.get() != NULL)
       {
         DicomMap tags;
-        resource.GetMainDicomTags(tags, request_.GetLevel());
+        resource.GetAllMainDicomTags(tags);
         tags.Merge(requestedTags);
         match = lookup_->IsMatch(tags);
       }
--- a/OrthancServer/Sources/Search/DatabaseConstraint.cpp	Tue Jul 09 15:56:53 2024 +0200
+++ b/OrthancServer/Sources/Search/DatabaseConstraint.cpp	Tue Jul 09 18:05:54 2024 +0200
@@ -37,6 +37,7 @@
 #  include <OrthancException.h>
 #endif
 
+#include <boost/lexical_cast.hpp>
 #include <cassert>
 
 
@@ -284,4 +285,64 @@
       return *constraints_[index];
     }
   }
+
+
+  std::string DatabaseConstraints::Format() const
+  {
+    std::string s;
+
+    for (size_t i = 0; i < constraints_.size(); i++)
+    {
+      assert(constraints_[i] != NULL);
+      const DatabaseConstraint& constraint = *constraints_[i];
+      s += "Constraint " + boost::lexical_cast<std::string>(i) + " at " + EnumerationToString(constraint.GetLevel()) +
+        ": " + constraint.GetTag().Format();
+
+      switch (constraint.GetConstraintType())
+      {
+        case ConstraintType_Equal:
+          s += " == " + constraint.GetSingleValue();
+          break;
+
+        case ConstraintType_SmallerOrEqual:
+          s += " <= " + constraint.GetSingleValue();
+          break;
+
+        case ConstraintType_GreaterOrEqual:
+          s += " >= " + constraint.GetSingleValue();
+          break;
+
+        case ConstraintType_Wildcard:
+          s += " ~~ " + constraint.GetSingleValue();
+          break;
+
+        case ConstraintType_List:
+        {
+          s += " in [ ";
+          bool first = true;
+          for (size_t i = 0; i < constraint.GetValuesCount(); i++)
+          {
+            if (first)
+            {
+              first = false;
+            }
+            else
+            {
+              s += ", ";
+            }
+            s += constraint.GetValue(i);
+          }
+          s += "]";
+          break;
+        }
+
+        default:
+          throw OrthancException(ErrorCode_InternalError);
+      }
+
+      s += "\n";
+    }
+
+    return s;
+  }
 }
--- a/OrthancServer/Sources/Search/DatabaseConstraint.h	Tue Jul 09 15:56:53 2024 +0200
+++ b/OrthancServer/Sources/Search/DatabaseConstraint.h	Tue Jul 09 18:05:54 2024 +0200
@@ -178,5 +178,7 @@
     }
 
     const DatabaseConstraint& GetConstraint(size_t index) const;
+
+    std::string Format() const;
   };
 }
--- a/OrthancServer/Sources/Search/DicomTagConstraint.cpp	Tue Jul 09 15:56:53 2024 +0200
+++ b/OrthancServer/Sources/Search/DicomTagConstraint.cpp	Tue Jul 09 18:05:54 2024 +0200
@@ -215,6 +215,24 @@
   }
 
 
+  static bool HasIntersection(const std::set<std::string>& expected,
+                              const std::string& values)
+  {
+    std::vector<std::string> tokens;
+    Toolbox::TokenizeString(tokens, values, '\\');
+
+    for (size_t i = 0; i < tokens.size(); i++)
+    {
+      if (expected.find(tokens[i]) != expected.end())
+      {
+        return true;
+      }
+    }
+
+    return false;
+  }
+
+
   bool DicomTagConstraint::IsMatch(const std::string& value) const
   {
     NormalizedString source(value, caseSensitive_);
@@ -224,7 +242,17 @@
       case ConstraintType_Equal:
       {
         NormalizedString reference(GetValue(), caseSensitive_);
-        return source.GetValue() == reference.GetValue();
+
+        if (GetTag() == DICOM_TAG_MODALITIES_IN_STUDY)
+        {
+          std::set<std::string> expected;
+          expected.insert(reference.GetValue());
+          return HasIntersection(expected, source.GetValue());
+        }
+        else
+        {
+          return source.GetValue() == reference.GetValue();
+        }
       }
 
       case ConstraintType_SmallerOrEqual:
@@ -251,17 +279,16 @@
 
       case ConstraintType_List:
       {
+        std::set<std::string> references;
+
         for (std::set<std::string>::const_iterator
                it = values_.begin(); it != values_.end(); ++it)
         {
           NormalizedString reference(*it, caseSensitive_);
-          if (source.GetValue() == reference.GetValue())
-          {
-            return true;
-          }
+          references.insert(reference.GetValue());
         }
 
-        return false;
+        return HasIntersection(references, source.GetValue());
       }
 
       default:
--- a/OrthancServer/Sources/ServerContext.cpp	Tue Jul 09 15:56:53 2024 +0200
+++ b/OrthancServer/Sources/ServerContext.cpp	Tue Jul 09 18:05:54 2024 +0200
@@ -1564,12 +1564,6 @@
 
       ResourceFinder finder(queryLevel, false /* TODO-FIND: don't expand for now */);
       finder.SetDatabaseLimits(databaseLimit);
-
-      if (databaseLimit != 0)
-      {
-        finder.SetLimits(0, databaseLimit + 1);
-      }
-
       finder.SetDatabaseLookup(lookup);
       finder.SetLabels(labels);
       finder.SetLabelsConstraint(labelsConstraint);