Skip to content

Commit

Permalink
[qtcontacts-sqlite] Improve handling of multiple sync target constitu…
Browse files Browse the repository at this point in the history 
…ents. Contributes to MER#1053

Previously, the syncUpdate() function built a mapping from the
modified constituent to the appropriate sync target constituent,
in order to affect the appropriate sync target constituent instead
of the modified constituent.

The function previously did not consider the possibility that
multiple sync target constituents might be aggregated by a single
aggregate contact, and thus the mapping was incomplete.

This commit improves the mapping so that a modification to a
sync target constituent will not be re-directed to modify a
different sync target constituent (in the case where multiple
STCs exist for that contact).

It also improves the unit test to ensure that the state of the
server-side contact cache managed by the test sync adapter is
always consistent.

Contributes to MER#1053
  • Loading branch information
Chris Adams committed Jun 2, 2015
1 parent 03e406f commit e497b56
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 24 deletions.
47 changes: 38 additions & 9 deletions src/engine/contactwriter.cpp
View file
Expand Up @@ -4607,7 +4607,6 @@ QContactManager::Error ContactWriter::syncUpdate(const QString &syncTarget,
}

m_database.clearTemporaryContactIdsTable(syncConstituentsTable);

if (!m_database.createTemporaryContactIdsTable(syncConstituentsTable, ids)) {
QTCONTACTS_SQLITE_WARNING(QString::fromLatin1("Error populating syncConstituents temporary table"));
return QContactManager::UnspecifiedError;
Expand All @@ -4631,19 +4630,50 @@ QContactManager::Error ContactWriter::syncUpdate(const QString &syncTarget,
query.reportError("Failed to fetch local constituent ids for sync update");
return QContactManager::UnspecifiedError;
}

QSet<quint32> allStcIds;
QMultiHash<quint32, quint32> modifiedToStcId;
QHash<quint32, quint32> aggregateForStcId;
while (query.next()) {
const quint32 stConstituentId = query.value<quint32>(0);
const quint32 modifiedConstituentId = query.value<quint32>(1);
const quint32 aggregateId = query.value<quint32>(2);

constituentAggregateIds.insert(modifiedConstituentId, aggregateId);
if (stConstituentId) {
modifiedToStcId.insertMulti(modifiedConstituentId, stConstituentId);
allStcIds.insert(stConstituentId);
aggregateForStcId.insert(stConstituentId, aggregateId);
}
}

if (stConstituentId && (stConstituentId != modifiedConstituentId)) {
// We may need to modify the sync target constituent also
stConstituents.insert(modifiedConstituentId, stConstituentId);
affectedContactIds.insert(stConstituentId);

constituentAggregateIds.insert(stConstituentId, aggregateId);
Q_FOREACH (quint32 modifiedId, modifiedToStcId.keys()) {
if (allStcIds.contains(modifiedId)) {
// this modified constituent is also a sync target constituent.
// we don't need to create a mapping.
} else {
// this modified constituent is NOT a sync target constituent.
// find an appropriate sync target constituent for this modified contact.
QList<quint32> stcIds = modifiedToStcId.values(modifiedId);
quint32 mappedStc = 0;
Q_FOREACH (quint32 stcId, stcIds) {
if (modifiedToStcId.contains(stcId)) {
// this stc was also modified. Fall back to this if no others exist.
mappedStc = stcId;
} else {
// unmodified stc. Use this if possible, since other modifications
// might involve removal (so mapped changes would in that case fail).
mappedStc = stcId;
break;
}
}
// use the found sync target contact in our mapping.
if (!mappedStc) {
QTCONTACTS_SQLITE_WARNING(QString::fromLatin1("No sync target constituent found for modified contact:") << modifiedId);
} else {
stConstituents.insert(modifiedId, mappedStc);
affectedContactIds.insert(mappedStc);
constituentAggregateIds.insert(mappedStc, aggregateForStcId[mappedStc]);
}
}
}
}
Expand Down Expand Up @@ -5091,7 +5121,6 @@ QContactManager::Error ContactWriter::syncUpdate(const QString &syncTarget,
// add those ids to the change signal accumulator
foreach (const QContactId &removedAggId, removedAggregateIds) {
m_removedIds.insert(removedAggId);

const quint32 aggId(ContactId::databaseId(removedAggId));
aggregatesOfRemoved.removeAll(aggId);
}
Expand Down
2 changes: 1 addition & 1 deletion src/extensions/twowaycontactsyncadapter_impl.h
View file
Expand Up @@ -725,7 +725,7 @@ QList<QPair<QContact, QContact> > TwoWayContactSyncAdapterPrivate::createUpdateL
QTCONTACTS_SQLITE_TWCSA_DEBUG_LOG("contact:" << pcGuid << "was never seen locally but reported as deleted remotely...");
}
} else {
QTCONTACTS_SQLITE_TWCSA_DEBUG_LOG("contact:" << prevContact.detail<QContactGuid>().guid() << "marked as deleted remotely, found at index" << prevIndex);
QTCONTACTS_SQLITE_TWCSA_DEBUG_LOG("contact:" << prevContact.detail<QContactGuid>().guid() << "with id" << id << "marked as deleted remotely, found at index" << prevIndex);
deletePositions.append(prevIndex);
syncState.m_remotelyDeletedThisSync.append(id);
}
Expand Down
46 changes: 33 additions & 13 deletions tests/auto/aggregation/testsyncadapter.cpp
View file
Expand Up @@ -84,7 +84,7 @@ void TestSyncAdapter::mergeRemoteDuplicates(const QString &accountId)
{
Q_FOREACH (const QString &dupGuid, m_remoteServerDuplicates[accountId].values()) {
m_remoteAddMods[accountId].remove(dupGuid); // shouldn't be any here anyway.
m_remoteDeletions[accountId].append(m_remoteServerContacts[accountId][dupGuid]);
m_remoteDeletions[accountId].append(m_remoteServerContacts[accountId].value(dupGuid));
m_remoteServerContacts[accountId].remove(dupGuid);
}
m_remoteServerDuplicates[accountId].clear();
Expand Down Expand Up @@ -112,10 +112,12 @@ void TestSyncAdapter::addRemoteContact(const QString &accountId, const QString &
if (m_remoteServerContacts[accountId].contains(contactGuidStr)) {
// this is an intentional duplicate. we have special handling for duplicates.
QString duplicateGuidString = contactGuidStr + ":" + QString::number(m_remoteServerDuplicates[accountId].values(contactGuidStr).size() + 1);
QContactGuid guid; guid.setGuid(duplicateGuidString); newContact.saveDetail(&guid);
m_remoteServerDuplicates[accountId].insert(contactGuidStr, duplicateGuidString);
m_remoteServerContacts[accountId].insert(duplicateGuidString, newContact);
m_remoteAddMods[accountId].insert(duplicateGuidString);
} else {
QContactGuid guid; guid.setGuid(contactGuidStr); newContact.saveDetail(&guid);
m_remoteServerContacts[accountId].insert(contactGuidStr, newContact);
m_remoteAddMods[accountId].insert(contactGuidStr);
}
Expand All @@ -124,7 +126,7 @@ void TestSyncAdapter::addRemoteContact(const QString &accountId, const QString &
void TestSyncAdapter::removeRemoteContact(const QString &accountId, const QString &fname, const QString &lname)
{
const QString contactGuidStr(TSA_GUID_STRING(accountId, fname, lname));
QContact remContact = m_remoteServerContacts[accountId][contactGuidStr];
QContact remContact = m_remoteServerContacts[accountId].value(contactGuidStr);

// stop tracking the contact if we are currently tracking it.
m_remoteAddMods[accountId].remove(contactGuidStr);
Expand All @@ -149,7 +151,7 @@ void TestSyncAdapter::setRemoteContact(const QString &accountId, const QString &
somd.setGroupId(setContact.id().toString());
setContact.saveDetail(&somd);

m_remoteServerContacts[accountId].insert(contactGuidStr, setContact);
m_remoteServerContacts[accountId][contactGuidStr] = setContact;
m_remoteAddMods[accountId].insert(contactGuidStr);
}

Expand All @@ -161,12 +163,12 @@ void TestSyncAdapter::changeRemoteContactPhone(const QString &accountId, const
return;
}

QContact modContact = m_remoteServerContacts[accountId][contactGuidStr];
QContact modContact = m_remoteServerContacts[accountId].value(contactGuidStr);
QContactPhoneNumber mcp = modContact.detail<QContactPhoneNumber>();
mcp.setNumber(modPhone);
modContact.saveDetail(&mcp);

m_remoteServerContacts[accountId].insert(contactGuidStr, modContact);
m_remoteServerContacts[accountId][contactGuidStr] = modContact;
m_remoteAddMods[accountId].insert(contactGuidStr);
}

Expand All @@ -178,12 +180,12 @@ void TestSyncAdapter::changeRemoteContactEmail(const QString &accountId, const
return;
}

QContact modContact = m_remoteServerContacts[accountId][contactGuidStr];
QContact modContact = m_remoteServerContacts[accountId].value(contactGuidStr);
QContactEmailAddress mce = modContact.detail<QContactEmailAddress>();
mce.setEmailAddress(modEmail);
modContact.saveDetail(&mce);

m_remoteServerContacts[accountId].insert(contactGuidStr, modContact);
m_remoteServerContacts[accountId][contactGuidStr] = modContact;
m_remoteAddMods[accountId].insert(contactGuidStr);
}

Expand All @@ -195,7 +197,7 @@ void TestSyncAdapter::changeRemoteContactName(const QString &accountId, const QS
return;
}

QContact modContact = m_remoteServerContacts[accountId][contactGuidStr];
QContact modContact = m_remoteServerContacts[accountId].value(contactGuidStr);
QContactName mcn = modContact.detail<QContactName>();
if (modfname.isEmpty() && modlname.isEmpty()) {
modContact.removeDetail(&mcn);
Expand All @@ -205,8 +207,11 @@ void TestSyncAdapter::changeRemoteContactName(const QString &accountId, const QS
modContact.saveDetail(&mcn);
}

m_remoteServerContacts[accountId].insert(contactGuidStr, modContact);
m_remoteAddMods[accountId].insert(contactGuidStr);
const QString modContactGuidStr(TSA_GUID_STRING(accountId, modfname, modlname));
m_remoteServerContacts[accountId].remove(contactGuidStr);
m_remoteAddMods[accountId].remove(contactGuidStr);
m_remoteServerContacts[accountId][modContactGuidStr] = modContact;
m_remoteAddMods[accountId].insert(modContactGuidStr);
}

void TestSyncAdapter::performTwoWaySync(const QString &accountId)
Expand Down Expand Up @@ -333,11 +338,26 @@ void TestSyncAdapter::upsyncLocalChanges(const QDateTime &,
const QString &accountId)
{
// first, apply the local changes to our in memory store.
foreach (const QContact &c, locallyAdded + locallyModified) {
foreach (const QContact &c, locallyAdded) {
setRemoteContact(accountId, c.detail<QContactName>().firstName(), c.detail<QContactName>().lastName(), c);
}
foreach (const QContact &c, locallyModified) {
// we cannot simply call setRemoteContact since the name might be modified or empty due to a previous test.
Q_FOREACH (const QString &storedGuid, m_remoteServerContacts[m_accountId].keys()) {
if (m_remoteServerContacts[m_accountId][storedGuid].id() == c.id()) {
m_remoteServerContacts[m_accountId][storedGuid] = c;
m_remoteAddMods[accountId].insert(storedGuid);
}
}
}
foreach (const QContact &c, locallyDeleted) {
removeRemoteContact(accountId, c.detail<QContactName>().firstName(), c.detail<QContactName>().lastName());
// we cannot simply call removeRemoteContact since the name might be modified or empty due to a previous test.
QMap<QString, QContact> remoteServerContacts = m_remoteServerContacts[m_accountId];
Q_FOREACH (const QString &storedGuid, remoteServerContacts.keys()) {
if (remoteServerContacts.value(storedGuid).id() == c.id()) {
m_remoteServerContacts[m_accountId].remove(storedGuid);
}
}
}

// then trigger finalize after a simulated network delay.
Expand Down Expand Up @@ -383,7 +403,7 @@ bool TestSyncAdapter::downsyncWasRequired(const QString &accountId) const
QContact TestSyncAdapter::remoteContact(const QString &accountId, const QString &fname, const QString &lname) const
{
const QString contactGuidStr(TSA_GUID_STRING(accountId, fname, lname));
return m_remoteServerContacts[accountId][contactGuidStr];
return m_remoteServerContacts[accountId].value(contactGuidStr);
}

QSet<QContactId> TestSyncAdapter::modifiedIds(const QString &accountId) const
Expand Down
2 changes: 1 addition & 1 deletion tests/auto/aggregation/tst_aggregation.cpp
View file
Expand Up @@ -6592,7 +6592,7 @@ void tst_Aggregation::testSyncAdapter()
QVERIFY(tsaFourAggId != QContactId());
tsa.performTwoWaySync(accountId);
QTRY_COMPARE(finishedSpy.count(), 7);
QVERIFY(tsa.downsyncWasRequired(accountId));
QVERIFY(!tsa.downsyncWasRequired(accountId)); // there were no remote changes
QVERIFY(tsa.upsyncWasRequired(accountId));
QVERIFY(haveExpectedContent(tsa.remoteContact(accountId, QStringLiteral("Jennifer"), QStringLiteral("TsaFour")),
QString(), TestSyncAdapter::ImplicitlyModifiable, QStringLiteral("jennifer@tsafour.com")));
Expand Down

0 comments on commit e497b56

Please sign in to comment.