diff --git a/src/Symfony/Component/Workflow/Tests/StateMachineTest.php b/src/Symfony/Component/Workflow/Tests/StateMachineTest.php index 5ece3c36d0..7f76c4a70b 100644 --- a/src/Symfony/Component/Workflow/Tests/StateMachineTest.php +++ b/src/Symfony/Component/Workflow/Tests/StateMachineTest.php @@ -3,7 +3,10 @@ namespace Symfony\Component\Workflow\Tests; use PHPUnit\Framework\TestCase; +use Symfony\Component\EventDispatcher\EventDispatcher; +use Symfony\Component\Workflow\Event\GuardEvent; use Symfony\Component\Workflow\StateMachine; +use Symfony\Component\Workflow\TransitionBlocker; class StateMachineTest extends TestCase { @@ -38,4 +41,70 @@ class StateMachineTest extends TestCase $this->assertTrue($net->can($subject, 't2')); $this->assertTrue($net->can($subject, 't3')); } + + public function testBuildTransitionBlockerList() + { + $definition = $this->createComplexStateMachineDefinition(); + + $net = new StateMachine($definition); + $subject = new \stdClass(); + + $subject->marking = 'a'; + $this->assertTrue($net->buildTransitionBlockerList($subject, 't1')->isEmpty()); + $subject->marking = 'd'; + $this->assertTrue($net->buildTransitionBlockerList($subject, 't1')->isEmpty()); + + $subject->marking = 'b'; + $this->assertFalse($net->buildTransitionBlockerList($subject, 't1')->isEmpty()); + } + + public function testBuildTransitionBlockerListWithMultipleTransitions() + { + $definition = $this->createComplexStateMachineDefinition(); + + $net = new StateMachine($definition); + $subject = new \stdClass(); + + $subject->marking = 'b'; + $this->assertTrue($net->buildTransitionBlockerList($subject, 't2')->isEmpty()); + $this->assertTrue($net->buildTransitionBlockerList($subject, 't3')->isEmpty()); + } + + public function testBuildTransitionBlockerListReturnsExpectedReasonOnBranchMerge() + { + $definition = $this->createComplexStateMachineDefinition(); + + $dispatcher = new EventDispatcher(); + $net = new StateMachine($definition, null, $dispatcher); + + $dispatcher->addListener('workflow.guard', function (GuardEvent $event) { + $event->addTransitionBlocker(new TransitionBlocker(\sprintf('Transition blocker of place %s', $event->getTransition()->getFroms()[0]), 'blocker')); + }); + + $subject = new \stdClass(); + + // There may be multiple transitions with the same name. Make sure that transitions + // that are not enabled by the marking are evaluated. + // see https://github.com/symfony/symfony/issues/28432 + + // Test if when you are in place "a"trying transition "t1" then returned + // blocker list contains guard blocker instead blockedByMarking + $subject->marking = 'a'; + $transitionBlockerList = $net->buildTransitionBlockerList($subject, 't1'); + $this->assertCount(1, $transitionBlockerList); + $blockers = iterator_to_array($transitionBlockerList); + + $this->assertSame('Transition blocker of place a', $blockers[0]->getMessage()); + $this->assertSame('blocker', $blockers[0]->getCode()); + + // Test if when you are in place "d" trying transition "t1" then + // returned blocker list contains guard blocker instead blockedByMarking + $subject->marking = 'd'; + $transitionBlockerList = $net->buildTransitionBlockerList($subject, 't1'); + $this->assertCount(1, $transitionBlockerList); + $blockers = iterator_to_array($transitionBlockerList); + + $this->assertSame('Transition blocker of place d', $blockers[0]->getMessage()); + $this->assertSame('blocker', $blockers[0]->getCode()); + } } diff --git a/src/Symfony/Component/Workflow/Tests/WorkflowTest.php b/src/Symfony/Component/Workflow/Tests/WorkflowTest.php index 87f4800248..75ceb1f108 100644 --- a/src/Symfony/Component/Workflow/Tests/WorkflowTest.php +++ b/src/Symfony/Component/Workflow/Tests/WorkflowTest.php @@ -195,6 +195,32 @@ class WorkflowTest extends TestCase $workflow->buildTransitionBlockerList($subject, '404 Not Found'); } + public function testBuildTransitionBlockerList() + { + $definition = $this->createComplexWorkflowDefinition(); + $subject = new \stdClass(); + $subject->marking = null; + $workflow = new Workflow($definition, new MultipleStateMarkingStore()); + + $this->assertTrue($workflow->buildTransitionBlockerList($subject, 't1')->isEmpty()); + $this->assertFalse($workflow->buildTransitionBlockerList($subject, 't2')->isEmpty()); + + $subject->marking = array('b' => 1); + + $this->assertFalse($workflow->buildTransitionBlockerList($subject, 't1')->isEmpty()); + $this->assertFalse($workflow->buildTransitionBlockerList($subject, 't2')->isEmpty()); + + $subject->marking = array('b' => 1, 'c' => 1); + + $this->assertFalse($workflow->buildTransitionBlockerList($subject, 't1')->isEmpty()); + $this->assertTrue($workflow->buildTransitionBlockerList($subject, 't2')->isEmpty()); + + $subject->marking = array('f' => 1); + + $this->assertFalse($workflow->buildTransitionBlockerList($subject, 't5')->isEmpty()); + $this->assertTrue($workflow->buildTransitionBlockerList($subject, 't6')->isEmpty()); + } + public function testBuildTransitionBlockerListReturnsReasonsProvidedByMarking() { $definition = $this->createComplexWorkflowDefinition(); diff --git a/src/Symfony/Component/Workflow/TransitionBlockerList.php b/src/Symfony/Component/Workflow/TransitionBlockerList.php index 34f9437cda..b7ef5a157c 100644 --- a/src/Symfony/Component/Workflow/TransitionBlockerList.php +++ b/src/Symfony/Component/Workflow/TransitionBlockerList.php @@ -37,6 +37,17 @@ final class TransitionBlockerList implements \IteratorAggregate, \Countable $this->blockers[] = $blocker; } + public function has(string $code): bool + { + foreach ($this->blockers as $blocker) { + if ($code === $blocker->getCode()) { + return true; + } + } + + return false; + } + public function clear(): void { $this->blockers = array(); diff --git a/src/Symfony/Component/Workflow/Workflow.php b/src/Symfony/Component/Workflow/Workflow.php index 21676e0fc8..11d82daa01 100644 --- a/src/Symfony/Component/Workflow/Workflow.php +++ b/src/Symfony/Component/Workflow/Workflow.php @@ -119,7 +119,15 @@ class Workflow implements WorkflowInterface $transitionBlockerList = $this->buildTransitionBlockerListForTransition($subject, $marking, $transition); if ($transitionBlockerList->isEmpty()) { - continue; + return $transitionBlockerList; + } + + // We prefer to return transitions blocker by something else than + // marking. Because it means the marking was OK. Transitions are + // deterministic: it's not possible to have many transitions enabled + // at the same time that match the same marking with the same name + if (!$transitionBlockerList->has(TransitionBlocker::BLOCKED_BY_MARKING)) { + return $transitionBlockerList; } }