# HG changeset patch # User Sebastien Jodogne # Date 1546520686 -3600 # Node ID e6c13ddd26d95bbcbcfba9f73c158bddf70f7460 # Parent 19764fc60adec2efcff6f25d0510c5dedb45331b all integration tests passing with LookupResources extension diff -r 19764fc60ade -r e6c13ddd26d9 Framework/Common/DatabaseManager.cpp --- 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 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 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(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(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; + } } } diff -r 19764fc60ade -r e6c13ddd26d9 Framework/Common/DatabaseManager.h --- 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_; std::auto_ptr 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 statement_; + + public: + StandaloneStatement(DatabaseManager& manager, + const std::string& sql); + + virtual ~StandaloneStatement(); + + void Execute(const Dictionary& parameters); + }; }; } diff -r 19764fc60ade -r e6c13ddd26d9 Framework/Common/Query.cpp --- 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 { diff -r 19764fc60ade -r e6c13ddd26d9 Framework/Plugins/IndexBackend.cpp --- 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(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(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 } diff -r 19764fc60ade -r e6c13ddd26d9 Framework/Plugins/IndexBackend.h --- 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 diff -r 19764fc60ade -r e6c13ddd26d9 Framework/Plugins/OrthancCppDatabasePlugin.h --- 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 };