Allowed to use Handlers on conversation timeout (#1217)

* handler for ConversationHandler.END (timeout one) #1136

* review fixes

* review fixes

* review fixes

* review fixes

* docs and tests

* fixing stuff

* Fix problem

* fix conftest

* now it should work

* Add ConversationTimeoutContext

As discussed in the developers group. Use a class as the jobs context over using a dict.

* less verbosity

AUTHORS.rst

@@ -69,6 +69,7 @@ The following wonderful people contributed directly or indirectly to this projec
 - `Trainer Jono <https://github.com/Tr-Jono>`_
 - `Valentijn <https://github.com/Faalentijn>`_
 - `voider1 <https://github.com/voider1>`_
+- `Vorobjev Simon <https://github.com/simonvorobjev>`_
 - `Wagner Macedo <https://github.com/wagnerluis1982>`_
 - `wjt <https://github.com/wjt>`_
 

requirements-dev.txt

@@ -5,6 +5,6 @@ flaky
 yapf
 pre-commit
 beautifulsoup4
-pytest
+pytest>=3.8.1
 pytest-timeout
 wheel

telegram/ext/conversationhandler.py

@@ -26,6 +26,13 @@ from telegram.ext import (Handler, CallbackQueryHandler, InlineQueryHandler,
 from telegram.utils.promise import Promise
 
 
+class _ConversationTimeoutContext(object):
+    def __init__(self, conversation_key, update, dispatcher):
+        self.conversation_key = conversation_key
+        self.update = update
+        self.dispatcher = dispatcher
+
+
 class ConversationHandler(Handler):
     """
     A handler to hold a conversation with a single user by managing four collections of other
@@ -38,8 +45,8 @@ class ConversationHandler(Handler):
 
     The second collection, a ``dict`` named :attr:`states`, contains the different conversation
     steps and one or more associated handlers that should be used if the user sends a message when
-    the conversation with them is currently in that state. You will probably use mostly
-    :class:`telegram.ext.MessageHandler` and :class:`telegram.ext.RegexHandler` here.
+    the conversation with them is currently in that state. Here you can also define a state for
+    :attr:`TIMEOUT` to define the behavior when :attr:`conversation_timeout` is exceeded.
 
     The third collection, a ``list`` named :attr:`fallbacks`, is used if the user is currently in a
     conversation but the state has either no associated handler or the handler that is associated
@@ -55,7 +62,8 @@ class ConversationHandler(Handler):
     To change the state of conversation, the callback function of a handler must return the new
     state after responding to the user. If it does not return anything (returning ``None`` by
     default), the state will not change. To end the conversation, the callback function must
-    return :attr:`END` or ``-1``.
+    return :attr:`END` or ``-1``. To handle the conversation timeout, use handler
+    :attr:`TIMEOUT` or ``-2``.
 
     Attributes:
         entry_points (List[:class:`telegram.ext.Handler`]): A list of ``Handler`` objects that can
@@ -78,7 +86,9 @@ class ConversationHandler(Handler):
             ID.
         conversation_timeout (:obj:`float`|:obj:`datetime.timedelta`): Optional. When this handler
             is inactive more than this timeout (in seconds), it will be automatically ended. If
-            this value is 0 (default), there will be no timeout.
+            this value is 0 (default), there will be no timeout. when it's triggered. The last
+            received update will be handled by ALL the handler's who's `check_update` method
+            returns True that are in the state :attr:`ConversationHandler.TIMEOUT`.
         name (:obj:`str`): Optional. The name for this conversationhandler. Required for
             persistence
         persistent (:obj:`bool`): Optional. If the conversations dict for this handler should be
@@ -114,9 +124,11 @@ class ConversationHandler(Handler):
             Default is ``True``.
         per_message (:obj:`bool`, optional): If the conversationkey should contain the Message's
             ID. Default is ``False``.
-        conversation_timeout (:obj:`float`|:obj:`datetime.timedelta`, optional): When this handler
-            is inactive more than this timeout (in seconds), it will be automatically ended. If
-            this value is 0 or None (default), there will be no timeout.
+        conversation_timeout (:obj:`float` | :obj:`datetime.timedelta`, optional): When this
+            handler is inactive more than this timeout (in seconds), it will be automatically
+            ended. If this value is 0 or None (default), there will be no timeout. The last
+            received update will be handled by ALL the handler's who's `check_update` method
+            returns True that are in the state :attr:`ConversationHandler.TIMEOUT`.
         name (:obj:`str`, optional): The name for this conversationhandler. Required for
             persistence
         persistent (:obj:`bool`, optional): If the conversations dict for this handler should be
@@ -128,6 +140,8 @@ class ConversationHandler(Handler):
     """
     END = -1
     """:obj:`int`: Used as a constant to return when a conversation is ended."""
+    TIMEOUT = -2
+    """:obj:`int`: Used as a constant to handle state when a conversation is timed out."""
 
     def __init__(self,
                  entry_points,
@@ -324,8 +338,7 @@ class ConversationHandler(Handler):
         if self.conversation_timeout and new_state != self.END:
             self.timeout_jobs[conversation_key] = dispatcher.job_queue.run_once(
                 self._trigger_timeout, self.conversation_timeout,
-                context=conversation_key
-            )
+                context=_ConversationTimeoutContext(conversation_key, update, dispatcher))
 
         self.update_state(new_state, conversation_key)
 
@@ -350,5 +363,11 @@ class ConversationHandler(Handler):
                 self.persistence.update_conversation(self.name, key, new_state)
 
     def _trigger_timeout(self, bot, job):
-        del self.timeout_jobs[job.context]
-        self.update_state(self.END, job.context)
+        self.logger.debug('conversation timeout was triggered!')
+        del self.timeout_jobs[job.context.conversation_key]
+        handlers = self.states.get(self.TIMEOUT, [])
+        for handler in handlers:
+            check = handler.check_update(job.context.update)
+            if check is not None and check is not False:
+                handler.handle_update(job.context.update, job.context.dispatcher, check)
+        self.update_state(self.END, job.context.conversation_key)

telegram/ext/dispatcher.py

@@ -322,16 +322,18 @@ class Dispatcher(object):
                     if check is not None and check is not False:
                         handler.handle_update(update, self, check)
                         if self.persistence:
-                            if self.persistence.store_chat_data and update.effective_chat.id:
+                            if self.persistence.store_chat_data and update.effective_chat:
                                 chat_id = update.effective_chat.id
                                 try:
-                                    self.persistence.update_chat_data(chat_id, self.chat_data[chat_id])
+                                    self.persistence.update_chat_data(chat_id,
+                                                                      self.chat_data[chat_id])
                                 except Exception:
                                     self.logger.exception('Saving chat data raised an error')
-                            if self.persistence.store_user_data and update.effective_user.id:
+                            if self.persistence.store_user_data and update.effective_user:
                                 user_id = update.effective_user.id
                                 try:
-                                    self.persistence.update_user_data(user_id, self.user_data[user_id])
+                                    self.persistence.update_user_data(user_id,
+                                                                      self.user_data[user_id])
                                 except Exception:
                                     self.logger.exception('Saving user data raised an error')
                         break

tests/conftest.py

@@ -104,6 +104,8 @@ def dp(_dp):
     _dp.__exception_event = Event()
     _dp.__async_queue = Queue()
     _dp.__async_threads = set()
+    _dp.persistence = None
+    _dp.use_context = False
     if _dp._Dispatcher__singleton_semaphore.acquire(blocking=0):
         Dispatcher._set_singleton(_dp)
     yield _dp

tests/test_conversationhandler.py

@@ -23,7 +23,8 @@ import pytest
 
 from telegram import (CallbackQuery, Chat, ChosenInlineResult, InlineQuery, Message,
                       PreCheckoutQuery, ShippingQuery, Update, User, MessageEntity)
-from telegram.ext import (ConversationHandler, CommandHandler, CallbackQueryHandler)
+from telegram.ext import (ConversationHandler, CommandHandler, CallbackQueryHandler,
+                          MessageHandler, Filters)
 
 
 @pytest.fixture(scope='class')
@@ -63,9 +64,10 @@ class TestConversationHandler(object):
                 CommandHandler('keepCoding', self.code),
                 CommandHandler('gettingThirsty', self.start),
                 CommandHandler('drinkMore', self.drink)
-            ],
+            ]
         }
         self.fallbacks = [CommandHandler('eat', self.start)]
+        self.is_timeout = False
 
     # State handlers
     def _set_state(self, update, state):
@@ -91,6 +93,13 @@ class TestConversationHandler(object):
     def code(self, bot, update):
         return self._set_state(update, self.CODING)
 
+    def passout(self, bot, update):
+        assert update.message.text == '/brew'
+        self.is_timeout = True
+
+    def passout2(self, bot, update):
+        self.is_timeout = True
+
     # Tests
     def test_per_all_false(self):
         with pytest.raises(ValueError, match="can't all be 'False'"):
@@ -159,6 +168,7 @@ class TestConversationHandler(object):
         dp.process_update(Update(update_id=0, message=message))
         message.text = '/end'
         message.entities[0].length = len('/end')
+        caplog.clear()
         with caplog.at_level(logging.ERROR):
             dp.process_update(Update(update_id=0, message=message))
         assert len(caplog.records) == 0
@@ -459,3 +469,50 @@ class TestConversationHandler(object):
         dp.job_queue.tick()
         assert handler.conversations.get((self.group.id, user1.id)) is None
         assert handler.conversations.get((self.group.id, user2.id)) is None
+
+    def test_conversation_handler_timeout_state(self, dp, bot, user1):
+        states = self.states
+        states.update({ConversationHandler.TIMEOUT: [
+            CommandHandler('brew', self.passout),
+            MessageHandler(~Filters.regex('oding'), self.passout2)
+        ]})
+        handler = ConversationHandler(entry_points=self.entry_points, states=states,
+                                      fallbacks=self.fallbacks, conversation_timeout=0.5)
+        dp.add_handler(handler)
+        # CommandHandler timeout
+        message = Message(0, user1, None, self.group, text='/start',
+                          entities=[MessageEntity(type=MessageEntity.BOT_COMMAND, offset=0,
+                                                  length=len('/start'))],
+                          bot=bot)
+        dp.process_update(Update(update_id=0, message=message))
+        message.text = '/brew'
+        message.entities[0].length = len('/brew')
+        dp.process_update(Update(update_id=0, message=message))
+        sleep(0.5)
+        dp.job_queue.tick()
+        assert handler.conversations.get((self.group.id, user1.id)) is None
+        assert self.is_timeout
+
+        # MessageHandler timeout
+        self.is_timeout = False
+        message.text = '/start'
+        message.entities[0].length = len('/start')
+        dp.process_update(Update(update_id=1, message=message))
+        sleep(0.5)
+        dp.job_queue.tick()
+        assert handler.conversations.get((self.group.id, user1.id)) is None
+        assert self.is_timeout
+
+        # Timeout but no valid handler
+        self.is_timeout = False
+        dp.process_update(Update(update_id=0, message=message))
+        message.text = '/brew'
+        message.entities[0].length = len('/brew')
+        dp.process_update(Update(update_id=0, message=message))
+        message.text = '/startCoding'
+        message.entities[0].length = len('/startCoding')
+        dp.process_update(Update(update_id=0, message=message))
+        sleep(0.5)
+        dp.job_queue.tick()
+        assert handler.conversations.get((self.group.id, user1.id)) is None
+        assert not self.is_timeout