diff options
author | Thanos Doukoudakis <thanos.doukoudakis@isode.com> | 2017-05-17 13:15:07 (GMT) |
---|---|---|
committer | Thanos Doukoudakis <thanos.doukoudakis@isode.com> | 2018-05-14 09:55:30 (GMT) |
commit | b7849f50877dffd2755445a986d856340eed59c2 (patch) | |
tree | a76adbbc613525bec214995c176e6f0e37d8b5a0 /Swift | |
parent | 4663da31954d989f8535b94c4a78a0f4515542a4 (diff) | |
download | swift-b7849f50877dffd2755445a986d856340eed59c2.zip swift-b7849f50877dffd2755445a986d856340eed59c2.tar.bz2 |
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
Diffstat (limited to 'Swift')
-rw-r--r-- | Swift/Controllers/Chat/ChatController.cpp | 1 | ||||
-rw-r--r-- | Swift/Controllers/Chat/ChatController.h | 2 | ||||
-rw-r--r-- | Swift/Controllers/Chat/ChatControllerBase.cpp | 22 | ||||
-rw-r--r-- | Swift/Controllers/Chat/ChatControllerBase.h | 5 | ||||
-rw-r--r-- | Swift/Controllers/Chat/UnitTest/ChatControllerTest.cpp | 162 | ||||
-rw-r--r-- | Swift/Controllers/SConscript | 1 | ||||
-rw-r--r-- | Swift/Controllers/UIInterfaces/ChatWindow.h | 1 | ||||
-rw-r--r-- | Swift/QtUI/QtChatWindow.cpp | 7 | ||||
-rw-r--r-- | Swift/QtUI/QtChatWindow.h | 1 | ||||
-rw-r--r-- | Swift/QtUI/QtWebKitChatView.cpp | 25 | ||||
-rw-r--r-- | Swift/QtUI/QtWebKitChatView.h | 3 | ||||
-rw-r--r-- | Swift/resources/themes/Default/main.css | 44 |
12 files changed, 271 insertions, 3 deletions
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 @@ -361,2 +361,3 @@ void ChatController::setOnline(bool online) { chatWindow_->setAckState(stanzaIdPair.second, ChatWindow::Failed); + failedStanzas_[stanzaIdPair.second] = stanzaIdPair.first; } 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 @@ -98,4 +98,2 @@ namespace Swift { std::string lastStatusChangeString_; - std::map<std::shared_ptr<Stanza>, std::string> unackedStanzas_; - std::map<std::string, std::string> requestedReceipts_; StatusShow::Type lastShownStatus_; 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 @@ -22,2 +22,4 @@ #include <Swiften/Elements/Delay.h> +#include <Swiften/Elements/DeliveryReceipt.h> +#include <Swiften/Elements/DeliveryReceiptRequest.h> #include <Swiften/Elements/MUCInvitationPayload.h> @@ -46,2 +48,3 @@ ChatControllerBase::ChatControllerBase(const JID& self, StanzaChannel* stanzaCha 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)); @@ -64,2 +67,3 @@ ChatWindow* ChatControllerBase::detachChatWindow() { chatWindow_->onAllMessagesRead.disconnect(boost::bind(&ChatControllerBase::handleAllMessagesRead, this)); + scopedConnectionResendMessage_.disconnect(); ChatWindow* chatWindow = chatWindow_; @@ -399,2 +403,20 @@ void ChatControllerBase::handleMediatedMUCInvitation(Message::ref message) { +void ChatControllerBase::handleResendMessageRequest(const std::string& id) { + if (failedStanzas_.find(id) != failedStanzas_.end()) { + if (auto resendMsg = std::dynamic_pointer_cast<Message>(failedStanzas_[id])) { + stanzaChannel_->sendMessage(resendMsg); + if (stanzaChannel_->getStreamManagementEnabled()) { + chatWindow_->setAckState(id, ChatWindow::Pending); + unackedStanzas_[failedStanzas_[id]] = id; + } + if (resendMsg->getPayload<DeliveryReceiptRequest>()) { + 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 @@ -117,2 +117,3 @@ namespace Swift { virtual JID messageCorrectionJID(const JID& fromJID) = 0; + virtual void handleResendMessageRequest(const std::string& id); @@ -157,2 +158,6 @@ namespace Swift { SettingsProvider* settings_; + boost::signals2::scoped_connection scopedConnectionResendMessage_; + std::map<std::string, std::string> requestedReceipts_; + std::map<std::shared_ptr<Stanza>, std::string> unackedStanzas_; + std::map<std::string, std::shared_ptr<Stanza>> 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 <memory> +#include <string> + +#include <gtest/gtest.h> +#include <hippomocks.h> + +#include <Swiften/Avatars/NullAvatarManager.h> +#include <Swiften/Client/ClientBlockListManager.h> +#include <Swiften/Client/DummyStanzaChannel.h> +#include <Swiften/Client/NickResolver.h> +#include <Swiften/Disco/DummyEntityCapsProvider.h> +#include <Swiften/Network/DummyTimerFactory.h> +#include <Swiften/Queries/DummyIQChannel.h> +#include <Swiften/Roster/XMPPRoster.h> +#include <Swiften/Roster/XMPPRosterImpl.h> +#include <Swiften/VCards/VCardManager.h> +#include <Swiften/VCards/VCardMemoryStorage.h> + +#include <Swift/Controllers/Chat/AutoAcceptMUCInviteDecider.h> +#include <Swift/Controllers/Chat/ChatController.h> +#include <Swift/Controllers/Chat/ChatMessageParser.h> +#include <Swift/Controllers/Chat/Chattables.h> +#include <Swift/Controllers/Settings/DummySettingsProvider.h> +#include <Swift/Controllers/UIEvents/UIEventStream.h> +#include <Swift/Controllers/UIInterfaces/ChatWindowFactory.h> +#include <Swift/Controllers/UnitTest/MockChatWindow.h> +#include <Swift/Controllers/XMPPEvents/EventController.h> + +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> 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::shared_ptr<Stanza>, std::string> getUnackedStanzas() { return unackedStanzas_; } + std::map<std::string, std::shared_ptr<Stanza>> 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<ChatMessageParser>(std::map<std::string, std::string>(), highlightManager_->getConfiguration(), ChatMessageParser::Mode::GroupChat); + mocks_ = new MockRepository(); + window_ = new MockChatWindow(); + chatWindowFactory_ = mocks_->InterfaceMock<ChatWindowFactory>(); + mocks_->ExpectCall(chatWindowFactory_, ChatWindowFactory::createChatWindow).With(other_, uiEventStream_).Return(window_); + chattables_ = std::make_unique<Chattables>(); + + 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> chatMessageParser_; + AutoAcceptMUCInviteDecider* autoAcceptMUCInviteDecider_; + std::unique_ptr<Chattables> 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 @@ -99,2 +99,3 @@ if env["SCONS_STAGE"] == "build" : File("Chat/UnitTest/ChatsManagerTest.cpp"), + File("Chat/UnitTest/ChatControllerTest.cpp"), File("Chat/UnitTest/MUCControllerTest.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 @@ -256,2 +256,3 @@ namespace Swift { boost::signals2::signal<void (const std::string&, bool isCorrection)> onSendMessageRequest; + boost::signals2::signal<void (const std::string&)> onResendMessageRequest; boost::signals2::signal<void ()> onSendCorrectionMessageRequest; 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 @@ -1056,2 +1056,9 @@ void QtChatWindow::handleFocusTimerTick() { +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 @@ -96,2 +96,3 @@ namespace Swift { 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 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 @@ -55,2 +55,5 @@ const QString QtWebKitChatView::ButtonFileTransferOpenFile = QString("filetransf const QString QtWebKitChatView::ButtonMUCInvite = QString("mucinvite"); +const QString QtWebKitChatView::ButtonResendMessage = QString("resend-message"); +const QString QtWebKitChatView::ButtonResendPopup = QString("popup-resend"); + @@ -608,2 +611,6 @@ QString QtWebKitChatView::buildChatWindowButton(const QString& name, const QStri +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("<div class=\"popup\" onclick='chatwindow.buttonClicked(\"%3\", \"%5\", \"6\", \"7\", \"8\", \"9\")' \"><img src='%1' title='%2'/><span class=\"popuptext\" id=\"resendPopup\" ><div class=\"resendButton\" onclick='chatwindow.buttonClicked(\"%4\", \"%5\", \"6\", \"7\", \"8\", \"9\")'>Resend</div></span></div>").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) { @@ -809,2 +816,16 @@ void QtWebKitChatView::handleHTMLButtonClicked(QString id, QString encodedArgume } + 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 { @@ -966,3 +987,5 @@ void QtWebKitChatView::setAckState(std::string const& id, ChatWindow::AckState s break; - case ChatWindow::Failed: xml = "<img src='qrc:/icons/error.png' title='" + tr("This message may not have been transmitted.") + "'/>"; break; + case ChatWindow::Failed: + xml = buildChatWindowPopupImageButton(tr("This message may not have been sent. Click to resend."), "qrc:/icons/error.png", P2QSTRING(id)); + break; } 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 @@ -44,2 +44,4 @@ namespace Swift { static const QString ButtonMUCInvite; + static const QString ButtonResendMessage; + static const QString ButtonResendPopup; public: @@ -159,2 +161,3 @@ namespace Swift { 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()); 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 @@ -236 +236,45 @@ span.swift_receipt > img { } + +.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; +} |