symfony · Tetragramat · Sep 17, 2018 · Sep 17, 2018 · Oct 15, 2018 · Oct 19, 2018
diff --git 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,76 @@ public function testCanWithMultipleTransition()
        $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();
+
+        // If you are in place "a" you should be able to apply "t1"
+        $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 testBuildTransitionBlockerListWithMultipleTransition()
+    {
+        $definition = $this->createComplexStateMachineDefinition();
+
+        $net = new StateMachine($definition);
+        $subject = new \stdClass();
+
+        // If you are in place "b" you should be able to apply "t1" and "t2"
+        $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();
+
+        // When there are multiple transitions with the same name then method buildTransitionBlockerList might
+        // return result of different transition than expected.
+        // For example: buildTransitionBlockerList foreach two transitions "t1" where one starts in place "a" and second in place "d" and
+        // we are currently in place "a". Method buildTransitionBlockerList first processes transition "t1" with from place "a"
+        // but there is no break so it continues to transition "t1" with from place "d" where it exits loop because there are no more
+        // transitions with name "t1". So returned transition blocker list contains result of transition "t1" with from place "d" because
+        // it was executed lastest. This result is incorrect because we got "Blocked by marking" transition blocker of transition "t1"
+        // with from place "d" instead of guard blocker "Transition blocker of place a" of transition "t1" with from place "a".
+        // This test checks if this bug does not happen.
+
+        // 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
@@ -195,6 +195,34 @@ public function testBuildTransitionBlockerListReturnsUndefinedTransition()
         $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());
+        // In a workflow net, all "from" places should contain a token to enable
+        // the transition.
+        $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();

@@ -118,8 +118,15 @@ public function buildTransitionBlockerList($subject, string $transitionName): Tr
 
             $transitionBlockerList = $this->buildTransitionBlockerListForTransition($subject, $marking, $transition);
 
-            if ($transitionBlockerList->isEmpty()) {
-                continue;
+            // For case of non unique transition names exit the loop on transition that is supposed to be validated.
+            // Else we might get different blocker list containing blockedByMarking instead of blocker from the guard event.
+            // Reason is if you have transition "t" from "a" to "c" and transition "t" from "b" to "c" and you are in place "a"
+            // transitions loop might exit in transition with "from" place "b"
+            // which means you would get $transitionBlockerList containing blockedByMarking instead of guard blocker or none.
+            foreach ($transition->getFroms() as $place) {
+                if ($marking->has($place)) {
+                    break 2;
+                }
             }
         }