From 0f078be02cd6818aeacc935a1ab790b41267a00d Mon Sep 17 00:00:00 2001 From: Kevin Smith 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(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(CHILDREN[i]); } + JID withResource(const JID& jid, const String& resource) { + return JID(jid.toBare().toString() + "/" + resource); + } + + void testPresence() { + std::vector 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(dynamic_cast(CHILDREN[0])->getChildren()[0]); + CPPUNIT_ASSERT_EQUAL(presence->getStatus(), item->getStatusText()); + ContactRosterItem* item2 = dynamic_cast(dynamic_cast(CHILDREN[1])->getChildren()[0]); + CPPUNIT_ASSERT_EQUAL(presence->getStatus(), item2->getStatusText()); + + }; + + void testHighestPresence() { + 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"); + stanzaChannel_->onPresenceReceived(lowPresence); + stanzaChannel_->onPresenceReceived(highPresence); + ContactRosterItem* item = dynamic_cast(dynamic_cast(CHILDREN[0])->getChildren()[0]); + CPPUNIT_ASSERT_EQUAL(highPresence->getStatus(), item->getStatusText()); + }; + + void testNotHighestPresence() { + 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"); + stanzaChannel_->onPresenceReceived(highPresence); + stanzaChannel_->onPresenceReceived(lowPresence); + ContactRosterItem* item = dynamic_cast(dynamic_cast(CHILDREN[0])->getChildren()[0]); + CPPUNIT_ASSERT_EQUAL(highPresence->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 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) { - 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 > 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)> onPresenceChange; boost::signal onPresenceSubscriptionRequest; + boost::signal onPresenceSubscriptionRevoked; private: void handleIncomingPresence(boost::shared_ptr presence); -- cgit v0.10.2-6-g49f6