changeset 4589:bec74e29f86b db-changes

attaching the listener to transactions in IDatabaseWrapper
author Sebastien Jodogne <s.jodogne@gmail.com>
date Fri, 12 Mar 2021 15:33:47 +0100
parents 94147ce2f097
children 4a0bf1019335
files OrthancServer/Plugins/Engine/OrthancPluginDatabase.cpp OrthancServer/Plugins/Engine/OrthancPluginDatabase.h OrthancServer/Sources/Database/IDatabaseWrapper.h OrthancServer/Sources/Database/SQLiteDatabaseWrapper.cpp OrthancServer/Sources/Database/SQLiteDatabaseWrapper.h OrthancServer/Sources/Database/StatelessDatabaseOperations.cpp OrthancServer/Sources/Database/StatelessDatabaseOperations.h OrthancServer/Sources/OrthancRestApi/OrthancRestResources.cpp OrthancServer/Sources/ServerIndex.cpp OrthancServer/UnitTestsSources/ServerIndexTests.cpp
diffstat 10 files changed, 219 insertions(+), 147 deletions(-) [+]
line wrap: on
line diff
--- a/OrthancServer/Plugins/Engine/OrthancPluginDatabase.cpp	Wed Mar 10 17:15:01 2021 +0100
+++ b/OrthancServer/Plugins/Engine/OrthancPluginDatabase.cpp	Fri Mar 12 15:33:47 2021 +0100
@@ -62,9 +62,18 @@
     }
 
   public:
-    explicit Transaction(OrthancPluginDatabase& that) :
+    explicit Transaction(OrthancPluginDatabase& that,
+                         IDatabaseListener& listener) :
       that_(that)
     {
+      assert(that_.listener_ == NULL);
+      that_.listener_ = &listener;   // TODO - STORE IN TRANSACTION
+    }
+
+    virtual ~Transaction()
+    {
+      assert(that_.listener_ != NULL);
+      that_.listener_ = NULL;   // TODO - STORE IN TRANSACTION
     }
 
     void Begin()
@@ -310,12 +319,39 @@
   }
 
 
+  namespace
+  {
+    class VoidListener : public IDatabaseListener
+    {
+    public:
+      virtual void SignalRemainingAncestor(ResourceType parentType,
+                                           const std::string& publicId)
+      {
+        throw OrthancException(ErrorCode_InternalError);  // Should be read-only transaction
+      }
+      
+      virtual void SignalAttachmentDeleted(const FileInfo& info)
+      {
+        throw OrthancException(ErrorCode_InternalError);  // Should be read-only transaction
+      }
+
+      virtual void SignalResourceDeleted(ResourceType type,
+                                         const std::string& publicId)
+      {
+        throw OrthancException(ErrorCode_InternalError);  // Should be read-only transaction
+      }      
+    };
+  }
+
+
   void OrthancPluginDatabase::Open()
   {
     CheckSuccess(backend_.open(payload_));
 
+    VoidListener listener;
+    
     {
-      Transaction transaction(*this);
+      Transaction transaction(*this, listener);
       transaction.Begin();
 
       std::string tmp;
@@ -889,11 +925,12 @@
   }
 
 
-  IDatabaseWrapper::ITransaction* OrthancPluginDatabase::StartTransaction(TransactionType type)
+  IDatabaseWrapper::ITransaction* OrthancPluginDatabase::StartTransaction(TransactionType type,
+                                                                          IDatabaseListener& listener)
   {
     // TODO - Take advantage of "type"
-    
-    std::unique_ptr<Transaction> transaction(new Transaction(*this));
+
+    std::unique_ptr<Transaction> transaction(new Transaction(*this, listener));
     transaction->Begin();
     return transaction.release();
   }
@@ -953,9 +990,11 @@
   void OrthancPluginDatabase::Upgrade(unsigned int targetVersion,
                                       IStorageArea& storageArea)
   {
+    VoidListener listener;
+    
     if (extensions_.upgradeDatabase != NULL)
     {
-      Transaction transaction(*this);
+      Transaction transaction(*this, listener);
       transaction.Begin();
 
       OrthancPluginErrorCode code = extensions_.upgradeDatabase(
--- a/OrthancServer/Plugins/Engine/OrthancPluginDatabase.h	Wed Mar 10 17:15:01 2021 +0100
+++ b/OrthancServer/Plugins/Engine/OrthancPluginDatabase.h	Fri Mar 12 15:33:47 2021 +0100
@@ -297,15 +297,10 @@
                                      bool isProtected) 
       ORTHANC_OVERRIDE;
 
-    virtual IDatabaseWrapper::ITransaction* StartTransaction(TransactionType type)
+    virtual IDatabaseWrapper::ITransaction* StartTransaction(TransactionType type,
+                                                             IDatabaseListener& listener)
       ORTHANC_OVERRIDE;
 
-    virtual void SetListener(IDatabaseListener& listener) 
-      ORTHANC_OVERRIDE
-    {
-      listener_ = &listener;
-    }
-
     virtual unsigned int GetDatabaseVersion() 
       ORTHANC_OVERRIDE;
 
--- a/OrthancServer/Sources/Database/IDatabaseWrapper.h	Wed Mar 10 17:15:01 2021 +0100
+++ b/OrthancServer/Sources/Database/IDatabaseWrapper.h	Fri Mar 12 15:33:47 2021 +0100
@@ -199,9 +199,8 @@
     virtual void SetProtectedPatient(int64_t internalId, 
                                      bool isProtected) = 0;
 
-    virtual ITransaction* StartTransaction(TransactionType type) = 0;
-
-    virtual void SetListener(IDatabaseListener& listener) = 0;
+    virtual ITransaction* StartTransaction(TransactionType type,
+                                           IDatabaseListener& listener) = 0;
 
     virtual unsigned int GetDatabaseVersion() = 0;
 
--- a/OrthancServer/Sources/Database/SQLiteDatabaseWrapper.cpp	Wed Mar 10 17:15:01 2021 +0100
+++ b/OrthancServer/Sources/Database/SQLiteDatabaseWrapper.cpp	Fri Mar 12 15:33:47 2021 +0100
@@ -47,30 +47,30 @@
 
 namespace Orthanc
 {
-  namespace Internals
+  class SQLiteDatabaseWrapper::SignalFileDeleted : public SQLite::IScalarFunction
   {
-    class SignalFileDeleted : public SQLite::IScalarFunction
+  private:
+    SQLiteDatabaseWrapper& sqlite_;
+
+  public:
+    SignalFileDeleted(SQLiteDatabaseWrapper& sqlite) :
+      sqlite_(sqlite)
     {
-    private:
-      IDatabaseListener& listener_;
+    }
 
-    public:
-      SignalFileDeleted(IDatabaseListener& listener) :
-        listener_(listener)
-      {
-      }
+    virtual const char* GetName() const ORTHANC_OVERRIDE
+    {
+      return "SignalFileDeleted";
+    }
 
-      virtual const char* GetName() const ORTHANC_OVERRIDE
-      {
-        return "SignalFileDeleted";
-      }
+    virtual unsigned int GetCardinality() const ORTHANC_OVERRIDE
+    {
+      return 7;
+    }
 
-      virtual unsigned int GetCardinality() const ORTHANC_OVERRIDE
-      {
-        return 7;
-      }
-
-      virtual void Compute(SQLite::FunctionContext& context) ORTHANC_OVERRIDE
+    virtual void Compute(SQLite::FunctionContext& context) ORTHANC_OVERRIDE
+    {
+      if (sqlite_.listener_ != NULL)
       {
         std::string uncompressedMD5, compressedMD5;
 
@@ -91,100 +91,105 @@
                       static_cast<CompressionType>(context.GetIntValue(3)),
                       static_cast<uint64_t>(context.GetInt64Value(4)),
                       compressedMD5);
-        
-        listener_.SignalAttachmentDeleted(info);
-      }
-    };
 
-    class SignalResourceDeleted : public SQLite::IScalarFunction
-    {
-    private:
-      IDatabaseListener& listener_;
+        sqlite_.listener_->SignalAttachmentDeleted(info);
+      }
+    }
+  };
+    
+
+  class SQLiteDatabaseWrapper::SignalResourceDeleted : public SQLite::IScalarFunction
+  {
+  private:
+    SQLiteDatabaseWrapper& sqlite_;
 
-    public:
-      SignalResourceDeleted(IDatabaseListener& listener) :
-        listener_(listener)
-      {
-      }
+  public:
+    SignalResourceDeleted(SQLiteDatabaseWrapper& sqlite) :
+      sqlite_(sqlite)
+    {
+    }
 
-      virtual const char* GetName() const ORTHANC_OVERRIDE
-      {
-        return "SignalResourceDeleted";
-      }
+    virtual const char* GetName() const ORTHANC_OVERRIDE
+    {
+      return "SignalResourceDeleted";
+    }
 
-      virtual unsigned int GetCardinality() const ORTHANC_OVERRIDE
+    virtual unsigned int GetCardinality() const ORTHANC_OVERRIDE
+    {
+      return 2;
+    }
+
+    virtual void Compute(SQLite::FunctionContext& context) ORTHANC_OVERRIDE
+    {
+      if (sqlite_.listener_ != NULL)
       {
-        return 2;
+        sqlite_.listener_->SignalResourceDeleted(static_cast<ResourceType>(context.GetIntValue(1)),
+                                                 context.GetStringValue(0));
       }
-
-      virtual void Compute(SQLite::FunctionContext& context) ORTHANC_OVERRIDE
-      {
-        listener_.SignalResourceDeleted(static_cast<ResourceType>(context.GetIntValue(1)),
-                                        context.GetStringValue(0));
-      }
-    };
+    }
+  };
 
-    class SignalRemainingAncestor : public SQLite::IScalarFunction
-    {
-    private:
-      bool hasRemainingAncestor_;
-      std::string remainingPublicId_;
-      ResourceType remainingType_;
-
-    public:
-      SignalRemainingAncestor() : 
-        hasRemainingAncestor_(false)
-      {
-      }
+  
+  class SQLiteDatabaseWrapper::SignalRemainingAncestor : public SQLite::IScalarFunction
+  {
+  private:
+    bool hasRemainingAncestor_;
+    std::string remainingPublicId_;
+    ResourceType remainingType_;
 
-      void Reset()
-      {
-        hasRemainingAncestor_ = false;
-      }
+  public:
+    SignalRemainingAncestor() : 
+      hasRemainingAncestor_(false)
+    {
+    }
 
-      virtual const char* GetName() const ORTHANC_OVERRIDE
-      {
-        return "SignalRemainingAncestor";
-      }
+    void Reset()
+    {
+      hasRemainingAncestor_ = false;
+    }
 
-      virtual unsigned int GetCardinality() const ORTHANC_OVERRIDE
-      {
-        return 2;
-      }
+    virtual const char* GetName() const ORTHANC_OVERRIDE
+    {
+      return "SignalRemainingAncestor";
+    }
 
-      virtual void Compute(SQLite::FunctionContext& context) ORTHANC_OVERRIDE
-      {
-        CLOG(TRACE, SQLITE) << "There exists a remaining ancestor with public ID \""
-                            << context.GetStringValue(0) << "\" of type "
-                            << context.GetIntValue(1);
+    virtual unsigned int GetCardinality() const ORTHANC_OVERRIDE
+    {
+      return 2;
+    }
+
+    virtual void Compute(SQLite::FunctionContext& context) ORTHANC_OVERRIDE
+    {
+      CLOG(TRACE, SQLITE) << "There exists a remaining ancestor with public ID \""
+                          << context.GetStringValue(0) << "\" of type "
+                          << context.GetIntValue(1);
 
-        if (!hasRemainingAncestor_ ||
-            remainingType_ >= context.GetIntValue(1))
-        {
-          hasRemainingAncestor_ = true;
-          remainingPublicId_ = context.GetStringValue(0);
-          remainingType_ = static_cast<ResourceType>(context.GetIntValue(1));
-        }
+      if (!hasRemainingAncestor_ ||
+          remainingType_ >= context.GetIntValue(1))
+      {
+        hasRemainingAncestor_ = true;
+        remainingPublicId_ = context.GetStringValue(0);
+        remainingType_ = static_cast<ResourceType>(context.GetIntValue(1));
       }
-
-      bool HasRemainingAncestor() const
-      {
-        return hasRemainingAncestor_;
-      }
+    }
 
-      const std::string& GetRemainingAncestorId() const
-      {
-        assert(hasRemainingAncestor_);
-        return remainingPublicId_;
-      }
+    bool HasRemainingAncestor() const
+    {
+      return hasRemainingAncestor_;
+    }
 
-      ResourceType GetRemainingAncestorType() const
-      {
-        assert(hasRemainingAncestor_);
-        return remainingType_;
-      }
-    };
-  }
+    const std::string& GetRemainingAncestorId() const
+    {
+      assert(hasRemainingAncestor_);
+      return remainingPublicId_;
+    }
+
+    ResourceType GetRemainingAncestorType() const
+    {
+      assert(hasRemainingAncestor_);
+      return remainingType_;
+    }
+  };
 
 
   void SQLiteDatabaseWrapper::GetChangesInternal(std::list<ServerIndexChange>& target,
@@ -436,8 +441,9 @@
       t.Commit();
     }
 
-    signalRemainingAncestor_ = new Internals::SignalRemainingAncestor;
-    db_.Register(signalRemainingAncestor_);
+    signalRemainingAncestor_ = dynamic_cast<SignalRemainingAncestor*>(db_.Register(new SignalRemainingAncestor));
+    db_.Register(new SignalFileDeleted(*this));
+    db_.Register(new SignalResourceDeleted(*this));
   }
 
 
@@ -503,14 +509,6 @@
   }
 
 
-  void SQLiteDatabaseWrapper::SetListener(IDatabaseListener& listener)
-  {
-    listener_ = &listener;
-    db_.Register(new Internals::SignalFileDeleted(listener));
-    db_.Register(new Internals::SignalResourceDeleted(listener));
-  }
-
-
   void SQLiteDatabaseWrapper::ClearTable(const std::string& tableName)
   {
     db_.Execute("DELETE FROM " + tableName);    
@@ -603,10 +601,14 @@
     int64_t                               initialDiskSize_;
 
   public:
-    ReadWriteTransaction(SQLiteDatabaseWrapper& that) :
+    ReadWriteTransaction(SQLiteDatabaseWrapper& that,
+                         IDatabaseListener& listener) :
       that_(that),
       transaction_(new SQLite::Transaction(that_.db_))
     {
+      assert(that_.listener_ == NULL);
+      that_.listener_ = &listener;   // TODO - STORE IN TRANSACTION
+
 #if defined(NDEBUG)
       // Release mode
       initialDiskSize_ = 0;
@@ -616,6 +618,12 @@
 #endif
     }
 
+    virtual ~ReadWriteTransaction()
+    {
+      assert(that_.listener_ != NULL);    
+      that_.listener_ = NULL;   // TODO - STORE IN TRANSACTION
+    }
+
     void Begin()
     {
       transaction_->Begin();
@@ -638,7 +646,24 @@
 
   class SQLiteDatabaseWrapper::ReadOnlyTransaction : public IDatabaseWrapper::ITransaction
   {
+  private:
+    SQLiteDatabaseWrapper&  that_;
+    
   public:
+    ReadOnlyTransaction(SQLiteDatabaseWrapper& that,
+                        IDatabaseListener& listener) :
+      that_(that)
+    {
+      assert(that_.listener_ == NULL);
+      that_.listener_ = &listener;
+    }
+
+    virtual ~ReadOnlyTransaction()
+    {
+      assert(that_.listener_ != NULL);    
+      that_.listener_ = NULL;
+    }
+
     virtual void Rollback() ORTHANC_OVERRIDE
     {
     }
@@ -653,17 +678,18 @@
   };
 
 
-  IDatabaseWrapper::ITransaction* SQLiteDatabaseWrapper::StartTransaction(TransactionType type)
+  IDatabaseWrapper::ITransaction* SQLiteDatabaseWrapper::StartTransaction(TransactionType type,
+                                                                          IDatabaseListener& listener)
   {
     switch (type)
     {
       case TransactionType_ReadOnly:
-        return new ReadOnlyTransaction;  // This is a no-op transaction in SQLite (thanks to mutex)
+        return new ReadOnlyTransaction(*this, listener);  // This is a no-op transaction in SQLite (thanks to mutex)
 
       case TransactionType_ReadWrite:
       {
         std::unique_ptr<ReadWriteTransaction> transaction;
-        transaction.reset(new ReadWriteTransaction(*this));
+        transaction.reset(new ReadWriteTransaction(*this, listener));
         transaction->Begin();
         return transaction.release();
       }
--- a/OrthancServer/Sources/Database/SQLiteDatabaseWrapper.h	Wed Mar 10 17:15:01 2021 +0100
+++ b/OrthancServer/Sources/Database/SQLiteDatabaseWrapper.h	Fri Mar 12 15:33:47 2021 +0100
@@ -43,11 +43,6 @@
 
 namespace Orthanc
 {
-  namespace Internals
-  {
-    class SignalRemainingAncestor;
-  }
-
   /**
    * This class manages an instance of the Orthanc SQLite database. It
    * translates low-level requests into SQL statements. Mutual
@@ -61,13 +56,16 @@
     public Compatibility::ISetResourcesContent
   {
   private:
+    class SignalFileDeleted;
+    class SignalResourceDeleted;
+    class SignalRemainingAncestor;
     class ReadOnlyTransaction;
     class ReadWriteTransaction;
     class LookupFormatter;
 
     IDatabaseListener* listener_;
     SQLite::Connection db_;
-    Internals::SignalRemainingAncestor* signalRemainingAncestor_;
+    SignalRemainingAncestor* signalRemainingAncestor_;
     unsigned int version_;
 
     void GetChangesInternal(std::list<ServerIndexChange>& target,
@@ -100,9 +98,6 @@
       db_.Close();
     }
 
-    virtual void SetListener(IDatabaseListener& listener)
-      ORTHANC_OVERRIDE;
-
     virtual bool LookupParent(int64_t& parentId,
                               int64_t resourceId)
       ORTHANC_OVERRIDE;
@@ -125,7 +120,8 @@
     virtual void GetLastChange(std::list<ServerIndexChange>& target /*out*/)
       ORTHANC_OVERRIDE;
 
-    virtual IDatabaseWrapper::ITransaction* StartTransaction(TransactionType type)
+    virtual IDatabaseWrapper::ITransaction* StartTransaction(TransactionType type,
+                                                             IDatabaseListener& listener)
       ORTHANC_OVERRIDE;
 
     virtual void FlushToDisk()
--- a/OrthancServer/Sources/Database/StatelessDatabaseOperations.cpp	Wed Mar 10 17:15:01 2021 +0100
+++ b/OrthancServer/Sources/Database/StatelessDatabaseOperations.cpp	Fri Mar 12 15:33:47 2021 +0100
@@ -533,7 +533,7 @@
         throw OrthancException(ErrorCode_NullPointer);
       }      
       
-      transaction_.reset(db_.StartTransaction(type));
+      transaction_.reset(db_.StartTransaction(type, *context_));
       if (transaction_.get() == NULL)
       {
         throw OrthancException(ErrorCode_NullPointer);
--- a/OrthancServer/Sources/Database/StatelessDatabaseOperations.h	Wed Mar 10 17:15:01 2021 +0100
+++ b/OrthancServer/Sources/Database/StatelessDatabaseOperations.h	Fri Mar 12 15:33:47 2021 +0100
@@ -53,7 +53,7 @@
     typedef std::list<FileInfo> Attachments;
     typedef std::map<std::pair<ResourceType, MetadataType>, std::string>  MetadataMap;
 
-    class ITransactionContext : public boost::noncopyable
+    class ITransactionContext : public IDatabaseListener
     {
     public:
       virtual ~ITransactionContext()
--- a/OrthancServer/Sources/OrthancRestApi/OrthancRestResources.cpp	Wed Mar 10 17:15:01 2021 +0100
+++ b/OrthancServer/Sources/OrthancRestApi/OrthancRestResources.cpp	Fri Mar 12 15:33:47 2021 +0100
@@ -51,8 +51,6 @@
 #include "../ServerToolbox.h"
 #include "../SliceOrdering.h"
 
-#include "../../Plugins/Engine/OrthancPlugins.h"
-
 // This "include" is mandatory for Release builds using Linux Standard Base
 #include <boost/math/special_functions/round.hpp>
 
--- a/OrthancServer/Sources/ServerIndex.cpp	Wed Mar 10 17:15:01 2021 +0100
+++ b/OrthancServer/Sources/ServerIndex.cpp	Fri Mar 12 15:33:47 2021 +0100
@@ -51,8 +51,7 @@
 
 namespace Orthanc
 {
-  class ServerIndex::Listener : public IDatabaseListener,
-                                public ServerIndex::ITransactionContext
+  class ServerIndex::Listener : public StatelessDatabaseOperations::ITransactionContext
   {
   private:
     struct FileToRemove
@@ -333,6 +332,23 @@
       {
         return listener_.GetCompressedSizeDelta();
       }
+
+      virtual void SignalRemainingAncestor(ResourceType parentType,
+                                           const std::string& publicId) ORTHANC_OVERRIDE
+      {
+        listener_.SignalRemainingAncestor(parentType, publicId);
+      }
+
+      virtual void SignalAttachmentDeleted(const FileInfo& info) ORTHANC_OVERRIDE
+      {
+        listener_.SignalAttachmentDeleted(info);
+      }
+
+      virtual void SignalResourceDeleted(ResourceType type,
+                                         const std::string& publicId) ORTHANC_OVERRIDE
+      {
+        listener_.SignalResourceDeleted(type, publicId);
+      }
     };
 
     ServerIndex& index_;
@@ -437,7 +453,6 @@
     maximumPatients_(0)
   {
     listener_.reset(new Listener(context));
-    db.SetListener(*listener_);
 
     SetTransactionContextFactory(new TransactionContextFactory(*this));
 
--- a/OrthancServer/UnitTestsSources/ServerIndexTests.cpp	Wed Mar 10 17:15:01 2021 +0100
+++ b/OrthancServer/UnitTestsSources/ServerIndexTests.cpp	Fri Mar 12 15:33:47 2021 +0100
@@ -83,7 +83,7 @@
     }       
 
     virtual void SignalResourceDeleted(ResourceType type,
-                                       const std::string& publicId)
+                                       const std::string& publicId) ORTHANC_OVERRIDE
     {
       LOG(INFO) << "Deleted resource " << publicId << " of type " << EnumerationToString(type);
       deletedResources_.push_back(publicId);
@@ -96,6 +96,7 @@
   protected:
     std::unique_ptr<TestDatabaseListener>  listener_;
     std::unique_ptr<SQLiteDatabaseWrapper> index_;
+    std::unique_ptr<IDatabaseWrapper::ITransaction>  transaction_;
 
   public:
     DatabaseWrapperTest()
@@ -106,12 +107,15 @@
     {
       listener_.reset(new TestDatabaseListener);
       index_.reset(new SQLiteDatabaseWrapper);
-      index_->SetListener(*listener_);
       index_->Open();
+      transaction_.reset(index_->StartTransaction(TransactionType_ReadWrite, *listener_));
     }
 
     virtual void TearDown() ORTHANC_OVERRIDE
     {
+      transaction_->Commit(0);
+      transaction_.reset();
+      
       index_->Close();
       index_.reset(NULL);
       listener_.reset(NULL);