changeset 70:e6c13ddd26d9 db-changes

all integration tests passing with LookupResources extension
author Sebastien Jodogne <s.jodogne@gmail.com>
date Thu, 03 Jan 2019 14:04:46 +0100
parents 19764fc60ade
children d40c5fecd160
files Framework/Common/DatabaseManager.cpp Framework/Common/DatabaseManager.h Framework/Common/Query.cpp Framework/Plugins/IndexBackend.cpp Framework/Plugins/IndexBackend.h Framework/Plugins/OrthancCppDatabasePlugin.h
diffstat 6 files changed, 328 insertions(+), 143 deletions(-) [+]
line wrap: on
line diff
--- a/Framework/Common/DatabaseManager.cpp	Thu Jan 03 10:07:27 2019 +0100
+++ b/Framework/Common/DatabaseManager.cpp	Thu Jan 03 14:04:46 2019 +0100
@@ -279,34 +279,6 @@
   }
 
 
-  IResult& DatabaseManager::CachedStatement::GetResult() const
-  {
-    if (result_.get() == NULL)
-    {
-      LOG(ERROR) << "Accessing the results of a statement without having executed it";
-      throw Orthanc::OrthancException(Orthanc::ErrorCode_BadSequenceOfCalls);
-    }
-
-    return *result_;
-  }
-
-
-  void DatabaseManager::CachedStatement::Setup(const char* sql)
-  {
-    statement_ = manager_.LookupCachedStatement(location_);
-
-    if (statement_ == NULL)
-    {
-      query_.reset(new Query(sql));
-    }
-    else
-    {
-      LOG(TRACE) << "Reusing cached statement from "
-                 << location_.GetFile() << ":" << location_.GetLine();
-    }
-  }
-
-
   DatabaseManager::Transaction::Transaction(DatabaseManager& manager) :
     lock_(manager.mutex_),
     manager_(manager),
@@ -347,40 +319,72 @@
     }
   }
 
-  
-  DatabaseManager::CachedStatement::CachedStatement(const StatementLocation& location,
-                                                    DatabaseManager& manager,
-                                                    const char* sql) :
-    manager_(manager),
-    lock_(manager_.mutex_),
-    database_(manager_.GetDatabase()),
-    location_(location),
-    transaction_(manager_.GetTransaction())
+
+  IResult& DatabaseManager::StatementBase::GetResult() const
   {
-    Setup(sql);
-  }
+    if (result_.get() == NULL)
+    {
+      LOG(ERROR) << "Accessing the results of a statement without having executed it";
+      throw Orthanc::OrthancException(Orthanc::ErrorCode_BadSequenceOfCalls);
+    }
 
-      
-  DatabaseManager::CachedStatement::CachedStatement(const StatementLocation& location,
-                                                    Transaction& transaction,
-                                                    const char* sql) :
-    manager_(transaction.GetManager()),
-    lock_(manager_.mutex_),
-    database_(manager_.GetDatabase()),
-    location_(location),
-    transaction_(manager_.GetTransaction())
-  {
-    Setup(sql);
+    return *result_;
   }
 
 
-  DatabaseManager::CachedStatement::~CachedStatement()
+  void DatabaseManager::StatementBase::SetQuery(Query* query)
+  {
+    std::auto_ptr<Query> protection(query);
+    
+    if (query_.get() != NULL)
+    {
+      LOG(ERROR) << "Cannot set twice a query";
+      throw Orthanc::OrthancException(Orthanc::ErrorCode_BadSequenceOfCalls);
+    }
+
+    if (query == NULL)
+    {
+      throw Orthanc::OrthancException(Orthanc::ErrorCode_NullPointer);
+    }
+
+    query_.reset(protection.release());
+  }
+
+  
+  void DatabaseManager::StatementBase::SetResult(IResult* result)
+  {
+    std::auto_ptr<IResult> protection(result);
+    
+    if (result_.get() != NULL)
+    {
+      LOG(ERROR) << "Cannot execute twice a statement";
+      throw Orthanc::OrthancException(Orthanc::ErrorCode_BadSequenceOfCalls);
+    }
+
+    if (result == NULL)
+    {
+      throw Orthanc::OrthancException(Orthanc::ErrorCode_NullPointer);
+    }
+
+    result_.reset(protection.release());
+  }
+
+  
+  DatabaseManager::StatementBase::StatementBase(DatabaseManager& manager) :
+    manager_(manager),
+    lock_(manager_.mutex_),
+    transaction_(manager_.GetTransaction())
+  {
+  }
+
+
+  DatabaseManager::StatementBase::~StatementBase()
   {
     manager_.ReleaseImplicitTransaction();
   }
+
   
-      
-  void DatabaseManager::CachedStatement::SetReadOnly(bool readOnly)
+  void DatabaseManager::StatementBase::SetReadOnly(bool readOnly)
   {
     if (query_.get() != NULL)
     {
@@ -389,8 +393,8 @@
   }
 
 
-  void DatabaseManager::CachedStatement::SetParameterType(const std::string& parameter,
-                                                          ValueType type)
+  void DatabaseManager::StatementBase::SetParameterType(const std::string& parameter,
+                                                        ValueType type)
   {
     if (query_.get() != NULL)
     {
@@ -398,44 +402,7 @@
     }
   }
       
-      
-  void DatabaseManager::CachedStatement::Execute()
-  {
-    Dictionary parameters;
-    Execute(parameters);
-  }
-
-
-  void DatabaseManager::CachedStatement::Execute(const Dictionary& parameters)
-  {
-    if (result_.get() != NULL)
-    {
-      LOG(ERROR) << "Cannot execute twice a statement";
-      throw Orthanc::OrthancException(Orthanc::ErrorCode_BadSequenceOfCalls);
-    }
-
-    try
-    {
-      if (query_.get() != NULL)
-      {
-        // Register the newly-created statement
-        assert(statement_ == NULL);
-        statement_ = &manager_.CacheStatement(location_, *query_);
-        query_.reset(NULL);
-      }
-        
-      assert(statement_ != NULL);
-      result_.reset(transaction_.Execute(*statement_, parameters));
-    }
-    catch (Orthanc::OrthancException& e)
-    {
-      manager_.CloseIfUnavailable(e.GetErrorCode());
-      throw;
-    }
-  }
-
-
-  bool DatabaseManager::CachedStatement::IsDone() const
+  bool DatabaseManager::StatementBase::IsDone() const
   {
     try
     {
@@ -449,7 +416,7 @@
   }
 
 
-  void DatabaseManager::CachedStatement::Next()
+  void DatabaseManager::StatementBase::Next()
   {
     try
     {
@@ -463,7 +430,7 @@
   }
 
 
-  size_t DatabaseManager::CachedStatement::GetResultFieldsCount() const
+  size_t DatabaseManager::StatementBase::GetResultFieldsCount() const
   {
     try
     {
@@ -477,8 +444,8 @@
   }
 
 
-  void DatabaseManager::CachedStatement::SetResultFieldType(size_t field,
-                                                            ValueType type)
+  void DatabaseManager::StatementBase::SetResultFieldType(size_t field,
+                                                          ValueType type)
   {
     try
     {
@@ -495,7 +462,7 @@
   }
 
 
-  const IValue& DatabaseManager::CachedStatement::GetResultField(size_t index) const
+  const IValue& DatabaseManager::StatementBase::GetResultField(size_t index) const
   {
     try
     {
@@ -506,5 +473,88 @@
       manager_.CloseIfUnavailable(e.GetErrorCode());
       throw;
     }
+  }  
+  
+  
+  DatabaseManager::CachedStatement::CachedStatement(const StatementLocation& location,
+                                                    DatabaseManager& manager,
+                                                    const std::string& sql) :
+    StatementBase(manager),
+    location_(location)
+  {
+    statement_ = GetManager().LookupCachedStatement(location_);
+
+    if (statement_ == NULL)
+    {
+      SetQuery(new Query(sql));
+    }
+    else
+    {
+      LOG(TRACE) << "Reusing cached statement from "
+                 << location_.GetFile() << ":" << location_.GetLine();
+    }
+  }
+
+      
+  void DatabaseManager::CachedStatement::Execute(const Dictionary& parameters)
+  {
+    try
+    {
+      std::auto_ptr<Query> query(ReleaseQuery());
+      
+      if (query.get() != NULL)
+      {
+        // Register the newly-created statement
+        assert(statement_ == NULL);
+        statement_ = &GetManager().CacheStatement(location_, *query);
+      }
+        
+      assert(statement_ != NULL);
+      SetResult(GetTransaction().Execute(*statement_, parameters));
+    }
+    catch (Orthanc::OrthancException& e)
+    {
+      GetManager().CloseIfUnavailable(e.GetErrorCode());
+      throw;
+    }
+  }
+  
+  
+  DatabaseManager::StandaloneStatement::StandaloneStatement(DatabaseManager& manager,
+                                                            const std::string& sql) :
+    StatementBase(manager)
+  {
+    SetQuery(new Query(sql));
+  }
+
+      
+  DatabaseManager::StandaloneStatement::~StandaloneStatement()
+  {
+    // The result must be removed before the statement, cf. (*)
+    ClearResult();
+    statement_.reset();
+  }
+
+
+  void DatabaseManager::StandaloneStatement::Execute(const Dictionary& parameters)
+  {
+    try
+    {
+      std::auto_ptr<Query> query(ReleaseQuery());
+      assert(query.get() != NULL);
+
+      // The "statement_" object must be kept as long as the "IResult"
+      // is not destroyed, as the "IResult" can make calls to the
+      // statement (this is the case for SQLite and MySQL) - (*)
+      statement_.reset(GetManager().GetDatabase().Compile(*query));
+      assert(statement_.get() != NULL);
+
+      SetResult(GetTransaction().Execute(*statement_, parameters));
+    }
+    catch (Orthanc::OrthancException& e)
+    {
+      GetManager().CloseIfUnavailable(e.GetErrorCode());
+      throw;
+    }
   }
 }
--- a/Framework/Common/DatabaseManager.h	Thu Jan 03 10:07:27 2019 +0100
+++ b/Framework/Common/DatabaseManager.h	Thu Jan 03 14:04:46 2019 +0100
@@ -111,36 +111,51 @@
     };
 
 
-    class CachedStatement : public boost::noncopyable
+    class StatementBase : public boost::noncopyable
     {
     private:
       DatabaseManager&                     manager_;
       boost::recursive_mutex::scoped_lock  lock_;
-      IDatabase&                           database_;
-      StatementLocation                    location_;
       ITransaction&                        transaction_;
-      IPrecompiledStatement*               statement_;
       std::auto_ptr<Query>                 query_;
       std::auto_ptr<IResult>               result_;
 
-      void Setup(const char* sql);
-
       IResult& GetResult() const;
 
-    public:
-      CachedStatement(const StatementLocation& location,
-                      DatabaseManager& manager,
-                      const char* sql);
+    protected:
+      DatabaseManager& GetManager() const
+      {
+        return manager_;
+      }
+
+      ITransaction& GetTransaction() const
+      {
+        return transaction_;
+      }
+      
+      void SetQuery(Query* query);
+
+      void SetResult(IResult* result);
 
-      CachedStatement(const StatementLocation& location,
-                      Transaction& transaction,
-                      const char* sql);
+      void ClearResult()
+      {
+        result_.reset();
+      }
 
-      ~CachedStatement();
+      Query* ReleaseQuery()
+      {
+        return query_.release();
+      }
 
+    public:
+      StatementBase(DatabaseManager& manager);
+
+      virtual ~StatementBase();
+
+      // Used only by SQLite
       IDatabase& GetDatabase()
       {
-        return database_;
+        return manager_.GetDatabase();
       }
 
       void SetReadOnly(bool readOnly);
@@ -148,10 +163,6 @@
       void SetParameterType(const std::string& parameter,
                             ValueType type);
       
-      void Execute();
-
-      void Execute(const Dictionary& parameters);
-
       bool IsDone() const;
       
       void Next();
@@ -163,5 +174,41 @@
       
       const IValue& GetResultField(size_t index) const;
     };
+
+
+    class CachedStatement : public StatementBase
+    {
+    private:
+      StatementLocation       location_;
+      IPrecompiledStatement*  statement_;
+
+    public:
+      CachedStatement(const StatementLocation& location,
+                      DatabaseManager& manager,
+                      const std::string& sql);
+
+      void Execute()
+      {
+        Dictionary parameters;
+        Execute(parameters);
+      }
+
+      void Execute(const Dictionary& parameters);
+    };
+
+
+    class StandaloneStatement : public StatementBase
+    {
+    private:
+      std::auto_ptr<IPrecompiledStatement>  statement_;
+      
+    public:
+      StandaloneStatement(DatabaseManager& manager,
+                          const std::string& sql);
+
+      virtual ~StandaloneStatement();
+
+      void Execute(const Dictionary& parameters);
+    };
   };
 }
--- a/Framework/Common/Query.cpp	Thu Jan 03 10:07:27 2019 +0100
+++ b/Framework/Common/Query.cpp	Thu Jan 03 14:04:46 2019 +0100
@@ -125,8 +125,8 @@
 
     if (found == parameters_.end())
     {
-      LOG(ERROR) << "Inexistent parameter in a SQL query: " << parameter;
-      throw Orthanc::OrthancException(Orthanc::ErrorCode_InexistentItem);
+      throw Orthanc::OrthancException(Orthanc::ErrorCode_InexistentItem,
+                                      "Inexistent parameter in a SQL query: " + parameter);
     }
     else
     {
@@ -142,8 +142,8 @@
 
     if (found == parameters_.end())
     {
-      LOG(ERROR) << "Ignoring inexistent parameter in a SQL query: " << parameter;
-      throw Orthanc::OrthancException(Orthanc::ErrorCode_ParameterOutOfRange);
+      throw Orthanc::OrthancException(Orthanc::ErrorCode_InexistentItem,
+                                      "Inexistent parameter in a SQL query: " + parameter);
     }
     else
     {
--- a/Framework/Plugins/IndexBackend.cpp	Thu Jan 03 10:07:27 2019 +0100
+++ b/Framework/Plugins/IndexBackend.cpp	Thu Jan 03 14:04:46 2019 +0100
@@ -56,7 +56,7 @@
   }
 
   
-  int64_t IndexBackend::ReadInteger64(const DatabaseManager::CachedStatement& statement,
+  int64_t IndexBackend::ReadInteger64(const DatabaseManager::StatementBase& statement,
                                       size_t field)
   {
     if (statement.IsDone())
@@ -78,7 +78,7 @@
   }
 
 
-  int32_t IndexBackend::ReadInteger32(const DatabaseManager::CachedStatement& statement,
+  int32_t IndexBackend::ReadInteger32(const DatabaseManager::StatementBase& statement,
                                       size_t field)
   {
     if (statement.IsDone())
@@ -100,11 +100,11 @@
   }
 
     
-  std::string IndexBackend::ReadString(const DatabaseManager::CachedStatement& statement,
+  std::string IndexBackend::ReadString(const DatabaseManager::StatementBase& statement,
                                        size_t field)
   {
     const IValue& value = statement.GetResultField(field);
-      
+
     switch (value.GetType())
     {
       case ValueType_BinaryString:
@@ -1586,31 +1586,67 @@
   class IndexBackend::LookupFormatter : public Orthanc::ISqlLookupFormatter
   {
   private:
-    Dialect  dialect_;
+    Dialect     dialect_;
+    size_t      count_;
+    Dictionary  dictionary_;
 
+    static std::string FormatParameter(size_t index)
+    {
+      return "p" + boost::lexical_cast<std::string>(index);
+    }
+    
   public:
-    LookupFormatter(Dialect  dialect) :
-      dialect_(dialect)
+    LookupFormatter(Dialect dialect) :
+      dialect_(dialect),
+      count_(0)
     {
     }
 
     virtual std::string GenerateParameter(const std::string& value)
     {
+      const std::string key = FormatParameter(count_);
+
+      count_ ++;
+      dictionary_.SetUtf8Value(key, value);
+
+      return "${" + key + "}";
+    }
+
+    virtual std::string FormatResourceType(Orthanc::ResourceType level)
+    {
+      return boost::lexical_cast<std::string>(Orthanc::Plugins::Convert(level));
+    }
+
+    virtual std::string FormatWildcardEscape()
+    {
       switch (dialect_)
       {
-        case Dialect_MySQL:
-          break;
-        
+        case Dialect_SQLite:
         case Dialect_PostgreSQL:
-          break;
+          return "ESCAPE '\\'";
 
-        case Dialect_SQLite:
-          break;
+        case Dialect_MySQL:
+          return "ESCAPE '\\\\'";
 
         default:
           throw Orthanc::OrthancException(Orthanc::ErrorCode_NotImplemented);
       }
     }
+
+    void PrepareStatement(DatabaseManager::StandaloneStatement& statement) const
+    {
+      statement.SetReadOnly(true);
+      
+      for (size_t i = 0; i < count_; i++)
+      {
+        statement.SetParameterType(FormatParameter(i), ValueType_Utf8String);
+      }
+    }
+
+    const Dictionary& GetDictionary() const
+    {
+      return dictionary_;
+    }
   };
 #endif
 
@@ -1622,13 +1658,65 @@
                                      uint32_t limit,
                                      bool requestSomeInstance)
   {
+    LookupFormatter formatter(manager_.GetDialect());
+
     std::string sql;
+    Orthanc::ISqlLookupFormatter::Apply(sql, formatter, lookup,
+                                        Orthanc::Plugins::Convert(queryLevel), limit);
 
+    if (requestSomeInstance)
     {
-      LookupFormatter formatter(manager_.GetDialect());
-      Orthanc::ISqlLookupFormatter::Apply(sql, formatter, lookup,
-                                          Orthanc::Plugins::Convert(queryLevel), limit);
+      // Composite query to find some instance if requested
+      switch (queryLevel)
+      {
+        case OrthancPluginResourceType_Patient:
+          sql = ("SELECT patients.publicId, MIN(instances.publicId) FROM (" + sql + ") patients "
+                 "INNER JOIN Resources studies   ON studies.parentId   = patients.internalId "
+                 "INNER JOIN Resources series    ON series.parentId    = studies.internalId "
+                 "INNER JOIN Resources instances ON instances.parentId = series.internalId "
+                 "GROUP BY patients.publicId");
+          break;
+
+        case OrthancPluginResourceType_Study:
+          sql = ("SELECT studies.publicId, MIN(instances.publicId) FROM (" + sql + ") studies "
+                 "INNER JOIN Resources series    ON series.parentId    = studies.internalId "
+                 "INNER JOIN Resources instances ON instances.parentId = series.internalId "
+                 "GROUP BY studies.publicId");                 
+          break;
+
+        case OrthancPluginResourceType_Series:
+          sql = ("SELECT series.publicId, MIN(instances.publicId) FROM (" + sql + ") series "
+                 "INNER JOIN Resources instances ON instances.parentId = series.internalId "
+                 "GROUP BY series.publicId");
+          break;
+
+        case OrthancPluginResourceType_Instance:
+          sql = ("SELECT instances.publicId, instances.publicId FROM (" + sql + ") instances");
+          break;
+
+        default:
+          throw Orthanc::OrthancException(Orthanc::ErrorCode_InternalError);
+      }
     }
+
+    DatabaseManager::StandaloneStatement statement(GetManager(), sql);
+    formatter.PrepareStatement(statement);
+
+    statement.Execute(formatter.GetDictionary());
+
+    while (!statement.IsDone())
+    {
+      if (requestSomeInstance)
+      {
+        GetOutput().AnswerMatchingResource(ReadString(statement, 0), ReadString(statement, 1));
+      }
+      else
+      {
+        GetOutput().AnswerMatchingResource(ReadString(statement, 0));
+      }
+
+      statement.Next();
+    }    
   }
 #endif
 }
--- a/Framework/Plugins/IndexBackend.h	Thu Jan 03 10:07:27 2019 +0100
+++ b/Framework/Plugins/IndexBackend.h	Thu Jan 03 14:04:46 2019 +0100
@@ -40,13 +40,13 @@
       return manager_;
     }
     
-    static int64_t ReadInteger64(const DatabaseManager::CachedStatement& statement,
+    static int64_t ReadInteger64(const DatabaseManager::StatementBase& statement,
                                  size_t field);
 
-    static int32_t ReadInteger32(const DatabaseManager::CachedStatement& statement,
+    static int32_t ReadInteger32(const DatabaseManager::StatementBase& statement,
                                  size_t field);
     
-    static std::string ReadString(const DatabaseManager::CachedStatement& statement,
+    static std::string ReadString(const DatabaseManager::StatementBase& statement,
                                   size_t field);
     
     template <typename T>
--- a/Framework/Plugins/OrthancCppDatabasePlugin.h	Thu Jan 03 10:07:27 2019 +0100
+++ b/Framework/Plugins/OrthancCppDatabasePlugin.h	Thu Jan 03 14:04:46 2019 +0100
@@ -251,8 +251,7 @@
     }
 
 
-#if defined(ORTHANC_PLUGINS_VERSION_IS_ABOVE)         // Macro introduced in Orthanc 1.3.1
-#  if ORTHANC_PLUGINS_VERSION_IS_ABOVE(1, 5, 2)
+#if ORTHANC_PLUGINS_HAS_DATABASE_CONSTRAINT == 1
     void AnswerMatchingResource(const std::string& resourceId)
     {
       if (allowedAnswers_ != AllowedAnswers_All &&
@@ -267,8 +266,10 @@
 
       OrthancPluginDatabaseAnswerMatchingResource(context_, database_, &match);
     }
+#endif
 
 
+#if ORTHANC_PLUGINS_HAS_DATABASE_CONSTRAINT == 1
     void AnswerMatchingResource(const std::string& resourceId,
                                 const std::string& someInstanceId)
     {
@@ -284,7 +285,6 @@
 
       OrthancPluginDatabaseAnswerMatchingResource(context_, database_, &match);
     }
-#  endif
 #endif
   };