Add CLI --dry-run option for merge (#3254)

src/cli/Merge.cpp

@@ -39,6 +39,10 @@ const QCommandLineOption Merge::NoPasswordFromOption =
     QCommandLineOption(QStringList() << "no-password-from",
                        QObject::tr("Deactivate password key for the database to merge from."));
 
+const QCommandLineOption Merge::DryRunOption =
+    QCommandLineOption(QStringList() << "dry-run",
+                       QObject::tr("Only print the changes detected by the merge operation."));
+
 Merge::Merge()
 {
     name = QString("merge");
@@ -46,6 +50,7 @@ Merge::Merge()
     options.append(Merge::SameCredentialsOption);
     options.append(Merge::KeyFileFromOption);
     options.append(Merge::NoPasswordFromOption);
+    options.append(Merge::DryRunOption);
     positionalArguments.append({QString("database2"), QObject::tr("Path of the database to merge from."), QString("")});
 }
 
@@ -55,14 +60,18 @@ Merge::~Merge()
 
 int Merge::executeWithDatabase(QSharedPointer<Database> database, QSharedPointer<QCommandLineParser> parser)
 {
-    TextStream outputTextStream(Utils::STDOUT, QIODevice::WriteOnly);
+    TextStream outputTextStream(parser->isSet(Command::QuietOption) ? Utils::DEVNULL : Utils::STDOUT,
+                                QIODevice::WriteOnly);
     TextStream errorTextStream(Utils::STDERR, QIODevice::WriteOnly);
 
     const QStringList args = parser->positionalArguments();
 
+    QString toDatabasePath = args.at(0);
+    QString fromDatabasePath = args.at(1);
+
     QSharedPointer<Database> db2;
     if (!parser->isSet(Merge::SameCredentialsOption)) {
-        db2 = Utils::unlockDatabase(args.at(1),
+        db2 = Utils::unlockDatabase(fromDatabasePath,
                                     !parser->isSet(Merge::NoPasswordFromOption),
                                     parser->value(Merge::KeyFileFromOption),
                                     parser->isSet(Command::QuietOption) ? Utils::DEVNULL : Utils::STDOUT,
@@ -73,26 +82,29 @@ int Merge::executeWithDatabase(QSharedPointer<Database> database, QSharedPointer
     } else {
         db2 = QSharedPointer<Database>::create();
         QString errorMessage;
-        if (!db2->open(args.at(1), database->key(), &errorMessage, false)) {
+        if (!db2->open(fromDatabasePath, database->key(), &errorMessage, false)) {
             errorTextStream << QObject::tr("Error reading merge file:\n%1").arg(errorMessage);
             return EXIT_FAILURE;
         }
     }
 
     Merger merger(db2.data(), database.data());
-    bool databaseChanged = merger.merge();
+    QStringList changeList = merger.merge();
 
-    if (databaseChanged) {
+    for (QString mergeChange : changeList) {
+        outputTextStream << "\t" << mergeChange << endl;
+    }
+
+    if (!changeList.isEmpty() && !parser->isSet(Merge::DryRunOption)) {
         QString errorMessage;
-        if (!database->save(args.at(0), &errorMessage, true, false)) {
+        if (!database->save(toDatabasePath, &errorMessage, true, false)) {
             errorTextStream << QObject::tr("Unable to save database to file : %1").arg(errorMessage) << endl;
             return EXIT_FAILURE;
         }
-        if (!parser->isSet(Command::QuietOption)) {
+        outputTextStream << QObject::tr("Successfully merged %1 into %2.").arg(fromDatabasePath, toDatabasePath)
-            outputTextStream << "Successfully merged the database files." << endl;
+                         << endl;
-        }
+    } else {
-    } else if (!parser->isSet(Command::QuietOption)) {
+        outputTextStream << QObject::tr("Database was not modified by merge operation.") << endl;
-        outputTextStream << "Database was not modified by merge operation." << endl;
     }
 
     return EXIT_SUCCESS;

src/cli/Merge.h

@@ -31,6 +31,7 @@ public:
     static const QCommandLineOption SameCredentialsOption;
     static const QCommandLineOption KeyFileFromOption;
     static const QCommandLineOption NoPasswordFromOption;
+    static const QCommandLineOption DryRunOption;
 };
 
 #endif // KEEPASSXC_MERGE_H

src/cli/keepassxc-cli.1

@@ -77,6 +77,9 @@ Displays the program version.
 
 .SS "Merge options"
 
+.IP "-d, --dry-run <path>"
+Only print the changes detected by the merge operation.
+
 .IP "-f, --key-file-from <path>"
 Path of the key file for the second database.
 

src/core/Merger.cpp

@@ -60,7 +60,7 @@ void Merger::resetForcedMergeMode()
     m_mode = Group::Default;
 }
 
-bool Merger::merge()
+QStringList Merger::merge()
 {
     // Order of merge steps is important - it is possible that we
     // create some items before deleting them afterwards
@@ -74,9 +74,8 @@ bool Merger::merge()
     // At this point we have a list of changes we may want to show the user
     if (!changes.isEmpty()) {
         m_context.m_targetDb->markAsModified();
-        return true;
     }
-    return false;
+    return changes;
 }
 
 Merger::ChangeList Merger::mergeGroup(const MergeContext& context)

src/core/Merger.h

@@ -33,7 +33,7 @@ public:
     Merger(const Group* sourceGroup, Group* targetGroup);
     void setForcedMergeMode(Group::MergeMode mode);
     void resetForcedMergeMode();
-    bool merge();
+    QStringList merge();
 
 private:
     typedef QString Change;

src/gui/DatabaseWidget.cpp

@@ -894,9 +894,9 @@ void DatabaseWidget::mergeDatabase(bool accepted)
         }
 
         Merger merger(srcDb.data(), m_db.data());
-        bool databaseChanged = merger.merge();
+        QStringList changeList = merger.merge();
 
-        if (databaseChanged) {
+        if (!changeList.isEmpty()) {
             showMessage(tr("Successfully merged the database files."), MessageWidget::Information);
         } else {
             showMessage(tr("Database was not modified by merge operation."), MessageWidget::Information);

src/gui/MainWindow.cpp

@@ -401,7 +401,6 @@ MainWindow::MainWindow()
     connect(m_ui->actionUserGuide, SIGNAL(triggered()), SLOT(openUserGuide()));
     connect(m_ui->actionOnlineHelp, SIGNAL(triggered()), SLOT(openOnlineHelp()));
 
-
 #ifdef Q_OS_MACOS
     setUnifiedTitleAndToolBarOnMac(true);
     if (macUtils()->isDarkMode()) {

src/keeshare/ShareObserver.cpp

@@ -409,8 +409,8 @@ ShareObserver::Result ShareObserver::importSingedContainerInto(const KeeShareSet
                    qPrintable(sourceDb->rootGroup()->name()));
             Merger merger(sourceDb->rootGroup(), targetGroup);
             merger.setForcedMergeMode(Group::Synchronize);
-            const bool changed = merger.merge();
+            const QStringList changeList = merger.merge();
-            if (changed) {
+            if (!changeList.isEmpty()) {
                 return {reference.path, Result::Success, tr("Successful signed import")};
             }
         }
@@ -425,8 +425,8 @@ ShareObserver::Result ShareObserver::importSingedContainerInto(const KeeShareSet
                qPrintable(sourceDb->rootGroup()->name()));
         Merger merger(sourceDb->rootGroup(), targetGroup);
         merger.setForcedMergeMode(Group::Synchronize);
-        const bool changed = merger.merge();
+        const QStringList changeList = merger.merge();
-        if (changed) {
+        if (!changeList.isEmpty()) {
             return {reference.path, Result::Success, tr("Successful signed import")};
         }
         return {};
@@ -496,8 +496,8 @@ ShareObserver::Result ShareObserver::importUnsignedContainerInto(const KeeShareS
                    qPrintable(sourceDb->rootGroup()->name()));
             Merger merger(sourceDb->rootGroup(), targetGroup);
             merger.setForcedMergeMode(Group::Synchronize);
-            const bool changed = merger.merge();
+            const QStringList changeList = merger.merge();
-            if (changed) {
+            if (!changeList.isEmpty()) {
                 return {reference.path, Result::Success, tr("Successful signed import")};
             }
         }
@@ -511,8 +511,8 @@ ShareObserver::Result ShareObserver::importUnsignedContainerInto(const KeeShareS
                qPrintable(sourceDb->rootGroup()->name()));
         Merger merger(sourceDb->rootGroup(), targetGroup);
         merger.setForcedMergeMode(Group::Synchronize);
-        const bool changed = merger.merge();
+        const QStringList changeList = merger.merge();
-        if (changed) {
+        if (!changeList.isEmpty()) {
             return {reference.path, Result::Success, tr("Successful unsigned import")};
         }
         return {};

tests/TestCli.cpp

@@ -965,23 +965,27 @@ void TestCli::testMerge()
     Kdbx4Writer writer;
     Kdbx4Reader reader;
 
-    // load test database and save a copy
+    // load test database and save copies
     auto db = readTestDatabase();
     QVERIFY(db);
     TemporaryFile targetFile1;
     targetFile1.open();
     writer.writeDatabase(&targetFile1, db.data());
     targetFile1.close();
-
-    // save another copy with a different password
     TemporaryFile targetFile2;
     targetFile2.open();
+    writer.writeDatabase(&targetFile2, db.data());
+    targetFile2.close();
+
+    // save another copy with a different password
+    TemporaryFile targetFile3;
+    targetFile3.open();
     auto oldKey = db->key();
     auto key = QSharedPointer<CompositeKey>::create();
     key->addKey(QSharedPointer<PasswordKey>::create("b"));
     db->setKey(key);
-    writer.writeDatabase(&targetFile2, db.data());
+    writer.writeDatabase(&targetFile3, db.data());
-    targetFile2.close();
+    targetFile3.close();
     db->setKey(oldKey);
 
     // then add a new entry to the in-memory database and save another copy
@@ -1003,7 +1007,11 @@ void TestCli::testMerge()
     m_stdoutFile->seek(pos);
     m_stdoutFile->readLine();
     m_stderrFile->reset();
-    QCOMPARE(m_stdoutFile->readAll(), QByteArray("Successfully merged the database files.\n"));
+    QList<QByteArray> outLines1 = m_stdoutFile->readAll().split('\n');
+    QCOMPARE(outLines1.at(0).split('[').at(0), QByteArray("\tOverwriting Internet "));
+    QCOMPARE(outLines1.at(1).split('[').at(0), QByteArray("\tCreating missing Some Website "));
+    QCOMPARE(outLines1.at(2),
+             QString("Successfully merged %1 into %2.").arg(sourceFile.fileName(), targetFile1.fileName()).toUtf8());
 
     QFile readBack(targetFile1.fileName());
     readBack.open(QIODevice::ReadOnly);
@@ -1016,17 +1024,58 @@ void TestCli::testMerge()
     QCOMPARE(entry1->title(), QString("Some Website"));
     QCOMPARE(entry1->password(), QString("secretsecretsecret"));
 
+    // the dry run option should not modify the target database.
+    pos = m_stdoutFile->pos();
+    Utils::Test::setNextPassword("a");
+    mergeCmd.execute({"merge", "--dry-run", "-s", targetFile2.fileName(), sourceFile.fileName()});
+    m_stdoutFile->seek(pos);
+    m_stdoutFile->readLine();
+    m_stderrFile->reset();
+    QList<QByteArray> outLines2 = m_stdoutFile->readAll().split('\n');
+    QCOMPARE(outLines2.at(0).split('[').at(0), QByteArray("\tOverwriting Internet "));
+    QCOMPARE(outLines2.at(1).split('[').at(0), QByteArray("\tCreating missing Some Website "));
+    QCOMPARE(outLines2.at(2), QByteArray("Database was not modified by merge operation."));
+
+    QFile readBack2(targetFile2.fileName());
+    readBack2.open(QIODevice::ReadOnly);
+    mergedDb = QSharedPointer<Database>::create();
+    reader.readDatabase(&readBack2, oldKey, mergedDb.data());
+    readBack2.close();
+    QVERIFY(mergedDb);
+    entry1 = mergedDb->rootGroup()->findEntryByPath("/Internet/Some Website");
+    QVERIFY(!entry1);
+
+    // the dry run option can be used with the quiet option
+    pos = m_stdoutFile->pos();
+    Utils::Test::setNextPassword("a");
+    mergeCmd.execute({"merge", "--dry-run", "-s", "-q", targetFile2.fileName(), sourceFile.fileName()});
+    m_stdoutFile->seek(pos);
+    m_stdoutFile->readLine();
+    m_stderrFile->reset();
+    QCOMPARE(m_stdoutFile->readAll(), QByteArray(""));
+
+    readBack2.setFileName(targetFile2.fileName());
+    readBack2.open(QIODevice::ReadOnly);
+    mergedDb = QSharedPointer<Database>::create();
+    reader.readDatabase(&readBack2, oldKey, mergedDb.data());
+    readBack2.close();
+    QVERIFY(mergedDb);
+    entry1 = mergedDb->rootGroup()->findEntryByPath("/Internet/Some Website");
+    QVERIFY(!entry1);
+
     // try again with different passwords for both files
     pos = m_stdoutFile->pos();
     Utils::Test::setNextPassword("b");
     Utils::Test::setNextPassword("a");
-    mergeCmd.execute({"merge", targetFile2.fileName(), sourceFile.fileName()});
+    mergeCmd.execute({"merge", targetFile3.fileName(), sourceFile.fileName()});
     m_stdoutFile->seek(pos);
     m_stdoutFile->readLine();
     m_stdoutFile->readLine();
-    QCOMPARE(m_stdoutFile->readAll(), QByteArray("Successfully merged the database files.\n"));
+    QList<QByteArray> outLines3 = m_stdoutFile->readAll().split('\n');
+    QCOMPARE(outLines3.at(2),
+             QString("Successfully merged %1 into %2.").arg(sourceFile.fileName(), targetFile3.fileName()).toUtf8());
 
-    readBack.setFileName(targetFile2.fileName());
+    readBack.setFileName(targetFile3.fileName());
     readBack.open(QIODevice::ReadOnly);
     mergedDb = QSharedPointer<Database>::create();
     reader.readDatabase(&readBack, key, mergedDb.data());