From d99dee9c40d8c9c3444ed0f3851dd7195a06a5ab Mon Sep 17 00:00:00 2001 From: frostasm Date: Sun, 19 Nov 2017 21:06:09 +0200 Subject: [PATCH] Small refactoring related to references placeholders --- src/core/Database.cpp | 22 ++++---- src/core/Database.h | 6 +-- src/core/Entry.cpp | 15 +++--- src/core/Entry.h | 2 +- src/core/EntryAttachments.cpp | 2 +- src/core/EntryAttributes.cpp | 22 ++++---- src/core/EntryAttributes.h | 4 +- tests/TestEntry.cpp | 94 +++++++++++++++-------------------- 8 files changed, 78 insertions(+), 89 deletions(-) diff --git a/src/core/Database.cpp b/src/core/Database.cpp index 99dd94ef..7ae9308a 100644 --- a/src/core/Database.cpp +++ b/src/core/Database.cpp @@ -95,15 +95,15 @@ const Metadata* Database::metadata() const Entry* Database::resolveEntry(const Uuid& uuid) { - return recFindEntry(uuid, m_rootGroup); + return findEntryRecursive(uuid, m_rootGroup); } -Entry *Database::resolveEntry(const QString &text, EntryReferenceType referenceType) +Entry* Database::resolveEntry(const QString& text, EntryReferenceType referenceType) { - return recFindEntry(text, referenceType, m_rootGroup); + return findEntryRecursive(text, referenceType, m_rootGroup); } -Entry* Database::recFindEntry(const Uuid& uuid, Group* group) +Entry* Database::findEntryRecursive(const Uuid& uuid, Group* group) { const QList entryList = group->entries(); for (Entry* entry : entryList) { @@ -114,7 +114,7 @@ Entry* Database::recFindEntry(const Uuid& uuid, Group* group) const QList children = group->children(); for (Group* child : children) { - Entry* result = recFindEntry(uuid, child); + Entry* result = findEntryRecursive(uuid, child); if (result) { return result; } @@ -123,9 +123,9 @@ Entry* Database::recFindEntry(const Uuid& uuid, Group* group) return nullptr; } -Entry *Database::recFindEntry(const QString &text, EntryReferenceType referenceType, Group *group) +Entry* Database::findEntryRecursive(const QString& text, EntryReferenceType referenceType, Group* group) { - Q_ASSERT_X(referenceType != EntryReferenceType::Unknown, "Database::recFindEntry", + Q_ASSERT_X(referenceType != EntryReferenceType::Unknown, "Database::findEntryRecursive", "Can't search entry with \"referenceType\" parameter equal to \"Unknown\""); bool found = false; @@ -164,7 +164,7 @@ Entry *Database::recFindEntry(const QString &text, EntryReferenceType referenceT const QList children = group->children(); for (Group* child : children) { - Entry* result = recFindEntry(text, referenceType, child); + Entry* result = findEntryRecursive(text, referenceType, child); if (result) { return result; } @@ -175,10 +175,10 @@ Entry *Database::recFindEntry(const QString &text, EntryReferenceType referenceT Group* Database::resolveGroup(const Uuid& uuid) { - return recFindGroup(uuid, m_rootGroup); + return findGroupRecursive(uuid, m_rootGroup); } -Group* Database::recFindGroup(const Uuid& uuid, Group* group) +Group* Database::findGroupRecursive(const Uuid& uuid, Group* group) { if (group->uuid() == uuid) { return group; @@ -186,7 +186,7 @@ Group* Database::recFindGroup(const Uuid& uuid, Group* group) const QList children = group->children(); for (Group* child : children) { - Group* result = recFindGroup(uuid, child); + Group* result = findGroupRecursive(uuid, child); if (result) { return result; } diff --git a/src/core/Database.h b/src/core/Database.h index 14cb0bca..b20f897f 100644 --- a/src/core/Database.h +++ b/src/core/Database.h @@ -143,9 +143,9 @@ private slots: void startModifiedTimer(); private: - Entry* recFindEntry(const Uuid& uuid, Group* group); - Entry* recFindEntry(const QString& text, EntryReferenceType referenceType, Group* group); - Group* recFindGroup(const Uuid& uuid, Group* group); + Entry* findEntryRecursive(const Uuid& uuid, Group* group); + Entry* findEntryRecursive(const QString& text, EntryReferenceType referenceType, Group* group); + Group* findGroupRecursive(const Uuid& uuid, Group* group); void createRecycleBin(); diff --git a/src/core/Entry.cpp b/src/core/Entry.cpp index 65e2454e..11ae97d7 100644 --- a/src/core/Entry.cpp +++ b/src/core/Entry.cpp @@ -722,7 +722,7 @@ void Entry::updateModifiedSinceBegin() m_modifiedSinceBegin = true; } -QString Entry::resolveMultiplePlaceholdersRecursive(const QString &str, int maxDepth) const +QString Entry::resolveMultiplePlaceholdersRecursive(const QString& str, int maxDepth) const { if (maxDepth <= 0) { qWarning("Maximum depth of replacement has been reached. Entry uuid: %s", qPrintable(uuid().toHex())); @@ -746,7 +746,7 @@ QString Entry::resolveMultiplePlaceholdersRecursive(const QString &str, int maxD return result; } -QString Entry::resolvePlaceholderRecursive(const QString &placeholder, int maxDepth) const +QString Entry::resolvePlaceholderRecursive(const QString& placeholder, int maxDepth) const { const PlaceholderType typeOfPlaceholder = placeholderType(placeholder); switch (typeOfPlaceholder) { @@ -794,13 +794,12 @@ QString Entry::resolvePlaceholderRecursive(const QString &placeholder, int maxDe return placeholder; } -QString Entry::resolveReferencePlaceholderRecursive(const QString &placeholder, int maxDepth) const +QString Entry::resolveReferencePlaceholderRecursive(const QString& placeholder, int maxDepth) const { // resolving references in format: {REF:@:} // using format from http://keepass.info/help/base/fieldrefs.html at the time of writing - QRegularExpression* referenceRegExp = m_attributes->referenceRegExp(); - QRegularExpressionMatch match = referenceRegExp->match(placeholder); + QRegularExpressionMatch match = EntryAttributes::matchReference(placeholder); if (!match.hasMatch()) { return placeholder; } @@ -903,7 +902,7 @@ const Database* Entry::database() const } } -QString Entry::maskPasswordPlaceholders(const QString &str) const +QString Entry::maskPasswordPlaceholders(const QString& str) const { QString result = str; result.replace(QRegExp("(\\{PASSWORD\\})", Qt::CaseInsensitive, QRegExp::RegExp2), "******"); @@ -920,7 +919,7 @@ QString Entry::resolvePlaceholder(const QString& placeholder) const return resolvePlaceholderRecursive(placeholder, ResolveMaximumDepth); } -QString Entry::resolveUrlPlaceholder(const QString &str, Entry::PlaceholderType placeholderType) const +QString Entry::resolveUrlPlaceholder(const QString& str, Entry::PlaceholderType placeholderType) const { if (str.isEmpty()) return QString(); @@ -956,7 +955,7 @@ QString Entry::resolveUrlPlaceholder(const QString &str, Entry::PlaceholderType return QString(); } -Entry::PlaceholderType Entry::placeholderType(const QString &placeholder) const +Entry::PlaceholderType Entry::placeholderType(const QString& placeholder) const { if (!placeholder.startsWith(QLatin1Char('{')) || !placeholder.endsWith(QLatin1Char('}'))) { return PlaceholderType::NotPlaceholder; diff --git a/src/core/Entry.h b/src/core/Entry.h index af11478b..266254e6 100644 --- a/src/core/Entry.h +++ b/src/core/Entry.h @@ -181,7 +181,7 @@ public: QString maskPasswordPlaceholders(const QString& str) const; QString resolveMultiplePlaceholders(const QString& str) const; QString resolvePlaceholder(const QString& str) const; - QString resolveUrlPlaceholder(const QString &str, PlaceholderType placeholderType) const; + QString resolveUrlPlaceholder(const QString& str, PlaceholderType placeholderType) const; PlaceholderType placeholderType(const QString& placeholder) const; QString resolveUrl(const QString& url) const; diff --git a/src/core/EntryAttachments.cpp b/src/core/EntryAttachments.cpp index 8ff66277..bab11057 100644 --- a/src/core/EntryAttachments.cpp +++ b/src/core/EntryAttachments.cpp @@ -86,7 +86,7 @@ void EntryAttachments::remove(const QString& key) emit modified(); } -void EntryAttachments::remove(const QStringList &keys) +void EntryAttachments::remove(const QStringList& keys) { if (keys.isEmpty()) { return; diff --git a/src/core/EntryAttributes.cpp b/src/core/EntryAttributes.cpp index 21dcccf6..8cc7f2f0 100644 --- a/src/core/EntryAttributes.cpp +++ b/src/core/EntryAttributes.cpp @@ -34,8 +34,6 @@ const QString EntryAttributes::RememberCmdExecAttr = "_EXEC_CMD"; EntryAttributes::EntryAttributes(QObject* parent) : QObject(parent) - , m_referenceRegExp("\\{REF:(?[TUPANI])@(?[TUPANIO]):(?[^}]+)\\}", - QRegularExpression::CaseInsensitiveOption) { clear(); } @@ -67,12 +65,12 @@ QString EntryAttributes::value(const QString& key) const return m_attributes.value(key); } -bool EntryAttributes::contains(const QString &key) const +bool EntryAttributes::contains(const QString& key) const { return m_attributes.contains(key); } -bool EntryAttributes::containsValue(const QString &value) const +bool EntryAttributes::containsValue(const QString& value) const { return m_attributes.values().contains(value); } @@ -90,12 +88,7 @@ bool EntryAttributes::isReference(const QString& key) const } const QString data = value(key); - return m_referenceRegExp.match(data).hasMatch(); -} - -QRegularExpression* EntryAttributes::referenceRegExp() -{ - return &m_referenceRegExp; + return matchReference(data).hasMatch(); } void EntryAttributes::set(const QString& key, const QString& value, bool protect) @@ -266,6 +259,15 @@ bool EntryAttributes::operator!=(const EntryAttributes& other) const || m_protectedAttributes != other.m_protectedAttributes); } +QRegularExpressionMatch EntryAttributes::matchReference(const QString& text) +{ + static QRegularExpression referenceRegExp( + "\\{REF:(?[TUPANI])@(?[TUPANIO]):(?[^}]+)\\}", + QRegularExpression::CaseInsensitiveOption); + + return referenceRegExp.match(text); +} + void EntryAttributes::clear() { emit aboutToBeReset(); diff --git a/src/core/EntryAttributes.h b/src/core/EntryAttributes.h index f2838865..f483b8a9 100644 --- a/src/core/EntryAttributes.h +++ b/src/core/EntryAttributes.h @@ -39,7 +39,6 @@ public: bool containsValue(const QString& value) const; bool isProtected(const QString& key) const; bool isReference(const QString& key) const; - QRegularExpression *referenceRegExp(); void set(const QString& key, const QString& value, bool protect = false); void remove(const QString& key); void rename(const QString& oldKey, const QString& newKey); @@ -51,6 +50,8 @@ public: bool operator==(const EntryAttributes& other) const; bool operator!=(const EntryAttributes& other) const; + static QRegularExpressionMatch matchReference(const QString& text); + static const QString TitleKey; static const QString UserNameKey; static const QString PasswordKey; @@ -80,7 +81,6 @@ signals: private: QMap m_attributes; QSet m_protectedAttributes; - QRegularExpression m_referenceRegExp; }; #endif // KEEPASSX_ENTRYATTRIBUTES_H diff --git a/tests/TestEntry.cpp b/tests/TestEntry.cpp index 802a27b7..bb117645 100644 --- a/tests/TestEntry.cpp +++ b/tests/TestEntry.cpp @@ -281,7 +281,7 @@ void TestEntry::testResolveReferencePlaceholders() Group* group = new Group(); group->setParent(root); Entry* entry2 = new Entry(); - entry2->setGroup(root); + entry2->setGroup(group); entry2->setUuid(Uuid::random()); entry2->setTitle("Title2"); entry2->setUsername("Username2"); @@ -291,7 +291,7 @@ void TestEntry::testResolveReferencePlaceholders() entry2->attributes()->set("CustomAttribute2", "CustomAttributeValue2"); Entry* entry3 = new Entry(); - entry3->setGroup(root); + entry3->setGroup(group); entry3->setUuid(Uuid::random()); entry3->setTitle("{S:AttributeTitle}"); entry3->setUsername("{S:AttributeUsername}"); @@ -308,51 +308,43 @@ void TestEntry::testResolveReferencePlaceholders() tstEntry->setGroup(root); tstEntry->setUuid(Uuid::random()); - QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@I:%1}").arg(entry1->uuid().toHex())), QString("Title1")); - QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@T:%1}").arg(entry1->title())), QString("Title1")); - QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@U:%1}").arg(entry1->username())), QString("Title1")); - QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@P:%1}").arg(entry1->password())), QString("Title1")); - QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@A:%1}").arg(entry1->url())), QString("Title1")); - QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@N:%1}").arg(entry1->notes())), QString("Title1")); - QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@O:%1}").arg(entry1->attributes()->value("CustomAttribute1"))), QString("Title1")); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@I:%1}").arg(entry1->uuid().toHex())), entry1->title()); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@T:%1}").arg(entry1->title())), entry1->title()); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@U:%1}").arg(entry1->username())), entry1->title()); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@P:%1}").arg(entry1->password())), entry1->title()); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@A:%1}").arg(entry1->url())), entry1->title()); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@N:%1}").arg(entry1->notes())), entry1->title()); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@O:%1}").arg(entry1->attributes()->value("CustomAttribute1"))), entry1->title()); - QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@I:%1}").arg(entry1->uuid().toHex())), QString("Title1")); - QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@T:%1}").arg(entry1->title())), QString("Title1")); - QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:U@U:%1}").arg(entry1->username())), QString("Username1")); - QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:P@P:%1}").arg(entry1->password())), QString("Password1")); - QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:A@A:%1}").arg(entry1->url())), QString("Url1")); - QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:N@N:%1}").arg(entry1->notes())), QString("Notes1")); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@I:%1}").arg(entry1->uuid().toHex())), entry1->title()); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@T:%1}").arg(entry1->title())), entry1->title()); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:U@U:%1}").arg(entry1->username())), entry1->username()); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:P@P:%1}").arg(entry1->password())), entry1->password()); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:A@A:%1}").arg(entry1->url())), entry1->url()); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:N@N:%1}").arg(entry1->notes())), entry1->notes()); - QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@I:%1}").arg(entry2->uuid().toHex())), QString("Title2")); - QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@T:%1}").arg(entry2->title())), QString("Title2")); - QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@U:%1}").arg(entry2->username())), QString("Title2")); - QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@P:%1}").arg(entry2->password())), QString("Title2")); - QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@A:%1}").arg(entry2->url())), QString("Title2")); - QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@N:%1}").arg(entry2->notes())), QString("Title2")); - QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@O:%1}").arg(entry2->attributes()->value("CustomAttribute2"))), QString("Title2")); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@I:%1}").arg(entry2->uuid().toHex())), entry2->title()); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@T:%1}").arg(entry2->title())), entry2->title()); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@U:%1}").arg(entry2->username())), entry2->title()); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@P:%1}").arg(entry2->password())), entry2->title()); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@A:%1}").arg(entry2->url())), entry2->title()); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@N:%1}").arg(entry2->notes())), entry2->title()); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@O:%1}").arg(entry2->attributes()->value("CustomAttribute2"))), entry2->title()); - QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@T:%1}").arg(entry2->title())), QString("Title2")); - QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:U@U:%1}").arg(entry2->username())), QString("Username2")); - QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:P@P:%1}").arg(entry2->password())), QString("Password2")); - QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:A@A:%1}").arg(entry2->url())), QString("Url2")); - QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:N@N:%1}").arg(entry2->notes())), QString("Notes2")); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@T:%1}").arg(entry2->title())), entry2->title()); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:U@U:%1}").arg(entry2->username())), entry2->username()); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:P@P:%1}").arg(entry2->password())), entry2->password()); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:A@A:%1}").arg(entry2->url())), entry2->url()); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:N@N:%1}").arg(entry2->notes())), entry2->notes()); - QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@I:%1}").arg(entry3->uuid().toHex())), QString("TitleValue")); - QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@I:%1}").arg(entry3->uuid().toHex())), QString("TitleValue")); - QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:U@I:%1}").arg(entry3->uuid().toHex())), QString("UsernameValue")); - QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:P@I:%1}").arg(entry3->uuid().toHex())), QString("PasswordValue")); - QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:A@I:%1}").arg(entry3->uuid().toHex())), QString("UrlValue")); - QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:N@I:%1}").arg(entry3->uuid().toHex())), QString("NotesValue")); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@I:%1}").arg(entry3->uuid().toHex())), entry3->attributes()->value("AttributeTitle")); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@I:%1}").arg(entry3->uuid().toHex())), entry3->attributes()->value("AttributeTitle")); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:U@I:%1}").arg(entry3->uuid().toHex())), entry3->attributes()->value("AttributeUsername")); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:P@I:%1}").arg(entry3->uuid().toHex())), entry3->attributes()->value("AttributePassword")); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:A@I:%1}").arg(entry3->uuid().toHex())), entry3->attributes()->value("AttributeUrl")); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:N@I:%1}").arg(entry3->uuid().toHex())), entry3->attributes()->value("AttributeNotes")); } -static const char placeholders[] = { -'T', -'U', -'P', -'A', -'N', -}; - void TestEntry::testResolveNonIdPlaceholdersToUuid() { Database db; @@ -383,35 +375,31 @@ void TestEntry::testResolveNonIdPlaceholdersToUuid() referencedEntryNotes.setNotes("myNotes"); referencedEntryNotes.setUuid(Uuid::random()); - for (const auto searchIn : placeholders) { + const QList placeholders{'T', 'U', 'P', 'A', 'N'}; + for (const QChar searchIn : placeholders) { const Entry* referencedEntry = nullptr; QString newEntryNotesRaw("{REF:I@%1:%2}"); - switch(searchIn) { + switch(searchIn.toLatin1()) { case 'T': referencedEntry = &referencedEntryTitle; - newEntryNotesRaw = newEntryNotesRaw.arg( - QString(searchIn), referencedEntry->title()); + newEntryNotesRaw = newEntryNotesRaw.arg(searchIn, referencedEntry->title()); break; case 'U': referencedEntry = &referencedEntryUsername; - newEntryNotesRaw = newEntryNotesRaw.arg( - QString(searchIn), referencedEntry->username()); + newEntryNotesRaw = newEntryNotesRaw.arg(searchIn, referencedEntry->username()); break; case 'P': referencedEntry = &referencedEntryPassword; - newEntryNotesRaw = newEntryNotesRaw.arg( - QString(searchIn), referencedEntry->password()); + newEntryNotesRaw = newEntryNotesRaw.arg(searchIn, referencedEntry->password()); break; case 'A': referencedEntry = &referencedEntryUrl; - newEntryNotesRaw = newEntryNotesRaw.arg( - QString(searchIn), referencedEntry->url()); + newEntryNotesRaw = newEntryNotesRaw.arg(searchIn, referencedEntry->url()); break; case 'N': referencedEntry = &referencedEntryNotes; - newEntryNotesRaw = newEntryNotesRaw.arg( - QString(searchIn), referencedEntry->notes()); + newEntryNotesRaw = newEntryNotesRaw.arg(searchIn, referencedEntry->notes()); break; default: break;