summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorThanos Doukoudakis <thanos.doukoudakis@isode.com>2017-03-24 14:15:52 (GMT)
committerThanos Doukoudakis <thanos.doukoudakis@isode.com>2017-04-06 14:27:45 (GMT)
commit7d2e5e200d8449a4492c6fafa4811197e6fbe40b (patch)
treea4cc9d8d6eb6d98d60dee9e2b928a9e96392ebe9 /Swiften
parent20c50e3960ee68d26fc93738f03d27a6833b28bb (diff)
downloadswift-7d2e5e200d8449a4492c6fafa4811197e6fbe40b.zip
swift-7d2e5e200d8449a4492c6fafa4811197e6fbe40b.tar.bz2
Reset the chat state to active after a few seconds
Fix for swift-217 When a user sends a composing Chat State Notification a timer will start. If the user doesn't send or cancel the message before the timer expires, an active CSN will be sent. Test Info: Build on Windows and unit test pass. Tested the new functionality with Windows and Linux Client. Added some test cases to cover the scenario that user goes idle while a CSN composing state has been sent. Updated ChatStateNotifierTest to use gtest. Updated ChatsManagerTest to use a valid TimerFactory object instead of nullptr. Change-Id: I35201947e4f042805a6d9df1340a0335effcd657
Diffstat (limited to 'Swiften')
-rw-r--r--Swiften/Chat/ChatStateNotifier.cpp25
-rw-r--r--Swiften/Chat/ChatStateNotifier.h8
-rw-r--r--Swiften/Chat/UnitTest/ChatStateNotifierTest.cpp279
3 files changed, 178 insertions, 134 deletions
diff --git a/Swiften/Chat/ChatStateNotifier.cpp b/Swiften/Chat/ChatStateNotifier.cpp
index cbb9b0b..48a65ab 100644
--- a/Swiften/Chat/ChatStateNotifier.cpp
+++ b/Swiften/Chat/ChatStateNotifier.cpp
@@ -6,6 +6,7 @@
#include <Swiften/Chat/ChatStateNotifier.h>
+#include <cassert>
#include <memory>
#include <boost/bind.hpp>
@@ -14,16 +15,26 @@
#include <Swiften/Disco/EntityCapsProvider.h>
#include <Swiften/Elements/ChatState.h>
#include <Swiften/Elements/Message.h>
+#include <Swiften/Network/Timer.h>
+#include <Swiften/Network/TimerFactory.h>
+
namespace Swift {
-ChatStateNotifier::ChatStateNotifier(StanzaChannel* stanzaChannel, const JID& contact, EntityCapsProvider* entityCapsManager) : stanzaChannel_(stanzaChannel), entityCapsManager_(entityCapsManager), contact_(contact) {
+
+ChatStateNotifier::ChatStateNotifier(StanzaChannel* stanzaChannel, const JID& contact, EntityCapsProvider* entityCapsManager, TimerFactory* timerFactory, int idleTimeInMilliSecs) : stanzaChannel_(stanzaChannel), entityCapsManager_(entityCapsManager), contact_(contact) {
setContact(contact);
entityCapsManager_->onCapsChanged.connect(boost::bind(&ChatStateNotifier::handleCapsChanged, this, _1));
+ assert(timerFactory);
+ idleTimer_ = timerFactory->createTimer(idleTimeInMilliSecs);
+ assert(!!idleTimer_);
+ idleTimer_->onTick.connect(boost::bind(&ChatStateNotifier::userBecameIdleWhileTyping, this));
}
ChatStateNotifier::~ChatStateNotifier() {
entityCapsManager_->onCapsChanged.disconnect(boost::bind(&ChatStateNotifier::handleCapsChanged, this, _1));
+ idleTimer_->stop();
+ idleTimer_->onTick.disconnect(boost::bind(&ChatStateNotifier::userBecameIdleWhileTyping, this));
}
void ChatStateNotifier::setContact(const JID& contact) {
@@ -39,24 +50,36 @@ void ChatStateNotifier::setContactIsOnline(bool online) {
}
void ChatStateNotifier::setUserIsTyping() {
+ idleTimer_->stop();
bool should = contactShouldReceiveStates();
if (should && !userIsTyping_) {
userIsTyping_ = true;
changeState(ChatState::Composing);
}
+ if (should) {
+ idleTimer_->start();
+ }
}
void ChatStateNotifier::userSentMessage() {
+ idleTimer_->stop();
userIsTyping_ = false;
}
void ChatStateNotifier::userCancelledNewMessage() {
+ idleTimer_->stop();
if (userIsTyping_) {
userIsTyping_ = false;
changeState(ChatState::Active);
}
}
+void ChatStateNotifier::userBecameIdleWhileTyping() {
+ // For now we are returning to active state. When support for the Paused, Inactive and Gone states
+ // is implemeted, this function should Implement the Pause/Inactive functionality.
+ userCancelledNewMessage();
+}
+
void ChatStateNotifier::receivedMessageFromContact(bool hasActiveElement) {
contactHasSentActive_ = hasActiveElement;
}
diff --git a/Swiften/Chat/ChatStateNotifier.h b/Swiften/Chat/ChatStateNotifier.h
index a7af9e4..2b24c76 100644
--- a/Swiften/Chat/ChatStateNotifier.h
+++ b/Swiften/Chat/ChatStateNotifier.h
@@ -18,10 +18,13 @@
namespace Swift {
class StanzaChannel;
class EntityCapsProvider;
+ class TimerFactory;
+ class Timer;
+
class SWIFTEN_API ChatStateNotifier {
public:
- ChatStateNotifier(StanzaChannel* stanzaChannel, const JID& contact, EntityCapsProvider* entityCapsManager);
+ ChatStateNotifier(StanzaChannel* stanzaChannel, const JID& contact, EntityCapsProvider* entityCapsManager, TimerFactory* timerFactory, int idleTimeInMilliSecs);
~ChatStateNotifier();
void setContact(const JID& contact);
@@ -36,6 +39,7 @@ namespace Swift {
void setContactIsOnline(bool online);
private:
+ void userBecameIdleWhileTyping();
bool contactShouldReceiveStates();
void changeState(ChatState::ChatStateType type);
void handleCapsChanged(const JID& contact);
@@ -48,5 +52,7 @@ namespace Swift {
bool contactHasSentActive_;
bool userIsTyping_;
bool contactIsOnline_;
+ std::shared_ptr<Timer> idleTimer_;
+
};
}
diff --git a/Swiften/Chat/UnitTest/ChatStateNotifierTest.cpp b/Swiften/Chat/UnitTest/ChatStateNotifierTest.cpp
index 7eeb531..efd37d9 100644
--- a/Swiften/Chat/UnitTest/ChatStateNotifierTest.cpp
+++ b/Swiften/Chat/UnitTest/ChatStateNotifierTest.cpp
@@ -6,163 +6,178 @@
#include <boost/bind.hpp>
-#include <cppunit/extensions/HelperMacros.h>
-#include <cppunit/extensions/TestFactoryRegistry.h>
+#include <gtest/gtest.h>
#include <Swiften/Chat/ChatStateNotifier.h>
#include <Swiften/Client/DummyStanzaChannel.h>
#include <Swiften/Disco/DummyEntityCapsProvider.h>
+#include <Swiften/Network/DummyTimerFactory.h>
using namespace Swift;
-class ChatStateNotifierTest : public CppUnit::TestFixture {
- CPPUNIT_TEST_SUITE(ChatStateNotifierTest);
- CPPUNIT_TEST(testStartTypingReply_CapsNotIncluded);
- CPPUNIT_TEST(testStartTypingReply_CapsIncluded);
- CPPUNIT_TEST(testCancelledNewMessage);
- CPPUNIT_TEST(testContinueTypingReply_CapsIncluded);
- CPPUNIT_TEST(testTypeReplies_WentOffline);
- CPPUNIT_TEST(testContactShouldReceiveStates_CapsOnly);
- CPPUNIT_TEST(testContactShouldReceiveStates_CapsNorActive);
- CPPUNIT_TEST(testContactShouldReceiveStates_ActiveOverrideOn);
- CPPUNIT_TEST(testContactShouldReceiveStates_ActiveOverrideOff);
- CPPUNIT_TEST_SUITE_END();
-
-public:
- void setUp() {
+class ChatStateNotifierTest : public ::testing::Test {
+
+protected:
+ virtual void SetUp() {
stanzaChannel = new DummyStanzaChannel();
stanzaChannel->setAvailable(true);
entityCapsProvider = new DummyEntityCapsProvider();
- notifier_ = new ChatStateNotifier(stanzaChannel, JID("foo@bar.com/baz"), entityCapsProvider);
+ timerFactory_ = new DummyTimerFactory();
+ notifier_ = new ChatStateNotifier(stanzaChannel, JID("foo@bar.com/baz"), entityCapsProvider, timerFactory_, 2);
notifier_->setContactIsOnline(true);
}
- void tearDown() {
+ virtual void TearDown() {
delete notifier_;
+ delete timerFactory_;
delete entityCapsProvider;
delete stanzaChannel;
}
- void testStartTypingReply_CapsNotIncluded() {
- notifier_->setUserIsTyping();
- CPPUNIT_ASSERT_EQUAL(0, getComposingCount());
- }
-
- void testSendTwoMessages() {
- setContactHas85Caps();
- notifier_->setUserIsTyping();
- notifier_->userSentMessage();
- notifier_->setUserIsTyping();
- notifier_->userSentMessage();
- CPPUNIT_ASSERT_EQUAL(2, getComposingCount());
- }
-
- void testCancelledNewMessage() {
- setContactHas85Caps();
- notifier_->setUserIsTyping();
- notifier_->userCancelledNewMessage();
- CPPUNIT_ASSERT_EQUAL(1, getComposingCount());
- CPPUNIT_ASSERT_EQUAL(1, getActiveCount());
- CPPUNIT_ASSERT_EQUAL(ChatState::Active, stanzaChannel->sentStanzas[stanzaChannel->sentStanzas.size()-1]->getPayload<ChatState>()->getChatState());
- }
-
-
- void testContactShouldReceiveStates_CapsOnly() {
- setContactHas85Caps();
- std::shared_ptr<Message> message(new Message());
- notifier_->addChatStateRequest(message);
- CPPUNIT_ASSERT(message->getPayload<ChatState>());
- CPPUNIT_ASSERT_EQUAL(ChatState::Active, message->getPayload<ChatState>()->getChatState());
- }
-
- void testContactShouldReceiveStates_CapsNorActive() {
- std::shared_ptr<Message> message(new Message());
- notifier_->addChatStateRequest(message);
- CPPUNIT_ASSERT(!message->getPayload<ChatState>());
- }
-
- void testContactShouldReceiveStates_ActiveOverrideOn() {
- notifier_->receivedMessageFromContact(true);
- std::shared_ptr<Message> message(new Message());
- notifier_->addChatStateRequest(message);
- CPPUNIT_ASSERT(message->getPayload<ChatState>());
- CPPUNIT_ASSERT_EQUAL(ChatState::Active, message->getPayload<ChatState>()->getChatState());
- }
-
- void testContactShouldReceiveStates_ActiveOverrideOff() {
- setContactHas85Caps();
- notifier_->receivedMessageFromContact(false);
- /* I originally read the MUST NOT send after receiving without Active and
- * thought this should check for false, but I later found it was OPTIONAL
- * (MAY) behaviour only for if you didn't receive caps.
- */
- std::shared_ptr<Message> message(new Message());
- notifier_->addChatStateRequest(message);
- CPPUNIT_ASSERT(message->getPayload<ChatState>());
- CPPUNIT_ASSERT_EQUAL(ChatState::Active, message->getPayload<ChatState>()->getChatState());
- }
-
-
- void testStartTypingReply_CapsIncluded() {
- setContactHas85Caps();
- notifier_->setUserIsTyping();
- CPPUNIT_ASSERT_EQUAL(1, getComposingCount());
- }
-
- void testContinueTypingReply_CapsIncluded() {
- setContactHas85Caps();
- notifier_->setUserIsTyping();
- notifier_->setUserIsTyping();
- notifier_->setUserIsTyping();
- CPPUNIT_ASSERT_EQUAL(1, getComposingCount());
- notifier_->userSentMessage();
- notifier_->setUserIsTyping();
- CPPUNIT_ASSERT_EQUAL(2, getComposingCount());
-
+ void setContactHas85Caps() {
+ DiscoInfo::ref caps(new DiscoInfo());
+ caps->addFeature(DiscoInfo::ChatStatesFeature);
+ entityCapsProvider->caps[JID("foo@bar.com/baz")] = caps;
+ entityCapsProvider->onCapsChanged(JID("foo@bar.com/baz"));
}
- void testTypeReplies_WentOffline() {
- setContactHas85Caps();
- notifier_->setUserIsTyping();
- CPPUNIT_ASSERT_EQUAL(1, getComposingCount());
- notifier_->setContactIsOnline(false);
- notifier_->userSentMessage();
- notifier_->setUserIsTyping();
- CPPUNIT_ASSERT_EQUAL(1, getComposingCount());
- }
-
- private:
- void setContactHas85Caps() {
- DiscoInfo::ref caps(new DiscoInfo());
- caps->addFeature(DiscoInfo::ChatStatesFeature);
- entityCapsProvider->caps[JID("foo@bar.com/baz")] = caps;
- entityCapsProvider->onCapsChanged(JID("foo@bar.com/baz"));
- }
-
- int getComposingCount() const {
- int result = 0;
- for (auto&& stanza : stanzaChannel->sentStanzas) {
- if (stanza->getPayload<ChatState>() && stanza->getPayload<ChatState>()->getChatState() == ChatState::Composing) {
- result++;
- }
+ int getComposingCount() const {
+ int result = 0;
+ for (auto&& stanza : stanzaChannel->sentStanzas) {
+ if (stanza->getPayload<ChatState>() && stanza->getPayload<ChatState>()->getChatState() == ChatState::Composing) {
+ result++;
}
- return result;
}
+ return result;
+ }
- int getActiveCount() const {
- int result = 0;
- for (auto&& stanza : stanzaChannel->sentStanzas) {
- if (stanza->getPayload<ChatState>() && stanza->getPayload<ChatState>()->getChatState() == ChatState::Active) {
- result++;
- }
+ int getActiveCount() const {
+ int result = 0;
+ for (auto&& stanza : stanzaChannel->sentStanzas) {
+ if (stanza->getPayload<ChatState>() && stanza->getPayload<ChatState>()->getChatState() == ChatState::Active) {
+ result++;
}
- return result;
}
+ return result;
+ }
- private:
- DummyStanzaChannel* stanzaChannel;
- DummyEntityCapsProvider* entityCapsProvider;
- ChatStateNotifier* notifier_;
+ DummyStanzaChannel* stanzaChannel;
+ DummyEntityCapsProvider* entityCapsProvider;
+ DummyTimerFactory* timerFactory_;
+ ChatStateNotifier* notifier_;
};
-CPPUNIT_TEST_SUITE_REGISTRATION(ChatStateNotifierTest);
+TEST_F(ChatStateNotifierTest, testStartTypingReply_CapsNotIncluded) {
+ notifier_->setUserIsTyping();
+ ASSERT_EQ(0, getComposingCount());
+}
+
+TEST_F(ChatStateNotifierTest, testSendTwoMessages) {
+ setContactHas85Caps();
+ notifier_->setUserIsTyping();
+ notifier_->userSentMessage();
+ notifier_->setUserIsTyping();
+ notifier_->userSentMessage();
+ ASSERT_EQ(2, getComposingCount());
+}
+
+TEST_F(ChatStateNotifierTest, testCancelledNewMessage) {
+ setContactHas85Caps();
+ notifier_->setUserIsTyping();
+ notifier_->userCancelledNewMessage();
+ ASSERT_EQ(1, getComposingCount());
+ ASSERT_EQ(1, getActiveCount());
+ ASSERT_EQ(ChatState::Active, stanzaChannel->sentStanzas[stanzaChannel->sentStanzas.size() - 1]->getPayload<ChatState>()->getChatState());
+}
+
+TEST_F(ChatStateNotifierTest, testIdleWhileTypingNewMessage) {
+ setContactHas85Caps();
+ //The channel should be empty
+ ASSERT_EQ(0, getComposingCount());
+ ASSERT_EQ(0, getActiveCount());
+ notifier_->setUserIsTyping();
+ timerFactory_->setTime(1);
+ //1 Composing stanza is expected
+ ASSERT_EQ(1, getComposingCount());
+ ASSERT_EQ(0, getActiveCount());
+ timerFactory_->setTime(2);
+ //The idleTimer period has expired, the channel should have 1 composing and 1 active status stanza
+ ASSERT_EQ(1, getComposingCount());
+ ASSERT_EQ(1, getActiveCount());
+ ASSERT_EQ(ChatState::Active, stanzaChannel->sentStanzas[stanzaChannel->sentStanzas.size() - 1]->getPayload<ChatState>()->getChatState());
+ timerFactory_->setTime(4);
+ //At the second tick no further state stanzas should be sent.
+ ASSERT_EQ(1, getComposingCount());
+ ASSERT_EQ(1, getActiveCount());
+ ASSERT_EQ(ChatState::Active, stanzaChannel->sentStanzas[stanzaChannel->sentStanzas.size() - 1]->getPayload<ChatState>()->getChatState());
+}
+
+TEST_F(ChatStateNotifierTest, testIdleWhileTypingNewMessageNoCaps) {
+ notifier_->setUserIsTyping();
+ timerFactory_->setTime(3);
+ ASSERT_EQ(0, getComposingCount());
+ ASSERT_EQ(0, getActiveCount());
+}
+TEST_F(ChatStateNotifierTest, testContactShouldReceiveStates_CapsOnly) {
+ setContactHas85Caps();
+ std::shared_ptr<Message> message(new Message());
+ notifier_->addChatStateRequest(message);
+ EXPECT_TRUE(message->getPayload<ChatState>());
+ ASSERT_EQ(ChatState::Active, message->getPayload<ChatState>()->getChatState());
+}
+
+TEST_F(ChatStateNotifierTest, testContactShouldReceiveStates_CapsNorActive) {
+ std::shared_ptr<Message> message(new Message());
+ notifier_->addChatStateRequest(message);
+ EXPECT_TRUE(!message->getPayload<ChatState>());
+}
+
+TEST_F(ChatStateNotifierTest, testContactShouldReceiveStates_ActiveOverrideOn) {
+ notifier_->receivedMessageFromContact(true);
+ std::shared_ptr<Message> message(new Message());
+ notifier_->addChatStateRequest(message);
+ EXPECT_TRUE(message->getPayload<ChatState>());
+ ASSERT_EQ(ChatState::Active, message->getPayload<ChatState>()->getChatState());
+}
+
+TEST_F(ChatStateNotifierTest, testContactShouldReceiveStates_ActiveOverrideOff) {
+ setContactHas85Caps();
+ notifier_->receivedMessageFromContact(false);
+ /* I originally read the MUST NOT send after receiving without Active and
+ * thought this should check for false, but I later found it was OPTIONAL
+ * (MAY) behaviour only for if you didn't receive caps.
+ */
+ std::shared_ptr<Message> message(new Message());
+ notifier_->addChatStateRequest(message);
+ EXPECT_TRUE(message->getPayload<ChatState>());
+ ASSERT_EQ(ChatState::Active, message->getPayload<ChatState>()->getChatState());
+}
+
+
+TEST_F(ChatStateNotifierTest, testStartTypingReply_CapsIncluded) {
+ setContactHas85Caps();
+ notifier_->setUserIsTyping();
+ ASSERT_EQ(1, getComposingCount());
+}
+
+TEST_F(ChatStateNotifierTest, testContinueTypingReply_CapsIncluded) {
+ setContactHas85Caps();
+ notifier_->setUserIsTyping();
+ notifier_->setUserIsTyping();
+ notifier_->setUserIsTyping();
+ ASSERT_EQ(1, getComposingCount());
+ notifier_->userSentMessage();
+ notifier_->setUserIsTyping();
+ ASSERT_EQ(2, getComposingCount());
+
+}
+
+TEST_F(ChatStateNotifierTest, testTypeReplies_WentOffline) {
+ setContactHas85Caps();
+ notifier_->setUserIsTyping();
+ ASSERT_EQ(1, getComposingCount());
+ notifier_->setContactIsOnline(false);
+ notifier_->userSentMessage();
+ notifier_->setUserIsTyping();
+ ASSERT_EQ(1, getComposingCount());
+}