Mercurial > hg > orthanc
changeset 4623:95ffe3b6ef7c db-changes
handling of revisions for metadata
line wrap: on
line diff
--- a/NEWS Fri Apr 16 10:48:57 2021 +0200 +++ b/NEWS Fri Apr 16 17:13:03 2021 +0200 @@ -1,23 +1,26 @@ Pending changes in the mainline =============================== +General +------- + +* New configuration options related to multiple readers/writers: + - "DatabaseServerIdentifier" identifies the server in the DB among a pool of Orthanc servers + - "CheckRevisions" to protect against concurrent modifications of metadata and attachments + +REST API +-------- + +* API version upgraded to 12 +* "/.../{id}/metadata/{name}" URIs handle the HTTP headers "If-Match", "If-None-Match" and + "ETag" to cope with revisions + Plugins ------- -* Possibility to create database index plugins that don't lock a global mutex -* New option "DatabaseServerIdentifier" to identify the server among a pool of Orthanc servers - -Maintenance ------------ - -* Full refactoring of the database engine - - -Plugins -------- - -* New functions in the SDK: - - OrthancPluginCallRestApi() +* New function in the SDK: OrthancPluginCallRestApi() +* Full refactoring of the database engine to handle multiple readers/writers, which notably + implies the handling of retries in the case of collisions Maintenance -----------
--- a/OrthancFramework/Resources/CodeGeneration/ErrorCodes.json Fri Apr 16 10:48:57 2021 +0200 +++ b/OrthancFramework/Resources/CodeGeneration/ErrorCodes.json Fri Apr 16 17:13:03 2021 +0200 @@ -243,7 +243,7 @@ "Code": 43, "HttpStatus": 409, "Name": "Revision", - "Description": "A bad revision number was provided, indicates conflict between multiple updates" + "Description": "A bad revision number was provided, which might indicate conflict between multiple writers" },
--- a/OrthancFramework/Sources/Enumerations.cpp Fri Apr 16 10:48:57 2021 +0200 +++ b/OrthancFramework/Sources/Enumerations.cpp Fri Apr 16 17:13:03 2021 +0200 @@ -191,7 +191,7 @@ return "Database could not serialize access due to concurrent update, the transaction should be retried"; case ErrorCode_Revision: - return "A bad revision number was provided, indicates conflict between multiple updates"; + return "A bad revision number was provided, which might indicate conflict between multiple writers"; case ErrorCode_SQLiteNotOpened: return "SQLite: The database is not opened";
--- a/OrthancFramework/Sources/Enumerations.h Fri Apr 16 10:48:57 2021 +0200 +++ b/OrthancFramework/Sources/Enumerations.h Fri Apr 16 17:13:03 2021 +0200 @@ -136,7 +136,7 @@ ErrorCode_DiscontinuedAbi = 40 /*!< Calling a function that has been removed from the Orthanc Framework */, ErrorCode_BadRange = 41 /*!< Incorrect range request */, ErrorCode_DatabaseCannotSerialize = 42 /*!< Database could not serialize access due to concurrent update, the transaction should be retried */, - ErrorCode_Revision = 43 /*!< A bad revision number was provided, indicates conflict between multiple updates */, + ErrorCode_Revision = 43 /*!< A bad revision number was provided, which might indicate conflict between multiple writers */, ErrorCode_SQLiteNotOpened = 1000 /*!< SQLite: The database is not opened */, ErrorCode_SQLiteAlreadyOpened = 1001 /*!< SQLite: Connection is already open */, ErrorCode_SQLiteCannotOpen = 1002 /*!< SQLite: Unable to open the database */,
--- a/OrthancServer/Plugins/Engine/OrthancPluginDatabase.cpp Fri Apr 16 10:48:57 2021 +0200 +++ b/OrthancServer/Plugins/Engine/OrthancPluginDatabase.cpp Fri Apr 16 17:13:03 2021 +0200 @@ -772,7 +772,8 @@ MetadataType type = static_cast<MetadataType>(*it); std::string value; - if (LookupMetadata(value, id, type)) + int64_t revision; // Ignored + if (LookupMetadata(value, revision, id, type)) { target[type] = value; } @@ -1174,11 +1175,13 @@ virtual bool LookupMetadata(std::string& target, + int64_t& revision, int64_t id, MetadataType type) ORTHANC_OVERRIDE { ResetAnswers(); CheckSuccess(that_.backend_.lookupMetadata(that_.GetContext(), that_.payload_, id, static_cast<int32_t>(type))); + revision = 0; // Dummy value, as revisions were added in Orthanc 1.9.2 return ForwardSingleAnswer(target); } @@ -1336,8 +1339,10 @@ virtual void SetMetadata(int64_t id, MetadataType type, - const std::string& value) ORTHANC_OVERRIDE + const std::string& value, + int64_t revision) ORTHANC_OVERRIDE { + // "revision" is not used, as it was added in Orthanc 1.9.2 CheckSuccess(that_.backend_.setMetadata (that_.payload_, id, static_cast<int32_t>(type), value.c_str())); }
--- a/OrthancServer/Plugins/Engine/OrthancPluginDatabaseV3.cpp Fri Apr 16 10:48:57 2021 +0200 +++ b/OrthancServer/Plugins/Engine/OrthancPluginDatabaseV3.cpp Fri Apr 16 17:13:03 2021 +0200 @@ -706,10 +706,11 @@ virtual bool LookupMetadata(std::string& target, + int64_t& revision, int64_t id, MetadataType type) ORTHANC_OVERRIDE { - CheckSuccess(that_.backend_.lookupMetadata(transaction_, id, static_cast<int32_t>(type))); + CheckSuccess(that_.backend_.lookupMetadata(transaction_, &revision, id, static_cast<int32_t>(type))); CheckNoEvent(); return ReadSingleStringAnswer(target); } @@ -785,9 +786,10 @@ virtual void SetMetadata(int64_t id, MetadataType type, - const std::string& value) ORTHANC_OVERRIDE + const std::string& value, + int64_t revision) ORTHANC_OVERRIDE { - CheckSuccess(that_.backend_.setMetadata(transaction_, id, static_cast<int32_t>(type), value.c_str())); + CheckSuccess(that_.backend_.setMetadata(transaction_, id, static_cast<int32_t>(type), value.c_str(), revision)); CheckNoEvent(); }
--- a/OrthancServer/Plugins/Include/orthanc/OrthancCDatabasePlugin.h Fri Apr 16 10:48:57 2021 +0200 +++ b/OrthancServer/Plugins/Include/orthanc/OrthancCDatabasePlugin.h Fri Apr 16 17:13:03 2021 +0200 @@ -1253,6 +1253,7 @@ /* Answer is read using "readAnswerString()" */ OrthancPluginErrorCode (*lookupMetadata) (OrthancPluginDatabaseTransaction* transaction, + int64_t* revision /* out */, int64_t id, int32_t metadata); @@ -1299,7 +1300,8 @@ OrthancPluginErrorCode (*setMetadata) (OrthancPluginDatabaseTransaction* transaction, int64_t id, int32_t metadata, - const char* value); + const char* value, + int64_t revision); OrthancPluginErrorCode (*setProtectedPatient) (OrthancPluginDatabaseTransaction* transaction, int64_t id,
--- a/OrthancServer/Plugins/Include/orthanc/OrthancCPlugin.h Fri Apr 16 10:48:57 2021 +0200 +++ b/OrthancServer/Plugins/Include/orthanc/OrthancCPlugin.h Fri Apr 16 17:13:03 2021 +0200 @@ -240,7 +240,7 @@ OrthancPluginErrorCode_DiscontinuedAbi = 40 /*!< Calling a function that has been removed from the Orthanc Framework */, OrthancPluginErrorCode_BadRange = 41 /*!< Incorrect range request */, OrthancPluginErrorCode_DatabaseCannotSerialize = 42 /*!< Database could not serialize access due to concurrent update, the transaction should be retried */, - OrthancPluginErrorCode_Revision = 43 /*!< A bad revision number was provided, indicates conflict between multiple updates */, + OrthancPluginErrorCode_Revision = 43 /*!< A bad revision number was provided, which might indicate conflict between multiple writers */, OrthancPluginErrorCode_SQLiteNotOpened = 1000 /*!< SQLite: The database is not opened */, OrthancPluginErrorCode_SQLiteAlreadyOpened = 1001 /*!< SQLite: Connection is already open */, OrthancPluginErrorCode_SQLiteCannotOpen = 1002 /*!< SQLite: Unable to open the database */,
--- a/OrthancServer/Resources/Configuration.json Fri Apr 16 10:48:57 2021 +0200 +++ b/OrthancServer/Resources/Configuration.json Fri Apr 16 17:13:03 2021 +0200 @@ -768,16 +768,27 @@ // network protocol, expressed in bytes. This value affects both // Orthanc SCU and Orthanc SCP. It defaults to 16KB. The allowed // range is [4096,131072]. (new in Orthanc 1.9.0) - "MaximumPduLength" : 16384 + "MaximumPduLength" : 16384, // Arbitrary identifier of this Orthanc server when storing its - // global properties if a custom database plugin is used. This - // option is only useful in the case of multiple readers/writers, in - // order to avoid collisions between multiple Orthanc servers. If - // unset, this identifier is taken as a SHA-1 hash derived from the - // MAC adddresses of the network interfaces, and from the AET and - // TCP ports used by Orthanc. (new in Orthanc 1.9.2) + // global properties if a custom index plugin is used. This + // identifier is only useful in the case of multiple + // readers/writers, in order to avoid collisions between multiple + // Orthanc servers. If unset, this identifier is taken as a SHA-1 + // hash derived from the MAC adddresses of the network interfaces, + // and from the AET and TCP ports used by Orthanc. Manually setting + // this option is needed in Docker/Kubernetes environments. (new in + // Orthanc 1.9.2) /** - , "DatabaseServerIdentifier" : "Orthanc1" + "DatabaseServerIdentifier" : "Orthanc1", **/ + + // Whether Orthanc protects the modification of metadata and + // attachments using revisions, which is done using the HTTP headers + // "ETag", "If-Match" and "If-None-Match" in the calls to the REST + // API. This is needed to handle collisions between concurrent + // modifications in the case of multiple writers. The database + // back-end must support this option, which is notably *not* yet the + // case of the built-in SQLite index. (new in Orthanc 1.9.2) + "CheckRevisions" : false }
--- a/OrthancServer/Sources/Database/Compatibility/IGetChildrenMetadata.cpp Fri Apr 16 10:48:57 2021 +0200 +++ b/OrthancServer/Sources/Database/Compatibility/IGetChildrenMetadata.cpp Fri Apr 16 17:13:03 2021 +0200 @@ -56,7 +56,8 @@ it = children.begin(); it != children.end(); ++it) { std::string value; - if (database.LookupMetadata(value, *it, metadata)) + int64_t revision; // Ignored + if (database.LookupMetadata(value, revision, *it, metadata)) { target.push_back(value); }
--- a/OrthancServer/Sources/Database/Compatibility/IGetChildrenMetadata.h Fri Apr 16 10:48:57 2021 +0200 +++ b/OrthancServer/Sources/Database/Compatibility/IGetChildrenMetadata.h Fri Apr 16 17:13:03 2021 +0200 @@ -49,6 +49,7 @@ int64_t id) = 0; virtual bool LookupMetadata(std::string& target, + int64_t& revision, int64_t id, MetadataType type) = 0;
--- a/OrthancServer/Sources/Database/Compatibility/ISetResourcesContent.h Fri Apr 16 10:48:57 2021 +0200 +++ b/OrthancServer/Sources/Database/Compatibility/ISetResourcesContent.h Fri Apr 16 17:13:03 2021 +0200 @@ -56,7 +56,8 @@ virtual void SetMetadata(int64_t id, MetadataType type, - const std::string& value) = 0; + const std::string& value, + int64_t revision) = 0; static void Apply(ISetResourcesContent& that, const ResourcesContent& content)
--- a/OrthancServer/Sources/Database/IDatabaseWrapper.h Fri Apr 16 10:48:57 2021 +0200 +++ b/OrthancServer/Sources/Database/IDatabaseWrapper.h Fri Apr 16 17:13:03 2021 +0200 @@ -36,8 +36,6 @@ #include "../../../OrthancFramework/Sources/DicomFormat/DicomMap.h" #include "../../../OrthancFramework/Sources/FileStorage/FileInfo.h" #include "../../../OrthancFramework/Sources/FileStorage/IStorageArea.h" -#include "../../../OrthancFramework/Sources/SQLite/ITransaction.h" - #include "../ExportedResource.h" #include "../ServerIndexChange.h" #include "IDatabaseListener.h" @@ -166,6 +164,7 @@ bool shared) = 0; virtual bool LookupMetadata(std::string& target, + int64_t& revision, int64_t id, MetadataType type) = 0; @@ -189,7 +188,8 @@ virtual void SetMetadata(int64_t id, MetadataType type, - const std::string& value) = 0; + const std::string& value, + int64_t revision) = 0; virtual void SetProtectedPatient(int64_t internalId, bool isProtected) = 0;
--- a/OrthancServer/Sources/Database/ResourcesContent.cpp Fri Apr 16 10:48:57 2021 +0200 +++ b/OrthancServer/Sources/Database/ResourcesContent.cpp Fri Apr 16 17:13:03 2021 +0200 @@ -35,12 +35,119 @@ #include "ResourcesContent.h" #include "Compatibility/ISetResourcesContent.h" +#include "../ServerToolbox.h" + +#include "../../../OrthancFramework/Sources/DicomFormat/DicomArray.h" +#include "../../../OrthancFramework/Sources/OrthancException.h" #include <cassert> namespace Orthanc { + static void StoreMainDicomTagsInternal(ResourcesContent& target, + int64_t resource, + const DicomMap& tags) + { + DicomArray flattened(tags); + + for (size_t i = 0; i < flattened.GetSize(); i++) + { + const DicomElement& element = flattened.GetElement(i); + const DicomTag& tag = element.GetTag(); + const DicomValue& value = element.GetValue(); + if (!value.IsNull() && + !value.IsBinary()) + { + target.AddMainDicomTag(resource, tag, element.GetValue().GetContent()); + } + } + } + + + static void StoreIdentifiers(ResourcesContent& target, + int64_t resource, + ResourceType level, + const DicomMap& map) + { + const DicomTag* tags; + size_t size; + + ServerToolbox::LoadIdentifiers(tags, size, level); + + for (size_t i = 0; i < size; i++) + { + // The identifiers tags are a subset of the main DICOM tags + assert(DicomMap::IsMainDicomTag(tags[i])); + + const DicomValue* value = map.TestAndGetValue(tags[i]); + if (value != NULL && + !value->IsNull() && + !value->IsBinary()) + { + std::string s = ServerToolbox::NormalizeIdentifier(value->GetContent()); + target.AddIdentifierTag(resource, tags[i], s); + } + } + } + + + void ResourcesContent::AddMetadata(int64_t resourceId, + MetadataType metadata, + const std::string& value) + { + if (isNewResource_) + { + metadata_.push_back(Metadata(resourceId, metadata, value)); + } + else + { + // This would require to handle the incrementation of revision + // numbers in the database backend => only allow setting + // metadata on new resources + throw OrthancException(ErrorCode_NotImplemented); + } + } + + + void ResourcesContent::AddResource(int64_t resource, + ResourceType level, + const DicomMap& dicomSummary) + { + StoreIdentifiers(*this, resource, level, dicomSummary); + + DicomMap tags; + + switch (level) + { + case ResourceType_Patient: + dicomSummary.ExtractPatientInformation(tags); + break; + + case ResourceType_Study: + // Duplicate the patient tags at the study level (new in Orthanc 0.9.5 - db v6) + dicomSummary.ExtractPatientInformation(tags); + StoreMainDicomTagsInternal(*this, resource, tags); + + dicomSummary.ExtractStudyInformation(tags); + break; + + case ResourceType_Series: + dicomSummary.ExtractSeriesInformation(tags); + break; + + case ResourceType_Instance: + dicomSummary.ExtractInstanceInformation(tags); + break; + + default: + throw OrthancException(ErrorCode_InternalError); + } + + StoreMainDicomTagsInternal(*this, resource, tags); + } + + void ResourcesContent::Store(Compatibility::ISetResourcesContent& compatibility) const { for (std::list<TagValue>::const_iterator @@ -59,7 +166,8 @@ for (std::list<Metadata>::const_iterator it = metadata_.begin(); it != metadata_.end(); ++it) { - compatibility.SetMetadata(it->resourceId_, it->metadata_, it->value_); + assert(isNewResource_); + compatibility.SetMetadata(it->resourceId_, it->metadata_, it->value_, 0 /* initial revision number */); } } }
--- a/OrthancServer/Sources/Database/ResourcesContent.h Fri Apr 16 10:48:57 2021 +0200 +++ b/OrthancServer/Sources/Database/ResourcesContent.h Fri Apr 16 17:13:03 2021 +0200 @@ -89,10 +89,16 @@ typedef std::list<Metadata> ListMetadata; private: + bool isNewResource_; ListTags tags_; ListMetadata metadata_; public: + explicit ResourcesContent(bool isNewResource) : + isNewResource_(isNewResource) + { + } + void AddMainDicomTag(int64_t resourceId, const DicomTag& tag, const std::string& value) @@ -109,10 +115,7 @@ void AddMetadata(int64_t resourceId, MetadataType metadata, - const std::string& value) - { - metadata_.push_back(Metadata(resourceId, metadata, value)); - } + const std::string& value); void AddResource(int64_t resource, ResourceType level,
--- a/OrthancServer/Sources/Database/SQLiteDatabaseWrapper.cpp Fri Apr 16 10:48:57 2021 +0200 +++ b/OrthancServer/Sources/Database/SQLiteDatabaseWrapper.cpp Fri Apr 16 17:13:03 2021 +0200 @@ -850,6 +850,7 @@ virtual bool LookupMetadata(std::string& target, + int64_t& revision, int64_t id, MetadataType type) ORTHANC_OVERRIDE { @@ -865,6 +866,7 @@ else { target = s.ColumnString(0); + revision = 0; // TODO - REVISIONS return true; } } @@ -1033,8 +1035,10 @@ virtual void SetMetadata(int64_t id, MetadataType type, - const std::string& value) ORTHANC_OVERRIDE + const std::string& value, + int64_t revision) ORTHANC_OVERRIDE { + // TODO - REVISIONS SQLite::Statement s(db_, SQLITE_FROM_HERE, "INSERT OR REPLACE INTO Metadata VALUES(?, ?, ?)"); s.BindInt64(0, id); s.BindInt(1, type);
--- a/OrthancServer/Sources/Database/StatelessDatabaseOperations.cpp Fri Apr 16 10:48:57 2021 +0200 +++ b/OrthancServer/Sources/Database/StatelessDatabaseOperations.cpp Fri Apr 16 17:13:03 2021 +0200 @@ -1367,33 +1367,35 @@ bool StatelessDatabaseOperations::LookupMetadata(std::string& target, + int64_t& revision, const std::string& publicId, ResourceType expectedType, MetadataType type) { - class Operations : public ReadOnlyOperationsT5<bool&, std::string&, const std::string&, ResourceType, MetadataType> + class Operations : public ReadOnlyOperationsT6<bool&, std::string&, int64_t&, + const std::string&, ResourceType, MetadataType> { public: virtual void ApplyTuple(ReadOnlyTransaction& transaction, const Tuple& tuple) ORTHANC_OVERRIDE { - ResourceType rtype; + ResourceType resourceType; int64_t id; - if (!transaction.LookupResource(id, rtype, tuple.get<2>()) || - rtype != tuple.get<3>()) + if (!transaction.LookupResource(id, resourceType, tuple.get<3>()) || + resourceType != tuple.get<4>()) { throw OrthancException(ErrorCode_UnknownResource); } else { - tuple.get<0>() = transaction.LookupMetadata(tuple.get<1>(), id, tuple.get<4>()); + tuple.get<0>() = transaction.LookupMetadata(tuple.get<1>(), tuple.get<2>(), id, tuple.get<5>()); } } }; bool found; Operations operations; - operations.Apply(*this, found, target, publicId, expectedType, type); + operations.Apply(*this, found, target, revision, publicId, expectedType, type); return found; } @@ -2188,91 +2190,165 @@ } - void StatelessDatabaseOperations::SetMetadata(const std::string& publicId, + void StatelessDatabaseOperations::SetMetadata(int64_t& newRevision, + const std::string& publicId, MetadataType type, - const std::string& value) + const std::string& value, + bool hasOldRevision, + int64_t oldRevision) + { + class Operations : public IReadWriteOperations + { + private: + int64_t& newRevision_; + const std::string& publicId_; + MetadataType type_; + const std::string& value_; + bool hasOldRevision_; + int64_t oldRevision_; + + public: + Operations(int64_t& newRevision, + const std::string& publicId, + MetadataType type, + const std::string& value, + bool hasOldRevision, + int64_t oldRevision) : + newRevision_(newRevision), + publicId_(publicId), + type_(type), + value_(value), + hasOldRevision_(hasOldRevision), + oldRevision_(oldRevision) + { + } + + virtual void Apply(ReadWriteTransaction& transaction) ORTHANC_OVERRIDE + { + ResourceType resourceType; + int64_t id; + if (!transaction.LookupResource(id, resourceType, publicId_)) + { + throw OrthancException(ErrorCode_UnknownResource); + } + else + { + std::string oldValue; + int64_t expectedRevision; + if (transaction.LookupMetadata(oldValue, expectedRevision, id, type_)) + { + if (hasOldRevision_ && + expectedRevision != oldRevision_) + { + throw OrthancException(ErrorCode_Revision); + } + else + { + newRevision_ = oldRevision_ + 1; + } + } + else + { + // The metadata is not existing yet: Ignore "oldRevision" + // and initialize a new sequence of revisions + newRevision_ = 0; + } + + transaction.SetMetadata(id, type_, value_, newRevision_); + + if (IsUserMetadata(type_)) + { + transaction.LogChange(id, ChangeType_UpdatedMetadata, resourceType, publicId_); + } + } + } + }; + + Operations operations(newRevision, publicId, type, value, hasOldRevision, oldRevision); + Apply(operations); + } + + + void StatelessDatabaseOperations::OverwriteMetadata(const std::string& publicId, + MetadataType type, + const std::string& value) + { + int64_t newRevision; // Unused + SetMetadata(newRevision, publicId, type, value, false /* no old revision */, -1 /* dummy */); + } + + + bool StatelessDatabaseOperations::DeleteMetadata(const std::string& publicId, + MetadataType type, + bool hasRevision, + int64_t revision) { class Operations : public IReadWriteOperations { private: const std::string& publicId_; MetadataType type_; - const std::string& value_; + bool hasRevision_; + int64_t revision_; + bool found_; public: Operations(const std::string& publicId, MetadataType type, - const std::string& value) : + bool hasRevision, + int64_t revision) : publicId_(publicId), type_(type), - value_(value) + hasRevision_(hasRevision), + revision_(revision), + found_(false) { } + bool HasFound() const + { + return found_; + } + virtual void Apply(ReadWriteTransaction& transaction) ORTHANC_OVERRIDE { - ResourceType rtype; + ResourceType resourceType; int64_t id; - if (!transaction.LookupResource(id, rtype, publicId_)) + if (!transaction.LookupResource(id, resourceType, publicId_)) { throw OrthancException(ErrorCode_UnknownResource); } else { - transaction.SetMetadata(id, type_, value_); - - if (IsUserMetadata(type_)) + std::string s; + int64_t expectedRevision; + if (transaction.LookupMetadata(s, expectedRevision, id, type_)) { - transaction.LogChange(id, ChangeType_UpdatedMetadata, rtype, publicId_); + if (hasRevision_ && + expectedRevision != revision_) + { + throw OrthancException(ErrorCode_Revision); + } + + found_ = true; + transaction.DeleteMetadata(id, type_); + + if (IsUserMetadata(type_)) + { + transaction.LogChange(id, ChangeType_UpdatedMetadata, resourceType, publicId_); + } + } + else + { + found_ = false; } } } }; - Operations operations(publicId, type, value); + Operations operations(publicId, type, hasRevision, revision); Apply(operations); - } - - - void StatelessDatabaseOperations::DeleteMetadata(const std::string& publicId, - MetadataType type) - { - class Operations : public IReadWriteOperations - { - private: - const std::string& publicId_; - MetadataType type_; - - public: - Operations(const std::string& publicId, - MetadataType type) : - publicId_(publicId), - type_(type) - { - } - - virtual void Apply(ReadWriteTransaction& transaction) ORTHANC_OVERRIDE - { - ResourceType rtype; - int64_t id; - if (!transaction.LookupResource(id, rtype, publicId_)) - { - throw OrthancException(ErrorCode_UnknownResource); - } - else - { - transaction.DeleteMetadata(id, type_); - - if (IsUserMetadata(type_)) - { - transaction.LogChange(id, ChangeType_UpdatedMetadata, rtype, publicId_); - } - } - } - }; - - Operations operations(publicId, type); - Apply(operations); + return operations.HasFound(); } @@ -2421,9 +2497,9 @@ virtual void Apply(ReadWriteTransaction& transaction) ORTHANC_OVERRIDE { - ResourceType rtype; + ResourceType resourceType; int64_t id; - if (!transaction.LookupResource(id, rtype, publicId_)) + if (!transaction.LookupResource(id, resourceType, publicId_)) { throw OrthancException(ErrorCode_UnknownResource); } @@ -2433,7 +2509,7 @@ if (IsUserContentType(type_)) { - transaction.LogChange(id, ChangeType_UpdatedAttachment, rtype, publicId_); + transaction.LogChange(id, ChangeType_UpdatedAttachment, resourceType, publicId_); } } } @@ -2512,6 +2588,24 @@ std::unique_ptr<DicomInstanceHasher> hasher_; bool hasTransferSyntax_; DicomTransferSyntax transferSyntax_; + + static void ReplaceMetadata(ReadWriteTransaction& transaction, + int64_t instance, + MetadataType metadata, + const std::string& value) + { + std::string oldValue; + int64_t oldRevision; + + if (transaction.LookupMetadata(oldValue, oldRevision, instance, metadata)) + { + transaction.SetMetadata(instance, metadata, value, oldRevision + 1); + } + else + { + transaction.SetMetadata(instance, metadata, value, 0); + } + } public: explicit Operations(const ParsedDicomFile& dicom) @@ -2548,7 +2642,7 @@ transaction.ClearMainDicomTags(instance); { - ResourcesContent content; + ResourcesContent content(false /* prevent the setting of metadata */); content.AddResource(patient, ResourceType_Patient, summary_); content.AddResource(study, ResourceType_Study, summary_); content.AddResource(series, ResourceType_Series, summary_); @@ -2558,7 +2652,7 @@ if (hasTransferSyntax_) { - transaction.SetMetadata(instance, MetadataType_Instance_TransferSyntax, GetTransferSyntaxUid(transferSyntax_)); + ReplaceMetadata(transaction, instance, MetadataType_Instance_TransferSyntax, GetTransferSyntaxUid(transferSyntax_)); } const DicomValue* value; @@ -2566,7 +2660,7 @@ !value->IsNull() && !value->IsBinary()) { - transaction.SetMetadata(instance, MetadataType_Instance_SopClassUid, value->GetContent()); + ReplaceMetadata(transaction, instance, MetadataType_Instance_SopClassUid, value->GetContent()); } } }; @@ -2929,7 +3023,7 @@ { - ResourcesContent content; + ResourcesContent content(true /* new resource, metadata can be set */); // Populate the tags of the newly-created resources
--- a/OrthancServer/Sources/Database/StatelessDatabaseOperations.h Fri Apr 16 10:48:57 2021 +0200 +++ b/OrthancServer/Sources/Database/StatelessDatabaseOperations.h Fri Apr 16 17:13:03 2021 +0200 @@ -254,10 +254,11 @@ } bool LookupMetadata(std::string& target, + int64_t& revision, int64_t id, MetadataType type) { - return transaction_.LookupMetadata(target, id, type); + return transaction_.LookupMetadata(target, revision, id, type); } bool LookupParent(int64_t& parentId, @@ -359,9 +360,10 @@ void SetMetadata(int64_t id, MetadataType type, - const std::string& value) + const std::string& value, + int64_t revision) { - return transaction_.SetMetadata(id, type, value); + return transaction_.SetMetadata(id, type, value, revision); } void SetProtectedPatient(int64_t internalId, @@ -503,6 +505,7 @@ const std::string& publicId); bool LookupMetadata(std::string& target, + int64_t& revision, const std::string& publicId, ResourceType expectedType, MetadataType type); @@ -569,12 +572,22 @@ void SetProtectedPatient(const std::string& publicId, bool isProtected); - void SetMetadata(const std::string& publicId, + void SetMetadata(int64_t& newRevision /*out*/, + const std::string& publicId, MetadataType type, - const std::string& value); + const std::string& value, + bool hasOldRevision, + int64_t oldRevision); - void DeleteMetadata(const std::string& publicId, - MetadataType type); + // Same as "SetMetadata()", but doesn't care about revisions + void OverwriteMetadata(const std::string& publicId, + MetadataType type, + const std::string& value); + + bool DeleteMetadata(const std::string& publicId, + MetadataType type, + bool hasRevision, + int64_t revision); uint64_t IncrementGlobalSequence(GlobalProperty sequence, bool shared);
--- a/OrthancServer/Sources/OrthancRestApi/OrthancRestResources.cpp Fri Apr 16 10:48:57 2021 +0200 +++ b/OrthancServer/Sources/OrthancRestApi/OrthancRestResources.cpp Fri Apr 16 17:13:03 2021 +0200 @@ -63,6 +63,9 @@ static Orthanc::Semaphore throttlingSemaphore_(4); // TODO => PARAMETER? +static const std::string CHECK_REVISIONS = "CheckRevisions"; + + namespace Orthanc { static std::string GetDocumentationSampleResource(ResourceType type) @@ -1447,6 +1450,37 @@ } + static bool GetRevisionHeader(int64_t& revision /* out */, + const RestApiCall& call, + const std::string& header) + { + std::string lower; + Toolbox::ToLowerCase(lower, header); + + HttpToolbox::Arguments::const_iterator found = call.GetHttpHeaders().find(lower); + if (found == call.GetHttpHeaders().end()) + { + return false; + } + else + { + std::string value = Toolbox::StripSpaces(found->second); + Toolbox::RemoveSurroundingQuotes(value); + + try + { + revision = boost::lexical_cast<int64_t>(value); + return true; + } + catch (boost::bad_lexical_cast&) + { + throw OrthancException(ErrorCode_ParameterOutOfRange, "The \"" + header + + "\" HTTP header should contain the revision as an integer, but found: " + value); + } + } + } + + static void GetMetadata(RestApiGetCall& call) { if (call.IsDocumentation()) @@ -1459,7 +1493,9 @@ .SetDescription("Get the value of a metadata that is associated with the given " + r) .SetUriArgument("id", "Orthanc identifier of the " + r + " of interest") .SetUriArgument("name", "The name of the metadata, or its index (cf. `UserMetadata` configuration option)") - .AddAnswerType(MimeType_PlainText, "Value of the metadata"); + .AddAnswerType(MimeType_PlainText, "Value of the metadata") + .SetAnswerHeader("ETag", "Revision of the metadata, to be used in further `PUT` or `DELETE` operations") + .SetHttpHeader("If-None-Match", "Optional revision of the metadata, to check if its content has changed"); return; } @@ -1471,9 +1507,22 @@ MetadataType metadata = StringToMetadata(name); std::string value; - if (OrthancRestApi::GetIndex(call).LookupMetadata(value, publicId, level, metadata)) + int64_t revision; + if (OrthancRestApi::GetIndex(call).LookupMetadata(value, revision, publicId, level, metadata)) { - call.GetOutput().AnswerBuffer(value, MimeType_PlainText); + call.GetOutput().GetLowLevelOutput(). + AddHeader("ETag", "\"" + boost::lexical_cast<std::string>(revision) + "\""); // New in Orthanc 1.9.2 + + int64_t userRevision; + if (GetRevisionHeader(userRevision, call, "If-None-Match") && + revision == userRevision) + { + call.GetOutput().GetLowLevelOutput().SendStatus(HttpStatus_304_NotModified); + } + else + { + call.GetOutput().AnswerBuffer(value, MimeType_PlainText); + } } } @@ -1490,20 +1539,48 @@ .SetDescription("Delete some metadata associated with the given DICOM " + r + ". This call will fail if trying to delete a system metadata (i.e. whose index is < 1024).") .SetUriArgument("id", "Orthanc identifier of the " + r + " of interest") - .SetUriArgument("name", "The name of the metadata, or its index (cf. `UserMetadata` configuration option)"); + .SetUriArgument("name", "The name of the metadata, or its index (cf. `UserMetadata` configuration option)") + .SetHttpHeader("If-Match", "Revision of the metadata, to check if its content has not changed and can " + "be deleted. This option is mandatory if `CheckRevision` option is `true`."); return; } CheckValidResourceType(call); - - std::string publicId = call.GetUriComponent("id", ""); + const std::string publicId = call.GetUriComponent("id", ""); + std::string name = call.GetUriComponent("name", ""); MetadataType metadata = StringToMetadata(name); if (IsUserMetadata(metadata)) // It is forbidden to modify internal metadata - { - OrthancRestApi::GetIndex(call).DeleteMetadata(publicId, metadata); - call.GetOutput().AnswerBuffer("", MimeType_PlainText); + { + bool found; + int64_t revision; + if (GetRevisionHeader(revision, call, "if-match")) + { + found = OrthancRestApi::GetIndex(call).DeleteMetadata(publicId, metadata, true, revision); + } + else + { + OrthancConfiguration::ReaderLock lock; + if (lock.GetConfiguration().GetBooleanParameter(CHECK_REVISIONS, false)) + { + throw OrthancException(ErrorCode_Revision, + "HTTP header \"If-Match\" is missing, as \"CheckRevision\" is \"true\""); + } + else + { + found = OrthancRestApi::GetIndex(call).DeleteMetadata(publicId, metadata, false, -1 /* dummy value */); + } + } + + if (found) + { + call.GetOutput().AnswerBuffer("", MimeType_PlainText); + } + else + { + throw OrthancException(ErrorCode_UnknownResource); + } } else { @@ -1525,7 +1602,8 @@ ". This call will fail if trying to modify a system metadata (i.e. whose index is < 1024).") .SetUriArgument("id", "Orthanc identifier of the " + r + " of interest") .SetUriArgument("name", "The name of the metadata, or its index (cf. `UserMetadata` configuration option)") - .AddRequestType(MimeType_PlainText, "String value of the metadata"); + .AddRequestType(MimeType_PlainText, "String value of the metadata") + .SetHttpHeader("If-Match", "Revision of the metadata, if this is not the first time this metadata is set."); return; } @@ -1540,8 +1618,28 @@ if (IsUserMetadata(metadata)) // It is forbidden to modify internal metadata { - // It is forbidden to modify internal metadata - OrthancRestApi::GetIndex(call).SetMetadata(publicId, metadata, value); + int64_t oldRevision; + bool hasOldRevision = GetRevisionHeader(oldRevision, call, "if-match"); + + if (!hasOldRevision) + { + OrthancConfiguration::ReaderLock lock; + if (lock.GetConfiguration().GetBooleanParameter(CHECK_REVISIONS, false)) + { + // "StatelessDatabaseOperations::SetMetadata()" will ignore + // the actual value of "oldRevision" if the metadata is + // inexistent as expected + hasOldRevision = true; + oldRevision = -1; // dummy value + } + } + + int64_t newRevision; + OrthancRestApi::GetIndex(call).SetMetadata(newRevision, publicId, metadata, value, hasOldRevision, oldRevision); + + call.GetOutput().GetLowLevelOutput(). + AddHeader("ETag", "\"" + boost::lexical_cast<std::string>(newRevision) + "\""); // New in Orthanc 1.9.2 + call.GetOutput().AnswerBuffer("", MimeType_PlainText); } else
--- a/OrthancServer/Sources/OrthancWebDav.cpp Fri Apr 16 10:48:57 2021 +0200 +++ b/OrthancServer/Sources/OrthancWebDav.cpp Fri Apr 16 17:13:03 2021 +0200 @@ -69,7 +69,8 @@ MetadataType metadata) { std::string value; - if (context.GetIndex().LookupMetadata(value, publicId, level, metadata)) + int64_t revision; // Ignored + if (context.GetIndex().LookupMetadata(value, revision, publicId, level, metadata)) { try {
--- a/OrthancServer/Sources/ServerContext.cpp Fri Apr 16 10:48:57 2021 +0200 +++ b/OrthancServer/Sources/ServerContext.cpp Fri Apr 16 17:13:03 2021 +0200 @@ -860,7 +860,8 @@ { std::string s; - if (index_.LookupMetadata(s, instancePublicId, ResourceType_Instance, + int64_t revision; // Ignored + if (index_.LookupMetadata(s, revision, instancePublicId, ResourceType_Instance, MetadataType_Instance_PixelDataOffset)) { hasPixelDataOffset = false; @@ -971,8 +972,8 @@ if (DicomStreamReader::LookupPixelDataOffset(pixelDataOffset, dicom) && pixelDataOffset < dicom.size()) { - index_.SetMetadata(instancePublicId, MetadataType_Instance_PixelDataOffset, - boost::lexical_cast<std::string>(pixelDataOffset)); + index_.OverwriteMetadata(instancePublicId, MetadataType_Instance_PixelDataOffset, + boost::lexical_cast<std::string>(pixelDataOffset)); if (!area_.HasReadRange() || compressionEnabled_) @@ -1018,9 +1019,10 @@ } std::string s; + int64_t revision; // Ignored if (attachment.GetCompressionType() == CompressionType_None && - index_.LookupMetadata(s, instancePublicId, ResourceType_Instance, + index_.LookupMetadata(s, revision, instancePublicId, ResourceType_Instance, MetadataType_Instance_PixelDataOffset) && !s.empty()) { @@ -1643,7 +1645,8 @@ if (metadata == MetadataType_Instance_SopClassUid || metadata == MetadataType_Instance_TransferSyntax) { - if (index_.LookupMetadata(target, publicId, level, metadata)) + int64_t revision; // Ignored + if (index_.LookupMetadata(target, revision, publicId, level, metadata)) { return true; } @@ -1685,7 +1688,7 @@ target = value->GetContent(); // Store for reuse - index_.SetMetadata(publicId, metadata, target); + index_.OverwriteMetadata(publicId, metadata, target); return true; } else @@ -1698,7 +1701,8 @@ else { // No backward - return index_.LookupMetadata(target, publicId, level, metadata); + int64_t revision; // Ignored + return index_.LookupMetadata(target, revision, publicId, level, metadata); } }
--- a/OrthancServer/Sources/ServerToolbox.cpp Fri Apr 16 10:48:57 2021 +0200 +++ b/OrthancServer/Sources/ServerToolbox.cpp Fri Apr 16 17:13:03 2021 +0200 @@ -34,7 +34,6 @@ #include "PrecompiledHeadersServer.h" #include "ServerToolbox.h" -#include "../../OrthancFramework/Sources/DicomFormat/DicomArray.h" #include "../../OrthancFramework/Sources/DicomParsing/ParsedDicomFile.h" #include "../../OrthancFramework/Sources/FileStorage/StorageAccessor.h" #include "../../OrthancFramework/Sources/Logging.h" @@ -77,91 +76,6 @@ }; - static void StoreMainDicomTagsInternal(ResourcesContent& target, - int64_t resource, - const DicomMap& tags) - { - DicomArray flattened(tags); - - for (size_t i = 0; i < flattened.GetSize(); i++) - { - const DicomElement& element = flattened.GetElement(i); - const DicomTag& tag = element.GetTag(); - const DicomValue& value = element.GetValue(); - if (!value.IsNull() && - !value.IsBinary()) - { - target.AddMainDicomTag(resource, tag, element.GetValue().GetContent()); - } - } - } - - - static void StoreIdentifiers(ResourcesContent& target, - int64_t resource, - ResourceType level, - const DicomMap& map) - { - const DicomTag* tags; - size_t size; - - ServerToolbox::LoadIdentifiers(tags, size, level); - - for (size_t i = 0; i < size; i++) - { - // The identifiers tags are a subset of the main DICOM tags - assert(DicomMap::IsMainDicomTag(tags[i])); - - const DicomValue* value = map.TestAndGetValue(tags[i]); - if (value != NULL && - !value->IsNull() && - !value->IsBinary()) - { - std::string s = ServerToolbox::NormalizeIdentifier(value->GetContent()); - target.AddIdentifierTag(resource, tags[i], s); - } - } - } - - - void ResourcesContent::AddResource(int64_t resource, - ResourceType level, - const DicomMap& dicomSummary) - { - StoreIdentifiers(*this, resource, level, dicomSummary); - - DicomMap tags; - - switch (level) - { - case ResourceType_Patient: - dicomSummary.ExtractPatientInformation(tags); - break; - - case ResourceType_Study: - // Duplicate the patient tags at the study level (new in Orthanc 0.9.5 - db v6) - dicomSummary.ExtractPatientInformation(tags); - StoreMainDicomTagsInternal(*this, resource, tags); - - dicomSummary.ExtractStudyInformation(tags); - break; - - case ResourceType_Series: - dicomSummary.ExtractSeriesInformation(tags); - break; - - case ResourceType_Instance: - dicomSummary.ExtractInstanceInformation(tags); - break; - - default: - throw OrthancException(ErrorCode_InternalError); - } - - StoreMainDicomTagsInternal(*this, resource, tags); - } - - namespace ServerToolbox { bool FindOneChildInstance(int64_t& result, @@ -274,7 +188,7 @@ transaction.ClearMainDicomTags(resource); - ResourcesContent tags; + ResourcesContent tags(false /* prevent the setting of metadata */); tags.AddResource(resource, level, dicomSummary); transaction.SetResourcesContent(tags); }
--- a/OrthancServer/Sources/SliceOrdering.cpp Fri Apr 16 10:48:57 2021 +0200 +++ b/OrthancServer/Sources/SliceOrdering.cpp Fri Apr 16 17:13:03 2021 +0200 @@ -208,7 +208,8 @@ try { - if (index.LookupMetadata(s, instanceId, ResourceType_Instance, MetadataType_Instance_IndexInSeries)) + int64_t revision; // Ignored + if (index.LookupMetadata(s, revision, instanceId, ResourceType_Instance, MetadataType_Instance_IndexInSeries)) { indexInSeries_ = boost::lexical_cast<size_t>(Toolbox::StripSpaces(s)); hasIndexInSeries_ = true;
--- a/OrthancServer/Sources/main.cpp Fri Apr 16 10:48:57 2021 +0200 +++ b/OrthancServer/Sources/main.cpp Fri Apr 16 17:13:03 2021 +0200 @@ -724,7 +724,7 @@ PrintErrorCode(ErrorCode_DiscontinuedAbi, "Calling a function that has been removed from the Orthanc Framework"); PrintErrorCode(ErrorCode_BadRange, "Incorrect range request"); PrintErrorCode(ErrorCode_DatabaseCannotSerialize, "Database could not serialize access due to concurrent update, the transaction should be retried"); - PrintErrorCode(ErrorCode_Revision, "A bad revision number was provided, indicates conflict between multiple updates"); + PrintErrorCode(ErrorCode_Revision, "A bad revision number was provided, which might indicate conflict between multiple writers"); PrintErrorCode(ErrorCode_SQLiteNotOpened, "SQLite: The database is not opened"); PrintErrorCode(ErrorCode_SQLiteAlreadyOpened, "SQLite: Connection is already open"); PrintErrorCode(ErrorCode_SQLiteCannotOpen, "SQLite: Unable to open the database");
--- a/OrthancServer/UnitTestsSources/ServerIndexTests.cpp Fri Apr 16 10:48:57 2021 +0200 +++ b/OrthancServer/UnitTestsSources/ServerIndexTests.cpp Fri Apr 16 17:13:03 2021 +0200 @@ -305,12 +305,14 @@ CompressionType_ZlibWithSize, 21, "compressedMD5")); transaction_->AddAttachment(a[4], FileInfo("my dicom file", FileContentType_Dicom, 42, "md5")); transaction_->AddAttachment(a[6], FileInfo("world", FileContentType_Dicom, 44, "md5")); - transaction_->SetMetadata(a[4], MetadataType_RemoteAet, "PINNACLE"); + + // TODO - REVISIONS - "42" is revision number, that is not currently stored (*) + transaction_->SetMetadata(a[4], MetadataType_RemoteAet, "PINNACLE", 42); transaction_->GetAllMetadata(md, a[4]); ASSERT_EQ(1u, md.size()); ASSERT_EQ("PINNACLE", md[MetadataType_RemoteAet]); - transaction_->SetMetadata(a[4], MetadataType_ModifiedFrom, "TUTU"); + transaction_->SetMetadata(a[4], MetadataType_ModifiedFrom, "TUTU", 10); transaction_->GetAllMetadata(md, a[4]); ASSERT_EQ(2u, md.size()); @@ -341,14 +343,19 @@ ASSERT_EQ(7, b); ASSERT_EQ(ResourceType_Study, t); - ASSERT_TRUE(transaction_->LookupMetadata(s, a[4], MetadataType_RemoteAet)); - ASSERT_FALSE(transaction_->LookupMetadata(s, a[4], MetadataType_Instance_IndexInSeries)); + int64_t revision; + ASSERT_TRUE(transaction_->LookupMetadata(s, revision, a[4], MetadataType_RemoteAet)); + ASSERT_EQ(0, revision); // "0" instead of "42" because of (*) + ASSERT_FALSE(transaction_->LookupMetadata(s, revision, a[4], MetadataType_Instance_IndexInSeries)); + ASSERT_EQ(0, revision); ASSERT_EQ("PINNACLE", s); std::string u; - ASSERT_TRUE(transaction_->LookupMetadata(u, a[4], MetadataType_RemoteAet)); + ASSERT_TRUE(transaction_->LookupMetadata(u, revision, a[4], MetadataType_RemoteAet)); + ASSERT_EQ(0, revision); ASSERT_EQ("PINNACLE", u); - ASSERT_FALSE(transaction_->LookupMetadata(u, a[4], MetadataType_Instance_IndexInSeries)); + ASSERT_FALSE(transaction_->LookupMetadata(u, revision, a[4], MetadataType_Instance_IndexInSeries)); + ASSERT_EQ(0, revision); ASSERT_TRUE(transaction_->LookupGlobalProperty(s, GlobalProperty_FlushSleep, true)); ASSERT_FALSE(transaction_->LookupGlobalProperty(s, static_cast<GlobalProperty>(42), true)); @@ -1008,12 +1015,14 @@ } std::string s; - bool found = context.GetIndex().LookupMetadata(s, id, ResourceType_Instance, + int64_t revision; + bool found = context.GetIndex().LookupMetadata(s, revision, id, ResourceType_Instance, MetadataType_Instance_PixelDataOffset); if (withPixelData) { ASSERT_TRUE(found); + ASSERT_EQ(0, revision); ASSERT_GT(boost::lexical_cast<int>(s), 128 /* length of the DICOM preamble */); ASSERT_LT(boost::lexical_cast<size_t>(s), dicomSize); }