# HG changeset patch # User Sebastien Jodogne # Date 1699433971 -3600 # Node ID 08177310e269ba61322c552ff2bdfb2f8dee1ea2 # Parent 1c6708a0e0c6281b104a7e8dfe20b063ba9a4906# Parent 62bb63346185877e902c8f7ca9dc029c0325cee2 merge diff -r 1c6708a0e0c6 -r 08177310e269 NEWS --- a/NEWS Wed Nov 08 09:59:13 2023 +0100 +++ b/NEWS Wed Nov 08 09:59:31 2023 +0100 @@ -14,6 +14,10 @@ * API version upgraded to 22 * Added a route to delete completed jobs from history: DELETE /jobs/{id} +* All 'expand' GET arguments now accepts expand=true and expand=false values. + The /studies/../instances and sibling routes are the only whose expand is true if not specified. + These routes now accepts expand=false to simply list the child resources ids. + Maintenance ----------- @@ -26,9 +30,10 @@ * Housekeeper: Introduced a 'sleep' to lower CPU usage when idle. * Support multiple values in SpecificCharacterSet in C-Find answers: https://discourse.orthanc-server.org/t/c-find-fails-on-unknown-specific-character-set-iso-2022-ir-6-iso-2022-ir-100/3947 +* When exporting a study archive, make sure to use the PatientName from the study and not from the patient + in case of PatientID collision. * Upgraded dependencies for static builds: - boost 1.83.0 - Version 1.12.1 (2023-07-04) diff -r 1c6708a0e0c6 -r 08177310e269 OrthancServer/Plugins/Include/orthanc/OrthancCPlugin.h --- a/OrthancServer/Plugins/Include/orthanc/OrthancCPlugin.h Wed Nov 08 09:59:13 2023 +0100 +++ b/OrthancServer/Plugins/Include/orthanc/OrthancCPlugin.h Wed Nov 08 09:59:31 2023 +0100 @@ -846,7 +846,7 @@ **/ typedef enum { - OrthancPluginDicomToJsonFlags_None = 0, + OrthancPluginDicomToJsonFlags_None = 0, /*!< Default formatting */ OrthancPluginDicomToJsonFlags_IncludeBinary = (1 << 0), /*!< Include the binary tags */ OrthancPluginDicomToJsonFlags_IncludePrivateTags = (1 << 1), /*!< Include the private tags */ OrthancPluginDicomToJsonFlags_IncludeUnknownTags = (1 << 2), /*!< Include the tags unknown by the dictionary */ @@ -861,13 +861,13 @@ /** - * Flags to the creation of a DICOM file. + * Flags for the creation of a DICOM file. * @ingroup Toolbox * @see OrthancPluginCreateDicom() **/ typedef enum { - OrthancPluginCreateDicomFlags_None = 0, + OrthancPluginCreateDicomFlags_None = 0, /*!< Default mode */ OrthancPluginCreateDicomFlags_DecodeDataUriScheme = (1 << 0), /*!< Decode fields encoded using data URI scheme */ OrthancPluginCreateDicomFlags_GenerateIdentifiers = (1 << 1), /*!< Automatically generate DICOM identifiers */ @@ -984,32 +984,46 @@ **/ typedef enum { + /** + * Success: The DICOM instance is properly stored in the SCP + **/ OrthancPluginStorageCommitmentFailureReason_Success = 0, - /*!< Success: The DICOM instance is properly stored in the SCP */ - + + /** + * 0110H: A general failure in processing the operation was encountered + **/ OrthancPluginStorageCommitmentFailureReason_ProcessingFailure = 1, - /*!< 0110H: A general failure in processing the operation was encountered */ - + + /** + * 0112H: One or more of the elements in the Referenced SOP + * Instance Sequence was not available + **/ OrthancPluginStorageCommitmentFailureReason_NoSuchObjectInstance = 2, - /*!< 0112H: One or more of the elements in the Referenced SOP - Instance Sequence was not available */ - + + /** + * 0213H: The SCP does not currently have enough resources to + * store the requested SOP Instance(s) + **/ OrthancPluginStorageCommitmentFailureReason_ResourceLimitation = 3, - /*!< 0213H: The SCP does not currently have enough resources to - store the requested SOP Instance(s) */ - + + /** + * 0122H: Storage Commitment has been requested for a SOP Instance + * with a SOP Class that is not supported by the SCP + **/ OrthancPluginStorageCommitmentFailureReason_ReferencedSOPClassNotSupported = 4, - /*!< 0122H: Storage Commitment has been requested for a SOP - Instance with a SOP Class that is not supported by the SCP */ - + + /** + * 0119H: The SOP Class of an element in the Referenced SOP + * Instance Sequence did not correspond to the SOP class + * registered for this SOP Instance at the SCP + **/ OrthancPluginStorageCommitmentFailureReason_ClassInstanceConflict = 5, - /*!< 0119H: The SOP Class of an element in the Referenced SOP - Instance Sequence did not correspond to the SOP class registered - for this SOP Instance at the SCP */ - + + /** + * 0131H: The Transaction UID of the Storage Commitment Request is + * already in use + **/ OrthancPluginStorageCommitmentFailureReason_DuplicateTransactionUID = 6 - /*!< 0131H: The Transaction UID of the Storage Commitment Request - is already in use */ } OrthancPluginStorageCommitmentFailureReason; @@ -1032,17 +1046,23 @@ **/ typedef enum { + /** + * Load the whole DICOM file, including pixel data + **/ OrthancPluginLoadDicomInstanceMode_WholeDicom = 1, - /*!< Load the whole DICOM file, including pixel data */ - + + /** + * Load the whole DICOM file until pixel data, which speeds up the + * loading + **/ OrthancPluginLoadDicomInstanceMode_UntilPixelData = 2, - /*!< Load the whole DICOM file until pixel data, which will speed - up the loading */ - + + /** + * Load the whole DICOM file until pixel data, and replace pixel + * data by an empty tag whose VR (value representation) is the + * same as those of the original DICOM file + **/ OrthancPluginLoadDicomInstanceMode_EmptyPixelData = 3, - /*!< Load the whole DICOM file until pixel data, and replace pixel - data by an empty tag whose VR (value representation) is the same - as those of the original DICOM file */ _OrthancPluginLoadDicomInstanceMode_INTERNAL = 0x7fffffff } OrthancPluginLoadDicomInstanceMode; @@ -9223,7 +9243,7 @@ } _OrthancPluginRegisterDatabaseBackendV4; /** - * Register a custom database back-end. + * @brief Register a custom database back-end. * * This function was added in Orthanc SDK 1.12.0. It uses Google * Protocol Buffers for the communications between the Orthanc core diff -r 1c6708a0e0c6 -r 08177310e269 OrthancServer/Sources/OrthancRestApi/OrthancRestArchive.cpp --- a/OrthancServer/Sources/OrthancRestApi/OrthancRestArchive.cpp Wed Nov 08 09:59:13 2023 +0100 +++ b/OrthancServer/Sources/OrthancRestApi/OrthancRestArchive.cpp Wed Nov 08 09:59:31 2023 +0100 @@ -556,7 +556,7 @@ GetJobParameters(synchronous, extended, transcode, transferSyntax, priority, loaderThreads, body, DEFAULT_IS_EXTENDED); - std::unique_ptr job(new ArchiveJob(context, IS_MEDIA, extended)); + std::unique_ptr job(new ArchiveJob(context, IS_MEDIA, extended, ResourceType_Patient)); AddResourcesOfInterest(*job, body); if (transcode) @@ -627,7 +627,7 @@ extended = false; } - std::unique_ptr job(new ArchiveJob(context, IS_MEDIA, extended)); + std::unique_ptr job(new ArchiveJob(context, IS_MEDIA, extended, (LEVEL == ResourceType_Patient ? ResourceType_Patient : ResourceType_Study))); // use patient info from study except when exporting a patient job->AddResource(id, true, LEVEL); if (call.HasArgument(TRANSCODE)) @@ -679,7 +679,7 @@ GetJobParameters(synchronous, extended, transcode, transferSyntax, priority, loaderThreads, body, false /* by default, not extented */); - std::unique_ptr job(new ArchiveJob(context, IS_MEDIA, extended)); + std::unique_ptr job(new ArchiveJob(context, IS_MEDIA, extended, LEVEL)); job->AddResource(id, true, LEVEL); if (transcode) diff -r 1c6708a0e0c6 -r 08177310e269 OrthancServer/Sources/OrthancRestApi/OrthancRestModalities.cpp --- a/OrthancServer/Sources/OrthancRestApi/OrthancRestModalities.cpp Wed Nov 08 09:59:13 2023 +0100 +++ b/OrthancServer/Sources/OrthancRestApi/OrthancRestModalities.cpp Wed Nov 08 09:59:31 2023 +0100 @@ -843,7 +843,7 @@ return; } - const bool expand = call.HasArgument("expand"); + const bool expand = call.HasArgument("expand") && call.GetBooleanArgument("expand", true); const DicomToJsonFormat format = OrthancRestApi::GetDicomFormat(call, DicomToJsonFormat_Full); QueryAccessor query(call); @@ -1649,7 +1649,7 @@ OrthancRestApi::SetOfStrings peers; lock.GetConfiguration().GetListOfOrthancPeers(peers); - if (call.HasArgument("expand")) + if (call.HasArgument("expand") && call.GetBooleanArgument("expand", true)) { Json::Value result = Json::objectValue; for (OrthancRestApi::SetOfStrings::const_iterator @@ -1933,7 +1933,7 @@ OrthancRestApi::SetOfStrings modalities; lock.GetConfiguration().GetListOfDicomModalities(modalities); - if (call.HasArgument("expand")) + if (call.HasArgument("expand") && call.GetBooleanArgument("expand", true)) { Json::Value result = Json::objectValue; for (OrthancRestApi::SetOfStrings::const_iterator diff -r 1c6708a0e0c6 -r 08177310e269 OrthancServer/Sources/OrthancRestApi/OrthancRestResources.cpp --- a/OrthancServer/Sources/OrthancRestApi/OrthancRestResources.cpp Wed Nov 08 09:59:13 2023 +0100 +++ b/OrthancServer/Sources/OrthancRestApi/OrthancRestResources.cpp Wed Nov 08 09:59:31 2023 +0100 @@ -254,7 +254,7 @@ index.GetAllUuids(result, resourceType); } - AnswerListOfResources(call.GetOutput(), context, result, resourceType, call.HasArgument("expand"), + AnswerListOfResources(call.GetOutput(), context, result, resourceType, call.HasArgument("expand") && call.GetBooleanArgument("expand", true), OrthancRestApi::GetDicomFormat(call, DicomToJsonFormat_Human), requestedTags, true /* allowStorageAccess */); @@ -1701,7 +1701,7 @@ bool isNumeric = call.HasArgument("numeric"); - if (call.HasArgument("expand")) + if (call.HasArgument("expand") && call.GetBooleanArgument("expand", true)) { result = Json::objectValue; @@ -3324,12 +3324,15 @@ .SetDescription("Get detailed information about the child " + children + " of the DICOM " + resource + " whose Orthanc identifier is provided in the URL") .SetUriArgument("id", "Orthanc identifier of the " + resource + " of interest") + .SetHttpGetArgument("expand", RestApiCallDocumentation::Type_String, + "If false or missing, only retrieve the list of child " + children, false) .AddAnswerType(MimeType_Json, "JSON array containing information about the child DICOM " + children) .SetTruncatedJsonHttpGetSample(GetDocumentationSampleResource(start) + "/" + children, 5); return; } ServerIndex& index = OrthancRestApi::GetIndex(call); + ServerContext& context = OrthancRestApi::GetContext(call); std::set requestedTags; OrthancRestApi::GetRequestedTags(requestedTags, call); @@ -3355,21 +3358,10 @@ a.splice(a.begin(), b); } - Json::Value result = Json::arrayValue; - - const DicomToJsonFormat format = OrthancRestApi::GetDicomFormat(call, DicomToJsonFormat_Human); - - for (std::list::const_iterator - it = a.begin(); it != a.end(); ++it) - { - Json::Value resource; - if (OrthancRestApi::GetContext(call).ExpandResource(resource, *it, end, format, requestedTags, true /* allowStorageAccess */)) - { - result.append(resource); - } - } - - call.GetOutput().AnswerJson(result); + AnswerListOfResources(call.GetOutput(), context, a, type, !call.HasArgument("expand") || call.GetBooleanArgument("expand", false), // this "expand" is the only one to have a false default value to keep backward compatibility + OrthancRestApi::GetDicomFormat(call, DicomToJsonFormat_Human), + requestedTags, + true /* allowStorageAccess */); } diff -r 1c6708a0e0c6 -r 08177310e269 OrthancServer/Sources/OrthancRestApi/OrthancRestSystem.cpp --- a/OrthancServer/Sources/OrthancRestApi/OrthancRestSystem.cpp Wed Nov 08 09:59:13 2023 +0100 +++ b/OrthancServer/Sources/OrthancRestApi/OrthancRestSystem.cpp Wed Nov 08 09:59:31 2023 +0100 @@ -657,7 +657,7 @@ return; } - bool expand = call.HasArgument("expand"); + bool expand = call.HasArgument("expand") && call.GetBooleanArgument("expand", true); Json::Value v = Json::arrayValue; diff -r 1c6708a0e0c6 -r 08177310e269 OrthancServer/Sources/ServerJobs/ArchiveJob.cpp --- a/OrthancServer/Sources/ServerJobs/ArchiveJob.cpp Wed Nov 08 09:59:13 2023 +0100 +++ b/OrthancServer/Sources/ServerJobs/ArchiveJob.cpp Wed Nov 08 09:59:31 2023 +0100 @@ -306,6 +306,91 @@ } }; + // This enum defines specific resource types to be used when exporting the archive. + // It defines if we should use the PatientInfo from the Patient or from the Study. + enum ArchiveResourceType + { + ArchiveResourceType_Patient = 0, + ArchiveResourceType_PatientInfoFromStudy = 1, + ArchiveResourceType_Study = 2, + ArchiveResourceType_Series = 3, + ArchiveResourceType_Instance = 4 + }; + + ResourceType GetResourceIdType(ArchiveResourceType type) + { + switch (type) + { + case ArchiveResourceType_Patient: + return ResourceType_Patient; + case ArchiveResourceType_PatientInfoFromStudy: // get the Patient tags from the Study id + return ResourceType_Study; + case ArchiveResourceType_Study: + return ResourceType_Study; + case ArchiveResourceType_Series: + return ResourceType_Series; + case ArchiveResourceType_Instance: + return ResourceType_Instance; + default: + throw OrthancException(ErrorCode_ParameterOutOfRange); + } + } + + ResourceType GetResourceLevel(ArchiveResourceType type) + { + switch (type) + { + case ArchiveResourceType_Patient: + return ResourceType_Patient; + case ArchiveResourceType_PatientInfoFromStudy: // this is actually the same level as the Patient + return ResourceType_Patient; + case ArchiveResourceType_Study: + return ResourceType_Study; + case ArchiveResourceType_Series: + return ResourceType_Series; + case ArchiveResourceType_Instance: + return ResourceType_Instance; + default: + throw OrthancException(ErrorCode_ParameterOutOfRange); + } + } + + ArchiveResourceType GetArchiveResourceType(ResourceType type) + { + switch (type) + { + case ResourceType_Patient: + return ArchiveResourceType_Patient; + case ArchiveResourceType_Study: + return ArchiveResourceType_PatientInfoFromStudy; + case ResourceType_Series: + return ArchiveResourceType_Series; + case ResourceType_Instance: + return ArchiveResourceType_Instance; + default: + throw OrthancException(ErrorCode_ParameterOutOfRange); + } + } + + ArchiveResourceType GetChildResourceType(ArchiveResourceType type) + { + switch (type) + { + case ArchiveResourceType_Patient: + case ArchiveResourceType_PatientInfoFromStudy: + return ArchiveResourceType_Study; + + case ArchiveResourceType_Study: + return ArchiveResourceType_Series; + + case ArchiveResourceType_Series: + return ArchiveResourceType_Instance; + + default: + throw OrthancException(ErrorCode_ParameterOutOfRange); + } + } + class ArchiveJob::ResourceIdentifiers : public boost::noncopyable { @@ -410,7 +495,7 @@ { } - virtual void Open(ResourceType level, + virtual void Open(ArchiveResourceType level, const std::string& publicId) = 0; virtual void Close() = 0; @@ -439,7 +524,7 @@ // A "NULL" value for ArchiveIndex indicates a non-expanded node typedef std::map Resources; - ResourceType level_; + ArchiveResourceType level_; Resources resources_; // Only at patient/study/series level std::list instances_; // Only at instance level @@ -447,7 +532,7 @@ void AddResourceToExpand(ServerIndex& index, const std::string& id) { - if (level_ == ResourceType_Instance) + if (level_ == ArchiveResourceType_Instance) { FileInfo tmp; int64_t revision; // ignored @@ -464,7 +549,7 @@ public: - explicit ArchiveIndex(ResourceType level) : + explicit ArchiveIndex(ArchiveResourceType level) : level_(level) { } @@ -482,14 +567,14 @@ void Add(ServerIndex& index, const ResourceIdentifiers& resource) { - const std::string& id = resource.GetIdentifier(level_); + const std::string& id = resource.GetIdentifier(GetResourceIdType(level_)); Resources::iterator previous = resources_.find(id); - if (level_ == ResourceType_Instance) + if (level_ == ArchiveResourceType_Instance) { AddResourceToExpand(index, id); } - else if (resource.GetLevel() == level_) + else if (resource.GetLevel() == GetResourceLevel(level_)) { // Mark this resource for further expansion if (previous != resources_.end()) @@ -519,7 +604,7 @@ void Expand(ServerIndex& index) { - if (level_ == ResourceType_Instance) + if (level_ == ArchiveResourceType_Instance) { // Expanding an instance node makes no sense return; @@ -553,7 +638,7 @@ void Apply(IArchiveVisitor& visitor) const { - if (level_ == ResourceType_Instance) + if (level_ == ArchiveResourceType_Instance) { for (std::list::const_iterator it = instances_.begin(); it != instances_.end(); ++it) @@ -823,25 +908,29 @@ snprintf(instanceFormat_, sizeof(instanceFormat_) - 1, "%%08d.dcm"); } - virtual void Open(ResourceType level, + virtual void Open(ArchiveResourceType level, const std::string& publicId) ORTHANC_OVERRIDE { std::string path; DicomMap tags; - if (context_.GetIndex().GetMainDicomTags(tags, publicId, level, level)) + ResourceType resourceIdLevel = GetResourceIdType(level); + ResourceType interestLevel = (level == ArchiveResourceType_PatientInfoFromStudy ? ResourceType_Patient : resourceIdLevel); + + if (context_.GetIndex().GetMainDicomTags(tags, publicId, resourceIdLevel, interestLevel)) { switch (level) { - case ResourceType_Patient: + case ArchiveResourceType_Patient: + case ArchiveResourceType_PatientInfoFromStudy: path = GetTag(tags, DICOM_TAG_PATIENT_ID) + " " + GetTag(tags, DICOM_TAG_PATIENT_NAME); break; - case ResourceType_Study: + case ArchiveResourceType_Study: path = GetTag(tags, DICOM_TAG_ACCESSION_NUMBER) + " " + GetTag(tags, DICOM_TAG_STUDY_DESCRIPTION); break; - case ResourceType_Series: + case ArchiveResourceType_Series: { std::string modality = GetTag(tags, DICOM_TAG_MODALITY); path = modality + " " + GetTag(tags, DICOM_TAG_SERIES_DESCRIPTION); @@ -875,7 +964,7 @@ if (path.empty()) { - path = std::string("Unknown ") + EnumerationToString(level); + path = std::string("Unknown ") + EnumerationToString(GetResourceLevel(level)); } commands_.AddOpenDirectory(path.c_str()); @@ -911,7 +1000,7 @@ { } - virtual void Open(ResourceType level, + virtual void Open(ArchiveResourceType level, const std::string& publicId) ORTHANC_OVERRIDE { } @@ -1102,9 +1191,10 @@ ArchiveJob::ArchiveJob(ServerContext& context, bool isMedia, - bool enableExtendedSopClass) : + bool enableExtendedSopClass, + ResourceType jobLevel) : context_(context), - archive_(new ArchiveIndex(ResourceType_Patient)), // root + archive_(new ArchiveIndex(GetArchiveResourceType(jobLevel))), // get patient Info from this level isMedia_(isMedia), enableExtendedSopClass_(enableExtendedSopClass), currentStep_(0), diff -r 1c6708a0e0c6 -r 08177310e269 OrthancServer/Sources/ServerJobs/ArchiveJob.h --- a/OrthancServer/Sources/ServerJobs/ArchiveJob.h Wed Nov 08 09:59:13 2023 +0100 +++ b/OrthancServer/Sources/ServerJobs/ArchiveJob.h Wed Nov 08 09:59:31 2023 +0100 @@ -76,7 +76,8 @@ public: ArchiveJob(ServerContext& context, bool isMedia, - bool enableExtendedSopClass); + bool enableExtendedSopClass, + ResourceType jobLevel); virtual ~ArchiveJob(); diff -r 1c6708a0e0c6 -r 08177310e269 OrthancServer/UnitTestsSources/ServerJobsTests.cpp --- a/OrthancServer/UnitTestsSources/ServerJobsTests.cpp Wed Nov 08 09:59:13 2023 +0100 +++ b/OrthancServer/UnitTestsSources/ServerJobsTests.cpp Wed Nov 08 09:59:31 2023 +0100 @@ -750,7 +750,7 @@ // ArchiveJob { - ArchiveJob job(GetContext(), false, false); + ArchiveJob job(GetContext(), false, false, ResourceType_Patient); ASSERT_FALSE(job.Serialize(s)); // Cannot serialize this } diff -r 1c6708a0e0c6 -r 08177310e269 TODO --- a/TODO Wed Nov 08 09:59:13 2023 +0100 +++ b/TODO Wed Nov 08 09:59:31 2023 +0100 @@ -41,6 +41,19 @@ https://discourse.orthanc-server.org/t/performance-issue-when-adding-a-lot-of-jobs-in-the-queue/3915/2 Note: that might also be the right time to have a central jobs registry when working with multiple Orthanc instances on the same DB. +* Right now, some Stable events never occurs (e.g. when Orthanc is restarted before the event is triggered). + Since these events are used to e.g. generate dicom-web cache (or update it !), we should try + to make sure these events always happen. + - Generate the events when setting IsStable=true when starting an Orthanc (ok for SQLite) ? + - Also consider the use case of an Orthanc cluster that is being scaled-down just after one Orthanc instance + has received a few instances -> we can not only check for missing stable events at startup since no Orthanc will start. + We would need to maintain the list of "unstable" resources in DB instead of memory only. +* In prometheus metrics, implement Histograms or Exponential Histograms to measure durations. Right now, we only provide + "average" durations that are not very relevant + (https://opentelemetry.io/docs/specs/otel/metrics/data-model/#histogram) + - for job durations (+ have one histogram for each job) + - for HTTP request handling + - ... ============================ Documentation (Orthanc Book) @@ -131,7 +144,6 @@ https://discourse.orthanc-server.org/t/orthanc-convert-ybr-to-rgb-but-does-not-change-metadata/3533/9 * Implement a 'commit' route to force the Stable status earlier. https://discourse.orthanc-server.org/t/expediting-stability-of-a-dicom-study-new-api-endpoint/1684 - --------- Long-term @@ -216,7 +228,8 @@ to Orthanc and/or the Storage plugin (instead of passing a memory buffer) -> the object-storage plugin could "stream" the file to the storage. The HTTP server could also "stream" its response from file handles. Transcoding should be "file based" too. - +* To investigate: usage of mapped_file (not only in the indexer plugin): + https://discourse.orthanc-server.org/t/patch-for-orthanc-indexer-plugin-crashing-on-big-non-dicom-files/3849/7 ======== Database