Mercurial > hg > orthanc

changeset 5693:023787ecaff2 find-refactoring

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
cont
author Sebastien Jodogne <s.jodogne@gmail.com>
date Thu, 11 Jul 2024 21:37:20 +0200
parents 7c11a71927a9
children 4a85ee2cbe1f
files OrthancServer/Sources/Database/MainDicomTagsRegistry.cpp OrthancServer/Sources/Database/MainDicomTagsRegistry.h OrthancServer/Sources/Database/StatelessDatabaseOperations.cpp OrthancServer/Sources/OrthancRestApi/OrthancRestResources.cpp OrthancServer/Sources/ResourceFinder.cpp OrthancServer/Sources/ResourceFinder.h OrthancServer/Sources/Search/DicomTagConstraint.cpp OrthancServer/Sources/Search/DicomTagConstraint.h OrthancServer/UnitTestsSources/ServerIndexTests.cpp
diffstat 9 files changed, 135 insertions(+), 54 deletions(-) [+]
line wrap: on
line diff
--- a/OrthancServer/Sources/Database/MainDicomTagsRegistry.cpp	Thu Jul 11 20:20:50 2024 +0200
+++ b/OrthancServer/Sources/Database/MainDicomTagsRegistry.cpp	Thu Jul 11 21:37:20 2024 +0200
@@ -98,10 +98,12 @@
   }
 
 
-  void MainDicomTagsRegistry::NormalizeLookup(DatabaseConstraints& target,
+  bool MainDicomTagsRegistry::NormalizeLookup(DatabaseConstraints& target,
                                               const DatabaseLookup& source,
                                               ResourceType queryLevel) const
   {
+    bool isEquivalentLookup = true;
+
     target.Clear();
 
     for (size_t i = 0; i < source.GetConstraintsCount(); i++)
@@ -121,8 +123,20 @@
           level = ResourceType_Study;
         }
 
-        target.AddConstraint(source.GetConstraint(i).ConvertToDatabaseConstraint(level, type));
+        bool isEquivalentConstraint;
+        target.AddConstraint(source.GetConstraint(i).ConvertToDatabaseConstraint(isEquivalentConstraint, level, type));
+
+        if (!isEquivalentConstraint)
+        {
+          isEquivalentLookup = false;
+        }
+      }
+      else
+      {
+        isEquivalentLookup = false;
       }
     }
+
+    return isEquivalentLookup;
   }
 }
--- a/OrthancServer/Sources/Database/MainDicomTagsRegistry.h	Thu Jul 11 20:20:50 2024 +0200
+++ b/OrthancServer/Sources/Database/MainDicomTagsRegistry.h	Thu Jul 11 21:37:20 2024 +0200
@@ -75,7 +75,13 @@
                    DicomTagType& type,
                    const DicomTag& tag) const;
 
-    void NormalizeLookup(DatabaseConstraints& target,
+    /**
+     * Returns "true" iff. the normalized lookup is the same as the
+     * original DatabaseLookup. If "false" is returned, the target
+     * constraints are less strict than the original DatabaseLookup,
+     * so more resources will match them.
+     **/
+    bool NormalizeLookup(DatabaseConstraints& target,
                          const DatabaseLookup& source,
                          ResourceType queryLevel) const;
   };
--- a/OrthancServer/Sources/Database/StatelessDatabaseOperations.cpp	Thu Jul 11 20:20:50 2024 +0200
+++ b/OrthancServer/Sources/Database/StatelessDatabaseOperations.cpp	Thu Jul 11 21:37:20 2024 +0200
@@ -1552,7 +1552,8 @@
     DicomTagConstraint c(tag, ConstraintType_Equal, value, true, true);
 
     DatabaseConstraints query;
-    query.AddConstraint(c.ConvertToDatabaseConstraint(level, DicomTagType_Identifier));
+    bool isIdentical;  // unused
+    query.AddConstraint(c.ConvertToDatabaseConstraint(isIdentical, level, DicomTagType_Identifier));
 
 
     class Operations : public IReadOnlyOperations
--- a/OrthancServer/Sources/OrthancRestApi/OrthancRestResources.cpp	Thu Jul 11 20:20:50 2024 +0200
+++ b/OrthancServer/Sources/OrthancRestApi/OrthancRestResources.cpp	Thu Jul 11 21:37:20 2024 +0200
@@ -265,7 +265,8 @@
 
         uint64_t since = boost::lexical_cast<uint64_t>(call.GetArgument("since", ""));
         uint64_t limit = boost::lexical_cast<uint64_t>(call.GetArgument("limit", ""));
-        finder.SetLimits(since, limit);
+        finder.SetLimitsSince(since);
+        finder.SetLimitsCount(limit);
       }
 
       Json::Value answer;
@@ -3347,36 +3348,32 @@
       finder.SetDatabaseLimits(context.GetDatabaseLimits(level));
       finder.SetFormat(OrthancRestApi::GetDicomFormat(request, DicomToJsonFormat_Human));
 
-      size_t limit = 0;
       if (request.isMember(KEY_LIMIT))
       {
-        int tmp = request[KEY_LIMIT].asInt();
+        int64_t tmp = request[KEY_LIMIT].asInt64();
         if (tmp < 0)
         {
           throw OrthancException(ErrorCode_ParameterOutOfRange,
                                  "Field \"" + std::string(KEY_LIMIT) + "\" must be a positive integer");
         }
-
-        limit = static_cast<size_t>(tmp);
+        else
+        {
+          finder.SetLimitsCount(static_cast<uint64_t>(tmp));
+        }
       }
 
-      size_t since = 0;
       if (request.isMember(KEY_SINCE))
       {
-        int tmp = request[KEY_SINCE].asInt();
+        int64_t tmp = request[KEY_SINCE].asInt64();
         if (tmp < 0)
         {
           throw OrthancException(ErrorCode_ParameterOutOfRange,
                                  "Field \"" + std::string(KEY_SINCE) + "\" must be a positive integer");
         }
-
-        since = static_cast<size_t>(tmp);
-      }
-
-      if (request.isMember(KEY_LIMIT) ||
-          request.isMember(KEY_SINCE))
-      {
-        finder.SetLimits(since, limit);
+        else
+        {
+          finder.SetLimitsSince(static_cast<uint64_t>(tmp));
+        }
       }
 
       {
--- a/OrthancServer/Sources/ResourceFinder.cpp	Thu Jul 11 20:20:50 2024 +0200
+++ b/OrthancServer/Sources/ResourceFinder.cpp	Thu Jul 11 21:37:20 2024 +0200
@@ -425,7 +425,7 @@
   void ResourceFinder::UpdateRequestLimits()
   {
     // By default, use manual paging
-    isDatabasePaging_ = false;
+    pagingMode_ = PagingMode_FullManual;
 
     if (databaseLimits_ != 0)
     {
@@ -436,15 +436,34 @@
       request_.ClearLimits();
     }
 
-    if (hasLimits_)
+    if (lookup_.get() == NULL &&
+        (hasLimitsSince_ || hasLimitsCount_))
     {
-      if (lookup_.get() == NULL)
+      pagingMode_ = PagingMode_FullDatabase;
+      request_.SetLimits(limitsSince_, limitsCount_);
+    }
+
+    if (lookup_.get() != NULL &&
+        isSimpleLookup_ &&
+        (hasLimitsSince_ || hasLimitsCount_))
+    {
+      /**
+       * TODO-FIND: "IDatabaseWrapper::ApplyLookupResources()" only
+       * accept the "limit" argument.  The "since" must be implemented
+       * manually.
+       **/
+
+      if (hasLimitsSince_ &&
+          limitsSince_ != 0)
       {
-        isDatabasePaging_ = true;
-        request_.SetLimits(limitsSince_, limitsCount_);
+        pagingMode_ = PagingMode_ManualSkip;
+        request_.SetLimits(0, limitsCount_ + limitsSince_);
       }
-
-      // TODO-FIND: enable database paging on "simple" lookups that involve no normalization
+      else
+      {
+        pagingMode_ = PagingMode_FullDatabase;
+        request_.SetLimits(0, limitsCount_);
+      }
     }
 
     // TODO-FIND: More cases could be added, depending on "GetDatabaseCapabilities()"
@@ -455,8 +474,10 @@
                                  bool expand) :
     request_(level),
     databaseLimits_(0),
-    isDatabasePaging_(true),
-    hasLimits_(false),
+    isSimpleLookup_(true),
+    pagingMode_(PagingMode_FullManual),
+    hasLimitsSince_(false),
+    hasLimitsCount_(false),
     limitsSince_(0),
     limitsCount_(0),
     expand_(expand),
@@ -509,17 +530,30 @@
   }
 
 
-  void ResourceFinder::SetLimits(uint64_t since,
-                                 uint64_t count)
+  void ResourceFinder::SetLimitsSince(uint64_t since)
   {
-    if (hasLimits_)
+    if (hasLimitsSince_)
     {
       throw OrthancException(ErrorCode_BadSequenceOfCalls);
     }
     else
     {
-      hasLimits_ = true;
+      hasLimitsSince_ = true;
       limitsSince_ = since;
+      UpdateRequestLimits();
+    }
+  }
+
+
+  void ResourceFinder::SetLimitsCount(uint64_t count)
+  {
+    if (hasLimitsCount_)
+    {
+      throw OrthancException(ErrorCode_BadSequenceOfCalls);
+    }
+    else
+    {
+      hasLimitsCount_ = true;
       limitsCount_ = count;
       UpdateRequestLimits();
     }
@@ -540,7 +574,7 @@
     }
 
     MainDicomTagsRegistry registry;
-    registry.NormalizeLookup(request_.GetDicomTagConstraints(), lookup, request_.GetLevel());
+    isSimpleLookup_ = registry.NormalizeLookup(request_.GetDicomTagConstraints(), lookup, request_.GetLevel());
 
     // "request_.GetDicomTagConstraints()" only contains constraints on main DICOM tags
 
@@ -835,14 +869,21 @@
     context.GetIndex().ExecuteFind(response, request_);
 
     bool complete;
-    if (isDatabasePaging_)
+
+    switch (pagingMode_)
     {
-      complete = true;
-    }
-    else
-    {
-      complete = (databaseLimits_ == 0 ||
-                  response.GetSize() <= databaseLimits_);
+      case PagingMode_FullDatabase:
+      case PagingMode_ManualSkip:
+        complete = true;
+        break;
+
+      case PagingMode_FullManual:
+        complete = (databaseLimits_ == 0 ||
+                    response.GetSize() <= databaseLimits_);
+        break;
+
+      default:
+        throw OrthancException(ErrorCode_InternalError);
     }
 
     if (lookup_.get() != NULL)
@@ -895,18 +936,18 @@
 
       if (match)
       {
-        if (isDatabasePaging_)
+        if (pagingMode_ == PagingMode_FullDatabase)
         {
           visitor.Apply(resource, hasRequestedTags_, requestedTags);
         }
         else
         {
-          if (hasLimits_ &&
+          if (hasLimitsSince_ &&
               skipped < limitsSince_)
           {
             skipped++;
           }
-          else if (hasLimits_ &&
+          else if (hasLimitsCount_ &&
                    countResults >= limitsCount_)
           {
             // Too many results, don't mark as complete
--- a/OrthancServer/Sources/ResourceFinder.h	Thu Jul 11 20:20:50 2024 +0200
+++ b/OrthancServer/Sources/ResourceFinder.h	Thu Jul 11 21:37:20 2024 +0200
@@ -50,11 +50,20 @@
     };
 
   private:
+    enum PagingMode
+    {
+      PagingMode_FullDatabase,
+      PagingMode_FullManual,
+      PagingMode_ManualSkip
+    };
+
     FindRequest                      request_;
     uint64_t                         databaseLimits_;
     std::unique_ptr<DatabaseLookup>  lookup_;
-    bool                             isDatabasePaging_;
-    bool                             hasLimits_;
+    bool                             isSimpleLookup_;
+    PagingMode                       pagingMode_;
+    bool                             hasLimitsSince_;
+    bool                             hasLimitsCount_;
     uint64_t                         limitsSince_;
     uint64_t                         limitsCount_;
     bool                             expand_;
@@ -122,8 +131,9 @@
       format_ = format;
     }
 
-    void SetLimits(uint64_t since,
-                   uint64_t count);
+    void SetLimitsSince(uint64_t since);
+
+    void SetLimitsCount(uint64_t count);
 
     void SetDatabaseLookup(const DatabaseLookup& lookup);
 
--- a/OrthancServer/Sources/Search/DicomTagConstraint.cpp	Thu Jul 11 20:20:50 2024 +0200
+++ b/OrthancServer/Sources/Search/DicomTagConstraint.cpp	Thu Jul 11 21:37:20 2024 +0200
@@ -369,7 +369,8 @@
   }
 
 
-  DatabaseConstraint* DicomTagConstraint::ConvertToDatabaseConstraint(ResourceType level,
+  DatabaseConstraint* DicomTagConstraint::ConvertToDatabaseConstraint(bool& isIdentical,
+                                                                      ResourceType level,
                                                                       DicomTagType tagType) const
   {
     bool isIdentifier, caseSensitive;
@@ -392,13 +393,21 @@
 
     std::vector<std::string> values;
     values.reserve(values_.size());
-      
+
+    isIdentical = true;
+
     for (std::set<std::string>::const_iterator
            it = values_.begin(); it != values_.end(); ++it)
     {
       if (isIdentifier)
       {
-        values.push_back(ServerToolbox::NormalizeIdentifier(*it));
+        std::string normalized = ServerToolbox::NormalizeIdentifier(*it);
+        values.push_back(normalized);
+
+        if (normalized != *it)
+        {
+          isIdentical = false;
+        }
       }
       else
       {
--- a/OrthancServer/Sources/Search/DicomTagConstraint.h	Thu Jul 11 20:20:50 2024 +0200
+++ b/OrthancServer/Sources/Search/DicomTagConstraint.h	Thu Jul 11 21:37:20 2024 +0200
@@ -109,7 +109,8 @@
 
     std::string Format() const;
 
-    DatabaseConstraint* ConvertToDatabaseConstraint(ResourceType level,
+    DatabaseConstraint* ConvertToDatabaseConstraint(bool& isIdentical /* out */,
+                                                    ResourceType level,
                                                     DicomTagType tagType) const;
   };
 }
--- a/OrthancServer/UnitTestsSources/ServerIndexTests.cpp	Thu Jul 11 20:20:50 2024 +0200
+++ b/OrthancServer/UnitTestsSources/ServerIndexTests.cpp	Thu Jul 11 21:37:20 2024 +0200
@@ -167,7 +167,8 @@
       DicomTagConstraint c(tag, type, value, true, true);
       
       DatabaseConstraints lookup;
-      lookup.AddConstraint(c.ConvertToDatabaseConstraint(level, DicomTagType_Identifier));
+      bool isEquivalent;  // unused
+      lookup.AddConstraint(c.ConvertToDatabaseConstraint(isEquivalent, level, DicomTagType_Identifier));
 
       std::set<std::string> noLabel;
       transaction_->ApplyLookupResources(result, NULL, lookup, level, noLabel, LabelsConstraint_All, 0 /* no limit */);
@@ -187,8 +188,9 @@
       DicomTagConstraint c2(tag, type2, value2, true, true);
 
       DatabaseConstraints lookup;
-      lookup.AddConstraint(c1.ConvertToDatabaseConstraint(level, DicomTagType_Identifier));
-      lookup.AddConstraint(c2.ConvertToDatabaseConstraint(level, DicomTagType_Identifier));
+      bool isEquivalent;  // unused
+      lookup.AddConstraint(c1.ConvertToDatabaseConstraint(isEquivalent, level, DicomTagType_Identifier));
+      lookup.AddConstraint(c2.ConvertToDatabaseConstraint(isEquivalent, level, DicomTagType_Identifier));
       
       std::set<std::string> noLabel;
       transaction_->ApplyLookupResources(result, NULL, lookup, level, noLabel, LabelsConstraint_All, 0 /* no limit */);