From 241bbe1c1a5755972f3ec0532befa9329631934e Mon Sep 17 00:00:00 2001 From: Kevin Smith <git@kismith.co.uk> Date: Mon, 20 Dec 2010 09:51:03 +0000 Subject: Don't immediately send presence unneccesarily when registering directed presence senders. Hopefully Resolves: #691 Release-Notes: We hope to have fixed the bug where a MUC room would go into loop of parting and joining continually. diff --git a/Swiften/MUC/MUC.cpp b/Swiften/MUC/MUC.cpp index dd57698..309107c 100644 --- a/Swiften/MUC/MUC.cpp +++ b/Swiften/MUC/MUC.cpp @@ -63,7 +63,7 @@ void MUC::joinWithContextSince(const String &nick, const boost::posix_time::ptim } void MUC::part() { - presenceSender->removeDirectedPresenceReceiver(ownMUCJID); + presenceSender->removeDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::AndSendPresence); mucRegistry->removeMUC(getJID()); } @@ -76,7 +76,7 @@ void MUC::handleUserLeft(LeavingType type) { } occupants.clear(); joinComplete_ = false; - presenceSender->removeDirectedPresenceReceiver(ownMUCJID); + presenceSender->removeDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::DontSendPresence); } void MUC::handleIncomingPresence(boost::shared_ptr<Presence> presence) { @@ -158,7 +158,7 @@ void MUC::handleIncomingPresence(boost::shared_ptr<Presence> presence) { joinComplete_ = true; ownMUCJID = presence->getFrom(); onJoinComplete(getOwnNick()); - presenceSender->addDirectedPresenceReceiver(ownMUCJID); + presenceSender->addDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::DontSendPresence); } if (status.code == 201) { /* Room is created and locked */ @@ -179,8 +179,8 @@ void MUC::handleCreationConfigResponse(boost::shared_ptr<MUCOwnerPayload> /*unus if (error) { onJoinFailed(error); } else { - /* onJoinComplete(getOwnNick()); isn't needed here, the presence will cause an emit elsewhere. */ - presenceSender->addDirectedPresenceReceiver(ownMUCJID); + onJoinComplete(getOwnNick()); /* Previously, this wasn't needed here, as the presence duplication bug caused an emit elsewhere. */ + presenceSender->addDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::DontSendPresence); } } diff --git a/Swiften/Presence/DirectedPresenceSender.cpp b/Swiften/Presence/DirectedPresenceSender.cpp index aade6cf..d824554 100644 --- a/Swiften/Presence/DirectedPresenceSender.cpp +++ b/Swiften/Presence/DirectedPresenceSender.cpp @@ -38,9 +38,14 @@ boost::shared_ptr<Presence> DirectedPresenceSender::getLastSentUndirectedPresenc return presenceCopy; } -void DirectedPresenceSender::addDirectedPresenceReceiver(const JID& jid) { +/** + * Send future broadcast presence also to this JID. + * @param jid Non-roster JID to receive global presence updates. + * @param sendPresence Also send the current global presence immediately. + */ +void DirectedPresenceSender::addDirectedPresenceReceiver(const JID& jid, SendPresence sendPresence) { directedPresenceReceivers.insert(jid); - if (sender->isAvailable()) { + if (sendPresence == AndSendPresence && sender->isAvailable()) { if (lastSentUndirectedPresence && lastSentUndirectedPresence->getType() == Presence::Available) { boost::shared_ptr<Presence> presenceCopy(new Presence(*lastSentUndirectedPresence)); presenceCopy->setTo(jid); @@ -49,9 +54,14 @@ void DirectedPresenceSender::addDirectedPresenceReceiver(const JID& jid) { } } -void DirectedPresenceSender::removeDirectedPresenceReceiver(const JID& jid) { +/** + * Send future broadcast presence also to this JID. + * @param jid Non-roster JID to stop receiving global presence updates. + * @param sendPresence Also send presence type=unavailable immediately to jid. + */ +void DirectedPresenceSender::removeDirectedPresenceReceiver(const JID& jid, SendPresence sendPresence) { directedPresenceReceivers.erase(jid); - if (sender->isAvailable()) { + if (sendPresence == AndSendPresence && sender->isAvailable()) { boost::shared_ptr<Presence> presence(new Presence()); presence->setType(Presence::Unavailable); presence->setTo(jid); diff --git a/Swiften/Presence/DirectedPresenceSender.h b/Swiften/Presence/DirectedPresenceSender.h index b63a50e..207de3e 100644 --- a/Swiften/Presence/DirectedPresenceSender.h +++ b/Swiften/Presence/DirectedPresenceSender.h @@ -14,10 +14,11 @@ namespace Swift { class DirectedPresenceSender : public PresenceSender { public: + enum SendPresence {AndSendPresence, DontSendPresence}; DirectedPresenceSender(PresenceSender*); - void addDirectedPresenceReceiver(const JID&); - void removeDirectedPresenceReceiver(const JID&); + void addDirectedPresenceReceiver(const JID&, SendPresence); + void removeDirectedPresenceReceiver(const JID&, SendPresence); void sendPresence(Presence::ref); diff --git a/Swiften/Presence/UnitTest/DirectedPresenceSenderTest.cpp b/Swiften/Presence/UnitTest/DirectedPresenceSenderTest.cpp index a60c429..80f5ebd 100644 --- a/Swiften/Presence/UnitTest/DirectedPresenceSenderTest.cpp +++ b/Swiften/Presence/UnitTest/DirectedPresenceSenderTest.cpp @@ -19,8 +19,10 @@ class DirectedPresenceSenderTest : public CppUnit::TestFixture { CPPUNIT_TEST(testSendPresence_UndirectedPresenceWithDirectedPresenceReceivers); CPPUNIT_TEST(testSendPresence_DirectedPresenceWithDirectedPresenceReceivers); CPPUNIT_TEST(testAddDirectedPresenceReceiver); + CPPUNIT_TEST(testAddDirectedPresenceReceiver_WithoutSendingPresence); CPPUNIT_TEST(testAddDirectedPresenceReceiver_AfterSendingDirectedPresence); CPPUNIT_TEST(testRemoveDirectedPresenceReceiver); + CPPUNIT_TEST(testRemoveDirectedPresenceReceiver_WithoutSendingPresence); CPPUNIT_TEST_SUITE_END(); public: @@ -49,7 +51,7 @@ class DirectedPresenceSenderTest : public CppUnit::TestFixture { void testSendPresence_UndirectedPresenceWithDirectedPresenceReceivers() { std::auto_ptr<DirectedPresenceSender> testling(createPresenceSender()); - testling->addDirectedPresenceReceiver(JID("alice@wonderland.lit/teaparty")); + testling->addDirectedPresenceReceiver(JID("alice@wonderland.lit/teaparty"), DirectedPresenceSender::AndSendPresence); testling->sendPresence(testPresence); @@ -63,7 +65,7 @@ class DirectedPresenceSenderTest : public CppUnit::TestFixture { void testSendPresence_DirectedPresenceWithDirectedPresenceReceivers() { std::auto_ptr<DirectedPresenceSender> testling(createPresenceSender()); - testling->addDirectedPresenceReceiver(JID("alice@wonderland.lit/teaparty")); + testling->addDirectedPresenceReceiver(JID("alice@wonderland.lit/teaparty"), DirectedPresenceSender::AndSendPresence); channel->sentStanzas.clear(); testPresence->setTo(JID("foo@bar.com")); @@ -79,7 +81,7 @@ class DirectedPresenceSenderTest : public CppUnit::TestFixture { testling->sendPresence(testPresence); channel->sentStanzas.clear(); - testling->addDirectedPresenceReceiver(JID("alice@wonderland.lit/teaparty")); + testling->addDirectedPresenceReceiver(JID("alice@wonderland.lit/teaparty"), DirectedPresenceSender::AndSendPresence); CPPUNIT_ASSERT_EQUAL(1, static_cast<int>(channel->sentStanzas.size())); boost::shared_ptr<Presence> presence = boost::dynamic_pointer_cast<Presence>(channel->sentStanzas[0]); @@ -87,6 +89,16 @@ class DirectedPresenceSenderTest : public CppUnit::TestFixture { CPPUNIT_ASSERT_EQUAL(JID("alice@wonderland.lit/teaparty"), presence->getTo()); } + void testAddDirectedPresenceReceiver_WithoutSendingPresence() { + std::auto_ptr<DirectedPresenceSender> testling(createPresenceSender()); + testling->sendPresence(testPresence); + channel->sentStanzas.clear(); + + testling->addDirectedPresenceReceiver(JID("alice@wonderland.lit/teaparty"), DirectedPresenceSender::DontSendPresence); + + CPPUNIT_ASSERT_EQUAL(0, static_cast<int>(channel->sentStanzas.size())); + } + void testAddDirectedPresenceReceiver_AfterSendingDirectedPresence() { std::auto_ptr<DirectedPresenceSender> testling(createPresenceSender()); testling->sendPresence(testPresence); @@ -94,7 +106,7 @@ class DirectedPresenceSenderTest : public CppUnit::TestFixture { testling->sendPresence(secondTestPresence); channel->sentStanzas.clear(); - testling->addDirectedPresenceReceiver(JID("alice@wonderland.lit/teaparty")); + testling->addDirectedPresenceReceiver(JID("alice@wonderland.lit/teaparty"), DirectedPresenceSender::AndSendPresence); CPPUNIT_ASSERT_EQUAL(1, static_cast<int>(channel->sentStanzas.size())); boost::shared_ptr<Presence> presence = boost::dynamic_pointer_cast<Presence>(channel->sentStanzas[0]); @@ -104,10 +116,9 @@ class DirectedPresenceSenderTest : public CppUnit::TestFixture { void testRemoveDirectedPresenceReceiver() { std::auto_ptr<DirectedPresenceSender> testling(createPresenceSender()); - testling->addDirectedPresenceReceiver(JID("alice@wonderland.lit/teaparty")); - channel->sentStanzas.clear(); + testling->addDirectedPresenceReceiver(JID("alice@wonderland.lit/teaparty"), DirectedPresenceSender::DontSendPresence); - testling->removeDirectedPresenceReceiver(JID("alice@wonderland.lit/teaparty")); + testling->removeDirectedPresenceReceiver(JID("alice@wonderland.lit/teaparty"), DirectedPresenceSender::AndSendPresence); testling->sendPresence(testPresence); CPPUNIT_ASSERT_EQUAL(2, static_cast<int>(channel->sentStanzas.size())); @@ -115,6 +126,18 @@ class DirectedPresenceSenderTest : public CppUnit::TestFixture { CPPUNIT_ASSERT(channel->sentStanzas[1] == testPresence); } + void testRemoveDirectedPresenceReceiver_WithoutSendingPresence() { + std::auto_ptr<DirectedPresenceSender> testling(createPresenceSender()); + testling->addDirectedPresenceReceiver(JID("alice@wonderland.lit/teaparty"), DirectedPresenceSender::AndSendPresence); + channel->sentStanzas.clear(); + + testling->removeDirectedPresenceReceiver(JID("alice@wonderland.lit/teaparty"), DirectedPresenceSender::DontSendPresence); + testling->sendPresence(testPresence); + + CPPUNIT_ASSERT_EQUAL(1, static_cast<int>(channel->sentStanzas.size())); + CPPUNIT_ASSERT(channel->sentStanzas[0] == testPresence); + } + private: DirectedPresenceSender* createPresenceSender() { return new DirectedPresenceSender(stanzaChannelPresenceSender); -- cgit v0.10.2-6-g49f6