> > > Whether that'll work in practice depends on other factors, too, like how > well these backends support storing arbitrary X- extensions and their > filtering support. But without hashing, it won't work at all. > > > The other conceptual problem with the patch is the compile > > time choice > > between "hashing on and off". This needs to be a runtime > > check, so that > > depending on whether we use a local file backend or something > > else the > > right ID handling will be used. This in turn depends on the > > "IDs are 32 > > bit" capability check that we were discussing with the EDS > > developers - > > that still needs to be added to EDS. > > > > I don't believe this is the right solution. Doing the check at > > runtime, would of course be more reliable but it would also be less > > efficient. > > See above. With EDS, we cannot make compile time assumptions about the > backend. > > > I believe that a better fix for this would be to edit your 32bits IDs > > patch for EDS so that it adds -DUSE_32BIT_IDS to libebook-1.2.pc. > > This way, I could easily detect that at compilation time in > > QtContacts-EDS. It would be reliable and efficient. What do you think? > > That would be wrong because libebook might be used to access a non-file > backend. > > Hi,
Good point, I overlooked this possibility. In that case, I completely agree that checking the ID format at run-time is the only solution. I'm attaching a patch for this, I would appreciate if you could review it. Kr, -- Dr. Christophe Dumez Linux Software Engineer Intel Finland Oy - Open Source Technology Center
From 40a1d868dbc1075c25f00fe3e6411ca785fa7860 Mon Sep 17 00:00:00 2001 From: Christophe Dumez <[email protected]> Date: Mon, 6 Jun 2011 14:39:54 +0300 Subject: [PATCH] ID Hashing: Detect EDS contact ID format at run-time Detect if EDS uses 32 bit integers for contact IDs at run-time and switch to ID hashing if necessary. With this patch, it is no longer necessary to indicate which ID format to use at compilation time. --- engine/engine.pro | 5 -- engine/qcontactebook.cpp | 93 ++++++++++++++++++++++++++++------------------ engine/qcontactebook_p.h | 19 +++++---- 3 files changed, 68 insertions(+), 49 deletions(-) diff --git a/engine/engine.pro b/engine/engine.pro index 8afa2ce..100d581 100644 --- a/engine/engine.pro +++ b/engine/engine.pro @@ -9,11 +9,6 @@ CONFIG += mobility link_pkgconfig MOBILITY = contacts versit PKGCONFIG += libebook-1.2 -# ID hashing is required with unpatched EDS -# because EDS uses string IDS and we need -# 32 bit integers -# DEFINES += USE_ID_HASHING - # Enable this optimization if this version of EDS # supports optional/delayed vCard parsing in EBookView DEFINES += OPTIONAL_VCARD_PARSING diff --git a/engine/qcontactebook.cpp b/engine/qcontactebook.cpp index 1448210..f5c5bff 100644 --- a/engine/qcontactebook.cpp +++ b/engine/qcontactebook.cpp @@ -51,6 +51,7 @@ #include <qcontactdetails.h> #include <libebook/e-vcard.h> #include "qcontactebook_p.h" +#include "qebookidhash.h" #include "customproperties.h" /* Error handling Macros */ @@ -74,11 +75,17 @@ static const QString EBookFileIDPrefix = "pas-id-"; QString QContactEBook::QContactLocalId2UID(const QContactLocalId &contactId) const { -#ifdef USE_ID_HASHING - return m_idHash.value(contactId); -#else - return EBookFileIDPrefix + QString("%1").arg(contactId, 4, 16/*base*/, QChar('0')).toUpper(); -#endif + switch (m_edsIdFormat) { + case EDS::INTEGER: + return EBookFileIDPrefix + QString("%1").arg(contactId, 4, 16/*base*/, QChar('0')).toUpper(); + case EDS::STRING: + Q_ASSERT (m_idHash); + return m_idHash->value(contactId); + default: // Unknown + qWarning() << "Warning: QContactLocalId2UID() cannot not be called before any contact is retrieved!"; + qWarning() << "Warning: Returning an empty UID..."; + return QString(); + } } /** @@ -87,16 +94,31 @@ QString QContactEBook::QContactLocalId2UID(const QContactLocalId &contactId) con */ QContactLocalId QContactEBook::UID2QContactLocalId(const QString &uid) const { -#ifdef USE_ID_HASHING - return m_idHash.value(uid); -#else - // Input validation static QRegExp pasid_validator("^"+EBookFileIDPrefix+"[0-9a-f]{4}$", Qt::CaseInsensitive); - if (!pasid_validator.exactMatch(uid)) - return 0; - // The following returns 0 if the conversion fails - return uid.right(4).toUInt(0, 16/*base*/); -#endif + + switch (m_edsIdFormat) { + case EDS::INTEGER: + // Input validation + if (pasid_validator.exactMatch(uid)) { + // The following returns 0 if the conversion fails + return uid.right(4).toUInt(0, 16/*base*/); + } else { + return 0; + } + case EDS::STRING: + Q_ASSERT (m_idHash); + return m_idHash->value(uid); + default: + // Unknown format, auto-detect + if (pasid_validator.exactMatch(uid)) { + m_edsIdFormat = EDS::INTEGER; + } else { + m_edsIdFormat = EDS::STRING; + // Initialize local id hash + initLocalIdHash(); + } + return UID2QContactLocalId(uid); + } } static QContactManager::Error getErrorFromStatus(const EBookStatus status){ @@ -149,9 +171,6 @@ QContactManager::Error QContactEBook::getErrorFromError(const GError *gError){ struct cbSharedData{ QContactEBook *that; -#ifdef USE_ID_HASHING - QEbookIdHash *idHash; -#endif /** * true while initial view is still loading, false once that is * done; QtContacts change signals will be emitted only after the @@ -162,7 +181,8 @@ struct cbSharedData{ /*! Constructs a QContactEBook */ QContactEBook::QContactEBook(const QMap<QString, QString>& parameters, QObject* parent) : - QObject(parent), m_ebook(NULL), m_ebookview(NULL), m_cbSD(0) + QObject(parent), m_ebook(NULL), m_ebookview(NULL), m_cbSD(0), + m_edsIdFormat(EDS::UNKNOWN), m_idHash(0) { // It is safe to call g_type_init() more than once. // Since version 2.24 this also initializes the thread system @@ -180,6 +200,8 @@ QContactEBook::~QContactEBook() g_object_unref(m_ebook); if (m_cbSD) delete m_cbSD; + if (m_idHash) + delete m_idHash; } static QList<QContactLocalId> EContactListToLocalIdList(QContactEBook *ebook, @@ -266,14 +288,20 @@ static void contactsRemovedCB(EBookView *view, gpointer glist, gpointer data) GList *next = static_cast<GList *>(glist); QList<QContactLocalId> contactIds; + const EDS::IdFormat id_format = d->that->idFormat(); + + // We don't know the ID format yet, ignore the signal + if (id_format == EDS::UNKNOWN) return; + while (next) { QString uid = CONST_CHAR(next->data); -#ifdef USE_ID_HASHING - // Remove from the id hash - QContactLocalId id = d->idHash->take(uid); -#else - QContactLocalId id = d->that->UID2QContactLocalId(uid); -#endif + QContactLocalId id; + if (id_format == EDS::INTEGER) { + id = d->that->UID2QContactLocalId(uid); + } else { + // Remove from the id hash + id = d->that->idHash()->take(uid); + } if (id) contactIds << id; else @@ -358,17 +386,9 @@ void QContactEBook::initAddressBook(const QMap<QString, QString>& parameters){ if (!e_book_open(m_ebook, false, &gError)) FATAL_IF_ERROR(gError); - // Initialize local id hash -#ifdef USE_ID_HASHING - initLocalIdHash(); -#endif - /* Initialize callbacks shared data */ m_cbSD = new cbSharedData; m_cbSD->that = this; -#ifdef USE_ID_HASHING - m_cbSD->idHash = &m_idHash; -#endif m_cbSD->initializing = true; // catch-all query, optimized in EDS storage @@ -630,9 +650,11 @@ EContact* QContactEBook::toEContact(const QContact *contact, } /*! Fetches all the contacts from EDS and initializes our hash map */ -void QContactEBook::initLocalIdHash() +void QContactEBook::initLocalIdHash() const { -#ifdef USE_ID_HASHING + Q_ASSERT (m_edsIdFormat == EDS::STRING); + Q_ASSERT (!m_idHash); + m_idHash = new QEbookIdHash; // Fetch all contact ids EBookQuery *query = e_book_query_any_field_contains(""); GList *contacts = NULL; @@ -644,9 +666,8 @@ void QContactEBook::initLocalIdHash() // Register the id in the hash map EContact *eContact = E_CONTACT(item->data); QString uid = (const char*)e_contact_get_const(eContact, E_CONTACT_UID); - m_idHash.insert(uid); + m_idHash->insert(uid); } // Clean up g_list_free_full(contacts, g_object_unref); -#endif } diff --git a/engine/qcontactebook_p.h b/engine/qcontactebook_p.h index 8baf2bd..c2a7dfc 100644 --- a/engine/qcontactebook_p.h +++ b/engine/qcontactebook_p.h @@ -50,15 +50,16 @@ #include <libebook/e-book.h> #include "qcontactebookdebug_p.h" -#ifdef USE_ID_HASHING -#include "qebookidhash.h" -#endif - QTM_USE_NAMESPACE +namespace EDS { + enum IdFormat { UNKNOWN, STRING, INTEGER }; +} + /* Data shared with contact changes/added/removed callbacks */ struct cbSharedData; +class QEbookIdHash; class QContactEBook : public QObject { @@ -82,7 +83,9 @@ public: /* Reading - eContact/ebookContact to QContact methods */ QContact toQContact(EContact *eContact, const QString &managerURI, QContactManager::Error* error) const; - void initLocalIdHash(); + void initLocalIdHash() const; + inline EDS::IdFormat idFormat() const { return m_edsIdFormat; } + inline QEbookIdHash* idHash() const { return m_idHash; } signals: void contactsAdded(const QList<QContactLocalId>& contactIds); @@ -105,9 +108,9 @@ private: EBookView *m_ebookview; cbSharedData *m_cbSD; -#ifdef USE_ID_HASHING - QEbookIdHash m_idHash; -#endif + mutable EDS::IdFormat m_edsIdFormat; + mutable QEbookIdHash *m_idHash; + }; #endif -- 1.7.4.4
_______________________________________________ MeeGo-dev mailing list [email protected] http://lists.meego.com/listinfo/meego-dev http://wiki.meego.com/Mailing_list_guidelines
