changeset 5688:d0a264b803f1 find-refactoring

first implementation of database paging
author Sebastien Jodogne <s.jodogne@gmail.com>
date Tue, 09 Jul 2024 15:36:28 +0200
parents 11575590e493
children c14776d25491
files OrthancServer/Sources/Database/FindRequest.cpp OrthancServer/Sources/OrthancRestApi/OrthancRestResources.cpp OrthancServer/Sources/ResourceFinder.cpp OrthancServer/Sources/ResourceFinder.h OrthancServer/Sources/ServerContext.cpp OrthancServer/Sources/ServerContext.h
diffstat 6 files changed, 192 insertions(+), 25 deletions(-) [+]
line wrap: on
line diff
--- a/OrthancServer/Sources/Database/FindRequest.cpp	Tue Jul 09 12:51:46 2024 +0200
+++ b/OrthancServer/Sources/Database/FindRequest.cpp	Tue Jul 09 15:36:28 2024 +0200
@@ -185,16 +185,9 @@
   void FindRequest::SetLimits(uint64_t since,
                               uint64_t count)
   {
-    if (hasLimits_)
-    {
-      throw OrthancException(ErrorCode_BadSequenceOfCalls);
-    }
-    else
-    {
-      hasLimits_ = true;
-      limitsSince_ = since;
-      limitsCount_ = count;
-    }
+    hasLimits_ = true;
+    limitsSince_ = since;
+    limitsCount_ = count;
   }
 
 
--- a/OrthancServer/Sources/OrthancRestApi/OrthancRestResources.cpp	Tue Jul 09 12:51:46 2024 +0200
+++ b/OrthancServer/Sources/OrthancRestApi/OrthancRestResources.cpp	Tue Jul 09 15:36:28 2024 +0200
@@ -3344,6 +3344,7 @@
       const ResourceType level = StringToResourceType(request[KEY_LEVEL].asCString());
 
       ResourceFinder finder(level, expand);
+      finder.SetDatabaseLimits(context.GetDatabaseLimits(level));
       finder.SetFormat(OrthancRestApi::GetDicomFormat(request, DicomToJsonFormat_Human));
 
       size_t limit = 0;
--- a/OrthancServer/Sources/ResourceFinder.cpp	Tue Jul 09 12:51:46 2024 +0200
+++ b/OrthancServer/Sources/ResourceFinder.cpp	Tue Jul 09 15:36:28 2024 +0200
@@ -422,15 +422,49 @@
   }
 
 
+  void ResourceFinder::UpdateRequestLimits()
+  {
+    // TODO-FIND: Check this
+
+    if (lookup_.get() == NULL ||
+        lookup_->HasOnlyMainDicomTags())
+    {
+      isDatabasePaging_ = true;
+
+      if (hasLimits_)
+      {
+        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;
+    }
+  }
+
+
   ResourceFinder::ResourceFinder(ResourceType level,
                                  bool expand) :
     request_(level),
+    databaseLimits_(0),
+    isDatabasePaging_(true),
+    hasLimits_(false),
+    limitsSince_(0),
+    limitsCount_(0),
     expand_(expand),
     format_(DicomToJsonFormat_Human),
     allowStorageAccess_(true),
     hasRequestedTags_(false),
     includeAllMetadata_(false)
   {
+    UpdateRequestLimits();
+
     if (expand)
     {
       request_.SetRetrieveMainDicomTags(true);
@@ -466,9 +500,34 @@
   }
 
 
+  void ResourceFinder::SetDatabaseLimits(uint64_t limits)
+  {
+    databaseLimits_ = limits;
+    UpdateRequestLimits();
+  }
+
+
+  void ResourceFinder::SetLimits(uint64_t since,
+                                 uint64_t count)
+  {
+    if (hasLimits_)
+    {
+      throw OrthancException(ErrorCode_BadSequenceOfCalls);
+    }
+    else
+    {
+      hasLimits_ = true;
+      limitsSince_ = since;
+      limitsCount_ = count;
+      UpdateRequestLimits();
+    }
+  }
+
+
   void ResourceFinder::SetDatabaseLookup(const DatabaseLookup& lookup)
   {
     lookup_.reset(lookup.Clone());
+    UpdateRequestLimits();
 
     for (size_t i = 0; i < lookup.GetConstraintsCount(); i++)
     {
@@ -748,13 +807,31 @@
   }
 
   
-  void ResourceFinder::Execute(Json::Value& target,
+  void ResourceFinder::Execute(IVisitor& visitor,
                                ServerContext& context) const
   {
     FindResponse response;
     context.GetIndex().ExecuteFind(response, request_);
 
-    target = Json::arrayValue;
+    bool complete;
+    if (isDatabasePaging_)
+    {
+      complete = true;
+    }
+    else
+    {
+      complete = (databaseLimits_ == 0 ||
+                  response.GetSize() <= databaseLimits_);
+    }
+
+    if (lookup_.get() != NULL &&
+        !lookup_->HasOnlyMainDicomTags())
+    {
+      LOG(INFO) << "Number of candidate resources after fast DB filtering on main DICOM tags: " << response.GetSize();
+    }
+
+    size_t countResults = 0;
+    size_t skipped = 0;
 
     for (size_t i = 0; i < response.GetSize(); i++)
     {
@@ -798,26 +875,93 @@
 
       if (match)
       {
-        if (expand_)
+        if (isDatabasePaging_)
+        {
+          visitor.Apply(resource, hasRequestedTags_, requestedTags);
+        }
+        else
+        {
+          if (hasLimits_ &&
+              skipped < limitsSince_)
+          {
+            skipped++;
+          }
+          else if (hasLimits_ &&
+                   countResults >= limitsCount_)
+          {
+            // Too many results, don't mark as complete
+            complete = false;
+            break;
+          }
+          else
+          {
+            visitor.Apply(resource, hasRequestedTags_, requestedTags);
+            countResults++;
+          }
+        }
+      }
+    }
+
+    if (complete)
+    {
+      visitor.MarkAsComplete();
+    }
+  }
+
+
+  void ResourceFinder::Execute(Json::Value& target,
+                               ServerContext& context) const
+  {
+    class Visitor : public IVisitor
+    {
+    private:
+      const ResourceFinder&  that_;
+      ServerIndex& index_;
+      Json::Value& target_;
+
+    public:
+      Visitor(const ResourceFinder& that,
+              ServerIndex& index,
+              Json::Value& target) :
+        that_(that),
+        index_(index),
+        target_(target)
+      {
+      }
+
+      virtual void Apply(const FindResponse::Resource& resource,
+                         bool hasRequestedTags,
+                         const DicomMap& requestedTags) ORTHANC_OVERRIDE
+      {
+        if (that_.expand_)
         {
           Json::Value item;
-          Expand(item, resource, context.GetIndex());
+          that_.Expand(item, resource, index_);
 
-          if (hasRequestedTags_)
+          if (hasRequestedTags)
           {
             static const char* const REQUESTED_TAGS = "RequestedTags";
             item[REQUESTED_TAGS] = Json::objectValue;
-            FromDcmtkBridge::ToJson(item[REQUESTED_TAGS], requestedTags, format_);
+            FromDcmtkBridge::ToJson(item[REQUESTED_TAGS], requestedTags, that_.format_);
           }
 
-          target.append(item);
+          target_.append(item);
         }
         else
         {
-          target.append(resource.GetIdentifier());
+          target_.append(resource.GetIdentifier());
         }
       }
-    }
+
+      virtual void MarkAsComplete() ORTHANC_OVERRIDE
+      {
+      }
+    };
+
+    target = Json::arrayValue;
+
+    Visitor visitor(*this, context.GetIndex(), target);
+    Execute(visitor, context);
   }
 
 
--- a/OrthancServer/Sources/ResourceFinder.h	Tue Jul 09 12:51:46 2024 +0200
+++ b/OrthancServer/Sources/ResourceFinder.h	Tue Jul 09 15:36:28 2024 +0200
@@ -34,9 +34,29 @@
 
   class ResourceFinder : public boost::noncopyable
   {
+  public:
+    class IVisitor : public boost::noncopyable
+    {
+    public:
+      virtual ~IVisitor()
+      {
+      }
+
+      virtual void Apply(const FindResponse::Resource& resource,
+                         bool hasRequestedTags,
+                         const DicomMap& requestedTags) = 0;
+
+      virtual void MarkAsComplete() = 0;
+    };
+
   private:
     FindRequest                      request_;
+    uint64_t                         databaseLimits_;
     std::unique_ptr<DatabaseLookup>  lookup_;
+    bool                             isDatabasePaging_;
+    bool                             hasLimits_;
+    uint64_t                         limitsSince_;
+    uint64_t                         limitsCount_;
     bool                             expand_;
     DicomToJsonFormat                format_;
     bool                             allowStorageAccess_;
@@ -73,10 +93,14 @@
                 const FindResponse::Resource& resource,
                 ServerIndex& index) const;
 
+    void UpdateRequestLimits();
+
   public:
     ResourceFinder(ResourceType level,
                    bool expand);
 
+    void SetDatabaseLimits(uint64_t limits);
+
     bool IsAllowStorageAccess() const
     {
       return allowStorageAccess_;
@@ -99,10 +123,7 @@
     }
 
     void SetLimits(uint64_t since,
-                   uint64_t count)
-    {
-      request_.SetLimits(since, count);
-    }
+                   uint64_t count);
 
     void SetDatabaseLookup(const DatabaseLookup& lookup);
 
@@ -138,6 +159,9 @@
     void Execute(FindResponse& target,
                  ServerIndex& index) const;
 
+    void Execute(IVisitor& visitor,
+                 ServerContext& context) const;
+
     void Execute(Json::Value& target,
                  ServerContext& context) const;
 
--- a/OrthancServer/Sources/ServerContext.cpp	Tue Jul 09 12:51:46 2024 +0200
+++ b/OrthancServer/Sources/ServerContext.cpp	Tue Jul 09 15:36:28 2024 +0200
@@ -1539,8 +1539,7 @@
                             size_t since,
                             size_t limit)
   {    
-    unsigned int databaseLimit = (queryLevel == ResourceType_Instance ?
-                                  limitFindInstances_ : limitFindResults_);
+    const uint64_t databaseLimit = GetDatabaseLimits(queryLevel);
       
     std::vector<std::string> resources, instances;
     const DicomTagConstraint* dicomModalitiesConstraint = NULL;
@@ -1564,6 +1563,7 @@
        **/
 
       ResourceFinder finder(queryLevel, false /* TODO-FIND: don't expand for now */);
+      finder.SetDatabaseLimits(databaseLimit);
 
       if (databaseLimit != 0)
       {
--- a/OrthancServer/Sources/ServerContext.h	Tue Jul 09 12:51:46 2024 +0200
+++ b/OrthancServer/Sources/ServerContext.h	Tue Jul 09 15:36:28 2024 +0200
@@ -442,6 +442,11 @@
 
     void Stop();
 
+    uint64_t GetDatabaseLimits(ResourceType level) const
+    {
+      return (level == ResourceType_Instance ? limitFindInstances_ : limitFindResults_);
+    }
+
     void Apply(ILookupVisitor& visitor,
                const DatabaseLookup& lookup,
                ResourceType queryLevel,