fix: Multiple observers can watch the same callback (#387)

Author: fgmacedo

docs/releases/2.1.0.md

@@ -40,3 +40,4 @@ See {ref}`States from Enum types`.
 
 - Fixes [#369](https://github.com/fgmacedo/python-statemachine/issues/369) adding support to wrap
   methods used as {ref}`Actions` decorated with `functools.partial`.
+- Fixes [#384](https://github.com/fgmacedo/python-statemachine/issues/384) so multiple observers can watch the same callback.

statemachine/callbacks.py

@@ -19,6 +19,7 @@ def __init__(self, func, suppress_errors=False, cond=None):
         self.suppress_errors = suppress_errors
         self.cond = Callbacks(factory=ConditionWrapper).add(cond)
         self._callback = None
+        self._resolver_id = None
 
     def __repr__(self):
         return f"{type(self).__name__}({self.func!r})"
@@ -27,7 +28,7 @@ def __str__(self):
         return getattr(self.func, "__name__", self.func)
 
     def __eq__(self, other):
-        return self.func == getattr(other, "func", other)
+        return self.func == other.func and self._resolver_id == other._resolver_id
 
     def __hash__(self):
         return id(self)
@@ -45,6 +46,7 @@ def setup(self, resolver):
         """
         self.cond.setup(resolver)
         try:
+            self._resolver_id = getattr(resolver, "id", id(resolver))
             self._callback = resolver(self.func)
             return True
         except AttrNotFound:
@@ -144,15 +146,15 @@ def all(self, *args, **kwargs):
         return all(condition(*args, **kwargs) for condition in self)
 
     def _add(self, func, resolver=None, prepend=False, **kwargs):
-        if func in self.items:
-            return
-
         resolver = resolver or self._resolver
 
         callback = self.factory(func, **kwargs)
         if resolver is not None and not callback.setup(resolver):
             return
 
+        if callback in self.items:
+            return
+
         if prepend:
             self.items.insert(0, callback)
         else:

statemachine/dispatcher.py

@@ -37,6 +37,28 @@ def _get_func_by_attr(attr, *configs):
     return func, config.obj
 
 
+def _build_attr_wrapper(attr: str, obj):
+    # if `attr` is not callable, then it's an attribute or property,
+    # so `func` contains it's current value.
+    # we'll build a method that get's the fresh value for each call
+    getter = attrgetter(attr)
+
+    def wrapper(*args, **kwargs):
+        return getter(obj)
+
+    return wrapper
+
+
+def _build_sm_event_wrapper(func):
+    "Events already have the 'machine' parameter defined."
+
+    def wrapper(*args, **kwargs):
+        kwargs.pop("machine", None)
+        return func(*args, **kwargs)
+
+    return wrapper
+
+
 def ensure_callable(attr, *objects):
     """Ensure that `attr` is a callable, if not, tries to retrieve one from any of the given
     `objects`.
@@ -56,33 +78,24 @@ def ensure_callable(attr, *objects):
     func, obj = _get_func_by_attr(attr, *configs)
 
     if not callable(func):
-        # if `attr` is not callable, then it's an attribute or property,
-        # so `func` contains it's current value.
-        # we'll build a method that get's the fresh value for each call
-        getter = attrgetter(attr)
-
-        def wrapper(*args, **kwargs):
-            return getter(obj)
-
-        return wrapper
+        return _build_attr_wrapper(attr, obj)
 
     if getattr(func, "_is_sm_event", False):
-        "Events already have the 'machine' parameter defined."
-
-        def wrapper(*args, **kwargs):
-            kwargs.pop("machine")
-            return func(*args, **kwargs)
-
-        return wrapper
+        return _build_sm_event_wrapper(func)
 
     return SignatureAdapter.wrap(func)
 
 
 def resolver_factory(*objects):
     """Factory that returns a configured resolver."""
 
+    objects = [ObjectConfig.from_obj(obj) for obj in objects]
+
     @wraps(ensure_callable)
     def wrapper(attr):
         return ensure_callable(attr, *objects)
 
+    resolver_id = ".".join(str(id(obj.obj)) for obj in objects)
+    wrapper.id = resolver_id
+
     return wrapper

tests/test_dispatcher.py

@@ -112,3 +112,13 @@ def test_should_ignore_list_of_attrs(self, attr, expected_value):
         resolver = resolver_factory(org_config, person)
         resolved_method = resolver(attr)
         assert resolved_method() == expected_value
+
+    def test_should_generate_unique_ids(self):
+        person = Person("Frodo", "Bolseiro", "cpf")
+        org = Organization("The Lord fo the Rings", "cnpj")
+
+        resolver1 = resolver_factory(org, person)
+        resolver2 = resolver_factory(org, person)
+        resolver3 = resolver_factory(org, person)
+
+        assert resolver1.id == resolver2.id == resolver3.id

tests/testcases/issue384_multiple_observers.md

@@ -0,0 +1,57 @@
+### Issue 384
+
+A StateMachine that exercises the example given on issue
+#[384](https://github.com/fgmacedo/python-statemachine/issues/384).
+
+In this example, we register multiple observers to the same named callback.
+
+This works also as a regression test.
+
+```py
+>>> from statemachine import State
+>>> from statemachine import StateMachine
+
+>>> class MyObs:
+...     def on_move_car(self):
+...         print("I observed moving from 1")
+
+>>> class MyObs2:
+...     def on_move_car(self):
+...         print("I observed moving from 2")
+...
+
+
+>>> class Car(StateMachine):
+...     stopped = State(initial=True)
+...     moving = State()
+...
+...     move_car = stopped.to(moving)
+...     stop_car = moving.to(stopped)
+...
+...     def on_move_car(self):
+...         print("I'm moving")
+
+```
+
+Running:
+
+```py
+>>> car = Car()
+>>> obs = MyObs()
+>>> obs2 = MyObs2()
+>>> car.add_observer(obs)
+Car(model=Model(state=stopped), state_field='state', current_state='stopped')
+
+>>> car.add_observer(obs2)
+Car(model=Model(state=stopped), state_field='state', current_state='stopped')
+
+>>> car.add_observer(obs2)  # test to not register duplicated observer callbacks
+Car(model=Model(state=stopped), state_field='state', current_state='stopped')
+
+>>> car.move_car()
+I'm moving
+I observed moving from 1
+I observed moving from 2
+[None, None, None]
+
+```