From b0b175c73f8ce9f693adeb1137a6a50a9ec3e101 Mon Sep 17 00:00:00 2001
From: Kevin Smith <git@kismith.co.uk>
Date: Thu, 28 Oct 2010 08:33:43 +0100
Subject: Correctly mark roster items with the highest priority status.

Resolves: #668

diff --git a/Swift/Controllers/RosterController.cpp b/Swift/Controllers/RosterController.cpp
index 2f6c82c..1ce3266 100644
--- a/Swift/Controllers/RosterController.cpp
+++ b/Swift/Controllers/RosterController.cpp
@@ -235,16 +235,7 @@ void RosterController::handleIncomingPresence(Presence::ref newPresence) {
 	if (newPresence->getType() == Presence::Error) {
 		return;
 	}
-	Presence::ref appliedPresence(newPresence);
-	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(highestPresence));
+	roster_->applyOnItems(SetPresence(newPresence));
 }
 
 void RosterController::handleSubscriptionRequest(const JID& jid, const String& message) {
diff --git a/Swift/Controllers/UnitTest/RosterControllerTest.cpp b/Swift/Controllers/UnitTest/RosterControllerTest.cpp
index 4dd7b57..c0c0319 100644
--- a/Swift/Controllers/UnitTest/RosterControllerTest.cpp
+++ b/Swift/Controllers/UnitTest/RosterControllerTest.cpp
@@ -46,6 +46,7 @@ class RosterControllerTest : public CppUnit::TestFixture
 		CPPUNIT_TEST(testPresence);
 		CPPUNIT_TEST(testHighestPresence);
 		CPPUNIT_TEST(testNotHighestPresence);
+		CPPUNIT_TEST(testUnavailablePresence);
 		CPPUNIT_TEST_SUITE_END();
 
 	public:
@@ -151,6 +152,46 @@ class RosterControllerTest : public CppUnit::TestFixture
 		CPPUNIT_ASSERT_EQUAL(highPresence->getStatus(), item->getStatusText());
 	};
 
+	void testUnavailablePresence() {
+		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");
+		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);
+		stanzaChannel_->onPresenceReceived(highPresence);
+		stanzaChannel_->onPresenceReceived(highPresenceOffline);
+		ContactRosterItem* item = dynamic_cast<ContactRosterItem*>(dynamic_cast<GroupRosterItem*>(CHILDREN[0])->getChildren()[0]);
+		/* 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());
+		CPPUNIT_ASSERT_EQUAL(lowPresence->getStatus(), item->getStatusText());
+		stanzaChannel_->onPresenceReceived(lowPresenceOffline);
+		item = dynamic_cast<ContactRosterItem*>(dynamic_cast<GroupRosterItem*>(CHILDREN[0])->getChildren()[0]);
+		/* 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());
+		CPPUNIT_ASSERT_EQUAL(StatusShow::None, item->getStatusShow());
+		CPPUNIT_ASSERT_EQUAL(lowPresenceOffline->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 83bbbf7..fb4be3f 100644
--- a/Swiften/Presence/PresenceOracle.cpp
+++ b/Swiften/Presence/PresenceOracle.cpp
@@ -52,7 +52,11 @@ void PresenceOracle::handleIncomingPresence(Presence::ref presence) {
 			/* Don't have a bare-JID only offline presence once there are available presences */
 			jidMap.erase(bareJID);
 		}
-		jidMap[passedPresence->getFrom()] = passedPresence;
+		if (passedPresence->getType() == Presence::Unavailable && jidMap.size() > 1) {
+			jidMap.erase(passedPresence->getFrom());
+		} else {
+			jidMap[passedPresence->getFrom()] = passedPresence;
+		}
 		entries_[bareJID] = jidMap;
 		onPresenceChange(passedPresence);
 	}
diff --git a/Swiften/Presence/UnitTest/PresenceOracleTest.cpp b/Swiften/Presence/UnitTest/PresenceOracleTest.cpp
index 645f917..d3b4b20 100644
--- a/Swiften/Presence/UnitTest/PresenceOracleTest.cpp
+++ b/Swiften/Presence/UnitTest/PresenceOracleTest.cpp
@@ -20,7 +20,10 @@ class PresenceOracleTest : public CppUnit::TestFixture {
 		CPPUNIT_TEST(testReceivePresenceFromDifferentResources);
 		CPPUNIT_TEST(testSubscriptionRequest);
 		CPPUNIT_TEST(testReconnectResetsPresences);
-		CPPUNIT_TEST(testHighestPresence);
+		CPPUNIT_TEST(testHighestPresenceSingle);
+		CPPUNIT_TEST(testHighestPresenceMultiple);
+		CPPUNIT_TEST(testHighestPresenceGlobal);
+		CPPUNIT_TEST(testHighestPresenceChangePriority);
 		CPPUNIT_TEST_SUITE_END();
 
 	public:
@@ -39,17 +42,60 @@ class PresenceOracleTest : public CppUnit::TestFixture {
 			delete stanzaChannel_;
 		}
 
-		void testHighestPresence() {
+		void testHighestPresenceSingle() {
+					JID bareJID("alice@wonderland.lit");
+					Presence::ref fiveOn = makeOnline("blah", 5);
+					Presence::ref fiveOff = makeOffline("/blah");
+					CPPUNIT_ASSERT_EQUAL(Presence::ref(), oracle_->getHighestPriorityPresence(bareJID));
+					stanzaChannel_->onPresenceReceived(fiveOn);
+					CPPUNIT_ASSERT_EQUAL(fiveOn, oracle_->getHighestPriorityPresence(bareJID));
+					stanzaChannel_->onPresenceReceived(fiveOff);
+					CPPUNIT_ASSERT_EQUAL(fiveOff, oracle_->getHighestPriorityPresence(bareJID));
+				}
+
+		void testHighestPresenceMultiple() {
+			JID bareJID("alice@wonderland.lit");
+			Presence::ref fiveOn = makeOnline("blah", 5);
+			Presence::ref fiveOff = makeOffline("/blah");
+			Presence::ref tenOn = makeOnline("bert", 10);
+			Presence::ref tenOff = makeOffline("/bert");
+			stanzaChannel_->onPresenceReceived(fiveOn);
+			stanzaChannel_->onPresenceReceived(tenOn);
+			CPPUNIT_ASSERT_EQUAL(tenOn, oracle_->getHighestPriorityPresence(bareJID));
+			stanzaChannel_->onPresenceReceived(fiveOff);
+			CPPUNIT_ASSERT_EQUAL(tenOn, oracle_->getHighestPriorityPresence(bareJID));
+			stanzaChannel_->onPresenceReceived(fiveOn);
+			stanzaChannel_->onPresenceReceived(tenOff);
+			CPPUNIT_ASSERT_EQUAL(fiveOn, oracle_->getHighestPriorityPresence(bareJID));
+		}
+
+		void testHighestPresenceGlobal() {
 			JID bareJID("alice@wonderland.lit");
 			Presence::ref fiveOn = makeOnline("blah", 5);
 			Presence::ref fiveOff = makeOffline("/blah");
 			Presence::ref tenOn = makeOnline("bert", 10);
 			Presence::ref allOff = makeOffline("");
-			CPPUNIT_ASSERT_EQUAL(Presence::ref(), oracle_->getHighestPriorityPresence(bareJID));
 			stanzaChannel_->onPresenceReceived(fiveOn);
+			stanzaChannel_->onPresenceReceived(tenOn);
+			stanzaChannel_->onPresenceReceived(allOff);
+			CPPUNIT_ASSERT_EQUAL(allOff, oracle_->getHighestPriorityPresence(bareJID));
+		}
+
+		void testHighestPresenceChangePriority() {
+			JID bareJID("alice@wonderland.lit");
+			Presence::ref fiveOn = makeOnline("blah", 5);
+			Presence::ref fiveOff = makeOffline("/blah");
+			Presence::ref tenOn = makeOnline("bert", 10);
+			Presence::ref tenOnThree = makeOnline("bert", 3);
+			Presence::ref tenOff = makeOffline("/bert");
+			stanzaChannel_->onPresenceReceived(fiveOn);
+			stanzaChannel_->onPresenceReceived(tenOn);
+			stanzaChannel_->onPresenceReceived(tenOnThree);
 			CPPUNIT_ASSERT_EQUAL(fiveOn, oracle_->getHighestPriorityPresence(bareJID));
 			stanzaChannel_->onPresenceReceived(fiveOff);
-			CPPUNIT_ASSERT_EQUAL(fiveOff, oracle_->getHighestPriorityPresence(bareJID));
+			CPPUNIT_ASSERT_EQUAL(tenOnThree, oracle_->getHighestPriorityPresence(bareJID));
+			stanzaChannel_->onPresenceReceived(fiveOn);
+			CPPUNIT_ASSERT_EQUAL(fiveOn, oracle_->getHighestPriorityPresence(bareJID));
 		}
 
 		void testReceivePresence() {
diff --git a/Swiften/Roster/ContactRosterItem.cpp b/Swiften/Roster/ContactRosterItem.cpp
index 37fc6af..9c7af32 100644
--- a/Swiften/Roster/ContactRosterItem.cpp
+++ b/Swiften/Roster/ContactRosterItem.cpp
@@ -73,6 +73,7 @@ void ContactRosterItem::calculateShownPresence() {
 void ContactRosterItem::clearPresence() {
 	presences_.clear();
 	calculateShownPresence();
+	onDataChanged();
 }
 
 void ContactRosterItem::applyPresence(const String& resource, boost::shared_ptr<Presence> presence) {
@@ -88,7 +89,7 @@ void ContactRosterItem::applyPresence(const String& resource, boost::shared_ptr<
 				presences_.erase(resource);
 			}
 		}
-		if (presences_.size() > 0) {
+		if (presences_.size() == 0) {
 			offlinePresence_ = presence;
 		}
 	} else {
-- 
cgit v0.10.2-6-g49f6