--- a/NEWS	Wed Sep 16 10:22:25 2020 +0200
+++ b/NEWS	Wed Sep 16 13:22:30 2020 +0200
@@ -19,7 +19,7 @@
 * When checking DICOM allowed methods, if there are multiple modalities with the same AET, 
   differentiate them from the calling IP
 * Enable the access to raw frames in Philips ELSCINT1 proprietary compression
-
+* Fix handling of "ModalitiesInStudy" (0008,0061) in C-FIND and "/tools/find"
 
 
 Version 1.7.3 (2020-08-24)

--- a/OrthancServer/Sources/Search/DatabaseLookup.cpp	Wed Sep 16 10:22:25 2020 +0200
+++ b/OrthancServer/Sources/Search/DatabaseLookup.cpp	Wed Sep 16 13:22:30 2020 +0200
@@ -66,7 +66,7 @@
   }
 
 
-  void DatabaseLookup::AddConstraint(DicomTagConstraint* constraint)
+  void DatabaseLookup::AddConstraintInternal(DicomTagConstraint* constraint)
   {
     if (constraint == NULL)
     {
@@ -163,28 +163,20 @@
 
       if (!lower.empty())
       {
-        AddConstraint(new DicomTagConstraint
-                      (tag, ConstraintType_GreaterOrEqual, lower, caseSensitive, mandatoryTag));
+        AddConstraintInternal(new DicomTagConstraint
+                              (tag, ConstraintType_GreaterOrEqual, lower, caseSensitive, mandatoryTag));
       }
 
       if (!upper.empty())
       {
-        AddConstraint(new DicomTagConstraint
-                      (tag, ConstraintType_SmallerOrEqual, upper, caseSensitive, mandatoryTag));
+        AddConstraintInternal(new DicomTagConstraint
+                              (tag, ConstraintType_SmallerOrEqual, upper, caseSensitive, mandatoryTag));
       }
     }
-    else if (tag == DICOM_TAG_MODALITIES_IN_STUDY ||
-             dicomQuery.find('\\') != std::string::npos)
+    else if (dicomQuery.find('\\') != std::string::npos)
     {
       DicomTag fixedTag(tag);
 
-      if (tag == DICOM_TAG_MODALITIES_IN_STUDY)
-      {
-        // http://www.itk.org/Wiki/DICOM_QueryRetrieve_Explained
-        // http://dicomiseasy.blogspot.be/2012/01/dicom-queryretrieve-part-i.html  
-        fixedTag = DICOM_TAG_MODALITY;
-      }
-
       std::unique_ptr<DicomTagConstraint> constraint
         (new DicomTagConstraint(fixedTag, ConstraintType_List, caseSensitive, mandatoryTag));
 
@@ -196,7 +188,7 @@
         constraint->AddValue(items[i]);
       }
 
-      AddConstraint(constraint.release());
+      AddConstraintInternal(constraint.release());
     }
     else if (
       /**
@@ -219,13 +211,13 @@
       (dicomQuery.find('*') != std::string::npos ||
        dicomQuery.find('?') != std::string::npos))
     {
-      AddConstraint(new DicomTagConstraint
-                    (tag, ConstraintType_Wildcard, dicomQuery, caseSensitive, mandatoryTag));
+      AddConstraintInternal(new DicomTagConstraint
+                            (tag, ConstraintType_Wildcard, dicomQuery, caseSensitive, mandatoryTag));
     }
     else
     {
-      AddConstraint(new DicomTagConstraint
-                    (tag, ConstraintType_Equal, dicomQuery, caseSensitive, mandatoryTag));
+      AddConstraintInternal(new DicomTagConstraint
+                            (tag, ConstraintType_Equal, dicomQuery, caseSensitive, mandatoryTag));
     }
   }
 
@@ -313,4 +305,34 @@
 
     return true;
   }
+
+
+  std::string DatabaseLookup::Format() const
+  {
+    std::string s;
+    
+    for (size_t i = 0; i < constraints_.size(); i++)
+    {
+      assert(constraints_[i] != NULL);
+      s += ("Contraint " + boost::lexical_cast<std::string>(i) + ": " +
+            constraints_[i]->Format() + "\n");
+    }
+
+    return s;
+  }
+
+  
+  bool DatabaseLookup::HasTag(const DicomTag& tag) const
+  {
+    for (size_t i = 0; i < constraints_.size(); i++)
+    {
+      assert(constraints_[i] != NULL);
+      if (constraints_[i]->GetTag() == tag)
+      {
+        return true;
+      }
+    }
+
+    return false;
+  }
 }

--- a/OrthancServer/Sources/Search/DatabaseLookup.h	Wed Sep 16 10:22:25 2020 +0200
+++ b/OrthancServer/Sources/Search/DatabaseLookup.h	Wed Sep 16 13:22:30 2020 +0200
@@ -50,7 +50,7 @@
                                     bool caseSensitive,
                                     bool mandatoryTag);
 
-    void AddConstraint(DicomTagConstraint* constraint);  // Takes ownership
+    void AddConstraintInternal(DicomTagConstraint* constraint);  // Takes ownership
 
   public:
     DatabaseLookup()
@@ -87,6 +87,15 @@
                            bool caseSensitive,
                            bool mandatoryTag);
 
+    void AddConstraint(const DicomTagConstraint& constraint)
+    {
+      AddConstraintInternal(new DicomTagConstraint(constraint));
+    }
+
     bool HasOnlyMainDicomTags() const;
+
+    std::string Format() const;
+
+    bool HasTag(const DicomTag& tag) const;
   };
 }

--- a/OrthancServer/Sources/Search/DicomTagConstraint.cpp	Wed Sep 16 10:22:25 2020 +0200
+++ b/OrthancServer/Sources/Search/DicomTagConstraint.cpp	Wed Sep 16 13:22:30 2020 +0200
@@ -154,6 +154,16 @@
   }
 
 
+  DicomTagConstraint::DicomTagConstraint(const DicomTagConstraint& other) :
+    tag_(other.tag_),
+    constraintType_(other.constraintType_),
+    values_(other.values_),
+    caseSensitive_(other.caseSensitive_),
+    mandatory_(other.mandatory_)
+  {
+  }
+    
+
   DicomTagConstraint::DicomTagConstraint(const DatabaseConstraint& constraint) :
     tag_(constraint.GetTag()),
     constraintType_(constraint.GetConstraintType()),
@@ -333,7 +343,7 @@
           s += *it;
         }
 
-        return s + "]";
+        return s + " ]";
       }
 
       default:

--- a/OrthancServer/Sources/Search/DicomTagConstraint.h	Wed Sep 16 10:22:25 2020 +0200
+++ b/OrthancServer/Sources/Search/DicomTagConstraint.h	Wed Sep 16 13:22:30 2020 +0200
@@ -70,6 +70,8 @@
                        bool caseSensitive,
                        bool mandatory);
 
+    DicomTagConstraint(const DicomTagConstraint& other);
+    
     DicomTagConstraint(const DatabaseConstraint& constraint);
 
     const DicomTag& GetTag() const
@@ -77,6 +79,11 @@
       return tag_;
     }
 
+    void SetTag(const DicomTag& tag)
+    {
+      tag_ = tag;
+    }
+
     ConstraintType GetConstraintType() const
     {
       return constraintType_;

--- a/OrthancServer/Sources/ServerContext.cpp	Wed Sep 16 10:22:25 2020 +0200
+++ b/OrthancServer/Sources/ServerContext.cpp	Wed Sep 16 13:22:30 2020 +0200
@@ -34,10 +34,10 @@
 #include "PrecompiledHeadersServer.h"
 #include "ServerContext.h"
 
-#include "../../OrthancFramework/Sources/DicomParsing/Internals/DicomImageDecoder.h"
 #include "../../OrthancFramework/Sources/Cache/SharedArchive.h"
 #include "../../OrthancFramework/Sources/DicomParsing/DcmtkTranscoder.h"
 #include "../../OrthancFramework/Sources/DicomParsing/FromDcmtkBridge.h"
+#include "../../OrthancFramework/Sources/DicomParsing/Internals/DicomImageDecoder.h"
 #include "../../OrthancFramework/Sources/FileStorage/StorageAccessor.h"
 #include "../../OrthancFramework/Sources/HttpServer/FilesystemHttpSender.h"
 #include "../../OrthancFramework/Sources/HttpServer/HttpStreamTranscoder.h"
@@ -936,12 +936,12 @@
   }
 
 
-  void ServerContext::Apply(ILookupVisitor& visitor,
-                            const DatabaseLookup& lookup,
-                            ResourceType queryLevel,
-                            size_t since,
-                            size_t limit)
-  {
+  void ServerContext::ApplyInternal(ILookupVisitor& visitor,
+                                    const DatabaseLookup& lookup,
+                                    ResourceType queryLevel,
+                                    size_t since,
+                                    size_t limit)
+  {    
     unsigned int databaseLimit = (queryLevel == ResourceType_Instance ?
                                   limitFindInstances_ : limitFindResults_);
       
@@ -994,7 +994,6 @@
           continue;
         }
 
-#if 1
         // New in Orthanc 1.6.0: Only keep the main DICOM tags at the
         // level of interest for the query
         switch (queryLevel)
@@ -1016,17 +1015,6 @@
           default:
             throw OrthancException(ErrorCode_InternalError);
         }
-
-        // Special case of the "Modality" at the study level, in order
-        // to deal with C-FIND on "ModalitiesInStudy" (0008,0061).
-        // Check out integration test "test_rest_modalities_in_study".
-        if (queryLevel == ResourceType_Study)
-        {
-          dicom.CopyTagIfExists(tmp, DICOM_TAG_MODALITY);
-        }
-#else
-        dicom.Assign(tmp);  // This emulates Orthanc <= 1.5.8
-#endif
         
         hasOnlyMainDicomTags = true;
       }
@@ -1096,6 +1084,225 @@
   }
 
 
+
+  namespace
+  {
+    class ModalitiesInStudyVisitor : public ServerContext::ILookupVisitor
+    {
+    private:
+      class Study : public boost::noncopyable
+      {
+      private:
+        std::string            orthancId_;
+        std::string            instanceId_;
+        DicomMap               mainDicomTags_;
+        Json::Value            dicomAsJson_;
+        std::set<std::string>  modalitiesInStudy_;
+
+      public:
+        Study(const std::string& instanceId,
+              const DicomMap& seriesTags) :
+          instanceId_(instanceId),
+          dicomAsJson_(Json::nullValue)
+        {
+          {
+            DicomMap tmp;
+            tmp.Assign(seriesTags);
+            tmp.SetValue(DICOM_TAG_SOP_INSTANCE_UID, "dummy", false);
+            DicomInstanceHasher hasher(tmp);
+            orthancId_ = hasher.HashStudy();
+          }
+          
+          mainDicomTags_.MergeMainDicomTags(seriesTags, ResourceType_Study);
+          mainDicomTags_.MergeMainDicomTags(seriesTags, ResourceType_Patient);
+          AddModality(seriesTags);
+        }
+
+        void AddModality(const DicomMap& seriesTags)
+        {
+          std::string modality;
+          if (seriesTags.LookupStringValue(modality, DICOM_TAG_MODALITY, false) &&
+              !modality.empty())
+          {
+            modalitiesInStudy_.insert(modality);
+          }
+        }
+
+        void SetDicomAsJson(const Json::Value& dicomAsJson)
+        {
+          dicomAsJson_ = dicomAsJson;
+        }
+
+        const std::string& GetOrthancId() const
+        {
+          return orthancId_;
+        }
+
+        const std::string& GetInstanceId() const
+        {
+          return instanceId_;
+        }
+
+        const DicomMap& GetMainDicomTags() const
+        {
+          return mainDicomTags_;
+        }
+
+        const Json::Value* GetDicomAsJson() const
+        {
+          if (dicomAsJson_.type() == Json::nullValue)
+          {
+            return NULL;
+          }
+          else
+          {
+            return &dicomAsJson_;
+          }
+        } 
+      };
+      
+      typedef std::map<std::string, Study*>  Studies;
+      
+      bool     isDicomAsJsonNeeded_;
+      bool     complete_;
+      Studies  studies_;
+      
+    public:
+      ModalitiesInStudyVisitor(bool isDicomAsJsonNeeded) :
+        isDicomAsJsonNeeded_(isDicomAsJsonNeeded)
+      {
+      }
+
+      ~ModalitiesInStudyVisitor()
+      {
+        for (Studies::const_iterator it = studies_.begin(); it != studies_.end(); ++it)
+        {
+          assert(it->second != NULL);
+          delete it->second;
+        }
+
+        studies_.clear();
+      }
+      
+      virtual bool IsDicomAsJsonNeeded() const
+      {
+        return isDicomAsJsonNeeded_;
+      }
+      
+      virtual void MarkAsComplete()
+      {
+        complete_ = true;
+      }
+      
+      virtual void Visit(const std::string& publicId,
+                         const std::string& instanceId,
+                         const DicomMap& seriesTags,
+                         const Json::Value* dicomAsJson)
+      {
+        std::string studyInstanceUid;
+        if (seriesTags.LookupStringValue(studyInstanceUid, DICOM_TAG_STUDY_INSTANCE_UID, false))
+        {
+          Studies::iterator found = studies_.find(studyInstanceUid);
+          if (found == studies_.end())
+          {
+            // New study
+            std::unique_ptr<Study> study(new Study(instanceId, seriesTags));
+            
+            if (dicomAsJson != NULL)
+            {
+              study->SetDicomAsJson(*dicomAsJson);
+            }
+            
+            studies_[studyInstanceUid] = study.release();
+          }
+          else
+          {
+            // Already existing study
+            found->second->AddModality(seriesTags);
+          }
+        }
+      }
+
+      void Forward(ILookupVisitor& callerVisitor,
+                   size_t since,
+                   size_t limit) const
+     {
+        size_t index = 0;
+        size_t countForwarded = 0;
+        
+        for (Studies::const_iterator it = studies_.begin(); it != studies_.end(); ++it, index++)
+        {
+          if (limit == 0 ||
+              (index >= since &&
+               index < limit))
+          {
+            assert(it->second != NULL);
+            const Study& study = *it->second;
+
+            countForwarded++;
+            callerVisitor.Visit(study.GetOrthancId(), study.GetInstanceId(),
+                                study.GetMainDicomTags(), study.GetDicomAsJson());
+          }
+        }
+
+        if (countForwarded == studies_.size())
+        {
+          callerVisitor.MarkAsComplete();
+        }
+      }
+    };
+  }
+  
+
+  void ServerContext::Apply(ILookupVisitor& visitor,
+                            const DatabaseLookup& lookup,
+                            ResourceType queryLevel,
+                            size_t since,
+                            size_t limit)
+  {
+    if (queryLevel == ResourceType_Study &&
+        lookup.HasTag(DICOM_TAG_MODALITIES_IN_STUDY))
+    {
+      // Convert the study-level query, into a series-level query,
+      // where "ModalitiesInStudy" is replaced by "Modality"
+      DatabaseLookup seriesLookup;
+
+      for (size_t i = 0; i < lookup.GetConstraintsCount(); i++)
+      {
+        const DicomTagConstraint& constraint = lookup.GetConstraint(i);
+        if (constraint.GetTag() == DICOM_TAG_MODALITIES_IN_STUDY)
+        {
+          if ((constraint.GetConstraintType() == ConstraintType_Equal && constraint.GetValue().empty()) ||
+              (constraint.GetConstraintType() == ConstraintType_List && constraint.GetValues().empty()))
+          {
+            // Ignore universal lookup on "ModalitiesInStudy" (0008,0061),
+            // this should have been handled by the caller
+            return ApplyInternal(visitor, lookup, queryLevel, since, limit);
+          }
+          else
+          {
+            DicomTagConstraint modality(constraint);
+            modality.SetTag(DICOM_TAG_MODALITY);
+            seriesLookup.AddConstraint(modality);
+          }
+        }
+        else
+        {
+          seriesLookup.AddConstraint(constraint);
+        }
+      }
+
+      ModalitiesInStudyVisitor seriesVisitor(visitor.IsDicomAsJsonNeeded());
+      ApplyInternal(seriesVisitor, seriesLookup, ResourceType_Series, 0, 0);
+      seriesVisitor.Forward(visitor, since, limit);
+    }
+    else
+    {
+      ApplyInternal(visitor, lookup, queryLevel, since, limit);
+    }
+  }
+  
+
   bool ServerContext::LookupOrReconstructMetadata(std::string& target,
                                                   const std::string& publicId,
                                                   MetadataType metadata)

--- a/OrthancServer/Sources/ServerContext.h	Wed Sep 16 10:22:25 2020 +0200
+++ b/OrthancServer/Sources/ServerContext.h	Wed Sep 16 13:22:30 2020 +0200
@@ -80,6 +80,8 @@
       
       virtual void MarkAsComplete() = 0;
 
+      // NB: "dicomAsJson" must *not* be deleted, and can be NULL if
+      // "!IsDicomAsJsonNeeded()"
       virtual void Visit(const std::string& publicId,
                          const std::string& instanceId,
                          const DicomMap& mainDicomTags,
@@ -237,6 +239,12 @@
                                       DicomInstanceToStore& dicom,
                                       StoreInstanceMode mode);
 
+    void ApplyInternal(ILookupVisitor& visitor,
+                       const DatabaseLookup& lookup,
+                       ResourceType queryLevel,
+                       size_t since,
+                       size_t limit);
+
   public:
     class DicomCacheLocker : public boost::noncopyable
     {

--- a/OrthancServer/UnitTestsSources/DatabaseLookupTests.cpp	Wed Sep 16 10:22:25 2020 +0200
+++ b/OrthancServer/UnitTestsSources/DatabaseLookupTests.cpp	Wed Sep 16 13:22:30 2020 +0200
@@ -154,6 +154,9 @@
     ASSERT_EQ(ConstraintType_Equal, lookup.GetConstraint(0).GetConstraintType());
     ASSERT_EQ("HELLO", lookup.GetConstraint(0).GetValue());
     ASSERT_TRUE(lookup.GetConstraint(0).IsCaseSensitive());
+
+    ASSERT_TRUE(lookup.HasTag(DICOM_TAG_PATIENT_ID));
+    ASSERT_FALSE(lookup.HasTag(DICOM_TAG_PATIENT_NAME));
   }
 
   {