From b7849f50877dffd2755445a986d856340eed59c2 Mon Sep 17 00:00:00 2001 From: Thanos Doukoudakis Date: Wed, 17 May 2017 14:15:07 +0100 Subject: Allow resending messages without 198 acks This patch will allow to resend messages that have not been successfully delivered to the server. It requires that the server will have stream management enabled as described in XEP-198. Test-Information: Tested the changes on Windows and Linux. Modified the code to force messages to fail, and then tested the resend functionality. Added ChatControllerTest, with unit Tests for the resend case. The rest of unit test pass. Change-Id: Id095528247b25a47e34c5632b8469ebd4c97149b diff --git a/Swift/Controllers/Chat/ChatController.cpp b/Swift/Controllers/Chat/ChatController.cpp index e299d03..5f441f8 100644 --- a/Swift/Controllers/Chat/ChatController.cpp +++ b/Swift/Controllers/Chat/ChatController.cpp @@ -359,6 +359,7 @@ void ChatController::setOnline(bool online) { if (!online) { for (auto& stanzaIdPair : unackedStanzas_) { chatWindow_->setAckState(stanzaIdPair.second, ChatWindow::Failed); + failedStanzas_[stanzaIdPair.second] = stanzaIdPair.first; } unackedStanzas_.clear(); diff --git a/Swift/Controllers/Chat/ChatController.h b/Swift/Controllers/Chat/ChatController.h index 716b3ed..447e263 100644 --- a/Swift/Controllers/Chat/ChatController.h +++ b/Swift/Controllers/Chat/ChatController.h @@ -96,8 +96,6 @@ namespace Swift { std::string myLastMessageUIID_; bool isInMUC_; std::string lastStatusChangeString_; - std::map, std::string> unackedStanzas_; - std::map requestedReceipts_; StatusShow::Type lastShownStatus_; Tristate contactSupportsReceipts_; diff --git a/Swift/Controllers/Chat/ChatControllerBase.cpp b/Swift/Controllers/Chat/ChatControllerBase.cpp index 0e86d6c..4d4ca86 100644 --- a/Swift/Controllers/Chat/ChatControllerBase.cpp +++ b/Swift/Controllers/Chat/ChatControllerBase.cpp @@ -20,6 +20,8 @@ #include #include #include +#include +#include #include #include #include @@ -44,6 +46,7 @@ ChatControllerBase::ChatControllerBase(const JID& self, StanzaChannel* stanzaCha chatWindow_->onAllMessagesRead.connect(boost::bind(&ChatControllerBase::handleAllMessagesRead, this)); chatWindow_->onSendMessageRequest.connect(boost::bind(&ChatControllerBase::handleSendMessageRequest, this, _1, _2)); chatWindow_->onContinuationsBroken.connect(boost::bind(&ChatControllerBase::handleContinuationsBroken, this)); + scopedConnectionResendMessage_ = chatWindow_->onResendMessageRequest.connect(boost::bind(&ChatControllerBase::handleResendMessageRequest, this, _1)); entityCapsProvider_->onCapsChanged.connect(boost::bind(&ChatControllerBase::handleCapsChanged, this, _1)); highlighter_ = highlightManager->createHighlighter(nickResolver); ChatControllerBase::setOnline(stanzaChannel->isAvailable() && iqRouter->isAvailable()); @@ -62,6 +65,7 @@ ChatWindow* ChatControllerBase::detachChatWindow() { chatWindow_->onContinuationsBroken.disconnect(boost::bind(&ChatControllerBase::handleContinuationsBroken, this)); chatWindow_->onSendMessageRequest.disconnect(boost::bind(&ChatControllerBase::handleSendMessageRequest, this, _1, _2)); chatWindow_->onAllMessagesRead.disconnect(boost::bind(&ChatControllerBase::handleAllMessagesRead, this)); + scopedConnectionResendMessage_.disconnect(); ChatWindow* chatWindow = chatWindow_; chatWindow_ = nullptr; return chatWindow; @@ -397,4 +401,22 @@ void ChatControllerBase::handleMediatedMUCInvitation(Message::ref message) { handleGeneralMUCInvitation(inviteEvent); } +void ChatControllerBase::handleResendMessageRequest(const std::string& id) { + if (failedStanzas_.find(id) != failedStanzas_.end()) { + if (auto resendMsg = std::dynamic_pointer_cast(failedStanzas_[id])) { + stanzaChannel_->sendMessage(resendMsg); + if (stanzaChannel_->getStreamManagementEnabled()) { + chatWindow_->setAckState(id, ChatWindow::Pending); + unackedStanzas_[failedStanzas_[id]] = id; + } + if (resendMsg->getPayload()) { + requestedReceipts_[resendMsg->getID()] = id; + chatWindow_->setMessageReceiptState(id, ChatWindow::ReceiptRequested); + } + lastWasPresence_ = false; + failedStanzas_.erase(id); + } + } +} + } diff --git a/Swift/Controllers/Chat/ChatControllerBase.h b/Swift/Controllers/Chat/ChatControllerBase.h index 9da4d88..527196c 100644 --- a/Swift/Controllers/Chat/ChatControllerBase.h +++ b/Swift/Controllers/Chat/ChatControllerBase.h @@ -115,6 +115,7 @@ namespace Swift { * What JID should be used for last message correction (XEP-0308) tracking. */ virtual JID messageCorrectionJID(const JID& fromJID) = 0; + virtual void handleResendMessageRequest(const std::string& id); private: IDGenerator idGenerator_; @@ -155,6 +156,10 @@ namespace Swift { std::string roomSecurityMarking_; std::string previousMessageSecurityMarking_; SettingsProvider* settings_; + boost::signals2::scoped_connection scopedConnectionResendMessage_; + std::map requestedReceipts_; + std::map, std::string> unackedStanzas_; + std::map> failedStanzas_; Chattables& chattables_; }; } diff --git a/Swift/Controllers/Chat/UnitTest/ChatControllerTest.cpp b/Swift/Controllers/Chat/UnitTest/ChatControllerTest.cpp new file mode 100644 index 0000000..e010656 --- /dev/null +++ b/Swift/Controllers/Chat/UnitTest/ChatControllerTest.cpp @@ -0,0 +1,162 @@ +/* + * Copyright (c) 2018 Isode Limited. + * All rights reserved. + * See the COPYING file for more information. + */ + +#include +#include + +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +using namespace Swift; + +/** + * Most of the ChatController tests are in ChatsManagerTest. + * New tests related with ChatController should be added here, + * and old tests should be migrated when possible. + */ + +class ExtendedChatController : public ChatController { +public: + ExtendedChatController(const JID& self, StanzaChannel* stanzaChannel, IQRouter* iqRouter, ChatWindowFactory* chatWindowFactory, const JID &contact, NickResolver* nickResolver, PresenceOracle* presenceOracle, AvatarManager* avatarManager, bool isInMUC, bool useDelayForLatency, UIEventStream* eventStream, TimerFactory* timerFactory, EventController* eventController, EntityCapsProvider* entityCapsProvider, bool userWantsReceipts, HistoryController* historyController, MUCRegistry* mucRegistry, HighlightManager* highlightManager, ClientBlockListManager* clientBlockListManager, std::shared_ptr chatMessageParser, AutoAcceptMUCInviteDecider* autoAcceptMUCInviteDecider, SettingsProvider* settings, Chattables& chattables) : + ChatController(self, stanzaChannel, iqRouter, chatWindowFactory, contact, nickResolver, presenceOracle, avatarManager, isInMUC, useDelayForLatency, eventStream, timerFactory, eventController, entityCapsProvider, userWantsReceipts, historyController, mucRegistry, highlightManager, clientBlockListManager, chatMessageParser, autoAcceptMUCInviteDecider, settings, chattables) { + } + + std::map, std::string> getUnackedStanzas() { return unackedStanzas_; } + std::map> getFailedStanzas() { return failedStanzas_; } +}; + +class ChatControllerTest : public ::testing::Test { +protected: + virtual void SetUp() { + self_ = JID("alice@wonderland.lit"); + other_ = JID("whiterabbit@wonderland.lit"); + stanzaChannel_ = new DummyStanzaChannel(); + iqChannel_ = new DummyIQChannel(); + iqRouter_ = new IQRouter(iqChannel_); + eventController_ = new EventController(); + xmppRoster_ = new XMPPRosterImpl(); + vCardManager_ = new VCardManager(self_, iqRouter_, vCardMemoryStorage_); + mucRegistry_ = new MUCRegistry(); + nickResolver_ = new NickResolver(self_, xmppRoster_, vCardManager_, mucRegistry_); + presenceOracle_ = new PresenceOracle(stanzaChannel_, xmppRoster_); + avatarManager_ = new NullAvatarManager(); + uiEventStream_ = new UIEventStream(); + timerFactory_ = new DummyTimerFactory(); + entityCapsProvider_ = new DummyEntityCapsProvider(); + settings_ = new DummySettingsProvider(); + highlightManager_ = new HighlightManager(settings_); + highlightManager_->resetToDefaultConfiguration(); + clientBlockListManager_ = new ClientBlockListManager(iqRouter_); + autoAcceptMUCInviteDecider_ = new AutoAcceptMUCInviteDecider(self_.getDomain(), xmppRoster_, settings_); + chatMessageParser_ = std::make_shared(std::map(), highlightManager_->getConfiguration(), ChatMessageParser::Mode::GroupChat); + mocks_ = new MockRepository(); + window_ = new MockChatWindow(); + chatWindowFactory_ = mocks_->InterfaceMock(); + mocks_->ExpectCall(chatWindowFactory_, ChatWindowFactory::createChatWindow).With(other_, uiEventStream_).Return(window_); + chattables_ = std::make_unique(); + + controller_ = new ExtendedChatController(self_, stanzaChannel_, iqRouter_, chatWindowFactory_, other_, nickResolver_, presenceOracle_, avatarManager_, false, false, uiEventStream_, timerFactory_, eventController_, entityCapsProvider_, false, nullptr, mucRegistry_, highlightManager_, clientBlockListManager_, chatMessageParser_, nullptr, settings_, *chattables_); + } + virtual void TearDown() { + delete controller_; + chattables_.reset(); + chatMessageParser_.reset(); + delete autoAcceptMUCInviteDecider_; + delete clientBlockListManager_; + delete highlightManager_; + delete settings_; + delete entityCapsProvider_; + delete timerFactory_; + delete uiEventStream_; + delete avatarManager_; + delete presenceOracle_; + delete nickResolver_; + delete mucRegistry_; + delete vCardManager_; + delete xmppRoster_; + delete eventController_; + delete iqRouter_; + delete iqChannel_; + delete stanzaChannel_; + } + + JID self_, other_; + AvatarManager* avatarManager_ = nullptr; + ExtendedChatController* controller_ = nullptr; + ChatWindowFactory* chatWindowFactory_; + ClientBlockListManager* clientBlockListManager_; + EventController* eventController_ = nullptr; + EntityCapsProvider* entityCapsProvider_ = nullptr; + IQChannel* iqChannel_ = nullptr; + IQRouter* iqRouter_ = nullptr; + MockRepository* mocks_; + MockChatWindow* window_; + MUCRegistry* mucRegistry_ = nullptr; + NickResolver* nickResolver_ = nullptr; + PresenceOracle* presenceOracle_ = nullptr; + DummyStanzaChannel* stanzaChannel_ = nullptr; + TimerFactory* timerFactory_; + XMPPRosterImpl* xmppRoster_ = nullptr; + UIEventStream* uiEventStream_; + VCardManager* vCardManager_ = nullptr; + VCardMemoryStorage* vCardMemoryStorage_ = nullptr; + DummySettingsProvider* settings_; + HighlightManager* highlightManager_; + std::shared_ptr chatMessageParser_; + AutoAcceptMUCInviteDecider* autoAcceptMUCInviteDecider_; + std::unique_ptr chattables_; + +}; + +TEST_F(ChatControllerTest, testResendMessage) { + std::string msgBody("TestMsg"); + stanzaChannel_->setStreamManagementEnabled(true); + window_->onSendMessageRequest(msgBody, false); + { + auto failedStanzas = controller_->getFailedStanzas(); + auto unackedStanzas = controller_->getUnackedStanzas(); + ASSERT_EQ(failedStanzas.size(), 0); + ASSERT_EQ(unackedStanzas.size(), 1); + } + //Disconnecting to fail the stanza + controller_->setOnline(false); + controller_->setOnline(true); + { + auto failedStanzas = controller_->getFailedStanzas(); + auto unackedStanzas = controller_->getUnackedStanzas(); + ASSERT_EQ(failedStanzas.size(), 1); + ASSERT_EQ(unackedStanzas.size(), 0); + } + window_->onResendMessageRequest("id"); + { + auto failedStanzas = controller_->getFailedStanzas(); + auto unackedStanzas = controller_->getUnackedStanzas(); + ASSERT_EQ(failedStanzas.size(), 0); + ASSERT_EQ(unackedStanzas.size(), 1); + } +} diff --git a/Swift/Controllers/SConscript b/Swift/Controllers/SConscript index 0f50ac9..31b4541 100644 --- a/Swift/Controllers/SConscript +++ b/Swift/Controllers/SConscript @@ -97,6 +97,7 @@ if env["SCONS_STAGE"] == "build" : File("Chat/UnitTest/ChatListWindowChatTest.cpp"), File("Chat/UnitTest/ChatMessageParserTest.cpp"), File("Chat/UnitTest/ChatsManagerTest.cpp"), + File("Chat/UnitTest/ChatControllerTest.cpp"), File("Chat/UnitTest/MUCControllerTest.cpp"), File("Roster/UnitTest/LeastCommonSubsequenceTest.cpp"), File("Roster/UnitTest/RosterControllerTest.cpp"), diff --git a/Swift/Controllers/UIInterfaces/ChatWindow.h b/Swift/Controllers/UIInterfaces/ChatWindow.h index 13cbb7d..507269f 100644 --- a/Swift/Controllers/UIInterfaces/ChatWindow.h +++ b/Swift/Controllers/UIInterfaces/ChatWindow.h @@ -254,6 +254,7 @@ namespace Swift { boost::signals2::signal onClosed; boost::signals2::signal onAllMessagesRead; boost::signals2::signal onSendMessageRequest; + boost::signals2::signal onResendMessageRequest; boost::signals2::signal onSendCorrectionMessageRequest; boost::signals2::signal onUserTyping; boost::signals2::signal onUserCancelsTyping; diff --git a/Swift/QtUI/QtChatWindow.cpp b/Swift/QtUI/QtChatWindow.cpp index cbdab48..7c0a9d8 100644 --- a/Swift/QtUI/QtChatWindow.cpp +++ b/Swift/QtUI/QtChatWindow.cpp @@ -1054,4 +1054,11 @@ void QtChatWindow::handleFocusTimerTick() { focusTimer_.reset(); } +void QtChatWindow::resendMessage(const std::string& id) { + if (!isOnline_ || (blockingState_ == IsBlocked)) { + return; + } + onResendMessageRequest(id); +} + } diff --git a/Swift/QtUI/QtChatWindow.h b/Swift/QtUI/QtChatWindow.h index 1269165..8cc3283 100644 --- a/Swift/QtUI/QtChatWindow.h +++ b/Swift/QtUI/QtChatWindow.h @@ -94,6 +94,7 @@ namespace Swift { void replaceMessage(const ChatMessage& message, const std::string& id, const boost::posix_time::ptime& time); void replaceWithAction(const ChatMessage& message, const std::string& id, const boost::posix_time::ptime& time); + void resendMessage(const std::string& id); // File transfer related stuff std::string addFileTransfer(const std::string& senderName, const std::string& avatarPath, bool senderIsSelf, const std::string& filename, const boost::uintmax_t sizeInBytes, const std::string& description); void setFileTransferProgress(std::string id, const int percentageDone); diff --git a/Swift/QtUI/QtWebKitChatView.cpp b/Swift/QtUI/QtWebKitChatView.cpp index 92f0a2c..8ced8fc 100644 --- a/Swift/QtUI/QtWebKitChatView.cpp +++ b/Swift/QtUI/QtWebKitChatView.cpp @@ -53,6 +53,9 @@ const QString QtWebKitChatView::ButtonFileTransferSendRequest = QString("filetra const QString QtWebKitChatView::ButtonFileTransferAcceptRequest = QString("filetransfer-acceptrequest"); const QString QtWebKitChatView::ButtonFileTransferOpenFile = QString("filetransfer-openfile"); const QString QtWebKitChatView::ButtonMUCInvite = QString("mucinvite"); +const QString QtWebKitChatView::ButtonResendMessage = QString("resend-message"); +const QString QtWebKitChatView::ButtonResendPopup = QString("popup-resend"); + namespace { const double minimalFontScaling = 0.7; @@ -606,6 +609,10 @@ QString QtWebKitChatView::buildChatWindowButton(const QString& name, const QStri return html; } +QString QtWebKitChatView::buildChatWindowPopupImageButton(const QString& title, const QString& path, const QString& arg1, const QString& arg2, const QString& arg3, const QString& arg4, const QString& arg5) { + return QString("
Resend
").arg(path).arg(title).arg(QtWebKitChatView::ButtonResendPopup).arg(QtWebKitChatView::ButtonResendMessage).arg(encodeButtonArgument(arg1)).arg(encodeButtonArgument(arg2)).arg(encodeButtonArgument(arg3)).arg(encodeButtonArgument(arg4)).arg(encodeButtonArgument(arg5)); +} + void QtWebKitChatView::resizeEvent(QResizeEvent* event) { // This code ensures that if the user is scrolled all to the bottom of a chat view, // the view stays scrolled to the bottom if the view is resized or if the message @@ -807,6 +814,20 @@ void QtWebKitChatView::handleHTMLButtonClicked(QString id, QString encodedArgume eventStream_->send(std::make_shared(Q2PSTRING(roomJID), Q2PSTRING(password), boost::optional(), false, false, isImpromptu.contains("true"), isContinuation.contains("true"))); setMUCInvitationJoined(elementID); } + else if (id.startsWith(ButtonResendPopup)) { + QString chatID = arg1; + QWebElement message = document_.findFirst("#" + arg1); + if (!message.isNull()) { + QWebElement popuptext = message.findFirst("span#resendPopup.popuptext"); + if (!popuptext.isNull()) { + popuptext.toggleClass("show"); + } + } + } + else if (id.startsWith(ButtonResendMessage)) { + QString chatID = arg1; + window_->resendMessage(Q2PSTRING(chatID)); + } else { SWIFT_LOG(debug) << "Unknown HTML button! ( " << Q2PSTRING(id) << " )" << std::endl; } @@ -964,7 +985,9 @@ void QtWebKitChatView::setAckState(std::string const& id, ChatWindow::AckState s xml = ""; displayReceiptInfo(P2QSTRING(id), true); break; - case ChatWindow::Failed: xml = ""; break; + case ChatWindow::Failed: + xml = buildChatWindowPopupImageButton(tr("This message may not have been sent. Click to resend."), "qrc:/icons/error.png", P2QSTRING(id)); + break; } setAckXML(P2QSTRING(id), xml); } diff --git a/Swift/QtUI/QtWebKitChatView.h b/Swift/QtUI/QtWebKitChatView.h index a2eafe6..f0b4459 100644 --- a/Swift/QtUI/QtWebKitChatView.h +++ b/Swift/QtUI/QtWebKitChatView.h @@ -42,6 +42,8 @@ namespace Swift { static const QString ButtonFileTransferAcceptRequest; static const QString ButtonFileTransferOpenFile; static const QString ButtonMUCInvite; + static const QString ButtonResendMessage; + static const QString ButtonResendPopup; public: QtWebKitChatView(QtChatWindow* window, UIEventStream* eventStream, QtChatTheme* theme, QWidget* parent, SettingsProvider* settings, bool disableAutoScroll = false); ~QtWebKitChatView() override; @@ -157,6 +159,7 @@ namespace Swift { QString getHighlightSpanStart(const HighlightAction& highlight); QString chatMessageToHTML(const ChatWindow::ChatMessage& message); static QString buildChatWindowButton(const QString& name, const QString& id, const QString& arg1 = QString(), const QString& arg2 = QString(), const QString& arg3 = QString(), const QString& arg4 = QString(), const QString& arg5 = QString()); + static QString buildChatWindowPopupImageButton(const QString& title, const QString& path, const QString& arg1 = QString(), const QString& arg2 = QString(), const QString& arg3 = QString(), const QString& arg4 = QString(), const QString& arg5 = QString()); protected: void resizeEvent(QResizeEvent* event) override; diff --git a/Swift/resources/themes/Default/main.css b/Swift/resources/themes/Default/main.css index 64ab7bf..aa3d92d 100644 --- a/Swift/resources/themes/Default/main.css +++ b/Swift/resources/themes/Default/main.css @@ -234,3 +234,47 @@ div.time { span.swift_receipt > img { height: 12px; } + +.resendButton { +} + +.popup { + position: relative; + display: inline-block; + cursor: pointer; + user-select: none; +} + +/* The actual popup */ +.popup .popuptext { + visibility: hidden; + width: 50px; + background-color: #6eb6ce; + color: #fff; + text-align: center; + border-radius: 6px; + padding: 8px 0; + position: absolute; + z-index: 1; + bottom: 125%; + left: 50%; + margin-left: -25px; +} + +/* Popup arrow */ +.popup .popuptext::after { + content: ""; + position: absolute; + top: 100%; + left: 50%; + margin-left: -5px; + border-width: 5px; + border-style: solid; + border-color: #6eb6ce transparent transparent transparent; +} + +/* Toggle this class - hide and show the popup */ +.popup .show { + visibility: visible; + animation: fadeIn 1s; +} diff --git a/Swiften/Client/DummyStanzaChannel.h b/Swiften/Client/DummyStanzaChannel.h index 4cc0f7e..1ba70ad 100644 --- a/Swiften/Client/DummyStanzaChannel.h +++ b/Swiften/Client/DummyStanzaChannel.h @@ -48,8 +48,12 @@ namespace Swift { return available_; } + virtual void setStreamManagementEnabled(bool enable) { + streamManagement_ = enable; + } + virtual bool getStreamManagementEnabled() const { - return false; + return streamManagement_; } template bool isRequestAtIndex(size_t index, const JID& jid, IQ::Type type) { @@ -101,5 +105,6 @@ namespace Swift { bool available_ = true; bool uniqueIDs_ = false; unsigned int idCounter_ = 0; + bool streamManagement_ = false; }; } -- cgit v0.10.2-6-g49f6