Remove lock file and cleanup file handling #1231
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,7 +19,6 @@ | |
| #include "DatabaseTabWidget.h" | ||
|
|
||
| #include <QFileInfo> | ||
| #include <QTabWidget> | ||
| #include <QPushButton> | ||
|
|
||
|
|
@@ -42,8 +41,6 @@ | |
|
|
||
| DatabaseManagerStruct::DatabaseManagerStruct() | ||
| : dbWidget(nullptr) | ||
| , modified(false) | ||
| , readOnly(false) | ||
| { | ||
|
|
@@ -107,7 +104,7 @@ void DatabaseTabWidget::newDatabase() | |
| void DatabaseTabWidget::openDatabase() | ||
| { | ||
| QString filter = QString("%1 (*.kdbx);;%2 (*)").arg(tr("KeePass 2 Database"), tr("All files")); | ||
| QString fileName = fileDialog()->getOpenFileName(this, tr("Open database"), QDir::homePath(), | ||
| filter); | ||
| if (!fileName.isEmpty()) { | ||
| openDatabase(fileName); | ||
|
|
@@ -128,10 +125,10 @@ void DatabaseTabWidget::openDatabase(const QString& fileName, const QString& pw, | |
| QHashIterator<Database*, DatabaseManagerStruct> i(m_dbList); | ||
| while (i.hasNext()) { | ||
| i.next(); | ||
| if (i.value().fileInfo.canonicalFilePath() == canonicalFilePath) { | ||
| if (!i.value().dbWidget->dbHasKey() && !(pw.isNull() && keyFile.isEmpty())) { | ||
| // If the database is locked and a pw or keyfile is provided, unlock it | ||
| i.value().dbWidget->switchToOpenDatabase(i.value().fileInfo.absoluteFilePath(), pw, keyFile); | ||
| } else { | ||
| setCurrentIndex(databaseIndex(i.key())); | ||
| } | ||
|
|
@@ -157,63 +154,22 @@ void DatabaseTabWidget::openDatabase(const QString& fileName, const QString& pw, | |
| } | ||
| file.close(); | ||
|
|
||
| Database* db = new Database(); | ||
| dbStruct.dbWidget = new DatabaseWidget(db, this); | ||
| dbStruct.fileInfo = fileInfo; | ||
|
|
||
| insertDatabase(db, dbStruct); | ||
|
|
||
| if (dbStruct.readOnly) { | ||
| emit messageTab(tr("File opened in read only mode."), MessageWidget::Warning); | ||
| } | ||
|
|
||
| updateLastDatabases(dbStruct.fileInfo.absoluteFilePath()); | ||
|
|
||
| if (!pw.isNull() || !keyFile.isEmpty()) { | ||
| dbStruct.dbWidget->switchToOpenDatabase(dbStruct.fileInfo.absoluteFilePath(), pw, keyFile); | ||
| } else { | ||
| dbStruct.dbWidget->switchToOpenDatabase(dbStruct.fileInfo.absoluteFilePath()); | ||
| } | ||
| emit messageDismissGlobal(); | ||
| } | ||
|
|
@@ -322,15 +278,14 @@ bool DatabaseTabWidget::closeDatabase(Database* db) | |
| void DatabaseTabWidget::deleteDatabase(Database* db) | ||
| { | ||
| const DatabaseManagerStruct dbStruct = m_dbList.value(db); | ||
| bool emitDatabaseWithFileClosed = dbStruct.fileInfo.exists() && !dbStruct.readOnly; | ||
| QString filePath = dbStruct.fileInfo.absoluteFilePath(); | ||
|
|
||
| int index = databaseIndex(db); | ||
|
|
||
| removeTab(index); | ||
| toggleTabbar(); | ||
| m_dbList.remove(db); | ||
| delete dbStruct.dbWidget; | ||
| delete db; | ||
|
|
||
|
|
@@ -349,24 +304,31 @@ bool DatabaseTabWidget::closeAllDatabases() | |
| return true; | ||
| } | ||
|
|
||
| bool DatabaseTabWidget::saveDatabase(Database* db, QString filePath) | ||
| { | ||
| DatabaseManagerStruct& dbStruct = m_dbList[db]; | ||
|
|
||
| // Never allow saving a locked database; it causes corruption | ||
| Q_ASSERT(dbStruct.dbWidget->currentMode() != DatabaseWidget::LockedMode); | ||
| // Release build interlock | ||
| if (dbStruct.dbWidget->currentMode() == DatabaseWidget::LockedMode) { | ||
| // We return true since a save is not required | ||
| return true; | ||
| } | ||
|
|
||
| if (!dbStruct.readOnly) { | ||
| if (filePath.isEmpty()) { | ||
| filePath = dbStruct.fileInfo.canonicalFilePath(); | ||
| } | ||
|
|
||
| dbStruct.dbWidget->blockAutoReload(true); | ||
| QString errorMessage = db->saveToFile(filePath); | ||
| dbStruct.dbWidget->blockAutoReload(false); | ||
|
|
||
| if (errorMessage.isEmpty()) { | ||
| // successfully saved database file | ||
| dbStruct.modified = false; | ||
| dbStruct.fileInfo = QFileInfo(filePath); | ||
| dbStruct.dbWidget->databaseSaved(); | ||
| updateTabName(db); | ||
| emit messageDismissTab(); | ||
|
|
@@ -387,76 +349,31 @@ bool DatabaseTabWidget::saveDatabaseAs(Database* db) | |
| { | ||
| while (true) { | ||
| DatabaseManagerStruct& dbStruct = m_dbList[db]; | ||
| QString oldFilePath; | ||
| if (dbStruct.fileInfo.exists()) { | ||
| oldFilePath = dbStruct.fileInfo.absoluteFilePath(); | ||
| } else { | ||
| oldFilePath = QDir::toNativeSeparators(QDir::homePath() + "/" + tr("Passwords").append(".kdbx")); | ||
| } | ||
| QString newFilePath = fileDialog()->getSaveFileName(this, tr("Save database as"), oldFilePath, | ||
| tr("KeePass 2 Database").append(" (*.kdbx)"), | ||
| nullptr, 0, "kdbx"); | ||
| if (!newFilePath.isEmpty()) { | ||
| // Ensure we don't recurse back into this function | ||
| dbStruct.readOnly = false; | ||
|
|
||
| if (!saveDatabase(db, newFilePath)) { | ||
| // Failed to save, try again | ||
| continue; | ||
| } | ||
|
|
||
| dbStruct.dbWidget->updateFilePath(dbStruct.fileInfo.absoluteFilePath()); | ||
| updateLastDatabases(dbStruct.fileInfo.absoluteFilePath()); | ||
| return true; | ||
| } | ||
|
|
||
| // Canceled file selection | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -548,7 +465,7 @@ bool DatabaseTabWidget::canSave(int index) | |
| } | ||
|
|
||
| const DatabaseManagerStruct& dbStruct = indexDatabaseManagerStruct(index); | ||
| return dbStruct.modified && !dbStruct.readOnly; | ||
| } | ||
|
|
||
| bool DatabaseTabWidget::isModified(int index) | ||
|
|
@@ -566,7 +483,7 @@ QString DatabaseTabWidget::databasePath(int index) | |
| index = currentIndex(); | ||
| } | ||
|
|
||
| return indexDatabaseManagerStruct(index).fileInfo.absoluteFilePath(); | ||
| } | ||
|
|
||
|
|
||
|
|
@@ -579,21 +496,18 @@ void DatabaseTabWidget::updateTabName(Database* db) | |
|
|
||
| QString tabName; | ||
|
|
||
| if (dbStruct.fileInfo.exists()) { | ||
| if (db->metadata()->name().isEmpty()) { | ||
|
There was a problem hiding this comment. @droidmonkey was there a possibility that the database was There was a problem hiding this comment. I am not sure what you mean by this. There was a problem hiding this comment. @droidmonkey nevermind, I was thinking about a possible execution path that might have occurred with the previous code. |
||
| tabName = dbStruct.fileInfo.fileName(); | ||
| } else { | ||
| tabName = db->metadata()->name(); | ||
| } | ||
|
|
||
| setTabToolTip(index, dbStruct.fileInfo.absoluteFilePath()); | ||
| } else { | ||
| if (db->metadata()->name().isEmpty()) { | ||
| tabName = tr("New database"); | ||
| } else { | ||
| tabName = QString("%1 [%2]").arg(db->metadata()->name(), tr("New database")); | ||
| } | ||
| } | ||
|
|
@@ -629,8 +543,7 @@ void DatabaseTabWidget::updateTabNameFromDbWidgetSender() | |
| Group *autoload = db->rootGroup()->findChildByName("AutoOpen"); | ||
| if (autoload) { | ||
| const DatabaseManagerStruct& dbStruct = m_dbList.value(db); | ||
| QDir dbFolder(dbStruct.fileInfo.canonicalPath()); | ||
| for (auto entry : autoload->entries()) { | ||
| if (entry->url().isEmpty() || entry->password().isEmpty()) { | ||
| continue; | ||
|
|
@@ -756,17 +669,14 @@ void DatabaseTabWidget::lockDatabases() | |
| DatabaseWidget* dbWidget = static_cast<DatabaseWidget*>(widget(i)); | ||
| Database* db = databaseFromDatabaseWidget(dbWidget); | ||
|
|
||
|
There was a problem hiding this comment. @droidmonkey shouldn't we keep the There was a problem hiding this comment. Added back in, good idea. |
||
| if (dbWidget->currentMode() == DatabaseWidget::LockedMode || !dbWidget->dbHasKey()) { | ||
| continue; | ||
| } | ||
|
|
||
| // show the correct tab widget before we are asking questions about it | ||
| setCurrentWidget(dbWidget); | ||
|
|
||
| if (dbWidget->currentMode() == DatabaseWidget::EditMode && dbWidget->isEditWidgetModified()) { | ||
| QMessageBox::StandardButton result = | ||
| MessageBox::question( | ||
| this, tr("Lock database"), | ||
|
|
@@ -777,23 +687,7 @@ void DatabaseTabWidget::lockDatabases() | |
| } | ||
| } | ||
|
|
||
| if (m_dbList[db].modified) { | ||
| QMessageBox::StandardButton result = | ||
| MessageBox::question( | ||
| this, tr("Lock database"), | ||
|
|
@@ -828,7 +722,7 @@ void DatabaseTabWidget::modified() | |
| Database* db = static_cast<Database*>(sender()); | ||
| DatabaseManagerStruct& dbStruct = m_dbList[db]; | ||
|
|
||
| if (config()->get("AutoSaveAfterEveryChange").toBool() && !dbStruct.readOnly) { | ||
| saveDatabase(db); | ||
|
There was a problem hiding this comment. I would like an assertion here for There was a problem hiding this comment. I put one inside There was a problem hiding this comment. Perhaps you should leave both. The assertion will ensure we notice in debug mode, but the safeguard condition will ensure it never happens in production either. There was a problem hiding this comment. I was thinking that too. Will do! |
||
| return; | ||
| } | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This
ifwon't be reached if true since the above assert will abort the execution. Better remove one of themEDIT: Nevermind, the assert will only be executed in debug mode.