Skip to content

[Workflow] Guards events too "greedy" #45172

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
kissifrot opened this issue Jan 25, 2022 · 7 comments
Closed

[Workflow] Guards events too "greedy" #45172

kissifrot opened this issue Jan 25, 2022 · 7 comments

Comments

@kissifrot
Copy link

Symfony version(s) affected

5.4.2

Description

Hello,

As a follow up of #38960, when a transition is made, a guard event is sent for every possible transition we are getting from.

However in our case those guard event listeners are quite resource-intensive (lots of checks are made in the DB).
Would there a way to not fire all of them when we know we will be doing only one transition?

How to reproduce

A sample I borrowed in #38960

signature:
  type: 'state_machine'
  audit_trail:
     enabled: true
  marking_store:
     type: 'method'
     property: 'state'
  initial_marking: created
  supports:
    - App\Entity\Signature
  places:
    - created
    - code_sent
    - signed
  transitions:
     send_code:
       from: created
       to: code_sent
     sign:
       from: code_sent
       to: signed

And the listener:

<?php

namespace App\Workflow\Guard;

use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\Workflow\Event\GuardEvent;

class ReservationInProgressGuard implements EventSubscriberInterface
{
    public static function getSubscribedEvents(): array
    {
        return [
            'workflow.reservation.guard.send_code' => 'OnEvent',
            'workflow.reservation.guard.sign' => 'OnEvent',
        ];
    }

    public function OnEvent(GuardEvent $event): void
    {
        // Some heavy process which should be done only once per transition
    }
}

Possible Solution

It is stated in the doc guard events are always fired, but wouldn't it be possible to not fire them, or have an info in the even which states "OK I was called for the transition you listened to, let's not continue further" ?

Additional Context

No response

@alexislefebvre
Copy link
Contributor

Possible workaround: add a property in ReservationInProgressGuard, define it after the first call, then check that it's not defined before calling the checks. This would be similar to the // Make sure we're not called twice code shown for API Platform.

@lyrixx
Copy link
Member

lyrixx commented Jan 26, 2022

Can you create a small reproducer please?

@kissifrot
Copy link
Author

Here it is:

Workflow YAML config :

framework:
  workflows:
    test:
      type: state_machine
      supports:
        - App\Entity\MyEntity
      marking_store:
        type: method
        property: marking
      initial_marking: 'created'
      places:
        - 'created'
        - 'in-progress'
        - 'confirmed'
        - 'abandonned'
      transitions:
        - name: 'initialize'
          from: 'created'
          to: 'in-progress'
          metadata:
            title: Initialize
        - name: 'abandon'
          from: 'in-progress'
          to: 'abandonned'
          metadata:
            title: Abandon
        - name: 'confirm'
          from: 'in-progress'
          to: 'confirmed'
          metadata:
            title: Confirm

Guard Listener:

<?php

namespace App\Workflow\Guard;

use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\Workflow\Event\GuardEvent;

class TestInProgressGuard implements EventSubscriberInterface
{
    public static function getSubscribedEvents(): array
    {
        return [
            'workflow.test.guard.initialize' => 'onInProgress',
            'workflow.test.guard.confirm' => 'onInProgress',
        ];
    }

    public function onInProgress(GuardEvent $event): void
    {
        dump('I should be called only once if possible as I am heavy');
    }
}

Then in the code

<?php
class DetailsController extends AbstractController
{
    public function __invoke(WorkflowInterface $testWorkflow)
    {
        $data = new MyEntity();
        $testWorkflow->apply($data, 'initialize');
        // ...
    }
}

You will see two dumps in the profiler

@lyrixx
Copy link
Member

lyrixx commented Jan 27, 2022

It's because of the annonce event. You can disable it if you want.

Did it help?

@kissifrot
Copy link
Author

@lyrixx Yes it worked, sorry for the delay 😅
Documentation is a bit misleading in that case, as this indeed "removes" calls to getEnabledTransitions() and thus subsequent calls to the concerned guards.

Maybe add a warning in the documentation? ("Your guards will be called multiple times when checking for enabled transitions")

@lyrixx
Copy link
Member

lyrixx commented Feb 3, 2022

Feel free to open a PR to enhance the documentation. We will be happy to merge your PR 😊

@lyrixx lyrixx closed this as completed Feb 3, 2022
kissifrot added a commit to kissifrot/symfony-docs that referenced this issue Feb 14, 2022
As a follow-up of symfony/symfony#45172, add a note about guards which may be performance-heavy and easiest way to disable them being called too often

This is my first doc commit so don't hesitate to guide me 🙂
kissifrot added a commit to kissifrot/symfony-docs that referenced this issue Feb 14, 2022
As a follow up of symfony/symfony#45172, I'm adding a note mentioning ability to disable announce events to lower calls in guard events.

This is my first doc PR so don't hesitate to guide me 🙂
javiereguiluz added a commit to symfony/symfony-docs that referenced this issue Aug 9, 2022
… save some CPU (kissifrot)

This PR was squashed before being merged into the 5.4 branch.

Discussion
----------

Workflow - Add a note about guards events and ability to save some CPU

As a follow up of symfony/symfony#45172, I'm adding a note mentioning ability to disable announce events to lower calls in guard events.

Had to add a reference to next paragraph in case it would move elsewhere 🙂

This is my first doc PR so don't hesitate to guide me 🙂

<!--

If your pull request fixes a BUG, use the oldest maintained branch that contains
the bug (see https://symfony.com/releases for the list of maintained branches).

If your pull request documents a NEW FEATURE, use the same Symfony branch where
the feature was introduced (and `6.x` for features of unreleased versions).

-->

Commits
-------

e5c176c Workflow - Add a note about guards events and ability to save some CPU
@walva
Copy link
Contributor

walva commented Apr 24, 2024

I think this issue is related to this one : #27925 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants