From a1b72c5eef87cf7126fe1b4a0559e21aea8c195e Mon Sep 17 00:00:00 2001 From: Martin Kleusberg Date: Thu, 23 May 2013 17:41:16 +0200 Subject: [PATCH] Extend the entire savepoint logic to allow multiple active savepoints Allow multiple savepoints in DBBrowserDB, i.e. setRestorePoint() doesn't simply return if the DB is already set to dirty but creates an additional savepoint. Track the names of all savepoints which have not been either saved or reverted yet. Finally, and that's the whole point of this commit, change the savepoint logic of the EditTableDialog. We used to have a transaction started when opening a database which was only committed when the file was closed again. This way the EditFileDialog could savely create a savepoint when being opened and just release it when OK was clicked or revert to it when cancel was clicked. Getting rid of the transaction broke this logic. The dialog still worked but as the savepoint was released when the changes were applied all changes made via the dialog were immediately committed to the database. So clicking the revert button had no effect on those changes. This is at best an unexpected behaviour but could also be a problem when some changes are reverted while others aren't. So, having the option now of keeping multiple savepoints opened this problem can be fixed by just creating a new savepoint every time the dialog is opened, reverting to it when it is cancelled and _not_ releasing it when OK is clicked. I hope this works! --- src/EditTableDialog.cpp | 13 ++++---- src/EditTableDialog.h | 1 + src/MainWindow.cpp | 8 ++--- src/sqlitedb.cpp | 71 ++++++++++++++++++++++++++++------------- src/sqlitedb.h | 4 ++- 5 files changed, 63 insertions(+), 34 deletions(-) diff --git a/src/EditTableDialog.cpp b/src/EditTableDialog.cpp index 4dae1981..8968ba13 100644 --- a/src/EditTableDialog.cpp +++ b/src/EditTableDialog.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include "sqlitedb.h" @@ -13,7 +14,8 @@ EditTableDialog::EditTableDialog(DBBrowserDB* db, const QString& tableName, QWid pdb(db), curTable(tableName), m_table(tableName), - m_bNewTable(true) + m_bNewTable(true), + m_sRestorePointName(QString("edittable_%1_save_%2").arg(curTable).arg(QDateTime::currentMSecsSinceEpoch())) { // Create UI ui->setupUi(this); @@ -33,7 +35,7 @@ EditTableDialog::EditTableDialog(DBBrowserDB* db, const QString& tableName, QWid } // And create a savepoint - pdb->executeSQL(QString("SAVEPOINT edittable_%1_save;").arg(curTable), false); + pdb->setRestorePoint(m_sRestorePointName); // Update UI ui->editTableName->setText(curTable); @@ -106,7 +108,7 @@ void EditTableDialog::accept() { // Creation of new table // we commit immediatly so no need to setdirty - if(!pdb->executeSQL(m_table.sql(), false)) + if(!pdb->executeSQL(m_table.sql())) { QMessageBox::warning( this, @@ -142,16 +144,13 @@ void EditTableDialog::accept() } } - // Release the savepoint - pdb->executeSQL(QString("RELEASE SAVEPOINT edittable_%1_save;").arg(curTable), false); - QDialog::accept(); } void EditTableDialog::reject() { // Then rollback to our savepoint - pdb->executeSQL(QString("ROLLBACK TO SAVEPOINT edittable_%1_save;").arg(curTable), false); + pdb->revert(m_sRestorePointName); QDialog::reject(); } diff --git a/src/EditTableDialog.h b/src/EditTableDialog.h index 472b1187..43417925 100644 --- a/src/EditTableDialog.h +++ b/src/EditTableDialog.h @@ -55,6 +55,7 @@ private: QStringList fields; SQLiteSyntaxHighlighter* m_sqliteSyntaxHighlighter; bool m_bNewTable; + QString m_sRestorePointName; }; #endif diff --git a/src/MainWindow.cpp b/src/MainWindow.cpp index 4e0d2ae1..c3e718d5 100644 --- a/src/MainWindow.cpp +++ b/src/MainWindow.cpp @@ -382,9 +382,9 @@ void MainWindow::fileExit() { QString msg = tr("Do you want to save the changes made to the database file %1?").arg(db.curDBFilename); if(QMessageBox::question( this, QApplication::applicationName() ,msg, QMessageBox::Yes, QMessageBox::No)==QMessageBox::Yes) - db.save(); + db.saveAll(); else - db.revert(); //not really necessary, I think... but will not hurt. + db.revertAll(); //not really necessary, I think... but will not hurt. } db.close(); } @@ -775,7 +775,7 @@ void MainWindow::dbState( bool dirty ) void MainWindow::fileSave() { if(db.isOpen()) - db.save(); + db.saveAll(); } void MainWindow::fileRevert() @@ -784,7 +784,7 @@ void MainWindow::fileRevert() QString msg = tr("Are you sure you want to undo all changes made to the database file '%1' since the last save?").arg(db.curDBFilename); if(QMessageBox::question(this, QApplication::applicationName(), msg, QMessageBox::Yes | QMessageBox::Default, QMessageBox::No | QMessageBox::Escape) == QMessageBox::Yes) { - db.revert(); + db.revertAll(); populateStructure(); resetBrowser(); } diff --git a/src/sqlitedb.cpp b/src/sqlitedb.cpp index 4ff0a582..5f38a785 100644 --- a/src/sqlitedb.cpp +++ b/src/sqlitedb.cpp @@ -91,13 +91,16 @@ bool DBBrowserDB::open ( const QString & db) bool DBBrowserDB::setRestorePoint(const QString& pointname) { - if (!isOpen()) return false; - if (dirty) return false; + if(!isOpen()) + return false; + if(savepointList.contains(pointname)) + return true; - if (_db){ + if(_db) + { QString query = QString("SAVEPOINT %1;").arg(pointname); - sqlite3_exec(_db, query.toUtf8(), - NULL,NULL,NULL); + sqlite3_exec(_db, query.toUtf8(), NULL, NULL, NULL); + savepointList.append(pointname); setDirty(true); } return true; @@ -105,29 +108,52 @@ bool DBBrowserDB::setRestorePoint(const QString& pointname) bool DBBrowserDB::save(const QString& pointname) { - if (!isOpen()) return false; + if(!isOpen() || savepointList.contains(pointname) == false) + return false; - if (_db){ + if(_db) + { QString query = QString("RELEASE %1;").arg(pointname); - sqlite3_exec(_db, query.toUtf8(), - NULL,NULL,NULL); - setDirty(false); + sqlite3_exec(_db, query.toUtf8(), NULL,NULL,NULL); + savepointList.removeAll(pointname); + setDirty(!savepointList.empty()); } return true; } bool DBBrowserDB::revert(const QString& pointname) { - if (!isOpen()) return false; + if(!isOpen() || savepointList.contains(pointname) == false) + return false; - if (_db){ + if(_db) + { QString query = QString("ROLLBACK TO SAVEPOINT %1;").arg(pointname); - sqlite3_exec(_db, query.toUtf8(), - NULL,NULL,NULL); + sqlite3_exec(_db, query.toUtf8(), NULL, NULL, NULL); query = QString("RELEASE %1;").arg(pointname); - sqlite3_exec(_db, query.toUtf8(), - NULL,NULL,NULL); - setDirty(false); + sqlite3_exec(_db, query.toUtf8(), NULL, NULL, NULL); + savepointList.removeAll(pointname); + setDirty(!savepointList.empty()); + } + return true; +} + +bool DBBrowserDB::saveAll() +{ + foreach(const QString& point, savepointList) + { + if(!save(point)) + return false; + } + return true; +} + +bool DBBrowserDB::revertAll() +{ + foreach(const QString& point, savepointList) + { + if(!revert(point)) + return false; } return true; } @@ -188,14 +214,15 @@ void DBBrowserDB::close (){ QObject::tr("Do you want to save the changes " "made to the database file %1?").arg(curDBFilename), QMessageBox::Yes, QMessageBox::No) == QMessageBox::Yes) - save(); + saveAll(); else - revert(); //not really necessary, I think... but will not hurt. + revertAll(); //not really necessary, I think... but will not hurt. } sqlite3_close(_db); } _db = 0; objMap.clear(); + savepointList.clear(); } bool DBBrowserDB::dump(const QString& filename) @@ -491,13 +518,13 @@ bool DBBrowserDB::createTable(const QString& name, const QList& sql.append(");"); // Execute it - return executeSQL(sql, false); + return executeSQL(sql); } bool DBBrowserDB::addColumn(const QString& tablename, const sqlb::FieldPtr& field) { QString sql = QString("ALTER TABLE `%1` ADD COLUMN %2").arg(tablename).arg(field->toString()); - return executeSQL(sql, false); + return executeSQL(sql); } bool DBBrowserDB::renameColumn(const QString& tablename, const QString& from, const QString& to, const QString& type) @@ -686,7 +713,7 @@ bool DBBrowserDB::dropColumn(const QString& tablename, const QString& column) bool DBBrowserDB::renameTable(const QString& from_table, const QString& to_table) { QString sql = QString("ALTER TABLE `%1` RENAME TO `%2`").arg(from_table, to_table); - if(!executeSQL(sql, false)) + if(!executeSQL(sql)) { QString error = QObject::tr("Error renaming table '%1' to '%2'." "Message from database engine:\n%3").arg(from_table).arg(to_table).arg(lastErrorMessage); diff --git a/src/sqlitedb.h b/src/sqlitedb.h index 8496ec92..0ba25d7b 100644 --- a/src/sqlitedb.h +++ b/src/sqlitedb.h @@ -72,6 +72,8 @@ public: bool setRestorePoint(const QString& pointname = "RESTOREPOINT"); bool save (const QString& pointname = "RESTOREPOINT"); bool revert (const QString& pointname = "RESTOREPOINT"); + bool saveAll(); + bool revertAll(); bool dump( const QString & filename); bool executeSQL ( const QString & statement, bool dirtyDB=true, bool logsql=true); bool executeMultiSQL(const QString& statement, bool dirty = true, bool log = false); @@ -112,7 +114,6 @@ public: sqlite3 * _db; - QStringList decodeCSV(const QString & csvfilename, char sep, char quote, int maxrecords, int * numfields); objectMap objMap; @@ -122,6 +123,7 @@ public: MainWindow* mainWindow; + QStringList savepointList; private: bool dirty; };