Change bare JID presence lookup code to ignore priorities

Before presence handling code was handled by both, the ContactRosterItem in Swift and the PresenceOracle in Swiften. The ContactRosterItem also considered the presence priority for deciding what presence to show for a bare JID. With this code all full or bare JID presence requests are finally handled by the PresenceOracle. For bare JIDs it is looked up to a presence of one of the available resources of that JID regardless of the priorities. Test-Information: Adjusted tests according to above description and documentation in PresenceOracle. Change-Id: I972a4574f476cdf4d4b5593a035eb1c25ef2f8ba
author: Tobias Markmann <tm@ayena.de> 2015-10-27 16:20:09 (GMT)
committer: Tobias Markmann <tm@ayena.de> 2015-10-30 12:34:52 (GMT)
commit: 0f5ef716a50c8d9761cafda12aacf818cdfd6353 (patch)
tree: 4ed80dedc2fa8b04036eb7567282094ea8fe07f1 /Swift/Controllers
parent: 4320235bef1b601856b295a8d6411d1898048802 (diff)
download: swift-0f5ef716a50c8d9761cafda12aacf818cdfd6353.zip
swift-0f5ef716a50c8d9761cafda12aacf818cdfd6353.tar.bz2
8 files changed, 64 insertions, 80 deletions
diff --git a/Swift/Controllers/Chat/ChatController.cpp b/Swift/Controllers/Chat/ChatController.cpp
index bedc427..51c7349 100644
--- a/Swift/Controllers/Chat/ChatController.cpp
+++ b/Swift/Controllers/Chat/ChatController.cpp
@@ -71,7 +71,7 @@ ChatController::ChatController(const JID& self, StanzaChannel* stanzaChannel, IQ
 		theirPresence = presenceOracle->getLastPresence(contact);
 	} else {
 		startMessage = str(format(QT_TRANSLATE_NOOP("", "Starting chat with %1% - %2%")) % nick % contact.toBare().toString());
-		theirPresence = contact.isBare() ? presenceOracle->getHighestPriorityPresence(contact.toBare()) : presenceOracle->getLastPresence(contact);
+		theirPresence = contact.isBare() ? presenceOracle->getAccountPresence(contact) : presenceOracle->getLastPresence(contact);
 	}
 	Idle::ref idle;
 	if (theirPresence && (idle = theirPresence->getPayload<Idle>())) {
@@ -143,7 +143,7 @@ void ChatController::setToJID(const JID& jid) {
 	if (isInMUC_) {
 		presence = presenceOracle_->getLastPresence(jid);
 	} else {
-		presence = jid.isBare() ? presenceOracle_->getHighestPriorityPresence(jid.toBare()) : presenceOracle_->getLastPresence(jid);
+		presence = jid.isBare() ? presenceOracle_->getAccountPresence(jid.toBare()) : presenceOracle_->getLastPresence(jid);
 	}
 	chatStateNotifier_->setContactIsOnline(presence && presence->getType() == Presence::Available);
 	handleBareJIDCapsChanged(toJID_);
@@ -464,18 +464,18 @@ std::string ChatController::getStatusChangeString(boost::shared_ptr<Presence> pr
 }
 
 void ChatController::handlePresenceChange(boost::shared_ptr<Presence> newPresence) {
-	bool me = false;
-	if (toJID_.isBare()) {
-		newPresence = presenceOracle_->getHighestPriorityPresence(toJID_);
-		if ((newPresence ? newPresence->getShow() : StatusShow::None) != lastShownStatus_) {
-			me = true;
-		}
-	} else if (toJID_.equals(newPresence->getFrom(), JID::WithResource)) {
-		me = true;
+	bool relevantPresence = false;
+
+	if (toJID_.equals(newPresence->getFrom(), JID::WithoutResource)) {
+		// Presence matches ChatController JID.
+		newPresence = presenceOracle_->getAccountPresence(toJID_);
+		relevantPresence = true;
 	}
-	if (!me) {
+
+	if (!relevantPresence) {
 		return;
 	}
+
 	if (!newPresence) {
 		newPresence = boost::make_shared<Presence>();
 		newPresence->setType(Presence::Unavailable);
diff --git a/Swift/Controllers/Chat/ChatsManager.cpp b/Swift/Controllers/Chat/ChatsManager.cpp
index 343f490..8a06333 100644
--- a/Swift/Controllers/Chat/ChatsManager.cpp
+++ b/Swift/Controllers/Chat/ChatsManager.cpp
@@ -287,7 +287,7 @@ ChatListWindow::Chat ChatsManager::updateChatStatusAndAvatarHelper(const ChatLis
 		if (avatarManager_) {
 			fixedChat.avatarPath = avatarManager_->getAvatarPath(fixedChat.jid);
 		}
-		Presence::ref presence = presenceOracle_->getHighestPriorityPresence(fixedChat.jid.toBare());
+		Presence::ref presence = presenceOracle_->getAccountPresence(fixedChat.jid.toBare());
 		fixedChat.statusType = presence ? presence->getShow() : StatusShow::None;
 	}
 	return fixedChat;
@@ -409,7 +409,7 @@ ChatListWindow::Chat ChatsManager::createChatListChatItem(const JID& jid, const
 			unreadCount = controller->getUnreadCount();
 		}
 		JID bareishJID = mucRegistry_->isMUC(jid.toBare()) ? jid : jid.toBare();
-		Presence::ref presence = presenceOracle_->getHighestPriorityPresence(bareishJID);
+		Presence::ref presence = presenceOracle_->getAccountPresence(bareishJID);
 		StatusShow::Type type = presence ? presence->getShow() : StatusShow::None;
 		boost::filesystem::path avatarPath = avatarManager_ ? avatarManager_->getAvatarPath(bareishJID) : boost::filesystem::path();
 		return ChatListWindow::Chat(bareishJID, nickResolver_->jidToNick(bareishJID), activity, unreadCount, type, avatarPath, false, privateMessage);
diff --git a/Swift/Controllers/Chat/UserSearchController.cpp b/Swift/Controllers/Chat/UserSearchController.cpp
index 454a110..291472f 100644
--- a/Swift/Controllers/Chat/UserSearchController.cpp
+++ b/Swift/Controllers/Chat/UserSearchController.cpp
@@ -305,7 +305,7 @@ Contact::ref UserSearchController::convertJIDtoContact(const JID& jid) {
 	}
 
 	// presence lookup
-	Presence::ref presence = presenceOracle_->getHighestPriorityPresence(jid);
+	Presence::ref presence = presenceOracle_->getAccountPresence(jid);
 	if (presence) {
 		contact->statusType = presence->getShow();
 	} else {
diff --git a/Swift/Controllers/ContactsFromXMPPRoster.cpp b/Swift/Controllers/ContactsFromXMPPRoster.cpp
index d3d7364..1cab9b1 100644
--- a/Swift/Controllers/ContactsFromXMPPRoster.cpp
+++ b/Swift/Controllers/ContactsFromXMPPRoster.cpp
@@ -5,16 +5,15 @@
  */
 
 /*
- * Copyright (c) 2014 Isode Limited.
+ * Copyright (c) 2014-2015 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
 
 #include <Swift/Controllers/ContactsFromXMPPRoster.h>
 
-#include <Swiften/Base/foreach.h>
-
 #include <Swiften/Avatars/AvatarManager.h>
+#include <Swiften/Base/foreach.h>
 #include <Swiften/Presence/PresenceOracle.h>
 #include <Swiften/Roster/XMPPRoster.h>
 #include <Swiften/Roster/XMPPRosterItem.h>
@@ -32,7 +31,7 @@ std::vector<Contact::ref> ContactsFromXMPPRoster::getContacts(bool /*withMUCNick
 	std::vector<XMPPRosterItem> rosterItems = roster_->getItems();
 	foreach(const XMPPRosterItem& rosterItem, rosterItems) {
 		Contact::ref contact = boost::make_shared<Contact>(rosterItem.getName().empty() ? rosterItem.getJID().toString() : rosterItem.getName(), rosterItem.getJID(), StatusShow::None,"");
-		contact->statusType = presenceOracle_->getHighestPriorityPresence(contact->jid) ? presenceOracle_->getHighestPriorityPresence(contact->jid)->getShow() : StatusShow::None;
+		contact->statusType = presenceOracle_->getAccountPresence(contact->jid) ? presenceOracle_->getAccountPresence(contact->jid)->getShow() : StatusShow::None;
 		contact->avatarPath = avatarManager_->getAvatarPath(contact->jid);
 		results.push_back(contact);
 	}
diff --git a/Swift/Controllers/Roster/ContactRosterItem.cpp b/Swift/Controllers/Roster/ContactRosterItem.cpp
index fd6d1cd..ae05aee 100644
--- a/Swift/Controllers/Roster/ContactRosterItem.cpp
+++ b/Swift/Controllers/Roster/ContactRosterItem.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010 Isode Limited.
+ * Copyright (c) 2010-2015 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
@@ -8,9 +8,10 @@
 
 #include <boost/date_time/posix_time/posix_time.hpp>
 
-#include <Swiften/Base/foreach.h>
 #include <Swiften/Base/DateTime.h>
+#include <Swiften/Base/foreach.h>
 #include <Swiften/Elements/Idle.h>
+
 #include <Swift/Controllers/Intl.h>
 #include <Swift/Controllers/Roster/GroupRosterItem.h>
 
@@ -26,11 +27,11 @@ ContactRosterItem::~ContactRosterItem() {
 }
 
 StatusShow::Type ContactRosterItem::getStatusShow() const {
-	return shownPresence_ ? shownPresence_->getShow() : StatusShow::None;
+	return presence_ ? presence_->getShow() : StatusShow::None;
 }
 
 StatusShow::Type ContactRosterItem::getSimplifiedStatusShow() const {
-	switch (shownPresence_ ? shownPresence_->getShow() : StatusShow::None) {
+	switch (presence_ ? presence_->getShow() : StatusShow::None) {
 		case StatusShow::Online: return StatusShow::Online;
 		case StatusShow::Away: return StatusShow::Away;
 	 	case StatusShow::XA: return StatusShow::Away;
@@ -43,11 +44,11 @@ StatusShow::Type ContactRosterItem::getSimplifiedStatusShow() const {
 }
 
 std::string ContactRosterItem::getStatusText() const {
-	return shownPresence_ ? shownPresence_->getStatus() : "";
+	return presence_ ? presence_->getStatus() : "";
 }
 
 std::string ContactRosterItem::getIdleText() const {
-	Idle::ref idle = shownPresence_ ? shownPresence_->getPayload<Idle>() : Idle::ref();
+	Idle::ref idle = presence_ ? presence_->getPayload<Idle>() : Idle::ref();
 	if (!idle || idle->getSince().is_not_a_date_time()) {
 		return "";
 	} else {
@@ -56,9 +57,9 @@ std::string ContactRosterItem::getIdleText() const {
 }
 
 std::string ContactRosterItem::getOfflineSinceText() const {
-	if (offlinePresence_) {
-		boost::optional<boost::posix_time::ptime> delay = offlinePresence_->getTimestamp();
-		if (offlinePresence_->getType() == Presence::Unavailable && delay) {
+	if (presence_ && presence_->getType() == Presence::Unavailable) {
+		boost::optional<boost::posix_time::ptime> delay = presence_->getTimestamp();
+		if (delay) {
 			return dateTimeToLocalString(*delay);
 		}
 	}
@@ -88,42 +89,13 @@ const JID& ContactRosterItem::getDisplayJID() const {
 
 typedef std::pair<std::string, boost::shared_ptr<Presence> > StringPresencePair;
 
-void ContactRosterItem::calculateShownPresence() {
-	shownPresence_ = offlinePresence_;
-	foreach (StringPresencePair presencePair, presences_) {
-		boost::shared_ptr<Presence> presence = presencePair.second;
-		if (!shownPresence_ || presence->getPriority() > shownPresence_->getPriority() || presence->getShow() < shownPresence_->getShow()) {
-			shownPresence_ = presence;
-		}
-	}
-}
-
 void ContactRosterItem::clearPresence() {
-	presences_.clear();
-	calculateShownPresence();
+	presence_.reset();
 	onDataChanged();
 }
 
 void ContactRosterItem::applyPresence(const std::string& resource, boost::shared_ptr<Presence> presence) {
-	if (offlinePresence_) {
-		offlinePresence_ = boost::shared_ptr<Presence>();
-	}
-	if (presence->getType() == Presence::Unavailable) {
-		if (resource.empty()) {
-			/* Unavailable from the bare JID means all resources are offline.*/
-			presences_.clear();
-		} else {
-			if (presences_.find(resource) != presences_.end()) {
-				presences_.erase(resource);
-			}
-		}
-		if (presences_.empty()) {
-			offlinePresence_ = presence;
-		}
-	} else {
-		presences_[resource] = presence;
-	}
-	calculateShownPresence();
+	presence_ = presence;
 	onDataChanged();
 }
 
diff --git a/Swift/Controllers/Roster/ContactRosterItem.h b/Swift/Controllers/Roster/ContactRosterItem.h
index 54a6b60..ec04a8c 100644
--- a/Swift/Controllers/Roster/ContactRosterItem.h
+++ b/Swift/Controllers/Roster/ContactRosterItem.h
@@ -1,14 +1,14 @@
 /*
- * Copyright (c) 2010-2013 Isode Limited.
+ * Copyright (c) 2010-2015 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
 
 #pragma once
 
-#include <map>
 #include <set>
 #include <string>
+#include <vector>
 
 #include <boost/bind.hpp>
 #include <boost/date_time/posix_time/ptime.hpp>
@@ -16,9 +16,9 @@
 #include <boost/shared_ptr.hpp>
 
 #include <Swiften/Base/boost_bsignals.h>
+#include <Swiften/Elements/MUCOccupant.h>
 #include <Swiften/Elements/Presence.h>
 #include <Swiften/Elements/StatusShow.h>
-#include <Swiften/Elements/MUCOccupant.h>
 #include <Swiften/Elements/VCard.h>
 #include <Swiften/JID/JID.h>
 
@@ -56,12 +56,11 @@ class ContactRosterItem : public RosterItem {
 		void setDisplayJID(const JID& jid);
 		const JID& getDisplayJID() const;
 		void applyPresence(const std::string& resource, boost::shared_ptr<Presence> presence);
-		void clearPresence();
-		void calculateShownPresence();
 		const std::vector<std::string>& getGroups() const;
 		/** Only used so a contact can know about the groups it's in*/
 		void addGroup(const std::string& group);
 		void removeGroup(const std::string& group);
+		void clearPresence();
 
 		MUCOccupant::Role getMUCRole() const;
 		void setMUCRole(const MUCOccupant::Role& role);
@@ -85,9 +84,7 @@ class ContactRosterItem : public RosterItem {
 		JID displayJID_;
 		boost::posix_time::ptime lastAvailableTime_;
 		boost::filesystem::path avatarPath_;
-		std::map<std::string, boost::shared_ptr<Presence> > presences_;
-		boost::shared_ptr<Presence> offlinePresence_;
-		boost::shared_ptr<Presence> shownPresence_;
+		boost::shared_ptr<Presence> presence_;
 		std::vector<std::string> groups_;
 		MUCOccupant::Role mucRole_;
 		MUCOccupant::Affiliation mucAffiliation_;
diff --git a/Swift/Controllers/Roster/RosterController.cpp b/Swift/Controllers/Roster/RosterController.cpp
index 73efa43..751ca32 100644
--- a/Swift/Controllers/Roster/RosterController.cpp
+++ b/Swift/Controllers/Roster/RosterController.cpp
@@ -79,7 +79,6 @@ RosterController::RosterController(const JID& jid, XMPPRoster* xmppRoster, Avata
 	xmppRoster_->onJIDRemoved.connect(boost::bind(&RosterController::handleOnJIDRemoved, this, _1));
 	xmppRoster_->onRosterCleared.connect(boost::bind(&RosterController::handleRosterCleared, this));
 	subscriptionManager_->onPresenceSubscriptionRequest.connect(boost::bind(&RosterController::handleSubscriptionRequest, this, _1, _2));
-	presenceOracle_->onPresenceChange.connect(boost::bind(&RosterController::handleIncomingPresence, this, _1));
 	uiEventConnection_ = uiEventStream->onUIEvent.connect(boost::bind(&RosterController::handleUIEvent, this, _1));
 
 	vcardManager_->onOwnVCardChanged.connect(boost::bind(&RosterController::handleOwnVCardChanged, this, _1));
@@ -334,7 +333,8 @@ void RosterController::handleIncomingPresence(Presence::ref newPresence) {
 	if (newPresence->getType() == Presence::Error) {
 		return;
 	}
-	roster_->applyOnItems(SetPresence(newPresence));
+	Presence::ref accountPresence = presenceOracle_->getAccountPresence(newPresence->getFrom().toBare());
+	roster_->applyOnItems(SetPresence(accountPresence));
 }
 
 void RosterController::handleSubscriptionRequest(const JID& jid, const std::string& message) {
@@ -380,6 +380,9 @@ void RosterController::handlePresenceChanged(Presence::ref presence) {
 		ownContact_->applyPresence(std::string(), presence);
 		mainWindow_->setMyContactRosterItem(ownContact_);
 	}
+	else {
+		handleIncomingPresence(presence);
+	}
 }
 
 boost::optional<XMPPRosterItem> RosterController::getItem(const JID& jid) const {
diff --git a/Swift/Controllers/Roster/UnitTest/RosterControllerTest.cpp b/Swift/Controllers/Roster/UnitTest/RosterControllerTest.cpp
index 9320af1..d774e6d 100644
--- a/Swift/Controllers/Roster/UnitTest/RosterControllerTest.cpp
+++ b/Swift/Controllers/Roster/UnitTest/RosterControllerTest.cpp
@@ -194,10 +194,17 @@ class RosterControllerTest : public CppUnit::TestFixture {
 		groups.push_back("testGroup1");
 		JID from("test@testdomain.com");
 		xmppRoster_->addContact(from, "name", groups, RosterItemPayload::Both);
+
 		Presence::ref lowPresence(new Presence());
 		lowPresence->setFrom(withResource(from, "bob"));
 		lowPresence->setPriority(2);
+		lowPresence->setShow(StatusShow::Away);
 		lowPresence->setStatus("Not here");
+		Presence::ref lowPresenceOffline(new Presence());
+		lowPresenceOffline->setFrom(withResource(from, "bob"));
+		lowPresenceOffline->setStatus("Signing out");
+		lowPresenceOffline->setType(Presence::Unavailable);
+
 		Presence::ref highPresence(new Presence());
 		highPresence->setFrom(withResource(from, "bert"));
 		highPresence->setPriority(10);
@@ -205,28 +212,34 @@ class RosterControllerTest : public CppUnit::TestFixture {
 		Presence::ref highPresenceOffline(new Presence());
 		highPresenceOffline->setFrom(withResource(from, "bert"));
 		highPresenceOffline->setType(Presence::Unavailable);
-		Presence::ref lowPresenceOffline(new Presence());
-		lowPresenceOffline->setFrom(withResource(from, "bob"));
-		lowPresenceOffline->setStatus("Signing out");
-		lowPresenceOffline->setType(Presence::Unavailable);
+
 		stanzaChannel_->onPresenceReceived(lowPresence);
+		Presence::ref accountPresence = presenceOracle_->getAccountPresence(from);
+		CPPUNIT_ASSERT_EQUAL(StatusShow::Away, accountPresence->getShow());
+
 		stanzaChannel_->onPresenceReceived(highPresence);
+		accountPresence = presenceOracle_->getAccountPresence(from);
+		CPPUNIT_ASSERT_EQUAL(StatusShow::Online, accountPresence->getShow());
+
 		stanzaChannel_->onPresenceReceived(highPresenceOffline);
+
+		// After this, the roster should show the low presence.
 		ContactRosterItem* item = dynamic_cast<ContactRosterItem*>(dynamic_cast<GroupRosterItem*>(CHILDREN[0])->getChildren()[0]);
 		CPPUNIT_ASSERT(item);
-		/* A verification that if the test fails, it's the RosterController, not the PresenceOracle. */
-		Presence::ref high = presenceOracle_->getHighestPriorityPresence(from);
-		CPPUNIT_ASSERT_EQUAL(Presence::Available, high->getType());
-		CPPUNIT_ASSERT_EQUAL(lowPresence->getStatus(), high->getStatus());
-		CPPUNIT_ASSERT_EQUAL(StatusShow::Online, item->getStatusShow());
+
+		Presence::ref low = presenceOracle_->getAccountPresence(from);
+
+		CPPUNIT_ASSERT_EQUAL(Presence::Available, low->getType());
+		CPPUNIT_ASSERT_EQUAL(lowPresence->getStatus(), low->getStatus());
+		CPPUNIT_ASSERT_EQUAL(lowPresence->getShow(), item->getStatusShow());
 		CPPUNIT_ASSERT_EQUAL(lowPresence->getStatus(), item->getStatusText());
 		stanzaChannel_->onPresenceReceived(lowPresenceOffline);
 		item = dynamic_cast<ContactRosterItem*>(dynamic_cast<GroupRosterItem*>(CHILDREN[0])->getChildren()[0]);
 		CPPUNIT_ASSERT(item);
 		/* A verification that if the test fails, it's the RosterController, not the PresenceOracle. */
-		high = presenceOracle_->getHighestPriorityPresence(from);
-		CPPUNIT_ASSERT_EQUAL(Presence::Unavailable, high->getType());
-		CPPUNIT_ASSERT_EQUAL(lowPresenceOffline->getStatus(), high->getStatus());
+		low = presenceOracle_->getHighestPriorityPresence(from);
+		CPPUNIT_ASSERT_EQUAL(Presence::Unavailable, low->getType());
+		CPPUNIT_ASSERT_EQUAL(lowPresenceOffline->getStatus(), low->getStatus());
 		CPPUNIT_ASSERT_EQUAL(StatusShow::None, item->getStatusShow());
 		CPPUNIT_ASSERT_EQUAL(lowPresenceOffline->getStatus(), item->getStatusText());
 	}
author	Tobias Markmann <tm@ayena.de>	2015-10-27 16:20:09 (GMT)
committer	Tobias Markmann <tm@ayena.de>	2015-10-30 12:34:52 (GMT)
commit	0f5ef716a50c8d9761cafda12aacf818cdfd6353 (patch)
tree	4ed80dedc2fa8b04036eb7567282094ea8fe07f1 /Swift/Controllers
parent	4320235bef1b601856b295a8d6411d1898048802 (diff)
download	swift-0f5ef716a50c8d9761cafda12aacf818cdfd6353.zip swift-0f5ef716a50c8d9761cafda12aacf818cdfd6353.tar.bz2