Correct behaviors when saving database fails

* Mark database dirty if saving fails
* Restore database file from backup if unsafe save fails between deleting database file and copying temporary file into place
* Improve error message display for opening and saving database files
* Do not automatically retry saving after failure. This prevents deletion of the backup database file and improves user awareness of issues.

src/core/Database.cpp

@@ -222,6 +222,7 @@ bool Database::save(const QString& filePath, QString* error, bool atomic, bool b
                 return true;
             }
         }
+
         if (error) {
             *error = saveFile.errorString();
         }
@@ -246,18 +247,25 @@ bool Database::save(const QString& filePath, QString* error, bool atomic, bool b
             // due to an undocumented difference in how the function handles
             // errors. This prevents errors when saving across file systems.
             if (tempFile.QFile::rename(filePath)) {
-                // successfully saved database file
+                // successfully saved the database
                 tempFile.setAutoRemove(false);
                 setFilePath(filePath);
                 return true;
+            } else {
+                // restore the database from the backup
+                if (backup) {
+                    restoreDatabase(filePath);
                 }
             }
+        }
+
         if (error) {
             *error = tempFile.errorString();
         }
     }
 
     // Saving failed
+    markAsModified();
     return false;
 }
 
@@ -316,6 +324,24 @@ bool Database::backupDatabase(const QString& filePath)
     return QFile::copy(filePath, backupFilePath);
 }
 
+/**
+ * Restores the database file from the backup file with
+ * name <filename>.old.<extension> to filePath. This will
+ * overwrite the existing file!
+ *
+ * @param filePath Path to the file to restore
+ * @return true on success
+ */
+bool Database::restoreDatabase(const QString& filePath)
+{
+    static auto re = QRegularExpression("^(.*?)(\\.[^.]+)?$");
+
+    auto match = re.match(filePath);
+    auto backupFilePath = match.captured(1) + ".old" + match.captured(2);
+    QFile::remove(filePath);
+    return QFile::copy(backupFilePath, filePath);
+}
+
 bool Database::isReadOnly() const
 {
     return m_data.isReadOnly;

src/core/Database.h

@@ -172,6 +172,7 @@ private:
 
     bool writeDatabase(QIODevice* device, QString* error = nullptr);
     bool backupDatabase(const QString& filePath);
+    bool restoreDatabase(const QString& filePath);
 
     Metadata* const m_metadata;
     DatabaseData m_data;

src/gui/DatabaseTabWidget.cpp

@@ -150,7 +150,8 @@ void DatabaseTabWidget::addDatabaseTab(const QString& filePath,
     QFileInfo fileInfo(filePath);
     QString canonicalFilePath = fileInfo.canonicalFilePath();
     if (canonicalFilePath.isEmpty()) {
-        emit messageGlobal(tr("The database file does not exist or is not accessible."), MessageWidget::Error);
+        emit messageGlobal(tr("Failed to open %1. It either does not exist or is not accessible.").arg(filePath),
+                           MessageWidget::Error);
         return;
     }
 

src/gui/DatabaseWidget.cpp

@@ -89,6 +89,7 @@ DatabaseWidget::DatabaseWidget(QSharedPointer<Database> db, QWidget* parent)
     , m_databaseOpenWidget(new DatabaseOpenWidget(this))
     , m_keepass1OpenWidget(new KeePass1OpenWidget(this))
     , m_groupView(new GroupView(m_db.data(), m_mainSplitter))
+    , m_saveAttempts(0)
     , m_fileWatcher(new DelayingFileWatcher(this))
 {
     m_messageWidget->setHidden(true);
@@ -859,6 +860,7 @@ void DatabaseWidget::loadDatabase(bool accepted)
         replaceDatabase(openWidget->database());
         switchToMainView();
         m_fileWatcher->restart();
+        m_saveAttempts = 0;
         emit databaseUnlocked();
     } else {
         m_fileWatcher->stop();
@@ -1512,7 +1514,7 @@ EntryView* DatabaseWidget::entryView()
  * @param attempt current save attempt or -1 to disable attempts
  * @return true on success
  */
-bool DatabaseWidget::save(int attempt)
+bool DatabaseWidget::save()
 {
     // Never allow saving a locked database; it causes corruption
     Q_ASSERT(!isLocked());
@@ -1527,6 +1529,8 @@ bool DatabaseWidget::save(int attempt)
     }
 
     blockAutoReload(true);
+    ++m_saveAttempts;
+
     // TODO: Make this async, but lock out the database widget to prevent re-entrance
     bool useAtomicSaves = config()->get("UseAtomicSaves", true).toBool();
     QString errorMessage;
@@ -1534,14 +1538,11 @@ bool DatabaseWidget::save(int attempt)
     blockAutoReload(false);
 
     if (ok) {
+        m_saveAttempts = 0;
         return true;
     }
 
-    if (attempt >= 0 && attempt <= 2) {
+    if (m_saveAttempts > 2 && useAtomicSaves) {
-        return save(attempt + 1);
-    }
-
-    if (attempt > 2 && useAtomicSaves) {
         // Saving failed 3 times, issue a warning and attempt to resolve
         auto result = MessageBox::question(this,
                                            tr("Disable safe saves?"),
@@ -1552,11 +1553,15 @@ bool DatabaseWidget::save(int attempt)
                                            MessageBox::Disable);
         if (result == MessageBox::Disable) {
             config()->set("UseAtomicSaves", false);
-            return save(attempt + 1);
+            return save();
         }
     }
 
-    showMessage(tr("Writing the database failed.\n%1").arg(errorMessage), MessageWidget::Error);
+    showMessage(tr("Writing the database failed: %1").arg(errorMessage),
+                MessageWidget::Error,
+                true,
+                MessageWidget::LongAutoHideTimeout);
+
     return false;
 }
 
@@ -1585,8 +1590,9 @@ bool DatabaseWidget::saveAs()
             // Ensure we don't recurse back into this function
             m_db->setReadOnly(false);
             m_db->setFilePath(newFilePath);
+            m_saveAttempts = 0;
 
-            if (!save(-1)) {
+            if (!save()) {
                 // Failed to save, try again
                 continue;
             }

src/gui/DatabaseWidget.h

@@ -144,7 +144,7 @@ signals:
 
 public slots:
     bool lock();
-    bool save(int attempt = 0);
+    bool save();
     bool saveAs();
 
     void replaceDatabase(QSharedPointer<Database> db);
@@ -255,6 +255,8 @@ private:
     QUuid m_groupBeforeLock;
     QUuid m_entryBeforeLock;
 
+    int m_saveAttempts;
+
     // Search state
     EntrySearcher* m_EntrySearcher;
     QString m_lastSearchText;

src/gui/MessageWidget.cpp

@@ -23,6 +23,7 @@
 #include <QUrl>
 
 const int MessageWidget::DefaultAutoHideTimeout = 6000;
+const int MessageWidget::LongAutoHideTimeout = 15000;
 const int MessageWidget::DisableAutoHide = -1;
 
 MessageWidget::MessageWidget(QWidget* parent)

src/gui/MessageWidget.h

@@ -33,6 +33,7 @@ public:
     int autoHideTimeout() const;
 
     static const int DefaultAutoHideTimeout;
+    static const int LongAutoHideTimeout;
     static const int DisableAutoHide;
 
 signals: