# HG changeset patch
# User Sebastien Jodogne <s.jodogne@gmail.com>
# Date 1548418252 -3600
# Node ID 860aed8258c35a9e090208889ecd13ead8b74db9
# Parent  6953a4e475b334f3b8a185d9014d3654c50a1ad0
Fix issue #73 (/modalities/{modalityId}/store raises 500 errors instead of 404)

diff -r 6953a4e475b3 -r 860aed8258c3 NEWS
--- a/NEWS	Fri Jan 25 09:47:13 2019 +0100
+++ b/NEWS	Fri Jan 25 13:10:52 2019 +0100
@@ -11,6 +11,7 @@
 
 * Don't return tags whose group is below 0x0008 in C-FIND SCP answers
 * Fix compatibility with DICOMweb plugin (allow multipart answers over HTTP Keep-Alive)
+* Fix issue #73 (/modalities/{modalityId}/store raises 500 errors instead of 404)
 * Fix issue #90 (C-Find shall match missing tags to null/empty string)
 * Fix issue #119 (/patients/.../archive returns a 500 when JobsHistorySize is 0)
 * Fix issue #128 (Asynchronous C-MOVE: invalid number of remaining sub-operations)
diff -r 6953a4e475b3 -r 860aed8258c3 OrthancServer/OrthancRestApi/OrthancRestModalities.cpp
--- a/OrthancServer/OrthancRestApi/OrthancRestModalities.cpp	Fri Jan 25 09:47:13 2019 +0100
+++ b/OrthancServer/OrthancRestApi/OrthancRestModalities.cpp	Fri Jan 25 13:10:52 2019 +0100
@@ -813,7 +813,7 @@
    * DICOM C-Store SCU
    ***************************************************************************/
 
-  static bool GetInstancesToExport(Json::Value& otherArguments,
+  static void GetInstancesToExport(Json::Value& otherArguments,
                                    SetOfInstancesJob& job,
                                    const std::string& remote,
                                    RestApiPostCall& call)
@@ -834,7 +834,7 @@
     else if (!call.ParseJsonRequest(request))
     {
       // Bad JSON request
-      return false;
+      throw OrthancException(ErrorCode_BadFileFormat, "Must provide a JSON value");
     }
 
     if (request.isString())
@@ -843,6 +843,11 @@
       request = Json::arrayValue;
       request.append(item);
     }
+    else if (!request.isArray() &&
+             !request.isObject())
+    {
+      throw OrthancException(ErrorCode_BadFileFormat, "Must provide a JSON object, or a JSON array of strings");
+    }
 
     const Json::Value* resources;
     if (request.isArray())
@@ -854,13 +859,15 @@
       if (request.type() != Json::objectValue ||
           !request.isMember(KEY_RESOURCES))
       {
-        return false;
+        throw OrthancException(ErrorCode_BadFileFormat,
+                               "Missing field in JSON: \"" + std::string(KEY_RESOURCES) + "\"");
       }
 
       resources = &request[KEY_RESOURCES];
       if (!resources->isArray())
       {
-        return false;
+        throw OrthancException(ErrorCode_BadFileFormat,
+                               "JSON field \"" + std::string(KEY_RESOURCES) + "\" must contain an array");
       }
 
       // Copy the remaining arguments
@@ -882,24 +889,24 @@
     {
       if (!(*resources) [i].isString())
       {
-        return false;
+        throw OrthancException(ErrorCode_BadFileFormat,
+                               "Resources to be exported must be specified as a JSON array of strings");
       }
 
       std::string stripped = Toolbox::StripSpaces((*resources) [i].asString());
       if (!Toolbox::IsSHA1(stripped))
       {
-        return false;
+        throw OrthancException(ErrorCode_BadFileFormat,
+                               "This string is not a valid Orthanc identifier: " + stripped);
       }
 
+      context.AddChildInstances(job, stripped);
+
       if (logExportedResources)
       {
         context.GetIndex().LogExportedResource(stripped, remote);
       }
-
-      context.AddChildInstances(job, stripped);
     }
-
-    return true;
   }
 
 
@@ -912,26 +919,25 @@
     Json::Value request;
     std::auto_ptr<DicomModalityStoreJob> job(new DicomModalityStoreJob(context));
 
-    if (GetInstancesToExport(request, *job, remote, call))
-    {
-      std::string localAet = Toolbox::GetJsonStringField
-        (request, "LocalAet", context.GetDefaultLocalApplicationEntityTitle());
-      std::string moveOriginatorAET = Toolbox::GetJsonStringField
-        (request, "MoveOriginatorAet", context.GetDefaultLocalApplicationEntityTitle());
-      int moveOriginatorID = Toolbox::GetJsonIntegerField
-        (request, "MoveOriginatorID", 0 /* By default, not a C-MOVE */);
+    GetInstancesToExport(request, *job, remote, call);
+
+    std::string localAet = Toolbox::GetJsonStringField
+      (request, "LocalAet", context.GetDefaultLocalApplicationEntityTitle());
+    std::string moveOriginatorAET = Toolbox::GetJsonStringField
+      (request, "MoveOriginatorAet", context.GetDefaultLocalApplicationEntityTitle());
+    int moveOriginatorID = Toolbox::GetJsonIntegerField
+      (request, "MoveOriginatorID", 0 /* By default, not a C-MOVE */);
 
-      job->SetLocalAet(localAet);
-      job->SetRemoteModality(MyGetModalityUsingSymbolicName(remote));
+    job->SetLocalAet(localAet);
+    job->SetRemoteModality(MyGetModalityUsingSymbolicName(remote));
 
-      if (moveOriginatorID != 0)
-      {
-        job->SetMoveOriginator(moveOriginatorAET, moveOriginatorID);
-      }
+    if (moveOriginatorID != 0)
+    {
+      job->SetMoveOriginator(moveOriginatorAET, moveOriginatorID);
+    }
 
-      OrthancRestApi::GetApi(call).SubmitCommandsJob
-        (call, job.release(), true /* synchronous by default */, request);
-    }
+    OrthancRestApi::GetApi(call).SubmitCommandsJob
+      (call, job.release(), true /* synchronous by default */, request);
   }
 
 
@@ -1059,22 +1065,21 @@
     Json::Value request;
     std::auto_ptr<OrthancPeerStoreJob> job(new OrthancPeerStoreJob(context));
 
-    if (GetInstancesToExport(request, *job, remote, call))
-    {
-      OrthancConfiguration::ReaderLock lock;
+    GetInstancesToExport(request, *job, remote, call);
+    
+    OrthancConfiguration::ReaderLock lock;
 
-      WebServiceParameters peer;
-      if (lock.GetConfiguration().LookupOrthancPeer(peer, remote))
-      {
-        job->SetPeer(peer);    
-        OrthancRestApi::GetApi(call).SubmitCommandsJob
-          (call, job.release(), true /* synchronous by default */, request);
-      }
-      else
-      {
-        throw OrthancException(ErrorCode_UnknownResource,
-                               "No peer with symbolic name: " + remote);
-      }
+    WebServiceParameters peer;
+    if (lock.GetConfiguration().LookupOrthancPeer(peer, remote))
+    {
+      job->SetPeer(peer);    
+      OrthancRestApi::GetApi(call).SubmitCommandsJob
+        (call, job.release(), true /* synchronous by default */, request);
+    }
+    else
+    {
+      throw OrthancException(ErrorCode_UnknownResource,
+                             "No peer with symbolic name: " + remote);
     }
   }