diff --git a/CHANGES/1641.bugfix.rst b/CHANGES/1641.bugfix.rst new file mode 100644 index 00000000..b4ebe2f4 --- /dev/null +++ b/CHANGES/1641.bugfix.rst @@ -0,0 +1,20 @@ +Resolved incorrect ordering of registered handlers in the :class:`aiogram.fsm.scene.Scene` +object caused by :code:`inspect.getmembers` returning sorted members. +Handlers are now registered in the order of their definition within the class, +ensuring proper execution sequence, especially when handling filters with different +levels of specificity. + +For backward compatibility, the old behavior can be restored by setting the +:code:`attrs_resolver=inspect_members_resolver` parameter in the :class:`aiogram.fsm.scene.Scene`: + +.. code-block:: python + + from aiogram.utils.class_attrs_resolver import inspect_members_resolver + + + class MyScene(Scene, attrs_resolver=inspect_members_resolver): + +In this case, the handlers will be registered in the order returned by :code:`inspect.getmembers`. + +By default, the :code:`attrs_resolver` parameter is set to :code:`get_sorted_mro_attrs_resolver` now, +so you **don't need** to specify it explicitly. diff --git a/aiogram/client/default.py b/aiogram/client/default.py index 6c857be0..78dd0aa3 100644 --- a/aiogram/client/default.py +++ b/aiogram/client/default.py @@ -1,8 +1,9 @@ from __future__ import annotations -import sys from dataclasses import dataclass -from typing import TYPE_CHECKING, Any, Dict, Optional +from typing import TYPE_CHECKING, Any, Optional + +from aiogram.utils.dataclass import dataclass_kwargs if TYPE_CHECKING: from aiogram.types import LinkPreviewOptions @@ -28,13 +29,7 @@ class Default: return f"<{self}>" -_dataclass_properties: Dict[str, Any] = {} -if sys.version_info >= (3, 10): - # Speedup attribute access for dataclasses in Python 3.10+ - _dataclass_properties.update({"slots": True, "kw_only": True}) - - -@dataclass(**_dataclass_properties) +@dataclass(**dataclass_kwargs(slots=True, kw_only=True)) class DefaultBotProperties: """ Default bot properties. diff --git a/aiogram/fsm/scene.py b/aiogram/fsm/scene.py index 0cbd0e8b..1344b4d0 100644 --- a/aiogram/fsm/scene.py +++ b/aiogram/fsm/scene.py @@ -20,6 +20,10 @@ from aiogram.fsm.context import FSMContext from aiogram.fsm.state import State from aiogram.fsm.storage.memory import MemoryStorageRecord from aiogram.types import TelegramObject, Update +from aiogram.utils.class_attrs_resolver import ( + ClassAttrsResolver, + get_sorted_mro_attrs_resolver, +) class HistoryManager: @@ -214,6 +218,15 @@ class SceneConfig: """Reset scene history on enter""" callback_query_without_state: Optional[bool] = None """Allow callback query without state""" + attrs_resolver: ClassAttrsResolver = get_sorted_mro_attrs_resolver + """ + Attributes resolver. + + .. danger:: + This attribute should only be changed when you know what you are doing. + + .. versionadded:: 3.19.0 + """ async def _empty_handler(*args: Any, **kwargs: Any) -> None: @@ -302,6 +315,7 @@ class Scene: reset_data_on_enter = kwargs.pop("reset_data_on_enter", None) reset_history_on_enter = kwargs.pop("reset_history_on_enter", None) callback_query_without_state = kwargs.pop("callback_query_without_state", None) + attrs_resolver = kwargs.pop("attrs_resolver", None) super().__init_subclass__(**kwargs) @@ -322,8 +336,13 @@ class Scene: reset_history_on_enter = parent_scene_config.reset_history_on_enter if callback_query_without_state is None: callback_query_without_state = parent_scene_config.callback_query_without_state + if attrs_resolver is None: + attrs_resolver = parent_scene_config.attrs_resolver - for name, value in inspect.getmembers(cls): + if attrs_resolver is None: + attrs_resolver = get_sorted_mro_attrs_resolver + + for name, value in attrs_resolver(cls): if scene_handlers := getattr(value, "__aiogram_handler__", None): handlers.extend(scene_handlers) if isinstance(value, ObserverDecorator): @@ -346,6 +365,7 @@ class Scene: reset_data_on_enter=reset_data_on_enter, reset_history_on_enter=reset_history_on_enter, callback_query_without_state=callback_query_without_state, + attrs_resolver=attrs_resolver, ) @classmethod diff --git a/aiogram/utils/class_attrs_resolver.py b/aiogram/utils/class_attrs_resolver.py new file mode 100644 index 00000000..83d0da45 --- /dev/null +++ b/aiogram/utils/class_attrs_resolver.py @@ -0,0 +1,86 @@ +import inspect +from dataclasses import dataclass +from operator import itemgetter +from typing import Any, Generator, NamedTuple, Protocol + +from aiogram.utils.dataclass import dataclass_kwargs + + +class ClassAttrsResolver(Protocol): + def __call__(self, cls: type) -> Generator[tuple[str, Any], None, None]: ... + + +def inspect_members_resolver(cls: type) -> Generator[tuple[str, Any], None, None]: + """ + Inspects and resolves attributes of a given class. + + This function uses the `inspect.getmembers` utility to yield all attributes of + a provided class. The output is a generator that produces tuples containing + attribute names and their corresponding values. This function is suitable for + analyzing class attributes dynamically. However, it guarantees alphabetical + order of attributes. + + :param cls: The class for which the attributes will be resolved. + :return: A generator yielding tuples containing attribute names and their values. + """ + yield from inspect.getmembers(cls) + + +def get_reversed_mro_unique_attrs_resolver(cls: type) -> Generator[tuple[str, Any], None, None]: + """ + Resolve and yield attributes from the reversed method resolution order (MRO) of a given class. + + This function iterates through the reversed MRO of a class and yields attributes + that have not yet been encountered. It avoids duplicates by keeping track of + attribute names that have already been processed. + + :param cls: The class for which the attributes will be resolved. + :return: A generator yielding tuples containing attribute names and their values. + """ + known_attrs = set() + for base in reversed(inspect.getmro(cls)): + for name, value in base.__dict__.items(): + if name in known_attrs: + continue + + yield name, value + known_attrs.add(name) + + +class _Position(NamedTuple): + in_mro: int + in_class: int + + +@dataclass(**dataclass_kwargs(slots=True)) +class _AttributeContainer: + position: _Position + value: Any + + def __lt__(self, other: "_AttributeContainer") -> bool: + return self.position < other.position + + +def get_sorted_mro_attrs_resolver(cls: type) -> Generator[tuple[str, Any], None, None]: + """ + Resolve and yield attributes from the method resolution order (MRO) of a given class. + + Iterates through a class's method resolution order (MRO) and collects its attributes + along with their respective positions in the MRO and the class hierarchy. This generator + yields a tuple containing the name of each attribute and its associated value. + + :param cls: The class for which the attributes will be resolved. + :return: A generator yielding tuples containing attribute names and their values. + """ + attributes: dict[str, _AttributeContainer] = {} + for position_in_mro, base in enumerate(inspect.getmro(cls)): + for position_in_class, (name, value) in enumerate(vars(base).items()): + position = _Position(position_in_mro, position_in_class) + if attribute := attributes.get(name): + attribute.position = position + continue + + attributes[name] = _AttributeContainer(value=value, position=position) + + for name, attribute in sorted(attributes.items(), key=itemgetter(1)): + yield name, attribute.value diff --git a/aiogram/utils/dataclass.py b/aiogram/utils/dataclass.py new file mode 100644 index 00000000..96b9a202 --- /dev/null +++ b/aiogram/utils/dataclass.py @@ -0,0 +1,64 @@ +""" +This module contains utility functions for working with dataclasses in Python. + +DO NOT USE THIS MODULE DIRECTLY. IT IS INTENDED FOR INTERNAL USE ONLY. +""" + +import sys +from typing import Any, Union + + +def dataclass_kwargs( + init: Union[bool, None] = None, + repr: Union[bool, None] = None, + eq: Union[bool, None] = None, + order: Union[bool, None] = None, + unsafe_hash: Union[bool, None] = None, + frozen: Union[bool, None] = None, + match_args: Union[bool, None] = None, + kw_only: Union[bool, None] = None, + slots: Union[bool, None] = None, + weakref_slot: Union[bool, None] = None, +) -> dict[str, Any]: + """ + Generates a dictionary of keyword arguments that can be passed to a Python + dataclass. This function allows specifying attributes related to the behavior + and configuration of dataclasses, including attributes added in specific + Python versions. This abstraction improves compatibility across different + Python versions by ensuring only the parameters supported by the current + version are included. + + :return: A dictionary containing the specified dataclass configuration that + dynamically adapts to the current Python version. + """ + params = {} + + # All versions + if init is not None: + params["init"] = init + if repr is not None: + params["repr"] = repr + if eq is not None: + params["eq"] = eq + if order is not None: + params["order"] = order + if unsafe_hash is not None: + params["unsafe_hash"] = unsafe_hash + if frozen is not None: + params["frozen"] = frozen + + # Added in 3.10 + if sys.version_info >= (3, 10): + if match_args is not None: + params["match_args"] = match_args + if kw_only is not None: + params["kw_only"] = kw_only + if slots is not None: + params["slots"] = slots + + # Added in 3.11 + if sys.version_info >= (3, 11): + if weakref_slot is not None: + params["weakref_slot"] = weakref_slot + + return params diff --git a/tests/test_fsm/test_scene.py b/tests/test_fsm/test_scene.py index b22ef975..ad86a727 100644 --- a/tests/test_fsm/test_scene.py +++ b/tests/test_fsm/test_scene.py @@ -1680,3 +1680,73 @@ class TestSceneInheritance: assert child_2_handler.handler is _empty_handler assert child_3_handler.handler is not _empty_handler + + +def collect_handler_names(scene): + return [handler.handler.__name__ for handler in scene.__scene_config__.handlers] + + +class TestSceneHandlersOrdering: + def test_correct_ordering(self): + class Scene1(Scene): + @on.message() + async def handler1(self, message: Message) -> None: + pass + + @on.message() + async def handler2(self, message: Message) -> None: + pass + + class Scene2(Scene): + @on.message() + async def handler2(self, message: Message) -> None: + pass + + @on.message() + async def handler1(self, message: Message) -> None: + pass + + assert collect_handler_names(Scene1) == ["handler1", "handler2"] + assert collect_handler_names(Scene2) == ["handler2", "handler1"] + + def test_inherited_handlers_follow_mro_order(self): + class Scene1(Scene): + @on.message() + async def handler1(self, message: Message) -> None: + pass + + @on.message() + async def handler2(self, message: Message) -> None: + pass + + class Scene2(Scene1): + @on.message() + async def handler3(self, message: Message) -> None: + pass + + @on.message() + async def handler4(self, message: Message) -> None: + pass + + assert collect_handler_names(Scene2) == ["handler3", "handler4", "handler1", "handler2"] + + def test_inherited_overwrite_follow_mro_order(self): + class Scene1(Scene): + @on.message() + async def handler1(self, message: Message) -> None: + pass + + @on.message() + async def handler2(self, message: Message) -> None: + pass + + class Scene2(Scene1): + @on.message() + async def handler2(self, message: Message) -> None: + pass + + @on.message() + async def handler3(self, message: Message) -> None: + pass + + assert collect_handler_names(Scene2) == ["handler3", "handler1", "handler2"] diff --git a/tests/test_utils/test_class_attrs_resolver.py b/tests/test_utils/test_class_attrs_resolver.py new file mode 100644 index 00000000..aecf3257 --- /dev/null +++ b/tests/test_utils/test_class_attrs_resolver.py @@ -0,0 +1,81 @@ +import pytest + +from aiogram.utils.class_attrs_resolver import ( + get_reversed_mro_unique_attrs_resolver, + get_sorted_mro_attrs_resolver, + inspect_members_resolver, +) + + +class SimpleClass1: + def method1(self): + pass + + def method2(self): + pass + + +class SimpleClass2: + def method2(self): + pass + + def method1(self): + pass + + +class InheritedClass1(SimpleClass1): + def method3(self): + pass + + def method4(self): + pass + + +class InheritedClass2(SimpleClass1): + def method2(self): + pass + + def method3(self): + pass + + +class TestClassAttrsResolver: + @pytest.mark.parametrize( + "cls, resolver, expected", + [ + # inspect_members_resolver + (SimpleClass1, inspect_members_resolver, ["method1", "method2"]), + (SimpleClass2, inspect_members_resolver, ["method1", "method2"]), + ( + InheritedClass1, + inspect_members_resolver, + ["method1", "method2", "method3", "method4"], + ), + (InheritedClass2, inspect_members_resolver, ["method1", "method2", "method3"]), + # get_reversed_mro_unique_attrs_resolver + (SimpleClass1, get_reversed_mro_unique_attrs_resolver, ["method1", "method2"]), + (SimpleClass2, get_reversed_mro_unique_attrs_resolver, ["method2", "method1"]), + ( + InheritedClass1, + get_reversed_mro_unique_attrs_resolver, + ["method1", "method2", "method3", "method4"], + ), + ( + InheritedClass2, + get_reversed_mro_unique_attrs_resolver, + ["method1", "method2", "method3"], + ), + # get_sorted_mro_attrs_resolver + (SimpleClass1, get_sorted_mro_attrs_resolver, ["method1", "method2"]), + (SimpleClass2, get_sorted_mro_attrs_resolver, ["method2", "method1"]), + ( + InheritedClass1, + get_sorted_mro_attrs_resolver, + ["method3", "method4", "method1", "method2"], + ), + (InheritedClass2, get_sorted_mro_attrs_resolver, ["method3", "method1", "method2"]), + ], + ) + def test_resolve_class_attrs(self, cls, resolver, expected): + names = [name for name, _ in resolver(cls) if not name.startswith("__")] + assert names == expected diff --git a/tests/test_utils/test_dataclass.py b/tests/test_utils/test_dataclass.py new file mode 100644 index 00000000..27da088d --- /dev/null +++ b/tests/test_utils/test_dataclass.py @@ -0,0 +1,51 @@ +from unittest.mock import patch + +import pytest + +from aiogram.utils.dataclass import dataclass_kwargs + +ALL_VERSIONS = { + "init": True, + "repr": True, + "eq": True, + "order": True, + "unsafe_hash": True, + "frozen": True, +} +ADDED_IN_3_10 = {"match_args": True, "kw_only": True, "slots": True} +ADDED_IN_3_11 = {"weakref_slot": True} + +PY_310 = {**ALL_VERSIONS, **ADDED_IN_3_10} +PY_311 = {**PY_310, **ADDED_IN_3_11} +LATEST_PY = PY_311 + + +class TestDataclassKwargs: + @pytest.mark.parametrize( + "py_version,expected", + [ + ((3, 9, 0), ALL_VERSIONS), + ((3, 9, 2), ALL_VERSIONS), + ((3, 10, 2), PY_310), + ((3, 11, 0), PY_311), + ((4, 13, 0), LATEST_PY), + ], + ) + def test_dataclass_kwargs(self, py_version, expected): + with patch("sys.version_info", py_version): + + assert ( + dataclass_kwargs( + init=True, + repr=True, + eq=True, + order=True, + unsafe_hash=True, + frozen=True, + match_args=True, + kw_only=True, + slots=True, + weakref_slot=True, + ) + == expected + )