Correct saving files to DropBox/Drive/OneDrive

* Replaces QSaveFile with QTemporaryFile
* Added backup before save config setting
* This method may cause data loss (see comments)

src/core/Config.cpp

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

src/core/Database.cpp

@@ -19,7 +19,7 @@
 #include "Database.h"
 
 #include <QFile>
-#include <QSaveFile>
+#include <QTemporaryFile>
 #include <QTextStream>
 #include <QTimer>
 #include <QXmlStreamReader>
@@ -470,30 +470,52 @@ Database* Database::unlockFromStdin(QString databaseFilename, QString keyFilenam
     return Database::openDatabaseFile(databaseFilename, compositeKey);
 }
 
-QString Database::saveToFile(QString filePath)
+/**
+ * Save the database to a 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.
+ *
+ * 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
+ * wrong moment.
+ *
+ * @param filePath Absolute path of the file to save
+ * @param keepOld Rename the original database file instead of deleting
+ * @return error string, if any
+ */
+QString Database::saveToFile(QString filePath, bool keepOld)
 {
     KeePass2Writer writer;
-    QSaveFile saveFile(filePath);
-    if (saveFile.open(QIODevice::WriteOnly)) {
-
+    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();
         }
 
-        if (saveFile.commit()) {
-            // successfully saved database file
-            return QString();
-        } else {
-            return saveFile.errorString();
+        saveFile.close(); // flush to disk
+
+        if (keepOld) {
+            QFile::remove(filePath + ".old");
+            QFile::rename(filePath, filePath + ".old");
+        }
+
+        QFile::remove(filePath);
+        if (saveFile.rename(filePath)) {
+            // successfully saved database file
+            saveFile.setAutoRemove(false);
+            return {};
         }
-    } else {
-        return saveFile.errorString();
     }
+
+    return saveFile.errorString();
 }
 
 QSharedPointer<Kdf> Database::kdf() const

src/core/Database.h

@@ -111,7 +111,7 @@ public:
     void emptyRecycleBin();
     void setEmitModified(bool value);
     void merge(const Database* other);
-    QString saveToFile(QString filePath);
+    QString saveToFile(QString filePath, bool keepOld = false);
 
     /**
      * Returns a unique id that is only valid as long as the Database exists.

src/gui/DatabaseTabWidget.cpp

@@ -28,6 +28,7 @@
 #include "core/Database.h"
 #include "core/Group.h"
 #include "core/Metadata.h"
+#include "core/AsyncTask.h"
 #include "format/CsvExporter.h"
 #include "gui/Clipboard.h"
 #include "gui/DatabaseWidget.h"
@@ -322,7 +323,8 @@ bool DatabaseTabWidget::saveDatabase(Database* db, QString filePath)
         }
 
         dbStruct.dbWidget->blockAutoReload(true);
-        QString errorMessage = db->saveToFile(filePath);
+        // TODO: Make this async, but lock out the database widget to prevent re-entrance
+        QString errorMessage = db->saveToFile(filePath, config()->get("BackupBeforeSave").toBool());
         dbStruct.dbWidget->blockAutoReload(false);
 
         if (errorMessage.isEmpty()) {

src/gui/SettingsWidget.cpp

@@ -117,6 +117,7 @@ void SettingsWidget::loadSettings()
         config()->get("OpenPreviousDatabasesOnStartup").toBool());
     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->autoReloadOnChangeCheckBox->setChecked(config()->get("AutoReloadOnChange").toBool());
     m_generalUi->minimizeOnCopyCheckBox->setChecked(config()->get("MinimizeOnCopy").toBool());
     m_generalUi->useGroupIconOnEntryCreationCheckBox->setChecked(config()->get("UseGroupIconOnEntryCreation").toBool());
@@ -193,6 +194,7 @@ void SettingsWidget::saveSettings()
     config()->set("AutoSaveAfterEveryChange",
                   m_generalUi->autoSaveAfterEveryChangeCheckBox->isChecked());
     config()->set("AutoSaveOnExit", m_generalUi->autoSaveOnExitCheckBox->isChecked());
+    config()->set("BackupBeforeSave", m_generalUi->backupBeforeSaveCheckBox->isChecked());
     config()->set("AutoReloadOnChange", m_generalUi->autoReloadOnChangeCheckBox->isChecked());
     config()->set("MinimizeOnCopy", m_generalUi->minimizeOnCopyCheckBox->isChecked());
     config()->set("UseGroupIconOnEntryCreation",

src/gui/SettingsWidgetGeneral.ui

@@ -84,6 +84,13 @@
          </property>
         </widget>
        </item>
+       <item>
+        <widget class="QCheckBox" name="backupBeforeSaveCheckBox">
+         <property name="text">
+          <string>Backup database file before saving</string>
+         </property>
+        </widget>
+       </item>
        <item>
         <widget class="QCheckBox" name="autoReloadOnChangeCheckBox">
          <property name="text">