Cleanup database save functions

* Make a clear distinction between saving to the existing file path and saving to a new file path
* Use proper save function calls in CLI
This commit is contained in:
Jonathan White 2019-10-20 18:38:52 -04:00
parent a876b3b72f
commit 6b746913e4
12 changed files with 33 additions and 57 deletions

View File

@ -74,8 +74,7 @@ int Add::executeWithDatabase(QSharedPointer<Database> database, QSharedPointer<Q
TextStream errorTextStream(Utils::STDERR, QIODevice::WriteOnly);
const QStringList args = parser->positionalArguments();
const QString& databasePath = args.at(0);
const QString& entryPath = args.at(1);
auto& entryPath = args.at(1);
// Cannot use those 2 options at the same time!
if (parser->isSet(Add::GenerateOption) && parser->isSet(Add::PasswordPromptOption)) {
@ -119,7 +118,7 @@ int Add::executeWithDatabase(QSharedPointer<Database> database, QSharedPointer<Q
}
QString errorMessage;
if (!database->save(databasePath, &errorMessage, true, false)) {
if (!database->save(&errorMessage, true, false)) {
errorTextStream << QObject::tr("Writing the database failed %1.").arg(errorMessage) << endl;
return EXIT_FAILURE;
}

View File

@ -43,7 +43,6 @@ int AddGroup::executeWithDatabase(QSharedPointer<Database> database, QSharedPoin
TextStream errorTextStream(Utils::STDERR, QIODevice::WriteOnly);
const QStringList args = parser->positionalArguments();
const QString& databasePath = args.at(0);
const QString& groupPath = args.at(1);
QStringList pathParts = groupPath.split("/");
@ -68,7 +67,7 @@ int AddGroup::executeWithDatabase(QSharedPointer<Database> database, QSharedPoin
newGroup->setParent(parentGroup);
QString errorMessage;
if (!database->save(databasePath, &errorMessage, true, false)) {
if (!database->save(&errorMessage, true, false)) {
errorTextStream << QObject::tr("Writing the database failed %1.").arg(errorMessage) << endl;
return EXIT_FAILURE;
}

View File

@ -97,7 +97,7 @@ int Create::execute(const QStringList& arguments)
db->setKey(key);
QString errorMessage;
if (!db->save(databaseFilename, &errorMessage, true, false)) {
if (!db->saveAs(databaseFilename, &errorMessage, true, false)) {
err << QObject::tr("Failed to save the database: %1.").arg(errorMessage) << endl;
return EXIT_FAILURE;
}

View File

@ -65,7 +65,6 @@ int Edit::executeWithDatabase(QSharedPointer<Database> database, QSharedPointer<
TextStream errorTextStream(Utils::STDERR, QIODevice::WriteOnly);
const QStringList args = parser->positionalArguments();
const QString& databasePath = args.at(0);
const QString& entryPath = args.at(1);
// Cannot use those 2 options at the same time!
@ -126,7 +125,7 @@ int Edit::executeWithDatabase(QSharedPointer<Database> database, QSharedPointer<
entry->endUpdate();
QString errorMessage;
if (!database->save(databasePath, &errorMessage, true, false)) {
if (!database->save(&errorMessage, true, false)) {
errorTextStream << QObject::tr("Writing the database failed: %1").arg(errorMessage) << endl;
return EXIT_FAILURE;
}

View File

@ -90,7 +90,7 @@ int Import::execute(const QStringList& arguments)
return EXIT_FAILURE;
}
if (!db.save(dbPath, &errorMessage, true, false)) {
if (!db.saveAs(dbPath, &errorMessage, true, false)) {
errorTextStream << QObject::tr("Failed to save the database: %1.").arg(errorMessage) << endl;
return EXIT_FAILURE;
}

View File

@ -68,8 +68,8 @@ int Merge::executeWithDatabase(QSharedPointer<Database> database, QSharedPointer
const QStringList args = parser->positionalArguments();
QString toDatabasePath = args.at(0);
QString fromDatabasePath = args.at(1);
auto& toDatabasePath = args.at(0);
auto& fromDatabasePath = args.at(1);
QSharedPointer<Database> db2;
if (!parser->isSet(Merge::SameCredentialsOption)) {
@ -94,13 +94,13 @@ int Merge::executeWithDatabase(QSharedPointer<Database> database, QSharedPointer
Merger merger(db2.data(), database.data());
QStringList changeList = merger.merge();
for (QString mergeChange : changeList) {
for (auto& mergeChange : changeList) {
outputTextStream << "\t" << mergeChange << endl;
}
if (!changeList.isEmpty() && !parser->isSet(Merge::DryRunOption)) {
QString errorMessage;
if (!database->save(toDatabasePath, &errorMessage, true, false)) {
if (!database->save(&errorMessage, true, false)) {
errorTextStream << QObject::tr("Unable to save database to file : %1").arg(errorMessage) << endl;
return EXIT_FAILURE;
}

View File

@ -44,7 +44,6 @@ int Move::executeWithDatabase(QSharedPointer<Database> database, QSharedPointer<
TextStream errorTextStream(Utils::STDERR, QIODevice::WriteOnly);
const QStringList args = parser->positionalArguments();
const QString& databasePath = args.at(0);
const QString& entryPath = args.at(1);
const QString& destinationPath = args.at(2);
@ -70,7 +69,7 @@ int Move::executeWithDatabase(QSharedPointer<Database> database, QSharedPointer<
entry->endUpdate();
QString errorMessage;
if (!database->save(databasePath, &errorMessage, true, false)) {
if (!database->save(&errorMessage, true, false)) {
errorTextStream << QObject::tr("Writing the database failed %1.").arg(errorMessage) << endl;
return EXIT_FAILURE;
}

View File

@ -38,8 +38,7 @@ Remove::Remove()
int Remove::executeWithDatabase(QSharedPointer<Database> database, QSharedPointer<QCommandLineParser> parser)
{
bool quiet = parser->isSet(Command::QuietOption);
QString databasePath = parser->positionalArguments().at(0);
QString entryPath = parser->positionalArguments().at(1);
auto& entryPath = parser->positionalArguments().at(1);
TextStream outputTextStream(quiet ? Utils::DEVNULL : Utils::STDOUT, QIODevice::WriteOnly);
TextStream errorTextStream(Utils::STDERR, QIODevice::WriteOnly);
@ -61,7 +60,7 @@ int Remove::executeWithDatabase(QSharedPointer<Database> database, QSharedPointe
};
QString errorMessage;
if (!database->save(databasePath, &errorMessage, true, false)) {
if (!database->save(&errorMessage, true, false)) {
errorTextStream << QObject::tr("Unable to save database to file: %1").arg(errorMessage) << endl;
return EXIT_FAILURE;
}

View File

@ -42,7 +42,6 @@ RemoveGroup::~RemoveGroup()
int RemoveGroup::executeWithDatabase(QSharedPointer<Database> database, QSharedPointer<QCommandLineParser> parser)
{
bool quiet = parser->isSet(Command::QuietOption);
QString databasePath = parser->positionalArguments().at(0);
QString groupPath = parser->positionalArguments().at(1);
TextStream outputTextStream(quiet ? Utils::DEVNULL : Utils::STDOUT, QIODevice::WriteOnly);
@ -70,7 +69,7 @@ int RemoveGroup::executeWithDatabase(QSharedPointer<Database> database, QSharedP
};
QString errorMessage;
if (!database->save(databasePath, &errorMessage, true, false)) {
if (!database->save(&errorMessage, true, false)) {
errorTextStream << QObject::tr("Unable to save database to file: %1").arg(errorMessage) << endl;
return EXIT_FAILURE;
}

View File

@ -36,7 +36,6 @@
#include <QXmlStreamReader>
QHash<QUuid, QPointer<Database>> Database::s_uuidMap;
QHash<QString, QPointer<Database>> Database::s_filePathMap;
Database::Database()
: m_metadata(new Metadata(this))
@ -161,10 +160,8 @@ bool Database::open(const QString& filePath, QSharedPointer<const CompositeKey>
}
/**
* Save the database back to the file is has been opened from.
* This method behaves the same as its overloads.
*
* @see Database::save(const QString&, bool, bool, QString*)
* Save the database to the current file path. It is an error to call this function
* if no file path has been defined.
*
* @param atomic Use atomic file transactions
* @param backup Backup the existing database file, if exists
@ -176,20 +173,21 @@ bool Database::save(QString* error, bool atomic, bool backup)
Q_ASSERT(!m_data.filePath.isEmpty());
if (m_data.filePath.isEmpty()) {
if (error) {
*error = tr("Could not save, database has no file name.");
*error = tr("Could not save, database does not point to a valid file.");
}
return false;
}
return save(m_data.filePath, error, atomic, backup);
return saveAs(m_data.filePath, error, atomic, backup);
}
/**
* Save the database to a file.
* Save the database to a specific file.
*
* This function uses QTemporaryFile instead of QSaveFile due to a bug
* in Qt (https://bugreports.qt.io/browse/QTBUG-57299) that may prevent
* the QSaveFile from renaming itself when using Dropbox, Drive, or OneDrive.
* If atmoic is false, this function uses QTemporaryFile instead of QSaveFile
* due to a bug in Qt (https://bugreports.qt.io/browse/QTBUG-57299) that may
* prevent the QSaveFile from renaming itself when using Dropbox, Google Drive,
* or OneDrive.
*
* The risk in using QTemporaryFile is that the rename function is not atomic
* and may result in loss of data if there is a crash or power loss at the
@ -201,17 +199,20 @@ bool Database::save(QString* error, bool atomic, bool backup)
* @param error error message in case of failure
* @return true on success
*/
bool Database::save(const QString& filePath, QString* error, bool atomic, bool backup)
bool Database::saveAs(const QString& filePath, QString* error, bool atomic, bool backup)
{
// Disallow saving to the same file if read-only
if (m_data.isReadOnly && filePath == m_data.filePath) {
Q_ASSERT_X(false, "Database::save", "Could not save, database file is read-only.");
Q_ASSERT_X(false, "Database::saveAs", "Could not save, database file is read-only.");
if (error) {
*error = tr("Could not save, database file is read-only.");
}
return false;
}
// Clear read-only flag
setReadOnly(false);
auto& canonicalFilePath = QFileInfo::exists(filePath) ? QFileInfo(filePath).canonicalFilePath() : filePath;
if (atomic) {
@ -486,18 +487,11 @@ QString Database::canonicalFilePath() const
void Database::setFilePath(const QString& filePath)
{
if (filePath == m_data.filePath) {
return;
if (filePath != m_data.filePath) {
QString oldPath = m_data.filePath;
m_data.filePath = filePath;
emit filePathChanged(oldPath, filePath);
}
if (s_filePathMap.contains(m_data.filePath)) {
s_filePathMap.remove(m_data.filePath);
}
QString oldPath = m_data.filePath;
m_data.filePath = filePath;
s_filePathMap.insert(m_data.filePath, this);
emit filePathChanged(oldPath, filePath);
}
QList<DeletedObject> Database::deletedObjects()
@ -797,15 +791,6 @@ Database* Database::databaseByUuid(const QUuid& uuid)
return s_uuidMap.value(uuid, nullptr);
}
/**
* @param filePath file path of the database
* @return pointer to the database or nullptr if the database has not been opened
*/
Database* Database::databaseByFilePath(const QString& filePath)
{
return s_filePathMap.value(filePath, nullptr);
}
void Database::startModifiedTimer()
{
if (!m_emitModified) {

View File

@ -71,7 +71,7 @@ public:
QString* error = nullptr,
bool readOnly = false);
bool save(QString* error = nullptr, bool atomic = true, bool backup = false);
bool save(const QString& filePath, QString* error = nullptr, bool atomic = true, bool backup = false);
bool saveAs(const QString& filePath, QString* error = nullptr, bool atomic = true, bool backup = false);
bool extract(QByteArray&, QString* error = nullptr);
bool import(const QString& xmlExportPath, QString* error = nullptr);
@ -129,7 +129,6 @@ public:
QByteArray transformedMasterKey() const;
static Database* databaseByUuid(const QUuid& uuid);
static Database* databaseByFilePath(const QString& filePath);
public slots:
void markAsModified();
@ -192,7 +191,6 @@ private:
QUuid m_uuid;
static QHash<QUuid, QPointer<Database>> s_uuidMap;
static QHash<QString, QPointer<Database>> s_filePathMap;
};
#endif // KEEPASSX_DATABASE_H

View File

@ -1586,7 +1586,6 @@ EntryView* DatabaseWidget::entryView()
* ask to disable safe saves if it is unable to save after the third attempt.
* Set `attempt` to -1 to disable this behavior.
*
* @param attempt current save attempt or -1 to disable attempts
* @return true on success
*/
bool DatabaseWidget::save()