Mercurial > hg > orthanc-databases
changeset 23:b2ff1cd2907a
handling of implicit transactions in DatabaseManager
line wrap: on
line diff
--- a/Framework/Common/DatabaseManager.cpp Wed Jul 11 14:39:59 2018 +0200 +++ b/Framework/Common/DatabaseManager.cpp Thu Jul 12 10:44:17 2018 +0200 @@ -156,11 +156,11 @@ { if (transaction_.get() == NULL) { - LOG(TRACE) << "Automatically creating a database transaction"; + LOG(TRACE) << "Automatically creating an implicit database transaction"; try { - transaction_.reset(GetDatabase().CreateTransaction()); + transaction_.reset(GetDatabase().CreateTransaction(true)); } catch (Orthanc::OrthancException& e) { @@ -173,6 +173,27 @@ return *transaction_; } + + void DatabaseManager::ReleaseImplicitTransaction() + { + if (transaction_.get() != NULL && + transaction_->IsImplicit()) + { + LOG(TRACE) << "Committing an implicit database transaction"; + + try + { + transaction_->Commit(); + transaction_.reset(NULL); + } + catch (Orthanc::OrthancException& e) + { + // Don't throw the exception, as we are in CachedStatement destructor + LOG(ERROR) << "Error while committing an implicit database transaction: " << e.What(); + } + } + } + DatabaseManager::DatabaseManager(IDatabaseFactory* factory) : // Takes ownership factory_(factory) @@ -194,27 +215,11 @@ { if (transaction_.get() != NULL) { -#if 0 - // TODO: This should be the right implementation - if (transaction_->IsReadOnly()) - { - LOG(TRACE) << "Rollback of an uncommitted read-only transaction to start another transaction"; - transaction_->Rollback(); - transaction_.reset(NULL); - } - else - { - LOG(ERROR) << "Cannot rollback an uncommitted write transaction to start another transaction"; - throw Orthanc::OrthancException(Orthanc::ErrorCode_Database); - } -#else - LOG(INFO) << "Committing an uncommitted transaction to start another transaction"; - transaction_->Commit(); - transaction_.reset(NULL); -#endif + LOG(ERROR) << "Cannot start another transaction while there is an uncommitted transaction"; + throw Orthanc::OrthancException(Orthanc::ErrorCode_Database); } - transaction_.reset(GetDatabase().CreateTransaction()); + transaction_.reset(GetDatabase().CreateTransaction(false)); } catch (Orthanc::OrthancException& e) { @@ -302,6 +307,47 @@ } + DatabaseManager::Transaction::Transaction(DatabaseManager& manager) : + lock_(manager.mutex_), + manager_(manager), + database_(manager.GetDatabase()), + committed_(false) + { + manager_.StartTransaction(); + } + + + DatabaseManager::Transaction::~Transaction() + { + if (!committed_) + { + try + { + manager_.RollbackTransaction(); + } + catch (Orthanc::OrthancException& e) + { + // Don't rethrow the exception as we are in a destructor + LOG(ERROR) << "Uncatched error during some transaction rollback: " << e.What(); + } + } + } + + + void DatabaseManager::Transaction::Commit() + { + if (committed_) + { + throw Orthanc::OrthancException(Orthanc::ErrorCode_BadSequenceOfCalls); + } + else + { + manager_.CommitTransaction(); + committed_ = true; + } + } + + DatabaseManager::CachedStatement::CachedStatement(const StatementLocation& location, DatabaseManager& manager, const char* sql) : @@ -327,6 +373,12 @@ Setup(sql); } + + DatabaseManager::CachedStatement::~CachedStatement() + { + manager_.ReleaseImplicitTransaction(); + } + void DatabaseManager::CachedStatement::SetReadOnly(bool readOnly) {
--- a/Framework/Common/DatabaseManager.h Wed Jul 11 14:39:59 2018 +0200 +++ b/Framework/Common/DatabaseManager.h Thu Jul 12 10:44:17 2018 +0200 @@ -54,6 +54,8 @@ ITransaction& GetTransaction(); + void ReleaseImplicitTransaction(); + public: DatabaseManager(IDatabaseFactory* factory); // Takes ownership @@ -81,30 +83,21 @@ void RollbackTransaction(); + // This class is only used in the "StorageBackend" class Transaction : public boost::noncopyable { private: boost::recursive_mutex::scoped_lock lock_; DatabaseManager& manager_; IDatabase& database_; + bool committed_; public: - Transaction(DatabaseManager& manager) : - lock_(manager.mutex_), - manager_(manager), - database_(manager.GetDatabase()) - { - } + Transaction(DatabaseManager& manager); - void Commit() - { - manager_.CommitTransaction(); - } - - void Rollback() - { - manager_.RollbackTransaction(); - } + ~Transaction(); + + void Commit(); DatabaseManager& GetManager() { @@ -143,6 +136,8 @@ Transaction& transaction, const char* sql); + ~CachedStatement(); + IDatabase& GetDatabase() { return database_;
--- a/Framework/Common/IDatabase.h Wed Jul 11 14:39:59 2018 +0200 +++ b/Framework/Common/IDatabase.h Thu Jul 12 10:44:17 2018 +0200 @@ -38,6 +38,6 @@ virtual IPrecompiledStatement* Compile(const Query& query) = 0; - virtual ITransaction* CreateTransaction() = 0; + virtual ITransaction* CreateTransaction(bool isImplicit) = 0; }; }
--- a/Framework/Common/ITransaction.h Wed Jul 11 14:39:59 2018 +0200 +++ b/Framework/Common/ITransaction.h Thu Jul 12 10:44:17 2018 +0200 @@ -34,6 +34,8 @@ { } + virtual bool IsImplicit() const = 0; + virtual bool IsReadOnly() const = 0; virtual void Rollback() = 0;
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/Framework/Common/ImplicitTransaction.cpp Thu Jul 12 10:44:17 2018 +0200 @@ -0,0 +1,150 @@ +/** + * Orthanc - A Lightweight, RESTful DICOM Store + * Copyright (C) 2012-2016 Sebastien Jodogne, Medical Physics + * Department, University Hospital of Liege, Belgium + * Copyright (C) 2017-2018 Osimis S.A., Belgium + * + * This program is free software: you can redistribute it and/or + * modify it under the terms of the GNU Affero General Public License + * as published by the Free Software Foundation, either version 3 of + * the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + **/ + + +#include "ImplicitTransaction.h" + +#include <Core/Logging.h> +#include <Core/OrthancException.h> + +#include <memory> + +namespace OrthancDatabases +{ + ImplicitTransaction::ImplicitTransaction() : + state_(State_Ready), + readOnly_(true) + { + } + + + ImplicitTransaction::~ImplicitTransaction() + { + switch (state_) + { + case State_Committed: + case State_Ready: + break; + + case State_Executed: + LOG(ERROR) << "An implicit transaction has not been committed"; + break; + + default: + throw Orthanc::OrthancException(Orthanc::ErrorCode_InternalError); + } + } + + + void ImplicitTransaction::Rollback() + { + LOG(ERROR) << "Cannot rollback an implicit transaction"; + throw Orthanc::OrthancException(Orthanc::ErrorCode_BadSequenceOfCalls); + } + + + void ImplicitTransaction::Commit() + { + switch (state_) + { + case State_Ready: + LOG(ERROR) << "Cannot commit an implicit transaction that has not been executed yet"; + throw Orthanc::OrthancException(Orthanc::ErrorCode_BadSequenceOfCalls); + + case State_Executed: + state_ = State_Committed; + break; + + case State_Committed: + LOG(ERROR) << "Cannot commit twice an implicit transaction"; + throw Orthanc::OrthancException(Orthanc::ErrorCode_BadSequenceOfCalls); + + default: + throw Orthanc::OrthancException(Orthanc::ErrorCode_InternalError); + } + } + + + void ImplicitTransaction::CheckStateForExecution() + { + switch (state_) + { + case State_Ready: + // OK + break; + + case State_Executed: +#if defined(NDEBUG) + // Release build. Ignore such errors. + LOG(INFO) << "Cannot execute more than one statement in an implicit transaction"; + break; +#else + /** + * Debug build. This allows to detect errors wrt. the handling + * of transactions in the Orthanc core. + * + * In Orthanc <= 1.3.2: problems in "/changes" (a transaction + * was missing because of GetPublicId()). + **/ + LOG(ERROR) << "Cannot execute more than one statement in an implicit transaction"; + throw Orthanc::OrthancException(Orthanc::ErrorCode_BadSequenceOfCalls); +#endif + + case State_Committed: + LOG(ERROR) << "Cannot commit twice an implicit transaction"; + throw Orthanc::OrthancException(Orthanc::ErrorCode_BadSequenceOfCalls); + + default: + throw Orthanc::OrthancException(Orthanc::ErrorCode_InternalError); + } + } + + + IResult* ImplicitTransaction::Execute(IPrecompiledStatement& statement, + const Dictionary& parameters) + { + CheckStateForExecution(); + std::auto_ptr<IResult> result(ExecuteInternal(statement, parameters)); + + if (!statement.IsReadOnly()) + { + readOnly_ = false; + } + + state_ = State_Executed; + return result.release(); + } + + + void ImplicitTransaction::ExecuteWithoutResult(IPrecompiledStatement& statement, + const Dictionary& parameters) + { + CheckStateForExecution(); + ExecuteWithoutResultInternal(statement, parameters); + + if (!statement.IsReadOnly()) + { + readOnly_ = false; + } + + state_ = State_Executed; + } +} +
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/Framework/Common/ImplicitTransaction.h Thu Jul 12 10:44:17 2018 +0200 @@ -0,0 +1,75 @@ +/** + * Orthanc - A Lightweight, RESTful DICOM Store + * Copyright (C) 2012-2016 Sebastien Jodogne, Medical Physics + * Department, University Hospital of Liege, Belgium + * Copyright (C) 2017-2018 Osimis S.A., Belgium + * + * This program is free software: you can redistribute it and/or + * modify it under the terms of the GNU Affero General Public License + * as published by the Free Software Foundation, either version 3 of + * the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + **/ + + +#pragma once + +#include "ITransaction.h" + +namespace OrthancDatabases +{ + class ImplicitTransaction : public ITransaction + { + private: + enum State + { + State_Ready, + State_Executed, + State_Committed + }; + + State state_; + bool readOnly_; + + void CheckStateForExecution(); + + protected: + virtual IResult* ExecuteInternal(IPrecompiledStatement& statement, + const Dictionary& parameters) = 0; + + virtual void ExecuteWithoutResultInternal(IPrecompiledStatement& statement, + const Dictionary& parameters) = 0; + + public: + ImplicitTransaction(); + + virtual ~ImplicitTransaction(); + + virtual bool IsImplicit() const + { + return true; + } + + virtual bool IsReadOnly() const + { + return readOnly_; + } + + virtual void Rollback(); + + virtual void Commit(); + + virtual IResult* Execute(IPrecompiledStatement& statement, + const Dictionary& parameters); + + virtual void ExecuteWithoutResult(IPrecompiledStatement& statement, + const Dictionary& parameters); + }; +}
--- a/Framework/MySQL/MySQLDatabase.cpp Wed Jul 11 14:39:59 2018 +0200 +++ b/Framework/MySQL/MySQLDatabase.cpp Thu Jul 12 10:44:17 2018 +0200 @@ -24,6 +24,7 @@ #include "MySQLResult.h" #include "MySQLStatement.h" #include "MySQLTransaction.h" +#include "../Common/ImplicitTransaction.h" #include "../Common/Integer64Value.h" #include <Core/Logging.h> @@ -104,7 +105,7 @@ } - void MySQLDatabase::Open() + void MySQLDatabase::OpenInternal(const char* db) { if (mysql_ != NULL) { @@ -118,9 +119,6 @@ throw Orthanc::OrthancException(Orthanc::ErrorCode_InternalError); } - const char* db = (parameters_.GetDatabase().empty() ? NULL : - parameters_.GetDatabase().c_str()); - const char* socket = (parameters_.GetUnixSocket().empty() ? NULL : parameters_.GetUnixSocket().c_str()); @@ -147,6 +145,42 @@ } } + + void MySQLDatabase::Open() + { + if (parameters_.GetDatabase().empty()) + { + throw Orthanc::OrthancException(Orthanc::ErrorCode_ParameterOutOfRange); + } + else + { + OpenInternal(parameters_.GetDatabase().c_str()); + } + } + + + void MySQLDatabase::ClearDatabase(const MySQLParameters& parameters) + { + MySQLDatabase db(parameters); + db.OpenRoot(); + + const std::string& database = parameters.GetDatabase(); + + { + MySQLTransaction t(db); + + if (!db.DoesDatabaseExist(t, database)) + { + LOG(ERROR) << "Inexistent database, please create it first: " << database; + throw Orthanc::OrthancException(Orthanc::ErrorCode_UnknownResource); + } + + db.Execute("DROP DATABASE " + database, false); + db.Execute("CREATE DATABASE " + database, false); + t.Commit(); + } + } + namespace { @@ -378,14 +412,51 @@ } - ITransaction* MySQLDatabase::CreateTransaction() + + namespace + { + class MySQLImplicitTransaction : public ImplicitTransaction + { + private: + MySQLDatabase& db_; + + protected: + virtual IResult* ExecuteInternal(IPrecompiledStatement& statement, + const Dictionary& parameters) + { + return dynamic_cast<MySQLStatement&>(statement).Execute(*this, parameters); + } + + virtual void ExecuteWithoutResultInternal(IPrecompiledStatement& statement, + const Dictionary& parameters) + { + dynamic_cast<MySQLStatement&>(statement).ExecuteWithoutResult(*this, parameters); + } + + public: + MySQLImplicitTransaction(MySQLDatabase& db) : + db_(db) + { + } + }; + } + + + ITransaction* MySQLDatabase::CreateTransaction(bool isImplicit) { if (mysql_ == NULL) { throw Orthanc::OrthancException(Orthanc::ErrorCode_BadSequenceOfCalls); } - return new MySQLTransaction(*this); + if (isImplicit) + { + return new MySQLImplicitTransaction(*this); + } + else + { + return new MySQLTransaction(*this); + } }
--- a/Framework/MySQL/MySQLDatabase.h Wed Jul 11 14:39:59 2018 +0200 +++ b/Framework/MySQL/MySQLDatabase.h Thu Jul 12 10:44:17 2018 +0200 @@ -40,6 +40,8 @@ MySQLParameters parameters_; MYSQL *mysql_; + void OpenInternal(const char* database); + void Close(); public: @@ -58,6 +60,13 @@ void Open(); + void OpenRoot() + { + OpenInternal(NULL); + } + + static void ClearDatabase(const MySQLParameters& parameters); + bool LookupGlobalStringVariable(std::string& value, const std::string& variable); @@ -82,7 +91,7 @@ virtual IPrecompiledStatement* Compile(const Query& query); - virtual ITransaction* CreateTransaction(); + virtual ITransaction* CreateTransaction(bool isImplicit); static void GlobalFinalization(); };
--- a/Framework/MySQL/MySQLParameters.cpp Wed Jul 11 14:39:59 2018 +0200 +++ b/Framework/MySQL/MySQLParameters.cpp Thu Jul 12 10:44:17 2018 +0200 @@ -104,6 +104,22 @@ void MySQLParameters::SetDatabase(const std::string& database) { + if (database.empty()) + { + LOG(ERROR) << "Empty database name"; + throw Orthanc::OrthancException(Orthanc::ErrorCode_ParameterOutOfRange); + } + + for (size_t i = 0; i < database.length(); i++) + { + if (!isalnum(database [i])) + { + LOG(ERROR) << "Only alphanumeric characters are allowed in a " + << "MySQL database name: \"" << database << "\""; + throw Orthanc::OrthancException(Orthanc::ErrorCode_ParameterOutOfRange); + } + } + database_ = database; }
--- a/Framework/MySQL/MySQLStatement.cpp Wed Jul 11 14:39:59 2018 +0200 +++ b/Framework/MySQL/MySQLStatement.cpp Thu Jul 12 10:44:17 2018 +0200 @@ -422,7 +422,7 @@ } - IResult* MySQLStatement::Execute(MySQLTransaction& transaction, + IResult* MySQLStatement::Execute(ITransaction& transaction, const Dictionary& parameters) { std::list<int> intParameters; @@ -525,7 +525,7 @@ } - void MySQLStatement::ExecuteWithoutResult(MySQLTransaction& transaction, + void MySQLStatement::ExecuteWithoutResult(ITransaction& transaction, const Dictionary& parameters) { std::auto_ptr<IResult> dummy(Execute(transaction, parameters));
--- a/Framework/MySQL/MySQLStatement.h Wed Jul 11 14:39:59 2018 +0200 +++ b/Framework/MySQL/MySQLStatement.h Thu Jul 12 10:44:17 2018 +0200 @@ -26,7 +26,6 @@ #endif #include "MySQLDatabase.h" -#include "MySQLTransaction.h" #include "../Common/GenericFormatter.h" namespace OrthancDatabases @@ -69,10 +68,10 @@ IValue* FetchResultField(size_t i); - IResult* Execute(MySQLTransaction& transaction, + IResult* Execute(ITransaction& transaction, const Dictionary& parameters); - void ExecuteWithoutResult(MySQLTransaction& transaction, + void ExecuteWithoutResult(ITransaction& transaction, const Dictionary& parameters); }; }
--- a/Framework/MySQL/MySQLTransaction.h Wed Jul 11 14:39:59 2018 +0200 +++ b/Framework/MySQL/MySQLTransaction.h Thu Jul 12 10:44:17 2018 +0200 @@ -42,6 +42,11 @@ virtual ~MySQLTransaction(); + virtual bool IsImplicit() const + { + return false; + } + virtual bool IsReadOnly() const { return readOnly_;
--- a/Framework/PostgreSQL/PostgreSQLDatabase.cpp Wed Jul 11 14:39:59 2018 +0200 +++ b/Framework/PostgreSQL/PostgreSQLDatabase.cpp Thu Jul 12 10:44:17 2018 +0200 @@ -24,6 +24,7 @@ #include "PostgreSQLResult.h" #include "PostgreSQLStatement.h" #include "PostgreSQLTransaction.h" +#include "../Common/ImplicitTransaction.h" #include <Core/Logging.h> #include <Core/OrthancException.h> @@ -192,8 +193,44 @@ } - ITransaction* PostgreSQLDatabase::CreateTransaction() + namespace { - return new PostgreSQLTransaction(*this); + class PostgreSQLImplicitTransaction : public ImplicitTransaction + { + private: + PostgreSQLDatabase& db_; + + protected: + virtual IResult* ExecuteInternal(IPrecompiledStatement& statement, + const Dictionary& parameters) + { + return dynamic_cast<PostgreSQLStatement&>(statement).Execute(*this, parameters); + } + + virtual void ExecuteWithoutResultInternal(IPrecompiledStatement& statement, + const Dictionary& parameters) + { + dynamic_cast<PostgreSQLStatement&>(statement).ExecuteWithoutResult(*this, parameters); + } + + public: + PostgreSQLImplicitTransaction(PostgreSQLDatabase& db) : + db_(db) + { + } + }; + } + + + ITransaction* PostgreSQLDatabase::CreateTransaction(bool isImplicit) + { + if (isImplicit) + { + return new PostgreSQLImplicitTransaction(*this); + } + else + { + return new PostgreSQLTransaction(*this); + } } }
--- a/Framework/PostgreSQL/PostgreSQLDatabase.h Wed Jul 11 14:39:59 2018 +0200 +++ b/Framework/PostgreSQL/PostgreSQLDatabase.h Thu Jul 12 10:44:17 2018 +0200 @@ -72,6 +72,6 @@ virtual IPrecompiledStatement* Compile(const Query& query); - virtual ITransaction* CreateTransaction(); + virtual ITransaction* CreateTransaction(bool isImplicit); }; }
--- a/Framework/PostgreSQL/PostgreSQLStatement.cpp Wed Jul 11 14:39:59 2018 +0200 +++ b/Framework/PostgreSQL/PostgreSQLStatement.cpp Thu Jul 12 10:44:17 2018 +0200 @@ -487,7 +487,7 @@ }; - IResult* PostgreSQLStatement::Execute(PostgreSQLTransaction& transaction, + IResult* PostgreSQLStatement::Execute(ITransaction& transaction, const Dictionary& parameters) { for (size_t i = 0; i < formatter_.GetParametersCount(); i++) @@ -533,7 +533,7 @@ } - void PostgreSQLStatement::ExecuteWithoutResult(PostgreSQLTransaction& transaction, + void PostgreSQLStatement::ExecuteWithoutResult(ITransaction& transaction, const Dictionary& parameters) { std::auto_ptr<IResult> dummy(Execute(transaction, parameters));
--- a/Framework/PostgreSQL/PostgreSQLStatement.h Wed Jul 11 14:39:59 2018 +0200 +++ b/Framework/PostgreSQL/PostgreSQLStatement.h Thu Jul 12 10:44:17 2018 +0200 @@ -109,10 +109,10 @@ return database_; } - IResult* Execute(PostgreSQLTransaction& transaction, + IResult* Execute(ITransaction& transaction, const Dictionary& parameters); - void ExecuteWithoutResult(PostgreSQLTransaction& transaction, + void ExecuteWithoutResult(ITransaction& transaction, const Dictionary& parameters); }; }
--- a/Framework/PostgreSQL/PostgreSQLTransaction.h Wed Jul 11 14:39:59 2018 +0200 +++ b/Framework/PostgreSQL/PostgreSQLTransaction.h Thu Jul 12 10:44:17 2018 +0200 @@ -43,6 +43,11 @@ ~PostgreSQLTransaction(); + virtual bool IsImplicit() const + { + return false; + } + virtual bool IsReadOnly() const { return readOnly_;
--- a/Framework/SQLite/SQLiteDatabase.cpp Wed Jul 11 14:39:59 2018 +0200 +++ b/Framework/SQLite/SQLiteDatabase.cpp Thu Jul 12 10:44:17 2018 +0200 @@ -23,6 +23,7 @@ #include "SQLiteStatement.h" #include "SQLiteTransaction.h" +#include "../Common/ImplicitTransaction.h" #include <Core/OrthancException.h> @@ -41,10 +42,45 @@ { return new SQLiteStatement(*this, query); } + + namespace + { + class SQLiteImplicitTransaction : public ImplicitTransaction + { + private: + SQLiteDatabase& db_; + + protected: + virtual IResult* ExecuteInternal(IPrecompiledStatement& statement, + const Dictionary& parameters) + { + return dynamic_cast<SQLiteStatement&>(statement).Execute(*this, parameters); + } - ITransaction* SQLiteDatabase::CreateTransaction() + virtual void ExecuteWithoutResultInternal(IPrecompiledStatement& statement, + const Dictionary& parameters) + { + dynamic_cast<SQLiteStatement&>(statement).ExecuteWithoutResult(*this, parameters); + } + + public: + SQLiteImplicitTransaction(SQLiteDatabase& db) : + db_(db) + { + } + }; + } + + ITransaction* SQLiteDatabase::CreateTransaction(bool isImplicit) { - return new SQLiteTransaction(*this); + if (isImplicit) + { + return new SQLiteImplicitTransaction(*this); + } + else + { + return new SQLiteTransaction(*this); + } } }
--- a/Framework/SQLite/SQLiteDatabase.h Wed Jul 11 14:39:59 2018 +0200 +++ b/Framework/SQLite/SQLiteDatabase.h Thu Jul 12 10:44:17 2018 +0200 @@ -71,6 +71,6 @@ virtual IPrecompiledStatement* Compile(const Query& query); - virtual ITransaction* CreateTransaction(); + virtual ITransaction* CreateTransaction(bool isImplicit); }; }
--- a/Framework/SQLite/SQLiteStatement.cpp Wed Jul 11 14:39:59 2018 +0200 +++ b/Framework/SQLite/SQLiteStatement.cpp Thu Jul 12 10:44:17 2018 +0200 @@ -104,7 +104,7 @@ } - IResult* SQLiteStatement::Execute(SQLiteTransaction& transaction, + IResult* SQLiteStatement::Execute(ITransaction& transaction, const Dictionary& parameters) { BindParameters(parameters); @@ -112,7 +112,7 @@ } - void SQLiteStatement::ExecuteWithoutResult(SQLiteTransaction& transaction, + void SQLiteStatement::ExecuteWithoutResult(ITransaction& transaction, const Dictionary& parameters) { BindParameters(parameters);
--- a/Framework/SQLite/SQLiteStatement.h Wed Jul 11 14:39:59 2018 +0200 +++ b/Framework/SQLite/SQLiteStatement.h Thu Jul 12 10:44:17 2018 +0200 @@ -55,10 +55,10 @@ Orthanc::SQLite::Statement& GetObject(); - IResult* Execute(SQLiteTransaction& transaction, + IResult* Execute(ITransaction& transaction, const Dictionary& parameters); - void ExecuteWithoutResult(SQLiteTransaction& transaction, + void ExecuteWithoutResult(ITransaction& transaction, const Dictionary& parameters); }; }
--- a/Framework/SQLite/SQLiteTransaction.h Wed Jul 11 14:39:59 2018 +0200 +++ b/Framework/SQLite/SQLiteTransaction.h Thu Jul 12 10:44:17 2018 +0200 @@ -41,6 +41,11 @@ public: SQLiteTransaction(SQLiteDatabase& database); + virtual bool IsImplicit() const + { + return false; + } + virtual bool IsReadOnly() const { return readOnly_;
--- a/MySQL/Plugins/MySQLIndex.cpp Wed Jul 11 14:39:59 2018 +0200 +++ b/MySQL/Plugins/MySQLIndex.cpp Thu Jul 12 10:44:17 2018 +0200 @@ -56,42 +56,9 @@ throw Orthanc::OrthancException(Orthanc::ErrorCode_Plugin); } - if (parameters_.GetDatabase().empty()) - { - LOG(ERROR) << "Empty database name"; - throw Orthanc::OrthancException(Orthanc::ErrorCode_ParameterOutOfRange); - } - - for (size_t i = 0; i < parameters_.GetDatabase().length(); i++) - { - if (!isalnum(parameters_.GetDatabase() [i])) - { - LOG(ERROR) << "Only alphanumeric characters are allowed in a " - << "MySQL database name: \"" << parameters_.GetDatabase() << "\""; - throw Orthanc::OrthancException(Orthanc::ErrorCode_ParameterOutOfRange); - } - } - if (clearAll_) { - MySQLParameters p = parameters_; - const std::string database = p.GetDatabase(); - p.SetDatabase(""); - - MySQLDatabase db(p); - db.Open(); - - MySQLTransaction t(db); - - if (!db.DoesDatabaseExist(t, database)) - { - LOG(ERROR) << "Inexistent database, please create it first: " << database; - throw Orthanc::OrthancException(Orthanc::ErrorCode_UnknownResource); - } - - db.Execute("DROP DATABASE " + database, false); - db.Execute("CREATE DATABASE " + database, false); - t.Commit(); + MySQLDatabase::ClearDatabase(parameters_); } std::auto_ptr<MySQLDatabase> db(new MySQLDatabase(parameters_));
--- a/MySQL/UnitTests/UnitTestsMain.cpp Wed Jul 11 14:39:59 2018 +0200 +++ b/MySQL/UnitTests/UnitTestsMain.cpp Thu Jul 12 10:44:17 2018 +0200 @@ -28,6 +28,7 @@ #include "../../Framework/MySQL/MySQLDatabase.h" #include "../../Framework/MySQL/MySQLResult.h" #include "../../Framework/MySQL/MySQLStatement.h" +#include "../../Framework/MySQL/MySQLTransaction.h" #include "../../Framework/Plugins/IndexUnitTests.h" #include <Core/Logging.h> @@ -130,6 +131,59 @@ } +TEST(MySQL, ImplicitTransaction) +{ + OrthancDatabases::MySQLDatabase::ClearDatabase(globalParameters_); + OrthancDatabases::MySQLDatabase db(globalParameters_); + db.Open(); + + { + OrthancDatabases::MySQLTransaction t(db); + ASSERT_FALSE(db.DoesTableExist(t, "test")); + ASSERT_FALSE(db.DoesTableExist(t, "test2")); + } + + { + std::auto_ptr<OrthancDatabases::ITransaction> t(db.CreateTransaction(false)); + ASSERT_FALSE(t->IsImplicit()); + } + + { + OrthancDatabases::Query query("CREATE TABLE test(id INT)", false); + std::auto_ptr<OrthancDatabases::IPrecompiledStatement> s(db.Compile(query)); + + std::auto_ptr<OrthancDatabases::ITransaction> t(db.CreateTransaction(true)); + ASSERT_TRUE(t->IsImplicit()); + ASSERT_THROW(t->Commit(), Orthanc::OrthancException); + ASSERT_THROW(t->Rollback(), Orthanc::OrthancException); + + OrthancDatabases::Dictionary args; + t->ExecuteWithoutResult(*s, args); + ASSERT_THROW(t->Rollback(), Orthanc::OrthancException); + t->Commit(); + + ASSERT_THROW(t->Commit(), Orthanc::OrthancException); + } + + { + // An implicit transaction does not need to be explicitely committed + OrthancDatabases::Query query("CREATE TABLE test2(id INT)", false); + std::auto_ptr<OrthancDatabases::IPrecompiledStatement> s(db.Compile(query)); + + std::auto_ptr<OrthancDatabases::ITransaction> t(db.CreateTransaction(true)); + + OrthancDatabases::Dictionary args; + t->ExecuteWithoutResult(*s, args); + } + + { + OrthancDatabases::MySQLTransaction t(db); + ASSERT_TRUE(db.DoesTableExist(t, "test")); + ASSERT_TRUE(db.DoesTableExist(t, "test2")); + } +} + + int main(int argc, char **argv) { if (argc < 5)
--- a/PostgreSQL/UnitTests/PostgreSQLTests.cpp Wed Jul 11 14:39:59 2018 +0200 +++ b/PostgreSQL/UnitTests/PostgreSQLTests.cpp Thu Jul 12 10:44:17 2018 +0200 @@ -45,20 +45,16 @@ using namespace OrthancDatabases; -extern OrthancDatabases::PostgreSQLParameters globalParameters_; +extern PostgreSQLParameters globalParameters_; -static OrthancDatabases::PostgreSQLDatabase* CreateTestDatabase(bool clearAll) +static PostgreSQLDatabase* CreateTestDatabase() { - std::auto_ptr<OrthancDatabases::PostgreSQLDatabase> pg - (new OrthancDatabases::PostgreSQLDatabase(globalParameters_)); + std::auto_ptr<PostgreSQLDatabase> pg + (new PostgreSQLDatabase(globalParameters_)); pg->Open(); - - if (clearAll) - { - pg->ClearAll(); - } + pg->ClearAll(); return pg.release(); } @@ -75,7 +71,7 @@ TEST(PostgreSQL, Basic) { - std::auto_ptr<PostgreSQLDatabase> pg(CreateTestDatabase(true)); + std::auto_ptr<PostgreSQLDatabase> pg(CreateTestDatabase()); ASSERT_FALSE(pg->DoesTableExist("Test")); pg->Execute("CREATE TABLE Test(name INTEGER, value BIGINT)"); @@ -148,7 +144,7 @@ TEST(PostgreSQL, String) { - std::auto_ptr<PostgreSQLDatabase> pg(CreateTestDatabase(true)); + std::auto_ptr<PostgreSQLDatabase> pg(CreateTestDatabase()); pg->Execute("CREATE TABLE Test(name INTEGER, value VARCHAR(40))"); @@ -194,7 +190,7 @@ TEST(PostgreSQL, Transaction) { - std::auto_ptr<PostgreSQLDatabase> pg(CreateTestDatabase(true)); + std::auto_ptr<PostgreSQLDatabase> pg(CreateTestDatabase()); pg->Execute("CREATE TABLE Test(name INTEGER, value INTEGER)"); @@ -262,7 +258,7 @@ TEST(PostgreSQL, LargeObject) { - std::auto_ptr<PostgreSQLDatabase> pg(CreateTestDatabase(true)); + std::auto_ptr<PostgreSQLDatabase> pg(CreateTestDatabase()); ASSERT_EQ(0, CountLargeObjects(*pg)); pg->Execute("CREATE TABLE Test(name VARCHAR, value OID)"); @@ -340,13 +336,13 @@ TEST(PostgreSQL, StorageArea) { - OrthancDatabases::PostgreSQLStorageArea storageArea(globalParameters_); + PostgreSQLStorageArea storageArea(globalParameters_); storageArea.SetClearAll(true); { - OrthancDatabases::DatabaseManager::Transaction transaction(storageArea.GetManager()); - OrthancDatabases::PostgreSQLDatabase& db = - dynamic_cast<OrthancDatabases::PostgreSQLDatabase&>(transaction.GetDatabase()); + DatabaseManager::Transaction transaction(storageArea.GetManager()); + PostgreSQLDatabase& db = + dynamic_cast<PostgreSQLDatabase&>(transaction.GetDatabase()); ASSERT_EQ(0, CountLargeObjects(db)); @@ -395,3 +391,49 @@ transaction.Commit(); } } + + +TEST(PostgreSQL, ImplicitTransaction) +{ + std::auto_ptr<PostgreSQLDatabase> db(CreateTestDatabase()); + + ASSERT_FALSE(db->DoesTableExist("test")); + ASSERT_FALSE(db->DoesTableExist("test2")); + + { + std::auto_ptr<OrthancDatabases::ITransaction> t(db->CreateTransaction(false)); + ASSERT_FALSE(t->IsImplicit()); + } + + { + Query query("CREATE TABLE test(id INT)", false); + std::auto_ptr<IPrecompiledStatement> s(db->Compile(query)); + + std::auto_ptr<ITransaction> t(db->CreateTransaction(true)); + ASSERT_TRUE(t->IsImplicit()); + ASSERT_THROW(t->Commit(), Orthanc::OrthancException); + ASSERT_THROW(t->Rollback(), Orthanc::OrthancException); + + Dictionary args; + t->ExecuteWithoutResult(*s, args); + ASSERT_THROW(t->Rollback(), Orthanc::OrthancException); + t->Commit(); + + ASSERT_THROW(t->Commit(), Orthanc::OrthancException); + } + + { + // An implicit transaction does not need to be explicitely committed + Query query("CREATE TABLE test2(id INT)", false); + std::auto_ptr<IPrecompiledStatement> s(db->Compile(query)); + + std::auto_ptr<ITransaction> t(db->CreateTransaction(true)); + + Dictionary args; + t->ExecuteWithoutResult(*s, args); + } + + ASSERT_TRUE(db->DoesTableExist("test")); + ASSERT_TRUE(db->DoesTableExist("test2")); +} +
--- a/Resources/CMake/DatabasesFrameworkConfiguration.cmake Wed Jul 11 14:39:59 2018 +0200 +++ b/Resources/CMake/DatabasesFrameworkConfiguration.cmake Thu Jul 12 10:44:17 2018 +0200 @@ -64,6 +64,7 @@ ${ORTHANC_DATABASES_ROOT}/Framework/Common/Dictionary.cpp ${ORTHANC_DATABASES_ROOT}/Framework/Common/FileValue.cpp ${ORTHANC_DATABASES_ROOT}/Framework/Common/GenericFormatter.cpp + ${ORTHANC_DATABASES_ROOT}/Framework/Common/ImplicitTransaction.cpp ${ORTHANC_DATABASES_ROOT}/Framework/Common/Integer64Value.cpp ${ORTHANC_DATABASES_ROOT}/Framework/Common/NullValue.cpp ${ORTHANC_DATABASES_ROOT}/Framework/Common/Query.cpp
--- a/SQLite/UnitTests/UnitTestsMain.cpp Wed Jul 11 14:39:59 2018 +0200 +++ b/SQLite/UnitTests/UnitTestsMain.cpp Thu Jul 12 10:44:17 2018 +0200 @@ -19,8 +19,8 @@ **/ +#include "../../Framework/SQLite/SQLiteDatabase.h" #include "../Plugins/SQLiteIndex.h" -#include "../../Framework/Plugins/IndexUnitTests.h" #include <Core/Logging.h> #include <Core/SystemToolbox.h> @@ -28,6 +28,8 @@ #include <gtest/gtest.h> +#include "../../Framework/Plugins/IndexUnitTests.h" + TEST(SQLiteIndex, Lock) { @@ -57,6 +59,51 @@ } +TEST(SQLite, ImplicitTransaction) +{ + OrthancDatabases::SQLiteDatabase db; + db.OpenInMemory(); + + ASSERT_FALSE(db.DoesTableExist("test")); + ASSERT_FALSE(db.DoesTableExist("test2")); + + { + std::auto_ptr<OrthancDatabases::ITransaction> t(db.CreateTransaction(false)); + ASSERT_FALSE(t->IsImplicit()); + } + + { + OrthancDatabases::Query query("CREATE TABLE test(id INT)", false); + std::auto_ptr<OrthancDatabases::IPrecompiledStatement> s(db.Compile(query)); + + std::auto_ptr<OrthancDatabases::ITransaction> t(db.CreateTransaction(true)); + ASSERT_TRUE(t->IsImplicit()); + ASSERT_THROW(t->Commit(), Orthanc::OrthancException); + ASSERT_THROW(t->Rollback(), Orthanc::OrthancException); + + OrthancDatabases::Dictionary args; + t->ExecuteWithoutResult(*s, args); + ASSERT_THROW(t->Rollback(), Orthanc::OrthancException); + t->Commit(); + + ASSERT_THROW(t->Commit(), Orthanc::OrthancException); + } + + { + // An implicit transaction does not need to be explicitely committed + OrthancDatabases::Query query("CREATE TABLE test2(id INT)", false); + std::auto_ptr<OrthancDatabases::IPrecompiledStatement> s(db.Compile(query)); + + std::auto_ptr<OrthancDatabases::ITransaction> t(db.CreateTransaction(true)); + + OrthancDatabases::Dictionary args; + t->ExecuteWithoutResult(*s, args); + } + + ASSERT_TRUE(db.DoesTableExist("test")); + ASSERT_TRUE(db.DoesTableExist("test2")); +} + int main(int argc, char **argv) {