From e8f2c9d126ac59aa65a1c3400aad83c97b38c9a7 Mon Sep 17 00:00:00 2001 From: Robin Ebert Date: Tue, 3 Aug 2021 15:37:47 +0200 Subject: [PATCH] CLI: Replace locate command with search * Introduce search CLI command to replace locate command. Search can provide the same functionality but in a more fine-grained fashion * Replace use of Group::locate in code: Use EntrySearcher in clip cli command best-match option. This removes the matching against group hierarchy of an entry which is kind of nonsense as clip expects exactly one match. Matching against groups can be done using search command. * Remove obsolete Group::locate method --- docs/man/keepassxc-cli.1.adoc | 9 +-- src/cli/CMakeLists.txt | 2 +- src/cli/Clip.cpp | 13 +-- src/cli/Command.cpp | 4 +- src/cli/{Locate.cpp => Search.cpp} | 17 ++-- src/cli/{Locate.h => Search.h} | 10 +-- src/core/Group.cpp | 24 ------ src/core/Group.h | 1 - tests/TestCli.cpp | 125 +++++++++++++++++------------ tests/TestCli.h | 2 +- tests/TestGroup.cpp | 60 -------------- tests/TestGroup.h | 1 - 12 files changed, 103 insertions(+), 165 deletions(-) rename src/cli/{Locate.cpp => Search.cpp} (77%) rename src/cli/{Locate.h => Search.h} (86%) diff --git a/docs/man/keepassxc-cli.1.adoc b/docs/man/keepassxc-cli.1.adoc index 906e45e05..3696f090f 100644 --- a/docs/man/keepassxc-cli.1.adoc +++ b/docs/man/keepassxc-cli.1.adoc @@ -90,10 +90,6 @@ It provides the ability to query and modify the entries of a KeePass database, d If both the key file and password are empty, no database will be created. The new database will be in kdbx 4 format. - -*locate* [_options_] <__database__> <__term__>:: - Locates all the entries that match a specific search term in a database. - *ls* [_options_] <__database__> [_group_]:: Lists the contents of a group in a database. If no group is specified, it will default to the root group. @@ -127,6 +123,9 @@ It provides the ability to query and modify the entries of a KeePass database, d If the database has a recycle bin, the group will be moved there. If the group is already in the recycle bin, it will be removed permanently. +*search* [_options_] <__database__> <__term__>:: + Searches all entries that match a specific search term in a database. + *show* [_options_] <__database__> <__entry__>:: Shows the title, username, password, URL and notes of a database entry. Can also show the current TOTP. @@ -221,7 +220,7 @@ The same password generation options as documented for the generate command can Will report an error if no TOTP is configured for the entry. *-b*, *--best*:: - Try to find and copy to clipboard a unique entry matching the input (similar to *-locate*) + Try to find and copy to clipboard a unique entry matching the input If a unique matching entry is found it will be copied to the clipboard. If multiple entries are found they will be listed to refine the search. (no clip performed) diff --git a/src/cli/CMakeLists.txt b/src/cli/CMakeLists.txt index 7f058312d..4e6defd14 100644 --- a/src/cli/CMakeLists.txt +++ b/src/cli/CMakeLists.txt @@ -32,12 +32,12 @@ set(cli_SOURCES Import.cpp Info.cpp List.cpp - Locate.cpp Merge.cpp Move.cpp Open.cpp Remove.cpp RemoveGroup.cpp + Search.cpp Show.cpp) add_library(cli STATIC ${cli_SOURCES}) diff --git a/src/cli/Clip.cpp b/src/cli/Clip.cpp index 8f61e73c4..7c4a9bb1f 100644 --- a/src/cli/Clip.cpp +++ b/src/cli/Clip.cpp @@ -18,6 +18,7 @@ #include "Clip.h" #include "Utils.h" +#include "core/EntrySearcher.h" #include "core/Group.h" #include "core/Tools.h" @@ -78,16 +79,18 @@ int Clip::executeWithDatabase(QSharedPointer database, QSharedPointer< QString entryPath; if (parser->isSet(Clip::BestMatchOption)) { - QStringList results = database->rootGroup()->locate(args.at(1)); + EntrySearcher searcher; + const auto& searchTerm = args.at(1); + const auto results = searcher.search(QString("title:%1").arg(searchTerm), database->rootGroup(), true); if (results.count() > 1) { err << QObject::tr("Multiple entries matching:") << endl; - for (const QString& result : asConst(results)) { - err << result << endl; + for (const Entry* result : results) { + err << result->path().prepend('/') << endl; } return EXIT_FAILURE; } else { - entryPath = (results.isEmpty()) ? args.at(1) : results[0]; - out << QObject::tr("Used matching entry: %1").arg(entryPath) << endl; + entryPath = (results.isEmpty()) ? searchTerm : results[0]->path().prepend('/'); + out << QObject::tr("Using matching entry: %1").arg(entryPath) << endl; } } else { entryPath = args.at(1); diff --git a/src/cli/Command.cpp b/src/cli/Command.cpp index f03091926..0699921ac 100644 --- a/src/cli/Command.cpp +++ b/src/cli/Command.cpp @@ -31,12 +31,12 @@ #include "Import.h" #include "Info.h" #include "List.h" -#include "Locate.h" #include "Merge.h" #include "Move.h" #include "Open.h" #include "Remove.h" #include "RemoveGroup.h" +#include "Search.h" #include "Show.h" #include "Utils.h" @@ -173,7 +173,6 @@ namespace Commands s_commands.insert(QStringLiteral("estimate"), QSharedPointer(new Estimate())); s_commands.insert(QStringLiteral("generate"), QSharedPointer(new Generate())); s_commands.insert(QStringLiteral("help"), QSharedPointer(new Help())); - s_commands.insert(QStringLiteral("locate"), QSharedPointer(new Locate())); s_commands.insert(QStringLiteral("ls"), QSharedPointer(new List())); s_commands.insert(QStringLiteral("merge"), QSharedPointer(new Merge())); s_commands.insert(QStringLiteral("mkdir"), QSharedPointer(new AddGroup())); @@ -181,6 +180,7 @@ namespace Commands s_commands.insert(QStringLiteral("open"), QSharedPointer(new Open())); s_commands.insert(QStringLiteral("rm"), QSharedPointer(new Remove())); s_commands.insert(QStringLiteral("rmdir"), QSharedPointer(new RemoveGroup())); + s_commands.insert(QStringLiteral("search"), QSharedPointer(new Search())); s_commands.insert(QStringLiteral("show"), QSharedPointer(new Show())); if (interactive) { diff --git a/src/cli/Locate.cpp b/src/cli/Search.cpp similarity index 77% rename from src/cli/Locate.cpp rename to src/cli/Search.cpp index 8a2791324..44f1743e3 100644 --- a/src/cli/Locate.cpp +++ b/src/cli/Search.cpp @@ -15,36 +15,37 @@ * along with this program. If not, see . */ -#include "Locate.h" +#include "Search.h" #include #include "Utils.h" +#include "core/EntrySearcher.h" #include "core/Group.h" -Locate::Locate() +Search::Search() { - name = QString("locate"); + name = QString("search"); description = QObject::tr("Find entries quickly."); positionalArguments.append({QString("term"), QObject::tr("Search term."), QString("")}); } -int Locate::executeWithDatabase(QSharedPointer database, QSharedPointer parser) +int Search::executeWithDatabase(QSharedPointer database, QSharedPointer parser) { auto& out = Utils::STDOUT; auto& err = Utils::STDERR; const QStringList args = parser->positionalArguments(); - const QString& searchTerm = args.at(1); - QStringList results = database->rootGroup()->locate(searchTerm); + EntrySearcher searcher; + auto results = searcher.search(args.at(1), database->rootGroup(), true); if (results.isEmpty()) { err << "No results for that search term." << endl; return EXIT_FAILURE; } - for (const QString& result : asConst(results)) { - out << result << endl; + for (const Entry* result : asConst(results)) { + out << result->path().prepend('/') << endl; } return EXIT_SUCCESS; } diff --git a/src/cli/Locate.h b/src/cli/Search.h similarity index 86% rename from src/cli/Locate.h rename to src/cli/Search.h index 66419614e..a7ea2a276 100644 --- a/src/cli/Locate.h +++ b/src/cli/Search.h @@ -15,17 +15,17 @@ * along with this program. If not, see . */ -#ifndef KEEPASSXC_LOCATE_H -#define KEEPASSXC_LOCATE_H +#ifndef KEEPASSXC_SEARCH_H +#define KEEPASSXC_SEARCH_H #include "DatabaseCommand.h" -class Locate : public DatabaseCommand +class Search : public DatabaseCommand { public: - Locate(); + Search(); int executeWithDatabase(QSharedPointer db, QSharedPointer parser) override; }; -#endif // KEEPASSXC_LOCATE_H +#endif // KEEPASSXC_SEARCH_H diff --git a/src/core/Group.cpp b/src/core/Group.cpp index a6bb4173c..2f6ace769 100644 --- a/src/core/Group.cpp +++ b/src/core/Group.cpp @@ -1079,30 +1079,6 @@ bool Group::resolveAutoTypeEnabled() const } } -QStringList Group::locate(const QString& locateTerm, const QString& currentPath) const -{ - // TODO: Replace with EntrySearcher - QStringList response; - if (locateTerm.isEmpty()) { - return response; - } - - for (const Entry* entry : asConst(m_entries)) { - QString entryPath = currentPath + entry->title(); - if (entryPath.contains(locateTerm, Qt::CaseInsensitive)) { - response << entryPath; - } - } - - for (const Group* group : asConst(m_children)) { - for (const QString& path : group->locate(locateTerm, currentPath + group->name() + QString("/"))) { - response << path; - } - } - - return response; -} - Entry* Group::addEntryWithPath(const QString& entryPath) { if (entryPath.isEmpty() || findEntryByPath(entryPath)) { diff --git a/src/core/Group.h b/src/core/Group.h index 172bb6596..f259cd8b8 100644 --- a/src/core/Group.h +++ b/src/core/Group.h @@ -114,7 +114,6 @@ public: Entry* findEntryBySearchTerm(const QString& term, EntryReferenceType referenceType); Group* findGroupByUuid(const QUuid& uuid); Group* findGroupByPath(const QString& groupPath); - QStringList locate(const QString& locateTerm, const QString& currentPath = {"/"}) const; Entry* addEntryWithPath(const QString& entryPath); void setUuid(const QUuid& uuid); void setName(const QString& name); diff --git a/tests/TestCli.cpp b/tests/TestCli.cpp index e56ad240f..116fe4836 100644 --- a/tests/TestCli.cpp +++ b/tests/TestCli.cpp @@ -41,12 +41,12 @@ #include "cli/Import.h" #include "cli/Info.h" #include "cli/List.h" -#include "cli/Locate.h" #include "cli/Merge.h" #include "cli/Move.h" #include "cli/Open.h" #include "cli/Remove.h" #include "cli/RemoveGroup.h" +#include "cli/Search.h" #include "cli/Show.h" #include "cli/Utils.h" @@ -226,7 +226,6 @@ void TestCli::testBatchCommands() QVERIFY(Commands::getCommand("generate")); QVERIFY(Commands::getCommand("help")); QVERIFY(Commands::getCommand("import")); - QVERIFY(Commands::getCommand("locate")); QVERIFY(Commands::getCommand("ls")); QVERIFY(Commands::getCommand("merge")); QVERIFY(Commands::getCommand("mkdir")); @@ -235,6 +234,7 @@ void TestCli::testBatchCommands() QVERIFY(Commands::getCommand("rm")); QVERIFY(Commands::getCommand("rmdir")); QVERIFY(Commands::getCommand("show")); + QVERIFY(Commands::getCommand("search")); QVERIFY(!Commands::getCommand("doesnotexist")); QCOMPARE(Commands::getCommands().size(), 22); } @@ -254,7 +254,6 @@ void TestCli::testInteractiveCommands() QVERIFY(Commands::getCommand("exit")); QVERIFY(Commands::getCommand("generate")); QVERIFY(Commands::getCommand("help")); - QVERIFY(Commands::getCommand("locate")); QVERIFY(Commands::getCommand("ls")); QVERIFY(Commands::getCommand("merge")); QVERIFY(Commands::getCommand("mkdir")); @@ -264,6 +263,7 @@ void TestCli::testInteractiveCommands() QVERIFY(Commands::getCommand("rm")); QVERIFY(Commands::getCommand("rmdir")); QVERIFY(Commands::getCommand("show")); + QVERIFY(Commands::getCommand("search")); QVERIFY(!Commands::getCommand("doesnotexist")); QCOMPARE(Commands::getCommands().size(), 22); } @@ -1274,55 +1274,6 @@ void TestCli::testList() QCOMPARE(m_stdout->readAll(), QByteArray()); } -void TestCli::testLocate() -{ - Locate locateCmd; - QVERIFY(!locateCmd.name.isEmpty()); - QVERIFY(locateCmd.getDescriptionLine().contains(locateCmd.name)); - - setInput("a"); - execCmd(locateCmd, {"locate", m_dbFile->fileName(), "Sample"}); - m_stderr->readLine(); // Skip password prompt - QCOMPARE(m_stderr->readAll(), QByteArray()); - QCOMPARE(m_stdout->readAll(), QByteArray("/Sample Entry\n")); - - // Quiet option - setInput("a"); - execCmd(locateCmd, {"locate", m_dbFile->fileName(), "-q", "Sample"}); - QCOMPARE(m_stderr->readAll(), QByteArray()); - QCOMPARE(m_stdout->readAll(), QByteArray("/Sample Entry\n")); - - setInput("a"); - execCmd(locateCmd, {"locate", m_dbFile->fileName(), "Does Not Exist"}); - m_stderr->readLine(); // skip password prompt - QCOMPARE(m_stderr->readAll(), QByteArray("No results for that search term.\n")); - QCOMPARE(m_stdout->readAll(), QByteArray()); - - // write a modified database - auto db = readDatabase(); - QVERIFY(db); - auto* group = db->rootGroup()->findGroupByPath("/General/"); - QVERIFY(group); - auto* entry = new Entry(); - entry->setUuid(QUuid::createUuid()); - entry->setTitle("New Entry"); - group->addEntry(entry); - - TemporaryFile tmpFile; - tmpFile.open(); - tmpFile.close(); - db->saveAs(tmpFile.fileName()); - - setInput("a"); - execCmd(locateCmd, {"locate", tmpFile.fileName(), "New"}); - QCOMPARE(m_stdout->readAll(), QByteArray("/General/New Entry\n")); - - setInput("a"); - execCmd(locateCmd, {"locate", tmpFile.fileName(), "Entry"}); - QCOMPARE(m_stdout->readAll(), - QByteArray("/Sample Entry\n/General/New Entry\n/Homebanking/Subgroup/Subgroup Entry\n")); -} - void TestCli::testMerge() { Merge mergeCmd; @@ -1694,6 +1645,76 @@ void TestCli::testRemoveQuiet() QVERIFY(!db->rootGroup()->findEntryByPath(QString("/%1/Sample Entry").arg(Group::tr("Recycle Bin")))); } +void TestCli::testSearch() +{ + Search searchCmd; + QVERIFY(!searchCmd.name.isEmpty()); + QVERIFY(searchCmd.getDescriptionLine().contains(searchCmd.name)); + + setInput("a"); + execCmd(searchCmd, {"search", m_dbFile->fileName(), "Sample"}); + m_stderr->readLine(); // Skip password prompt + QCOMPARE(m_stderr->readAll(), QByteArray()); + QCOMPARE(m_stdout->readAll(), QByteArray("/Sample Entry\n")); + + // Quiet option + setInput("a"); + execCmd(searchCmd, {"search", m_dbFile->fileName(), "-q", "Sample"}); + QCOMPARE(m_stderr->readAll(), QByteArray()); + QCOMPARE(m_stdout->readAll(), QByteArray("/Sample Entry\n")); + + setInput("a"); + execCmd(searchCmd, {"search", m_dbFile->fileName(), "Does Not Exist"}); + m_stderr->readLine(); // skip password prompt + QCOMPARE(m_stderr->readAll(), QByteArray("No results for that search term.\n")); + QCOMPARE(m_stdout->readAll(), QByteArray()); + + // write a modified database + auto db = readDatabase(); + QVERIFY(db); + auto* group = db->rootGroup()->findGroupByPath("/General/"); + QVERIFY(group); + auto* entry = new Entry(); + entry->setUuid(QUuid::createUuid()); + entry->setTitle("New Entry"); + group->addEntry(entry); + + TemporaryFile tmpFile; + tmpFile.open(); + tmpFile.close(); + db->saveAs(tmpFile.fileName()); + + setInput("a"); + execCmd(searchCmd, {"search", tmpFile.fileName(), "title:New"}); + QCOMPARE(m_stdout->readAll(), QByteArray("/General/New Entry\n")); + + setInput("a"); + execCmd(searchCmd, {"search", tmpFile.fileName(), "title:Entry"}); + QCOMPARE(m_stdout->readAll(), + QByteArray("/Sample Entry\n/General/New Entry\n/Homebanking/Subgroup/Subgroup Entry\n")); + + setInput("a"); + execCmd(searchCmd, {"search", tmpFile.fileName(), "group:General"}); + QCOMPARE(m_stdout->readAll(), QByteArray("/General/New Entry\n")); + + setInput("a"); + execCmd(searchCmd, {"search", tmpFile.fileName(), "group:NewDatabase"}); + QCOMPARE(m_stdout->readAll(), QByteArray("/Sample Entry\n")); + + setInput("a"); + execCmd(searchCmd, {"search", tmpFile.fileName(), "group:/NewDatabase"}); + QCOMPARE(m_stdout->readAll(), + QByteArray("/Sample Entry\n/General/New Entry\n/Homebanking/Subgroup/Subgroup Entry\n")); + + setInput("a"); + execCmd(searchCmd, {"search", tmpFile.fileName(), "url:bank"}); + QCOMPARE(m_stdout->readAll(), QByteArray("/Homebanking/Subgroup/Subgroup Entry\n")); + + setInput("a"); + execCmd(searchCmd, {"search", tmpFile.fileName(), "u:User Name"}); + QCOMPARE(m_stdout->readAll(), QByteArray("/Sample Entry\n/Homebanking/Subgroup/Subgroup Entry\n")); +} + void TestCli::testShow() { Show showCmd; diff --git a/tests/TestCli.h b/tests/TestCli.h index 1d294c1cc..299264dd7 100644 --- a/tests/TestCli.h +++ b/tests/TestCli.h @@ -65,7 +65,6 @@ private slots: void testHelp(); void testInteractiveCommands(); void testList(); - void testLocate(); void testMerge(); void testMergeWithKeys(); void testMove(); @@ -73,6 +72,7 @@ private slots: void testRemove(); void testRemoveGroup(); void testRemoveQuiet(); + void testSearch(); void testShow(); void testInvalidDbFiles(); void testYubiKeyOption(); diff --git a/tests/TestGroup.cpp b/tests/TestGroup.cpp index 4bcad7a4c..ccc94cc90 100644 --- a/tests/TestGroup.cpp +++ b/tests/TestGroup.cpp @@ -690,66 +690,6 @@ void TestGroup::testPrint() QVERIFY(output.contains(QString("subgroup/entry3\n"))); } -void TestGroup::testLocate() -{ - Database* db = new Database(); - - Entry* entry1 = new Entry(); - entry1->setTitle("entry1"); - entry1->setGroup(db->rootGroup()); - - Entry* entry2 = new Entry(); - entry2->setTitle("entry2"); - entry2->setGroup(db->rootGroup()); - - Group* group1 = new Group(); - group1->setName("group1"); - group1->setParent(db->rootGroup()); - - Group* group2 = new Group(); - group2->setName("group2"); - group2->setParent(group1); - - Entry* entry3 = new Entry(); - entry3->setTitle("entry3"); - entry3->setGroup(group1); - - Entry* entry43 = new Entry(); - entry43->setTitle("entry43"); - entry43->setGroup(group1); - - Entry* google = new Entry(); - google->setTitle("Google"); - google->setGroup(group2); - - QStringList results = db->rootGroup()->locate("entry"); - QVERIFY(results.size() == 4); - QVERIFY(results.contains("/group1/entry43")); - - results = db->rootGroup()->locate("entry1"); - QVERIFY(results.size() == 1); - QVERIFY(results.contains("/entry1")); - - results = db->rootGroup()->locate("Entry1"); - QVERIFY(results.size() == 1); - QVERIFY(results.contains("/entry1")); - - results = db->rootGroup()->locate("invalid"); - QVERIFY(results.isEmpty()); - - results = db->rootGroup()->locate("google"); - QVERIFY(results.size() == 1); - QVERIFY(results.contains("/group1/group2/Google")); - - results = db->rootGroup()->locate("group1"); - QVERIFY(results.size() == 3); - QVERIFY(results.contains("/group1/entry3")); - QVERIFY(results.contains("/group1/entry43")); - QVERIFY(results.contains("/group1/group2/Google")); - - delete db; -} - void TestGroup::testAddEntryWithPath() { Database* db = new Database(); diff --git a/tests/TestGroup.h b/tests/TestGroup.h index cb15f7114..b773ea017 100644 --- a/tests/TestGroup.h +++ b/tests/TestGroup.h @@ -39,7 +39,6 @@ private slots: void testFindEntry(); void testFindGroupByPath(); void testPrint(); - void testLocate(); void testAddEntryWithPath(); void testIsRecycled(); void testCopyDataFrom();