From b0b175c73f8ce9f693adeb1137a6a50a9ec3e101 Mon Sep 17 00:00:00 2001 From: Kevin Smith 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 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(dynamic_cast(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(dynamic_cast(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 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) { @@ -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