From fb31718360057be5a9f344c893bda3b11a1ca9d1 Mon Sep 17 00:00:00 2001 From: Hinrich Mahler Date: Fri, 1 Jan 2021 21:40:42 +0100 Subject: [PATCH 1/2] Adjust calling of update_persistence (cherry picked from commit 89c522d88395141f7a1d70a10868bb58881b4a90) --- telegram/ext/dispatcher.py | 30 ++++++++++++---- tests/test_dispatcher.py | 72 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 7 deletions(-) diff --git a/telegram/ext/dispatcher.py b/telegram/ext/dispatcher.py index 51597f06309..cfcd34f557a 100644 --- a/telegram/ext/dispatcher.py +++ b/telegram/ext/dispatcher.py @@ -402,11 +402,17 @@ def stop(self) -> None: def has_running_threads(self) -> bool: return self.running or bool(self.__async_threads) - def process_update(self, update: Union[str, Update, TelegramError]) -> None: - """Processes a single update. + def process_update(self, update: Any) -> None: + """Processes a single update and updates the persistence. + + Note: + If the update is handled by least one synchronously running handlers (i.e. + ``run_async=False`), :meth:`update_persistence` is called *once* after all handlers + synchronous handlers are done. Each asynchronously running handler will trigger + :meth:`update_persistence` on its own. Args: - update (:obj:`str` | :class:`telegram.Update` | :class:`telegram.TelegramError`): + update (:class:`telegram.Update` | :obj:`object` | :class:`telegram.TelegramError`): The update to process. """ @@ -420,6 +426,8 @@ def process_update(self, update: Union[str, Update, TelegramError]) -> None: return context = None + handled = False + sync_modes = [] for group in self.groups: try: @@ -428,11 +436,9 @@ def process_update(self, update: Union[str, Update, TelegramError]) -> None: if check is not None and check is not False: if not context and self.use_context: context = CallbackContext.from_update(update, self) + handled = True + sync_modes.append(handler.run_async) handler.handle_update(update, self, check, context) - - # If handler runs async updating immediately doesn't make sense - if not handler.run_async: - self.update_persistence(update=update) break # Stop processing with any other handler. @@ -452,6 +458,16 @@ def process_update(self, update: Union[str, Update, TelegramError]) -> None: except Exception: self.logger.exception('An uncaught error was raised while handling the error.') + # Update persistence, if handled + handled_only_async = all(sync_modes) + if handled: + # Respect default settings + if all(mode is DEFAULT_FALSE for mode in sync_modes) and self.bot.defaults: + handled_only_async = not self.bot.defaults.run_async + # If update was only handled by async handlers, we don't need to update here + if not handled_only_async: + self.update_persistence(update=update) + def add_handler(self, handler: Handler, group: int = DEFAULT_GROUP) -> None: """Register a handler. diff --git a/tests/test_dispatcher.py b/tests/test_dispatcher.py index 3ce3f34c605..e8fbfbfe14a 100644 --- a/tests/test_dispatcher.py +++ b/tests/test_dispatcher.py @@ -35,6 +35,7 @@ ) from telegram.ext.dispatcher import run_async, Dispatcher, DispatcherHandlerStop from telegram.utils.deprecate import TelegramDeprecationWarning +from telegram.utils.helpers import DEFAULT_FALSE from tests.conftest import create_dp from collections import defaultdict @@ -804,3 +805,74 @@ def callback(update, context): assert cdp.persistence.test_flag_bot_data assert not cdp.persistence.test_flag_user_data assert cdp.persistence.test_flag_chat_data + + def test_update_persistence_once_per_update(self, monkeypatch, dp): + def update_persistence(*args, **kwargs): + self.count += 1 + + def dummy_callback(*args): + pass + + monkeypatch.setattr(dp, 'update_persistence', update_persistence) + + for group in range(5): + dp.add_handler(MessageHandler(Filters.text, dummy_callback), group=group) + + update = Update(1, message=Message(1, None, Chat(1, ''), from_user=None, text=None)) + dp.process_update(update) + assert self.count == 0 + + update = Update(1, message=Message(1, None, Chat(1, ''), from_user=None, text='text')) + dp.process_update(update) + assert self.count == 1 + + def test_update_persistence_all_async(self, monkeypatch, dp): + def update_persistence(*args, **kwargs): + self.count += 1 + + def dummy_callback(*args, **kwargs): + pass + + monkeypatch.setattr(dp, 'update_persistence', update_persistence) + monkeypatch.setattr(dp, 'run_async', dummy_callback) + + for group in range(5): + dp.add_handler( + MessageHandler(Filters.text, dummy_callback, run_async=True), group=group + ) + + update = Update(1, message=Message(1, None, Chat(1, ''), from_user=None, text='Text')) + dp.process_update(update) + assert self.count == 0 + + dp.bot.defaults = Defaults(run_async=True) + try: + for group in range(5): + dp.add_handler(MessageHandler(Filters.text, dummy_callback), group=group) + + update = Update(1, message=Message(1, None, Chat(1, ''), from_user=None, text='Text')) + dp.process_update(update) + assert self.count == 0 + finally: + dp.bot.defaults = None + + @pytest.mark.parametrize('run_async', [DEFAULT_FALSE, False]) + def test_update_persistence_one_sync(self, monkeypatch, dp, run_async): + def update_persistence(*args, **kwargs): + self.count += 1 + + def dummy_callback(*args, **kwargs): + pass + + monkeypatch.setattr(dp, 'update_persistence', update_persistence) + monkeypatch.setattr(dp, 'run_async', dummy_callback) + + for group in range(5): + dp.add_handler( + MessageHandler(Filters.text, dummy_callback, run_async=True), group=group + ) + dp.add_handler(MessageHandler(Filters.text, dummy_callback, run_async=run_async), group=5) + + update = Update(1, message=Message(1, None, Chat(1, ''), from_user=None, text='Text')) + dp.process_update(update) + assert self.count == 1 From c44a3920c256a37887b54cdc67af408660455580 Mon Sep 17 00:00:00 2001 From: Hinrich Mahler Date: Sat, 2 Jan 2021 10:54:34 +0100 Subject: [PATCH 2/2] Fix tests and stuff --- telegram/ext/defaults.py | 2 +- telegram/ext/dispatcher.py | 2 +- tests/test_dispatcher.py | 22 ++++++++++++++++++++++ tests/test_jobqueue.py | 2 +- tests/test_message.py | 21 ++++++++++++--------- 5 files changed, 37 insertions(+), 12 deletions(-) diff --git a/telegram/ext/defaults.py b/telegram/ext/defaults.py index a4ab04a55fb..a907fcc3653 100644 --- a/telegram/ext/defaults.py +++ b/telegram/ext/defaults.py @@ -173,7 +173,7 @@ def tzinfo(self, value: Any) -> NoReturn: ) @property - def run_async(self) -> Optional[bool]: + def run_async(self) -> bool: return self._run_async @run_async.setter diff --git a/telegram/ext/dispatcher.py b/telegram/ext/dispatcher.py index cfcd34f557a..bfdce78dc88 100644 --- a/telegram/ext/dispatcher.py +++ b/telegram/ext/dispatcher.py @@ -463,7 +463,7 @@ def process_update(self, update: Any) -> None: if handled: # Respect default settings if all(mode is DEFAULT_FALSE for mode in sync_modes) and self.bot.defaults: - handled_only_async = not self.bot.defaults.run_async + handled_only_async = self.bot.defaults.run_async # If update was only handled by async handlers, we don't need to update here if not handled_only_async: self.update_persistence(update=update) diff --git a/tests/test_dispatcher.py b/tests/test_dispatcher.py index e8fbfbfe14a..7695c6c4d2c 100644 --- a/tests/test_dispatcher.py +++ b/tests/test_dispatcher.py @@ -876,3 +876,25 @@ def dummy_callback(*args, **kwargs): update = Update(1, message=Message(1, None, Chat(1, ''), from_user=None, text='Text')) dp.process_update(update) assert self.count == 1 + + @pytest.mark.parametrize('run_async,expected', [(DEFAULT_FALSE, 1), (False, 1), (True, 0)]) + def test_update_persistence_defaults_async(self, monkeypatch, dp, run_async, expected): + def update_persistence(*args, **kwargs): + self.count += 1 + + def dummy_callback(*args, **kwargs): + pass + + monkeypatch.setattr(dp, 'update_persistence', update_persistence) + monkeypatch.setattr(dp, 'run_async', dummy_callback) + dp.bot.defaults = Defaults(run_async=run_async) + + try: + for group in range(5): + dp.add_handler(MessageHandler(Filters.text, dummy_callback), group=group) + + update = Update(1, message=Message(1, None, Chat(1, ''), from_user=None, text='Text')) + dp.process_update(update) + assert self.count == expected + finally: + dp.bot.defaults = None diff --git a/tests/test_jobqueue.py b/tests/test_jobqueue.py index 69b981f5699..a371b45746e 100644 --- a/tests/test_jobqueue.py +++ b/tests/test_jobqueue.py @@ -314,7 +314,7 @@ def test_run_monthly(self, job_queue, timezone): next_months_days = calendar.monthrange(now.year, now.month + 1)[1] expected_reschedule_time += dtm.timedelta(this_months_days) - if next_months_days < this_months_days: + if day > next_months_days: expected_reschedule_time += dtm.timedelta(next_months_days) expected_reschedule_time = timezone.normalize(expected_reschedule_time) diff --git a/tests/test_message.py b/tests/test_message.py index a4349d63bdd..95c4f6724fa 100644 --- a/tests/test_message.py +++ b/tests/test_message.py @@ -1406,18 +1406,21 @@ def make_assertion(*args, **kwargs): def test_default_quote(self, message): message.bot.defaults = Defaults() - message.bot.defaults._quote = False - assert message._quote(None, None) is None + try: + message.bot.defaults._quote = False + assert message._quote(None, None) is None - message.bot.defaults._quote = True - assert message._quote(None, None) == message.message_id + message.bot.defaults._quote = True + assert message._quote(None, None) == message.message_id - message.bot.defaults._quote = None - message.chat.type = Chat.PRIVATE - assert message._quote(None, None) is None + message.bot.defaults._quote = None + message.chat.type = Chat.PRIVATE + assert message._quote(None, None) is None - message.chat.type = Chat.GROUP - assert message._quote(None, None) + message.chat.type = Chat.GROUP + assert message._quote(None, None) + finally: + message.bot.defaults = None def test_equality(self): id_ = 1