Skip to content

Commit a3cc90d

Browse files
lyrixxlyrixx
committed
bug #29137 [Workflow][FrameworkBundle] fixed guard event names for transitions (destillat, lyrixx)
This PR was merged into the 3.4 branch. Discussion ---------- [Workflow][FrameworkBundle] fixed guard event names for transitions | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #28018 symfony/symfony#28007 (comment) | License | MIT | Doc PR | There is a bug when many transitions are defined with the same name. I finished destillat's work and rebase against 3.4 as it's a bug fix. There another point of failure, but it could not be fixed on 3.4. I will be a need feature. The issue is related to `Workflow::can($subject, $transitionName)`. Since the transitionName could be not unique, we will need to support passing an instance of Transition. A new PR is incomming Commits ------- 83dc473dd6 [FrameworkBundle] fixed guard event names for transitions fb88bfc79a [FrameworkBundle] fixed guard event names for transitions
2 parents c6cd2af + 66cbef4 commit a3cc90d

File tree

3 files changed

+94
-6
lines changed

3 files changed

+94
-6
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 <fabien@symfony.com>
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: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ class GuardListener
3232
private $roleHierarchy;
3333
private $validator;
3434

35-
public function __construct($configuration, ExpressionLanguage $expressionLanguage, TokenStorageInterface $tokenStorage, AuthorizationCheckerInterface $authenticationChecker, AuthenticationTrustResolverInterface $trustResolver, RoleHierarchyInterface $roleHierarchy = null, ValidatorInterface $validator = null)
35+
public function __construct(array $configuration, ExpressionLanguage $expressionLanguage, TokenStorageInterface $tokenStorage, AuthorizationCheckerInterface $authenticationChecker, AuthenticationTrustResolverInterface $trustResolver, RoleHierarchyInterface $roleHierarchy = null, ValidatorInterface $validator = null)
3636
{
3737
$this->configuration = $configuration;
3838
$this->expressionLanguage = $expressionLanguage;
@@ -49,7 +49,22 @@ public function onTransition(GuardEvent $event, $eventName)
4949
return;
5050
}
5151

52-
if (!$this->expressionLanguage->evaluate($this->configuration[$eventName], $this->getVariables($event))) {
52+
$eventConfiguration = (array) $this->configuration[$eventName];
53+
foreach ($eventConfiguration as $guard) {
54+
if ($guard instanceof GuardExpression) {
55+
if ($guard->getTransition() !== $event->getTransition()) {
56+
continue;
57+
}
58+
$this->validateGuardExpression($event, $guard->getExpression());
59+
} else {
60+
$this->validateGuardExpression($event, $guard);
61+
}
62+
}
63+
}
64+
65+
private function validateGuardExpression(GuardEvent $event, $expression)
66+
{
67+
if (!$this->expressionLanguage->evaluate($expression, $this->getVariables($event))) {
5368
$event->setBlocked(true);
5469
}
5570
}

‎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;
@@ -20,12 +21,17 @@ class GuardListenerTest extends TestCase
2021
private $authenticationChecker;
2122
private $validator;
2223
private $listener;
24+
private $configuration;
2325

2426
protected function setUp()
2527
{
26-
$configuration = array(
28+
$this->configuration = array(
2729
'test_is_granted' => 'is_granted("something")',
2830
'test_is_valid' => 'is_valid(subject)',
31+
'test_expression' => array(
32+
new GuardExpression(new Transition('name', 'from', 'to'), '!is_valid(subject)'),
33+
new GuardExpression(new Transition('name', 'from', 'to'), 'is_valid(subject)'),
34+
),
2935
);
3036
$expressionLanguage = new ExpressionLanguage();
3137
$token = $this->getMockBuilder(TokenInterface::class)->getMock();
@@ -35,7 +41,7 @@ protected function setUp()
3541
$this->authenticationChecker = $this->getMockBuilder(AuthorizationCheckerInterface::class)->getMock();
3642
$trustResolver = $this->getMockBuilder(AuthenticationTrustResolverInterface::class)->getMock();
3743
$this->validator = $this->getMockBuilder(ValidatorInterface::class)->getMock();
38-
$this->listener = new GuardListener($configuration, $expressionLanguage, $tokenStorage, $this->authenticationChecker, $trustResolver, null, $this->validator);
44+
$this->listener = new GuardListener($this->configuration, $expressionLanguage, $tokenStorage, $this->authenticationChecker, $trustResolver, null, $this->validator);
3945
}
4046

4147
protected function tearDown()
@@ -96,11 +102,38 @@ public function testWithValidatorSupportedEventAndAccept()
96102
$this->assertFalse($event->isBlocked());
97103
}
98104

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

105138
return new GuardEvent($subject, $subject->marking, $transition);
106139
}

0 commit comments

Comments
 (0)