# HG changeset patch # User Sebastien Jodogne # Date 1544020021 -3600 # Node ID 10c610e80b15b6e4cd7abdbbbd9bac14dc00e793 # Parent 9c0b0a6d8b549972aa40326fdf53d52848124340 refactoring diff -r 9c0b0a6d8b54 -r 10c610e80b15 Core/SerializationToolbox.cpp --- 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 { diff -r 9c0b0a6d8b54 -r 10c610e80b15 OrthancServer/OrthancRestApi/OrthancRestApi.cpp --- 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); } diff -r 9c0b0a6d8b54 -r 10c610e80b15 OrthancServer/OrthancRestApi/OrthancRestApi.h --- 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, diff -r 9c0b0a6d8b54 -r 10c610e80b15 OrthancServer/OrthancRestApi/OrthancRestArchive.cpp --- 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& tmp, ServerContext& context, std::auto_ptr& job, const std::string& filename) @@ -78,6 +126,9 @@ job->SetDescription("REST API"); + boost::shared_ptr 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 tmp(new TemporaryFile); - std::auto_ptr 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 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 + template static void CreateBatchMedia(RestApiPostCall& call) { ServerContext& context = OrthancRestApi::GetContext(call); - boost::shared_ptr tmp(new TemporaryFile); - std::auto_ptr 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 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 tmp(new TemporaryFile); - std::auto_ptr job(new ArchiveJob(tmp, context, false, false)); + std::auto_ptr 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 tmp(new TemporaryFile); - std::auto_ptr job(new ArchiveJob(tmp, context, true, call.HasArgument("extended"))); + std::auto_ptr job(new ArchiveJob(context, true, call.HasArgument("extended"))); job->AddResource(id); - SubmitJob(call, tmp, context, job, id + ".zip"); + SubmitJob(call, context, job, id + ".zip"); } diff -r 9c0b0a6d8b54 -r 10c610e80b15 OrthancServer/ServerJobs/ArchiveJob.cpp --- 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& 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& 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); diff -r 9c0b0a6d8b54 -r 10c610e80b15 OrthancServer/ServerJobs/ArchiveJob.h --- 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& synchronousTarget, - ServerContext& context, + ArchiveJob(ServerContext& context, bool isMedia, bool enableExtendedSopClass); + void SetSynchronousTarget(boost::shared_ptr& synchronousTarget); + void SetDescription(const std::string& description) { description_ = description; diff -r 9c0b0a6d8b54 -r 10c610e80b15 UnitTestsSources/MultiThreadingTests.cpp --- 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 tmp(new TemporaryFile); - ArchiveJob job(tmp, GetContext(), false, false); + ArchiveJob job(GetContext(), false, false); ASSERT_FALSE(job.Serialize(s)); // Cannot serialize this }