[Workflow] Cleaned the transition blocker implementations

src/Symfony/Component/Workflow/Event/GuardEvent.php

@@ -36,18 +36,18 @@ class GuardEvent extends Event
 
     public function isBlocked(): bool
     {
-        return 0 !== count($this->transitionBlockerList);
+        return !$this->transitionBlockerList->isEmpty();
     }
 
     public function setBlocked(bool $blocked): void
     {
         if (!$blocked) {
-            $this->transitionBlockerList = new TransitionBlockerList();
+            $this->transitionBlockerList->reset();
 
             return;
         }
 
-        $this->transitionBlockerList->add(TransitionBlocker::createUnknownReason($this->getTransition()->getName()));
+        $this->transitionBlockerList->add(TransitionBlocker::createUnknown());
     }
 
     public function getTransitionBlockerList(): TransitionBlockerList

src/Symfony/Component/Workflow/EventListener/GuardListener.php

@@ -18,6 +18,7 @@ use Symfony\Component\Security\Core\Role\RoleHierarchyInterface;
 use Symfony\Component\Validator\Validator\ValidatorInterface;
 use Symfony\Component\Workflow\Event\GuardEvent;
 use Symfony\Component\Workflow\Exception\InvalidTokenConfigurationException;
+use Symfony\Component\Workflow\TransitionBlocker;
 
 /**
  * @author Grégoire Pineau <lyrixx@lyrixx.info>
@@ -49,8 +50,11 @@ class GuardListener
             return;
         }
 
-        if (!$this->expressionLanguage->evaluate($this->configuration[$eventName], $this->getVariables($event))) {
+        $expression = $this->configuration[$eventName];
-            $event->setBlocked(true);
+
+        if (!$this->expressionLanguage->evaluate($expression, $this->getVariables($event))) {
+            $blocker = TransitionBlocker::createBlockedByExpressionGuardListener($expression);
+            $event->addTransitionBlocker($blocker);
         }
     }
 

src/Symfony/Component/Workflow/Exception/NotEnabledTransitionException.php

@@ -14,15 +14,17 @@ namespace Symfony\Component\Workflow\Exception;
 use Symfony\Component\Workflow\TransitionBlockerList;
 
 /**
- * Thrown by the workflow when a transition is not enabled.
+ * Thrown by Workflow when a not enabled transition is applied on a subject.
+ *
+ * @author Grégoire Pineau <lyrixx@lyrixx.info>
  */
-class BlockedTransitionException extends LogicException
+class NotEnabledTransitionException extends LogicException
 {
     private $transitionBlockerList;
 
-    public function __construct(string $message, TransitionBlockerList $transitionBlockerList)
+    public function __construct(string $transitionName, string $workflowName, TransitionBlockerList $transitionBlockerList)
     {
-        parent::__construct($message);
+        parent::__construct(sprintf('Transition "%s" is not enabled for workflow "%s".', $transitionName, $workflowName));
 
         $this->transitionBlockerList = $transitionBlockerList;
     }

src/Symfony/Component/Workflow/Exception/UndefinedTransitionException.php

@@ -13,7 +13,13 @@ namespace Symfony\Component\Workflow\Exception;
 
 /**
  * Thrown by Workflow when an undefined transition is applied on a subject.
+ *
+ * @author Grégoire Pineau <lyrixx@lyrixx.info>
  */
 class UndefinedTransitionException extends LogicException
 {
+    public function __construct(string $transitionName, string $workflowName)
+    {
+        parent::__construct(sprintf('Transition "%s" is not defined for workflow "%s".', $transitionName, $workflowName));
+    }
 }

src/Symfony/Component/Workflow/Tests/WorkflowTest.php

@@ -7,7 +7,7 @@ use Symfony\Component\EventDispatcher\EventDispatcher;
 use Symfony\Component\Workflow\Definition;
 use Symfony\Component\Workflow\Event\Event;
 use Symfony\Component\Workflow\Event\GuardEvent;
-use Symfony\Component\Workflow\Exception\BlockedTransitionException;
+use Symfony\Component\Workflow\Exception\NotEnabledTransitionException;
 use Symfony\Component\Workflow\Marking;
 use Symfony\Component\Workflow\MarkingStore\MarkingStoreInterface;
 use Symfony\Component\Workflow\MarkingStore\MultipleStateMarkingStore;
@@ -164,20 +164,6 @@ class WorkflowTest extends TestCase
         $this->assertSame(array('workflow_name.guard.t3'), $dispatchedEvents);
     }
 
-    /**
-     * @expectedException \Symfony\Component\Workflow\Exception\LogicException
-     * @expectedExceptionMessage Unable to apply transition "t2" for workflow "unnamed".
-     */
-    public function testApplyWithImpossibleTransition()
-    {
-        $definition = $this->createComplexWorkflowDefinition();
-        $subject = new \stdClass();
-        $subject->marking = null;
-        $workflow = new Workflow($definition, new MultipleStateMarkingStore());
-
-        $workflow->apply($subject, 't2');
-    }
-
     public function testCanWithSameNameTransition()
     {
         $definition = $this->createWorkflowWithSameNameTransition();
@@ -195,6 +181,99 @@ class WorkflowTest extends TestCase
         $this->assertTrue($workflow->can($subject, 'to_a'));
     }
 
+    /**
+     * @expectedException \Symfony\Component\Workflow\Exception\UndefinedTransitionException
+     * @expectedExceptionMessage Transition "404 Not Found" is not defined for workflow "unnamed".
+     */
+    public function testBuildTransitionBlockerListReturnsUndefinedTransition()
+    {
+        $definition = $this->createSimpleWorkflowDefinition();
+        $subject = new \stdClass();
+        $subject->marking = null;
+        $workflow = new Workflow($definition);
+
+        $workflow->buildTransitionBlockerList($subject, '404 Not Found');
+    }
+
+    public function testBuildTransitionBlockerListReturnsReasonsProvidedByMarking()
+    {
+        $definition = $this->createComplexWorkflowDefinition();
+        $subject = new \stdClass();
+        $subject->marking = null;
+        $workflow = new Workflow($definition, new MultipleStateMarkingStore());
+
+        $transitionBlockerList = $workflow->buildTransitionBlockerList($subject, 't2');
+        $this->assertCount(1, $transitionBlockerList);
+        $blockers = iterator_to_array($transitionBlockerList);
+        $this->assertSame('The marking does not enable the transition.', $blockers[0]->getMessage());
+        $this->assertSame('19beefc8-6b1e-4716-9d07-a39bd6d16e34', $blockers[0]->getCode());
+    }
+
+    public function testBuildTransitionBlockerListReturnsReasonsProvidedInGuards()
+    {
+        $definition = $this->createSimpleWorkflowDefinition();
+        $subject = new \stdClass();
+        $subject->marking = null;
+        $dispatcher = new EventDispatcher();
+        $workflow = new Workflow($definition, new MultipleStateMarkingStore(), $dispatcher);
+
+        $dispatcher->addListener('workflow.guard', function (GuardEvent $event) {
+            $event->addTransitionBlocker(new TransitionBlocker('Transition blocker 1', 'blocker_1'));
+            $event->addTransitionBlocker(new TransitionBlocker('Transition blocker 2', 'blocker_2'));
+        });
+        $dispatcher->addListener('workflow.guard', function (GuardEvent $event) {
+            $event->addTransitionBlocker(new TransitionBlocker('Transition blocker 3', 'blocker_3'));
+        });
+        $dispatcher->addListener('workflow.guard', function (GuardEvent $event) {
+            $event->setBlocked(true);
+        });
+
+        $transitionBlockerList = $workflow->buildTransitionBlockerList($subject, 't1');
+        $this->assertCount(4, $transitionBlockerList);
+        $blockers = iterator_to_array($transitionBlockerList);
+        $this->assertSame('Transition blocker 1', $blockers[0]->getMessage());
+        $this->assertSame('blocker_1', $blockers[0]->getCode());
+        $this->assertSame('Transition blocker 2', $blockers[1]->getMessage());
+        $this->assertSame('blocker_2', $blockers[1]->getCode());
+        $this->assertSame('Transition blocker 3', $blockers[2]->getMessage());
+        $this->assertSame('blocker_3', $blockers[2]->getCode());
+        $this->assertSame('Unknown reason.', $blockers[3]->getMessage());
+        $this->assertSame('e8b5bbb9-5913-4b98-bfa6-65dbd228a82a', $blockers[3]->getCode());
+    }
+
+    /**
+     * @expectedException \Symfony\Component\Workflow\Exception\UndefinedTransitionException
+     * @expectedExceptionMessage Transition "404 Not Found" is not defined for workflow "unnamed".
+     */
+    public function testApplyWithNotExisingTransition()
+    {
+        $definition = $this->createComplexWorkflowDefinition();
+        $subject = new \stdClass();
+        $subject->marking = null;
+        $workflow = new Workflow($definition, new MultipleStateMarkingStore());
+
+        $workflow->apply($subject, '404 Not Found');
+    }
+
+    public function testApplyWithNotEnabledTransition()
+    {
+        $definition = $this->createComplexWorkflowDefinition();
+        $subject = new \stdClass();
+        $subject->marking = null;
+        $workflow = new Workflow($definition, new MultipleStateMarkingStore());
+
+        try {
+            $workflow->apply($subject, 't2');
+
+            $this->fail('Should throw an exception');
+        } catch (NotEnabledTransitionException $e) {
+            $this->assertSame('Transition "t2" is not enabled for workflow "unnamed".', $e->getMessage());
+            $this->assertCount(1, $e->getTransitionBlockerList());
+            $list = iterator_to_array($e->getTransitionBlockerList());
+            $this->assertSame('The marking does not enable the transition.', $list[0]->getMessage());
+        }
+    }
+
     public function testApply()
     {
         $definition = $this->createComplexWorkflowDefinition();
@@ -413,116 +492,6 @@ class WorkflowTest extends TestCase
         $this->assertSame('to_a', $transitions[1]->getName());
         $this->assertSame('to_a', $transitions[2]->getName());
     }
-
-    public function testWhyCannotReturnsReasonsProvidedInGuards()
-    {
-        $definition = $this->createSimpleWorkflowDefinition();
-        $subject = new \stdClass();
-        $subject->marking = null;
-        $dispatcher = new EventDispatcher();
-        $workflow = new Workflow($definition, new MultipleStateMarkingStore(), $dispatcher);
-
-        $guardsAddingTransitionBlockers = array(
-            function (GuardEvent $event) {
-                $event->addTransitionBlocker(new TransitionBlocker('Transition blocker 1', 'blocker_1'));
-                $event->addTransitionBlocker(new TransitionBlocker('Transition blocker 2', 'blocker_2'));
-            },
-            function (GuardEvent $event) {
-                $event->addTransitionBlocker(new TransitionBlocker('Transition blocker 3', 'blocker_3'));
-            },
-        );
-
-        foreach ($guardsAddingTransitionBlockers as $guard) {
-            $dispatcher->addListener('workflow.guard', $guard);
-        }
-
-        $transitionBlockerList = $workflow->buildTransitionBlockerList($subject, 't1');
-
-        $this->assertCount(3, $transitionBlockerList);
-
-        $assertTransitionBlockerPresentByCodeFn = function (string $code) use ($transitionBlockerList) {
-            $this->assertNotNull(
-                $transitionBlockerList->findByCode($code),
-                sprintf('Workflow did not produce transition blocker with code "%s"', $code)
-            );
-        };
-
-        $assertTransitionBlockerPresentByCodeFn('blocker_1');
-        $assertTransitionBlockerPresentByCodeFn('blocker_2');
-        $assertTransitionBlockerPresentByCodeFn('blocker_3');
-    }
-
-    public function testWhyCannotReturnsTransitionNotDefinedReason()
-    {
-        $definition = $this->createSimpleWorkflowDefinition();
-        $subject = new \stdClass();
-        $subject->marking = null;
-        $workflow = new Workflow($definition);
-
-        $transitionBlockerList = $workflow->buildTransitionBlockerList($subject, 'undefined_transition_name');
-
-        $this->assertCount(1, $transitionBlockerList);
-        $this->assertEquals(
-            TransitionBlocker::REASON_TRANSITION_NOT_DEFINED,
-            $transitionBlockerList->get(0)->getCode()
-        );
-    }
-
-    public function testWhyCannotReturnsTransitionNotApplicableReason()
-    {
-        $definition = $this->createSimpleWorkflowDefinition();
-        $subject = new \stdClass();
-        $subject->marking = null;
-        $workflow = new Workflow($definition);
-
-        $transitionBlockerList = $workflow->buildTransitionBlockerList($subject, 't2');
-
-        $this->assertCount(1, $transitionBlockerList);
-        $this->assertEquals(
-            TransitionBlocker::REASON_TRANSITION_NOT_APPLICABLE,
-            $transitionBlockerList->get(0)->getCode()
-        );
-    }
-
-    public function testApplyConveysTheTransitionBlockers()
-    {
-        $definition = $this->createSimpleWorkflowDefinition();
-        $subject = new \stdClass();
-        $subject->marking = null;
-        $dispatcher = new EventDispatcher();
-        $workflow = new Workflow($definition, new MultipleStateMarkingStore(), $dispatcher);
-
-        $dispatcher->addListener('workflow.guard', function (GuardEvent $event) {
-            $event->addTransitionBlocker(new TransitionBlocker('Transition blocker 3', 'blocker_1'));
-        });
-
-        try {
-            $workflow->apply($subject, 't1');
-        } catch (BlockedTransitionException $exception) {
-            $this->assertNotNull(
-                $exception->getTransitionBlockerList()->findByCode('blocker_1'),
-                'Workflow failed to convey it could not transition subject because of expected blocker'
-            );
-
-            return;
-        }
-
-        $this->fail('Workflow failed to prevent a transition from happening');
-    }
-
-    /**
-     * @expectedException \Symfony\Component\Workflow\Exception\UndefinedTransitionException
-     * @expectedExceptionMessage Transition "undefined_transition" is not defined in workflow "unnamed".
-     */
-    public function testApplyWithUndefinedTransition()
-    {
-        $definition = $this->createSimpleWorkflowDefinition();
-        $subject = new \stdClass();
-        $subject->marking = null;
-        $workflow = new Workflow($definition);
-
-        $workflow->apply($subject, 'undefined_transition');
-    }
 }
 
 class EventDispatcherMock implements \Symfony\Component\EventDispatcher\EventDispatcherInterface

src/Symfony/Component/Workflow/TransitionBlocker.php

@@ -14,22 +14,22 @@ namespace Symfony\Component\Workflow;
 /**
  * A reason why a transition cannot be performed for a subject.
  */
-class TransitionBlocker
+final class TransitionBlocker
 {
-    const REASON_TRANSITION_NOT_DEFINED = '80f2a8e9-ee53-408a-9dd8-cce09e031db8';
+    const BLOCKED_BY_MARKING = '19beefc8-6b1e-4716-9d07-a39bd6d16e34';
-    const REASON_TRANSITION_NOT_APPLICABLE = '19beefc8-6b1e-4716-9d07-a39bd6d16e34';
+    const BLOCKED_BY_EXPRESSION_GUARD_LISTENER = '326a1e9c-0c12-11e8-ba89-0ed5f89f718b';
-    const REASON_TRANSITION_UNKNOWN = 'e8b5bbb9-5913-4b98-bfa6-65dbd228a82a';
+    const UNKNOWN = 'e8b5bbb9-5913-4b98-bfa6-65dbd228a82a';
 
     private $message;
     private $code;
-
-    /**
-     * @var array This is useful if you would like to pass around the condition values, that
-     *            blocked the transition. E.g. for a condition "distance must be larger than
-     *            5 miles", you might want to pass around the value of 5.
-     */
     private $parameters;
 
+    /**
+     * @param string $code       Code is a machine-readable string, usually an UUID
+     * @param array  $parameters This is useful if you would like to pass around the condition values, that
+     *                           blocked the transition. E.g. for a condition "distance must be larger than
+     *                           5 miles", you might want to pass around the value of 5.
+     */
     public function __construct(string $message, string $code, array $parameters = array())
     {
         $this->message = $message;
@@ -38,61 +38,40 @@ class TransitionBlocker
     }
 
     /**
-     * Create a blocker, that says the transition cannot be made because it is undefined
+     * Create a blocker that says the transition cannot be made because it is
-     * in a workflow.
+     * not enabled.
      *
-     * @param string $transitionName
+     * It means the subject is in wrong place (i.e. status):
-     * @param string $workflowName
+     * * If the workflow is a state machine: the subject is not in the previous place of the transition.
-     *
+     * * If the workflow is a workflow: the subject is not in all previous places of the transition.
-     * @return static
      */
-    public static function createNotDefined(string $transitionName, string $workflowName): self
+    public static function createBlockedByMarking(Marking $marking): self
     {
-        $message = sprintf('Transition "%s" is not defined in workflow "%s".', $transitionName, $workflowName);
+        return new static('The marking does not enable the transition.', self::BLOCKED_BY_MARKING, array(
-        $parameters = array(
+            'marking' => $marking,
-            'transitionName' => $transitionName,
+        ));
-            'workflowName' => $workflowName,
-        );
-
-        return new static($message, self::REASON_TRANSITION_NOT_DEFINED, $parameters);
     }
 
     /**
-     * Create a blocker, that says the transition cannot be made because the subject
+     * Creates a blocker that says the transition cannot be made because it has
-     * is in wrong place (i.e. status).
+     * been blocked by the expression guard listener.
-     *
-     * @param string $transitionName
-     *
-     * @return static
      */
-    public static function createNotApplicable(string $transitionName): self
+    public static function createBlockedByExpressionGuardListener(string $expression): self
     {
-        $message = sprintf('Transition "%s" cannot be made, because the subject is not in the required place.', $transitionName);
+        return new static('The expression blocks the transition.', self::BLOCKED_BY_EXPRESSION_GUARD_LISTENER, array(
-        $parameters = array(
+            'expression' => $expression,
-            'transitionName' => $transitionName,
+        ));
-        );
-
-        return new static($message, self::REASON_TRANSITION_NOT_APPLICABLE, $parameters);
     }
 
     /**
-     * Create a blocker, that says the transition cannot be made because of unknown
+     * Creates a blocker that says the transition cannot be made because of an
-     * reason.
+     * unknown reason.
      *
      * This blocker code is chiefly for preserving backwards compatibility.
-     *
-     * @param string $transitionName
-     *
-     * @return static
      */
-    public static function createUnknownReason(string $transitionName): self
+    public static function createUnknown(): self
     {
-        $message = sprintf('Transition "%s" cannot be made, because of unknown reason.', $transitionName);
+        return new static('Unknown reason.', self::UNKNOWN);
-        $parameters = array(
-            'transitionName' => $transitionName,
-        );
-
-        return new static($message, self::REASON_TRANSITION_UNKNOWN, $parameters);
     }
 
     public function getMessage(): string

src/Symfony/Component/Workflow/TransitionBlockerList.php

@@ -13,14 +13,20 @@ namespace Symfony\Component\Workflow;
 
 /**
  * A list of transition blockers.
+ *
+ * @author Grégoire Pineau <lyrixx@lyrixx.info>
  */
-class TransitionBlockerList implements \IteratorAggregate, \Countable
+final class TransitionBlockerList implements \IteratorAggregate, \Countable
 {
-    /** @var TransitionBlocker[] */
+    private $blockers;
-    private $blockers = array();
 
+    /**
+     * @param TransitionBlocker[] $blockers
+     */
     public function __construct(array $blockers = array())
     {
+        $this->blockers = array();
+
         foreach ($blockers as $blocker) {
             $this->add($blocker);
         }
@@ -31,18 +37,14 @@ class TransitionBlockerList implements \IteratorAggregate, \Countable
         $this->blockers[] = $blocker;
     }
 
-    public function get(int $offset): TransitionBlocker
+    public function reset(): void
     {
-        if (!isset($this->blockers[$offset])) {
+        $this->blockers = array();
-            throw new \OutOfBoundsException(sprintf('The offset "%s" does not exist.', $offset));
-        }
-
-        return $this->blockers[$offset];
     }
 
-    public function has(int $offset): bool
+    public function isEmpty(): bool
     {
-        return isset($this->blockers[$offset]);
+        return !$this->blockers;
     }
 
     /**
@@ -57,17 +59,6 @@ class TransitionBlockerList implements \IteratorAggregate, \Countable
 
     public function count(): int
     {
-        return count($this->blockers);
+        return \count($this->blockers);
-    }
-
-    public function findByCode(string $code): ?TransitionBlocker
-    {
-        foreach ($this as $transitionBlocker) {
-            if ($transitionBlocker->getCode() === $code) {
-                return $transitionBlocker;
-            }
-        }
-
-        return null;
     }
 }

src/Symfony/Component/Workflow/Workflow.php

@@ -15,7 +15,7 @@ use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Symfony\Component\Workflow\Event\Event;
 use Symfony\Component\Workflow\Event\GuardEvent;
 use Symfony\Component\Workflow\Exception\LogicException;
-use Symfony\Component\Workflow\Exception\BlockedTransitionException;
+use Symfony\Component\Workflow\Exception\NotEnabledTransitionException;
 use Symfony\Component\Workflow\Exception\UndefinedTransitionException;
 use Symfony\Component\Workflow\MarkingStore\MarkingStoreInterface;
 use Symfony\Component\Workflow\MarkingStore\MultipleStateMarkingStore;
@@ -83,7 +83,22 @@ class Workflow implements WorkflowInterface
      */
     public function can($subject, $transitionName)
     {
-        return 0 === count($this->buildTransitionBlockerList($subject, $transitionName));
+        $transitions = $this->definition->getTransitions();
+        $marking = $this->getMarking($subject);
+
+        foreach ($transitions as $transition) {
+            if ($transition->getName() !== $transitionName) {
+                continue;
+            }
+
+            $transitionBlockerList = $this->buildTransitionBlockerListForTransition($subject, $marking, $transition);
+
+            if ($transitionBlockerList->isEmpty()) {
+                return true;
+            }
+        }
+
+        return false;
     }
 
     /**
@@ -91,49 +106,51 @@ class Workflow implements WorkflowInterface
      */
     public function buildTransitionBlockerList($subject, string $transitionName): TransitionBlockerList
     {
-        $blockerList = $this->getEnabledTransitionsByNameOrTransitionBlockerList($subject, $transitionName);
+        $transitions = $this->definition->getTransitions();
+        $marking = $this->getMarking($subject);
+        $transitionBlockerList = null;
 
-        // It means there are no blockers, so we return an empty list
+        foreach ($transitions as $transition) {
-        if (!$blockerList instanceof TransitionBlockerList) {
+            if ($transition->getName() !== $transitionName) {
-            return new TransitionBlockerList();
+                continue;
+            }
+
+            $transitionBlockerList = $this->buildTransitionBlockerListForTransition($subject, $marking, $transition);
+
+            if ($transitionBlockerList->isEmpty()) {
+                continue;
+            }
         }
 
-        return $blockerList;
+        if (!$transitionBlockerList) {
+            throw new UndefinedTransitionException($transitionName, $this->name);
+        }
+
+        return $transitionBlockerList;
     }
 
     /**
      * {@inheritdoc}
      */
-    public function apply($subject, string $transitionName): Marking
+    public function apply($subject, $transitionName)
     {
-        $transitionsOrTransitionBlockerList = $this->getEnabledTransitionsByNameOrTransitionBlockerList(
+        $marking = $this->getMarking($subject);
-            $subject,
-            $transitionName
-        );
 
-        if ($transitionsOrTransitionBlockerList instanceof TransitionBlockerList) {
+        $transitionBlockerList = null;
-            $transitionBlockerList = $transitionsOrTransitionBlockerList;
+        $applied = false;
 
-            if ($transitionBlockerList->findByCode(TransitionBlocker::REASON_TRANSITION_NOT_DEFINED)) {
+        foreach ($this->definition->getTransitions() as $transition) {
-                throw new UndefinedTransitionException(
+            if ($transition->getName() !== $transitionName) {
-                    sprintf('Transition "%s" is not defined in workflow "%s".', $transitionName, $this->name)
+                continue;
-                );
             }
 
-            throw new BlockedTransitionException(
+            $transitionBlockerList = $this->buildTransitionBlockerListForTransition($subject, $marking, $transition);
-                sprintf('Unable to apply transition "%s" for workflow "%s".', $transitionName, $this->name),
+            if (!$transitionBlockerList->isEmpty()) {
-                $transitionBlockerList
+                continue;
-            );
+            }
-        }
 
-        $transitions = $transitionsOrTransitionBlockerList;
+            $applied = true;
 
-        // We can shortcut the getMarking method in order to boost performance,
-        // since the "getEnabledTransitions" method already checks the Marking
-        // state
-        $marking = $this->markingStore->getMarking($subject);
-
-        foreach ($transitions as $transition) {
             $this->leave($subject, $transition, $marking);
 
             $this->transition($subject, $transition, $marking);
@@ -149,6 +166,14 @@ class Workflow implements WorkflowInterface
             $this->announce($subject, $transition, $marking);
         }
 
+        if (!$transitionBlockerList) {
+            throw new UndefinedTransitionException($transitionName, $this->name);
+        }
+
+        if (!$applied) {
+            throw new NotEnabledTransitionException($transitionName, $this->name, $transitionBlockerList);
+        }
+
         return $marking;
     }
 
@@ -157,16 +182,17 @@ class Workflow implements WorkflowInterface
      */
     public function getEnabledTransitions($subject)
     {
-        $enabled = array();
+        $enabledTransitions = array();
         $marking = $this->getMarking($subject);
 
         foreach ($this->definition->getTransitions() as $transition) {
-            if (0 === count($this->doCan($subject, $marking, $transition))) {
+            $transitionBlockerList = $this->buildTransitionBlockerListForTransition($subject, $marking, $transition);
-                $enabled[] = $transition;
+            if ($transitionBlockerList->isEmpty()) {
+                $enabledTransitions[] = $transition;
             }
         }
 
-        return $enabled;
+        return $enabledTransitions;
     }
 
     /**
@@ -193,33 +219,45 @@ class Workflow implements WorkflowInterface
         return $this->markingStore;
     }
 
-    private function doCan($subject, Marking $marking, Transition $transition): TransitionBlockerList
+    private function buildTransitionBlockerListForTransition($subject, Marking $marking, Transition $transition)
     {
         foreach ($transition->getFroms() as $place) {
             if (!$marking->has($place)) {
-                return new TransitionBlockerList(array(TransitionBlocker::createNotApplicable($transition->getName())));
+                return new TransitionBlockerList(array(
+                    TransitionBlocker::createBlockedByMarking($marking),
+                ));
             }
         }
 
-        return $this->guardTransition($subject, $marking, $transition);
-    }
-
-    private function guardTransition($subject, Marking $marking, Transition $transition): TransitionBlockerList
-    {
         if (null === $this->dispatcher) {
             return new TransitionBlockerList();
         }
 
+        $event = $this->guardTransition($subject, $marking, $transition);
+
+        if ($event->isBlocked()) {
+            return $event->getTransitionBlockerList();
+        }
+
+        return new TransitionBlockerList();
+    }
+
+    private function guardTransition($subject, Marking $marking, Transition $transition): ?GuardEvent
+    {
+        if (null === $this->dispatcher) {
+            return null;
+        }
+
         $event = new GuardEvent($subject, $marking, $transition, $this->name);
 
         $this->dispatcher->dispatch('workflow.guard', $event);
         $this->dispatcher->dispatch(sprintf('workflow.%s.guard', $this->name), $event);
         $this->dispatcher->dispatch(sprintf('workflow.%s.guard.%s', $this->name, $transition->getName()), $event);
 
-        return $event->getTransitionBlockerList();
+        return $event;
     }
 
-    private function leave($subject, Transition $transition, Marking $marking)
+    private function leave($subject, Transition $transition, Marking $marking): void
     {
         $places = $transition->getFroms();
 
@@ -239,7 +277,7 @@ class Workflow implements WorkflowInterface
         }
     }
 
-    private function transition($subject, Transition $transition, Marking $marking)
+    private function transition($subject, Transition $transition, Marking $marking): void
     {
         if (null === $this->dispatcher) {
             return;
@@ -252,7 +290,7 @@ class Workflow implements WorkflowInterface
         $this->dispatcher->dispatch(sprintf('workflow.%s.transition.%s', $this->name, $transition->getName()), $event);
     }
 
-    private function enter($subject, Transition $transition, Marking $marking)
+    private function enter($subject, Transition $transition, Marking $marking): void
     {
         $places = $transition->getTos();
 
@@ -272,7 +310,7 @@ class Workflow implements WorkflowInterface
         }
     }
 
-    private function entered($subject, Transition $transition, Marking $marking)
+    private function entered($subject, Transition $transition, Marking $marking): void
     {
         if (null === $this->dispatcher) {
             return;
@@ -288,7 +326,7 @@ class Workflow implements WorkflowInterface
         }
     }
 
-    private function completed($subject, Transition $transition, Marking $marking)
+    private function completed($subject, Transition $transition, Marking $marking): void
     {
         if (null === $this->dispatcher) {
             return;
@@ -301,7 +339,7 @@ class Workflow implements WorkflowInterface
         $this->dispatcher->dispatch(sprintf('workflow.%s.completed.%s', $this->name, $transition->getName()), $event);
     }
 
-    private function announce($subject, Transition $initialTransition, Marking $marking)
+    private function announce($subject, Transition $initialTransition, Marking $marking): void
     {
         if (null === $this->dispatcher) {
             return;
@@ -316,75 +354,4 @@ class Workflow implements WorkflowInterface
             $this->dispatcher->dispatch(sprintf('workflow.%s.announce.%s', $this->name, $transition->getName()), $event);
         }
     }
-
-    /**
-     * @param string $transitionName
-     *
-     * @return Transition[]
-     */
-    private function getTransitionsByName(string $transitionName): array
-    {
-        return array_filter(
-            $this->definition->getTransitions(),
-            function (Transition $transition) use ($transitionName) {
-                return $transition->getName() === $transitionName;
-            }
-        );
-    }
-
-    /**
-     * Returns all enabled transitions or the most specific transition blocker list.
-     *
-     * @param object $subject        A subject
-     * @param string $transitionName
-     *
-     * @return Transition[]|TransitionBlockerList All enabled transitions or a blocker list
-     *                                            if no enabled transitions can be found
-     */
-    private function getEnabledTransitionsByNameOrTransitionBlockerList($subject, string $transitionName)
-    {
-        $eligibleTransitions = $this->getTransitionsByName($transitionName);
-
-        if (!$eligibleTransitions) {
-            return new TransitionBlockerList(array(TransitionBlocker::createNotDefined($transitionName, $this->name)));
-        }
-
-        $marking = $this->getMarking($subject);
-        $transitions = array();
-
-        /** @var TransitionBlockerList[] $transitionBlockerLists */
-        $transitionBlockerLists = array();
-
-        foreach ($eligibleTransitions as $transition) {
-            $transitionBlockerLists[]
-                = $transitionBlockerList
-                = $this->doCan($subject, $marking, $transition);
-
-            if (0 === count($transitionBlockerList)) {
-                $transitions[] = $transition;
-            }
-        }
-
-        if ($transitions) {
-            return $transitions;
-        }
-
-        // Try to find the most specific blocker list, by ignoring the ones
-        // that say, that the transition is impossible, because it's not applicable.
-        // If such a list is not found, then apparently the transition is really
-        // not applicable. All this makes more sense when there are many transitions
-        // with the same name. In case every transition has a unique name, only one
-        // blocker list is retrieved, which is exactly the behaviour we're trying
-        // to reproduce in the case when there are many transitions with the same
-        // name.
-        //
-        // Also, at this point we are guaranteed to have at least 1 blocker list.
-        foreach ($transitionBlockerLists as $transitionBlockerList) {
-            if (!$transitionBlockerList->findByCode(TransitionBlocker::REASON_TRANSITION_NOT_APPLICABLE)) {
-                return $transitionBlockerList;
-            }
-        }
-
-        return $transitionBlockerLists[0];
-    }
 }

src/Symfony/Component/Workflow/WorkflowInterface.php

@@ -11,9 +11,7 @@
 
 namespace Symfony\Component\Workflow;
 
-use Symfony\Component\Workflow\Exception\BlockedTransitionException;
 use Symfony\Component\Workflow\Exception\LogicException;
-use Symfony\Component\Workflow\Exception\UndefinedTransitionException;
 use Symfony\Component\Workflow\MarkingStore\MarkingStoreInterface;
 
 /**
@@ -39,18 +37,13 @@ interface WorkflowInterface
      * @param string $transitionName A transition
      *
      * @return bool true if the transition is enabled
-     *
-     * @throws LogicException
      */
     public function can($subject, $transitionName);
 
     /**
-     * Returns transition blockers explaining why a transition cannot be made.
+     * Builds a TransitionBlockerList to know why a transition is blocked.
      *
-     * @param object $subject        A subject
+     * @param object $subject A subject
-     * @param string $transitionName A transition
-     *
-     * @return TransitionBlockerList Empty if the transition is possible
      */
     public function buildTransitionBlockerList($subject, string $transitionName): TransitionBlockerList;
 
@@ -62,10 +55,9 @@ interface WorkflowInterface
      *
      * @return Marking The new Marking
      *
-     * @throws BlockedTransitionException   If the transition is not applicable
+     * @throws LogicException If the transition is not applicable
-     * @throws UndefinedTransitionException If the transition does not exist
      */
-    public function apply($subject, string $transitionName);
+    public function apply($subject, $transitionName);
 
     /**
      * Returns all enabled transitions.