fix: Fix passing conditions callbacks as references. (#431)

Author: fgmacedo

statemachine/dispatcher.py

@@ -19,12 +19,7 @@ def from_obj(cls, obj, skip_attrs=None):
         if isinstance(obj, ObjectConfig):
             return obj
         else:
-            return cls(obj, set(skip_attrs) if skip_attrs else set(), str(id(obj)))
-
-    def getattr(self, attr):
-        if attr in self.skip_attrs:
-            return
-        return getattr(self.obj, attr, None)
+            return cls(obj, skip_attrs or set(), str(id(obj)))
 
 
 class WrapSearchResult:
@@ -88,29 +83,74 @@ def wrapper(*args, **kwargs):
         return wrapper
 
 
-def search_callable(attr, *configs) -> Generator[WrapSearchResult, None, None]:
-    if callable(attr) or isinstance(attr, property):
-        yield CallableSearchResult(attr, attr, None)
-    else:
-        for config in configs:
-            func = config.getattr(attr)
-            if func is None:
-                continue
-            if not callable(func):
-                yield AttributeCallableSearchResult(attr, config.obj, config.resolver_id)
+def _search_callable_attr_is_property(
+    attr, configs: tuple[ObjectConfig]
+) -> "WrapSearchResult | None":
+    # if the attr is a property, we'll try to find the object that has the
+    # property on the configs
+    attr_name = attr.fget.__name__
+    for obj, _skip_attrs, resolver_id in configs:
+        func = getattr(type(obj), attr_name, None)
+        if func is not None and func is attr:
+            return AttributeCallableSearchResult(attr_name, obj, resolver_id)
+    return None
 
-            if getattr(func, "_is_sm_event", False):
-                yield EventSearchResult(attr, func, config.resolver_id)
 
-            yield CallableSearchResult(attr, func, config.resolver_id)
+def _search_callable_attr_is_callable(attr, configs: tuple[ObjectConfig]) -> WrapSearchResult:
+    # if the attr is an unbounded method, we'll try to find the bounded method
+    # on the configs
+    if not hasattr(attr, "__self__"):
+        for obj, _skip_attrs, resolver_id in configs:
+            func = getattr(obj, attr.__name__, None)
+            if func is not None and func.__func__ is attr:
+                return CallableSearchResult(attr.__name__, func, resolver_id)
 
+    return CallableSearchResult(attr, attr, None)
+
+
+def _search_callable_in_configs(
+    attr, configs: tuple[ObjectConfig]
+) -> Generator[WrapSearchResult, None, None]:
+    for obj, skip_attrs, resolver_id in configs:
+        if attr in skip_attrs:
+            continue
+
+        if not hasattr(obj, attr):
+            continue
+
+        func = getattr(obj, attr)
+        if not callable(func):
+            yield AttributeCallableSearchResult(attr, obj, resolver_id)
+
+        if getattr(func, "_is_sm_event", False):
+            yield EventSearchResult(attr, func, resolver_id)
+
+        yield CallableSearchResult(attr, func, resolver_id)
 
-def resolver_factory(*objects):
-    """Factory that returns a configured resolver."""
 
-    objects = [ObjectConfig.from_obj(obj) for obj in objects]
+def search_callable(attr, configs: tuple) -> Generator[WrapSearchResult, None, None]:  # noqa: C901
+    if isinstance(attr, property):
+        result = _search_callable_attr_is_property(attr, configs)
+        if result is not None:
+            yield result
+        return
+
+    if callable(attr):
+        yield _search_callable_attr_is_callable(attr, configs)
+        return
+
+    yield from _search_callable_in_configs(attr, configs)
+
+
+def resolver_factory(objects: tuple[ObjectConfig]):
+    """Factory that returns a configured resolver."""
 
     def wrapper(attr) -> Generator[WrapSearchResult, None, None]:
-        yield from search_callable(attr, *objects)
+        yield from search_callable(attr, objects)
 
     return wrapper
+
+
+def resolver_factory_from_objects(*objects: tuple[Any]):
+    configs = tuple(ObjectConfig.from_obj(o) for o in objects)
+    return resolver_factory(configs)

statemachine/signature.py

@@ -9,6 +9,7 @@
 
 def _make_key(method):
     method = method.func if isinstance(method, partial) else method
+    method = method.fget if isinstance(method, property) else method
     if isinstance(method, MethodType):
         return hash(
             (

statemachine/state.py

@@ -139,6 +139,9 @@ def __repr__(self):
             f"initial={self.initial!r}, final={self.final!r})"
         )
 
+    def __str__(self):
+        return self.name
+
     def __get__(self, machine, owner):
         if machine is None:
             return self

statemachine/statemachine.py

@@ -81,7 +81,6 @@ def __init__(
         if self._abstract:
             raise InvalidDefinition(_("There are no states or transitions."))
 
-        self._initial_transition = Transition(None, self._get_initial_state(), event="__initial__")
         self._setup()
         self._activate_initial_state()
 
@@ -125,16 +124,17 @@ def _activate_initial_state(self):
         if self.current_state_value is None:
             # send an one-time event `__initial__` to enter the current state.
             # current_state = self.current_state
-            self._initial_transition.before.clear()
-            self._initial_transition.on.clear()
-            self._initial_transition.after.clear()
+            initial_transition = Transition(None, self._get_initial_state(), event="__initial__")
+            initial_transition.before.clear()
+            initial_transition.on.clear()
+            initial_transition.after.clear()
 
             event_data = EventData(
                 trigger_data=TriggerData(
                     machine=self,
-                    event=self._initial_transition.event,
+                    event=initial_transition.event,
                 ),
-                transition=self._initial_transition,
+                transition=initial_transition,
             )
             self._activate(event_data)
 
@@ -157,29 +157,31 @@ def _iterate_states_and_transitions(self):
             yield state
             yield from state.transitions
 
+    def _build_register(self, observers):
+        return partial(self._callbacks_registry.register, resolver=resolver_factory(observers))
+
     def _setup(self):
-        machine = ObjectConfig.from_obj(self, skip_attrs=self._get_protected_attrs())
-        model = ObjectConfig.from_obj(self.model, skip_attrs={self.state_field})
-        add_observer_visitor = self._build_observers_visitor(machine, model)
+        register = self._build_register(
+            (
+                ObjectConfig.from_obj(self, skip_attrs=self._get_protected_attrs()),
+                ObjectConfig.from_obj(self.model, skip_attrs={self.state_field}),
+            )
+        )
         check_callbacks = self._callbacks_registry.check
 
         for visited in self._iterate_states_and_transitions():
             visited._setup()
 
         for visited in self._iterate_states_and_transitions():
-            add_observer_visitor(visited)
+            visited._add_observer(register)
 
         for visited in self._iterate_states_and_transitions():
-            visited._check_callbacks(check_callbacks)
-
-    def _build_observers_visitor(self, *observers):
-        resolver = resolver_factory(*observers)
-        _register = partial(self._callbacks_registry.register, resolver=resolver)
-
-        def add_observer_visitor(visited):
-            visited._add_observer(_register)
-
-        return add_observer_visitor
+            try:
+                visited._check_callbacks(check_callbacks)
+            except Exception as err:
+                raise InvalidDefinition(
+                    f"Error on {visited!s} when resolving callbacks: {err}"
+                ) from err
 
     def add_observer(self, *observers):
         """Add an observer.
@@ -192,10 +194,12 @@ def add_observer(self, *observers):
             :ref:`observers`.
         """
         self._observers.update({o: None for o in observers})
+        observers = tuple(ObjectConfig.from_obj(o) for o in observers)
 
-        add_observer_visitor = self._build_observers_visitor(*observers)
+        register = self._build_register(observers)
         for visited in self._iterate_states_and_transitions():
-            add_observer_visitor(visited)
+            visited._add_observer(register)
+
         return self
 
     def _repr_html_(self):

statemachine/transition.py

@@ -73,6 +73,9 @@ def __repr__(self):
             f"internal={self.internal!r})"
         )
 
+    def __str__(self):
+        return f"transition {self.event!s} from {self.source!s} to {self.target!s}"
+
     def _setup(self):
         before = self.before.add
         on = self.on.add

tests/examples/order_control_rich_model_machine.py

@@ -8,7 +8,7 @@
 
 from statemachine import State
 from statemachine import StateMachine
-from statemachine.exceptions import AttrNotFound
+from statemachine.exceptions import InvalidDefinition
 
 
 class Order:
@@ -59,11 +59,16 @@ class OrderControl(StateMachine):
 
 try:
     control = OrderControl()
-except AttrNotFound as e:
+except InvalidDefinition as e:
     assert (  # noqa: PT017
-        str(e) == "Did not found name 'payment_received' from model or statemachine"
+        str(e)
+        == (
+            "Error on transition process_order from Processing to Shipping when resolving "
+            "callbacks: Did not found name 'payment_received' from model or statemachine"
+        )
     )
 
+
 # %%
 # Now initializing with a proper ``order`` instance.
 

tests/test_callbacks.py

@@ -9,7 +9,7 @@
 from statemachine.callbacks import CallbackMetaList
 from statemachine.callbacks import CallbacksExecutor
 from statemachine.callbacks import CallbacksRegistry
-from statemachine.dispatcher import resolver_factory
+from statemachine.dispatcher import resolver_factory_from_objects
 from statemachine.exceptions import InvalidDefinition
 
 
@@ -23,7 +23,9 @@ def __init__(self):
                 ["life_meaning", "name", "a_method"],
             )
             self.registry = CallbacksRegistry()
-            self.executor = self.registry.register(self.callbacks, resolver=resolver_factory(self))
+            self.executor = self.registry.register(
+                self.callbacks, resolver=resolver_factory_from_objects(self)
+            )
 
         @property
         def life_meaning(self):
@@ -49,7 +51,7 @@ def do_something(self, *args, **kwargs):
         obj = MyObject()
 
         meta_list.add(obj.do_something)
-        executor.add(meta_list, resolver_factory(obj))
+        executor.add(meta_list, resolver_factory_from_objects(obj))
 
         executor.call(1, 2, 3, a="x", b="y")
 
@@ -80,7 +82,7 @@ def last_one(self, *args, **kwargs):
         callbacks.add("my_method").add("other_method")
         callbacks.add("last_one")
 
-        registry.register(callbacks, resolver_factory(obj))
+        registry.register(callbacks, resolver_factory_from_objects(obj))
 
         registry[callbacks].call(1, 2, 3, a="x", b="y")
 
@@ -111,7 +113,7 @@ def test_raise_error_if_didnt_found_attr(self, suppress_errors):
         callbacks = CallbackMetaList()
         registry = CallbacksRegistry()
 
-        register = partial(registry.register, resolver=resolver_factory(self))
+        register = partial(registry.register, resolver=resolver_factory_from_objects(self))
 
         callbacks.add(
             "this_does_no_exist",
@@ -139,7 +141,7 @@ def func3():
             return {"key": "value"}
 
         callbacks.add([func1, func2, func3])
-        registry.register(callbacks, resolver_factory(object()))
+        registry.register(callbacks, resolver_factory_from_objects(object()))
 
         results = registry[callbacks].call(1, 2, 3, a="x", b="y")
 
@@ -170,7 +172,7 @@ def hero_lowercase(hero):
         def race_uppercase(race):
             return race.upper()
 
-        x.registry.register(x.callbacks, resolver=resolver_factory(x))
+        x.registry.register(x.callbacks, resolver=resolver_factory_from_objects(x))
 
         assert x.executor.call(hero="Gandalf", race="Maia") == [
             42,

tests/test_dispatcher.py

@@ -1,7 +1,7 @@
 import pytest
 
 from statemachine.dispatcher import ObjectConfig
-from statemachine.dispatcher import resolver_factory
+from statemachine.dispatcher import resolver_factory_from_objects
 from statemachine.dispatcher import search_callable
 
 
@@ -46,28 +46,28 @@ def kwargs(self, request):
     def test_return_same_object_if_already_a_callable(self):
         model = Person("Frodo", "Bolseiro")
         expected = model.get_full_name
-        actual = next(search_callable(expected)).wrap()
+        actual = next(search_callable(expected, [])).wrap()
         assert actual.__name__ == expected.__name__
         assert actual.__doc__ == expected.__doc__
 
     def test_retrieve_a_method_from_its_name(self, args, kwargs):
         model = Person("Frodo", "Bolseiro")
         expected = model.get_full_name
-        method = next(search_callable("get_full_name", ObjectConfig.from_obj(model))).wrap()
+        method = next(search_callable("get_full_name", [ObjectConfig.from_obj(model)])).wrap()
 
         assert method.__name__ == expected.__name__
         assert method.__doc__ == expected.__doc__
         assert method(*args, **kwargs) == "Frodo Bolseiro"
 
     def test_retrieve_a_callable_from_a_property_name(self, args, kwargs):
         model = Person("Frodo", "Bolseiro")
-        method = next(search_callable("first_name", ObjectConfig.from_obj(model))).wrap()
+        method = next(search_callable("first_name", [ObjectConfig.from_obj(model)])).wrap()
 
         assert method(*args, **kwargs) == "Frodo"
 
     def test_retrieve_callable_from_a_property_name_that_should_keep_reference(self, args, kwargs):
         model = Person("Frodo", "Bolseiro")
-        method = next(search_callable("first_name", ObjectConfig.from_obj(model))).wrap()
+        method = next(search_callable("first_name", [ObjectConfig.from_obj(model)])).wrap()
 
         model.first_name = "Bilbo"
 
@@ -88,7 +88,7 @@ def test_should_chain_resolutions(self, attr, expected_value):
         person = Person("Frodo", "Bolseiro", "cpf")
         org = Organization("The Lord fo the Rings", "cnpj")
 
-        resolver = resolver_factory(org, person)
+        resolver = resolver_factory_from_objects(org, person)
         resolved_method = next(resolver(attr)).wrap()
         assert resolved_method() == expected_value
 
@@ -107,6 +107,6 @@ def test_should_ignore_list_of_attrs(self, attr, expected_value):
 
         org_config = ObjectConfig.from_obj(org, {"get_full_name"})
 
-        resolver = resolver_factory(org_config, person)
+        resolver = resolver_factory_from_objects(org_config, person)
         resolved_method = next(resolver(attr)).wrap()
         assert resolved_method() == expected_value

tests/test_transition_list.py

@@ -2,7 +2,7 @@
 
 from statemachine import State
 from statemachine.callbacks import CallbacksRegistry
-from statemachine.dispatcher import resolver_factory
+from statemachine.dispatcher import resolver_factory_from_objects
 
 
 def test_transition_list_or_operator():
@@ -58,6 +58,6 @@ def my_callback():
         transition = s1.transitions[0]
         callback_list = getattr(transition, list_attr_name)
 
-        registry.register(callback_list, resolver_factory(object()))
+        registry.register(callback_list, resolver_factory_from_objects(object()))
 
         assert registry[callback_list].call() == [expected_value]