Allow specifing database backup paths. (#7035)

- Default backupFilePath is '{DB_FILENAME}.old.kdbx' to conform to existing standards
- Implement backupPathPattern tests.
- Show tooltip on how to format database backup location text field.

tests/TestDatabase.cpp

@@ -75,29 +75,26 @@ void TestDatabase::testSave()
     // Test safe saves
     db->metadata()->setName("test");
     QVERIFY(db->isModified());
-    QVERIFY2(db->save(Database::Atomic, false, &error), error.toLatin1());
+    QVERIFY2(db->save(Database::Atomic, QString(), &error), error.toLatin1());
     QVERIFY(!db->isModified());
 
     // Test temp-file saves
     db->metadata()->setName("test2");
-    QVERIFY2(db->save(Database::TempFile, false, &error), error.toLatin1());
+    QVERIFY2(db->save(Database::TempFile, QString(), &error), error.toLatin1());
     QVERIFY(!db->isModified());
 
     // Test direct-write saves
     db->metadata()->setName("test3");
-    QVERIFY2(db->save(Database::DirectWrite, false, &error), error.toLatin1());
+    QVERIFY2(db->save(Database::DirectWrite, QString(), &error), error.toLatin1());
     QVERIFY(!db->isModified());
 
     // Test save backups
+    TemporaryFile backupFile;
+    auto backupFilePath = backupFile.fileName();
     db->metadata()->setName("test4");
-    QVERIFY2(db->save(Database::Atomic, true, &error), error.toLatin1());
+    QVERIFY2(db->save(Database::Atomic, backupFilePath, &error), error.toLatin1());
     QVERIFY(!db->isModified());
 
-    // Confirm backup exists and then delete it
-    auto re = QRegularExpression("(\\.[^.]+)$");
-    auto match = re.match(tempFile.fileName());
-    auto backupFilePath = tempFile.fileName();
-    backupFilePath = backupFilePath.replace(re, "") + ".old" + match.captured(1);
     QVERIFY(QFile::exists(backupFilePath));
     QFile::remove(backupFilePath);
     QVERIFY(!QFile::exists(backupFilePath));
@@ -123,7 +120,7 @@ void TestDatabase::testSignals()
     QTRY_COMPARE(spyModified.count(), 1);
 
     QSignalSpy spySaved(db.data(), SIGNAL(databaseSaved()));
-    QVERIFY(db->save(Database::Atomic, false, &error));
+    QVERIFY(db->save(Database::Atomic, QString(), &error));
     QCOMPARE(spySaved.count(), 1);
 
     // Short delay to allow file system settling to reduce test failures

tests/TestTools.cpp

@@ -17,6 +17,8 @@
 
 #include "TestTools.h"
 
+#include "core/Clock.h"
+
 #include <QTest>
 
 QTEST_GUILESS_MAIN(TestTools)
@@ -108,3 +110,55 @@ void TestTools::testValidUuid()
     QVERIFY(!Tools::isValidUuid(longUuid));
     QVERIFY(!Tools::isValidUuid(nonHexUuid));
 }
+
+void TestTools::testBackupFilePatternSubstitution_data()
+{
+    QTest::addColumn<QString>("pattern");
+    QTest::addColumn<QString>("dbFilePath");
+    QTest::addColumn<QString>("expectedSubstitution");
+
+    static const auto DEFAULT_DB_FILE_NAME = QStringLiteral("KeePassXC");
+    static const auto DEFAULT_DB_FILE_PATH = QStringLiteral("/tmp/") + DEFAULT_DB_FILE_NAME + QStringLiteral(".kdbx");
+    static const auto NOW = Clock::currentDateTime();
+    auto DEFAULT_FORMATTED_TIME = NOW.toString("dd_MM_yyyy_hh-mm-ss");
+
+    QTest::newRow("Null pattern") << QString() << DEFAULT_DB_FILE_PATH << QString();
+    QTest::newRow("Empty pattern") << QString("") << DEFAULT_DB_FILE_PATH << QString("");
+    QTest::newRow("Null database path") << "valid_pattern" << QString() << QString();
+    QTest::newRow("Empty database path") << "valid_pattern" << QString("") << QString();
+    QTest::newRow("Unclosed/invalid pattern") << "{DB_FILENAME" << DEFAULT_DB_FILE_PATH << "{DB_FILENAME";
+    QTest::newRow("Unknown pattern") << "{NO_MATCH}" << DEFAULT_DB_FILE_PATH << "{NO_MATCH}";
+    QTest::newRow("Do not replace escaped patterns (filename)")
+        << "\\{DB_FILENAME\\}" << DEFAULT_DB_FILE_PATH << "{DB_FILENAME}";
+    QTest::newRow("Do not replace escaped patterns (time)")
+        << "\\{TIME:dd.MM.yyyy\\}" << DEFAULT_DB_FILE_PATH << "{TIME:dd.MM.yyyy}";
+    QTest::newRow("Multiple patterns should be replaced")
+        << "{DB_FILENAME} {TIME} {DB_FILENAME}" << DEFAULT_DB_FILE_PATH
+        << DEFAULT_DB_FILE_NAME + QStringLiteral(" ") + DEFAULT_FORMATTED_TIME + QStringLiteral(" ")
+               + DEFAULT_DB_FILE_NAME;
+    QTest::newRow("Default time pattern") << "{TIME}" << DEFAULT_DB_FILE_PATH << DEFAULT_FORMATTED_TIME;
+    QTest::newRow("Default time pattern (empty formatter)")
+        << "{TIME:}" << DEFAULT_DB_FILE_PATH << DEFAULT_FORMATTED_TIME;
+    QTest::newRow("Custom time pattern") << "{TIME:dd-ss}" << DEFAULT_DB_FILE_PATH << NOW.toString("dd-ss");
+    QTest::newRow("Invalid custom time pattern") << "{TIME:dd/-ss}" << DEFAULT_DB_FILE_PATH << NOW.toString("dd/-ss");
+    QTest::newRow("Recursive substitution") << "{TIME:'{TIME}'}" << DEFAULT_DB_FILE_PATH << DEFAULT_FORMATTED_TIME;
+    QTest::newRow("{DB_FILENAME} substitution")
+        << "some {DB_FILENAME} thing" << DEFAULT_DB_FILE_PATH
+        << QStringLiteral("some ") + DEFAULT_DB_FILE_NAME + QStringLiteral(" thing");
+    QTest::newRow("{DB_FILENAME} substitution with multiple extensions") << "some {DB_FILENAME} thing"
+                                                                         << "/tmp/KeePassXC.kdbx.ext"
+                                                                         << "some KeePassXC.kdbx thing";
+    // Not relevant right now, added test anyway
+    QTest::newRow("There should be no substitution loops") << "{DB_FILENAME}"
+                                                           << "{TIME:'{DB_FILENAME}'}.ext"
+                                                           << "{DB_FILENAME}";
+}
+
+void TestTools::testBackupFilePatternSubstitution()
+{
+    QFETCH(QString, pattern);
+    QFETCH(QString, dbFilePath);
+    QFETCH(QString, expectedSubstitution);
+
+    QCOMPARE(Tools::substituteBackupFilePath(pattern, dbFilePath), expectedSubstitution);
+}

tests/TestTools.h

@@ -29,6 +29,8 @@ private slots:
     void testIsBase64();
     void testEnvSubstitute();
     void testValidUuid();
+    void testBackupFilePatternSubstitution_data();
+    void testBackupFilePatternSubstitution();
 };
 
 #endif // KEEPASSX_TESTTOOLS_H

tests/gui/TestGui.cpp

@@ -82,22 +82,10 @@ int main(int argc, char* argv[])
     return QTest::qExec(&tc, argc, argv);
 }
 
-static QString dbFileName = QStringLiteral(KEEPASSX_TEST_DATA_DIR).append("/NewDatabase.kdbx");
-
 void TestGui::initTestCase()
 {
     QVERIFY(Crypto::init());
     Config::createTempFileInstance();
-    // Disable autosave so we can test the modified file indicator
-    config()->set(Config::AutoSaveAfterEveryChange, false);
-    config()->set(Config::AutoSaveOnExit, false);
-    // Enable the tray icon so we can test hiding/restoring the windowQByteArray
-    config()->set(Config::GUI_ShowTrayIcon, true);
-    // Disable advanced settings mode (activate within individual tests to test advanced settings)
-    config()->set(Config::GUI_AdvancedSettings, false);
-    // Disable the update check first time alert
-    config()->set(Config::UpdateCheckMessageShown, true);
-
     Application::bootstrap();
 
     m_mainWindow.reset(new MainWindow());
@@ -106,11 +94,22 @@ void TestGui::initTestCase()
     m_mainWindow->resize(1024, 768);
 }
 
-// Every test starts with opening the temp database
+// Every test starts with resetting config settings and opening the temp database
 void TestGui::init()
 {
+    // Reset config to defaults
+    config()->resetToDefaults();
+    // Disable autosave so we can test the modified file indicator
+    config()->set(Config::AutoSaveAfterEveryChange, false);
+    config()->set(Config::AutoSaveOnExit, false);
+    // Enable the tray icon so we can test hiding/restoring the windowQByteArray
+    config()->set(Config::GUI_ShowTrayIcon, true);
+    // Disable the update check first time alert
+    config()->set(Config::UpdateCheckMessageShown, true);
+
     // Copy the test database file to the temporary file
-    QVERIFY(m_dbFile.copyFromFile(dbFileName));
+    auto origFilePath = QDir(KEEPASSX_TEST_DATA_DIR).absoluteFilePath("NewDatabase.kdbx");
+    QVERIFY(m_dbFile.copyFromFile(origFilePath));
 
     m_dbFileName = QFileInfo(m_dbFile.fileName()).fileName();
     m_dbFilePath = m_dbFile.fileName();
@@ -354,6 +353,8 @@ void TestGui::testAutoreloadDatabase()
     cleanup();
     init();
 
+    config()->set(Config::AutoReloadOnChange, false);
+
     // Test rejecting new file in autoreload
     MessageBox::setNextAnswer(MessageBox::No);
     // Overwrite the current database with the temp data
@@ -1277,11 +1278,76 @@ void TestGui::testSave()
     QTRY_COMPARE(m_tabWidget->tabText(m_tabWidget->currentIndex()), QString("testSave*"));
 
     triggerAction("actionDatabaseSave");
-    QCOMPARE(m_tabWidget->tabName(m_tabWidget->currentIndex()), QString("testSave"));
+    QTRY_COMPARE(m_tabWidget->tabName(m_tabWidget->currentIndex()), QString("testSave"));
 
     checkDatabase();
 }
 
+void TestGui::testSaveBackupPath_data()
+{
+    QTest::addColumn<QString>("backupFilePathPattern");
+    QTest::addColumn<QString>("expectedBackupFile");
+
+    // Absolute paths should remain absolute
+    TemporaryFile tmpFile;
+    QVERIFY(tmpFile.open());
+    tmpFile.remove();
+
+    QTest::newRow("Absolute backup path") << tmpFile.fileName() << tmpFile.fileName();
+    // relative paths should be resolved to database parent directory
+    QTest::newRow("Relative backup path (implicit)") << "other_dir/test.old.kdbx"
+                                                     << "other_dir/test.old.kdbx";
+    QTest::newRow("Relative backup path (explicit)") << "./other_dir2/test2.old.kdbx"
+                                                     << "other_dir2/test2.old.kdbx";
+
+    QTest::newRow("Path with placeholders") << "{DB_FILENAME}.old.kdbx"
+                                            << "KeePassXC.old.kdbx";
+    // empty path should be replaced with default pattern
+    QTest::newRow("Empty path") << QString("") << config()->getDefault(Config::BackupFilePathPattern).toString();
+    // {DB_FILENAME} should be replaced with database filename
+    QTest::newRow("") << "{DB_FILENAME}_.old.kdbx"
+                      << "{DB_FILENAME}_.old.kdbx";
+}
+
+void TestGui::testSaveBackupPath()
+{
+    /**
+     * Tests that the backupFilePathPattern config entry is respected. We do not test patterns like {TIME} etc here
+     * as this is done in a separate test case. We do however check {DB_FILENAME} as this is a feature of the
+     * performBackup() function.
+     */
+
+    // Get test data
+    QFETCH(QString, backupFilePathPattern);
+    QFETCH(QString, expectedBackupFile);
+
+    // Enable automatic backups
+    config()->set(Config::BackupBeforeSave, true);
+    config()->set(Config::BackupFilePathPattern, backupFilePathPattern);
+
+    // Replace placeholders and resolve relative paths. This cannot be done in the _data() function as the
+    // db path/filename is not known yet
+    auto dbFileInfo = QFileInfo(m_dbFilePath);
+    if (!QDir::isAbsolutePath(expectedBackupFile)) {
+        expectedBackupFile = QDir(dbFileInfo.absolutePath()).absoluteFilePath(expectedBackupFile);
+    }
+    expectedBackupFile.replace("{DB_FILENAME}", dbFileInfo.completeBaseName());
+
+    // Save a modified database
+    auto prevName = m_db->metadata()->name();
+    m_db->metadata()->setName("testBackupPathPattern");
+
+    QTRY_COMPARE(m_tabWidget->tabText(m_tabWidget->currentIndex()), QString("testBackupPathPattern*"));
+    triggerAction("actionDatabaseSave");
+    QTRY_COMPARE(m_tabWidget->tabText(m_tabWidget->currentIndex()), QString("testBackupPathPattern"));
+
+    // Test that the backup file has the previous database name
+    checkDatabase(expectedBackupFile, prevName);
+
+    // Clean up
+    QFile(expectedBackupFile).remove();
+}
+
 void TestGui::testDatabaseSettings()
 {
     m_db->metadata()->setName("testDatabaseSettings");
@@ -1301,7 +1367,7 @@ void TestGui::testDatabaseSettings()
     QCOMPARE(m_db->kdf()->rounds(), 123456);
 
     triggerAction("actionDatabaseSave");
-    QCOMPARE(m_tabWidget->tabText(m_tabWidget->currentIndex()), QString("testDatabaseSettings"));
+    QTRY_COMPARE(m_tabWidget->tabText(m_tabWidget->currentIndex()), QString("testDatabaseSettings"));
 
     advancedToggle->setChecked(false);
     QApplication::processEvents();
@@ -1365,6 +1431,8 @@ void TestGui::testDragAndDropKdbxFiles()
     const int openedDatabasesCount = m_tabWidget->count();
 
     const QString badDatabaseFilePath(QString(KEEPASSX_TEST_DATA_DIR).append("/NotDatabase.notkdbx"));
+    const QString goodDatabaseFilePath(QString(KEEPASSX_TEST_DATA_DIR).append("/NewDatabase.kdbx"));
+
     QMimeData badMimeData;
     badMimeData.setUrls({QUrl::fromLocalFile(badDatabaseFilePath)});
     QDragEnterEvent badDragEvent(QPoint(1, 1), Qt::LinkAction, &badMimeData, Qt::LeftButton, Qt::NoModifier);
@@ -1378,7 +1446,7 @@ void TestGui::testDragAndDropKdbxFiles()
     QCOMPARE(m_tabWidget->count(), openedDatabasesCount);
 
     QMimeData goodMimeData;
-    goodMimeData.setUrls({QUrl::fromLocalFile(dbFileName)});
+    goodMimeData.setUrls({QUrl::fromLocalFile(goodDatabaseFilePath)});
     QDragEnterEvent goodDragEvent(QPoint(1, 1), Qt::LinkAction, &goodMimeData, Qt::LeftButton, Qt::NoModifier);
     qApp->notify(m_mainWindow.data(), &goodDragEvent);
     QCOMPARE(goodDragEvent.isAccepted(), true);
@@ -1700,17 +1768,18 @@ void TestGui::addCannedEntries()
     QTest::mouseClick(editEntryWidgetButtonBox->button(QDialogButtonBox::Ok), Qt::LeftButton);
 }
 
-void TestGui::checkDatabase(QString dbFileName)
+void TestGui::checkDatabase(const QString& filePath, const QString& expectedDbName)
 {
-    if (dbFileName.isEmpty()) {
-        dbFileName = m_dbFilePath;
-    }
-
     auto key = QSharedPointer<CompositeKey>::create();
     key->addKey(QSharedPointer<PasswordKey>::create("a"));
     auto dbSaved = QSharedPointer<Database>::create();
-    QVERIFY(dbSaved->open(dbFileName, key, nullptr, false));
-    QCOMPARE(dbSaved->metadata()->name(), m_db->metadata()->name());
+    QVERIFY(dbSaved->open(filePath, key, nullptr, false));
+    QCOMPARE(dbSaved->metadata()->name(), expectedDbName);
+}
+
+void TestGui::checkDatabase(const QString& filePath)
+{
+    checkDatabase(filePath.isEmpty() ? m_dbFilePath : filePath, m_db->metadata()->name());
 }
 
 void TestGui::triggerAction(const QString& name)

tests/gui/TestGui.h

@@ -57,6 +57,8 @@ private slots:
     void testSaveAs();
     void testSaveBackup();
     void testSave();
+    void testSaveBackupPath();
+    void testSaveBackupPath_data();
     void testDatabaseSettings();
     void testKeePass1Import();
     void testDatabaseLocking();
@@ -67,7 +69,8 @@ private slots:
 
 private:
     void addCannedEntries();
-    void checkDatabase(QString dbFileName = "");
+    void checkDatabase(const QString& filePath, const QString& expectedDbName);
+    void checkDatabase(const QString& filePath = {});
     void triggerAction(const QString& name);
     void dragAndDropGroup(const QModelIndex& sourceIndex,
                           const QModelIndex& targetIndex,