summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDaniel Vrátil <dvratil@kde.org>2016-07-22 09:08:20 (GMT)
committerDaniel Vrátil <dvratil@kde.org>2016-07-22 12:26:11 (GMT)
commit2c4cf6d53c4e86d26b5409c137c6caab87c4d426 (patch)
treefc6a40c404325b0ad2fdf7a73b58c9c8326ff06b
parent5fc9962412a0f3d7475dde4b99391cef60a5f44f (diff)
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
-rw-r--r--autotests/identitytest.cpp157
-rw-r--r--autotests/identitytest.h9
-rw-r--r--src/identity.cpp109
3 files changed, 233 insertions, 42 deletions
diff --git a/autotests/identitytest.cpp b/autotests/identitytest.cpp
index a35b042..85a661d 100644
--- a/autotests/identitytest.cpp
+++ b/autotests/identitytest.cpp
@@ -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());
}
diff --git a/autotests/identitytest.h b/autotests/identitytest.h
index 6ca1a57..8710fc2 100644
--- a/autotests/identitytest.h
+++ b/autotests/identitytest.h
@@ -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();
diff --git a/src/identity.cpp b/src/identity.cpp
index 6e002c3..367fb49 100644
--- a/src/identity.cpp
+++ b/src/identity.cpp
@@ -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);
+ }
}
}