From a1f0727765529c9f2258ec6d7f1af054e731e3a1 Mon Sep 17 00:00:00 2001
From: Emmanuel Lepage Vallee <elv1313@gmail.com>
Date: Wed, 3 May 2017 02:26:22 -0400
Subject: [PATCH] contactMethod: Prevent duplication of registered name CMs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
**WARNING** This makes the setRegisteredName method private and
thus break the API. That method could not stay public as this
would potentially corrupt the indexes.
This patch adds the registered names to the global name
directory. If `::getNumber()` is used with a known registered
name, no new CM are created.
This patch fixes the "most common" case, but doesn't implement the
following corner cases:
* If the registered name is already part of the directory, but
not fully linked to an account and/or RingId.
* If there is multiple name service
For the multiple name service, the
`NameDirectory::registeredNameFound` signal will need to use full
URIs for the names (ring:foo@ns.ring.cx). This is out of this
patch scope and will have side effects that needs to be addressed
in each clients.
Change-Id: If06561dc4cb56453cc77092f4c30c633e852b7fd
Reviewed-by: Nicolas Jäger <nicolas.jager@savoirfairelinux.com>
---
src/contactmethod.cpp | 24 ++++++++++++++++--
src/contactmethod.h | 5 ++--
src/phonedirectorymodel.cpp | 46 ++++++++++++++++++++++++++++++++++-
src/private/contactmethod_p.h | 2 ++
4 files changed, 72 insertions(+), 5 deletions(-)
diff --git a/src/contactmethod.cpp b/src/contactmethod.cpp
index cb0a50d6..fd39ec80 100644
--- a/src/contactmethod.cpp
+++ b/src/contactmethod.cpp
@@ -91,6 +91,11 @@ void ContactMethodPrivate::rebased(ContactMethod* other)
emit n->rebased(other);
}
+void ContactMethodPrivate::registeredNameSet(const QString& name)
+{
+ foreach (ContactMethod* n, m_lParents)
+ emit n->registeredNameSet(name);
+}
const ContactMethod* ContactMethod::BLANK()
{
@@ -320,9 +325,24 @@ void ContactMethod::setTracked(bool track)
}
/// Set the registered name
-void ContactMethod::setRegisteredName(const QString& registeredName)
+void ContactMethodPrivate::setRegisteredName(const QString& registeredName)
{
- d_ptr->m_RegisteredName = registeredName;
+ if (registeredName.isEmpty() || registeredName == m_RegisteredName)
+ return;
+
+ // If this happens, better create another ContactMethod manually to avoid
+ // all the corner cases. Doing this, for example, allows to keep track of
+ // the name in the private index of unique names kept by the
+ // PhoneDirectoryModel
+ if (!m_RegisteredName.isEmpty()) {
+ qWarning() << "A registered name is already set for this ContactMethod"
+ << m_RegisteredName << registeredName;
+ return;
+ }
+
+ m_RegisteredName = registeredName;
+ registeredNameSet(registeredName);
+ changed();
}
///Allow phonedirectorymodel to change presence status
diff --git a/src/contactmethod.h b/src/contactmethod.h
index f9e725e6..f7af5b61 100644
--- a/src/contactmethod.h
+++ b/src/contactmethod.h
@@ -88,7 +88,7 @@ public:
Q_PROPERTY(URI::ProtocolHint protocolHint READ protocolHint )
Q_PROPERTY(bool isReachable READ isReachable )
Q_PROPERTY(Certificate* certificate READ certificate )
- Q_PROPERTY(QString registeredName READ registeredName WRITE setRegisteredName )
+ Q_PROPERTY(QString registeredName READ registeredName NOTIFY registeredNameSet )
Q_PROPERTY(QString getBestId READ getBestId )
// Q_PROPERTY(QHash<QString,int> alternativeNames READ alternativeNames )
@@ -145,7 +145,6 @@ public:
Q_INVOKABLE void setAccount (Account* account );
Q_INVOKABLE void setPerson (Person* contact );
Q_INVOKABLE void setTracked (bool track );
- Q_INVOKABLE void setRegisteredName(const QString& registeredName);
void setCategory (NumberCategory* cat );
void setBookmarked (bool bookmarked );
void setUid (const QString& uri );
@@ -232,6 +231,8 @@ Q_SIGNALS:
void contactChanged(Person* newContact, Person* oldContact);
/// The number of unread text messages has changed
void unreadTextMessageCountChanged();
+ /// When the registered name is set
+ void registeredNameSet(const QString& name);
};
Q_DECLARE_METATYPE(ContactMethod*)
diff --git a/src/phonedirectorymodel.cpp b/src/phonedirectorymodel.cpp
index a598ed87..8acf19ff 100644
--- a/src/phonedirectorymodel.cpp
+++ b/src/phonedirectorymodel.cpp
@@ -36,6 +36,7 @@
#include "collectioninterface.h"
#include "dbus/presencemanager.h"
#include "globalinstances.h"
+#include "private/contactmethod_p.h"
#include "interfaces/pixmapmanipulatori.h"
#include "personmodel.h"
#include "dbus/configurationmanager.h"
@@ -322,6 +323,7 @@ void PhoneDirectoryModelPrivate::setAccount(ContactMethod* number, Account* acco
wrap = new NumberWrapper();
m_hDirectory [extendedUri] = wrap;
m_hSortedNumbers[extendedUri] = wrap;
+ wrap->numbers << number;
}
else {
@@ -817,13 +819,55 @@ PhoneDirectoryModelPrivate::slotRegisteredNameFound(const Account* account, Name
foreach (ContactMethod* cm, wrap->numbers) {
if (cm->account() == account) {
cm->incrementAlternativeName(name, QDateTime::currentDateTime().toTime_t());
- cm->setRegisteredName(name);
+ cm->d_ptr->setRegisteredName(name);
+
+ // Add the CM to the directory using the registered name too.
+ // Note that in theory the wrapper can exist already if the
+ // user was either offline in a call attempt or if there is a
+ // collision with a SIP account.
+ if (!m_hDirectory.contains(name)) {
+ //TODO support multiple name service, use proper URIs for names
+ auto wrap2 = new NumberWrapper();
+ m_hDirectory [name] = wrap2;
+ m_hSortedNumbers[name] = wrap2;
+ wrap2->numbers << cm;
+ }
+ else {
+ auto wrapper = m_hDirectory.value(name);
+ // Merge the existing CMs now that it is known that the RingId match the username
+ foreach(ContactMethod* n, wrapper->numbers) {
+
+ // If the account is the same and (as we know) it is a registered name
+ // there is 100% porbability of match
+ const bool compAccount = n->account() &&
+ n->account() == cm->account();
+
+ // Less certain, but close enough. We have a contact with a phone
+ // number corresponding with a registeredName and `ring:` in front.
+ // it *could* use a different name service. Anyway, for now this
+ // isn't widespread enough to care.
+ const bool compContact = (!n->account()) && n->contact() &&
+ n->uri().schemeType() == URI::SchemeType::RING;
+
+ if (n != cm && (compAccount || compContact)) {
+ n->merge(cm);
+ }
+ }
+ }
+
+ // Only add it once
+ if (!m_hDirectory[name]->numbers.indexOf(cm)) {
+ //TODO check if some deduplication can be performed
+ m_hDirectory[name]->numbers << cm;
+ }
} else {
qDebug() << "registered name: uri matches but not account" << name << address << account << cm->account();
}
}
} else {
// got a registered name for a CM which hasn't been created yet
+ // This can be left as-is to save memory. Those CMs are never freed.
+ // It is generally preferred to create as little as possible.
}
}
diff --git a/src/private/contactmethod_p.h b/src/private/contactmethod_p.h
index b9d8a023..4e3be2c7 100644
--- a/src/private/contactmethod_p.h
+++ b/src/private/contactmethod_p.h
@@ -59,10 +59,12 @@ public:
void trackedChanged(bool);
void primaryNameChanged(const QString& name);
void rebased(ContactMethod* other);
+ void registeredNameSet(const QString& registeredName);
//Helpers
void setTextRecording(Media::TextRecording* r);
void setCertificate (Certificate*);
+ void setRegisteredName(const QString& registeredName);
private:
ContactMethod* q_ptr;
--
GitLab