From 286ef105d813a12640d64b71101cafe53f212234 Mon Sep 17 00:00:00 2001 From: Tobias Markmann Date: Mon, 27 Oct 2014 15:51:07 +0100 Subject: Resend presence to MUC on join completion only if it changed since join. Test-Information: Added test case to assure presence is not resend after join completion if it did not change. The other test cases are untouched and still all succeed. Change-Id: I2aace1aee8ca3deab9cd9050a25233617b3b0678 diff --git a/Swiften/Elements/Presence.cpp b/Swiften/Elements/Presence.cpp index 38b8a4c..0e8d400 100644 --- a/Swiften/Elements/Presence.cpp +++ b/Swiften/Elements/Presence.cpp @@ -42,5 +42,4 @@ void Presence::setStatus(const std::string& status) { updatePayload(boost::make_shared(status)); } - } diff --git a/Swiften/MUC/MUCImpl.cpp b/Swiften/MUC/MUCImpl.cpp index ab5faf4..778c290 100644 --- a/Swiften/MUC/MUCImpl.cpp +++ b/Swiften/MUC/MUCImpl.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -66,6 +67,22 @@ std::map MUCImpl::getOccupants() const { return occupants; } +bool MUCImpl::isEqualExceptID(const Presence& lhs, const Presence& rhs) { + bool isEqual = false; + if (lhs.getFrom() == rhs.getFrom() && lhs.getTo() == rhs.getTo() && lhs.getStatus() == rhs.getStatus() && lhs.getShow() == rhs.getShow()) { + CapsInfo::ref lhsCaps = lhs.getPayload(); + CapsInfo::ref rhsCaps = rhs.getPayload(); + + if (!!lhsCaps && !!rhsCaps) { + isEqual = (*lhsCaps == *rhsCaps); + } + else { + isEqual = (!lhsCaps && !rhsCaps); + } + } + return isEqual; +} + void MUCImpl::internalJoin(const std::string &nick) { //TODO: history request joinComplete_ = false; @@ -75,7 +92,7 @@ void MUCImpl::internalJoin(const std::string &nick) { ownMUCJID = JID(ownMUCJID.getNode(), ownMUCJID.getDomain(), nick); - Presence::ref joinPresence = boost::make_shared(*presenceSender->getLastSentUndirectedPresence()); + Presence::ref joinPresence = presenceSender->getLastSentUndirectedPresence() ? (*presenceSender->getLastSentUndirectedPresence())->clone() : boost::make_shared(); assert(joinPresence->getType() == Presence::Available); joinPresence->setTo(ownMUCJID); MUCPayload::ref mucPayload = boost::make_shared(); @@ -86,7 +103,7 @@ void MUCImpl::internalJoin(const std::string &nick) { mucPayload->setPassword(*password); } joinPresence->addPayload(mucPayload); - + joinRequestPresence_ = joinPresence; presenceSender->sendPresence(joinPresence); } @@ -137,7 +154,13 @@ void MUCImpl::handleIncomingPresence(Presence::ref presence) { } else { joinSucceeded_ = true; - presenceSender->addDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::AndSendPresence); + presenceSender->addDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::DontSendPresence); + if (presenceSender->getLastSentUndirectedPresence() && !isEqualExceptID(**(presenceSender->getLastSentUndirectedPresence()), *joinRequestPresence_)) { + // our presence changed between join request and join complete, send current presence to MUC + Presence::ref latestPresence = boost::make_shared(**presenceSender->getLastSentUndirectedPresence()); + latestPresence->setTo(ownMUCJID); + presenceSender->sendPresence(latestPresence); + } } } @@ -245,6 +268,7 @@ void MUCImpl::handleIncomingPresence(Presence::ref presence) { } onJoinComplete(getOwnNick()); } + // MUC status 201: a new room has been created if (status.code == 201) { isLocked = true; /* Room is created and locked */ @@ -259,6 +283,7 @@ void MUCImpl::handleIncomingPresence(Presence::ref presence) { requestConfigurationForm(); } else { + // Accept default room configuration and create an instant room http://xmpp.org/extensions/xep-0045.html#createroom-instant MUCOwnerPayload::ref mucPayload(new MUCOwnerPayload()); presenceSender->addDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::DontSendPresence); mucPayload->setPayload(boost::make_shared
(Form::SubmitType)); diff --git a/Swiften/MUC/MUCImpl.h b/Swiften/MUC/MUCImpl.h index 8eabb80..2979d70 100644 --- a/Swiften/MUC/MUCImpl.h +++ b/Swiften/MUC/MUCImpl.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010-2013 Kevin Smith + * Copyright (c) 2010-2014 Kevin Smith * Licensed under the GNU General Public License v3. * See Documentation/Licenses/GPLv3.txt for more information. */ @@ -94,6 +94,12 @@ namespace Swift { return ownMUCJID.getResource(); } + /** + * This function compares two Presence elements for equality based on to, from, status, show and entity capability information. + * @return True if equal; else otherwise. + */ + static bool isEqualExceptID(const Presence& lhs, const Presence& rhs); + private: void handleIncomingPresence(Presence::ref presence); void internalJoin(const std::string& nick); @@ -119,5 +125,6 @@ namespace Swift { bool unlocking; bool isUnlocked_; boost::optional password; + Presence::ref joinRequestPresence_; }; } diff --git a/Swiften/MUC/UnitTest/MUCTest.cpp b/Swiften/MUC/UnitTest/MUCTest.cpp index 773edb6..e6f4608 100644 --- a/Swiften/MUC/UnitTest/MUCTest.cpp +++ b/Swiften/MUC/UnitTest/MUCTest.cpp @@ -28,6 +28,7 @@ class MUCTest : public CppUnit::TestFixture { CPPUNIT_TEST(testJoin); CPPUNIT_TEST(testJoin_ChangePresenceDuringJoinDoesNotSendPresenceBeforeJoinSuccess); CPPUNIT_TEST(testJoin_ChangePresenceDuringJoinResendsPresenceAfterJoinSuccess); + CPPUNIT_TEST(testJoin_NoPresenceChangeDuringJoinDoesNotResendAfterJoinSuccess); CPPUNIT_TEST(testCreateInstant); CPPUNIT_TEST(testReplicateBug); CPPUNIT_TEST(testNicknameChange); @@ -85,6 +86,19 @@ class MUCTest : public CppUnit::TestFixture { CPPUNIT_ASSERT_EQUAL(std::string("Test"), p->getStatus()); } + void testJoin_NoPresenceChangeDuringJoinDoesNotResendAfterJoinSuccess() { + MUC::ref testling = createMUC(JID("foo@bar.com")); + testling->joinAs("Alice"); + + receivePresence(JID("foo@bar.com/Rabbit"), "Here"); + + CPPUNIT_ASSERT_EQUAL(1, static_cast(channel->sentStanzas.size())); + Presence::ref p = channel->getStanzaAtIndex(0); + CPPUNIT_ASSERT(p); + CPPUNIT_ASSERT_EQUAL(JID("foo@bar.com/Alice"), p->getTo()); + CPPUNIT_ASSERT_EQUAL(std::string(""), p->getStatus()); + } + void testCreateInstant() { MUC::ref testling = createMUC(JID("rabbithole@wonderland.lit")); testling->joinAs("Alice"); diff --git a/Swiften/Presence/DirectedPresenceSender.cpp b/Swiften/Presence/DirectedPresenceSender.cpp index ec0bd3f..c6cddbd 100644 --- a/Swiften/Presence/DirectedPresenceSender.cpp +++ b/Swiften/Presence/DirectedPresenceSender.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010 Remko Tronçon + * Copyright (c) 2010-2014 Remko Tronçon * Licensed under the GNU General Public License v3. * See Documentation/Licenses/GPLv3.txt for more information. */ @@ -31,10 +31,10 @@ void DirectedPresenceSender::sendPresence(boost::shared_ptr presence) } /** - * Gets either the last broadcast presence, or an empty stanza if none has been sent. + * Gets the last broadcast presence, if none has been send the returned optional is not set. */ -boost::shared_ptr DirectedPresenceSender::getLastSentUndirectedPresence() { - boost::shared_ptr presenceCopy(lastSentUndirectedPresence ? new Presence(*lastSentUndirectedPresence) : new Presence()); +boost::optional DirectedPresenceSender::getLastSentUndirectedPresence() const { + boost::optional presenceCopy = lastSentUndirectedPresence ? boost::optional((*lastSentUndirectedPresence)->clone()) : boost::optional(); return presenceCopy; } @@ -46,8 +46,8 @@ boost::shared_ptr DirectedPresenceSender::getLastSentUndirectedPresenc void DirectedPresenceSender::addDirectedPresenceReceiver(const JID& jid, SendPresence sendPresence) { directedPresenceReceivers.insert(jid); if (sendPresence == AndSendPresence && sender->isAvailable()) { - if (lastSentUndirectedPresence && lastSentUndirectedPresence->getType() == Presence::Available) { - boost::shared_ptr presenceCopy(new Presence(*lastSentUndirectedPresence)); + if (lastSentUndirectedPresence && (*lastSentUndirectedPresence)->getType() == Presence::Available) { + boost::shared_ptr presenceCopy((*lastSentUndirectedPresence)->clone()); presenceCopy->setTo(jid); sender->sendPresence(presenceCopy); } diff --git a/Swiften/Presence/DirectedPresenceSender.h b/Swiften/Presence/DirectedPresenceSender.h index 0eb16a4..511e177 100644 --- a/Swiften/Presence/DirectedPresenceSender.h +++ b/Swiften/Presence/DirectedPresenceSender.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010 Remko Tronçon + * Copyright (c) 2010-2014 Remko Tronçon * Licensed under the GNU General Public License v3. * See Documentation/Licenses/GPLv3.txt for more information. */ @@ -8,6 +8,8 @@ #include +#include + #include #include #include @@ -23,12 +25,12 @@ namespace Swift { void sendPresence(Presence::ref); - Presence::ref getLastSentUndirectedPresence(); + boost::optional getLastSentUndirectedPresence() const; bool isAvailable() const; private: - Presence::ref lastSentUndirectedPresence; + boost::optional lastSentUndirectedPresence; PresenceSender* sender; std::set directedPresenceReceivers; }; -- cgit v0.10.2-6-g49f6