Mercurial > hg > orthanc

changeset 4607:f75c63aa9de0 db-changes

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
differentiating between shared and private global properties
author Sebastien Jodogne <s.jodogne@gmail.com>
date Tue, 30 Mar 2021 18:10:27 +0200
parents d01702fb29a9
children de5e6b04442d
files OrthancServer/Plugins/Engine/OrthancPluginDatabase.cpp OrthancServer/Plugins/Engine/OrthancPluginDatabaseV3.cpp OrthancServer/Plugins/Engine/OrthancPlugins.cpp OrthancServer/Sources/Database/IDatabaseWrapper.h OrthancServer/Sources/Database/SQLiteDatabaseWrapper.cpp OrthancServer/Sources/Database/StatelessDatabaseOperations.cpp OrthancServer/Sources/Database/StatelessDatabaseOperations.h OrthancServer/Sources/OrthancConfiguration.cpp OrthancServer/Sources/OrthancRestApi/OrthancRestAnonymizeModify.cpp OrthancServer/Sources/ServerContext.cpp OrthancServer/UnitTestsSources/ServerIndexTests.cpp
diffstat 11 files changed, 111 insertions(+), 56 deletions(-) [+]
line wrap: on
line diff
--- 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<int32_t>(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_)
--- 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<int32_t>(property)));
+      const char* id = (shared ? "" : that_.serverIdentifier_.c_str());
+      
+      CheckSuccess(that_.backend_.lookupGlobalProperty(transaction_, id, static_cast<int32_t>(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<int32_t>(property), value.c_str()));
+      const char* id = (shared ? "" : that_.serverIdentifier_.c_str());
+      
+      CheckSuccess(that_.backend_.setGlobalProperty(transaction_, id, static_cast<int32_t>(property), value.c_str()));
       CheckNoEvent();
     }
 
--- 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<GlobalProperty>(p.property), p.value);
+          lock.GetContext().GetIndex().SetGlobalProperty(static_cast<GlobalProperty>(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<GlobalProperty>(p.property), p.value);
+          result = lock.GetContext().GetIndex().GetGlobalProperty(static_cast<GlobalProperty>(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<const _OrthancPluginRegisterDatabaseBackend*>(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<const _OrthancPluginRegisterDatabaseBackendV2*>(parameters);
--- 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;
--- 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";
--- 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<bool&, std::string&, GlobalProperty>
+    class Operations : public ReadOnlyOperationsT4<bool&, std::string&, GlobalProperty, bool>
     {
     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<std::string>(newValue_));
+        transaction.SetGlobalProperty(sequence_, shared_, boost::lexical_cast<std::string>(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);
   }
 
--- 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,
--- 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
     {
--- 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<std::string>(seq);
   }
 
--- 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)
       {
--- 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<GlobalProperty>(42)));
+  ASSERT_TRUE(transaction_->LookupGlobalProperty(s, GlobalProperty_FlushSleep, true));
+  ASSERT_FALSE(transaction_->LookupGlobalProperty(s, static_cast<GlobalProperty>(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();