# HG changeset patch # User Sebastien Jodogne # Date 1431703173 -7200 # Node ID 94ffb597d2974d48638c7187325e337ac39f6a3a # Parent 0649c5aef34a4a47cca5ae74d83ad508aa141300 refactoring of C-Find SCP diff -r 0649c5aef34a -r 94ffb597d297 OrthancServer/DicomFindQuery.cpp --- a/OrthancServer/DicomFindQuery.cpp Fri May 15 15:34:32 2015 +0200 +++ b/OrthancServer/DicomFindQuery.cpp Fri May 15 17:19:33 2015 +0200 @@ -310,8 +310,9 @@ return filteredLevels_.find(level) != filteredLevels_.end(); } - bool DicomFindQuery::FilterMainDicomTags(const DicomMap& mainTags, - ResourceType level) const + bool DicomFindQuery::FilterMainDicomTags(const std::string& resourceId, + ResourceType level, + const DicomMap& mainTags) const { std::set tags; mainTags.GetTags(tags); @@ -320,7 +321,8 @@ it = tags.begin(); it != tags.end(); ++it) { Constraints::const_iterator constraint = constraints_.find(*it); - if (!constraint->second->Apply(mainTags.GetValue(*it).AsString())) + if (constraint != constraints_.end() && + !constraint->second->Apply(mainTags.GetValue(*it).AsString())) { return false; } diff -r 0649c5aef34a -r 94ffb597d297 OrthancServer/DicomFindQuery.h --- a/OrthancServer/DicomFindQuery.h Fri May 15 15:34:32 2015 +0200 +++ b/OrthancServer/DicomFindQuery.h Fri May 15 17:19:33 2015 +0200 @@ -98,8 +98,9 @@ virtual bool HasMainDicomTagsFilter(ResourceType level) const; - virtual bool FilterMainDicomTags(const DicomMap& mainTags, - ResourceType level) const; + virtual bool FilterMainDicomTags(const std::string& resourceId, + ResourceType level, + const DicomMap& mainTags) const; virtual bool HasInstanceFilter() const; diff -r 0649c5aef34a -r 94ffb597d297 OrthancServer/OrthancFindRequestHandler.cpp --- a/OrthancServer/OrthancFindRequestHandler.cpp Fri May 15 15:34:32 2015 +0200 +++ b/OrthancServer/OrthancFindRequestHandler.cpp Fri May 15 17:19:33 2015 +0200 @@ -41,164 +41,15 @@ #include "OrthancInitialization.h" #include "FromDcmtkBridge.h" -namespace Orthanc -{ - static bool IsWildcard(const std::string& constraint) - { - return (constraint.find('-') != std::string::npos || - constraint.find('*') != std::string::npos || - constraint.find('\\') != std::string::npos || - constraint.find('?') != std::string::npos); - } - - static bool ApplyRangeConstraint(const std::string& value, - const std::string& constraint) - { - size_t separator = constraint.find('-'); - std::string lower, upper, v; - Toolbox::ToLowerCase(lower, constraint.substr(0, separator)); - Toolbox::ToLowerCase(upper, constraint.substr(separator + 1)); - Toolbox::ToLowerCase(v, value); - - if (lower.size() == 0 && upper.size() == 0) - { - return false; - } - - if (lower.size() == 0) - { - return v <= upper; - } - - if (upper.size() == 0) - { - return v >= lower; - } - - return (v >= lower && v <= upper); - } - - - static bool ApplyListConstraint(const std::string& value, - const std::string& constraint) - { - std::string v1; - Toolbox::ToLowerCase(v1, value); - - std::vector items; - Toolbox::TokenizeString(items, constraint, '\\'); - - for (size_t i = 0; i < items.size(); i++) - { - std::string lower; - Toolbox::ToLowerCase(lower, items[i]); - if (lower == v1) - { - return true; - } - } - - return false; - } +#include "ResourceFinder.h" +#include "DicomFindQuery.h" - static bool Matches(const std::string& value, - const std::string& constraint) - { - // http://www.itk.org/Wiki/DICOM_QueryRetrieve_Explained - // http://dicomiseasy.blogspot.be/2012/01/dicom-queryretrieve-part-i.html - - if (constraint.find('-') != std::string::npos) - { - return ApplyRangeConstraint(value, constraint); - } - - if (constraint.find('\\') != std::string::npos) - { - return ApplyListConstraint(value, constraint); - } - - if (constraint.find('*') != std::string::npos || - constraint.find('?') != std::string::npos) - { - // TODO - Cache the constructed regular expression - boost::regex pattern(Toolbox::WildcardToRegularExpression(constraint), - boost::regex::icase /* case insensitive search */); - return boost::regex_match(value, pattern); - } - else - { - std::string v, c; - Toolbox::ToLowerCase(v, value); - Toolbox::ToLowerCase(c, constraint); - return v == c; - } - } - - - static bool LookupOneInstance(std::string& result, - ServerIndex& index, - const std::string& id, - ResourceType type) - { - if (type == ResourceType_Instance) - { - result = id; - return true; - } - - std::string childId; - - { - std::list children; - index.GetChildInstances(children, id); - - if (children.empty()) - { - return false; - } - - childId = children.front(); - } - - return LookupOneInstance(result, index, childId, GetChildResourceType(type)); - } - - - static bool Matches(const Json::Value& resource, - const DicomArray& query) - { - for (size_t i = 0; i < query.GetSize(); i++) - { - if (query.GetElement(i).GetValue().IsNull() || - query.GetElement(i).GetTag() == DICOM_TAG_QUERY_RETRIEVE_LEVEL || - query.GetElement(i).GetTag() == DICOM_TAG_SPECIFIC_CHARACTER_SET || - query.GetElement(i).GetTag() == DICOM_TAG_MODALITIES_IN_STUDY) - { - continue; - } - - std::string tag = query.GetElement(i).GetTag().Format(); - std::string value; - if (resource.isMember(tag)) - { - value = resource.get(tag, Json::arrayValue).get("Value", "").asString(); - } - - if (!Matches(value, query.GetElement(i).GetValue().AsString())) - { - return false; - } - } - - return true; - } - - +namespace Orthanc +{ static void AddAnswer(DicomFindAnswers& answers, const Json::Value& resource, - const DicomArray& query, - bool isFirst) + const DicomArray& query) { DicomMap result; @@ -230,10 +81,7 @@ if (result.GetSize() == 0) { - if (isFirst) - { - LOG(WARNING) << "The C-FIND request does not return any DICOM tag"; - } + LOG(WARNING) << "The C-FIND request does not return any DICOM tag"; } else { @@ -242,258 +90,146 @@ } - static bool ApplyModalitiesInStudyFilter(std::list& filteredStudies, - const std::list& studies, - const DicomMap& input, - ServerIndex& index) + namespace { - filteredStudies.clear(); - - const DicomValue& v = input.GetValue(DICOM_TAG_MODALITIES_IN_STUDY); - if (v.IsNull()) - { - return false; - } - - // Move the allowed modalities into a "std::set" - std::vector tmp; - Toolbox::TokenizeString(tmp, v.AsString(), '\\'); - - std::set modalities; - for (size_t i = 0; i < tmp.size(); i++) - { - modalities.insert(tmp[i]); - } - - // Loop over the studies - for (std::list::const_iterator - it = studies.begin(); it != studies.end(); ++it) + class CFindQuery : public DicomFindQuery { - try + private: + DicomFindAnswers& answers_; + ServerIndex& index_; + const DicomArray& query_; + bool hasModalitiesInStudy_; + std::set modalitiesInStudy_; + + public: + CFindQuery(DicomFindAnswers& answers, + ServerIndex& index, + const DicomArray& query) : + answers_(answers), + index_(index), + query_(query), + hasModalitiesInStudy_(false) + { + } + + void SetModalitiesInStudy(const std::string& value) + { + hasModalitiesInStudy_ = true; + + std::vector tmp; + Toolbox::TokenizeString(tmp, value, '\\'); + + for (size_t i = 0; i < tmp.size(); i++) + { + modalitiesInStudy_.insert(tmp[i]); + } + } + + virtual bool HasMainDicomTagsFilter(ResourceType level) const { - // We are considering a single study. Check whether one of - // its child series matches one of the modalities. - Json::Value study; - if (index.LookupResource(study, *it, ResourceType_Study)) + if (DicomFindQuery::HasMainDicomTagsFilter(level)) + { + return true; + } + + return (level == ResourceType_Study && + hasModalitiesInStudy_); + } + + virtual bool FilterMainDicomTags(const std::string& resourceId, + ResourceType level, + const DicomMap& mainTags) const + { + if (!DicomFindQuery::FilterMainDicomTags(resourceId, level, mainTags)) { - // Loop over the series of the considered study. - for (Json::Value::ArrayIndex j = 0; j < study["Series"].size(); j++) // (*) + return false; + } + + if (level != ResourceType_Study || + !hasModalitiesInStudy_) + { + return true; + } + + try + { + // We are considering a single study, and the + // "MODALITIES_IN_STUDY" tag is set in the C-Find. Check + // whether one of its child series matches one of the + // modalities. + + Json::Value study; + if (index_.LookupResource(study, resourceId, ResourceType_Study)) { - Json::Value series; - if (index.LookupResource(series, study["Series"][j].asString(), ResourceType_Series)) + // Loop over the series of the considered study. + for (Json::Value::ArrayIndex j = 0; j < study["Series"].size(); j++) { - // Get the modality of this series - if (series["MainDicomTags"].isMember("Modality")) + Json::Value series; + if (index_.LookupResource(series, study["Series"][j].asString(), ResourceType_Series)) { - std::string modality = series["MainDicomTags"]["Modality"].asString(); - if (modalities.find(modality) != modalities.end()) + // Get the modality of this series + if (series["MainDicomTags"].isMember("Modality")) { - // This series of the considered study matches one - // of the required modalities. Take the study into - // consideration for future filtering. - filteredStudies.push_back(*it); - - // We have finished considering this study. Break the study loop at (*). - break; + std::string modality = series["MainDicomTags"]["Modality"].asString(); + if (modalitiesInStudy_.find(modality) != modalitiesInStudy_.end()) + { + // This series of the considered study matches one + // of the required modalities. Take the study into + // consideration for future filtering. + return true; + } } } } } } - } - catch (OrthancException&) - { - // This resource has probably been deleted during the find request - } - } - - return true; - } - + catch (OrthancException&) + { + // This resource has probably been deleted during the find request + } - namespace - { - class CandidateResources - { - private: - ServerIndex& index_; - ModalityManufacturer manufacturer_; - ResourceType level_; - bool isFilterApplied_; - std::set filtered_; - - static void ListToSet(std::set& target, - const std::list& source) - { - for (std::list::const_iterator - it = source.begin(); it != source.end(); ++it) - { - target.insert(*it); - } + return false; } - void ApplyExactFilter(const DicomTag& tag, const std::string& value) + virtual bool HasInstanceFilter() const { - LOG(INFO) << "Applying exact filter on tag " - << FromDcmtkBridge::GetName(tag) << " (value: " << value << ")"; - - std::list resources; - index_.LookupIdentifier(resources, tag, value, level_); - - if (isFilterApplied_) - { - std::set s; - ListToSet(s, resources); - - std::set tmp = filtered_; - filtered_.clear(); - - for (std::set::const_iterator - it = tmp.begin(); it != tmp.end(); ++it) - { - if (s.find(*it) != s.end()) - { - filtered_.insert(*it); - } - } - } - else - { - assert(filtered_.empty()); - isFilterApplied_ = true; - ListToSet(filtered_, resources); - } - } - - public: - CandidateResources(ServerIndex& index, - ModalityManufacturer manufacturer) : - index_(index), - manufacturer_(manufacturer), - level_(ResourceType_Patient), - isFilterApplied_(false) - { + return true; } - ResourceType GetLevel() const + virtual bool FilterInstance(const std::string& instanceId, + const Json::Value& content) const { - return level_; - } + bool ok = DicomFindQuery::FilterInstance(instanceId, content); - void GoDown() - { - assert(level_ != ResourceType_Instance); - - if (isFilterApplied_) + if (ok) { - std::set tmp = filtered_; - - filtered_.clear(); - - for (std::set::const_iterator - it = tmp.begin(); it != tmp.end(); ++it) - { - std::list children; - index_.GetChildren(children, *it); - ListToSet(filtered_, children); - } + // Add this resource to the answers + AddAnswer(answers_, content, query_); } - switch (level_) - { - case ResourceType_Patient: - level_ = ResourceType_Study; - break; - - case ResourceType_Study: - level_ = ResourceType_Series; - break; - - case ResourceType_Series: - level_ = ResourceType_Instance; - break; - - default: - throw OrthancException(ErrorCode_InternalError); - } - } - - void Flatten(std::list& resources) const - { - resources.clear(); - - if (isFilterApplied_) - { - for (std::set::const_iterator - it = filtered_.begin(); it != filtered_.end(); ++it) - { - resources.push_back(*it); - } - } - else - { - index_.GetAllUuids(resources, level_); - } - } - - void ApplyFilter(const DicomTag& tag, const DicomMap& query) - { - if (query.HasTag(tag)) - { - const DicomValue& value = query.GetValue(tag); - if (!value.IsNull()) - { - std::string value = query.GetValue(tag).AsString(); - if (!IsWildcard(value)) - { - ApplyExactFilter(tag, value); - } - } - } + return ok; } }; } - bool OrthancFindRequestHandler::HasReachedLimit(const DicomFindAnswers& answers, - ResourceType level) const - { - switch (level) - { - case ResourceType_Patient: - case ResourceType_Study: - case ResourceType_Series: - return (maxResults_ != 0 && answers.GetSize() >= maxResults_); - - case ResourceType_Instance: - return (maxInstances_ != 0 && answers.GetSize() >= maxInstances_); - - default: - throw OrthancException(ErrorCode_InternalError); - } - } - bool OrthancFindRequestHandler::Handle(DicomFindAnswers& answers, const DicomMap& input, const std::string& callingAETitle) { /** - * Retrieve the manufacturer of this modality. + * Ensure that the calling modality is known to Orthanc. **/ - ModalityManufacturer manufacturer; - - { - RemoteModalityParameters modality; + RemoteModalityParameters modality; - if (!Configuration::LookupDicomModalityUsingAETitle(modality, callingAETitle)) - { - throw OrthancException("Unknown modality"); - } + if (!Configuration::LookupDicomModalityUsingAETitle(modality, callingAETitle)) + { + throw OrthancException("Unknown modality"); + } - manufacturer = modality.GetManufacturer(); - } + // ModalityManufacturer manufacturer = modality.GetManufacturer(); /** @@ -532,139 +268,63 @@ /** - * Retrieve the candidate resources for this query level. Whenever - * possible, we avoid returning ALL the resources for this query - * level, as it would imply reading the JSON file on the harddisk - * for each of them. + * Build up the query object. **/ - CandidateResources candidates(context_.GetIndex(), manufacturer); - - for (;;) + CFindQuery findQuery(answers, context_.GetIndex(), query); + findQuery.SetLevel(level); + + for (size_t i = 0; i < query.GetSize(); i++) { - switch (candidates.GetLevel()) - { - case ResourceType_Patient: - candidates.ApplyFilter(DICOM_TAG_PATIENT_ID, input); - break; - - case ResourceType_Study: - candidates.ApplyFilter(DICOM_TAG_STUDY_INSTANCE_UID, input); - candidates.ApplyFilter(DICOM_TAG_ACCESSION_NUMBER, input); - break; + const DicomTag tag = query.GetElement(i).GetTag(); - case ResourceType_Series: - candidates.ApplyFilter(DICOM_TAG_SERIES_INSTANCE_UID, input); - break; - - case ResourceType_Instance: - candidates.ApplyFilter(DICOM_TAG_SOP_INSTANCE_UID, input); - break; - - default: - throw OrthancException(ErrorCode_InternalError); - } - - if (candidates.GetLevel() == level) + if (query.GetElement(i).GetValue().IsNull() || + tag == DICOM_TAG_QUERY_RETRIEVE_LEVEL || + tag == DICOM_TAG_SPECIFIC_CHARACTER_SET) { - break; + continue; } - candidates.GoDown(); - } - - std::list resources; - candidates.Flatten(resources); - + std::string value = query.GetElement(i).GetValue().AsString(); - /** - * Apply filtering on modalities for studies, if asked (this is an - * extension to standard DICOM) - * http://www.medicalconnections.co.uk/kb/Filtering_on_and_Retrieving_the_Modality_in_a_C_FIND - **/ - - if (level == ResourceType_Study && - input.HasTag(DICOM_TAG_MODALITIES_IN_STUDY)) - { - std::list filtered; - if (ApplyModalitiesInStudyFilter(filtered, resources, input, context_.GetIndex())) + if (tag == DICOM_TAG_MODALITIES_IN_STUDY) { - resources = filtered; + findQuery.SetModalitiesInStudy(value); + } + else + { + findQuery.SetConstraint(tag, value); } } + /** - * Loop over all the resources for this query level. + * Run the query. **/ - LOG(INFO) << "Number of candidate resources after exact filtering on the identifiers only: " << resources.size(); - - bool isFirst = true; - - for (std::list::const_iterator - resource = resources.begin(); resource != resources.end(); ++resource) - { - try - { - std::string instance; - if (LookupOneInstance(instance, context_.GetIndex(), *resource, level)) - { - Json::Value info; - context_.ReadJson(info, instance); + ResourceFinder finder(context_); - if (Matches(info, query)) - { - if (HasReachedLimit(answers, level)) - { - // Too many results, stop before recording this new match - return false; - } + switch (level) + { + case ResourceType_Patient: + case ResourceType_Study: + case ResourceType_Series: + finder.SetMaxResults(maxResults_); + break; - AddAnswer(answers, info, query, isFirst); - isFirst = false; - } - } - } - catch (OrthancException&) - { - // This resource has probably been deleted during the find request - } + case ResourceType_Instance: + finder.SetMaxResults(maxInstances_); + break; + + default: + throw OrthancException(ErrorCode_InternalError); } - LOG(INFO) << "Number of candidate resources after filtering on all tags: " << answers.GetSize(); + std::list tmp; + bool finished = finder.Apply(tmp, findQuery); - return true; // All the matching resources have been returned + LOG(INFO) << "Number of matching resources: " << tmp.size(); + + return finished; } } - - - -/** - * TODO : Case-insensitive match for PN value representation (Patient - * Name). Case-senstive match for all the other value representations. - * - * Reference: DICOM PS 3.4 - * - C.2.2.2.1 ("Single Value Matching") - * - C.2.2.2.4 ("Wild Card Matching") - * http://medical.nema.org/Dicom/2011/11_04pu.pdf - * - * "Except for Attributes with a PN Value Representation, only - * entities with values which match exactly the value specified in the - * request shall match. This matching is case-sensitive, i.e., - * sensitive to the exact encoding of the key attribute value in - * character sets where a letter may have multiple encodings (e.g., - * based on its case, its position in a word, or whether it is - * accented) - * - * For Attributes with a PN Value Representation (e.g., Patient Name - * (0010,0010)), an application may perform literal matching that is - * either case-sensitive, or that is insensitive to some or all - * aspects of case, position, accent, or other character encoding - * variants." - * - * (0008,0018) UI SOPInstanceUID => Case-sensitive - * (0008,0050) SH AccessionNumber => Case-sensitive - * (0010,0020) LO PatientID => Case-sensitive - * (0020,000D) UI StudyInstanceUID => Case-sensitive - * (0020,000E) UI SeriesInstanceUID => Case-sensitive - **/ diff -r 0649c5aef34a -r 94ffb597d297 OrthancServer/OrthancRestApi/OrthancRestResources.cpp --- a/OrthancServer/OrthancRestApi/OrthancRestResources.cpp Fri May 15 15:34:32 2015 +0200 +++ b/OrthancServer/OrthancRestApi/OrthancRestResources.cpp Fri May 15 17:19:33 2015 +0200 @@ -36,6 +36,7 @@ #include "../ServerToolbox.h" #include "../FromDcmtkBridge.h" #include "../ResourceFinder.h" +#include "../DicomFindQuery.h" #include @@ -868,13 +869,8 @@ std::string level = request["Level"].asString(); - /*ResourceFinder finder(context); - finder.SetLevel(StringToResourceType(level.c_str())); - - if (request.isMember("CaseSensitive")) - { - finder.SetCaseSensitive(request["CaseSensitive"].asBool()); - } + DicomFindQuery query; + query.SetLevel(StringToResourceType(level.c_str())); Json::Value::Members members = request["Query"].getMemberNames(); for (size_t i = 0; i < members.size(); i++) @@ -884,12 +880,14 @@ throw OrthancException(ErrorCode_BadRequest); } - finder.AddTag(members[i], request["Query"][members[i]].asString()); - } - + query.SetConstraint(FromDcmtkBridge::ParseTag(members[i]), + request["Query"][members[i]].asString()); + } + std::list resources; - finder.Apply(resources); - AnswerListOfResources(call.GetOutput(), context.GetIndex(), resources, finder.GetLevel(), expand);*/ + ResourceFinder finder(context); + finder.Apply(resources, query); + AnswerListOfResources(call.GetOutput(), context.GetIndex(), resources, query.GetLevel(), expand); } else { diff -r 0649c5aef34a -r 94ffb597d297 OrthancServer/ResourceFinder.cpp --- a/OrthancServer/ResourceFinder.cpp Fri May 15 15:34:32 2015 +0200 +++ b/OrthancServer/ResourceFinder.cpp Fri May 15 17:19:33 2015 +0200 @@ -209,7 +209,7 @@ DicomMap mainTags; if (index_.GetMainDicomTags(mainTags, *it, level_)) { - if (query.FilterMainDicomTags(mainTags, level_)) + if (query.FilterMainDicomTags(*it, level_, mainTags)) { filtered_.insert(*it); } diff -r 0649c5aef34a -r 94ffb597d297 OrthancServer/ResourceFinder.h --- a/OrthancServer/ResourceFinder.h Fri May 15 15:34:32 2015 +0200 +++ b/OrthancServer/ResourceFinder.h Fri May 15 17:19:33 2015 +0200 @@ -55,8 +55,9 @@ virtual bool HasMainDicomTagsFilter(ResourceType level) const = 0; - virtual bool FilterMainDicomTags(const DicomMap& mainTags, - ResourceType level) const = 0; + virtual bool FilterMainDicomTags(const std::string& resourceId, + ResourceType level, + const DicomMap& mainTags) const = 0; virtual bool HasInstanceFilter() const = 0; diff -r 0649c5aef34a -r 94ffb597d297 Resources/CMake/DcmtkConfiguration.cmake --- a/Resources/CMake/DcmtkConfiguration.cmake Fri May 15 15:34:32 2015 +0200 +++ b/Resources/CMake/DcmtkConfiguration.cmake Fri May 15 17:19:33 2015 +0200 @@ -99,6 +99,7 @@ ${DCMTK_SOURCES_DIR}/oflog/libsrc/windebap.cc ${DCMTK_SOURCES_DIR}/oflog/libsrc/winsock.cc ) + elseif (${CMAKE_SYSTEM_NAME} STREQUAL "Windows") list(REMOVE_ITEM DCMTK_SOURCES ${DCMTK_SOURCES_DIR}/oflog/libsrc/unixsock.cc