Mercurial > hg > orthanc
changeset 4197:b1d528687e25
merge
| author | Sebastien Jodogne <s.jodogne@gmail.com> |
|---|---|
| date | Wed, 16 Sep 2020 13:30:01 +0200 |
| parents | 37310bb1cd30 (diff) db38b2ad4c4a (current diff) |
| children | c671331ea1ef |
| files | NEWS OrthancServer/Resources/Configuration.json |
| diffstat | 9 files changed, 323 insertions(+), 55 deletions(-) [+] |
line wrap: on
line diff
--- a/NEWS Wed Sep 16 11:24:08 2020 +0200 +++ b/NEWS Wed Sep 16 13:30:01 2020 +0200 @@ -5,10 +5,10 @@ ------- * New configuration options to enable HTTP peers identification through certificates: - "SslVerifyPeers" & "SslTrustedClientCertificates" -* New configuration option "SyncStorageArea" to commit the files on disk "inside" the DB - transaction and avoid DB - File system discrepencies in case of hard shutdown - of the machine running Orthanc. This comes with a cost: DICOM file ingestion is slower. + "SslVerifyPeers" and "SslTrustedClientCertificates" +* New configuration option "SyncStorageArea" to immediately commit the files onto the disk + (through fsync()), so as to avoid discrepencies between DB and filesystem in case of hard + shutdown of the machine running Orthanc. This slows down adding new files into Orthanc. Maintenance ----------- @@ -19,7 +19,8 @@ * When checking DICOM allowed methods, if there are multiple modalities with the same AET, differentiate them from the calling IP * Enable the access to raw frames in Philips ELSCINT1 proprietary compression -* Support empty key passwords when using client certificates +* Support empty key passwords when using HTTP client certificates +* Fix handling of "ModalitiesInStudy" (0008,0061) in C-FIND and "/tools/find" Version 1.7.3 (2020-08-24)
--- a/OrthancServer/Resources/Configuration.json Wed Sep 16 11:24:08 2020 +0200 +++ b/OrthancServer/Resources/Configuration.json Wed Sep 16 13:30:01 2020 +0200 @@ -147,20 +147,21 @@ // Whether or not SSL is enabled "SslEnabled" : false, - // Path to the SSL certificate used by the HTTP server. - // Certifcate must be stored in the PEM format. - // meaningful only if SslEnabled is true. - // The file must contain both the certificate and the private key. + // Path to the SSL certificate used by the HTTP server. The file + // must be stored in the PEM format, and must contain both the + // certificate and the private key. This option is only meaningful + // if "SslEnabled" is true. "SslCertificate" : "certificate.pem", - // Whether or not peer client certificates shall be checked. - // meaningfull only if SslEnabled is true + // Whether or not peer client certificates shall be checked. This + // option is only meaningfull if "SslEnabled" is true. "SslVerifyPeers" : false, - // Path to the SSL certificate(s) that are trusted to verify - // peers identify. - // Certifcate(s) must be stored in the PEM format. - // meaningfull only if SslVerifyPeers is true + // Path to a file containing the concatenation of the client SSL + // certificate(s) that are trusted to verify the identify of remote + // HTTP clients. The individual certificate(s) must be stored in the + // PEM format. This option is only meaningfull if "SslVerifyPeers" + // is true. "SslTrustedClientCertificates" : "trustedClientCertificates.pem", // Whether or not the password protection is enabled (using HTTP
--- a/OrthancServer/Sources/Search/DatabaseLookup.cpp Wed Sep 16 11:24:08 2020 +0200 +++ b/OrthancServer/Sources/Search/DatabaseLookup.cpp Wed Sep 16 13:30:01 2020 +0200 @@ -66,7 +66,7 @@ } - void DatabaseLookup::AddConstraint(DicomTagConstraint* constraint) + void DatabaseLookup::AddConstraintInternal(DicomTagConstraint* constraint) { if (constraint == NULL) { @@ -163,28 +163,20 @@ if (!lower.empty()) { - AddConstraint(new DicomTagConstraint - (tag, ConstraintType_GreaterOrEqual, lower, caseSensitive, mandatoryTag)); + AddConstraintInternal(new DicomTagConstraint + (tag, ConstraintType_GreaterOrEqual, lower, caseSensitive, mandatoryTag)); } if (!upper.empty()) { - AddConstraint(new DicomTagConstraint - (tag, ConstraintType_SmallerOrEqual, upper, caseSensitive, mandatoryTag)); + AddConstraintInternal(new DicomTagConstraint + (tag, ConstraintType_SmallerOrEqual, upper, caseSensitive, mandatoryTag)); } } - else if (tag == DICOM_TAG_MODALITIES_IN_STUDY || - dicomQuery.find('\\') != std::string::npos) + else if (dicomQuery.find('\\') != std::string::npos) { DicomTag fixedTag(tag); - if (tag == DICOM_TAG_MODALITIES_IN_STUDY) - { - // http://www.itk.org/Wiki/DICOM_QueryRetrieve_Explained - // http://dicomiseasy.blogspot.be/2012/01/dicom-queryretrieve-part-i.html - fixedTag = DICOM_TAG_MODALITY; - } - std::unique_ptr<DicomTagConstraint> constraint (new DicomTagConstraint(fixedTag, ConstraintType_List, caseSensitive, mandatoryTag)); @@ -196,7 +188,7 @@ constraint->AddValue(items[i]); } - AddConstraint(constraint.release()); + AddConstraintInternal(constraint.release()); } else if ( /** @@ -219,13 +211,13 @@ (dicomQuery.find('*') != std::string::npos || dicomQuery.find('?') != std::string::npos)) { - AddConstraint(new DicomTagConstraint - (tag, ConstraintType_Wildcard, dicomQuery, caseSensitive, mandatoryTag)); + AddConstraintInternal(new DicomTagConstraint + (tag, ConstraintType_Wildcard, dicomQuery, caseSensitive, mandatoryTag)); } else { - AddConstraint(new DicomTagConstraint - (tag, ConstraintType_Equal, dicomQuery, caseSensitive, mandatoryTag)); + AddConstraintInternal(new DicomTagConstraint + (tag, ConstraintType_Equal, dicomQuery, caseSensitive, mandatoryTag)); } } @@ -313,4 +305,34 @@ return true; } + + + std::string DatabaseLookup::Format() const + { + std::string s; + + for (size_t i = 0; i < constraints_.size(); i++) + { + assert(constraints_[i] != NULL); + s += ("Contraint " + boost::lexical_cast<std::string>(i) + ": " + + constraints_[i]->Format() + "\n"); + } + + return s; + } + + + bool DatabaseLookup::HasTag(const DicomTag& tag) const + { + for (size_t i = 0; i < constraints_.size(); i++) + { + assert(constraints_[i] != NULL); + if (constraints_[i]->GetTag() == tag) + { + return true; + } + } + + return false; + } }
--- a/OrthancServer/Sources/Search/DatabaseLookup.h Wed Sep 16 11:24:08 2020 +0200 +++ b/OrthancServer/Sources/Search/DatabaseLookup.h Wed Sep 16 13:30:01 2020 +0200 @@ -50,7 +50,7 @@ bool caseSensitive, bool mandatoryTag); - void AddConstraint(DicomTagConstraint* constraint); // Takes ownership + void AddConstraintInternal(DicomTagConstraint* constraint); // Takes ownership public: DatabaseLookup() @@ -87,6 +87,15 @@ bool caseSensitive, bool mandatoryTag); + void AddConstraint(const DicomTagConstraint& constraint) + { + AddConstraintInternal(new DicomTagConstraint(constraint)); + } + bool HasOnlyMainDicomTags() const; + + std::string Format() const; + + bool HasTag(const DicomTag& tag) const; }; }
--- a/OrthancServer/Sources/Search/DicomTagConstraint.cpp Wed Sep 16 11:24:08 2020 +0200 +++ b/OrthancServer/Sources/Search/DicomTagConstraint.cpp Wed Sep 16 13:30:01 2020 +0200 @@ -154,6 +154,16 @@ } + DicomTagConstraint::DicomTagConstraint(const DicomTagConstraint& other) : + tag_(other.tag_), + constraintType_(other.constraintType_), + values_(other.values_), + caseSensitive_(other.caseSensitive_), + mandatory_(other.mandatory_) + { + } + + DicomTagConstraint::DicomTagConstraint(const DatabaseConstraint& constraint) : tag_(constraint.GetTag()), constraintType_(constraint.GetConstraintType()), @@ -333,7 +343,7 @@ s += *it; } - return s + "]"; + return s + " ]"; } default:
--- a/OrthancServer/Sources/Search/DicomTagConstraint.h Wed Sep 16 11:24:08 2020 +0200 +++ b/OrthancServer/Sources/Search/DicomTagConstraint.h Wed Sep 16 13:30:01 2020 +0200 @@ -70,6 +70,8 @@ bool caseSensitive, bool mandatory); + DicomTagConstraint(const DicomTagConstraint& other); + DicomTagConstraint(const DatabaseConstraint& constraint); const DicomTag& GetTag() const @@ -77,6 +79,11 @@ return tag_; } + void SetTag(const DicomTag& tag) + { + tag_ = tag; + } + ConstraintType GetConstraintType() const { return constraintType_;
--- a/OrthancServer/Sources/ServerContext.cpp Wed Sep 16 11:24:08 2020 +0200 +++ b/OrthancServer/Sources/ServerContext.cpp Wed Sep 16 13:30:01 2020 +0200 @@ -34,10 +34,10 @@ #include "PrecompiledHeadersServer.h" #include "ServerContext.h" -#include "../../OrthancFramework/Sources/DicomParsing/Internals/DicomImageDecoder.h" #include "../../OrthancFramework/Sources/Cache/SharedArchive.h" #include "../../OrthancFramework/Sources/DicomParsing/DcmtkTranscoder.h" #include "../../OrthancFramework/Sources/DicomParsing/FromDcmtkBridge.h" +#include "../../OrthancFramework/Sources/DicomParsing/Internals/DicomImageDecoder.h" #include "../../OrthancFramework/Sources/FileStorage/StorageAccessor.h" #include "../../OrthancFramework/Sources/HttpServer/FilesystemHttpSender.h" #include "../../OrthancFramework/Sources/HttpServer/HttpStreamTranscoder.h" @@ -936,12 +936,12 @@ } - void ServerContext::Apply(ILookupVisitor& visitor, - const DatabaseLookup& lookup, - ResourceType queryLevel, - size_t since, - size_t limit) - { + void ServerContext::ApplyInternal(ILookupVisitor& visitor, + const DatabaseLookup& lookup, + ResourceType queryLevel, + size_t since, + size_t limit) + { unsigned int databaseLimit = (queryLevel == ResourceType_Instance ? limitFindInstances_ : limitFindResults_); @@ -994,7 +994,6 @@ continue; } -#if 1 // New in Orthanc 1.6.0: Only keep the main DICOM tags at the // level of interest for the query switch (queryLevel) @@ -1016,17 +1015,6 @@ default: throw OrthancException(ErrorCode_InternalError); } - - // Special case of the "Modality" at the study level, in order - // to deal with C-FIND on "ModalitiesInStudy" (0008,0061). - // Check out integration test "test_rest_modalities_in_study". - if (queryLevel == ResourceType_Study) - { - dicom.CopyTagIfExists(tmp, DICOM_TAG_MODALITY); - } -#else - dicom.Assign(tmp); // This emulates Orthanc <= 1.5.8 -#endif hasOnlyMainDicomTags = true; } @@ -1096,6 +1084,225 @@ } + + namespace + { + class ModalitiesInStudyVisitor : public ServerContext::ILookupVisitor + { + private: + class Study : public boost::noncopyable + { + private: + std::string orthancId_; + std::string instanceId_; + DicomMap mainDicomTags_; + Json::Value dicomAsJson_; + std::set<std::string> modalitiesInStudy_; + + public: + Study(const std::string& instanceId, + const DicomMap& seriesTags) : + instanceId_(instanceId), + dicomAsJson_(Json::nullValue) + { + { + DicomMap tmp; + tmp.Assign(seriesTags); + tmp.SetValue(DICOM_TAG_SOP_INSTANCE_UID, "dummy", false); + DicomInstanceHasher hasher(tmp); + orthancId_ = hasher.HashStudy(); + } + + mainDicomTags_.MergeMainDicomTags(seriesTags, ResourceType_Study); + mainDicomTags_.MergeMainDicomTags(seriesTags, ResourceType_Patient); + AddModality(seriesTags); + } + + void AddModality(const DicomMap& seriesTags) + { + std::string modality; + if (seriesTags.LookupStringValue(modality, DICOM_TAG_MODALITY, false) && + !modality.empty()) + { + modalitiesInStudy_.insert(modality); + } + } + + void SetDicomAsJson(const Json::Value& dicomAsJson) + { + dicomAsJson_ = dicomAsJson; + } + + const std::string& GetOrthancId() const + { + return orthancId_; + } + + const std::string& GetInstanceId() const + { + return instanceId_; + } + + const DicomMap& GetMainDicomTags() const + { + return mainDicomTags_; + } + + const Json::Value* GetDicomAsJson() const + { + if (dicomAsJson_.type() == Json::nullValue) + { + return NULL; + } + else + { + return &dicomAsJson_; + } + } + }; + + typedef std::map<std::string, Study*> Studies; + + bool isDicomAsJsonNeeded_; + bool complete_; + Studies studies_; + + public: + ModalitiesInStudyVisitor(bool isDicomAsJsonNeeded) : + isDicomAsJsonNeeded_(isDicomAsJsonNeeded) + { + } + + ~ModalitiesInStudyVisitor() + { + for (Studies::const_iterator it = studies_.begin(); it != studies_.end(); ++it) + { + assert(it->second != NULL); + delete it->second; + } + + studies_.clear(); + } + + virtual bool IsDicomAsJsonNeeded() const + { + return isDicomAsJsonNeeded_; + } + + virtual void MarkAsComplete() + { + complete_ = true; + } + + virtual void Visit(const std::string& publicId, + const std::string& instanceId, + const DicomMap& seriesTags, + const Json::Value* dicomAsJson) + { + std::string studyInstanceUid; + if (seriesTags.LookupStringValue(studyInstanceUid, DICOM_TAG_STUDY_INSTANCE_UID, false)) + { + Studies::iterator found = studies_.find(studyInstanceUid); + if (found == studies_.end()) + { + // New study + std::unique_ptr<Study> study(new Study(instanceId, seriesTags)); + + if (dicomAsJson != NULL) + { + study->SetDicomAsJson(*dicomAsJson); + } + + studies_[studyInstanceUid] = study.release(); + } + else + { + // Already existing study + found->second->AddModality(seriesTags); + } + } + } + + void Forward(ILookupVisitor& callerVisitor, + size_t since, + size_t limit) const + { + size_t index = 0; + size_t countForwarded = 0; + + for (Studies::const_iterator it = studies_.begin(); it != studies_.end(); ++it, index++) + { + if (limit == 0 || + (index >= since && + index < limit)) + { + assert(it->second != NULL); + const Study& study = *it->second; + + countForwarded++; + callerVisitor.Visit(study.GetOrthancId(), study.GetInstanceId(), + study.GetMainDicomTags(), study.GetDicomAsJson()); + } + } + + if (countForwarded == studies_.size()) + { + callerVisitor.MarkAsComplete(); + } + } + }; + } + + + void ServerContext::Apply(ILookupVisitor& visitor, + const DatabaseLookup& lookup, + ResourceType queryLevel, + size_t since, + size_t limit) + { + if (queryLevel == ResourceType_Study && + lookup.HasTag(DICOM_TAG_MODALITIES_IN_STUDY)) + { + // Convert the study-level query, into a series-level query, + // where "ModalitiesInStudy" is replaced by "Modality" + DatabaseLookup seriesLookup; + + for (size_t i = 0; i < lookup.GetConstraintsCount(); i++) + { + const DicomTagConstraint& constraint = lookup.GetConstraint(i); + if (constraint.GetTag() == DICOM_TAG_MODALITIES_IN_STUDY) + { + if ((constraint.GetConstraintType() == ConstraintType_Equal && constraint.GetValue().empty()) || + (constraint.GetConstraintType() == ConstraintType_List && constraint.GetValues().empty())) + { + // Ignore universal lookup on "ModalitiesInStudy" (0008,0061), + // this should have been handled by the caller + return ApplyInternal(visitor, lookup, queryLevel, since, limit); + } + else + { + DicomTagConstraint modality(constraint); + modality.SetTag(DICOM_TAG_MODALITY); + seriesLookup.AddConstraint(modality); + } + } + else + { + seriesLookup.AddConstraint(constraint); + } + } + + ModalitiesInStudyVisitor seriesVisitor(visitor.IsDicomAsJsonNeeded()); + ApplyInternal(seriesVisitor, seriesLookup, ResourceType_Series, 0, 0); + seriesVisitor.Forward(visitor, since, limit); + } + else + { + ApplyInternal(visitor, lookup, queryLevel, since, limit); + } + } + + bool ServerContext::LookupOrReconstructMetadata(std::string& target, const std::string& publicId, MetadataType metadata)
--- a/OrthancServer/Sources/ServerContext.h Wed Sep 16 11:24:08 2020 +0200 +++ b/OrthancServer/Sources/ServerContext.h Wed Sep 16 13:30:01 2020 +0200 @@ -80,6 +80,8 @@ virtual void MarkAsComplete() = 0; + // NB: "dicomAsJson" must *not* be deleted, and can be NULL if + // "!IsDicomAsJsonNeeded()" virtual void Visit(const std::string& publicId, const std::string& instanceId, const DicomMap& mainDicomTags, @@ -237,6 +239,12 @@ DicomInstanceToStore& dicom, StoreInstanceMode mode); + void ApplyInternal(ILookupVisitor& visitor, + const DatabaseLookup& lookup, + ResourceType queryLevel, + size_t since, + size_t limit); + public: class DicomCacheLocker : public boost::noncopyable {
--- a/OrthancServer/UnitTestsSources/DatabaseLookupTests.cpp Wed Sep 16 11:24:08 2020 +0200 +++ b/OrthancServer/UnitTestsSources/DatabaseLookupTests.cpp Wed Sep 16 13:30:01 2020 +0200 @@ -154,6 +154,9 @@ ASSERT_EQ(ConstraintType_Equal, lookup.GetConstraint(0).GetConstraintType()); ASSERT_EQ("HELLO", lookup.GetConstraint(0).GetValue()); ASSERT_TRUE(lookup.GetConstraint(0).IsCaseSensitive()); + + ASSERT_TRUE(lookup.HasTag(DICOM_TAG_PATIENT_ID)); + ASSERT_FALSE(lookup.HasTag(DICOM_TAG_PATIENT_NAME)); } {