From f85642741d9bd6a48c98092ec5fd905de6b0207f Mon Sep 17 00:00:00 2001 From: Matthias Drexler Date: Sat, 22 Jun 2019 15:38:02 +0200 Subject: [PATCH] Autocomplete usernames based on most frequent in database * Fixes #3126 * Limit autocompletion to the top ten used usernames - Load common usernames when database is opened - Transition from QLineEdit to QComboBox for usernames - Dropdown menu of the combobox lets user choose a common username - Common usernames are autocompleted via inline completion - Common usernames are sorted by frequency (first) and name (second) --- src/core/Config.cpp | 1 - src/core/Database.cpp | 14 +++++++++++ src/core/Database.h | 5 ++++ src/core/Entry.cpp | 5 ++++ src/core/Entry.h | 1 + src/core/Group.cpp | 36 ++++++++++++++++++++++++++++ src/core/Group.h | 1 + src/gui/entry/EditEntryWidget.cpp | 23 ++++++++++++++---- src/gui/entry/EditEntryWidget.h | 3 +++ src/gui/entry/EditEntryWidgetMain.ui | 4 ++-- tests/TestGroup.cpp | 26 ++++++++++++++++++++ tests/TestGroup.h | 1 + tests/gui/TestGui.cpp | 24 ++++++++++++++++--- 13 files changed, 134 insertions(+), 10 deletions(-) diff --git a/src/core/Config.cpp b/src/core/Config.cpp index c9009236a..caf41c5d9 100644 --- a/src/core/Config.cpp +++ b/src/core/Config.cpp @@ -179,7 +179,6 @@ void Config::init(const QString& fileName) m_defaults.insert("SearchLimitGroup", false); m_defaults.insert("MinimizeOnCopy", false); m_defaults.insert("MinimizeOnOpenUrl", false); - m_defaults.insert("UseGroupIconOnEntryCreation", false); m_defaults.insert("AutoTypeEntryTitleMatch", true); m_defaults.insert("AutoTypeEntryURLMatch", true); m_defaults.insert("AutoTypeDelay", 25); diff --git a/src/core/Database.cpp b/src/core/Database.cpp index 571406059..6e69af5b0 100644 --- a/src/core/Database.cpp +++ b/src/core/Database.cpp @@ -54,6 +54,7 @@ Database::Database() connect(m_metadata, SIGNAL(metadataModified()), this, SLOT(markAsModified())); connect(m_timer, SIGNAL(timeout()), SIGNAL(databaseModified())); + connect(this, SIGNAL(databaseSaved()), SLOT(updateCommonUsernames())); m_modified = false; m_emitModified = true; @@ -149,6 +150,8 @@ bool Database::open(const QString& filePath, QSharedPointer setFilePath(filePath); dbFile.close(); + updateCommonUsernames(); + setInitialized(ok); markAsClean(); @@ -525,6 +528,17 @@ void Database::addDeletedObject(const QUuid& uuid) addDeletedObject(delObj); } +QList Database::commonUsernames() +{ + return m_commonUsernames; +} + +void Database::updateCommonUsernames(int topN) +{ + m_commonUsernames.clear(); + m_commonUsernames.append(rootGroup()->usernamesRecursive(topN)); +} + const QUuid& Database::cipher() const { return m_data.cipher; diff --git a/src/core/Database.h b/src/core/Database.h index 09602f764..86d90c6a7 100644 --- a/src/core/Database.h +++ b/src/core/Database.h @@ -106,6 +106,8 @@ public: bool containsDeletedObject(const DeletedObject& uuid) const; void setDeletedObjects(const QList& delObjs); + QList commonUsernames(); + bool hasKey() const; QSharedPointer key() const; bool setKey(const QSharedPointer& key, @@ -131,6 +133,7 @@ public: public slots: void markAsModified(); void markAsClean(); + void updateCommonUsernames(int topN = 10); signals: void filePathChanged(const QString& oldPath, const QString& newPath); @@ -184,6 +187,8 @@ private: bool m_modified = false; bool m_emitModified; + QList m_commonUsernames; + QUuid m_uuid; static QHash> s_uuidMap; static QHash> s_filePathMap; diff --git a/src/core/Entry.cpp b/src/core/Entry.cpp index 2ad73b055..c1f6286d4 100644 --- a/src/core/Entry.cpp +++ b/src/core/Entry.cpp @@ -341,6 +341,11 @@ bool Entry::isExpired() const return m_data.timeInfo.expires() && m_data.timeInfo.expiryTime() < Clock::currentDateTimeUtc(); } +bool Entry::isAttributeReference(const QString& key) const +{ + return m_attributes->isReference(key); +} + bool Entry::isAttributeReferenceOf(const QString& key, const QUuid& uuid) const { if (!m_attributes->isReference(key)) { diff --git a/src/core/Entry.h b/src/core/Entry.h index c5f59f2e3..45eb95ac5 100644 --- a/src/core/Entry.h +++ b/src/core/Entry.h @@ -111,6 +111,7 @@ public: bool hasTotp() const; bool isExpired() const; + bool isAttributeReference(const QString& key) const; bool isAttributeReferenceOf(const QString& key, const QUuid& uuid) const; void replaceReferencesWithValues(const Entry* other); bool hasReferences() const; diff --git a/src/core/Group.cpp b/src/core/Group.cpp index 84ad531be..9be878785 100644 --- a/src/core/Group.cpp +++ b/src/core/Group.cpp @@ -813,6 +813,42 @@ QSet Group::customIconsRecursive() const return result; } +QList Group::usernamesRecursive(int topN) const +{ + // Collect all usernames and sort for easy counting + QHash countedUsernames; + for (const auto* entry : entriesRecursive()) { + const auto username = entry->username(); + if (!username.isEmpty() && !entry->isAttributeReference(EntryAttributes::UserNameKey)) { + countedUsernames.insert(username, ++countedUsernames[username]); + } + } + + // Sort username/frequency pairs by frequency and name + QList> sortedUsernames; + for (const auto& key : countedUsernames.keys()) { + sortedUsernames.append({key, countedUsernames[key]}); + } + + auto comparator = [](const QPair& arg1, const QPair& arg2) { + if (arg1.second == arg2.second) { + return arg1.first < arg2.first; + } + return arg1.second > arg2.second; + }; + + std::sort(sortedUsernames.begin(), sortedUsernames.end(), comparator); + + // Take first topN usernames if set + QList usernames; + int actualUsernames = topN < 0 ? sortedUsernames.size() : std::min(topN, sortedUsernames.size()); + for (int i = 0; i < actualUsernames; i++) { + usernames.append(sortedUsernames[i].first); + } + + return usernames; +} + Group* Group::findGroupByUuid(const QUuid& uuid) { if (uuid.isNull()) { diff --git a/src/core/Group.h b/src/core/Group.h index 9fe65d69d..4b1204465 100644 --- a/src/core/Group.h +++ b/src/core/Group.h @@ -158,6 +158,7 @@ public: QList groupsRecursive(bool includeSelf) const; QList groupsRecursive(bool includeSelf); QSet customIconsRecursive() const; + QList usernamesRecursive(int topN = -1) const; Group* clone(Entry::CloneFlags entryFlags = DefaultEntryCloneFlags, CloneFlags groupFlags = DefaultCloneFlags) const; diff --git a/src/gui/entry/EditEntryWidget.cpp b/src/gui/entry/EditEntryWidget.cpp index b3dd6e3b4..523b8cdcd 100644 --- a/src/gui/entry/EditEntryWidget.cpp +++ b/src/gui/entry/EditEntryWidget.cpp @@ -85,6 +85,8 @@ EditEntryWidget::EditEntryWidget(QWidget* parent) , m_autoTypeAssocModel(new AutoTypeAssociationsModel(this)) , m_autoTypeDefaultSequenceGroup(new QButtonGroup(this)) , m_autoTypeWindowSequenceGroup(new QButtonGroup(this)) + , m_usernameCompleter(new QCompleter(this)) + , m_usernameCompleterModel(new QStringListModel(this)) { setupMain(); setupAdvanced(); @@ -129,6 +131,12 @@ void EditEntryWidget::setupMain() m_mainUi->setupUi(m_mainWidget); addPage(tr("Entry"), FilePath::instance()->icon("actions", "document-edit"), m_mainWidget); + m_mainUi->usernameComboBox->setEditable(true); + m_usernameCompleter->setCompletionMode(QCompleter::InlineCompletion); + m_usernameCompleter->setCaseSensitivity(Qt::CaseSensitive); + m_usernameCompleter->setModel(m_usernameCompleterModel); + m_mainUi->usernameComboBox->setCompleter(m_usernameCompleter); + m_mainUi->togglePasswordButton->setIcon(filePath()->onOffIcon("actions", "password-show")); m_mainUi->togglePasswordGeneratorButton->setIcon(filePath()->icon("actions", "password-generator")); #ifdef WITH_XC_NETWORKING @@ -273,7 +281,7 @@ void EditEntryWidget::setupEntryUpdate() { // Entry tab connect(m_mainUi->titleEdit, SIGNAL(textChanged(QString)), this, SLOT(setModified())); - connect(m_mainUi->usernameEdit, SIGNAL(textChanged(QString)), this, SLOT(setModified())); + connect(m_mainUi->usernameComboBox->lineEdit(), SIGNAL(textChanged(QString)), this, SLOT(setModified())); connect(m_mainUi->passwordEdit, SIGNAL(textChanged(QString)), this, SLOT(setModified())); connect(m_mainUi->passwordRepeatEdit, SIGNAL(textChanged(QString)), this, SLOT(setModified())); connect(m_mainUi->urlEdit, SIGNAL(textChanged(QString)), this, SLOT(setModified())); @@ -707,7 +715,7 @@ void EditEntryWidget::setForms(Entry* entry, bool restore) m_customData->copyDataFrom(entry->customData()); m_mainUi->titleEdit->setReadOnly(m_history); - m_mainUi->usernameEdit->setReadOnly(m_history); + m_mainUi->usernameComboBox->lineEdit()->setReadOnly(m_history); m_mainUi->urlEdit->setReadOnly(m_history); m_mainUi->passwordEdit->setReadOnly(m_history); m_mainUi->passwordRepeatEdit->setReadOnly(m_history); @@ -742,7 +750,7 @@ void EditEntryWidget::setForms(Entry* entry, bool restore) m_historyWidget->setEnabled(!m_history); m_mainUi->titleEdit->setText(entry->title()); - m_mainUi->usernameEdit->setText(entry->username()); + m_mainUi->usernameComboBox->lineEdit()->setText(entry->username()); m_mainUi->urlEdit->setText(entry->url()); m_mainUi->passwordEdit->setText(entry->password()); m_mainUi->passwordRepeatEdit->setText(entry->password()); @@ -751,6 +759,13 @@ void EditEntryWidget::setForms(Entry* entry, bool restore) m_mainUi->expirePresets->setEnabled(!m_history); m_mainUi->togglePasswordButton->setChecked(config()->get("security/passwordscleartext").toBool()); + QList commonUsernames = m_db->commonUsernames(); + m_usernameCompleterModel->setStringList(commonUsernames); + QString usernameToRestore = m_mainUi->usernameComboBox->lineEdit()->text(); + m_mainUi->usernameComboBox->clear(); + m_mainUi->usernameComboBox->addItems(commonUsernames); + m_mainUi->usernameComboBox->lineEdit()->setText(usernameToRestore); + m_mainUi->notesEdit->setPlainText(entry->notes()); m_advancedUi->attachmentsWidget->setEntryAttachments(entry->attachments()); @@ -910,7 +925,7 @@ void EditEntryWidget::updateEntryData(Entry* entry) const entry->attachments()->copyDataFrom(m_advancedUi->attachmentsWidget->entryAttachments()); entry->customData()->copyDataFrom(m_customData.data()); entry->setTitle(m_mainUi->titleEdit->text().replace(newLineRegex, " ")); - entry->setUsername(m_mainUi->usernameEdit->text().replace(newLineRegex, " ")); + entry->setUsername(m_mainUi->usernameComboBox->lineEdit()->text().replace(newLineRegex, " ")); entry->setUrl(m_mainUi->urlEdit->text().replace(newLineRegex, " ")); entry->setPassword(m_mainUi->passwordEdit->text()); entry->setExpires(m_mainUi->expireCheck->isChecked()); diff --git a/src/gui/entry/EditEntryWidget.h b/src/gui/entry/EditEntryWidget.h index aea3c894b..dd7bf8c07 100644 --- a/src/gui/entry/EditEntryWidget.h +++ b/src/gui/entry/EditEntryWidget.h @@ -20,6 +20,7 @@ #define KEEPASSX_EDITENTRYWIDGET_H #include +#include #include #include #include @@ -175,6 +176,8 @@ private: AutoTypeAssociationsModel* const m_autoTypeAssocModel; QButtonGroup* const m_autoTypeDefaultSequenceGroup; QButtonGroup* const m_autoTypeWindowSequenceGroup; + QCompleter* const m_usernameCompleter; + QStringListModel* const m_usernameCompleterModel; Q_DISABLE_COPY(EditEntryWidget) }; diff --git a/src/gui/entry/EditEntryWidgetMain.ui b/src/gui/entry/EditEntryWidgetMain.ui index 3e759fec7..5ed534dc2 100644 --- a/src/gui/entry/EditEntryWidgetMain.ui +++ b/src/gui/entry/EditEntryWidgetMain.ui @@ -164,7 +164,7 @@ - + @@ -191,7 +191,7 @@ titleEdit - usernameEdit + usernameComboBox passwordEdit passwordRepeatEdit togglePasswordButton diff --git a/tests/TestGroup.cpp b/tests/TestGroup.cpp index f78cb96af..bd3d36081 100644 --- a/tests/TestGroup.cpp +++ b/tests/TestGroup.cpp @@ -1181,3 +1181,29 @@ void TestGroup::testApplyGroupIconRecursively() QVERIFY(subsubgroup->iconNumber() == iconForGroups); QVERIFY(subsubgroupEntry->iconNumber() == iconForEntries); } + +void TestGroup::testUsernamesRecursive() +{ + Database* database = new Database(); + + // Create a subgroup + Group* subgroup = new Group(); + subgroup->setName("Subgroup"); + subgroup->setParent(database->rootGroup()); + + // Generate entries in the root group and the subgroup + Entry* rootGroupEntry = database->rootGroup()->addEntryWithPath("Root group entry"); + rootGroupEntry->setUsername("Name1"); + + Entry* subgroupEntry = subgroup->addEntryWithPath("Subgroup entry"); + subgroupEntry->setUsername("Name2"); + + Entry* subgroupEntryReusingUsername = subgroup->addEntryWithPath("Another subgroup entry"); + subgroupEntryReusingUsername->setUsername("Name2"); + + QList usernames = database->rootGroup()->usernamesRecursive(); + QCOMPARE(usernames.size(), 2); + QVERIFY(usernames.contains("Name1")); + QVERIFY(usernames.contains("Name2")); + QVERIFY(usernames.indexOf("Name2") < usernames.indexOf("Name1")); +} diff --git a/tests/TestGroup.h b/tests/TestGroup.h index 0ee735949..dbe5d6f4d 100644 --- a/tests/TestGroup.h +++ b/tests/TestGroup.h @@ -48,6 +48,7 @@ private slots: void testChildrenSort(); void testHierarchy(); void testApplyGroupIconRecursively(); + void testUsernamesRecursive(); }; #endif // KEEPASSX_TESTGROUP_H diff --git a/tests/gui/TestGui.cpp b/tests/gui/TestGui.cpp index 3579d90f3..af5107288 100644 --- a/tests/gui/TestGui.cpp +++ b/tests/gui/TestGui.cpp @@ -507,7 +507,7 @@ void TestGui::testEditEntry() QVERIFY(okButton); QCOMPARE(m_dbWidget->currentMode(), DatabaseWidget::Mode::EditMode); titleEdit->setText("multiline\ntitle"); - editEntryWidget->findChild("usernameEdit")->setText("multiline\nusername"); + editEntryWidget->findChild("usernameComboBox")->lineEdit()->setText("multiline\nusername"); editEntryWidget->findChild("passwordEdit")->setText("multiline\npassword"); editEntryWidget->findChild("passwordRepeatEdit")->setText("multiline\npassword"); editEntryWidget->findChild("urlEdit")->setText("multiline\nurl"); @@ -594,6 +594,10 @@ void TestGui::testAddEntry() auto* editEntryWidget = m_dbWidget->findChild("editEntryWidget"); auto* titleEdit = editEntryWidget->findChild("titleEdit"); QTest::keyClicks(titleEdit, "test"); + auto* usernameComboBox = editEntryWidget->findChild("usernameComboBox"); + QVERIFY(usernameComboBox); + QTest::mouseClick(usernameComboBox, Qt::LeftButton); + QTest::keyClicks(usernameComboBox, "AutocompletionUsername"); auto* editEntryWidgetButtonBox = editEntryWidget->findChild("buttonBox"); QTest::mouseClick(editEntryWidgetButtonBox->button(QDialogButtonBox::Ok), Qt::LeftButton); @@ -602,17 +606,31 @@ void TestGui::testAddEntry() Entry* entry = entryView->entryFromIndex(item); QCOMPARE(entry->title(), QString("test")); + QCOMPARE(entry->username(), QString("AutocompletionUsername")); QCOMPARE(entry->historyItems().size(), 0); + m_db->updateCommonUsernames(); + // Add entry "something 2" QTest::mouseClick(entryNewWidget, Qt::LeftButton); QTest::keyClicks(titleEdit, "something 2"); + QTest::mouseClick(usernameComboBox, Qt::LeftButton); + QTest::keyClicks(usernameComboBox, "Auto"); + QTest::keyPress(usernameComboBox, Qt::Key_Right); auto* passwordEdit = editEntryWidget->findChild("passwordEdit"); auto* passwordRepeatEdit = editEntryWidget->findChild("passwordRepeatEdit"); QTest::keyClicks(passwordEdit, "something 2"); QTest::keyClicks(passwordRepeatEdit, "something 2"); QTest::mouseClick(editEntryWidgetButtonBox->button(QDialogButtonBox::Ok), Qt::LeftButton); + QCOMPARE(m_dbWidget->currentMode(), DatabaseWidget::Mode::ViewMode); + item = entryView->model()->index(1, 1); + entry = entryView->entryFromIndex(item); + + QCOMPARE(entry->title(), QString("something 2")); + QCOMPARE(entry->username(), QString("AutocompletionUsername")); + QCOMPARE(entry->historyItems().size(), 0); + // Add entry "something 5" but click cancel button (does NOT add entry) QTest::mouseClick(entryNewWidget, Qt::LeftButton); QTest::keyClicks(titleEdit, "something 5"); @@ -1063,8 +1081,8 @@ void TestGui::testEntryPlaceholders() auto* editEntryWidget = m_dbWidget->findChild("editEntryWidget"); auto* titleEdit = editEntryWidget->findChild("titleEdit"); QTest::keyClicks(titleEdit, "test"); - QLineEdit* usernameEdit = editEntryWidget->findChild("usernameEdit"); - QTest::keyClicks(usernameEdit, "john"); + QComboBox* usernameComboBox = editEntryWidget->findChild("usernameComboBox"); + QTest::keyClicks(usernameComboBox, "john"); QLineEdit* urlEdit = editEntryWidget->findChild("urlEdit"); QTest::keyClicks(urlEdit, "{TITLE}.{USERNAME}"); auto* editEntryWidgetButtonBox = editEntryWidget->findChild("buttonBox");