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..791457bb92 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 !$this->transitionBlockerList->isEmpty(); + } + + public function setBlocked(bool $blocked): void + { + if (!$blocked) { + $this->transitionBlockerList->reset(); + + return; + } + + $this->transitionBlockerList->add(TransitionBlocker::createUnknown()); + } + + public function getTransitionBlockerList(): TransitionBlockerList + { + return $this->transitionBlockerList; + } + + public function addTransitionBlocker(TransitionBlocker $transitionBlocker): void + { + $this->transitionBlockerList->add($transitionBlocker); } } 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/NotEnabledTransitionException.php b/src/Symfony/Component/Workflow/Exception/NotEnabledTransitionException.php new file mode 100644 index 0000000000..48a8c260ff --- /dev/null +++ b/src/Symfony/Component/Workflow/Exception/NotEnabledTransitionException.php @@ -0,0 +1,36 @@ + + * + * 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 Workflow when a not enabled transition is applied on a subject. + * + * @author Grégoire Pineau + */ +class NotEnabledTransitionException extends LogicException +{ + private $transitionBlockerList; + + public function __construct(string $transitionName, string $workflowName, TransitionBlockerList $transitionBlockerList) + { + parent::__construct(sprintf('Transition "%s" is not enabled for workflow "%s".', $transitionName, $workflowName)); + + $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..04022e90cc --- /dev/null +++ b/src/Symfony/Component/Workflow/Exception/UndefinedTransitionException.php @@ -0,0 +1,25 @@ + + * + * 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. + * + * @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 547fb03e82..4451357d53 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\NotEnabledTransitionException; 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 @@ -162,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(); @@ -193,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(); diff --git a/src/Symfony/Component/Workflow/TransitionBlocker.php b/src/Symfony/Component/Workflow/TransitionBlocker.php new file mode 100644 index 0000000000..d5307038a6 --- /dev/null +++ b/src/Symfony/Component/Workflow/TransitionBlocker.php @@ -0,0 +1,91 @@ + + * + * 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. + */ +final class TransitionBlocker +{ + 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; + 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; + $this->code = $code; + $this->parameters = $parameters; + } + + /** + * Create a blocker that says the transition cannot be made because it is + * not enabled. + * + * 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 createBlockedByMarking(Marking $marking): self + { + return new static('The marking does not enable the transition.', self::BLOCKED_BY_MARKING, array( + 'marking' => $marking, + )); + } + + /** + * Creates a blocker that says the transition cannot be made because it has + * been blocked by the expression guard listener. + */ + public static function createBlockedByExpressionGuardListener(string $expression): self + { + return new static('The expression blocks the transition.', self::BLOCKED_BY_EXPRESSION_GUARD_LISTENER, array( + 'expression' => $expression, + )); + } + + /** + * Creates a blocker that says the transition cannot be made because of an + * unknown reason. + * + * This blocker code is chiefly for preserving backwards compatibility. + */ + public static function createUnknown(): self + { + return new static('Unknown reason.', self::UNKNOWN); + } + + 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..85a9888fc3 --- /dev/null +++ b/src/Symfony/Component/Workflow/TransitionBlockerList.php @@ -0,0 +1,64 @@ + + * + * 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. + * + * @author Grégoire Pineau + */ +final class TransitionBlockerList implements \IteratorAggregate, \Countable +{ + private $blockers; + + /** + * @param TransitionBlocker[] $blockers + */ + public function __construct(array $blockers = array()) + { + $this->blockers = array(); + + foreach ($blockers as $blocker) { + $this->add($blocker); + } + } + + public function add(TransitionBlocker $blocker): void + { + $this->blockers[] = $blocker; + } + + public function reset(): void + { + $this->blockers = array(); + } + + public function isEmpty(): bool + { + return !$this->blockers; + } + + /** + * {@inheritdoc} + * + * @return \ArrayIterator|TransitionBlocker[] + */ + public function getIterator() + { + return new \ArrayIterator($this->blockers); + } + + public function count(): int + { + return \count($this->blockers); + } +} diff --git a/src/Symfony/Component/Workflow/Workflow.php b/src/Symfony/Component/Workflow/Workflow.php index ac52794c2d..b31cd12ce7 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\NotEnabledTransitionException; +use Symfony\Component\Workflow\Exception\UndefinedTransitionException; use Symfony\Component\Workflow\MarkingStore\MarkingStoreInterface; use Symfony\Component\Workflow\MarkingStore\MultipleStateMarkingStore; @@ -85,15 +87,13 @@ class Workflow implements WorkflowInterface $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 ($transition->getName() !== $transitionName) { + continue; } - if ($transitionName === $transition->getName() && $this->doCan($subject, $marking, $transition)) { + $transitionBlockerList = $this->buildTransitionBlockerListForTransition($subject, $marking, $transition); + + if ($transitionBlockerList->isEmpty()) { return true; } } @@ -104,19 +104,48 @@ class Workflow implements WorkflowInterface /** * {@inheritdoc} */ - public function apply($subject, $transitionName) + public function buildTransitionBlockerList($subject, string $transitionName): TransitionBlockerList { - $transitions = $this->getEnabledTransitions($subject); - - // 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; + $transitions = $this->definition->getTransitions(); + $marking = $this->getMarking($subject); + $transitionBlockerList = null; foreach ($transitions as $transition) { - if ($transitionName !== $transition->getName()) { + if ($transition->getName() !== $transitionName) { + continue; + } + + $transitionBlockerList = $this->buildTransitionBlockerListForTransition($subject, $marking, $transition); + + if ($transitionBlockerList->isEmpty()) { + continue; + } + } + + if (!$transitionBlockerList) { + throw new UndefinedTransitionException($transitionName, $this->name); + } + + return $transitionBlockerList; + } + + /** + * {@inheritdoc} + */ + public function apply($subject, $transitionName) + { + $marking = $this->getMarking($subject); + + $transitionBlockerList = null; + $applied = false; + + foreach ($this->definition->getTransitions() as $transition) { + if ($transition->getName() !== $transitionName) { + continue; + } + + $transitionBlockerList = $this->buildTransitionBlockerListForTransition($subject, $marking, $transition); + if (!$transitionBlockerList->isEmpty()) { continue; } @@ -137,8 +166,12 @@ class Workflow implements WorkflowInterface $this->announce($subject, $transition, $marking); } + if (!$transitionBlockerList) { + throw new UndefinedTransitionException($transitionName, $this->name); + } + if (!$applied) { - throw new LogicException(sprintf('Unable to apply transition "%s" for workflow "%s".', $transitionName, $this->name)); + throw new NotEnabledTransitionException($transitionName, $this->name, $transitionBlockerList); } return $marking; @@ -149,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 ($this->doCan($subject, $marking, $transition)) { - $enabled[] = $transition; + $transitionBlockerList = $this->buildTransitionBlockerListForTransition($subject, $marking, $transition); + if ($transitionBlockerList->isEmpty()) { + $enabledTransitions[] = $transition; } } - return $enabled; + return $enabledTransitions; } /** @@ -185,32 +219,33 @@ class Workflow implements WorkflowInterface return $this->markingStore; } - private function doCan($subject, Marking $marking, Transition $transition) + private function buildTransitionBlockerListForTransition($subject, Marking $marking, Transition $transition) { foreach ($transition->getFroms() as $place) { if (!$marking->has($place)) { - return false; + return new TransitionBlockerList(array( + TransitionBlocker::createBlockedByMarking($marking), + )); } } - if (true === $this->guardTransition($subject, $marking, $transition)) { - return false; + if (null === $this->dispatcher) { + return new TransitionBlockerList(); } - return true; + $event = $this->guardTransition($subject, $marking, $transition); + + if ($event->isBlocked()) { + return $event->getTransitionBlockerList(); + } + + return new TransitionBlockerList(); } - /** - * @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): ?GuardEvent { if (null === $this->dispatcher) { - return; + return null; } $event = new GuardEvent($subject, $marking, $transition, $this->name); @@ -219,10 +254,10 @@ 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; } - private function leave($subject, Transition $transition, Marking $marking) + private function leave($subject, Transition $transition, Marking $marking): void { $places = $transition->getFroms(); @@ -242,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; @@ -255,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(); @@ -275,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; @@ -291,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; @@ -304,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; diff --git a/src/Symfony/Component/Workflow/WorkflowInterface.php b/src/Symfony/Component/Workflow/WorkflowInterface.php index 15ee8a4ec8..2460963cba 100644 --- a/src/Symfony/Component/Workflow/WorkflowInterface.php +++ b/src/Symfony/Component/Workflow/WorkflowInterface.php @@ -37,11 +37,16 @@ interface WorkflowInterface * @param string $transitionName A transition * * @return bool true if the transition is enabled - * - * @throws LogicException */ public function can($subject, $transitionName); + /** + * Builds a TransitionBlockerList to know why a transition is blocked. + * + * @param object $subject A subject + */ + public function buildTransitionBlockerList($subject, string $transitionName): TransitionBlockerList; + /** * Fire a transition. * @@ -51,7 +56,6 @@ interface WorkflowInterface * @return Marking The new Marking * * @throws LogicException If the transition is not applicable - * @throws LogicException If the transition does not exist */ public function apply($subject, $transitionName);