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

src/cli/Add.cpp

@@ -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;
     }

src/cli/AddGroup.cpp

@@ -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;
     }

src/cli/Create.cpp

@@ -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;
     }

src/cli/Edit.cpp

@@ -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;
     }

src/cli/Import.cpp

@@ -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;
     }

src/cli/Merge.cpp

@@ -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;
         }

src/cli/Move.cpp

@@ -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;
     }

src/cli/Remove.cpp

@@ -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;
     }

src/cli/RemoveGroup.cpp

@@ -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;
     }

src/core/Database.cpp

@@ -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) {

src/core/Database.h

@@ -71,7 +71,7 @@ class Database : public QObject
               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 @@ class Database : public QObject
     QByteArray transformedMasterKey() const;
 
     static Database* databaseByUuid(const QUuid& uuid);
-    static Database* databaseByFilePath(const QString& filePath);
 
 public slots:
     void markAsModified();
@@ -192,7 +191,6 @@ private slots:
 
     QUuid m_uuid;
     static QHash<QUuid, QPointer<Database>> s_uuidMap;
-    static QHash<QString, QPointer<Database>> s_filePathMap;
 };
 
 #endif // KEEPASSX_DATABASE_H

src/gui/DatabaseWidget.cpp

@@ -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()