Commit 33b28a97 authored by Ingo Klöcker's avatar Ingo Klöcker
Browse files

Try hard to keep the currently selected key/item if the model changes

This ensures that the selection does not change for example after
setting the owner trust of the currently selected key.

* Store the current selection if an imminent model change is announced
* Restore (if possible) the selection after the model change
* Tell the model to use the key cache and remove the custom "Loading
  keys" item only after the initial key listing.
* Select the default key only after the initial key listing.
* In setCurrentKey() look up the key by fingerprint instead of by the
  key object because the latter fails if the key was updated.
* In setCurrentKey() fall back to selecting the default key if the
  given key isn't found and selectPerfectIdMatch() also fails.
* Add removeCustomItem() (currently only used in the test).

GnuPG-bug-id: 5511
parent 87498641
Pipeline #68109 passed with stage
in 8 minutes and 11 seconds
......@@ -36,6 +36,7 @@ ecm_add_tests(
)
ecm_add_tests(
keyselectioncombotest.cpp
keyserverconfigtest.cpp
newkeyapprovaldialogtest.cpp
LINK_LIBRARIES KF5::Libkleo Qt::Widgets Qt::Test
......
/*
autotests/keyselectioncombotest.cpp
This file is part of libkleopatra's test suite.
SPDX-FileCopyrightText: 2021 g10 Code GmbH
SPDX-FileContributor: Ingo Klöcker <dev@ingo-kloecker.de>
SPDX-License-Identifier: GPL-2.0-or-later
*/
#include <Libkleo/Formatting>
#include <Libkleo/KeyCache>
#include <Libkleo/KeySelectionCombo>
#include <QSignalSpy>
#include <QTest>
#include <gpgme++/key.h>
#include <gpgme++/keylistresult.h>
#include <gpgme.h>
#include <memory>
using namespace Kleo;
namespace
{
auto mapValidity(GpgME::UserID::Validity validity)
{
switch (validity) {
default:
case GpgME::UserID::Unknown: return GPGME_VALIDITY_UNKNOWN;
case GpgME::UserID::Undefined: return GPGME_VALIDITY_UNDEFINED;
case GpgME::UserID::Never: return GPGME_VALIDITY_NEVER;
case GpgME::UserID::Marginal: return GPGME_VALIDITY_MARGINAL;
case GpgME::UserID::Full: return GPGME_VALIDITY_FULL;
case GpgME::UserID::Ultimate: return GPGME_VALIDITY_ULTIMATE;
}
}
GpgME::Key createTestKey(const char *uid, GpgME::Protocol protocol = GpgME::UnknownProtocol, KeyUsage usage = KeyUsage::AnyUsage,
GpgME::UserID::Validity validity = GpgME::UserID::Full)
{
static int count = 0;
count++;
gpgme_key_t key;
gpgme_key_from_uid(&key, uid);
Q_ASSERT(key);
Q_ASSERT(key->uids);
if (protocol != GpgME::UnknownProtocol) {
key->protocol = protocol == GpgME::OpenPGP ? GPGME_PROTOCOL_OpenPGP : GPGME_PROTOCOL_CMS;
}
const QByteArray fingerprint = QByteArray::number(count, 16).rightJustified(40, '0');
key->fpr = strdup(fingerprint.constData());
key->revoked = 0;
key->expired = 0;
key->disabled = 0;
key->can_encrypt = int(usage == KeyUsage::AnyUsage || usage == KeyUsage::Encrypt);
key->can_sign = int(usage == KeyUsage::AnyUsage || usage == KeyUsage::Sign);
key->secret = 1;
key->uids->validity = mapValidity(validity);
return GpgME::Key(key, false);
}
auto testKey(const char *address, GpgME::Protocol protocol = GpgME::UnknownProtocol)
{
const auto email = GpgME::UserID::addrSpecFromString(address);
const auto keys = KeyCache::instance()->findByEMailAddress(email);
for (const auto &key: keys) {
if (protocol == GpgME::UnknownProtocol || key.protocol() == protocol) {
return key;
}
}
return GpgME::Key();
}
void waitForKeySelectionComboBeingInitialized(const KeySelectionCombo *combo)
{
QVERIFY(combo);
const auto spy = std::make_unique<QSignalSpy>(combo, &KeySelectionCombo::keyListingFinished);
QVERIFY(spy->isValid());
QVERIFY(spy->wait(10));
}
}
class KeySelectionComboTest: public QObject
{
Q_OBJECT
private Q_SLOTS:
void init()
{
// hold a reference to the key cache to avoid rebuilding while the test is running
mKeyCache = KeyCache::instance();
KeyCache::mutableInstance()->setKeys({
createTestKey("sender@example.net", GpgME::OpenPGP, KeyUsage::AnyUsage),
createTestKey("sender@example.net", GpgME::CMS, KeyUsage::AnyUsage),
createTestKey("Full Trust <prefer-openpgp@example.net>", GpgME::OpenPGP, KeyUsage::Encrypt),
createTestKey("Trusted S/MIME <prefer-smime@example.net>", GpgME::CMS, KeyUsage::Encrypt),
createTestKey("Marginal Validity <marginal-openpgp@example.net>", GpgME::OpenPGP, KeyUsage::Encrypt, GpgME::UserID::Marginal),
});
}
void cleanup()
{
// verify that nobody else holds a reference to the key cache
QVERIFY(mKeyCache.use_count() == 1);
mKeyCache.reset();
}
void test__verify_test_keys()
{
QVERIFY(!testKey("sender@example.net", GpgME::OpenPGP).isNull());
QVERIFY(!testKey("sender@example.net", GpgME::CMS).isNull());
QVERIFY(!testKey("Full Trust <prefer-openpgp@example.net>", GpgME::OpenPGP).isNull());
QVERIFY(!testKey("Trusted S/MIME <prefer-smime@example.net>", GpgME::CMS).isNull());
QVERIFY(!testKey("Marginal Validity <marginal-openpgp@example.net>", GpgME::OpenPGP).isNull());
}
void test__after_initialization_default_key_is_current_key()
{
const auto combo = std::make_unique<KeySelectionCombo>();
combo->setDefaultKey(QString::fromLatin1(testKey("Full Trust <prefer-openpgp@example.net>", GpgME::OpenPGP).primaryFingerprint()));
waitForKeySelectionComboBeingInitialized(combo.get());
QCOMPARE(combo->currentKey().primaryFingerprint(), testKey("Full Trust <prefer-openpgp@example.net>", GpgME::OpenPGP).primaryFingerprint());
}
void test__currently_selected_key_is_retained_if_cache_is_updated()
{
const auto combo = std::make_unique<KeySelectionCombo>();
combo->setDefaultKey(QString::fromLatin1(testKey("Full Trust <prefer-openpgp@example.net>", GpgME::OpenPGP).primaryFingerprint()));
waitForKeySelectionComboBeingInitialized(combo.get());
combo->setCurrentIndex(3);
QCOMPARE(combo->currentKey().primaryFingerprint(), testKey("Trusted S/MIME <prefer-smime@example.net>", GpgME::CMS).primaryFingerprint());
Q_EMIT KeyCache::mutableInstance()->keyListingDone(GpgME::KeyListResult{});
QCOMPARE(combo->currentKey().primaryFingerprint(), testKey("Trusted S/MIME <prefer-smime@example.net>", GpgME::CMS).primaryFingerprint());
}
void test__default_key_is_selected_if_currently_selected_key_is_gone_after_model_update()
{
const auto combo = std::make_unique<KeySelectionCombo>();
combo->setDefaultKey(QString::fromLatin1(testKey("Full Trust <prefer-openpgp@example.net>", GpgME::OpenPGP).primaryFingerprint()));
waitForKeySelectionComboBeingInitialized(combo.get());
combo->setCurrentIndex(3);
QCOMPARE(combo->currentKey().primaryFingerprint(), testKey("Trusted S/MIME <prefer-smime@example.net>", GpgME::CMS).primaryFingerprint());
KeyCache::mutableInstance()->setKeys({
testKey("sender@example.net", GpgME::OpenPGP),
testKey("sender@example.net", GpgME::CMS),
testKey("Full Trust <prefer-openpgp@example.net>", GpgME::OpenPGP),
testKey("Marginal Validity <marginal-openpgp@example.net>", GpgME::OpenPGP),
});
QCOMPARE(combo->currentKey().primaryFingerprint(), testKey("Full Trust <prefer-openpgp@example.net>", GpgME::OpenPGP).primaryFingerprint());
}
void test__currently_selected_custom_item_is_retained_if_cache_is_updated()
{
const auto combo = std::make_unique<KeySelectionCombo>();
combo->prependCustomItem({}, {}, QStringLiteral("custom1"));
combo->appendCustomItem({}, {}, QStringLiteral("custom2"));
combo->setDefaultKey(QString::fromLatin1(testKey("Full Trust <prefer-openpgp@example.net>", GpgME::OpenPGP).primaryFingerprint()));
waitForKeySelectionComboBeingInitialized(combo.get());
combo->setCurrentIndex(combo->count() - 1);
QCOMPARE(combo->currentData(), QStringLiteral("custom2"));
Q_EMIT KeyCache::mutableInstance()->keyListingDone(GpgME::KeyListResult{});
QCOMPARE(combo->currentData(), QStringLiteral("custom2"));
}
void test__default_key_is_selected_if_currently_selected_custom_item_is_gone_after_model_update()
{
const auto combo = std::make_unique<KeySelectionCombo>();
combo->prependCustomItem({}, {}, QStringLiteral("custom1"));
combo->appendCustomItem({}, {}, QStringLiteral("custom2"));
combo->setDefaultKey(QString::fromLatin1(testKey("Full Trust <prefer-openpgp@example.net>", GpgME::OpenPGP).primaryFingerprint()));
waitForKeySelectionComboBeingInitialized(combo.get());
combo->setCurrentIndex(combo->count() - 1);
QCOMPARE(combo->currentData(), QStringLiteral("custom2"));
combo->removeCustomItem(QStringLiteral("custom2"));
QCOMPARE(combo->currentKey().primaryFingerprint(), testKey("Full Trust <prefer-openpgp@example.net>", GpgME::OpenPGP).primaryFingerprint());
}
private:
std::shared_ptr<const KeyCache> mKeyCache;
};
QTEST_MAIN(KeySelectionComboTest)
#include "keyselectioncombotest.moc"
......@@ -376,6 +376,26 @@ public:
q->setCurrentKey(defaultKey);
}
void storeCurrentSelectionBeforeModelChange()
{
keyBeforeModelChange = q->currentKey();
customItemBeforeModelChange = q->currentData();
}
void restoreCurrentSelectionAfterModelChange()
{
if (!keyBeforeModelChange.isNull()) {
q->setCurrentKey(keyBeforeModelChange);
} else if (customItemBeforeModelChange.isValid()) {
const auto index = q->findData(customItemBeforeModelChange);
if (index != -1) {
q->setCurrentIndex(index);
} else {
updateWithDefaultKey();
}
}
}
Kleo::AbstractKeyListModel *model = nullptr;
SortFilterProxyModel *sortFilterProxy = nullptr;
ProxyModel *proxyModel = nullptr;
......@@ -383,8 +403,11 @@ public:
QMap<GpgME::Protocol, QString> defaultKeys;
bool wasEnabled = false;
bool useWasEnabled = false;
bool secretOnly;
bool secretOnly = false;
bool initialKeyListingDone = false;
QString mPerfectMatchMbox;
GpgME::Key keyBeforeModelChange;
QVariant customItemBeforeModelChange;
private:
KeySelectionCombo * const q;
......@@ -416,7 +439,7 @@ KeySelectionCombo::KeySelectionCombo(bool secretOnly, QWidget* parent)
this, [this](int row) {
if (row >= 0 && row < d->proxyModel->rowCount()) {
if (d->proxyModel->isCustomItem(row)) {
Q_EMIT customItemSelected(d->proxyModel->index(row, 0).data(Qt::UserRole));
Q_EMIT customItemSelected(currentData(Qt::UserRole));
} else {
Q_EMIT currentKeyChanged(currentKey());
}
......@@ -425,6 +448,19 @@ KeySelectionCombo::KeySelectionCombo(bool secretOnly, QWidget* parent)
d->cache = Kleo::KeyCache::mutableInstance();
connect(model(), &QAbstractItemModel::rowsAboutToBeInserted,
this, [this] () { d->storeCurrentSelectionBeforeModelChange(); });
connect(model(), &QAbstractItemModel::rowsInserted,
this, [this] () { d->restoreCurrentSelectionAfterModelChange(); });
connect(model(), &QAbstractItemModel::rowsAboutToBeRemoved,
this, [this] () { d->storeCurrentSelectionBeforeModelChange(); });
connect(model(), &QAbstractItemModel::rowsRemoved,
this, [this] () { d->restoreCurrentSelectionAfterModelChange(); });
connect(model(), &QAbstractItemModel::modelAboutToBeReset,
this, [this] () { d->storeCurrentSelectionBeforeModelChange(); });
connect(model(), &QAbstractItemModel::modelReset,
this, [this] () { d->restoreCurrentSelectionAfterModelChange(); });
QTimer::singleShot(0, this, &KeySelectionCombo::init);
}
......@@ -439,8 +475,10 @@ void KeySelectionCombo::init()
this, [this]() {
// Set useKeyCache ensures that the cache is populated
// so this can be a blocking call if the cache is not initialized
d->model->useKeyCache(true, d->secretOnly ? KeyList::SecretKeysOnly : KeyList::AllKeys);
d->proxyModel->removeCustomItem(QStringLiteral("-libkleo-loading-keys"));
if (!d->initialKeyListingDone) {
d->model->useKeyCache(true, d->secretOnly ? KeyList::SecretKeysOnly : KeyList::AllKeys);
d->proxyModel->removeCustomItem(QStringLiteral("-libkleo-loading-keys"));
}
// We use the useWasEnabled state variable to decide if we should
// change the enable / disable state based on the keylist done signal.
......@@ -458,7 +496,10 @@ void KeySelectionCombo::init()
});
connect(this, &KeySelectionCombo::keyListingFinished, this, [this]() {
d->updateWithDefaultKey();
if (!d->initialKeyListingDone) {
d->updateWithDefaultKey();
d->initialKeyListingDone = true;
}
});
if (!d->cache->initialized()) {
......@@ -505,11 +546,11 @@ GpgME::Key Kleo::KeySelectionCombo::currentKey() const
void Kleo::KeySelectionCombo::setCurrentKey(const GpgME::Key &key)
{
const int idx = findData(QVariant::fromValue(key), KeyList::KeyRole, Qt::MatchExactly);
const int idx = findData(QString::fromLatin1(key.primaryFingerprint()), KeyList::FingerprintRole, Qt::MatchExactly);
if (idx > -1) {
setCurrentIndex(idx);
} else {
d->selectPerfectIdMatch();
} else if (!d->selectPerfectIdMatch()) {
d->updateWithDefaultKey();
}
setToolTip(currentData(Qt::ToolTipRole).toString());
}
......@@ -565,6 +606,11 @@ void KeySelectionCombo::prependCustomItem(const QIcon &icon, const QString &text
prependCustomItem(icon, text, data, QString());
}
void KeySelectionCombo::removeCustomItem(const QVariant &data)
{
d->proxyModel->removeCustomItem(data);
}
void Kleo::KeySelectionCombo::setDefaultKey(const QString &fingerprint, GpgME::Protocol proto)
{
d->defaultKeys.insert(proto, fingerprint);
......
......@@ -55,6 +55,7 @@ public:
void appendCustomItem(const QIcon &icon, const QString &text, const QVariant &data);
void prependCustomItem(const QIcon &icon, const QString &text, const QVariant &data, const QString &toolTip);
void appendCustomItem(const QIcon &icon, const QString &text, const QVariant &data, const QString &toolTip);
void removeCustomItem(const QVariant &data);
Q_SIGNALS:
void customItemSelected(const QVariant &data);
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment