-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[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
Comments
Possible workaround: add a property in |
Can you create a small reproducer please? |
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 Yes it worked, sorry for the delay 😅 Maybe add a warning in the documentation? ("Your guards will be called multiple times when checking for enabled transitions") |
Feel free to open a PR to enhance the documentation. We will be happy to merge your PR 😊 |
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 🙂
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 🙂
… 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
I think this issue is related to this one : #27925 (comment) |
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
And the listener:
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
The text was updated successfully, but these errors were encountered: