From 0f078be02cd6818aeacc935a1ab790b41267a00d Mon Sep 17 00:00:00 2001
From: Kevin Smith <git@kismith.co.uk>
Date: Sun, 24 Oct 2010 13:17:55 +0100
Subject: Use the highest priority for a contact's roster item, not the newest.

Resolves: #654
Release-Notes: Contacts online from several clients at once should now show the highest priority presence, not the most recent.

diff --git a/Swift/Controllers/RosterController.cpp b/Swift/Controllers/RosterController.cpp
index 9e50f0e..dfe34b0 100644
--- a/Swift/Controllers/RosterController.cpp
+++ b/Swift/Controllers/RosterController.cpp
@@ -236,12 +236,15 @@ void RosterController::handleIncomingPresence(Presence::ref newPresence) {
 		return;
 	}
 	Presence::ref appliedPresence(newPresence);
-	if (newPresence->getType() == Presence::Unsubscribe) {
-		/* In 3921bis, subscription removal isn't followed by a presence push of unavailable*/
-		appliedPresence = boost::shared_ptr<Presence>(new Presence());
-		appliedPresence->setFrom(newPresence->getFrom().toBare());
+	JID newJID(appliedPresence->getFrom());
+	Presence::ref highestPresence = presenceOracle_->getHighestPriorityPresence(appliedPresence->getFrom().toBare());
+	JID highestJID(highestPresence->getFrom());
+	bool highestPriority = (newJID == highestJID || highestPresence->getType() == Presence::Unavailable);
+	if (!highestPriority) {
+		/* Update in case there are full-JID roster entries.*/
+		roster_->applyOnItems(SetPresence(appliedPresence, JID::WithResource));
 	}
-	roster_->applyOnItems(SetPresence(appliedPresence));
+	roster_->applyOnItems(SetPresence(highestPresence));
 }
 
 void RosterController::handleSubscriptionRequest(const JID& jid, const String& message) {
diff --git a/Swift/Controllers/UnitTest/RosterControllerTest.cpp b/Swift/Controllers/UnitTest/RosterControllerTest.cpp
index 92336f6..e4770d7 100644
--- a/Swift/Controllers/UnitTest/RosterControllerTest.cpp
+++ b/Swift/Controllers/UnitTest/RosterControllerTest.cpp
@@ -43,6 +43,9 @@ class RosterControllerTest : public CppUnit::TestFixture
 		CPPUNIT_TEST(testReceiveRename);
 		CPPUNIT_TEST(testSendRename);
 		CPPUNIT_TEST(testSendRegroup);
+		CPPUNIT_TEST(testPresence);
+		CPPUNIT_TEST(testHighestPresence);
+		CPPUNIT_TEST(testNotHighestPresence);
 		CPPUNIT_TEST_SUITE_END();
 
 	public:
@@ -88,6 +91,66 @@ class RosterControllerTest : public CppUnit::TestFixture
 		return dynamic_cast<GroupRosterItem*>(CHILDREN[i]);
 	}
 
+	JID withResource(const JID& jid, const String& resource) {
+		return JID(jid.toBare().toString() + "/" + resource);
+	}
+
+	void testPresence() {
+		std::vector<String> groups;
+		groups.push_back("testGroup1");
+		groups.push_back("testGroup2");
+		JID from("test@testdomain.com");
+		xmppRoster_->addContact(from, "name", groups, RosterItemPayload::Both);
+		Presence::ref presence(new Presence());
+		presence->setFrom(withResource(from, "bob"));
+		presence->setPriority(2);
+		presence->setStatus("So totally here");
+		stanzaChannel_->onPresenceReceived(presence);
+		ContactRosterItem* item = dynamic_cast<ContactRosterItem*>(dynamic_cast<GroupRosterItem*>(CHILDREN[0])->getChildren()[0]);
+		CPPUNIT_ASSERT_EQUAL(presence->getStatus(), item->getStatusText());
+		ContactRosterItem* item2 = dynamic_cast<ContactRosterItem*>(dynamic_cast<GroupRosterItem*>(CHILDREN[1])->getChildren()[0]);
+		CPPUNIT_ASSERT_EQUAL(presence->getStatus(), item2->getStatusText());
+
+	};
+
+	void testHighestPresence() {
+		std::vector<String> groups;
+		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->setStatus("Not here");
+		Presence::ref highPresence(new Presence());
+		highPresence->setFrom(withResource(from, "bert"));
+		highPresence->setPriority(10);
+		highPresence->setStatus("So totally here");
+		stanzaChannel_->onPresenceReceived(lowPresence);
+		stanzaChannel_->onPresenceReceived(highPresence);
+		ContactRosterItem* item = dynamic_cast<ContactRosterItem*>(dynamic_cast<GroupRosterItem*>(CHILDREN[0])->getChildren()[0]);
+		CPPUNIT_ASSERT_EQUAL(highPresence->getStatus(), item->getStatusText());
+	};
+
+	void testNotHighestPresence() {
+		std::vector<String> groups;
+		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->setStatus("Not here");
+		Presence::ref highPresence(new Presence());
+		highPresence->setFrom(withResource(from, "bert"));
+		highPresence->setPriority(10);
+		highPresence->setStatus("So totally here");
+		stanzaChannel_->onPresenceReceived(highPresence);
+		stanzaChannel_->onPresenceReceived(lowPresence);
+		ContactRosterItem* item = dynamic_cast<ContactRosterItem*>(dynamic_cast<GroupRosterItem*>(CHILDREN[0])->getChildren()[0]);
+		CPPUNIT_ASSERT_EQUAL(highPresence->getStatus(), item->getStatusText());
+	};
+
 		void testAdd() {
 			std::vector<String> groups;
 			groups.push_back("testGroup1");
diff --git a/Swiften/Presence/PresenceOracle.cpp b/Swiften/Presence/PresenceOracle.cpp
index 439a84d..83bbbf7 100644
--- a/Swiften/Presence/PresenceOracle.cpp
+++ b/Swiften/Presence/PresenceOracle.cpp
@@ -30,17 +30,31 @@ void PresenceOracle::handleStanzaChannelAvailableChanged(bool available) {
 }
 
 
-void PresenceOracle::handleIncomingPresence(boost::shared_ptr<Presence> presence) {
-	JID bareJID = JID(presence->getFrom().toBare());
-
+void PresenceOracle::handleIncomingPresence(Presence::ref presence) {
+	JID bareJID(presence->getFrom().toBare());
 	if (presence->getType() == Presence::Subscribe) {
 		onPresenceSubscriptionRequest(bareJID, presence->getStatus());
-	} 
-	else {
+	} else {
+		Presence::ref passedPresence = presence;
+		if (presence->getType() == Presence::Unsubscribe) {
+			/* 3921bis says that we don't follow up with an unavailable, so simulate this ourselves */
+			onPresenceSubscriptionRevoked(bareJID, presence->getStatus());
+			passedPresence = Presence::ref(new Presence());
+			passedPresence->setType(Presence::Unavailable);
+			passedPresence->setFrom(bareJID);
+			passedPresence->setStatus(presence->getStatus());
+		}
 		std::map<JID, boost::shared_ptr<Presence> > jidMap = entries_[bareJID];
-		jidMap[presence->getFrom()] = presence;
+		if (passedPresence->getFrom().isBare() && presence->getType() == Presence::Unavailable) {
+			/* Have a bare-JID only presence of offline */
+			jidMap.clear();
+		} else if (passedPresence->getType() == Presence::Available) {
+			/* Don't have a bare-JID only offline presence once there are available presences */
+			jidMap.erase(bareJID);
+		}
+		jidMap[passedPresence->getFrom()] = passedPresence;
 		entries_[bareJID] = jidMap;
-		onPresenceChange(presence);
+		onPresenceChange(passedPresence);
 	}
 }
 
diff --git a/Swiften/Presence/PresenceOracle.h b/Swiften/Presence/PresenceOracle.h
index 8c4fce4..b0d1e15 100644
--- a/Swiften/Presence/PresenceOracle.h
+++ b/Swiften/Presence/PresenceOracle.h
@@ -26,6 +26,7 @@ class StanzaChannel;
 		public:
 			boost::signal<void (boost::shared_ptr<Presence>)> onPresenceChange;
 			boost::signal<void (const JID&, const String&)> onPresenceSubscriptionRequest;
+			boost::signal<void (const JID&, const String&)> onPresenceSubscriptionRevoked;
 
 		private:
 			void handleIncomingPresence(boost::shared_ptr<Presence> presence);
-- 
cgit v0.10.2-6-g49f6