diff --git a/src/Symfony/Component/Workflow/Tests/StateMachineTest.php b/src/Symfony/Component/Workflow/Tests/StateMachineTest.php index 5ece3c36d0..da8156b8fb 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,68 @@ 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" applying 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" applying 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/Workflow.php b/src/Symfony/Component/Workflow/Workflow.php index 21676e0fc8..38dd961aba 100644 --- a/src/Symfony/Component/Workflow/Workflow.php +++ b/src/Symfony/Component/Workflow/Workflow.php @@ -92,7 +92,11 @@ class Workflow implements WorkflowInterface continue; } - $transitionBlockerList = $this->buildTransitionBlockerListForTransition($subject, $marking, $transition); + try { + $transitionBlockerList = $this->buildTransitionBlockerListForTransition($subject, $marking, $transition); + } catch (NotEnabledTransitionException $e) { + $transitionBlockerList = $e->getTransitionBlockerList(); + } if ($transitionBlockerList->isEmpty()) { return true; @@ -116,10 +120,13 @@ class Workflow implements WorkflowInterface continue; } - $transitionBlockerList = $this->buildTransitionBlockerListForTransition($subject, $marking, $transition); + try { + $transitionBlockerList = $this->buildTransitionBlockerListForTransition($subject, $marking, $transition); - if ($transitionBlockerList->isEmpty()) { - continue; + return $transitionBlockerList; + } catch (NotEnabledTransitionException $e) { + // a transition with the same name is defined for other places too + $transitionBlockerList = $e->getTransitionBlockerList(); } } @@ -146,11 +153,15 @@ class Workflow implements WorkflowInterface continue; } - $transitionBlockerList = $this->buildTransitionBlockerListForTransition($subject, $marking, $transition); - if (!$transitionBlockerList->isEmpty()) { - continue; + try { + $transitionBlockerList = $this->buildTransitionBlockerListForTransition($subject, $marking, $transition); + } catch (NotEnabledTransitionException $e) { + $transitionBlockerList = $e->getTransitionBlockerList(); + } + + if ($transitionBlockerList->isEmpty()) { + $approvedTransitionQueue[] = $transition; } - $approvedTransitionQueue[] = $transition; } foreach ($approvedTransitionQueue as $transition) { @@ -191,7 +202,12 @@ class Workflow implements WorkflowInterface $marking = $this->getMarking($subject); foreach ($this->definition->getTransitions() as $transition) { - $transitionBlockerList = $this->buildTransitionBlockerListForTransition($subject, $marking, $transition); + try { + $transitionBlockerList = $this->buildTransitionBlockerListForTransition($subject, $marking, $transition); + } catch (NotEnabledTransitionException $e) { + $transitionBlockerList = $e->getTransitionBlockerList(); + } + if ($transitionBlockerList->isEmpty()) { $enabledTransitions[] = $transition; } @@ -236,9 +252,7 @@ class Workflow implements WorkflowInterface { foreach ($transition->getFroms() as $place) { if (!$marking->has($place)) { - return new TransitionBlockerList(array( - TransitionBlocker::createBlockedByMarking($marking), - )); + throw new NotEnabledTransitionException($subject, $transition->getName(), $this, new TransitionBlockerList(array(TransitionBlocker::createBlockedByMarking($marking)))); } }