From cb08d760f5ba06b19b18a9cb25aeec3dfafccf77 Mon Sep 17 00:00:00 2001
From: Hugo Lefeuvre <hugo.lefeuvre@savoirfairelinux.com>
Date: Fri, 4 May 2018 17:30:59 -0400
Subject: [PATCH] contactmodel: Add banned contacts logic
The old lrc bannedaccountmodel suffers from many issues, both in its
design and functions. In order to be able to later remove this code,
migrate the banned contacts logic to the contactmodel.
contactmodel:
- add a bannedContacts list with its mutex to store uris of banned
contacts (not necessary but important for perfs).
- modify addContact() to call directly addContact() (daemon side)
when called with a banned contact. This will replace the old
remove() method from the bannedAccountModel.
- add a getBannedContacts method.
- modify slotContactAdded to maintain the bannedContacts list and
the old lrc. Also, when called with a banned contact, only refresh
the filters, do not emit contactAdded signal.
- modify slotContactRemoved to maintain the bannedContacts list.
- modify addToContacts to maintain the bannedContacts list and
update banned flags from the contactInfo.
bannedcontactmodel (old lrc):
- do not update daemon when remove is called since it collides with
the contactmodel.
_Using the bannedcontactmodel from old LRC for maintaining banned
contacts is now DEPRECATED and support for this functionnality is
planned to be dropped in a near future. Please migrate to the new
contactmodel banned contacts features._
The recommended way to unban a contact is to call addContact. This
is symmetric to the way we ban contacts (calling removeContact).
configuration manager mock:
- add a synchronization mechanism to the configuration manager mock
in order to avoid race conditions during concurrent execution of
several removeContact / addContact functions
- modify addContact to correctly behave if called with a banned
contact: Instead of re-adding the contact, banned and removed
flags should be cleared.
- modify removeContact to use find_if instead of a for loop and
erase contacts from the contacts list instead of simply removing
them.
tests:
- add a unbanContact helper.
- fix testPlaceCallWithBannedContact to check callId instead of
conversation size which is pointless because invariant.
- modify all banned contacts related tests to unban and re-check.
- add a testBanUnbanContact test.
Change-Id: I42b90559ca4baa12f0596bb927009ce6ba8b4ea3
Reviewed-by: Sebastien Blin <sebastien.blin@savoirfairelinux.com>
---
src/api/contactmodel.h | 5 +
src/bannedcontactmodel.cpp | 7 +-
src/bannedcontactmodel.h | 2 +-
src/contactmodel.cpp | 104 +++++++++++++---
test/contactmodeltester.cpp | 47 +++++++
test/contactmodeltester.h | 5 +
test/conversationmodeltester.cpp | 163 +++++++++++++++++++------
test/conversationmodeltester.h | 12 +-
test/mocks/configurationmanager_mock.h | 61 ++++++---
9 files changed, 324 insertions(+), 82 deletions(-)
diff --git a/src/api/contactmodel.h b/src/api/contactmodel.h
index 92b2bc2d..b0abbc32 100644
--- a/src/api/contactmodel.h
+++ b/src/api/contactmodel.h
@@ -78,6 +78,11 @@ public:
* @throws out_of_range exception if can't find the contact
*/
const contact::Info getContact(const std::string& contactUri) const;
+ /**
+ * get list of banned contacts.
+ * @return list of banned contacts uris as string
+ */
+ const std::list<std::string>& getBannedContacts() const;
/**
* @param contactUri
* @return empty string if no contact, else the uri in db
diff --git a/src/bannedcontactmodel.cpp b/src/bannedcontactmodel.cpp
index e4b7e881..e3683593 100644
--- a/src/bannedcontactmodel.cpp
+++ b/src/bannedcontactmodel.cpp
@@ -150,7 +150,7 @@ BannedContactModel::add(ContactMethod* cm)
* @param cm, the ContactMethod to remove from the list.
*/
void
-BannedContactModel::remove(ContactMethod* cm)
+BannedContactModel::remove(ContactMethod* cm, bool updatedaemon)
{
// Do not remove contact if contact isn't banned
auto rowIndex = d_ptr->m_lBanned.indexOf(cm);
@@ -166,7 +166,10 @@ BannedContactModel::remove(ContactMethod* cm)
return;
}
- ConfigurationManager::instance().addContact(cm->account()->id(), cm->uri());
+ if (updatedaemon) {
+ qWarning() << "deprecated method: updating daemon using BannedContactModel::remove is deprecated, please use ContactModel::addContact";
+ ConfigurationManager::instance().addContact(cm->account()->id(), cm->uri());
+ }
}
/**
diff --git a/src/bannedcontactmodel.h b/src/bannedcontactmodel.h
index 98143df0..9a5ca1b5 100644
--- a/src/bannedcontactmodel.h
+++ b/src/bannedcontactmodel.h
@@ -44,7 +44,7 @@ public:
// Helper
void add(ContactMethod* cm);
- void remove(ContactMethod* cm);
+ void remove(ContactMethod* cm, bool updatedaemon = true);
bool isBanned(ContactMethod* cm);
private:
diff --git a/src/contactmodel.cpp b/src/contactmodel.cpp
index 51e1fc0f..150f4cba 100644
--- a/src/contactmodel.cpp
+++ b/src/contactmodel.cpp
@@ -76,8 +76,9 @@ public:
* @note: the cm must corresponds to a profile in the database.
* @param cm ContactMethod.
* @param type
+ * @param banned whether contact is banned or not
*/
- void addToContacts(ContactMethod* cm, const profile::Type& type);
+ void addToContacts(ContactMethod* cm, const profile::Type& type, bool banned = false);
// Helpers
const ContactModel& linked;
@@ -86,7 +87,9 @@ public:
// Containers
ContactModel::ContactInfoMap contacts;
+ std::list<std::string> bannedContacts;
std::mutex contactsMtx_;
+ std::mutex bannedContactsMtx_;
public Q_SLOTS:
/**
@@ -192,6 +195,15 @@ ContactModel::addContact(contact::Info contactInfo)
{
auto& profile = contactInfo.profileInfo;
+ // If passed contact is a banned contact, call the daemon to unban it
+ auto it = std::find(pimpl_->bannedContacts.begin(), pimpl_->bannedContacts.end(), profile.uri);
+ if (it != pimpl_->bannedContacts.end()) {
+ qDebug("Unban-ing contact %s", profile.uri.c_str());
+ ConfigurationManager::instance().addContact(owner.id.c_str(), profile.uri.c_str());
+ // bannedContacts will be updated in slotContactAdded
+ return;
+ }
+
if ((owner.profileInfo.type != profile.type) and
(profile.type == profile::Type::RING or profile.type == profile::Type::SIP)) {
qDebug() << "ContactModel::addContact, types invalids.";
@@ -291,6 +303,12 @@ ContactModel::getContact(const std::string& contactUri) const
return pimpl_->contacts.at(contactUri);
}
+const std::list<std::string>&
+ContactModel::getBannedContacts() const
+{
+ return pimpl_->bannedContacts;
+}
+
const std::string
ContactModel::getContactProfileId(const std::string& contactUri) const
{
@@ -450,7 +468,7 @@ ContactModelPimpl::fillsWithRINGContacts() {
for (auto contact_info : contacts_vector) {
auto cm = PhoneDirectoryModel::instance().getNumber(contact_info["id"], account);
std::lock_guard<std::mutex> lk(contactsMtx_);
- addToContacts(cm, linked.owner.profileInfo.type);
+ addToContacts(cm, linked.owner.profileInfo.type, contact_info["banned"] == "true");
}
// Add pending contacts
@@ -515,11 +533,41 @@ ContactModelPimpl::slotContactAdded(const std::string& accountId, const std::str
return;
}
auto* cm = PhoneDirectoryModel::instance().getNumber(QString(contactUri.c_str()), account);
+ auto contact = contacts.find(contactUri);
+
+ bool isBanned = false;
+
{
+ // Always get contactsMtx_ lock before bannedContactsMtx_.
std::lock_guard<std::mutex> lk(contactsMtx_);
- addToContacts(cm, linked.owner.profileInfo.type);
+
+ {
+ // Check whether contact is banned or not
+ std::lock_guard<std::mutex> lk(bannedContactsMtx_);
+ auto it = std::find(bannedContacts.begin(), bannedContacts.end(), contact->second.profileInfo.uri);
+
+ isBanned = (it != bannedContacts.end());
+
+ // If contact is banned, do not re-add it, simply update its flag and the banned contacts list
+ if (isBanned) {
+ bannedContacts.erase(it);
+
+ /* Update old LRC.
+ This method should NOT make any function call that requires the contactsMtx_ lock
+ otherwise we will get into a deadlock. This is only here for old-lrc transition. */
+ account->bannedContactModel()->remove(cm, false);
+ }
+
+ addToContacts(cm, linked.owner.profileInfo.type, false);
+ }
+ }
+
+ if (isBanned) {
+ // Update the smartlist
+ linked.owner.conversationModel->refreshFilter();
+ } else {
+ emit linked.contactAdded(contactUri);
}
- emit linked.contactAdded(contactUri);
}
void
@@ -529,6 +577,7 @@ ContactModelPimpl::slotContactRemoved(const std::string& accountId, const std::s
return;
{
+ // Always get contactsMtx_ lock before bannedContactsMtx_.
std::lock_guard<std::mutex> lk(contactsMtx_);
auto contact = contacts.find(contactUri);
@@ -539,12 +588,28 @@ ContactModelPimpl::slotContactRemoved(const std::string& accountId, const std::s
auto* account = AccountModel::instance().getById(linked.owner.id.c_str());
if (not account) {
- qDebug() << "ContactModel::slotContactsAdded(), nullptr";
+ qDebug() << "ContactModel::slotContactsRemoved(), nullptr";
return;
}
auto* cm = PhoneDirectoryModel::instance().getNumber(QString(contactUri.c_str()), account);
+
+ // Update bannedContactModel from old LRC
account->bannedContactModel()->add(cm);
+
+ // Update bannedContacts index
+ bannedContacts.emplace_back(contact->second.profileInfo.uri);
} else {
+ if (contact->second.isBanned) {
+ // Contact was banned, update bannedContacts
+ std::lock_guard<std::mutex> lk(bannedContactsMtx_);
+ auto it = std::find(bannedContacts.begin(), bannedContacts.end(), contact->second.profileInfo.uri);
+ if (it == bannedContacts.end()) {
+ // should not happen
+ qDebug("ContactModel::slotContactsRemoved(): Contact is banned but not present in bannedContacts. This is most likely the result of an earlier bug.");
+ } else {
+ bannedContacts.erase(it);
+ }
+ }
database::removeContact(db, linked.owner.profileInfo.uri, contactUri);
contacts.erase(contactUri);
}
@@ -559,25 +624,22 @@ ContactModelPimpl::slotContactRemoved(const std::string& accountId, const std::s
}
void
-ContactModelPimpl::addToContacts(ContactMethod* cm, const profile::Type& type)
+ContactModelPimpl::addToContacts(ContactMethod* cm, const profile::Type& type, bool banned)
{
- if (!cm) return;
+ if (!cm) {
+ qDebug() << "addToContacts: Called with NULL contact method.";
+ }
+
auto contactUri = cm->uri().toStdString();
auto contactId = database::getProfileId(db, contactUri);
if (contactId.empty()) {
contactId = database::getOrInsertProfile(db, contactUri, "", "",
to_string(linked.owner.profileInfo.type));
}
+
auto contactInfo = database::buildContactFromProfileId(db, contactId);
contactInfo.registeredName = cm->registeredName().toStdString();
-
- auto* account = AccountModel::instance().getById(linked.owner.id.c_str());
- if (not account) {
- qDebug() << "ContactModel::addToContacts(), nullptr";
- return;
- }
-
- contactInfo.isBanned = account->bannedContactModel()->isBanned(cm);
+ contactInfo.isBanned = banned;
contactInfo.isPresent = cm->isPresent();
contactInfo.profileInfo.type = type; // Because PENDING should not be stored in the database
auto iter = contacts.find(contactInfo.profileInfo.uri);
@@ -585,6 +647,10 @@ ContactModelPimpl::addToContacts(ContactMethod* cm, const profile::Type& type)
iter->second = contactInfo;
else
contacts.emplace_hint(iter, contactInfo.profileInfo.uri, contactInfo);
+
+ if (banned) {
+ bannedContacts.emplace_back(contactUri);
+ }
}
void
@@ -668,7 +734,7 @@ ContactModelPimpl::slotIncomingCall(const std::string& fromId, const std::string
// The conversation model will create an entry and link the incomingCall.
auto* cm = PhoneDirectoryModel::instance().getNumber(QString(fromId.c_str()), account);
auto type = (linked.owner.profileInfo.type == profile::Type::RING) ? profile::Type::PENDING : profile::Type::SIP;
- addToContacts(cm, type);
+ addToContacts(cm, type, false);
emitContactAdded = true;
}
}
@@ -698,7 +764,7 @@ ContactModelPimpl::slotNewAccountMessage(std::string& accountId,
// Contact not found, load profile from database.
// The conversation model will create an entry and link the incomingCall.
auto* cm = PhoneDirectoryModel::instance().getNumber(QString(from.c_str()), account);
- addToContacts(cm, profile::Type::PENDING);
+ addToContacts(cm, profile::Type::PENDING, false);
}
}
emit linked.newAccountMessage(accountId, from, payloads);
@@ -710,7 +776,7 @@ ContactModelPimpl::slotNewAccountTransfer(long long dringId, datatransfer::Info
if (info.accountId != linked.owner.id) return;
auto* account = AccountModel::instance().getById(linked.owner.id.c_str());
if (not account) {
- qDebug() << "ContactModel::slotNewAccountMessage(), nullptr";
+ qDebug() << "ContactModel::slotNewAccountTransfer(), nullptr";
return;
}
@@ -720,7 +786,7 @@ ContactModelPimpl::slotNewAccountTransfer(long long dringId, datatransfer::Info
// Contact not found, load profile from database.
// The conversation model will create an entry and link the incomingCall.
auto* cm = PhoneDirectoryModel::instance().getNumber(QString(info.peerUri.c_str()), account);
- addToContacts(cm, profile::Type::PENDING);
+ addToContacts(cm, profile::Type::PENDING, false);
}
}
emit linked.newAccountTransfer(dringId, info);
diff --git a/test/contactmodeltester.cpp b/test/contactmodeltester.cpp
index 83193187..958fc562 100644
--- a/test/contactmodeltester.cpp
+++ b/test/contactmodeltester.cpp
@@ -48,7 +48,54 @@ ContactModelTester::ContactModelTester()
void
ContactModelTester::setUp()
{
+}
+
+void
+ContactModelTester::testBanUnbanContact()
+{
+ // "bigbadjohn" should not be in "ring1" contacts.
+ CPPUNIT_ASSERT_THROW(accInfo_.contactModel->getContact("bigbadjohn"), std::out_of_range);
+
+ // Search and add the temporaryContact
+ accInfo_.contactModel->searchContact("bigbadjohn");
+ WaitForSignalHelper(*accInfo_.contactModel,
+ SIGNAL(modelUpdated())).wait(1000);
+ auto temporaryContact = accInfo_.contactModel->getContact("");
+ std::string uri = std::string("bigbadjohn");
+ CPPUNIT_ASSERT_EQUAL(temporaryContact.profileInfo.uri, uri);
+
+ accInfo_.contactModel->addContact(temporaryContact);
+ auto contactAdded = WaitForSignalHelper(*accInfo_.contactModel,
+ SIGNAL(contactAdded(const std::string& contactUri))).wait(1000);
+ CPPUNIT_ASSERT(contactAdded);
+
+ // Ban contact
+ accInfo_.contactModel->removeContact(uri, true);
+ auto contactBanned = WaitForSignalHelper(ConfigurationManager::instance(),
+ SIGNAL(lrc::api::ConversationModel::filterChanged())).wait(1000);
+ CPPUNIT_ASSERT_EQUAL(contactBanned, true);
+
+ auto contactInfo = accInfo_.contactModel->getContact(uri);
+ CPPUNIT_ASSERT_EQUAL(contactInfo.isBanned, true);
+
+ // Re-ban contact, make sure it isn't a problem
+ accInfo_.contactModel->removeContact(uri, true);
+ contactBanned = WaitForSignalHelper(ConfigurationManager::instance(),
+ SIGNAL(lrc::api::ConversationModel::filterChanged())).wait(1000);
+ CPPUNIT_ASSERT_EQUAL(contactBanned, true);
+
+ contactInfo = accInfo_.contactModel->getContact(uri);
+ CPPUNIT_ASSERT_EQUAL(contactInfo.isBanned, true);
+
+ // Unban contact, make sure it worked
+ contactInfo = accInfo_.contactModel->getContact(uri);
+ accInfo_.contactModel->addContact(contactInfo);
+ bool contactUnbanned = WaitForSignalHelper(ConfigurationManager::instance(),
+ SIGNAL(lrc::api::ConversationModel::filterChanged())).wait(1000);
+ CPPUNIT_ASSERT_EQUAL(contactUnbanned, true);
+ contactInfo = accInfo_.contactModel->getContact(uri);
+ CPPUNIT_ASSERT_EQUAL(contactInfo.isBanned, false);
}
void
diff --git a/test/contactmodeltester.h b/test/contactmodeltester.h
index 4ea98736..f4d6c1de 100644
--- a/test/contactmodeltester.h
+++ b/test/contactmodeltester.h
@@ -52,6 +52,7 @@ class ContactModelTester : public CppUnit::TestFixture {
CPPUNIT_TEST(testRmSIPContact);
CPPUNIT_TEST(testRmTemporaryContact);
CPPUNIT_TEST(testCountPendingRequests);
+ CPPUNIT_TEST(testBanUnbanContact);
CPPUNIT_TEST_SUITE_END();
public:
@@ -116,6 +117,10 @@ public:
* Count contact requests when banned contacts exists
*/
void testCountPendingRequestsWithBlockedContact();
+ /**
+ * Try to ban and unban contacts
+ */
+ void testBanUnbanContact();
/**
* Method automatically called after each test by CppUnit
*/
diff --git a/test/conversationmodeltester.cpp b/test/conversationmodeltester.cpp
index a66240ef..336b8fcc 100644
--- a/test/conversationmodeltester.cpp
+++ b/test/conversationmodeltester.cpp
@@ -70,35 +70,55 @@ ConversationModelTester::testAddValidConversation()
void
ConversationModelTester::testPlaceCallWithBannedContact()
{
- // bannedContact should not be in contacts
+ // badguy0 should not be in contacts
CPPUNIT_ASSERT(!isAContact("badguy0"));
+ // so, add him to contacts
auto uri = addToContacts("badguy0");
-
- // badguy0 should now be in contacts
CPPUNIT_ASSERT(isAContact("badguy0"));
- // Ban badguy0
+ // and ban him
banContact(uri);
auto contactInfo = accInfo_.contactModel->getContact(uri);
CPPUNIT_ASSERT_EQUAL(contactInfo.isBanned, true);
- // So, now that badguy0 is banned, calling him should be forbidden
+ // find conversation
auto conversations = accInfo_.conversationModel->allFilteredConversations();
- bool conversationExists = false;
- for (const auto& conversation: conversations) {
- if (std::find(conversation.participants.begin(), conversation.participants.end(), contactInfo.profileInfo.uri) != conversation.participants.end()) {
- conversationExists = true;
- // Try to call banned contact
- auto baseInteractionsSize = conversation.interactions.size();
- accInfo_.conversationModel->placeCall(conversation.uid);
- // Make sure call didn't succeed
- CPPUNIT_ASSERT_EQUAL((int)baseInteractionsSize, (int)conversation.interactions.size());
- break;
- }
- }
+ auto conversation = std::find_if(conversations.begin(), conversations.end(),
+ [&contactInfo](const lrc::api::conversation::Info& conversation) {
+ return std::find(conversation.participants.begin(),
+ conversation.participants.end(),
+ contactInfo.profileInfo.uri) != conversation.participants.end();
+ });
- CPPUNIT_ASSERT(conversationExists);
+ CPPUNIT_ASSERT(conversation != conversations.end());
+
+ // now that badguy0 is banned, calling him should be forbidden
+ auto baseInteractionsSize = conversation->interactions.size();
+ accInfo_.conversationModel->placeCall(conversation->uid);
+
+ // make sure call didn't succeed
+ CPPUNIT_ASSERT(conversation->callId.empty());
+
+ // unban badguy0
+ unbanContact(uri);
+ contactInfo = accInfo_.contactModel->getContact(uri);
+ CPPUNIT_ASSERT_EQUAL(contactInfo.isBanned, false);
+
+ // call again, should succeed
+ accInfo_.conversationModel->placeCall(conversation->uid);
+
+ // make sure call succeeded
+ conversations = accInfo_.conversationModel->allFilteredConversations();
+ conversation = std::find_if(conversations.begin(), conversations.end(),
+ [&contactInfo](const lrc::api::conversation::Info& conversation) {
+ return std::find(conversation.participants.begin(),
+ conversation.participants.end(),
+ contactInfo.profileInfo.uri) != conversation.participants.end();
+ });
+
+ CPPUNIT_ASSERT(conversation != conversations.end());
+ CPPUNIT_ASSERT(!conversation->callId.empty());
}
void
@@ -109,14 +129,14 @@ ConversationModelTester::testFilterBannedContact()
CPPUNIT_ASSERT(!isAContact("bannedContacte"));
CPPUNIT_ASSERT(!isAContact("bannedContac"));
- auto newContactUri = addToContacts("bannedContact");
+ auto uri = addToContacts("bannedContact");
// bannedContact now should be in contacts
CPPUNIT_ASSERT(isAContact("bannedContact"));
- // Ban bannedContact
- banContact(newContactUri);
- auto contactInfo = accInfo_.contactModel->getContact(newContactUri);
+ // ban bannedContact
+ banContact(uri);
+ auto contactInfo = accInfo_.contactModel->getContact(uri);
CPPUNIT_ASSERT_EQUAL(contactInfo.isBanned, true);
// Make sure bannedContact doesn't appear is non-perfect-match filter searches
@@ -142,12 +162,39 @@ ConversationModelTester::testFilterBannedContact()
CPPUNIT_ASSERT_EQUAL(1, (int)accInfo_.conversationModel->allFilteredConversations().size());
isTemporary = accInfo_.conversationModel->filteredConversation(0).participants.front() == "";
CPPUNIT_ASSERT(!isTemporary);
+
+ // Unban bannedContact
+ unbanContact(uri);
+ contactInfo = accInfo_.contactModel->getContact(uri);
+ CPPUNIT_ASSERT_EQUAL(contactInfo.isBanned, false);
+
+ // Make sure bannedContact appears is non-perfect-match filter searches
+ // We expect 2 (temporary item + bannedContact)
+ accInfo_.conversationModel->setFilter("bannedContac");
+ WaitForSignalHelper(*accInfo_.contactModel,
+ SIGNAL(modelUpdated())).wait(1000);
+ CPPUNIT_ASSERT_EQUAL(2, (int)accInfo_.conversationModel->allFilteredConversations().size());
+
+ // Here we expect 1 (temporary item)
+ accInfo_.conversationModel->setFilter("bannedContacte");
+ WaitForSignalHelper(*accInfo_.contactModel,
+ SIGNAL(modelUpdated())).wait(1000);
+ CPPUNIT_ASSERT_EQUAL(1, (int)accInfo_.conversationModel->allFilteredConversations().size());
+
+ // Make sure bannedContact appears in perfect-match filter searches
+ // We expect 1 (bannedContact)
+ accInfo_.conversationModel->setFilter("bannedContact");
+ WaitForSignalHelper(*accInfo_.contactModel,
+ SIGNAL(modelUpdated())).wait(1000);
+ CPPUNIT_ASSERT_EQUAL(1, (int)accInfo_.conversationModel->allFilteredConversations().size());
+ isTemporary = accInfo_.conversationModel->filteredConversation(0).participants.front() == "";
+ CPPUNIT_ASSERT(!isTemporary);
}
void
ConversationModelTester::testSendMessageToBannedContact()
{
- // bannedContact should not be in contacts
+ // badguy1 should not be in contacts
CPPUNIT_ASSERT(!isAContact("badguy1"));
auto uri = addToContacts("badguy1");
@@ -162,20 +209,41 @@ ConversationModelTester::testSendMessageToBannedContact()
// So, now that badguy is banned, sending a message should be forbidden
auto conversations = accInfo_.conversationModel->allFilteredConversations();
- bool conversationExists = false;
- for (const auto& conversation: conversations) {
- if (std::find(conversation.participants.begin(), conversation.participants.end(), contactInfo.profileInfo.uri) != conversation.participants.end()) {
- conversationExists = true;
- // Try to send message to banned contact
- auto baseInteractionsSize = conversation.interactions.size();
- accInfo_.conversationModel->sendMessage(conversation.uid, "Hello banned !");
- // Make sure message didn't arrive (but contact added is already here)
- CPPUNIT_ASSERT_EQUAL((int)baseInteractionsSize, (int)conversation.interactions.size());
- break;
- }
- }
+ auto conversation = std::find_if(conversations.begin(), conversations.end(),
+ [&contactInfo](const lrc::api::conversation::Info& conversation) {
+ return std::find(conversation.participants.begin(),
+ conversation.participants.end(),
+ contactInfo.profileInfo.uri) != conversation.participants.end();
+ });
- CPPUNIT_ASSERT(conversationExists);
+ CPPUNIT_ASSERT(conversation != conversations.end());
+
+ // Try to send message to banned contact
+ auto baseInteractionsSize = conversation->interactions.size();
+ accInfo_.conversationModel->sendMessage(conversation->uid, "Hello banned !");
+ // Make sure message didn't arrive (but contact added is already here)
+ CPPUNIT_ASSERT_EQUAL((int)baseInteractionsSize, (int)conversation->interactions.size());
+
+ // Unban badguy1
+ unbanContact(uri);
+ contactInfo = accInfo_.contactModel->getContact(uri);
+ CPPUNIT_ASSERT_EQUAL(contactInfo.isBanned, false);
+
+ // Now send message again, should succeed
+ accInfo_.conversationModel->sendMessage(conversation->uid, "Hello unbanned !");
+
+ // Make sure message arrived
+ conversations = accInfo_.conversationModel->allFilteredConversations();
+ conversation = std::find_if(conversations.begin(), conversations.end(),
+ [&contactInfo](const lrc::api::conversation::Info& conversation) {
+ return std::find(conversation.participants.begin(),
+ conversation.participants.end(),
+ contactInfo.profileInfo.uri) != conversation.participants.end();
+ });
+
+ CPPUNIT_ASSERT(conversation != conversations.end());
+
+ CPPUNIT_ASSERT_EQUAL((int)baseInteractionsSize + 1, (int)conversation->interactions.size());
}
void
@@ -375,11 +443,16 @@ ConversationModelTester::testReceiveMessageAndSetRead()
void
ConversationModelTester::testPlaceCall()
{
+ // Get first conversation and make sure it is empty
auto conversations = accInfo_.conversationModel->allFilteredConversations();
CPPUNIT_ASSERT(conversations.size() != 0);
auto firstConversation = accInfo_.conversationModel->filteredConversation(0);
CPPUNIT_ASSERT(firstConversation.callId.empty());
+
+ // Place a call
accInfo_.conversationModel->placeCall(firstConversation.uid);
+
+ // Get first conversation again and make sure it isn't empty anymore (call suceeded)
conversations = accInfo_.conversationModel->allFilteredConversations();
CPPUNIT_ASSERT(conversations.size() != 0);
auto newConv = accInfo_.conversationModel->filteredConversation(0);
@@ -487,7 +560,7 @@ ConversationModelTester::tearDown()
}
bool
-ConversationModelTester::hasConversationWithContact(std::string uri)
+ConversationModelTester::hasConversationWithContact(const std::string& uri)
{
auto conversations = accInfo_.conversationModel->allFilteredConversations();
auto i = std::find_if(conversations.begin(), conversations.end(),
@@ -500,7 +573,7 @@ ConversationModelTester::hasConversationWithContact(std::string uri)
}
void
-ConversationModelTester::banContact(std::string uri)
+ConversationModelTester::banContact(const std::string& uri)
{
accInfo_.contactModel->removeContact(uri, true);
auto contactBanned = WaitForSignalHelper(ConfigurationManager::instance(),
@@ -508,14 +581,24 @@ ConversationModelTester::banContact(std::string uri)
CPPUNIT_ASSERT_EQUAL(contactBanned, true);
}
+void
+ConversationModelTester::unbanContact(const std::string& uri)
+{
+ auto contactInfo = accInfo_.contactModel->getContact(uri);
+ accInfo_.contactModel->addContact(contactInfo);
+ auto contactUnbanned = WaitForSignalHelper(ConfigurationManager::instance(),
+ SIGNAL(lrc::api::ConversationModel::filterChanged())).wait(2000);
+ CPPUNIT_ASSERT_EQUAL(contactUnbanned, true);
+}
+
bool
-ConversationModelTester::isAContact(std::string uri)
+ConversationModelTester::isAContact(const std::string& uri)
{
return !accInfo_.contactModel->getContactProfileId(uri).empty();
}
std::string
-ConversationModelTester::addToContacts(std::string username)
+ConversationModelTester::addToContacts(const std::string& username)
{
// Search contact
accInfo_.conversationModel->setFilter(username);
diff --git a/test/conversationmodeltester.h b/test/conversationmodeltester.h
index 07ff662e..2aac71ff 100644
--- a/test/conversationmodeltester.h
+++ b/test/conversationmodeltester.h
@@ -137,19 +137,23 @@ protected:
/**
* Ban contact with passed uri
*/
- void banContact(std::string uri);
+ void banContact(const std::string& uri);
+ /**
+ * Unban contact with passed uri
+ */
+ void unbanContact(const std::string& uri);
/**
* Return whether passed uri already maps to a contact or not
*/
- bool isAContact(std::string uri);
+ bool isAContact(const std::string& uri);
/**
* Add passed usename to contacts and return its uri
*/
- std::string addToContacts(std::string username);
+ std::string addToContacts(const std::string& username);
/**
* Return whether a converation with passed contact uri exists or not
*/
- bool hasConversationWithContact(std::string uri);
+ bool hasConversationWithContact(const std::string& uri);
};
} // namespace test
diff --git a/test/mocks/configurationmanager_mock.h b/test/mocks/configurationmanager_mock.h
index ea3eee0d..f23d418b 100644
--- a/test/mocks/configurationmanager_mock.h
+++ b/test/mocks/configurationmanager_mock.h
@@ -52,6 +52,7 @@ class ConfigurationManagerInterface: public QObject
private:
QMap<QString, VectorMapStringString> accountToContactsMap;
QStringList availableContacts_;
+ std::mutex contactsMtx_;
public:
@@ -623,30 +624,58 @@ public Q_SLOTS: // METHODS
{
if (getAccountList().indexOf(accountId) == -1) return;
auto contacts = accountToContactsMap[accountId];
- for (auto c = 0 ; c < contacts.size() ; ++c) {
- if (contacts.at(c)["id"] == uri) {
- if (ban) {
- contacts[c].insert("removed", "true");
- contacts[c].insert("banned", "true");
- } else {
- contacts.remove(c);
- }
- emit contactRemoved(accountId, uri, ban);
+
+ {
+ std::lock_guard<std::mutex> lk(contactsMtx_);
+ auto i = std::find_if(
+ contacts.begin(), contacts.end(),
+ [&uri](auto contact) {
+ return contact["id"] == uri;
+ });
+
+ if (i == contacts.end()) {
return;
}
+
+ if (ban) {
+ i->insert("removed", "true");
+ i->insert("banned", "true");
+ } else {
+ contacts.erase(i);
+ }
}
+
+ emit contactRemoved(accountId, uri, ban);
}
void addContact(const QString &accountId, const QString &uri)
{
if (getAccountList().indexOf(accountId) == -1) return;
- auto contact = QMap<QString, QString>();
- contact.insert("id", uri);
- contact.insert("added", "true");
- contact.insert("removed", "false");
- contact.insert("confirmed", "true");
- contact.insert("banned", "false");
- accountToContactsMap[accountId].push_back(contact);
+ auto& cm = accountToContactsMap[accountId];
+
+ {
+ std::lock_guard<std::mutex> lk(contactsMtx_);
+ auto i = std::find_if(
+ cm.begin(), cm.end(),
+ [&uri](auto contact) {
+ return contact["id"] == uri;
+ });
+
+ if (i != cm.end()) {
+ // Contact is already there, erase it before adding it back.
+ // This is important to reset the banned/removed flags.
+ cm.erase(i);
+ }
+
+ auto contact = QMap<QString, QString>();
+ contact.insert("id", uri);
+ contact.insert("added", "true");
+ contact.insert("removed", "false");
+ contact.insert("confirmed", "true");
+ contact.insert("banned", "false");
+ cm.push_back(contact);
+ }
+
emit contactAdded(accountId, uri, true);
}
--
GitLab