Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit c562e71

Browse filesBrowse files
committed
bug #31584 [Workflow] Do not trigger extra guards (lyrixx)
This PR was merged into the 3.4 branch. Discussion ---------- [Workflow] Do not trigger extra guards | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #31582 | License | MIT | Doc PR | --- With this patch, guards are executed only on wanted transitions **Note for merger**: This is already fixed (in a different manner) in 4.2, So this patch should not be included in 4.2, instead take: #31585 or discard changes of Workflow class but keep tests Commits ------- ad06197 [Workflow] Do not trigger extra guard
2 parents d0bea7a + ad06197 commit c562e71
Copy full SHA for c562e71

File tree

2 files changed

+49
-16
lines changed
Filter options

2 files changed

+49
-16
lines changed

‎src/Symfony/Component/Workflow/Tests/WorkflowTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Workflow/Tests/WorkflowTest.php
+39Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,45 @@ public function testApplyWithEventDispatcher()
304304
$this->assertSame($eventNameExpected, $eventDispatcher->dispatchedEvents);
305305
}
306306

307+
public function testApplyDoesNotTriggerExtraGuardWithEventDispatcher()
308+
{
309+
$transitions[] = new Transition('a-b', 'a', 'b');
310+
$transitions[] = new Transition('a-c', 'a', 'c');
311+
$definition = new Definition(['a', 'b', 'c'], $transitions);
312+
313+
$subject = new \stdClass();
314+
$subject->marking = null;
315+
$eventDispatcher = new EventDispatcherMock();
316+
$workflow = new Workflow($definition, new MultipleStateMarkingStore(), $eventDispatcher, 'workflow_name');
317+
318+
$eventNameExpected = [
319+
'workflow.guard',
320+
'workflow.workflow_name.guard',
321+
'workflow.workflow_name.guard.a-b',
322+
'workflow.leave',
323+
'workflow.workflow_name.leave',
324+
'workflow.workflow_name.leave.a',
325+
'workflow.transition',
326+
'workflow.workflow_name.transition',
327+
'workflow.workflow_name.transition.a-b',
328+
'workflow.enter',
329+
'workflow.workflow_name.enter',
330+
'workflow.workflow_name.enter.b',
331+
'workflow.entered',
332+
'workflow.workflow_name.entered',
333+
'workflow.workflow_name.entered.b',
334+
'workflow.completed',
335+
'workflow.workflow_name.completed',
336+
'workflow.workflow_name.completed.a-b',
337+
'workflow.announce',
338+
'workflow.workflow_name.announce',
339+
];
340+
341+
$marking = $workflow->apply($subject, 'a-b');
342+
343+
$this->assertSame($eventNameExpected, $eventDispatcher->dispatchedEvents);
344+
}
345+
307346
public function testEventName()
308347
{
309348
$definition = $this->createComplexWorkflowDefinition();

‎src/Symfony/Component/Workflow/Workflow.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Workflow/Workflow.php
+10-16Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -125,22 +125,20 @@ public function can($subject, $transitionName)
125125
*/
126126
public function apply($subject, $transitionName)
127127
{
128-
$transitions = $this->getEnabledTransitions($subject);
129-
130-
// We can shortcut the getMarking method in order to boost performance,
131-
// since the "getEnabledTransitions" method already checks the Marking
132-
// state
133-
$marking = $this->markingStore->getMarking($subject);
134-
135-
$applied = false;
128+
$marking = $this->getMarking($subject);
129+
$transitions = [];
136130

137-
foreach ($transitions as $transition) {
138-
if ($transitionName !== $transition->getName()) {
139-
continue;
131+
foreach ($this->definition->getTransitions() as $transition) {
132+
if ($transitionName === $transition->getName() && $this->doCan($subject, $marking, $transition)) {
133+
$transitions[] = $transition;
140134
}
135+
}
141136

142-
$applied = true;
137+
if (!$transitions) {
138+
throw new LogicException(sprintf('Unable to apply transition "%s" for workflow "%s".', $transitionName, $this->name));
139+
}
143140

141+
foreach ($transitions as $transition) {
144142
$this->leave($subject, $transition, $marking);
145143

146144
$this->transition($subject, $transition, $marking);
@@ -156,10 +154,6 @@ public function apply($subject, $transitionName)
156154
$this->announce($subject, $transition, $marking);
157155
}
158156

159-
if (!$applied) {
160-
throw new LogicException(sprintf('Unable to apply transition "%s" for workflow "%s".', $transitionName, $this->name));
161-
}
162-
163157
return $marking;
164158
}
165159

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.