# HG changeset patch # User Sebastien Jodogne # Date 1550994675 -3600 # Node ID 6ce10c3b1eb78e3b7c487701fc9f53af15067677 # Parent 5b7f56cf4a38408d625ed61c9248db3abd97e4a7 Fix issue #131 (C-MOVE failure due to duplicate StudyInstanceUID in the database) diff -r 5b7f56cf4a38 -r 6ce10c3b1eb7 NEWS --- a/NEWS Sat Feb 23 15:58:59 2019 +0100 +++ b/NEWS Sun Feb 24 08:51:15 2019 +0100 @@ -18,10 +18,11 @@ Maintenance ----------- -* Fix issue #134 (/patient/modify gives 500, should really be 400) * Accept SOP classes: BreastProjectionXRayImageStorageForProcessing/Presentation * More tolerance wrt. missing DICOM tags that must be returned by Orthanc C-FIND SCP * New CMake option: "-DMSVC_MULTIPLE_PROCESSES=ON" for parallel build with Visual Studio +* Fix issue #131 (C-MOVE failure due to duplicate StudyInstanceUID in the database) +* Fix issue #134 (/patient/modify gives 500, should really be 400) * Upgraded dependencies for static builds (notably on Windows): - boost 1.69.0 - curl 1.64.0 diff -r 5b7f56cf4a38 -r 6ce10c3b1eb7 OrthancServer/OrthancMoveRequestHandler.cpp --- a/OrthancServer/OrthancMoveRequestHandler.cpp Sat Feb 23 15:58:59 2019 +0100 +++ b/OrthancServer/OrthancMoveRequestHandler.cpp Sun Feb 24 08:51:15 2019 +0100 @@ -64,7 +64,7 @@ public: SynchronousMove(ServerContext& context, const std::string& targetAet, - const std::string& publicId, + const std::vector& publicIds, const std::string& originatorAet, uint16_t originatorId) : context_(context), @@ -73,22 +73,25 @@ originatorAet_(originatorAet), originatorId_(originatorId) { - LOG(INFO) << "Sending resource " << publicId << " to modality \"" - << targetAet << "\" in synchronous mode"; - - std::list tmp; - context_.GetIndex().GetChildInstances(tmp, publicId); - - instances_.reserve(tmp.size()); - for (std::list::iterator it = tmp.begin(); it != tmp.end(); ++it) - { - instances_.push_back(*it); - } - { OrthancConfiguration::ReaderLock lock; remote_ = lock.GetConfiguration().GetModalityUsingAet(targetAet); } + + for (size_t i = 0; i < publicIds.size(); i++) + { + LOG(INFO) << "Sending resource " << publicIds[i] << " to modality \"" + << targetAet << "\" in synchronous mode"; + + std::list tmp; + context_.GetIndex().GetChildInstances(tmp, publicIds[i]); + + instances_.reserve(tmp.size()); + for (std::list::iterator it = tmp.begin(); it != tmp.end(); ++it) + { + instances_.push_back(*it); + } + } } virtual unsigned int GetSubOperationCount() const @@ -131,16 +134,13 @@ public: AsynchronousMove(ServerContext& context, const std::string& targetAet, - const std::string& publicId, + const std::vector& publicIds, const std::string& originatorAet, uint16_t originatorId) : context_(context), job_(new DicomModalityStoreJob(context)), position_(0) { - LOG(INFO) << "Sending resource " << publicId << " to modality \"" - << targetAet << "\" in asynchronous mode"; - job_->SetDescription("C-MOVE"); job_->SetPermissive(true); job_->SetLocalAet(context.GetDefaultLocalApplicationEntityTitle()); @@ -154,17 +154,23 @@ { job_->SetMoveOriginator(originatorAet, originatorId); } - - std::list tmp; - context_.GetIndex().GetChildInstances(tmp, publicId); - countInstances_ = tmp.size(); + for (size_t i = 0; i < publicIds.size(); i++) + { + LOG(INFO) << "Sending resource " << publicIds[i] << " to modality \"" + << targetAet << "\" in asynchronous mode"; + + std::list tmp; + context_.GetIndex().GetChildInstances(tmp, publicIds[i]); - job_->Reserve(tmp.size()); + countInstances_ = tmp.size(); + + job_->Reserve(job_->GetCommandsCount() + tmp.size()); - for (std::list::iterator it = tmp.begin(); it != tmp.end(); ++it) - { - job_->AddInstance(*it); + for (std::list::iterator it = tmp.begin(); it != tmp.end(); ++it) + { + job_->AddInstance(*it); + } } } @@ -192,9 +198,9 @@ } - bool OrthancMoveRequestHandler::LookupIdentifier(std::string& publicId, - ResourceType level, - const DicomMap& input) + bool OrthancMoveRequestHandler::LookupIdentifiers(std::vector& publicIds, + ResourceType level, + const DicomMap& input) { DicomTag tag(0, 0); // Dummy initialization @@ -232,19 +238,10 @@ { return false; } - - const std::string& content = value.GetContent(); - - std::vector ids; - context_.GetIndex().LookupIdentifierExact(ids, level, tag, content); - - if (ids.size() != 1) - { - return false; - } else { - publicId = ids[0]; + const std::string& content = value.GetContent(); + context_.GetIndex().LookupIdentifierExact(publicIds, level, tag, content); return true; } } @@ -252,10 +249,16 @@ static IMoveRequestIterator* CreateIterator(ServerContext& context, const std::string& targetAet, - const std::string& publicId, + const std::vector& publicIds, const std::string& originatorAet, uint16_t originatorId) { + if (publicIds.empty()) + { + throw OrthancException(ErrorCode_BadRequest, + "C-MOVE request matching no resource stored in Orthanc"); + } + bool synchronous; { @@ -265,11 +268,11 @@ if (synchronous) { - return new SynchronousMove(context, targetAet, publicId, originatorAet, originatorId); + return new SynchronousMove(context, targetAet, publicIds, originatorAet, originatorId); } else { - return new AsynchronousMove(context, targetAet, publicId, originatorAet, originatorId); + return new AsynchronousMove(context, targetAet, publicIds, originatorAet, originatorId); } } @@ -314,19 +317,19 @@ // query level: Start from the instance level, going up to the // patient level until a valid DICOM identifier is found. - std::string publicId; + std::vector publicIds; - if (LookupIdentifier(publicId, ResourceType_Instance, input) || - LookupIdentifier(publicId, ResourceType_Series, input) || - LookupIdentifier(publicId, ResourceType_Study, input) || - LookupIdentifier(publicId, ResourceType_Patient, input)) + if (LookupIdentifiers(publicIds, ResourceType_Instance, input) || + LookupIdentifiers(publicIds, ResourceType_Series, input) || + LookupIdentifiers(publicIds, ResourceType_Study, input) || + LookupIdentifiers(publicIds, ResourceType_Patient, input)) { - return CreateIterator(context_, targetAet, publicId, originatorAet, originatorId); + return CreateIterator(context_, targetAet, publicIds, originatorAet, originatorId); } else { // No identifier is present in the request. - throw OrthancException(ErrorCode_BadRequest); + throw OrthancException(ErrorCode_BadRequest, "Invalid fields in a C-MOVE request"); } } @@ -338,15 +341,15 @@ * Lookup for the resource to be sent. **/ - std::string publicId; + std::vector publicIds; - if (LookupIdentifier(publicId, level, input)) + if (LookupIdentifiers(publicIds, level, input)) { - return CreateIterator(context_, targetAet, publicId, originatorAet, originatorId); + return CreateIterator(context_, targetAet, publicIds, originatorAet, originatorId); } else { - throw OrthancException(ErrorCode_BadRequest); + throw OrthancException(ErrorCode_BadRequest, "Invalid fields in a C-MOVE request"); } } } diff -r 5b7f56cf4a38 -r 6ce10c3b1eb7 OrthancServer/OrthancMoveRequestHandler.h --- a/OrthancServer/OrthancMoveRequestHandler.h Sat Feb 23 15:58:59 2019 +0100 +++ b/OrthancServer/OrthancMoveRequestHandler.h Sun Feb 24 08:51:15 2019 +0100 @@ -43,9 +43,9 @@ private: ServerContext& context_; - bool LookupIdentifier(std::string& publicId, - ResourceType level, - const DicomMap& input); + bool LookupIdentifiers(std::vector& publicIds, + ResourceType level, + const DicomMap& input); public: OrthancMoveRequestHandler(ServerContext& context) :