Add direct write save option

* Closes #6335
* Modify application settings presentation to  allow for alternative saving strategies
* Transition Database::save calls to using flags to control saving behavior. Reduces boolean flags on function call.
* Made direct write save option a local setting to prevent unintentional carry over between platforms.

share/translations/keepassxc_en.ts

@@ -380,10 +380,6 @@
         <source>Automatically launch KeePassXC at system startup</source>
         <translation type="unfinished"></translation>
     </message>
-    <message>
-        <source>Safely save database files (disable if experiencing problems with Dropbox, etc.)</source>
-        <translation type="unfinished"></translation>
-    </message>
     <message>
         <source>User Interface</source>
         <translation type="unfinished"></translation>
@@ -432,6 +428,18 @@
         <source>Hide expired entries from Auto-Type</source>
         <translation type="unfinished"></translation>
     </message>
+    <message>
+        <source>Use alternative saving method (may solve problems with Dropbox, Google Drive, GVFS, etc.)</source>
+        <translation type="unfinished"></translation>
+    </message>
+    <message>
+        <source>Temporary file moved into place</source>
+        <translation type="unfinished"></translation>
+    </message>
+    <message>
+        <source>Directly write to database file (dangerous)</source>
+        <translation type="unfinished"></translation>
+    </message>
 </context>
 <context>
     <name>ApplicationSettingsWidgetSecurity</name>

src/cli/Add.cpp

@@ -121,7 +121,7 @@ int Add::executeWithDatabase(QSharedPointer<Database> database, QSharedPointer<Q
     }
 
     QString errorMessage;
-    if (!database->save(&errorMessage, true, false)) {
+    if (!database->save(Database::Atomic, false, &errorMessage)) {
         err << QObject::tr("Writing the database failed %1.").arg(errorMessage) << endl;
         return EXIT_FAILURE;
     }

src/cli/AddGroup.cpp

@@ -63,7 +63,7 @@ int AddGroup::executeWithDatabase(QSharedPointer<Database> database, QSharedPoin
     newGroup->setParent(parentGroup);
 
     QString errorMessage;
-    if (!database->save(&errorMessage, true, false)) {
+    if (!database->save(Database::Atomic, false, &errorMessage)) {
         err << QObject::tr("Writing the database failed %1.").arg(errorMessage) << endl;
         return EXIT_FAILURE;
     }

src/cli/Create.cpp

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

src/cli/Edit.cpp

@@ -126,7 +126,7 @@ int Edit::executeWithDatabase(QSharedPointer<Database> database, QSharedPointer<
     entry->endUpdate();
 
     QString errorMessage;
-    if (!database->save(&errorMessage, true, false)) {
+    if (!database->save(Database::Atomic, false, &errorMessage)) {
         err << QObject::tr("Writing the database failed: %1").arg(errorMessage) << endl;
         return EXIT_FAILURE;
     }

src/cli/Import.cpp

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

src/cli/Merge.cpp

@@ -95,7 +95,7 @@ int Merge::executeWithDatabase(QSharedPointer<Database> database, QSharedPointer
 
     if (!changeList.isEmpty() && !parser->isSet(Merge::DryRunOption)) {
         QString errorMessage;
-        if (!database->save(&errorMessage, true, false)) {
+        if (!database->save(Database::Atomic, false, &errorMessage)) {
             err << QObject::tr("Unable to save database to file : %1").arg(errorMessage) << endl;
             return EXIT_FAILURE;
         }

src/cli/Move.cpp

@@ -65,7 +65,7 @@ int Move::executeWithDatabase(QSharedPointer<Database> database, QSharedPointer<
     entry->endUpdate();
 
     QString errorMessage;
-    if (!database->save(&errorMessage, true, false)) {
+    if (!database->save(Database::Atomic, false, &errorMessage)) {
         err << QObject::tr("Writing the database failed %1.").arg(errorMessage) << endl;
         return EXIT_FAILURE;
     }

src/cli/Remove.cpp

@@ -53,7 +53,7 @@ int Remove::executeWithDatabase(QSharedPointer<Database> database, QSharedPointe
     };
 
     QString errorMessage;
-    if (!database->save(&errorMessage, true, false)) {
+    if (!database->save(Database::Atomic, false, &errorMessage)) {
         err << QObject::tr("Unable to save database to file: %1").arg(errorMessage) << endl;
         return EXIT_FAILURE;
     }

src/cli/RemoveGroup.cpp

@@ -63,7 +63,7 @@ int RemoveGroup::executeWithDatabase(QSharedPointer<Database> database, QSharedP
     };
 
     QString errorMessage;
-    if (!database->save(&errorMessage, true, false)) {
+    if (!database->save(Database::Atomic, false, &errorMessage)) {
         err << QObject::tr("Unable to save database to file: %1").arg(errorMessage) << endl;
         return EXIT_FAILURE;
     }

src/core/Config.cpp

@@ -62,6 +62,7 @@ static const QHash<Config::ConfigKey, ConfigDirective> configStrings = {
     {Config::AutoSaveNonDataChanges,{QS("AutoSaveNonDataChanges"), Roaming, true}},
     {Config::BackupBeforeSave,{QS("BackupBeforeSave"), Roaming, false}},
     {Config::UseAtomicSaves,{QS("UseAtomicSaves"), Roaming, true}},
+    {Config::UseDirectWriteSaves,{QS("UseDirectWriteSaves"), Local, false}},
     {Config::SearchLimitGroup,{QS("SearchLimitGroup"), Roaming, false}},
     {Config::MinimizeOnOpenUrl,{QS("MinimizeOnOpenUrl"), Roaming, false}},
     {Config::HideWindowOnCopy,{QS("HideWindowOnCopy"), Roaming, false}},

src/core/Config.h

@@ -44,6 +44,7 @@ public:
         AutoSaveNonDataChanges,
         BackupBeforeSave,
         UseAtomicSaves,
+        UseDirectWriteSaves,
         SearchLimitGroup,
         MinimizeOnOpenUrl,
         HideWindowOnCopy,

src/core/Database.cpp

@@ -181,7 +181,7 @@ bool Database::isSaving()
  * @param backup Backup the existing database file, if exists
  * @return true on success
  */
-bool Database::save(QString* error, bool atomic, bool backup)
+bool Database::save(SaveAction action, bool backup, QString* error)
 {
     Q_ASSERT(!m_data.filePath.isEmpty());
     if (m_data.filePath.isEmpty()) {
@@ -191,7 +191,7 @@ bool Database::save(QString* error, bool atomic, bool backup)
         return false;
     }
 
-    return saveAs(m_data.filePath, error, atomic, backup);
+    return saveAs(m_data.filePath, action, backup, error);
 }
 
 /**
@@ -212,7 +212,7 @@ bool Database::save(QString* error, bool atomic, bool backup)
  * @param backup Backup the existing database file, if exists
  * @return true on success
  */
-bool Database::saveAs(const QString& filePath, QString* error, bool atomic, bool backup)
+bool Database::saveAs(const QString& filePath, SaveAction action, bool backup, QString* error)
 {
     // Disallow overlapping save operations
     if (isSaving()) {
@@ -260,7 +260,7 @@ bool Database::saveAs(const QString& filePath, QString* error, bool atomic, bool
     QFileInfo fileInfo(filePath);
     auto realFilePath = fileInfo.exists() ? fileInfo.canonicalFilePath() : fileInfo.absoluteFilePath();
     bool isNewFile = !QFile::exists(realFilePath);
-    bool ok = AsyncTask::runAndWaitForFuture([&] { return performSave(realFilePath, error, atomic, backup); });
+    bool ok = AsyncTask::runAndWaitForFuture([&] { return performSave(realFilePath, action, backup, error); });
     if (ok) {
         markAsClean();
         setFilePath(filePath);
@@ -276,14 +276,19 @@ bool Database::saveAs(const QString& filePath, QString* error, bool atomic, bool
     return ok;
 }
 
-bool Database::performSave(const QString& filePath, QString* error, bool atomic, bool backup)
+bool Database::performSave(const QString& filePath, SaveAction action, bool backup, QString* error)
 {
+    if (backup) {
+        backupDatabase(filePath);
+    }
+
 #if QT_VERSION >= QT_VERSION_CHECK(5, 10, 0)
     QFileInfo info(filePath);
     auto createTime = info.exists() ? info.birthTime() : QDateTime::currentDateTime();
 #endif
 
-    if (atomic) {
+    switch (action) {
+    case Atomic: {
         QSaveFile saveFile(filePath);
         if (saveFile.open(QIODevice::WriteOnly)) {
             // write the database to the file
@@ -291,12 +296,8 @@ bool Database::performSave(const QString& filePath, QString* error, bool atomic,
                 return false;
             }
 
-            if (backup) {
-                backupDatabase(filePath);
-            }
-
 #if QT_VERSION >= QT_VERSION_CHECK(5, 10, 0)
-            // Retain orginal creation time
+            // Retain original creation time
             saveFile.setFileTime(createTime, QFile::FileBirthTime);
 #endif
 
@@ -309,20 +310,17 @@ bool Database::performSave(const QString& filePath, QString* error, bool atomic,
         if (error) {
             *error = saveFile.errorString();
         }
-    } else {
+        break;
+    }
+    case TempFile: {
         QTemporaryFile tempFile;
         if (tempFile.open()) {
             // write the database to the file
             if (!writeDatabase(&tempFile, error)) {
                 return false;
             }
-
             tempFile.close(); // flush to disk
 
-            if (backup) {
-                backupDatabase(filePath);
-            }
-
             // Delete the original db and move the temp file in place
             auto perms = QFile::permissions(filePath);
             QFile::remove(filePath);
@@ -353,6 +351,23 @@ bool Database::performSave(const QString& filePath, QString* error, bool atomic,
         if (error) {
             *error = tempFile.errorString();
         }
+        break;
+    }
+    case DirectWrite: {
+        // Open the original database file for direct-write
+        QFile dbFile(filePath);
+        if (dbFile.open(QIODevice::WriteOnly | QIODevice::Truncate)) {
+            if (!writeDatabase(&dbFile, error)) {
+                return false;
+            }
+            dbFile.close();
+            return true;
+        }
+        if (error) {
+            *error = dbFile.errorString();
+        }
+        break;
+    }
     }
 
     // Saving failed

src/core/Database.h

@@ -63,6 +63,13 @@ public:
     };
     static const quint32 CompressionAlgorithmMax = CompressionGZip;
 
+    enum SaveAction
+    {
+        Atomic, // Saves are transactional and atomic
+        TempFile, // Write to a temporary location then move into place, may be non-atomic
+        DirectWrite, // Directly write to the destination file (dangerous)
+    };
+
     Database();
     explicit Database(const QString& filePath);
     ~Database() override;
@@ -72,8 +79,8 @@ public:
               QSharedPointer<const CompositeKey> key,
               QString* error = nullptr,
               bool readOnly = false);
-    bool save(QString* error = nullptr, bool atomic = true, bool backup = false);
+    bool save(SaveAction action = Atomic, bool backup = false, QString* error = nullptr);
-    bool saveAs(const QString& filePath, QString* error = nullptr, bool atomic = true, bool backup = false);
+    bool saveAs(const QString& filePath, SaveAction action = Atomic, bool backup = false, QString* error = nullptr);
     bool extract(QByteArray&, QString* error = nullptr);
     bool import(const QString& xmlExportPath, QString* error = nullptr);
 
@@ -198,7 +205,7 @@ private:
     bool writeDatabase(QIODevice* device, QString* error = nullptr);
     bool backupDatabase(const QString& filePath);
     bool restoreDatabase(const QString& filePath);
-    bool performSave(const QString& filePath, QString* error, bool atomic, bool backup);
+    bool performSave(const QString& filePath, SaveAction flags, bool backup, QString* error);
     void startModifiedTimer();
     void stopModifiedTimer();
 

src/gui/ApplicationSettingsWidget.cpp

@@ -109,6 +109,8 @@ ApplicationSettingsWidget::ApplicationSettingsWidget(QWidget* parent)
     connect(m_generalUi->systrayShowCheckBox, SIGNAL(toggled(bool)), SLOT(systrayToggled(bool)));
     connect(m_generalUi->rememberLastDatabasesCheckBox, SIGNAL(toggled(bool)), SLOT(rememberDatabasesToggled(bool)));
     connect(m_generalUi->resetSettingsButton, SIGNAL(clicked()), SLOT(resetSettings()));
+    connect(m_generalUi->useAlternativeSaveCheckBox, SIGNAL(toggled(bool)),
+            m_generalUi->alternativeSaveComboBox, SLOT(setEnabled(bool)));
 
     connect(m_secUi->clearClipboardCheckBox, SIGNAL(toggled(bool)),
             m_secUi->clearClipboardSpinBox, SLOT(setEnabled(bool)));
@@ -186,7 +188,8 @@ void ApplicationSettingsWidget::loadSettings()
     m_generalUi->autoSaveOnExitCheckBox->setChecked(config()->get(Config::AutoSaveOnExit).toBool());
     m_generalUi->autoSaveNonDataChangesCheckBox->setChecked(config()->get(Config::AutoSaveNonDataChanges).toBool());
     m_generalUi->backupBeforeSaveCheckBox->setChecked(config()->get(Config::BackupBeforeSave).toBool());
-    m_generalUi->useAtomicSavesCheckBox->setChecked(config()->get(Config::UseAtomicSaves).toBool());
+    m_generalUi->useAlternativeSaveCheckBox->setChecked(!config()->get(Config::UseAtomicSaves).toBool());
+    m_generalUi->alternativeSaveComboBox->setCurrentIndex(config()->get(Config::UseDirectWriteSaves).toBool() ? 1 : 0);
     m_generalUi->autoReloadOnChangeCheckBox->setChecked(config()->get(Config::AutoReloadOnChange).toBool());
     m_generalUi->minimizeAfterUnlockCheckBox->setChecked(config()->get(Config::MinimizeAfterUnlock).toBool());
     m_generalUi->minimizeOnOpenUrlCheckBox->setChecked(config()->get(Config::MinimizeOnOpenUrl).toBool());
@@ -323,7 +326,8 @@ void ApplicationSettingsWidget::saveSettings()
     config()->set(Config::AutoSaveOnExit, m_generalUi->autoSaveOnExitCheckBox->isChecked());
     config()->set(Config::AutoSaveNonDataChanges, m_generalUi->autoSaveNonDataChangesCheckBox->isChecked());
     config()->set(Config::BackupBeforeSave, m_generalUi->backupBeforeSaveCheckBox->isChecked());
-    config()->set(Config::UseAtomicSaves, m_generalUi->useAtomicSavesCheckBox->isChecked());
+    config()->set(Config::UseAtomicSaves, !m_generalUi->useAlternativeSaveCheckBox->isChecked());
+    config()->set(Config::UseDirectWriteSaves, m_generalUi->alternativeSaveComboBox->currentIndex() == 1);
     config()->set(Config::AutoReloadOnChange, m_generalUi->autoReloadOnChangeCheckBox->isChecked());
     config()->set(Config::MinimizeAfterUnlock, m_generalUi->minimizeAfterUnlockCheckBox->isChecked());
     config()->set(Config::MinimizeOnOpenUrl, m_generalUi->minimizeOnOpenUrlCheckBox->isChecked());

src/gui/ApplicationSettingsWidgetGeneral.ui

@@ -7,7 +7,7 @@
     <x>0</x>
     <y>0</y>
     <width>605</width>
-    <height>1279</height>
+    <height>968</height>
    </rect>
   </property>
   <layout class="QVBoxLayout" name="verticalLayout_3" stretch="0">
@@ -59,7 +59,7 @@
             <x>0</x>
             <y>0</y>
             <width>581</width>
-            <height>1235</height>
+            <height>924</height>
            </rect>
           </property>
           <layout class="QVBoxLayout" name="verticalLayout_8">
@@ -264,13 +264,6 @@
                 </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">
@@ -279,15 +272,72 @@
                </widget>
               </item>
               <item>
-               <widget class="QCheckBox" name="useAtomicSavesCheckBox">
+               <widget class="QCheckBox" name="backupBeforeSaveCheckBox">
                 <property name="text">
-                 <string>Safely save database files (disable if experiencing problems with Dropbox, etc.)</string>
+                 <string>Backup database file before saving</string>
-                </property>
-                <property name="checked">
-                 <bool>true</bool>
                 </property>
                </widget>
               </item>
+              <item>
+               <widget class="QCheckBox" name="useAlternativeSaveCheckBox">
+                <property name="text">
+                 <string>Use alternative saving method (may solve problems with Dropbox, Google Drive, GVFS, etc.)</string>
+                </property>
+               </widget>
+              </item>
+              <item>
+               <layout class="QHBoxLayout" name="horizontalLayout_3">
+                <property name="spacing">
+                 <number>0</number>
+                </property>
+                <item>
+                 <spacer name="horizontalSpacer_8">
+                  <property name="orientation">
+                   <enum>Qt::Horizontal</enum>
+                  </property>
+                  <property name="sizeType">
+                   <enum>QSizePolicy::Fixed</enum>
+                  </property>
+                  <property name="sizeHint" stdset="0">
+                   <size>
+                    <width>30</width>
+                    <height>20</height>
+                   </size>
+                  </property>
+                 </spacer>
+                </item>
+                <item>
+                 <widget class="QComboBox" name="alternativeSaveComboBox">
+                  <property name="enabled">
+                   <bool>false</bool>
+                  </property>
+                  <item>
+                   <property name="text">
+                    <string>Temporary file moved into place</string>
+                   </property>
+                  </item>
+                  <item>
+                   <property name="text">
+                    <string>Directly write to database file (dangerous)</string>
+                   </property>
+                  </item>
+                 </widget>
+                </item>
+                <item>
+                 <spacer name="horizontalSpacer_7">
+                  <property name="orientation">
+                   <enum>Qt::Horizontal</enum>
+                  </property>
+                  <property name="sizeHint" stdset="0">
+                   <size>
+                    <width>40</width>
+                    <height>20</height>
+                   </size>
+                  </property>
+                 </spacer>
+                </item>
+               </layout>
+              </item>
              </layout>
             </widget>
            </item>
@@ -1046,8 +1096,7 @@
   <tabstop>autoSaveOnExitCheckBox</tabstop>
   <tabstop>autoSaveNonDataChangesCheckBox</tabstop>
   <tabstop>backupBeforeSaveCheckBox</tabstop>
-  <tabstop>autoReloadOnChangeCheckBox</tabstop>
+  <tabstop>useAlternativeSaveCheckBox</tabstop>
-  <tabstop>useAtomicSavesCheckBox</tabstop>
   <tabstop>useGroupIconOnEntryCreationCheckBox</tabstop>
   <tabstop>minimizeOnOpenUrlCheckBox</tabstop>
   <tabstop>hideWindowOnCopyCheckBox</tabstop>

src/gui/DatabaseWidget.cpp

@@ -1867,16 +1867,20 @@ bool DatabaseWidget::performSave(QString& errorMessage, const QString& fileName)
     m_groupView->setDisabled(true);
     QApplication::processEvents();
 
+    Database::SaveAction saveAction = Database::Atomic;
+    if (!config()->get(Config::UseAtomicSaves).toBool()) {
+        if (config()->get(Config::UseDirectWriteSaves).toBool()) {
+            saveAction = Database::DirectWrite;
+        } else {
+            saveAction = Database::TempFile;
+        }
+    }
+
     bool ok;
     if (fileName.isEmpty()) {
-        ok = m_db->save(&errorMessage,
+        ok = m_db->save(saveAction, config()->get(Config::BackupBeforeSave).toBool(), &errorMessage);
-                        config()->get(Config::UseAtomicSaves).toBool(),
-                        config()->get(Config::BackupBeforeSave).toBool());
     } else {
-        ok = m_db->saveAs(fileName,
+        ok = m_db->saveAs(fileName, saveAction, config()->get(Config::BackupBeforeSave).toBool(), &errorMessage);
-                          &errorMessage,
-                          config()->get(Config::UseAtomicSaves).toBool(),
-                          config()->get(Config::BackupBeforeSave).toBool());
     }
 
     // Return control

tests/TestDatabase.cpp

@@ -75,17 +75,22 @@ void TestDatabase::testSave()
     // Test safe saves
     db->metadata()->setName("test");
     QVERIFY(db->isModified());
-    QVERIFY2(db->save(&error), error.toLatin1());
+    QVERIFY2(db->save(Database::Atomic, false, &error), error.toLatin1());
     QVERIFY(!db->isModified());
 
-    // Test unsafe saves
+    // Test temp-file saves
     db->metadata()->setName("test2");
-    QVERIFY2(db->save(&error, false, false), error.toLatin1());
+    QVERIFY2(db->save(Database::TempFile, false, &error), error.toLatin1());
+    QVERIFY(!db->isModified());
+
+    // Test direct-write saves
+    db->metadata()->setName("test3");
+    QVERIFY2(db->save(Database::DirectWrite, false, &error), error.toLatin1());
     QVERIFY(!db->isModified());
 
     // Test save backups
-    db->metadata()->setName("test3");
+    db->metadata()->setName("test4");
-    QVERIFY2(db->save(&error, true, true), error.toLatin1());
+    QVERIFY2(db->save(Database::Atomic, true, &error), error.toLatin1());
     QVERIFY(!db->isModified());
 
     // Confirm backup exists and then delete it
@@ -118,7 +123,7 @@ void TestDatabase::testSignals()
     QTRY_COMPARE(spyModified.count(), 1);
 
     QSignalSpy spySaved(db.data(), SIGNAL(databaseSaved()));
-    QVERIFY(db->save(&error));
+    QVERIFY(db->save(Database::Atomic, false, &error));
     QCOMPARE(spySaved.count(), 1);
 
     // Short delay to allow file system settling to reduce test failures