# HG changeset patch # User Alain Mazy # Date 1680553892 -7200 # Node ID 5874e5dd9a3832decf70fedea43c8f2cd03cd84a # Parent feba2b0e91bc93dccf7f0b5b9b2188dfb2a0b6d1# Parent 802e2e3d9bfc63b3fafeeb92ff471d3b8b3ad9c4 merge diff -r 802e2e3d9bfc -r 5874e5dd9a38 NEWS --- a/NEWS Mon Apr 03 17:00:51 2023 +0200 +++ b/NEWS Mon Apr 03 22:31:32 2023 +0200 @@ -21,6 +21,9 @@ to allow "/instances/../export" that is now disabled by default * Fix issue 214: VOILUTSequence is not returned in Wado-RS * Fix /tools/reset crashing when ExtraMainDicomTags were defined +* Fix Housekeeper plugin infinite loop if Orthanc is empty. +* Fix a crash in /tools/reconstruct triggered by the Housekeeper plugin + when only changing the StorageCompression. Version 1.11.3 (2023-02-03) diff -r 802e2e3d9bfc -r 5874e5dd9a38 OrthancServer/Plugins/Samples/Housekeeper/Plugin.cpp --- a/OrthancServer/Plugins/Samples/Housekeeper/Plugin.cpp Mon Apr 03 17:00:51 2023 +0200 +++ b/OrthancServer/Plugins/Samples/Housekeeper/Plugin.cpp Mon Apr 03 22:31:32 2023 +0200 @@ -488,36 +488,51 @@ OrthancPlugins::RestApiGet(changes, "/changes?since=" + boost::lexical_cast(pluginStatus_.lastProcessedChange) + "&limit=100", false); } - for (Json::ArrayIndex i = 0; i < changes["Changes"].size(); i++) + if (changes["Changes"].size() > 0) { - const Json::Value& change = changes["Changes"][i]; - int64_t seq = change["Seq"].asInt64(); - - if (change["ChangeType"] == "NewStudy") // some StableStudy might be missing if orthanc was shutdown during a StableAge -> consider only the NewStudy events that can not be missed + for (Json::ArrayIndex i = 0; i < changes["Changes"].size(); i++) { - Json::Value result; - Json::Value request; - if (needsReingest) + const Json::Value& change = changes["Changes"][i]; + int64_t seq = change["Seq"].asInt64(); + + if (change["ChangeType"] == "NewStudy") // some StableStudy might be missing if orthanc was shutdown during a StableAge -> consider only the NewStudy events that can not be missed + { + Json::Value result; + Json::Value request; + if (needsReingest) + { + request["ReconstructFiles"] = true; + } + OrthancPlugins::RestApiPost(result, "/studies/" + change["ID"].asString() + "/reconstruct", request, false); + } + { - request["ReconstructFiles"] = true; + boost::recursive_mutex::scoped_lock lock(pluginStatusMutex_); + + pluginStatus_.lastProcessedChange = seq; + + if (seq >= pluginStatus_.lastChangeToProcess) // we are done ! + { + return true; + } } - OrthancPlugins::RestApiPost(result, "/studies/" + change["ID"].asString() + "/reconstruct", request, false); + + if (change["ChangeType"] == "NewStudy") + { + boost::this_thread::sleep(boost::posix_time::milliseconds(throttleDelay_ * 1000)); + } } - + } + else + { + // if the change list is empty and Done is true, it means that there is nothing to process anymore + if (changes["Done"].asBool()) { boost::recursive_mutex::scoped_lock lock(pluginStatusMutex_); - pluginStatus_.lastProcessedChange = seq; + pluginStatus_.lastProcessedChange = changes["Last"].asInt64(); - if (seq >= pluginStatus_.lastChangeToProcess) // we are done ! - { - return true; - } - } - - if (change["ChangeType"] == "NewStudy") - { - boost::this_thread::sleep(boost::posix_time::milliseconds(throttleDelay_ * 1000)); + return true; } } diff -r 802e2e3d9bfc -r 5874e5dd9a38 OrthancServer/Sources/Database/StatelessDatabaseOperations.cpp --- a/OrthancServer/Sources/Database/StatelessDatabaseOperations.cpp Mon Apr 03 17:00:51 2023 +0200 +++ b/OrthancServer/Sources/Database/StatelessDatabaseOperations.cpp Mon Apr 03 22:31:32 2023 +0200 @@ -2956,6 +2956,10 @@ MetadataType metadata, const std::string& value) { + if (metadata == 15) + { + LOG(INFO) << "toto"; + } content.AddMetadata(instance, metadata, value); instanceMetadata[metadata] = value; } @@ -3081,13 +3085,21 @@ { IDatabaseWrapper::CreateInstanceResult status; int64_t instanceId; + + bool isNewInstance = transaction.CreateInstance(status, instanceId, hashPatient_, + hashStudy_, hashSeries_, hashInstance_); + + if (isReconstruct_ && isNewInstance) + { + // In case of reconstruct, we just want to modify the attachments and some metadata like the TransferSyntex + // The DicomTags and many metadata have already been updated before we get here in ReconstructInstance + throw OrthancException(ErrorCode_InternalError, "New instance while reconstructing; this should not happen."); + } // Check whether this instance is already stored - if (!transaction.CreateInstance(status, instanceId, hashPatient_, - hashStudy_, hashSeries_, hashInstance_)) + if (!isNewInstance && !isReconstruct_) { // The instance already exists - if (overwrite_) { // Overwrite the old instance @@ -3098,7 +3110,7 @@ if (!transaction.CreateInstance(status, instanceId, hashPatient_, hashStudy_, hashSeries_, hashInstance_)) { - throw OrthancException(ErrorCode_InternalError); + throw OrthancException(ErrorCode_InternalError, "No new instance while overwriting; this should not happen."); } } else @@ -3173,14 +3185,19 @@ for (Attachments::const_iterator it = attachments_.begin(); it != attachments_.end(); ++it) { + if (isReconstruct_) + { + // we are replacing attachments during a reconstruction + transaction.DeleteAttachment(instanceId, it->GetContentType()); + } + transaction.AddAttachment(instanceId, *it, 0 /* this is the first revision */); } - + if (!isReconstruct_) { ResourcesContent content(true /* new resource, metadata can be set */); - // Attach the user-specified metadata (in case of reconstruction, metadata_ contains all past metadata, including the system ones we want to keep) for (MetadataMap::const_iterator it = metadata_.begin(); it != metadata_.end(); ++it) @@ -3209,62 +3226,52 @@ } } - // Populate the tags of the newly-created resources - - content.AddResource(instanceId, ResourceType_Instance, dicomSummary_); - SetInstanceMetadata(content, instanceMetadata_, instanceId, MetadataType_MainDicomTagsSignature, DicomMap::GetMainDicomTagsSignature(ResourceType_Instance)); // New in Orthanc 1.11.0 - SetMainDicomSequenceMetadata(content, instanceId, dicomSummary_, ResourceType_Instance); // new in Orthanc 1.11.1 - - if (status.isNewSeries_) - { - content.AddResource(status.seriesId_, ResourceType_Series, dicomSummary_); - content.AddMetadata(status.seriesId_, MetadataType_MainDicomTagsSignature, DicomMap::GetMainDicomTagsSignature(ResourceType_Series)); // New in Orthanc 1.11.0 - SetMainDicomSequenceMetadata(content, status.seriesId_, dicomSummary_, ResourceType_Series); // new in Orthanc 1.11.1 - } - - if (status.isNewStudy_) - { - content.AddResource(status.studyId_, ResourceType_Study, dicomSummary_); - content.AddMetadata(status.studyId_, MetadataType_MainDicomTagsSignature, DicomMap::GetMainDicomTagsSignature(ResourceType_Study)); // New in Orthanc 1.11.0 - SetMainDicomSequenceMetadata(content, status.studyId_, dicomSummary_, ResourceType_Study); // new in Orthanc 1.11.1 - } - - if (status.isNewPatient_) + if (!isReconstruct_) { - content.AddResource(status.patientId_, ResourceType_Patient, dicomSummary_); - content.AddMetadata(status.patientId_, MetadataType_MainDicomTagsSignature, DicomMap::GetMainDicomTagsSignature(ResourceType_Patient)); // New in Orthanc 1.11.0 - SetMainDicomSequenceMetadata(content, status.patientId_, dicomSummary_, ResourceType_Patient); // new in Orthanc 1.11.1 - } - - // Attach the auto-computed metadata for the patient/study/series levels - std::string now = SystemToolbox::GetNowIsoString(true /* use UTC time (not local time) */); - content.AddMetadata(status.seriesId_, MetadataType_LastUpdate, now); - content.AddMetadata(status.studyId_, MetadataType_LastUpdate, now); - content.AddMetadata(status.patientId_, MetadataType_LastUpdate, now); - - if (status.isNewSeries_) - { - if (hasExpectedInstances_) + // Populate the tags of the newly-created resources + content.AddResource(instanceId, ResourceType_Instance, dicomSummary_); + SetInstanceMetadata(content, instanceMetadata_, instanceId, MetadataType_MainDicomTagsSignature, DicomMap::GetMainDicomTagsSignature(ResourceType_Instance)); // New in Orthanc 1.11.0 + SetMainDicomSequenceMetadata(content, instanceId, dicomSummary_, ResourceType_Instance); // new in Orthanc 1.11.1 + + if (status.isNewSeries_) + { + content.AddResource(status.seriesId_, ResourceType_Series, dicomSummary_); + content.AddMetadata(status.seriesId_, MetadataType_MainDicomTagsSignature, DicomMap::GetMainDicomTagsSignature(ResourceType_Series)); // New in Orthanc 1.11.0 + SetMainDicomSequenceMetadata(content, status.seriesId_, dicomSummary_, ResourceType_Series); // new in Orthanc 1.11.1 + } + + if (status.isNewStudy_) + { + content.AddResource(status.studyId_, ResourceType_Study, dicomSummary_); + content.AddMetadata(status.studyId_, MetadataType_MainDicomTagsSignature, DicomMap::GetMainDicomTagsSignature(ResourceType_Study)); // New in Orthanc 1.11.0 + SetMainDicomSequenceMetadata(content, status.studyId_, dicomSummary_, ResourceType_Study); // new in Orthanc 1.11.1 + } + + if (status.isNewPatient_) { - content.AddMetadata(status.seriesId_, MetadataType_Series_ExpectedNumberOfInstances, - boost::lexical_cast(expectedInstances_)); + content.AddResource(status.patientId_, ResourceType_Patient, dicomSummary_); + content.AddMetadata(status.patientId_, MetadataType_MainDicomTagsSignature, DicomMap::GetMainDicomTagsSignature(ResourceType_Patient)); // New in Orthanc 1.11.0 + SetMainDicomSequenceMetadata(content, status.patientId_, dicomSummary_, ResourceType_Patient); // new in Orthanc 1.11.1 } - // New in Orthanc 1.9.0 - content.AddMetadata(status.seriesId_, MetadataType_RemoteAet, - origin_.GetRemoteAetC()); - } - - if (hasTransferSyntax_) - { - // New in Orthanc 1.2.0 - SetInstanceMetadata(content, instanceMetadata_, instanceId, - MetadataType_Instance_TransferSyntax, - GetTransferSyntaxUid(transferSyntax_)); - } - - if (!isReconstruct_) // don't change origin metadata - { + // Attach the auto-computed metadata for the patient/study/series levels + std::string now = SystemToolbox::GetNowIsoString(true /* use UTC time (not local time) */); + content.AddMetadata(status.seriesId_, MetadataType_LastUpdate, now); + content.AddMetadata(status.studyId_, MetadataType_LastUpdate, now); + content.AddMetadata(status.patientId_, MetadataType_LastUpdate, now); + + if (status.isNewSeries_) + { + if (hasExpectedInstances_) + { + content.AddMetadata(status.seriesId_, MetadataType_Series_ExpectedNumberOfInstances, + boost::lexical_cast(expectedInstances_)); + } + + // New in Orthanc 1.9.0 + content.AddMetadata(status.seriesId_, MetadataType_RemoteAet, + origin_.GetRemoteAetC()); + } // Attach the auto-computed metadata for the instance level, // reflecting these additions into the input metadata map SetInstanceMetadata(content, instanceMetadata_, instanceId, @@ -3298,6 +3305,17 @@ } } + // Following metadatas are also updated if reconstructing the instance. + // They might be missing since they have been introduced along Orthanc versions. + + if (hasTransferSyntax_) + { + // New in Orthanc 1.2.0 + SetInstanceMetadata(content, instanceMetadata_, instanceId, + MetadataType_Instance_TransferSyntax, + GetTransferSyntaxUid(transferSyntax_)); + } + if (hasPixelDataOffset_) { // New in Orthanc 1.9.1 diff -r 802e2e3d9bfc -r 5874e5dd9a38 OrthancServer/Sources/ServerToolbox.cpp --- a/OrthancServer/Sources/ServerToolbox.cpp Mon Apr 03 17:00:51 2023 +0200 +++ b/OrthancServer/Sources/ServerToolbox.cpp Mon Apr 03 22:31:32 2023 +0200 @@ -281,14 +281,8 @@ std::string resultPublicId; // ignored std::unique_ptr dicomInstancetoStore(DicomInstanceToStore::CreateFromParsedDicomFile(locker.GetDicom())); - context.GetIndex().GetAllMetadata(instanceMetadata, *it, ResourceType_Instance); - - for (InstanceMetadata::const_iterator itm = instanceMetadata.begin(); - itm != instanceMetadata.end(); ++itm) - { - dicomInstancetoStore->AddMetadata(ResourceType_Instance, itm->first, itm->second); - } - + // TODO: TranscodeAndStore and specifically ServerIndex::Store have been "poluted" by the isReconstruct parameter + // we should very likely refactor it context.TranscodeAndStore(resultPublicId, dicomInstancetoStore.get(), StoreInstanceMode_OverwriteDuplicate, true); } }