From 4d10e10096f9325470564f8bc596d4f3ec06f051 Mon Sep 17 00:00:00 2001 From: d-ph Date: Sun, 29 Oct 2017 17:58:58 +0000 Subject: [PATCH 1/2] [Workflow] Add transition blockers --- src/Symfony/Component/Workflow/CHANGELOG.md | 4 +- .../Component/Workflow/Event/GuardEvent.php | 41 ++++- .../Exception/BlockedTransitionException.php | 34 ++++ .../UndefinedTransitionException.php | 19 ++ .../Component/Workflow/Tests/WorkflowTest.php | 112 ++++++++++++ .../Component/Workflow/TransitionBlocker.php | 112 ++++++++++++ .../Workflow/TransitionBlockerList.php | 73 ++++++++ src/Symfony/Component/Workflow/Workflow.php | 168 ++++++++++++------ .../Component/Workflow/WorkflowInterface.php | 18 +- 9 files changed, 522 insertions(+), 59 deletions(-) create mode 100644 src/Symfony/Component/Workflow/Exception/BlockedTransitionException.php create mode 100644 src/Symfony/Component/Workflow/Exception/UndefinedTransitionException.php create mode 100644 src/Symfony/Component/Workflow/TransitionBlocker.php create mode 100644 src/Symfony/Component/Workflow/TransitionBlockerList.php 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. From 2b8faffb415c4a3c4bf7891c45e7740f722076ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Pineau?= Date: Wed, 7 Feb 2018 16:57:22 +0100 Subject: [PATCH 2/2] [Workflow] Cleaned the transition blocker implementations --- .../Component/Workflow/Event/GuardEvent.php | 6 +- .../Workflow/EventListener/GuardListener.php | 8 +- ....php => NotEnabledTransitionException.php} | 10 +- .../UndefinedTransitionException.php | 6 + .../Component/Workflow/Tests/WorkflowTest.php | 219 ++++++++---------- .../Component/Workflow/TransitionBlocker.php | 79 +++---- .../Workflow/TransitionBlockerList.php | 37 ++- src/Symfony/Component/Workflow/Workflow.php | 205 +++++++--------- .../Component/Workflow/WorkflowInterface.php | 16 +- 9 files changed, 248 insertions(+), 338 deletions(-) rename src/Symfony/Component/Workflow/Exception/{BlockedTransitionException.php => NotEnabledTransitionException.php} (57%) diff --git a/src/Symfony/Component/Workflow/Event/GuardEvent.php b/src/Symfony/Component/Workflow/Event/GuardEvent.php index 1ccdd91088..791457bb92 100644 --- a/src/Symfony/Component/Workflow/Event/GuardEvent.php +++ b/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 diff --git a/src/Symfony/Component/Workflow/EventListener/GuardListener.php b/src/Symfony/Component/Workflow/EventListener/GuardListener.php index a4114e68e1..912dc5dada 100644 --- a/src/Symfony/Component/Workflow/EventListener/GuardListener.php +++ b/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 @@ -49,8 +50,11 @@ class GuardListener return; } - if (!$this->expressionLanguage->evaluate($this->configuration[$eventName], $this->getVariables($event))) { - $event->setBlocked(true); + $expression = $this->configuration[$eventName]; + + if (!$this->expressionLanguage->evaluate($expression, $this->getVariables($event))) { + $blocker = TransitionBlocker::createBlockedByExpressionGuardListener($expression); + $event->addTransitionBlocker($blocker); } } diff --git a/src/Symfony/Component/Workflow/Exception/BlockedTransitionException.php b/src/Symfony/Component/Workflow/Exception/NotEnabledTransitionException.php similarity index 57% rename from src/Symfony/Component/Workflow/Exception/BlockedTransitionException.php rename to src/Symfony/Component/Workflow/Exception/NotEnabledTransitionException.php index 51af064f9f..48a8c260ff 100644 --- a/src/Symfony/Component/Workflow/Exception/BlockedTransitionException.php +++ b/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 */ -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; } diff --git a/src/Symfony/Component/Workflow/Exception/UndefinedTransitionException.php b/src/Symfony/Component/Workflow/Exception/UndefinedTransitionException.php index f3246c4681..04022e90cc 100644 --- a/src/Symfony/Component/Workflow/Exception/UndefinedTransitionException.php +++ b/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 */ class UndefinedTransitionException extends LogicException { + public function __construct(string $transitionName, string $workflowName) + { + parent::__construct(sprintf('Transition "%s" is not defined for workflow "%s".', $transitionName, $workflowName)); + } } diff --git a/src/Symfony/Component/Workflow/Tests/WorkflowTest.php b/src/Symfony/Component/Workflow/Tests/WorkflowTest.php index d97d81f615..4451357d53 100644 --- a/src/Symfony/Component/Workflow/Tests/WorkflowTest.php +++ b/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 diff --git a/src/Symfony/Component/Workflow/TransitionBlocker.php b/src/Symfony/Component/Workflow/TransitionBlocker.php index fe5c6f5099..d5307038a6 100644 --- a/src/Symfony/Component/Workflow/TransitionBlocker.php +++ b/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 REASON_TRANSITION_NOT_APPLICABLE = '19beefc8-6b1e-4716-9d07-a39bd6d16e34'; - const REASON_TRANSITION_UNKNOWN = 'e8b5bbb9-5913-4b98-bfa6-65dbd228a82a'; + const BLOCKED_BY_MARKING = '19beefc8-6b1e-4716-9d07-a39bd6d16e34'; + const BLOCKED_BY_EXPRESSION_GUARD_LISTENER = '326a1e9c-0c12-11e8-ba89-0ed5f89f718b'; + 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 - * in a workflow. + * Create a blocker that says the transition cannot be made because it is + * not enabled. * - * @param string $transitionName - * @param string $workflowName - * - * @return static + * It means the subject is in wrong place (i.e. status): + * * 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. */ - 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); - $parameters = array( - 'transitionName' => $transitionName, - 'workflowName' => $workflowName, - ); - - return new static($message, self::REASON_TRANSITION_NOT_DEFINED, $parameters); + return new static('The marking does not enable the transition.', self::BLOCKED_BY_MARKING, array( + 'marking' => $marking, + )); } /** - * 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 + * Creates a blocker that says the transition cannot be made because it has + * been blocked by the expression guard listener. */ - 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); - $parameters = array( - 'transitionName' => $transitionName, - ); - - return new static($message, self::REASON_TRANSITION_NOT_APPLICABLE, $parameters); + return new static('The expression blocks the transition.', self::BLOCKED_BY_EXPRESSION_GUARD_LISTENER, array( + 'expression' => $expression, + )); } /** - * Create a blocker, that says the transition cannot be made because of unknown - * reason. + * Creates a blocker that says the transition cannot be made because of an + * 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); - $parameters = array( - 'transitionName' => $transitionName, - ); - - return new static($message, self::REASON_TRANSITION_UNKNOWN, $parameters); + return new static('Unknown reason.', self::UNKNOWN); } public function getMessage(): string diff --git a/src/Symfony/Component/Workflow/TransitionBlockerList.php b/src/Symfony/Component/Workflow/TransitionBlockerList.php index 6bc9dd53a2..85a9888fc3 100644 --- a/src/Symfony/Component/Workflow/TransitionBlockerList.php +++ b/src/Symfony/Component/Workflow/TransitionBlockerList.php @@ -13,14 +13,20 @@ namespace Symfony\Component\Workflow; /** * A list of transition blockers. + * + * @author Grégoire Pineau */ -class TransitionBlockerList implements \IteratorAggregate, \Countable +final class TransitionBlockerList implements \IteratorAggregate, \Countable { - /** @var TransitionBlocker[] */ - private $blockers = array(); + private $blockers; + /** + * @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])) { - throw new \OutOfBoundsException(sprintf('The offset "%s" does not exist.', $offset)); - } - - return $this->blockers[$offset]; + $this->blockers = array(); } - 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); - } - - public function findByCode(string $code): ?TransitionBlocker - { - foreach ($this as $transitionBlocker) { - if ($transitionBlocker->getCode() === $code) { - return $transitionBlocker; - } - } - - return null; + return \count($this->blockers); } } diff --git a/src/Symfony/Component/Workflow/Workflow.php b/src/Symfony/Component/Workflow/Workflow.php index 8be05639cb..b31cd12ce7 100644 --- a/src/Symfony/Component/Workflow/Workflow.php +++ b/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 - if (!$blockerList instanceof TransitionBlockerList) { - return new TransitionBlockerList(); + foreach ($transitions as $transition) { + if ($transition->getName() !== $transitionName) { + 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( - $subject, - $transitionName - ); + $marking = $this->getMarking($subject); - if ($transitionsOrTransitionBlockerList instanceof TransitionBlockerList) { - $transitionBlockerList = $transitionsOrTransitionBlockerList; + $transitionBlockerList = null; + $applied = false; - if ($transitionBlockerList->findByCode(TransitionBlocker::REASON_TRANSITION_NOT_DEFINED)) { - throw new UndefinedTransitionException( - sprintf('Transition "%s" is not defined in workflow "%s".', $transitionName, $this->name) - ); + foreach ($this->definition->getTransitions() as $transition) { + if ($transition->getName() !== $transitionName) { + continue; } - throw new BlockedTransitionException( - sprintf('Unable to apply transition "%s" for workflow "%s".', $transitionName, $this->name), - $transitionBlockerList - ); - } + $transitionBlockerList = $this->buildTransitionBlockerListForTransition($subject, $marking, $transition); + if (!$transitionBlockerList->isEmpty()) { + 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))) { - $enabled[] = $transition; + $transitionBlockerList = $this->buildTransitionBlockerListForTransition($subject, $marking, $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]; - } } diff --git a/src/Symfony/Component/Workflow/WorkflowInterface.php b/src/Symfony/Component/Workflow/WorkflowInterface.php index 180e444d7e..2460963cba 100644 --- a/src/Symfony/Component/Workflow/WorkflowInterface.php +++ b/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 string $transitionName A transition - * - * @return TransitionBlockerList Empty if the transition is possible + * @param object $subject A subject */ 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 UndefinedTransitionException If the transition does not exist + * @throws LogicException If the transition is not applicable */ - public function apply($subject, string $transitionName); + public function apply($subject, $transitionName); /** * Returns all enabled transitions.