Commit 2c4cf6d5 authored by Daniel Vrátil's avatar Daniel Vrátil 🤖
Browse files

Extend IdentityTest and fix all bugs it discovered

Fixes main serialization and deserialization of Identity to raw data.
The serializer was serializing QStrings, while the deserializer was
expecting QVariants, which did not behave as expected. The serializer
now serializes everything as QVariants too.

Fixes serialization of Signature - signature was never being stored
in mPropertiesMap, so the key was always empty. We instead serialize
mSignature directly. Calls to setSignature() etc. now also insert the
mSignature into the mPropertiesMap.

Improves operator==() to handle comparision between manually created
and deserialized Identity. Deserialization will fill mPropertiesMap
with lots of empty QVariants, but the default ctor will not so simple
mPropertiesMap == other.mPropertiesMap comparision was failing. The
algorithm now assumes that if a key is not present in one map but
is present in another the maps are considered equal as long as the
value is invalid.

Differential Revision: https://phabricator.kde.org/D2254
parent 5fc99624
......@@ -26,6 +26,7 @@
#include <QMimeData>
#include <QStandardPaths>
#include <QDataStream>
using namespace KIdentityManagement;
......@@ -36,6 +37,153 @@ void IdentityTester::initTestCase()
QStandardPaths::setTestModeEnabled(true);
}
bool IdentityTester::compareIdentities(const Identity &actual, const Identity &expected)
{
bool ok = false;
[&]() {
QCOMPARE(actual.uoid(), expected.uoid());
// Don't compare isDefault - only one copy can be default, so this property
// is not copied! It does not affect result of operator==() either.
//QCOMPARE(actual.isDefault(), expected.isDefault());
QCOMPARE(actual.identityName(), expected.identityName());
QCOMPARE(actual.fullName(), expected.fullName());
QCOMPARE(actual.organization(), expected.organization());
QCOMPARE(actual.pgpEncryptionKey(), expected.pgpEncryptionKey());
QCOMPARE(actual.pgpSigningKey(), expected.pgpSigningKey());
QCOMPARE(actual.smimeEncryptionKey(), expected.smimeEncryptionKey());
QCOMPARE(actual.smimeSigningKey(), expected.smimeSigningKey());
QCOMPARE(actual.preferredCryptoMessageFormat(), expected.preferredCryptoMessageFormat());
QCOMPARE(actual.emailAliases(), expected.emailAliases());
QCOMPARE(actual.primaryEmailAddress(), expected.primaryEmailAddress());
QCOMPARE(actual.vCardFile(), expected.vCardFile());
QCOMPARE(actual.replyToAddr(), expected.replyToAddr());
QCOMPARE(actual.bcc(), expected.bcc());
QCOMPARE(actual.cc(), expected.cc());
QCOMPARE(actual.attachVcard(), expected.attachVcard());
QCOMPARE(actual.autocorrectionLanguage(), expected.autocorrectionLanguage());
QCOMPARE(actual.disabledFcc(), expected.disabledFcc());
QCOMPARE(actual.pgpAutoSign(), expected.pgpAutoSign());
QCOMPARE(actual.pgpAutoEncrypt(), expected.pgpAutoEncrypt());
QCOMPARE(actual.defaultDomainName(), expected.defaultDomainName());
QCOMPARE(actual.signatureText(), expected.signatureText());
QCOMPARE(const_cast<Identity&>(actual).signature(), const_cast<Identity&>(expected).signature());
QCOMPARE(actual.transport(), expected.transport());
QCOMPARE(actual.fcc(), expected.fcc());
QCOMPARE(actual.drafts(), expected.drafts());
QCOMPARE(actual.templates(), expected.templates());
QCOMPARE(actual.dictionary(), expected.dictionary());
QCOMPARE(actual.isXFaceEnabled(), expected.isXFaceEnabled());
QCOMPARE(actual.xface(), expected.xface());
ok = true;
}();
return ok;
}
void IdentityTester::test_Identity()
{
Identity identity;
identity.setUoid(42);
QCOMPARE(identity.uoid(), 42u);
identity.setIsDefault(true);
QCOMPARE(identity.isDefault(), true);
identity.setIdentityName(QStringLiteral("01234"));
QCOMPARE(identity.identityName(), QStringLiteral("01234"));
identity.setFullName(QStringLiteral("Daniel Vrátil"));
QCOMPARE(identity.fullName(), QStringLiteral("Daniel Vrátil"));
identity.setOrganization(QStringLiteral("KDE"));
QCOMPARE(identity.organization(), QStringLiteral("KDE"));
identity.setPGPEncryptionKey("0x0123456789ABCDEF");
QCOMPARE(identity.pgpEncryptionKey(), QByteArray("0x0123456789ABCDEF"));
identity.setPGPSigningKey("0xFEDCBA9876543210");
QCOMPARE(identity.pgpSigningKey(), QByteArray("0xFEDCBA9876543210"));
identity.setSMIMEEncryptionKey("0xABCDEF0123456789");
QCOMPARE(identity.smimeEncryptionKey(), QByteArray("0xABCDEF0123456789"));
identity.setSMIMESigningKey("0xFEDCBA9876543210");
QCOMPARE(identity.smimeSigningKey(), QByteArray("0xFEDCBA9876543210"));
identity.setPreferredCryptoMessageFormat(QStringLiteral("PGP"));
QCOMPARE(identity.preferredCryptoMessageFormat(), QStringLiteral("PGP"));
identity.setPrimaryEmailAddress(QStringLiteral("dvratil@kde.org"));
const QStringList aliases = { QStringLiteral("dvratil1@kde.org"), QStringLiteral("example@example.org") };
identity.setEmailAliases(aliases);
QCOMPARE(identity.emailAliases(), aliases);
QVERIFY(identity.matchesEmailAddress(QStringLiteral("dvratil@kde.org")));
QVERIFY(identity.matchesEmailAddress(aliases[0]));
QVERIFY(identity.matchesEmailAddress(aliases[1]));
QCOMPARE(identity.primaryEmailAddress(), QStringLiteral("dvratil@kde.org"));
const auto vcardFile = QStringLiteral("BEGIN:VCARD\n"
"VERSION:2.1\n"
"N:Vrátil;Daniel;;\n"
"END:VCARD");
identity.setVCardFile(vcardFile);
QCOMPARE(identity.vCardFile(), vcardFile);
identity.setReplyToAddr(QStringLiteral("dvratil+reply@kde.org"));
QCOMPARE(identity.replyToAddr(), QStringLiteral("dvratil+reply@kde.org"));
identity.setBcc(QStringLiteral("dvratil+bcc@kde.org"));
QCOMPARE(identity.bcc(), QStringLiteral("dvratil+bcc@kde.org"));
identity.setCc(QStringLiteral("dvratil+cc@kde.org"));
QCOMPARE(identity.cc(), QStringLiteral("dvratil+cc@kde.org"));
identity.setAttachVcard(true);
QCOMPARE(identity.attachVcard(), true);
identity.setAutocorrectionLanguage(QStringLiteral("cs_CZ"));
QCOMPARE(identity.autocorrectionLanguage(), QStringLiteral("cs_CZ"));
identity.setDisabledFcc(true);
QCOMPARE(identity.disabledFcc(), true);
identity.setPgpAutoSign(true);
QCOMPARE(identity.pgpAutoSign(), true);
identity.setPgpAutoEncrypt(true);
QCOMPARE(identity.pgpAutoEncrypt(), true);
identity.setDefaultDomainName(QStringLiteral("kde.org"));
QCOMPARE(identity.defaultDomainName(), QStringLiteral("kde.org"));
Signature sig;
sig.setEnabledSignature(true);
sig.setText(QStringLiteral("Regards,\nDaniel"));
identity.setSignature(sig);
QCOMPARE(identity.signature(), sig);
identity.setTransport(QStringLiteral("smtp"));
QCOMPARE(identity.transport(), QStringLiteral("smtp"));
identity.setFcc(QStringLiteral("123")); // must be an Akonadi::Collection::Id
QCOMPARE(identity.fcc(), QStringLiteral("123"));
identity.setDrafts(QStringLiteral("124"));
QCOMPARE(identity.drafts(), QStringLiteral("124"));
identity.setTemplates(QStringLiteral("125"));
QCOMPARE(identity.templates(), QStringLiteral("125"));
identity.setDictionary(QStringLiteral("Čeština"));
QCOMPARE(identity.dictionary(), QStringLiteral("Čeština"));
identity.setXFaceEnabled(true);
QCOMPARE(identity.isXFaceEnabled(), true);
identity.setXFace(QStringLiteral(":-P"));
QCOMPARE(identity.xface(), QStringLiteral(":-P"));
// Test copy
{
const Identity copy = identity;
QCOMPARE(copy, identity);
// Test that the operator==() actually works
QVERIFY(compareIdentities(copy, identity));
identity.setXFace(QStringLiteral(":-("));
QVERIFY(!(copy == identity));
}
// Test serialization
{
QByteArray ba;
QDataStream inStream(&ba, QIODevice::WriteOnly);
inStream << identity;
Identity copy;
QDataStream outStream(&ba, QIODevice::ReadOnly);
outStream >> copy;
// We already verified that operator==() works, so this should be enough
QVERIFY(compareIdentities(copy, identity));
}
}
void IdentityTester::test_NullIdentity()
{
IdentityManager manager;
......@@ -82,20 +230,15 @@ void IdentityTester::test_Aliases()
void IdentityTester::test_toMimeData()
{
IdentityManager manager;
Identity &identity = manager.newFromScratch(QStringLiteral("Test1"));
Identity identity(QStringLiteral("Test1"));
identity.setFullName(QStringLiteral("name"));
QMimeData mimeData;
identity.populateMimeData(&mimeData);
Identity identity2 = Identity::fromMimeData(&mimeData);
// The deserializer fills in the QHash will lots of invalid variants, which is OK, but the CTOR doesn't fill
// The hash with the missing fields, so this comparison will fail. Maybe do a smarter Identity== where missing
// from the hash or being invalid is equivalent
QEXPECT_FAIL("", "The deserialized instance is different", Continue);
QVERIFY(compareIdentities(identity, identity2));
QCOMPARE(identity, identity2);
QEXPECT_FAIL("", "Missing qRegisterMetaTypeStreamOperators() I guess ?", Continue);
QCOMPARE(identity.fullName(), identity2.fullName());
}
......@@ -22,12 +22,21 @@
#include <qobject.h>
namespace KIdentityManagement {
class Identity;
}
class IdentityTester : public QObject
{
Q_OBJECT
private:
bool compareIdentities(const KIdentityManagement::Identity &actual,
const KIdentityManagement::Identity &expected);
private Q_SLOTS:
void initTestCase();
void test_Identity();
void test_NullIdentity();
void test_Aliases();
void test_toMimeData();
......
......@@ -34,11 +34,16 @@ using namespace KIdentityManagement;
// TODO: should use a kstaticdeleter?
static Identity *identityNull = Q_NULLPTR;
Q_DECLARE_METATYPE(KIdentityManagement::Signature)
Identity::Identity(const QString &id, const QString &fullName,
const QString &emailAddr, const QString &organization,
const QString &replyToAddr)
: mIsDefault(false)
{
qRegisterMetaType<Signature>();
qRegisterMetaTypeStreamOperators<Signature>();
setProperty(QLatin1String(s_uoid), 0);
setProperty(QLatin1String(s_identity), id);
setProperty(QLatin1String(s_name), fullName);
......@@ -163,33 +168,34 @@ QDataStream &KIdentityManagement::operator<<
(QDataStream &stream, const KIdentityManagement::Identity &i)
{
return stream << static_cast<quint32>(i.uoid())
<< i.identityName()
<< i.fullName()
<< i.organization()
<< i.pgpSigningKey()
<< i.pgpEncryptionKey()
<< i.smimeSigningKey()
<< i.smimeEncryptionKey()
<< i.primaryEmailAddress()
<< i.emailAliases()
<< i.replyToAddr()
<< i.bcc()
<< i.vCardFile()
<< i.transport()
<< i.fcc()
<< i.drafts()
<< i.templates()
<< i.mPropertiesMap[QLatin1String(s_signature)]
<< i.dictionary()
<< i.xface()
<< i.preferredCryptoMessageFormat()
<< i.cc()
<< i.attachVcard()
<< i.autocorrectionLanguage()
<< i.disabledFcc()
<< i.pgpAutoSign()
<< i.pgpAutoEncrypt()
<< i.defaultDomainName();
<< i.mPropertiesMap[QLatin1String(s_identity)]
<< i.mPropertiesMap[QLatin1String(s_name)]
<< i.mPropertiesMap[QLatin1String(s_organization)]
<< i.mPropertiesMap[QLatin1String(s_pgps)]
<< i.mPropertiesMap[QLatin1String(s_pgpe)]
<< i.mPropertiesMap[QLatin1String(s_smimes)]
<< i.mPropertiesMap[QLatin1String(s_smimee)]
<< i.mPropertiesMap[QLatin1String(s_primaryEmail)]
<< i.mPropertiesMap[QLatin1String(s_emailAliases)]
<< i.mPropertiesMap[QLatin1String(s_replyto)]
<< i.mPropertiesMap[QLatin1String(s_bcc)]
<< i.mPropertiesMap[QLatin1String(s_vcard)]
<< i.mPropertiesMap[QLatin1String(s_transport)]
<< i.mPropertiesMap[QLatin1String(s_fcc)]
<< i.mPropertiesMap[QLatin1String(s_drafts)]
<< i.mPropertiesMap[QLatin1String(s_templates)]
<< i.mSignature
<< i.mPropertiesMap[QLatin1String(s_dict)]
<< i.mPropertiesMap[QLatin1String(s_xface)]
<< i.mPropertiesMap[QLatin1String(s_xfaceenabled)]
<< i.mPropertiesMap[QLatin1String(s_prefcrypt)]
<< i.mPropertiesMap[QLatin1String(s_cc)]
<< i.mPropertiesMap[QLatin1String(s_attachVcard)]
<< i.mPropertiesMap[QLatin1String(s_autocorrectionLanguage)]
<< i.mPropertiesMap[QLatin1String(s_disabledFcc)]
<< i.mPropertiesMap[QLatin1String(s_pgpautosign)]
<< i.mPropertiesMap[QLatin1String(s_pgpautoencrypt)]
<< i.mPropertiesMap[QLatin1String(s_defaultDomainName)];
}
QDataStream &KIdentityManagement::operator>>
......@@ -214,9 +220,10 @@ QDataStream &KIdentityManagement::operator>>
>> i.mPropertiesMap[QLatin1String(s_fcc)]
>> i.mPropertiesMap[QLatin1String(s_drafts)]
>> i.mPropertiesMap[QLatin1String(s_templates)]
>> i.mPropertiesMap[QLatin1String(s_signature)]
>> i.mSignature
>> i.mPropertiesMap[QLatin1String(s_dict)]
>> i.mPropertiesMap[QLatin1String(s_xface)]
>> i.mPropertiesMap[QLatin1String(s_xfaceenabled)]
>> i.mPropertiesMap[QLatin1String(s_prefcrypt)]
>> i.mPropertiesMap[QLatin1String(s_cc)]
>> i.mPropertiesMap[QLatin1String(s_attachVcard)]
......@@ -264,8 +271,32 @@ bool Identity::operator>= (const Identity &other) const
bool Identity::operator== (const Identity &other) const
{
return mPropertiesMap == other.mPropertiesMap &&
mSignature == other.mSignature;
// The deserializer fills in the QHash will lots of invalid variants, which
// is OK, but the CTOR doesn't fill the hash with the missing fields, so
// regular mPropertiesMap == other.mPropertiesMap comparison will fail.
// This algo considers both maps equal even if one map does not contain the
// key and the other one contains the key but with an invalid value
for (const auto &pair : { qMakePair(mPropertiesMap, other.mPropertiesMap),
qMakePair(other.mPropertiesMap, mPropertiesMap) }) {
const auto lhs = pair.first;
const auto rhs = pair.second;
for (auto lhsIt = lhs.constBegin(), lhsEnd = lhs.constEnd(); lhsIt != lhsEnd; ++lhsIt) {
const auto rhsIt = rhs.constFind(lhsIt.key());
// Does the other map contain the key?
if (rhsIt == rhs.constEnd()) {
// It does not, so check if our value is invalid, if yes, consider it
// equal to not present and continue
if (lhsIt->isValid()) {
return false;
}
} else if (lhsIt.value() != rhsIt.value()) {
// Both maps have the key, but different value -> different maps
return false;
}
}
}
return mSignature == other.mSignature;
}
bool Identity::operator!= (const Identity &other) const
......@@ -277,7 +308,11 @@ bool Identity::operator!= (const Identity &other) const
QVariant Identity::property(const QString &key) const
{
return mPropertiesMap.value(key);
if (key == QLatin1String(s_signature)) {
return QVariant::fromValue(mSignature);
} else {
return mPropertiesMap.value(key);
}
}
QString Identity::fullEmailAddr(void) const
......@@ -493,11 +528,15 @@ QString Identity::autocorrectionLanguage() const
void Identity::setProperty(const QString &key, const QVariant &value)
{
if (value.isNull() ||
(value.type() == QVariant::String && value.toString().isEmpty())) {
mPropertiesMap.remove(key);
if (key == QLatin1String(s_signature)) {
mSignature = value.value<Signature>();
} else {
mPropertiesMap.insert(key, value);
if (value.isNull() ||
(value.type() == QVariant::String && value.toString().isEmpty())) {
mPropertiesMap.remove(key);
} else {
mPropertiesMap.insert(key, value);
}
}
}
......
Supports Markdown
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