Mercurial > hg > orthanc

changeset 3001:7695a9c81099

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
refactoring /tools/find using LookupResource::IVisitor
author Sebastien Jodogne <s.jodogne@gmail.com>
date Tue, 11 Dec 2018 18:36:38 +0100
parents 0a52af0c66e7
children 9ceb7dafae2e
files OrthancServer/OrthancFindRequestHandler.cpp OrthancServer/OrthancRestApi/OrthancRestResources.cpp OrthancServer/Search/LookupIdentifierQuery.cpp OrthancServer/Search/LookupIdentifierQuery.h OrthancServer/Search/LookupResource.h OrthancServer/ServerContext.cpp OrthancServer/ServerContext.h UnitTestsSources/ServerIndexTests.cpp
diffstat 8 files changed, 141 insertions(+), 53 deletions(-) [+]
line wrap: on
line diff
--- a/OrthancServer/OrthancFindRequestHandler.cpp	Tue Dec 11 13:21:34 2018 +0100
+++ b/OrthancServer/OrthancFindRequestHandler.cpp	Tue Dec 11 18:36:38 2018 +0100
@@ -623,8 +623,6 @@
 
       if (FilterQueryTag(value, level, tag, manufacturer))
       {
-        // TODO - Move this to "ResourceLookup::AddDicomConstraint()"
-
         ValueRepresentation vr = FromDcmtkBridge::LookupValueRepresentation(tag);
 
         // DICOM specifies that searches must be case sensitive, except
--- a/OrthancServer/OrthancRestApi/OrthancRestResources.cpp	Tue Dec 11 13:21:34 2018 +0100
+++ b/OrthancServer/OrthancRestApi/OrthancRestResources.cpp	Tue Dec 11 18:36:38 2018 +0100
@@ -1269,6 +1269,42 @@
   }
 
 
+  namespace 
+  {
+    class FindVisitor : public LookupResource::IVisitor
+    {
+    private:
+      bool                    isComplete_;
+      std::list<std::string>  resources_;
+
+    public:
+      FindVisitor() :
+        isComplete_(false)
+      {
+      }
+
+      virtual void MarkAsComplete()
+      {
+        isComplete_ = true;  // Unused information as of Orthanc 1.5.0
+      }
+
+      virtual void Visit(const std::string& publicId,
+                         const Json::Value& dicom)
+      {
+        resources_.push_back(publicId);
+      }
+
+      void Answer(RestApiOutput& output,
+                  ServerIndex& index,
+                  ResourceType level,
+                  bool expand) const
+      {
+        AnswerListOfResources(output, index, resources_, level, expand);
+      }
+    };
+  }
+
+
   static void Find(RestApiPostCall& call)
   {
     static const char* const KEY_CASE_SENSITIVE = "CaseSensitive";
@@ -1281,15 +1317,43 @@
     ServerContext& context = OrthancRestApi::GetContext(call);
 
     Json::Value request;
-    if (call.ParseJsonRequest(request) &&
-        request.type() == Json::objectValue &&
-        request.isMember(KEY_LEVEL) &&
-        request.isMember(KEY_QUERY) &&
-        request[KEY_LEVEL].type() == Json::stringValue &&
-        request[KEY_QUERY].type() == Json::objectValue &&
-        (!request.isMember(KEY_CASE_SENSITIVE) || request[KEY_CASE_SENSITIVE].type() == Json::booleanValue) &&
-        (!request.isMember(KEY_LIMIT) || request[KEY_LIMIT].type() == Json::intValue) &&
-        (!request.isMember(KEY_SINCE) || request[KEY_SINCE].type() == Json::intValue))
+    if (!call.ParseJsonRequest(request) ||
+        request.type() != Json::objectValue)
+    {
+      throw OrthancException(ErrorCode_BadRequest, 
+                             "The body must contain a JSON object");
+    }
+    else if (!request.isMember(KEY_LEVEL) ||
+             request[KEY_LEVEL].type() != Json::stringValue)
+    {
+      throw OrthancException(ErrorCode_BadRequest, 
+                             "Field \"" + std::string(KEY_LEVEL) + "\" is missing, or should be a string");
+    }
+    else if (!request.isMember(KEY_QUERY) &&
+             request[KEY_QUERY].type() != Json::objectValue)
+    {
+      throw OrthancException(ErrorCode_BadRequest, 
+                             "Field \"" + std::string(KEY_QUERY) + "\" is missing, or should be a JSON object");
+    }
+    else if (request.isMember(KEY_CASE_SENSITIVE) && 
+             request[KEY_CASE_SENSITIVE].type() != Json::booleanValue)
+    {
+      throw OrthancException(ErrorCode_BadRequest, 
+                             "Field \"" + std::string(KEY_CASE_SENSITIVE) + "\" should be a Boolean");
+    }
+    else if (request.isMember(KEY_LIMIT) && 
+             request[KEY_LIMIT].type() != Json::intValue)
+    {
+      throw OrthancException(ErrorCode_BadRequest, 
+                             "Field \"" + std::string(KEY_LIMIT) + "\" should be an integer");
+    }
+    else if (request.isMember(KEY_SINCE) &&
+             request[KEY_SINCE].type() != Json::intValue)
+    {
+      throw OrthancException(ErrorCode_BadRequest, 
+                             "Field \"" + std::string(KEY_SINCE) + "\" should be an integer");
+    }
+    else
     {
       bool expand = false;
       if (request.isMember(KEY_EXPAND))
@@ -1309,7 +1373,8 @@
         int tmp = request[KEY_LIMIT].asInt();
         if (tmp < 0)
         {
-          throw OrthancException(ErrorCode_ParameterOutOfRange);
+          throw OrthancException(ErrorCode_ParameterOutOfRange,
+                                 "Field \"" + std::string(KEY_LIMIT) + "\" should be a positive integer");
         }
 
         limit = static_cast<size_t>(tmp);
@@ -1321,7 +1386,8 @@
         int tmp = request[KEY_SINCE].asInt();
         if (tmp < 0)
         {
-          throw OrthancException(ErrorCode_ParameterOutOfRange);
+          throw OrthancException(ErrorCode_ParameterOutOfRange,
+                                 "Field \"" + std::string(KEY_SINCE) + "\" should be a positive integer");
         }
 
         since = static_cast<size_t>(tmp);
@@ -1336,7 +1402,8 @@
       {
         if (request[KEY_QUERY][members[i]].type() != Json::stringValue)
         {
-          throw OrthancException(ErrorCode_BadRequest);
+          throw OrthancException(ErrorCode_BadRequest,
+                                 "Tag \"" + members[i] + "\" should be associated with a string");
         }
 
         query.AddDicomConstraint(FromDcmtkBridge::ParseTag(members[i]), 
@@ -1344,15 +1411,9 @@
                                  caseSensitive);
       }
 
-      bool isComplete;
-      std::list<std::string> resources;
-      context.Apply(isComplete, resources, query, since, limit);
-      AnswerListOfResources(call.GetOutput(), context.GetIndex(),
-                            resources, query.GetLevel(), expand);
-    }
-    else
-    {
-      throw OrthancException(ErrorCode_BadRequest);
+      FindVisitor visitor;
+      context.Apply(visitor, query, since, limit);
+      visitor.Answer(call.GetOutput(), context.GetIndex(), query.GetLevel(), expand);
     }
   }
 
--- a/OrthancServer/Search/LookupIdentifierQuery.cpp	Tue Dec 11 13:21:34 2018 +0100
+++ b/OrthancServer/Search/LookupIdentifierQuery.cpp	Tue Dec 11 18:36:38 2018 +0100
@@ -34,9 +34,10 @@
 #include "../PrecompiledHeadersServer.h"
 #include "LookupIdentifierQuery.h"
 
+#include "../../Core/DicomParsing/FromDcmtkBridge.h"
 #include "../../Core/OrthancException.h"
+#include "../ServerToolbox.h"
 #include "SetOfResources.h"
-#include "../../Core/DicomParsing/FromDcmtkBridge.h"
 
 #include <cassert>
 
@@ -44,6 +45,28 @@
 
 namespace Orthanc
 {
+  LookupIdentifierQuery::SingleConstraint::
+  SingleConstraint(const DicomTag& tag,
+                   IdentifierConstraintType type,
+                   const std::string& value) : 
+    tag_(tag),
+    type_(type),
+    value_(ServerToolbox::NormalizeIdentifier(value))
+  {
+  }
+
+
+  LookupIdentifierQuery::RangeConstraint::
+  RangeConstraint(const DicomTag& tag,
+                  const std::string& start,
+                  const std::string& end) : 
+    tag_(tag),
+    start_(ServerToolbox::NormalizeIdentifier(start)),
+    end_(ServerToolbox::NormalizeIdentifier(end))
+  {
+  }
+
+
   LookupIdentifierQuery::Disjunction::~Disjunction()
   {
     for (size_t i = 0; i < singleConstraints_.size(); i++)
@@ -84,6 +107,12 @@
   }
 
 
+  bool LookupIdentifierQuery::IsIdentifier(const DicomTag& tag)
+  {
+    return ServerToolbox::IsIdentifier(tag, level_);
+  }
+
+
   void LookupIdentifierQuery::AddConstraint(DicomTag tag,
                                             IdentifierConstraintType type,
                                             const std::string& value)
--- a/OrthancServer/Search/LookupIdentifierQuery.h	Tue Dec 11 13:21:34 2018 +0100
+++ b/OrthancServer/Search/LookupIdentifierQuery.h	Tue Dec 11 18:36:38 2018 +0100
@@ -33,7 +33,6 @@
 
 #pragma once
 
-#include "../ServerToolbox.h"
 #include "../IDatabaseWrapper.h"
 
 #include "SetOfResources.h"
@@ -79,12 +78,7 @@
     public:
       SingleConstraint(const DicomTag& tag,
                        IdentifierConstraintType type,
-                       const std::string& value) : 
-        tag_(tag),
-        type_(type),
-        value_(ServerToolbox::NormalizeIdentifier(value))
-      {
-      }
+                       const std::string& value);
 
       const DicomTag& GetTag() const
       {
@@ -113,12 +107,7 @@
     public:
       RangeConstraint(const DicomTag& tag,
                       const std::string& start,
-                      const std::string& end) : 
-        tag_(tag),
-        start_(ServerToolbox::NormalizeIdentifier(start)),
-        end_(ServerToolbox::NormalizeIdentifier(end))
-      {
-      }
+                      const std::string& end);
 
       const DicomTag& GetTag() const
       {
@@ -189,10 +178,7 @@
 
     ~LookupIdentifierQuery();
 
-    bool IsIdentifier(const DicomTag& tag)
-    {
-      return ServerToolbox::IsIdentifier(tag, level_);
-    }
+    bool IsIdentifier(const DicomTag& tag);
 
     void AddConstraint(DicomTag tag,
                        IdentifierConstraintType type,
--- a/OrthancServer/Search/LookupResource.h	Tue Dec 11 13:21:34 2018 +0100
+++ b/OrthancServer/Search/LookupResource.h	Tue Dec 11 18:36:38 2018 +0100
@@ -82,6 +82,19 @@
                     IDatabaseWrapper& database) const;
 
   public:
+    class IVisitor : public boost::noncopyable
+    {
+    public:
+      virtual ~IVisitor()
+      {
+      }
+
+      virtual void MarkAsComplete() = 0;
+
+      virtual void Visit(const std::string& publicId,
+                         const Json::Value& dicom) = 0;
+    };
+
     LookupResource(ResourceType level);
 
     ~LookupResource();
--- a/OrthancServer/ServerContext.cpp	Tue Dec 11 13:21:34 2018 +0100
+++ b/OrthancServer/ServerContext.cpp	Tue Dec 11 18:36:38 2018 +0100
@@ -773,21 +773,19 @@
   }
 
 
-  void ServerContext::Apply(bool& isComplete, 
-                            std::list<std::string>& result,
+  void ServerContext::Apply(LookupResource::IVisitor& visitor,
                             const ::Orthanc::LookupResource& lookup,
                             size_t since,
                             size_t limit)
   {
-    result.clear();
-    isComplete = true;
-
     std::vector<std::string> resources, instances;
     GetIndex().FindCandidates(resources, instances, lookup);
 
     assert(resources.size() == instances.size());
 
+    size_t countResults = 0;
     size_t skipped = 0;
+
     for (size_t i = 0; i < instances.size(); i++)
     {
       // TODO - Don't read the full JSON from the disk if only "main
@@ -802,17 +800,19 @@
           skipped++;
         }
         else if (limit != 0 &&
-                 result.size() >= limit)
+                 countResults >= limit)
         {
-          isComplete = false;
-          return;  // too many results
+          return;  // too many results, don't mark as complete
         }
         else
         {
-          result.push_back(resources[i]);
+          visitor.Visit(resources[i], dicom);
+          countResults ++;
         }
       }
     }
+
+    visitor.MarkAsComplete();
   }
 
 
--- a/OrthancServer/ServerContext.h	Tue Dec 11 13:21:34 2018 +0100
+++ b/OrthancServer/ServerContext.h	Tue Dec 11 18:36:38 2018 +0100
@@ -38,6 +38,7 @@
 #include "LuaScripting.h"
 #include "OrthancHttpHandler.h"
 #include "ServerIndex.h"
+#include "Search/LookupResource.h"
 
 #include "../Core/Cache/MemoryCache.h"
 #include "../Core/Cache/SharedArchive.h"
@@ -334,8 +335,7 @@
 
     void Stop();
 
-    void Apply(bool& isComplete, 
-               std::list<std::string>& result,
+    void Apply(LookupResource::IVisitor& visitor,
                const ::Orthanc::LookupResource& lookup,
                size_t since,
                size_t limit);
--- a/UnitTestsSources/ServerIndexTests.cpp	Tue Dec 11 13:21:34 2018 +0100
+++ b/UnitTestsSources/ServerIndexTests.cpp	Tue Dec 11 18:36:38 2018 +0100
@@ -38,9 +38,10 @@
 #include "../Core/FileStorage/MemoryStorageArea.h"
 #include "../Core/Logging.h"
 #include "../OrthancServer/DatabaseWrapper.h"
+#include "../OrthancServer/Search/LookupIdentifierQuery.h"
 #include "../OrthancServer/ServerContext.h"
 #include "../OrthancServer/ServerIndex.h"
-#include "../OrthancServer/Search/LookupIdentifierQuery.h"
+#include "../OrthancServer/ServerToolbox.h"
 
 #include <ctype.h>
 #include <algorithm>