Skip to content

Commit b922edf

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '4.1'
* 4.1: Undeprecate the single-colon notation for controllers Command::addOption should allow int in $default Update symfony links to https [Form] Fixed keeping hash of equal \DateTimeInterface on submit [PhpUnitBridge] Fix typo [Routing] generate(null) should throw an exception [Form] Minor fixes in docs and cs [Workflow] Made code simpler [Config] Unset key during normalization [Form] Fixed empty data for compound date types invalidate forms on transformation failures [FrameworkBundle] fixed guard event names for transitions method buildTransitionBlockerList returns TransitionBlockerList of expected transition [FrameworkBundle] fixed guard event names for transitions [PropertyAccessor] Fix unable to write to singular property using setter while plural adder/remover exist
2 parents ec2b697 + 1ef13e6 commit b922edf

File tree

8 files changed

+208
-8
lines changed

8 files changed

+208
-8
lines changed

‎EventListener/GuardExpression.php

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <[email protected]>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Workflow\EventListener;
13+
14+
use Symfony\Component\Workflow\Transition;
15+
16+
class GuardExpression
17+
{
18+
private $transition;
19+
20+
private $expression;
21+
22+
/**
23+
* @param string $expression
24+
*/
25+
public function __construct(Transition $transition, $expression)
26+
{
27+
$this->transition = $transition;
28+
$this->expression = $expression;
29+
}
30+
31+
public function getTransition()
32+
{
33+
return $this->transition;
34+
}
35+
36+
public function getExpression()
37+
{
38+
return $this->expression;
39+
}
40+
}

‎EventListener/GuardListener.php

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,21 @@ public function onTransition(GuardEvent $event, $eventName)
5050
return;
5151
}
5252

53-
$expression = $this->configuration[$eventName];
53+
$eventConfiguration = (array) $this->configuration[$eventName];
54+
foreach ($eventConfiguration as $guard) {
55+
if ($guard instanceof GuardExpression) {
56+
if ($guard->getTransition() !== $event->getTransition()) {
57+
continue;
58+
}
59+
$this->validateGuardExpression($event, $guard->getExpression());
60+
} else {
61+
$this->validateGuardExpression($event, $guard);
62+
}
63+
}
64+
}
5465

66+
private function validateGuardExpression(GuardEvent $event, string $expression)
67+
{
5568
if (!$this->expressionLanguage->evaluate($expression, $this->getVariables($event))) {
5669
$blocker = TransitionBlocker::createBlockedByExpressionGuardListener($expression);
5770
$event->addTransitionBlocker($blocker);

‎Tests/EventListener/GuardListenerTest.php

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use Symfony\Component\Validator\Validator\ValidatorInterface;
1212
use Symfony\Component\Workflow\Event\GuardEvent;
1313
use Symfony\Component\Workflow\EventListener\ExpressionLanguage;
14+
use Symfony\Component\Workflow\EventListener\GuardExpression;
1415
use Symfony\Component\Workflow\EventListener\GuardListener;
1516
use Symfony\Component\Workflow\Marking;
1617
use Symfony\Component\Workflow\Transition;
@@ -21,12 +22,17 @@ class GuardListenerTest extends TestCase
2122
private $authenticationChecker;
2223
private $validator;
2324
private $listener;
25+
private $configuration;
2426

2527
protected function setUp()
2628
{
27-
$configuration = array(
29+
$this->configuration = array(
2830
'test_is_granted' => 'is_granted("something")',
2931
'test_is_valid' => 'is_valid(subject)',
32+
'test_expression' => array(
33+
new GuardExpression(new Transition('name', 'from', 'to'), '!is_valid(subject)'),
34+
new GuardExpression(new Transition('name', 'from', 'to'), 'is_valid(subject)'),
35+
),
3036
);
3137
$expressionLanguage = new ExpressionLanguage();
3238
$token = $this->getMockBuilder(TokenInterface::class)->getMock();
@@ -36,7 +42,7 @@ protected function setUp()
3642
$this->authenticationChecker = $this->getMockBuilder(AuthorizationCheckerInterface::class)->getMock();
3743
$trustResolver = $this->getMockBuilder(AuthenticationTrustResolverInterface::class)->getMock();
3844
$this->validator = $this->getMockBuilder(ValidatorInterface::class)->getMock();
39-
$this->listener = new GuardListener($configuration, $expressionLanguage, $tokenStorage, $this->authenticationChecker, $trustResolver, null, $this->validator);
45+
$this->listener = new GuardListener($this->configuration, $expressionLanguage, $tokenStorage, $this->authenticationChecker, $trustResolver, null, $this->validator);
4046
}
4147

4248
protected function tearDown()
@@ -97,11 +103,38 @@ public function testWithValidatorSupportedEventAndAccept()
97103
$this->assertFalse($event->isBlocked());
98104
}
99105

100-
private function createEvent()
106+
public function testWithGuardExpressionWithNotSupportedTransition()
107+
{
108+
$event = $this->createEvent();
109+
$this->configureValidator(false);
110+
$this->listener->onTransition($event, 'test_expression');
111+
112+
$this->assertFalse($event->isBlocked());
113+
}
114+
115+
public function testWithGuardExpressionWithSupportedTransition()
116+
{
117+
$event = $this->createEvent($this->configuration['test_expression'][1]->getTransition());
118+
$this->configureValidator(true, true);
119+
$this->listener->onTransition($event, 'test_expression');
120+
121+
$this->assertFalse($event->isBlocked());
122+
}
123+
124+
public function testGuardExpressionBlocks()
125+
{
126+
$event = $this->createEvent($this->configuration['test_expression'][1]->getTransition());
127+
$this->configureValidator(true, false);
128+
$this->listener->onTransition($event, 'test_expression');
129+
130+
$this->assertTrue($event->isBlocked());
131+
}
132+
133+
private function createEvent(Transition $transition = null)
101134
{
102135
$subject = new \stdClass();
103136
$subject->marking = new Marking();
104-
$transition = new Transition('name', 'from', 'to');
137+
$transition = $transition ?: new Transition('name', 'from', 'to');
105138

106139
$workflow = $this->getMockBuilder(WorkflowInterface::class)->getMock();
107140

‎Tests/StateMachineTest.php

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@
33
namespace Symfony\Component\Workflow\Tests;
44

55
use PHPUnit\Framework\TestCase;
6+
use Symfony\Component\EventDispatcher\EventDispatcher;
7+
use Symfony\Component\Workflow\Event\GuardEvent;
68
use Symfony\Component\Workflow\StateMachine;
9+
use Symfony\Component\Workflow\TransitionBlocker;
710

811
class StateMachineTest extends TestCase
912
{
@@ -38,4 +41,70 @@ public function testCanWithMultipleTransition()
3841
$this->assertTrue($net->can($subject, 't2'));
3942
$this->assertTrue($net->can($subject, 't3'));
4043
}
44+
45+
public function testBuildTransitionBlockerList()
46+
{
47+
$definition = $this->createComplexStateMachineDefinition();
48+
49+
$net = new StateMachine($definition);
50+
$subject = new \stdClass();
51+
52+
$subject->marking = 'a';
53+
$this->assertTrue($net->buildTransitionBlockerList($subject, 't1')->isEmpty());
54+
$subject->marking = 'd';
55+
$this->assertTrue($net->buildTransitionBlockerList($subject, 't1')->isEmpty());
56+
57+
$subject->marking = 'b';
58+
$this->assertFalse($net->buildTransitionBlockerList($subject, 't1')->isEmpty());
59+
}
60+
61+
public function testBuildTransitionBlockerListWithMultipleTransitions()
62+
{
63+
$definition = $this->createComplexStateMachineDefinition();
64+
65+
$net = new StateMachine($definition);
66+
$subject = new \stdClass();
67+
68+
$subject->marking = 'b';
69+
$this->assertTrue($net->buildTransitionBlockerList($subject, 't2')->isEmpty());
70+
$this->assertTrue($net->buildTransitionBlockerList($subject, 't3')->isEmpty());
71+
}
72+
73+
public function testBuildTransitionBlockerListReturnsExpectedReasonOnBranchMerge()
74+
{
75+
$definition = $this->createComplexStateMachineDefinition();
76+
77+
$dispatcher = new EventDispatcher();
78+
$net = new StateMachine($definition, null, $dispatcher);
79+
80+
$dispatcher->addListener('workflow.guard', function (GuardEvent $event) {
81+
$event->addTransitionBlocker(new TransitionBlocker(\sprintf('Transition blocker of place %s', $event->getTransition()->getFroms()[0]), 'blocker'));
82+
});
83+
84+
$subject = new \stdClass();
85+
86+
// There may be multiple transitions with the same name. Make sure that transitions
87+
// that are not enabled by the marking are evaluated.
88+
// see https://github.com/symfony/symfony/issues/28432
89+
90+
// Test if when you are in place "a"trying transition "t1" then returned
91+
// blocker list contains guard blocker instead blockedByMarking
92+
$subject->marking = 'a';
93+
$transitionBlockerList = $net->buildTransitionBlockerList($subject, 't1');
94+
$this->assertCount(1, $transitionBlockerList);
95+
$blockers = iterator_to_array($transitionBlockerList);
96+
97+
$this->assertSame('Transition blocker of place a', $blockers[0]->getMessage());
98+
$this->assertSame('blocker', $blockers[0]->getCode());
99+
100+
// Test if when you are in place "d" trying transition "t1" then
101+
// returned blocker list contains guard blocker instead blockedByMarking
102+
$subject->marking = 'd';
103+
$transitionBlockerList = $net->buildTransitionBlockerList($subject, 't1');
104+
$this->assertCount(1, $transitionBlockerList);
105+
$blockers = iterator_to_array($transitionBlockerList);
106+
107+
$this->assertSame('Transition blocker of place d', $blockers[0]->getMessage());
108+
$this->assertSame('blocker', $blockers[0]->getCode());
109+
}
41110
}

‎Tests/WorkflowTest.php

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,32 @@ public function testBuildTransitionBlockerListReturnsUndefinedTransition()
195195
$workflow->buildTransitionBlockerList($subject, '404 Not Found');
196196
}
197197

198+
public function testBuildTransitionBlockerList()
199+
{
200+
$definition = $this->createComplexWorkflowDefinition();
201+
$subject = new \stdClass();
202+
$subject->marking = null;
203+
$workflow = new Workflow($definition, new MultipleStateMarkingStore());
204+
205+
$this->assertTrue($workflow->buildTransitionBlockerList($subject, 't1')->isEmpty());
206+
$this->assertFalse($workflow->buildTransitionBlockerList($subject, 't2')->isEmpty());
207+
208+
$subject->marking = array('b' => 1);
209+
210+
$this->assertFalse($workflow->buildTransitionBlockerList($subject, 't1')->isEmpty());
211+
$this->assertFalse($workflow->buildTransitionBlockerList($subject, 't2')->isEmpty());
212+
213+
$subject->marking = array('b' => 1, 'c' => 1);
214+
215+
$this->assertFalse($workflow->buildTransitionBlockerList($subject, 't1')->isEmpty());
216+
$this->assertTrue($workflow->buildTransitionBlockerList($subject, 't2')->isEmpty());
217+
218+
$subject->marking = array('f' => 1);
219+
220+
$this->assertFalse($workflow->buildTransitionBlockerList($subject, 't5')->isEmpty());
221+
$this->assertTrue($workflow->buildTransitionBlockerList($subject, 't6')->isEmpty());
222+
}
223+
198224
public function testBuildTransitionBlockerListReturnsReasonsProvidedByMarking()
199225
{
200226
$definition = $this->createComplexWorkflowDefinition();

‎TransitionBlockerList.php

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,17 @@ public function add(TransitionBlocker $blocker): void
3737
$this->blockers[] = $blocker;
3838
}
3939

40+
public function has(string $code): bool
41+
{
42+
foreach ($this->blockers as $blocker) {
43+
if ($code === $blocker->getCode()) {
44+
return true;
45+
}
46+
}
47+
48+
return false;
49+
}
50+
4051
public function clear(): void
4152
{
4253
$this->blockers = array();

‎Workflow.php

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,15 @@ public function buildTransitionBlockerList($subject, string $transitionName): Tr
119119
$transitionBlockerList = $this->buildTransitionBlockerListForTransition($subject, $marking, $transition);
120120

121121
if ($transitionBlockerList->isEmpty()) {
122-
continue;
122+
return $transitionBlockerList;
123+
}
124+
125+
// We prefer to return transitions blocker by something else than
126+
// marking. Because it means the marking was OK. Transitions are
127+
// deterministic: it's not possible to have many transitions enabled
128+
// at the same time that match the same marking with the same name
129+
if (!$transitionBlockerList->has(TransitionBlocker::BLOCKED_BY_MARKING)) {
130+
return $transitionBlockerList;
123131
}
124132
}
125133

‎composer.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
"type": "library",
44
"description": "Symfony Workflow Component",
55
"keywords": ["workflow", "petrinet", "place", "transition", "statemachine", "state"],
6-
"homepage": "http://symfony.com",
6+
"homepage": "https://symfony.com",
77
"license": "MIT",
88
"authors": [
99
{
@@ -16,7 +16,7 @@
1616
},
1717
{
1818
"name": "Symfony Community",
19-
"homepage": "http://symfony.com/contributors"
19+
"homepage": "https://symfony.com/contributors"
2020
}
2121
],
2222
"require": {

0 commit comments

Comments
 (0)