Mercurial > hg > orthanc-databases

changeset 230:675f8322eb7c

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
refactored StorageBackend by introducing an accessor class
author Sebastien Jodogne <s.jodogne@gmail.com>
date Tue, 06 Apr 2021 11:17:00 +0200
parents 5744f09b0b1b
children 0a9b48d19643
files Framework/Plugins/StorageBackend.cpp Framework/Plugins/StorageBackend.h MySQL/UnitTests/UnitTestsMain.cpp PostgreSQL/UnitTests/PostgreSQLTests.cpp
diffstat 4 files changed, 217 insertions(+), 189 deletions(-) [+]
line wrap: on
line diff
--- a/Framework/Plugins/StorageBackend.cpp	Sat Apr 03 18:56:14 2021 +0200
+++ b/Framework/Plugins/StorageBackend.cpp	Tue Apr 06 11:17:00 2021 +0200
@@ -84,92 +84,112 @@
   }
     
 
-  void StorageBackend::Create(DatabaseManager::Transaction& transaction,
-                              const std::string& uuid,
-                              const void* content,
-                              size_t size,
-                              OrthancPluginContentType type)
+  void StorageBackend::Accessor::Create(const std::string& uuid,
+                                        const void* content,
+                                        size_t size,
+                                        OrthancPluginContentType type)
   {
-    DatabaseManager::CachedStatement statement(
-      STATEMENT_FROM_HERE, GetManager(),
-      "INSERT INTO StorageArea VALUES (${uuid}, ${content}, ${type})");
+    DatabaseManager::Transaction transaction(manager_, TransactionType_ReadWrite);
+
+    {
+      DatabaseManager::CachedStatement statement(
+        STATEMENT_FROM_HERE, manager_,
+        "INSERT INTO StorageArea VALUES (${uuid}, ${content}, ${type})");
      
-    statement.SetParameterType("uuid", ValueType_Utf8String);
-    statement.SetParameterType("content", ValueType_File);
-    statement.SetParameterType("type", ValueType_Integer64);
+      statement.SetParameterType("uuid", ValueType_Utf8String);
+      statement.SetParameterType("content", ValueType_File);
+      statement.SetParameterType("type", ValueType_Integer64);
 
-    Dictionary args;
-    args.SetUtf8Value("uuid", uuid);
-    args.SetFileValue("content", content, size);
-    args.SetIntegerValue("type", type);
+      Dictionary args;
+      args.SetUtf8Value("uuid", uuid);
+      args.SetFileValue("content", content, size);
+      args.SetIntegerValue("type", type);
      
-    statement.Execute(args);
+      statement.Execute(args);
+    }
+
+    transaction.Commit();
   }
 
 
-  void StorageBackend::Read(StorageBackend::IFileContentVisitor& target,
-                            DatabaseManager::Transaction& transaction, 
-                            const std::string& uuid,
-                            OrthancPluginContentType type) 
+  void StorageBackend::Accessor::Read(StorageBackend::IFileContentVisitor& visitor,
+                                      const std::string& uuid,
+                                      OrthancPluginContentType type) 
   {
-    DatabaseManager::CachedStatement statement(
-      STATEMENT_FROM_HERE, GetManager(),
-      "SELECT content FROM StorageArea WHERE uuid=${uuid} AND type=${type}");
+    DatabaseManager::Transaction transaction(manager_, TransactionType_ReadOnly);
+
+    {
+      DatabaseManager::CachedStatement statement(
+        STATEMENT_FROM_HERE, manager_,
+        "SELECT content FROM StorageArea WHERE uuid=${uuid} AND type=${type}");
+     
+      statement.SetParameterType("uuid", ValueType_Utf8String);
+      statement.SetParameterType("type", ValueType_Integer64);
+
+      Dictionary args;
+      args.SetUtf8Value("uuid", uuid);
+      args.SetIntegerValue("type", type);
      
-    statement.SetParameterType("uuid", ValueType_Utf8String);
-    statement.SetParameterType("type", ValueType_Integer64);
+      statement.Execute(args);
 
-    Dictionary args;
-    args.SetUtf8Value("uuid", uuid);
-    args.SetIntegerValue("type", type);
-     
-    statement.Execute(args);
+      if (statement.IsDone())
+      {
+        throw Orthanc::OrthancException(Orthanc::ErrorCode_UnknownResource);
+      }
+      else if (statement.GetResultFieldsCount() != 1)
+      {
+        throw Orthanc::OrthancException(Orthanc::ErrorCode_Database);        
+      }
+      else
+      {
+        const IValue& value = statement.GetResultField(0);
+      
+        switch (value.GetType())
+        {
+          case ValueType_File:
+            visitor.Assign(dynamic_cast<const FileValue&>(value).GetContent());
+            break;
 
-    if (statement.IsDone())
-    {
-      throw Orthanc::OrthancException(Orthanc::ErrorCode_UnknownResource);
+          case ValueType_BinaryString:
+            visitor.Assign(dynamic_cast<const BinaryStringValue&>(value).GetContent());
+            break;
+
+          default:
+            throw Orthanc::OrthancException(Orthanc::ErrorCode_Database);        
+        }
+      }
     }
-    else if (statement.GetResultFieldsCount() != 1)
-    {
-      throw Orthanc::OrthancException(Orthanc::ErrorCode_Database);        
-    }
-    else
+
+    transaction.Commit();
+
+    if (!visitor.IsSuccess())
     {
-      const IValue& value = statement.GetResultField(0);
-      
-      switch (value.GetType())
-      {
-        case ValueType_File:
-          target.Assign(dynamic_cast<const FileValue&>(value).GetContent());
-          break;
-
-        case ValueType_BinaryString:
-          target.Assign(dynamic_cast<const BinaryStringValue&>(value).GetContent());
-          break;
-
-        default:
-          throw Orthanc::OrthancException(Orthanc::ErrorCode_Database);        
-      }
+      throw Orthanc::OrthancException(Orthanc::ErrorCode_Database, "Could not read attachment from the storage area");
     }
   }
 
 
-  void StorageBackend::Remove(DatabaseManager::Transaction& transaction,
-                              const std::string& uuid,
-                              OrthancPluginContentType type)
+  void StorageBackend::Accessor::Remove(const std::string& uuid,
+                                        OrthancPluginContentType type)
   {
-    DatabaseManager::CachedStatement statement(
-      STATEMENT_FROM_HERE, GetManager(),
-      "DELETE FROM StorageArea WHERE uuid=${uuid} AND type=${type}");
+    DatabaseManager::Transaction transaction(manager_, TransactionType_ReadWrite);
+
+    {
+      DatabaseManager::CachedStatement statement(
+        STATEMENT_FROM_HERE, manager_,
+        "DELETE FROM StorageArea WHERE uuid=${uuid} AND type=${type}");
      
-    statement.SetParameterType("uuid", ValueType_Utf8String);
-    statement.SetParameterType("type", ValueType_Integer64);
+      statement.SetParameterType("uuid", ValueType_Utf8String);
+      statement.SetParameterType("type", ValueType_Integer64);
 
-    Dictionary args;
-    args.SetUtf8Value("uuid", uuid);
-    args.SetIntegerValue("type", type);
+      Dictionary args;
+      args.SetUtf8Value("uuid", uuid);
+      args.SetIntegerValue("type", type);
      
-    statement.Execute(args);
+      statement.Execute(args);
+    }
+      
+    transaction.Commit();
   }
 
 
@@ -189,11 +209,12 @@
       {
         throw Orthanc::OrthancException(Orthanc::ErrorCode_BadSequenceOfCalls);
       }
-
-      DatabaseManager::Transaction transaction(backend_->GetManager(), TransactionType_ReadWrite);
-      backend_->Create(transaction, uuid, content, static_cast<size_t>(size), type);
-      transaction.Commit();
-      return OrthancPluginErrorCode_Success;
+      else
+      {
+        StorageBackend::Accessor accessor(*backend_);
+        accessor.Create(uuid, content, static_cast<size_t>(size), type);
+        return OrthancPluginErrorCode_Success;
+      }
     }
     ORTHANC_PLUGINS_DATABASE_CATCH;
   }
@@ -217,7 +238,7 @@
       {
       }
 
-      bool IsSuccess() const
+      virtual bool IsSuccess() const ORTHANC_OVERRIDE
       {
         return success_;
       }
@@ -254,27 +275,21 @@
       {
         throw Orthanc::OrthancException(Orthanc::ErrorCode_BadSequenceOfCalls);
       }
-      
-      if (target == NULL)
+      else if (target == NULL)
       {
         throw Orthanc::OrthancException(Orthanc::ErrorCode_NullPointer);
       }
-      
-      Visitor visitor(target);
-      
+      else
       {
-        DatabaseManager::Transaction transaction(backend_->GetManager(), TransactionType_ReadOnly);
-        backend_->Read(visitor, transaction, uuid, type);
+        Visitor visitor(target);
 
-        if (!visitor.IsSuccess())
         {
-          throw Orthanc::OrthancException(Orthanc::ErrorCode_Database);          
+          StorageBackend::Accessor accessor(*backend_);
+          accessor.Read(visitor, uuid, type);
         }
-        
-        transaction.Commit();
+
+        return OrthancPluginErrorCode_Success;
       }
-      
-      return OrthancPluginErrorCode_Success;
     }
     ORTHANC_PLUGINS_DATABASE_CATCH;
   }
@@ -302,7 +317,7 @@
       {
       }
 
-      bool IsSuccess() const
+      virtual bool IsSuccess() const ORTHANC_OVERRIDE
       {
         return success_;
       }
@@ -351,28 +366,22 @@
       {
         throw Orthanc::OrthancException(Orthanc::ErrorCode_BadSequenceOfCalls);
       }
-      
-      if (data == NULL ||
-          size == NULL)
+      else if (data == NULL ||
+               size == NULL)
       {
         throw Orthanc::OrthancException(Orthanc::ErrorCode_NullPointer);
       }
-      
-      Visitor visitor(data, size);
-
+      else
       {
-        DatabaseManager::Transaction transaction(backend_->GetManager(), TransactionType_ReadOnly);
-        backend_->Read(visitor, transaction, uuid, type);
+        Visitor visitor(data, size);
 
-        if (!visitor.IsSuccess())
         {
-          throw Orthanc::OrthancException(Orthanc::ErrorCode_Database);          
-        }        
-        
-        transaction.Commit();
+          StorageBackend::Accessor accessor(*backend_);
+          accessor.Read(visitor, uuid, type);
+        }
+
+        return OrthancPluginErrorCode_Success;
       }
-
-      return OrthancPluginErrorCode_Success;
     }
     ORTHANC_PLUGINS_DATABASE_CATCH;
   }
@@ -383,10 +392,16 @@
   {
     try
     {
-      DatabaseManager::Transaction transaction(backend_->GetManager(), TransactionType_ReadWrite);
-      backend_->Remove(transaction, uuid, type);
-      transaction.Commit();
-      return OrthancPluginErrorCode_Success;
+      if (backend_.get() == NULL)
+      {
+        throw Orthanc::OrthancException(Orthanc::ErrorCode_BadSequenceOfCalls);
+      }
+      else
+      {
+        StorageBackend::Accessor accessor(*backend_);
+        accessor.Remove(uuid, type);
+        return OrthancPluginErrorCode_Success;
+      }
     }
     ORTHANC_PLUGINS_DATABASE_CATCH;
   }
@@ -440,10 +455,9 @@
   }
 
 
-  void StorageBackend::ReadToString(std::string& target,
-                                    DatabaseManager::Transaction& transaction, 
-                                    const std::string& uuid,
-                                    OrthancPluginContentType type)
+  void StorageBackend::Accessor::ReadToString(std::string& target,
+                                              const std::string& uuid,
+                                              OrthancPluginContentType type)
   {
     class Visitor : public StorageBackend::IFileContentVisitor
     {
@@ -458,7 +472,7 @@
       {
       }
 
-      bool IsSuccess() const
+      virtual bool IsSuccess() const ORTHANC_OVERRIDE
       {
         return success_;
       }
@@ -479,12 +493,6 @@
     
 
     Visitor visitor(target);
-      
-    Read(visitor, transaction, uuid, type);
-      
-    if (!visitor.IsSuccess())
-    {
-      throw Orthanc::OrthancException(Orthanc::ErrorCode_Database);          
-    }
+    Read(visitor, uuid, type);
   }
 }
--- a/Framework/Plugins/StorageBackend.h	Sat Apr 03 18:56:14 2021 +0200
+++ b/Framework/Plugins/StorageBackend.h	Tue Apr 06 11:17:00 2021 +0200
@@ -33,6 +33,11 @@
   private:
     std::unique_ptr<DatabaseManager>   manager_;
 
+    DatabaseManager& GetManager();
+    
+  protected:
+    void SetDatabase(IDatabase* database);  // Takes ownership
+
   public:
     class IFileContentVisitor : public boost::noncopyable
     {
@@ -42,43 +47,46 @@
       }
 
       virtual void Assign(const std::string& content) = 0;
+
+      virtual bool IsSuccess() const = 0;
+    };
+
+    class Accessor : public boost::noncopyable
+    {
+    private:
+      DatabaseManager&  manager_;
+
+    public:
+      Accessor(StorageBackend& backend) :
+        manager_(backend.GetManager())
+      {
+      }
+        
+      void Create(const std::string& uuid,
+                  const void* content,
+                  size_t size,
+                  OrthancPluginContentType type);
+
+      void Read(IFileContentVisitor& visitor,
+                const std::string& uuid,
+                OrthancPluginContentType type);
+
+      void Remove(const std::string& uuid,
+                  OrthancPluginContentType type);
+
+      // For unit tests
+      void ReadToString(std::string& target,
+                        const std::string& uuid,
+                        OrthancPluginContentType type);
     };
     
     virtual ~StorageBackend()
     {
     }
 
-    void SetDatabase(IDatabase* database);  // Takes ownership
-
-    DatabaseManager& GetManager();
-    
-    // NB: These methods will always be invoked in mutual exclusion,
-    // as having access to some "DatabaseManager::Transaction" implies
-    // that the parent "DatabaseManager" is locked
-    virtual void Create(DatabaseManager::Transaction& transaction,
-                        const std::string& uuid,
-                        const void* content,
-                        size_t size,
-                        OrthancPluginContentType type) ORTHANC_FINAL;
-
-    virtual void Read(IFileContentVisitor& target,
-                      DatabaseManager::Transaction& transaction, 
-                      const std::string& uuid,
-                      OrthancPluginContentType type) ORTHANC_FINAL;
-
-    virtual void Remove(DatabaseManager::Transaction& transaction,
-                        const std::string& uuid,
-                        OrthancPluginContentType type) ORTHANC_FINAL;
-
     static void Register(OrthancPluginContext* context,
                          StorageBackend* backend);   // Takes ownership
 
     static void Finalize();
-
-    // For unit tests
-    void ReadToString(std::string& target,
-                      DatabaseManager::Transaction& transaction, 
-                      const std::string& uuid,
-                      OrthancPluginContentType type);
   };
 }
--- a/MySQL/UnitTests/UnitTestsMain.cpp	Sat Apr 03 18:56:14 2021 +0200
+++ b/MySQL/UnitTests/UnitTestsMain.cpp	Tue Apr 06 11:17:00 2021 +0200
@@ -137,41 +137,50 @@
 
 static int64_t CountFiles(OrthancDatabases::MySQLDatabase& db)
 {
-  OrthancDatabases::Query query("SELECT COUNT(*) FROM StorageArea", true);
-  OrthancDatabases::MySQLStatement s(db, query);
-  OrthancDatabases::MySQLTransaction t(db, OrthancDatabases::TransactionType_ReadOnly);
-  OrthancDatabases::Dictionary d;
-  std::unique_ptr<OrthancDatabases::IResult> result(s.Execute(t, d));
-  return dynamic_cast<const OrthancDatabases::Integer64Value&>(result->GetField(0)).GetValue();
+  OrthancDatabases::MySQLTransaction transaction(db, OrthancDatabases::TransactionType_ReadOnly);
+
+  int64_t count;
+  {
+    OrthancDatabases::Query query("SELECT COUNT(*) FROM StorageArea", true);
+    OrthancDatabases::MySQLStatement s(db, query);
+    OrthancDatabases::MySQLTransaction t(db, OrthancDatabases::TransactionType_ReadOnly);
+    OrthancDatabases::Dictionary d;
+    std::unique_ptr<OrthancDatabases::IResult> result(s.Execute(t, d));
+    count = dynamic_cast<const OrthancDatabases::Integer64Value&>(result->GetField(0)).GetValue();
+  }
+
+  transaction.Commit();
+  return count;
 }
 
 
 TEST(MySQL, StorageArea)
 {
-  OrthancDatabases::MySQLStorageArea storageArea(globalParameters_, true);
+  std::unique_ptr<OrthancDatabases::MySQLDatabase> database(
+    OrthancDatabases::MySQLDatabase::OpenDatabaseConnection(globalParameters_));
+  
+  OrthancDatabases::MySQLStorageArea storageArea(globalParameters_, true /* clear database */);
 
   {
-    OrthancDatabases::DatabaseManager::Transaction transaction(storageArea.GetManager(), OrthancDatabases::TransactionType_ReadWrite);
-    OrthancDatabases::MySQLDatabase& db = 
-      dynamic_cast<OrthancDatabases::MySQLDatabase&>(transaction.GetDatabase());
-
-    ASSERT_EQ(0, CountFiles(db));
+    OrthancDatabases::MySQLStorageArea::Accessor accessor(storageArea);
+    
+    ASSERT_EQ(0, CountFiles(*database));
   
     for (int i = 0; i < 10; i++)
     {
       std::string uuid = boost::lexical_cast<std::string>(i);
       std::string value = "Value " + boost::lexical_cast<std::string>(i * 2);
-      storageArea.Create(transaction, uuid, value.c_str(), value.size(), OrthancPluginContentType_Unknown);
+      accessor.Create(uuid, value.c_str(), value.size(), OrthancPluginContentType_Unknown);
     }
 
     std::string buffer;
-    ASSERT_THROW(storageArea.ReadToString(buffer, transaction, "nope", OrthancPluginContentType_Unknown), 
+    ASSERT_THROW(accessor.ReadToString(buffer, "nope", OrthancPluginContentType_Unknown), 
                  Orthanc::OrthancException);
   
-    ASSERT_EQ(10, CountFiles(db));
-    storageArea.Remove(transaction, "5", OrthancPluginContentType_Unknown);
+    ASSERT_EQ(10, CountFiles(*database));
+    accessor.Remove("5", OrthancPluginContentType_Unknown);
 
-    ASSERT_EQ(9, CountFiles(db));
+    ASSERT_EQ(9, CountFiles(*database));
 
     for (int i = 0; i < 10; i++)
     {
@@ -181,25 +190,22 @@
 
       if (i == 5)
       {
-        ASSERT_THROW(storageArea.ReadToString(buffer, transaction, uuid, OrthancPluginContentType_Unknown), 
+        ASSERT_THROW(accessor.ReadToString(buffer, uuid, OrthancPluginContentType_Unknown), 
                      Orthanc::OrthancException);
       }
       else
       {
-        storageArea.ReadToString(buffer, transaction, uuid, OrthancPluginContentType_Unknown);
+        accessor.ReadToString(buffer, uuid, OrthancPluginContentType_Unknown);
         ASSERT_EQ(expected, buffer);
       }
     }
 
     for (int i = 0; i < 10; i++)
     {
-      storageArea.Remove(transaction, boost::lexical_cast<std::string>(i),
-                         OrthancPluginContentType_Unknown);
+      accessor.Remove(boost::lexical_cast<std::string>(i), OrthancPluginContentType_Unknown);
     }
 
-    ASSERT_EQ(0, CountFiles(db));
-
-    transaction.Commit();
+    ASSERT_EQ(0, CountFiles(*database));
   }
 }
 
--- a/PostgreSQL/UnitTests/PostgreSQLTests.cpp	Sat Apr 03 18:56:14 2021 +0200
+++ b/PostgreSQL/UnitTests/PostgreSQLTests.cpp	Tue Apr 06 11:17:00 2021 +0200
@@ -65,10 +65,19 @@
 
 static int64_t CountLargeObjects(PostgreSQLDatabase& db)
 {
-  // Count the number of large objects in the DB
-  PostgreSQLStatement s(db, "SELECT COUNT(*) FROM pg_catalog.pg_largeobject");
-  PostgreSQLResult r(s);
-  return r.GetInteger64(0);
+  PostgreSQLTransaction transaction(db, TransactionType_ReadOnly);
+
+  int64_t count;
+  
+  {
+    // Count the number of large objects in the DB
+    PostgreSQLStatement s(db, "SELECT COUNT(*) FROM pg_catalog.pg_largeobject");
+    PostgreSQLResult r(s);
+    count = r.GetInteger64(0);
+  }
+
+  transaction.Commit();
+  return count;
 }
 
 
@@ -349,30 +358,30 @@
 
 TEST(PostgreSQL, StorageArea)
 {
+  std::unique_ptr<PostgreSQLDatabase> database(PostgreSQLDatabase::OpenDatabaseConnection(globalParameters_));
+  
   PostgreSQLStorageArea storageArea(globalParameters_, true /* clear database */);
 
   {
-    DatabaseManager::Transaction transaction(storageArea.GetManager(), TransactionType_ReadWrite);
-    PostgreSQLDatabase& db = 
-      dynamic_cast<PostgreSQLDatabase&>(transaction.GetDatabase());
-
-    ASSERT_EQ(0, CountLargeObjects(db));
+    PostgreSQLStorageArea::Accessor accessor(storageArea);
+    
+    ASSERT_EQ(0, CountLargeObjects(*database));
   
     for (int i = 0; i < 10; i++)
     {
       std::string uuid = boost::lexical_cast<std::string>(i);
       std::string value = "Value " + boost::lexical_cast<std::string>(i * 2);
-      storageArea.Create(transaction, uuid, value.c_str(), value.size(), OrthancPluginContentType_Unknown);
+      accessor.Create(uuid, value.c_str(), value.size(), OrthancPluginContentType_Unknown);
     }
 
     std::string buffer;
-    ASSERT_THROW(storageArea.ReadToString(buffer, transaction, "nope", OrthancPluginContentType_Unknown), 
+    ASSERT_THROW(accessor.ReadToString(buffer, "nope", OrthancPluginContentType_Unknown), 
                  Orthanc::OrthancException);
   
-    ASSERT_EQ(10, CountLargeObjects(db));
-    storageArea.Remove(transaction, "5", OrthancPluginContentType_Unknown);
+    ASSERT_EQ(10, CountLargeObjects(*database));
+    accessor.Remove("5", OrthancPluginContentType_Unknown);
 
-    ASSERT_EQ(9, CountLargeObjects(db));
+    ASSERT_EQ(9, CountLargeObjects(*database));
 
     for (int i = 0; i < 10; i++)
     {
@@ -381,25 +390,22 @@
 
       if (i == 5)
       {
-        ASSERT_THROW(storageArea.ReadToString(buffer, transaction, uuid, OrthancPluginContentType_Unknown), 
+        ASSERT_THROW(accessor.ReadToString(buffer, uuid, OrthancPluginContentType_Unknown), 
                      Orthanc::OrthancException);
       }
       else
       {
-        storageArea.ReadToString(buffer, transaction, uuid, OrthancPluginContentType_Unknown);
+        accessor.ReadToString(buffer, uuid, OrthancPluginContentType_Unknown);
         ASSERT_EQ(expected, buffer);
       }
     }
 
     for (int i = 0; i < 10; i++)
     {
-      storageArea.Remove(transaction, boost::lexical_cast<std::string>(i),
-                         OrthancPluginContentType_Unknown);
+      accessor.Remove(boost::lexical_cast<std::string>(i), OrthancPluginContentType_Unknown);
     }
 
-    ASSERT_EQ(0, CountLargeObjects(db));
-
-    transaction.Commit();
+    ASSERT_EQ(0, CountLargeObjects(*database));
   }
 }