From cf90536b5b015289ce8834b5417ba0fc7636cee6 Mon Sep 17 00:00:00 2001 From: Tobias Markmann Date: Tue, 17 May 2016 21:05:30 +0200 Subject: Fix MUC invitation request being overwritten by presence change After an initial presence change, when a client received a MUC invite, a potential following offline presence could replace the previous MUC invite request in the chat view. This commit fixes the issue. Test-Information: Added unit test verifying the new behavior. Verified absence of described bug in Swift GUI. All tests pass on OS X 10.11.5. Change-Id: I8fd9c7ad3f5f5009f48fc3d86017cd94e1998f01 diff --git a/Swift/Controllers/Chat/ChatController.h b/Swift/Controllers/Chat/ChatController.h index 3291190..206ee71 100644 --- a/Swift/Controllers/Chat/ChatController.h +++ b/Swift/Controllers/Chat/ChatController.h @@ -90,7 +90,6 @@ namespace Swift { ChatStateTracker* chatStateTracker_; std::string myLastMessageUIID_; bool isInMUC_; - bool lastWasPresence_; std::string lastStatusChangeString_; std::map, std::string> unackedStanzas_; std::map requestedReceipts_; diff --git a/Swift/Controllers/Chat/ChatControllerBase.cpp b/Swift/Controllers/Chat/ChatControllerBase.cpp index 15f6112..a54c89e 100644 --- a/Swift/Controllers/Chat/ChatControllerBase.cpp +++ b/Swift/Controllers/Chat/ChatControllerBase.cpp @@ -97,6 +97,7 @@ void ChatControllerBase::handleDayChangeTick() { dateChangeTimer_->stop(); boost::posix_time::ptime now = boost::posix_time::second_clock::local_time(); chatWindow_->addSystemMessage(chatMessageParser_->parseMessageBody(str(format(QT_TRANSLATE_NOOP("", "The day is now %1%")) % std::string(boost::posix_time::to_iso_extended_string(now)).substr(0,10))), ChatWindow::DefaultDirection); + lastWasPresence_ = false; dayTicked(); createDayChangeTimer(); } @@ -382,6 +383,7 @@ void ChatControllerBase::handleGeneralMUCInvitation(MUCInviteEvent::ref event) { updateMessageCount(); chatWindow_->addMUCInvitation(senderDisplayNameFromMessage(event->getInviter()), event->getRoomJID(), event->getReason(), event->getPassword(), event->getDirect(), event->getImpromptu()); eventController_->handleIncomingEvent(event); + lastWasPresence_ = false; } void ChatControllerBase::handleMUCInvitation(Message::ref message) { diff --git a/Swift/Controllers/Chat/ChatControllerBase.h b/Swift/Controllers/Chat/ChatControllerBase.h index 71390d9..b97d7af 100644 --- a/Swift/Controllers/Chat/ChatControllerBase.h +++ b/Swift/Controllers/Chat/ChatControllerBase.h @@ -138,5 +138,6 @@ namespace Swift { std::shared_ptr chatMessageParser_; AutoAcceptMUCInviteDecider* autoAcceptMUCInviteDecider_; UIEventStream* eventStream_; + bool lastWasPresence_ = false; }; } diff --git a/Swift/Controllers/Chat/MUCController.h b/Swift/Controllers/Chat/MUCController.h index ec3f0c4..e0ffd7e 100644 --- a/Swift/Controllers/Chat/MUCController.h +++ b/Swift/Controllers/Chat/MUCController.h @@ -153,7 +153,6 @@ namespace Swift { TabComplete* completer_; bool parting_; bool joined_; - bool lastWasPresence_; bool shouldJoinOnReconnect_; bool doneGettingHistory_; boost::signals2::scoped_connection avatarChangedConnection_; diff --git a/Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp b/Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp index 434ac8e..f3908d6 100644 --- a/Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp +++ b/Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -83,6 +84,7 @@ class ChatsManagerTest : public CppUnit::TestFixture { CPPUNIT_TEST(testChatControllerHighlightingNotificationDeduplicateSounds); CPPUNIT_TEST(testChatControllerMeMessageHandling); CPPUNIT_TEST(testChatControllerMeMessageHandlingInMUC); + CPPUNIT_TEST(testPresenceChangeDoesNotReplaceMUCInvite); CPPUNIT_TEST_SUITE_END(); public: @@ -103,7 +105,6 @@ public: directedPresenceSender_ = new DirectedPresenceSender(presenceSender_); mucManager_ = new MUCManager(stanzaChannel_, iqRouter_, directedPresenceSender_, mucRegistry_); uiEventStream_ = new UIEventStream(); -// entityCapsManager_ = new EntityCapsManager(capsProvider_, stanzaChannel_); entityCapsProvider_ = new DummyEntityCapsProvider(); chatListWindowFactory_ = mocks_->InterfaceMock(); mucSearchWindowFactory_ = mocks_->InterfaceMock(); @@ -190,9 +191,6 @@ public: JID messageJID2("testling@test.com/resource2"); - //MockChatWindow* window2 = new MockChatWindow();//mocks_->InterfaceMock(); - //mocks_->ExpectCall(chatWindowFactory_, ChatWindowFactory::createChatWindow).With(messageJID2, uiEventStream_).Return(window2); - std::shared_ptr message2(new Message()); message2->setFrom(messageJID2); std::string body2("This is a legible message. .cmaulm.chul"); @@ -332,9 +330,6 @@ public: JID messageJID2("testling@test.com/resource2"); - //MockChatWindow* window2 = new MockChatWindow();//mocks_->InterfaceMock(); - //mocks_->ExpectCall(chatWindowFactory_, ChatWindowFactory::createChatWindow).With(messageJID2, uiEventStream_).Return(window2); - std::shared_ptr message2(new Message()); message2->setFrom(messageJID2); message2->setBody("This is a legible message2."); @@ -883,6 +878,49 @@ public: CPPUNIT_ASSERT_EQUAL(std::string("says hello with a test message with foo and foo"), window->bodyFromMessage(window->lastAddedAction_)); } + void testPresenceChangeDoesNotReplaceMUCInvite() { + JID messageJID("testling@test.com/resource1"); + + auto generateIncomingPresence = [=](Presence::Type type) { + auto presence = std::make_shared(); + presence->setType(type); + presence->setFrom(messageJID); + presence->setTo(jid_); + return presence; + }; + + stanzaChannel_->onPresenceReceived(generateIncomingPresence(Presence::Available)); + + MockChatWindow* window = new MockChatWindow(); + mocks_->ExpectCall(chatWindowFactory_, ChatWindowFactory::createChatWindow).With(messageJID, uiEventStream_).Return(window); + + std::shared_ptr message(new Message()); + message->setFrom(messageJID); + std::string body("This is a legible message. >HEH@)oeueu"); + message->setBody(body); + manager_->handleIncomingMessage(message); + CPPUNIT_ASSERT_EQUAL(body, MockChatWindow::bodyFromMessage(window->lastAddedMessage_)); + + auto incomingMUCInvite = std::make_shared(); + incomingMUCInvite->setFrom(messageJID); + + auto invitePayload = std::make_shared(); + invitePayload->setJID("room@muc.service.com"); + incomingMUCInvite->addPayload(invitePayload); + + stanzaChannel_->onPresenceReceived(generateIncomingPresence(Presence::Unavailable)); + stanzaChannel_->onPresenceReceived(generateIncomingPresence(Presence::Available)); + + window->resetLastMessages(); + + manager_->handleIncomingMessage(incomingMUCInvite); + CPPUNIT_ASSERT_EQUAL(JID("room@muc.service.com"), window->lastMUCInvitationJID_); + + stanzaChannel_->onPresenceReceived(generateIncomingPresence(Presence::Unavailable)); + CPPUNIT_ASSERT_EQUAL(std::string(""), MockChatWindow::bodyFromMessage(window->lastReplacedMessage_)); + CPPUNIT_ASSERT_EQUAL(std::string("testling@test.com has gone offline."), MockChatWindow::bodyFromMessage(window->lastAddedPresence_)); + } + private: std::shared_ptr makeDeliveryReceiptTestMessage(const JID& from, const std::string& id) { std::shared_ptr message = std::make_shared(); diff --git a/Swift/Controllers/UnitTest/MockChatWindow.h b/Swift/Controllers/UnitTest/MockChatWindow.h index f9b183d..4f874e1 100644 --- a/Swift/Controllers/UnitTest/MockChatWindow.h +++ b/Swift/Controllers/UnitTest/MockChatWindow.h @@ -43,7 +43,9 @@ namespace Swift { virtual void replaceLastMessage(const ChatMessage& message, const TimestampBehaviour /*timestampBehaviour*/) { lastReplacedMessage_ = message; } - virtual void replaceSystemMessage(const ChatMessage& /*message*/, const std::string& /*id*/, const TimestampBehaviour /*timestampBehaviour*/) {} + virtual void replaceSystemMessage(const ChatMessage& message, const std::string& /*id*/, const TimestampBehaviour /*timestampBehaviour*/) { + lastReplacedSystemMessage_ = message; + } // File transfer related stuff virtual std::string addFileTransfer(const std::string& /*senderName*/, bool /*senderIsSelf*/,const std::string& /*filename*/, const boost::uintmax_t /*sizeInBytes*/, const std::string& /*description*/) { return nullptr; } @@ -77,7 +79,9 @@ namespace Swift { void setAvailableOccupantActions(const std::vector&/* actions*/) {} void setSubject(const std::string& /*subject*/) {} virtual void showRoomConfigurationForm(Form::ref) {} - virtual void addMUCInvitation(const std::string& /*senderName*/, const JID& /*jid*/, const std::string& /*reason*/, const std::string& /*password*/, bool = true, bool = false, bool = false) {} + virtual void addMUCInvitation(const std::string& /*senderName*/, const JID& jid, const std::string& /*reason*/, const std::string& /*password*/, bool = true, bool = false, bool = false) { + lastMUCInvitationJID_ = jid; + } virtual std::string addWhiteboardRequest(bool) {return "";} virtual void setWhiteboardSessionStatus(std::string, const ChatWindow::WhiteboardSessionState){} @@ -109,7 +113,7 @@ namespace Swift { } void resetLastMessages() { - lastAddedMessage_ = lastAddedAction_ = lastAddedPresence_ = lastReplacedMessage_ = lastAddedSystemMessage_ = ChatMessage(); + lastAddedMessage_ = lastAddedAction_ = lastAddedPresence_ = lastReplacedMessage_ = lastAddedSystemMessage_ = lastReplacedSystemMessage_ = ChatMessage(); } std::string name_; @@ -118,6 +122,8 @@ namespace Swift { ChatMessage lastAddedPresence_; ChatMessage lastReplacedMessage_; ChatMessage lastAddedSystemMessage_; + ChatMessage lastReplacedSystemMessage_; + JID lastMUCInvitationJID_; std::vector labels_; bool labelsEnabled_; bool impromptuMUCSupported_; -- cgit v0.10.2-6-g49f6