From 0468ee852e54f81043ff9589951c1d9f88a1848e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Remko=20Tron=C3=A7on?= Date: Sat, 5 Feb 2011 18:04:44 +0100 Subject: Fixed some bugs with MUC joining. diff --git a/Swiften/Elements/MUCOwnerPayload.h b/Swiften/Elements/MUCOwnerPayload.h index c418cc6..6c3e5f0 100644 --- a/Swiften/Elements/MUCOwnerPayload.h +++ b/Swiften/Elements/MUCOwnerPayload.h @@ -13,6 +13,8 @@ namespace Swift { class MUCOwnerPayload : public Payload { public: + typedef boost::shared_ptr ref; + MUCOwnerPayload() { } diff --git a/Swiften/Elements/MUCPayload.h b/Swiften/Elements/MUCPayload.h index 5d7c4f5..b8210e1 100644 --- a/Swiften/Elements/MUCPayload.h +++ b/Swiften/Elements/MUCPayload.h @@ -16,6 +16,8 @@ namespace Swift { class MUCPayload : public Payload { public: + typedef boost::shared_ptr ref; + MUCPayload() { maxChars_ = -1; maxStanzas_ = -1; diff --git a/Swiften/MUC/MUC.cpp b/Swiften/MUC/MUC.cpp index 15a9a4f..486ba27 100644 --- a/Swiften/MUC/MUC.cpp +++ b/Swiften/MUC/MUC.cpp @@ -8,6 +8,7 @@ #include #include +#include #include "Swiften/Presence/DirectedPresenceSender.h" #include "Swiften/Client/StanzaChannel.h" @@ -37,30 +38,34 @@ void MUC::joinAs(const String &nick) { internalJoin(nick); } +/** + * Join the MUC with context since date. + */ +void MUC::joinWithContextSince(const String &nick, const boost::posix_time::ptime& since) { + joinSince_ = since; + internalJoin(nick); +} + void MUC::internalJoin(const String &nick) { //TODO: password //TODO: history request - mucRegistry->addMUC(getJID()); joinComplete_ = false; + joinSucceeded_ = false; + + mucRegistry->addMUC(getJID()); + ownMUCJID = JID(ownMUCJID.getNode(), ownMUCJID.getDomain(), nick); - presenceSender->addDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::DontSendPresence); - boost::shared_ptr joinPresence(presenceSender->getLastSentUndirectedPresence()); + + Presence::ref joinPresence = boost::make_shared(*presenceSender->getLastSentUndirectedPresence()); assert(joinPresence->getType() == Presence::Available); joinPresence->setTo(ownMUCJID); - boost::shared_ptr mucPayload(new MUCPayload()); + MUCPayload::ref mucPayload = boost::make_shared(); if (joinSince_ != boost::posix_time::not_a_date_time) { mucPayload->setSince(joinSince_); } joinPresence->addPayload(mucPayload); - presenceSender->sendPresence(joinPresence); -} -/** - * Join the MUC with context since date. - */ -void MUC::joinWithContextSince(const String &nick, const boost::posix_time::ptime& since) { - joinSince_ = since; - internalJoin(nick); + presenceSender->sendPresence(joinPresence); } void MUC::part() { @@ -77,27 +82,34 @@ void MUC::handleUserLeft(LeavingType type) { } occupants.clear(); joinComplete_ = false; + joinSucceeded_ = false; presenceSender->removeDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::DontSendPresence); } -void MUC::handleIncomingPresence(boost::shared_ptr presence) { +void MUC::handleIncomingPresence(Presence::ref presence) { if (!isFromMUC(presence->getFrom())) { return; } - boost::shared_ptr mucPayload; - foreach (boost::shared_ptr payload, presence->getPayloads()) { + + MUCUserPayload::ref mucPayload; + foreach (MUCUserPayload::ref payload, presence->getPayloads()) { if (payload->getItems().size() > 0 || payload->getStatusCodes().size() > 0) { mucPayload = payload; } } - if (!joinComplete_) { + // On the first incoming presence, check if our join has succeeded + // (i.e. we start getting non-error presence from the MUC) or not + if (!joinSucceeded_) { if (presence->getType() == Presence::Error) { String reason; - presenceSender->removeDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::AndSendPresence); onJoinFailed(presence->getPayload()); return; } + else { + joinSucceeded_ = true; + presenceSender->addDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::AndSendPresence); + } } String nick = presence->getFrom().getResource(); @@ -121,7 +133,8 @@ void MUC::handleIncomingPresence(boost::shared_ptr presence) { if (presence->getFrom() == ownMUCJID) { handleUserLeft(Part); return; - } else { + } + else { std::map::iterator i = occupants.find(nick); if (i != occupants.end()) { //TODO: part type @@ -129,7 +142,8 @@ void MUC::handleIncomingPresence(boost::shared_ptr presence) { occupants.erase(i); } } - } else if (presence->getType() == Presence::Available) { + } + else if (presence->getType() == Presence::Available) { std::map::iterator it = occupants.find(nick); MUCOccupant occupant(nick, role, affiliation); bool isJoin = true; @@ -158,19 +172,24 @@ void MUC::handleIncomingPresence(boost::shared_ptr presence) { if (status.code == 110) { /* Simply knowing this is your presence is enough, 210 doesn't seem to be necessary. */ joinComplete_ = true; - presenceSender->removeDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::DontSendPresence); - ownMUCJID = presence->getFrom(); - presenceSender->addDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::DontSendPresence); + if (ownMUCJID != presence->getFrom()) { + presenceSender->removeDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::DontSendPresence); + ownMUCJID = presence->getFrom(); + presenceSender->addDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::AndSendPresence); + } onJoinComplete(getOwnNick()); } if (status.code == 201) { /* Room is created and locked */ /* Currently deal with this by making an instant room */ - presenceSender->removeDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::DontSendPresence); - ownMUCJID = presence->getFrom(); - boost::shared_ptr mucPayload(new MUCOwnerPayload()); + if (ownMUCJID != presence->getFrom()) { + presenceSender->removeDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::DontSendPresence); + ownMUCJID = presence->getFrom(); + presenceSender->addDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::AndSendPresence); + } + MUCOwnerPayload::ref mucPayload(new MUCOwnerPayload()); presenceSender->addDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::DontSendPresence); - mucPayload->setPayload(boost::shared_ptr(new Form(Form::SubmitType))); + mucPayload->setPayload(boost::make_shared
(Form::SubmitType)); GenericRequest* request = new GenericRequest(IQ::Set, getJID(), mucPayload, iqRouter_); request->onResponse.connect(boost::bind(&MUC::handleCreationConfigResponse, this, _1, _2)); request->send(); @@ -180,7 +199,7 @@ void MUC::handleIncomingPresence(boost::shared_ptr presence) { } -void MUC::handleCreationConfigResponse(boost::shared_ptr /*unused*/, ErrorPayload::ref error) { +void MUC::handleCreationConfigResponse(MUCOwnerPayload::ref /*unused*/, ErrorPayload::ref error) { if (error) { presenceSender->removeDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::AndSendPresence); onJoinFailed(error); diff --git a/Swiften/MUC/MUC.h b/Swiften/MUC/MUC.h index 44759d5..cdef292 100644 --- a/Swiften/MUC/MUC.h +++ b/Swiften/MUC/MUC.h @@ -48,14 +48,14 @@ namespace Swift { /*void queryRoomItems(); */ String getCurrentNick(); void part(); - void handleIncomingMessage(boost::shared_ptr message); + void handleIncomingMessage(Message::ref message); /** Expose public so it can be called when e.g. user goes offline */ void handleUserLeft(LeavingType); public: boost::signal onJoinComplete; - boost::signal)> onJoinFailed; - boost::signal)> onOccupantPresenceChange; + boost::signal onJoinFailed; + boost::signal onOccupantPresenceChange; boost::signal onOccupantRoleChanged; boost::signal onOccupantAffiliationChanged; boost::signal onOccupantJoined; @@ -74,9 +74,9 @@ namespace Swift { } private: - void handleIncomingPresence(boost::shared_ptr presence); + void handleIncomingPresence(Presence::ref presence); void internalJoin(const String& nick); - void handleCreationConfigResponse(boost::shared_ptr, ErrorPayload::ref); + void handleCreationConfigResponse(MUCOwnerPayload::ref, ErrorPayload::ref); private: JID ownMUCJID; @@ -85,6 +85,7 @@ namespace Swift { DirectedPresenceSender* presenceSender; MUCRegistry* mucRegistry; std::map occupants; + bool joinSucceeded_; bool joinComplete_; boost::bsignals::scoped_connection scopedConnection_; boost::posix_time::ptime joinSince_; diff --git a/Swiften/MUC/UnitTest/MUCTest.cpp b/Swiften/MUC/UnitTest/MUCTest.cpp index 1e3582b..fd07711 100644 --- a/Swiften/MUC/UnitTest/MUCTest.cpp +++ b/Swiften/MUC/UnitTest/MUCTest.cpp @@ -21,7 +21,8 @@ using namespace Swift; class MUCTest : public CppUnit::TestFixture { CPPUNIT_TEST_SUITE(MUCTest); CPPUNIT_TEST(testJoin); - CPPUNIT_TEST(testJoin_ChangePresenceDuringJoin); + CPPUNIT_TEST(testJoin_ChangePresenceDuringJoinDoesNotSendPresenceBeforeJoinSuccess); + CPPUNIT_TEST(testJoin_ChangePresenceDuringJoinResendsPresenceAfterJoinSuccess); /*CPPUNIT_TEST(testJoin_Success); CPPUNIT_TEST(testJoin_Fail);*/ CPPUNIT_TEST_SUITE_END(); @@ -53,15 +54,26 @@ class MUCTest : public CppUnit::TestFixture { CPPUNIT_ASSERT_EQUAL(JID("foo@bar.com/Alice"), p->getTo()); } - void testJoin_ChangePresenceDuringJoin() { + void testJoin_ChangePresenceDuringJoinDoesNotSendPresenceBeforeJoinSuccess() { MUC::ref testling = createMUC(JID("foo@bar.com")); testling->joinAs("Alice"); presenceSender->sendPresence(Presence::create("Test")); + CPPUNIT_ASSERT_EQUAL(2, static_cast(channel->sentStanzas.size())); + } + + void testJoin_ChangePresenceDuringJoinResendsPresenceAfterJoinSuccess() { + MUC::ref testling = createMUC(JID("foo@bar.com")); + testling->joinAs("Alice"); + + presenceSender->sendPresence(Presence::create("Test")); + receivePresence(JID("foo@bar.com/Rabbit"), "Here"); + CPPUNIT_ASSERT_EQUAL(3, static_cast(channel->sentStanzas.size())); Presence::ref p = channel->getStanzaAtIndex(2); CPPUNIT_ASSERT(p); CPPUNIT_ASSERT_EQUAL(JID("foo@bar.com/Alice"), p->getTo()); + CPPUNIT_ASSERT_EQUAL(String("Test"), p->getStatus()); } /*void testJoin_Success() { @@ -91,14 +103,14 @@ class MUCTest : public CppUnit::TestFixture { joinResults.push_back(r); } - /*void receivePresence(const JID& jid, const String& status, const MUCUserPayload::Item& item) { + void receivePresence(const JID& jid, const String& status) { Presence::ref p = Presence::create(status); p->setFrom(jid); - MUCUserPayload::ref mucUserPayload = boost::make_shared(); - mucUserPayload->addItem(item); - p->addPayload(mucUserPayload); + //MUCUserPayload::ref mucUserPayload = boost::make_shared(); + //mucUserPayload->addItem(item); + //p->addPayload(mucUserPayload); channel->onPresenceReceived(p); - }*/ + } private: DummyStanzaChannel* channel; -- cgit v0.10.2-6-g49f6