diff --git a/AUTHORS.rst b/AUTHORS.rst index bd53c1823..7c9fa21c0 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -69,6 +69,7 @@ The following wonderful people contributed directly or indirectly to this projec - `Trainer Jono `_ - `Valentijn `_ - `voider1 `_ +- `Vorobjev Simon `_ - `Wagner Macedo `_ - `wjt `_ diff --git a/requirements-dev.txt b/requirements-dev.txt index 0d80e1f4b..81fba33f4 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -5,6 +5,6 @@ flaky yapf pre-commit beautifulsoup4 -pytest +pytest>=3.8.1 pytest-timeout wheel diff --git a/telegram/ext/conversationhandler.py b/telegram/ext/conversationhandler.py index 4f7425e2d..f19b75d2d 100644 --- a/telegram/ext/conversationhandler.py +++ b/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) diff --git a/telegram/ext/dispatcher.py b/telegram/ext/dispatcher.py index 234183d68..69a259f79 100644 --- a/telegram/ext/dispatcher.py +++ b/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 diff --git a/tests/conftest.py b/tests/conftest.py index a25b8b9b3..e38e8f570 100644 --- a/tests/conftest.py +++ b/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 diff --git a/tests/test_conversationhandler.py b/tests/test_conversationhandler.py index c8c739ded..7964d1d4f 100644 --- a/tests/test_conversationhandler.py +++ b/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