Mercurial > hg > orthanc

changeset 2966:10c610e80b15

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
refactoring
author Sebastien Jodogne <s.jodogne@gmail.com>
date Wed, 05 Dec 2018 15:27:01 +0100
parents 9c0b0a6d8b54
children 146eaed9b02c
files Core/SerializationToolbox.cpp OrthancServer/OrthancRestApi/OrthancRestApi.cpp OrthancServer/OrthancRestApi/OrthancRestApi.h OrthancServer/OrthancRestApi/OrthancRestArchive.cpp OrthancServer/ServerJobs/ArchiveJob.cpp OrthancServer/ServerJobs/ArchiveJob.h UnitTestsSources/MultiThreadingTests.cpp
diffstat 7 files changed, 152 insertions(+), 55 deletions(-) [+]
line wrap: on
line diff
--- a/Core/SerializationToolbox.cpp	Wed Dec 05 14:33:47 2018 +0100
+++ b/Core/SerializationToolbox.cpp	Wed Dec 05 15:27:01 2018 +0100
@@ -47,7 +47,8 @@
           !value.isMember(field.c_str()) ||
           value[field.c_str()].type() != Json::stringValue)
       {
-        throw OrthancException(ErrorCode_BadFileFormat);
+        throw OrthancException(ErrorCode_BadFileFormat,
+                               "String value expected in field: " + field);
       }
       else
       {
@@ -64,7 +65,8 @@
           (value[field.c_str()].type() != Json::intValue &&
            value[field.c_str()].type() != Json::uintValue))
       {
-        throw OrthancException(ErrorCode_BadFileFormat);
+        throw OrthancException(ErrorCode_BadFileFormat,
+                               "Integer value expected in field: " + field);
       }
       else
       {
@@ -80,7 +82,8 @@
 
       if (tmp < 0)
       {
-        throw OrthancException(ErrorCode_BadFileFormat);
+        throw OrthancException(ErrorCode_BadFileFormat,
+                               "Unsigned integer value expected in field: " + field);
       }
       else
       {
@@ -96,7 +99,8 @@
           !value.isMember(field.c_str()) ||
           value[field.c_str()].type() != Json::booleanValue)
       {
-        throw OrthancException(ErrorCode_BadFileFormat);
+        throw OrthancException(ErrorCode_BadFileFormat,
+                               "Boolean value expected in field: " + field);
       }
       else
       {
@@ -113,7 +117,8 @@
           !value.isMember(field.c_str()) ||
           value[field.c_str()].type() != Json::arrayValue)
       {
-        throw OrthancException(ErrorCode_BadFileFormat);
+        throw OrthancException(ErrorCode_BadFileFormat,
+                               "List of strings expected in field: " + field);
       }
 
       const Json::Value& arr = value[field.c_str()];
@@ -124,7 +129,8 @@
       {
         if (arr[i].type() != Json::stringValue)
         {
-          throw OrthancException(ErrorCode_BadFileFormat);        
+          throw OrthancException(ErrorCode_BadFileFormat,
+                                 "List of strings expected in field: " + field);
         }
         else
         {
@@ -172,7 +178,8 @@
           !value.isMember(field.c_str()) ||
           value[field.c_str()].type() != Json::arrayValue)
       {
-        throw OrthancException(ErrorCode_BadFileFormat);
+        throw OrthancException(ErrorCode_BadFileFormat,
+                               "Set of DICOM tags expected in field: " + field);
       }
 
       const Json::Value& arr = value[field.c_str()];
@@ -186,7 +193,8 @@
         if (arr[i].type() != Json::stringValue ||
             !DicomTag::ParseHexadecimal(tag, arr[i].asCString()))
         {
-          throw OrthancException(ErrorCode_BadFileFormat);        
+          throw OrthancException(ErrorCode_BadFileFormat,
+                                 "Set of DICOM tags expected in field: " + field);
         }
         else
         {
@@ -204,7 +212,8 @@
           !value.isMember(field.c_str()) ||
           value[field.c_str()].type() != Json::objectValue)
       {
-        throw OrthancException(ErrorCode_BadFileFormat);
+        throw OrthancException(ErrorCode_BadFileFormat,
+                               "Associative array of strings to strings expected in field: " + field);
       }
 
       const Json::Value& source = value[field.c_str()];
@@ -219,7 +228,8 @@
 
         if (tmp.type() != Json::stringValue)
         {
-          throw OrthancException(ErrorCode_BadFileFormat);        
+          throw OrthancException(ErrorCode_BadFileFormat,
+                                 "Associative array of string to strings expected in field: " + field);
         }
         else
         {
@@ -237,7 +247,8 @@
           !value.isMember(field.c_str()) ||
           value[field.c_str()].type() != Json::objectValue)
       {
-        throw OrthancException(ErrorCode_BadFileFormat);
+        throw OrthancException(ErrorCode_BadFileFormat,
+                               "Associative array of DICOM tags to strings expected in field: " + field);
       }
 
       const Json::Value& source = value[field.c_str()];
@@ -255,7 +266,8 @@
         if (!DicomTag::ParseHexadecimal(tag, members[i].c_str()) ||
             tmp.type() != Json::stringValue)
         {
-          throw OrthancException(ErrorCode_BadFileFormat);        
+          throw OrthancException(ErrorCode_BadFileFormat,
+                                 "Associative array of DICOM tags to strings expected in field: " + field);
         }
         else
         {
--- a/OrthancServer/OrthancRestApi/OrthancRestApi.cpp	Wed Dec 05 14:33:47 2018 +0100
+++ b/OrthancServer/OrthancRestApi/OrthancRestApi.cpp	Wed Dec 05 15:27:01 2018 +0100
@@ -176,9 +176,13 @@
 
   
   bool OrthancRestApi::IsSynchronousJobRequest(bool isDefaultSynchronous,
-                                               const Json::Value& body) const
+                                               const Json::Value& body)
   {
-    if (body.isMember(KEY_SYNCHRONOUS))
+    if (body.type() != Json::objectValue)
+    {
+      return isDefaultSynchronous;
+    }
+    else if (body.isMember(KEY_SYNCHRONOUS))
     {
       return SerializationToolbox::ReadBoolean(body, KEY_SYNCHRONOUS);
     }
--- a/OrthancServer/OrthancRestApi/OrthancRestApi.h	Wed Dec 05 14:33:47 2018 +0100
+++ b/OrthancServer/OrthancRestApi/OrthancRestApi.h	Wed Dec 05 15:27:01 2018 +0100
@@ -103,8 +103,8 @@
                               ResourceType resourceType,
                               StoreStatus status) const;
 
-    bool IsSynchronousJobRequest(bool isDefaultSynchronous,
-                                 const Json::Value& body) const;
+    static bool IsSynchronousJobRequest(bool isDefaultSynchronous,
+                                        const Json::Value& body);
     
     void SubmitGenericJob(RestApiPostCall& call,
                           IJob* job,
--- a/OrthancServer/OrthancRestApi/OrthancRestArchive.cpp	Wed Dec 05 14:33:47 2018 +0100
+++ b/OrthancServer/OrthancRestApi/OrthancRestArchive.cpp	Wed Dec 05 15:27:01 2018 +0100
@@ -35,38 +35,86 @@
 #include "OrthancRestApi.h"
 
 #include "../../Core/HttpServer/FilesystemHttpSender.h"
+#include "../../Core/SerializationToolbox.h"
 #include "../ServerJobs/ArchiveJob.h"
 
 namespace Orthanc
 {
-  static bool AddResourcesOfInterest(ArchiveJob& job,
-                                     RestApiPostCall& call)
+  static const char* const KEY_RESOURCES = "Resources";
+  static const char* const KEY_EXTENDED = "Extended";
+  
+  static void AddResourcesOfInterestFromArray(ArchiveJob& job,
+                                              const Json::Value& resources)
   {
-    Json::Value resources;
-    if (call.ParseJsonRequest(resources) &&
-        resources.type() == Json::arrayValue)
+    if (resources.type() != Json::arrayValue)
     {
-      for (Json::Value::ArrayIndex i = 0; i < resources.size(); i++)
+      throw OrthancException(ErrorCode_BadFileFormat,
+                             "Expected a list of strings (Orthanc identifiers)");
+    }
+    
+    for (Json::Value::ArrayIndex i = 0; i < resources.size(); i++)
+    {
+      if (resources[i].type() != Json::stringValue)
       {
-        if (resources[i].type() != Json::stringValue)
-        {
-          return false;   // Bad request
-        }
-
+        throw OrthancException(ErrorCode_BadFileFormat,
+                               "Expected a list of strings (Orthanc identifiers)");
+      }
+      else
+      {
         job.AddResource(resources[i].asString());
       }
+    }
+  }
 
-      return true;
+  
+  static void AddResourcesOfInterest(ArchiveJob& job         /* inout */,
+                                     const Json::Value& body /* in */)
+  {
+    if (body.type() == Json::arrayValue)
+    {
+      AddResourcesOfInterestFromArray(job, body);
+    }
+    else if (body.type() == Json::objectValue)
+    {
+      if (body.isMember(KEY_RESOURCES))
+      {
+        AddResourcesOfInterestFromArray(job, body[KEY_RESOURCES]);
+      }
+      else
+      {
+        throw OrthancException(ErrorCode_BadFileFormat,
+                               "Missing field " + std::string(KEY_RESOURCES) +
+                               " in the JSON body");
+      }
     }
     else
     {
-      return false;
+      throw OrthancException(ErrorCode_BadFileFormat);
+    }
+  }
+
+
+  static void GetJobParameters(bool& synchronous,         /* out */
+                               bool& extended,            /* out */
+                               const Json::Value& body,   /* in */
+                               const bool defaultExtended /* in */)
+  {
+    synchronous = OrthancRestApi::IsSynchronousJobRequest
+      (true /* synchronous by default */, body);
+
+    if (body.type() == Json::objectValue &&
+        body.isMember(KEY_EXTENDED))
+    {
+      extended = SerializationToolbox::ReadBoolean(body, KEY_EXTENDED);
+    }
+    else
+    {
+      extended = defaultExtended;
     }
   }
 
 
   static void SubmitJob(RestApiCall& call,
-                        boost::shared_ptr<TemporaryFile>& tmp,
                         ServerContext& context,
                         std::auto_ptr<ArchiveJob>& job,
                         const std::string& filename)
@@ -78,6 +126,9 @@
 
     job->SetDescription("REST API");
 
+    boost::shared_ptr<TemporaryFile> tmp(new TemporaryFile);
+    job->SetSynchronousTarget(tmp);
+    
     Json::Value publicContent;
     if (context.GetJobsEngine().GetRegistry().SubmitAndWait
         (publicContent, job.release(), 0 /* TODO priority */))
@@ -101,27 +152,43 @@
   {
     ServerContext& context = OrthancRestApi::GetContext(call);
 
-    boost::shared_ptr<TemporaryFile> tmp(new TemporaryFile);
-    std::auto_ptr<ArchiveJob> job(new ArchiveJob(tmp, context, false, false));
-
-    if (AddResourcesOfInterest(*job, call))
+    Json::Value body;
+    if (call.ParseJsonRequest(body))
     {
-      SubmitJob(call, tmp, context, job, "Archive.zip");
+      bool synchronous, extended;
+      GetJobParameters(synchronous, extended, body, false /* by default, not extended */);
+      
+      std::auto_ptr<ArchiveJob> job(new ArchiveJob(context, false, extended));
+      AddResourcesOfInterest(*job, body);
+      SubmitJob(call, context, job, "Archive.zip");
+    }
+    else
+    {
+      throw OrthancException(ErrorCode_BadFileFormat,
+                             "Expected a list of resources to archive in the body");
     }
   }  
 
   
-  template <bool Extended>
+  template <bool DEFAULT_EXTENDED>
   static void CreateBatchMedia(RestApiPostCall& call)
   {
     ServerContext& context = OrthancRestApi::GetContext(call);
 
-    boost::shared_ptr<TemporaryFile> tmp(new TemporaryFile);
-    std::auto_ptr<ArchiveJob> job(new ArchiveJob(tmp, context, true, Extended));
-
-    if (AddResourcesOfInterest(*job, call))
+    Json::Value body;
+    if (call.ParseJsonRequest(body))
     {
-      SubmitJob(call, tmp, context, job, "Archive.zip");
+      bool synchronous, extended;
+      GetJobParameters(synchronous, extended, body, DEFAULT_EXTENDED);
+      
+      std::auto_ptr<ArchiveJob> job(new ArchiveJob(context, true, extended));
+      AddResourcesOfInterest(*job, body);
+      SubmitJob(call, context, job, "Archive.zip");
+    }
+    else
+    {
+      throw OrthancException(ErrorCode_BadFileFormat,
+                             "Expected a list of resources to archive in the body");
     }
   }
   
@@ -132,11 +199,10 @@
 
     std::string id = call.GetUriComponent("id", "");
 
-    boost::shared_ptr<TemporaryFile> tmp(new TemporaryFile);
-    std::auto_ptr<ArchiveJob> job(new ArchiveJob(tmp, context, false, false));
+    std::auto_ptr<ArchiveJob> job(new ArchiveJob(context, false, false));
     job->AddResource(id);
 
-    SubmitJob(call, tmp, context, job, id + ".zip");
+    SubmitJob(call, context, job, id + ".zip");
   }
 
 
@@ -146,11 +212,10 @@
 
     std::string id = call.GetUriComponent("id", "");
 
-    boost::shared_ptr<TemporaryFile> tmp(new TemporaryFile);
-    std::auto_ptr<ArchiveJob> job(new ArchiveJob(tmp, context, true, call.HasArgument("extended")));
+    std::auto_ptr<ArchiveJob> job(new ArchiveJob(context, true, call.HasArgument("extended")));
     job->AddResource(id);
 
-    SubmitJob(call, tmp, context, job, id + ".zip");
+    SubmitJob(call, context, job, id + ".zip");
   }
 
 
--- a/OrthancServer/ServerJobs/ArchiveJob.cpp	Wed Dec 05 14:33:47 2018 +0100
+++ b/OrthancServer/ServerJobs/ArchiveJob.cpp	Wed Dec 05 15:27:01 2018 +0100
@@ -778,11 +778,9 @@
   };
 
 
-  ArchiveJob::ArchiveJob(boost::shared_ptr<TemporaryFile>& synchronousTarget,
-                         ServerContext& context,
+  ArchiveJob::ArchiveJob(ServerContext& context,
                          bool isMedia,
                          bool enableExtendedSopClass) :
-    synchronousTarget_(synchronousTarget),
     context_(context),
     archive_(new ArchiveIndex(ResourceType_Patient)),  // root
     isMedia_(isMedia),
@@ -791,10 +789,23 @@
     instancesCount_(0),
     uncompressedSize_(0)
   {
-    if (synchronousTarget.get() == NULL)
+  }
+
+
+  void ArchiveJob::SetSynchronousTarget(boost::shared_ptr<TemporaryFile>& target)
+  {
+    if (target.get() == NULL)
     {
       throw OrthancException(ErrorCode_NullPointer);
     }
+    else if (synchronousTarget_.get() != NULL)
+    {
+      throw OrthancException(ErrorCode_BadSequenceOfCalls);
+    }
+    else
+    {
+      synchronousTarget_ = target;
+    }
   }
 
 
@@ -819,6 +830,11 @@
   
   void ArchiveJob::Start()
   {
+    if (synchronousTarget_.get() == NULL)
+    {
+      throw OrthancException(ErrorCode_BadSequenceOfCalls);
+    }
+    
     if (writer_.get() != NULL)
     {
       throw OrthancException(ErrorCode_BadSequenceOfCalls);
--- a/OrthancServer/ServerJobs/ArchiveJob.h	Wed Dec 05 14:33:47 2018 +0100
+++ b/OrthancServer/ServerJobs/ArchiveJob.h	Wed Dec 05 15:27:01 2018 +0100
@@ -63,11 +63,12 @@
     uint64_t                              uncompressedSize_;
 
   public:
-    ArchiveJob(boost::shared_ptr<TemporaryFile>& synchronousTarget,
-               ServerContext& context,
+    ArchiveJob(ServerContext& context,
                bool isMedia,
                bool enableExtendedSopClass);
 
+    void SetSynchronousTarget(boost::shared_ptr<TemporaryFile>& synchronousTarget);
+    
     void SetDescription(const std::string& description)
     {
       description_ = description;
--- a/UnitTestsSources/MultiThreadingTests.cpp	Wed Dec 05 14:33:47 2018 +0100
+++ b/UnitTestsSources/MultiThreadingTests.cpp	Wed Dec 05 15:27:01 2018 +0100
@@ -1475,8 +1475,7 @@
   // ArchiveJob
 
   {
-    boost::shared_ptr<TemporaryFile> tmp(new TemporaryFile);
-    ArchiveJob job(tmp, GetContext(), false, false);
+    ArchiveJob job(GetContext(), false, false);
     ASSERT_FALSE(job.Serialize(s));  // Cannot serialize this
   }