# HG changeset patch # User Sebastien Jodogne # Date 1617120627 -7200 # Node ID f75c63aa9de0b1cca20e23f264979e0797244e03 # Parent d01702fb29a97b432c89d49092471ab0964e2f2d differentiating between shared and private global properties diff -r d01702fb29a9 -r f75c63aa9de0 OrthancServer/Plugins/Engine/OrthancPluginDatabase.cpp --- a/OrthancServer/Plugins/Engine/OrthancPluginDatabase.cpp Tue Mar 30 16:34:23 2021 +0200 +++ b/OrthancServer/Plugins/Engine/OrthancPluginDatabase.cpp Tue Mar 30 18:10:27 2021 +0200 @@ -1129,8 +1129,12 @@ virtual bool LookupGlobalProperty(std::string& target, - GlobalProperty property) ORTHANC_OVERRIDE + GlobalProperty property, + bool shared) ORTHANC_OVERRIDE { + // "shared" is unused, as database plugins using Orthanc SDK <= + // 1.9.1 are not compatible with multiple readers/writers + ResetAnswers(); CheckSuccess(that_.backend_.lookupGlobalProperty @@ -1291,8 +1295,12 @@ virtual void SetGlobalProperty(GlobalProperty property, + bool shared, const std::string& value) ORTHANC_OVERRIDE { + // "shared" is unused, as database plugins using Orthanc SDK <= + // 1.9.1 are not compatible with multiple readers/writers + CheckSuccess(that_.backend_.setGlobalProperty (that_.payload_, static_cast(property), value.c_str())); } @@ -1527,7 +1535,7 @@ std::string tmp; fastGetTotalSize_ = - (transaction.LookupGlobalProperty(tmp, GlobalProperty_GetTotalSizeIsFast) && + (transaction.LookupGlobalProperty(tmp, GlobalProperty_GetTotalSizeIsFast, true /* unused in old databases */) && tmp == "1"); if (fastGetTotalSize_) diff -r d01702fb29a9 -r f75c63aa9de0 OrthancServer/Plugins/Engine/OrthancPluginDatabaseV3.cpp --- a/OrthancServer/Plugins/Engine/OrthancPluginDatabaseV3.cpp Tue Mar 30 16:34:23 2021 +0200 +++ b/OrthancServer/Plugins/Engine/OrthancPluginDatabaseV3.cpp Tue Mar 30 18:10:27 2021 +0200 @@ -694,10 +694,12 @@ virtual bool LookupGlobalProperty(std::string& target, - GlobalProperty property) ORTHANC_OVERRIDE + GlobalProperty property, + bool shared) ORTHANC_OVERRIDE { - CheckSuccess(that_.backend_.lookupGlobalProperty( - transaction_, that_.serverIdentifier_.c_str(), static_cast(property))); + const char* id = (shared ? "" : that_.serverIdentifier_.c_str()); + + CheckSuccess(that_.backend_.lookupGlobalProperty(transaction_, id, static_cast(property))); CheckNoEvent(); return ReadSingleStringAnswer(target); } @@ -761,10 +763,12 @@ virtual void SetGlobalProperty(GlobalProperty property, + bool shared, const std::string& value) ORTHANC_OVERRIDE { - CheckSuccess(that_.backend_.setGlobalProperty(transaction_, that_.serverIdentifier_.c_str(), - static_cast(property), value.c_str())); + const char* id = (shared ? "" : that_.serverIdentifier_.c_str()); + + CheckSuccess(that_.backend_.setGlobalProperty(transaction_, id, static_cast(property), value.c_str())); CheckNoEvent(); } diff -r d01702fb29a9 -r f75c63aa9de0 OrthancServer/Plugins/Engine/OrthancPlugins.cpp --- a/OrthancServer/Plugins/Engine/OrthancPlugins.cpp Tue Mar 30 16:34:23 2021 +0200 +++ b/OrthancServer/Plugins/Engine/OrthancPlugins.cpp Tue Mar 30 18:10:27 2021 +0200 @@ -4212,8 +4212,11 @@ } else { + // TODO - Plugins can only access global properties of their + // own Orthanc server (no access to the shared global properties) PImpl::ServerContextLock lock(*pimpl_); - lock.GetContext().GetIndex().SetGlobalProperty(static_cast(p.property), p.value); + lock.GetContext().GetIndex().SetGlobalProperty(static_cast(p.property), + false /* not shared */, p.value); return true; } } @@ -4226,8 +4229,11 @@ std::string result; { + // TODO - Plugins can only access global properties of their + // own Orthanc server (no access to the shared global properties) PImpl::ServerContextLock lock(*pimpl_); - result = lock.GetContext().GetIndex().GetGlobalProperty(static_cast(p.property), p.value); + result = lock.GetContext().GetIndex().GetGlobalProperty(static_cast(p.property), + false /* not shared */, p.value); } *(p.result) = CopyString(result); @@ -5024,9 +5030,7 @@ case _OrthancPluginService_RegisterDatabaseBackend: { - // TODO - WARN ABOUT PERFORMANCE - - CLOG(INFO, PLUGINS) << "Plugin has registered a custom database back-end"; + LOG(WARNING) << "Performance warning: Plugin has registered a custom database back-end with an old API"; const _OrthancPluginRegisterDatabaseBackend& p = *reinterpret_cast(parameters); @@ -5049,9 +5053,7 @@ case _OrthancPluginService_RegisterDatabaseBackendV2: { - // TODO - WARN ABOUT PERFORMANCE - - CLOG(INFO, PLUGINS) << "Plugin has registered a custom database back-end"; + LOG(WARNING) << "Performance warning: Plugin has registered a custom database back-end with an old API"; const _OrthancPluginRegisterDatabaseBackendV2& p = *reinterpret_cast(parameters); diff -r d01702fb29a9 -r f75c63aa9de0 OrthancServer/Sources/Database/IDatabaseWrapper.h --- a/OrthancServer/Sources/Database/IDatabaseWrapper.h Tue Mar 30 16:34:23 2021 +0200 +++ b/OrthancServer/Sources/Database/IDatabaseWrapper.h Tue Mar 30 18:10:27 2021 +0200 @@ -155,8 +155,15 @@ int64_t id, FileContentType contentType) = 0; + /** + * If "shared" is "true", the property is shared by all the + * Orthanc servers that access the same database. If "shared" is + * "false", the property is private to the server (cf. the + * "DatabaseServerIdentifier" configuration option). + **/ virtual bool LookupGlobalProperty(std::string& target, - GlobalProperty property) = 0; + GlobalProperty property, + bool shared) = 0; virtual bool LookupMetadata(std::string& target, int64_t id, @@ -175,6 +182,7 @@ int64_t patientIdToAvoid) = 0; virtual void SetGlobalProperty(GlobalProperty property, + bool shared, const std::string& value) = 0; virtual void ClearMainDicomTags(int64_t id) = 0; diff -r d01702fb29a9 -r f75c63aa9de0 OrthancServer/Sources/Database/SQLiteDatabaseWrapper.cpp --- a/OrthancServer/Sources/Database/SQLiteDatabaseWrapper.cpp Tue Mar 30 16:34:23 2021 +0200 +++ b/OrthancServer/Sources/Database/SQLiteDatabaseWrapper.cpp Tue Mar 30 18:10:27 2021 +0200 @@ -827,8 +827,12 @@ virtual bool LookupGlobalProperty(std::string& target, - GlobalProperty property) ORTHANC_OVERRIDE + GlobalProperty property, + bool shared) ORTHANC_OVERRIDE { + // The "shared" info is not used by the SQLite database, as it + // can only be used by one Orthanc server. + SQLite::Statement s(db_, SQLITE_FROM_HERE, "SELECT value FROM GlobalProperties WHERE property=?"); s.BindInt(0, property); @@ -964,8 +968,12 @@ virtual void SetGlobalProperty(GlobalProperty property, + bool shared, const std::string& value) ORTHANC_OVERRIDE { + // The "shared" info is not used by the SQLite database, as it + // can only be used by one Orthanc server. + SQLite::Statement s(db_, SQLITE_FROM_HERE, "INSERT OR REPLACE INTO GlobalProperties VALUES(?, ?)"); s.BindInt(0, property); s.BindString(1, value); @@ -1318,7 +1326,7 @@ // Check the version of the database std::string tmp; - if (!transaction->LookupGlobalProperty(tmp, GlobalProperty_DatabaseSchemaVersion)) + if (!transaction->LookupGlobalProperty(tmp, GlobalProperty_DatabaseSchemaVersion, true /* unused in SQLite */)) { tmp = "Unknown"; } @@ -1343,7 +1351,7 @@ // New in Orthanc 1.5.1 if (version_ == 6) { - if (!transaction->LookupGlobalProperty(tmp, GlobalProperty_GetTotalSizeIsFast) || + if (!transaction->LookupGlobalProperty(tmp, GlobalProperty_GetTotalSizeIsFast, true /* unused in SQLite */) || tmp != "1") { LOG(INFO) << "Installing the SQLite triggers to track the size of the attachments"; diff -r d01702fb29a9 -r f75c63aa9de0 OrthancServer/Sources/Database/StatelessDatabaseOperations.cpp --- a/OrthancServer/Sources/Database/StatelessDatabaseOperations.cpp Tue Mar 30 16:34:23 2021 +0200 +++ b/OrthancServer/Sources/Database/StatelessDatabaseOperations.cpp Tue Mar 30 18:10:27 2021 +0200 @@ -1658,31 +1658,33 @@ bool StatelessDatabaseOperations::LookupGlobalProperty(std::string& value, - GlobalProperty property) + GlobalProperty property, + bool shared) { - class Operations : public ReadOnlyOperationsT3 + class Operations : public ReadOnlyOperationsT4 { public: virtual void ApplyTuple(ReadOnlyTransaction& transaction, const Tuple& tuple) ORTHANC_OVERRIDE { // TODO - CANDIDATE FOR "TransactionType_Implicit" - tuple.get<0>() = transaction.LookupGlobalProperty(tuple.get<1>(), tuple.get<2>()); + tuple.get<0>() = transaction.LookupGlobalProperty(tuple.get<1>(), tuple.get<2>(), tuple.get<3>()); } }; bool found; Operations operations; - operations.Apply(*this, found, value, property); + operations.Apply(*this, found, value, property, shared); return found; } std::string StatelessDatabaseOperations::GetGlobalProperty(GlobalProperty property, + bool shared, const std::string& defaultValue) { std::string s; - if (LookupGlobalProperty(s, property)) + if (LookupGlobalProperty(s, property, shared)) { return s; } @@ -2273,18 +2275,22 @@ } - uint64_t StatelessDatabaseOperations::IncrementGlobalSequence(GlobalProperty sequence) + uint64_t StatelessDatabaseOperations::IncrementGlobalSequence(GlobalProperty sequence, + bool shared) { class Operations : public IReadWriteOperations { private: uint64_t newValue_; GlobalProperty sequence_; + bool shared_; public: - explicit Operations(GlobalProperty sequence) : + Operations(GlobalProperty sequence, + bool shared) : newValue_(0), // Dummy initialization - sequence_(sequence) + sequence_(sequence), + shared_(shared) { } @@ -2297,7 +2303,7 @@ { std::string oldString; - if (transaction.LookupGlobalProperty(oldString, sequence_)) + if (transaction.LookupGlobalProperty(oldString, sequence_, shared_)) { uint64_t oldValue; @@ -2320,11 +2326,11 @@ newValue_ = 1; } - transaction.SetGlobalProperty(sequence_, boost::lexical_cast(newValue_)); + transaction.SetGlobalProperty(sequence_, shared_, boost::lexical_cast(newValue_)); } }; - Operations operations(sequence); + Operations operations(sequence, shared); Apply(operations); assert(operations.GetNewValue() != 0); return operations.GetNewValue(); @@ -2364,29 +2370,33 @@ void StatelessDatabaseOperations::SetGlobalProperty(GlobalProperty property, + bool shared, const std::string& value) { class Operations : public IReadWriteOperations { private: GlobalProperty property_; + bool shared_; const std::string& value_; public: Operations(GlobalProperty property, + bool shared, const std::string& value) : property_(property), + shared_(shared), value_(value) { } virtual void Apply(ReadWriteTransaction& transaction) ORTHANC_OVERRIDE { - transaction.SetGlobalProperty(property_, value_); + transaction.SetGlobalProperty(property_, shared_, value_); } }; - Operations operations(property, value); + Operations operations(property, shared, value); Apply(operations); } diff -r d01702fb29a9 -r f75c63aa9de0 OrthancServer/Sources/Database/StatelessDatabaseOperations.h --- a/OrthancServer/Sources/Database/StatelessDatabaseOperations.h Tue Mar 30 16:34:23 2021 +0200 +++ b/OrthancServer/Sources/Database/StatelessDatabaseOperations.h Tue Mar 30 18:10:27 2021 +0200 @@ -246,9 +246,10 @@ } bool LookupGlobalProperty(std::string& target, - GlobalProperty property) + GlobalProperty property, + bool shared) { - return transaction_.LookupGlobalProperty(target, property); + return transaction_.LookupGlobalProperty(target, property, shared); } bool LookupMetadata(std::string& target, @@ -349,9 +350,10 @@ } void SetGlobalProperty(GlobalProperty property, + bool shared, const std::string& value) { - transaction_.SetGlobalProperty(property, value); + transaction_.SetGlobalProperty(property, shared, value); } void SetMetadata(int64_t id, @@ -520,9 +522,11 @@ const std::string& value); bool LookupGlobalProperty(std::string& value, - GlobalProperty property); + GlobalProperty property, + bool shared); std::string GetGlobalProperty(GlobalProperty property, + bool shared, const std::string& defaultValue); bool GetMainDicomTags(DicomMap& result, @@ -564,13 +568,15 @@ void DeleteMetadata(const std::string& publicId, MetadataType type); - uint64_t IncrementGlobalSequence(GlobalProperty sequence); + uint64_t IncrementGlobalSequence(GlobalProperty sequence, + bool shared); void DeleteChanges(); void DeleteExportedResources(); void SetGlobalProperty(GlobalProperty property, + bool shared, const std::string& value); void DeleteAttachment(const std::string& publicId, diff -r d01702fb29a9 -r f75c63aa9de0 OrthancServer/Sources/OrthancConfiguration.cpp --- a/OrthancServer/Sources/OrthancConfiguration.cpp Tue Mar 30 16:34:23 2021 +0200 +++ b/OrthancServer/Sources/OrthancConfiguration.cpp Tue Mar 30 18:10:27 2021 +0200 @@ -53,6 +53,7 @@ static const char* const ORTHANC_PEERS = "OrthancPeers"; static const char* const ORTHANC_PEERS_IN_DB = "OrthancPeersInDatabase"; static const char* const TEMPORARY_DIRECTORY = "TemporaryDirectory"; +static const char* const DATABASE_SERVER_IDENTIFIER = "DatabaseServerIdentifier"; namespace Orthanc { @@ -254,7 +255,7 @@ } else { - std::string property = serverIndex_->GetGlobalProperty(GlobalProperty_Modalities, "{}"); + std::string property = serverIndex_->GetGlobalProperty(GlobalProperty_Modalities, false /* not shared */, "{}"); Json::Value modalities; if (Toolbox::ReadJson(modalities, property)) @@ -293,7 +294,7 @@ } else { - std::string property = serverIndex_->GetGlobalProperty(GlobalProperty_Peers, "{}"); + std::string property = serverIndex_->GetGlobalProperty(GlobalProperty_Peers, false /* not shared */, "{}"); Json::Value peers; if (Toolbox::ReadJson(peers, property)) @@ -369,7 +370,7 @@ std::string s; Toolbox::WriteFastJson(s, modalities); - serverIndex_->SetGlobalProperty(GlobalProperty_Modalities, s); + serverIndex_->SetGlobalProperty(GlobalProperty_Modalities, false /* not shared */, s); } } else @@ -401,7 +402,7 @@ std::string s; Toolbox::WriteFastJson(s, peers); - serverIndex_->SetGlobalProperty(GlobalProperty_Peers, s); + serverIndex_->SetGlobalProperty(GlobalProperty_Peers, false /* not shared */, s); } } else @@ -1018,9 +1019,17 @@ { std::string id; - if (LookupStringParameter(id, "DatabaseServerIdentifier")) + if (LookupStringParameter(id, DATABASE_SERVER_IDENTIFIER)) { - return id; + if (id.empty()) + { + throw OrthancException(ErrorCode_ParameterOutOfRange, "Global configuration option \"" + + std::string(DATABASE_SERVER_IDENTIFIER) + "\" cannot be empty"); + } + else + { + return id; + } } else { diff -r d01702fb29a9 -r f75c63aa9de0 OrthancServer/Sources/OrthancRestApi/OrthancRestAnonymizeModify.cpp --- a/OrthancServer/Sources/OrthancRestApi/OrthancRestAnonymizeModify.cpp Tue Mar 30 16:34:23 2021 +0200 +++ b/OrthancServer/Sources/OrthancRestApi/OrthancRestAnonymizeModify.cpp Tue Mar 30 18:10:27 2021 +0200 @@ -53,7 +53,7 @@ static std::string GeneratePatientName(ServerContext& context) { - uint64_t seq = context.GetIndex().IncrementGlobalSequence(GlobalProperty_AnonymizationSequence); + uint64_t seq = context.GetIndex().IncrementGlobalSequence(GlobalProperty_AnonymizationSequence, true /* shared */); return "Anonymized" + boost::lexical_cast(seq); } diff -r d01702fb29a9 -r f75c63aa9de0 OrthancServer/Sources/ServerContext.cpp --- a/OrthancServer/Sources/ServerContext.cpp Tue Mar 30 16:34:23 2021 +0200 +++ b/OrthancServer/Sources/ServerContext.cpp Tue Mar 30 18:10:27 2021 +0200 @@ -215,7 +215,7 @@ if (loadJobsFromDatabase) { std::string serialized; - if (index_.LookupGlobalProperty(serialized, GlobalProperty_JobsRegistry)) + if (index_.LookupGlobalProperty(serialized, GlobalProperty_JobsRegistry, false /* not shared */)) { LOG(WARNING) << "Reloading the jobs from the last execution of Orthanc"; @@ -261,7 +261,7 @@ std::string serialized; Toolbox::WriteFastJson(serialized, value); - index_.SetGlobalProperty(GlobalProperty_JobsRegistry, serialized); + index_.SetGlobalProperty(GlobalProperty_JobsRegistry, false /* not shared */, serialized); } catch (OrthancException& e) { diff -r d01702fb29a9 -r f75c63aa9de0 OrthancServer/UnitTestsSources/ServerIndexTests.cpp --- a/OrthancServer/UnitTestsSources/ServerIndexTests.cpp Tue Mar 30 16:34:23 2021 +0200 +++ b/OrthancServer/UnitTestsSources/ServerIndexTests.cpp Tue Mar 30 18:10:27 2021 +0200 @@ -251,7 +251,7 @@ ASSERT_EQ(3u, t.size()); } - transaction_->SetGlobalProperty(GlobalProperty_FlushSleep, "World"); + transaction_->SetGlobalProperty(GlobalProperty_FlushSleep, true, "World"); transaction_->AttachChild(a[0], a[1]); transaction_->AttachChild(a[1], a[2]); @@ -350,8 +350,8 @@ ASSERT_EQ("PINNACLE", u); ASSERT_FALSE(transaction_->LookupMetadata(u, a[4], MetadataType_Instance_IndexInSeries)); - ASSERT_TRUE(transaction_->LookupGlobalProperty(s, GlobalProperty_FlushSleep)); - ASSERT_FALSE(transaction_->LookupGlobalProperty(s, static_cast(42))); + ASSERT_TRUE(transaction_->LookupGlobalProperty(s, GlobalProperty_FlushSleep, true)); + ASSERT_FALSE(transaction_->LookupGlobalProperty(s, static_cast(42), true)); ASSERT_EQ("World", s); FileInfo att; @@ -402,11 +402,11 @@ CheckTableRecordCount(3, "GlobalProperties"); std::string tmp; - ASSERT_TRUE(transaction_->LookupGlobalProperty(tmp, GlobalProperty_DatabaseSchemaVersion)); + ASSERT_TRUE(transaction_->LookupGlobalProperty(tmp, GlobalProperty_DatabaseSchemaVersion, true)); ASSERT_EQ("6", tmp); - ASSERT_TRUE(transaction_->LookupGlobalProperty(tmp, GlobalProperty_FlushSleep)); + ASSERT_TRUE(transaction_->LookupGlobalProperty(tmp, GlobalProperty_FlushSleep, true)); ASSERT_EQ("World", tmp); - ASSERT_TRUE(transaction_->LookupGlobalProperty(tmp, GlobalProperty_GetTotalSizeIsFast)); + ASSERT_TRUE(transaction_->LookupGlobalProperty(tmp, GlobalProperty_GetTotalSizeIsFast, true)); ASSERT_EQ("1", tmp); ASSERT_EQ(3u, listener_->deletedFiles_.size()); @@ -623,10 +623,10 @@ ServerIndex& index = context.GetIndex(); - ASSERT_EQ(1u, index.IncrementGlobalSequence(GlobalProperty_AnonymizationSequence)); - ASSERT_EQ(2u, index.IncrementGlobalSequence(GlobalProperty_AnonymizationSequence)); - ASSERT_EQ(3u, index.IncrementGlobalSequence(GlobalProperty_AnonymizationSequence)); - ASSERT_EQ(4u, index.IncrementGlobalSequence(GlobalProperty_AnonymizationSequence)); + ASSERT_EQ(1u, index.IncrementGlobalSequence(GlobalProperty_AnonymizationSequence, true)); + ASSERT_EQ(2u, index.IncrementGlobalSequence(GlobalProperty_AnonymizationSequence, true)); + ASSERT_EQ(3u, index.IncrementGlobalSequence(GlobalProperty_AnonymizationSequence, true)); + ASSERT_EQ(4u, index.IncrementGlobalSequence(GlobalProperty_AnonymizationSequence, true)); context.Stop(); db.Close();