summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMichael Goddard <michael.goddard@nokia.com>2011-04-07 17:11:52 +1000
committerMichael Goddard <michael.goddard@nokia.com>2011-04-11 10:43:00 +1000
commitd933fd97230acc119c7d0fbbf5db31b4fb738a86 (patch)
treed682bd707d2300c2fcef6fed1dbf1daf072395ab
parent8ffd5cf183146fc50c18d9248b0d8be827670774 (diff)
Fix up the error handling when the manager calls the engines.
When the engine is called by the manager, it was passed the manager's error state directly, so that it could be modified. But if the engine emitted signals, and slots connected to those themselves called back into the manager, the error state could be overwritten. This is particularly noticeable when the engine does something like "return *error == QContactManager::NoError;" after emitting signals, on the assumption that the error state was still valid. Change-Id: Iaa17ab6471fd91c22f46760e5ae5974a6c0fb487 Task-number: QTMOBILITY-1499 Reviewed-by: Chris Adams Reviewed-by: Xizhi Zhu
-rw-r--r--src/contacts/qcontactmanager.cpp195
-rw-r--r--src/contacts/qcontactmanager_p.cpp10
-rw-r--r--src/contacts/qcontactmanager_p.h37
-rw-r--r--src/organizer/qorganizermanager.cpp129
-rw-r--r--src/organizer/qorganizermanager_p.cpp10
-rw-r--r--src/organizer/qorganizermanager_p.h37
-rw-r--r--tests/auto/qcontactmanager/tst_qcontactmanager.cpp55
-rw-r--r--tests/auto/qorganizermanager/tst_qorganizermanager.cpp66
8 files changed, 323 insertions, 216 deletions
diff --git a/src/contacts/qcontactmanager.cpp b/src/contacts/qcontactmanager.cpp
index 1e156f3ac4..1731ec665e 100644
--- a/src/contacts/qcontactmanager.cpp
+++ b/src/contacts/qcontactmanager.cpp
@@ -418,7 +418,7 @@ Q_DEFINE_LATIN1_CONSTANT(QContactManager::ParameterValueOnlyOtherProcesses, "Onl
*/
QContactManager::Error QContactManager::error() const
{
- return d->m_error;
+ return d->m_lastError;
}
/*!
@@ -432,7 +432,7 @@ QContactManager::Error QContactManager::error() const
*/
QMap<int, QContactManager::Error> QContactManager::errorMap() const
{
- return d->m_errorMap;
+ return d->m_lastErrorMap;
}
/*!
@@ -440,9 +440,8 @@ QMap<int, QContactManager::Error> QContactManager::errorMap() const
*/
QList<QContactLocalId> QContactManager::contactIds(const QList<QContactSortOrder>& sortOrders) const
{
- d->m_error = QContactManager::NoError;
- d->m_errorMap.clear();
- return d->m_engine->contactIds(QContactFilter(), sortOrders, &d->m_error);
+ QContactManagerSyncOpErrorHolder h(this);
+ return d->m_engine->contactIds(QContactFilter(), sortOrders, &h.error);
}
/*!
@@ -451,9 +450,8 @@ QList<QContactLocalId> QContactManager::contactIds(const QList<QContactSortOrder
*/
QList<QContactLocalId> QContactManager::contactIds(const QContactFilter& filter, const QList<QContactSortOrder>& sortOrders) const
{
- d->m_error = QContactManager::NoError;
- d->m_errorMap.clear();
- return d->m_engine->contactIds(filter, sortOrders, &d->m_error);
+ QContactManagerSyncOpErrorHolder h(this);
+ return d->m_engine->contactIds(filter, sortOrders, &h.error);
}
/*!
@@ -469,9 +467,8 @@ QList<QContactLocalId> QContactManager::contactIds(const QContactFilter& filter,
*/
QList<QContact> QContactManager::contacts(const QList<QContactSortOrder>& sortOrders, const QContactFetchHint& fetchHint) const
{
- d->m_error = QContactManager::NoError;
- d->m_errorMap.clear();
- return d->m_engine->contacts(QContactFilter(), sortOrders, fetchHint, &d->m_error);
+ QContactManagerSyncOpErrorHolder h(this);
+ return d->m_engine->contacts(QContactFilter(), sortOrders, fetchHint, &h.error);
}
/*!
@@ -490,9 +487,8 @@ QList<QContact> QContactManager::contacts(const QList<QContactSortOrder>& sortOr
*/
QList<QContact> QContactManager::contacts(const QContactFilter& filter, const QList<QContactSortOrder>& sortOrders, const QContactFetchHint& fetchHint) const
{
- d->m_error = QContactManager::NoError;
- d->m_errorMap.clear();
- return d->m_engine->contacts(filter, sortOrders, fetchHint, &d->m_error);
+ QContactManagerSyncOpErrorHolder h(this);
+ return d->m_engine->contacts(filter, sortOrders, fetchHint, &h.error);
}
/*!
@@ -512,9 +508,8 @@ QList<QContact> QContactManager::contacts(const QContactFilter& filter, const QL
*/
QContact QContactManager::contact(const QContactLocalId& contactId, const QContactFetchHint& fetchHint) const
{
- d->m_error = QContactManager::NoError;
- d->m_errorMap.clear();
- return d->m_engine->contact(contactId, fetchHint, &d->m_error);
+ QContactManagerSyncOpErrorHolder h(this);
+ return d->m_engine->contact(contactId, fetchHint, &h.error);
}
/*!
@@ -537,15 +532,9 @@ QContact QContactManager::contact(const QContactLocalId& contactId, const QConta
*/
QList<QContact> QContactManager::contacts(const QList<QContactLocalId>& localIds, const QContactFetchHint &fetchHint, QMap<int, QContactManager::Error> *errorMap) const
{
- d->m_error = QContactManager::NoError;
- d->m_errorMap.clear();
+ QContactManagerSyncOpErrorHolder h(this, errorMap);
- QList<QContact> retn = d->m_engine->contacts(localIds, fetchHint, &d->m_errorMap, &d->m_error);
-
- if (errorMap)
- *errorMap = d->m_errorMap;
-
- return retn;
+ return d->m_engine->contacts(localIds, fetchHint, &h.errorMap, &h.error);
}
/*!
@@ -586,12 +575,12 @@ QList<QContact> QContactManager::contacts(const QList<QContactLocalId>& localIds
*/
bool QContactManager::saveContact(QContact* contact)
{
- d->m_errorMap.clear();
+ QContactManagerSyncOpErrorHolder h(this);
+
if (contact) {
- d->m_error = QContactManager::NoError;
- return d->m_engine->saveContact(contact, &d->m_error);
+ return d->m_engine->saveContact(contact, &h.error);
} else {
- d->m_error = QContactManager::BadArgumentError;
+ h.error = QContactManager::BadArgumentError;
return false;
}
}
@@ -604,9 +593,8 @@ bool QContactManager::saveContact(QContact* contact)
*/
bool QContactManager::removeContact(const QContactLocalId& contactId)
{
- d->m_error = QContactManager::NoError;
- d->m_errorMap.clear();
- return d->m_engine->removeContact(contactId, &d->m_error);
+ QContactManagerSyncOpErrorHolder h(this);
+ return d->m_engine->removeContact(contactId, &h.error);
}
/*!
@@ -626,20 +614,14 @@ bool QContactManager::removeContact(const QContactLocalId& contactId)
*/
bool QContactManager::saveContacts(QList<QContact>* contacts, QMap<int, QContactManager::Error>* errorMap)
{
- bool retn = false;
- d->m_errorMap.clear();
+ QContactManagerSyncOpErrorHolder h(this, errorMap);
if (contacts) {
- d->m_error = QContactManager::NoError;
- retn = d->m_engine->saveContacts(contacts, &d->m_errorMap, &d->m_error);
+ return d->m_engine->saveContacts(contacts, &h.errorMap, &h.error);
} else {
- d->m_error =QContactManager::BadArgumentError;
+ h.error = QContactManager::BadArgumentError;
+ return false;
}
-
- if (errorMap)
- *errorMap = d->m_errorMap;
-
- return retn;
}
/*!
@@ -663,20 +645,14 @@ bool QContactManager::saveContacts(QList<QContact>* contacts, QMap<int, QContact
*/
bool QContactManager::saveContacts(QList<QContact>* contacts, const QStringList& definitionMask, QMap<int, QContactManager::Error>* errorMap)
{
- bool retn = false;
- d->m_errorMap.clear();
+ QContactManagerSyncOpErrorHolder h(this, errorMap);
if (contacts) {
- d->m_error = QContactManager::NoError;
- retn = d->m_engine->saveContacts(contacts, definitionMask, &d->m_errorMap, &d->m_error);
+ return d->m_engine->saveContacts(contacts, definitionMask, &h.errorMap, &h.error);
} else {
- d->m_error =QContactManager::BadArgumentError;
+ h.error = QContactManager::BadArgumentError;
+ return false;
}
-
- if (errorMap)
- *errorMap = d->m_errorMap;
-
- return retn;
}
/*!
@@ -705,20 +681,14 @@ bool QContactManager::saveContacts(QList<QContact>* contacts, const QStringList&
*/
bool QContactManager::removeContacts(const QList<QContactLocalId>& contactIds, QMap<int, QContactManager::Error>* errorMap)
{
- bool retn = false;
- d->m_errorMap.clear();
+ QContactManagerSyncOpErrorHolder h(this, errorMap);
if (!contactIds.isEmpty()) {
- d->m_error = QContactManager::NoError;
- retn = d->m_engine->removeContacts(contactIds, &d->m_errorMap, &d->m_error);
+ return d->m_engine->removeContacts(contactIds, &h.errorMap, &h.error);
} else {
- d->m_error = QContactManager::BadArgumentError;
+ h.error = QContactManager::BadArgumentError;
+ return false;
}
-
- if (errorMap)
- *errorMap = d->m_errorMap;
-
- return retn;
}
/*!
@@ -729,9 +699,8 @@ bool QContactManager::removeContacts(const QList<QContactLocalId>& contactIds, Q
*/
QContact QContactManager::compatibleContact(const QContact& original)
{
- d->m_error = QContactManager::NoError;
- d->m_errorMap.clear();
- return d->m_engine->compatibleContact(original, &d->m_error);
+ QContactManagerSyncOpErrorHolder h(this);
+ return d->m_engine->compatibleContact(original, &h.error);
}
/*!
@@ -745,9 +714,8 @@ QContact QContactManager::compatibleContact(const QContact& original)
*/
QString QContactManager::synthesizedContactDisplayLabel(const QContact& contact) const
{
- d->m_error = QContactManager::NoError;
- d->m_errorMap.clear();
- return d->m_engine->synthesizedDisplayLabel(contact, &d->m_error);
+ QContactManagerSyncOpErrorHolder h(this);
+ return d->m_engine->synthesizedDisplayLabel(contact, &h.error);
}
/*!
@@ -770,12 +738,11 @@ QString QContactManager::synthesizedContactDisplayLabel(const QContact& contact)
*/
void QContactManager::synthesizeContactDisplayLabel(QContact *contact) const
{
- d->m_errorMap.clear();
+ QContactManagerSyncOpErrorHolder h(this);
if (contact) {
- d->m_error = QContactManager::NoError;
- QContactManagerEngine::setContactDisplayLabel(contact, d->m_engine->synthesizedDisplayLabel(*contact, &d->m_error));
+ QContactManagerEngine::setContactDisplayLabel(contact, d->m_engine->synthesizedDisplayLabel(*contact, &h.error));
} else {
- d->m_error = QContactManager::BadArgumentError;
+ h.error = QContactManager::BadArgumentError;
}
}
@@ -792,9 +759,8 @@ void QContactManager::synthesizeContactDisplayLabel(QContact *contact) const
*/
bool QContactManager::setSelfContactId(const QContactLocalId& contactId)
{
- d->m_error = QContactManager::NoError;
- d->m_errorMap.clear();
- return d->m_engine->setSelfContactId(contactId, &d->m_error);
+ QContactManagerSyncOpErrorHolder h(this);
+ return d->m_engine->setSelfContactId(contactId, &h.error);
}
/*!
@@ -806,9 +772,8 @@ bool QContactManager::setSelfContactId(const QContactLocalId& contactId)
*/
QContactLocalId QContactManager::selfContactId() const
{
- d->m_error = QContactManager::NoError;
- d->m_errorMap.clear();
- return d->m_engine->selfContactId(&d->m_error);
+ QContactManagerSyncOpErrorHolder h(this);
+ return d->m_engine->selfContactId(&h.error);
}
/*!
@@ -817,9 +782,8 @@ QContactLocalId QContactManager::selfContactId() const
*/
QList<QContactRelationship> QContactManager::relationships(const QContactId& participantId, QContactRelationship::Role role) const
{
- d->m_error = QContactManager::NoError;
- d->m_errorMap.clear();
- return d->m_engine->relationships(QString(), participantId, role, &d->m_error);
+ QContactManagerSyncOpErrorHolder h(this);
+ return d->m_engine->relationships(QString(), participantId, role, &h.error);
}
/*!
@@ -829,9 +793,8 @@ QList<QContactRelationship> QContactManager::relationships(const QContactId& par
*/
QList<QContactRelationship> QContactManager::relationships(const QString& relationshipType, const QContactId& participantId, QContactRelationship::Role role) const
{
- d->m_error = QContactManager::NoError;
- d->m_errorMap.clear();
- return d->m_engine->relationships(relationshipType, participantId, role, &d->m_error);
+ QContactManagerSyncOpErrorHolder h(this);
+ return d->m_engine->relationships(relationshipType, participantId, role, &h.error);
}
/*!
@@ -849,12 +812,11 @@ QList<QContactRelationship> QContactManager::relationships(const QString& relati
*/
bool QContactManager::saveRelationship(QContactRelationship* relationship)
{
- d->m_errorMap.clear();
+ QContactManagerSyncOpErrorHolder h(this);
if (relationship) {
- d->m_error = QContactManager::NoError;
- return d->m_engine->saveRelationship(relationship, &d->m_error);
+ return d->m_engine->saveRelationship(relationship, &h.error);
} else {
- d->m_error =QContactManager::BadArgumentError;
+ h.error = QContactManager::BadArgumentError;
return false;
}
}
@@ -866,20 +828,14 @@ bool QContactManager::saveRelationship(QContactRelationship* relationship)
*/
bool QContactManager::saveRelationships(QList<QContactRelationship>* relationships, QMap<int, QContactManager::Error>* errorMap)
{
- bool retn = false;
- d->m_errorMap.clear();
+ QContactManagerSyncOpErrorHolder h(this, errorMap);
if (relationships) {
- d->m_error = QContactManager::NoError;
- retn = d->m_engine->saveRelationships(relationships, &d->m_errorMap, &d->m_error);
+ return d->m_engine->saveRelationships(relationships, &h.errorMap, &h.error);
} else {
- d->m_error =QContactManager::BadArgumentError;
+ h.error = QContactManager::BadArgumentError;
+ return false;
}
-
- if (errorMap)
- *errorMap = d->m_errorMap;
-
- return retn;
}
/*!
@@ -890,9 +846,8 @@ bool QContactManager::saveRelationships(QList<QContactRelationship>* relationshi
*/
bool QContactManager::removeRelationship(const QContactRelationship& relationship)
{
- d->m_error = QContactManager::NoError;
- d->m_errorMap.clear();
- return d->m_engine->removeRelationship(relationship, &d->m_error);
+ QContactManagerSyncOpErrorHolder h(this);
+ return d->m_engine->removeRelationship(relationship, &h.error);
}
@@ -903,14 +858,8 @@ bool QContactManager::removeRelationship(const QContactRelationship& relationshi
*/
bool QContactManager::removeRelationships(const QList<QContactRelationship>& relationships, QMap<int, QContactManager::Error>* errorMap)
{
- d->m_error = QContactManager::NoError;
- d->m_errorMap.clear();
- bool retn = d->m_engine->removeRelationships(relationships, &d->m_errorMap, &d->m_error);
-
- if (errorMap)
- *errorMap = d->m_errorMap;
-
- return retn;
+ QContactManagerSyncOpErrorHolder h(this, errorMap);
+ return d->m_engine->removeRelationships(relationships, &h.errorMap, &h.error);
}
/*!
@@ -919,53 +868,49 @@ bool QContactManager::removeRelationships(const QList<QContactRelationship>& rel
*/
QMap<QString, QContactDetailDefinition> QContactManager::detailDefinitions(const QString& contactType) const
{
- d->m_errorMap.clear();
+ QContactManagerSyncOpErrorHolder h(this);
if (!supportedContactTypes().contains(contactType)) {
- d->m_error =QContactManager::InvalidContactTypeError;
+ h.error = QContactManager::InvalidContactTypeError;
return QMap<QString, QContactDetailDefinition>();
}
- d->m_error = QContactManager::NoError;
- return d->m_engine->detailDefinitions(contactType, &d->m_error);
+ return d->m_engine->detailDefinitions(contactType, &h.error);
}
/*! Returns the definition identified by the given \a definitionName that is valid for the contacts whose type is the given \a contactType in this store, or a default-constructed QContactDetailDefinition if no such definition exists */
QContactDetailDefinition QContactManager::detailDefinition(const QString& definitionName, const QString& contactType) const
{
- d->m_errorMap.clear();
+ QContactManagerSyncOpErrorHolder h(this);
if (!supportedContactTypes().contains(contactType)) {
- d->m_error =QContactManager::InvalidContactTypeError;
+ h.error = QContactManager::InvalidContactTypeError;
return QContactDetailDefinition();
}
- d->m_error = QContactManager::NoError;
- return d->m_engine->detailDefinition(definitionName, contactType, &d->m_error);
+ return d->m_engine->detailDefinition(definitionName, contactType, &h.error);
}
/*! Persists the given definition \a def in the database, which is valid for contacts whose type is the given \a contactType. Returns true if the definition was saved successfully, otherwise returns false */
bool QContactManager::saveDetailDefinition(const QContactDetailDefinition& def, const QString& contactType)
{
- d->m_errorMap.clear();
+ QContactManagerSyncOpErrorHolder h(this);
if (!supportedContactTypes().contains(contactType)) {
- d->m_error =QContactManager::InvalidContactTypeError;
+ h.error = QContactManager::InvalidContactTypeError;
return false;
}
- d->m_error = QContactManager::NoError;
- return d->m_engine->saveDetailDefinition(def, contactType, &d->m_error);
+ return d->m_engine->saveDetailDefinition(def, contactType, &h.error);
}
/*! Removes the detail definition identified by \a definitionName from the database, which is valid for contacts whose type is the given \a contactType. Returns true if the definition was removed successfully, otherwise returns false */
bool QContactManager::removeDetailDefinition(const QString& definitionName, const QString& contactType)
{
- d->m_errorMap.clear();
+ QContactManagerSyncOpErrorHolder h(this);
if (!supportedContactTypes().contains(contactType)) {
- d->m_error =QContactManager::InvalidContactTypeError;
+ h.error = QContactManager::InvalidContactTypeError;
return false;
}
- d->m_error = QContactManager::NoError;
- return d->m_engine->removeDetailDefinition(definitionName, contactType, &d->m_error);
+ return d->m_engine->removeDetailDefinition(definitionName, contactType, &h.error);
}
/*!
diff --git a/src/contacts/qcontactmanager_p.cpp b/src/contacts/qcontactmanager_p.cpp
index 494b3b2b5c..9772e09964 100644
--- a/src/contacts/qcontactmanager_p.cpp
+++ b/src/contacts/qcontactmanager_p.cpp
@@ -137,7 +137,7 @@ void QContactManagerData::createEngine(const QString& managerName, const QMap<QS
/* See if we got a fast hit */
QList<QContactManagerEngineFactory*> factories = m_engines.values(builtManagerName);
- m_error = QContactManager::NoError;
+ m_lastError = QContactManager::NoError;
while(!found) {
foreach (QContactManagerEngineFactory* f, factories) {
@@ -145,7 +145,7 @@ void QContactManagerData::createEngine(const QString& managerName, const QMap<QS
if (implementationVersion == -1 ||//no given implementation version required
versions.isEmpty() || //the manager engine factory does not report any version
versions.contains(implementationVersion)) {
- QContactManagerEngine* engine = f->engine(parameters, &m_error);
+ QContactManagerEngine* engine = f->engine(parameters, &m_lastError);
// if it's a V2, use it
m_engine = qobject_cast<QContactManagerEngineV2*>(engine);
if (!m_engine && engine) {
@@ -170,13 +170,13 @@ void QContactManagerData::createEngine(const QString& managerName, const QMap<QS
// XXX remove this
// the engine factory could lie to us, so check the real implementation version
if (m_engine && (implementationVersion != -1 && m_engine->managerVersion() != implementationVersion)) {
- m_error = QContactManager::VersionMismatchError;
+ m_lastError = QContactManager::VersionMismatchError;
m_engine = 0;
}
if (!m_engine) {
- if (m_error == QContactManager::NoError)
- m_error = QContactManager::DoesNotExistError;
+ if (m_lastError == QContactManager::NoError)
+ m_lastError = QContactManager::DoesNotExistError;
m_engine = new QContactInvalidEngine();
}
}
diff --git a/src/contacts/qcontactmanager_p.h b/src/contacts/qcontactmanager_p.h
index 13fa359215..ee8fd164c9 100644
--- a/src/contacts/qcontactmanager_p.h
+++ b/src/contacts/qcontactmanager_p.h
@@ -74,7 +74,7 @@ class QContactManagerData
public:
QContactManagerData()
: m_engine(0),
- m_error(QContactManager::NoError)
+ m_lastError(QContactManager::NoError)
{
}
@@ -87,13 +87,14 @@ public:
static QContactManagerEngineV2* engine(const QContactManager* manager);
QContactManagerEngineV2* m_engine;
- QContactManager::Error m_error;
- QMap<int, QContactManager::Error> m_errorMap;
+ QContactManager::Error m_lastError;
+ QMap<int, QContactManager::Error> m_lastErrorMap;
/* Manager plugins */
static QHash<QString, QContactManagerEngineFactory*> m_engines;
static QSet<QContactManager*> m_aliveEngines;
static QContactManagerData* managerData(QContactManager* manager) {return manager->d;}
+ static QContactManagerData* managerData(const QContactManager* manager) {return manager->d;} // laziness to avoid const_cast
static QList<QContactActionManagerPlugin*> m_actionManagers;
static bool m_discoveredStatic;
static QStringList m_pluginPaths;
@@ -104,6 +105,36 @@ private:
Q_DISABLE_COPY(QContactManagerData)
};
+/*
+ Helper to hold the error state of a synchronous operation - when destructed, updates the
+ manager's last error variables to the result of this operation. This means that during
+ callbacks the error state can't be modified behind the engines back. and it's more conceptually
+ correct.
+ */
+class QContactManagerSyncOpErrorHolder
+{
+public:
+ QContactManagerSyncOpErrorHolder(const QContactManager* m, QMap<int, QContactManager::Error> *pUserError = 0)
+ : error(QContactManager::NoError),
+ data(QContactManagerData::managerData(m)),
+ userError(pUserError)
+ {
+ }
+
+ ~QContactManagerSyncOpErrorHolder()
+ {
+ data->m_lastError = error;
+ data->m_lastErrorMap = errorMap;
+ if (userError)
+ *userError = errorMap;
+ }
+
+ QContactManager::Error error;
+ QContactManagerData* data;
+ QMap<int, QContactManager::Error> errorMap;
+ QMap<int, QContactManager::Error> *userError;
+};
+
QTM_END_NAMESPACE
#endif
diff --git a/src/organizer/qorganizermanager.cpp b/src/organizer/qorganizermanager.cpp
index 0641e0f054..76e7d93fa9 100644
--- a/src/organizer/qorganizermanager.cpp
+++ b/src/organizer/qorganizermanager.cpp
@@ -336,7 +336,7 @@ QOrganizerManager::~QOrganizerManager()
/*! Return the error code of the most recent operation */
QOrganizerManager::Error QOrganizerManager::error() const
{
- return d->m_error;
+ return d->m_lastError;
}
/*!
@@ -350,7 +350,7 @@ QOrganizerManager::Error QOrganizerManager::error() const
*/
QMap<int, QOrganizerManager::Error> QOrganizerManager::errorMap() const
{
- return d->m_errorMap;
+ return d->m_lastErrorMap;
}
/*!
@@ -367,9 +367,8 @@ QMap<int, QOrganizerManager::Error> QOrganizerManager::errorMap() const
*/
QList<QOrganizerItem> QOrganizerManager::itemOccurrences(const QOrganizerItem& parentItem, const QDateTime& periodStart, const QDateTime& periodEnd, int maxCount, const QOrganizerItemFetchHint& fetchHint) const
{
- d->m_error = QOrganizerManager::NoError;
- d->m_errorMap.clear();
- return d->m_engine->itemOccurrences(parentItem, periodStart, periodEnd, maxCount, fetchHint, &d->m_error);
+ QOrganizerManagerSyncOpErrorHolder h(this);
+ return d->m_engine->itemOccurrences(parentItem, periodStart, periodEnd, maxCount, fetchHint, &h.error);
}
/*!
@@ -378,8 +377,8 @@ QList<QOrganizerItem> QOrganizerManager::itemOccurrences(const QOrganizerItem& p
*/
QList<QOrganizerItemId> QOrganizerManager::itemIds(const QOrganizerItemFilter& filter, const QList<QOrganizerItemSortOrder>& sortOrders) const
{
- d->m_error = QOrganizerManager::NoError;
- return d->m_engine->itemIds(QDateTime(), QDateTime(), filter, sortOrders, &d->m_error);
+ QOrganizerManagerSyncOpErrorHolder h(this);
+ return d->m_engine->itemIds(QDateTime(), QDateTime(), filter, sortOrders, &h.error);
}
/*!
@@ -392,9 +391,8 @@ QList<QOrganizerItemId> QOrganizerManager::itemIds(const QOrganizerItemFilter& f
*/
QList<QOrganizerItemId> QOrganizerManager::itemIds(const QDateTime& startDate, const QDateTime& endDate, const QOrganizerItemFilter& filter, const QList<QOrganizerItemSortOrder>& sortOrders) const
{
- d->m_error = QOrganizerManager::NoError;
- d->m_errorMap.clear();
- return d->m_engine->itemIds(startDate, endDate, filter, sortOrders, &d->m_error);
+ QOrganizerManagerSyncOpErrorHolder h(this);
+ return d->m_engine->itemIds(startDate, endDate, filter, sortOrders, &h.error);
}
/*!
@@ -416,9 +414,8 @@ QList<QOrganizerItemId> QOrganizerManager::itemIds(const QDateTime& startDate, c
*/
QList<QOrganizerItem> QOrganizerManager::items(const QOrganizerItemFilter& filter, const QList<QOrganizerItemSortOrder>& sortOrders, const QOrganizerItemFetchHint& fetchHint) const
{
- d->m_error = QOrganizerManager::NoError;
- d->m_errorMap.clear();
- return d->m_engine->items(QDateTime(), QDateTime(), filter, sortOrders, fetchHint, &d->m_error);
+ QOrganizerManagerSyncOpErrorHolder h(this);
+ return d->m_engine->items(QDateTime(), QDateTime(), filter, sortOrders, fetchHint, &h.error);
}
/*!
@@ -444,9 +441,8 @@ QList<QOrganizerItem> QOrganizerManager::items(const QOrganizerItemFilter& filte
*/
QList<QOrganizerItem> QOrganizerManager::items(const QDateTime& startDate, const QDateTime& endDate, const QOrganizerItemFilter& filter, const QList<QOrganizerItemSortOrder>& sortOrders, const QOrganizerItemFetchHint& fetchHint) const
{
- d->m_error = QOrganizerManager::NoError;
- d->m_errorMap.clear();
- return d->m_engine->items(startDate, endDate, filter, sortOrders, fetchHint, &d->m_error);
+ QOrganizerManagerSyncOpErrorHolder h(this);
+ return d->m_engine->items(startDate, endDate, filter, sortOrders, fetchHint, &h.error);
}
/*!
@@ -472,9 +468,8 @@ QList<QOrganizerItem> QOrganizerManager::items(const QDateTime& startDate, const
*/
QList<QOrganizerItem> QOrganizerManager::itemsForExport(const QDateTime& startDate, const QDateTime& endDate, const QOrganizerItemFilter& filter, const QList<QOrganizerItemSortOrder>& sortOrders, const QOrganizerItemFetchHint& fetchHint) const
{
- d->m_error = QOrganizerManager::NoError;
- d->m_errorMap.clear();
- return d->m_engine->itemsForExport(startDate, endDate, filter, sortOrders, fetchHint, &d->m_error);
+ QOrganizerManagerSyncOpErrorHolder h(this);
+ return d->m_engine->itemsForExport(startDate, endDate, filter, sortOrders, fetchHint, &h.error);
}
/*!
@@ -494,9 +489,8 @@ QList<QOrganizerItem> QOrganizerManager::itemsForExport(const QDateTime& startDa
*/
QOrganizerItem QOrganizerManager::item(const QOrganizerItemId& organizeritemId, const QOrganizerItemFetchHint& fetchHint) const
{
- d->m_error = QOrganizerManager::NoError;
- d->m_errorMap.clear();
- return d->m_engine->item(organizeritemId, fetchHint, &d->m_error);
+ QOrganizerManagerSyncOpErrorHolder h(this);
+ return d->m_engine->item(organizeritemId, fetchHint, &h.error);
}
/*!
@@ -535,12 +529,11 @@ QOrganizerItem QOrganizerManager::item(const QOrganizerItemId& organizeritemId,
*/
bool QOrganizerManager::saveItem(QOrganizerItem* item)
{
- d->m_errorMap.clear();
+ QOrganizerManagerSyncOpErrorHolder h(this);
if (item) {
- d->m_error = QOrganizerManager::NoError;
- return d->m_engine->saveItem(item, &d->m_error);
+ return d->m_engine->saveItem(item, &h.error);
} else {
- d->m_error = QOrganizerManager::BadArgumentError;
+ h.error = QOrganizerManager::BadArgumentError;
return false;
}
}
@@ -552,9 +545,8 @@ bool QOrganizerManager::saveItem(QOrganizerItem* item)
*/
bool QOrganizerManager::removeItem(const QOrganizerItemId& organizeritemId)
{
- d->m_error = QOrganizerManager::NoError;
- d->m_errorMap.clear();
- return d->m_engine->removeItem(organizeritemId, &d->m_error);
+ QOrganizerManagerSyncOpErrorHolder h(this);
+ return d->m_engine->removeItem(organizeritemId, &h.error);
}
/*!
@@ -579,14 +571,13 @@ bool QOrganizerManager::removeItem(const QOrganizerItemId& organizeritemId)
*/
bool QOrganizerManager::saveItems(QList<QOrganizerItem>* items)
{
- d->m_errorMap.clear();
+ QOrganizerManagerSyncOpErrorHolder h(this);
if (!items) {
- d->m_error = QOrganizerManager::BadArgumentError;
+ h.error = QOrganizerManager::BadArgumentError;
return false;
}
- d->m_error = QOrganizerManager::NoError;
- return d->m_engine->saveItems(items, &d->m_errorMap, &d->m_error);
+ return d->m_engine->saveItems(items, &h.errorMap, &h.error);
}
/*!
@@ -611,14 +602,13 @@ bool QOrganizerManager::saveItems(QList<QOrganizerItem>* items)
*/
bool QOrganizerManager::removeItems(const QList<QOrganizerItemId>& organizeritemIds)
{
- d->m_errorMap.clear();
+ QOrganizerManagerSyncOpErrorHolder h(this);
if (organizeritemIds.isEmpty()) {
- d->m_error = QOrganizerManager::BadArgumentError;
+ h.error = QOrganizerManager::BadArgumentError;
return false;
}
- d->m_error = QOrganizerManager::NoError;
- return d->m_engine->removeItems(organizeritemIds, &d->m_errorMap, &d->m_error);
+ return d->m_engine->removeItems(organizeritemIds, &h.errorMap, &h.error);
}
/*!
@@ -626,9 +616,8 @@ bool QOrganizerManager::removeItems(const QList<QOrganizerItemId>& organizeritem
*/
QOrganizerCollection QOrganizerManager::defaultCollection() const
{
- d->m_error = QOrganizerManager::NoError;
- d->m_errorMap.clear();
- return d->m_engine->defaultCollection(&d->m_error);
+ QOrganizerManagerSyncOpErrorHolder h(this);
+ return d->m_engine->defaultCollection(&h.error);
}
/*!
@@ -636,9 +625,8 @@ QOrganizerCollection QOrganizerManager::defaultCollection() const
*/
QOrganizerCollection QOrganizerManager::collection(const QOrganizerCollectionId& collectionId) const
{
- d->m_error = QOrganizerManager::NoError;
- d->m_errorMap.clear();
- return d->m_engine->collection(collectionId, &d->m_error);
+ QOrganizerManagerSyncOpErrorHolder h(this);
+ return d->m_engine->collection(collectionId, &h.error);
}
/*!
@@ -646,9 +634,8 @@ QOrganizerCollection QOrganizerManager::collection(const QOrganizerCollectionId&
*/
QList<QOrganizerCollection> QOrganizerManager::collections() const
{
- d->m_error = QOrganizerManager::NoError;
- d->m_errorMap.clear();
- return d->m_engine->collections(&d->m_error);
+ QOrganizerManagerSyncOpErrorHolder h(this);
+ return d->m_engine->collections(&h.error);
}
/*!
@@ -676,12 +663,11 @@ QList<QOrganizerCollection> QOrganizerManager::collections() const
*/
bool QOrganizerManager::saveCollection(QOrganizerCollection* collection)
{
- d->m_errorMap.clear();
+ QOrganizerManagerSyncOpErrorHolder h(this);
if (collection) {
- d->m_error = QOrganizerManager::NoError;
- return d->m_engine->saveCollection(collection, &d->m_error);
+ return d->m_engine->saveCollection(collection, &h.error);
} else {
- d->m_error = QOrganizerManager::BadArgumentError;
+ h.error = QOrganizerManager::BadArgumentError;
return false;
}
}
@@ -696,9 +682,8 @@ bool QOrganizerManager::saveCollection(QOrganizerCollection* collection)
*/
bool QOrganizerManager::removeCollection(const QOrganizerCollectionId& collectionId)
{
- d->m_error = QOrganizerManager::NoError;
- d->m_errorMap.clear();
- return d->m_engine->removeCollection(collectionId, &d->m_error);
+ QOrganizerManagerSyncOpErrorHolder h(this);
+ return d->m_engine->removeCollection(collectionId, &h.error);
}
/*!
@@ -707,9 +692,8 @@ bool QOrganizerManager::removeCollection(const QOrganizerCollectionId& collectio
*/
QOrganizerItem QOrganizerManager::compatibleItem(const QOrganizerItem& original) const
{
- d->m_error = QOrganizerManager::NoError;
- d->m_errorMap.clear();
- return d->m_engine->compatibleItem(original, &d->m_error);
+ QOrganizerManagerSyncOpErrorHolder h(this);
+ return d->m_engine->compatibleItem(original, &h.error);
}
/*!
@@ -718,9 +702,8 @@ QOrganizerItem QOrganizerManager::compatibleItem(const QOrganizerItem& original)
*/
QOrganizerCollection QOrganizerManager::compatibleCollection(const QOrganizerCollection& original) const
{
- d->m_error = QOrganizerManager::NoError;
- d->m_errorMap.clear();
- return d->m_engine->compatibleCollection(original, &d->m_error);
+ QOrganizerManagerSyncOpErrorHolder h(this);
+ return d->m_engine->compatibleCollection(original, &h.error);
}
/*!
@@ -729,53 +712,49 @@ QOrganizerCollection QOrganizerManager::compatibleCollection(const QOrganizerCol
*/
QMap<QString, QOrganizerItemDetailDefinition> QOrganizerManager::detailDefinitions(const QString& organizeritemType) const
{
- d->m_errorMap.clear();
+ QOrganizerManagerSyncOpErrorHolder h(this);
if (!supportedItemTypes().contains(organizeritemType)) {
- d->m_error = QOrganizerManager::InvalidItemTypeError;
+ h.error = QOrganizerManager::InvalidItemTypeError;
return QMap<QString, QOrganizerItemDetailDefinition>();
}
- d->m_error = QOrganizerManager::NoError;
- return d->m_engine->detailDefinitions(organizeritemType, &d->m_error);
+ return d->m_engine->detailDefinitions(organizeritemType, &h.error);
}
/*! Returns the definition identified by the given \a definitionName that is valid for the organizer items whose type is the given \a organizeritemType in this store, or a default-constructed QOrganizerItemDetailDefinition if no such definition exists */
QOrganizerItemDetailDefinition QOrganizerManager::detailDefinition(const QString& definitionName, const QString& organizeritemType) const
{
- d->m_errorMap.clear();
+ QOrganizerManagerSyncOpErrorHolder h(this);
if (!supportedItemTypes().contains(organizeritemType)) {
- d->m_error = QOrganizerManager::InvalidItemTypeError;
+ h.error = QOrganizerManager::InvalidItemTypeError;
return QOrganizerItemDetailDefinition();
}
- d->m_error = QOrganizerManager::NoError;
- return d->m_engine->detailDefinition(definitionName, organizeritemType, &d->m_error);
+ return d->m_engine->detailDefinition(definitionName, organizeritemType, &h.error);
}
/*! Persists the given definition \a def in the database, which is valid for organizer items whose type is the given \a organizeritemType. Returns true if the definition was saved successfully, otherwise returns false */
bool QOrganizerManager::saveDetailDefinition(const QOrganizerItemDetailDefinition& def, const QString& organizeritemType)
{
- d->m_errorMap.clear();
+ QOrganizerManagerSyncOpErrorHolder h(this);
if (!supportedItemTypes().contains(organizeritemType)) {
- d->m_error = QOrganizerManager::InvalidItemTypeError;
+ h.error = QOrganizerManager::InvalidItemTypeError;
return false;
}
- d->m_error = QOrganizerManager::NoError;
- return d->m_engine->saveDetailDefinition(def, organizeritemType, &d->m_error);
+ return d->m_engine->saveDetailDefinition(def, organizeritemType, &h.error);
}
/*! Removes the detail definition identified by \a definitionName from the database, which is valid for organizer items whose type is the given \a organizeritemType. Returns true if the definition was removed successfully, otherwise returns false */
bool QOrganizerManager::removeDetailDefinition(const QString& definitionName, const QString& organizeritemType)
{
- d->m_errorMap.clear();
+ QOrganizerManagerSyncOpErrorHolder h(this);
if (!supportedItemTypes().contains(organizeritemType)) {
- d->m_error = QOrganizerManager::InvalidItemTypeError;
+ h.error = QOrganizerManager::InvalidItemTypeError;
return false;
}
- d->m_error = QOrganizerManager::NoError;
- return d->m_engine->removeDetailDefinition(definitionName, organizeritemType, &d->m_error);
+ return d->m_engine->removeDetailDefinition(definitionName, organizeritemType, &h.error);
}
/*!
diff --git a/src/organizer/qorganizermanager_p.cpp b/src/organizer/qorganizermanager_p.cpp
index af33d0ddf5..00449eba8b 100644
--- a/src/organizer/qorganizermanager_p.cpp
+++ b/src/organizer/qorganizermanager_p.cpp
@@ -114,7 +114,7 @@ void QOrganizerManagerData::createEngine(const QString& managerName, const QMap<
/* See if we got a fast hit */
QList<QOrganizerManagerEngineFactory*> factories = m_engines.values(builtManagerName);
- m_error = QOrganizerManager::NoError;
+ m_lastError = QOrganizerManager::NoError;
while(!found) {
foreach (QOrganizerManagerEngineFactory* f, factories) {
@@ -122,7 +122,7 @@ void QOrganizerManagerData::createEngine(const QString& managerName, const QMap<
if (implementationVersion == -1 ||//no given implementation version required
versions.isEmpty() || //the manager engine factory does not report any version
versions.contains(implementationVersion)) {
- m_engine = f->engine(parameters, &m_error);
+ m_engine = f->engine(parameters, &m_lastError);
found = true;
break;
}
@@ -141,13 +141,13 @@ void QOrganizerManagerData::createEngine(const QString& managerName, const QMap<
// XXX remove this
// the engine factory could lie to us, so check the real implementation version
if (m_engine && (implementationVersion != -1 && m_engine->managerVersion() != implementationVersion)) {
- m_error = QOrganizerManager::VersionMismatchError;
+ m_lastError = QOrganizerManager::VersionMismatchError;
m_engine = 0;
}
if (!m_engine) {
- if (m_error == QOrganizerManager::NoError)
- m_error = QOrganizerManager::DoesNotExistError;
+ if (m_lastError == QOrganizerManager::NoError)
+ m_lastError = QOrganizerManager::DoesNotExistError;
m_engine = new QOrganizerItemInvalidEngine();
}
}
diff --git a/src/organizer/qorganizermanager_p.h b/src/organizer/qorganizermanager_p.h
index 99447fe624..a3ae60c0ee 100644
--- a/src/organizer/qorganizermanager_p.h
+++ b/src/organizer/qorganizermanager_p.h
@@ -72,7 +72,7 @@ class QOrganizerManagerData
public:
QOrganizerManagerData()
: m_engine(0),
- m_error(QOrganizerManager::NoError)
+ m_lastError(QOrganizerManager::NoError)
{
}
@@ -87,8 +87,8 @@ public:
static QOrganizerCollectionEngineId* createEngineCollectionId(const QString& managerName, const QMap<QString, QString>& parameters, const QString& engineIdString);
QOrganizerManagerEngine* m_engine;
- QOrganizerManager::Error m_error;
- QMap<int, QOrganizerManager::Error> m_errorMap;
+ QOrganizerManager::Error m_lastError;
+ QMap<int, QOrganizerManager::Error> m_lastErrorMap;
/* Manager plugins */
static QHash<QString, QOrganizerManagerEngineFactory*> m_engines;
@@ -98,10 +98,41 @@ public:
static void loadFactories();
static void loadStaticFactories();
+ static QOrganizerManagerData* managerData(const QOrganizerManager*m) {return m->d;}
+
private:
Q_DISABLE_COPY(QOrganizerManagerData)
};
+/*
+ Helper to hold the error state of a synchronous operation - when destructed, updates the
+ manager's last error variables to the result of this operation. This means that during
+ callbacks the error state can't be modified behind the engines back. and it's more conceptually
+ correct.
+ */
+class QOrganizerManagerSyncOpErrorHolder
+{
+public:
+ QOrganizerManagerSyncOpErrorHolder(const QOrganizerManager* m, QMap<int, QOrganizerManager::Error> *pUserError = 0)
+ : error(QOrganizerManager::NoError),
+ data(QOrganizerManagerData::managerData(m)),
+ userError(pUserError)
+ {
+ }
+
+ ~QOrganizerManagerSyncOpErrorHolder()
+ {
+ data->m_lastError = error;
+ data->m_lastErrorMap = errorMap;
+ if (userError)
+ *userError = errorMap;
+ }
+
+ QOrganizerManager::Error error;
+ QOrganizerManagerData* data;
+ QMap<int, QOrganizerManager::Error> errorMap;
+ QMap<int, QOrganizerManager::Error> *userError;
+};
QTM_END_NAMESPACE
diff --git a/tests/auto/qcontactmanager/tst_qcontactmanager.cpp b/tests/auto/qcontactmanager/tst_qcontactmanager.cpp
index bfc06b35f3..60a834e81f 100644
--- a/tests/auto/qcontactmanager/tst_qcontactmanager.cpp
+++ b/tests/auto/qcontactmanager/tst_qcontactmanager.cpp
@@ -204,6 +204,7 @@ private slots:
void changeSet();
void fetchHint();
void engineDefaultSchema();
+ void errorSemantics();
/* Special test with special data */
void uriParsing_data();
@@ -3663,5 +3664,59 @@ void tst_QContactManager::lateDeletion()
cm->setParent(qApp); // now do nothing
}
+class errorSemanticsTester : public QObject {
+ Q_OBJECT;
+public:
+ bool initialErrorWasDoesNotExist;
+ bool slotErrorWasBadArgument;
+ QContactManager* mManager;
+
+ errorSemanticsTester(QContactManager* manager)
+ : initialErrorWasDoesNotExist(false),
+ slotErrorWasBadArgument(false),
+ mManager(manager)
+ {
+ connect(manager, SIGNAL(contactsAdded(QList<QContactLocalId>)), this, SLOT(handleAdded()));
+ }
+
+public slots:
+ void handleAdded()
+ {
+ // Make sure the initial error state is correct
+ initialErrorWasDoesNotExist = mManager->error() == QContactManager::DoesNotExistError;
+ // Now force a different error
+ mManager->removeContacts(QList<QContactLocalId>());
+ slotErrorWasBadArgument = mManager->error() == QContactManager::BadArgumentError;
+ // and return
+ }
+};
+
+void tst_QContactManager::errorSemantics()
+{
+ /*
+ Test to make sure that calling functions in response to signals doesn't upset the correct error results
+ This relies on the memory engine emitting signals before e.g. saveContacts returns
+ */
+
+ QContactManager m("memory");
+ errorSemanticsTester t(&m);
+
+ QVERIFY(m.error() == QContactManager::NoError);
+
+ QContactDetailDefinition nameDef = m.detailDefinition(QContactName::DefinitionName, QContactType::TypeContact);
+ QContact alice = createContact(nameDef, "Alice", "inWonderland", "1234567");
+
+ // Try creating some specific error so we can test it later on
+ QContact a = m.contact(567);
+ QVERIFY(m.error() == QContactManager::DoesNotExistError);
+
+ // Now save something
+ QVERIFY(m.saveContact(&alice));
+
+ QVERIFY(t.initialErrorWasDoesNotExist);
+ QVERIFY(t.slotErrorWasBadArgument);
+ QVERIFY(m.error() == QContactManager::NoError);
+}
+
QTEST_MAIN(tst_QContactManager)
#include "tst_qcontactmanager.moc"
diff --git a/tests/auto/qorganizermanager/tst_qorganizermanager.cpp b/tests/auto/qorganizermanager/tst_qorganizermanager.cpp
index a2265b8a8c..26d7160896 100644
--- a/tests/auto/qorganizermanager/tst_qorganizermanager.cpp
+++ b/tests/auto/qorganizermanager/tst_qorganizermanager.cpp
@@ -190,6 +190,7 @@ private slots:
void changeSet();
void fetchHint();
void testFilterFunction();
+ void errorSemantics();
/* Special test with special data */
void uriParsing_data();
@@ -4154,5 +4155,70 @@ void tst_QOrganizerManager::collections()
}
}
+class errorSemanticsTester : public QObject {
+ Q_OBJECT;
+public:
+ bool initialErrorWasDoesNotExist;
+ bool slotErrorWasBadArgument;
+ QOrganizerManager* mManager;
+
+ errorSemanticsTester(QOrganizerManager* manager)
+ : initialErrorWasDoesNotExist(false),
+ slotErrorWasBadArgument(false),
+ mManager(manager)
+ {
+ connect(manager, SIGNAL(itemsAdded(QList<QOrganizerItemId>)), this, SLOT(handleAdded()));
+ }
+
+public slots:
+ void handleAdded()
+ {
+ // Make sure the initial error state is correct
+ initialErrorWasDoesNotExist = mManager->error() == QOrganizerManager::DoesNotExistError;
+ // Now force a different error
+ mManager->removeItems(QList<QOrganizerItemId>());
+ slotErrorWasBadArgument = mManager->error() == QOrganizerManager::BadArgumentError;
+ // and return
+ }
+};
+
+void tst_QOrganizerManager::errorSemantics()
+{
+ /*
+ Test to make sure that calling functions in response to signals doesn't upset the correct error results
+ This relies on the memory engine emitting signals before e.g. saveItems returns
+ */
+
+ QOrganizerManager m("memory");
+ errorSemanticsTester t(&m);
+
+ QVERIFY(m.error() == QOrganizerManager::NoError);
+
+ QString type;
+ if (m.detailDefinitions(QOrganizerItemType::TypeNote).count())
+ type = QLatin1String(QOrganizerItemType::TypeNote);
+ else if (m.detailDefinitions(QOrganizerItemType::TypeTodo).count())
+ type = QLatin1String(QOrganizerItemType::TypeTodo);
+ else
+ QSKIP("This manager does not support note or todo item", SkipSingle);
+
+ QOrganizerItem item;
+ item.setType(type);
+ item.setDisplayLabel("This is a note");
+ item.setDescription("This note is a particularly notey note");
+
+ // Try creating some specific error so we can test it later on
+ QVERIFY(!m.removeItem(QOrganizerItemId()));
+ QVERIFY(m.error() == QOrganizerManager::DoesNotExistError);
+
+ // Now save something
+ QVERIFY(m.saveItem(&item));
+
+ QVERIFY(t.initialErrorWasDoesNotExist);
+ QVERIFY(t.slotErrorWasBadArgument);
+ QVERIFY(m.error() == QOrganizerManager::NoError);
+}
+
+
QTEST_MAIN(tst_QOrganizerManager)
#include "tst_qorganizermanager.moc"