Workflow/State Machine, guard event is triggered with the wrong transition

[netstrada-master]

Symfony version(s) affected

6.0.8

Description

I want to implement a state machine for a membership subscription.
Here is the diagram:

And here the workflow config:

framework:
    workflows:
        membership:
            type: 'state_machine'
            marking_store:
                type: 'method'
                property: 'state'
            supports:
                - App\Entity\Membership
            initial_marking: requested
            places:
                - requested
                - accepted
                - pending_payment
                - pending_information
                - approved
                - expired
                - declined
            transitions:
                submit:
                    from: requested
                    to: accepted
                approve:
                    from: requested
                    to: accepted
                decline:
                    from: requested
                    to: declined
                start_payment:
                    from: accepted
                    to: pending_payment
                complete_payment:
                    from: pending_payment
                    to: pending_information
                fill_information:
                    from: pending_information
                    to: approved
                dismiss:
                    from: approved
                    to: expired

I wanted to create a guard event for the transition complete_payment where I would check if in the context array there is a token that eventually I would use to validate the payment (like validating the token of an instant payment notification).
To do so I've created an Event Subscriber and listen to:

 'workflow.membership.guard.complete_payment' => ['guardPaymentCheck'],

Surprisingly, when I apply the transition start_payment I see that the guard is triggered.
Also, once I apply complete_payment with or without the context argument, in the Event Subscriber I still get an empty array.

src/EventSubscriber/MembershipPaymentCheckSubscriber.php

<?php
namespace App\EventSubscriber;

use App\Entity\Membership;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\Workflow\Event\GuardEvent;

class MembershipPaymentCheckSubscriber implements EventSubscriberInterface
{
    public function guardPaymentCheck(GuardEvent $event)
    {

        /** @var Membership $membership */
        $membership = $event->getSubject();
        $context = $event->getContext();
        echo "TRIGGERED Context: ".serialize($context)." ";
        if (empty($context['token'])) {
            $event->setBlocked(true, 'Payment failed.');
        }
    }

    public static function getSubscribedEvents()
    {
        return [
            'workflow.membership.guard.complete_payment' => ['guardPaymentCheck'],
        ];
    }
}

Here is an action of a controller I use for testing:

#[Route("/test", name: "membership_test")]
    public function test(Request $request, EntityManagerInterface $entityManager, WorkflowInterface $membershipStateMachine)
    {
        /** @var Membership $m */
        $m = $entityManager->find(Membership::class, 1);
        try {
            echo " #1 ";
            $membershipStateMachine->apply($m, 'submit');
            echo " #2 ";
            $membershipStateMachine->apply($m, 'start_payment');
            echo " #3 ";
            $membershipStateMachine->apply($m, 'complete_payment');
            echo " #4 ";
//            $membershipStateMachine->apply($m, 'complete_payment', ['token' => 'test']);
        } catch (\Exception $exception) {
            die($exception->getMessage());

        }

        echo " || done : ";
        die($m->getState());

        return $this->redirectToRoute("membership_index");
    }

and this is what I get when I visit the action page

 #1  #2 TRIGGERED Context: a:0:{}  #3 TRIGGERED Context: a:0:{} Transition "complete_payment" is not enabled for workflow "membership".

As you can see after #2 the guard is executed, as well after the #3 .
Please note that, decommenting the line after echo " #4 "; I still get

#1  #2 TRIGGERED Context: a:0:{}  #3 TRIGGERED Context: a:0:{} Transition "complete_payment" is not enabled for workflow "membership".

since the exception rise from the first apply in complete payment.

Instead, decommenting only the line after echo " #3 "; this is what I got:

 #1  #2 TRIGGERED Context: a:0:{}  #3  #4 TRIGGERED Context: a:0:{} Transition "complete_payment" is not enabled for workflow "membership".

So still not data into context.

How to reproduce

To reproduce it, simply create a symfony project and use the configuration I have pasted and create a controller with that action.

Possible Solution

No response

Additional Context

No response

[DylanKas]

I may be wrong here, but I don't think there is an issue.
You are blocking the complete_payment transition in your guard.

So when trying to apply the transition, you get a NotEnabledTransitionException.

The second parameter of setBlocked add your message in the transition blocker list:

https://github.com/symfony/workflow/blob/5.4/Event/GuardEvent.php

    public function setBlocked(bool $blocked, string $message = null): void
    {
        if (!$blocked) {
            $this->transitionBlockerList->clear();

            return;
        }

        $this->transitionBlockerList->add(TransitionBlocker::createUnknown($message));
    }

so I believe you need to do something like this to access your message:

$blockerList->buildTransitionBlockerList($m, 'complete_payment');

Here $blockerList is a TransitionBlockerList so you can use this object to see why your transition has been blocked.

[netstrada-master]

Hey @DylanKas, thanks so much for your answer!
After reading again the documentation I think I understand why the Guard is fired also when I apply the start_payment transition.
I think it is because, as the documentation is saying, that GuardEvent(s)

are invoked every time a call to Workflow::can(), Workflow::apply() or Workflow::getEnabledTransitions()

see https://symfony.com/doc/current/workflow.html#guard-events

So perhaps after apply the start_payment transition, the framework is also processing inside the getEnabledTransitions and therefore the guard for complete_payment gets triggered.
@DylanKas can it be the case? At least this would solve this "mystery".

If so, here I have the last question:
I've seen that the GuardEvent do not have the context array filled with the props;
should I assume that my idea to pass a token in the context argument and let him validate in the GuardEvent is a bad approach?

[AlexBevilacqua]

First,

The context is accessible in all events except for the workflow.guard events

This is your main problem.

Then, the first triggering of your guard is probably caused by the announce event being dispatched when #2 completes (you're right, internally it's calling getAvailableTransitions, or another of those methods that does in fact trigger your guard).

You can disable the announce event, but your first problem remains.

And btw, to conclude.. of course your guard is triggered again when you proceed to try to apply #3. The fact that there's a blocker make it trigger the exception, because it cannot be applied.

(P.s. really don't think there's any bug of the workflow component here)

[cjgordon]

Hi @AlexBevilacqua

Is my understanding correct that it is expected when you call:
$stateMachine->apply($document, 'to_published', ['somecontext' => 'the context.' ]);

That when the guard is triggered and you get the context in the guard function:
$context = $guardEvent->getContext();

That the context is an empty array? I think that may be @netstrada's issue.

For me it is weird that the GuardEvent has no context but then in the transition function the TransitionEvent does have the context.

This may be intentional however?

Cheers
Chris

[AlexBevilacqua]

Hi @cjgordon

Yes, that's correct.
I personally find it a bit weird too, but that's also how it's documented

https://symfony.com/doc/current/workflow.html#using-events

Right below the list of events:

The context is accessible in all events except for the workflow.guard events

That's why I believe it's fully intentional indeed, and not a bug at the current state of things..

[cjgordon]

That is a fair assessment.

I worked around it for my purposes by using temporary property on the entity.

Not sure if that will help your scenario @netstrada

[lyrixx]

Hello,

Your listener is called because after the transition complete_payment is applied, the workflow searches for new available available transitions, in order to dispatch a new event.

ref: https://github.com/symfony/symfony/blob/6.2/src/Symfony/Component/Workflow/Workflow.php#L240

You can control what events event (workflow.announce) are dispatched thanks to the configuration

---

about

Also, once I apply complete_payment with or without the context argument, in the Event Subscriber I still get an empty array.

This is done on purpose, to avoid any side effect. The guard must vote with the subject only, not with the context. Context is here only for additional data to be saved. So if you need more context to be able to take a decision, you must attach it to your subject (but it can be volatil data, it does't have to be stored)

[vovkin]

Hi @lyrixx,

regarding

This is done on purpose, to avoid any side effect. The guard must vote with the subject only, not with the context. Context is here only for additional data to be saved. So if you need more context to be able to take a decision, you must attach it to your subject (but it can be volatil data, it does't have to be stored)

I wonder if being able to use context isn't better solution in this case. Because then context is used only within workflow, while with proposed solution it has to be attached to the subject, and often subject is an entity. As result entity gets properties which are used solely for making decision in workflow, and it is tightly bound through these properties to the specific component of the framework.

Context is allowed for other Event objects, is there a good reason for not having it for GuardEvent?
Being able to pass extra data and take decision based on that seems logical, and results in simpler code. It allows to keep guard for one case (e.g. for building list of available transitions) and disable for other (like applying transition under some circumstances) in easy way, w/o polutting entity with extra properties.

Is it possible to consider reintroducing context in guard events? I could create PR.