pytransitions / transitions

A lightweight, object-oriented finite state machine implementation in Python with many extensions
MIT License
5.49k stars 524 forks source link

get_trigger does not check if the state is from state as should be #499

Closed ranvirp closed 3 years ago

ranvirp commented 3 years ago

Below is the code for get_triggers. It does not seem to check if the state is 'FROM' state:

   def get_triggers(self, *args):
        """ Collects all triggers FROM certain states.
        Args:
            *args: Tuple of source states.

        Returns:
            list of transition/trigger names.
        """
        states = set(args)
        return [t for (t, ev) in self.events.items() if any(state in ev.transitions for state in states)]
aleneum commented 3 years ago

Hello @ranvirp,

please provide a minimal example to illustrate the desired behaviour and the actual behaviour. In the above code, args are converted into a set of states which will be compared to the source state of every transition.

ranvirp commented 3 years ago

I checked with one example and get_trigger is working fine. Just wondering, which part it checks about state being first element of transitions list.?

aleneum commented 3 years ago

A machine in transitions has a dictionary of events. The key is the event name (aka trigger) and the value the event itself. Each event contains a dictionary where each key is a source state (string) and each value is a list of possible transitions.

ranvirp commented 3 years ago

Can you guide me in the following code, where I am wrong?. Thanks in advance

from functools import partial

from transitions import Machine, EventData

class TestMachine(Machine):
    initialstates = ["RoomNotCreated", "RoomCreated", "WaitingForUsers"]

    def __init__(self):
        Machine.__init__(self, model=self, states=TestMachine.initialstates, initial="RoomNotCreated", auto_transitions=False, ignore_invalid_triggers=True, send_event=True)
        # TRIGGER source dest conditions, unless, before, after prepare
        self.users = []
        self.roomname = ""
        self.add_transition("create_room", "RoomNotCreated", "RoomCreated", prepare='before_create_room',
                               after='join_room')
        self.add_transition("join_room", "RoomCreated", "WaitingForUsers", prepare='user_joined')
        self.add_transition("join_room", "WaitingForUsers", None, prepare='user_joined',
                               conditions=['sufficient_users_not_joined'])
    def before_create_room(self, event):

        self.roomname = event.kwargs.get('room', 'dummy')
    def sufficient_users_not_joined(self, event):
        return False
    def user_joined(self, event):
        self.users.append(event.kwargs.get('user',"??"))
    def _can_trigger(self, model, *args, **kwargs):
        # We can omit the first two arguments state and event since they are only needed for
        # actual state transitions. We do have to pass the machine (self) and the model as well as
        # args and kwargs meant for the callbacks.
        e = EventData(None, None, self, model, args, kwargs)

        return [trigger_name for trigger_name in self.get_triggers(model.state)
                if any(all(c.check(e) for c in t.conditions)
                       for ts in self.events[trigger_name].transitions.values()
                       for t in ts)]

    # override Machine.add_model to assign 'can_trigger' to the model
    def add_model(self, model, initial=None):
        super(TestMachine, self).add_model(model, initial)
        setattr(model, 'can_trigger', partial(self._can_trigger, model))
if __name__ == "__main__":
    a = TestMachine()
    a.create_room("hello")
    print(a.state)
    print(a.can_trigger()) #should output [] but outputs ["join_room"]
aleneum commented 3 years ago

Hello @ranvirp,

this issue tracker is mainly for bug reports and feature requests. If you have questions about how to use transitions, please post your question on Stackoverflow. Tag your question with [python] and [transitions] to make sure, that users of transitions will receive a notification. If the SO community cannot answer you question, you can notify me by opening a new issue.

aleneum commented 3 years ago

I quote myself why this is the approach I favour:

Your question gains higher visibility since most developers look for help there. The targeted community is larger; Some people will even help you to formulate a good question. People get 'rewarded' with 'reputation' to help you. You also gain reputation in case this questions pops up more frequently. It's a win-win situation.

ranvirp commented 3 years ago

Ok. Thanks. Just wanted to point out that in this example, can_trigger does not output correct value since it does not check for if the source state of transition is as intended.

aleneum commented 3 years ago

Well, if its a bug, the issue tracker is of course the right place to post about it. But please provide a minimal example (just as you would do on SO) and define what you expected to see and what actually happened.

ranvirp commented 3 years ago

I am sorry. posted this on SO. I can see now that get_triggers is fine. can_trigger as mentioned in https://github.com/pytransitions/transitions/issues/256 seems to not generate correct output if we have duplicate trigger names. My apologies for posting in wrong place.

aleneum commented 3 years ago

Don't worry. I hope you will find a solution as soon as possible.

aleneum commented 3 years ago

PeekMachine is also part of the FAQ. I adjusted the example a bit. Now it should also work for more than one transition for each trigger name (with multiple sources). Basically _can_trigger now looks like this:

    def _can_trigger(self, model, *args, **kwargs):
        e = EventData(None, None, self, model, args, kwargs)

        return [trigger_name for trigger_name in self.get_triggers(model.state)
                if any(all(c.check(e) for c in t.conditions)
                       for t in self.events[trigger_name].transitions[model.state])]

I also adjusted add_model to work with the default model setting:

    def add_model(self, model, initial=None):
        for mod in listify(model):
            mod = self if mod == 'self' else mod
            if mod not in self.models:
                setattr(mod, 'can_trigger', partial(self._can_trigger, mod))
                super(PeekMachine, self).add_model(mod, initial)