Fix bugs in ID handling and aggregation

src/engine/contactwriter.cpp

@@ -4203,22 +4203,27 @@ QContactManager::Error ContactWriter::regenerateAggregates(const QList<quint32>
             }
         }
 
-        // Step two: search for the "local" contact and promote its details first
+        // Step two: search for the "local" contacts and promote their details first
+        bool foundFirstLocal = false;
         for (int i = 1; i < readList.size(); ++i) { // start from 1 to skip aggregate
             QContact curr = readList.at(i);
             if (curr.details<QContactDeactivated>().count())
                 continue;
             if (ContactCollectionId::databaseId(curr.collectionId()) != ContactsDatabase::LocalAddressbookCollectionId)
                 continue;
-            QList<QContactDetail> currDetails = curr.details();
-            for (int j = 0; j < currDetails.size(); ++j) {
-                QContactDetail currDet = currDetails.at(j);
-                if (promoteDetailType(currDet.type(), definitionMask, false)) {
-                    // promote this detail to the aggregate.
-                    aggregateContact.saveDetail(&currDet, QContact::IgnoreAccessConstraints);
+            if (!foundFirstLocal) {
+                foundFirstLocal = true;
+                const QList<QContactDetail> currDetails = curr.details();
+                for (int j = 0; j < currDetails.size(); ++j) {
+                    QContactDetail currDet = currDetails.at(j);
+                    if (promoteDetailType(currDet.type(), definitionMask, false)) {
+                        // unconditionally promote this detail to the aggregate.
+                        aggregateContact.saveDetail(&currDet, QContact::IgnoreAccessConstraints);
+                    }
                 }
+            } else {
+                promoteDetailsToAggregate(curr, &aggregateContact, definitionMask, false);
             }
-            break; // we've successfully promoted the local contact's details to the aggregate.
         }
 
         // Step Three: promote data from details of other related contacts

src/extensions/qtcontacts-extensions.h

@@ -106,7 +106,7 @@ namespace QtContactsSqliteExtensions {
 
 QTCONTACTS_USE_NAMESPACE
 
-QContactId apiContactId(quint32);
+QContactId apiContactId(quint32, const QString &managerUri);
 quint32 internalContactId(const QContactId &);
 
 enum NormalizePhoneNumberFlag {

src/extensions/qtcontacts-extensions_impl.h

@@ -1,5 +1,6 @@
 /*
- * Copyright (C) 2013 Jolla Ltd. <matthew.vogt@jollamobile.com>
+ * Copyright (C) 2013 - 2014 Jolla Ltd.
+ * Copyright (C) 2019 - 2020 Open Mobile Platform LLC.
  *
  * You may use this file under the terms of the BSD license as follows:
  *
@@ -177,27 +178,16 @@ QString normalize(const QString &input, int flags, int maxCharacters)
 
 namespace QtContactsSqliteExtensions {
 
-QContactId apiContactId(quint32 iid)
+QContactId apiContactId(quint32 iid, const QString &managerUri)
 {
-    QContactId contactId;
-    if (iid != 0) {
-        static const QString idStr(QString::fromLatin1("qtcontacts:org.nemomobile.contacts.sqlite::sql-%1"));
-        contactId = QContactId::fromString(idStr.arg(iid));
-        if (contactId.isNull()) {
-            qWarning() << "Unable to formulate valid ID from:" << iid;
-        }
-    }
-    return contactId;
+    return QContactId(managerUri, QByteArrayLiteral("sql-") + QByteArray::number(iid));
 }
 
 quint32 internalContactId(const QContactId &id)
 {
-    if (!id.isNull()) {
-        QStringList components = id.toString().split(QChar::fromLatin1(':'));
-        const QString &idComponent = components.isEmpty() ? QString() : components.last();
-        if (idComponent.startsWith(QString::fromLatin1("sql-"))) {
-            return idComponent.mid(4).toUInt();
-        }
+    const QByteArray localId = id.localId();
+    if (localId.startsWith(QByteArrayLiteral("sql-"))) {
+        return localId.mid(4).toUInt();
     }
     return 0;
 }

tests/auto/aggregation/tst_aggregation.cpp

@@ -2481,6 +2481,116 @@ void tst_Aggregation::alterRelationships()
     QCOMPARE(aggregateAlice.details<QContactPhoneNumber>().size(), 1);
     QCOMPARE(aggregateAlice.detail<QContactPhoneNumber>().number(), localAlice.detail<QContactPhoneNumber>().number());
     QCOMPARE(aggregateAlice.details<QContactEmailAddress>().size(), 0);
+
+    // finally, create two new local addresbook contacts,
+    // which we will later manually aggregate together.
+    QContact localEdmond;
+    QContactName en;
+    en.setMiddleName("Edmond");
+    en.setFirstName("test");
+    en.setLastName("alterRelationshipsLocal");
+    localEdmond.saveDetail(&en);
+    QContactPhoneNumber ep;
+    ep.setNumber("8765432");
+    ep.setSubTypes(QList<int>() << QContactPhoneNumber::SubTypeMobile);
+    ep.setDetailUri("edmond-alterRelationships-phone");
+    localEdmond.saveDetail(&ep);
+    QContactHobby eh;
+    eh.setHobby("Surfing");
+    localEdmond.saveDetail(&eh);
+    QContactGuid eg;
+    eg.setGuid("alterRelationships-Edmond");
+    localEdmond.saveDetail(&eg);
+
+    QContact localFred;
+    QContactName fn;
+    fn.setMiddleName("Fred");
+    fn.setFirstName("trial");
+    fn.setLastName("alterRelationshipsLocal");
+    localFred.saveDetail(&fn);
+    QContactPhoneNumber fp;
+    fp.setNumber("9876543");
+    fp.setSubTypes(QList<int>() << QContactPhoneNumber::SubTypeMobile);
+    fp.setDetailUri("fred-alterRelationships-phone");
+    localFred.saveDetail(&fp);
+    QContactHobby fh;
+    fh.setHobby("Bowling");
+    localFred.saveDetail(&fh);
+    QContactGuid fg;
+    fg.setGuid("alterRelationships-Fred");
+    localFred.saveDetail(&fg);
+
+    QVERIFY(m_cm->saveContact(&localEdmond));
+    QVERIFY(m_cm->saveContact(&localFred));
+
+    // fetch the contacts, ensure we have what we expect.
+    QContact aggregateEdmond, aggregateFred;
+    allContacts = m_cm->contacts(allCollections);
+    foreach (const QContact &curr, allContacts) {
+        QContactName currName = curr.detail<QContactName>();
+        if (currName.middleName() == QLatin1String("Alice") && currName.lastName() == QLatin1String("alterRelationships")) {
+            if (curr.collectionId() == testAddressbook.id()) {
+                localAlice = curr;
+            } else {
+                QCOMPARE(curr.collectionId().localId(), aggregateAddressbookId());
+                aggregateAlice = curr;
+            }
+        } else if (currName.middleName() == QLatin1String("Bob") && currName.lastName() == QLatin1String("alterRelationships")) {
+            if (curr.collectionId() == trialAddressbook.id()) {
+                localBob = curr;
+            } else {
+                QCOMPARE(curr.collectionId().localId(), aggregateAddressbookId());
+                aggregateBob = curr;
+            }
+        } else if (currName.middleName() == QLatin1String("Edmond") && currName.lastName() == QLatin1String("alterRelationshipsLocal")) {
+            if (curr.collectionId().localId() == localAddressbookId()) {
+                localEdmond = curr;
+            } else {
+                QCOMPARE(curr.collectionId().localId(), aggregateAddressbookId());
+                aggregateEdmond = curr;
+            }
+        } else if (currName.middleName() == QLatin1String("Fred") && currName.lastName() == QLatin1String("alterRelationshipsLocal")) {
+            if (curr.collectionId().localId() == localAddressbookId()) {
+                localFred = curr;
+            } else {
+                QCOMPARE(curr.collectionId().localId(), aggregateAddressbookId());
+                aggregateFred = curr;
+            }
+        }
+    }
+
+    QCOMPARE(localEdmond.collectionId().localId(), localAddressbookId());
+    QCOMPARE(localFred.collectionId().localId(), localAddressbookId());
+    QCOMPARE(aggregateEdmond.collectionId().localId(), aggregateAddressbookId());
+    QCOMPARE(aggregateFred.collectionId().localId(), aggregateAddressbookId());
+
+    remSpyCount = remSpy.count();
+
+    // Aggregate localFred into aggregateEdmond
+    relationship = makeRelationship(QContactRelationship::Aggregates, aggregateEdmond.id(), localFred.id());
+    QVERIFY(m_cm->saveRelationship(&relationship));
+
+    // Remove the relationship between localFred and aggregateFred
+    relationship = makeRelationship(QContactRelationship::Aggregates, aggregateFred.id(), localFred.id());
+    QVERIFY(m_cm->removeRelationship(relationship));
+
+    // The childless aggregate should have been removed
+    QTRY_VERIFY(remSpy.count() > remSpyCount);
+    QVERIFY(m_remAccumulatedIds.contains(ContactId::apiId(aggregateFred)));
+    remSpyCount = remSpy.count();
+
+    // Reload the aggregateEdmond, and ensure that it has the required details.
+    aggregateEdmond = m_cm->contact(aggregateEdmond.id());
+    QCOMPARE(aggregateEdmond.details<QContactPhoneNumber>().size(), 2);
+    QCOMPARE(aggregateEdmond.details<QContactHobby>().size(), 2);
+    QVERIFY((aggregateEdmond.details<QContactPhoneNumber>().at(0).number() == ep.number()
+                || aggregateEdmond.details<QContactPhoneNumber>().at(1).number() == ep.number())
+            && (aggregateEdmond.details<QContactPhoneNumber>().at(0).number() == fp.number()
+                || aggregateEdmond.details<QContactPhoneNumber>().at(1).number() == fp.number()));
+    QVERIFY((aggregateEdmond.details<QContactHobby>().at(0).hobby() == eh.hobby()
+                || aggregateEdmond.details<QContactHobby>().at(1).hobby() == eh.hobby())
+            && (aggregateEdmond.details<QContactHobby>().at(0).hobby() == fh.hobby()
+                || aggregateEdmond.details<QContactHobby>().at(1).hobby() == fh.hobby()));
 }
 
 void tst_Aggregation::aggregationHeuristic_data()

tests/auto/qcontactmanagerfiltering/tst_qcontactmanagerfiltering.cpp

@@ -2514,6 +2514,17 @@ void tst_QContactManagerFiltering::allFiltering()
     QList<QContactId> contacts = contactsAddedToManagers.values(cm);
     QContactFilter f; // default = permissive
     QList<QContactId> ids = cm->contactIds(f);
+
+    // Remove the "self contact" (aggregate and local) if required
+    if (cm->managerUri().contains(QStringLiteral("org.nemomobile.contacts.sqlite"))) {
+        for (int i = ids.size() - 1; i >= 0; --i) {
+            if (ids[i].localId() == QByteArrayLiteral("sql-1")
+                    || ids[i].localId() == QByteArrayLiteral("sql-2")) {
+                ids.removeAt(i);
+            }
+        }
+    }
+
     QCOMPARE(ids.count(), contacts.size());
     QString output = convertIds(contacts, ids, 'a', 'k'); // don't include the convenience filtering contacts
     QString expected = convertIds(contacts, contacts, 'a', 'k'); // :)