From b2d3eae9fd085cd91b3efac53dca81fd450d5393 Mon Sep 17 00:00:00 2001 From: Tobias Markmann Date: Wed, 4 May 2016 16:15:53 +0200 Subject: Fix /me message handling for highlighted messages Test-Information: Added a unit test to test for the fix. Tests pass on OS X 10.11.4. Change-Id: Ibf071ae47663bfefdc856339932de6a1fe4a642d diff --git a/Swift/Controllers/Chat/ChatMessageParser.cpp b/Swift/Controllers/Chat/ChatMessageParser.cpp index d639b06..ec7df6c 100644 --- a/Swift/Controllers/Chat/ChatMessageParser.cpp +++ b/Swift/Controllers/Chat/ChatMessageParser.cpp @@ -14,7 +14,6 @@ #include #include -#include #include @@ -68,13 +67,12 @@ namespace Swift { return parsedMessage; } - ChatWindow::ChatMessage ChatMessageParser::emoticonHighlight(const ChatWindow::ChatMessage& message) - { + ChatWindow::ChatMessage ChatMessageParser::emoticonHighlight(const ChatWindow::ChatMessage& message) { ChatWindow::ChatMessage parsedMessage = message; std::string regexString; /* Parse two, emoticons */ - foreach (StringPair emoticon, emoticons_) { + for (StringPair emoticon : emoticons_) { /* Construct a regexp that finds an instance of any of the emoticons inside a group * at the start or end of the line, or beside whitespace. */ @@ -91,7 +89,7 @@ namespace Swift { boost::regex emoticonRegex(regexString); ChatWindow::ChatMessage newMessage; - foreach (std::shared_ptr part, parsedMessage.getParts()) { + for (std::shared_ptr part : parsedMessage.getParts()) { std::shared_ptr textPart; if ((textPart = std::dynamic_pointer_cast(part))) { try { @@ -142,8 +140,7 @@ namespace Swift { return parsedMessage; } - ChatWindow::ChatMessage ChatMessageParser::splitHighlight(const ChatWindow::ChatMessage& message, const std::string& nick) - { + ChatWindow::ChatMessage ChatMessageParser::splitHighlight(const ChatWindow::ChatMessage& message, const std::string& nick) { ChatWindow::ChatMessage parsedMessage = message; for (size_t i = 0; i < highlightRules_->getSize(); ++i) { @@ -156,9 +153,9 @@ namespace Swift { continue; /* do not try to highlight text, if no highlight color is specified */ } const std::vector keywordRegex = rule.getKeywordRegex(nick); - foreach(const boost::regex& regex, keywordRegex) { + for (const boost::regex& regex : keywordRegex) { ChatWindow::ChatMessage newMessage; - foreach (std::shared_ptr part, parsedMessage.getParts()) { + for (std::shared_ptr part : parsedMessage.getParts()) { std::shared_ptr textPart; if ((textPart = std::dynamic_pointer_cast(part))) { try { @@ -191,7 +188,7 @@ namespace Swift { newMessage.append(part); } } - parsedMessage = newMessage; + parsedMessage.setParts(newMessage.getParts()); } } diff --git a/Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp b/Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp index 0a36180..434ac8e 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 @@ -81,6 +82,7 @@ class ChatsManagerTest : public CppUnit::TestFixture { CPPUNIT_TEST(testChatControllerHighlightingNotificationTesting); CPPUNIT_TEST(testChatControllerHighlightingNotificationDeduplicateSounds); CPPUNIT_TEST(testChatControllerMeMessageHandling); + CPPUNIT_TEST(testChatControllerMeMessageHandlingInMUC); CPPUNIT_TEST_SUITE_END(); public: @@ -88,9 +90,7 @@ public: mocks_ = new MockRepository(); jid_ = JID("test@test.com/resource"); stanzaChannel_ = new DummyStanzaChannel(); - iqChannel_ = new DummyIQChannel(); - iqRouter_ = new IQRouter(iqChannel_); -// capsProvider_ = new DummyCapsProvider(); + iqRouter_ = new IQRouter(stanzaChannel_); eventController_ = new EventController(); chatWindowFactory_ = mocks_->InterfaceMock(); joinMUCWindowFactory_ = mocks_->InterfaceMock(); @@ -116,6 +116,7 @@ public: wbSessionManager_ = new WhiteboardSessionManager(iqRouter_, stanzaChannel_, presenceOracle_, entityCapsProvider_); wbManager_ = new WhiteboardManager(whiteboardWindowFactory_, uiEventStream_, nickResolver_, wbSessionManager_); highlightManager_ = new HighlightManager(settings_); + highlightManager_->resetToDefaultRulesList(); handledHighlightActions_ = 0; soundsPlayed_.clear(); highlightManager_->onHighlight.connect(boost::bind(&ChatsManagerTest::handleHighlightAction, this, _1)); @@ -132,7 +133,6 @@ public: void tearDown() { delete highlightManager_; - //delete chatListWindowFactory delete profileSettings_; delete avatarManager_; delete manager_; @@ -149,10 +149,9 @@ public: delete presenceOracle_; delete nickResolver_; delete mucRegistry_; + delete iqRouter_; delete stanzaChannel_; delete eventController_; - delete iqRouter_; - delete iqChannel_; delete uiEventStream_; delete mucManager_; delete xmppRoster_; @@ -173,7 +172,7 @@ public: std::string body("This is a legible message. >HEH@)oeueu"); message->setBody(body); manager_->handleIncomingMessage(message); - CPPUNIT_ASSERT_EQUAL(body, window->lastMessageBody_); + CPPUNIT_ASSERT_EQUAL(body, MockChatWindow::bodyFromMessage(window->lastAddedMessage_)); } void testSecondOpenWindowIncoming() { @@ -187,7 +186,7 @@ public: std::string body1("This is a legible message. >HEH@)oeueu"); message1->setBody(body1); manager_->handleIncomingMessage(message1); - CPPUNIT_ASSERT_EQUAL(body1, window1->lastMessageBody_); + CPPUNIT_ASSERT_EQUAL(body1, MockChatWindow::bodyFromMessage(window1->lastAddedMessage_)); JID messageJID2("testling@test.com/resource2"); @@ -199,7 +198,7 @@ public: std::string body2("This is a legible message. .cmaulm.chul"); message2->setBody(body2); manager_->handleIncomingMessage(message2); - CPPUNIT_ASSERT_EQUAL(body2, window1->lastMessageBody_); + CPPUNIT_ASSERT_EQUAL(body2, MockChatWindow::bodyFromMessage(window1->lastAddedMessage_)); } void testFirstOpenWindowOutgoing() { @@ -225,7 +224,7 @@ public: std::string body("This is a legible message. mjuga3089gm8G(*>M)@*("); message->setBody(body); manager_->handleIncomingMessage(message); - CPPUNIT_ASSERT_EQUAL(body, window->lastMessageBody_); + CPPUNIT_ASSERT_EQUAL(body, MockChatWindow::bodyFromMessage(window->lastAddedMessage_)); } void testSecondWindow() { @@ -261,7 +260,7 @@ public: std::string messageBody1("This is a legible message."); message1->setBody(messageBody1); manager_->handleIncomingMessage(message1); - CPPUNIT_ASSERT_EQUAL(messageBody1, window->lastMessageBody_); + CPPUNIT_ASSERT_EQUAL(messageBody1, MockChatWindow::bodyFromMessage(window->lastAddedMessage_)); std::shared_ptr jid1Online(new Presence()); jid1Online->setFrom(JID(fullJIDString1)); @@ -275,7 +274,7 @@ public: std::string messageBody2("This is another legible message."); message2->setBody(messageBody2); manager_->handleIncomingMessage(message2); - CPPUNIT_ASSERT_EQUAL(messageBody2, window->lastMessageBody_); + CPPUNIT_ASSERT_EQUAL(messageBody2, MockChatWindow::bodyFromMessage(window->lastAddedMessage_)); } /** @@ -362,14 +361,14 @@ public: std::string body3("This is a legible message3."); message3->setBody(body3); manager_->handleIncomingMessage(message3); - CPPUNIT_ASSERT_EQUAL(body3, window1->lastMessageBody_); + CPPUNIT_ASSERT_EQUAL(body3, MockChatWindow::bodyFromMessage(window1->lastAddedMessage_)); std::shared_ptr message2b(new Message()); message2b->setFrom(messageJID2); std::string body2b("This is a legible message2b."); message2b->setBody(body2b); manager_->handleIncomingMessage(message2b); - CPPUNIT_ASSERT_EQUAL(body2b, window1->lastMessageBody_); + CPPUNIT_ASSERT_EQUAL(body2b, MockChatWindow::bodyFromMessage(window1->lastAddedMessage_)); } /** @@ -385,15 +384,15 @@ public: std::shared_ptr message = makeDeliveryReceiptTestMessage(messageJID, "1"); manager_->handleIncomingMessage(message); - Stanza::ref stanzaContactOnRoster = stanzaChannel_->getStanzaAtIndex(0); - CPPUNIT_ASSERT_EQUAL(st(1), stanzaChannel_->sentStanzas.size()); + Stanza::ref stanzaContactOnRoster = stanzaChannel_->getStanzaAtIndex(1); + CPPUNIT_ASSERT_EQUAL(st(1), stanzaChannel_->countSentStanzaOfType()); CPPUNIT_ASSERT(stanzaContactOnRoster->getPayload() != nullptr); xmppRoster_->removeContact(messageJID); message->setID("2"); manager_->handleIncomingMessage(message); - CPPUNIT_ASSERT_EQUAL(st(1), stanzaChannel_->sentStanzas.size()); + CPPUNIT_ASSERT_EQUAL(st(1), stanzaChannel_->countSentStanzaOfType()); } /** @@ -409,14 +408,14 @@ public: std::shared_ptr message = makeDeliveryReceiptTestMessage(messageJID, "1"); manager_->handleIncomingMessage(message); - CPPUNIT_ASSERT_EQUAL(st(0), stanzaChannel_->sentStanzas.size()); + CPPUNIT_ASSERT_EQUAL(st(0), stanzaChannel_->countSentStanzaOfType()); xmppRoster_->addContact(messageJID, "foo", std::vector(), RosterItemPayload::Both); message->setID("2"); manager_->handleIncomingMessage(message); - CPPUNIT_ASSERT_EQUAL(st(1), stanzaChannel_->sentStanzas.size()); - Stanza::ref stanzaContactOnRoster = stanzaChannel_->getStanzaAtIndex(0); + CPPUNIT_ASSERT_EQUAL(st(1), stanzaChannel_->countSentStanzaOfType()); + Stanza::ref stanzaContactOnRoster = stanzaChannel_->getStanzaAtIndex(1); CPPUNIT_ASSERT(stanzaContactOnRoster->getPayload() != nullptr); } @@ -469,8 +468,8 @@ public: window->onSendMessageRequest("hello there", false); // A bare message is send because no resources is bound. - CPPUNIT_ASSERT_EQUAL(sender, stanzaChannel_->getStanzaAtIndex(0)->getTo()); - CPPUNIT_ASSERT(stanzaChannel_->getStanzaAtIndex(0)->getPayload()); + CPPUNIT_ASSERT_EQUAL(sender, stanzaChannel_->getStanzaAtIndex(1)->getTo()); + CPPUNIT_ASSERT(stanzaChannel_->getStanzaAtIndex(1)->getPayload()); // Two resources respond with message receipts. foreach(const JID& senderJID, senderResource) { @@ -479,7 +478,7 @@ public: receiptReply->setTo(ownJID); std::shared_ptr receipt = std::make_shared(); - receipt->setReceivedID(stanzaChannel_->getStanzaAtIndex(0)->getID()); + receipt->setReceivedID(stanzaChannel_->getStanzaAtIndex(1)->getID()); receiptReply->addPayload(receipt); manager_->handleIncomingMessage(receiptReply); } @@ -514,7 +513,7 @@ public: window->onSendMessageRequest("great to hear.", false); // The chat session is bound to the full JID of the first resource. - CPPUNIT_ASSERT_EQUAL(senderResource[0], stanzaChannel_->getStanzaAtIndex(2)->getTo()); + CPPUNIT_ASSERT_EQUAL(senderResource[0], stanzaChannel_->getStanzaAtIndex(3)->getTo()); CPPUNIT_ASSERT(stanzaChannel_->getStanzaAtIndex(2)->getPayload()); // Receive random receipt from second sender resource. @@ -545,8 +544,8 @@ public: window->onSendMessageRequest("okay", false); // The chat session is now bound to the full JID of the second resource. - CPPUNIT_ASSERT_EQUAL(senderResource[1], stanzaChannel_->getStanzaAtIndex(4)->getTo()); - CPPUNIT_ASSERT(stanzaChannel_->getStanzaAtIndex(4)->getPayload()); + CPPUNIT_ASSERT_EQUAL(senderResource[1], stanzaChannel_->getStanzaAtIndex(5)->getTo()); + CPPUNIT_ASSERT(stanzaChannel_->getStanzaAtIndex(5)->getPayload()); } void testChatControllerFullJIDBindingOnTypingAndNotActive() { @@ -584,8 +583,8 @@ public: window->onSendMessageRequest("hello there", false); // A bare message is send because no resources is bound. - CPPUNIT_ASSERT_EQUAL(sender, stanzaChannel_->getStanzaAtIndex(0)->getTo()); - CPPUNIT_ASSERT(stanzaChannel_->getStanzaAtIndex(0)->getPayload()); + CPPUNIT_ASSERT_EQUAL(sender, stanzaChannel_->getStanzaAtIndex(1)->getTo()); + CPPUNIT_ASSERT(stanzaChannel_->getStanzaAtIndex(1)->getPayload()); // Two resources respond with message receipts. foreach(const JID& senderJID, senderResource) { @@ -632,8 +631,8 @@ public: window->onSendMessageRequest("great to hear.", false); // The chat session is now bound to the full JID of the first resource due to its recent composing message. - CPPUNIT_ASSERT_EQUAL(senderResource[0], stanzaChannel_->getStanzaAtIndex(2)->getTo()); - CPPUNIT_ASSERT(stanzaChannel_->getStanzaAtIndex(2)->getPayload()); + CPPUNIT_ASSERT_EQUAL(senderResource[0], stanzaChannel_->getStanzaAtIndex(3)->getTo()); + CPPUNIT_ASSERT(stanzaChannel_->getStanzaAtIndex(3)->getPayload()); // Reply with a message including a CSN from the other resource. reply = std::make_shared(); @@ -649,8 +648,8 @@ public: window->onSendMessageRequest("ping.", false); // The chat session is now bound to the full JID of the second resource due to its recent composing message. - CPPUNIT_ASSERT_EQUAL(senderResource[1], stanzaChannel_->getStanzaAtIndex(3)->getTo()); - CPPUNIT_ASSERT(stanzaChannel_->getStanzaAtIndex(3)->getPayload()); + CPPUNIT_ASSERT_EQUAL(senderResource[1], stanzaChannel_->getStanzaAtIndex(4)->getTo()); + CPPUNIT_ASSERT(stanzaChannel_->getStanzaAtIndex(4)->getPayload()); } void testChatControllerPMPresenceHandling() { @@ -697,14 +696,14 @@ public: CPPUNIT_ASSERT_EQUAL(false, window->impromptuMUCSupported_); - std::shared_ptr infoRequest= iqChannel_->iqs_[1]; + std::shared_ptr infoRequest = std::dynamic_pointer_cast(stanzaChannel_->sentStanzas[1]); std::shared_ptr infoResponse = IQ::createResult(infoRequest->getFrom(), infoRequest->getTo(), infoRequest->getID()); DiscoInfo info; info.addIdentity(DiscoInfo::Identity("Shakespearean Chat Service", "conference", "text")); info.addFeature("http://jabber.org/protocol/muc"); infoResponse->addPayload(std::make_shared(info)); - iqChannel_->onIQReceived(infoResponse); + stanzaChannel_->onIQReceived(infoResponse); CPPUNIT_ASSERT_EQUAL(true, window->impromptuMUCSupported_); manager_->setOnline(false); @@ -722,14 +721,14 @@ public: std::shared_ptr message = makeDeliveryReceiptTestMessage(messageJID, "1"); manager_->handleIncomingMessage(message); - CPPUNIT_ASSERT_EQUAL(st(0), stanzaChannel_->sentStanzas.size()); + CPPUNIT_ASSERT_EQUAL(st(0), stanzaChannel_->countSentStanzaOfType()); xmppRoster_->addContact(messageJID, "foo", std::vector(), to); message->setID("2"); manager_->handleIncomingMessage(message); - CPPUNIT_ASSERT_EQUAL(st(1), stanzaChannel_->sentStanzas.size()); - Stanza::ref stanzaContactOnRoster = stanzaChannel_->getStanzaAtIndex(0); + CPPUNIT_ASSERT_EQUAL(st(1), stanzaChannel_->countSentStanzaOfType()); + Stanza::ref stanzaContactOnRoster = stanzaChannel_->getStanzaAtIndex(1); CPPUNIT_ASSERT(stanzaContactOnRoster->getPayload() != nullptr); } @@ -815,7 +814,73 @@ public: std::string body("/me is feeling delighted."); message->setBody(body); manager_->handleIncomingMessage(message); - CPPUNIT_ASSERT_EQUAL(std::string("is feeling delighted."), window->lastAddedActionBody_); + CPPUNIT_ASSERT_EQUAL(std::string("is feeling delighted."), window->bodyFromMessage(window->lastAddedAction_)); + } + + void testChatControllerMeMessageHandlingInMUC() { + JID mucJID("mucroom@rooms.test.com"); + std::string nickname = "toodles"; + + // add highlight rule for 'foo' + HighlightRule fooHighlight; + fooHighlight.setKeywords({"foo"}); + fooHighlight.setMatchMUC(true); + fooHighlight.getAction().setTextBackground("green"); + highlightManager_->insertRule(0, fooHighlight); + + MockChatWindow* window = new MockChatWindow(); + mocks_->ExpectCall(chatWindowFactory_, ChatWindowFactory::createChatWindow).With(mucJID, uiEventStream_).Return(window); + + uiEventStream_->send(std::make_shared(mucJID, boost::optional(), nickname)); + + auto genRemoteMUCPresence = [=]() { + auto presence = Presence::create(); + presence->setFrom(mucJID.withResource(nickname)); + presence->setTo(jid_); + return presence; + }; + + { + auto presence = genRemoteMUCPresence(); + auto userPayload = std::make_shared(); + userPayload->addStatusCode(110); + userPayload->addItem(MUCItem(MUCOccupant::Owner, jid_, MUCOccupant::Moderator)); + presence->addPayload(userPayload); + stanzaChannel_->onPresenceReceived(presence); + } + + { + auto presence = genRemoteMUCPresence(); + presence->setFrom(mucJID.withResource("someDifferentNickname")); + auto userPayload = std::make_shared(); + userPayload->addItem(MUCItem(MUCOccupant::Member, JID("foo@bar.com"), MUCOccupant::Moderator)); + presence->addPayload(userPayload); + stanzaChannel_->onPresenceReceived(presence); + } + + window->onSendMessageRequest("/me sends a test message with foo", false); + + window->resetLastMessages(); + { + Message::ref mucMirrored = std::make_shared(); + mucMirrored->setFrom(mucJID.withResource(nickname)); + mucMirrored->setTo(jid_); + mucMirrored->setType(Message::Groupchat); + mucMirrored->setBody("/me sends a test message with foo"); + manager_->handleIncomingMessage(mucMirrored); + } + CPPUNIT_ASSERT_EQUAL(std::string("sends a test message with foo"), window->bodyFromMessage(window->lastAddedAction_)); + + window->resetLastMessages(); + { + Message::ref mucMirrored = std::make_shared(); + mucMirrored->setFrom(mucJID.withResource("someDifferentNickname")); + mucMirrored->setTo(jid_); + mucMirrored->setType(Message::Groupchat); + mucMirrored->setBody("/me says hello with a test message with foo and foo"); + manager_->handleIncomingMessage(mucMirrored); + } + CPPUNIT_ASSERT_EQUAL(std::string("says hello with a test message with foo and foo"), window->bodyFromMessage(window->lastAddedAction_)); } private: @@ -843,7 +908,6 @@ private: JID jid_; ChatsManager* manager_; DummyStanzaChannel* stanzaChannel_; - DummyIQChannel* iqChannel_; IQRouter* iqRouter_; EventController* eventController_; ChatWindowFactory* chatWindowFactory_; diff --git a/Swift/Controllers/UnitTest/MockChatWindow.h b/Swift/Controllers/UnitTest/MockChatWindow.h index 1430f4f..f9b183d 100644 --- a/Swift/Controllers/UnitTest/MockChatWindow.h +++ b/Swift/Controllers/UnitTest/MockChatWindow.h @@ -19,12 +19,12 @@ namespace Swift { virtual ~MockChatWindow(); virtual std::string addMessage(const ChatMessage& message, const std::string& /*senderName*/, bool /*senderIsSelf*/, std::shared_ptr /*label*/, const std::string& /*avatarPath*/, const boost::posix_time::ptime& /*time*/) { - lastMessageBody_ = bodyFromMessage(message); + lastAddedMessage_ = message; return "id"; } virtual std::string addAction(const ChatMessage& message, const std::string& /*senderName*/, bool /*senderIsSelf*/, std::shared_ptr /*label*/, const std::string& /*avatarPath*/, const boost::posix_time::ptime& /*time*/) { - lastAddedActionBody_ =bodyFromMessage(message); + lastAddedAction_ = message; return "id"; } @@ -94,18 +94,27 @@ namespace Swift { virtual void setBookmarkState(RoomBookmarkState) {} static std::string bodyFromMessage(const ChatMessage& message) { + std::string body; std::shared_ptr text; + std::shared_ptr highlight; foreach (std::shared_ptr part, message.getParts()) { if ((text = std::dynamic_pointer_cast(part))) { - return text->text; + body += text->text; + } + else if ((highlight = std::dynamic_pointer_cast(part))) { + body += highlight->text; } } - return ""; + return body; + } + + void resetLastMessages() { + lastAddedMessage_ = lastAddedAction_ = lastAddedPresence_ = lastReplacedMessage_ = lastAddedSystemMessage_ = ChatMessage(); } std::string name_; - std::string lastMessageBody_; - std::string lastAddedActionBody_; + ChatMessage lastAddedMessage_; + ChatMessage lastAddedAction_; ChatMessage lastAddedPresence_; ChatMessage lastReplacedMessage_; ChatMessage lastAddedSystemMessage_; diff --git a/Swiften/Client/DummyStanzaChannel.h b/Swiften/Client/DummyStanzaChannel.h index 48b611c..fc2f05b 100644 --- a/Swiften/Client/DummyStanzaChannel.h +++ b/Swiften/Client/DummyStanzaChannel.h @@ -79,6 +79,16 @@ namespace Swift { return std::dynamic_pointer_cast(sentStanzas[index]); } + template size_t countSentStanzaOfType() { + size_t count = 0; + for (auto& stanza : sentStanzas) { + if (std::dynamic_pointer_cast(stanza)) { + count++; + } + } + return count; + } + std::vector getPeerCertificateChain() const { return std::vector(); } diff --git a/Swiften/Elements/MUCItem.h b/Swiften/Elements/MUCItem.h index 9ea6d77..3c89e68 100644 --- a/Swiften/Elements/MUCItem.h +++ b/Swiften/Elements/MUCItem.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011 Isode Limited. + * Copyright (c) 2011-2016 Isode Limited. * All rights reserved. * See the COPYING file for more information. */ @@ -8,9 +8,12 @@ #include #include + namespace Swift { struct MUCItem { MUCItem() {} + MUCItem(MUCOccupant::Affiliation affiliation, const JID& jid, MUCOccupant::Role role) : realJID(jid), affiliation(affiliation), role(role) {} + MUCItem(MUCOccupant::Affiliation affiliation, MUCOccupant::Role role) : affiliation(affiliation), role(role) {} boost::optional realJID; boost::optional nick; boost::optional affiliation; -- cgit v0.10.2-6-g49f6