Skip to content

Commit

Permalink
[qtcontacts-sqlite] Properly emit changes to display label groups. Co…
Browse files Browse the repository at this point in the history
…ntributes to JB#45836

Previously we emitted asynchronously before the transaction was
committed.  This commit ensures that we emit only if the transaction
is committed successfully by the writer thread, ensuring that there
is no race between readers attempting to access the updated
display label groups list, and the completion of the write.
  • Loading branch information
Chris Adams committed May 16, 2019
1 parent 737cd10 commit a283882
Show file tree
Hide file tree
Showing 7 changed files with 22 additions and 8 deletions.
6 changes: 6 additions & 0 deletions src/engine/contactnotifier.cpp
Expand Up @@ -160,6 +160,12 @@ void ContactNotifier::relationshipsRemoved(const QSet<QContactId> &contactIds)
}
}

void ContactNotifier::displayLabelGroupsChanged()
{
QDBusMessage message = createSignal("displayLabelGroupsChanged", m_nonprivileged);
QDBusConnection::sessionBus().send(message);
}

bool ContactNotifier::connect(const char *name, const char *signature, QObject *receiver, const char *slot)
{
static QDBusConnection connection(QDBusConnection::sessionBus());
Expand Down
1 change: 1 addition & 0 deletions src/engine/contactnotifier.h
Expand Up @@ -55,6 +55,7 @@ class ContactNotifier
void selfContactIdChanged(QContactId oldId, QContactId newId);
void relationshipsAdded(const QSet<QContactId> &contactIds);
void relationshipsRemoved(const QSet<QContactId> &contactIds);
void displayLabelGroupsChanged();

bool connect(const char *name, const char *signature, QObject *receiver, const char *slot);
};
Expand Down
11 changes: 5 additions & 6 deletions src/engine/contactsdatabase.cpp
Expand Up @@ -1924,6 +1924,7 @@ static bool executeDisplayLabelGroupLocalizationStatements(QSqlDatabase &databas
}

// for every single contact in our database, read the data required to generate the display label group data.
bool emitDisplayLabelGroupChange = false;
QVariantList contactIds;
QVariantList displayLabelGroups;
QVariantList displayLabelGroupSortOrders;
Expand Down Expand Up @@ -1959,7 +1960,7 @@ static bool executeDisplayLabelGroupLocalizationStatements(QSqlDatabase &databas
c.saveDetail(&n);
c.saveDetail(&dl);

const QString dlg = cdb->determineDisplayLabelGroup(c);
const QString dlg = cdb->determineDisplayLabelGroup(c, &emitDisplayLabelGroupChange);
displayLabelGroups.append(dlg);
displayLabelGroupSortOrders.append(cdb->displayLabelGroupSortValue(dlg));
}
Expand Down Expand Up @@ -3473,7 +3474,7 @@ QString ContactsDatabase::displayLabelGroupPreferredProperty() const
return retn;
}

QString ContactsDatabase::determineDisplayLabelGroup(const QContact &c)
QString ContactsDatabase::determineDisplayLabelGroup(const QContact &c, bool *emitDisplayLabelGroupChange)
{
// Read system setting to determine whether display label group
// should be generated from last name, first name, or display label.
Expand Down Expand Up @@ -3532,19 +3533,17 @@ QString ContactsDatabase::determineDisplayLabelGroup(const QContact &c)
}
}

if (!group.isEmpty() && !m_knownDisplayLabelGroupsSortValues.contains(group)) {
if (emitDisplayLabelGroupChange && !group.isEmpty() && !m_knownDisplayLabelGroupsSortValues.contains(group)) {
// We are about to write a contact to the database which has a
// display label group which previously was not known / observed.
// Calculate the sort value for the display label group,
// and add it to our map of displayLabelGroup->sortValue.
// Note: this should be thread-safe since we only call this method within writes.
*emitDisplayLabelGroupChange = true;
m_knownDisplayLabelGroupsSortValues.insert(
group, ::displayLabelGroupSortValue(
group,
m_knownDisplayLabelGroupsSortValues));

// and invoke engine->_q_displayLabelGroupsChanged() asynchronously.
QMetaObject::invokeMethod(m_engine, "_q_displayLabelGroupsChanged", Qt::QueuedConnection);
}

return group;
Expand Down
2 changes: 1 addition & 1 deletion src/engine/contactsdatabase.h
Expand Up @@ -159,7 +159,7 @@ class ContactsDatabase

void regenerateDisplayLabelGroups();
QString displayLabelGroupPreferredProperty() const;
QString determineDisplayLabelGroup(const QContact &c);
QString determineDisplayLabelGroup(const QContact &c, bool *emitDisplayLabelGroupChange = Q_NULLPTR);
QStringList displayLabelGroups() const;
int displayLabelGroupSortValue(const QString &group) const;

Expand Down
1 change: 1 addition & 0 deletions src/engine/contactsengine.cpp
Expand Up @@ -898,6 +898,7 @@ QContactManager::Error ContactsEngine::open()
m_notifier->connect("selfContactIdChanged", "uu", this, SLOT(_q_selfContactIdChanged(quint32,quint32)));
m_notifier->connect("relationshipsAdded", "au", this, SLOT(_q_relationshipsAdded(QVector<quint32>)));
m_notifier->connect("relationshipsRemoved", "au", this, SLOT(_q_relationshipsRemoved(QVector<quint32>)));
m_notifier->connect("displayLabelGroupsChanged", "", this, SLOT(_q_displayLabelGroupsChanged()));
}
} else {
QTCONTACTS_SQLITE_WARNING(QString::fromLatin1("Unable to open asynchronous engine database connection"));
Expand Down
8 changes: 7 additions & 1 deletion src/engine/contactwriter.cpp
Expand Up @@ -139,6 +139,7 @@ ContactWriter::ContactWriter(ContactsEngine &engine, ContactsDatabase &database,
, m_database(database)
, m_notifier(notifier)
, m_reader(reader)
, m_displayLabelGroupsChanged(false)
{
Q_ASSERT(notifier);
Q_ASSERT(reader);
Expand Down Expand Up @@ -177,6 +178,10 @@ bool ContactWriter::commitTransaction()
return false;
}

if (m_displayLabelGroupsChanged) {
m_notifier->displayLabelGroupsChanged();
m_displayLabelGroupsChanged = false;
}
if (!m_addedIds.isEmpty()) {
m_notifier->contactsAdded(m_addedIds.toList());
m_addedIds.clear();
Expand Down Expand Up @@ -218,6 +223,7 @@ void ContactWriter::rollbackTransaction()
m_presenceChangedIds.clear();
m_changedIds.clear();
m_addedIds.clear();
m_displayLabelGroupsChanged = false;
}

QContactManager::Error ContactWriter::setIdentity(ContactsDatabase::Identity identity, QContactId contactId)
Expand Down Expand Up @@ -5591,7 +5597,7 @@ ContactsDatabase::Query ContactWriter::bindContactDetails(const QContact &contac
QContactDisplayLabel label = contact.detail<QContactDisplayLabel>();
const QString displayLabel = label.label().trimmed();
query.bindValue(0, displayLabel);
const QString displayLabelGroup = m_database.determineDisplayLabelGroup(contact);
const QString displayLabelGroup = m_database.determineDisplayLabelGroup(contact, &m_displayLabelGroupsChanged);
query.bindValue(1, displayLabelGroup);
const int displayLabelGroupSortOrder = m_database.displayLabelGroupSortValue(displayLabelGroup);
query.bindValue(2, displayLabelGroupSortOrder);
Expand Down
1 change: 1 addition & 0 deletions src/engine/contactwriter.h
Expand Up @@ -172,6 +172,7 @@ class ContactWriter
ContactNotifier *m_notifier;
ContactReader *m_reader;

bool m_displayLabelGroupsChanged;
QSet<QContactId> m_addedIds;
QSet<QContactId> m_removedIds;
QSet<QContactId> m_changedIds;
Expand Down

0 comments on commit a283882

Please sign in to comment.