Skip to content

Commit 568340c

Browse files
author
Robin Chalas
committed
Merge branch '4.4' into 5.0
* 4.4: (38 commits) reset the kernel cache after each test [HttpKernel] Ability to define multiple kernel.reset tags [Routing] Continue supporting single colon in object route loaders [FWBundle] Remove unused parameter [Intl] [Workflow] fixes English grammar typos [Filesystem] [Serializer] fixes English grammar typo mailer: mailchimp bridge is throwing undefined index _id when setting message id in mandrill http transport has_roles should be is_granted in upgrade files [HttpClient] Fix early cleanup of pushed HTTP/2 responses skip test on incompatible PHP versions [HttpKernel] Don't cache "not-fresh" state [FrameworkBundle][Cache] Don't deep-merge cache pools configuration [Messenger] Adding exception to amqp transport in case amqp ext is not installed [SecurityBundle] Don't require a user provider for the anonymous listener [Monolog Bridge] Fixed accessing static property as non static. Improve Symfony description [Mailer] Add UPGRADE entries about Envelope and MessageEvent [FrameworkBundle] fix leftover mentioning "secret:" processor Add DateTimeZoneNormalizer into Dependency Injection [Messenger] Error when specified default bus is not among the configured ...
2 parents caeaed9 + da0cf4e commit 568340c

File tree

3 files changed

+64
-24
lines changed

3 files changed

+64
-24
lines changed

‎MarkingStore/MethodMarkingStore.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public function getMarking(object $subject): Marking
5151
$method = 'get'.ucfirst($this->property);
5252

5353
if (!method_exists($subject, $method)) {
54-
throw new LogicException(sprintf('The method "%s::%s()" does not exists.', \get_class($subject), $method));
54+
throw new LogicException(sprintf('The method "%s::%s()" does not exist.', \get_class($subject), $method));
5555
}
5656

5757
$marking = $subject->{$method}();
@@ -81,7 +81,7 @@ public function setMarking(object $subject, Marking $marking, array $context = [
8181
$method = 'set'.ucfirst($this->property);
8282

8383
if (!method_exists($subject, $method)) {
84-
throw new LogicException(sprintf('The method "%s::%s()" does not exists.', \get_class($subject), $method));
84+
throw new LogicException(sprintf('The method "%s::%s()" does not exist.', \get_class($subject), $method));
8585
}
8686

8787
$subject->{$method}($marking, $context);

‎Tests/StateMachineTest.php

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use PHPUnit\Framework\TestCase;
66
use Symfony\Component\EventDispatcher\EventDispatcher;
77
use Symfony\Component\Workflow\Event\GuardEvent;
8+
use Symfony\Component\Workflow\Exception\NotEnabledTransitionException;
89
use Symfony\Component\Workflow\StateMachine;
910
use Symfony\Component\Workflow\TransitionBlocker;
1011

@@ -84,27 +85,52 @@ public function testBuildTransitionBlockerListReturnsExpectedReasonOnBranchMerge
8485
$subject = new Subject();
8586

8687
// There may be multiple transitions with the same name. Make sure that transitions
87-
// that are not enabled by the marking are evaluated.
88+
// that are enabled by the marking are evaluated.
8889
// see https://github.com/symfony/symfony/issues/28432
8990

90-
// Test if when you are in place "a"trying transition "t1" then returned
91+
// Test if when you are in place "a" and trying to apply "t1" then it returns
9192
// blocker list contains guard blocker instead blockedByMarking
9293
$subject->setMarking('a');
9394
$transitionBlockerList = $net->buildTransitionBlockerList($subject, 't1');
9495
$this->assertCount(1, $transitionBlockerList);
9596
$blockers = iterator_to_array($transitionBlockerList);
96-
9797
$this->assertSame('Transition blocker of place a', $blockers[0]->getMessage());
9898
$this->assertSame('blocker', $blockers[0]->getCode());
9999

100-
// Test if when you are in place "d" trying transition "t1" then
101-
// returned blocker list contains guard blocker instead blockedByMarking
100+
// Test if when you are in place "d" and trying to apply "t1" then
101+
// it returns blocker list contains guard blocker instead blockedByMarking
102102
$subject->setMarking('d');
103103
$transitionBlockerList = $net->buildTransitionBlockerList($subject, 't1');
104104
$this->assertCount(1, $transitionBlockerList);
105105
$blockers = iterator_to_array($transitionBlockerList);
106-
107106
$this->assertSame('Transition blocker of place d', $blockers[0]->getMessage());
108107
$this->assertSame('blocker', $blockers[0]->getCode());
109108
}
109+
110+
public function testApplyReturnsExpectedReasonOnBranchMerge()
111+
{
112+
$definition = $this->createComplexStateMachineDefinition();
113+
114+
$dispatcher = new EventDispatcher();
115+
$net = new StateMachine($definition, null, $dispatcher);
116+
117+
$dispatcher->addListener('workflow.guard', function (GuardEvent $event) {
118+
$event->addTransitionBlocker(new TransitionBlocker(sprintf('Transition blocker of place %s', $event->getTransition()->getFroms()[0]), 'blocker'));
119+
});
120+
121+
$subject = new Subject();
122+
123+
// There may be multiple transitions with the same name. Make sure that all transitions
124+
// that are enabled by the marking are evaluated.
125+
// see https://github.com/symfony/symfony/issues/34489
126+
127+
try {
128+
$net->apply($subject, 't1');
129+
$this->fail();
130+
} catch (NotEnabledTransitionException $e) {
131+
$blockers = iterator_to_array($e->getTransitionBlockerList());
132+
$this->assertSame('Transition blocker of place a', $blockers[0]->getMessage());
133+
$this->assertSame('blocker', $blockers[0]->getCode());
134+
}
135+
}
110136
}

‎Workflow.php

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -154,25 +154,47 @@ public function apply(object $subject, string $transitionName, array $context =
154154
{
155155
$marking = $this->getMarking($subject);
156156

157-
$transitionBlockerList = null;
158-
$applied = false;
159-
$approvedTransitionQueue = [];
157+
$transitionExist = false;
158+
$approvedTransitions = [];
159+
$bestTransitionBlockerList = null;
160160

161161
foreach ($this->definition->getTransitions() as $transition) {
162162
if ($transition->getName() !== $transitionName) {
163163
continue;
164164
}
165165

166-
$transitionBlockerList = $this->buildTransitionBlockerListForTransition($subject, $marking, $transition);
167-
if (!$transitionBlockerList->isEmpty()) {
166+
$transitionExist = true;
167+
168+
$tmpTransitionBlockerList = $this->buildTransitionBlockerListForTransition($subject, $marking, $transition);
169+
170+
if ($tmpTransitionBlockerList->isEmpty()) {
171+
$approvedTransitions[] = $transition;
172+
continue;
173+
}
174+
175+
if (!$bestTransitionBlockerList) {
176+
$bestTransitionBlockerList = $tmpTransitionBlockerList;
168177
continue;
169178
}
170-
$approvedTransitionQueue[] = $transition;
179+
180+
// We prefer to return transitions blocker by something else than
181+
// marking. Because it means the marking was OK. Transitions are
182+
// deterministic: it's not possible to have many transitions enabled
183+
// at the same time that match the same marking with the same name
184+
if (!$tmpTransitionBlockerList->has(TransitionBlocker::BLOCKED_BY_MARKING)) {
185+
$bestTransitionBlockerList = $tmpTransitionBlockerList;
186+
}
187+
}
188+
189+
if (!$transitionExist) {
190+
throw new UndefinedTransitionException($subject, $transitionName, $this);
171191
}
172192

173-
foreach ($approvedTransitionQueue as $transition) {
174-
$applied = true;
193+
if (!$approvedTransitions) {
194+
throw new NotEnabledTransitionException($subject, $transitionName, $this, $bestTransitionBlockerList);
195+
}
175196

197+
foreach ($approvedTransitions as $transition) {
176198
$this->leave($subject, $transition, $marking);
177199

178200
$context = $this->transition($subject, $transition, $marking, $context);
@@ -188,14 +210,6 @@ public function apply(object $subject, string $transitionName, array $context =
188210
$this->announce($subject, $transition, $marking);
189211
}
190212

191-
if (!$transitionBlockerList) {
192-
throw new UndefinedTransitionException($subject, $transitionName, $this);
193-
}
194-
195-
if (!$applied) {
196-
throw new NotEnabledTransitionException($subject, $transitionName, $this, $transitionBlockerList);
197-
}
198-
199213
return $marking;
200214
}
201215

0 commit comments

Comments
 (0)