From d36e6eb0cbebf9c4ea021411a2fa97c21db5aacb Mon Sep 17 00:00:00 2001 From: Christian Hohnstaedt Date: Tue, 21 Nov 2017 19:06:41 +0100 Subject: [PATCH] Implement nested transactions. The DbTransaction class automatically rolls back when the scope is left (destructor) and no commit happenned. Every transaction begin will increment the counter, each commit/rollback will decrement it. Only if all transactions finished with a commit, a final database commit will be performed. --- lib/Makefile | 2 +- lib/db_base.cpp | 33 +++------ lib/db_base.h | 1 - lib/db_crl.cpp | 23 ++++--- lib/db_temp.cpp | 8 +-- lib/pki_base.h | 73 +------------------- lib/pki_x509.cpp | 2 +- lib/sql.cpp | 146 ++++++++++++++++++++++++++++++++++++++++ lib/sql.h | 59 ++++++++++++++++ lib/x509rev.cpp | 28 ++------ lib/x509rev.h | 1 - widgets/MW_database.cpp | 7 +- widgets/MainWindow.cpp | 19 +++--- xca.pro | 32 +++++---- 14 files changed, 274 insertions(+), 160 deletions(-) create mode 100644 lib/sql.cpp create mode 100644 lib/sql.h diff --git a/lib/Makefile b/lib/Makefile index ba56678b..394e8a2b 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -9,7 +9,7 @@ MOCNAMES=db_crl db_key db_temp db_x509 db_x509req db_x509super db_base db_token\ pki_base pki_multi pki_evp pki_scard pass_info pki_pkcs7 main NAMES=$(MOCNAMES) asn1int oid x509rev asn1time version \ x509v3ext func load_obj x509name db \ - pk11_attribute pkcs11 pkcs11_lib Passwd builtin_curves entropy + pk11_attribute pkcs11 pkcs11_lib Passwd builtin_curves entropy sql OBJS=$(patsubst %, %.o, $(NAMES)) $(patsubst %, moc_%.o, $(MOCNAMES)) diff --git a/lib/db_base.cpp b/lib/db_base.cpp index 752bafe4..e8c8c8a2 100644 --- a/lib/db_base.cpp +++ b/lib/db_base.cpp @@ -206,29 +206,21 @@ void db_base::sortIndicatorChanged(int logicalIndex, Qt::SortOrder order) allHeaders[logicalIndex]->sortIndicator = order; } -QSqlError db_base::insertPKI_noTransaction(pki_base *pki) +void db_base::insertPKI(pki_base *pki) { + Transaction; + if (!TransBegin()) + return; QSqlError e = pki->insertSql(); if (e.isValid()) { mainwin->dbSqlError(e); - return e; + TransRollback(); + return; } lookup[pki->getSqlItemId().toULongLong()] = pki; inToCont(pki); emit columnsContentChanged(); - return e; -} - -void db_base::insertPKI(pki_base *pki) -{ - QSqlDatabase db = QSqlDatabase::database(); - if (db.transaction()) { - QSqlError e = insertPKI_noTransaction(pki); - if (e.isValid()) - db.rollback(); - else - db.commit(); - } + TransCommit(); } QString db_base::pem2QString(QModelIndexList indexes) const @@ -274,15 +266,12 @@ void db_base::deletePKI(QModelIndex idx) } catch (errorEx &err) { MainWindow::Error(err); } - - if (db.transaction()) { + Transaction; + if (TransBegin()) { QSqlError e = pki->deleteSql(); - if (e.isValid()) { - db.rollback(); - } else { - db.commit(); + TransDone(e); + if (!e.isValid()) remFromCont(idx); - } mainwin->dbSqlError(e); } } catch (errorEx &err) { diff --git a/lib/db_base.h b/lib/db_base.h index 246629ab..9dd34742 100644 --- a/lib/db_base.h +++ b/lib/db_base.h @@ -103,7 +103,6 @@ class db_base: public QAbstractItemModel virtual void updateHeaders(); virtual ~db_base(); virtual void insertPKI(pki_base *pki); - virtual QSqlError insertPKI_noTransaction(pki_base *pki); pki_base *getByName(QString desc); pki_base *getByReference(pki_base *refpki); pki_base *getByPtr(void *); diff --git a/lib/db_crl.cpp b/lib/db_crl.cpp index 8cd8b05c..3960f784 100644 --- a/lib/db_crl.cpp +++ b/lib/db_crl.cpp @@ -106,8 +106,12 @@ pki_base *db_crl::insert(pki_base *item) delete(crl); return NULL; } - insertPKI(crl); - revokeCerts(crl); + Transaction; + if (TransBegin()) { + insertPKI(crl); + revokeCerts(crl); + TransCommit(); + } return crl; } @@ -232,7 +236,6 @@ void db_crl::newItem() void db_crl::newItem(pki_x509 *cert) { - bool transact = false; if (!cert) return; @@ -270,7 +273,6 @@ void db_crl::newItem(pki_x509 *cert) "issuer:copy", &ext_ctx)); } } - QSqlError err; if (widget->setCrlNumber->isChecked()) { a1int num; num.setDec(widget->crlNumber->text()); @@ -281,16 +283,17 @@ void db_crl::newItem(pki_x509 *cert) crl->setLastUpdate(widget->lastUpdate->getDate()); crl->setNextUpdate(widget->nextUpdate->getDate()); crl->sign(cert->getRefKey(), widget->hashAlgo->currentHash()); - if (!db.transaction()) + + Transaction; + if (!TransBegin()) throw errorEx(tr("Failed to initiate DB transaction")); - transact = true; cert->setCrlExpire(widget->nextUpdate->getDate()); SQL_PREPARE(q, "UPDATE authority set crlNo=?, crlExpire=? WHERE item=?"); q.bindValue(0, (uint)cert->getCrlNumber().getLong()); q.bindValue(1, widget->nextUpdate->getDate().toPlain()); q.bindValue(2, cert->getSqlItemId()); q.exec(); - err = q.lastError(); + QSqlError err = q.lastError(); if (err.isValid()) throw errorEx(tr("Database error: ").arg(err.text())); SQL_PREPARE(q, "UPDATE revocations set crlNo=? " @@ -301,16 +304,14 @@ void db_crl::newItem(pki_x509 *cert) err = q.lastError(); if (err.isValid()) throw errorEx(tr("Database error: ").arg(err.text())); - insertPKI_noTransaction(crl); + insertPKI(crl); err = db.lastError(); if (err.isValid()) throw errorEx(tr("Database error: ").arg(err.text())); - db.commit(); + TransCommit(); createSuccess((crl)); } catch (errorEx &err) { - if (transact) - db.rollback(); MainWindow::Error(err); if (crl) delete crl; diff --git a/lib/db_temp.cpp b/lib/db_temp.cpp index c7a77394..703d08c8 100644 --- a/lib/db_temp.cpp +++ b/lib/db_temp.cpp @@ -153,12 +153,12 @@ bool db_temp::alterTemp(pki_temp *temp) { XSqlQuery q; QSqlError e; - QSqlDatabase db = QSqlDatabase::database(); if (!runTempDlg(temp)) return false; - if (!db.transaction()) + Transaction; + if (!TransBegin()) return false; SQL_PREPARE(q, "UPDATE templates SET version=?, template=? WHERE item=?"); q.bindValue(0, TMPL_VERSION); @@ -168,10 +168,10 @@ bool db_temp::alterTemp(pki_temp *temp) e = q.lastError(); mainwin->dbSqlError(e); if (e.isValid()) { - db.rollback(); + TransRollback(); return false; } updateItem(temp, temp->getIntName(), temp->getComment()); - db.commit(); + TransCommit(); return true; } diff --git a/lib/pki_base.h b/lib/pki_base.h index ebdd1e34..5af08dcb 100644 --- a/lib/pki_base.h +++ b/lib/pki_base.h @@ -11,22 +11,17 @@ #include #include #include -#include #include "asn1time.h" #include "pkcs11_lib.h" #include "db.h" #include "base.h" #include "headerlist.h" +#include "sql.h" #define __ME QString("(%1:%2)").arg(getClassName()).arg(getIntName()) #define pki_openssl_error() _openssl_error(__ME, C_FILE, __LINE__) #define pki_ign_openssl_error() _ign_openssl_error(__ME, C_FILE, __LINE__) -#define SQL_PREPARE(q,cmd) do { \ - (q).prepare(cmd); \ - (q).location(__FILE__,__LINE__); \ -} while (0) - enum pki_source { unknown, imported, @@ -42,72 +37,6 @@ enum pki_source { #define VIEW_item_source 4 #define VIEW_item_comment 5 -class XSqlQuery: public QSqlQuery -{ - QString lastq; - const char *file; - int line; - public: - QString query_details() - { - QString lq = lastq; - QList list = boundValues().values(); - QStringList sl; - for (int i = 0; i < list.size(); ++i) - sl << list.at(i).toString(); - if (sl.size()) - lq += QString("[%1]").arg(sl.join(", ")); - return QString("%1:%2 (%3)") - .arg(file).arg(line).arg(lq); - } - QSqlError lastError() - { - QSqlError e = QSqlQuery::lastError(); - if (!e.isValid()) - return e; - QString dt = e.driverText(); - e.setDriverText(QString("%1 - %2") - .arg(dt).arg(query_details())); - return e; - } - XSqlQuery() : QSqlQuery() { } - XSqlQuery(QString q) : QSqlQuery(q) - { - file = ""; line = 0; - lastq = q; - } - bool exec(QString q) - { - lastq = q; - file = ""; line = 0; - return QSqlQuery::exec(q); - } - bool exec() - { - QString res; - setForwardOnly(true); - bool r = QSqlQuery::exec(); - if (isSelect()) - res = QString("Rows selected: %1").arg(size()); - else - res = QString("Rows affected: %1") - .arg(numRowsAffected()); - qDebug() << QString("QUERY: %1 - %2") - .arg(query_details()).arg(res); - return r; - } - bool prepare(QString q) - { - lastq = q; - setForwardOnly(true); - return QSqlQuery::prepare(q); - } - void location(const char *f, int l) - { - file = f; line = l; - } -}; - class pki_base : public QObject { Q_OBJECT diff --git a/lib/pki_x509.cpp b/lib/pki_x509.cpp index 3fc35658..d0e1852e 100644 --- a/lib/pki_x509.cpp +++ b/lib/pki_x509.cpp @@ -111,7 +111,7 @@ QSqlError pki_x509::insertSqlData() q.bindValue(1, now.toPlain()); q.exec(); if (fromDataRevList.size() > 0) - fromDataRevList.sqlUpdateNoTrans(sqlItemId); + fromDataRevList.sqlUpdate(sqlItemId); return q.lastError(); } diff --git a/lib/sql.cpp b/lib/sql.cpp new file mode 100644 index 00000000..452c6160 --- /dev/null +++ b/lib/sql.cpp @@ -0,0 +1,146 @@ +/* vi: set sw=4 ts=4: + * + * Copyright (C) 2017 Christian Hohnstaedt. + * + * All rights reserved. + */ + +#include +#include +#include "base.h" +#include "sql.h" + +int DbTransaction::mutex; +int DbTransaction::error; + +void DbTransaction::debug(const char *func, const char *file, int line) +{ + QString f = file; + qDebug() << QString("%1(%2) Transaction: %3 Level %4, E:%5 ") + .arg(file + QString(file).lastIndexOf("/") +1) + .arg(line).arg(func).arg(mutex).arg(error); +} + +DbTransaction::DbTransaction() +{ + has_begun = false; +} + +DbTransaction::~DbTransaction() +{ + if (has_begun) + rollback("Destructor", 0); +} + +bool DbTransaction::begin(const char *file, int line) +{ + QSqlDatabase db = QSqlDatabase::database(); + if (db.transaction()) { + has_begun = true; + if (mutex++ == 0) + error = 0; + debug("Begin", file, line); + return true; + } + return false; +} + +bool DbTransaction::commit(const char *file, int line) +{ + if (mutex > 0) + mutex--; + else + qCritical() << "Unbalanced DB Transaction (commit)"; + debug("Commit", file, line); + has_begun = false; + if (mutex > 0) + return true; + QSqlDatabase db = QSqlDatabase::database(); + return error ? db.rollback() : db.commit(); +} + +bool DbTransaction::rollback(const char *file, int line) +{ + error++; + if (mutex > 0) + mutex--; + else + qCritical() << "Unbalanced DB Transaction (rollback)"; + debug("Rollback", file, line); + has_begun = false; + if (mutex > 0) + return true; + QSqlDatabase db = QSqlDatabase::database(); + return db.rollback(); +} + +bool DbTransaction::done(QSqlError e, const char *file, int line) +{ + return e.isValid() ? rollback(file, line) : commit(file, line); +} + + +QString XSqlQuery::query_details() +{ + QString lq = lastq; + QList list = boundValues().values(); + QStringList sl; + for (int i = 0; i < list.size(); ++i) + sl << list.at(i).toString(); + if (sl.size()) + lq += QString("[%1]").arg(sl.join(", ")); + return QString("%1:%2 (%3)").arg(file).arg(line).arg(lq); +} + +QSqlError XSqlQuery::lastError() +{ + QSqlError e = QSqlQuery::lastError(); + if (!e.isValid()) + return e; + QString dt = e.driverText(); + e.setDriverText(QString("%1 - %2").arg(dt).arg(query_details())); + return e; +} + +XSqlQuery::XSqlQuery() : QSqlQuery() +{ +} + +XSqlQuery::XSqlQuery(QString q) : QSqlQuery(q) +{ + file = ""; line = 0; + lastq = q; +} + +bool XSqlQuery::exec(QString q) +{ + lastq = q; + file = ""; line = 0; + return QSqlQuery::exec(q); +} + +bool XSqlQuery::exec() +{ + QString res; + setForwardOnly(true); + bool r = QSqlQuery::exec(); + if (isSelect()) + res = QString("Rows selected: %1").arg(size()); + else + res = QString("Rows affected: %1").arg(numRowsAffected()); + qDebug() << QString("QUERY: %1 - %2").arg(query_details()).arg(res); + return r; +} + +bool XSqlQuery::prepare(QString q) +{ + lastq = q; + setForwardOnly(true); + return QSqlQuery::prepare(q); +} + +void XSqlQuery::location(const char *f, int l) +{ + file = f + QString(f).lastIndexOf("/") +1; + line = l; +} diff --git a/lib/sql.h b/lib/sql.h new file mode 100644 index 00000000..ba31bbcc --- /dev/null +++ b/lib/sql.h @@ -0,0 +1,59 @@ +/* vi: set sw=4 ts=4: + * + * Copyright (C) 2017 Christian Hohnstaedt. + * + * All rights reserved. + */ + +#ifndef __SQL_H +#define __SQL_H + +#include + +#define SQL_PREPARE(q,cmd) do { \ + (q).prepare(cmd); \ + (q).location(__FILE__,__LINE__); \ +} while (0) + +class DbTransaction +{ + private: + static int mutex; + static int error; + bool has_begun; + void debug(const char *func, const char *file, int line); + + public: + DbTransaction(); + ~DbTransaction(); + bool begin(const char *file, int line); + bool commit(const char *file, int line); + bool rollback(const char *file, int line); + bool done(QSqlError e, const char *file, int line); +}; + +#define Transaction DbTransaction __trans +#define TransBegin() __trans.begin(__FILE__, __LINE__) +#define TransCommit() __trans.commit(__FILE__, __LINE__) +#define TransRollback() __trans.rollback(__FILE__, __LINE__) +#define TransDone(e) __trans.done(e, __FILE__, __LINE__); + +class XSqlQuery: public QSqlQuery +{ + private: + QString lastq; + const char *file; + int line; + public: + XSqlQuery(); + XSqlQuery(QString q); + + QString query_details(); + QSqlError lastError(); + bool exec(QString q); + bool exec(); + bool prepare(QString q); + void location(const char *f, int l); +}; + +#endif diff --git a/lib/x509rev.cpp b/lib/x509rev.cpp index 71ad0c9d..487e5f5d 100644 --- a/lib/x509rev.cpp +++ b/lib/x509rev.cpp @@ -252,10 +252,13 @@ x509revList x509revList::fromSql(QVariant caId) return list; } -bool x509revList::sqlUpdateNoTrans(QVariant caId) +bool x509revList::sqlUpdate(QVariant caId) { XSqlQuery q; - QSqlDatabase db = QSqlDatabase::database(); + Transaction; + + if (!TransBegin()) + return false; SQL_PREPARE(q, "DELETE FROM revocations WHERE caId=?"); q.bindValue(0, caId); @@ -271,26 +274,9 @@ bool x509revList::sqlUpdateNoTrans(QVariant caId) x509rev r = at(i); r.executeQuery(q); if (q.lastError().isValid()) - return false; + return false; } merged = false; + TransCommit(); return true; } - -bool x509revList::sqlUpdate(QVariant caId) -{ - return sqlUpdateNoTrans(caId); -#warning Fix nested db.transaction() -#if 0 - QSqlDatabase db = QSqlDatabase::database(); - - if (!db.transaction()) - return false; - if (!sqlUpdateNoTrans(caId)) { - db.rollback(); - return false; - } - db.commit(); - return true; -#endif -} diff --git a/lib/x509rev.h b/lib/x509rev.h index f2aea590..9004dc9a 100644 --- a/lib/x509rev.h +++ b/lib/x509rev.h @@ -132,6 +132,5 @@ class x509revList : public QList } } bool sqlUpdate(QVariant caId); - bool sqlUpdateNoTrans(QVariant caId); }; #endif diff --git a/widgets/MW_database.cpp b/widgets/MW_database.cpp index 15f973ae..e4357f50 100644 --- a/widgets/MW_database.cpp +++ b/widgets/MW_database.cpp @@ -330,19 +330,20 @@ QSqlError MainWindow::initSqlDB() QString schema = getSetting("schema"); i = schema.toInt(); } - if (!db.transaction()) + Transaction; + if (!TransBegin()) return db.lastError(); for (; i < ARRAY_SIZE(schemas); i++) { foreach(QString sql, schemas[i]) { qDebug("EXEC[%d]: '%s'", i, CCHAR(sql)); if (!q.exec(sql)) { - db.rollback(); + TransRollback(); return q.lastError(); } } } - db.commit(); + TransCommit(); return QSqlError(); } diff --git a/widgets/MainWindow.cpp b/widgets/MainWindow.cpp index 37c7b2a7..9efa4fec 100644 --- a/widgets/MainWindow.cpp +++ b/widgets/MainWindow.cpp @@ -655,12 +655,13 @@ void MainWindow::changeDbPass() QList key_list = keys->sqlSELECTpki( "SELECT item FROM private_keys WHERE ownPass=0"); - if (!db.transaction()) { - errorEx e(tr("Transaction start failed")); - Error(e); - return; - } try { + Transaction; + if (!TransBegin()) { + errorEx e(tr("Transaction start failed")); + Error(e); + return; + } foreach(pki_evp *key, key_list) { EVP_PKEY *evp = key->decryptKey(); key->set_evp_key(evp); @@ -668,14 +669,12 @@ void MainWindow::changeDbPass() key->sqlUpdatePrivateKey(); } storeSetting("pwhash", passhash); + TransCommit(); + pki_evp::passHash = passhash; + pki_evp::passwd = pass; } catch (errorEx &e) { Error(e); - db.rollback(); - return; } - db.commit(); - pki_evp::passHash = passhash; - pki_evp::passwd = pass; } int MainWindow::initPass(QString dbName) diff --git a/xca.pro b/xca.pro index 9956f3f3..25401c4e 100644 --- a/xca.pro +++ b/xca.pro @@ -4,7 +4,7 @@ TARGET = xca DEPENDPATH += . lang lib ui widgets INCLUDEPATH += . lib widgets QMAKE_MAKEFILE = makefile -QT += widgets +QT = gui core sql widgets RESOURCES = img/imgres.rcc RC_FILE = img/w32res.rc @@ -73,6 +73,7 @@ HEADERS += lib/asn1int.h \ lib/x509v3ext.h \ lib/builtin_curves.h \ lib/entropy.h \ + lib/sql.h \ widgets/CertDetail.h \ widgets/CertExtend.h \ widgets/clicklabel.h \ @@ -101,9 +102,10 @@ HEADERS += lib/asn1int.h \ widgets/X509SuperTreeView.h \ widgets/XcaHeaderView.h \ widgets/OidResolver.h \ - widgets/ItemCombo.h \ - widgets/XcaDialog.h \ - widgets/XcaProxyModel.h + widgets/ItemCombo.h \ + widgets/XcaDialog.h \ + widgets/XcaProxyModel.h \ + widgets/OpenDb.h FORMS += ui/CaProperties.ui \ ui/CertDetail.ui \ @@ -125,7 +127,9 @@ FORMS += ui/CaProperties.ui \ ui/v3ext.ui \ ui/OidResolver.ui \ ui/XcaDialog.ui \ - ui/RevocationList.ui + ui/RevocationList.ui \ + ui/OpenDb.ui \ + ui/ItemProperties.ui SOURCES += lib/asn1int.cpp \ lib/asn1time.cpp \ @@ -165,6 +169,7 @@ SOURCES += lib/asn1int.cpp \ lib/builtin_curves.cpp \ lib/entropy.cpp \ lib/version.cpp \ + lib/sql.cpp \ widgets/CertDetail.cpp \ widgets/CertExtend.cpp \ widgets/clicklabel.cpp \ @@ -190,13 +195,14 @@ SOURCES += lib/asn1int.cpp \ widgets/SearchPkcs11.cpp \ widgets/RevocationList.cpp \ widgets/XcaTreeView.cpp \ - widgets/CertTreeView.cpp \ - widgets/KeyTreeView.cpp \ - widgets/ReqTreeView.cpp \ - widgets/TempTreeView.cpp \ - widgets/X509SuperTreeView.cpp \ - widgets/XcaHeaderView.cpp \ - widgets/OidResolver.cpp \ - widgets/XcaProxyModel.cpp + widgets/CertTreeView.cpp \ + widgets/KeyTreeView.cpp \ + widgets/ReqTreeView.cpp \ + widgets/TempTreeView.cpp \ + widgets/X509SuperTreeView.cpp \ + widgets/XcaHeaderView.cpp \ + widgets/OidResolver.cpp \ + widgets/XcaProxyModel.cpp \ + widgets/OpenDb.cpp TRANSLATIONS += lang/xca_de.ts lang/xca_es.ts lang/xca_ru.ts lang/xca.ts lang/xca_tr.ts lang/xca_fr.ts lang/xca_hr.ts lang/xca_sk.ts