From 84eec03cb78a47f4677d189cdb00df882336147c Mon Sep 17 00:00:00 2001 From: louib Date: Tue, 18 Jun 2019 21:45:24 -0400 Subject: [PATCH] Add CLI --dry-run option for merge (#3254) --- src/cli/Merge.cpp | 34 +++++++++++------ src/cli/Merge.h | 1 + src/cli/keepassxc-cli.1 | 3 ++ src/core/Merger.cpp | 5 +-- src/core/Merger.h | 2 +- src/gui/DatabaseWidget.cpp | 4 +- src/gui/MainWindow.cpp | 1 - src/keeshare/ShareObserver.cpp | 16 ++++---- tests/TestCli.cpp | 67 +++++++++++++++++++++++++++++----- 9 files changed, 98 insertions(+), 35 deletions(-) diff --git a/src/cli/Merge.cpp b/src/cli/Merge.cpp index 8e4cf8ad0..d71ca9c95 100644 --- a/src/cli/Merge.cpp +++ b/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, QSharedPointer 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 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, QSharedPointer } else { db2 = QSharedPointer::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 << "Successfully merged the database files." << endl; - } - } else if (!parser->isSet(Command::QuietOption)) { - outputTextStream << "Database was not modified by merge operation." << endl; + outputTextStream << QObject::tr("Successfully merged %1 into %2.").arg(fromDatabasePath, toDatabasePath) + << endl; + } else { + outputTextStream << QObject::tr("Database was not modified by merge operation.") << endl; } return EXIT_SUCCESS; diff --git a/src/cli/Merge.h b/src/cli/Merge.h index 108f654c3..236e39242 100644 --- a/src/cli/Merge.h +++ b/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 diff --git a/src/cli/keepassxc-cli.1 b/src/cli/keepassxc-cli.1 index 511eeea4d..9d0f3553a 100644 --- a/src/cli/keepassxc-cli.1 +++ b/src/cli/keepassxc-cli.1 @@ -77,6 +77,9 @@ Displays the program version. .SS "Merge options" +.IP "-d, --dry-run " +Only print the changes detected by the merge operation. + .IP "-f, --key-file-from " Path of the key file for the second database. diff --git a/src/core/Merger.cpp b/src/core/Merger.cpp index c70cd4b04..f3729023a 100644 --- a/src/core/Merger.cpp +++ b/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) diff --git a/src/core/Merger.h b/src/core/Merger.h index 03a47a27f..712f4fde3 100644 --- a/src/core/Merger.h +++ b/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; diff --git a/src/gui/DatabaseWidget.cpp b/src/gui/DatabaseWidget.cpp index 824dab371..ed2327322 100644 --- a/src/gui/DatabaseWidget.cpp +++ b/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); diff --git a/src/gui/MainWindow.cpp b/src/gui/MainWindow.cpp index 292caa924..327c18efc 100644 --- a/src/gui/MainWindow.cpp +++ b/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()) { diff --git a/src/keeshare/ShareObserver.cpp b/src/keeshare/ShareObserver.cpp index 290ce1d15..e8931be1b 100644 --- a/src/keeshare/ShareObserver.cpp +++ b/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(); - if (changed) { + const QStringList changeList = merger.merge(); + 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(); - if (changed) { + const QStringList changeList = merger.merge(); + 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(); - if (changed) { + const QStringList changeList = merger.merge(); + 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(); - if (changed) { + const QStringList changeList = merger.merge(); + if (!changeList.isEmpty()) { return {reference.path, Result::Success, tr("Successful unsigned import")}; } return {}; diff --git a/tests/TestCli.cpp b/tests/TestCli.cpp index 530e1eabe..a7bb067c8 100644 --- a/tests/TestCli.cpp +++ b/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::create(); key->addKey(QSharedPointer::create("b")); db->setKey(key); - writer.writeDatabase(&targetFile2, db.data()); - targetFile2.close(); + writer.writeDatabase(&targetFile3, db.data()); + 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 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 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::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::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 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::create(); reader.readDatabase(&readBack, key, mergedDb.data());