Skip to content

Commit

Permalink
bug #31584 [Workflow] Do not trigger extra guards (lyrixx)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
fabpot committed May 27, 2019
2 parents d0bea7a + ad06197 commit c562e71
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 16 deletions.
39 changes: 39 additions & 0 deletions src/Symfony/Component/Workflow/Tests/WorkflowTest.php
Expand Up @@ -304,6 +304,45 @@ public function testApplyWithEventDispatcher()
$this->assertSame($eventNameExpected, $eventDispatcher->dispatchedEvents);
}

public function testApplyDoesNotTriggerExtraGuardWithEventDispatcher()
{
$transitions[] = new Transition('a-b', 'a', 'b');
$transitions[] = new Transition('a-c', 'a', 'c');
$definition = new Definition(['a', 'b', 'c'], $transitions);

$subject = new \stdClass();
$subject->marking = null;
$eventDispatcher = new EventDispatcherMock();
$workflow = new Workflow($definition, new MultipleStateMarkingStore(), $eventDispatcher, 'workflow_name');

$eventNameExpected = [
'workflow.guard',
'workflow.workflow_name.guard',
'workflow.workflow_name.guard.a-b',
'workflow.leave',
'workflow.workflow_name.leave',
'workflow.workflow_name.leave.a',
'workflow.transition',
'workflow.workflow_name.transition',
'workflow.workflow_name.transition.a-b',
'workflow.enter',
'workflow.workflow_name.enter',
'workflow.workflow_name.enter.b',
'workflow.entered',
'workflow.workflow_name.entered',
'workflow.workflow_name.entered.b',
'workflow.completed',
'workflow.workflow_name.completed',
'workflow.workflow_name.completed.a-b',
'workflow.announce',
'workflow.workflow_name.announce',
];

$marking = $workflow->apply($subject, 'a-b');

$this->assertSame($eventNameExpected, $eventDispatcher->dispatchedEvents);
}

public function testEventName()
{
$definition = $this->createComplexWorkflowDefinition();
Expand Down
26 changes: 10 additions & 16 deletions src/Symfony/Component/Workflow/Workflow.php
Expand Up @@ -125,22 +125,20 @@ public function can($subject, $transitionName)
*/
public function apply($subject, $transitionName)
{
$transitions = $this->getEnabledTransitions($subject);

// We can shortcut the getMarking method in order to boost performance,
// since the "getEnabledTransitions" method already checks the Marking
// state
$marking = $this->markingStore->getMarking($subject);

$applied = false;
$marking = $this->getMarking($subject);
$transitions = [];

foreach ($transitions as $transition) {
if ($transitionName !== $transition->getName()) {
continue;
foreach ($this->definition->getTransitions() as $transition) {
if ($transitionName === $transition->getName() && $this->doCan($subject, $marking, $transition)) {
$transitions[] = $transition;
}
}

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

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

$this->transition($subject, $transition, $marking);
Expand All @@ -156,10 +154,6 @@ public function apply($subject, $transitionName)
$this->announce($subject, $transition, $marking);
}

if (!$applied) {
throw new LogicException(sprintf('Unable to apply transition "%s" for workflow "%s".', $transitionName, $this->name));
}

return $marking;
}

Expand Down

0 comments on commit c562e71

Please sign in to comment.