After 3 failed saves, offer to disable safe saves

* User is prompted to disable safe saves after three failed attempts
* Completely retooled basic settings to group settings logically
* Added setting for "atomic saves"

src/core/Config.cpp

@@ -115,6 +115,7 @@ void Config::init(const QString& fileName)
     m_defaults.insert("AutoReloadOnChange", true);
     m_defaults.insert("AutoSaveOnExit", false);
     m_defaults.insert("BackupBeforeSave", false);
+    m_defaults.insert("UseAtomicSaves", true);
     m_defaults.insert("SearchLimitGroup", false);
     m_defaults.insert("MinimizeOnCopy", false);
     m_defaults.insert("UseGroupIconOnEntryCreation", false);

src/core/Database.cpp

@@ -19,6 +19,7 @@
 #include "Database.h"
 
 #include <QFile>
+#include <QSaveFile>
 #include <QTemporaryFile>
 #include <QTextStream>
 #include <QTimer>
@@ -482,40 +483,92 @@ Database* Database::unlockFromStdin(QString databaseFilename, QString keyFilenam
  * wrong moment.
  *
  * @param filePath Absolute path of the file to save
- * @param keepOld Rename the original database file instead of deleting
+ * @param atomic Use atomic file transactions
+ * @param backup Backup the existing database file, if exists
  * @return error string, if any
  */
-QString Database::saveToFile(QString filePath, bool keepOld)
-{
-    KeePass2Writer writer;
-    QTemporaryFile saveFile;
-    if (saveFile.open()) {
-        // write the database to the file
-        setEmitModified(false);
-        writer.writeDatabase(&saveFile, this);
-        setEmitModified(true);
-
-        if (writer.hasError()) {
-            // the writer failed
-            return writer.errorString();
+QString Database::saveToFile(QString filePath, bool atomic, bool backup)
+{
+    QString error;
+    if (atomic) {
+        QSaveFile saveFile(filePath);
+        if (saveFile.open(QIODevice::WriteOnly)) {
+            // write the database to the file
+            error = writeDatabase(&saveFile);
+            if (!error.isEmpty()) {
+                return error;
+            }
+
+            if (backup) {
+                backupDatabase(filePath);
+            }
+
+            if (saveFile.commit()) {
+                // successfully saved database file
+                return {};
+            }
         }
-
-        saveFile.close(); // flush to disk
-
-        if (keepOld) {
-            QFile::remove(filePath + ".old");
-            QFile::rename(filePath, filePath + ".old");
+        error = saveFile.errorString();
+    } else {
+        QTemporaryFile tempFile;
+        if (tempFile.open()) {
+            // write the database to the file
+            error = writeDatabase(&tempFile);
+            if (!error.isEmpty()) {
+                return error;
+            }
+
+            tempFile.close(); // flush to disk
+
+            if (backup) {
+                backupDatabase(filePath);
+            }
+
+            // Delete the original db and move the temp file in place
+            QFile::remove(filePath);
+            if (tempFile.rename(filePath)) {
+                // successfully saved database file
+                tempFile.setAutoRemove(false);
+                return {};
+            }
         }
+        error = tempFile.errorString();
+    }
+    // Saving failed
+    return error;
+}
 
-        QFile::remove(filePath);
-        if (saveFile.rename(filePath)) {
-            // successfully saved database file
-            saveFile.setAutoRemove(false);
-            return {};
-        }
+QString Database::writeDatabase(QIODevice* device)
+{
+    KeePass2Writer writer;
+    setEmitModified(false);
+    writer.writeDatabase(device, this);
+    setEmitModified(true);
+
+    if (writer.hasError()) {
+        // the writer failed
+        return writer.errorString();
     }
+    return {};
+}
 
-    return saveFile.errorString();
+/**
+ * Remove the old backup and replace it with a new one
+ * backups are named <filename>.old.kdbx4
+ *
+ * @param filePath Path to the file to backup
+ * @return
+ */
+bool Database::backupDatabase(QString filePath)
+{
+    QString backupFilePath = filePath;
+    backupFilePath.replace(".kdbx", ".old.kdbx", Qt::CaseInsensitive);
+    if (!backupFilePath.endsWith(".old.kdbx")) {
+        // Fallback in case of poorly named file
+        backupFilePath = filePath + ".old";
+    }
+    QFile::remove(backupFilePath);
+    return QFile::copy(filePath, backupFilePath);
 }
 
 QSharedPointer<Kdf> Database::kdf() const

src/core/Database.h

@@ -32,6 +32,7 @@ enum class EntryReferenceType;
 class Group;
 class Metadata;
 class QTimer;
+class QIODevice;
 
 struct DeletedObject
 {
@@ -111,7 +112,7 @@ class Database : public QObject
     void emptyRecycleBin();
     void setEmitModified(bool value);
     void merge(const Database* other);
-    QString saveToFile(QString filePath, bool keepOld = false);
+    QString saveToFile(QString filePath, bool atomic = true, bool backup = false);
 
     /**
      * Returns a unique id that is only valid as long as the Database exists.
@@ -144,6 +145,8 @@ private slots:
     Group* findGroupRecursive(const Uuid& uuid, Group* group);
 
     void createRecycleBin();
+    QString writeDatabase(QIODevice* device);
+    bool backupDatabase(QString filePath);
 
     Metadata* const m_metadata;
     Group* m_rootGroup;

src/gui/DatabaseTabWidget.cpp

@@ -44,6 +44,7 @@ DatabaseManagerStruct::DatabaseManagerStruct()
     : dbWidget(nullptr)
     , modified(false)
     , readOnly(false)
+    , saveAttempts(0)
 {
 }
 
@@ -324,12 +325,14 @@ bool DatabaseTabWidget::saveDatabase(Database* db, QString filePath)
 
         dbStruct.dbWidget->blockAutoReload(true);
         // TODO: Make this async, but lock out the database widget to prevent re-entrance
-        QString errorMessage = db->saveToFile(filePath, config()->get("BackupBeforeSave").toBool());
+        bool useAtomicSaves = config()->get("UseAtomicSaves").toBool();
+        QString errorMessage = db->saveToFile(filePath, useAtomicSaves, config()->get("BackupBeforeSave").toBool());
         dbStruct.dbWidget->blockAutoReload(false);
 
         if (errorMessage.isEmpty()) {
             // successfully saved database file
             dbStruct.modified = false;
+            dbStruct.saveAttempts = 0;
             dbStruct.fileInfo = QFileInfo(filePath);
             dbStruct.dbWidget->databaseSaved();
             updateTabName(db);
@@ -338,6 +341,22 @@ bool DatabaseTabWidget::saveDatabase(Database* db, QString filePath)
         } else {
             dbStruct.modified = true;
             updateTabName(db);
+
+            if (++dbStruct.saveAttempts > 2 && useAtomicSaves) {
+                // Saving failed 3 times, issue a warning and attempt to resolve
+                auto choice = MessageBox::question(this, tr("Disable safe saves?"),
+                                                   tr("KeePassXC has failed to save the database multiple times. "
+                                                   "This is likely caused by file sync services holding a lock on "
+                                                   "the save file.\nDisable safe saves and try again?"),
+                                                   QMessageBox::Yes | QMessageBox::No, QMessageBox::Yes);
+                if (choice == QMessageBox::Yes) {
+                    config()->set("UseAtomicSaves", false);
+                    return saveDatabase(db, filePath);
+                }
+                // Reset save attempts without changing anything
+                dbStruct.saveAttempts = 0;
+            }
+
             emit messageTab(tr("Writing the database failed.").append("\n").append(errorMessage),
                             MessageWidget::Error);
             return false;

src/gui/DatabaseTabWidget.h

@@ -40,6 +40,7 @@ struct DatabaseManagerStruct
     QFileInfo fileInfo;
     bool modified;
     bool readOnly;
+    int saveAttempts;
 };
 
 Q_DECLARE_TYPEINFO(DatabaseManagerStruct, Q_MOVABLE_TYPE);

src/gui/SettingsWidget.cpp

@@ -118,6 +118,7 @@ void SettingsWidget::loadSettings()
     m_generalUi->autoSaveAfterEveryChangeCheckBox->setChecked(config()->get("AutoSaveAfterEveryChange").toBool());
     m_generalUi->autoSaveOnExitCheckBox->setChecked(config()->get("AutoSaveOnExit").toBool());
     m_generalUi->backupBeforeSaveCheckBox->setChecked(config()->get("BackupBeforeSave").toBool());
+    m_generalUi->useAtomicSavesCheckBox->setChecked(config()->get("UseAtomicSaves").toBool());
     m_generalUi->autoReloadOnChangeCheckBox->setChecked(config()->get("AutoReloadOnChange").toBool());
     m_generalUi->minimizeOnCopyCheckBox->setChecked(config()->get("MinimizeOnCopy").toBool());
     m_generalUi->useGroupIconOnEntryCreationCheckBox->setChecked(config()->get("UseGroupIconOnEntryCreation").toBool());
@@ -195,6 +196,7 @@ void SettingsWidget::saveSettings()
                   m_generalUi->autoSaveAfterEveryChangeCheckBox->isChecked());
     config()->set("AutoSaveOnExit", m_generalUi->autoSaveOnExitCheckBox->isChecked());
     config()->set("BackupBeforeSave", m_generalUi->backupBeforeSaveCheckBox->isChecked());
+    config()->set("UseAtomicSaves", m_generalUi->useAtomicSavesCheckBox->isChecked());
     config()->set("AutoReloadOnChange", m_generalUi->autoReloadOnChangeCheckBox->isChecked());
     config()->set("MinimizeOnCopy", m_generalUi->minimizeOnCopyCheckBox->isChecked());
     config()->set("UseGroupIconOnEntryCreation",