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
author: Thanos Doukoudakis <thanos.doukoudakis@isode.com> 2017-03-24 14:15:52 (GMT)
committer: Thanos Doukoudakis <thanos.doukoudakis@isode.com> 2017-04-06 14:27:45 (GMT)
commit: 7d2e5e200d8449a4492c6fafa4811197e6fbe40b (patch)
tree: a4cc9d8d6eb6d98d60dee9e2b928a9e96392ebe9 /Swiften
parent: 20c50e3960ee68d26fc93738f03d27a6833b28bb (diff)
download: swift-7d2e5e200d8449a4492c6fafa4811197e6fbe40b.zip
swift-7d2e5e200d8449a4492c6fafa4811197e6fbe40b.tar.bz2
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());
+}
author	Thanos Doukoudakis <thanos.doukoudakis@isode.com>	2017-03-24 14:15:52 (GMT)
committer	Thanos Doukoudakis <thanos.doukoudakis@isode.com>	2017-04-06 14:27:45 (GMT)
commit	7d2e5e200d8449a4492c6fafa4811197e6fbe40b (patch)
tree	a4cc9d8d6eb6d98d60dee9e2b928a9e96392ebe9 /Swiften
parent	20c50e3960ee68d26fc93738f03d27a6833b28bb (diff)
download	swift-7d2e5e200d8449a4492c6fafa4811197e6fbe40b.zip swift-7d2e5e200d8449a4492c6fafa4811197e6fbe40b.tar.bz2