diff --git a/src/Symfony/Component/Workflow/CHANGELOG.md b/src/Symfony/Component/Workflow/CHANGELOG.md index 1bdca90c9b..79996f4626 100644 --- a/src/Symfony/Component/Workflow/CHANGELOG.md +++ b/src/Symfony/Component/Workflow/CHANGELOG.md @@ -4,10 +4,12 @@ CHANGELOG 4.1.0 ----- - * Deprecate the usage of `add(Workflow $workflow, $supportStrategy)` in `Workflow/Registry`, use `addWorkflow(WorkflowInterface, $supportStrategy)` instead. + * Deprecate the usage of `add(Workflow $workflow, $supportStrategy)` in `Workflow/Registry`, use `addWorkflow(WorkflowInterface, $supportStrategy)` instead. * Deprecate the usage of `SupportStrategyInterface`, use `WorkflowSupportStrategyInterface` instead. * The `Workflow` class now implements `WorkflowInterface`. * Deprecated the class `ClassInstanceSupportStrategy` in favor of the class `InstanceOfSupportStrategy`. + * Added TransitionBlockers as a way to pass around reasons why exactly + transitions can't be made. 4.0.0 ----- diff --git a/src/Symfony/Component/Workflow/Event/GuardEvent.php b/src/Symfony/Component/Workflow/Event/GuardEvent.php index bf4b6f3971..1ccdd91088 100644 --- a/src/Symfony/Component/Workflow/Event/GuardEvent.php +++ b/src/Symfony/Component/Workflow/Event/GuardEvent.php @@ -11,21 +11,52 @@ namespace Symfony\Component\Workflow\Event; +use Symfony\Component\Workflow\Marking; +use Symfony\Component\Workflow\Transition; +use Symfony\Component\Workflow\TransitionBlocker; +use Symfony\Component\Workflow\TransitionBlockerList; + /** * @author Fabien Potencier * @author Grégoire Pineau */ class GuardEvent extends Event { - private $blocked = false; + private $transitionBlockerList; - public function isBlocked() + /** + * {@inheritdoc} + */ + public function __construct($subject, Marking $marking, Transition $transition, $workflowName = 'unnamed') { - return $this->blocked; + parent::__construct($subject, $marking, $transition, $workflowName); + + $this->transitionBlockerList = new TransitionBlockerList(); } - public function setBlocked($blocked) + public function isBlocked(): bool { - $this->blocked = (bool) $blocked; + return 0 !== count($this->transitionBlockerList); + } + + public function setBlocked(bool $blocked): void + { + if (!$blocked) { + $this->transitionBlockerList = new TransitionBlockerList(); + + return; + } + + $this->transitionBlockerList->add(TransitionBlocker::createUnknownReason($this->getTransition()->getName())); + } + + public function getTransitionBlockerList(): TransitionBlockerList + { + return $this->transitionBlockerList; + } + + public function addTransitionBlocker(TransitionBlocker $transitionBlocker): void + { + $this->transitionBlockerList->add($transitionBlocker); } } diff --git a/src/Symfony/Component/Workflow/Exception/BlockedTransitionException.php b/src/Symfony/Component/Workflow/Exception/BlockedTransitionException.php new file mode 100644 index 0000000000..51af064f9f --- /dev/null +++ b/src/Symfony/Component/Workflow/Exception/BlockedTransitionException.php @@ -0,0 +1,34 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Workflow\Exception; + +use Symfony\Component\Workflow\TransitionBlockerList; + +/** + * Thrown by the workflow when a transition is not enabled. + */ +class BlockedTransitionException extends LogicException +{ + private $transitionBlockerList; + + public function __construct(string $message, TransitionBlockerList $transitionBlockerList) + { + parent::__construct($message); + + $this->transitionBlockerList = $transitionBlockerList; + } + + public function getTransitionBlockerList(): TransitionBlockerList + { + return $this->transitionBlockerList; + } +} diff --git a/src/Symfony/Component/Workflow/Exception/UndefinedTransitionException.php b/src/Symfony/Component/Workflow/Exception/UndefinedTransitionException.php new file mode 100644 index 0000000000..f3246c4681 --- /dev/null +++ b/src/Symfony/Component/Workflow/Exception/UndefinedTransitionException.php @@ -0,0 +1,19 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Workflow\Exception; + +/** + * Thrown by Workflow when an undefined transition is applied on a subject. + */ +class UndefinedTransitionException extends LogicException +{ +} diff --git a/src/Symfony/Component/Workflow/Tests/WorkflowTest.php b/src/Symfony/Component/Workflow/Tests/WorkflowTest.php index 547fb03e82..d97d81f615 100644 --- a/src/Symfony/Component/Workflow/Tests/WorkflowTest.php +++ b/src/Symfony/Component/Workflow/Tests/WorkflowTest.php @@ -7,10 +7,12 @@ 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\Marking; use Symfony\Component\Workflow\MarkingStore\MarkingStoreInterface; use Symfony\Component\Workflow\MarkingStore\MultipleStateMarkingStore; use Symfony\Component\Workflow\Transition; +use Symfony\Component\Workflow\TransitionBlocker; use Symfony\Component\Workflow\Workflow; class WorkflowTest extends TestCase @@ -411,6 +413,116 @@ 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 diff --git a/src/Symfony/Component/Workflow/TransitionBlocker.php b/src/Symfony/Component/Workflow/TransitionBlocker.php new file mode 100644 index 0000000000..fe5c6f5099 --- /dev/null +++ b/src/Symfony/Component/Workflow/TransitionBlocker.php @@ -0,0 +1,112 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Workflow; + +/** + * A reason why a transition cannot be performed for a subject. + */ +class TransitionBlocker +{ + const REASON_TRANSITION_NOT_DEFINED = '80f2a8e9-ee53-408a-9dd8-cce09e031db8'; + const REASON_TRANSITION_NOT_APPLICABLE = '19beefc8-6b1e-4716-9d07-a39bd6d16e34'; + const REASON_TRANSITION_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; + + public function __construct(string $message, string $code, array $parameters = array()) + { + $this->message = $message; + $this->code = $code; + $this->parameters = $parameters; + } + + /** + * Create a blocker, that says the transition cannot be made because it is undefined + * in a workflow. + * + * @param string $transitionName + * @param string $workflowName + * + * @return static + */ + public static function createNotDefined(string $transitionName, string $workflowName): self + { + $message = sprintf('Transition "%s" is not defined in workflow "%s".', $transitionName, $workflowName); + $parameters = array( + '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 + * is in wrong place (i.e. status). + * + * @param string $transitionName + * + * @return static + */ + public static function createNotApplicable(string $transitionName): self + { + $message = sprintf('Transition "%s" cannot be made, because the subject is not in the required place.', $transitionName); + $parameters = array( + '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 + * reason. + * + * This blocker code is chiefly for preserving backwards compatibility. + * + * @param string $transitionName + * + * @return static + */ + public static function createUnknownReason(string $transitionName): self + { + $message = sprintf('Transition "%s" cannot be made, because of unknown reason.', $transitionName); + $parameters = array( + 'transitionName' => $transitionName, + ); + + return new static($message, self::REASON_TRANSITION_UNKNOWN, $parameters); + } + + public function getMessage(): string + { + return $this->message; + } + + public function getCode(): string + { + return $this->code; + } + + public function getParameters(): array + { + return $this->parameters; + } +} diff --git a/src/Symfony/Component/Workflow/TransitionBlockerList.php b/src/Symfony/Component/Workflow/TransitionBlockerList.php new file mode 100644 index 0000000000..6bc9dd53a2 --- /dev/null +++ b/src/Symfony/Component/Workflow/TransitionBlockerList.php @@ -0,0 +1,73 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Workflow; + +/** + * A list of transition blockers. + */ +class TransitionBlockerList implements \IteratorAggregate, \Countable +{ + /** @var TransitionBlocker[] */ + private $blockers = array(); + + public function __construct(array $blockers = array()) + { + foreach ($blockers as $blocker) { + $this->add($blocker); + } + } + + public function add(TransitionBlocker $blocker): void + { + $this->blockers[] = $blocker; + } + + public function get(int $offset): TransitionBlocker + { + if (!isset($this->blockers[$offset])) { + throw new \OutOfBoundsException(sprintf('The offset "%s" does not exist.', $offset)); + } + + return $this->blockers[$offset]; + } + + public function has(int $offset): bool + { + return isset($this->blockers[$offset]); + } + + /** + * {@inheritdoc} + * + * @return \ArrayIterator|TransitionBlocker[] + */ + public function getIterator() + { + return new \ArrayIterator($this->blockers); + } + + public function count(): int + { + return count($this->blockers); + } + + public function findByCode(string $code): ?TransitionBlocker + { + foreach ($this as $transitionBlocker) { + if ($transitionBlocker->getCode() === $code) { + return $transitionBlocker; + } + } + + return null; + } +} diff --git a/src/Symfony/Component/Workflow/Workflow.php b/src/Symfony/Component/Workflow/Workflow.php index ac52794c2d..8be05639cb 100644 --- a/src/Symfony/Component/Workflow/Workflow.php +++ b/src/Symfony/Component/Workflow/Workflow.php @@ -15,6 +15,8 @@ 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\UndefinedTransitionException; use Symfony\Component\Workflow\MarkingStore\MarkingStoreInterface; use Symfony\Component\Workflow\MarkingStore\MultipleStateMarkingStore; @@ -81,47 +83,57 @@ class Workflow implements WorkflowInterface */ public function can($subject, $transitionName) { - $transitions = $this->definition->getTransitions(); - $marking = $this->getMarking($subject); - - foreach ($transitions as $transition) { - foreach ($transition->getFroms() as $place) { - if (!$marking->has($place)) { - // do not emit guard events for transitions where the marking does not contain - // all "from places" (thus the transition couldn't be applied anyway) - continue 2; - } - } - - if ($transitionName === $transition->getName() && $this->doCan($subject, $marking, $transition)) { - return true; - } - } - - return false; + return 0 === count($this->buildTransitionBlockerList($subject, $transitionName)); } /** * {@inheritdoc} */ - public function apply($subject, $transitionName) + public function buildTransitionBlockerList($subject, string $transitionName): TransitionBlockerList { - $transitions = $this->getEnabledTransitions($subject); + $blockerList = $this->getEnabledTransitionsByNameOrTransitionBlockerList($subject, $transitionName); + + // It means there are no blockers, so we return an empty list + if (!$blockerList instanceof TransitionBlockerList) { + return new TransitionBlockerList(); + } + + return $blockerList; + } + + /** + * {@inheritdoc} + */ + public function apply($subject, string $transitionName): Marking + { + $transitionsOrTransitionBlockerList = $this->getEnabledTransitionsByNameOrTransitionBlockerList( + $subject, + $transitionName + ); + + if ($transitionsOrTransitionBlockerList instanceof TransitionBlockerList) { + $transitionBlockerList = $transitionsOrTransitionBlockerList; + + if ($transitionBlockerList->findByCode(TransitionBlocker::REASON_TRANSITION_NOT_DEFINED)) { + throw new UndefinedTransitionException( + sprintf('Transition "%s" is not defined in workflow "%s".', $transitionName, $this->name) + ); + } + + throw new BlockedTransitionException( + sprintf('Unable to apply transition "%s" for workflow "%s".', $transitionName, $this->name), + $transitionBlockerList + ); + } + + $transitions = $transitionsOrTransitionBlockerList; // 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); - $applied = false; - foreach ($transitions as $transition) { - if ($transitionName !== $transition->getName()) { - continue; - } - - $applied = true; - $this->leave($subject, $transition, $marking); $this->transition($subject, $transition, $marking); @@ -137,10 +149,6 @@ class Workflow implements WorkflowInterface $this->announce($subject, $transition, $marking); } - if (!$applied) { - throw new LogicException(sprintf('Unable to apply transition "%s" for workflow "%s".', $transitionName, $this->name)); - } - return $marking; } @@ -153,7 +161,7 @@ class Workflow implements WorkflowInterface $marking = $this->getMarking($subject); foreach ($this->definition->getTransitions() as $transition) { - if ($this->doCan($subject, $marking, $transition)) { + if (0 === count($this->doCan($subject, $marking, $transition))) { $enabled[] = $transition; } } @@ -185,32 +193,21 @@ class Workflow implements WorkflowInterface return $this->markingStore; } - private function doCan($subject, Marking $marking, Transition $transition) + private function doCan($subject, Marking $marking, Transition $transition): TransitionBlockerList { foreach ($transition->getFroms() as $place) { if (!$marking->has($place)) { - return false; + return new TransitionBlockerList(array(TransitionBlocker::createNotApplicable($transition->getName()))); } } - if (true === $this->guardTransition($subject, $marking, $transition)) { - return false; - } - - return true; + return $this->guardTransition($subject, $marking, $transition); } - /** - * @param object $subject - * @param Marking $marking - * @param Transition $transition - * - * @return bool|void boolean true if this transition is guarded, ie you cannot use it - */ - private function guardTransition($subject, Marking $marking, Transition $transition) + private function guardTransition($subject, Marking $marking, Transition $transition): TransitionBlockerList { if (null === $this->dispatcher) { - return; + return new TransitionBlockerList(); } $event = new GuardEvent($subject, $marking, $transition, $this->name); @@ -219,7 +216,7 @@ class Workflow implements WorkflowInterface $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->isBlocked(); + return $event->getTransitionBlockerList(); } private function leave($subject, Transition $transition, Marking $marking) @@ -319,4 +316,75 @@ 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]; + } } diff --git a/src/Symfony/Component/Workflow/WorkflowInterface.php b/src/Symfony/Component/Workflow/WorkflowInterface.php index 15ee8a4ec8..180e444d7e 100644 --- a/src/Symfony/Component/Workflow/WorkflowInterface.php +++ b/src/Symfony/Component/Workflow/WorkflowInterface.php @@ -11,7 +11,9 @@ 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; /** @@ -42,6 +44,16 @@ interface WorkflowInterface */ public function can($subject, $transitionName); + /** + * Returns transition blockers explaining why a transition cannot be made. + * + * @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; + /** * Fire a transition. * @@ -50,10 +62,10 @@ interface WorkflowInterface * * @return Marking The new Marking * - * @throws LogicException If the transition is not applicable - * @throws LogicException If the transition does not exist + * @throws BlockedTransitionException If the transition is not applicable + * @throws UndefinedTransitionException If the transition does not exist */ - public function apply($subject, $transitionName); + public function apply($subject, string $transitionName); /** * Returns all enabled transitions.