changeset 5567:f3562c1a150d find-refactoring

FindRequest: group metadata and tag constrains in a single class, allow ordering against metadata
author Alain Mazy <am@orthanc.team>
date Tue, 23 Apr 2024 16:49:44 +0200
parents 8b507b1514eb
children b0b5546f1b9f
files OrthancServer/Sources/Database/Compatibility/GenericFind.cpp OrthancServer/Sources/Database/FindRequest.cpp OrthancServer/Sources/Database/FindRequest.h OrthancServer/Sources/Database/SQLiteDatabaseWrapper.cpp
diffstat 4 files changed, 174 insertions(+), 96 deletions(-) [+]
line wrap: on
line diff
--- a/OrthancServer/Sources/Database/Compatibility/GenericFind.cpp	Tue Apr 23 14:37:11 2024 +0200
+++ b/OrthancServer/Sources/Database/Compatibility/GenericFind.cpp	Tue Apr 23 16:49:44 2024 +0200
@@ -37,14 +37,13 @@
           !request.GetOrthancIdentifiers().HasStudyId() &&
           !request.GetOrthancIdentifiers().HasSeriesId() &&
           !request.GetOrthancIdentifiers().HasInstanceId() &&
-          request.GetTagConstraintsCount() == 0 &&
+          request.GetFilterConstraintsCount() == 0 &&
           !request.IsRetrieveTagsAtLevel(ResourceType_Patient) &&
           !request.IsRetrieveTagsAtLevel(ResourceType_Study) &&
           !request.IsRetrieveTagsAtLevel(ResourceType_Series) &&
           !request.IsRetrieveTagsAtLevel(ResourceType_Instance) &&
-          request.GetTagOrdering().empty() &&
-          request.GetLabels().empty() &&
-          request.GetMetadataConstraints().empty())
+          request.GetOrdering().empty() &&
+          request.GetLabels().empty())
       {
         std::list<std::string> ids;
 
--- a/OrthancServer/Sources/Database/FindRequest.cpp	Tue Apr 23 14:37:11 2024 +0200
+++ b/OrthancServer/Sources/Database/FindRequest.cpp	Tue Apr 23 16:49:44 2024 +0200
@@ -59,10 +59,10 @@
 
   FindRequest::FindRequest(ResourceType level) :
     level_(level),
-    responseContent_(ResponseContent_IdentifiersOnly),
     hasLimits_(false),
     limitsSince_(0),
     limitsCount_(0),
+    responseContent_(ResponseContent_IdentifiersOnly),
     retrievePatientTags_(false),
     retrieveStudyTags_(false),
     retrieveSeriesTags_(false),
@@ -73,7 +73,13 @@
 
   FindRequest::~FindRequest()
   {
-    for (std::deque<TagConstraint*>::iterator it = tagConstraints_.begin(); it != tagConstraints_.end(); ++it)
+    for (std::deque<FilterConstraint*>::iterator it = filterConstraints_.begin(); it != filterConstraints_.end(); ++it)
+    {
+      assert(*it != NULL);
+      delete *it;
+    }
+
+    for (std::deque<Ordering*>::iterator it = ordering_.begin(); it != ordering_.end(); ++it)
     {
       assert(*it != NULL);
       delete *it;
@@ -81,7 +87,7 @@
   }
 
 
-  void FindRequest::AddTagConstraint(TagConstraint* constraint /* takes ownership */)
+  void FindRequest::AddFilterConstraint(FilterConstraint* constraint /* takes ownership */)
   {
     if (constraint == NULL)
     {
@@ -89,21 +95,21 @@
     }
     else
     {
-      tagConstraints_.push_back(constraint);
+      filterConstraints_.push_back(constraint);
     }
   }
 
 
-  const FindRequest::TagConstraint& FindRequest::GetTagConstraint(size_t index) const
+  const FindRequest::FilterConstraint& FindRequest::GetFilterConstraint(size_t index) const
   {
-    if (index >= tagConstraints_.size())
+    if (index >= filterConstraints_.size())
     {
       throw OrthancException(ErrorCode_ParameterOutOfRange);
     }
     else
     {
-      assert(tagConstraints_[index] != NULL);
-      return *tagConstraints_[index];
+      assert(filterConstraints_[index] != NULL);
+      return *filterConstraints_[index];
     }
   }
 
@@ -204,39 +210,16 @@
   }
 
 
-  void FindRequest::SetTagOrdering(DicomTag tag,
-                                   Ordering ordering)
+  void FindRequest::AddOrdering(const DicomTag& tag,
+                                OrderingDirection direction)
   {
-    switch (ordering)
-    {
-      case Ordering_None:
-        tagOrdering_.erase(tag);
-        break;
-
-      case Ordering_Ascending:
-        tagOrdering_[tag] = Ordering_Ascending;
-        break;
-
-      case Ordering_Descending:
-        tagOrdering_[tag] = Ordering_Descending;
-        break;
-
-      default:
-        throw OrthancException(ErrorCode_ParameterOutOfRange);
-    }
+    ordering_.push_back(new Ordering(Key(tag), direction));
   }
 
-
-  void FindRequest::AddMetadataConstraint(MetadataType metadata,
-                                          const std::string& value)
+  void FindRequest::AddOrdering(MetadataType metadataType, 
+                                OrderingDirection direction)
   {
-    if (metadataConstraints_.find(metadata) == metadataConstraints_.end())
-    {
-      metadataConstraints_[metadata] = value;
-    }
-    else
-    {
-      throw OrthancException(ErrorCode_BadSequenceOfCalls);
-    }
+    ordering_.push_back(new Ordering(Key(metadataType), direction));
   }
+
 }
--- a/OrthancServer/Sources/Database/FindRequest.h	Tue Apr 23 14:37:11 2024 +0200
+++ b/OrthancServer/Sources/Database/FindRequest.h	Tue Apr 23 16:49:44 2024 +0200
@@ -25,11 +25,13 @@
 #include "../../../OrthancFramework/Sources/DicomFormat/DicomTag.h"
 #include "../ServerEnumerations.h"
 #include "OrthancIdentifiers.h"
+//#include "../Search/DatabaseConstraint.h"
 
 #include <deque>
 #include <map>
 #include <set>
-
+#include <cassert>
+#include <boost/shared_ptr.hpp>
 
 namespace Orthanc
 {
@@ -60,42 +62,119 @@
       ConstraintType_List
     };
 
+    enum KeyType  // used for ordering and filters
+    {
+      KeyType_DicomTag,
+      KeyType_Metadata
+    };
 
-    enum Ordering
+    enum OrderingDirection
+    {
+      OrderingDirection_Ascending,
+      OrderingDirection_Descending
+    };
+
+    enum LabelsConstraint
+    {
+      LabelsConstraint_All,
+      LabelsConstraint_Any,
+      LabelsConstraint_None
+    };
+
+    class Key
     {
-      Ordering_Ascending,
-      Ordering_Descending,
-      Ordering_None
+      KeyType                       type_;
+      boost::shared_ptr<DicomTag>   dicomTag_;
+      MetadataType                  metadata_;
+    public:
+      Key(const DicomTag& dicomTag) :
+        type_(KeyType_DicomTag),
+        dicomTag_(new DicomTag(dicomTag)),
+        metadata_(MetadataType_EndUser)
+      {
+      }
+
+      Key(MetadataType metadata) :
+        type_(KeyType_Metadata),
+        metadata_(metadata)
+      {
+      }
+
+      KeyType GetType() const
+      {
+        return type_;
+      }
+
+      const DicomTag& GetDicomTag() const
+      {
+        assert(GetType() == KeyType_DicomTag);
+        return *dicomTag_;
+      }
+
+      MetadataType GetMetadataType() const
+      {
+        assert(GetType() == KeyType_Metadata);
+        return metadata_;
+      }
+    };
+
+    class Ordering : public boost::noncopyable
+    {
+      OrderingDirection   direction_;
+      Key                 key_;
+
+    public:
+      Ordering(const Key& key,
+               OrderingDirection direction) :
+        direction_(direction),
+        key_(key)
+      {
+      }
+
+    public:
+      KeyType GetKeyType() const
+      {
+        return key_.GetType();
+      }
+
+      OrderingDirection GetDirection() const
+      {
+        return direction_;
+      }
+
+      MetadataType GetMetadataType() const
+      {
+        return key_.GetMetadataType();
+      }
+
+      DicomTag GetDicomTag() const
+      {
+        return key_.GetDicomTag();
+      }
     };
 
 
-    class TagConstraint : public boost::noncopyable
+    class FilterConstraint : public boost::noncopyable
     {
-    private:
-      DicomTag  tag_;
-
-    public:
-      TagConstraint(DicomTag tag) :
-        tag_(tag)
+      Key              key_;
+    
+    protected:
+      FilterConstraint(const Key& key) :
+        key_(key)
       {
       }
 
-      virtual ~TagConstraint()
+    public:
+      virtual ~FilterConstraint()
       {
       }
 
-      virtual DicomTag GetTag() const
-      {
-        return tag_;
-      }
-
       virtual ConstraintType GetType() const = 0;
-
       virtual bool IsCaseSensitive() const = 0;  // Needed for PN VR
     };
 
 
-    class MandatoryConstraint : public TagConstraint
+    class MandatoryConstraint : public FilterConstraint
     {
     public:
       virtual ConstraintType GetType() const ORTHANC_OVERRIDE
@@ -105,15 +184,15 @@
     };
 
 
-    class StringConstraint : public TagConstraint
+    class StringConstraint : public FilterConstraint
     {
     private:
       bool  caseSensitive_;
 
     public:
-      StringConstraint(DicomTag tag,
+      StringConstraint(Key key,
                        bool caseSensitive) :
-        TagConstraint(tag),
+        FilterConstraint(key),
         caseSensitive_(caseSensitive)
       {
       }
@@ -131,10 +210,10 @@
       std::string  value_;
 
     public:
-      explicit EqualityConstraint(DicomTag tag,
+      explicit EqualityConstraint(Key key,
                                   bool caseSensitive,
                                   const std::string& value) :
-        StringConstraint(tag, caseSensitive),
+        StringConstraint(key, caseSensitive),
         value_(value)
       {
       }
@@ -158,11 +237,11 @@
       std::string  end_;    // Inclusive
 
     public:
-      RangeConstraint(DicomTag tag,
+      RangeConstraint(Key key,
                       bool caseSensitive,
                       const std::string& start,
                       const std::string& end) :
-        StringConstraint(tag, caseSensitive),
+        StringConstraint(key, caseSensitive),
         start_(start),
         end_(end)
       {
@@ -191,10 +270,10 @@
       std::string  value_;
 
     public:
-      explicit WildcardConstraint(DicomTag& tag,
+      explicit WildcardConstraint(Key& key,
                                   bool caseSensitive,
                                   const std::string& value) :
-        StringConstraint(tag, caseSensitive),
+        StringConstraint(key, caseSensitive),
         value_(value)
       {
       }
@@ -217,9 +296,9 @@
       std::set<std::string>  values_;
 
     public:
-      ListConstraint(DicomTag tag,
+      ListConstraint(Key key,
                      bool caseSensitive) :
-        StringConstraint(tag, caseSensitive)
+        StringConstraint(key, caseSensitive)
       {
       }
 
@@ -236,20 +315,26 @@
 
 
   private:
-    ResourceType                         level_;
-    ResponseContent                      responseContent_;
-    OrthancIdentifiers                   orthancIdentifiers_;
-    std::deque<TagConstraint*>           tagConstraints_;
+
+    // filter & ordering fields
+    ResourceType                         level_;                // The level of the response (the filtering on tags, labels and metadata also happens at this level)
+    OrthancIdentifiers                   orthancIdentifiers_;   // The response must belong to this Orthanc resources hierarchy
+    std::deque<FilterConstraint*>        filterConstraints_;    // All tags and metadata filters (note: the order is not important)
     bool                                 hasLimits_;
     uint64_t                             limitsSince_;
     uint64_t                             limitsCount_;
+    std::set<std::string>                labels_;
+    LabelsConstraint                     labelsContraint_;
+    std::deque<Ordering*>                ordering_;             // The ordering criteria (note: the order is important !)
+
+    // response fields
+    ResponseContent                      responseContent_;
+    
+    // TODO: check if these 4 options are required.  We might just have a retrieveParentTags that could be part of the ResponseContent enum ?
     bool                                 retrievePatientTags_;
     bool                                 retrieveStudyTags_;
     bool                                 retrieveSeriesTags_;
     bool                                 retrieveInstanceTags_;
-    std::map<DicomTag, Ordering>         tagOrdering_;
-    std::set<std::string>                labels_;
-    std::map<MetadataType, std::string>  metadataConstraints_;
 
     bool IsCompatibleLevel(ResourceType levelOfInterest) const;
 
@@ -263,6 +348,10 @@
       return level_;
     }
 
+    // void GetDatabaseConstraints(std::vector<DatabaseConstraint>& target) const;  // conversion to DatabaseConstraint is required to feed to the LookupFormatter
+    // void GetOrdering(std::vector<Ordering>& target) const;
+
+
     void SetResponseContent(ResponseContent content)
     {
       responseContent_ = content;
@@ -308,14 +397,14 @@
       return orthancIdentifiers_;
     }
 
-    void AddTagConstraint(TagConstraint* constraint /* takes ownership */);
+    void AddFilterConstraint(FilterConstraint* constraint /* takes ownership */);
 
-    size_t GetTagConstraintsCount() const
+    size_t GetFilterConstraintsCount() const
     {
-      return tagConstraints_.size();
+      return filterConstraints_.size();
     }
 
-    const TagConstraint& GetTagConstraint(size_t index) const;
+    const FilterConstraint& GetFilterConstraint(size_t index) const;
 
     void SetLimits(uint64_t since,
                    uint64_t count);
@@ -335,12 +424,13 @@
 
     bool IsRetrieveTagsAtLevel(ResourceType levelOfInterest) const;
 
-    void SetTagOrdering(DicomTag tag,
-                        Ordering ordering);
+    void AddOrdering(const DicomTag& tag, OrderingDirection direction);
+
+    void AddOrdering(MetadataType metadataType, OrderingDirection direction);
 
-    const std::map<DicomTag, Ordering>& GetTagOrdering() const
+    const std::deque<Ordering*>& GetOrdering() const
     {
-      return tagOrdering_;
+      return ordering_;
     }
 
     void AddLabel(const std::string& label)
@@ -352,13 +442,5 @@
     {
       return labels_;
     }
-
-    void AddMetadataConstraint(MetadataType metadata,
-                               const std::string& value);
-
-    const std::map<MetadataType, std::string>& GetMetadataConstraints() const
-    {
-      return metadataConstraints_;
-    }
   };
 }
--- a/OrthancServer/Sources/Database/SQLiteDatabaseWrapper.cpp	Tue Apr 23 14:37:11 2024 +0200
+++ b/OrthancServer/Sources/Database/SQLiteDatabaseWrapper.cpp	Tue Apr 23 16:49:44 2024 +0200
@@ -1143,8 +1143,22 @@
     virtual void ExecuteFind(FindResponse& response,
                              const FindRequest& request) ORTHANC_OVERRIDE
     {
+#if 1
       Compatibility::GenericFind find(*this);
       find.Execute(response, request);
+#else
+      {
+        SQLite::Statement s(db_, SQLITE_FROM_HERE, "DROP TABLE IF EXISTS FilteredResourcesIds");
+        s.Run();
+      }
+
+      {
+        std::string sql;
+        // sql = "CREATE TEMPORARY TABLE FilteredResourcesIds AS ";
+        sql = "..";
+        SQLite::Statement s(db_, SQLITE_FROM_HERE_DYNAMIC(sql), sql);
+      }
+#endif
     }
   };