Skip to content
This repository has been archived by the owner on Sep 4, 2021. It is now read-only.

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge branch 'phonenumbermatch-jb38835' into 'master'
More robust phone number matching using libphonenumber

See merge request mer-core/libcontacts!15
  • Loading branch information
Venemo committed Feb 7, 2020
2 parents cdf4012 + 2f6013c commit 06adaa5
Show file tree
Hide file tree
Showing 6 changed files with 145 additions and 21 deletions.
1 change: 1 addition & 0 deletions rpm/libcontacts-qt5.spec
Expand Up @@ -15,6 +15,7 @@ BuildRequires: pkgconfig(mlite5)
BuildRequires: pkgconfig(mlocale5)
BuildRequires: pkgconfig(mce)
BuildRequires: pkgconfig(qtcontacts-sqlite-qt5-extensions) >= 0.2.31
BuildRequires: libphonenumber-devel
BuildRequires: qt5-qttools
BuildRequires: qt5-qttools-linguist

Expand Down
63 changes: 46 additions & 17 deletions src/seasidecache.cpp
Expand Up @@ -73,6 +73,7 @@

#include <mce/dbus-names.h>
#include <mce/mode-names.h>
#include <phonenumbers/phonenumberutil.h>

QTVERSIT_USE_NAMESPACE

Expand Down Expand Up @@ -2209,16 +2210,18 @@ bool SeasideCache::updateContactIndexing(const QContact &oldContact, const QCont
resolveUnknownAddresses(address.first, address.second, item);
}

if (!m_phoneNumberIds.contains(address.second, iid))
m_phoneNumberIds.insert(address.second, iid);
CachedPhoneNumber cachedPhoneNumber(normalizePhoneNumber(phoneNumber.number()), iid);

if (!m_phoneNumberIds.contains(address.second, cachedPhoneNumber))
m_phoneNumberIds.insert(address.second, cachedPhoneNumber);
}
}

// Remove any addresses no longer available for this contact
if (!oldAddresses.isEmpty()) {
modified = true;
foreach (const StringPair &address, oldAddresses) {
m_phoneNumberIds.remove(address.second, iid);
m_phoneNumberIds.remove(address.second);
}
oldAddresses.clear();
}
Expand Down Expand Up @@ -2886,7 +2889,8 @@ void SeasideCache::addressRequestStateChanged(QContactAbstractRequest::State sta
const QList<QContact> &resolvedContacts = it.key()->contacts();

if (!resolvedContacts.isEmpty()) {
if (resolvedContacts.count() == 1) {
if (resolvedContacts.count() == 1 && data.first != QString()) {
// Return the result because it is the only resolved contact; however still filter out false positive phone number matches
item = itemById(apiId(resolvedContacts.first()), false);
} else {
// Lookup the result in our updated indexes
Expand Down Expand Up @@ -3309,23 +3313,48 @@ void SeasideCache::resolveAddress(ResolveListener *listener, const QString &firs

SeasideCache::CacheItem *SeasideCache::itemMatchingPhoneNumber(const QString &number, const QString &normalized, bool requireComplete)
{
QMultiHash<QString, quint32>::const_iterator it = m_phoneNumberIds.find(number), end = m_phoneNumberIds.constEnd();
if (it != end) {
// How many matches are there for this number?
int matchCount = 1;
QMultiHash<QString, quint32>::const_iterator matchingIt = it + 1;
while ((matchingIt != end) && (matchingIt.key() == number)) {
++matchCount;
++matchingIt;
QMultiHash<QString, CachedPhoneNumber>::const_iterator it = m_phoneNumberIds.find(number), end = m_phoneNumberIds.constEnd();
if (it == end)
return 0;

QHash<QString, quint32> possibleMatches;
::i18n::phonenumbers::PhoneNumberUtil *util = ::i18n::phonenumbers::PhoneNumberUtil::GetInstance();
::std::string normalizedStdStr = normalized.toStdString();

for (QMultiHash<QString, CachedPhoneNumber>::const_iterator matchingIt = it;
matchingIt != end && matchingIt.key() == number;
++matchingIt) {

const CachedPhoneNumber &cachedPhoneNumber = matchingIt.value();

// Bypass libphonenumber if the numbers match exactly
if (matchingIt->normalizedNumber == normalized)
return itemById(cachedPhoneNumber.iid, requireComplete);

::i18n::phonenumbers::PhoneNumberUtil::MatchType matchType =
util->IsNumberMatchWithTwoStrings(normalizedStdStr, cachedPhoneNumber.normalizedNumber.toStdString());

switch (matchType) {
case ::i18n::phonenumbers::PhoneNumberUtil::EXACT_MATCH:
// This is the optimal outcome
return itemById(cachedPhoneNumber.iid, requireComplete);
case ::i18n::phonenumbers::PhoneNumberUtil::NSN_MATCH:
case ::i18n::phonenumbers::PhoneNumberUtil::SHORT_NSN_MATCH:
// Store numbers whose NSN (national significant number) might match
// Example: if +36701234567 is calling, then 1234567 is an NSN match
possibleMatches.insert(cachedPhoneNumber.normalizedNumber, cachedPhoneNumber.iid);
break;
default:
// Either couldn't parse the number or it was NO_MATCH, ignore it
break;
}
}
if (matchCount == 1)
return itemById(*it, requireComplete);

// Choose the best match from these contacts
int bestMatchLength = 0;
CacheItem *matchItem = 0;
for ( ; matchCount > 0; ++it, --matchCount) {
if (CacheItem *item = existingItem(*it)) {
for (QHash<QString, quint32>::const_iterator matchingIt = possibleMatches.begin(); matchingIt != possibleMatches.end(); ++matchingIt) {
if (CacheItem *item = existingItem(*matchingIt)) {
int matchLength = bestPhoneNumberMatchLength(item->contact, normalized);
if (matchLength > bestMatchLength) {
bestMatchLength = matchLength;
Expand All @@ -3342,9 +3371,9 @@ SeasideCache::CacheItem *SeasideCache::itemMatchingPhoneNumber(const QString &nu
}
return matchItem;
}
}

return 0;

}

int SeasideCache::contactIndex(quint32 iid, FilterType filterType)
Expand Down
17 changes: 16 additions & 1 deletion src/seasidecache.h
Expand Up @@ -139,6 +139,21 @@ class CONTACTCACHE_EXPORT SeasideCache : public QObject
void *key;
};

struct CachedPhoneNumber
{
CachedPhoneNumber() {}
CachedPhoneNumber(const QString &n, quint32 i) : normalizedNumber(n), iid(i) {}
CachedPhoneNumber(const CachedPhoneNumber &other) : normalizedNumber(other.normalizedNumber), iid(other.iid) {}

bool operator==(const CachedPhoneNumber &other) const
{
return other.normalizedNumber == normalizedNumber && other.iid == iid;
}

QString normalizedNumber;
quint32 iid;
};

struct CacheItem
{
CacheItem() : itemData(0), iid(0), statusFlags(0), contactState(ContactAbsent), listeners(0), filterMatchRole(-1) {}
Expand Down Expand Up @@ -422,7 +437,7 @@ private slots:
QBasicTimer m_expiryTimer;
QBasicTimer m_fetchTimer;
QHash<quint32, CacheItem> m_people;
QMultiHash<QString, quint32> m_phoneNumberIds;
QMultiHash<QString, CachedPhoneNumber> m_phoneNumberIds;
QHash<QString, quint32> m_emailAddressIds;
QHash<QPair<QString, QString>, quint32> m_onlineAccountIds;
QHash<QContactId, QContact> m_contactsToSave;
Expand Down
1 change: 1 addition & 0 deletions src/src.pro
Expand Up @@ -22,6 +22,7 @@ packagesExist(mlite5) {
warning("mlite not available. Some functionality may not work as expected.")
}
PKGCONFIG += mlocale5 mce qtcontacts-sqlite-qt5-extensions
LIBS += -lphonenumber

DEFINES += CONTACTCACHE_BUILD

Expand Down
83 changes: 80 additions & 3 deletions tests/tst_resolve/tst_resolve.cpp
Expand Up @@ -60,7 +60,9 @@ public slots:
QList<QContactId> m_createdContacts;

private slots:
void resolveByPhone_data();
void resolveByPhone();
void resolveByPhoneNotFound_data();
void resolveByPhoneNotFound();
void resolveByEmail();
void resolveByEmailNotFound();
Expand Down Expand Up @@ -145,34 +147,109 @@ void tst_Resolve::makeContacts()
QVERIFY(makeContact("Daffy", "Duck", "+358470009955", "daffyd@example.com", ""));
QVERIFY(makeContact("Dafferd", "Duck", "", "daffy.d@example.com", ""));
QVERIFY(makeContact("Ernest", "Everest", "+358477758885", "", ""));
QVERIFY(makeContact("John", "Smith", "+36701234567", "", ""));
QVERIFY(makeContact("Jane", "Smith", "06207654321", "", ""));
}

void tst_Resolve::resolveByPhone_data()
{
QTest::addColumn<QString>("country");
QTest::addColumn<QString>("number");
QTest::addColumn<QString>("matchingName");

QTest::newRow("Daffy international")
<< "FI" << "+358470009955" << "Daffy";
QTest::newRow("Daffy international direct dial")
<< "FI" << "00358470009955" << "Daffy";

QTest::newRow("John international")
<< "HU" << "+36701234567" << "John";
QTest::newRow("John international direct dial")
<< "HU" << "0036701234567" << "John";
QTest::newRow("John international without plus")
<< "HU" << "36701234567" << "John";
QTest::newRow("John full national")
<< "HU" << "06701234567" << "John";

QTest::newRow("Jane international")
<< "HU" << "+36207654321" << "Jane";
QTest::newRow("Jane international without plus")
<< "HU" << "36207654321" << "Jane";
QTest::newRow("Jane full national")
<< "HU" << "06207654321" << "Jane";
}

void tst_Resolve::resolveByPhone()
{
// Note:
// When running these tests, make sure you don't have any of the above
// numbers in your contact list, as those will interfere with the results.

QFETCH(QString, country);
QFETCH(QString, number);
QFETCH(QString, matchingName);
Q_UNUSED(country)

SeasideCache::CacheItem *item;
TestResolveListener listener;
QString number("+358470009955");

item = SeasideCache::resolvePhoneNumber(&listener, number, true);
if (!item) {
QTRY_VERIFY(listener.m_resolved);
item = listener.m_item;
}

QVERIFY(item != 0);

QContactName name = item->contact.detail<QContactName>();
QCOMPARE(name.firstName(), QString::fromLatin1("Daffy"));
QCOMPARE(name.firstName(), matchingName);
}

void tst_Resolve::resolveByPhoneNotFound_data()
{
QTest::addColumn<QString>("country");
QTest::addColumn<QString>("number");

QTest::newRow("Non-existent British number")
<< "GB" << "+44123456789";
QTest::newRow("Non-existent Finnish number")
<< "FI" << "+35898765432100";

QTest::newRow("Daffy same NSN different country")
<< "HU" << "+36470009955";
QTest::newRow("Daffy same number same country different area code")
<< "FI" << "+358570009955";

QTest::newRow("John same NSN different country")
<< "HU" << "+44701234567";
QTest::newRow("John same number same country different area code")
<< "HU" << "+36201234567";

QTest::newRow("Jane same number different area code international")
<< "HU" << "+36307654321";
QTest::newRow("Jane same number different area code full national")
<< "HU" << "06307654321";
}

void tst_Resolve::resolveByPhoneNotFound()
{
// Note:
// When running these tests, make sure you don't have any of the above
// numbers in your contact list, as those will interfere with the results.

QFETCH(QString, country);
QFETCH(QString, number);
Q_UNUSED(country)

SeasideCache::CacheItem *item;
TestResolveListener listener;
QString number("+358470000000");

item = SeasideCache::resolvePhoneNumber(&listener, number, true);
if (!item) {
QTRY_VERIFY(listener.m_resolved);
item = listener.m_item;
} else {
qWarning() << "should not have resolved:" << number << "to" << item->displayLabel;
}

QCOMPARE(item, (SeasideCache::CacheItem *)0);
Expand Down
1 change: 1 addition & 0 deletions tests/tst_resolve/tst_resolve.pro
Expand Up @@ -4,6 +4,7 @@ TARGET = tst_resolve
QT += contacts-private dbus

PKGCONFIG += mlocale5
LIBS += -lphonenumber

# We need the moc output for ContactManagerEngine from sqlite-extensions
extensionsIncludePath = $$system(pkg-config --cflags-only-I qtcontacts-sqlite-qt5-extensions)
Expand Down

0 comments on commit 06adaa5

Please sign in to comment.